Conversation
Two defects in Studio's Issued Tokens screen trace back to one gap here: ChainSummary names no identity. 1. N+1. With no identity on the row, Studio walked /delegations/by-jti once per chain to find out who each one belonged to. Listing 200 chains cost 201 requests against this service, so it had to cap the list at 100 to stay responsive — which made the cap itself visible to operators. 2. A filter that cannot answer its own question. With nothing to filter on server-side, "show me this agent's tokens" ran client-side over the capped page. An empty result meant "not among these N", but the screen rendered it as "no tokens issued to this agent" — a false negative on a forensics screen. ListChains now resolves the root through jti, joining on COALESCE(mission_id, jti) — mission_id IS the root jti — rather than picking the shallowest credential inside the window. That matters: the root credential of a long-running chain is often issued before `since`, and picking within the window would silently relabel chains as the range moved. The identity join is LEFT, so a chain whose identity was deleted still lists; those are exactly the tokens an offboarding review looks for. root_identity_id filters before the LIMIT, which is the whole point — an empty response is now a finding rather than a statement about one page. Both are pinned by integration tests, including one that issues the wanted chain FIRST and asks for limit=1, so a filter applied after the limit fails rather than passing on a small fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![highflame-oracle[bot]](https://avatars.githubusercontent.com/u/141064602?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔮 Oracle Review
🎯 Start Here
internal/handler/delegation.go (~20 min) — Security changes in delegation.go
📋 PR Summary
What this PR does: Adds root identity fields (root_identity_id, root_identity_name, root_wimse_uri) to ChainSummary and introduces root_identity_id filtering to ListChains endpoint to resolve N+1 query issues and enable accurate server-side filtering in Studio.
Key changes:
- Extended ChainSummary struct with root identity metadata fields
- Added root_identity_id filter parameter to ListChains handler with tenant context propagation
- Implemented pre-LIMIT filtering via LEFT JOIN on COALESCE(mission_id, jti) to correctly resolve root identity even when root credential precedes the query window
- Added two integration tests validating filter behavior and filter-before-limit ordering
Areas affected: delegation chains API, ChainSummary data model, Studio Issued Tokens screen integration
Testing notes: Integration tests specifically validate filter-before-limit ordering by issuing the wanted chain first and using limit=1, ensuring filter failures rather than false passes on small fixtures.
🔍 Code Review
This is a well-architected fix that addresses both performance (N+1 queries) and correctness (false negatives in forensics) issues. The implementation thoughtfully handles edge cases like deleted identities via LEFT JOIN and ensures root identity resolution is consistent with existing behavior by joining on COALESCE(mission_id, jti).
What's good:
- ✨ Excellent attention to edge cases: handling deleted identities with LEFT JOIN ensures chains remain visible for offboarding reviews
- ✨ Smart root identity resolution using COALESCE(mission_id, jti) prevents silent relabeling as time windows shift
- ✨ Deliberate test design that validates filter-before-limit ordering by ordering fixture issuance to catch post-limit filtering bugs
- ✨ Clear downstream impact documentation showing how Studio can remove client-side enrichment and raise row caps
Review Stats:
Generated by Oracle - Highflame's AI Code Reviewer
Review on #339. A malformed root_identity_id was not rejected — it simply matched no rows. On this endpoint an empty chain list means "this agent holds no tokens in this window", so a typo answered 200 with [] and read as a clean bill of health. That is the same false-negative class the filter was added to remove, reachable through the filter itself. It now carries a UUID pattern and 400s, with empty kept explicitly valid because that is the "no filter" sentinel a cleared agent picker sends. The comparison was `root.identity_id::text = ?`, which had two problems the review's second note points at from a different angle. It cast the column away on every row, so the index on identity_id could not be used; and Postgres renders a uuid canonically lowercase, so an upper-case UUID — equally valid, UUIDs are case-insensitive — matched nothing and produced the false negative again. Now `root.identity_id = NULLIF(?, '')::uuid`: NULLIF turns the sentinel into NULL before the cast, so "no filter" never reaches ::uuid, and the handler's pattern guarantees anything that does will parse. For the record on the nil-UUID case raised in review: all-zeros is not the empty sentinel, so it is applied as a filter and matches nothing rather than bypassing the filter. Tests: four malformed ids must 4xx and never 500, empty must stay 200, and an upper-case UUID must match the same chain as its lower-case form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The root-identity resolution through the chain's root JTI is a subtle but important choice. There is another boundary I would keep explicit as this grows: identity discovery should remain separate from authorization. Knowing the root identity of a delegation chain should tell the system who originated the authority, but it should not by itself grant execution permission to downstream tools. For agentic systems, the useful audit relation is often: root identity -> delegated principal -> policy decision -> concrete action -> outcome That distinction becomes valuable when a chain is long-lived and the original credential predates the query window, exactly like the case you're protecting here. This is an area we've been working on in Aegisora as well: keeping identity, authorization, execution and evidence as separately bound objects rather than collapsing them into one credential. |
3 participants
Two defects in Studio's Issued Tokens screen (highflame-studio#1587 audit) trace back to one gap here:
ChainSummarynames no identity.1. N+1 against this service. With no identity on the row, Studio walks
/delegations/by-jtionce per chain to find out who each belongs to. Listing 200 chains costs 201 requests, so Studio has had to cap the list at 100 to stay responsive — a cap operators can see.2. A filter that cannot answer its own question. With nothing to filter on server-side, "show me this agent's tokens" runs client-side over the capped page. An empty result means "not among these N", but the screen renders it as "no tokens issued to this agent" — a false negative on a forensics screen.
What changed
ChainSummarygainsroot_identity_id,root_identity_name,root_wimse_uri, andListChainsacceptsroot_identity_idas a filter.The root is resolved through jti — joining on
COALESCE(mission_id, jti), sincemission_idis the root jti — rather than by picking the shallowest credential inside the window. That distinction matters: the root credential of a long-running chain is often issued beforesince, and picking within the window would silently relabel chains as the range moved. It also matches what the per-row/by-jtiwalk returns today, so consumers see no behaviour change beyond the new fields.The identity join is
LEFT:identity_idisON DELETE SET NULL, and a chain whose identity was deleted must still list — those are exactly the tokens an offboarding review is looking for.The filter runs before
LIMIT. That ordering is the entire value: an empty response becomes a finding rather than a statement about one page.Tests
Two integration tests. The filter test issues the wanted chain first (making it the older one) and asks for
limit=1, so a filter applied after the limit fails rather than passing on a small fixture.Downstream
Once released, Studio drops the enrichment loop in its chains route, passes the agent filter to the server, and can raise its row cap again.
🤖 Generated with Claude Code