← the record

Review 1174024081d1

action.yml at 2b02f27989, reviewed by claude-code, verified by codex.

repo
git@github.com:pdbethke/corralai
commit
2b02f27989aa0d597607a1b85eeef28595fe00c7
scope
action.yml
reviewer
claude-code (2.1.263 (Claude Code))
verifier
codex (codex-cli 0.152.0) different model than the reviewer different vendor: anthropic → openai
substrate
workspace (a detached worktree at the commit; not a jail)
started
2026-09-08 20:25 UTC
audited party
Peter D Bethke · co-authored: Claude Opus 5 (1M context)
files shown
1 file(s)

Findings (5)

R1 medium reproduced

append_summary bounds each report at 900,000 bytes against a limit that applies to the WHOLE step summary, so with a review configured the audit report plus one review report writes ~1.8MB — over the 1MiB cap the bound exists to respect — and neither call reports truncation, losing the entire summary including the audit verdict.

where
action.yml:434
tier
REPRODUCED (declared REPRODUCED)
the script that was executed
#!/bin/sh
# The shipped append_summary(), extracted verbatim from action.yml and run
# the way the step runs it: once for the audit, once for a review scope.
set -u

W=$(mktemp -d) || exit 1
trap 'rm -rf "$W"' EXIT

sed -n '/^        append_summary() {/,/^        }$/p' action.yml | sed 's/^        //' > "$W/fn.sh"
if ! grep -q 'GITHUB_STEP_SUMMARY' "$W/fn.sh"; then
  echo "could not extract append_summary from action.yml -- not the defect"; exit 1
fi

LIMIT=$(sed -n 's/^ *limit=\([0-9][0-9]*\).*$/\1/p' "$W/fn.sh" | head -1)
if [ -z "$LIMIT" ]; then echo "no per-report limit found -- not the defect"; exit 1; fi

echo "per-report bound in action.yml    : $LIMIT bytes"
echo "GitHub whole-summary limit        : 1048576 bytes (1MiB, as action.yml:432 states)"
echo "append_summary call sites:"
grep -n 'append_summary "' action.yml
echo "(the second is inside the per-scope review loop; review-max-scopes defaults to 5)"
echo

# Two reports, each EXACTLY at the per-report bound, so neither is truncated.
c=aaaaaaaaaa
c="$c$c$c$c$c$c$c$c$c$c"
c="$c$c$c$c$c$c$c$c$c$c"          # 1000 bytes
n=$(( LIMIT / 1000 ))
: > "$W/audit.txt"
i=0
while [ "$i" -lt "$n" ]; do printf '%s' "$c" >> "$W/audit.txt"; i=$((i+1)); done
rem=$(( LIMIT - n * 1000 ))
if [ "$rem" -gt 0 ]; then printf "%${rem}s" '' | tr ' ' a >> "$W/audit.txt"; fi
cp "$W/audit.txt" "$W/review.txt"

GITHUB_STEP_SUMMARY="$W/summary.md"; export GITHUB_STEP_SUMMARY
: > "$GITHUB_STEP_SUMMARY"
bash -c '. "$1"; append_summary "Corral - adversarial test audit" "$2"; append_summary "Corral - review of internal/x" "$3"' _ "$W/fn.sh" "$W/audit.txt" "$W/review.txt" || { echo "append_summary itself errored -- not the defect"; exit 1; }

A=$(wc -c < "$W/audit.txt" | tr -d ' ')
R=$(wc -c < "$W/review.txt" | tr -d ' ')
T=$(wc -c < "$GITHUB_STEP_SUMMARY" | tr -d ' ')
echo "audit report appended             : $A bytes"
echo "review report appended            : $R bytes"
echo "resulting \$GITHUB_STEP_SUMMARY     : $T bytes"

if grep -q 'Report truncated' "$GITHUB_STEP_SUMMARY"; then
  echo "a call reported truncation -- not the defect"; exit 1
fi
echo "neither call emitted a 'Report truncated' notice: both believe they wrote a bounded, complete report"

if [ "$T" -gt 1048576 ]; then
  echo
  echo "DEFECT: $T bytes > 1048576. The bound is per-report; the limit is per-summary."
  echo "One review scope is enough; review-max-scopes defaults to 5, so up to 6 x $LIMIT bytes."
  echo "Per action.yml:432-433 GitHub rejects this OUTRIGHT, losing the audit verdict too."
  exit 0
