From 043f235649870daab433fba02959238d8fb9a95c Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 21 Aug 2026 14:30:15 +0200 Subject: [PATCH 01/10] security: add a hidden-instruction gate and refuse PR changes to CI Two related holes, one of which makes the other enforceable. LAYER 0 -- hidden-instruction audit. Indirect prompt injection needs a carrier, and the carrier is invisible Unicode: zero-width sequences, bidirectional overrides, the Unicode Tags block, stray byte-order marks. Those are invisible in an editor and in a diff, but a model reading the file consumes them. Published work has demonstrated agent platforms executing shell commands from instructions hidden this way inside skill markdown -- which is an artifact class this project generates and writes into auto-load directories. scripts/security-injection.py scans every tracked file (2051 files, 1.33 GB) in under seven seconds and refuses any carrier codepoint. It does NOT match on wording. The persuasion half of an injection is natural language, so it is unbounded and translatable -- refusal rates fall from roughly 79% in English to as low as 23% in some low-resource languages, and a homoglyph defeats a keyword list outright. A word list cannot gate that honestly. The carrier half is finite, language- independent, and has no legitimate use in source, so it gates with a false-positive rate near zero. This catches HIDING, not persuasion; a green result means nothing is concealed from the reviewer, which is what makes ordinary human review trustworthy. The allowlist ships EMPTY. The two zero-width spaces in php_lsp.c -- used to write a comment terminator inside a comment -- are rephrased rather than blessed, because zero entries is a materially stronger position than one: the first exception anyone adds becomes a visible event instead of joining a list. Entries pin the sha256 of a single LINE, located by content rather than by number, so unrelated edits never churn them but editing blessed text invalidates it. `--update` emits a placeholder the gate rejects, so an exception cannot be produced mechanically. CI INTEGRITY -- the circularity problem. Every other check here is defined by files inside the pull request, and `pull_request` runs the merge commit's definition. A hostile PR does not need to defeat a gate; it edits the gate, or deletes the step that calls it, and CI reports green over the change that disabled it. `ci-ok` is no help because its own `needs:` list lives in the PR too. ci-integrity.yml closes that with `pull_request_target`, so its definition comes from the base branch and the PR cannot edit its effect on itself. It never checks out or executes pull-request content -- it asks the API for the changed-path list and compares strings. Paths under .github/, scripts/, test-infrastructure/ and tests/*.sh are refused; CI changes land through break-glass instead. Makefile.cbm is deliberately not guarded: the gates are invoked directly from workflow YAML rather than through a make target, and guarding it would block nearly every feature PR. The gate is inert until `ci-integrity` is a required status check. Verified: injecting Tags-block characters fails the scan and reverting restores it; a placeholder allowlist entry is rejected; editing a blessed line invalidates it as both a fresh hit and a stale entry; the selftest covers every carrier class and confirms silence on the em dashes, box-drawing banners and CJK i18n strings this tree genuinely contains. The tripwire's path matching was checked against real pull requests, and its refusal path was run under `set -euo pipefail` after fixing an `&&` that would have aborted the script on exactly that path. Signed-off-by: Martin Vogel --- .github/workflows/_security.yml | 4 + .github/workflows/ci-integrity.yml | 111 ++++++++++++ CONTRIBUTING.md | 2 +- SECURITY.md | 3 +- internal/cbm/lsp/php_lsp.c | 5 +- scripts/security-injection.py | 279 +++++++++++++++++++++++++++++ 6 files changed, 400 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/ci-integrity.yml create mode 100644 scripts/security-injection.py diff --git a/.github/workflows/_security.yml b/.github/workflows/_security.yml index 7232d87457..5cb8e62b75 100644 --- a/.github/workflows/_security.yml +++ b/.github/workflows/_security.yml @@ -15,6 +15,10 @@ jobs: timeout-minutes: 5 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: "Layer 0: Hidden-instruction self-test" + run: scripts/security-injection.py --selftest + - name: "Layer 0: Hidden-instruction audit" + run: scripts/security-injection.py - name: "Layer 1: Static allow-list audit" run: scripts/security-audit.sh - name: "Layer 6: UI security audit" diff --git a/.github/workflows/ci-integrity.yml b/.github/workflows/ci-integrity.yml new file mode 100644 index 0000000000..c1a781dda7 --- /dev/null +++ b/.github/workflows/ci-integrity.yml @@ -0,0 +1,111 @@ +name: CI integrity + +# Refuses any pull request that modifies how CI itself works. +# +# WHY THIS EXISTS — the circularity problem. Every other check in this repo is +# defined by files inside the pull request. Under the `pull_request` trigger +# GitHub runs the workflow as it exists in the MERGE COMMIT, so a hostile PR +# does not need to defeat a gate: it edits the gate, or deletes the step that +# calls it, and CI reports green over the change that disabled it. `ci-ok` is no +# help, because its own `needs:` list lives in the PR too. +# +# SECURITY — this uses `pull_request_target`, which runs the workflow definition +# from the BASE branch instead of the pull request. That is the entire point: +# this file cannot be edited by the PR it is judging. Deleting it in the PR does +# nothing, because the base copy is the one that runs. +# +# The safety rule that makes `pull_request_target` sound here: this job NEVER +# checks out, builds, or executes pull-request code. It does not check out +# anything at all. It asks the API for the LIST of changed paths and compares +# strings. No PR content is interpolated into a shell command; paths arrive as +# newline-delimited data on stdin and are matched with `case`, not `eval`. +# Do not add `actions/checkout`, do not add a `ref:`, do not add a build step. +# +# WHY `Makefile.cbm` IS NOT GUARDED — the security gates are invoked directly +# from workflow YAML (`run: scripts/security-audit.sh`), never through a make +# target, so the Makefile cannot silently neuter one. Guarding it would block +# nearly every feature PR, because adding a source file edits it. + +on: + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review] + branches: [main] + +permissions: + contents: read + pull-requests: read + +jobs: + ci-integrity: + name: ci-integrity + runs-on: ubuntu-latest + steps: + - name: Refuse pull-request changes to CI-defining paths + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + + # Paths that define or are executed by CI. Under-covering is the + # dangerous direction: anything CI runs can neuter a gate, so this + # list is deliberately broad rather than precise. + is_guarded() { + case "$1" in + .github/*) return 0 ;; + scripts/*) return 0 ;; + test-infrastructure/*) return 0 ;; + tests/*.sh) return 0 ;; + *) return 1 ;; + esac + } + + changed="$(gh api --paginate \ + "repos/$REPO/pulls/$PR_NUMBER/files" \ + --jq '.[].filename')" + + hits="" + while IFS= read -r path; do + [ -n "$path" ] || continue + if is_guarded "$path"; then + hits="${hits}${path}"$'\n' + fi + done <<< "$changed" + + if [ -z "$hits" ]; then + echo "OK: this pull request does not modify CI-defining paths." + exit 0 + fi + + echo "=== CI INTEGRITY GATE: REFUSED ===" + echo + echo "This pull request modifies files that define or are executed by CI:" + echo + # `if` rather than `[ … ] && …`: under `set -e` the && form returns 1 + # on the trailing empty line, which would abort the script on exactly + # the refusal path this message exists to explain. + while IFS= read -r path; do + if [ -n "$path" ]; then + printf ' %s\n' "$path" + fi + done <<< "$hits" + echo + echo "A pull request cannot be allowed to change the checks that judge it." + echo "That is not a statement about this change or its author -- the gate" + echo "cannot tell intent, so it refuses the whole class." + echo + echo "HOW THIS LANDS:" + echo " A maintainer reviews the CI change on its own merits and merges it" + echo " under the break-glass protocol. Contributors do not need to do" + echo " anything differently: open the PR as normal and say in the" + echo " description what the CI change does and why." + echo + echo " To exercise a branch's own CI definitions before that happens, a" + echo " maintainer can dispatch them explicitly:" + echo " gh workflow run dry-run.yml --ref " + echo + echo " Keeping CI changes in their own pull request, separate from the" + echo " feature work, makes both halves reviewable and is the fastest" + echo " route through this gate." + exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96ce8f861e..6f95e8bba1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -153,7 +153,7 @@ If in doubt, open an issue and ask. We take security seriously. All PRs go through: - Manual security review (dangerous calls, network access, file writes, prompt injection) -- Automated 8-layer security audit in CI +- Automated 9-layer security audit in CI - Vendored dependency integrity checks If you add a new `system()`, `popen()`, `fork()`, or network call, it must be justified and added to `scripts/security-allowlist.txt`. diff --git a/SECURITY.md b/SECURITY.md index 48da1d1a1b..fd2d99eaa3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -99,7 +99,8 @@ This project implements multiple layers of security verification. Every release ### Build-Time (CI — every commit) -- **8-layer security audit suite** runs on every build: +- **9-layer security audit suite** runs on every build: + - Layer 0: Hidden-instruction audit (invisible/bidi/tag Unicode across the whole tree) - Layer 1: Static allow-list for dangerous calls (`system`/`popen`/`fork`) + hardcoded URLs - Layer 2: Binary string audit (URLs, credentials, dangerous commands) - Layer 3: Network egress monitoring via strace (Linux) diff --git a/internal/cbm/lsp/php_lsp.c b/internal/cbm/lsp/php_lsp.c index 4caf5ba0fd..1bac2293c3 100644 --- a/internal/cbm/lsp/php_lsp.c +++ b/internal/cbm/lsp/php_lsp.c @@ -489,7 +489,8 @@ const CBMType *php_parse_type_node(PHPLSPContext *ctx, TSNode node) { /* ── PHPDoc minimal parser ──────────────────────────────────────── */ -/* Strip leading "/**", trailing "*​/", and per-line "*" prefixes. Returns a +/* Strip the leading "/**", the trailing star-slash, and per-line "*" prefixes. + * Returns a * mutable arena-allocated cleaned copy. */ static char *phpdoc_clean(CBMArena *a, const char *raw) { if (!raw) @@ -788,7 +789,7 @@ static void bind_phpdoc_var(PHPLSPContext *ctx, const char *docstring) { } /* Walk siblings backwards from `node` to find a leading PHPDoc comment - * ("/**...*​/"). Returns cleaned doc text or NULL. */ + * (a "/**" ... star-slash block). Returns cleaned doc text or NULL. */ static char *fetch_leading_phpdoc(PHPLSPContext *ctx, TSNode node) { TSNode parent = ts_node_parent(node); if (ts_node_is_null(parent)) diff --git a/scripts/security-injection.py b/scripts/security-injection.py new file mode 100644 index 0000000000..2db198ace4 --- /dev/null +++ b/scripts/security-injection.py @@ -0,0 +1,279 @@ +#!/usr/bin/env python3 +"""Layer 0: hidden-instruction audit -- source-level check on the whole tree. + +Scans every tracked file for Unicode that is INVISIBLE or DIRECTION-CONTROLLING +in a normal editor and diff view, but which a language model reading the file +still consumes. Those characters are the carrier layer of indirect prompt +injection: zero-width sequences, bidirectional overrides (Trojan Source), the +Unicode Tags block, and stray byte-order marks. + +WHY THIS IS A HARD GATE AND A KEYWORD LIST IS NOT +------------------------------------------------ +The persuasion layer of an injection ("ignore previous instructions...") is +natural language, so it is unbounded and translatable -- published refusal rates +fall from roughly 79% in English to as low as 23% in some low-resource +languages, and a homoglyph substitution defeats a keyword match outright. A word +list cannot gate that honestly. + +The CARRIER layer is different. It is finite, it is language-independent, and it +has no legitimate use in source code at all. That makes it gateable with a false +positive rate near zero: this check reports what it found rather than what it +guessed, and "no invisible characters are present" is an arithmetic statement +rather than a judgement. + +This check therefore catches HIDING, not persuasion. Plain visible text that +argues with a model passes it, by design. Do not read a green result as "no +injection"; read it as "nothing is concealed from the reviewer", which is what +makes ordinary human review trustworthy. + +ALLOWLIST +--------- +scripts/injection-allowlist.txt, one entry per line: + + # why this occurrence is safe + +The sha256 is of the LINE that contains the character, not of the file, and the +line is located by content rather than by number. Editing anything elsewhere in +the file does not disturb the entry; editing the blessed line itself invalidates +it and the gate fails. That is deliberate -- an attacker who appends to an +already-blessed region must also update a checksum, which turns an invisible +edit into a one-line diff a reviewer can see. + +Entries require a written justification. `--update` emits a placeholder that +this gate rejects, so an allowlist entry cannot be produced mechanically. +""" + +import hashlib +import subprocess +import sys +from pathlib import Path + +# Carrier codepoints. Every range here is invisible or direction-controlling in +# a normal editor; none has a legitimate use in source. Ordinary non-ASCII -- +# em dashes, box-drawing banners, CJK in the i18n strings -- is NOT listed and +# must never be, or the gate becomes noise and gets switched off. +CARRIERS = { + (0x200B, 0x200F): "zero-width / directional mark", + (0x202A, 0x202E): "bidirectional override (Trojan Source)", + (0x2060, 0x2064): "word joiner / invisible operator", + (0x2066, 0x2069): "directional isolate", + (0xFEFF, 0xFEFF): "byte-order mark", + (0x00AD, 0x00AD): "soft hyphen", + (0xE0000, 0xE007F): "Unicode Tags block (invisible instruction carrier)", +} + +ALLOWLIST = "scripts/injection-allowlist.txt" +PLACEHOLDER = "TODO" + + +# Flattened for lookup speed: the gate scans every character of every +# non-ASCII file, so a set membership test beats walking the ranges. +_CARRIER_LABEL = { + cp: label + for (low, high), label in CARRIERS.items() + for cp in range(low, high + 1) +} + + +def classify(codepoint): + return _CARRIER_LABEL.get(codepoint) + + +def tracked_files(root): + out = subprocess.run( + ["git", "-C", str(root), "ls-files", "-z"], + capture_output=True, check=True, + ).stdout + return [p.decode() for p in out.split(b"\0") if p] + + +def scan(root): + """Yield (path, line_no, line_text, sha256_of_line, [(char, label), ...]).""" + for rel in tracked_files(root): + full = root / rel + try: + raw = full.read_bytes() + except (OSError, ValueError): + continue + # Fast path: a pure-ASCII file cannot hold a carrier. `bytes.isascii` + # is a C-level scan; the equivalent Python loop more than doubles the + # runtime of the whole gate on this tree. + if raw.isascii(): + continue + try: + text = raw.decode("utf-8") + except UnicodeDecodeError: + continue # binary; not a review surface + # Second fast path: check the file once before doing per-line work. + if not any(classify(ord(ch)) for ch in text): + continue + for line_no, line in enumerate(text.split("\n"), 1): + found = [ + (ch, classify(ord(ch))) for ch in line if classify(ord(ch)) + ] + if found: + digest = hashlib.sha256(line.encode("utf-8")).hexdigest() + yield rel, line_no, line, digest, found + + +def load_allowlist(root): + """Return {(sha256, path): why}. Entries without a real why are dropped.""" + path = root / ALLOWLIST + entries, malformed = {}, [] + if not path.exists(): + return entries, malformed + for raw_no, raw in enumerate(path.read_text(encoding="utf-8").split("\n"), 1): + line = raw.strip() + if not line or line.startswith("#"): + continue + body, _, why = line.partition("#") + parts = body.split() + if len(parts) != 2: + malformed.append((raw_no, "expected ' # why'", line)) + continue + digest, rel = parts + if len(digest) != 64 or not all(c in "0123456789abcdef" for c in digest): + malformed.append((raw_no, "first field is not a sha256", line)) + continue + why = why.strip() + if not why or PLACEHOLDER in why: + malformed.append( + (raw_no, "needs a written justification, not a placeholder", line) + ) + continue + entries[(digest, rel)] = why + return entries, malformed + + +def render(ch): + return f"U+{ord(ch):04X}" + + +def selftest(): + """Prove the gate catches what it claims and ignores what it must. + + A gate nobody has seen fail is indistinguishable from a gate that cannot + fail, so this runs in CI beside the real scan. + """ + failures = [] + + def check(ok, what): + if not ok: + failures.append(what) + + # Every carrier class must be recognised. + for cp, what in [ + (0x200B, "zero-width space"), + (0x200E, "left-to-right mark"), + (0x202E, "right-to-left override (Trojan Source)"), + (0x2060, "word joiner"), + (0x2066, "directional isolate"), + (0xFEFF, "byte-order mark"), + (0x00AD, "soft hyphen"), + (0xE0001, "Unicode Tags block"), + (0xE007F, "Unicode Tags block terminator"), + ]: + check(classify(cp) is not None, f"missed carrier {what} (U+{cp:04X})") + + # Legitimate non-ASCII that this repo genuinely contains must NOT trip it. + # If any of these ever start failing the gate becomes noise and gets + # switched off, which is worse than not having it. + for cp, what in [ + (0x2014, "em dash (prose throughout)"), + (0x2500, "box drawing (section banners)"), + (0x2192, "rightwards arrow (comments)"), + (0x4E2D, "CJK ideograph (i18n strings)"), + (0x00E9, "e-acute (contributor names)"), + (0x2713, "check mark"), + (0x1F916, "emoji"), + ]: + check(classify(cp) is None, f"false positive on {what} (U+{cp:04X})") + + # A blessed line is pinned by content: changing it must invalidate the entry. + line = 'x = "a\u200bb";' + other = 'x = "a\u200bc";' + check( + hashlib.sha256(line.encode()).hexdigest() + != hashlib.sha256(other.encode()).hexdigest(), + "line hash did not change when the blessed line changed", + ) + + if failures: + for f in failures: + print(f"SELFTEST FAIL: {f}") + return 1 + print(f"OK: selftest passed ({len(_CARRIER_LABEL)} carrier codepoints known).") + return 0 + + +def main(argv): + if "--selftest" in argv: + return selftest() + + update = "--update" in argv + rest = [a for a in argv if not a.startswith("--")] + root = Path(rest[0]).resolve() if rest else Path.cwd() + + allowed, malformed = load_allowlist(root) + hits = list(scan(root)) + + if update: + lines = [ + "# Hidden-instruction allowlist -- see scripts/security-injection.py.", + "# Each entry pins the sha256 of ONE line. Editing that line breaks the", + "# entry and fails the gate, on purpose. Replace every TODO with a real", + "# justification; the gate rejects placeholders.", + "", + ] + for rel, line_no, _line, digest, found in hits: + names = ", ".join(sorted({render(c) for c, _ in found})) + lines.append(f"{digest} {rel} # TODO: why is {names} safe here?") + (root / ALLOWLIST).write_text("\n".join(lines) + "\n", encoding="utf-8") + print(f"wrote {len(hits)} entries to {ALLOWLIST}") + print("Each still needs a written justification before the gate will pass.") + return 0 + + problems = 0 + for raw_no, why, line in malformed: + print(f"FAIL: {ALLOWLIST}:{raw_no}: {why}\n {line}") + problems += 1 + + unexplained = [] + for rel, line_no, line, digest, found in hits: + if (digest, rel) in allowed: + continue + unexplained.append((rel, line_no, line, digest, found)) + + if unexplained: + print("=== HIDDEN-INSTRUCTION AUDIT: REFUSED ===\n") + for rel, line_no, line, digest, found in unexplained: + names = ", ".join(f"{render(c)} ({lbl})" for c, lbl in found) + shown = "".join( + f"<{render(c)}>" if classify(ord(c)) else c for c in line + ).strip() + print(f"{rel}:{line_no}: {names}") + print(f" {shown}") + print(f" sha256 {digest}\n") + print("These characters are invisible in an editor and in a diff, but a") + print("model reading the file still consumes them.\n") + print("PREFERRED FIX: rephrase so the character is not needed. An") + print("allowlist entry is a permanent exception and should be rare.") + print(f"If it is genuinely required, add to {ALLOWLIST}:\n") + for rel, _n, _l, digest, _f in unexplained: + print(f" {digest} {rel} # ") + problems += len(unexplained) + + stale = set(allowed) - {(d, r) for r, _n, _l, d, _f in hits} + for digest, rel in sorted(stale): + print(f"FAIL: stale allowlist entry (line no longer present): {digest} {rel}") + problems += 1 + + if problems: + return 1 + print(f"OK: no hidden-instruction carriers outside the allowlist " + f"({len(allowed)} allowed, {len(hits)} occurrence(s) total).") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) From ca9818b47018a8af2ebe6f892b32f9ee1bbb6863 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 21 Aug 2026 15:27:52 +0200 Subject: [PATCH 02/10] security: catch prose smuggled into generated parser symbol tables Tier 2 of the hidden-instruction gate. A generated LR parser's string table holds grammar symbol names and punctuation terminals; an English sentence in there is anomalous by construction. That is the check which cleared a 353,744-line vendored parser by hand during review, and this mechanises it. The threshold was MEASURED rather than guessed. Across all 159 vendored grammars: 48,271 string literals, of which 63 contain a space, because multi-word keywords are real -- "is not", "not in", "static get". The longest legitimate literal is three words ("hide empty description"), so four is the tightest threshold with zero false positives, and it still catches a four-word instruction. Current tree: 0 findings. Deliberately NOT shipped: a tree-wide scan for hiding constructs. The measurement did not support it. `" + check(content_report(buried)["verdict"] == "refuse", + "a payload inside a downgraded HTML comment failed to refuse") + check(content_report("")["verdict"] == "note", + "a bare HTML comment should be noted, not refused") shown = json.dumps(content_report(secret, redact=False)) check("SENTINELWORD" in shown, "--show-payload must still give a human the literal text") @@ -799,21 +821,21 @@ def main(argv): f"({report['finding_count']} finding(s), " f"{report['chars_scanned']} chars)") for f in report["findings"]: - print(f" line {f['line']}: {f['detail']}") + print(f" [{f['severity']}] line {f['line']}: {f['detail']}") print(f" {f['excerpt']}") print(f"\n{report['guidance']}") - return 1 if report["verdict"] == "findings" else 0 + return 1 if report["verdict"] == "refuse" else 0 if "--metadata" in argv: target = Path(argv[argv.index("--metadata") + 1]) text = target.read_text(encoding="utf-8", errors="replace") - findings = list(scan_metadata(text)) + findings = [f for f in scan_metadata(text) if f[3] == REFUSE] if not findings: print(f"OK: no hidden-instruction findings in pull-request metadata " f"({len(text)} chars scanned).") return 0 print("=== PULL-REQUEST METADATA: REFUSED ===\n") - for line_no, line, detail in findings: + for line_no, line, detail, _sev in findings: shown = "".join( f"" if classify(ord(c)) else c for c in line ).strip()