Vault names accept . and ..; vault import and the vault part of a cross-vault move skip the vault name check #68

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).

Problem

The vault name check accepts . and .., and two commands do not call it at all: secret vault import and the vault part of a move between vaults (secret mv vault:name ...). Consequences:

vault import .. (or .) writes a public key, metadata and an unlocker into the state directory or any other existing directory.
A cross-vault move can copy a secret into any other location that holds a vault layout.
vault create . creates a vault inside vaults.d, whose secrets.d and unlockers.d then list as vaults.

No data is lost on these paths: a move deletes its source only after copying, and replaces an existing secret only with --force.

Definition of done

The vault name rule rejects "", ., .. and anything containing a path separator.
Every command that turns a user-supplied vault name into a path calls that one rule on the name as typed, before building the path: vault create, vault import, vault select, vault remove, and the vault part of mv.
Tests: each command with ., .. and a/b fails and leaves the state directory unchanged (record before and after, as in #65).
TODO.md updated in the same commit.

Model: opus-5-5

Found by the audit for https://git.eeqj.de/sneak/secret/issues/33 (comment on that issue). ## Problem The vault name check accepts `.` and `..`, and two commands do not call it at all: `secret vault import` and the vault part of a move between vaults (`secret mv vault:name ...`). Consequences: - `vault import ..` (or `.`) writes a public key, metadata and an unlocker into the state directory or any other existing directory. - A cross-vault move can copy a secret into any other location that holds a vault layout. - `vault create .` creates a vault inside `vaults.d`, whose `secrets.d` and `unlockers.d` then list as vaults. No data is lost on these paths: a move deletes its source only after copying, and replaces an existing secret only with `--force`. ## Definition of done - The vault name rule rejects `""`, `.`, `..` and anything containing a path separator. - Every command that turns a user-supplied vault name into a path calls that one rule on the name as typed, before building the path: `vault create`, `vault import`, `vault select`, `vault remove`, and the vault part of `mv`. - Tests: each command with `.`, `..` and `a/b` fails and leaves the state directory unchanged (record before and after, as in https://git.eeqj.de/sneak/secret/pulls/65). - `TODO.md` updated in the same commit. Model: opus-5-5

clawbot referenced this issue

2026-10-04 02:52:25 +02:00

Stop `secret mv` deleting a secret moved onto itself (closes #73) #76

Sign in to join this conversation.