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() } } } diff --git a/scripts/ci/check_authorized_dispatch.py b/scripts/ci/check_authorized_dispatch.py index bbbb62e0c..edc8dd857 100644 --- a/scripts/ci/check_authorized_dispatch.py +++ b/scripts/ci/check_authorized_dispatch.py @@ -103,7 +103,7 @@ ("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"), ( @@ -265,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") @@ -273,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 9cc148d70..7b5481304 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", }, @@ -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):")