proposal: cascade revocation spec for agent delegation chains#16
proposal: cascade revocation spec for agent delegation chains#16aeoess wants to merge 4 commits intoopenagentidentityprotocol:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a draft specification document describing “cascade revocation” semantics for agent delegation chains, including suggested data structures, revocation/validation algorithms, and security properties, with a reference implementation link.
Changes:
- Introduces proposed
Delegation,RevocationRecord, andChainRegistrystructures for representing delegation graphs. - Documents cascade revocation, batch revocation, and chain validation algorithms and required security properties.
- Includes adversarial scenarios and scope-matching rules intended to guide interoperable implementations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
docs/cascade-revocation.md
Outdated
| maxDepth: number // Maximum sub-delegation depth allowed | ||
| currentDepth: number // Current depth in the chain (0 = root) | ||
| expiresAt: string // ISO 8601 expiration timestamp | ||
| createdAt: string // ISO 8601 creation timestamp | ||
| parentDelegationId?: string // ID of parent delegation (null for root) | ||
| revoked: boolean // Revocation status | ||
| revokedAt?: string // ISO 8601 revocation timestamp |
There was a problem hiding this comment.
In the Delegation interface, parentDelegationId?: string is marked optional (implying undefined when absent) but the comment says it is null for root. Please make the type/comment consistent (e.g., parentDelegationId?: string with “undefined/omitted for root”, or parentDelegationId: string | null) and align this with the ChainRegistry.parents map behavior for root delegations.
docs/cascade-revocation.md
Outdated
| delegatorId: string // Agent granting authority | ||
| delegateId: string // Agent receiving authority | ||
| scope: string[] // Permitted action scopes | ||
| spendLimit?: number // Maximum spend in delegation currency |
There was a problem hiding this comment.
spendLimit?: number is described as “Maximum spend in delegation currency”, but the Delegation structure does not define what the currency is (or whether it is inherited from the parent / implied by scope). This leaves the limit ambiguous for implementers; consider adding an explicit currency (or clarifying the unit/inheritance rules).
| spendLimit?: number // Maximum spend in delegation currency | |
| currency?: string // Delegation currency (ISO 4217 code, e.g. "USD"); if omitted, protocol default applies | |
| spendLimit?: number // Maximum spend in smallest unit of `currency` (e.g. cents); if omitted, no spend constraint |
docs/cascade-revocation.md
Outdated
| 5. Get all children from chain registry | ||
| 6. For each child: | ||
| a. Recursively call cascadeRevoke(child.delegationId, reason, revokerKey) |
There was a problem hiding this comment.
In the cascadeRevoke pseudocode, the chain registry’s children is defined as Map<string, string[]> (child delegation IDs), but step 6a calls cascadeRevoke(child.delegationId, ...) as if child were an object. This should either iterate over IDs directly (e.g., cascadeRevoke(childId, ...)) or redefine what children contains.
| 5. Get all children from chain registry | |
| 6. For each child: | |
| a. Recursively call cascadeRevoke(child.delegationId, reason, revokerKey) | |
| 5. Get all child delegationIds from chain registry | |
| 6. For each childId: | |
| a. Recursively call cascadeRevoke(childId, reason, revokerKey) |
docs/cascade-revocation.md
Outdated
| 1. For each delegation in chain: | ||
| a. If revoked: return {valid: false, error: "revoked link"} | ||
| b. If expired: return {valid: false, error: "expired link"} | ||
| c. If not found in registry: return {valid: false, error: "unknown delegation"} | ||
| 2. For each adjacent pair (parent, child): | ||
| a. If parent.delegateId != child.delegatorId: return {valid: false, error: "chain break"} |
There was a problem hiding this comment.
validateChain(delegationIds[]) takes IDs, but step 1 iterates “for each delegation in chain” and then accesses delegation fields (revoked/expired/etc.) without specifying the required lookup/order. Please clarify the input contract (ordered list of Delegation objects vs IDs) and, if IDs are intended, add the explicit store/registry lookup step and ordering requirements before validating adjacent pairs.
| 1. For each delegation in chain: | |
| a. If revoked: return {valid: false, error: "revoked link"} | |
| b. If expired: return {valid: false, error: "expired link"} | |
| c. If not found in registry: return {valid: false, error: "unknown delegation"} | |
| 2. For each adjacent pair (parent, child): | |
| a. If parent.delegateId != child.delegatorId: return {valid: false, error: "chain break"} | |
| 0. Resolve orderedDelegations[] from registry: | |
| a. For each delegationId in delegationIds (in the order provided): | |
| i. Lookup delegation in registry by delegationId | |
| ii. If not found: return {valid: false, error: "unknown delegation"} | |
| iii. Append delegation to orderedDelegations[] | |
| 1. For each delegation in orderedDelegations: | |
| a. If revoked: return {valid: false, error: "revoked link"} | |
| b. If expired: return {valid: false, error: "expired link"} | |
| 2. For each adjacent pair (parent, child) in orderedDelegations: | |
| a. If parent.delegationId != child.parentDelegationId: return {valid: false, error: "invalid parent reference"} | |
| b. If parent.delegateId != child.delegatorId: return {valid: false, error: "chain break"} |
docs/cascade-revocation.md
Outdated
|
|
||
| When an agent sub-delegates, the child delegation MUST be strictly narrower than the parent: | ||
|
|
||
| - **Scope:** Child scope MUST be a subset of parent scope. scopeCovers(parentScope, childScope) must be true for every scope in the child. |
There was a problem hiding this comment.
The scope narrowing rule references scopeCovers(parentScope, childScope) even though scopeCovers(granted, required) is later described as operating on single scope strings. Please specify the exact algorithm for arrays (e.g., “for each required scope in child, there exists a granted scope in parent that covers it”) so implementers don’t interpret subset/coverage differently.
| - **Scope:** Child scope MUST be a subset of parent scope. scopeCovers(parentScope, childScope) must be true for every scope in the child. | |
| - **Scope:** Child scope MUST be a subset of parent scope. For array-valued scopes, this means: for each `requiredScope` in the child's `scope` array there MUST exist some `grantedScope` in the parent's `scope` array such that `scopeCovers(grantedScope, requiredScope)` is true, where `scopeCovers` operates on individual scope strings. |
docs/cascade-revocation.md
Outdated
| - **Total:** There is no partial cascade. Either all descendants are revoked or the operation fails atomically. | ||
|
|
There was a problem hiding this comment.
The “Total” property claims cascade revocation is atomic (“either all descendants are revoked or the operation fails atomically”), but the algorithm description doesn’t state any transactional/rollback requirements on the underlying store (and recursion implies multiple writes). Please define the required consistency/transaction semantics (or relax the claim) so implementations know what guarantees they must provide.
| - **Total:** There is no partial cascade. Either all descendants are revoked or the operation fails atomically. | |
| - **Total (Transactional):** There is no partial cascade. Either all descendants are revoked and their revocation records durably persisted, or none are. Implementations MUST execute steps 3–7 for the entire cascade (including all recursive calls) within a single atomic transaction or equivalent mechanism that guarantees all-or-nothing visibility of writes and rollback on failure. | |
| **Implementation note:** In systems whose storage layer does not support multi-record transactions, implementations MUST NOT claim to provide the **Total (Transactional)** property and MUST instead document that cascade revocation is best-effort and may leave partial revocation state on failure. |
…null-for-root semantics Addresses Copilot review: optional (?) implies undefined when absent, but the comment specifies null for root delegations. Changed to explicit string | null union type for consistency with ChainRegistry.parents map behavior.
|
Good catch. Fixed in af73a75 — changed |
Addresses review feedback from @jamescao: - Rewrote data structures as JSON Schema (2020-12 draft) matching AIP conventions - Moved spec to proposals/ directory - Converted docs to Mintlify .mdx format - Removed SDK reference — spec is now protocol-agnostic - Fixed parentDelegationId type: string | null (not optional)
|
Updated per your feedback:
Should be a cleaner fit with the rest of the repo. Let me know if anything else needs adjusting. |
- Add currency field (ISO 4217) to Delegation schema - Fix cascadeRevoke pseudocode: childId not child.delegationId - Add explicit registry lookup step to validateChain - Clarify scopeCovers array algorithm for sub-delegation - Define transactional semantics for Total property - Document best-effort fallback for stores without transactions
|
All 6 Copilot review items addressed in 0905c6a:
|
Cascade revocation spec for agent delegation chains, per discussion in #13 with @ArangoGutierrez.
Defines data structures, algorithm (DFS cascade, batch revocation, chain validation), sub-delegation constraints, adversarial scenarios, and policy engine integration.
Reference implementation: agent-passport-system on npm.