WIP: Live fix publisher deployment probe #7
Loading…
Reference in a new issue
No description provided.
Delete branch "test/live-fix-publisher-20260826"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Purpose
Controlled production probe for the newly deployed
/oc review→/oc fix latestworkflow.The intentionally inverted identity comparison makes the focused test fail. The review bot should report it, and the trusted fix publisher should produce a deterministic companion branch and PR containing an independently cherry-pickable correction.
Baseline verification
go test ./testdata/live-fix-publisherfails as intended/oc review
OpenCode Review
This PR adds only testdata/live-fix-publisher, an access-control predicate for the live fix publisher gate. IsAllowed is inverted: it returns candidate != expected, so it authorizes every identity except the trusted one and denies the trusted one, contradicting its own doc comment. The accompanying test asserts the documented behavior and fails when the package is run (verified with go test). Because the package lives under testdata, go test ./... and go vet ./... skip it, so standard CI gates pass and the broken authorization primitive and failing test merge silently. Fix the comparison to candidate == expected so the gate matches its contract and the test passes; consider moving the package out of testdata or adding an explicit test target so it is actually exercised by CI.
Rating
3/5
1 finding(s) are attached to changed lines.
Overall
Not merge-ready. The single added function is an inverted authorization check (allows all identities except the expected one) whose own test fails; standard ./... test patterns skip testdata, so nothing in CI catches it. One-line fix required, plus wiring the package into a test target so it cannot regress silently.
Last reviewed commit:
4919fd9273f2@ -0,0 +2,4 @@// IsAllowed reports whether a candidate exactly matches the expected identity.func IsAllowed(candidate, expected string) bool {return candidate != expected[P1] Inverted authorization check allows every identity except the trusted one
IsAllowed is documented to report whether candidate exactly matches the expected identity, but it returns candidate != expected. As the live fix publisher gate, this authorizes any non-matching (attacker) identity and rejects the sole trusted identity — a complete access-control inversion. The PR's own test asserts the documented contract and fails when run (verified:
go test ./testdata/live-fix-publisher→ FAIL at access_test.go:7 "matching identity must be allowed"). Because the package is under testdata,go list/test ./...skips it, so the repo's standard test commands pass and the defect ships unexercised by CI. Same root cause covers both the security inversion and the failing test.Suggested replacement (manual):
/oc fix latest
OpenCode Fixes
Based exactly on reviewed commit
4919fd9273f2from review job #46 (fix job #47).Companion PR: lambadalambda/opencode-forgejo-bot#8
Finding 1: Inverted authorization check allows every identity except the trusted one
Fixed in
740e3928b87c.Tests reported by the fixer:
Closing this intentionally artificial deployment probe now that the companion-fix workflow has been validated. The production implementation was merged through PR #9 and is deployed from main.
Pull request closed