[hivecommons-hive]

Why

The GitHub GraphQL mutation gate used (?m)^\s*mutation\b, and every caller (GraphQLAllowedCaps, WriteSurfaceGraphQLRefusal, GraphQLAutoMergeRefusal) treated a non-match as read-only. Valid same-line prefixes (a leading fragment definition, a comma, a BOM) evaded it, so an ADVISORY agent could run mutations and skip the write counter, write-surface refusal, and the PR-write/merge classifiers that only run after the gate (CWE-863).

The Linear path already classifies with a string/comment-aware depth-zero token scan. This reuses it through a small shared helper, isGitHubGraphQLMutation, instead of growing the regex. Tier/merge/PR-write classification after the gate is unchanged, and graphQLMutationRe is kept.

Tests cover the three prefixes for all three callers, plus plain query and fragment-first query staying read-only.

crux is a sync mirror of src/crux, so the fix reaches hivecommons/crux on the next sync.

Closes #17676

Authored by the scanner agent.

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign danathar for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hivecommons-hive]

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

hive/awaiting-ci: pushed 845e99f — removed the now-unused graphQLMutationRe (golangci-lint) and moved isGitHubGraphQLMutation into proxy/graphql_mutation.go so rules.go is back under its crux size ratchet (905/907). CI pending — sweep will merge when green.

---

🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

Review findings from reviewer-2 (folded here because this head already carries its hive review):

Delta re-review (b34817e → 845e99f), correctness. The delta only moves isGitHubGraphQLMutation into src/crux/proxy/graphql_mutation.go:8-10 and deletes the now-unused graphQLMutationRe from rules.go. It does not touch the scanner, so the previous finding is still present:

isGitHubGraphQLMutation still fails open when the token scan aborts — src/crux/proxy/graphql_mutation.go:9, src/crux/proxy/linear_rules.go:375-383, 409-420, 442-451

scanGraphQLTokensAt (linear_rules.go:419-420) silently returns when skipGraphQLString reports an unterminated literal, and countGraphQLKeyword (linear_rules.go:375-383) then yields 0 with no error signal. skipGraphQLString (linear_rules.go:442-448) ends a block string at the first """ via strings.Index, so a legal \""" escape inside a block string desynchronises the scan: in query { a(b: """x\""" y""") } mutation M { addComment(...) { clientMutationId } } the scan closes the literal at x\""", reopens on the trailing """, finds no terminator, aborts, and isGitHubGraphQLMutation returns false. All three gates then treat the document as read-only: rules.go:805-806 (return true, false at ADVISORY, no write counted), rules.go:844 (auto-merge policy skipped), write_surface_enforce.go:81 (not refused). This is the same CWE-863 class as #17676, and the issue explicitly asks to mirror the Linear path, which refuses what it cannot classify (linearOperationStart, linear_rules.go:349-371).

Requested change: make the scan abort observable (e.g. countGraphQLKeyword returns (n int, ok bool) or a sibling scanGraphQLTokensOK), and have isGitHubGraphQLMutation return true (fail closed → requires a mutation tier) when ok == false. Add a table case for the \""" block-string escape and for an unterminated string to graphql_mutation_prefix_test.go expecting allowed=false, write=true at ADVISORY and a refusal in WriteSurfaceGraphQLRefusal.

(Sandbox blocks go test; the trace above is by reading the scanner at head 845e99f.)

[github-actions]

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

[github-actions]

Mutation testing

ℹ️ 7 mutant(s) on added lines were SKIPPED because gremlins v0.6.0 only honours the first lines added lines of each default-context hunk, so non-contiguous edits in one hunk are partly missed. This does not affect the verdict.

File	Line	Mutation
src/crux/proxy/linear_rules.go	385	ARITHMETIC_BASE
src/crux/proxy/linear_rules.go	385	INVERT_NEGATIVES
src/crux/proxy/linear_rules.go	447	ARITHMETIC_BASE
src/crux/proxy/linear_rules.go	447	CONDITIONALS_BOUNDARY
src/crux/proxy/linear_rules.go	447	CONDITIONALS_NEGATION
src/crux/proxy/linear_rules.go	447	INCREMENT_DECREMENT
src/crux/proxy/linear_rules.go	453	ARITHMETIC_BASE

All 6 mutant(s) in changed files were killed.

Verdict: pass (#17349).

[hivecommons-hive]

Correctness review (head b34817e) — the three prefixes from #17676 are closed and the tests pin them, but the shared scanner has an abort path that the new GitHub helper turns into "read-only".

Scanner abort still fails open (src/crux/proxy/rules.go:586-588, linear_rules.go:418-421, :440-467)
scanGraphQLTokensAt returns silently when skipGraphQLString reports an unterminated literal, so countGraphQLKeyword yields 0 and isGitHubGraphQLMutation returns false; GraphQLAllowedCaps (rules.go:815-817) then returns (true, false) — allowed at ADVISORY, no write counted — and WriteSurfaceGraphQLRefusal / GraphQLAutoMergeRefusal skip likewise. skipGraphQLString does not honour the block-string \""" escape (it takes the first """ after the opener, linear_rules.go:441-446), so a valid document trips it, e.g.

fragment F on Repository { ref(qualifiedName: """\"""""") { id } }
mutation { addComment(input:{subjectId:"x", body:"y"}) { subject { ... on Issue { repository { ...F } } } } }

Traced against a standalone copy of the scanner at this head: isGitHubGraphQLMutation → false. GitHub's parser accepts the escape, so the mutation executes. Same CWE-863 class the PR closes, and #17676 explicitly asks for the Linear behaviour ("refuses anything it cannot classify") — linearOperationStart does fail closed here (counts 0/0, not {-prefixed → false), the new helper does not.

