github-workflow 1.10: harden gh-workflow against untrusted GitHub content and racy mutations

[chrisdpurcell]

Summary

Deferred hardening findings for gh-workflow (github-workflow package), verified at HEAD 1922e320 under internal/ghworkflow/ during security read H13. These items were deliberately kept out of the v5.28.0 train because they need a 1.10 github-workflow cut. Line numbers below should be re-checked before acting on them — the tree has moved since the read.

Findings

1. MED — closepr.go ~297-314 / relation/evaluate.go ~507-528 / topology.go ~300 — Final-Disposition evidence is accepted from any comment author; the authoring actor is discarded. Exploit/failure scenario: a third party posts a comment that reads as a clear disposition, creating a permanent CONFLICT or silently overriding the operator's --reason. Fix: treat evidence as authoritative only when authored by the authenticated actor or an explicit allowlist.
2. MED-LOW — closepr.go ~182-183, relation/evaluate.go ~528, cli/envelope.go ~196-210, mutate/ready.go ~138 — untrusted GitHub text (%s, raw error text carrying up to 200 B of response body) reaches the envelope/terminal without SanitizeText; that sanitizer (render/safetext.go) is applied only to summary/receipt rendering. Failure scenario: attacker-controlled text (issue/PR body, comment, or API error body) is echoed unsanitized to the terminal or JSON envelope. Fix: move the sanitizer into a leaf package and apply it in cli.WriteEnvelope.
3. LOW — mutate/merge.go ~199 — EnableAutoMerge is called without expectedHeadOid. Failure scenario: a push lands after arming auto-merge and before the merge fires, so ungated content merges. Fix: pass the validated head SHA through to the auto-merge call.
4. LOW — mutate/ready.go ~159 — no conditional guard exists between the gate read and the mutation that follows it (TOCTOU window).
5. LOW — render/repo.go ~187-191 — the origin host is never compared against the API base host, so a non-GitHub origin can be made to write to a same-named github.com repository.
6. LOW — cli/cli.go ~298-314 — ResolveRepoFile can walk past the checkout root, so policy.toml / org-schema.yaml can be sourced from an ancestor directory outside the intended checkout.
7. INFO — audit/command.go ~161 — organization schema text is printed unsanitized.
8. Operability (not security) — no retry / Retry-After handling on HTTP 403/429; a rate-limit response is currently misreported as ErrUnauthorized.

Note: item #3 from the same H13 read (PR trailer forgery) was already fixed in 1.9 (#232). 1.9's release-class enforcement is declared-but-not-enforced by design per ADR 0031; enforcing it, if wanted, is itself a 1.10 candidate.

Acceptance criteria

• Final-Disposition evidence is accepted only from the authenticated actor or an explicit allowlist; a third-party comment can no longer force a CONFLICT or override an operator-supplied --reason.
• All untrusted GitHub text reaching cli.WriteEnvelope (including raw API error bodies) passes through a sanitizer applied at that leaf, not only in summary/receipt rendering.
• EnableAutoMerge passes the validated expectedHeadOid so a late push cannot merge ungated content.
• The gate-read/mutation window in mutate/ready.go is closed with an appropriate conditional guard, or the residual race is explicitly documented as accepted.
• The resolved origin host is validated against the API base host before any write.
• ResolveRepoFile cannot resolve a path outside the checkout root.
• Organization schema text printed by audit/command.go is sanitized before output.
• 403/429 responses are retried with Retry-After handling and are no longer misreported as ErrUnauthorized.
• A decision is recorded on whether 1.9's declared-but-not-enforced release class (ADR 0031) is enforced as part of this cut or explicitly deferred again.

Out of scope

• Item spec-validate (v4.0.0): ID checker rejects ADR & external-ID references, misparses license tokens; Markdown formatter divergence #3 (PR trailer forgery) — already fixed in 1.9 (feat(standards): cut github-workflow 1.9 with the ADR 0031 admission classifier #232).
• Enforcing the 1.9 release-class declaration itself, unless the owner elects to fold it into this cut.
• Any change to organization schema (Issue Types, fields, or values).

Evidence / references

• Security read H13, session 2026-09-01.
• Verified at HEAD 1922e320, files under internal/ghworkflow/ (line numbers approximate; re-check before citing further, the tree has moved since the read).
• Related: feat(standards): cut github-workflow 1.9 with the ADR 0031 admission classifier #232 (1.9 fix for PR trailer forgery), ADR 0031 (declared-but-not-enforced release class).

Provenance

security read H13, session 2026-09-01, deferred from v5.28.0 because it needs a 1.10 github-workflow cut.

[chrisdpurcell]

3 — Comment for #234 (AC9 decision)

AC9 — release-class enforcement: explicitly deferred again; it stays declared-but-not-enforced
in github-workflow 1.10.

ADR 0031 (docs/adr/adr-0031-integration-branch-admission-model.md) D3 is unchanged by this cut.

Reason:

1. The release class is authored by a single owner directly on main, and it is already
   matched structurally by the release_subject_prefix this repository declares — the
   classifier identifies the commit correctly today; only the admission is taken on the
   author's word.
2. Real enforcement means a CI job that can verify a release commit's provenance independently
   of the author, which the owner deferred to a later cut. Shipping half of it inside 1.10
   would add a gate that cannot fail honestly.
3. Nothing in 1.10's security scope depends on it: the eight hardening findings all concern
   untrusted GitHub content and racy mutations, not commit admission.

Revisit when either a second author gains write access to main, or the enforcement job the
ADR contemplates is actually scheduled.

---

Pre-merge security read residuals accepted for 1.10 and tracked in the follow-up issue filed today: disposition attribution by login only (a maintainer comment edit keeps the author), the apiMessage JSON branch is not bounded by maxMessageBytes, GET /user as a hard precondition rejects App/installation tokens and [bot] logins, and the trusted-author allowlist has no config surface. Final shipped binary: 7,348,350 B (resealed after the security fold-in).