Skip to content

Commit 7dca8f8

Browse files
committed
security: close three input-level evasions of the gate and tripwire
Attacked the inputs rather than the patterns this time. Three worked. ONE INVALID BYTE HID A WHOLE FILE. The scanner abandoned any file that failed to decode as UTF-8, so appending a single 0xFF made it skip every readable line in that file, plaintext payload included. A complete evasion costing one byte. Files are now decoded with replacement rather than abandoned, and a NUL byte -- git's own binary heuristic -- is what marks a file as genuinely not a review surface. Only three tracked files reach that path today: a PNG, a Windows ETW manifest and the nomic blob. Better still, the evasion is now its own signal: a file with a TEXT extension that is not valid UTF-8 is reported, because a stray byte in a .md or .c is anomalous regardless of what surrounds it. THE TRIPWIRE MATCHED CASE-SENSITIVELY. `Scripts/evil.sh` and `.GitHub/workflows/` walked past it, and on a case-insensitive checkout those are the same files as the guarded ones. Now matched with POSIX character classes rather than `${v,,}` (bash 4 only) or `tr` (external), so the logic can be tested on any shell -- which matters for a gate nobody can run locally the way CI runs it. THE FILES ENDPOINT CAPS AT 3000. A pull request padded past that limit would hide a CI change in the tail, and the gate would report green over a change set it never saw. It now compares what the API returned against the count the PR itself declares and REFUSES when they disagree, rather than passing on partial data. All three are pinned in the selftest, alongside the requirement that a PNG with invalid UTF-8 stays silent -- the hardening must not turn every binary into a finding. Verified end to end against real pull requests: #1422 refused (4 guarded paths), #1245 refused (42 guarded, 392/392 received so no truncation), #1778 refused (2 guarded), #1678 passes. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent ddfdea4 commit 7dca8f8

2 files changed

Lines changed: 87 additions & 16 deletions

File tree

