fix(ci): preflight-tidy gate 1 allow indirect-only go.mod (#973)#974
Merged
Conversation
v0.2.4's first dispatch (run 27370455797) failed at gate 1 with `preflight-tidy: go.mod modified — aborting`. The actual `make tidy` diff modified 11 sub-module go.mod files — but every change was a `// indirect` line adjustment: tidy was adding axonops/syncmap + golang.org/x/crypto and removing kr/text as the transitive set shifted after v0.2.2's publication. These are routine Go-toolchain housekeeping, not engineering changes. Real engineering changes (a new direct require, a version bump on an existing direct require, a change to the module / go / toolchain / replace / retract / exclude directives) ARE security signals — but `// indirect`-only adjustments are not, and gate 4's sum.golang.org cross-check catches any tampered axonops/audit/* hash that would feed into them. This refines gate 1 to classify go.mod changes into three buckets: - goModIndirectOnly: pass through to gates 2+ - goModDirectRequire: reject with the new `preflight-tidy: go.mod direct require modified — aborting` - goModDirective: reject with the new `preflight-tidy: go.mod module/go/toolchain/replace directive modified — aborting` The classifier is a line-oriented matcher (not a full go.mod parser) — every line of a `go mod tidy` diff is either a known directive prefix, a require entry with or without `// indirect`, or a structural marker (`require (`, `)`, blank, in-block comment). Unrecognised shapes fail closed to goModDirective. Changes: - cmd/release-tool/cmd_preflight_tidy_check.go: replace hasGoModChange with goModChangeKind enum + classifyGoModChanges + classifyGoModLine + startsWithGoModDirective helpers. New msgGoModDirectRequire + msgGoModDirective constants; old msgGoModModified retired. - cmd/release-tool/cmd_preflight_tidy_check_test.go: replace the single TestPreflightTidyCheck_Gate1_GoModModifiedAborts test with 7 table-driven tests (DirectRequireAdd, DirectRequireBump, ModuleDirective, GoDirective, ToolchainDirective, ReplaceDirective, IndirectOnlyAllowed). The IndirectOnlyAllowed fixture mirrors v0.2.4's actual blocked diff shape. - tests/release-scripts/release-yml-grep.bats: rename test_release_yml_preflight_tidy_enforces_go_sum_only → ..._rejects_direct_require_changes; update the property-style exact-strings test to anchor on both new strings. - docs/releasing.md: failure-mode table row 1 reflects the new classification + both new error strings. - CHANGELOG.md: [Unreleased] / Fixed entry. Verifications: - `go test ./cmd/release-tool/... -race -count=1` clean. - `golangci-lint run ./cmd/release-tool/...` 0 issues. - `make test-release-scripts` 117/117 pass. - `make regen-llms-check` clean. Closes #973.
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
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
v0.2.4 dispatch failed at gate 1 with
go.mod modified — abortingbecause tidy adjusted// indirectrequires across 11 sub-module go.mod files (transitive deps shifting after v0.2.2: addingaxonops/syncmap,golang.org/x/crypto; removingkr/text). These are routine Go-toolchain housekeeping, not engineering changes.This refines gate 1 to classify go.mod changes:
preflight-tidy: go.mod direct require modified — abortingpreflight-tidy: go.mod module/go/toolchain/replace directive modified — abortingThe single
msgGoModModifiedconstant is replaced by two more-specific constants. Unrecognised line shapes fail closed to the directive reject path.Why this is a bug, not a workaround
Gate 1 was designed during #967 to block any go.mod change because a new direct require could introduce a malicious module. But that threat manifests through direct-require changes, not indirect-line shuffles — and gate 4 (sum.golang.org cross-check) catches any axonops/audit/* tampering that would cause those adjustments to be malicious. As written, gate 1 made the release flow unusable: every release naturally produces some transitive churn.
Test plan
go test ./cmd/release-tool/... -race -count=1— all release-tool tests pass.golangci-lint run ./cmd/release-tool/...— 0 issues.make test-release-scripts— 117/117 pass (bats grep anchors updated).make regen-llms-check— clean...._DirectRequireAdd / Bump / ModuleDirective / GoDirective / ToolchainDirective / ReplaceDirective / IndirectOnlyAllowed. TheIndirectOnlyAllowedfixture mirrors v0.2.4's actual blocked diff shape.tag-allafter this merges.Closes #973.