Skip to content

fix(claude-automerge): catch oauth2/* and main.go in risk-tier globs#77

Merged
topcoder1 merged 1 commit into
mainfrom
claude/fix-automerge-oauth2-main-go-globs
May 24, 2026
Merged

fix(claude-automerge): catch oauth2/* and main.go in risk-tier globs#77
topcoder1 merged 1 commit into
mainfrom
claude/fix-automerge-oauth2-main-go-globs

Conversation

@topcoder1
Copy link
Copy Markdown
Owner

Summary

The risk-tier path glob list in claude-author-automerge.yml was missing two categories that exist in the policy's intent but not in the regex:

  1. oauth2/ as a path segment — the (auth|login|session|oauth|sso) alternation required the literal segment oauth. The Go convention is oauth2/ (e.g., golang.org/x/oauth2), and oauth2 is oauth followed by 2, not oauth followed by / or end-of-string, so the anchor (/|$) rejects it.
  2. Go service entrypoints (main.go at any depth) — no pattern covered these at all.

Observed gap (2026-05-24, on whois-api-llc/wxa-mcp-server)

PR Files touched Expected Actual Pattern that should match
#193 internal/oauth2/server.go, main.go block auto-enabled by app/github-actions (^|/)main\.go$, ^(.*/)?(...|oauth2|...)(/|$)
#194 internal/auth/security.go block correctly blocked ^(.*/)?(auth|...)(/|$) ✓ already worked
#197 internal/oauth2/server.go block auto-enabled by app/github-actions ^(.*/)?(...|oauth2|...)(/|$)

PR #194 blocking correctly is the proof the auth alternation works as designed — only the missing oauth2 and main.go paths leaked through.

New glob list (additions only)

- ^(.*/)?(auth|login|session|oauth|sso)(/|$)
+ ^(.*/)?(auth|login|session|oauth|oauth2|sso)(/|$)
  ^(.*/)?secrets(/|$)
  ...
  ^(.*/)?(billing|payment[s]?|pricing|invoice[s]?)(/|$)
+ (^|/)main\.go$
  (^|/)Dockerfile(\..*)?$
  ...

Test coverage

selftest/test_automerge_risk_patterns.sh updated in lock-step. Six new RISKY cases and five new SAFE cases:

  • RISKY: internal/auth/security.go, internal/oauth2/server.go, internal/oauth2/handler.go, pkg/oauth2/token.go, main.go, cmd/server/main.go, cmd/wxa-mcp-server/main.go
  • SAFE (must NOT match): main_test.go, internal/foo/main_test.go, internal/oauth2.md (doc with substring), cmd/server/mainview.go (starts with main but not literal main.go), src/oauth2helper.go (oauth2 substring, not a path segment)

All 41 RISKY + 16 SAFE cases pass locally:

$ bash selftest/test_automerge_risk_patterns.sh
...
OK: all risk-pattern cases pass.

Smoke test plan (post-merge)

After this PR lands, callers reference @main so the new globs go fleet-wide immediately. Smoke test:

  1. Open a trivial Claude-authored PR on whois-api-llc/wxa-mcp-server touching only internal/oauth2/ (e.g., add a doc comment).
  2. Confirm claude-author-automerge declines with the sticky risk-tier comment instead of enabling auto-merge.
  3. Close or squash the test PR.

Auto-merge rationale

In the manual-merge category — touches .github/workflows/**. The risk-tier path-scan will correctly block this PR on the new patterns; manual click-merge required. (Also, per CLAUDE.md, topcoder1/ci-workflows itself doesn't install the caller — filename collision with the reusable definition — so PRs to this repo always require manual merge regardless.)

Codex pre-review

Skipped: diff is purely additive regex + test cases (~30 LOC), and the regression surface is covered by the selftest, not by Codex.

Test plan

  • bash selftest/test_automerge_risk_patterns.sh passes locally (41 RISKY + 16 SAFE)
  • Post-merge: smoke-test on whois-api-llc/wxa-mcp-server per plan above

🤖 Generated with Claude Code

The auth alternation `(auth|login|session|oauth|sso)` required the literal
segment "oauth" — `internal/oauth2/server.go` slipped through because
"oauth2" ≠ "oauth" under `(/|$)` anchoring. Go service entrypoints
(`main.go` at any depth) had no pattern at all.

Add `oauth2` to the alternation and a new `(^|/)main\.go$` pattern.
Mirror both into the selftest with 6 new RISKY cases (oauth2 + main.go
variants under common Go layouts) and 5 new SAFE cases (`main_test.go`,
`mainview.go`, `oauth2helper.go`, `oauth2.md` doc — all substrings that
must NOT match).

Why: 2026-05-24 burn on whois-api-llc/wxa-mcp-server. PRs #193 (rename
+ main.go) and #197 (template.URL drop in internal/oauth2/server.go) had
auto-merge enabled automatically by app/github-actions despite touching
the oauth implementation. PR #194 — which touched internal/auth/ — was
correctly blocked, confirming the `auth` segment match worked and only
the `oauth2` + `main.go` paths leaked.

Auto-merge rationale: in the manual-merge category (touches
.github/workflows/**). Risk-tier path-scan will correctly block this PR
on the new patterns themselves; manual click-merge required.

Codex pre-review: not run — diff is purely additive regex + test cases
(under ~30 LOC). Selftest exercises 41 RISKY + 16 SAFE paths and all
pass; the regression surface is covered by the test, not the model.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the risk:blocked Risk class: blocked label May 24, 2026
@github-actions
Copy link
Copy Markdown

Risk class: blocked — manual merge required.

This PR touches one of the blocked path categories from .github/risk-paths.yml (Dockerfiles, docker-compose, .github/workflows/**, **/.env*, **/secrets*, infra/, terraform/, k8s/, or the classifier config itself).