fi
echo "stayed under 1MiB -- defect not demonstrated"
exit 1
what it printed
per-report bound in action.yml    : 900000 bytes
GitHub whole-summary limit        : 1048576 bytes (1MiB, as action.yml:432 states)
append_summary call sites:
461:        append_summary "Corral — adversarial test audit" "$report"
521:            append_summary "Corral — review of $scope" "$rreport"
(the second is inside the per-scope review loop; review-max-scopes defaults to 5)

audit report appended             : 900000 bytes
review report appended            : 900000 bytes
resulting $GITHUB_STEP_SUMMARY     : 1800088 bytes
neither call emitted a 'Report truncated' notice: both believe they wrote a bounded, complete report

DEFECT: 1800088 bytes > 1048576. The bound is per-report; the limit is per-summary.
One review scope is enough; review-max-scopes defaults to 5, so up to 6 x 900000 bytes.
Per action.yml:432-433 GitHub rejects this OUTRIGHT, losing the audit verdict too.

refutation — codex STANDS

I checked append_summary and both call sites. Each invocation independently permits 900,000 report bytes and appends headings and fences to the same GITHUB_STEP_SUMMARY file; the audit call at action.yml:461 and review-loop call at action.yml:521 therefore can exceed the stated whole-step 1MiB limit without either invocation satisfying its own size > limit truncation condition. I found no aggregate-size accounting or reset between those calls.

ref 1174024081d1de39e5374a837e5626ae3760abec0ad0d1f606a85ec7f28e8768#R1

R2 medium reproduced

The test-command description states the reserved-flag rule wrongly in both directions: it names --record as "the live collision" although --record is not a flag of `certify --repo` (the only certify the action runs) and so is never refused, while omitting --timeout and --json, which are, so the ordinary commands `go test ./... -timeout 30m` and `go test -json ./...` are rejected with exit 2 by a rule the operator was told does not cover them.

where
action.yml:9
tier
REPRODUCED (declared REPRODUCED)
the script that was executed
#!/bin/sh
# checkArgvNoFlagCollision derives its names from fs.VisitAll over
# certify --repo's OWN flag set (cmd/corral/certify_repo.go:2220-2243), so the
# authoritative list is the flags registered in runCertifyRepo. Compare that
# with the list action.yml's test-command description gives the operator.
set -u

documented=$(sed -n "s/.*one of corral.s own (\\([^)]*\\)).*/\\1/p" action.yml \
  | tr ',' '\n' | sed 's/[^a-z-]//g' | sed 's/^-*//' | grep -v '^$' | sort -u)
if [ -z "$documented" ]; then echo "could not read the documented list -- not the defect"; exit 1; fi

real=$(grep -oE 'fs\.(String|Bool|Int|Int64|Float64|Uint|Duration)(Var)?\(([^,]*, )?"[a-z0-9][a-z0-9-]*"' cmd/corral/certify_repo.go \
  | sed 's/.*"\([a-z0-9-]*\)"$/\1/' | sort -u)
# A walk that found nothing must not pass green.
for must in min-kill-rate preflight substrate diff-base top; do
  printf '%s\n' "$real" | grep -qxF "$must" || { echo "flag-set extraction broken ($must missing) -- not the defect"; exit 1; }
done

echo "documented in action.yml as corral's own flags:"; printf '%s\n' "$documented" | sed 's/^/  --/'
echo

# The rule, exactly as certify_repo.go:2224-2240 implements it.
collides() {
  trimmed=$(printf '%s' "$1" | sed 's/^-*//')
  [ "$trimmed" = "$1" ] && return 1
  [ -z "$trimmed" ] && return 1
  name=${trimmed%%=*}
  printf '%s\n' "$real" | grep -qxF "$name"
}

rc=0

# Direction A: documented as a collision, but not a flag of certify --repo.
echo "== documented, but NOT a flag of certify --repo =="
found_over=0
for f in $documented; do
  if ! printf '%s\n' "$real" | grep -qxF "$f"; then
    echo "  --$f is named in action.yml:9 but is registered nowhere in cmd/corral/certify_repo.go:"
    echo "      grep -c '\"$f\"' cmd/corral/certify_repo.go = $(grep -c "\"$f\"" cmd/corral/certify_repo.go)"
    grep -n "\"$f\"" cmd/corral/certify_local.go | head -1 | sed 's/^/      it belongs to certify --local: /'
    found_over=1
  fi