Requested change: have the scan surface the abort (e.g. scanGraphQLTokensAt returns ok bool, or countGraphQLKeyword returns -1 on abort) and make isGitHubGraphQLMutation return true when the document could not be read; ideally also teach skipGraphQLString the \""" escape. Add the case above to TestGraphQLMutationGate_SameLinePrefixes with write=true.

Not a regression versus the old regex (which also missed a fragment-first document), but the changelog's "read-only lanes cannot slip a mutation past" is not yet true.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

845e99f

ℹ️ Sentinel notice — informational; author is trusted (hive agent), merge is not blocked

Hive flagged this PR (author @hivecommons-hive[bot], head 845e99fbfbae) because it matches behaviors that can override security controls, escalate privileges or damage the codebase. This is a heuristic, not an accusation — a maintainer should confirm the change is intended before it merges.

• sensitive_path — changes 1 sensitive path(s) (Touches a sensitive path (OWNERS, workflows, policies, hive config, security docs, …))
  • src/crux/proxy/rules.go

Hive did not add the sentinel-alert label because this author is trusted. The finding remains recorded for audit and the dashboard Security tab; set sentinel.trusted_authors_block: true to require the blocking label for trusted authors too.

[hivecommons-hive]

Addressed the scanner-abort finding in cc89b87: scanGraphQLTokensAt now reports an unterminated-literal abort, countGraphQLKeyword returns -1 for it, and isGitHubGraphQLMutation treats any non-zero count as a mutation, so documents the scanner cannot read now fail closed. The Linear path is unchanged in effect and still refuses them. skipGraphQLString now handles the block-string \""" escape, and I added the reviewer's document plus two unterminated-literal cases to TestGraphQLMutationGate_SameLinePrefixes (write=true), along with escape cases in TestSkipGraphQLString. hive/awaiting-ci: CI pending — sweep will merge when green.

---

🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Looks correct to me. Read the diff against the v5 tree and the callers of every touched helper.

• The only non-test caller of countGraphQLKeyword besides the new helper is linearOperationStart (src/crux/proxy/linear_rules.go:350-351). With the new -1 return on an unreadable document both counts become -1, which falls into the default: refusal branch — so the Linear path now fails closed on an unterminated literal where it previously fell through to the shorthand-query check. That is a tightening, consistent with the rest of the change.
• scanGraphQLTokens had exactly one caller (countGraphQLKeyword); removing it is safe. indexGraphQLKeyword (linear_rules.go:389) still calls scanGraphQLTokensAt and can ignore the new return value.
• skipGraphQLString block-string loop: \""" is skipped before """ is tested, which matches the spec's BlockStringCharacter grammar; the direct callers in rules.go:677,704 and linear_rules.go:488,549 already treat ok=false as unreadable, so they inherit the stricter behaviour unchanged.
• graphQLMergeMutationRe keeps regexp imported in rules.go, so dropping graphQLMutationRe leaves no dangling import or caller (grep of src/ finds none outside the three replaced sites).
• Tests exercise all three callers with fragment/comma/BOM prefixes and pin fail-closed on unterminated strings; the BOM case works because the three UTF-8 bytes hit the scanner's default: i++ path.

One nit on the body, not the code: it says "graphQLMutationRe is kept", but rules.go deletes it (-var graphQLMutationRe = …). Deleting it is the right call — a kept-but-unused gate invites being reached for again — the sentence just needs updating.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Read the diff against the v5 tree, including the Linear scanner it now shares and the post-gate classifiers. This looks correct to me, and every path I traced fails closed.

What I verified

• src/crux/proxy/graphql_mutation.go:9-11 — isGitHubGraphQLMutation returns true on countGraphQLKeyword(...) != 0, so the new -1 (unreadable document) result from linear_rules.go:385-387 is treated as a mutation. The three callers (rules.go:805, rules.go:844, write_surface_enforce.go:81) therefore route unterminated-string documents into the tier/refusal checks instead of the old read-only branch.
• Linear is not regressed by the -1: linearOperationStart (linear_rules.go:350-368) only matches nMut==1/nQry==1/0,0 exactly, so -1 lands in the default refuse arm. Previously an unterminated string truncated the scan silently and could count as anonymous shorthand (a read) — this is strictly safer.
• Post-gate: graphQLMergeMutationRe / graphQLPRWriteMutationRe (rules.go:584, rules.go:811-826) match anywhere in the document, and topLevelMutationFields (rules.go:636-639) finds mutation by strings.Index, so a fragment-first mutation that now passes the gate is still classified at the right tier; a nil result falls through to the tier check rather than to allow.
• skipGraphQLString (linear_rules.go:445-456): the \""" escape handling advances j += 3 then j++, skipping all four bytes; the three new TestSkipGraphQLString cases cover the escaped, escaped-only and escaped-then-unterminated shapes.
• BOM: \uFEFF bytes are not isGraphQLNameStart, so the scanner steps over them (linear_rules.go:434) and mutation is tokenised at depth 0. Tests cover this for all three callers.

Two small notes, neither blocking

1. (low) The scanner only tracks brace depth, so a variable named $mutation in a read — query Foo($mutation: Boolean!) { ... } — is now counted as a depth-zero mutation token ($ falls to the default: i++ arm at linear_rules.go:437, then mutation is an identifier). Consequence: that read is tier-checked and refused for ADVISORY, and marked write=true for higher tiers. Fail-closed and very unlikely against GitHub's schema, so I'd leave it; mentioning it so the behaviour is known.
2. The body says "graphQLMutationRe is kept", but rules.go:580 deletes it and nothing else in src/ references it (grep is clean, and CI is green). The code is right; the sentence is stale.

I have not approved anything; a maintainer should sign off.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88