feat(github-workflow): cut 1.10 with the #234 hardening, shared-read dedup, land, and a stripped binary - #251
Merged
Merged
Conversation
…d binary Eight findings from security read H13 share one shape: gh-workflow trusted content it did not author, or acted on state it had read earlier without re-checking it, or misclassified a refusal. All eight are fixed in one cut, because payload bytes are immutable and every one of them changes the shipped binary. Disposition evidence is attributed. The `Final-Disposition:` record is an ordinary PR comment, so through 1.9 any account that could comment could pin a permanent CONFLICT on a pull request or stand in for the operator's `--reason`. The client resolves its own login once per run (`GET /user`, cached), the topology hands it to the engine as `TrustedAuthors`, and both the `close --pr` path and the read-only Post-merge gate ignore every other author. The engine holds no identity of its own, so an empty trusted set sees no evidence at all — fail-closed by construction. Sanitizing moved out of `render` into the leaf package `safetext` and is applied in `cli.WriteEnvelope` and in `ghapi.apiMessage`, which is where a non-2xx response body becomes error text. `render.SanitizeText` stays as a one-line alias. The envelope copies before encoding: findings and steps are slices the caller still owns. `EnableAutoMerge` now requires the validated head SHA and sends it as `expectedHeadOid`; an empty value is refused locally. `ready` re-observes the head immediately before the mark-ready mutation and refuses one that moved, at the cost of one read — GraphQL offers no conditional form of that mutation, so the remaining one-round-trip window is documented as accepted rather than claimed closed. `ResolveRepoFile` stops at the enclosing checkout root and refuses to search outside a checkout at all; `render.Repository` carries the origin host so `VerifyAPIHost` can refuse a checkout whose remote is not the host the tool writes to. Rate-limited 403 and 429 responses are waited out with `Retry-After` and retried within a bounded budget on both the read and write paths, and a limit that outlasts the retries unwraps to the new `ErrRateLimited` rather than to `ErrUnauthorized`. Payload 1.10 is a copy of 1.9 with the version surfaces and the two prose accounts edited; 1.9 stays byte-immutable and moves to `retained`. #234's ninth criterion is answered "deferred again": the `release` class stays declared and unenforced, which README, adopt.md, and the CHANGELOG now say plainly. `scripts/build-gh-workflow.sh` adds `-s -w` (#228 lever 1), which drops the symbol table and DWARF: 10,432,350 bytes at 1.9 against 7,307,390 at 1.10. Panic traces keep their function names and line numbers, which come from the runtime's pclntab. Earlier payloads stay unstripped and are never rebuilt. Refs #234, #228
…1.10 security read `gh-workflow land --pr N` runs the whole admission of one pull request as one transaction (#236 C13): it advances a governing issue still `Ready` to `In progress`, then performs exactly the `ready` and `merge` operations — the same code paths, the same fresh gates — and ends by naming the merge commit with the `git diff` that proves the admitted head's changed paths now read the same on the integration branch. The proof is printed rather than executed, because this tool has never required Git to be installed. Fail-closed throughout: any refusal stops the sequence and emits the receipt of which boundaries completed. `--auto` is not offered, since a merge GitHub has not performed yet cannot be proved. Four request-dedup items from the efficiency survey land with it (#227 E4 1, 2, 4, 5). `topology.Prefetched` becomes a memo with unexported fields and one accessor per read, so a value can never claim a read that did not happen — the earlier "non-nil asserts every read was performed" contract forced a caller holding only some reads to pass nil, and would have answered the one-open-Final rule from an empty list. It now also memoizes the repository's merge settings and each base branch's enforcement, and serves a governing issue from the open-issue list a `summary` already holds. That last cache is positive only: a closed or absent issue falls through to the API, so a Final governed by a completed issue never reads as unresolved. `receipt --pr` projects the pull request and its CI from the topology it just loaded, and `check --issue` projects the issue it read for the shape check. Measured on the package's own fixtures: summary 22 -> 16 requests, `receipt --pr` 10 -> 8. Golden output is byte-identical; E4#3 (concurrency) was declined. The pre-merge security read of ba4b9719 is folded in. `Repository.FromOrigin` closes the host bypass: Git accepts hostless remotes (`owner/repo`, `:owner/repo`), which produced an origin-derived pair with an empty Host that skipped both ValidateHost and VerifyAPIHost and was written to api.github.com. `admission`'s human report sanitizes commit subjects and messages, GraphQL error text passes the same encode-and-bound step REST bodies do, and `ResolveRepoFile` resolves symlinks and refuses a target outside the checkout root. `ready`'s head re-read guard moves ahead of the governing-issue write, so a head that moved leaves nothing advanced. Payload 1.10 gains the `land` row in SKILL.md and pr-standard.md and `land` in the envelope schema's command enum; SKILL.md sat at exactly its NFR-006 byte ceiling, so the new text was paid for by compressing prose in the same file. The binary is resealed (7,348,350 B) and the digest chain recomputed. `test_github_workflow_1_8.py`'s catalog role map still asserted 1.9 as default, which the 1.10 cut left stale; it is corrected here. Refs #227, #236, #234
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
github-workflow 1.10: hardens
gh-workflowper #234 AC1–AC8 (host-pinned API base with hostless-origin refusal, trusted-author allowlist, sanitized text on every output path,expectedHeadOidon auto-merge,GHW-PR-READY-HEAD-MOVEDguarded before the first write, root-stoppedpolicy.tomlresolution with symlink containment, bounded GraphQL error text), deduplicates the shared reads behind one memoizedPrefetched(summary22 → 16 requests on the two-PR fixture,receipt --pr10 → 8), addsgh-workflow land --pr N [--method M]as the one-transaction admission (#236 C13), and ships the binary stripped (-s -w, 10,432,350 → 7,348,350 bytes, pclntab retained). Pre-merge security read (/tmp/ps529-session/reviews/sec-gw110.md) MUST-FIX 1 and SHOULD-FIX 2, 3, 4, 7 are folded; 5, 6, 8, 9 are accepted residuals recorded on #234.Governing work
Final: #234
Acceptance coverage
internal/ghworkflow/{render,ghapi,mutate,admission,cli,safetext,topology}; tests inrender/repo_test.go,render/command_test.go,mutate/paired_test.go,cli/cli_test.go; contract teststests/package_contract/test_github_workflow_1_10.py(11 passed). AC9 deferred again (recorded on github-workflow 1.10: harden gh-workflow against untrusted GitHub content and racy mutations #234).-s -w -buildid= -X main.version=1.10; go-verify-binary matches a rebuild (6868b345…).topology.Prefetchedmemo +render.Fetchseeding;TestSummaryReusesSharedReadsWithinOneCommand,TestSummaryResolvesAGoverningIssueThatIsClosed.mutate/land.go;TestLandRunsTheWholeAdmissionAsOneTransaction,TestLandStopsAtTheFirstRefusalAndReportsWhatCompleted;SKILL.mdandreferences/pr-standard.mddocument it within the NFR-006 budget (70 lines / 11,998 B).Verification
go build ./... && go vet ./...,rexec -- make go-check(0 issues; sealed bytes match rebuild), four standards validators after rebase (validate-packages, validate-graph, render-catalog --check, sync-payload-projection --check), ruff on edited tests,tests/package_contractremote (1790 passed; 19 markdown-tooling oracle reds are the missingnpm cion that workspace, unrelated; 3 activation reds are the by-design reconcile lag).check-release --baseline v5.28.0reports onlyPC-RELEASE-PROJECTION.