From 50376e456bf8ec8fa65f727b72c4dd7305ef00d7 Mon Sep 17 00:00:00 2001 From: Hatta Zainal Date: Sat, 15 Aug 2026 00:12:55 +0800 Subject: [PATCH 1/3] 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/3] 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/3] 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"), (