From 28f44f043cabb3f9dc5d1a263507af3980348a92 Mon Sep 17 00:00:00 2001 From: Brian Majewski Date: Wed, 24 Jun 2026 13:17:28 -0700 Subject: [PATCH] test: recipients.txt security regression fixtures + dangling-symlink fix (EGB-283) Swap _load_recipients check order so [ -L ] (symlink) runs before [ ! -e ] (missing), closing the gap where a dangling symlink bypassed the refusal and silently fell back to legacy single-key mode. Add 6 SECURITY-tagged fixtures to test/recipients.bats: dangling-symlink refused (the ordering gap), shell-metachar injection (no execution), extra- age-flag-looking line, control/ANSI chars, embedded whitespace via add, and symlinked file refused on add. All 6 pass immediately after the ordering fix; the existing rails (_validate_age_recipient, -L checks) were already tight enough that only the production swap was needed. Co-Authored-By: Claude Sonnet 4.6 --- secrets | 6 ++--- test/recipients.bats | 59 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/secrets b/secrets index 5711842..421fc5f 100755 --- a/secrets +++ b/secrets @@ -135,13 +135,13 @@ _validate_recipient_name() { RECIPIENT_ARGS=() _load_recipients() { RECIPIENT_ARGS=() + if [ -L "$RECIPIENTS_FILE" ]; then + die "Refusing to read symlinked $RECIPIENTS_FILE_NAME (security)." + fi if [ ! -e "$RECIPIENTS_FILE" ]; then RECIPIENT_ARGS=(-r "$(get_pubkey)") return 0 fi - if [ -L "$RECIPIENTS_FILE" ]; then - die "Refusing to read symlinked $RECIPIENTS_FILE_NAME (security)." - fi local line trimmed n=0 while IFS= read -r line || [ -n "$line" ]; do trimmed="${line#"${line%%[![:space:]]*}"}" # lstrip diff --git a/test/recipients.bats b/test/recipients.bats index c02888c..ae13b22 100644 --- a/test/recipients.bats +++ b/test/recipients.bats @@ -290,3 +290,62 @@ make_second_identity() { [ "$status" -ne 0 ] [[ "$output" == *"recipient"* ]] || false } + +@test "SECURITY: a dangling symlink recipients.txt is refused, not silently ignored" { + init_with_remote + rm -f "$SECRETS_DIR/recipients.txt" + ln -s "$TEST_TMPDIR/does-not-exist.txt" "$SECRETS_DIR/recipients.txt" + create_project_dir myproj + run "$SECRETS_BIN" push + [ "$status" -ne 0 ] + [[ "$output" == *"symlink"* ]] || false +} + +@test "SECURITY: recipients.txt with shell metacharacters is rejected, no execution" { + init_with_remote + STORE_PUB=$(age-keygen -y "$SECRETS_DIR/key.txt") + printf '%s\nage1$(touch %s/pwned)\n' "$STORE_PUB" "$TEST_TMPDIR" > "$SECRETS_DIR/recipients.txt" + create_project_dir myproj + run "$SECRETS_BIN" push + [ "$status" -ne 0 ] + [ ! -e "$TEST_TMPDIR/pwned" ] + [[ "$output" == *"Invalid recipient"* ]] || false +} + +@test "SECURITY: recipients.txt line that looks like an extra age flag is rejected" { + init_with_remote + STORE_PUB=$(age-keygen -y "$SECRETS_DIR/key.txt") + printf '%s\n-i /etc/passwd\n' "$STORE_PUB" > "$SECRETS_DIR/recipients.txt" + create_project_dir myproj + run "$SECRETS_BIN" push + [ "$status" -ne 0 ] + [[ "$output" == *"Invalid recipient"* ]] || false +} + +@test "SECURITY: control/ANSI characters in recipients.txt are rejected" { + init_with_remote + STORE_PUB=$(age-keygen -y "$SECRETS_DIR/key.txt") + printf '%s\nage1%b\n' "$STORE_PUB" 'aaaa\033[31mevil' > "$SECRETS_DIR/recipients.txt" + create_project_dir myproj + run "$SECRETS_BIN" push + [ "$status" -ne 0 ] +} + +@test "SECURITY: recipients add rejects a key with embedded whitespace" { + init_with_remote + run "$SECRETS_BIN" recipients add "age1aaaa bbbb" + [ "$status" -ne 0 ] + [[ "$output" == *"valid age recipient"* ]] || false +} + +@test "SECURITY: a symlinked recipients.txt is refused on add and rm too" { + init_with_remote + make_second_identity + STORE_PUB=$(age-keygen -y "$SECRETS_DIR/key.txt") + printf '%s\n' "$STORE_PUB" > "$TEST_TMPDIR/elsewhere.txt" + rm -f "$SECRETS_DIR/recipients.txt" + ln -s "$TEST_TMPDIR/elsewhere.txt" "$SECRETS_DIR/recipients.txt" + run "$SECRETS_BIN" recipients add "$BOB_PUB" + [ "$status" -ne 0 ] + [[ "$output" == *"symlink"* ]] || false +}