ci: unbreak Static gates (stale allowlist paths, plus a forbidden name reused) - #242
Open
laksamanakeris wants to merge 3 commits into
Open
ci: unbreak Static gates (stale allowlist paths, plus a forbidden name reused)#242laksamanakeris wants to merge 3 commits into
laksamanakeris wants to merge 3 commits into
Conversation
`AuthorizedCapability::into_scope` consumes a granted capability, and the authorized-dispatch gate forbids it by name in external transports because reaching it there would mean a transport unwrapping authority it never consumed. `ClientRequestScope::into_scope` was added later with the same name and unwraps only the peer address a scope was resolved against, granting nothing. Because the gate matches by name, the six transports that unwrap the address binding (native, pgwire, RESP, and three HTTP routes) read as bypassing authorization, and the gate has failed on every pull request since. An allowlist entry would fix the report by exempting the name `into_scope` per file, which would also exempt a genuine capability unwrap appearing in those files later; renaming keeps the forbidden name meaning exactly one thing. Pure rename: definition, nine call sites, and the builder/client_scope tests.
`043d76090` split `control/scatter_gather.rs` into a directory and moved `build_graph_traverse_sql` (with its `canonical_direction_sql` and `canonical_label_sql` helpers) into `control/scatter_gather/remote_sql.rs`, but left the reconstructed-SQL gate's `PATH_CANONICAL_HELPERS` key on the old path. The helpers stopped being recognised as canonical, so the gate reported the two helper-built fragments as unquoted interpolation and failed on a file whose quoting never changed. The gate runs on pull requests only, so main carries the break silently and every PR opened since inherits it. Repointing the key restores recognition without widening it: replacing the `quote_literal(node_id)` argument in that same builder with a bare `node_id` still fails the gate, so the site keeps its teeth for anything the two allowlisted helpers do not cover. (cherry picked from commit e072368)
…task `7102813b8` moved the native DROP ARRAY path out of `sql_loop.rs` into the new `sql_dispatch_task.rs`, carrying its `authorize_native_task(..)?` -> `into_physical_task()` pair with it. `sql_loop.rs` is in ALLOWED_REFERENCES for exactly that seam; the new file is not, so the gate has flagged the moved call ever since. The call is unchanged: the capability is consumed first and the raw task is only reached through it, which is the condition the allowlist documents. This does not cover the six `into_scope` references also failing on main; those are a separate finding, reported on the PR. (cherry picked from commit 1491327)
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.
Static gates has been failing on every pull request since 2026-08-09. It runs on
pull_requestonly, somainhas had nowhere to report it, and every PR opened since inherits the failure. Three independent causes, none of them a real unsafety.1. Reconstructed-SQL gate: allowlist keyed on a moved file
043d76090splitcontrol/scatter_gather.rsinto a directory, movingbuild_graph_traverse_sqland itscanonical_direction_sql/canonical_label_sqlhelpers intocontrol/scatter_gather/remote_sql.rs. The gate'sPATH_CANONICAL_HELPERSkey stayed on the old path, anddirect_canonicalonly treats a bare helper call as canonical when its name appears in that per-path set, so the two helper-built fragments started reading as unquoted interpolation. The quoting in that file never changed.Repointing the key is the fix, and it does not widen the gate: replacing
quote_literal(node_id)with a barenode_idin that sameformat!still fails, so the site keeps its teeth for everything the two helpers do not cover.2. Authorized-dispatch gate: capability seam moved to a new file
7102813b8moved the native DROP ARRAY path out ofsql_loop.rsinto the newsql_dispatch_task.rs, carrying itsauthorize_native_task(..)?->into_physical_task()pair.sql_loop.rsis inALLOWED_REFERENCESfor exactly that seam; the new file was not. The capability is still consumed before the raw task is reached, which is the condition the allowlist documents.3. Authorized-dispatch gate: a forbidden name reused for an unrelated API
This is the one worth reading closely.
AuthorizedCapability::into_scopeconsumes a granted capability. The gate forbids it by name in external transports, because a transport reaching it would be unwrapping authority it never consumed; the threecontrol/array_sync/*call sites are individually allowlisted for it.cc3b19989then addedClientRequestScope::into_scope, whose body isself.scopeand whose doc says "Consume the binding once admission has run, keeping the scope". It unwraps the peer address a scope was resolved against. It consumes no capability and grants no authority. Since the gate matches by bare name, the six transports that unwrap that binding (native, pgwire, RESP, and three HTTP routes) all began reporting as authorization bypasses, plus the definition itself under the "must not be public" rule.Renaming it to
into_resolved_scopeis the fix here rather than seven allowlist entries.ALLOWED_REFERENCESis keyed(path, name), so exemptinginto_scopeinpgwire/handler/routing/planning.rswould also exempt a genuine capability unwrap appearing in that file later: it would trade a false positive for a permanent blind spot in six transport files, in the exact files this gate exists to watch. The rename keeps the forbidden name meaning one thing.Pure rename: the definition, nine call sites, and the builder/client_scope tests. No behavior change.
Verification
check_warm_storage_gate.sh,check_format_expects.sh,check_calvin_determinism.sh,check_plane_separation.sh,check_raft_transport_boundary.py,check_reconstructed_sql.py,check_authorized_dispatch.py,check_robust_parsing.py,check_advisory_ignores.py, each with its--self-testwhere it has one.cargo check -p nodedb --all-targetsclean;cargo fmt --all --checkclean.cargo nextest run -p nodedb --lib -E 'test(/request_scope::/)': 20/20.e00d371c0(2026-08-09), 8 fromcc3b19989(2026-08-10).#241 is blocked on this and carries no CI changes of its own.