feat(claude-lane-incident-aggregator): write the incident issue with the ambient token

[kyle-sexton]

Summary

The aggregator maintains its incident issue in this repository, so every
create, update, and close is a same-repo write: create-issue-from-file leaves
repository at its default of ${{ github.repository }}, and the close step
names context.repo. A same-repo issue write needs nothing but issues: write
on the ambient GITHUB_TOKEN
(workflow-syntax,
the official example),
so the aggregator can maintain its issue today, with no App and no new secret.

The App credential seam survives for the half that genuinely needs it. The
ambient token 404s on a private consumer repository (verified live against
melodic-software/medley), while public cross-repo reads succeed:

credential	role
ambient GITHUB_TOKEN (always)	the write to this repository's incident issue; every cross-repository read of a public consumer
App installation token (only with CLAUDE_LANE_INCIDENT_APP_PRIVATE_KEY)	cross-repository reads of private consumers

The App token is minted read-only, because it authors nothing — and the
audit now refuses a minted write permission anywhere in the file, because a
token minted with one is authority permissions: does not govern.

The design: the write is its own job

Granting issues: write retires the guarantee that rested on the job holding no
write scope at all. That guarantee is restored rather than replaced, by
splitting the workflow:

job	holds	does
poll	only read scopes	checkout, App-token mint, the polls, the inline github-script steps, the shell, the dry-run report. Renders the incident body and hands it on as an artifact.
write	issues: write (+ contents: read)	three steps: download that artifact, create-issue-from-file, close a recovered incident. No checkout, no run:, no repository code.

Nothing in poll can mutate anything with its own token, however its steps
are wired — so its steps do not have to be classified as safe. That is the whole
point, and it is why this shape replaces the previous one.

Two honest qualifications, both now stated in the workflow header rather than
implied away. poll still decides what write acts on: the incident body,
through the artifact it uploads, and the target issue, through the
issue-number output — guarded only by a positive-safe-integer parse. And a
job's permissions: govern only the ambient token, so a credential MINTED
inside poll would write regardless; that is why the credential surface is
pinned rather than reasoned about.

The dry-run gate is write's job-level if:, written inline rather than shared
through a job env var because env is not an available context in
jobs.<job_id>.if — that key sees only github, needs, vars and inputs
(context availability).
The dry-run report step in poll carries the exact negation, and a test
extracts both operators and asserts they are opposed, because the shared
expression that used to guarantee it no longer exists.

!= 'true' fails open, deliberately: anything that is not exactly the
string true enables writes. github.event.inputs is absent on schedule, so
the hourly cycle writes; only a dispatch that explicitly asked for a dry run
does not.

An adversarial pass against the FIRST cut of this split found the mint rule
keyed on a single action name, so a different App-token action spent the same
allowlisted private key and minted issues: write inside the read-only job with
the audit reporting nothing. That is fixed on the right axis — the rule is now
over the SECRET, everywhere in the file, not over an action name — and the same
change closed a PAT bound in a workflow-level env:, a job-level env:, or a
job outputs:, none of which the step-walking scan had ever looked at.

Why not keep hardening the previous gate

The previous revision kept issues: write on one 9-step job and tried to make
that safe by classifying every step — a hand-written YAML parser plus
bespoke shell-command and JavaScript-identifier scanners. Three independent
adversarial passes each found a fresh bypass of it:

• if bash /tmp/x.sh; then … — the shell scan took the first word of a
  fragment, so a compound-command keyword swallowed the real command
• a second job minting its own App token with permission-issues: write — never
  audited at all, because that job's own permissions: were read-only
• github.paginate("DELETE /repos/…") — an allowlisted call whose route is a
  string literal, and string literals were blanked by design