‎.github/workflows/ci-integrity.yml‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,48 @@ jobs:
5151
# Paths that define or are executed by CI. Under-covering is the
5252
# dangerous direction: anything CI runs can neuter a gate, so this
5353
# list is deliberately broad rather than precise.
54+
# Matching is CASE-INSENSITIVE. `Scripts/evil.sh` and `.GitHub/`
55+
# slipped past a case-sensitive test, and on a case-insensitive
56+
# checkout they are the same files as the guarded ones anyway.
5457
is_guarded() {
58+
# Character classes rather than a lowercase expansion: `${v,,}`
59+
# needs bash 4, and `tr` is an external command. This form is
60+
# POSIX, depends on nothing, and can be tested on any shell --
61+
# which matters more than elegance for a gate nobody can run
62+
# locally the way CI runs it.
5563
case "$1" in
56-
.github/*) return 0 ;;
57-
scripts/*) return 0 ;;
58-
test-infrastructure/*) return 0 ;;
59-
tests/*.sh) return 0 ;;
60-
*) return 1 ;;
64+
.[Gg][Ii][Tt][Hh][Uu][Bb]/*) return 0 ;;
65+
[Ss][Cc][Rr][Ii][Pp][Tt][Ss]/*) return 0 ;;
66+
[Tt][Ee][Ss][Tt]-[Ii][Nn][Ff][Rr][Aa][Ss][Tt][Rr][Uu][Cc][Tt][Uu][Rr][Ee]/*)
67+
return 0 ;;
68+
[Tt][Ee][Ss][Tt][Ss]/*.[Ss][Hh]) return 0 ;;
69+
*) return 1 ;;
6170
esac
6271
}
6372
6473
changed="$(gh api --paginate \
6574
"repos/$REPO/pulls/$PR_NUMBER/files" \
6675
--jq '.[].filename')"
6776
77+
# FAIL CLOSED ON TRUNCATION. The files endpoint is capped at 3000
78+
# entries, so a pull request padded past that limit could hide a CI
79+
# change in the tail and this gate would never see it. Compare what
80+
# we received against the count the PR itself reports, and refuse
81+
# rather than pass when they disagree.
82+
declared="$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '.changed_files')"
83+
received="$(printf '%s\n' "$changed" | grep -c . || true)"
84+
if [ "$received" -lt "$declared" ]; then
85+
echo "=== CI INTEGRITY GATE: REFUSED ==="
86+
echo
87+
echo "The pull request reports $declared changed files but the API"
88+
echo "returned only $received. GitHub caps that endpoint at 3000"
89+
echo "entries, so part of this change set is invisible to the gate"
90+
echo "and a CI modification could be hiding in the tail."
91+
echo
92+
echo "Split the pull request into reviewable pieces."
93+
exit 1
94+
fi
95+
6896
hits=""
6997
while IFS= read -r path; do
7098
[ -n "$path" ] || continue

‎scripts/security-injection.py‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,41 @@ def obfuscation_findings(rel, text):
372372
)
373373

374374

375+
# Extensions whose contents a human or an agent reads as text. A file with one
376+
# of these that is NOT valid UTF-8 is itself the finding: a single stray byte
377+
# used to make this gate skip the entire file, payload included, which was a
378+
# complete one-byte evasion.
379+
TEXT_EXTENSIONS = frozenset({
380+
".c", ".h", ".cc", ".cpp", ".hpp", ".py", ".sh", ".bash", ".ps1", ".js",
381+
".ts", ".tsx", ".jsx", ".json", ".jsonc", ".yml", ".yaml", ".toml", ".ini",
382+
".cfg", ".md", ".txt", ".rst", ".html", ".css", ".sql", ".nix", ".mk",
383+
".cmake", ".gradle", ".rb", ".go", ".rs", ".java", ".kt", ".swift", ".php",
384+
".pl", ".lua", ".vim", ".el", ".patch", ".diff", ".man", ".xml", ".svg",
385+
})
386+
387+
388+
def decode_for_scan(rel, raw):
389+
"""Return (text, finding) for a file's bytes.
390+
391+
A NUL byte means genuinely binary -- git's own heuristic -- and those are
392+
not a review surface, so they are skipped. Everything else is decoded with
393+
replacement rather than abandoned: refusing to decode a file was itself the
394+
evasion, because one invalid byte hid every readable line around it.
395+
"""
396+
if b"\x00" in raw:
397+
return None, None
398+
try:
399+
return raw.decode("utf-8"), None
400+
except UnicodeDecodeError as exc:
401+
text = raw.decode("utf-8", errors="replace")
402+
if Path(rel).suffix.lower() in TEXT_EXTENSIONS:
403+
return text, (
404+
f"text file is not valid UTF-8 at byte {exc.start} -- a stray "
405+
f"byte makes readers skip content they would otherwise show"
406+
)
407+
return text, None
408+
409+
375410
def scan_tree(root):
376411
"""Walk the tree ONCE, running every tier per file.
377412
@@ -386,23 +421,17 @@ def scan_tree(root):
386421
raw = (root / rel).read_bytes()
387422
except (OSError, ValueError):
388423
continue
389-
text = None
424+
text, decode_finding = decode_for_scan(rel, raw)
425+
if text is None:
426+
continue # NUL bytes: genuinely binary, not a review surface
427+
if decode_finding:
428+
yield "phrase", rel, 1, "", decode_finding
390429
if not raw.isascii():
391-
try:
392-
text = raw.decode("utf-8")
393-
except UnicodeDecodeError:
394-
continue # binary; not a review surface
395430
if any(classify(ord(ch)) for ch in text):
396431
for line_no, line in enumerate(text.split("\n"), 1):
397432
found = [(c, classify(ord(c))) for c in line if classify(ord(c))]
398433
if found:
399434
yield "carrier", rel, line_no, line, found
400-
if text is None:
401-
try:
402-
text = raw.decode("utf-8")
403-
except UnicodeDecodeError:
404-
continue
405-
406435
if rel.startswith("internal/cbm/vendored/grammars/") and rel.endswith("parser.c"):
407436
for m in _LITERAL.finditer(text):
408437
lit = m.group(1)
@@ -603,6 +632,20 @@ def caught(text):
603632
]:
604633
check(not caught(text), f"false positive on {what}")
605634

635+
# DECODE HARDENING. A single invalid byte used to make the gate skip an
636+
# entire file, hiding every readable line around it -- a one-byte evasion.
637+
text, finding = decode_for_scan("notes.md", b"hello \xff world")
638+
check(text is not None, "invalid UTF-8 in a text file must still be scanned")
639+
check(finding is not None, "invalid UTF-8 in a text file must be reported")
640+
check(
641+
"world" in (text or ""),
642+
"content after an invalid byte must remain visible to the scanner",
643+
)
644+
_, png = decode_for_scan("image.png", b"\x89PNG\r\n\xff\xfe")
645+
check(png is None, "invalid UTF-8 in a binary extension must not be reported")
646+
binary, _ = decode_for_scan("blob.bin", b"a\x00b")
647+
check(binary is None, "NUL bytes must be treated as binary and skipped")
648+
606649
# A blessed line is pinned by content: changing it must invalidate the entry.
607650
line = 'x = "a\u200bb";'
608651
other = 'x = "a\u200bc";'

0 commit comments

Comments
 (0)