done
[ "$found_over" = 1 ] || { echo "  (none)"; rc=1; }

# Direction B: real collisions the description omits, in ordinary test commands.
echo
echo "== real collisions the description omits =="
found_under=0
for f in timeout json all tests goals owner commit push prior mutants swarm; do
  if printf '%s\n' "$real" | grep -qxF "$f" && ! printf '%s\n' "$documented" | grep -qxF "$f"; then
    echo "  --$f  (registered at: $(grep -nE "fs\\.[A-Za-z0-9]+\\((&[A-Za-z0-9_.]+, )?\"$f\"" cmd/corral/certify_repo.go | head -1 | cut -d: -f1))"
    found_under=1
  fi
done
[ "$found_under" = 1 ] || { echo "  (none)"; rc=1; }

echo
echo "== consequence for real test-command values =="
for tok in -timeout -json --record; do
  if collides "$tok"; then verdict="REFUSED, exit 2"; else verdict="accepted, runs"; fi
  echo "  token $tok -> $verdict"
done
echo "  so: test-command 'go test ./... -timeout 30m'  -> exit 2, undocumented"
echo "      test-command 'go test -json ./...'         -> exit 2, undocumented"
echo "      test-command 'cypress run --record KEY'    -> runs fine, though action.yml:9"
echo "                                                    calls it 'the live collision'"

if [ "$rc" = 0 ] && collides -timeout && collides -json && ! collides --record; then
  echo
  echo "DEFECT: action.yml:9 both over-warns (--record) and under-warns (--timeout, --json)."
  exit 0
fi
echo "defect not demonstrated"; exit 1
what it printed
documented in action.yml as corral's own flags:
  --diff-base
  --min-kill-rate
  --preflight
  --record
  --substrate
  --top

== documented, but NOT a flag of certify --repo ==
  --record is named in action.yml:9 but is registered nowhere in cmd/corral/certify_repo.go:
      grep -c '"record"' cmd/corral/certify_repo.go = 0
      it belongs to certify --local: 112:	recordFlag := fs.String("record", "", "write a replayable tape of the run (the pool's reasoning beats, task lifecycle, and findings) to this JSON file — the same {events:[…]} shape the corralai.dev cockpit replays")

== real collisions the description omits ==
  --timeout  (registered at: 104)
  --json  (registered at: 77)
  --all  (registered at: 63)
  --tests  (registered at: 61)
  --goals  (registered at: 60)
  --commit  (registered at: 74)
  --push  (registered at: 80)
  --prior  (registered at: 90)
  --mutants  (registered at: 91)
  --swarm  (registered at: 75)

== consequence for real test-command values ==
  token -timeout -> REFUSED, exit 2
  token -json -> REFUSED, exit 2
  token --record -> accepted, runs
  so: test-command 'go test ./... -timeout 30m'  -> exit 2, undocumented
      test-command 'go test -json ./...'         -> exit 2, undocumented
      test-command 'cypress run --record KEY'    -> runs fine, though action.yml:9
                                                    calls it 'the live collision'

DEFECT: action.yml:9 both over-warns (--record) and under-warns (--timeout, --json).

refutation — codex STANDS

I compared action.yml:9 with runCertifyRepo and checkArgvNoFlagCollision. The guard visits the certify --repo FlagSet, which includes timeout and json, while record is not registered there. Thus the named Go test arguments collide and --record does not, contrary to the description’s parenthesized list and explicit Cypress warning.

ref 1174024081d1de39e5374a837e5626ae3760abec0ad0d1f606a85ec7f28e8768#R2

R3 medium no script

The step sets motherduck_token — a real credential variable name — unconditionally from a possibly-empty input, so a workflow that supplies the token at job/workflow level has it blanked by the step and an md: push fails for a missing token that was present; the same script goes to explicit lengths (lines 208-264) never to export an empty value for any model API key.

where
action.yml:178
tier
CODE-READ (declared CODE-READ)

refutation — codex STANDS

