Skip to content

feat(gateway): granular resource scoping - #17

Merged
marcorivm merged 1 commit into
open-edition/06-gateway-conditionsfrom
open-edition/07-gateway-resource-scope
Aug 8, 2026
Merged

feat(gateway): granular resource scoping#17
marcorivm merged 1 commit into
open-edition/06-gateway-conditionsfrom
open-edition/07-gateway-resource-scope

Conversation

@marcorivm

@marcorivm marcorivm commented Aug 6, 2026

Copy link
Copy Markdown
Member

Granular resource scoping in the gateway — restricting a credential to specific GitHub repos or Dropbox folders. 13 files, +1,551/−20 — no test files at this layer beyond scope.rs's own unit tests.

policy_engine/scope.rs::evaluate_scope dispatches on ("github-app"|"github", Repositories) and ("dropbox", Folders); anything else falls to Indeterminate → Blocked.

Two things to know, both documented in #9's review doc:

  1. This is the fork's enforcement model — we guard at the request layer and inject an ordinary broad token. Upstream v1.45.0 instead mints a per-request scoped credential and fails closed when it can't, via ee_apps::has_request_guard (a hardcoded false here). Adopting v1.45.0 without implementing that function silently disables everything in this PR, with cargo test still green.
  2. scope.rs::parse maps an empty list to None — "empty list = all". A stored {"repositories": []} therefore means unrestricted, not deny-all. Upstream v1.45.0 adds API-side validation against new empty lists; existing rows would still need an audit.

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

What it actually does

1,195 of the ~1,551 lines are a single new file, policy_engine/scope.rs.

A connection can carry a stored granular session policy (agent_app_connections.session_policy) confining an injected credential to specific resources — GitHub limited to certain repos, Dropbox to certain folders. Before this PR the OSS gateway threaded that value around (ResolvedRules.session_policy) but never enforced it; the field carried #[cfg_attr(not(edition_cloud), allow(dead_code))], i.e. it was known-dead in OSS. This is the missing enforcement.

Mechanically it's a tightening pass applied after the engine decision, before injection:

  • forward.rs::forward_request, after evaluate() produces (decision, matched_rule) (:237), calls policy_engine::apply_resource_scope(decision, provider, host, session_policy, path, match_input) (:261). It can only tighten: an existing Blocked/BlockedByDefaultPolicy passes through untouched, never re-attributed or loosened; an allow-family decision (Allow, ManualApproval, RateLimited) becomes Blocked { rule_name: "resource scope" } if the addressed resource is out of scope or undeterminable. The doc states it as final = max(engine_verdict, scope_verdict) — monotone, so it composes safely with feat(gateway): org-scope enforcement with user/group principals #15's engine.
  • websocket.rs::handle_websocket mirrors this (~:163) for defence-in-depth, though no covered provider serves resource-addressed operations over WS.
  • Provider and session_policy reach the call via new plumbing through ResolvedRules (mitm.rs), handle_http_proxy (gateway.rs), and the multi-connection loop in connect.rs — the connection that actually serves the request (provider_serves_request) is the one whose provider/session_policy/connection_id are attributed together, explicitly so provider isn't decoupled from the scope gating it.
  • Buffering is additionally gated on needs_scope_body(provider, host, session_policy) — true only for a Dropbox {folders} connection on api.dropboxapi.com RPC endpoints. The content host content.dropboxapi.com must not buffer, since its body is the file itself.

Why it exists

#15 gates who reaches a host/app, #16 gates what the request contains, #17 gates which specific resource an already-permitted connection can touch. Without it, "grant this agent GitHub, but only acme/frontend" was an API/UI promise the gateway didn't keep.

Reading order

  1. policy_engine/scope.rs — top to bottom; the module doc is a full spec (provider coverage, fail-closed policy, path-traversal handling). Nearly the entire PR.
  2. gateway/forward.rs — the two new call sites (~:172 buffering, ~:261 apply_resource_scope) and how scope_blocked drops rule attribution.
  3. gateway/websocket.rs — the mirrored call (~:163); confirm it's identical in intent, not a divergent copy.
  4. gateway/mitm.rsResolvedRules.provider field addition and its doc comment.
  5. connect.rs — the multi-connection attribution described above.
  6. apps/web/src/lib/granular-access/configs/{github-app,dropbox}.ts — the validateEntry additions encode, in comments, the exact gateway parsing rules (repo_in_scope needs owner/repo; folder_in_scope needs a leading slash). Useful as a cross-check that UI and gateway agree.
  7. apps/web/src/lib/policy-editor/resource-scope.tsx — where "deselect everything" is coerced to null before it reaches the API.

