scripts at 5b173fbc66, reviewed by claude-code, verified by codex.
build-warehouse-parquet.py exports scans.parquet and scan_files.parquet with `SELECT * FROM scanstore.<table>` and no PUBLIC_REPOS predicate, so every repo corral has scanned on that machine — including private ones — is published to the static site, directly contradicting the module's own stated invariant that 'a repo is exported only if it appears in PUBLIC_REPOS'.
#!/bin/sh
# Runs build-warehouse-parquet.py's own main() against a stubbed duckdb driver
# that RECORDS the SQL it issues, then inspects the COPY statements that produce
# the published parquet extracts.
cp site/src/data/warehouse-manifest.json /tmp/wm.bak 2>/dev/null
python3 - <<'PYEOF'
import sys, types, os, importlib.util, tempfile
captured = []
fake = types.ModuleType("duckdb")
class Error(Exception):
pass
fake.Error = Error
class Res:
def __init__(self, v):
self.v = v
def fetchone(self):
return (self.v,)
class Con:
def execute(self, sql):
captured.append(sql)
return Res(7) if sql.lstrip().startswith("SELECT count(*)") else Res(0)
def create_function(self, *a, **k):
pass
fake.connect = lambda *a, **k: Con()
sys.modules["duckdb"] = fake
spec = importlib.util.spec_from_file_location("bwp", "scripts/build-warehouse-parquet.py")
m = importlib.util.module_from_spec(spec)
spec.loader.exec_module(m)
out = tempfile.mkdtemp()
sys.argv = ["build-warehouse-parquet.py", "--out", out]
m.main()
print()
print("=== every COPY the generator issues to produce a published extract ===")
unfiltered = []
for sql in captured:
s = sql.lstrip()
if not s.startswith("COPY"):
continue
dest = os.path.basename(s.split("TO '")[1].split("'")[0])
guarded = "short_repo(repo) IN" in s
print(" %-22s allowlist predicate present: %s" % (dest, guarded))
if not guarded:
unfiltered.append(dest)
print(" SQL: %s" % " ".join(s.split()))
print()
print("PUBLIC_REPOS allowlist =", sorted(m.PUBLIC_REPOS))
print("module docstring: 'a repo is exported only if it appears in PUBLIC_REPOS below'")
print("internal/scanstore/rows.go's Scan row carries Owner, Repo, Commit.")
print()
print("EXPORTED WITH NO ALLOWLIST:", unfiltered)
sys.exit(0 if unfiltered else 1)
PYEOF
rc=$?
[ -f /tmp/wm.bak ] && cp /tmp/wm.bak site/src/data/warehouse-manifest.json
exit $rcaudit_ledger: 7 rows exported, 0 withheld (repo not on the public allowlist)
bug_catches: 7 rows exported, 0 withheld (repo not on the public allowlist)
scans: 7 rows exported
scan_files: 7 rows exported
wrote to /tmp/tmpuds3g5zz/ and site/src/data/warehouse-manifest.json — rebuild the site to pick them up
=== every COPY the generator issues to produce a published extract ===
audit_ledger.parquet allowlist predicate present: True
bug_catches.parquet allowlist predicate present: True
scans.parquet allowlist predicate present: False
SQL: COPY (SELECT * FROM scanstore.scans) TO '/tmp/tmpuds3g5zz/scans.parquet' (FORMAT parquet)
scan_files.parquet allowlist predicate present: False
SQL: COPY (SELECT * FROM scanstore.scan_files) TO '/tmp/tmpuds3g5zz/scan_files.parquet' (FORMAT parquet)
PUBLIC_REPOS allowlist = ['debug-js/debug', 'google/uuid', 'local', 'minitest/minitest', 'more-itertools/more-itertools', 'pdbethke/corralai', 'pdbethke/sportspicker-core', 'threedaymonk/text', 'vercel/ms', 'wvanbergen/chunky_png']
module docstring: 'a repo is exported only if it appears in PUBLIC_REPOS below'
internal/scanstore/rows.go's Scan row carries Owner, Repo, Commit.
EXPORTED WITH NO ALLOWLIST: ['scans.parquet', 'scan_files.parquet']I traced all four export queries and looked for a narrower table-level privacy contract. The audit_ledger and bug_catches queries contain the PUBLIC_REPOS predicate, while scans and scan_files are copied with SELECT * and Scan explicitly carries Owner and Repo at internal/scanstore/rows.go:20-22. The module-wide privacy promise is not limited to the first two tables, and no downstream filter protects the published parquet files.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R1
export()'s refusal sentinel -1 is fed straight into `total - kept`, so when a populated extract is protected from an empty source the tool prints '-1 rows exported' and publishes a `withheld` count one HIGHER than the source table's entire row count.
#!/bin/sh
# Drives build-warehouse-parquet.py's main() through the documented refusal path
# (source store empty + a populated extract already committed) with a stubbed
# duckdb driver, and reads back the withheld counts it publishes.
cp site/src/data/warehouse-manifest.json /tmp/wm2.bak 2>/dev/null
python3 - <<'PYEOF'
import sys, types, os, io, json, importlib.util, tempfile, contextlib
fake = types.ModuleType("duckdb")
class Error(Exception):
pass
fake.Error = Error
class Res:
def __init__(self, v):
self.v = v
def fetchone(self):
return (self.v,)
class Con:
def execute(self, sql):
s = sql.lstrip()
if s.startswith("SELECT count(*) FROM ("): # export()'s precheck: source is empty
return Res(0)
if "build.build_records" in s:
return Res(7) # the ledger really holds 7 rows
if "bugcatch.bugcatch_observations" in s:
return Res(5) # bugcatch really holds 5 rows
return Res(0)
def create_function(self, *a, **k):
pass
fake.connect = lambda *a, **k: Con()
sys.modules["duckdb"] = fake
spec = importlib.util.spec_from_file_location("bwp", "scripts/build-warehouse-parquet.py")
m = importlib.util.module_from_spec(spec)
spec.loader.exec_module(m)
out = tempfile.mkdtemp()
# a populated extract already exists -> export() REFUSES and returns -1
for n in ("audit_ledger.parquet", "bug_catches.parquet"):
with open(os.path.join(out, n), "w") as fh:
fh.write("already-published-data")
sys.argv = ["build-warehouse-parquet.py", "--out", out]
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
m.main()
text = buf.getvalue()
print("=== what the generator printed ===")
print(text)
man = json.load(open("site/src/data/warehouse-manifest.json", encoding="utf-8"))
w = man["withheld"]
print("=== the manifest it published to the site ===")
print(json.dumps(man, indent=2))
print()
print("build.build_records total rows : 7")
print("audit_ledger 'withheld' as reported :", w["audit_ledger"], "(more than the whole table)")
print("bugcatch_observations total rows : 5")
print("bug_catches 'withheld' as reported :", w["bug_catches"], "(more than the whole table)")
ok = ("-1 rows exported" in text
and w["audit_ledger"] == 8
and w["bug_catches"] == 6)
sys.exit(0 if ok else 1)
PYEOF
rc=$?
[ -f /tmp/wm2.bak ] && cp /tmp/wm2.bak site/src/data/warehouse-manifest.json
exit $rc=== what the generator printed ===
REFUSED to overwrite /tmp/tmpx2mvi1m1/audit_ledger.parquet: the query returned 0 rows and a populated extract already exists (source store is empty on this machine)
audit_ledger: -1 rows exported, 8 withheld (repo not on the public allowlist)
REFUSED to overwrite /tmp/tmpx2mvi1m1/bug_catches.parquet: the query returned 0 rows and a populated extract already exists (source store is empty on this machine)
bug_catches: -1 rows exported, 6 withheld (repo not on the public allowlist)
scans: 0 rows exported
scan_files: 0 rows exported
wrote to /tmp/tmpx2mvi1m1/ and site/src/data/warehouse-manifest.json — rebuild the site to pick them up
=== the manifest it published to the site ===
{
"generated_note": "built by scripts/build-warehouse-parquet.py \u2014 do not hand-edit",
"tables": {
"audit_ledger": 0,
"bug_catches": 0
},
"withheld": {
"audit_ledger": 8,
"bug_catches": 6
}
}
build.build_records total rows : 7
audit_ledger 'withheld' as reported : 8 (more than the whole table)
bugcatch_observations total rows : 5
bug_catches 'withheld' as reported : 6 (more than the whole table)The refusal branch in export() returns -1 at scripts/build-warehouse-parquet.py:76. Both filtered exports then compute and publish total - kept without checking that sentinel, producing total+1 and a negative exported count. The later manifest recount of existing files does not repair the incorrect withheld values.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R2
check-licensing.sh accepts a .go file that contains the SPDX string anywhere — including inside a string literal — rather than as a header, and add-spdx.sh has the same blindness, so an unlicensed Go file passes the CI licensing gate AND is refused by the fixer meant to repair it.
#!/bin/sh # Plants a tracked .go file with NO license header whose only occurrence of the # gate's search string is inside a string constant, then runs both the CI gate # and its fixer. before=$(bash scripts/check-licensing.sh 2>&1); before_rc=$? mkdir -p internal/licensingprobe cat > internal/licensingprobe/probe.go <<'GO' package licensingprobe // This file carries NO license header on line 1. The only occurrence of the // gate's search string in it is inside the constant below. const Header = "// SPDX-License-Identifier: Elastic-2.0" GO git add -f internal/licensingprobe/probe.go >/dev/null 2>&1 after=$(bash scripts/check-licensing.sh 2>&1); after_rc=$? bash scripts/add-spdx.sh > /tmp/fixer.out 2>&1 echo "=== the planted file (first 3 lines) ===" head -3 internal/licensingprobe/probe.go echo echo "=== check-licensing.sh BEFORE planting (rc=$before_rc) ===" echo "$before" echo echo "=== check-licensing.sh AFTER planting an unlicensed .go file (rc=$after_rc) ===" echo "$after" echo echo "=== add-spdx.sh (the fixer) output ===" cat /tmp/fixer.out echo if [ "$before" = "$after" ] && [ "$before_rc" = "$after_rc" ] \ && ! grep -q licensingprobe /tmp/fixer.out \ && ! head -1 internal/licensingprobe/probe.go | grep -q 'SPDX-License-Identifier'; then echo "DEFECT: internal/licensingprobe/probe.go has no license header, yet the gate's" echo "output and exit code are byte-identical to before it existed, and add-spdx.sh" echo "refuses to header it. Both match the SPDX string ANYWHERE in the file rather" echo "than as the file's header, so such a file stays unlicensed with both green." exit 0 fi exit 1
=== the planted file (first 3 lines) === package licensingprobe // This file carries NO license header on line 1. The only occurrence of the === check-licensing.sh BEFORE planting (rc=0) === OK: all licensing invariants hold === check-licensing.sh AFTER planting an unlicensed .go file (rc=0) === OK: all licensing invariants hold === add-spdx.sh (the fixer) output === DEFECT: internal/licensingprobe/probe.go has no license header, yet the gate's output and exit code are byte-identical to before it existed, and add-spdx.sh refuses to header it. Both match the SPDX string ANYWHERE in the file rather than as the file's header, so such a file stays unlicensed with both green.
check-licensing.sh uses an unanchored grep over each complete Go file, and add-spdx.sh likewise tests for SPDX-License-Identifier anywhere in the file. Neither requires the identifier to be a leading comment or header, so the reviewer's string-literal input satisfies both checks while remaining headerless.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R3
scrub-golden-run.py exempts any absolute path that merely BEGINS with '/work', '/tmp' or '/root' via str.startswith, so host-identifying paths such as /workspaces/<username>/... (the GitHub Codespaces layout) pass the deny gate the docstring says treats an escaped host path as a deny, not a manifest entry.
#!/bin/sh
python3 - <<'PYEOF'
import importlib.util, sys
spec = importlib.util.spec_from_file_location("scrub", "scripts/scrub-golden-run.py")
m = importlib.util.module_from_spec(spec)
spec.loader.exec_module(m)
print("SAFE_PATH_PREFIXES =", m.SAFE_PATH_PREFIXES)
print("matched at scripts/scrub-golden-run.py:121 with path.startswith(...), which is a")
print("STRING prefix test, not a PATH-SEGMENT test.")
print()
cases = [
"/workspaces/pdbethke/private-client-repo/src/main.go",
"/rootfs-backup/pdbethke/.ssh/id_ed25519",
"/tmpl/customers/acme/notes.txt",
]
missed = []
for p in cases:
off = m.scan_deny("the run wrote %s" % p, "", "")
print(" %-54s -> deny offenses: %r" % (p, off))
if not off:
missed.append(p)
control = m.scan_deny("the run wrote /etc/some/config/file", "", "")
print(" %-54s -> deny offenses: %r" % ("/etc/some/config/file (control)", control))
print()
print("docstring: 'an absolute path outside the demo containers own internal roots")
print("(/work, /tmp, /root) - an escaped host path is a deny, not a manifest entry.'")
print()
print("HOST PATHS THAT ESCAPED THE DENY GATE:", missed)
sys.exit(0 if missed and control else 1)
PYEOFSAFE_PATH_PREFIXES = ('/work', '/tmp', '/root')
matched at scripts/scrub-golden-run.py:121 with path.startswith(...), which is a
STRING prefix test, not a PATH-SEGMENT test.
/workspaces/pdbethke/private-client-repo/src/main.go -> deny offenses: []
/rootfs-backup/pdbethke/.ssh/id_ed25519 -> deny offenses: []
/tmpl/customers/acme/notes.txt -> deny offenses: []
/etc/some/config/file (control) -> deny offenses: [('absolute path outside demo-container roots', '/etc/some/config/file')]
docstring: 'an absolute path outside the demo containers own internal roots
(/work, /tmp, /root) - an escaped host path is a deny, not a manifest entry.'
HOST PATHS THAT ESCAPED THE DENY GATE: ['/workspaces/pdbethke/private-client-repo/src/main.go', '/rootfs-backup/pdbethke/.ssh/id_ed25519', '/tmpl/customers/acme/notes.txt']The generic absolute-path check calls path.startswith(SAFE_PATH_PREFIXES), where the tuple is exactly ('/work', '/tmp', '/root'). This is a lexical prefix test, so /workspaces, /tmpl, and /rootfs-backup are exempt even though they are outside the three documented roots. The manifest may expose them for human review, but that does not refute the claim that they bypass the automated deny gate.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R4
capture-recording-video.sh's slug argument names only the output mp4 and never selects the recording; its default output name (pool-fence-crossvendor) disagrees with the hero slug the built site actually renders (corral-audits-corral), so a default run overwrites site/public/recordings/pool-fence-crossvendor.mp4 with footage of a different run.
#!/bin/sh
python3 - <<'PYEOF'
import re, sys
cap = open("scripts/capture-recording-video.sh", encoding="utf-8").read()
hero = open("site/src/components/Hero.astro", encoding="utf-8").read()
script_default = re.search(r'^SLUG="\$\{1:-([^}]*)\}"', cap, re.M).group(1)
hero_default = re.search(r"CORRALAI_HERO_SLUG \|\| '([^']*)'", hero).group(1)
print("capture-recording-video.sh default OUT slug :", script_default)
print("Hero.astro default hero slug (what renders) :", hero_default)
print()
print("EVERY line of capture-recording-video.sh containing 'SLUG':")
for i, line in enumerate(cap.splitlines(), 1):
if "SLUG" in line:
print(" %3d: %s" % (i, line))
print()
print("The slug reaches only OUT=\"$OUTDIR/$SLUG.mp4\". Hero.astro reads")
print("CORRALAI_HERO_SLUG at ASTRO BUILD TIME; this script serves the already-built")
print("site/dist and neither exports that variable nor rebuilds the site.")
print()
if script_default != hero_default:
print("DEFECT: a default run writes '%s.mp4' containing the '%s' recording."
% (script_default, hero_default))
sys.exit(0)
sys.exit(1)
PYEOFcapture-recording-video.sh default OUT slug : pool-fence-crossvendor
Hero.astro default hero slug (what renders) : corral-audits-corral
EVERY line of capture-recording-video.sh containing 'SLUG':
7: # (CORRALAI_HERO_SLUG, default pool-fence-crossvendor) — no brain, no keys: the
29: SLUG="${1:-pool-fence-crossvendor}"
35: OUT="$OUTDIR/$SLUG.mp4"
The slug reaches only OUT="$OUTDIR/$SLUG.mp4". Hero.astro reads
CORRALAI_HERO_SLUG at ASTRO BUILD TIME; this script serves the already-built
site/dist and neither exports that variable nor rebuilds the site.
DEFECT: a default run writes 'pool-fence-crossvendor.mp4' containing the 'corral-audits-corral' recording.The script assigns its argument only to SLUG and derives only the output filename from it. It serves an already-built site and does not rebuild or select a recording. Hero.astro defaults independently to corral-audits-corral, while the capture script defaults its output to pool-fence-crossvendor.mp4; repository tests also explicitly pin corral-audits-corral as the current default hero.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R5
export-golden-run.sh runs the deny scan and the human-review manifest only over the replay stream; the .meta.json sidecar's `directive` (free text fetched from the brain) is retrieved AFTER the operator's confirmation prompt, never deny-scanned and never shown in the manifest, yet it is written into site/src/data/recordings/*.meta.json and published.
The replay is deny-scanned and manifested before confirmation at scripts/export-golden-run.sh:225-238. Only afterward, at lines 243-257, the script fetches history and copies the mission directive into META_JSON. That metadata is neither passed through scrub-golden-run.py nor printed for review before being written to the public sidecar.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R6
check-security.sh's gofmt invariant redirects stderr to /dev/null and treats empty stdout as clean, so if gofmt is absent, non-executable, or errors on a file, `bad` is empty and the formatting gate silently passes — the opposite of the gosec branch immediately below it, which fails closed when its tool is missing.
The script has set -u and pipefail but not set -e. It captures `git ls-files '*.go' | xargs gofmt -l 2>/dev/null` without testing the substitution's exit status, then treats only nonempty stdout as failure. Thus a missing or failing gofmt can leave bad empty and avoid setting fail, unlike the explicit missing-tool handling for gosec.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R7
gen-before-after.py's splice() calls text.index(start) with no guard, so a target file that has lost its before-after markers crashes with a ValueError traceback instead of the loud, named failure gen-cost-table.sh:39 gives for exactly the same condition — under --check the drift gate dies rather than reporting.
splice() calls text.index for both markers before the --check branch can report drift. A missing marker therefore raises an uncaught ValueError and emits a generic traceback. This does make the gate fail, but it does not provide the deliberate named diagnostic claimed by the surrounding comparison, so the finding is accurate as a diagnostic-quality defect.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R8
export-golden-run.sh interpolates the operator-supplied $MISSION_ID directly into a python3 -c program (`x.get('id') == $MISSION_ID`), so a non-numeric --mission value is a syntax error or arbitrary Python rather than an argument-validation failure.
--mission stores an arbitrary string with no numeric validation, and line 246 expands it directly into Python source rather than passing it through argv or JSON. A nonnumeric value can cause SyntaxError, while a valid Python expression is evaluated in the predicate. The value's later use as a URL parameter does not constrain the earlier shell-to-Python source interpolation.
ref 9759aee2e1ea1f4fb8439c91e6353a2f42779e4ba472bc92d2e054759b32da41#R9
Recorded so that "found nothing here" can be told apart from "never looked here".
This scope is a pile of gates, generators and privacy filters, and it is written by someone who clearly knows the failure modes — the comments are full of hard-won rules ('a gate that enumerates where a property holds is not a gate', 'an escaped host path is a deny, not a manifest entry', 'a verification that verified nothing must never read as a pass'). The defects I found are all the same shape as the rules the comments state: the rule is enforced at one door and skipped at another, or the check matches a substring where it means a property. Finding 1 is the serious one — build-warehouse-parquet.py's docstring promises an allowlist and then exports two of its four tables (`scans`, `scan_files`, 1446 published rows) with no allowlist predicate at all, and the site's own manifest confirms it by having no `withheld` entry for them. Finding 2 is the sibling bug in the same file: the refusal path returns -1 and that -1 is then subtracted, so the published 'withheld' count exceeds the source table's total row count. Finding 3 is the same privacy gate missing at a second door in export-golden-run.sh: the deny scan and the human manifest cover the replay stream, but the `.meta.json` sidecar's `directive` field is fetched from the brain AFTER the operator has already confirmed and is written to the public site unscanned. Finding 4 is the licensing CI gate matching the SPDX string anywhere in a .go file rather than as a header — and the fixer that is supposed to repair it has the same blindness, so such a file is unlicensed forever with both tools green. Finding 5 is a prefix-vs-path bug in the deny list: `/workspaces/<username>/…` is exempt because it begins with `/work`. Finding 6 is a launch-asset mislabel: the video capture script's default output name and the hero it actually records disagree. Findings 7–9 I could argue but not execute. What I did not review: whether these scripts' claims about corral's Go behaviour are true — I read the Go only far enough to confirm that `scans` carries Owner/Repo.
None of the nine findings could be refuted. R1–R5 are supported by the cited code and exact probes. R6–R9 also survive code inspection: the metadata bypass, fail-open gofmt invocation, unhandled marker error, and direct Python interpolation are present as stated.