I checked the Run corral step’s env map and credential setup. action.yml:178 assigns motherduck_token directly from the optional input, so an omitted input produces an empty step-level value that shadows an ambient variable of the same name. Unlike provider keys, motherduck_token is not passed through declare_key’s empty-value-preserving logic.

ref 1174024081d1de39e5374a837e5626ae3760abec0ad0d1f606a85ec7f28e8768#R3

R4 low no script

CORRAL_ATTEST_PATH is written to $GITHUB_ENV at line 290, before the test-command validation that can exit 1 at lines 331/343/389, so a run that never reaches corral still triggers the predicate step, which then warns "see the run step's own warning for why" — pointing the reader at a corral warning that was never emitted because corral never ran.

where
action.yml:290
tier
CODE-READ (declared CODE-READ)

refutation — codex STANDS

I followed the ordering and later step condition. CORRAL_ATTEST_PATH is appended to GITHUB_ENV at action.yml:290, while several test-command validation exits occur afterward and before corral is invoked. The always()-guarded predicate step is selected whenever that environment value exists and, on the absent statement, emits wording that specifically directs the reader to a run-step warning even though these paths emitted an error and no corral warning.

ref 1174024081d1de39e5374a837e5626ae3760abec0ad0d1f606a85ec7f28e8768#R4

R5 low no script

attest and push-source are compared with a literal string equality against "true", so any other truthy spelling an operator writes (yes, on, 1, TRUE) silently disables the receipt or the source push rather than being refused, contrary to the file's own stated standard elsewhere that a bad value is "refused rather than silently ignored".

where
action.yml:288
tier
CODE-READ (declared CODE-READ)

refutation — codex REFUTED

The claim assumes yes, on, 1, and TRUE are valid “truthy spellings,” but composite-action inputs here are strings, not YAML booleans with implicit truth coercion. The attest description at action.yml:42 expressly says to set the input to the literal string 'true'; action.yml:288 implements exactly that documented contract. push-source likewise has the string default "false" and is implemented as an explicit opt-in flag. No repository-wide promise says every noncanonical spelling of a string-valued toggle must be rejected—the quoted “refused rather than silently ignored” language applies to specific invalid mappings or names elsewhere. Therefore silently treating undocumented strings as disabled does not establish the stated honesty defect.

ref 1174024081d1de39e5374a837e5626ae3760abec0ad0d1f606a85ec7f28e8768#R5

Looked at and could not break (12)

Recorded so that "found nothing here" can be told apart from "never looked here".

Reviewer's opinion

action.yml is unusually well-defended at the shell level — the test-command parser, the key-collision refusal, the pipefail/exit-status handling and the base-ref fetch all survived the doors I could find, and several are already pinned by real tests in cmd/corral/action_test.go. The defects I found are in the two places nobody has a test pointed at: the file's PROSE, and the reporting side channel that was added last. Finding 1 is the reporting one — append_summary bounds each report to 900,000 bytes to respect GitHub's 1MiB summary cap, but the cap is on the whole file and the function is now called once for the audit plus once per review scope, so the exact failure the bound exists to prevent (the summary rejected outright, verdict and all) is reachable with a single review scope and no truncation notice anywhere. It is the repo's own recurring shape: a guard applied per-call to a resource that is shared. Finding 2 is the test-command description, which states the reserved-flag constraint wrongly in BOTH directions — it names --record as 'the live collision' when --record is not a flag of `certify --repo` at all (it belongs to `certify --local`, which this action never runs), while omitting --timeout and --json, which are, so `go test ./... -timeout 30m` and `go test -json ./...` are refused with exit 2 by a rule the docs say does not cover them. The guard in the code derives its names from fs.VisitAll precisely so it cannot drift; the sentence describing it was hand-written and did. Finding 3 is the same one-door/other-door shape as 1: the script goes to real trouble never to export an empty credential (lines 257-264), and then wires motherduck_token — the one real credential variable name in the env block — straight to a possibly-empty input at line 178. Findings 4 and 5 are small honesty defects. I did not find a way to make the audit itself pass green when it should fail, and I did not find a shell-injection path.

Verifier's opinion

R1–R4 stand: the cited control flow and shared-resource behavior support each claim, and I found no narrower implementation detail that defeats the named inputs. R5 is refuted because it invents undocumented “truthy” aliases for string-valued composite-action inputs; the action expressly documents the enabling value as the literal string 'true'.