What to scrutinise

1. The dispatch table is a closed allowlist.

match (provider, scope) {
    ("github-app" | "github", ResourceScope::Repositories(allowed)) => github_scope(strip_port(host), path, &allowed),
    ("dropbox", ResourceScope::Folders(allowed)) => dropbox_scope(host, path, input, &allowed),
    _ => ScopeVerdict::Indeterminate,
}

"github" and "github-app" are genuinely distinct provider identifiers (apps.rs:245 PAT-based, apps.rs:279 installed App), so covering both is correct, not redundant. But _ => Indeterminate maps straight to Blocked — so any other provider that somehow has a session_policy (a new provider added to granular_access configs without a matching scope.rs extractor, or a scope authored via direct API call) is silently and totally blocked. There is no test or compile-time link between apps/web/src/lib/granular-access/configs/ (what the UI can scope) and this match arm (what the gateway can enforce) — a cross-language contract held together by convention. Worth asserting somewhere: walk the known granular-access provider list, assert each has a scope.rs arm.

2. parse's "empty list = all", and its consequence.

scope.rs::parse (:80) returns None for {} and for {"repositories": []} (ListShape::Empty => return None, // empty list = all). None means "no scope" → evaluate_scope returns InScope unconditionally → the gate is a complete no-op.

This is consistent and intentional across the whole stack, not a gateway quirk:

  • sessionPolicySchema (packages/api/src/validations/policy.ts) explicitly accepts {repositories: []}, doc-commented "Empty/absent = all", with a test named accepts an empty list (empty = all resources).
  • resource-scope.tsx can never produce that shape — deselecting everything coerces the policy to null (emit() returns null when Object.keys(p).length === 0), and each config's buildPolicy returns {} rather than {folders: []}.

The consequence: there is currently no way anywhere in this product to express "deny this connection all resources" via granular scope. Deselecting everything un-restricts. Fine while every consumer knows the convention — but the schema validates {repositories: []} as legitimate, so a future engineer reading only the Zod schema (a bulk-revoke admin action, an ops script PATCHing session_policy directly) has no signal it means the opposite of what it looks like. A real deny-all today has to be a network/app-level Block rule, which is a much heavier hammer — it blocks the whole host rather than one connection's resource access.

Cross-PR note: upstream v1.45.0 adds validation rejecting empty repo/folder lists. Adopting it would invert the convention this fork documents and tests. Upstream's behaviour is arguably better, but it's a deliberate semantic change, not a free security fix.

3. Fail-closed coverage is thorough — spot-check rather than re-derive.

  • github_repo_ref (:250) treats /repositories/{id} (numeric legacy) and /graphql as Indeterminate, not NotRepoAddressed — correct, both can name a repo the URL extractor can't see.
  • Path traversal: has_traversal/is_dot_segment (:208-224) reject any ./.. segment including percent-encoded forms, specifically because the forwarding layer's URL builder collapses these after this check would run. A scope check on the raw path could otherwise pass a request whose actual upstream target is a different, out-of-scope repo. A real path-normalization bypass class, clearly considered.
  • is_non_resource_rpc (:388) is a small hardcoded Dropbox allowlist (get_current_account, get_space_usage, check/user, check/app, */continue). Any Dropbox RPC not on it, with no path in its body, fails closed. Safe by construction, but this list needs active maintenance as Dropbox's API evolves.

4. The connect.rs attribution is new code, not a bug patched in flight. resolved_provider did not exist before this PR; it's introduced here, inside the provider_serves_request(...) guard from the start. Correct as shipped. Still worth verifying provider_serves_request classifies correctly for a multi-connection project (GitHub + Dropbox on overlapping hosts) — though note a misclassification degrades to over-blocking (wrong provider → IndeterminateBlocked), not a scope bypass.

Design decisions worth questioning

  • Resource scope is a global post-hoc tightening pass, independent of rule generation — deliberately, so it "enforce[s] on legacy / pre-cutover projects too," since session policy is a property of the connection, not the rule. Reasonable, but it means scope cannot be expressed as a rule: no priority, no identity condition, no "scope applies only for agent X." It's connection-wide and always-on. A future need for per-agent scoping on a shared connection has no slot here.
  • The provider allowlist is a static Rust match while the authoring surface is a pluggable TS registry — two independently maintained lists, no shared source of truth.

Test coverage reality

scope.rs is heavily tested: 31 new #[test] functions, all inline #[cfg(test)] mod tests (no separate file) — parse's shape matrix including parse_treats_empty_and_absent_as_unscoped, GitHub path parsing (api vs git-over-HTTPS vs raw content, numeric/GraphQL indeterminate), Dropbox arg extraction (RPC body vs content header, batch entries, traversal rejection), folder/repo matching. Deepest unit coverage of any file across the three gateway PRs.

But every one is a pure-function test of scope.rs in isolation. New tests per changed file:

file new tests
policy_engine/scope.rs 31
gateway/forward.rs 0
gateway/websocket.rs 0
gateway/mitm.rs 0
gateway.rs 0
connect.rs 0
policy_engine.rs 0

Genuinely unverified:

  • The wiring — that forward.rs calls apply_resource_scope with the correct provider/session_policy/host/match_input, in the right order relative to evaluate() and injection. This is exactly the connect.rs multi-connection attribution flagged above, with zero integration coverage.
  • The websocket mirror — nothing asserts the two call sites stay in sync; a future edit to one's argument order could silently diverge.
  • needs_scope_body's wiring into the combined buffering condition — no test proves a Dropbox-folder-scoped request on api.dropboxapi.com actually gets buffered end to end.

Highest-value test to add: one that constructs a ResolvedRules with provider: Some("dropbox") and a folders policy and proves the buffering predicate, the apply_resource_scope call, and the block-response path all connect for one concrete out-of-scope request.


How #15, #16 and #17 compose at request time

  1. Token-intercept / default-interception short-circuits (pre-existing) — a synthetic response returns here, before any policy check.
  2. App-availability pre-check (apps::app_availability_block) — cloud-only, no-op in OSS.
  3. Body-buffering decision — union of feat(gateway): body/header condition matching #16's needs_body_buffer, feat(gateway): granular resource scoping #17's needs_scope_body, and the pre-existing hooks::needs_request_body. Determines whether MatchInput.body is populated for step 4.
  4. feat(gateway): org-scope enforcement with user/group principals #15 + feat(gateway): body/header condition matching #16 engine decisionevaluate() loads (from the ~60s connect cache) org+project rule sets and the principal set, runs the two-level hard-floor first-match evaluator, with conditions evaluated under the fail-closed-by-action law.
  5. feat(gateway): granular resource scoping #17 tightening passapply_resource_scope() narrows an allow to Blocked if the addressed resource is out of scope. Never loosens.
  6. Early return on any non-allow — no body consumed further, no injection.
  7. Credential injection (inject::apply_injections) — reached only if 4 and 5 both resolved to allow.

The one path reaching credential-shaped output without passing steps 3-6 is step 1forward.rs's rules.intercept_token branch, serving a cached OAuth access_token in a JSON body. Confirmed unchanged across all three PR boundaries, so it's pre-existing baseline, not introduced by this stack. But it means "policy is authoritative for every request" is not literally true end to end, and whether that's an acceptable boundary (SDK token refresh vs proxied API traffic) is a standing product decision worth being aware of.


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.

Reconciliation Stage H. The gateway enforces a connection's session_policy
as a monotone tightening on the final two-level decision — GitHub repos
(from the URL path) and Dropbox folders (from the buffered JSON body or the
Dropbox-API-Arg header). Reuses Stage G's body buffer via an OR-composed
needs_scope_body (no second buffer; the content host never buffers the file
body). provider is threaded through BOTH ResolvedRules construction sites
(the http-proxy path was silently dropping it). Fail-closed throughout:
dot-segment path traversal, path-less content RPCs, unknown providers with
a scope set, and unparseable requests all deny; uncovered providers deny
while scoped. +31 tests, no agent-group, no migration.
@marcorivm
marcorivm force-pushed the open-edition/07-gateway-resource-scope branch from 5fa5509 to e5fbe1f Compare August 8, 2026 18:30
@marcorivm
marcorivm merged commit 85a1511 into main Aug 8, 2026
@marcorivm
marcorivm deleted the open-edition/07-gateway-resource-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