Auto-merge is refused by claude-author-automerge.yml. A maintainer should review the diff and click "Squash and merge" themselves.

(This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.")

@github-actions
Copy link
Copy Markdown

Coverage Floor — mode: enforce

metric value
measured 100.0%
floor (current) 99.0%
target 100.0%
last bumped 2026-05-12

@claude
Copy link
Copy Markdown

claude Bot commented May 24, 2026

No issues found. Regex anchoring is correct for both new patterns — oauth2 segment boundary and main.go literal match — and the SAFE test cases cover the relevant false-positive boundaries.

@topcoder1 topcoder1 merged commit 02ed23d into main May 24, 2026
13 checks passed
@topcoder1 topcoder1 deleted the claude/fix-automerge-oauth2-main-go-globs branch May 24, 2026 20:59
topcoder1 added a commit that referenced this pull request May 24, 2026
…ions (#78)

## Summary

Sibling-gap audit follow-up to
[#77](#77) (which added
`oauth2` + `main.go`). The same literal-segment-alternation trap exists
for several other auth-adjacent and billing-adjacent directory names the
global policy intends to cover but the regex never enumerated.

## Additions

| Category | Before | After |
|---|---|---|
| Auth alternation | `(auth\|login\|session\|oauth\|oauth2\|sso)` |
`(auth\|login\|signin\|signup\|logout\|session[s]?\|oauth\|oauth2\|sso\|jwt\|mfa\|totp\|webauthn\|passkey)`
|
| Billing alternation | `(billing\|payment[s]?\|pricing\|invoice[s]?)` |
`(billing\|payment[s]?\|pricing\|invoice[s]?\|subscription[s]?\|checkout\|refund[s]?)`
|
| Secrets | `^(.*/)?secrets(/\|$)` | `^(.*/)?secret[s]?(/\|$)` |

## Test coverage

Selftest grows from 41 → 60 RISKY cases (19 new segment-match examples
across all three categories) and 16 → 25 SAFE cases (9 new
substring/filename-prefix counter-examples like `sessionsutil.go`,
`passkeystore.go`, `subscriber.go`, `secretly.go`,
`test_authorization_logic.py`, `docs/checkout-flow.md`).

```
$ bash selftest/test_automerge_risk_patterns.sh
OK: all risk-pattern cases pass.
```

## Scope limit (documented inline)

Pattern remains **path-segment-anchored**, not filename-prefix.
Rails/Django/Express conventions like
`controllers/sessions_controller.rb` or `routes/logout.py` are
intentionally NOT matched globally — those belong in per-caller
`.github/risk-paths.yml`. Catching them via the global regex would also
over-match `helpers/auth_helper.py` and similar adjacent files.
**Precision over recall.**

Vendor names (`stripe`, `paypal`, `braintree`) intentionally skipped for
the same reason — per-caller.

## Auto-merge rationale

In the **manual-merge** category (touches `.github/workflows/**`). The
risk-tier path-scan will correctly block this PR on the new patterns
themselves; manual click-merge required. (Also, `topcoder1/ci-workflows`
doesn't install the caller, so PRs to this repo always require manual
merge regardless.)

## Codex pre-review

Skipped — purely additive regex + test cases (~50 LOC), regression
surface covered by the selftest.

## Test plan

- [x] `bash selftest/test_automerge_risk_patterns.sh` passes locally (60
RISKY + 25 SAFE)
- [x] Untracked the `actionlint` binary that got accidentally staged
(follow-up commit + .gitignore)
- [ ] Post-merge: spot-check on next Claude-authored PR touching e.g.
`internal/jwt/` or `api/checkout/` on a fleet repo

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:blocked Risk class: blocked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant