From 50376e456bf8ec8fa65f727b72c4dd7305ef00d7 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 15 Aug 2026 00:12:55 +0800 Subject: [PATCH 1/4] refactor(security): rename ClientRequestScope::into_scope `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. --- .../src/control/security/request_scope/builder.rs | 14 +++++++------- .../control/security/request_scope/client_scope.rs | 12 +++++++++--- .../server/http/routes/query/materialized.rs | 2 +- .../src/control/server/http/routes/query/ndjson.rs | 2 +- .../server/http/routes/ws_rpc/execute_sql.rs | 6 ++++-- nodedb/src/control/server/ilp_batch/dispatch.rs | 2 +- .../src/control/server/native/session/request.rs | 2 +- .../server/pgwire/handler/routing/planning.rs | 2 +- nodedb/src/control/server/resp/gateway_dispatch.rs | 2 +- .../src/control/server/shared/ddl/user_dispatch.rs | 4 ++-- 10 files changed, 28 insertions(+), 20 deletions(-) diff --git a/nodedb/src/control/security/request_scope/builder.rs b/nodedb/src/control/security/request_scope/builder.rs index a82681484..12df074ac 100644 --- a/nodedb/src/control/security/request_scope/builder.rs +++ b/nodedb/src/control/security/request_scope/builder.rs @@ -542,7 +542,7 @@ mod tests { let inside = RequestAuthScope::builder(&identity, stores) .build_for_client("10.0.0.1:5432") - .into_scope(); + .into_resolved_scope(); assert_eq!( inside.auth().metadata.get("scope_status.pro:all"), Some(&nodedb_types::Value::String("active".into())) @@ -550,7 +550,7 @@ mod tests { let outside = RequestAuthScope::builder(&identity, stores) .build_for_client("203.0.113.9:5432") - .into_scope(); + .into_resolved_scope(); assert!( !outside.auth().metadata.contains_key("scope_status.pro:all"), "a request from outside the permitted network must not get the scope" @@ -624,7 +624,7 @@ mod tests { let scope = RequestAuthScope::builder(&identity, stores) .build_for_client("10.0.0.1:5432") - .into_scope(); + .into_resolved_scope(); assert_eq!(scope.auth().risk_score, None); } @@ -639,7 +639,7 @@ mod tests { let scope = RequestAuthScope::builder(&identity, stores) .build_for_client("10.0.0.1:5432") - .into_scope(); + .into_resolved_scope(); let score = scope .auth() @@ -668,7 +668,7 @@ mod tests { let scope = RequestAuthScope::builder(&identity, stores) .build_for_client("10.0.0.1:5432") - .into_scope(); + .into_resolved_scope(); let refusal = scorer .refusal_for(scope.auth()) @@ -689,7 +689,7 @@ mod tests { let scope = RequestAuthScope::builder(&identity, stores) .build_for_client("http") - .into_scope(); + .into_resolved_scope(); assert_eq!(scope.auth().risk_score, None); assert!(scorer.refusal_for(scope.auth()).is_some()); @@ -713,7 +713,7 @@ mod tests { let scope = RequestAuthScope::builder(&identity, stores) .build_for_client("10.0.0.1:5432") - .into_scope(); + .into_resolved_scope(); assert_eq!(scope.auth().risk_score, None); } diff --git a/nodedb/src/control/security/request_scope/client_scope.rs b/nodedb/src/control/security/request_scope/client_scope.rs index 9d5a445c0..e4ac2ff00 100644 --- a/nodedb/src/control/security/request_scope/client_scope.rs +++ b/nodedb/src/control/security/request_scope/client_scope.rs @@ -110,7 +110,13 @@ impl<'a, 'p> ClientRequestScope<'a, 'p> { } /// Consume the binding once admission has run, keeping the scope. - pub fn into_scope(self) -> RequestAuthScope<'a> { + /// + /// Named apart from `AuthorizedCapability::into_scope`: that one consumes a + /// granted capability, while this only unwraps the address it was resolved + /// against. The authorized-dispatch gate forbids the capability API by + /// name in external transports, so a second `into_scope` here would read as + /// every transport bypassing authorization. + pub fn into_resolved_scope(self) -> RequestAuthScope<'a> { self.scope } } @@ -173,7 +179,7 @@ mod tests { } #[test] - fn into_scope_keeps_the_resolved_scope() { + fn into_resolved_scope_keeps_the_resolved_scope() { let identity = identity(); let grants = ScopeGrantStore::new(); let quotas = QuotaManager::new(); @@ -186,7 +192,7 @@ mod tests { DatabaseId::new(9), "10.0.0.1:5432", ) - .into_scope(); + .into_resolved_scope(); assert_eq!(scope.database_id(), DatabaseId::new(9)); } diff --git a/nodedb/src/control/server/http/routes/query/materialized.rs b/nodedb/src/control/server/http/routes/query/materialized.rs index 92f48a4f3..bb1cfe5d2 100644 --- a/nodedb/src/control/server/http/routes/query/materialized.rs +++ b/nodedb/src/control/server/http/routes/query/materialized.rs @@ -82,7 +82,7 @@ pub async fn query( &request, "sql", )?; - let scope = request.into_scope(); + let scope = request.into_resolved_scope(); let rate_limit_headers = super::super::super::rate_limit_headers::rate_limit_headers(&rate_limit_result); diff --git a/nodedb/src/control/server/http/routes/query/ndjson.rs b/nodedb/src/control/server/http/routes/query/ndjson.rs index f3d8c2304..244ac4812 100644 --- a/nodedb/src/control/server/http/routes/query/ndjson.rs +++ b/nodedb/src/control/server/http/routes/query/ndjson.rs @@ -114,7 +114,7 @@ pub async fn query_ndjson( Ok(result) => result, Err(error) => return ApiError::from(error).into_response(), }; - let scope = request.into_scope(); + let scope = request.into_resolved_scope(); let rate_limit_headers = super::super::super::rate_limit_headers::rate_limit_headers(&rate_limit_result); diff --git a/nodedb/src/control/server/http/routes/ws_rpc/execute_sql.rs b/nodedb/src/control/server/http/routes/ws_rpc/execute_sql.rs index ee3982192..1ee5c0aa4 100644 --- a/nodedb/src/control/server/http/routes/ws_rpc/execute_sql.rs +++ b/nodedb/src/control/server/http/routes/ws_rpc/execute_sql.rs @@ -53,8 +53,10 @@ pub async fn execute_sql( // success; a denial still fails the request closed via `?`. crate::control::server::session_auth::check_request_admission(shared, &request, "sql")?; - let (clean_sql, scope) = - crate::control::server::session_auth::apply_per_query_on_deny(sql, request.into_scope()); + let (clean_sql, scope) = crate::control::server::session_auth::apply_per_query_on_deny( + sql, + request.into_resolved_scope(), + ); // Planning and lease admission run as one retried unit so a descriptor // drain starting between them is absorbed rather than surfaced. The scope // is retained through every orchestrated or Data-Plane execution and diff --git a/nodedb/src/control/server/ilp_batch/dispatch.rs b/nodedb/src/control/server/ilp_batch/dispatch.rs index b52cd4517..cecb2dacd 100644 --- a/nodedb/src/control/server/ilp_batch/dispatch.rs +++ b/nodedb/src/control/server/ilp_batch/dispatch.rs @@ -190,7 +190,7 @@ async fn flush_ilp_batch_inner( // read differently on this transport than it does on a planned `INSERT`. let scope = ClientRequestScope::for_database(identity, state.auth_stores(), database_id, peer_addr) - .into_scope(); + .into_resolved_scope(); crate::control::planner::rls_injection::inject_rls(&mut tasks, &state.rls, scope.auth())?; // A spent hard quota refuses the batch before any of it is staged. The diff --git a/nodedb/src/control/server/native/session/request.rs b/nodedb/src/control/server/native/session/request.rs index 994f16cac..f75c8c120 100644 --- a/nodedb/src/control/server/native/session/request.rs +++ b/nodedb/src/control/server/native/session/request.rs @@ -190,7 +190,7 @@ impl NativeSession { let ctx = DispatchCtx { state: &self.state, identity, - scope: request_scope.into_scope(), + scope: request_scope.into_resolved_scope(), query_ctx: &self.query_ctx, sessions: &self.sessions, peer_addr: &self.peer_addr, diff --git a/nodedb/src/control/server/pgwire/handler/routing/planning.rs b/nodedb/src/control/server/pgwire/handler/routing/planning.rs index ec8b364fa..08a41ff5d 100644 --- a/nodedb/src/control/server/pgwire/handler/routing/planning.rs +++ b/nodedb/src/control/server/pgwire/handler/routing/planning.rs @@ -183,7 +183,7 @@ impl NodeDbPgHandler { // immediately. let scope = scope_builder .build_for_client(&peer_addr.to_string()) - .into_scope(); + .into_resolved_scope(); // Request-admission already ran once for this statement in // `execute_single_sql`, before it branched to `shared::ddl::dispatch` diff --git a/nodedb/src/control/server/resp/gateway_dispatch.rs b/nodedb/src/control/server/resp/gateway_dispatch.rs index 30845baa9..1c790cab2 100644 --- a/nodedb/src/control/server/resp/gateway_dispatch.rs +++ b/nodedb/src/control/server/resp/gateway_dispatch.rs @@ -231,7 +231,7 @@ fn authorize_resp_task( // call covers the whole protocol, including the IP-blacklist half via // `session.peer_addr` (set at connection accept). crate::control::server::session_auth::check_request_admission(state, &request, operation)?; - let scope = request.into_scope(); + let scope = request.into_resolved_scope(); // Row-level security is injected here, before the capability is minted, for // the same reason the native path injects before dispatch: the plan the diff --git a/nodedb/src/control/server/shared/ddl/user_dispatch.rs b/nodedb/src/control/server/shared/ddl/user_dispatch.rs index 3dde7b2de..b78509864 100644 --- a/nodedb/src/control/server/shared/ddl/user_dispatch.rs +++ b/nodedb/src/control/server/shared/ddl/user_dispatch.rs @@ -167,7 +167,7 @@ fn authorize_for_identity( &request, operation_for_plan(&plan), )?; - request.into_scope() + request.into_resolved_scope() } }; @@ -234,7 +234,7 @@ fn resolve_dispatch_scope<'a>( match admission { RequestAdmission::AlreadyAdmitted => builder.build(), RequestAdmission::NotYetAdmitted { peer_addr } => { - builder.build_for_client(peer_addr).into_scope() + builder.build_for_client(peer_addr).into_resolved_scope() } } } From e27b69f02b40e7302d789a628885c9b3902b5366 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Fri, 14 Aug 2026 23:35:25 +0800 Subject: [PATCH 2/4] ci(sql-gate): point the canonical-helper allowlist at the moved file `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 e07236889712ad79e51a88ccc8c1d6bd734936ea) --- scripts/ci/check_reconstructed_sql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/check_reconstructed_sql.py b/scripts/ci/check_reconstructed_sql.py index 9cc148d70..c40a65149 100644 --- a/scripts/ci/check_reconstructed_sql.py +++ b/scripts/ci/check_reconstructed_sql.py @@ -39,7 +39,7 @@ "json_value_to_sql_literal", }, "control/event_trigger.rs": {"canonical_trigger_template_sql"}, - "control/scatter_gather.rs": { + "control/scatter_gather/remote_sql.rs": { "canonical_direction_sql", "canonical_label_sql", }, From 894586c0ebdf391f8329c2f61e2e584dcec38044 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Fri, 14 Aug 2026 23:56:44 +0800 Subject: [PATCH 3/4] ci(dispatch-gate): allow the capability seam moved into sql_dispatch_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 14913273c08d0643bde83d2dbc49077e9b91d40c) --- scripts/ci/check_authorized_dispatch.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/ci/check_authorized_dispatch.py b/scripts/ci/check_authorized_dispatch.py index bbbb62e0c..6bae5f326 100644 --- a/scripts/ci/check_authorized_dispatch.py +++ b/scripts/ci/check_authorized_dispatch.py @@ -104,6 +104,7 @@ ("control/array_sync/inbound_propose.rs", "into_scope"), ("control/array_sync/snapshot_assembly.rs", "into_scope"), ("control/server/native/dispatch/sql_loop.rs", "into_physical_task"), + ("control/server/native/dispatch/sql_dispatch_task.rs", "into_physical_task"), ("control/server/sync/raft_dispatch/response.rs", "propose_sync_write"), ("control/server/sync/raft_dispatch/write.rs", "propose_sync_write"), ( From 3002d6f3494331e7b5b6d432819f9650ebce59e0 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Sat, 22 Aug 2026 14:59:52 +0800 Subject: [PATCH 4/4] ci: fail CI gates when their exemption entries go stale A moved or renamed file silently disarms the entry that named it, letting new violations through instead of being caught. Both check_authorized_dispatch.py and check_reconstructed_sql.py now scan their exemption tables for entries that no longer match any file or occurrence and fail the gate if any are found, and the now-dead sql_loop.rs reference is dropped. --- scripts/ci/check_authorized_dispatch.py | 41 ++++++++++++++++++++++++- scripts/ci/check_reconstructed_sql.py | 31 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/scripts/ci/check_authorized_dispatch.py b/scripts/ci/check_authorized_dispatch.py index 6bae5f326..edc8dd857 100644 --- a/scripts/ci/check_authorized_dispatch.py +++ b/scripts/ci/check_authorized_dispatch.py @@ -103,7 +103,6 @@ ("control/array_sync/inbound.rs", "into_scope"), ("control/array_sync/inbound_propose.rs", "into_scope"), ("control/array_sync/snapshot_assembly.rs", "into_scope"), - ("control/server/native/dispatch/sql_loop.rs", "into_physical_task"), ("control/server/native/dispatch/sql_dispatch_task.rs", "into_physical_task"), ("control/server/sync/raft_dispatch/response.rs", "propose_sync_write"), ("control/server/sync/raft_dispatch/write.rs", "propose_sync_write"), @@ -266,6 +265,35 @@ def public_raw_api_violations(rel: str, source: str) -> list[tuple[int, str]]: return found +def stale_exemptions() -> list[str]: + """Report exemptions that no longer name a live occurrence. + + An exemption is keyed on (path, symbol). Move the file or rename the + symbol and the entry stops matching anything — it does not fail, it + silently stops being an exemption, and the next occurrence added to + that path is waved through instead of being caught. Both halves of + this gate have already been disarmed that way by an ordinary file + move, so a dead entry is treated as a gate failure, not as tidiness. + + The symbol is looked for in masked source, so an entry kept alive only + by a comment or a string literal still reports as dead. + """ + stale: list[str] = [] + for label, exemptions in ( + ("ALLOWED_DEFINITIONS", ALLOWED_DEFINITIONS), + ("ALLOWED_REFERENCES", ALLOWED_REFERENCES), + ): + for rel, name in sorted(exemptions): + path = SRC / rel + if not path.is_file(): + stale.append(f"{label}: ({rel!r}, {name!r}) names a file that does not exist") + continue + masked = mask_rust(path.read_text(encoding="utf-8")) + if not re.search(rf"\b{re.escape(name)}\b", masked): + stale.append(f"{label}: ({rel!r}, {name!r}) matches no occurrence in that file") + return stale + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--self-test", action="store_true") @@ -274,6 +302,17 @@ def main() -> int: self_test() return 0 + stale = stale_exemptions() + if stale: + print("ERROR: stale exemptions — the gate is disarmed where they point:", file=sys.stderr) + for entry in stale: + print(f" {entry}", file=sys.stderr) + print( + "Repoint each entry at the code's current location, or drop it if the seam is gone.", + file=sys.stderr, + ) + return 1 + errors: list[str] = [] for path in SRC.rglob("*.rs"): rel = path.relative_to(SRC).as_posix() diff --git a/scripts/ci/check_reconstructed_sql.py b/scripts/ci/check_reconstructed_sql.py index c40a65149..7b5481304 100644 --- a/scripts/ci/check_reconstructed_sql.py +++ b/scripts/ci/check_reconstructed_sql.py @@ -871,12 +871,43 @@ def self_test() -> int: return 1 if failed else 0 +def stale_canonical_helpers() -> list[str]: + """Report PATH_CANONICAL_HELPERS entries that no longer name live code. + + Each entry widens the gate for one file: the named helpers are read as + canonical there, so a bare call to one stops counting as unquoted + interpolation. The key is a path. Move the file and the entry keeps + widening nothing, while the helpers at their new home silently lose + their canonical status and the file starts reporting false violations — + which is how this gate came to fail on every pull request. A dead entry + is therefore a gate failure, not tidiness. + """ + stale: list[str] = [] + for rel, helpers in sorted(PATH_CANONICAL_HELPERS.items()): + path = RUST_ROOT / rel + if not path.is_file(): + stale.append(f"{rel!r} names a file that does not exist") + continue + source = path.read_text(encoding="utf-8") + for helper in sorted(helpers): + if not re.search(rf"\b{re.escape(helper)}\b", source): + stale.append(f"{rel!r} exempts {helper!r}, which does not appear in that file") + return stale + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--self-test", action="store_true") args = parser.parse_args() if args.self_test: return self_test() + stale = stale_canonical_helpers() + if stale: + print("FAIL: stale canonical-helper exemptions — the gate is misaimed where they point:") + for entry in stale: + print(f" {entry}") + print("\nRepoint each entry at the code's current location, or drop it if the helper is gone.") + return 1 findings = scan_paths(path for root in SCAN_ROOTS for path in root.rglob("*.rs")) if findings: print(f"FAIL: {len(findings)} reconstructed-SQL gate violation(s):")