fix(secret-gate): a path git quotes, a placeholder comment, a glued report line
Three ways the fail-closed gate failed open, all reproduced by new tests first: - `git diff --name-only` quotes any path it cannot print as UTF-8 (core.quotePath, on by default). The quoted string was handed back to `git diff -- <path>`, which matches nothing, and an empty diff is `continue`d — so a credential under `café/` committed cleanly. The list is now NUL-delimited and read from a temp file, because command substitution deletes NUL bytes and the names must survive to the loop that reads them. - The placeholder filter matched the whole added line, so `API_TOKEN=<live> # example, placeholder, changeme` was exempt on the strength of its own comment. The filter now reads the assigned value only — the token up to whitespace or `#`. Truncation errs toward blocking; ALLOW_SECRETS remains the way out. - `report+=` ended on a command substitution, so the next file's name was glued onto the previous file's last hit line and the report blamed the wrong file. Three tests, all red before the change: a quoted path, a placeholder comment beside a live value, and two leaking files each named on their own line. Refs #60
This commit is contained in:
1 parent
06f45e83c1
commit
812be6a0a6
2 files changed
+68
-10
No files matched your search
+36
-10
@@ -34,24 +34,36 @@ placeholders='changeme|change[-_]|placeholder|redacted|example|sample|dummy|noke
|
|||||||
# how a gate gets permanently overridden.
|
# how a gate gets permanently overridden.
|
||||||
assign_exempt='(_FILE|_PATH|_DIR|_URL|_ENDPOINT|_SOCKET)[[:space:]]*[:=]|[:=][[:space:]]*["'"'"']?(/|\.\./)'
|
assign_exempt='(_FILE|_PATH|_DIR|_URL|_ENDPOINT|_SOCKET)[[:space:]]*[:=]|[:=][[:space:]]*["'"'"']?(/|\.\./)'
|
||||||
# Fail closed: a gate that cannot read the staged diff must not report success.
|
# Fail closed: a gate that cannot read the staged diff must not report success.
|
||||||
if ! files=$(git diff --cached --diff-filter=ACM --name-only 2>&1); then
|
#
|
||||||
|
# `-z`, and the list lives in a file. Two traps decide this: git quotes any path it cannot print
|
||||||
|
# as UTF-8 (core.quotePath is on by default), and a quoted name handed back to
|
||||||
|
# `git diff -- <path>` matches nothing — the file is skipped, and a skipped file used to be a
|
||||||
|
# silent pass. Command substitution cannot hold the list either, because $() deletes NUL bytes,
|
||||||
|
# so the names go to a temp file and the loop reads them with a NUL delimiter.
|
||||||
|
TMPD=$(mktemp -d) || TMPD=""
|
||||||
|
if [ -z "$TMPD" ]; then
|
||||||
|
echo "SECRET GATE: cannot create a temp dir — refusing to commit." >&2
|
||||||
|
[ -n "${ALLOW_SECRETS:-}" ] && exit 0
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
trap 'rm -rf "$TMPD"' EXIT
|
||||||
|
if ! git diff --cached --diff-filter=ACM --name-only -z > "$TMPD/files" 2> "$TMPD/err"; then
|
||||||
{
|
{
|
||||||
echo "SECRET GATE: cannot read the staged diff — refusing to commit."
|
echo "SECRET GATE: cannot read the staged diff — refusing to commit."
|
||||||
printf ' %s\n' "$files"
|
sed 's/^/ /' "$TMPD/err"
|
||||||
echo "Override deliberately: ALLOW_SECRETS=1 git commit ..."
|
echo "Override deliberately: ALLOW_SECRETS=1 git commit ..."
|
||||||
} >&2
|
} >&2
|
||||||
[ -n "${ALLOW_SECRETS:-}" ] && exit 0
|
[ -n "${ALLOW_SECRETS:-}" ] && exit 0
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
# stderr is captured so the failure branch can quote it; on success it must not
|
# git's stderr stays out of the name list on purpose: a `warning:` line mistaken for a path
|
||||||
# leak a `warning:` line into the file list and have it mistaken for a path.
|
# becomes a file that "cannot be read", which is how a gate starts skipping the real ones.
|
||||||
files=$(printf '%s\n' "$files" | grep -vE '^(warning|fatal|error):' || true)
|
[ -s "$TMPD/files" ] || exit 0
|
||||||
[ -z "${files//[[:space:]]/}" ] && exit 0
|
|
||||||
|
|
||||||
report=""
|
report=""
|
||||||
exempt=""
|
exempt=""
|
||||||
count=0
|
count=0
|
||||||
while IFS= read -r f; do
|
while IFS= read -r -d '' f; do
|
||||||
[ -z "$f" ] && continue
|
[ -z "$f" ] && continue
|
||||||
# Self-exemption, deliberately an exact filename list and not `tests/`: the
|
# Self-exemption, deliberately an exact filename list and not `tests/`: the
|
||||||
# gate's fixtures must contain credential-shaped strings or there is nothing to
|
# gate's fixtures must contain credential-shaped strings or there is nothing to
|
||||||
@@ -69,15 +81,29 @@ while IFS= read -r f; do
|
|||||||
[ -z "$added" ] && continue
|
[ -z "$added" ] && continue
|
||||||
hits=$(printf '%s\n' "$added" | grep -Ei "$shapes" || true)
|
hits=$(printf '%s\n' "$added" | grep -Ei "$shapes" || true)
|
||||||
if [ "${f##*.}" != "md" ]; then
|
if [ "${f##*.}" != "md" ]; then
|
||||||
assigns=$(printf '%s\n' "$added" | grep -Ei "$assign" | grep -Ev "$assign_exempt" | grep -Eiv "$placeholders" || true)
|
# The placeholder filter reads the VALUE, never the whole line. A trailing comment that
|
||||||
|
# says `# example` says nothing about the token the same line assigns; scoping the filter
|
||||||
|
# to what follows the separator is what keeps `API_TOKEN=<real> # placeholder` a block.
|
||||||
|
assigns=$(printf '%s\n' "$added" | grep -Ei "$assign" | grep -Ev "$assign_exempt" \
|
||||||
|
| while IFS= read -r line; do
|
||||||
|
# The judged value is the assigned token alone: it ends at whitespace or `#`, so a
|
||||||
|
# trailing comment cannot vouch for it. Truncating a quoted value early only ever errs
|
||||||
|
# toward blocking, which is the safe direction for a gate that can be overridden.
|
||||||
|
value=${line#*[:=]}
|
||||||
|
value=${value#"${value%%[![:space:]]*}"}
|
||||||
|
value=${value%%[[:space:]]*}
|
||||||
|
value=${value%%#*}
|
||||||
|
printf '%s\n' "$value" | grep -Eiq "$placeholders" && continue
|
||||||
|
printf '%s\n' "$line"
|
||||||
|
done)
|
||||||
hits=$(printf '%s\n%s\n' "$hits" "$assigns" | sed '/^[[:space:]]*$/d')
|
hits=$(printf '%s\n%s\n' "$hits" "$assigns" | sed '/^[[:space:]]*$/d')
|
||||||
fi
|
fi
|
||||||
if [ -n "$hits" ]; then
|
if [ -n "$hits" ]; then
|
||||||
n=$(printf '%s\n' "$hits" | wc -l)
|
n=$(printf '%s\n' "$hits" | wc -l)
|
||||||
count=$((count + n))
|
count=$((count + n))
|
||||||
report+=" $f"$'\n'"$(printf '%s\n' "$hits" | cut -c1-100 | sed 's/^/ /')"
|
report+=" $f"$'\n'"$(printf '%s\n' "$hits" | cut -c1-100 | sed 's/^/ /')"$'\n'
|
||||||
fi
|
fi
|
||||||
done <<< "$files"
|
done < "$TMPD/files"
|
||||||
|
|
||||||
# Say so out loud: an exemption that prints nothing is how a carve-out becomes
|
# Say so out loud: an exemption that prints nothing is how a carve-out becomes
|
||||||
# invisible, and invisible is the property a secret wants.
|
# invisible, and invisible is the property a secret wants.
|
||||||
|
|||||||
@@ -157,3 +157,35 @@ export API_KEY_FILE=/run/secrets/api_key'
|
|||||||
[ "$status" -ne 0 ]
|
[ "$status" -ne 0 ]
|
||||||
[[ "$output" == *"tests/other.bats"* ]]
|
[[ "$output" == *"tests/other.bats"* ]]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@test "a credential under a path git quotes is still read" {
|
||||||
|
# core.quotePath is on by default, so `git diff --name-only` prints "caf\303\251/…" for a
|
||||||
|
# non-ASCII directory. Feed that quoted string back to `git diff -- <path>` and the diff is
|
||||||
|
# empty — and a gate that skips an unreadable file reports success. This is the fail-closed
|
||||||
|
# promise breaking on its own terms, not a cosmetic quoting complaint.
|
||||||
|
try_commit "café/postgres.env" 'POSTGRES_PASSWORD=hunter2corp2026'
|
||||||
|
[ "$status" -ne 0 ]
|
||||||
|
[[ "$output" == *"postgres.env"* ]]
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "a placeholder word elsewhere on the line does not exempt a live value" {
|
||||||
|
# The rule is about the VALUE. `# example` in a comment says nothing about the token the
|
||||||
|
# line assigns, but a whole-line placeholder filter lets the comment vouch for the secret.
|
||||||
|
try_commit "app.env" 'API_TOKEN=hunter2corp2026 # example, placeholder, changeme'
|
||||||
|
[ "$status" -ne 0 ]
|
||||||
|
[[ "$output" == *"app.env"* ]]
|
||||||
|
}
|
||||||
|
|
||||||
|
@test "each hit is attributed to the file it came from" {
|
||||||
|
# The report is how the next agent finds the leak. If one file's last hit line runs into the
|
||||||
|
# next file's name, the audit starts at a clean file and the leaker keeps its name.
|
||||||
|
mkdir -p "$REPO/a" "$REPO/b"
|
||||||
|
printf 'A_PASSWORD=hunter2corp2026\n' > "$REPO/a/one.env"
|
||||||
|
printf 'B_PASSWORD=hunter2corp2026\n' > "$REPO/b/two.env"
|
||||||
|
run git -C "$REPO" add -A
|
||||||
|
run git -C "$REPO" commit -q -m "two leakers"
|
||||||
|
[ "$status" -ne 0 ]
|
||||||
|
printf '%s\n' "$output" | grep -qx ' a/one.env'
|
||||||
|
printf '%s\n' "$output" | grep -qx ' b/two.env'
|
||||||
|
! printf '%s\n' "$output" | grep -qE 'hunter2corp2026 +b/two\.env'
|
||||||
|
}
|
||||||
Reference in new issue
Block a user