Author SHA1 Message Date
sneak 2650577d26 Reject invalid secret names before any command builds a path (closes #33)
check / check (push) Successful in 49s
`secret rm ..` deleted the whole vault; `secret rm .` and `secret rm ""`
deleted every secret. rm, mv, the version commands, encrypt and decrypt
built paths from the name unchecked; import checked it only after
reading the source file.

Each now calls vault.ValidateSecretName, which wraps the existing rule,
on the name as given, before building any path; MoveSecret checks both
names before switching the current vault. Its error and README.md state
the rule. The test-only copy of the rule in internal/secret is removed.

The regression test runs each rejected command on a copy of two
in-memory vaults and requires the exact error and an unchanged state
directory.

Model: opus-5-5
2026-10-03 15:44:19 +00:00
clawbot 7c6531eaf7 Keep the keychain unlocker passphrase in locked memory (closes #36)
check / check (push) Successful in 39s
The passphrase protecting the keychain unlocker's age key was a plain
string passed through encoding/json, leaving copies in ordinary memory
when an unlocker was created and each time one was used.

It is now generated into a locked buffer, and KeychainData, moved to
keychaindata.go, which is not darwin-only so its tests run on Linux,
writes and reads the keychain JSON itself: encode copies the parts
straight into a locked buffer, and decodeKeychainData takes the
passphrase from a json.RawMessage that it wipes. The JSON field names
are unchanged. keychainunlocker.go only calls this code and stores the
item from the locked buffer without a string copy.

Model: opus-5-5
2026-10-03 17:07:56 +02:00
clawbot d52b4f1240 Let a plain docker build pass and stamp the git version (closes #57)
check / check (push) Successful in 1m1s
The size tests skip a case whose secret needs more locked memory than
the process can lock, found by locking a buffer of that size: memguard
panics otherwise, and a plain `docker build .` runs under an 8 MiB
RLIMIT_MEMLOCK. script/cibuild, or any process allowed to lock past the
limit, runs every case.

The build stage stamps the VERSION build argument, else
`git describe --tags --always`, and fails when .git is present but
yields no version. `make build` stamps `git describe` too instead of
the fixed 0.1.0. .dockerignore keeps .git/config out; script/docker is
now the canonical copy.

Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
2026-10-02 14:16:02 +02:00
20 changed files with 745 additions and 166 deletions
+6
View File
@@ -1,3 +1,9 @@
# .git is sent without its config. Without a VERSION build argument the
# stage that compiles runs `git describe --tags --always` on .git, which
# does not need .git/config; that file can hold a credential, such as a
# password in a remote URL or the token the CI checkout step stores there.
.git/config
# Build artifacts # Build artifacts
secret secret
coverage.out coverage.out
+14 -1
View File
@@ -27,7 +27,20 @@ RUN go mod download
COPY . . COPY . .
RUN make test RUN make test
RUN make build
# The version stamped into the binary: the VERSION build argument when one
# is given, otherwise `git describe --tags --always` of the .git the build
# context carries: the tag on a tagged commit, tag-N-gHASH on a commit after
# one, the short commit when no tag is reachable. A context that carries .git
# and still yields no version fails the build.
ARG VERSION
RUN version="${VERSION:-$(git describe --tags --always)}"; \
if [ -e .git ] && { [ -z "$version" ] || [ "$version" = dev ] || \
[ "$version" = unknown ]; }; then \
echo "no version could be derived although the build context carries .git" >&2; \
exit 1; \
fi; \
make build VERSION="${version:-dev}"
# Runtime stage # Runtime stage
# alpine 3.23 (2026-03-10) # alpine 3.23 (2026-03-10)
+1 -1
View File
@@ -2,7 +2,7 @@ export CGO_ENABLED=1
export DOCKER_HOST := ssh://root@ber1app1.local export DOCKER_HOST := ssh://root@ber1app1.local
# Version information # Version information
VERSION := 0.1.0 VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
GIT_COMMIT := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown") GIT_COMMIT := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
LDFLAGS := -X 'git.eeqj.de/sneak/secret/internal/cli.Version=$(VERSION)' \ LDFLAGS := -X 'git.eeqj.de/sneak/secret/internal/cli.Version=$(VERSION)' \
-X 'git.eeqj.de/sneak/secret/internal/cli.GitCommit=$(GIT_COMMIT)' -X 'git.eeqj.de/sneak/secret/internal/cli.GitCommit=$(GIT_COMMIT)'
+3 -1
View File
@@ -113,7 +113,9 @@ automatically switch to another vault if removing the current one.
Adds a secret to the current vault. Reads the secret value from stdin. Adds a secret to the current vault. Reads the secret value from stdin.
- `--force, -f`: Overwrite existing secret - `--force, -f`: Overwrite existing secret
**Secret Name Format:** `[a-z0-9\.\-\_\/]+` **Secret Name Format:** only ASCII letters, digits, `.`, `-`, `_` and `/`
are allowed, and a name must not be empty, start with `.` or `/`, end with
`/`, contain `//`, or have `..` as a path segment.
- Forward slashes (`/`) are converted to percent signs (`%`) for storage - Forward slashes (`/`) are converted to percent signs (`%`) for storage
- Examples: `database/password`, `api.key`, `ssh_private_key` - Examples: `database/password`, `api.key`, `ssh_private_key`
+26 -8
View File
@@ -25,6 +25,26 @@ Bring the repo into policy compliance in one commit:
# Completed Steps # Completed Steps
- 2026-10-03: Every command that builds a path from a secret name
checks the name first with `vault.ValidateSecretName` and touches
nothing when it is invalid: `rm`, `mv` (both names, within a vault
and between vaults, before switching the current vault), `import`,
`version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error
and `README.md` state the naming rule. Before, `secret rm ..`
deleted the whole vault and `secret rm .` every secret in it.
- 2026-10-03: The keychain unlocker's age key passphrase stays in
locked memory: it is generated into a locked buffer, and the
keychain JSON is written and read by `KeychainData` code in
`internal/secret/keychaindata.go` (tested on Linux) without
`encoding/json` holding it; the JSON field names are unchanged.
- 2026-10-02: A plain `docker build .` builds again: the size tests
skip a case that needs more locked memory than the process can
lock, and run every case under `script/cibuild`. The image stamps the
`VERSION` build argument, else `git describe --tags --always`, into
`Version`, and fails if `.git` is present but yields no version;
`make build` stamps `git describe` too, not a fixed `0.1.0`.
`.dockerignore` keeps `.git/config` out; `script/docker` is the
canonical copy.
- 2026-08-07: Updated golangci-lint to v2.12.2 with the canonical - 2026-08-07: Updated golangci-lint to v2.12.2 with the canonical
`.golangci.yml` (all linters enabled minus the standard disable `.golangci.yml` (all linters enabled minus the standard disable
list, `lll` 88, tests linted); bumped the `Dockerfile` lint-stage list, `lll` 88, tests linted); bumped the `Dockerfile` lint-stage
@@ -80,16 +100,14 @@ Bring the repo into policy compliance in one commit:
- Command injection: GPG key IDs passed unescaped to exec.Command - Command injection: GPG key IDs passed unescaped to exec.Command
(pgpunlocker.go:323-327); data.String() passed unescaped to the (pgpunlocker.go:323-327); data.String() passed unescaped to the
security command (keychainunlocker.go:472-476). security command (keychainunlocker.go:472-476).
- Memory security: KeychainData stores AgePrivKeyPassphrase as a - Memory security: age identity .String() creates unprotected
plain string (keychainunlocker.go:342,393-396); age identity copies (keychainunlocker.go:356, pgpunlocker.go:256,
.String() creates unprotected copies (keychainunlocker.go:356, version.go:155); age secret key held in a plain string in
pgpunlocker.go:256, version.go:155); age secret key held in a cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
plain string in cli/crypto.go:86,91,113; private keys exposed via to GPGEncryptFunc and EncryptWithPassphrase.
buffer.Bytes() to GPGEncryptFunc and EncryptWithPassphrase.
- Race conditions: no file locking in vault/secrets.go:142-176; - Race conditions: no file locking in vault/secrets.go:142-176;
non-atomic writes can leave the vault inconsistent. non-atomic writes can leave the vault inconsistent.
- Input validation: dots in secret names risk path traversal - Input validation: no maximum secret size (DoS).
(vault/secrets.go:75-99); no maximum secret size (DoS).
- Timing attacks: bytes.Equal passphrase compare (cli/init.go: - Timing attacks: bytes.Equal passphrase compare (cli/init.go:
209-216); non-constant-time public key compare (vault.go:95-100). 209-216); non-constant-time public key compare (vault.go:95-100).
- High priority: - High priority:
+1 -1
View File
@@ -16,6 +16,7 @@ require (
github.com/stretchr/testify v1.8.4 github.com/stretchr/testify v1.8.4
github.com/tyler-smith/go-bip39 v1.1.0 github.com/tyler-smith/go-bip39 v1.1.0
golang.org/x/crypto v0.38.0 golang.org/x/crypto v0.38.0
golang.org/x/sys v0.33.0
golang.org/x/term v0.32.0 golang.org/x/term v0.32.0
) )
@@ -31,7 +32,6 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-isatty v0.0.20 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/spf13/pflag v1.0.6 // indirect github.com/spf13/pflag v1.0.6 // indirect
golang.org/x/sys v0.33.0 // indirect
golang.org/x/text v0.25.0 // indirect golang.org/x/text v0.25.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect
) )
+10
View File
@@ -122,6 +122,11 @@ func (cli *Instance) resolveEncryptionKey(
// Encrypt encrypts data using an age secret key stored in a secret // Encrypt encrypts data using an age secret key stored in a secret
func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error { func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
@@ -191,6 +196,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
// Decrypt decrypts data using an age secret key stored in a secret // Decrypt decrypts data using an age secret key stored in a secret
func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error { func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
+267
View File
@@ -0,0 +1,267 @@
package cli_test
import (
"maps"
"os"
"slices"
"strings"
"testing"
"git.eeqj.de/sneak/secret/internal/cli"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/awnumar/memguard"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
)
const (
// testStateDir is the in-memory state directory of the test vaults.
testStateDir = "/test/state"
// testPassphrase protects the passphrase unlocker of each test vault.
testPassphrase = "test-passphrase"
// testVersion is a version name in the format the vault uses.
testVersion = "20260101.001"
// missingFile is an import source that does not exist, so an import
// that opened it before checking the name would fail with another error.
missingFile = "/no/such/file"
)
// newTwoVaultFs returns an in-memory filesystem holding the vaults "work"
// and "default", the current one. Each holds the secret "x" and a
// passphrase unlocker, so both secrets.d and unlockers.d have contents.
//
//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
func newTwoVaultFs(t *testing.T) afero.Fs {
t.Helper()
t.Setenv(secret.EnvMnemonic, testMnemonic)
fs := afero.NewMemMapFs()
for _, name := range []string{"work", "default"} {
vlt, err := vault.CreateVault(fs, testStateDir, name)
require.NoError(t, err)
err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
require.NoError(t, err)
_, err = vlt.CreatePassphraseUnlocker(
memguard.NewBufferFromBytes([]byte(testPassphrase)))
require.NoError(t, err)
}
return fs
}
// snapshotStateDir maps every file under the state directory to its
// contents, and every directory, written with a trailing "/", to "". Two
// snapshots are equal only if nothing in it was added, removed or changed.
func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
t.Helper()
tree := map[string]string{}
err := afero.Walk(fs, testStateDir, func(
path string, info os.FileInfo, err error,
) error {
if err != nil {
return err
}
if info.IsDir() {
tree[path+"/"] = ""
return nil
}
content, err := afero.ReadFile(fs, path)
if err != nil {
return err
}
tree[path] = string(content)
return nil
})
require.NoError(t, err)
return tree
}
// newFsFromSnapshot returns a new in-memory filesystem holding exactly the
// directories and files recorded by snapshotStateDir.
//
//nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs {
t.Helper()
fs := afero.NewMemMapFs()
// In sorted order every directory comes before its contents.
for _, path := range slices.Sorted(maps.Keys(tree)) {
dir, isDir := strings.CutSuffix(path, "/")
if isDir {
require.NoError(t, fs.MkdirAll(dir, secret.DirPerms))
continue
}
err := afero.WriteFile(fs, path, []byte(tree[path]), secret.FilePerms)
require.NoError(t, err)
}
return fs
}
// requireRejectedAndUnchanged runs a command on a copy of the state
// directory recorded in before. It requires exactly the error
// vault.ValidateSecretName gives for the rejected name, so that a later
// check rejecting the name does not count, and everything under the state
// directory as it was: the error alone proves nothing, since it could come
// after the vault had already been deleted.
func requireRejectedAndUnchanged(
t *testing.T, before map[string]string, rejected string,
run func(c *cli.Instance) error,
) {
t.Helper()
fs := newFsFromSnapshot(t, before)
err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
require.Equal(t, before, snapshotStateDir(t, fs))
require.ErrorIs(t, err, vault.ErrInvalidSecretName)
require.EqualError(t, err, vault.ValidateSecretName(rejected).Error())
}
// TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
// https://git.eeqj.de/sneak/secret/issues/33, where `secret rm ..` deleted
// the whole vault, and `secret rm .` or `secret rm ""` every secret in it.
// Moves and imports use --force, so that only the name check stands in
// the way.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
// Creating a passphrase unlocker is slow by design, so the vaults are
// created once and each case runs on its own copy of them.
before := snapshotStateDir(t, newTwoVaultFs(t))
vaultDir := testStateDir + "/vaults.d/default"
require.Contains(t, before, vaultDir+"/secrets.d/x/")
require.Contains(t, before, vaultDir+"/unlockers.d/passphrase/")
require.Equal(t, "default", before[testStateDir+"/currentvault"])
cmd := &cobra.Command{}
tests := []struct {
command string
rejected string // the secret name the command must reject
run func(c *cli.Instance) error
}{
{"rm ..", "..", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "..", false)
}},
{"rm .", ".", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, ".", false)
}},
{`rm ""`, "", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "", false)
}},
{"rm ../../etc", "../../etc", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "../../etc", false)
}},
{"mv --force .. x", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "..", "x", true)
}},
{"mv --force x ..", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "x", "..", true)
}},
{`mv --force x ""`, "", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "x", "", true)
}},
// "work" is not the current vault: a move within it must not
// select it when a name is rejected.
{"mv --force work:.. work:x", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "work:..", "work:x", true)
}},
{"mv --force work:x work:..", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "work:x", "work:..", true)
}},
{"mv --force default:.. work", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:..", "work", true)
}},
{"mv --force default:.. work:y", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:..", "work:y", true)
}},
{"mv --force default:x work:..", "..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:x", "work:..", true)
}},
{"import --force ..", "..", func(c *cli.Instance) error {
return c.ImportSecret(cmd, "..", missingFile, true)
}},
{"import --force .", ".", func(c *cli.Instance) error {
return c.ImportSecret(cmd, ".", missingFile, true)
}},
{"import --force ../../etc", "../../etc", func(c *cli.Instance) error {
return c.ImportSecret(cmd, "../../etc", missingFile, true)
}},
{"version list ..", "..", func(c *cli.Instance) error {
return c.ListVersions(cmd, "..")
}},
{"version promote ..", "..", func(c *cli.Instance) error {
return c.PromoteVersion(cmd, "..", testVersion)
}},
{"version rm ..", "..", func(c *cli.Instance) error {
return c.RemoveVersion(cmd, "..", testVersion)
}},
{"encrypt ..", "..", func(c *cli.Instance) error {
return c.Encrypt("..", "", "")
}},
{"decrypt ..", "..", func(c *cli.Instance) error {
return c.Decrypt("..", "", "")
}},
}
for _, tt := range tests {
t.Run(tt.command, func(t *testing.T) {
requireRejectedAndUnchanged(t, before, tt.rejected, tt.run)
})
}
}
// TestMoveToVaultNameRenamesInCurrentVault checks that `secret mv x work`,
// where "work" is also the name of a vault, renames the secret "x" to "work"
// in the current vault and changes nothing else.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestMoveToVaultNameRenamesInCurrentVault(t *testing.T) {
before := snapshotStateDir(t, newTwoVaultFs(t))
fs := newFsFromSnapshot(t, before)
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
err := c.MoveSecret(&cobra.Command{}, "x", "work", false)
require.NoError(t, err)
// Expected: the state as before, with everything under the current
// vault's secrets.d/x/ now under secrets.d/work/.
oldDir := testStateDir + "/vaults.d/default/secrets.d/x/"
newDir := testStateDir + "/vaults.d/default/secrets.d/work/"
want := map[string]string{}
for path, content := range before {
rest, found := strings.CutPrefix(path, oldDir)
if found {
path = newDir + rest
}
want[path] = content
}
require.Contains(t, want, newDir)
require.Equal(t, want, snapshotStateDir(t, fs))
}
+39 -17
View File
@@ -603,6 +603,11 @@ func printSecretsTable(
func (cli *Instance) ImportSecret( func (cli *Instance) ImportSecret(
cmd *cobra.Command, secretName, sourceFile string, force bool, cmd *cobra.Command, secretName, sourceFile string, force bool,
) error { ) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
@@ -649,6 +654,11 @@ func (cli *Instance) ImportSecret(
// RemoveSecret removes a secret from the vault // RemoveSecret removes a secret from the vault
func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error { func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
@@ -702,13 +712,8 @@ func (cli *Instance) MoveSecret(
srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source) srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest) destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
// If neither is qualified, this is a simple within-vault rename
if !srcQualified && !destQualified {
return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
}
// Cross-vault move requires source to be qualified // Cross-vault move requires source to be qualified
if !srcQualified { if !srcQualified && destQualified {
return errCrossVaultSourceUnqualified return errCrossVaultSourceUnqualified
} }
@@ -716,31 +721,46 @@ func (cli *Instance) MoveSecret(
// Format: "work:secret default" means move to vault "default" // Format: "work:secret default" means move to vault "default"
// Format: "work:secret default:newname" means move to vault "default" // Format: "work:secret default:newname" means move to vault "default"
// with a new name // with a new name
if !destQualified { if srcQualified && !destQualified {
// Check if dest is actually a vault name // Check if dest is actually a vault name
vaults, err := vault.ListVaults(cli.fs, cli.stateDir) vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
if err == nil && slices.Contains(vaults, dest) { if err == nil && slices.Contains(vaults, dest) {
// dest is a vault name, use source secret name // dest is a vault name, use source secret name
destVaultName = dest destVaultName = dest
destSecretName = srcSecretName destSecretName = srcSecretName
} } else {
// dest is a secret name in source vault
// If destVaultName is still empty, dest is a secret name in source vault
if destVaultName == "" {
destVaultName = srcVaultName destVaultName = srcVaultName
destSecretName = dest
} }
} }
// If destination secret name is empty, use source secret name // If destination secret name is empty, use source secret name. A plain
if destSecretName == "" { // rename keeps it empty, so that the check below rejects it.
if srcQualified && destSecretName == "" {
destSecretName = srcSecretName destSecretName = srcSecretName
} }
// Check both names, for every form of the move, before selecting a vault
// below, so that a rejected move leaves the current vault as it was.
err := vault.ValidateSecretName(srcSecretName)
if err != nil {
return err
}
err = vault.ValidateSecretName(destSecretName)
if err != nil {
return err
}
// If neither is qualified, this is a simple within-vault rename
if !srcQualified && !destQualified {
return cli.moveSecretWithinVault(cmd, srcSecretName, destSecretName, force)
}
// Same vault? Use simple rename if possible (optimization) // Same vault? Use simple rename if possible (optimization)
if srcVaultName == destVaultName { if srcVaultName == destVaultName {
// Select the vault and do a simple move // Select the vault and do a simple move
err := vault.SelectVault(cli.fs, cli.stateDir, srcVaultName) err = vault.SelectVault(cli.fs, cli.stateDir, srcVaultName)
if err != nil { if err != nil {
return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err) return fmt.Errorf("failed to select vault '%s': %w", srcVaultName, err)
} }
@@ -753,7 +773,8 @@ func (cli *Instance) MoveSecret(
cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force) cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
} }
// moveSecretWithinVault handles rename within the current vault // moveSecretWithinVault handles rename within the current vault. Its caller,
// MoveSecret, has already checked both secret names.
func (cli *Instance) moveSecretWithinVault( func (cli *Instance) moveSecretWithinVault(
cmd *cobra.Command, source, dest string, force bool, cmd *cobra.Command, source, dest string, force bool,
) error { ) error {
@@ -808,7 +829,8 @@ func (cli *Instance) moveSecretWithinVault(
return nil return nil
} }
// moveSecretCrossVault handles moving between different vaults // moveSecretCrossVault handles moving between different vaults. Its caller,
// MoveSecret, has already checked both secret names.
func (cli *Instance) moveSecretCrossVault( func (cli *Instance) moveSecretCrossVault(
cmd *cobra.Command, cmd *cobra.Command,
srcVaultName, srcSecretName, srcVaultName, srcSecretName,
+44
View File
@@ -17,11 +17,51 @@ import (
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
) )
// testVaultName is the vault name used by the size tests. // testVaultName is the vault name used by the size tests.
const testVaultName = "test-vault" const testVaultName = "test-vault"
// lockedBytesPerSecretByte bounds the locked memory that storing a secret
// holds at once: the buffers it is read into reach up to 1.5 times its
// size, and they are then copied into one more buffer of its size.
const lockedBytesPerSecretByte = 3
// skipIfLockedMemoryTooLow skips the test when this process cannot lock
// the memory a secret of size bytes needs, found by locking a buffer of
// that size and releasing it. memguard panics, ending the whole test run,
// when it cannot lock a buffer, and a plain `docker build .` runs the
// tests under an 8 MiB locked-memory limit (RLIMIT_MEMLOCK). A process
// allowed to lock past that limit runs every case.
func skipIfLockedMemoryTooLow(t *testing.T, size int) {
t.Helper()
need := lockedBytesPerSecretByte * size
buf, err := unix.Mmap(-1, 0, need,
unix.PROT_READ|unix.PROT_WRITE, unix.MAP_PRIVATE|unix.MAP_ANON)
require.NoError(t, err)
lockErr := unix.Mlock(buf)
// Unmapping the buffer also unlocks it.
err = unix.Munmap(buf)
require.NoError(t, err)
if lockErr != nil {
var limit unix.Rlimit
err = unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit)
require.NoError(t, err)
t.Skipf("a %d-byte secret needs up to %d bytes of locked memory, "+
"which could not be locked under the locked-memory limit "+
"(RLIMIT_MEMLOCK) of %d bytes: %v",
size, need, limit.Cur, lockErr)
}
}
// newSizeTestVault creates an in-memory vault unlocked with the test // newSizeTestVault creates an in-memory vault unlocked with the test
// mnemonic and returns the filesystem and vault. // mnemonic and returns the filesystem and vault.
// //
@@ -59,6 +99,7 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) {
// verifies the outcome. // verifies the outcome.
func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
t.Helper() t.Helper()
skipIfLockedMemoryTooLow(t, size)
fs, vlt := newSizeTestVault(t) fs, vlt := newSizeTestVault(t)
@@ -110,6 +151,7 @@ func runAddSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
// verifies the outcome. // verifies the outcome.
func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) { func runImportSecretSizeCase(t *testing.T, size int, wantErr bool, errMsg string) {
t.Helper() t.Helper()
skipIfLockedMemoryTooLow(t, size)
fs, vlt := newSizeTestVault(t) fs, vlt := newSizeTestVault(t)
@@ -300,6 +342,8 @@ func TestAddSecretBufferGrowth(t *testing.T) {
for _, size := range sizes { for _, size := range sizes {
t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) { t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) {
skipIfLockedMemoryTooLow(t, size)
fs, vlt := newSizeTestVault(t) fs, vlt := newSizeTestVault(t)
// Create test data of exactly the specified size // Create test data of exactly the specified size
+15
View File
@@ -112,6 +112,11 @@ func VersionCommands(cli *Instance) *cobra.Command {
func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error { func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
secret.Debug("ListVersions called", "secret_name", secretName) secret.Debug("ListVersions called", "secret_name", secretName)
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
@@ -239,6 +244,11 @@ func formatVersionTime(t *time.Time) string {
func (cli *Instance) PromoteVersion( func (cli *Instance) PromoteVersion(
cmd *cobra.Command, secretName string, version string, cmd *cobra.Command, secretName string, version string,
) error { ) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
@@ -282,6 +292,11 @@ func (cli *Instance) PromoteVersion(
func (cli *Instance) RemoveVersion( func (cli *Instance) RemoveVersion(
cmd *cobra.Command, secretName string, version string, cmd *cobra.Command, secretName string, version string,
) error { ) error {
err := vault.ValidateSecretName(secretName)
if err != nil {
return err
}
// Get current vault // Get current vault
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
-29
View File
@@ -1,29 +0,0 @@
//go:build darwin
package secret
import (
"crypto/rand"
"fmt"
"math/big"
)
// generateRandomString generates a random string of the specified length using the given character set
func generateRandomString(length int, charset string) (string, error) {
if length <= 0 {
return "", fmt.Errorf("length must be positive")
}
result := make([]byte, length)
charsetLen := big.NewInt(int64(len(charset)))
for i := range length {
randomIndex, err := rand.Int(rand.Reader, charsetLen)
if err != nil {
return "", fmt.Errorf("failed to generate random number: %w", err)
}
result[i] = charset[randomIndex.Int64()]
}
return string(result), nil
}
+142
View File
@@ -0,0 +1,142 @@
package secret
import (
"bytes"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"strings"
"github.com/awnumar/memguard"
)
var (
errPassphraseLength = errors.New(
"passphrase length must be a positive even number")
errPassphraseNotHex = errors.New(
"keychain passphrase must be lowercase hex")
errNoKeychainPassphrase = errors.New(
"keychain data has no agePrivKeyPassphrase string")
)
// KeychainData is what a keychain unlocker stores in the macOS keychain.
// It is stored as JSON, but encode and decodeKeychainData keep the
// passphrase out of encoding/json, which would leave copies of it in
// ordinary memory.
type KeychainData struct {
AgePublicKey string
AgePrivKeyPassphrase *memguard.LockedBuffer
EncryptedLongtermKey string
}
// generateRandomPassphrase returns length random lowercase hex characters
// in a locked buffer. The caller must destroy it.
func generateRandomPassphrase(length int) (*memguard.LockedBuffer, error) {
// Each random byte becomes two hex characters.
randomBytes := hex.DecodedLen(length)
if length <= 0 || hex.EncodedLen(randomBytes) != length {
return nil, errPassphraseLength
}
random := memguard.NewBufferRandom(randomBytes)
defer random.Destroy()
passphrase := memguard.NewBuffer(length)
hex.Encode(passphrase.Bytes(), random.Bytes())
passphrase.Freeze()
return passphrase, nil
}
// encode returns d as JSON in a locked buffer:
// {"agePublicKey":"...","agePrivKeyPassphrase":"...","encryptedLongtermKey":"..."}.
// The passphrase is copied straight into the buffer, so it must be hex,
// which JSON does not escape. The caller must destroy the returned buffer.
func (d *KeychainData) encode() (*memguard.LockedBuffer, error) {
if d.AgePrivKeyPassphrase == nil {
return nil, errNilPassphraseBuffer
}
if d.AgePrivKeyPassphrase.Size() == 0 {
return nil, errEmptyPassphrase
}
for _, c := range d.AgePrivKeyPassphrase.Bytes() {
if strings.IndexByte("0123456789abcdef", c) < 0 {
return nil, errPassphraseNotHex
}
}
publicKey, err := json.Marshal(d.AgePublicKey)
if err != nil {
return nil, fmt.Errorf("failed to encode age public key: %w", err)
}
longtermKey, err := json.Marshal(d.EncryptedLongtermKey)
if err != nil {
return nil, fmt.Errorf("failed to encode long-term key: %w", err)
}
parts := [][]byte{
[]byte(`{"agePublicKey":`), publicKey,
[]byte(`,"agePrivKeyPassphrase":"`), d.AgePrivKeyPassphrase.Bytes(),
[]byte(`","encryptedLongtermKey":`), longtermKey,
[]byte(`}`),
}
size := 0
for _, part := range parts {
size += len(part)
}
encoded := memguard.NewBuffer(size)
written := 0
for _, part := range parts {
written += copy(encoded.Bytes()[written:], part)
}
encoded.Freeze()
return encoded, nil
}
// decodeKeychainData parses keychain data written by encode. The caller
// must destroy the returned AgePrivKeyPassphrase.
func decodeKeychainData(data *memguard.LockedBuffer) (*KeychainData, error) {
if data == nil {
return nil, errNilDataBuffer
}
// json.Unmarshal gives a json.RawMessage field the field's JSON text
// unchanged, in the one copy RawMessage makes; it is wiped on return.
var fields struct {
AgePublicKey string `json:"agePublicKey"`
AgePrivKeyPassphrase json.RawMessage `json:"agePrivKeyPassphrase"`
EncryptedLongtermKey string `json:"encryptedLongtermKey"`
}
defer func() { memguard.WipeBytes(fields.AgePrivKeyPassphrase) }()
err := json.Unmarshal(data.Bytes(), &fields)
if err != nil {
return nil, fmt.Errorf("failed to parse keychain data: %w", err)
}
// json.Unmarshal accepted the JSON, so text that starts with a quote is
// a whole string. The passphrase is hex, so it is the text between the
// quotes.
quoted := fields.AgePrivKeyPassphrase
if !bytes.HasPrefix(quoted, []byte(`"`)) {
return nil, errNoKeychainPassphrase
}
return &KeychainData{
AgePublicKey: fields.AgePublicKey,
// NewBufferFromBytes wipes the bytes it copies.
AgePrivKeyPassphrase: memguard.NewBufferFromBytes(
quoted[1 : len(quoted)-1]),
EncryptedLongtermKey: fields.EncryptedLongtermKey,
}, nil
}
+118
View File
@@ -0,0 +1,118 @@
//nolint:testpackage // white-box test of unexported internals
package secret
import (
"encoding/json"
"testing"
"github.com/awnumar/memguard"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestGenerateRandomPassphrase(t *testing.T) {
t.Parallel()
first, err := generateRandomPassphrase(64)
require.NoError(t, err)
defer first.Destroy()
second, err := generateRandomPassphrase(64)
require.NoError(t, err)
defer second.Destroy()
assert.Regexp(t, `^[0-9a-f]{64}$`, first.String())
assert.NotEqual(t, first.String(), second.String())
assert.False(t, first.IsMutable())
for _, length := range []int{0, -2, 63} {
_, err := generateRandomPassphrase(length)
require.ErrorIs(t, err, errPassphraseLength, "length %d", length)
}
}
func TestKeychainDataEncodeDecode(t *testing.T) {
t.Parallel()
passphrase := memguard.NewBufferFromBytes([]byte("0a1b2c3d"))
defer passphrase.Destroy()
data := KeychainData{
AgePublicKey: "age1example",
AgePrivKeyPassphrase: passphrase,
EncryptedLongtermKey: "beef",
}
encoded, err := data.encode()
require.NoError(t, err)
defer encoded.Destroy()
assert.JSONEq(t,
`{"agePublicKey":"age1example",`+
`"agePrivKeyPassphrase":"0a1b2c3d",`+
`"encryptedLongtermKey":"beef"}`,
encoded.String())
assert.False(t, encoded.IsMutable())
decoded, err := decodeKeychainData(encoded)
require.NoError(t, err)
defer decoded.AgePrivKeyPassphrase.Destroy()
assert.Equal(t, "age1example", decoded.AgePublicKey)
assert.Equal(t, "0a1b2c3d", decoded.AgePrivKeyPassphrase.String())
assert.Equal(t, "beef", decoded.EncryptedLongtermKey)
}
func TestKeychainDataEncodeRejectsBadPassphrase(t *testing.T) {
t.Parallel()
tests := []struct {
name string
passphrase *memguard.LockedBuffer
wantErr error
}{
{"nil", nil, errNilPassphraseBuffer},
{"empty", memguard.NewBuffer(0), errEmptyPassphrase},
{
"not hex",
memguard.NewBufferFromBytes([]byte(`abc"def`)),
errPassphraseNotHex,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
data := KeychainData{AgePrivKeyPassphrase: tt.passphrase}
_, err := data.encode()
require.ErrorIs(t, err, tt.wantErr)
})
}
}
func TestDecodeKeychainDataRejectsBadData(t *testing.T) {
t.Parallel()
for _, text := range []string{
`{"agePublicKey":"age1example"}`,
`{"agePrivKeyPassphrase":42}`,
} {
data := memguard.NewBufferFromBytes([]byte(text))
_, err := decodeKeychainData(data)
data.Destroy()
require.ErrorIs(t, err, errNoKeychainPassphrase, text)
}
notJSON := memguard.NewBufferFromBytes([]byte(`{"agePrivKeyPassphrase":`))
defer notJSON.Destroy()
_, err := decodeKeychainData(notJSON)
var syntaxError *json.SyntaxError
require.ErrorAs(t, err, &syntaxError)
}
+14 -30
View File
@@ -45,13 +45,6 @@ type KeychainUnlocker struct {
fs afero.Fs fs afero.Fs
} }
// KeychainData represents the data stored in the macOS keychain
type KeychainData struct {
AgePublicKey string `json:"agePublicKey"`
AgePrivKeyPassphrase string `json:"agePrivKeyPassphrase"`
EncryptedLongtermKey string `json:"encryptedLongtermKey"`
}
// GetIdentity implements Unlocker interface for Keychain-based unlockers // GetIdentity implements Unlocker interface for Keychain-based unlockers
func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
DebugWith("Getting keychain unlocker identity", DebugWith("Getting keychain unlocker identity",
@@ -81,13 +74,18 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
slog.Int("data_length", len(keychainDataBytes)), slog.Int("data_length", len(keychainDataBytes)),
) )
// Move the keychain data into locked memory; this wipes keychainDataBytes
keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
defer keychainDataBuffer.Destroy()
// Step 3: Parse keychain data // Step 3: Parse keychain data
var keychainData KeychainData keychainData, err := decodeKeychainData(keychainDataBuffer)
if err := json.Unmarshal(keychainDataBytes, &keychainData); err != nil { if err != nil {
Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID()) Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID())
return nil, fmt.Errorf("failed to parse keychain data: %w", err) return nil, fmt.Errorf("failed to parse keychain data: %w", err)
} }
defer keychainData.AgePrivKeyPassphrase.Destroy()
Debug("Parsed keychain data successfully", "unlocker_id", k.GetID()) Debug("Parsed keychain data successfully", "unlocker_id", k.GetID())
@@ -109,11 +107,7 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
// Step 5: Decrypt the age private key using the passphrase from keychain // Step 5: Decrypt the age private key using the passphrase from keychain
Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID()) Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID())
// Create secure buffer for the keychain passphrase agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase)
passphraseBuffer := memguard.NewBufferFromBytes([]byte(keychainData.AgePrivKeyPassphrase))
defer passphraseBuffer.Destroy()
agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, passphraseBuffer)
if err != nil { if err != nil {
Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID()) Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID())
@@ -369,6 +363,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to generate age private key passphrase: %w", err) return nil, fmt.Errorf("failed to generate age private key passphrase: %w", err)
} }
defer agePrivKeyPassphrase.Destroy()
// Step 3: Store age recipient as plaintext // Step 3: Store age recipient as plaintext
ageRecipient := ageIdentity.Recipient().String() ageRecipient := ageIdentity.Recipient().String()
@@ -378,15 +373,12 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
} }
// Step 4: Encrypt age private key with the generated passphrase and store on disk // Step 4: Encrypt age private key with the generated passphrase and store on disk
// Create secure buffers for both the private key and passphrase // Create a secure buffer for the private key
agePrivKeyStr := ageIdentity.String() agePrivKeyStr := ageIdentity.String()
agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr)) agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
defer agePrivKeyBuffer.Destroy() defer agePrivKeyBuffer.Destroy()
passphraseBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyPassphrase)) encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, agePrivKeyPassphrase)
defer passphraseBuffer.Destroy()
encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, passphraseBuffer)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err) return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
} }
@@ -422,13 +414,10 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
EncryptedLongtermKey: hex.EncodeToString(encryptedLtPrivKeyToAge), EncryptedLongtermKey: hex.EncodeToString(encryptedLtPrivKeyToAge),
} }
keychainDataBytes, err := json.Marshal(keychainData) keychainDataBuffer, err := keychainData.encode()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to marshal keychain data: %w", err) return nil, fmt.Errorf("failed to encode keychain data: %w", err)
} }
// Create a secure buffer for keychain data
keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
defer keychainDataBuffer.Destroy() defer keychainDataBuffer.Destroy()
// Step 8: Store data in keychain // Step 8: Store data in keychain
@@ -501,7 +490,7 @@ func storeInKeychain(itemName string, data *memguard.LockedBuffer) error {
item.SetAccount(itemName) item.SetAccount(itemName)
item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName)) item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName))
item.SetDescription("Secret vault keychain data") item.SetDescription("Secret vault keychain data")
item.SetData([]byte(data.String())) item.SetData(data.Bytes())
item.SetSynchronizable(keychain.SynchronizableNo) item.SetSynchronizable(keychain.SynchronizableNo)
// Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth // Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth
item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly)
@@ -576,8 +565,3 @@ func deleteFromKeychain(itemName string) error {
return nil return nil
} }
// generateRandomPassphrase generates a random passphrase for encrypting the age private key
func generateRandomPassphrase(length int) (string, error) {
return generateRandomString(length, "0123456789abcdef")
}
-58
View File
@@ -310,64 +310,6 @@ func TestPerSecretKeyFunctionality(t *testing.T) {
}) })
} }
// For testing purposes only
func isValidSecretName(name string) bool {
if name == "" {
return false
}
// Valid characters for secret names: letters, numbers, dash, dot, underscore, slash
for _, char := range name {
if (char < 'a' || char > 'z') && // lowercase letters
(char < 'A' || char > 'Z') && // uppercase letters
(char < '0' || char > '9') && // numbers
char != '-' && // dash
char != '.' && // dot
char != '_' && // underscore
char != '/' { // slash
return false
}
}
return true
}
func TestSecretNameValidation(t *testing.T) {
t.Parallel()
tests := []struct {
name string
valid bool
}{
{"valid-name", true},
{"valid.name", true},
{"valid_name", true},
{"valid/path/name", true},
{"123valid", true},
{"", false},
{"Valid-Upper-Name", true}, // uppercase allowed
{"2025-11-21-ber1app1-vaultik-test-bucket-AKI", true}, // real-world uppercase key ID
{"MixedCase/Path/Name", true}, // mixed case with path
{"invalid name", false}, // space not allowed
{"invalid@name", false}, // @ not allowed
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
result := isValidSecretName(test.name)
if result != test.valid {
t.Errorf(
"isValidSecretName(%q) = %v, want %v",
test.name,
result,
test.valid,
)
}
})
}
}
func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) { func TestSecretGetValueWithEnvMnemonicUsesVaultDerivationIndex(t *testing.T) {
// This test demonstrates the bug where GetValue uses hardcoded index 0 // This test demonstrates the bug where GetValue uses hardcoded index 0
// instead of the vault's actual derivation index when using environment mnemonic // instead of the vault's actual derivation index when using environment mnemonic
+5 -4
View File
@@ -29,10 +29,11 @@ var (
// ErrNilValueBuffer indicates a nil value buffer was supplied. // ErrNilValueBuffer indicates a nil value buffer was supplied.
ErrNilValueBuffer = errors.New("value buffer is nil") ErrNilValueBuffer = errors.New("value buffer is nil")
// ErrInvalidSecretName indicates a secret name that does not match // ErrInvalidSecretName indicates a secret name that breaks the naming
// the allowed pattern [a-z0-9.\-_/]+. Composed as // rule: only ASCII letters, digits, '.', '-', '_' and '/'; not empty;
// "invalid secret name '<name>': must match pattern [a-z0-9.\-_/]+", // no leading '.' or '/', no trailing '/', no '//', no '..' path segment.
// or as "invalid secret name: <name>" by GetSecretObject. // Composed by ValidateSecretName as
// "invalid secret name '<name>': <the rule>".
ErrInvalidSecretName = errors.New("invalid secret name") ErrInvalidSecretName = errors.New("invalid secret name")
// ErrSecretExists indicates the secret already exists and --force // ErrSecretExists indicates the secret already exists and --force
+26 -12
View File
@@ -79,6 +79,7 @@ func (v *Vault) ListSecrets() ([]string, error) {
// - No leading or trailing slashes // - No leading or trailing slashes
// - No double slashes // - No double slashes
// - No names starting with dots // - No names starting with dots
// - No ".." path segments
func isValidSecretName(name string) bool { func isValidSecretName(name string) bool {
if name == "" { if name == "" {
return false return false
@@ -110,6 +111,22 @@ func isValidSecretName(name string) bool {
return matched return matched
} }
// ValidateSecretName returns an error wrapping ErrInvalidSecretName when
// name is not a valid secret name. Call it on the name exactly as the user
// gave it, before building any path from it.
func ValidateSecretName(name string) error {
if !isValidSecretName(name) {
return fmt.Errorf(
"%w '%s': only ASCII letters, digits, '.', '-', '_' and '/' are allowed, "+
"and a name must not be empty, start with '.' or '/', end with '/', "+
"contain '//', or have '..' as a path segment",
ErrInvalidSecretName, name,
)
}
return nil
}
// AddSecret adds a secret to this vault // AddSecret adds a secret to this vault
func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error { func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error {
if value == nil { if value == nil {
@@ -124,13 +141,11 @@ func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool)
) )
// Validate secret name // Validate secret name
if !isValidSecretName(name) { err := ValidateSecretName(name)
if err != nil {
secret.Debug("Invalid secret name provided", "secret_name", name) secret.Debug("Invalid secret name provided", "secret_name", name)
return fmt.Errorf( return err
"%w '%s': must match pattern [a-z0-9.\\-_/]+",
ErrInvalidSecretName, name,
)
} }
secret.Debug("Secret name validation passed", "secret_name", name) secret.Debug("Secret name validation passed", "secret_name", name)
@@ -358,8 +373,9 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
// GetSecretObject retrieves a Secret object with metadata loaded from this vault // GetSecretObject retrieves a Secret object with metadata loaded from this vault
func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) { func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
if !isValidSecretName(name) { err := ValidateSecretName(name)
return nil, fmt.Errorf("%w: %s", ErrInvalidSecretName, name) if err != nil {
return nil, err
} }
// First check if the secret exists by checking for the metadata file // First check if the secret exists by checking for the metadata file
@@ -628,13 +644,11 @@ func (v *Vault) updatePreviousVersion(
// version exist, and resolves an empty version to the current one. // version exist, and resolves an empty version to the current one.
func (v *Vault) resolveSecretVersion(name, version string) (string, error) { func (v *Vault) resolveSecretVersion(name, version string) (string, error) {
// Validate secret name to prevent path traversal // Validate secret name to prevent path traversal
if !isValidSecretName(name) { err := ValidateSecretName(name)
if err != nil {
secret.Debug("Invalid secret name provided", "secret_name", name) secret.Debug("Invalid secret name provided", "secret_name", name)
return "", fmt.Errorf( return "", err
"%w '%s': must match pattern [a-z0-9.\\-_/]+",
ErrInvalidSecretName, name,
)
} }
// Get vault directory // Get vault directory
+3 -2
View File
@@ -1,8 +1,9 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs script/check # script/cibuild: run the CI build. The Dockerfile runs script/check
# (via make check), so a successful build implies all checks pass. # (via make check), so a successful build implies all checks pass.
# The Gitea workflow runs this on push. The memlock ulimit is required # The Gitea workflow runs this on push. The memlock ulimit lets the tests
# because the test suite uses memguard, which mlocks memory. # that lock large secrets in memory (memguard mlocks them) run; under the
# lower limit of a plain `docker build .` they are skipped.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+11 -2
View File
@@ -1,7 +1,8 @@
#!/bin/sh #!/bin/sh
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname.
# Generic: needs no adaptation. # --no-cache because the gate phases the final stage depends on are RUN
# steps, and a cached one is a check that did not run.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -9,7 +10,15 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build -t "$("$SCRIPT_DIR/projectname")" . # Own line: a failing command substitution inside an argument does
# not trip `set -e`, so the inline form degrades silently to an
# empty constant. The VERSION build argument takes precedence over
# the version a build stage derives from the .git in the context.
version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown"
docker build --no-cache \
--build-arg VERSION="$version" \
-t "$("$SCRIPT_DIR/projectname")" .
} }
main "$@" main "$@"