Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions nodedb/src/control/security/request_scope/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,15 +542,15 @@ 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()))
);

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"
Expand Down Expand Up @@ -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);
}
Expand All @@ -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()
Expand Down Expand Up @@ -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())
Expand All @@ -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());
Expand All @@ -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);
}
Expand Down
12 changes: 9 additions & 3 deletions nodedb/src/control/security/request_scope/client_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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();
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion nodedb/src/control/server/http/routes/query/ndjson.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 4 additions & 2 deletions nodedb/src/control/server/http/routes/ws_rpc/execute_sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion nodedb/src/control/server/ilp_batch/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion nodedb/src/control/server/native/session/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion nodedb/src/control/server/resp/gateway_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions nodedb/src/control/server/shared/ddl/user_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn authorize_for_identity(
&request,
operation_for_plan(&plan),
)?;
request.into_scope()
request.into_resolved_scope()
}
};

Expand Down Expand Up @@ -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()
}
}
}
Expand Down
42 changes: 41 additions & 1 deletion scripts/ci/check_authorized_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
(
Expand Down Expand Up @@ -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")
Expand All @@ -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()
Expand Down
33 changes: 32 additions & 1 deletion scripts/ci/check_reconstructed_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down Expand Up @@ -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):")
Expand Down
Loading