secret version rm x ../../.. deletes the vault and beyond: the version argument goes into a path unchecked #67

New Issue

2026-10-03T14:42:10+02:00

clawbot commented

2026-10-03 14:42:10 +02:00

Found by the audit for #33 (comment on that issue). Same class of defect, not fixed by #65.

Problem

secret version rm, secret version promote and secret get --version put the version argument into a filesystem path without any check. The "cannot remove the current version" guard does not help, because the argument never equals the current version name. With an existing secret x:

secret version rm x "" or secret version rm x . deletes every version of x, the current one included.
secret version rm x .. deletes the secret x.
secret version rm x ../../.. deletes the whole vault. Each further .. climbs one level: all vaults, the state directory, then (Linux, default state directory) ~/.config and the home directory.

An empty shell variable is enough to trigger the first case. promote with such an argument rewrites the current-version pointer to a path outside the secret; get --version reads outside it.

Definition of done

The version argument of version rm, version promote and get --version is accepted only if it names an existing entry of that secret's versions directory, checked on the argument as typed, before any path is built. No filepath.Clean/prefix check; no second copy of a rule.
rm, promote and get --version with "", ., .., ../../.. and a/b each fail with a clear error and non-zero exit.
Regression tests record every file and directory under the state directory (with contents) before each rejected command and require an unchanged record afterwards, the same way the tests in #65 do; asserting only the error is not enough.
TODO.md updated in the same commit.

Sequencing

After #65 lands (same command files, and its test helper is reused).

Model: opus-5-5

Found by the audit for https://git.eeqj.de/sneak/secret/issues/33 (comment on that issue). Same class of defect, not fixed by https://git.eeqj.de/sneak/secret/pulls/65. ## Problem `secret version rm`, `secret version promote` and `secret get --version` put the version argument into a filesystem path without any check. The "cannot remove the current version" guard does not help, because the argument never equals the current version name. With an existing secret `x`: - `secret version rm x ""` or `secret version rm x .` deletes every version of `x`, the current one included. - `secret version rm x ..` deletes the secret `x`. - `secret version rm x ../../..` deletes the whole vault. Each further `..` climbs one level: all vaults, the state directory, then (Linux, default state directory) `~/.config` and the home directory. An empty shell variable is enough to trigger the first case. `promote` with such an argument rewrites the current-version pointer to a path outside the secret; `get --version` reads outside it. ## Definition of done - The version argument of `version rm`, `version promote` and `get --version` is accepted only if it names an existing entry of that secret's `versions` directory, checked on the argument as typed, before any path is built. No `filepath.Clean`/prefix check; no second copy of a rule. - `rm`, `promote` and `get --version` with `""`, `.`, `..`, `../../..` and `a/b` each fail with a clear error and non-zero exit. - Regression tests record every file and directory under the state directory (with contents) before each rejected command and require an unchanged record afterwards, the same way the tests in https://git.eeqj.de/sneak/secret/pulls/65 do; asserting only the error is not enough. - `TODO.md` updated in the same commit. ## Sequencing After https://git.eeqj.de/sneak/secret/pulls/65 lands (same command files, and its test helper is reused). Model: opus-5-5

clawbot added the critical label 2026-10-03 14:42:10 +02:00

clawbot commented

2026-10-03 14:42:10 +02:00

Labelled critical: secret version rm x ../../.. (or "", ., .. as the version) deletes every version of a secret, the secret, the whole vault, or directories above it, with nothing to stop it.

Model: opus-5-5

Labelled critical: `secret version rm x ../../..` (or `""`, `.`, `..` as the version) deletes every version of a secret, the secret, the whole vault, or directories above it, with nothing to stop it. Model: opus-5-5

clawbot referenced this issue

2026-10-04 01:31:25 +02:00

next → main: a plain docker build passes and stamps the version #59

clawbot referenced this issue from a commit

2026-10-04 01:51:16 +02:00

Accept a version argument only if the secret has that version (closes #67)

clawbot referenced this issue from a commit

2026-10-04 01:54:19 +02:00

Accept a version argument only if the secret has that version (closes #67)

clawbot referenced this issue from a commit

2026-10-04 01:59:41 +02:00

Accept a version argument only if the secret has that version (closes #67)