mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 18:50:18 +01:00
Brain sync scans binary files and split authorization values for secrets (GST-2292) (#4276)
* Brain sync scans binary files and split authorization values for secrets (GST-2292) Git showed a file with a NUL byte or UTF-16 text as "Binary files differ", so the push-time secret scan never saw an AWS key inside it. The sync and the pre-commit hook now diff with --text --no-textconv, the scanner also reads NUL-bearing lines as UTF-16 and with NULs removed, and a section git still did not diff as text is held as binary-not-scanned. A Bearer value added under an unchanged "authorization": line was missed because the + marker broke the match; each hunk's new text is now scanned as one block too. The docs say plainly that an already-committed secret stays held after deletion and only --skip-file clears it. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Secret scan catches split auth values in UTF-16 files and under diff.context=0 Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/red-team#2 reader=red-team Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Sync docs say a secret already committed in ~/.gstack stays held until you skip the file Approved docs-audit repair: approval a-029ea2b533be (GStack Minder GSTA-36, lane brief item 3). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Secret scan keeps the plain git diff as one reading beside the text reading The sync and the pre-commit hooks had replaced git's plain staged diff with a --text --no-textconv -U3 diff. That lost two things the old scan held: an unchanged line that the user's diff.context showed (5 lines away from an edit), and a secret that only a configured textconv driver decodes. The scanner now takes both diffs and holds a file when either reading matches; binary-not-scanned applies to the text reading only. Review-Finding: sol-audit-201c6a0#1 reader=cross-family-audit Review-Finding: sol-audit-201c6a0#2 reader=cross-family-audit Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Secret scan decodes JSON escapes in UTF-16 text and keeps its diff files private A \u-escaped AWS key inside a UTF-16 JSON file passed both scans, because the UTF-16 and NUL-stripped readings skipped the JSON decoding every other reading gets. The sync's two scan diffs, which now carry binary bytes, were written with the user's umask (0644 under /tmp); they are created 0600. Review-Finding: p3/outside-voice#1 reader=outside-voice Review-Finding: p3/outside-voice#2 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
d5f7ab5f20
commit
90fded4dd0
8 files changed
+362
-37
No files matched your search
@@ -431,7 +431,13 @@ if [ ! -f "$SCAN" ]; then
|
||||
echo "gstack-artifacts pre-commit: refusing commit — the secret scanner $SCAN is missing; rerun gstack-artifacts-init to reinstall this hook." >&2
|
||||
exit 1
|
||||
fi
|
||||
hits=$(git diff --cached --no-renames --no-color --no-ext-diff | python3 "$SCAN" gate -)
|
||||
TEXT=$(mktemp) || { echo "gstack-artifacts pre-commit: refusing commit — could not create a temporary file for the secret scan." >&2; exit 1; }
|
||||
trap 'rm -f "$TEXT"' EXIT
|
||||
if ! git diff --cached --no-renames --no-color --no-ext-diff --text --no-textconv -U3 > "$TEXT"; then
|
||||
echo "gstack-artifacts pre-commit: refusing commit — git could not read the staged diff for the secret scan." >&2
|
||||
exit 1
|
||||
fi
|
||||
hits=$(git diff --cached --no-renames --no-color --no-ext-diff | python3 "$SCAN" gate - "$TEXT")
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] && exit 0
|
||||
if [ "$rc" -eq 3 ]; then
|
||||
|
||||
@@ -216,7 +216,13 @@ if [ ! -f "$SCAN" ]; then
|
||||
echo "gstack-brain pre-commit: refusing commit — the secret scanner $SCAN is missing; rerun gstack-brain-restore to reinstall this hook." >&2
|
||||
exit 1
|
||||
fi
|
||||
hits=$(git diff --cached --no-renames --no-color --no-ext-diff | python3 "$SCAN" gate -)
|
||||
TEXT=$(mktemp) || { echo "gstack-brain pre-commit: refusing commit — could not create a temporary file for the secret scan." >&2; exit 1; }
|
||||
trap 'rm -f "$TEXT"' EXIT
|
||||
if ! git diff --cached --no-renames --no-color --no-ext-diff --text --no-textconv -U3 > "$TEXT"; then
|
||||
echo "gstack-brain pre-commit: refusing commit — git could not read the staged diff for the secret scan." >&2
|
||||
exit 1
|
||||
fi
|
||||
hits=$(git diff --cached --no-renames --no-color --no-ext-diff | python3 "$SCAN" gate - "$TEXT")
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] && exit 0
|
||||
if [ "$rc" -eq 3 ]; then
|
||||
|
||||
@@ -6,6 +6,20 @@ The whole diff is scanned as written, headers and context lines included, so
|
||||
a secret in a file name or one that lines up across an unchanged line is
|
||||
held. Each added line is also scanned with its JSON string literals decoded,
|
||||
so a secret hidden behind \u escapes or backslash-escaped quotes is caught.
|
||||
Each hunk's new text (its unchanged and added lines without their diff
|
||||
markers) is scanned as one block too, so a key on an unchanged line joins a
|
||||
value added under it.
|
||||
|
||||
Callers pass two readings of the same staged diff and every check runs on
|
||||
both, so neither can lose what the other holds. The first is the plain
|
||||
`git diff` the hook has always scanned, with the user's diff.context and
|
||||
textconv drivers. The second is the same diff with --text, --no-textconv and
|
||||
-U3, so a file git calls binary arrives as its bytes and a diff.context below
|
||||
3 cannot drop the unchanged lines around an edit. A line or hunk holding a
|
||||
NUL byte is also read as UTF-16 in both byte orders and alignments, and with
|
||||
its NULs removed. A section of the second reading that still says "Binary
|
||||
files ... differ" or carries a "GIT binary patch" was not diffed as text; it
|
||||
is flagged as binary-not-scanned (fail closed).
|
||||
|
||||
The first six pattern families are the original hook's; the four after them
|
||||
mirror lib/redact-patterns.ts HIGH rules (stripe.secret/restricted,
|
||||
@@ -15,10 +29,11 @@ here. Only the family name ever leaves this script, never the matched text.
|
||||
The diff is read as bytes and decoded with replacement, so undecodable bytes
|
||||
cannot crash the scan into a false "clean".
|
||||
|
||||
gate <diff|-> exit 0 clean, 3 hit (prints the family
|
||||
gate <diff|-> [<text-diff>] exit 0 clean, 3 hit (prints the family
|
||||
names); anything else is a scanner
|
||||
failure (fail closed). "-" reads stdin.
|
||||
attribute <diff> <names> <out> one pass: splits the diff on its
|
||||
attribute <diff> <names> <out> [<text-diff>]
|
||||
one pass: splits each diff on its
|
||||
"diff --git" headers, pairs each section
|
||||
with the NUL-separated names from
|
||||
"git diff --name-only -z" (same order),
|
||||
@@ -113,6 +128,50 @@ def added_lines(lines):
|
||||
return out
|
||||
|
||||
|
||||
def post_images(lines):
|
||||
"""Each hunk's new text as bytes: its unchanged and added lines, markers
|
||||
removed, joined on the newline byte git split them at."""
|
||||
out, cur, in_hunk = [], [], False
|
||||
for line in lines:
|
||||
if line.startswith(b"diff --git ") or line.startswith(b"@@"):
|
||||
if cur:
|
||||
out.append(b"\n".join(cur))
|
||||
cur, in_hunk = [], line.startswith(b"@@")
|
||||
elif in_hunk and line[:1] in (b" ", b"+"):
|
||||
cur.append(line[1:])
|
||||
if cur:
|
||||
out.append(b"\n".join(cur))
|
||||
return out
|
||||
|
||||
|
||||
def nul_readings(bodies):
|
||||
"""Each byte string holding a NUL byte, read with its NULs removed and as
|
||||
UTF-16LE and UTF-16BE from both byte alignments."""
|
||||
out = []
|
||||
for body in bodies:
|
||||
if b"\0" not in body:
|
||||
continue
|
||||
out.append(body.replace(b"\0", b"").decode("utf-8", "replace"))
|
||||
for chunk in (body, body[1:]):
|
||||
chunk = chunk[:len(chunk) // 2 * 2]
|
||||
out.append(chunk.decode("utf-16-le", "replace"))
|
||||
out.append(chunk.decode("utf-16-be", "replace"))
|
||||
return out
|
||||
|
||||
|
||||
def unscanned_binary(lines):
|
||||
"""True when a section's header says git did not diff a file as text."""
|
||||
in_hunk = False
|
||||
for line in lines:
|
||||
if line.startswith(b"diff --git "):
|
||||
in_hunk = False
|
||||
elif line.startswith(b"@@"):
|
||||
in_hunk = True
|
||||
elif not in_hunk and (line.startswith(b"Binary files ") or line == b"GIT binary patch"):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def decoded_in_place(text):
|
||||
"""text with every escaped JSON string literal replaced by its decoded
|
||||
value in quotes, so a key and its value are matched together even when
|
||||
@@ -127,13 +186,19 @@ def decoded_in_place(text):
|
||||
return json_string.sub(decode, text)
|
||||
|
||||
|
||||
def families(lines):
|
||||
def families(lines, text_reading=False):
|
||||
added = added_lines(lines)
|
||||
raw = b"\n".join(lines).decode("utf-8", "replace")
|
||||
texts = [raw] + [decoded_in_place(line) for line in added] + [s for line in added for s in decoded_strings(line)]
|
||||
posts = post_images(lines)
|
||||
post_texts = [post.decode("utf-8", "replace") for post in posts]
|
||||
texts = ([raw] + [decoded_in_place(line) for line in added] + [s for line in added for s in decoded_strings(line)]
|
||||
+ post_texts + [decoded_in_place(text) for text in post_texts]
|
||||
+ [t for nul in nul_readings([line[1:] for line in lines] + posts)
|
||||
for t in [nul, decoded_in_place(nul)] + decoded_strings(nul)])
|
||||
text = "\n".join(texts)
|
||||
return [name for name, rx, real in patterns
|
||||
if any(real is None or real(m) for m in rx.finditer(text))]
|
||||
found = ['binary-not-scanned'] if text_reading and unscanned_binary(lines) else []
|
||||
return found + [name for name, rx, real in patterns
|
||||
if any(real is None or real(m) for m in rx.finditer(text))]
|
||||
|
||||
|
||||
def read(path):
|
||||
@@ -143,9 +208,27 @@ def read(path):
|
||||
return f.read()
|
||||
|
||||
|
||||
def union(plain, text):
|
||||
hits = families(plain)
|
||||
if text is not None:
|
||||
hits += [h for h in families(text, text_reading=True) if h not in hits]
|
||||
return hits
|
||||
|
||||
|
||||
def sections(path):
|
||||
out = []
|
||||
for line in read(path).split(b"\n"):
|
||||
if line.startswith(b"diff --git "):
|
||||
out.append([line])
|
||||
elif out:
|
||||
out[-1].append(line)
|
||||
return out
|
||||
|
||||
|
||||
mode = sys.argv[1]
|
||||
if mode == "gate":
|
||||
hits = families(read(sys.argv[2]).split(b"\n"))
|
||||
text = read(sys.argv[3]).split(b"\n") if len(sys.argv) > 3 else None
|
||||
hits = union(read(sys.argv[2]).split(b"\n"), text)
|
||||
if hits:
|
||||
print(",".join(hits))
|
||||
sys.exit(3)
|
||||
@@ -155,18 +238,14 @@ if mode != "attribute":
|
||||
|
||||
diff_path, names_path, out_path = sys.argv[2:5]
|
||||
names = [n.decode("utf-8", "surrogateescape") for n in read(names_path).split(b"\0") if n]
|
||||
sections = []
|
||||
for line in read(diff_path).split(b"\n"):
|
||||
if line.startswith(b"diff --git "):
|
||||
sections.append([line])
|
||||
elif sections:
|
||||
sections[-1].append(line)
|
||||
if len(sections) != len(names):
|
||||
plain = sections(diff_path)
|
||||
text = sections(sys.argv[5]) if len(sys.argv) > 5 else [None] * len(plain)
|
||||
if len(plain) != len(names) or len(text) != len(names):
|
||||
sys.exit(4)
|
||||
with open(out_path, "w", encoding="utf-8", errors="surrogateescape", newline="\n") as out:
|
||||
for name, body in zip(names, sections):
|
||||
for name, body, text_body in zip(names, plain, text):
|
||||
if "\n" in name or "\t" in name:
|
||||
sys.exit(4)
|
||||
hits = families(body)
|
||||
hits = union(body, text_body)
|
||||
if hits:
|
||||
out.write(hits[0] + "\t" + name + "\n")
|
||||
+15
-7
@@ -206,12 +206,17 @@ sync_active() {
|
||||
|
||||
# Secret scan over a staged diff file: bin/gstack-brain-secret-scan.py, the
|
||||
# same scanner the artifacts repo's pre-commit hook runs. It scans the whole
|
||||
# diff as written and each added line with its JSON string literals decoded,
|
||||
# and prints only family names, never the matched text.
|
||||
# secret_scan gate <diff> exit 0 clean, 3 hit (prints the
|
||||
# diff as written, each added line with its JSON string literals decoded and
|
||||
# each hunk's new text, and prints only family names, never the matched text.
|
||||
# It reads two diffs of the same index: the plain diff (the user's
|
||||
# diff.context and textconv drivers apply) and one with --text --no-textconv
|
||||
# -U3, so binary files arrive as bytes and a small diff.context cannot hide
|
||||
# the unchanged lines. Every finding from either reading counts.
|
||||
# secret_scan gate <diff> [<text-diff>] exit 0 clean, 3 hit (prints the
|
||||
# family names); anything else is
|
||||
# a scanner failure (fail closed)
|
||||
# secret_scan attribute <diff> <names> <out> writes "<family>\t<path>" per
|
||||
# secret_scan attribute <diff> <names> <out> [<text-diff>]
|
||||
# writes "<family>\t<path>" per
|
||||
# flagged path; exit 0, or 4 when
|
||||
# the diff's sections and the
|
||||
# `git diff --name-only -z` names
|
||||
@@ -1038,9 +1043,11 @@ hold_flagged_paths() {
|
||||
WAITING=0
|
||||
: > "$scan.hits"
|
||||
: > "$scan.blobs"
|
||||
(umask 077 && : > "$scan.diff" && : > "$scan.text") || rc=1
|
||||
git -C "$GSTACK_HOME" diff --cached --no-renames --no-color --no-ext-diff > "$scan.diff" 2>/dev/null || rc=1
|
||||
git -C "$GSTACK_HOME" diff --cached --no-renames --no-color --no-ext-diff --text --no-textconv -U3 > "$scan.text" 2>/dev/null || rc=1
|
||||
git -C "$GSTACK_HOME" diff --cached --no-renames -z --name-only > "$scan.names" 2>/dev/null || rc=1
|
||||
[ "$rc" -eq 0 ] && { secret_scan attribute "$scan.diff" "$scan.names" "$scan.hits" || rc=1; }
|
||||
[ "$rc" -eq 0 ] && { secret_scan attribute "$scan.diff" "$scan.names" "$scan.hits" "$scan.text" || rc=1; }
|
||||
[ "$rc" -eq 0 ] && { append_invalid_json_hits "$scan.names" "$scan.hits" || rc=1; }
|
||||
# A successful add need not stage the requested spelling or current bytes
|
||||
# (case-insensitive indexes and assume-unchanged can both report success).
|
||||
@@ -1076,11 +1083,12 @@ hold_flagged_paths() {
|
||||
{ grep -vxF -f "$scan.unstage" "$paths_file" || true; } > "$paths_file.kept"
|
||||
mv -f "$paths_file.kept" "$paths_file"
|
||||
fi
|
||||
if ! git -C "$GSTACK_HOME" diff --cached --no-renames --no-color --no-ext-diff > "$scan.diff" 2>/dev/null; then
|
||||
if ! git -C "$GSTACK_HOME" diff --cached --no-renames --no-color --no-ext-diff > "$scan.diff" 2>/dev/null \
|
||||
|| ! git -C "$GSTACK_HOME" diff --cached --no-renames --no-color --no-ext-diff --text --no-textconv -U3 > "$scan.text" 2>/dev/null; then
|
||||
block_drain "git could not read the staged diff for the re-scan"
|
||||
return 1
|
||||
fi
|
||||
gate_out=$(secret_scan gate "$scan.diff") || gate_rc=$?
|
||||
gate_out=$(secret_scan gate "$scan.diff" "$scan.text") || gate_rc=$?
|
||||
[ "$gate_rc" -eq 0 ] && return 0
|
||||
if [ "$gate_rc" -eq 3 ]; then
|
||||
block_drain "a re-scan after holding back the flagged files still matched ($gate_out)"
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
important: security
|
||||
---
|
||||
- **Brain sync now scans binary files and catches an auth value added under an existing key.** Git shows a file with a NUL byte (or UTF-16 text) as "Binary files differ" instead of its contents, so the push-time secret scan never saw it and an AWS key inside such a note was pushed. The scan now diffs every file as text and also reads lines and edited blocks that hold NUL bytes as UTF-16 and with the NULs removed; if git still reports a file as binary instead of showing its bytes, the file is held as `binary-not-scanned`. A Bearer value added on the line under an unchanged `"authorization":` key also slipped through, because the diff's `+` marker broke the match; each edited block is now also scanned as the new file text, in UTF-16 files too, and a second reading of the diff always shows three unchanged lines around each edit so a `diff.context` setting of 0 cannot hide the key. The diff git normally shows, with your `diff.context` and any textconv driver, is still scanned too, so nothing the old scan held is lost. The pre-commit hook that `gstack-artifacts-init` and `gstack-brain-restore` install gets the same fix; rerun `gstack-artifacts-init` (or `gstack-brain-restore` on a machine you restored) to refresh it. docs/gbrain-sync.md now says plainly that a secret already committed in `~/.gstack` stays held after you delete it, and only `--skip-file` clears it.
|
||||
@@ -55,13 +55,17 @@ from it.
|
||||
**Held files.** When the secret scan flags a file, only that file is held
|
||||
back (with any file coupled to it, such as a decision log and its active
|
||||
snapshot); everything else syncs. The held file stays queued and is
|
||||
re-scanned on every sync, so editing it clears the hold automatically.
|
||||
re-scanned on every sync, so removing a secret that was never synced clears
|
||||
the hold automatically. A secret already committed in `~/.gstack` stays on a
|
||||
removed line of the next diff, so that file stays held until you skip it.
|
||||
`--status` lists each held file under `held`, with the scanner rule that
|
||||
matched (never the matched text), the files held with it (`dependents`) and
|
||||
both fixes:
|
||||
|
||||
1. **If it's a real secret**: edit the file to remove it. The next skill run
|
||||
re-scans and syncs it.
|
||||
1. **If it's a real secret**: edit the file to remove it. If that line was
|
||||
never synced, the next skill run re-scans and syncs it. If it is already
|
||||
committed in `~/.gstack`, skip the file (fix 2) and, if it was pushed,
|
||||
treat the secret as exposed.
|
||||
2. **If the match is a false positive** you never want synced:
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-brain-sync --skip-file <path>
|
||||
|
||||
+25
-7
@@ -160,12 +160,27 @@ The last four follow gstack's redaction rules, so placeholders such as
|
||||
`sk_live_EXAMPLE…`, `${DB_PASSWORD}` or `postgres:postgres@localhost` are
|
||||
not held.
|
||||
|
||||
The scan reads the whole staged diff, including file names and the unchanged
|
||||
lines shown around each edit. Each added line is also scanned with its JSON
|
||||
strings decoded, so a secret written with `\u` escapes or inside an escaped
|
||||
JSON string is still caught. Because unchanged lines are scanned too, a file
|
||||
that already holds a flagged line is held on every edit until you remove the
|
||||
line or skip the file.
|
||||
The scan reads the whole staged diff twice: once as `git diff` shows it to
|
||||
you (your `diff.context` and any textconv driver apply), and once as raw text
|
||||
with three unchanged lines around each edit. A match in either reading holds
|
||||
the file. Both readings include file names. Each added line is also
|
||||
scanned with its JSON strings decoded, so a secret written with `\u` escapes or inside an escaped
|
||||
JSON string is still caught. Each edited block is also read as the new file
|
||||
text, so a key on an unchanged line and a value added under it are matched
|
||||
together, whatever your git `diff.context` setting is. Files git treats as
|
||||
binary (a NUL byte, UTF-16 text) are scanned by their bytes, read plainly,
|
||||
without NULs and as UTF-16. If git still reports a file as binary instead of
|
||||
showing its bytes, the file is held as `binary-not-scanned`. Bytes that none
|
||||
of these readings turn into text, such as a compressed file, are not held
|
||||
unless a pattern matches them.
|
||||
|
||||
Because unchanged and removed lines are scanned too, a file that already holds
|
||||
a flagged line is held on every edit while that line is in the diff. If the
|
||||
flagged line was never synced, delete it and the next sync goes through. If it
|
||||
is already committed in `~/.gstack`, deleting it leaves it on a removed line
|
||||
of the next diff, so the file stays held and only `--skip-file` clears it.
|
||||
Skipping stops syncing the file, so a copy that was already pushed stays on
|
||||
the remote; if it was pushed, treat the secret as exposed.
|
||||
|
||||
If a scan hits, only the flagged file is held back (with any file coupled
|
||||
to it); everything else syncs. The held file stays queued and is re-scanned
|
||||
@@ -182,7 +197,10 @@ To remediate:
|
||||
2. If the match is a false positive on content you never want synced, run
|
||||
`gstack-brain-sync --skip-file <path>` to permanently exclude that path;
|
||||
`gstack-brain-sync --unskip-file <path>` undoes it.
|
||||
3. Otherwise, edit the file to remove the secret; the next skill run syncs it.
|
||||
3. Otherwise, edit the file to remove the secret. If that line was never
|
||||
synced, the next skill run syncs the file. If it is already committed in
|
||||
`~/.gstack`, the file stays held and only `--skip-file` clears it; if it
|
||||
was pushed, treat the secret as exposed.
|
||||
|
||||
There's a defense-in-depth hook at `~/.gstack/.git/hooks/pre-commit` that
|
||||
runs the same scanner (`bin/gstack-brain-secret-scan.py`) if you manually
|
||||
|
||||
+202
-2
@@ -312,6 +312,70 @@ describe('init + sync + restore round-trip', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the pre-commit hook init and restore write refuses an AWS key in a binary file (GST-2292)', () => {
|
||||
const body = Buffer.from('\0' + 'AKIA' + 'ABCDEFGHIJKLMNOP' + '\n', 'latin1');
|
||||
const commitRefused = (home: string) => {
|
||||
fs.mkdirSync(path.join(home, 'projects/p/ceo-plans'), { recursive: true });
|
||||
fs.writeFileSync(path.join(home, 'projects/p/ceo-plans/note.md'), body);
|
||||
git(['add', '-f', 'projects/p/ceo-plans/note.md'], home);
|
||||
const r = git(['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', 'commit', '-q', '-m', 'manual commit'], home);
|
||||
expect(r.status).not.toBe(0);
|
||||
expect(r.stderr).toContain('aws-access-key');
|
||||
};
|
||||
expect(run(['gstack-artifacts-init', '--remote', bareRemote]).status).toBe(0);
|
||||
commitRefused(tmpHome);
|
||||
const machineB = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-machineB-'));
|
||||
try {
|
||||
expect(run(['gstack-brain-restore', bareRemote], { env: { GSTACK_HOME: machineB } }).status).toBe(0);
|
||||
commitRefused(machineB);
|
||||
} finally {
|
||||
fs.rmSync(machineB, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('the scanner flags a binary section it was not given the bytes of', () => {
|
||||
const textDiff = path.join(tmpHome, 'text.diff');
|
||||
const scan = (diff: string) => {
|
||||
fs.writeFileSync(textDiff, diff);
|
||||
return spawnSync('python3', [path.join(BIN, 'gstack-brain-secret-scan.py'), 'gate', '-', textDiff], { input: diff.replace(/^Binary files .*$|^GIT binary patch$/gm, ''), encoding: 'utf-8', timeout: 30_000 });
|
||||
};
|
||||
const header = 'diff --git a/n.md b/n.md\nnew file mode 100644\nindex 0000000..1111111\n';
|
||||
for (const tail of ['Binary files /dev/null and b/n.md differ\n', 'GIT binary patch\nliteral 3\nKcmZ?wFaQ7m\n\n']) {
|
||||
const r = scan(header + tail);
|
||||
expect(r.status).toBe(3);
|
||||
expect(r.stdout.trim()).toBe('binary-not-scanned');
|
||||
}
|
||||
const text = scan(header + '--- /dev/null\n+++ b/n.md\n@@ -0,0 +1 @@\n+Binary files are mentioned here\n');
|
||||
expect(text.status).toBe(0);
|
||||
});
|
||||
|
||||
// GST-2292 audit: the hook keeps scanning the plain diff main scanned (the
|
||||
// user's diff.context and textconv drivers apply) beside the --text reading.
|
||||
for (const [name, setup, body] of [
|
||||
['an AWS key on a context line that diff.context 5 shows', (home: string) => {
|
||||
fs.writeFileSync(path.join(home, 'projects/p/ceo-plans/note.md'), ' AKIA' + 'ABCDEFGHIJKLMNOP\n1\n2\n3\nbefore\n');
|
||||
git(['add', '-f', 'projects/p/ceo-plans/note.md'], home);
|
||||
expect(git(['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', 'commit', '-q', '--no-verify', '-m', 'baseline'], home).status).toBe(0);
|
||||
git(['config', 'diff.context', '5'], home);
|
||||
}, ' AKIA' + 'ABCDEFGHIJKLMNOP\n1\n2\n3\nafter\n'],
|
||||
['an AWS key that only a textconv driver decodes', (home: string) => {
|
||||
fs.writeFileSync(path.join(home, '.git/info/attributes'), 'projects/p/ceo-plans/note.md diff=audit\n');
|
||||
git(['config', 'diff.audit.textconv', 'base64 -d'], home);
|
||||
}, Buffer.from('AKIA' + 'ABCDEFGHIJKLMNOP\n').toString('base64') + '\n'],
|
||||
] as [string, (home: string) => void, string][]) {
|
||||
test(`the pre-commit hook refuses ${name}`, () => {
|
||||
expect(run(['gstack-artifacts-init', '--remote', bareRemote]).status).toBe(0);
|
||||
fs.mkdirSync(path.join(tmpHome, 'projects/p/ceo-plans'), { recursive: true });
|
||||
fs.mkdirSync(path.join(tmpHome, '.git/info'), { recursive: true });
|
||||
setup(tmpHome);
|
||||
fs.writeFileSync(path.join(tmpHome, 'projects/p/ceo-plans/note.md'), body);
|
||||
git(['add', '-f', 'projects/p/ceo-plans/note.md']);
|
||||
const r = git(['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', 'commit', '-q', '-m', 'manual commit']);
|
||||
expect(r.status).not.toBe(0);
|
||||
expect(r.stderr).toContain('aws-access-key');
|
||||
});
|
||||
}
|
||||
|
||||
test('the pre-commit hook fails closed when its scanner is missing or fails', () => {
|
||||
expect(run(['gstack-artifacts-init', '--remote', bareRemote]).status).toBe(0);
|
||||
const hook = path.join(tmpHome, '.git/hooks/pre-commit');
|
||||
@@ -527,6 +591,141 @@ describe('gstack-brain-sync secret scan', () => {
|
||||
expect(statusJson().held.map((h: any) => h.path)).toEqual([rel]);
|
||||
});
|
||||
|
||||
// GST-2292: git prints "Binary files ... differ" for a file with a NUL byte
|
||||
// instead of its bytes, so the scan has to read the bytes some other way.
|
||||
function syncFile(rel: string, body: string | Buffer, env?: Record<string, string>) {
|
||||
fs.mkdirSync(path.dirname(path.join(tmpHome, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(tmpHome, rel), body);
|
||||
run(['gstack-brain-enqueue', rel]);
|
||||
expect(run(['gstack-brain-sync', '--once'], { env }).status).toBe(0);
|
||||
return JSON.parse(fs.readFileSync(path.join(tmpHome, '.brain-sync-status.json'), 'utf-8'));
|
||||
}
|
||||
const remoteFiles = () =>
|
||||
spawnSync('git', ['--git-dir=' + bareRemote, 'ls-tree', '-r', '--name-only', 'main'], { encoding: 'utf-8', timeout: 30_000 }).stdout;
|
||||
const AWS_KEY = 'AKIA' + 'ABCDEFGHIJKLMNOP';
|
||||
const BINARY_CASES: [string, Buffer][] = [
|
||||
['a NUL byte', Buffer.from('\0' + AWS_KEY + '\n', 'latin1')],
|
||||
['UTF-16LE text', Buffer.from('note ' + AWS_KEY + '\n', 'utf16le')],
|
||||
['UTF-16BE text', Buffer.from('note ' + AWS_KEY + '\n', 'utf16le').swap16()],
|
||||
];
|
||||
for (const [name, body] of BINARY_CASES) {
|
||||
test(`an AWS key in a file git calls binary (${name}) is held`, () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/note.md';
|
||||
const status = syncFile(rel, body);
|
||||
expect(status.status).toBe('held');
|
||||
expect(status.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
expect(remoteFiles()).not.toContain(rel);
|
||||
});
|
||||
}
|
||||
|
||||
test('an AWS key added to a clean binary file that already synced is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/note.md';
|
||||
const clean = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0x0d, 0x49, 0x48, 0x44, 0x52, 0xff, 0x00, 0x10]);
|
||||
const first = syncFile(rel, clean);
|
||||
expect(first.held_count ?? 0).toBe(0);
|
||||
expect(remoteFiles()).toContain(rel);
|
||||
const second = syncFile(rel, Buffer.concat([clean, Buffer.from('\n' + AWS_KEY + '\0', 'latin1')]));
|
||||
expect(second.status).toBe('held');
|
||||
expect(second.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
const shown = spawnSync('git', ['--git-dir=' + bareRemote, 'show', 'main:' + rel], { timeout: 30_000 }).stdout as Buffer;
|
||||
expect(shown.toString('latin1')).not.toContain(AWS_KEY);
|
||||
});
|
||||
|
||||
test('a Bearer value added under an unchanged authorization key is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/headers.md';
|
||||
const first = syncFile(rel, '{\n "authorization":\n}\n');
|
||||
expect(first.held_count ?? 0).toBe(0);
|
||||
const value = ' "Bearer ' + 'abcdef1234567890' + 'abcdef1234567890"\n';
|
||||
const second = syncFile(rel, '{\n "authorization":\n' + value + '}\n');
|
||||
expect(second.status).toBe('held');
|
||||
expect(second.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
});
|
||||
|
||||
test('a Bearer value added under an unchanged authorization key in UTF-16 text is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/headers.md';
|
||||
const first = syncFile(rel, Buffer.from('{\n "authorization":\n}\n', 'utf16le'));
|
||||
expect(first.held_count ?? 0).toBe(0);
|
||||
const value = ' "Bearer ' + 'abcdef1234567890' + 'abcdef1234567890"\n';
|
||||
const second = syncFile(rel, Buffer.from('{\n "authorization":\n' + value + '}\n', 'utf16le'));
|
||||
expect(second.status).toBe('held');
|
||||
expect(second.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
});
|
||||
|
||||
test('a Bearer value added under an unchanged authorization key is held when git diff.context is 0', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const env = { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.context', GIT_CONFIG_VALUE_0: '0' };
|
||||
const rel = 'projects/p/ceo-plans/headers.md';
|
||||
const first = syncFile(rel, '{\n "authorization":\n}\n', env);
|
||||
expect(first.held_count ?? 0).toBe(0);
|
||||
const value = ' "Bearer ' + 'abcdef1234567890' + 'abcdef1234567890"\n';
|
||||
const second = syncFile(rel, '{\n "authorization":\n' + value + '}\n', env);
|
||||
expect(second.status).toBe('held');
|
||||
expect(second.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
});
|
||||
|
||||
// GST-2292 audit: the scan keeps main's plain diff as one reading, so a
|
||||
// larger diff.context or a textconv driver still shows it what main saw.
|
||||
test('an AWS key on an unchanged line that the user\'s diff.context 5 shows is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/note.md';
|
||||
fs.mkdirSync(path.join(tmpHome, 'projects/p/ceo-plans'), { recursive: true });
|
||||
fs.writeFileSync(path.join(tmpHome, rel), ' ' + AWS_KEY + '\n1\n2\n3\nbefore\n');
|
||||
git(['add', '-f', rel]);
|
||||
expect(git(['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', 'commit', '-q', '--no-verify', '-m', 'baseline']).status).toBe(0);
|
||||
git(['config', 'diff.context', '5']);
|
||||
const status = syncFile(rel, ' ' + AWS_KEY + '\n1\n2\n3\nafter\n');
|
||||
expect(status.status).toBe('held');
|
||||
expect(status.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
});
|
||||
|
||||
test('a \\u-escaped AWS key in a UTF-16 JSON file is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/note.md';
|
||||
const status = syncFile(rel, Buffer.from('{"k":"\\u0041KIA' + 'ABCDEFGHIJKLMNOP"}\n', 'utf16le'));
|
||||
expect(status.status).toBe('held');
|
||||
expect(status.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
expect(remoteFiles()).not.toContain(rel);
|
||||
});
|
||||
|
||||
test('the scan files that hold staged bytes are private to the user', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const shimDir = path.join(tmpHome, 'shim');
|
||||
const modes = path.join(tmpHome, 'scan-modes.txt');
|
||||
fs.mkdirSync(shimDir, { recursive: true });
|
||||
const realGit = spawnSync('sh', ['-c', 'command -v git'], { encoding: 'utf-8', timeout: 30_000 }).stdout.trim();
|
||||
fs.writeFileSync(path.join(shimDir, 'git'), `#!/bin/sh\n"${realGit}" "$@"\nrc=$?\nfor f in "${tmpHome}"/brain-sync-paths.*.scan.diff "${tmpHome}"/brain-sync-paths.*.scan.text; do [ -f "$f" ] && ls -ln "$f" | cut -c1-10 >> '${modes}'; done\nexit $rc\n`, { mode: 0o755 });
|
||||
const status = syncFile('projects/p/ceo-plans/note.md', '# a plan\n', { PATH: shimDir + path.delimiter + process.env.PATH, TMPDIR: tmpHome });
|
||||
expect(status.held_count ?? 0).toBe(0);
|
||||
const seen = fs.readFileSync(modes, 'utf-8').split('\n').filter(Boolean);
|
||||
expect(seen.length).toBeGreaterThan(0);
|
||||
expect([...new Set(seen)]).toEqual(['-rw-------']);
|
||||
});
|
||||
|
||||
test('an AWS key that only a configured textconv driver decodes is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
const rel = 'projects/p/ceo-plans/note.md';
|
||||
fs.mkdirSync(path.join(tmpHome, '.git/info'), { recursive: true });
|
||||
fs.writeFileSync(path.join(tmpHome, '.git/info/attributes'), rel + ' diff=audit\n');
|
||||
git(['config', 'diff.audit.textconv', 'base64 -d']);
|
||||
const status = syncFile(rel, Buffer.from(AWS_KEY + '\n').toString('base64') + '\n');
|
||||
expect(status.status).toBe('held');
|
||||
expect(status.held.map((h: any) => h.path)).toEqual([rel]);
|
||||
expect(remoteFiles()).not.toContain(rel);
|
||||
});
|
||||
|
||||
test('a clean file whose name is an AWS access key is held', () => {
|
||||
run(['gstack-artifacts-init', '--remote', bareRemote]);
|
||||
run(['gstack-config', 'set', 'artifacts_sync_mode', 'full']);
|
||||
@@ -760,8 +959,9 @@ describe('gstack-brain-sync per-file hold', () => {
|
||||
expect(remoteLog()).toMatch(/sync: 997 file/);
|
||||
const calls = shim.calls();
|
||||
expect(calls.filter((c) => / add /.test(` ${c} `)).length).toBe(1);
|
||||
// attribution diff + name list + post-hold re-scan diff + empty-diff check
|
||||
expect(calls.filter((c) => c.includes('diff --cached')).length).toBeLessThanOrEqual(4);
|
||||
// attribution diffs (plain and --text, GST-2292) + name list + post-hold
|
||||
// re-scan diffs (plain and --text) + empty-diff check
|
||||
expect(calls.filter((c) => c.includes('diff --cached')).length).toBeLessThanOrEqual(6);
|
||||
expect(spoolFiles()).toHaveLength(3);
|
||||
}, 120_000);
|
||||
|
||||
|
||||
Reference in new issue
Block a user