• /[/*]/ and /`/ — regex literals the code-stripper mislexed, silently
  blanking real code
• const send = true ? fetch : null — a ternary's colon disguising an evaluated
  identifier as an object key
• plus the original six (bare-hyphen step, duplicate step name, - name: decoy
  in a block scalar, implicit-POST curl, github.rest aliasing, local ./
  action)

That is not a run of unlucky misses. A hand-rolled YAML parser plus custom shell
and JS scanners is a larger trusted computing base than the thing it
protects, and it kept yielding. So the scanners are deleted, not fixed.

What the audit does now

claude-lane-incident-write-gate.cjs no longer classifies steps. It proves the
shape:

• every job's effective permissions are resolved — a job with no
  permissions: inherits the workflow block; permissions: write-all is a
  string, not a mapping, so a mapping-scan would read it as granting nothing;
  and permissions that cannot be resolved from the file are treated as write;

• exactly one job is write-scoped, and it is write. This is a POSITIVE
  count: zero write-scoped jobs is a hard failure, because "looked, found
  nothing to check, passed" is the shape every bypass above produced;

• that job matches claude-lane-incident-write-job.pinned.yml byte for
  byte, so any edit to it — of any kind — fails CI and is read by a person;

• the on: trigger block is pinned, so a pull_request_target cannot quietly
  start running an issues: write job in an attacker-influenceable context;

• the concurrency block is pinned, so parallel runs cannot be made to race one
  issue body;

• no job declares container, services, defaults, strategy,
  environment, continue-on-error, a job-level uses:, or a non-literal
  runs-on; every job declares a positive timeout-minutes; no step declares
  continue-on-error;

• every uses: is a pinned owner/repo@<40-hex sha>;

• every credential reference in the whole file — the secrets context in
  any spelling, and github.token — lies inside one of two pinned regions — the write job, or the mint step, which is pinned byte-for-byte for
  the same reason. The match is on the bare WORD secrets, case-insensitively:
  expressions reach a context by property de-reference or by the [ ] index
  operator, so secrets['NAME'] names a secret without a dot, and
  toJSON(secrets) names none of them while dumping all of them. Matching the
  word covers every spelling at once rather than enumerating them.

  Every rule is pinned by a fixture. Verified by neutering each of the audit's
  22 violations.push sites one at a time and confirming a fixture stops
  matching its asserted message — a rule no fixture pins is a rule that could be
  deleted with CI staying green. Two poll-side expressions are permitted outside them and are
  themselves pinned as whole expressions: the != '' presence probe, which
  discloses only whether a secret is set, and the read fallback, which binds the
  ambient token into a job already proved to hold no write scope.

The YAML parser is kept — a verifier explicitly failed to break it, and
resolving permissions and enumerating jobs needs a real parse. Everything else
is gone: the identifier allowlist, the shell-command allowlist, the member-path
pins, the reflection rules, and the expression evaluator.

Test plan

• node --test .github/scripts/*.test.cjs — 475 pass, 0 fail (was 422).

• actionlint — clean.

• biome ci --error-on-warnings .github/scripts (v2.5.4, as ci.yml runs it) — clean.

• zizmor — advisory findings unchanged before/after; all pre-existing.

• comment-hygiene, editorconfig-checker, typos — no finding in a changed file.

• Fixture corpus: 43 complete workflows under
  .github/scripts/fixtures/claude-lane-incident-write-gate/, each fed to the
  audit's entry point and asserting the message its own rule produces — so a
  fixture rejected for failing to parse cannot be mistaken for one that tripped
  its rule. base.yml conforms (zero violations), so every other fixture
  differs from a passing workflow by exactly one property. Corpus entries are
  architecture-independent: they outlive a redesign of the audit in a way a test
  named after a defeated exploit does not.

  Coverage: the six original exploits, every bypass found since, and each
  carry-forward — inherited permissions, the write-all string form, a
  reusable-workflow job, a minted write permission, a mint by a DIFFERENT
  App-token action, a PAT in a job env: / a workflow env: / a job outputs:,
  continue-on-error on a job and on a step, a rewritten concurrency, a
  removed timeout, an added trigger, a non-literal runner, a shell override, an
  unpinned uses:, a YAML anchor, and zero write-scoped jobs.

• Parser, differentially validated against yq v4.53.3: 75 of 76 repo
  YAML files produce a byte-identical JSON projection, plus 36 of 36
  hand-built shapes. The exception is standards-sync.yml, which uses a real
  YAML anchor and is rejected loudly rather than approximated. Two divergences
  the fuzzing found (007 / 1e3 scalar resolution, |+ chomping) are fixed.

Verified rather than assumed

• env is not available in jobs.<job_id>.if — checked against the
  contexts-availability table, which is what forced the gate inline.
• actions/github-script reads github-token with required: true (checked
  against the pinned revision's own src/main.ts), so a script step cannot be
  handed an unauthenticated client. That is part of why write is byte-pinned
  rather than analyzed.
• An empty issue-number reaches create-issue-from-file as Number('') === 0,
  which is falsy, so it takes the CREATE branch — the first-ever incident opens
  an issue rather than erroring. Read from the pinned revision's own
  src/main.ts, and unchanged from the pre-split behavior.
• Same-run download-artifact needs no permissions: scope: the toolkit's
  artifact package requires actions: read only for the cross-run/cross-repo
  findBy path. This is inferred from that documented split rather than from an
  explicit "same-run needs nothing" statement, so it is flagged as an inference.
• The write job keeps contents: read alongside issues: write. Neither
  create-issue-from-file nor download-artifact documents its scope
  requirement, the single-job form ran them with it, and a permissions: block
  sets every unlisted scope to none. Dropping it on an inference would trade a
  security property worth nothing — a read cannot mutate — for a runtime failure
  on a schedule nobody is watching. issues remains the only write, which is
  the property the audit enforces.

Related

No linked issue — this closes nothing on its own. It relates to #228 and #238:
it removes the credential blocker that kept the Phase 4 watchdog dry-run-only,
while leaving the App seam in place for the private cross-repository reads those
issues still need.

Follow-ups this PR does not do:

• .github/CODEOWNERS names an owner for .github/scripts/** and this
  workflow, because the contract lane globs *.test.cjs — a deleted test file
  yields fewer tests and still reports green, and no assertion survives its own
  deletion. Requiring that review is a ruleset setting owned by github-iac.
• The claude-lane-incident label is applied on creation only and must be
  declared in github-iac's Labels.cs, or the next apply prunes it. Issue
  selection never keys on the label, so a pruned label degrades a sanity query
  and nothing else.

🤖 Generated with Claude Code

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/ci-workflows/actions/runs/30638725380
• Failure class: rate-limit
• Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":507,"total_cost_usd":0,"api_error_status":429,"class":"rate-limit"}

Re-run the job to retry the review. A new push does not re-trigger this lane.
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e1117edd4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[kyle-sexton]

Independent verification: DO NOT MERGE AS-IS — the replacement guarantee does not hold

A fresh-context verifier audited this PR and returned two failures. The shipped workflow is correctly gated today (both current writers carry the gate; no other current step mutates), so this is not "a dry run can write now". What fails is the durability claim — and it fails while the header instructs reviewers to stop looking.

FAIL — the write-gate partition is defeatable

The partition is enforced by regex over raw YAML text, not over parsed YAML. Six independently-constructed ungated write steps left the suite GREEN and were actionlint-clean:

1. Bare-hyphen sequence item, appended — the bullet regex /^ {6}- (.*)$/ requires a space after the hyphen, so the step is never enumerated; past the last writer its text is absorbed into a registered writer's stepSource, which is never mutation-scanned. (Mid-file this correctly goes RED; only the tail slot survives.)
2. Duplicate step name, appended — step names need not be unique. stepSource returns the first match (the real gated step, satisfying the asserts) and the enumerator skips registered names, so the impostor is never checked.
3. - name: decoy inside a block scalar — stepSource's terminator matches at any indent, including inside script: |, truncating the slice before the mutating call. Works anywhere in the file.
4. Implicit-POST curl in a run: step — the curl regex requires -X/--request; -d alone is an implicit POST. A run: step has no uses:, so the allowlist loop iterates zero times.
5. One binding + bracket access — const api = github.rest; api.issues[verb]({…}) defeats the .rest.<ns>.<verb>( chain match. The test's own comment claims it is "written against the shapes rather than the endpoints"; it is written against one spelling.
6. Local action — the allowlist regex /^\s*uses: ([^@\s]+)@/ never matches ./…, so the step is cleared. (Honest caveat: making this execute a write needed a companion local action file — the regex hole is unconditional, the working exploit was two files.)

Non-vacuity confirmed: unnamed steps, third-party writers, plain github.rest.issues.createComment, and shapes 1–2 inserted mid-file all correctly go RED.

FAIL — the header now carries a false universal claim

Lines 41–44, verbatim: "A new write step therefore cannot reach main ungated — unregistered it fails the no-mutation check, registered it fails the gate check. A reviewer confirms 'a dry run cannot write' by reading one env line and one test, not by tracing every branch."

Six counterexamples above. This is the load-bearing failure: it is the sentence telling a future reviewer that branch-tracing is unnecessary. The same false universal is repeated in the test file's write-gate section header.

PASSES worth recording

• Scope: exactly one permission delta, issues: read → write; nothing else widened. Top-level block unchanged.
• App seam intact: the mint step and both readers still work; the write path is App-independent (both writers author with GITHUB_TOKEN), so an App-less run can write — which is the intended outcome.
• Dry-run suppression holds: verified against the shipped expression for scheduled (no inputs), dry-run=true, and dry-run=false; writers and the report step are mutually exclusive by construction.
• actionlint clean; diff limited to the two intended files; full suite 422 pass / 0 fail.

Remediation direction (from the verifier)

Parse the workflow with a real YAML parser and enumerate steps structurally — that alone kills survivors 1–3 and makes the name: assertion unnecessary. Require every uses: to match a pinned owner/repo@<sha> form, rejecting ./… — kills survivor 6. Survivors 4–5 are inherent to scanning text for capability; the structural fix is to invert the rule: in a job holding a write scope, every step must be either a registered gated writer or match an explicit allowlist of read-only step shapes, rather than being cleared by merely failing to match a mutation pattern.

Also flagged: evaluateWriteGate's comment claims it evaluates "the SHIPPED gate expression rather than a restatement of it" — it regex-parses the expression and applies its own JS semantics. Outcomes agree, so not behavior-affecting, but it is the same class of defect as the header claim.

Author's note: this session has exhausted its model budget (resets Aug 4), so I cannot land the remediation myself. Recording it here so the finding is durable rather than lost with the session.

[claude]

Claude finished @kyle-sexton's task in 6m 14s —— View job

---

Security review complete

Scope: the 5 files changed in this PR (workflow permissions/credential changes, the new YAML parser, and the new write-gate audit script). Diff taken against 78dccef (the commit this branch forked from — origin/main...HEAD had no merge base in this shallow clone, but the file-level diff matches the PR's stated stats exactly).

I independently re-verified the six write-gate bypasses @kyle-sexton's earlier audit found (bare-hyphen tail item, duplicate step name, - name: decoy inside a block scalar, implicit-POST curl, alias+bracket-access Octokit call, local ./action uses). All six are closed by the structural YAML parser (workflow-yaml.cjs) and the rewritten default-deny audit (claude-lane-incident-write-gate.cjs) introduced in 83a33e9/00aacbe. Good rewrite — worth saying plainly, since the header's durability claim rests entirely on it holding.

Two findings, both posted inline with full detail and remediation direction:

IMPORTANT — object-spread aliasing evades the github-script capability scan
SCRIPT_ROOTS's lookbehind excludes a preceding . so path fragments like .github/scripts/... don't false-positive — but a JS spread ({...github}) also leaves a . immediately before the identifier, so const clone = {...github}; clone.rest.issues.createComment(...) in a non-writer step's script is invisible to the scan. This defeats the same property the PR just finished hardening. Confidence: high on the regex evasion (statically traced); the Octokit-runtime half (spread preserving working bound rest/paginate methods) is high-confidence but not execute-verified — Bash execution in this session required interactive approval that wasn't available.

IMPORTANT — losing the App credential mid-incident can silently auto-close a fleet-wide incident
This PR deliberately decouples WRITES_ENABLED from HAS_APP_CREDENTIAL so ambient-token writes work App-less — correct for open/update. But it means a credential loss mid-incident narrows polling to this repo alone without signaling that to classifyCycle, so this repo's own clean lane can rack up 3 clean cycles and auto-close an incident about private repositories that were never re-checked. Same defect @chatgpt-codex-connector already flagged on this PR (P1, .yml:135) — I traced it independently end-to-end through claude-lane-incident.cjs's classifyCycle/nextState and confirm it's real. Not externally exploitable (requires an org-level credential change), but it's a genuine gap in the incident-tracking system's own integrity.

No other issues found. Scope/permissions changes are otherwise sound: exactly one scope widened (issues: read → write), justified by the same-repo-write argument in the header; the App token is correctly minted read-only now; both uses: pins are unchanged SHA pins; no pull_request_target/workflow_run-with-secrets pattern; no credential leaves the job (persist-credentials: false retained, both writers hardcode secrets.GITHUB_TOKEN rather than any App-token path).

I did not implement fixes — this was a review-only request.

[claude]

Claude finished @kyle-sexton's task in 6m 23s —— View job

---

Security review complete

Scope: full re-review of the 5 files changed in this PR, against daec763 (HEAD after the newest push, "default-deny the inline-script scan too"). Focus: whether this commit closes the two IMPORTANT findings from the prior review round.

• Diff the PR against origin/main
• Re-check the object-spread aliasing finding (claude-lane-incident-write-gate.cjs) against daec763
• Re-check the App-credential-loss auto-close finding (claude-lane-incident-aggregator.yml)
• Sweep the rest of the diff for new logic flaws, injection surfaces, credential handling, permission changes
• Post findings as a review

Result: this push does not close the durability gap. It substantially hardens the free-identifier scan (undeclared globals like fetch, __original_require__, globalThis are now denied by absence; comments/strings are stripped before scanning) — genuine progress. But the underlying design gap is unchanged: the scan trusts any locally declared name unconditionally, without tracking what value flows into it. Traced two concrete bypasses that still leave auditWriteGate clean on a step that mutates the incident issue outside the registered writers:

CRITICAL — alias-then-use defeats both the Octokit-surface allowlist and the require() allowlist — posted inline with full trace and two PoCs:

• const clone = {...github}; clone.rest.issues.createComment(...) — the previously-reported object-spread bypass, still live; the (?<![\w$.]) lookbehind shared by IDENTIFIER/MEMBER_PATH still can't tell ...github (spread) from obj.github (member access), so the reference is invisible to the scan.
• const req = require; req("node:child_process").execSync(...) — new this round, and language-guaranteed rather than Octokit-shape-dependent: requireArguments() matches the literal substring "require(" in the raw script text, so calling through any alias skips its allowlist entirely, reaching arbitrary Node modules.

Confidence: the require-alias PoC is essentially certain (pure JS reference semantics, no runtime assumption). The github-spread PoC is high-confidence on the regex-evasion half (statically traced) with the Octokit-runtime half unexecuted, same as last round — this sandbox's Bash tool needed interactive approval that wasn't available in this session.

IMPORTANT — App-credential-loss auto-close gap is still open (repeat finding, posted inline for visibility) — confirmed this push's diff to the workflow file is header-comment-only, so the gap @chatgpt-codex-connector and I both flagged previously (losing the App credential mid-incident narrows polling to self-only with no signal to classifyCycle, letting a clean self-lane silently auto-close a fleet-wide incident about unrevisited private repos) is unchanged.

No new issues found elsewhere in the diff. The rest of the rewrite holds up: the six previously-defeated bypasses (bare-hyphen tail step, duplicate writer name, block-scalar - name: decoy, implicit-POST curl, github.rest alias+bracket verb, local ./ action) are all still correctly closed by the structural YAML parser and the default-deny redesign; permissions delta is still exactly issues: read → write; no pull_request_target/workflow_run-with-secrets pattern; both uses: pins unchanged SHA pins; persist-credentials: false retained; writers still hardcode secrets.GITHUB_TOKEN rather than any App-token path.
· branch feat/aggregator-ambient-token-writes

[kyle-sexton]

Important

Superseded. This comment documents the step-classifying gate, which has
since been deleted. Two further adversarial passes each found another
bypass of it (if/then swallowing a command in the shell scan;
isWriteScoped skipping a job that mints its own App token;
github.paginate with a mutating route in a blanked string literal; two
regex-literal mislexes). The write now lives in its own job with the working
job holding no write scope at all — see the PR body and the follow-up comment.
The eleven entries below are retained because they are all now fixture-corpus
entries, and because the record of what defeated the previous approach is the
reason the approach changed.

---

Write-gate remediation: what each new test covers, and the before/after output

Following the independent verification above. The regex-over-raw-YAML gate is
replaced with a default-deny partition over a structurally parsed workflow, and
each defeat is now a regression test whose expectation names the mechanism
that catches it — so a fix that happened to reject the mutation for an
unrelated reason would not satisfy it.

.github/scripts/claude-lane-incident-write-gate.cjs holds the audit;
.github/scripts/workflow-yaml.cjs holds the fail-closed parser it reads
through. Both are new. The suite calls auditWriteGate(source) on the shipped
workflow (expecting no violation) and on eleven deliberately mutated copies.

The eleven defeats, and the test that covers each

#	defeat	what now catches it	test expectation
1	bare-hyphen sequence item appended past the last writer	structural enumeration — a bare - is a real sequence item, so the step is step 10 rather than text absorbed into step 9	step 10 … reaches 'github.rest.issues.createComment'
2	second step reusing a registered writer's name	positional enumeration plus a once-only check on each registered writer	declares the registered writer 'Close the recovered incident issue' 2 times
3	- name: decoy inside a script: | block scalar	the block scalar is one opaque string to the parser, so the step is not truncated	step 5 ("Advance the incident state") … reaches 'github.rest.issues.createComment'
4	implicit-POST curl carrying no -X	the run: command allowlist — every command-position word must be read-only, so no spelling of a network call has to be anticipated	runs 'curl', which is not a read-only command
5	const api = github.rest; api.issues[verb]({…})	exact member paths — github.rest is not an endpoint, so naming it at all is the violation	reaches 'github.rest', which is not a read-only surface
6	local ./ action	every uses: must be a pinned owner/repo@<40-hex sha>	must pin 'uses:' to owner/repo@<40-hex sha>; found './.github/actions/incident-mirror'
7	fetch(…, {method:"POST"}) authenticated from process.env["INPUT_GITHUB-TOKEN"]	the free-identifier allowlist — fetch is neither declared nor permitted (and INPUT_*TOKEN trips the credential scan independently)	names 'fetch', which is neither declared in the script nor a permitted global
8	__original_require__("node:https") — contains no require( for the argument allowlist to match	the same free-identifier allowlist	names '__original_require__', which is neither declared in the script nor a permitted global
9	String.constructor("return fetch")()	every permitted global's member paths are pinned, not just the Octokit handle's — any value reaches a Function constructor through its prototype	reaches 'String.constructor', which is not a read-only surface
10	lookup.issues[verb](…) — the property name appears nowhere in the source	a subscript that is not a plain number is refused, since a computed property name is one the scan cannot read	subscripts with 'verb', a property name this scan cannot read
11	const send = true ? fetch : null — an evaluated identifier wearing an object key's colon	a key is recognized by POSITION (a colon after it and a { or , before it), not by the colon alone	names 'fetch', which is neither declared in the script nor a permitted global

7–11 are not from the original list. They were found adversarially reviewing the
replacement, and they are all the same failure one level down: the step audit
had inverted, but the scan it applies to an inline script had not. It pinned a
few roots and left everything else to a denylist. actions/github-script
defaults github-token to the ambient token, so all three non-writing script
steps already hold write capability and their code is the only control.
exec.exec("gh", …), io and glob are the same class as 7; 9 and 10 are the
reflection routes that survive an identifier allowlist whose admitted globals
keep unexamined members; 11 is that allowlist's own key exemption, used as a
hiding place.

Before / after

Before — each mutation applied to this branch's pre-remediation head
(00e365e), running that revision's own suite:

exploit 1: GREEN (gate defeated)  actionlint clean  — a bare-hyphen step appended past the last writer
exploit 2: GREEN (gate defeated)  actionlint clean  — a second step reusing a registered writer's name
exploit 3: GREEN (gate defeated)  actionlint clean  — a '- name:' decoy inside a block scalar
exploit 4: GREEN (gate defeated)  actionlint clean  — an implicit-POST curl carrying no method flag
exploit 5: GREEN (gate defeated)  actionlint clean  — one binding plus bracket access to the verb
exploit 6: GREEN (gate defeated)  actionlint clean  — a local ./ action, which carries no pin to check
exploit 7: GREEN (gate defeated)  actionlint clean  — a network call through a global the script scan never enumerated
exploit 8: GREEN (gate defeated)  actionlint clean  — the unscanned alias of require, which bypasses its argument allowlist
exploit 9: GREEN (gate defeated)  actionlint clean  — reflection to a Function constructor through a permitted global
exploit 10: GREEN (gate defeated)  actionlint clean  — a computed property name, which no member scan can read
exploit 11: GREEN (gate defeated)  actionlint clean  — a denied global disguised as an object key by a ternary's colon

After — the same eleven mutations, now as tests, on this head:

✔ the partition rejects a bare-hyphen step appended past the last writer
✔ the partition rejects a second step reusing a registered writer's name
✔ the partition rejects a '- name:' decoy inside a block scalar
✔ the partition rejects an implicit-POST curl carrying no method flag
✔ the partition rejects one binding plus bracket access to the verb
✔ the partition rejects a network call through a global the script scan never enumerated
✔ the partition rejects the unscanned alias of require, which bypasses its argument allowlist
✔ the partition rejects reflection to a Function constructor through a permitted global
✔ the partition rejects a computed property name, which no member scan can read
✔ the partition rejects a denied global disguised as an object key by a ternary's colon
✔ the partition rejects a local ./ action, which carries no pin to check

Whole suite:

ℹ tests 442
ℹ pass 442
ℹ fail 0

(422 before; +9 parser tests, +11 defeat regressions, net of the replaced
pattern-matching tests.)

Other checks

• actionlint on the workflow — clean.
• biome ci --error-on-warnings .github/scripts (v2.5.4, as ci.yml runs it) — clean.
• zizmor — 5 findings, byte-identical before and after; all pre-existing.
• comment-hygiene, editorconfig-checker, typos — no finding in any changed file.
• The parser was differentially validated against yq v4.53.3 over every YAML
  file in the tree: 75 of 76 produce a byte-identical JSON projection, plus
  36 of 36 hand-built shapes covering quoting, chomping, folding, flow
  collections, indentation and scalar resolution. The one exception is
  standards-sync.yml, which uses a real YAML anchor and is rejected loudly
  rather than approximated — the fail-closed contract. Two divergences the
  fuzzing found (007 / 1e3 scalar resolution, and |+ chomping) are fixed.

What this does not claim — and why that is written down

Defeats 7–11 were all found in one place: the scan applied to an INLINE SCRIPT.
Four rounds of closing holes in a lexical JavaScript scan is the approach
telling you something, so the last change is not another round of hardening —
it is the bound, recorded in both headers.

The step partition is structural: it reads a parsed document, so the
enumeration of steps, the uses: pins, the conditions and the job shape are
facts about the file. The inline-script scan is not: it is a lexical
approximation over code with comments and string literals blanked, with no scope
resolution and no expression grammar. A binding form it fails to recognize costs
a red build; a construct it mislexes is an admission. Defence in depth with a
ceiling, not a proof.

That ceiling cannot be engineered away for two steps. actions/github-script
reads github-token with required: true — verified against the pinned
revision's own src/main.ts, not from recall — so a script step cannot be handed
an unauthenticated client. With no App credential configured, the poll and the
issue lookup hold the ambient token, which carries issues: write. Their
capability cannot be removed, only their code read. Both headers now say so and
ask for exactly that.

So: the audit proves no step other than the two registered writers can mutate
anything, and that both writers carry the gate. It does not decide whether the
gate is the right condition, whether a writer's remaining conditions are
right, or what the inline scripts actually do. The sentence telling reviewers
they need not trace every branch is gone — it was false, and it was the sentence
that stopped the looking.

The do-not-merge label is untouched, and removing it is not mine to do.

[claude]

Claude finished @kyle-sexton's task in 2m 28s —— View job

---

Security review complete

• Fetch latest diff (HEAD 1b42f05) against origin/main (fork point 78dccef)
• Check whether the CRITICAL alias bypasses (object-spread, require alias) from the prior round are addressed
• Re-check the App-credential-loss auto-close finding
• Sweep rest of the diff (new commit: plain-scalar/keep-chomping fix in workflow-yaml.cjs) for new issues
• Post findings

This push (1b42f05) only touches .github/scripts/workflow-yaml.cjs's scalar resolution (typing 007/1e3 per the YAML 1.2 core schema, and fixing keep-chomping to retain one more trailing break) plus its own tests. That fix is internal to the contract-test parser's scalar handling — it doesn't touch step/job structure, doesn't affect auditWriteGate's conclusions, and introduces no new security-relevant surface. It does not touch claude-lane-incident-write-gate.cjs or the workflow file at all.

Both previously-reported findings remain open, unchanged, since this push doesn't touch the affected files:

CRITICAL — alias-then-use still defeats both the Octokit-surface allowlist and the require() allowlist (posted inline, re-traced against current HEAD)

• const clone = {...github}; clone.rest.issues.createComment(...) — the shared (?<![\w$.]) lookbehind on IDENTIFIER/MEMBER_PATH still can't distinguish a spread's . from a member-access ., so github inside {...github} is invisible to the scan, and clone's member path is skipped because CAPABILITY_ROOTS only checks paths literally rooted at github/core/context/process.
• const req = require; req("node:child_process").execSync(...) — requireArguments() still matches on the literal substring "require(", so calling through any alias skips the argument allowlist entirely. This half is language-guaranteed (pure reference semantics), not Octokit-shape-dependent.
• Confidence: require-alias essentially certain; github-spread high-confidence on the regex-evasion half (statically verified), Octokit-runtime half unexecuted (no interactive Bash approval available in this sandbox, same as both prior rounds).

IMPORTANT — App-credential-loss auto-close gap still open (posted inline)
WRITES_ENABLED still depends only on dry-run, never on HAS_APP_CREDENTIAL. If the App credential is lost mid-incident while private consumer repos are the reason it's open, polling silently narrows to self-only with no signal reaching classifyCycle/nextState, so three clean self-only cycles can auto-close an incident about repos that were never re-checked. Not externally exploitable — requires an org-level credential change.

No new issues found in the diff beyond these two repeat findings. Everything else re-verified sound: the eight originally-defeated bypasses (bare-hyphen tail step, duplicate writer name, block-scalar - name: decoy, implicit-POST curl, github.rest alias+bracket verb, local ./ action, undeclared-global network calls) remain correctly closed by the structural parser and default-deny redesign; permissions delta is still exactly issues: read → write; no pull_request_target/workflow_run-with-secrets pattern; both uses: pins are unchanged SHA pins; persist-credentials: false retained; writers still hardcode secrets.GITHUB_TOKEN rather than any App-token path. (Supply-chain/pin/trigger/permission-widening classes are zizmor's lane and not re-litigated here; zizmor's findings are reported byte-identical before/after per the PR description.)

Given the do-not-merge label and the CRITICAL finding's durability implications for the header's own claim ("no step other than the two registered writers can mutate anything"), this PR should stay blocked until the alias-tracking gap in the write-gate audit is closed — via value-tracking through simple aliases/spreads of the capability roots and require, or a real JS parser per this PR's own precedent of replacing the YAML regex with a structural parser.
· branch feat/aggregator-ambient-token-writes

[claude]

Security review in progress

• Gather context (diff at HEAD e747de7 vs previously-reviewed a5d1345)
• Review this push's actual diff ("match the secrets context by word, not by dotted path")
• Re-verify prior open findings (bracket-notation secrets[...] gap, App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

View job run

[claude]

Claude finished @kyle-sexton's task in 1m 44s —— View job

---

Security review complete

• Gather context (diff at HEAD d18a398 vs previously-reviewed a5d1345)
• Review this push's actual diff (a5d1345→e747de7: "match the secrets context by word, not by dotted path"; e747de7→d18a398: whitespace-only)
• Re-verify prior open findings (bracket-notation secret gap, App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

d18a398 is whitespace-only — trailing space removed after services:/strategy: in two fixture files. No logic change.

e747de7 closes the previously-reported IMPORTANT (bracket-notation secrets[...]) gap, and closes it more generally than requested. SECRET_REFERENCE changed from /secrets\.[A-Za-z_][A-Za-z0-9_]*/gu (dotted-path only) to /(?<![\w-])secrets(?![\w-])/giu — the bare word, case-insensitive, with no dot required. Traced against auditCredentialSurface (L198-215), which still runs over the whole raw file text (not step-scoped), so this one change catches all of:

• secrets['NAME'] / secrets["NAME"] — bracket notation, the specific gap I reported on a5d1345 (new secret-bracket.yml fixture: secrets['FLEET_ADMIN_PAT'], confirmed present at .github/scripts/fixtures/claude-lane-incident-write-gate/secret-bracket.yml:38)
• toJSON(secrets) — dumps every secret while naming none individually (new secret-tojson-dump.yml fixture)
• SECRETS/mixed-case spellings, since context names aren't documented as case-sensitive (new secret-uppercase.yml fixture)
• any future spelling nobody has thought of yet, since matching the word rather than enumerating syntaxes is the actual fix, not a patch for one case

All three new fixtures are wired into the test corpus with the correct expectation (is a credential expression outside the pinned regions). I re-traced the regex by hand against each case (lookbehind/lookahead correctly exclude secrets-foo/foosecrets as distinct tokens, and correctly admit secrets[, secrets., (secrets)) — this is a sound structural fix, not a narrowing of the same hole.

Both items I was tracking as open are now resolved or accounted for:

• The credential-surface CRITICAL (job/workflow-level env: blind spot + bracket-notation) — closed, across this push and a5d1345.
• The alias-bypass CRITICAL ({...github} spread, require alias defeating the inline-script scan) — closed structurally by dfd6280's job split: poll now holds contents: read / issues: read / pull-requests: read only (.yml#L134-136), so no identifier trick in its scripts can mutate anything regardless of what the lexical scanner does or doesn't recognize — confirmed unchanged by this push.

Still open — IMPORTANT, unaddressed across nine review rounds: losing the App credential mid-incident can silently auto-close a fleet-wide incident (repositories = [context.repo...] self-only fallback, unsignaled to classifyCycle/nextState). This push's diff doesn't touch claude-lane-incident.cjs, HAS_APP_CREDENTIAL, or the self-only fallback — confirmed unchanged. Not externally exploitable (requires an org-level credential change).

No new issues found. Permissions delta across the whole PR remains issues: read → write on write only, plus contents: read; both uses: pins unchanged SHA pins; no pull_request_target/workflow_run-with-secrets pattern (schedule/workflow_dispatch only); persist-credentials: false retained; writers hardcode secrets.GITHUB_TOKEN.

Given the do-not-merge label, this PR should stay blocked on maintainer judgment for the one remaining IMPORTANT finding (incident-tracking integrity on App-credential loss) — the write-gate's own security properties (write-scope isolation, byte-pinning, credential-surface closure) all now hold up against every adversarial PoC raised across this thread.
· branch feat/aggregator-ambient-token-writes

[claude]

Claude finished @kyle-sexton's task in 2m 20s —— View job

---

Security review complete

• Gather context (diff at HEAD 2371720 vs previously-reviewed d18a398)
• Review this push's actual diff ("scan github.token, and pin every rule with a fixture")
• Re-verify prior open findings (App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (2371720) only touches .github/scripts/claude-lane-incident-write-gate.cjs, its test file, and three new fixtures (job-not-a-mapping.yml, step-not-a-mapping.yml, secret-ambient-github-token.yml). It does not touch the workflow file at all.

What it does, and it holds up:

• Adds AMBIENT_TOKEN_REFERENCE to catch github.token/github['token'] (case- and whitespace-insensitive), which SECRET_REFERENCE's word-match on secrets never reached — github.token is the ambient credential under a name that names no secrets context. I confirmed by grep that no current file in this PR (workflow, both pinned job/step files) actually uses github.token anywhere, so this closes a theoretical future gap rather than an active one — consistent with the commit message.
• Traced the regex for evasions: dotted (github.token), bracket (github['token']/github["token"]), whitespace/newline-tolerant (\s*), and case-insensitive (giu) are all covered. The lookbehind (?<![\w-]) correctly avoids false-matching inside a longer identifier like mygithub.token. The one theoretical miss is optional-chaining syntax (github?.token) — not valid GitHub Actions expression syntax, so it can't function as a real ${{ }} credential reference, and the octokit client actions/github-script binds as github doesn't expose a usable .token property in ordinary JS either. Not a real bypass, not worth blocking on.
• auditCredentialSurface now scans both patterns (CREDENTIAL_PATTERNS) over the same whole-file text and same pinned-region logic as before — the fix is structurally consistent with the existing secrets scan rather than a special case.
• The two new "not a mapping" fixtures (job value is a scalar; step value is a scalar) confirm the defensive branches at job level and step level continue past the malformed entry rather than crash or silently skip checks — and since any single violation fails the whole audit, a job/step that's "not a mapping" can't produce a false pass even though it's excluded from the write-scope tally for that iteration (I traced the case where poll is malformed and write is a well-formed sole writer: writeScoped still correctly resolves to ['write'], but the overall audit still fails on the separate "is not a mapping" violation, so there's no path to a clean report).

Still open — IMPORTANT, unaddressed across nine-plus review rounds (repeat finding; not touched by this push): losing the App credential mid-incident can silently auto-close a fleet-wide incident. HAS_APP_CREDENTIAL (L144) and the self-only polling fallback (L296) are unchanged; write's gate (L610) still depends only on dry-run. If the App credential becomes unavailable while an incident is open because of private consumer-repo failures, polling still silently narrows to self-only with no signal reaching classifyCycle/nextState in claude-lane-incident.cjs (untouched by this entire PR), so three consecutive clean self-only cycles can still auto-close an incident whose real root cause was never re-checked. Not externally exploitable (requires an org-level credential change).

No new issues found. The previously-reported CRITICAL findings (secret-surface job/workflow-level env: blind spot, bracket-notation secrets[...], the pre-job-split JS alias bypasses) all remain closed as of a5d1345/e747de7/dfd6280. This push doesn't touch the workflow file, so permissions delta remains issues: read → write on write only plus contents: read; no pull_request_target/workflow_run-with-secrets pattern (schedule/workflow_dispatch only); both uses: pins unchanged SHA pins.

I wasn't able to execute node --test in this sandbox (Bash requires interactive approval not available here), so the new fixtures/assertions are verified by static trace of the regex and control flow rather than by running the suite — same caveat as several prior review rounds on this PR.

Given the do-not-merge label, this PR's own write-gate security properties now hold up against every adversarial PoC raised across this thread. The one remaining blocker is the IMPORTANT incident-tracking integrity gap above, which is a maintainer-judgment call, not a defect in this push.
· branch feat/aggregator-ambient-token-writes

[kyle-sexton]

Redesigned: the write is its own job, and the scanners are deleted

Supersedes the comment above, which documents the step-classifying gate. That
approach is gone.

Why it changed

Three independent adversarial passes each found a fresh bypass of the
step-classifying gate — if/then swallowing a command in the shell scan;
isWriteScoped skipping a job that mints its own App token;
github.paginate("DELETE /…") with the route in a blanked string literal; two
regex-literal mislexes; a ternary's colon disguising an evaluated identifier.
A hand-rolled YAML parser plus custom shell and JavaScript scanners is a larger
trusted computing base than the thing it protects. It kept yielding, so it was
deleted rather than hardened again.

The shape now

job	holds	does
poll	only read scopes	checkout, App-token mint, the polls, the inline scripts, the shell, the dry-run report. Hands the rendered body on as an artifact.
write	issues: write (+ contents: read)	three steps: download that artifact, create-issue-from-file, close a recovered incident. No checkout, no run:, no repository code.

Nothing in poll can mutate anything with its own token, so its steps do
not have to be classified. The audit stopped classifying steps and now proves
the shape: exactly one write-scoped job as a positive count (zero is a hard
failure — "looked, found nothing, passed" is what every bypass produced); that
job pinned byte for byte; the mint step pinned the same way; the on: and
concurrency blocks pinned; and every credential reference in the whole file
confined to those two pinned regions.

What the last adversarial pass found, and what it could not

It found a real bypass of the first cut, in a rule my own header claimed to
enforce — verbatim:

MINT_ACTION is the single literal actions/create-github-app-token, so the
permission-* check fires only for that one action. ANY other App-token
action passes untouched — and it can spend the already-allowlisted private key.

It landed tibdex/github-app-token minting issues: write inside the read-only
job with the audit silent. Fixed on the right axis: the rule is over the
secret, everywhere in the file, not over an action name. Self-review then
found five spellings that walked past that — secrets['NAME'], its spaced and
double-quoted variants, SECRETS.X, and toJSON(secrets) — because expressions
reach a context by . or the [ ] index operator. The match is now on the
bare word, case-insensitively, plus github.token, which reaches no secrets
context at all.

On re-verification it returned NO BYPASS FOUND, and could not break: the
byte pin ("indexOf(MARKER) then compare-to-EOF is sound"), effective-permissions
resolution ("complete in both directions"), GITHUB_OUTPUT injection, or the
parser — "Differential against yq v4 over all 54 workflows + fixtures: 0
divergences." Two residuals it noted and I have recorded at the code: the
region and the byte comparison share one offset (that coupling is what defeats a
forged marker), and the two allowlisted poll-side expressions are safe only
because both halves are provably read-only.

The corpus

43 fixtures, each a complete workflow fed to the audit's entry point and
asserting the message its own rule produces — so a fixture rejected for
failing to parse cannot be mistaken for one that tripped its rule. base.yml
conforms, so every other fixture differs from a passing workflow by exactly
one property.

Every rule is pinned. Verified by neutering each of the audit's 22
violations.push sites one at a time and confirming a fixture stops matching:
20 were pinned, the two survivors were the defensive not-a-mapping branches, and
both now have fixtures. A rule no fixture pins is a rule that could be deleted
with CI staying green.

ℹ tests 475   ℹ pass 475   ℹ fail 0        (was 422)

Correctness of the split, not just its gate

• poll declares action and issue-number; write references exactly those.
• The artifact is uploaded on exactly the predicate that runs write, so
  if-no-files-found: error cannot fire spuriously.
• download-artifact v8 reads v4+ artifacts and extracts a single named
  artifact directly to $GITHUB_WORKSPACE, which is where content-filepath
  reads it.
• Same-run download needs no permissions: scope — github-token is documented
  as cross-run/cross-repo only.
• An empty issue-number reaches create-issue-from-file as Number('') === 0,
  which is falsy, so the first-ever incident takes the CREATE branch rather
  than erroring. Read from the pinned revision's own src/main.ts.

What this does not claim

poll cannot mutate anything with its own token, but it decides what write
acts on: the body, through the artifact, and the target issue, through
issue-number — guarded only by a positive-safe-integer parse. Both headers say
so now. poll also holds ACTIONS_RUNTIME_TOKEN, scoped to this run's artifact
and cache backend; write reads no cache.

Two things remain unproven and are flagged rather than asserted: whether a
claude-lane-incident label that does not exist in the taxonomy auto-creates or
is dropped (the follow-up for github-iac's Labels.cs is unchanged either way),
and CODEOWNERS names an owner for .github/scripts/** but requiring that
review is a ruleset setting owned by github-iac, not by this repository.

do-not-merge is untouched, and removing it is not mine to do.

[claude]

Claude finished @kyle-sexton's task in 1m 32s —— View job

---

Security review complete

• Gather context (diff at HEAD 5d1f0a5 vs previously-reviewed 2371720)
• Review this push's actual diff ("bind mapping keys as data, never as a prototype assignment")
• Re-verify prior open finding (App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (5d1f0a5) fixes a real prototype-pollution-style parser bypass and backstops it with a second, parser-independent check — both are genuine, well-targeted security fixes, not just hardening.

The bug it fixes: .github/scripts/workflow-yaml.cjs's mapping parser previously bound every key with plain mapping[key] = value. For key === "__proto__" with an object value, that reassigns the mapping's prototype instead of creating an own property — the entry vanishes from Object.keys/JSON.stringify while its contents leak onto the parent object. A GitHub Actions job id of __proto__ is legal and the runner enumerates it normally, so a job named __proto__ carrying permissions: { issues: write } would run for real while auditWriteGate's permission walk (which iterates Object.keys(document.jobs)) never saw it exist — the exact "looked, found nothing to check, passed" shape this PR's whole narrative has been fixing, one layer below where the previous rounds looked. All four binding sites (flow mapping via the new setKey helper at workflow-yaml.cjs#L216-224, plus block-scalar, inline, null, sequence, and nested-node assignment in parseMapping) are converted to Object.defineProperty, which is key-agnostic. Verified: grep -n '\[key\]\s*=' over the file now matches only the doc comment, no live assignment.

The backstop: claude-lane-incident-write-gate.cjs adds auditWriteGrants, a raw-text regex scan (WRITE_PERMISSION / WRITE_ALL) for any <key>: write or write-all grant, independent of the parser. Its stated purpose is explicit and correct: this rule "never learns what a job is, so no parse can hide a grant from it" — i.e. it's designed to survive a future parser bug of this same class, not just this one. It also subsumes the old single-action-name mint-permission check from prior rounds, since a minted permission-issues: write matches the same <key>: write shape.

Verification, not just trust: the new proto-job.yml fixture reproduces the exact exploit (a __proto__ job with issues: write) and the new test a write hidden from the parse is still caught by the raw-text scan asserts both layers independently catch it — the parse now correctly enumerates poll,__proto__,write (so the job-count check fires), and separately the raw-text scan names the grant. workflow-yaml.test.cjs adds a matching unit test directly on the parser (__proto__ in both block and flow mapping form, plus confirming the duplicate-key guard now sees __proto__ too, since Object.hasOwn works correctly once the key is bound as data). Good adversarial-style regression: it tests the mechanism, not just the outcome.

The claude-lane-incident-aggregator.yml diff in this push is comment-only (rewording "issues: write" to "the issues write scope" in two header paragraphs) — needed because the new WRITE_PERMISSION regex scans the entire file including comments, and the literal string issues: write in prose would otherwise trip the new self-check outside the pinned regions. Not a logic or security change.

Still open — IMPORTANT, unaddressed across ten-plus review rounds (repeat finding; this push doesn't touch it): losing the App credential mid-incident can silently auto-close a fleet-wide incident. HAS_APP_CREDENTIAL (L144) and the self-only polling fallback (L296) are unchanged, and confirmed git diff of .github/scripts/claude-lane-incident.cjs is empty both for this push and for the whole PR against origin/main — classifyCycle/nextState remain untouched. If the App credential becomes unavailable while an incident is open because of private consumer-repo failures, polling still silently narrows to self-only with no signal reaching the classifier, so three consecutive clean self-only cycles can still auto-close an incident whose real root cause was never re-verified. Not externally exploitable — requires an org-level credential change.

No new issues found. Everything previously closed remains closed: the credential-surface scan (job/workflow-level env:, bracket notation, github.token) closed across a5d1345/e747de7/2371720; the pre-job-split JS alias bypasses ({...github} spread, require alias) closed structurally by the poll/write job split in dfd6280 since poll holds no write scope regardless of script content; permissions delta remains issues: read → write on write only plus contents: read; no pull_request_target/workflow_run-with-secrets pattern (schedule/workflow_dispatch only); both uses: pins unchanged SHA pins; persist-credentials: false retained; writers hardcode secrets.GITHUB_TOKEN.

Given the do-not-merge label, this PR's write-gate security properties (write-scope isolation, byte-pinning, credential-surface closure, and now parser-independent write-grant closure) hold up against every adversarial PoC raised across this thread. The one remaining blocker is the IMPORTANT incident-tracking integrity gap above, which is a maintainer-judgment call, not a defect in this push.
· branch feat/aggregator-ambient-token-writes

[claude]

Claude finished @kyle-sexton's task in 2m 23s —— View job

---

Security review complete

• Gather context (diff at HEAD 3a946f5 vs previously-reviewed 5d1f0a5)
• Review this push's actual diff ("decide a scope by allowlist, and scan vars")
• Re-verify prior open finding (App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (3a946f5) fixes two real, narrow gaps in the write-gate audit — both are genuine hardening, not new risk.

1. isWriteScoped was a blocklist (Array.prototype.includes("write")), which fails open on any scope value it doesn't literally recognize. A YAML folded scalar — issues: > with write on the next line — resolves to the string "write\n", which is !== "write", so the old check silently read the job as read-only. The fix inverts to an allowlist (READ_ONLY_SCOPES = {"read", "none"}): any scope value that isn't provably read-only counts as a write. I traced this by hand — Object.values(permissions).some(scope => typeof scope !== "string" || !READ_ONLY_SCOPES.has(scope)) correctly flags "write\n", non-string values, and any unanticipated spelling, defaulting to unsafe rather than safe. The new poll-permission-block-scalar fixture reproduces the exact bypass and asserts the job is now caught (found [poll, write]) — verified the fixture's issues: >\n write block scalar matches the described shape.

2. The credential-surface scan covered secrets and github.token but not vars. A PAT bound into a job env: via ${{ vars.SOME_TOKEN }} reaches every run:/script step in scope exactly like one from secrets, but was invisible to auditCredentialSurface. VARIABLE_REFERENCE closes it with the same bare-word, case-insensitive approach already used for secrets. Confirmed by grep that neither the shipped workflow nor either .pinned.yml file references vars/vars./vars[ anywhere, so this introduces no false positive against the current shipped file. The companion workflow-header edit (replacing a literal restatement of the jobs.<job_id>.if context list — which named vars in prose — with a link to GitHub's docs) is necessary and correct: without it, the new scan would trip on the header comment itself, since auditCredentialSurface scans the whole raw file text, not just executable YAML.

Both fixes are honestly scoped, not oversold — the new header addition (L35-43) explicitly names two remaining bounds (uses: pins checked by shape not identity; the credential scan matches three specific context words, so a fully-obfuscated expression like github[format('tok{0}','en')] could still evade it) and correctly notes both are harmless only because poll holds no write scope — consistent with the architecture, not a new gap.

No new issues found. The pinned files (claude-lane-incident-write-job.pinned.yml, claude-lane-incident-mint-step.pinned.yml) are untouched by this push (confirmed via diff). Fixture corpus grew from 45 to 47 non-base fixtures (confirmed: 48 files in the fixtures directory including base.yml), matching the commit message.

Still open — IMPORTANT, unaddressed across eleven review rounds (repeat finding; this push doesn't touch it): losing the App credential mid-incident can silently auto-close a fleet-wide incident. HAS_APP_CREDENTIAL, the self-only polling fallback, and claude-lane-incident.cjs's classifyCycle/nextState remain untouched by this entire PR (that file isn't in the PR's changed-files list at all). If the App credential becomes unavailable while an incident is open because of private consumer-repo failures, polling still silently narrows to self-only with no signal reaching the classifier, so three consecutive clean self-only cycles can still auto-close an incident whose real root cause was never re-verified. Not externally exploitable (requires an org-level credential change).

I wasn't able to execute node --test in this sandbox (Bash requires interactive approval not available here), so verification of this push is by static trace of the regex/allowlist logic and fixture content rather than a live test run — same caveat as most prior review rounds on this PR.

Given the do-not-merge label, this PR's write-gate security properties (write-scope isolation via allowlist rather than blocklist, byte-pinning, credential-surface closure across secrets/github.token/vars) now hold up against every adversarial PoC raised across this thread. The one remaining blocker is the IMPORTANT incident-tracking integrity gap above, which is a maintainer-judgment call, not a defect in this push.
· branch feat/aggregator-ambient-token-writes

[claude]

Claude finished @kyle-sexton's task in 3m 1s —— View job

---

Security review complete

• Gather context (diff at HEAD da25244 vs previously-reviewed 3a946f5)
• Review this push's actual diff ("prove a clean cycle observed what the incident implicates")
• Re-verify prior open finding (App-credential-loss auto-close)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (da25244) closes the IMPORTANT finding that has been open across twelve prior review rounds on this thread (App-credential loss silently auto-closing a fleet-wide incident about private repos that stopped being polled) — and it's a real, well-verified fix, not another mitigation of a symptom.

What it does: classifyCycle never had scope as an input, so a narrowed poll (self-only, after the App credential vanishes) that finds nothing wrong looks identical to a genuinely healthy fleet — no read error is raised, because nothing failed; the poll just asked a smaller question. This push adds a second, independent gate on top of classifyCycle: coverageGap checks the current cycle's actually-polled repository list against the open incident's own persisted evidence (previous.repositories), and nextState refuses to advance the clean-cycle counter — or write anything — when coverage is incomplete, exactly mirroring how indeterminate already behaves (hold, don't reset, don't advance).

I traced the wiring end to end and it holds:

• poll now publishes polled-repositories (.yml#L467) as the exact array the poll loop iterates (.yml#L293-L310) — including the self-only fallback branch — not a derived count, so it can't drift from what was actually read.
• The "Advance the incident state" step defaults an absent/malformed POLLED_REPOSITORIES to [] on a parse failure (.yml#L539-L546), so a plumbing mistake proves no coverage and fails toward holding the incident open, never toward closing it — correct fail-direction for a security-relevant gate.
• coverageGap compares names case-insensitively and trimmed on both sides (L389-L394), avoiding a false-negative "gap" from GitHub's case-insensitive repo names or human-typed dispatch input.
• Both this step's step (poll job) and the write job's byte-pinned region are unaffected — confirmed the diff touches only lines inside poll, and the write-gate's pin mechanism (WRITE_JOB_MARKER/MINT_MARKER, both indexOf-anchored, not line-number-anchored) is immune to changes earlier in the file. write/mint-step permissions and shape are untouched.
• Names entering coverageGap/describeCoverageGap's warning text come only from previous.repositories, which parseStateBlock→mergeRepositories already filters through isValidRepository (the strict owner/name grammar) before they're stored — so the new warning-message path doesn't reopen the log/body-injection contract this module's header is explicit about; the annotation-text/attacker-controlled-string boundary is unchanged.
• The three failure modes (unobserved, unlisted from the bounded 60-repo index, unparsableState for a hand-edited/pre-schema body) all correctly route through the same action: "none" value already used for indeterminate, so no new value reaches the write job's gating conditions (.yml#L626, .yml#L652) that could accidentally start gating on it.
• Test coverage is substantive, not just unit-level: claude-lane-incident-aggregator.test.cjs executes the shipped step script text (via extractStepScript) against the real env wiring rather than calling nextState directly — the same discipline already applied to the poll step — so a mis-wired env: key or an unparsed scope would fail a test rather than staying invisible. claude-lane-incident.test.cjs adds an explicit end-to-end regression (losing the App credential mid-incident cannot auto-close the incident) that walks the exact scenario this finding described, holds for 5 narrowed cycles, then confirms the same evidence closes it once coverage returns.

Honest documented tradeoff, not a defect: a legitimately-gone repository (archived, uninstalled) is indistinguishable from a lost credential, so a gap now holds the incident open indefinitely rather than auto-closing — the commit and header are explicit that this is the deliberate fail-direction (a stalled, visible incident over a silent false-recovery). Not a security issue.

No new issues found. This diff doesn't touch permissions, uses: pins, triggers, or the credential surface; poll remains read-only and write/the mint step remain byte-pinned and untouched. No injection surface, no widened scope, no new secret handling.

Given the extensive prior review history on this PR (write-gate isolation, byte-pinning, credential-surface closure via secrets/github.token/vars word-matching, the poll/write job split, and now this coverage gate) — every previously-reported CRITICAL and IMPORTANT finding across this thread now appears closed. The do-not-merge label remains a maintainer call, not something I can act on.
· branch feat/aggregator-ambient-token-writes

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 1m 11s —— View job

---

Security review complete

• Gather context (diff at HEAD 27e388c vs previously-reviewed da25244)
• Review this push's actual diff ("hold an incident whose index names nothing, not only one that failed to parse")
• Re-verify prior findings status
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (27e388c) touches only .github/scripts/claude-lane-incident.cjs and its test file — no workflow, permissions, trigger, or credential-surface changes. It's a narrow correction to the coverageGap fix landed in the previous round (da25244).

What it does: coverageGap's previous form special-cased previous === null (an unparsable state block) as unparsableState: true, but a state block that parsed successfully to an empty repositories: {} index fell through to the general path and, since implicated.length === 0 made both unobserved and unlisted trivially empty, returned null — "coverage complete." That's the same silent-close hole the previous commit fixed, reachable by a second route: a hand-edited body with "repositories": {} rather than one that fails to parse at all. coverageGap now computes implicated first via Object.keys(previous?.repositories ?? {}) (null-safe for both previous === null and a present-but-empty repositories), and treats implicated.length === 0 as the single unified namesNothing case — both "didn't parse" and "parsed to nothing" are the same claim (nothing) and get the same answer (hold open).

Traced for correctness, not just trusted:

• No null-dereference risk: previous.repositoriesSeen (line 406) is only reached when implicated.length > 0, which can only be true if previous is a real object with non-empty repositories — so previous is never null at that point.
• Confirmed the commit's own reachability claim by reading the call sites: mergeRepositories (L478) is the only path that builds repositories from a live poll, and it's fed from tally.repositories, which the poll only populates after validating the observation's full_name matches the fork-check — so an empty index from a real cycle isn't reachable; only a hand-edited issue body reaches this branch. Since editing the issue body already lets an actor close the incident directly, this was never a privilege-escalation path, just a correctness gap in the fail-direction.
• describeCoverageGap and both test additions (claude-lane-incident.test.cjs#L529-577) correctly cover both routes (previous: null and a parsed-but-empty index) and assert the gap holds across all three cycles of CLEAN_CYCLES_TO_CLOSE, not just that it fails to advance once — same discipline as the earlier coverage-gap regression test.

No new issues found. This diff doesn't touch permissions, uses: pins, triggers, the credential surface, or the write/mint-step pinned regions. No injection surface, no widened scope, no new secret handling.

Standing summary across this whole review thread: every previously-reported CRITICAL and IMPORTANT finding (write-gate step-classification bypasses, JS alias/spread evasions, credential-surface env:/bracket-notation/github.token/vars blind spots, the App-credential-loss auto-close integrity gap) has been closed across prior pushes, most recently the App-credential-loss gap in da25244 and now this second route to the same class of gap in 27e388c. This push introduces no new security-relevant surface; it strengthens the fail-direction of an already-landed fix. The do-not-merge label remains a maintainer call, not something I can act on.
· branch feat/aggregator-ambient-token-writes

[claude]

Claude finished @kyle-sexton's task in 56s —— View job

---

Security review complete

• Gather context (diff at HEAD fb4ef5f vs previously-reviewed 27e388c)
• Review this push's actual diff ("say in the incident body that a clean cycle must be a covered one")
• Re-verify prior findings status (App-credential-loss auto-close — fixed in 27e388c)
• Sweep full diff for new logic flaws / injection / credential handling / permission changes
• Post findings

This push (fb4ef5f) touches only .github/scripts/claude-lane-incident.cjs and .github/workflows/claude-lane-incident-aggregator.yml, and is text/comment-only. No executable logic changed beyond removing one dead branch:

• describeCoverageGap drops its if (!gap) return "coverage complete"; guard. I traced every call site: the only production caller is .yml#L568, if (coverageGap) core.warning(describeCoverageGap(coverageGap));, which already guards on truthiness before calling — so the removed branch was dead code, unreachable in the shipped workflow. Removing it changes nothing at runtime. The test file's three call sites all pass real gap objects, not null/undefined.
• The remaining changes are: a doc-comment on describeCoverageGap explaining the new invariant, rewritten prose in renderIssueBody's auto-generated incident-issue instructions (explaining to a human reader that only a covered clean cycle counts, and that a permanently-gone repository requires manual close), and a rewritten code comment above the poll job's polled-repositories output clarifying that "attempted" and "observed" scope coincide (any read failure raises an error and caps the cycle at indeterminate before coverageGap is even reached).

No security-relevant surface is touched: no permissions change, no uses: pin change, no trigger change, no credential/secret handling change, and the write job / mint step (byte-pinned regions) are untouched. The issue-body text changes land only as static strings the workflow already renders into the incident issue — not user input, not an expression evaluated by GitHub Actions.

No findings. Every previously-reported issue across this thread (write-gate step-classification bypasses, JS alias/spread evasions, credential-surface env:/bracket-notation/github.token/vars blind spots, and the App-credential-loss auto-close integrity gap closed in 27e388c) remains closed, and this push introduces nothing new to review.
· branch feat/aggregator-ambient-token-writes