fix: pre-landing review fixes (heal ordering, key untrack, content-aware gitignore)
Red-team + specialist findings from /ship pre-landing review: - CRITICAL: ensure_store_protections ran BEFORE `git pull --ff-only` in commit_and_push_secrets; a pull bringing history without .gitignore left the post-pull window unprotected and `git add -A` committed AND pushed key.txt (reproduced end-to-end). Heal now runs after the pull, immediately before staging. - CRITICAL: .gitignore can't untrack an already-tracked key.txt. Heal now defensively `git rm --cached`s a tracked key with a history-scrub warning. - Content-aware heal: a present .gitignore missing the key.txt line is rewritten, not skipped (security specialist). - Init guard now renders the real sanitized clone URL from .secrets-store when available, mirroring EGB-282's check_initialized. - Tests: positive heal-message assertions, non-empty key guard in the init test, 4 new regression tests (126 total).
This commit is contained in:
Brian Majewski
parent
5865c40d77
commit
2a7afc34dd
3 changed files with 82 additions and 7 deletions
|
|
@ -37,7 +37,7 @@ Single bash script (`secrets`) with subcommands: init, push, pull, list, rm, rek
|
|||
secrets # CLI script (~600 lines bash)
|
||||
hooks/pre-commit # Pre-commit hook template
|
||||
test/
|
||||
secrets.bats # bats-core test suite (118 tests)
|
||||
secrets.bats # bats-core test suite (126 tests)
|
||||
test_helper.bash # Shared setup/teardown
|
||||
README.md # User-facing documentation
|
||||
CLAUDE.md # This file
|
||||
|
|
|
|||
27
secrets
27
secrets
|
|
@ -724,10 +724,20 @@ EOF
|
|||
# pre-commit hook (hooks aren't cloned), and a half-initialized store may
|
||||
# lack .gitignore — without it, `git add -A` would commit key.txt.
|
||||
ensure_store_protections() {
|
||||
if [ ! -f "$SECRETS_DIR/.gitignore" ]; then
|
||||
# Content-aware: a present-but-corrupted .gitignore missing the key.txt
|
||||
# line is just as dangerous as a missing one.
|
||||
if [ ! -f "$SECRETS_DIR/.gitignore" ] || ! grep -qx 'key.txt' "$SECRETS_DIR/.gitignore"; then
|
||||
write_store_gitignore
|
||||
info "Restored store .gitignore"
|
||||
fi
|
||||
# .gitignore can't untrack an already-tracked key (legacy damage, or a
|
||||
# past window where .gitignore was missing). Remove it from the index so
|
||||
# the next commit drops it from the tip.
|
||||
if git -C "$SECRETS_DIR" ls-files --error-unmatch key.txt >/dev/null 2>&1; then
|
||||
git -C "$SECRETS_DIR" rm --cached --quiet key.txt
|
||||
echo "WARNING: key.txt was tracked in the store repo — untracked it now." >&2
|
||||
echo "It may still exist in git history; consider 'secrets rekey' and scrubbing history." >&2
|
||||
fi
|
||||
if [ ! -x "$SECRETS_DIR/.git/hooks/pre-commit" ]; then
|
||||
mkdir -p "$SECRETS_DIR/.git/hooks"
|
||||
install_hook
|
||||
|
|
@ -750,10 +760,14 @@ cmd_init() {
|
|||
# should clone their existing secrets repo, not init a fresh one.
|
||||
# Catch it BEFORE git init so we don't leave a half-initialized store.
|
||||
if [ -f "$KEY_FILE" ]; then
|
||||
# Render a runnable clone command when .secrets-store carried a remote
|
||||
# URL (already sanitized by resolve_store), mirroring check_initialized.
|
||||
local clone_src="<your-secrets-remote>"
|
||||
[ -n "${_REMOTE_URL:-}" ] && clone_src="$_REMOTE_URL"
|
||||
die "Found an existing key at $KEY_FILE but no repo at $SECRETS_DIR.
|
||||
If this is a second machine, don't run 'secrets init' — clone your existing secrets repo instead:
|
||||
|
||||
git clone <your-secrets-remote> $SECRETS_DIR
|
||||
git clone $clone_src $SECRETS_DIR
|
||||
|
||||
Your key file has been left untouched."
|
||||
fi
|
||||
|
|
@ -814,16 +828,17 @@ push_dir_to_project() {
|
|||
commit_and_push_secrets() {
|
||||
local message="$1"
|
||||
|
||||
# Must run before `git add -A`: a store missing its .gitignore would
|
||||
# otherwise stage and push key.txt.
|
||||
ensure_store_protections
|
||||
|
||||
if git -C "$SECRETS_DIR" remote get-url origin >/dev/null 2>&1; then
|
||||
if ! git -C "$SECRETS_DIR" pull --ff-only 2>/dev/null; then
|
||||
die "Fast-forward pull failed. Run 'secrets pull' first, then retry push."
|
||||
fi
|
||||
fi
|
||||
|
||||
# Must run AFTER the pull and immediately before `git add -A`: the pull
|
||||
# can remove or alter .gitignore (remote history that lacks it), and a
|
||||
# store missing the key.txt line would stage and push the private key.
|
||||
ensure_store_protections
|
||||
|
||||
git -C "$SECRETS_DIR" add -A
|
||||
if git -C "$SECRETS_DIR" diff --cached --quiet 2>/dev/null; then
|
||||
info "No changes to push (secrets unchanged)"
|
||||
|
|
|
|||
|
|
@ -1484,6 +1484,8 @@ gradle_project() {
|
|||
# `secrets init` instead of cloning their secrets repo.
|
||||
mkdir -p "$SECRETS_DIR"
|
||||
age-keygen -o "$SECRETS_DIR/key.txt" 2>/dev/null
|
||||
# Guard against a vacuous '' = '' comparison if age-keygen failed
|
||||
[ -s "$SECRETS_DIR/key.txt" ]
|
||||
local key_before
|
||||
key_before=$(cat "$SECRETS_DIR/key.txt")
|
||||
|
||||
|
|
@ -1503,6 +1505,7 @@ gradle_project() {
|
|||
|
||||
run "$SECRETS_BIN" push
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"Restored store .gitignore"* ]]
|
||||
[ -f "$SECRETS_DIR/.gitignore" ]
|
||||
grep -q "key.txt" "$SECRETS_DIR/.gitignore"
|
||||
# key.txt must never be tracked (push does `git add -A` in the store)
|
||||
|
|
@ -1560,6 +1563,7 @@ gradle_project() {
|
|||
|
||||
run "$SECRETS_BIN" push
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"Reinstalled pre-commit hook"* ]]
|
||||
[ -x "$SECRETS_DIR/.git/hooks/pre-commit" ]
|
||||
}
|
||||
|
||||
|
|
@ -1597,3 +1601,59 @@ gradle_project() {
|
|||
grep -qF '!**/.env.*.age' "$SECRETS_DIR/.gitignore"
|
||||
grep -qF '!**/.dev.vars.age' "$SECRETS_DIR/.gitignore"
|
||||
}
|
||||
|
||||
@test "push heals .gitignore removed by remote history before staging (key never pushed)" {
|
||||
init_with_remote
|
||||
create_project_dir
|
||||
"$SECRETS_BIN" push >/dev/null 2>&1
|
||||
# Remote history drops .gitignore (e.g. an old machine committed without it)
|
||||
git clone -q "$REMOTE_DIR" "$TEST_TMPDIR/other"
|
||||
git -C "$TEST_TMPDIR/other" rm -q .gitignore
|
||||
git -C "$TEST_TMPDIR/other" -c user.email=t@t -c user.name=t commit -qm "drop gitignore"
|
||||
git -C "$TEST_TMPDIR/other" push -q
|
||||
|
||||
echo "B=2" >> .env
|
||||
run "$SECRETS_BIN" push
|
||||
[ "$status" -eq 0 ]
|
||||
[ -f "$SECRETS_DIR/.gitignore" ]
|
||||
run git -C "$SECRETS_DIR" ls-files
|
||||
[[ "$output" != *"key.txt"* ]]
|
||||
}
|
||||
|
||||
@test "push untracks a previously committed key.txt with a warning" {
|
||||
init_with_remote
|
||||
# Simulate legacy damage: key.txt got committed in the past
|
||||
git -C "$SECRETS_DIR" add -f key.txt
|
||||
git -C "$SECRETS_DIR" -c user.email=t@t -c user.name=t commit -qm "oops"
|
||||
create_project_dir
|
||||
|
||||
run "$SECRETS_BIN" push
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"key.txt was tracked"* ]]
|
||||
run git -C "$SECRETS_DIR" ls-files
|
||||
[[ "$output" != *"key.txt"* ]]
|
||||
}
|
||||
|
||||
@test "push rewrites a store .gitignore that is missing the key.txt line" {
|
||||
init_with_remote
|
||||
printf '%s\n' '**/.env' > "$SECRETS_DIR/.gitignore"
|
||||
create_project_dir
|
||||
|
||||
run "$SECRETS_BIN" push
|
||||
[ "$status" -eq 0 ]
|
||||
grep -qx 'key.txt' "$SECRETS_DIR/.gitignore"
|
||||
run git -C "$SECRETS_DIR" ls-files
|
||||
[[ "$output" != *"key.txt"* ]]
|
||||
}
|
||||
|
||||
@test "init guard renders the real clone URL when .secrets-store carries a remote" {
|
||||
mkdir -p "$HOME/.secrets-work"
|
||||
age-keygen -o "$HOME/.secrets-work/key.txt" 2>/dev/null
|
||||
[ -s "$HOME/.secrets-work/key.txt" ]
|
||||
cd "$WORK_DIR"
|
||||
echo "work git@example.com:me/secrets-work.git" > .secrets-store
|
||||
|
||||
run "$SECRETS_BIN" init
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"git clone git@example.com:me/secrets-work.git"* ]]
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue