mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
Make invoke_query graph-scoped (one branch authority)
invoke_query gates reaching the curated stored-query surface — a graph-level capability. Per-branch/snapshot access is already enforced by the inner read/change gate in run_query/run_mutate (authorized against the resolved branch), so branch-scoping the outer gate was redundant AND wrong for snapshot reads (it defaulted to main). Drop the branch dimension: remove InvokeQuery from uses_branch_scope (it joins admin as graph-scoped) and authorize the boundary gate with branch: None. Lossless: an actor confined to branch X by their read/change rules can still only invoke a stored query that touches X. A rule that sets branch_scope on invoke_query is now rejected by validate() — write invoke_query in its own rule. Ripple (atomic): restructure the server invoke fixture so invoke_query sits in its own branch_scope-free rule; invert invoke_query_is_branch_scoped -> invoke_query_rejects_branch_scope; the per-graph authorize test uses branch: None; docs (policy.md, server.md, the InvokeQuery doc). No wire/OpenAPI change.
This commit is contained in:
parent
c9e13f3707
commit
ad2fc27849
5 changed files with 53 additions and 48 deletions
|
|
@ -57,16 +57,19 @@ pub enum PolicyAction {
|
|||
/// `omnigraph.yaml` and restarting.
|
||||
GraphList,
|
||||
/// Gates invoking a server-side stored query by name. Per-graph and
|
||||
/// branch-scoped, like `Read`/`Change`. In this release it is
|
||||
/// **coarse**: an `invoke_query` allow rule permits *any* stored
|
||||
/// query on the graph (there is no per-query dimension yet). A
|
||||
/// future, additive refinement adds an optional query-name scope to
|
||||
/// rules without changing rules written against the coarse action.
|
||||
/// **graph-scoped** (no branch dimension, like `Admin`): the per-branch
|
||||
/// access of the query body is enforced by the inner `Read`/`Change`
|
||||
/// gate, so branch-scoping this outer gate would be redundant (and was
|
||||
/// wrong for snapshot reads). A rule that sets `branch_scope` on
|
||||
/// `invoke_query` is rejected by `validate()`. In this release it is
|
||||
/// **coarse**: an `invoke_query` allow rule permits *any* stored query
|
||||
/// on the graph (no per-query dimension yet); a future, additive
|
||||
/// refinement adds an optional query-name scope.
|
||||
///
|
||||
/// This gate sits at the HTTP boundary. The underlying engine `_as`
|
||||
/// writers still enforce `Read`/`Change` per the stored query's body,
|
||||
/// so a stored *mutation* is double-gated: `invoke_query` to reach
|
||||
/// the tool, plus `change` for the write itself.
|
||||
/// This gate sits at the HTTP boundary. The engine `_as` writers still
|
||||
/// enforce `Read`/`Change` per the query body, so a stored *mutation*
|
||||
/// is double-gated: `invoke_query` to reach the tool, plus `change` for
|
||||
/// the write itself.
|
||||
InvokeQuery,
|
||||
}
|
||||
|
||||
|
|
@ -87,10 +90,7 @@ impl PolicyAction {
|
|||
}
|
||||
|
||||
fn uses_branch_scope(self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::Read | Self::Export | Self::Change | Self::InvokeQuery
|
||||
)
|
||||
matches!(self, Self::Read | Self::Export | Self::Change)
|
||||
}
|
||||
|
||||
fn uses_target_branch_scope(self) -> bool {
|
||||
|
|
@ -1306,7 +1306,7 @@ rules:
|
|||
"act-alice",
|
||||
&PolicyRequest {
|
||||
action: PolicyAction::InvokeQuery,
|
||||
branch: Some("main".to_string()),
|
||||
branch: None,
|
||||
target_branch: None,
|
||||
},
|
||||
)
|
||||
|
|
@ -1323,7 +1323,7 @@ rules:
|
|||
"act-bruno",
|
||||
&PolicyRequest {
|
||||
action: PolicyAction::InvokeQuery,
|
||||
branch: Some("main".to_string()),
|
||||
branch: None,
|
||||
target_branch: None,
|
||||
},
|
||||
)
|
||||
|
|
@ -1332,10 +1332,10 @@ rules:
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn invoke_query_is_branch_scoped() {
|
||||
// Unlike server-scoped actions, invoke_query accepts a
|
||||
// `branch_scope` qualifier — it runs against a branch like
|
||||
// read/change — so validation passes and the rule authorizes.
|
||||
fn invoke_query_rejects_branch_scope() {
|
||||
// invoke_query is graph-scoped (like admin) — per-branch access is
|
||||
// enforced by the inner read/change gate — so a rule that puts a
|
||||
// `branch_scope` qualifier on it is rejected at validate().
|
||||
let policy: PolicyConfig = serde_yaml::from_str(
|
||||
r#"
|
||||
version: 1
|
||||
|
|
@ -1350,19 +1350,11 @@ rules:
|
|||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
policy.validate().unwrap();
|
||||
let engine = PolicyCompiler::compile(&policy, "graph").unwrap();
|
||||
let allow = engine
|
||||
.authorize(
|
||||
"act-alice",
|
||||
&PolicyRequest {
|
||||
action: PolicyAction::InvokeQuery,
|
||||
branch: Some("review".to_string()),
|
||||
target_branch: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
assert!(allow.allowed);
|
||||
let err = policy.validate().unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("branch_scope") && err.contains("invoke_query"),
|
||||
"branch_scope on invoke_query must be rejected: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -2213,7 +2213,11 @@ async fn server_invoke_query(
|
|||
handle.policy.as_deref(),
|
||||
PolicyRequest {
|
||||
action: PolicyAction::InvokeQuery,
|
||||
branch: req.branch.clone().or_else(|| Some("main".to_string())),
|
||||
// Graph-scoped: no branch dimension. The per-branch/snapshot
|
||||
// access is enforced by the inner read/change gate in the
|
||||
// runner, so the outer gate must not resolve a branch (doing so
|
||||
// was wrong for snapshot reads).
|
||||
branch: None,
|
||||
target_branch: None,
|
||||
},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -249,25 +249,34 @@ groups:
|
|||
invoke_only: ["act-invokeonly"]
|
||||
protected_branches: [main]
|
||||
rules:
|
||||
- id: invokers-invoke-and-read
|
||||
# invoke_query is graph-scoped — its own rules, no branch_scope.
|
||||
- id: invokers-can-invoke
|
||||
allow:
|
||||
actors: { group: invokers }
|
||||
actions: [invoke_query, read]
|
||||
branch_scope: any
|
||||
- id: full-invoke-read-change
|
||||
actions: [invoke_query]
|
||||
- id: full-can-invoke
|
||||
allow:
|
||||
actors: { group: full }
|
||||
actions: [invoke_query, read, change]
|
||||
branch_scope: any
|
||||
- id: readers-read-only
|
||||
allow:
|
||||
actors: { group: readers }
|
||||
actions: [read]
|
||||
branch_scope: any
|
||||
- id: invoke-only-no-read
|
||||
actions: [invoke_query]
|
||||
- id: invoke-only-can-invoke
|
||||
allow:
|
||||
actors: { group: invoke_only }
|
||||
actions: [invoke_query]
|
||||
# read / change are branch-scoped.
|
||||
- id: invokers-can-read
|
||||
allow:
|
||||
actors: { group: invokers }
|
||||
actions: [read]
|
||||
branch_scope: any
|
||||
- id: full-can-read-change
|
||||
allow:
|
||||
actors: { group: full }
|
||||
actions: [read, change]
|
||||
branch_scope: any
|
||||
- id: readers-can-read
|
||||
allow:
|
||||
actors: { group: readers }
|
||||
actions: [read]
|
||||
branch_scope: any
|
||||
"#;
|
||||
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ Per-graph actions (bind to `Omnigraph::Graph::"<graph_id>"`):
|
|||
6. `branch_delete`
|
||||
7. `branch_merge`
|
||||
8. `admin` — reserved for policy-management surfaces (hot reload, audit log, approvals). No call site today; see MR-724 for the reservation rationale.
|
||||
9. `invoke_query` — gates invoking a server-side stored query (the `queries:` registry). Branch-scoped, like `read` / `change`. Coarse in this release: an `invoke_query` allow rule permits any stored query on the graph; a future, additive refinement adds an optional per-query-name scope without changing rules written against the coarse action. Enforced at `POST /queries/{name}` (see [server](server.md)). A stored *mutation* is double-gated: `invoke_query` to reach the tool, plus `change` for the write itself (the engine `_as` writers still enforce per the query body).
|
||||
9. `invoke_query` — gates invoking a server-side stored query (the `queries:` registry). Graph-scoped (like `admin`) — per-branch access is enforced by the inner `read` / `change` gate, so a rule that sets `branch_scope` on `invoke_query` is rejected. Coarse in this release: an `invoke_query` allow rule permits any stored query on the graph; a future, additive refinement adds an optional per-query-name scope without changing rules written against the coarse action. Enforced at `POST /queries/{name}` (see [server](server.md)). A stored *mutation* is double-gated: `invoke_query` to reach the tool, plus `change` for the write itself (the engine `_as` writers still enforce per the query body).
|
||||
|
||||
Server-scoped action (v0.6.0+; binds to `Omnigraph::Server::"root"`):
|
||||
|
||||
|
|
@ -24,7 +24,7 @@ Server-scoped actions cannot use `branch_scope` or `target_branch_scope` — the
|
|||
|
||||
## Scope kinds
|
||||
|
||||
- `branch_scope` — applied to source branch (`read`, `export`, `change`, `invoke_query`)
|
||||
- `branch_scope` — applied to source branch (`read`, `export`, `change`)
|
||||
- `target_branch_scope` — applied to destination (`schema_apply`, branch ops, run ops)
|
||||
- `protected_branches` — named list with special rules; rule scopes are `any | protected | unprotected`
|
||||
|
||||
|
|
|
|||
|
|
@ -68,7 +68,7 @@ List the graph's **`mcp.expose`** stored queries as a typed tool catalog — eno
|
|||
|
||||
Invoke a curated, server-side stored query by **name** — the source comes from the graph's `queries:` registry, so the client never sends `.gq`. Body (all fields optional): `{ "params": { … }, "branch": "main", "snapshot": null }`, where `params` keys match the query's declared parameters. The response is the **read envelope** (`ReadOutput`) for a stored read or the **mutation envelope** (`ChangeOutput`) for a stored mutation — serialized untagged, so the wire shape is identical to `/query` / `/mutate`.
|
||||
|
||||
- **Gate:** `invoke_query` (per-graph, branch-scoped) at the boundary. A stored *mutation* is **double-gated** — it also passes the engine's `change` gate, so an actor with `invoke_query` but not `change` gets `403`.
|
||||
- **Gate:** `invoke_query` (per-graph, graph-scoped) at the boundary. A stored *mutation* is **double-gated** — it also passes the engine's `change` gate, so an actor with `invoke_query` but not `change` gets `403`.
|
||||
- **Deny == unknown, for callers without `invoke_query`:** for a caller lacking the grant, an `invoke_query` denial and an unknown query name return the **same `404`** (identical body), so the catalog can't be probed. A caller that *holds* `invoke_query` may still get the inner gate's `403` for an existing query it can't `read`/`change` (the double-gate, above) — so existence is visible to grant-holders by design.
|
||||
- **Requires an explicit policy grant when auth is on.** In default-deny mode (bearer tokens but no `policy.file`), only `read` is permitted, so *every* `/queries/{name}` call returns `404` until an `invoke_query` rule is configured.
|
||||
- A stored mutation cannot target a `snapshot` (`400`); a parameter type error is a structured `400` naming the parameter.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue