Skip to content

ci: unbreak Static gates (stale allowlist paths, plus a forbidden name reused) - #242

Open
laksamanakeris wants to merge 3 commits into
NodeDB-Lab:mainfrom
laksamanakeris:fix/rename-client-scope-accessor
Open

ci: unbreak Static gates (stale allowlist paths, plus a forbidden name reused)#242
laksamanakeris wants to merge 3 commits into
NodeDB-Lab:mainfrom
laksamanakeris:fix/rename-client-scope-accessor

Conversation

@laksamanakeris

Copy link
Copy Markdown
Contributor

Static gates has been failing on every pull request since 2026-08-09. It runs on pull_request only, so main has 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

043d76090 split control/scatter_gather.rs into a directory, moving build_graph_traverse_sql and its canonical_direction_sql / canonical_label_sql helpers into control/scatter_gather/remote_sql.rs. The gate's PATH_CANONICAL_HELPERS key stayed on the old path, and direct_canonical only 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 bare node_id in that same format! 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

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. sql_loop.rs is in ALLOWED_REFERENCES for 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_scope consumes 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 three control/array_sync/* call sites are individually allowlisted for it.

cc3b19989 then added ClientRequestScope::into_scope, whose body is self.scope and 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_scope is the fix here rather than seven allowlist entries. ALLOWED_REFERENCES is keyed (path, name), so exempting into_scope in pgwire/handler/routing/planning.rs would 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

  • All nine Static gates steps pass locally: 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-test where it has one.
  • Both gate fixes were checked for teeth, not just for silence: reintroducing an unquoted argument at the repointed site still fails gate 1.
  • cargo check -p nodedb --all-targets clean; cargo fmt --all --check clean.
  • cargo nextest run -p nodedb --lib -E 'test(/request_scope::/)': 20/20.
  • Bisected the start of each failure: 1 violation from e00d371c0 (2026-08-09), 8 from cc3b19989 (2026-08-10).

#241 is blocked on this and carries no CI changes of its own.

`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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant