sneak/vaultik
1
0
Fork 0
Code Issues 20 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

Parse age_recipients when the config loads, and never print the entry in an error #153

New Issue
Closed
opened 2026-09-22 00:54:59 +02:00 by clawbot · 1 comment
clawbot commented 2026-09-22 00:54:59 +02:00
Collaborator
Copy Link
Copy Source

Found by the security review #73. Severity: high (a private key can reach stderr, logs and cron mail after one plausible operator mistake).

What is wrong

Config.Validate only checks that age_recipients is non-empty (internal/config/config.go:296). The first real parse is in blobgen.NewWriter (internal/blobgen/writer.go:61-63), reached when the packer starts its first blob, after the snapshot row exists and the source tree has been walked.

The error text carries the whole entry twice: vaultik's own %s, and age's error, which quotes its input in every failure branch. If someone pastes an AGE-SECRET-KEY-1... string into age_recipients, the private key is printed to stderr and the log (internal/cli/snapshot.go:98-99), once per file under --skip-errors (internal/snapshot/scanner.go:1308-1311). The same %s pattern sits in internal/crypto/encryption.go:42 and :148.

Lesser effects of the late parse: the placeholder written by config init (age1REPLACE_WITH_YOUR_PUBLIC_KEY, internal/cli/config.go:53) passes config load; ssh and plugin recipients are rejected only mid-run; a typo in a working config is found at the next scheduled backup.

It fails closed: nothing is ever written unencrypted.

Acceptable

  • Config.Validate parses every entry with age.ParseX25519Recipient. On failure the error names the position (age_recipients[1]), never the value, and does not wrap age's error. It says only X25519 age1... recipients are supported.
  • An entry starting with AGE-SECRET-KEY- (compare case-insensitively) gets its own message: a secret key was given where a public key belongs.
  • The value and the wrapped age error are removed from the remaining parse sites too (writer.go:63, encryption.go:42, :148), since callers that skip config.Load still reach them.
  • test/config.yaml:3 holds a placeholder second recipient (age1otherpubkey...); replace it with a valid X25519 public key and keep the two-recipient assertion in internal/config/config_test.go:49.

Definition of done

  1. Tests: the config init placeholder, an ssh-ed25519 recipient, a truncated age1 string and a valid two-recipient list through Validate.
  2. Test: a secret key passed as a recipient to Validate, blobgen.NewWriter and crypto.NewEncryptor; the key string does not appear in any returned error. (Skip the internal/crypto sites if that package has been removed by then.)
  3. No existing assertion weakened; make check green.

Optional and separate: NewScanner returns nil after logging when the packer cannot be built (scanner.go:161-163) and its caller does not check; unreachable today.

Line numbers are as of next at 6fcd8e1.

model: fable-5-1

Found by the security review https://git.eeqj.de/sneak/vaultik/issues/73. Severity: **high** (a private key can reach stderr, logs and cron mail after one plausible operator mistake). ## What is wrong `Config.Validate` only checks that `age_recipients` is non-empty (`internal/config/config.go:296`). The first real parse is in `blobgen.NewWriter` (`internal/blobgen/writer.go:61-63`), reached when the packer starts its first blob, after the snapshot row exists and the source tree has been walked. The error text carries the whole entry twice: vaultik's own `%s`, and age's error, which quotes its input in every failure branch. If someone pastes an `AGE-SECRET-KEY-1...` string into `age_recipients`, the private key is printed to stderr and the log (`internal/cli/snapshot.go:98-99`), once per file under `--skip-errors` (`internal/snapshot/scanner.go:1308-1311`). The same `%s` pattern sits in `internal/crypto/encryption.go:42` and `:148`. Lesser effects of the late parse: the placeholder written by `config init` (`age1REPLACE_WITH_YOUR_PUBLIC_KEY`, `internal/cli/config.go:53`) passes config load; ssh and plugin recipients are rejected only mid-run; a typo in a working config is found at the next scheduled backup. It fails closed: nothing is ever written unencrypted. ## Acceptable - `Config.Validate` parses every entry with `age.ParseX25519Recipient`. On failure the error names the position (`age_recipients[1]`), never the value, and does not wrap age's error. It says only X25519 `age1...` recipients are supported. - An entry starting with `AGE-SECRET-KEY-` (compare case-insensitively) gets its own message: a secret key was given where a public key belongs. - The value and the wrapped age error are removed from the remaining parse sites too (`writer.go:63`, `encryption.go:42`, `:148`), since callers that skip `config.Load` still reach them. - `test/config.yaml:3` holds a placeholder second recipient (`age1otherpubkey...`); replace it with a valid X25519 public key and keep the two-recipient assertion in `internal/config/config_test.go:49`. ## Definition of done 1. Tests: the `config init` placeholder, an `ssh-ed25519` recipient, a truncated `age1` string and a valid two-recipient list through `Validate`. 2. Test: a secret key passed as a recipient to `Validate`, `blobgen.NewWriter` and `crypto.NewEncryptor`; the key string does not appear in any returned error. (Skip the `internal/crypto` sites if that package has been removed by then.) 3. No existing assertion weakened; `make check` green. Optional and separate: `NewScanner` returns nil after logging when the packer cannot be built (`scanner.go:161-163`) and its caller does not check; unreachable today. Line numbers are as of `next` at `6fcd8e1`. model: fable-5-1
clawbot commented 2026-09-22 12:42:39 +02:00
Author
Collaborator
Copy Link
Copy Source

Implemented in #187.

Config.Validate now parses each age_recipients entry, so a bad recipient fails at config load rather than mid-backup. Errors name the position (age_recipients[N]), never the value, and do not wrap age's error; an entry beginning with AGE-SECRET-KEY- (case-insensitive) gets its own message. The value and wrapped age error are removed from the other parse sites too (blobgen.NewWriter, crypto.NewEncryptor/UpdateRecipients). test/config.yaml's placeholder second recipient is replaced with a valid X25519 key.

Tests cover the config init placeholder, an ssh-ed25519 recipient, a truncated age1 string, a valid two-recipient list, and that a secret key passed to Validate, blobgen.NewWriter, and crypto.NewEncryptor never appears in the returned error. make check is green.

Model: opus-4-8

Implemented in https://git.eeqj.de/sneak/vaultik/pulls/187. `Config.Validate` now parses each `age_recipients` entry, so a bad recipient fails at config load rather than mid-backup. Errors name the position (`age_recipients[N]`), never the value, and do not wrap age's error; an entry beginning with `AGE-SECRET-KEY-` (case-insensitive) gets its own message. The value and wrapped age error are removed from the other parse sites too (`blobgen.NewWriter`, `crypto.NewEncryptor`/`UpdateRecipients`). `test/config.yaml`'s placeholder second recipient is replaced with a valid X25519 key. Tests cover the `config init` placeholder, an `ssh-ed25519` recipient, a truncated `age1` string, a valid two-recipient list, and that a secret key passed to `Validate`, `blobgen.NewWriter`, and `crypto.NewEncryptor` never appears in the returned error. `make check` is green. Model: opus-4-8
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#153