1 Commits
Author SHA1 Message Date
sneak 0f5d884eb2 Reject invalid secret names before any command builds a path (closes #33)
check / check (push) Successful in 49s
`secret rm ..` resolved to the vault directory and 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
without checking it; import checked it only after reading the source
file.

vault.ValidateSecretName wraps the existing name rule and its error
states the rule. Each of those commands calls it on the name as given,
before building any path; a move checks both names before switching
the current vault. AddSecret, GetSecretVersion and GetSecretObject use
it too.

The regression test copies two in-memory vaults for each rejected
command and requires the exact error and an unchanged state directory.

Model: opus-5-5
2026-10-03 14:01:29 +00:00
3 changed files with 29 additions and 20 deletions
+1 -3
View File
@@ -113,9 +113,7 @@ 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:** only letters, digits, `.`, `-`, `_` and `/` are **Secret Name Format:** `[a-z0-9\.\-\_\/]+`
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`
+1 -1
View File
@@ -30,7 +30,7 @@ Bring the repo into policy compliance in one commit:
nothing when it is invalid: `rm`, `mv` (both names, within a vault nothing when it is invalid: `rm`, `mv` (both names, within a vault
and between vaults, before switching the current vault), `import`, and between vaults, before switching the current vault), `import`,
`version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error `version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error
and `README.md` state the naming rule. Before, `secret rm ..` deleted the whole states the naming rule. Before, `secret rm ..` deleted the whole
vault and `secret rm .` every secret in it. vault and `secret rm .` every secret in it.
- 2026-10-02: A plain `docker build .` builds again: the size tests - 2026-10-02: A plain `docker build .` builds again: the size tests
skip a case that needs more locked memory than the process can skip a case that needs more locked memory than the process can
+27 -16
View File
@@ -712,8 +712,13 @@ 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 && destQualified { if !srcQualified {
return errCrossVaultSourceUnqualified return errCrossVaultSourceUnqualified
} }
@@ -721,27 +726,29 @@ 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 srcQualified && !destQualified { if !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. A plain // If destination secret name is empty, use source secret name
// rename keeps it empty, so that the check below rejects it. if destSecretName == "" {
if srcQualified && destSecretName == "" {
destSecretName = srcSecretName destSecretName = srcSecretName
} }
// Check both names, for every form of the move, before selecting a vault // Check both names before selecting a vault below, so that a rejected
// below, so that a rejected move leaves the current vault as it was. // move leaves the current vault as it was.
err := vault.ValidateSecretName(srcSecretName) err := vault.ValidateSecretName(srcSecretName)
if err != nil { if err != nil {
return err return err
@@ -752,11 +759,6 @@ func (cli *Instance) MoveSecret(
return err 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
@@ -773,11 +775,20 @@ func (cli *Instance) MoveSecret(
cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force) cmd, srcVaultName, srcSecretName, destVaultName, destSecretName, force)
} }
// moveSecretWithinVault handles rename within the current vault. Its caller, // moveSecretWithinVault handles rename within the current vault
// 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 {
err := vault.ValidateSecretName(source)
if err != nil {
return err
}
err = vault.ValidateSecretName(dest)
if err != nil {
return err
}
currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
if err != nil { if err != nil {
return err return err