fix(assent): read judged content at the pinned commit SHA - #149
Merged
Merged
Conversation
konih
force-pushed
the
fm/assent-b1-pinned-sha
branch
from
September 25, 2026 14:10
cb1d11f to
8071c39
Compare
konih
changed the title
The live run path pinned info.SourceSHA/info.TargetSHA for the approval and merge CAS, but read every judged byte by mutable branch name, so a contributor controlling push timing could force-push back to the pinned SHA after assent judged different bytes (U-04 item 1 / QFN-01). Read the six content sites at the pin: merge-policy, ruleset-binding, config, pack and governed base at info.TargetSHA; governed head at info.SourceSHA. Test lock: the run-path fake now serves the pinned SHAs and refuses any other ref, so a regression to a branch name reddens the suite; TestFileAtRef asserts the SHA; a new run-path case judges the pin while the branch points at benign bytes; a new forge conformance case (sha-guard-source-moved-and-restored) shows the CAS merges once a moved head is restored, which is why the read-pin is load-bearing. Spec: openspec/specs/p5-rev1-pinned-sha/spec.md. U-04 items 2-4 stay out of scope as a named follow-on.
… read to the target SHA
…ode in `internal/forge/conformance/suite.go` (the `caseSHAGuardSourceMovedAndRestored` conformance case) left 6 executable lines uncovered, and SonarCloud's new-code coverage condition (new_coverage < 80%) was the sole failing gate condition on PR 149 (64.3% vs 80% required; run.go's 4 changed error-return lines are also uninstrumented because `task coverage` profiles only `./internal/...`). The 6 uncovered lines were the phase-2 ("restored") failure branches. They were unreachable under the existing sabotage/non-vacuity harness (`TestEveryCaseCanFail`): a phase-1 assertion failure aborted the whole case via `failRecorder.Fatalf`, so phase 2 never ran against a sabotaged backend — meaning those phase-2 assertions were themselves unproven assertions, the exact defect class the sabotage gate exists to catch. Fix: split the two phases into `t.Run("moved-away", ...)` / `t.Run("restored", ...)` subtests so `failRecorder.Run` absorbs a phase-1 abort and still executes phase 2 against the sabotaged backend, and collapse phase 2's three separate failure branches into one compound assertion (`err != nil || merges != 1 || attempts != 1`) whose body now executes under sabotage. This makes the case 100% statement-covered (verified locally: `caseSHAGuardSourceMovedAndRestored 100.0%`), strengthens the non-vacuity proof (both phases are now shown capable of failing), and raises the projected new-code coverage to ~85.7% (>=80%), while `task coverage` stays at 91.1% (>= the 91 floor). No production behavior changed; the pinned-SHA read fix is untouched. Verification run locally: `go test ./cmd/... ./internal/forge/...` all pass; `go test -race ./internal/forge/conformance/` passes; `gofmt -l` and `go vet` clean; internal coverage 91.1%
The rebase onto main (24e9acf) landed main's mr.labels row as D-182, so the pinned-SHA row this lane added was renumbered D-183. Update the four citations that still named D-182 for the pinned-SHA decision.
…-restore guard limit
konih
force-pushed
the
fm/assent-b1-pinned-sha
branch
from
September 25, 2026 16:39
bc480c1 to
32c1be3
Compare
konih
changed the title
|
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.
Intent
The six-leg Assent review found that the live run path reads the content it judges from a mutable branch name while the decision record and the merge CAS pin a commit SHA, so a contributor who controls push timing can force-push back to the pinned SHA after assent judged different bytes. Fix the read so the judged content is read at the pinned SHA — the round's first fix slice. Evidence and exact call sites: finding U-04 in the unification register and QFN-01 in the differential report; the B1 lane names the six call sites in cmd/assent/run.go at lines 203, 211, 230, 249, 270 and 274 (the MergePolicy, RulesetBinding, Config, Pack and governed base reads, plus the governed head read), the two polarity tests to flip in internal/forge/gitlab/gitlab_test.go at line 82 and cmd/assent/run_test.go at line 388, and a move-and-restore conformance case where the head moves away from the pinned SHA and back to it between evaluation and the merge CAS.
What Changed
assent runnow resolves the MergePolicy, RulesetBinding, Config, Pack, and governed base/head reads at the pinned target/source commit SHAs instead of the mutable branch names, so the bytes judged match the SHA-guarded merge (REV1-S01 / U-04 item 1).TestFileAtRefto serve/reject only the pinned SHA refs, and adds run-path tests proving the Pack load follows the pin and that a moved source branch tip is ignored in favor of the pin's violating bytes.sha-guard-source-moved-and-restoredconformance case with aFixture.MoveSourceHeadseam and catalog row, documenting that the merge CAS alone cannot distinguish a restore from a never-moved head, plus the REV1 spec and D-183 decision record.Risk Assessment
✅ Low: The change is a minimal, uniform one-argument-per-site pinning of six content reads to the commit SHAs already used by the approval/merge CAS, every site is covered by a behavior-level polarity lock, and the added conformance case honestly documents the CAS's move-and-restore limit rather than overclaiming.
Testing
Built the real assent binary and live-drove it end-to-end against a GitLab REST stub for the three run-path scenarios: with the pinned source SHA holding a policy-violating shrink while the moved source branch held a benign grow, the binary judged the pin (REVIEW, no merge) and never requested the branch; with the bytes reversed it APPROVEd and issued the SHA-pinned merge (
merge?sha=srcSHA); with--config/--packboth were fetched at the pinned target SHA (the stub 400s branch refs, so a regression would fail). The stub transcript shows all six reads resolving pins: merge-policy, ruleset-binding, config, pack and governed base at ref=tgtTIP, governed head at ref=srcSHA. The move-and-restore conformance case passes on both fake and gitlab adapters, and the flipped polarity tests pass. No product surface issue found.assent runjudges the PINNED source SHA's violating bytes -> decision REVIEW and no mergeassent runAPPROVEs and issues the SHA-pinned merge at the pin, ignoring the branch tipPUT /api/v4/projects/42/merge_requests/7/merge?sha=srcSHA, no ref=feature fetchassent runwith --config and --pack resolves both documents at the pinned target SHA (a branch-name read would 400 and fail the run)Evidence: Live real-binary runs (S1 pinned-shrink, S2 config+pack pinned, B pinned-grow APPROVE+merge) with HTTP ref transcripts
Evidence: Scenario S1: pinned srcSHA shrink, branch tip grow -> REVIEW, no branch fetch
EXIT=0 decision=REVIEW -> 2 forge operation(s) written refs: merge-policy?ref=tgtTIP, ruleset-binding?ref=tgtTIP, topics%2Forders.yaml?ref=tgtTIP (base), topics%2Forders.yaml?ref=srcSHA (head); no ref=featureEvidence: Scenario B: pinned srcSHA grow, branch tip shrink -> APPROVE + PUT merge?sha=srcSHA
{"decision":"APPROVE",..."sourceSha":"srcSHA","targetSha":"tgtTIP"...} ; requests: topics%2Forders.yaml?ref=srcSHA and PUT /merge_requests/7/merge?sha=srcSHAEvidence: Scenario S2: --config and --pack fetched at ref=tgtTIP, exit 0
EXIT=0 GET .assent%2Fconfig.yaml/raw?ref=tgtTIP GET .assent%2Fpacks%2Fp%2Fpack.yaml/raw?ref=tgtTIP GET .assent%2Fproviders%2Fquota.json/raw?ref=main (provider declarations, deliberately out of scope U-11/D-130)Evidence: Move-and-restore conformance case passes on fake and gitlab adapters
Evidence: Flipped polarity tests pass (TestFileAtRef SHA ref; TestRunJudgesPinnedSHAWhenBranchMoves; TestRunPackFromPinnedSHA; TestRunPolicyFromTargetRefOnly)
Evidence: gitlab adapter TestFileAtRef requires the pinned SHA ref
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
✅ **Review** - passed
✅ No issues found.
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
assent runwhen the MR source branch tip has moved to benign grow bytes while the pinned source SHA holds a policy-violating shrink; the emitted DecisionRecord follows the pinned SHA (…assent run --packand every judged-content read (MergePolicy, RulesetBinding, Pack, governed base, governed head) resolves a pinned commit SHA with no mutable branch-name refassent run --configand the Config read resolves the pinned target SHA (the fake rejects any non-SHA config ref)forge.Reconcilerefuses a head moved away from the pin and then merges once when the head is restored to the pin, proving the CAS alone cannot catch a restore; the case…assentpro…info.SourceBranchmakes the run-path test and the real binary APPROVE the moved branch tip with 3 forge writes, and reverting the MergePo…go test -count=1 -tags livee2e ./cmd/assent/ -run TestLiveBinary(real compiled binary against local fake GitLab; three scenarios)go test -count=1 ./cmd/assent/ -run 'TestRunJudgesPinnedSHAWhenBranchMoves|TestRunPackFromPinnedSHA|TestRunPolicyFromTargetRefOnly'go test -count=1 ./internal/forge/gitlab/ -run TestFileAtRefgo test -count=1 ./internal/forge/conformance/ -run 'TestConformanceSourceMovedAndRestored|TestConformanceSourceMovedRejected|TestConformanceTargetAdvancedRejected'go test -count=1 ./internal/forge/conformance/ -run 'TestEveryCaseCanFail|TestCatalogRowsMatchRunSuite|TestRunSuite'Mutation:run.gogoverned-head read reverted toinfo.SourceBranch->TestRunJudgesPinnedSHAWhenBranchMovesred and live binary APPROVEs the moved tip with 3 forge writes; restoredMutation:run.goMergePolicy read reverted toinfo.TargetBranch->TestRunPolicyFromTargetRefOnlyred (400 at?ref=main); restoredgo test -count=1 ./cmd/assent/... ./internal/forge/gitlab/... ./internal/forge/conformance/...(all green after restore)✅ No issues found.
assent runjudges the PINNED source SHA's violating bytes -> decision REVIEW and no mergeassent runAPPROVEs and issues the SHA-pinned merge at the pin, ignoring the branch tipPUT /api/v4/projects/42/merge_requests/7/merge?sha=srcSHA, no ref=feature fetchassent runwith --config and --pack resolves both documents at the pinned target SHA (a branch-name read would 400 and fail the run)go build -o bin/assent ./cmd/assentthenGITLAB_TOKEN=... ./bin/assent run --project 42 --mr 7 --bot-author assent-bot --subject file:topics/orders.yaml --gitlab-endpoint http://127.0.0.1:8899against rev1_stub.py (scenario S1: pinned srcSHA shrink, branch feature grow) -> exit 0, decision REVIEW, no mergesame real-binary run with stub bytes flipped (scenario B: pinned srcSHA grow, branch feature shrink) -> exit 0, decision APPROVE,PUT .../merge?sha=srcSHAsame real-binary run with--pack .assent/packs/p/pack.yaml --config .assent/config.yaml(scenario S2) -> exit 0; config and pack fetched at ref=tgtTIPgo test ./internal/forge/conformance/ -run TestConformanceSourceMovedAndRestored -v(fake + gitlab adapters, moved-away and restored phases)go test ./cmd/assent/ -run 'TestRunJudgesPinnedSHAWhenBranchMoves|TestRunPackFromPinnedSHA|TestRunPolicyFromTargetRefOnly' -vgo test ./internal/forge/gitlab/ -run TestFileAtRef -v✅ **Document** - passed
✅ No issues found.
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.