feat(node): signature-gated agent self-deregister (#29)#63
Merged
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.
Contributor
📝 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 docstrings
🧪 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}
Contributor
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.
beardthelion
added
kind:feature
Resolves a migration version collision in db/mod.rs: both this branch and #40 (now on main) added a version-5 migration. Kept main's released v5 (encrypted_blobs_blind_recipients) and renumbered this branch's agent_retirement migration to v6, appended after it. The two are independent (agents vs encrypted_blobs); only the version number collided. schema_migrations keys on version, and the strictly-increasing invariant test passes (1..6).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/gitlawb-node/src/db/mod.rs (1)
1076-1087: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider filtering by
status = 'active'in SQL for efficiency.The query fetches all agents (including revoked) and filters in memory via
filter_discoverable. For large registries, adding a WHERE clause reduces data transfer and memory usage:let rows = sqlx::query( - "SELECT did, trust_score, capabilities, registered_at, last_seen, status \ - FROM agents ORDER BY trust_score DESC", + "SELECT did, trust_score, capabilities, registered_at, last_seen, status \ + FROM agents WHERE status = 'active' ORDER BY trust_score DESC", )This keeps
filter_discoverablefor capability matching while letting the DB handle status filtering. Low priority if agent counts stay modest.🤖 Prompt for 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. In `@crates/gitlawb-node/src/db/mod.rs` around lines 1076 - 1087, The list_agents method currently fetches all agents from the database and filters them in memory via filter_discoverable, which is inefficient for large agent registries. Modify the SQL query in list_agents to add a WHERE clause that filters for agents with status = 'active' at the database level before results are transferred and mapped, while keeping the existing filter_discoverable call to handle the capability filtering logic.
🤖 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.
Nitpick comments:
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 1076-1087: The list_agents method currently fetches all agents
from the database and filters them in memory via filter_discoverable, which is
inefficient for large agent registries. Modify the SQL query in list_agents to
add a WHERE clause that filters for agents with status = 'active' at the
database level before results are transferred and mapped, while keeping the
existing filter_discoverable call to handle the capability filtering logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b65dd486-61f6-44af-b2f7-af8991debe3d
📒 Files selected for processing (1)
crates/gitlawb-node/src/db/mod.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
DELETE /api/v1/agents/{did}(revokes the caller’s own agent and returns204on success).statusis now included in agent API responses (for example,active/revoked).