Skip to content

fix: resolve declared dependencies at every hop of a chain, not just the root - #1947

Open
serguei-gorokhov wants to merge 1 commit into
rd/7-target-grammarfrom
rd/8-chained-resolution
Open

serguei-gorokhov wants to merge 1 commit into
rd/7-target-grammarfrom
rd/8-chained-resolution

Conversation

@serguei-gorokhov

Copy link
Copy Markdown

Applicable issues

Description of changes

ResolveResourceDependenciesFn.apply() skipped resolution whenever a per-request key was already present — true at every hop past the root, and true on a plain root call behind an interceptor (the post-interceptor handleDeployment re-entry already carries the interceptor's key). Identity was never the obstacle: ApiKeyData.initFromContext propagates extractedClaims/originalKey at every depth, so ProxyContext.userId is always the originating human — which is why placeholder-path resolution already worked at any depth. Only the reach check needed care: the general permission chain's own-bucket rule deliberately flips to the per-request key holder's own Keys/<app>/ sandbox — the wrong function to ask "does the human own this".

AccessService gains a second, purpose-built rule list — lookupOriginatingUserPermissions, evaluated over own-bucket-via-buildInitiatorBucket + getSharedAccess + getPublicAccess only. Rules that read the current per-request-key holder's own state (getAutoSharedAccess, getPerRequestPermissions, getAppResourceAccess, getAppSelfAccess) are deliberately excluded: including them would let a calling app launder grants it already holds into satisfying the next app's independent reach check, compounding privilege down a chain instead of every hop being checked against the human alone. lookupPermissions's existing signature and behavior are unchanged — zero-behavior-change for all 10 existing call sites.

Resolution now runs uniformly for root and chained calls with no depth-conditional branch.

source_deployment is added to the three ResourceDependencyAuditLog events (grant/denial/runtimeFail): once resolution runs at every hop, user_id/trace_id/actor evidence are identical at every hop, so nothing else in the audit trail distinguished "app2 got the grant" from "app1's call caused app2 to get the grant".

Behavior changes flagged for review

  • A required dependency that does not resolve on a chained or interceptor hop now hard-fails that call with 403. Before this PR such a hop silently skipped resolution and stayed callable regardless. This is the correct semantics (an app never half-works silently) and the same rule the root call has always enforced — but it is a new, previously-absent failure mode for existing chained deployments, reachable by any app that declares required: true and is invoked non-first in a chain by a user whose reach or consent does not cover the target. Operators who need the old behavior set required: false, which degrades exactly as before.
  • Scope narrowing — ask the team to confirm: root-call reach is now evaluated over three rules instead of eleven. For a normal user the answer is identical (own bucket, shared, public). It differs for a caller who could previously satisfy a dependency target only through getAdminAccess, getGlobalReaderAccess or getReviewAccess: an admin or global reader invoking a declaring app no longer has operator-role standing counted toward that app's dependency reach. Deliberate (a dependency target is the human's own/shared/public resource, not an operator-role-gated one) — if anyone wants admin standing to count, that is a design conversation, not a code tweak.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

🤖 Generated with Claude Code

…the root

ResolveResourceDependenciesFn.apply() skipped resolution whenever a
per-request key was already present -- true at every hop past the root,
and true on a plain root call behind an interceptor (the post-interceptor
handleDeployment re-entry already carries the interceptor's key). Identity
was never the obstacle: ApiKeyData.initFromContext propagates
extractedClaims/originalKey at every depth, so ProxyContext.userId is
always the originating human -- which is why placeholder-path resolution
already worked at any depth. Only the reach check needed care: the general
permission chain's own-bucket rule deliberately flips to the per-request
key holder's own Keys/<app>/ sandbox, the wrong function to ask "does the
human own this".

AccessService gains a second, purpose-built rule list --
lookupOriginatingUserPermissions, evaluated over own-bucket-via-
buildInitiatorBucket + getSharedAccess + getPublicAccess only. Rules that
read the current per-request-key holder's own state (getAutoSharedAccess,
getPerRequestPermissions, getAppResourceAccess, getAppSelfAccess) are
deliberately excluded: including them would let a calling app launder
grants it already holds into satisfying the next app's independent reach
check, compounding privilege down a chain instead of every hop being
checked against the human alone. admin/global-reader/review access are
also excluded as a stated scope narrowing (dependency targets are meant to
be the human's own/shared/public resources, not operator-role-gated ones).
lookupPermissions's existing signature and behavior are unchanged --
zero-behavior-change for all 10 existing call sites.

Resolution now runs uniformly for root and chained calls with no
depth-conditional branch. A required dependency that does not resolve on
a chained or interceptor hop now hard-fails that call (403) -- the same
rule the root call has always enforced, but a new, previously-absent
failure mode for existing chained deployments.

source_deployment is added to the three ResourceDependencyAuditLog events
(grant/denial/runtimeFail): once resolution runs at every hop, user_id/
trace_id/actor evidence are identical at every hop, so nothing else in the
audit trail distinguished "app2 got the grant" from "app1's call caused
app2 to get the grant".

Spec: documentation repo, offline-access-delegation/implementation-specs/pr4b-chained-call-resolution.md

Co-Authored-By: Claude Code <noreply@anthropic.com>
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