Skip to content

feat(gateway): org-scope enforcement with user/group principals - #15

Merged
marcorivm merged 1 commit into
open-edition/04-project-accessfrom
open-edition/05-gateway-org-scope
Aug 8, 2026
Merged

feat(gateway): org-scope enforcement with user/group principals#15
marcorivm merged 1 commit into
open-edition/04-project-accessfrom
open-edition/05-gateway-org-scope

Conversation

@marcorivm

@marcorivm marcorivm commented Aug 6, 2026

Copy link
Copy Markdown
Member

Org-scope enforcement in the gateway, with user and group principals. 6 files, +1,080/−176 — 1,080 lines of code with 22 inline #[cfg(test)] tests (14 in evaluate.rs, 3 each in enforce.rs and loaders.rs, 2 in assemble.rs). No separate test files, which is easy to mistake for no tests. What is genuinely untested is loaders.rs's SQL — the org-fencing CTE that is the actual trust boundary.

Where the API-side RBAC from #11#13 becomes actual request-time enforcement. Worth reading alongside policy_engine rather than in isolation.

Stack

Split out of the original 381-file #8. Upstream catch-up (v1.42.0 → v1.44.0) already landed as #10, so main is now v1.44.0 and everything below is our own code.

main (v1.44.0, after #10)
 └─ #14  01-tier1-ungating              23 files    +73/-442
     └─ #11  02-org-members-rbac        47 files  +5298/-36
         └─ #12  03-user-groups         16 files  +2963/-1
             └─ #13  04-project-access  33 files  +7701/-234
                 └─ #15  05-gateway-org-scope       6 files  +1080/-176
                     └─ #16  06-gateway-conditions  23 files  +2292/-211
                         └─ #17  07-gateway-resource-scope  13 files  +1551/-20
                             └─ #18  08-web-org-policy      23 files  +2421/-384
                                 └─ #8   spend budgets      20 files  +2523/-35
                                     └─ #9   upstream-sync tooling  8 files  +823/-0

Review and merge in order, top to bottom. Roughly half of each diff is tests.

@marcorivm

Copy link
Copy Markdown
Member Author

Context for the gateway stack (#15#16#17)

This fork lands into the OSS gateway build (apps/gateway/src/policy_engine/) what upstream keeps EE/Cloud-only (apps/gateway/src/ee/policy_engine.rs, swapped in via #[path] in main.rs). The module doc at the top of policy_engine.rs is explicit: before this PR it says org scope, directory identities and granular session policies "are OneCLI Cloud capabilities and have no code here." This PR rewrites that comment.

All three sit on upstream 345ac58 ("drop the legacy policy model"), which made policy_rules_v2 the only model, evaluated first-match: rules walked in (priority, id) order, first identity+target match wins.

Per-request pipeline (stable across all three):

  1. CONNECT (connect.rs) — resolve agent/org/project, load and cache policy + injection state ~60s (load_connect_v2 at connect.rs:272, derive_inject_selection at :280).
  2. Forward (gateway/forward.rs::forward_request) or WebSocket (gateway/websocket.rs::handle_websocket) — per request: policy decision, then credential injection, then send upstream.
  3. Decision → injection ordering is the crux: does every path that can inject a real credential also pass the new checks?

What it actually does

Before this PR, the engine understood only project-scoped rules with agent identities. Any user/group identity decoded to Identity::Other, which never matches — so an org admin's directory-based rule was silently inert at the gateway even if the API let you author it.

This makes the gateway a genuine two-level evaluator:

  • Loads org-scope rules alongside project rules (loaders::find_published_policy_rules_v2_by_org, loaders.rs:20) and, lazily, the connection's principal set — the resolved user_ids/group_ids a proxied request belongs to (loaders::load_principal_set, loaders.rs:58, a single indexed CTE mirroring the TS resolvePrincipalSet).
  • Identities now decode to Agent/User/Group/Other (assemble.rs::decode_identities), and identity_matches (evaluate.rs:29) checks User/Group against that principal set.
  • The two levels are evaluated independently by first-match, then combined by a hard-floor law in evaluate_outcome (evaluate.rs:219): strictest verdict wins, org wins ties, and a lone ALLOW at one level cannot open the other level's default-Block.

Nothing changes about where the decision happens — still policy_engine::evaluate() once per request, before injection, in both forward.rs and websocket.rs. What changes is what the decision is a function of.

Why it exists

This turns the org RBAC from earlier in the stack into actual request-time enforcement. Without it, an org admin defining "block group Contractors from Slack" in the UI would do nothing.

Reading order

  1. policy_engine.rs — module doc rewrite; establishes the new OSS scope claim.
  2. policy_engine/types.rs — new Identity::{User,Group}, RuleScope::{Organization,Project}, Rule.scope. Everything is built on these shapes.
  3. policy_engine/loaders.rs — new file. The org query and principal-set CTE. The trust boundary lives here.
  4. policy_engine/assemble.rs — decodes DB rows into scope-tagged, identity-kind-aware Rules.
  5. policy_engine/evaluate.rsload-bearing: identity_matches (:29) and evaluate_outcome (:219). The module doc is the design spec.
  6. policy_engine/enforce.rs — the seam: load_connect_v2 (:82) wires loaders → assemble; evaluate (:179) wires assemble → evaluate_outcome → PolicyDecision.

connect.rs/forward.rs/websocket.rs are unchanged here — skim only to confirm the call signature didn't change shape.

What to scrutinise

  • evaluate_outcome (evaluate.rs:219-252) is the highest-value logic in the PR. Trace by hand: org rule matches Allow, project has no explicit match, project default is Block, enforce_deny() true → effective_org drops to None → falls to project_default_blocksDenyDefault. Confirm that's intended, and confirm the symmetric case (project allow vs org default-block) behaves the same.
  • Request::enforce_deny() now gates two defaults instead of one. Both org_default_blocks and project_default_blocks must be computed under the same enforce_deny value — they are (both read the one local), but verify.
  • Fail-closed identity decode — a rule naming an unknown principal kind decodes to Identity::Other and never matches. Good default, but a future identity kind (say "team" from a later migration) silently makes any rule using it unmatchable. Should that log or alert rather than fail silently?
  • Principal-set staleness — a new fail-open window. The set resolves once at CONNECT and caches ~60s. Previously only agent identity mattered, which doesn't change mid-session. Now group membership matters and can change: a user added to a blocked group still gets through for up to 60s. Worth a conscious call on whether 60s is acceptable for a security-relevant signal, not just for injection creds.
  • Org-fencing in the principal-set CTE (loaders.rs::load_principal_set) — every arm is org-fenced (g.organization_id = $2, om.organization_id = $2) so another org's group grant can't leak in. This needs a human to verify against the actual schema (FK directions, uniqueness of (user_id, org_id) in organization_members) because no test exercises it.
  • enforce.rs claims "Fail-closed: every resolution query PROPAGATES its error." Verified: all three lazy loads use .context(...)? and the caller does .map_err(db_err)?, refusing the CONNECT. But note the consequence — a transient DB hiccup during principal resolution now fails the entire connection, a failure mode that didn't exist before at this granularity.

Design decisions worth questioning

  • Two independent first-match passes (rather than one merged list) is deliberate and documented — "a single merged first-match can honor at most one of 'identity beats strictness' and 'org is un-overridable'." But the levels can never see each other's rules while matching; an org rule can't say "unless a project rule already allowed this." Presumably intended, worth confirming.
  • strictness_rank treats rate-limit and approval modifiers as fixed ranks (2 and 1) regardless of severity. A project rule with a 1-req/day limit and an org rule with 10000-req/day tie on rank and org wins by left-bias, though the project rule is intuitively stricter. This mirrors the TS oracle (strictness.ts) by design — worth confirming that oracle is genuinely the source of truth and hasn't drifted.

Test coverage reality

Correcting the PR description: "no test files" is misleading. There are 22 new test functions in this diff — 14 in evaluate.rs, 3 in enforce.rs, 3 in loaders.rs, 2 in assemble.rs — all as inline #[cfg(test)] mod tests blocks rather than separate files. types.rs (pure data shapes) has none, which is fine.

What is genuinely unverified:

  • loaders.rs's real SQL. The org query and the load_principal_set CTE have zero coverage — the 3 tests there only exercise has_directory_identity, a pure Rust helper. Nothing runs the org-fencing CTE against a real or mocked Postgres. Given that CTE is the trust boundary, this is the single highest-value gap.
  • No integration test exercises evaluate() through the real forward.rs/websocket.rs/connect.rs call sites. All 22 tests call evaluate/evaluate_outcome directly with hand-built values. Whether forward.rs passes the right policy_host and winning_connection_id at the right point relative to injection is proven only by reading code.
  • The cross-engine parity claim. The module doc cites a "corpus parity test… decision-identical to the EE engine's project arm," but that test lives in the private EE repo and is stubbed here (ee/policy_engine/oss_parity_test.rs is an empty OSS stub). From this repo alone the claim cannot be verified.

Highest-value test to add: a DB test seeding groups, group_members, organization_members, project_access across two orgs, asserting load_principal_set returns only the requesting org's memberships.


Reviewer orientation guide — produced by analysing this PR's diff and surrounding code, not the commit messages. Claims about line numbers and behaviour are worth spot-checking as you read; where it says something is untested or risky, that was verified against the tree rather than inferred.

…44.0

Reconciliation Stage F. The OSS gateway now populates the org rule set and
the user/group PrincipalSet that upstream shipped but never filled: a new
loaders.rs adds the org published-rule loader and a principal CTE mirroring
the API's resolvePrincipalSet (users direct and via granted groups, active
members only; groups direct and inherited; fully org-fenced, agent-groups
dropped). Two-level evaluation mirrors upstream's own evaluator including
the hard-floor rule (a lone allow at one level cannot open the other
level's default block), so an org guardrail can't be bypassed by a project
allow. Fail-closed via upstream's anyhow refuse-CONNECT; our old
org_degraded/Fallback/kill-switch scaffolding is deleted. Empty org fails
OPEN. +21 tests, no agent-group, no signature changes to the call sites.
@marcorivm
marcorivm force-pushed the open-edition/05-gateway-org-scope branch from 1b5a1fb to ea822d1 Compare August 8, 2026 18:30
@marcorivm
marcorivm merged commit 85a1511 into main Aug 8, 2026
@marcorivm
marcorivm deleted the open-edition/05-gateway-org-scope branch August 8, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant