feat(node): signature-gated agent self-deregister (#29)#63
feat(node): signature-gated agent self-deregister (#29)#63beardthelion wants to merge 2 commits into
Conversation
Agents whose keys are rotated or compromised could never be removed from
the registry, so an orphaned DID persisted and could win ?capability=
routing. Give the DID's key holder a way to retire their own agent.
A v5 migration adds an agent lifecycle status (default 'active') plus a
deactivated_at timestamp. list_agents drops non-active agents via a pure
filter_discoverable helper, so a revoked DID no longer appears in
discovery or capability routing; get_agent stays unfiltered so a revoked
DID still resolves and surfaces its status.
DELETE /api/v1/agents/{did} lets an rfc9421-authenticated caller mark its
own agent revoked. Authorization is on the verified AuthenticatedDid via a
pure caller_matches_did gate (a DID can only retire itself), unit-tested
including the cross-DID rejection. Revocation is terminal: register_agent's
ON CONFLICT leaves status untouched, so re-registering a leaked DID cannot
resurrect it.
Authority is team-owned (the key holder); no external contract, chain, or
operator power. Lost-key retirement and any successor pointer are out of
scope.
|
Warning Review limit reached
More reviews will be available in 44 minutes and 50 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a signature-gated ChangesAgent Deregistration and Lifecycle Status
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthMiddleware
participant deregister_agent
participant caller_matches_did
participant revoke_agent as revoke_agent (DB)
Client->>AuthMiddleware: DELETE /api/v1/agents/{did} + HTTP signature
AuthMiddleware->>AuthMiddleware: verify signature + UCAN chain
AuthMiddleware->>deregister_agent: AuthenticatedDid + Path(did)
deregister_agent->>caller_matches_did: compare caller_did vs path_did
alt DIDs do not match
caller_matches_did-->>deregister_agent: false
deregister_agent-->>Client: 403 Forbidden
else DIDs match
caller_matches_did-->>deregister_agent: true
deregister_agent->>revoke_agent: UPDATE status='revoked', deactivated_at=now
alt row affected
revoke_agent-->>deregister_agent: true
deregister_agent-->>Client: 204 No Content
else not found
revoke_agent-->>deregister_agent: false
deregister_agent-->>Client: 404 Not Found
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 1095-1103: The revoke_agent method unconditionally updates the
deactivated_at timestamp every time it's called, which loses the original
deactivation timestamp if the method is retried. Modify the SQL UPDATE query in
revoke_agent to conditionally set deactivated_at only when it is currently NULL,
using COALESCE(deactivated_at, $2) so that the first deactivation timestamp is
preserved across multiple invocations of this idempotent operation.
In `@crates/gitlawb-node/src/server.rs`:
- Around line 151-153: The deregister_agent route for /api/v1/agents/{did}
currently inherits the add_auth_layers middleware which applies both
require_ucan_chain and require_signature authentication. This conflicts with the
self-deregistration model where a DID key holder should only need to provide an
RFC 9421 signature without requiring a UCAN chain. Remove this route from the
add_auth_layers middleware stack and instead apply only the require_signature
authentication layer directly to the route definition for deregister_agent to
allow self-retirement based on signature alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bef7bba2-514a-4c01-9d87-8ebfb697f04a
📒 Files selected for processing (3)
crates/gitlawb-node/src/api/agents.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/server.rs
A retried DELETE /api/v1/agents/{did} re-ran the revoke UPDATE and
overwrote deactivated_at with a fresh timestamp, losing the true
first-retired time. Use COALESCE(deactivated_at, $2) so the original
timestamp is kept; the row still matches so the idempotent 204 is
unchanged.
1 participant
Summary
Adds a signature-gated agent self-deregister so the holder of a DID's key can retire their own agent, removing rotated or compromised orphan DIDs from discovery and capability routing.
Motivation & context
Agents whose keys are rotated or compromised could never be removed from a node's registry. The orphaned DID persisted indefinitely and, because discovery had no liveness concept, could be the first match for
?capability=routing — routing callers to a DID whose private key is in the wrong hands.Closes #29
Kind of change
What changed
Crate:
gitlawb-node.status(defaultactive) and adeactivated_attimestamp to theagentstable.list_agentsnow returns onlyactiveagents via a purefilter_discoverablehelper, so a revoked DID no longer appears in discovery or capability routing.get_agentstays unfiltered so a revoked DID still resolves and surfaces its status.statusis surfaced onAgentResponse.DELETE /api/v1/agents/{did}lets an RFC 9421-authenticated caller mark its own agent revoked. The handler authorizes and acts on the verifiedAuthenticatedDid— never a value derived from the untrusted path — so the authorized identity and the mutated row are always the same.register_agent'sON CONFLICTleavesstatusuntouched, so re-registering a leaked DID cannot resurrect it.Authority is team-owned (the key holder); no external contract, chain, or operator power.
How a reviewer can verify
Manual: with a signed request,
DELETE /api/v1/agents/{your-own-did}returns 204 and the agent disappears fromGET /api/v1/agents; aDELETEof a different DID is refused; an unauthenticated request is rejected by the signature middleware.Before you request review
cargo test --workspacepasses locallycargo fmt --allandcargo clippy --workspace --all-targets -- -D warningsare cleanfeat(...),fix(...),docs(...)).env.exampleupdated if behavior or config changed (or N/A) — N/A, no new configProtocol & signing impact
did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formatsThe deregister endpoint authenticates via the existing RFC 9421 signature middleware and authorizes on the verified
did:keyidentity. It is additive — a new migration, a new endpoint, and an additivestatusfield — so existing nodes and previously signed history are unaffected.Notes for reviewers
Deferred to follow-ups, out of scope here: lost-key retirement via an operator-authed prune; DB-integration tests (the crate has no live-Postgres harness today); an optional
?status=parameter on the list endpoint.The on-chain rotation-contract approach raised in the issue thread was deliberately rejected: the retirement authority stays team-owned (the DID's key holder), with no dependency on an external contract or third-party service.
Summary by CodeRabbit