registry: per-identity token rotation via token_min_iat (fixes #2205) - #2208
Open
hognek wants to merge 5 commits into
Open
registry: per-identity token rotation via token_min_iat (fixes #2205)#2208hognek wants to merge 5 commits into
hognek wants to merge 5 commits into
Conversation
…des (D1) Implement cross-user agent delegation (milestone D1 of jaylfc#2012): - Add sponsor_contact_id column to agent_registry (schema + migration + store API) - Delegation-request envelope dispatch in peer inbox (kind=delegation_request) - Policy gate with per-project auto_approve_delegation knob (default OFF) - Sponsored scope tiers: {a2a_send, a2a_receive, project_tasks, canvas_read, registry_feeds_read} - Hard-deny files_write and decisions_write at scope validation - Decisions card integration: collab_delegation_gate kind triggers invite mint on approve - Revoke cascade: contact/membership change → revoke sponsored identities → unassign tasks - 3-level kill-switch: per-agent (existing), per-contact pause, per-instance panic - Invite metadata column with sponsor_contact_id propagation through redeem flow - Project store helpers: is_project_member(), get/set_project_setting() Fixes: jaylfc#2019
CRITICAL: Restore reply assignment in _apply_delegation_grant (regression from collapsed if/elif/else) WARNING: cascade_sponsor_revoke now filters agents by project_id via project_store.is_project_member for membership-scoped revoke WARNING: complete_delegation_approval re-applies scope denylist (validate_delegation_scopes) before minting invite SUGGESTION: Add kill_switch_reenable() for per-instance panic recovery SUGGESTION: A2A audit line emitted for contact-wide revoke across all projects the contact is a member of
WARNING: Re-validate project membership at approval time in complete_delegation_approval (fail-closed runtime check per design spec 5) WARNING: Remove fragile list_projects_for_member call (method does not exist on project_store). Simplify A2A audit to project-scoped only. SUGGESTION: Route error answer back when complete_delegation_approval raises, so the remote contact receives feedback on approval failure.
…#2205) Add token_min_iat INTEGER column (default 0) to agent_registry so tokens can be invalidated per-identity without revoking the whole identity. - migration _migration_v6_add_token_min_iat: idempotent ALTER TABLE with default 0 so the migration cannot lock the fleet out - AgentRegistryStore.bump_token_min_iat(canonical_id, ts): persist cutoff - Auth path (_verify_agent_scope): after the status check, reject tokens whose iat claim is < the record's token_min_iat with 401 'token superseded' - POST /api/agents/registry/{id}/rotate-tokens: session-owner/admin route that bumps the cutoff and writes a forensic audit-log entry - Tests: store-level (bump sets cutoff, default zero, nonexistent nil), auth-path (old token rejected, new token passes, default-zero safe), route (admin rotates, owner rotates, nonexistent 404)
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds per-identity JWT rotation cutoffs and a cross-user delegation flow with sponsored identities, scope restrictions, approval handling, invite metadata, cascade revocation, peer kill switches, and associated tests. ChangesToken rotation
Cross-user collaboration delegation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DelegatedAgent
participant PeerInbox
participant DelegationHandler
participant DecisionStore
participant InviteStore
participant AgentRegistry
DelegatedAgent->>PeerInbox: delegation_request
PeerInbox->>DelegationHandler: process request
DelegationHandler->>DecisionStore: create approval decision
DecisionStore-->>DelegationHandler: approved decision
DelegationHandler->>InviteStore: mint sponsored invite
InviteStore-->>DelegationHandler: invite_id
DelegationHandler->>AgentRegistry: register or update sponsored identity
AgentRegistry-->>DelegationHandler: registry record
sequenceDiagram
participant OwnerOrAdmin
participant RegistryRoute
participant AgentRegistryStore
participant GovernanceAudit
OwnerOrAdmin->>RegistryRoute: POST rotate-tokens
RegistryRoute->>AgentRegistryStore: bump token_min_iat
AgentRegistryStore-->>RegistryRoute: updated identity
RegistryRoute->>GovernanceAudit: record rotate-tokens event
RegistryRoute-->>OwnerOrAdmin: updated registry record
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
PR Summary by QodoRegistry: per-identity token rotation via token_min_iat
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| return { | ||
| "status": "approved", | ||
| "invite_id": invite["id"], |
There was a problem hiding this comment.
CRITICAL: KeyError — invite["id"] should be invite["record"]["invite_id"]
invite_store.mint() returns {"record": {..., "invite_id": ...}, "pin": ...}. Accessing invite["id"] raises an unhandled KeyError, causing a 500 whenever auto-approve is enabled.
Suggested change
| "invite_id": invite["id"], | |
| "invite_id": invite["record"]["invite_id"], |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| return { | ||
| "status": "approved", | ||
| "invite_id": invite["id"], |
There was a problem hiding this comment.
CRITICAL: KeyError — invite["id"] should be invite["record"]["invite_id"]
Same typo in complete_delegation_approval. invite_store.mint() returns a dict with record.invite_id, not a top-level id key. This crashes the approval flow when a delegation decision is approved.
Suggested change
| "invite_id": invite["id"], | |
| invite_id = invite["record"]["invite_id"], |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/routes/peer.py (1)
258-273: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDispatch branch sits below the "unrecognised kind" log.
Every
delegation_requestnow logsunrecognised kind — accepted, no dispatchbefore being dispatched, which will mislead anyone debugging the peer channel. Move the branch above the log (and drop the duplicatebody_dataread from Line 250).♻️ Proposed fix
+ # Dispatch delegation requests to the cross-user collab handler (D1). + if kind == "delegation_request": + from tinyagentos.delegation_handler import process_delegation_request + + result = await process_delegation_request( + request, contact_id=contact_id, envelope_body=body_data, + ) + return {"status": result.get("status", "unknown"), "detail": result} + # Log unrecognised kinds for debugging; they are accepted but not dispatched. logger.info( "peer_inbox: contact=%s kind=%s nonce=%s (unrecognised kind — accepted, no dispatch)", contact_id, kind, envelope.get("nonce", "?"), ) - # Dispatch delegation requests to the cross-user collab handler (D1). - if kind == "delegation_request": - from tinyagentos.delegation_handler import process_delegation_request - - body_data = envelope.get("body", {}) - result = await process_delegation_request( - request, contact_id=contact_id, envelope_body=body_data, - ) - return {"status": result.get("status", "unknown"), "detail": result} - return {"status": "received", "kind": kind, "nonce": envelope.get("nonce")}🤖 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 `@tinyagentos/routes/peer.py` around lines 258 - 273, Move the delegation_request dispatch branch in the peer message handler above the unrecognised-kind logger so recognized requests are not logged as unrecognised. Reuse the existing body_data value from the earlier code instead of reading envelope.get("body", {}) again, while preserving the process_delegation_request call and response handling.
🧹 Nitpick comments (10)
tinyagentos/routes/decisions.py (1)
476-479: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the extraneous
fprefix.♻️ Proposed fix
- error_msg = ( - f"Delegation approval failed: an internal error occurred while " - f"setting up the sponsored agent. Please retry or check the logs." - ) + error_msg = ( + "Delegation approval failed: an internal error occurred while " + "setting up the sponsored agent. Please retry or check the logs." + )🤖 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 `@tinyagentos/routes/decisions.py` around lines 476 - 479, Remove the unnecessary f-string prefix from the error_msg assignment in the delegation approval error-handling code, since the message contains no interpolated expressions. Preserve the existing message text unchanged.Source: Linters/SAST tools
tinyagentos/projects/project_store.py (1)
382-404: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated branches into one query.
♻️ Optional simplification
- if member_kind is not None: - async with self._db.execute( - "SELECT 1 FROM project_members " - "WHERE project_id = ? AND member_id = ? AND member_kind = ?", - (project_id, member_id, member_kind), - ) as cur: - return (await cur.fetchone()) is not None - else: - async with self._db.execute( - "SELECT 1 FROM project_members " - "WHERE project_id = ? AND member_id = ?", - (project_id, member_id), - ) as cur: - return (await cur.fetchone()) is not None + sql = "SELECT 1 FROM project_members WHERE project_id = ? AND member_id = ?" + params: list = [project_id, member_id] + if member_kind is not None: + sql += " AND member_kind = ?" + params.append(member_kind) + async with self._db.execute(sql, params) as cur: + return (await cur.fetchone()) is not None🤖 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 `@tinyagentos/projects/project_store.py` around lines 382 - 404, Update is_project_member to use a single SQL query and parameter set, making the member_kind predicate optional while preserving filtering by member_kind when provided and matching any kind when omitted. Remove the duplicated execute branches and retain the existing boolean result behavior.tinyagentos/delegation_handler.py (2)
563-589: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftKill switch is process-local and not persisted.
app.state._peer_disabledresets on restart and is not shared across workers, so a level-3 panic silently lapses (or applies to only one worker under multi-process deployment). Consider persisting the flag (settings/DB) and reading it inis_peer_disabled.🤖 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 `@tinyagentos/delegation_handler.py` around lines 563 - 589, The level-3 peer disable state currently exists only in process-local app state. Update kill_switch_per_instance, kill_switch_reenable, and is_peer_disabled to persist the flag through the existing shared settings or database mechanism, and have is_peer_disabled read that persisted value so the panic survives restarts and is consistent across workers.
500-522: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent swallow makes partial unassign undebuggable.
Line 521-522 drops every per-task failure with no log, so
unassigned_countunder-reports with no trace — exactly the forensic trail a kill-switch needs. Alsoexcept AttributeErroronly guards the call itself; an awaited coroutine raisingAttributeErrorinternally would be misrouted to the fallback.♻️ Proposed fix
try: await task_store.update_task(task_id, assignee_id=None, status="ready") count += 1 - except Exception: - pass + except Exception: + logger.warning( + "cascade_revoke: failed to unassign task %s from %s", + task_id, canonical_id, exc_info=True, + )🤖 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 `@tinyagentos/delegation_handler.py` around lines 500 - 522, Update the task-unassignment loop around task_store.update_task to log each per-task exception with the task identifier before continuing, preserving the count behavior for successful updates. Narrow the AttributeError handling around task_store.list_for_assignee so only a missing-method condition triggers the list_tasks fallback, while AttributeError raised during coroutine execution is propagated or handled as a real failure rather than misrouted.Source: Linters/SAST tools
tinyagentos/routes/project_invites.py (1)
1124-1131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract this metadata parsing (duplicated at Lines 1224-1231) and enforce a string sponsor id.
♻️ Proposed helper
def _sponsor_from_invite(invite: dict) -> str | None: meta = invite.get("metadata") or {} if isinstance(meta, str): try: meta = json.loads(meta) except (ValueError, TypeError): meta = {} if not isinstance(meta, dict): return None sponsor = meta.get("sponsor_contact_id") return sponsor if isinstance(sponsor, str) and sponsor.strip() else None- invite_metadata = invite.get("metadata") or {} - if isinstance(invite_metadata, str): - try: - invite_metadata = json.loads(invite_metadata) - except (ValueError, TypeError): - invite_metadata = {} - sponsor_contact_id = invite_metadata.get("sponsor_contact_id") if isinstance(invite_metadata, dict) else None + sponsor_contact_id = _sponsor_from_invite(invite)🤖 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 `@tinyagentos/routes/project_invites.py` around lines 1124 - 1131, The duplicated sponsor metadata parsing in the invite handling paths should be centralized and should only return valid non-empty string IDs. Add a helper such as _sponsor_from_invite, replace both parsing blocks around the existing extraction sites with calls to it, and preserve None for invalid, missing, non-dictionary, malformed, or blank sponsor_contact_id values.tests/test_collab_d1_delegation.py (1)
270-300: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the delegation mint paths.
The store/validation layers are covered, but
_auto_approve_delegationandcomplete_delegation_approval(the code that actually reads themintresult and theproject_invite_storeapp-state attribute) have no test. Both contain contract bugs flagged intinyagentos/delegation_handler.pythat a single test would surface.Want me to draft those tests?
🤖 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 `@tests/test_collab_d1_delegation.py` around lines 270 - 300, The existing TestInviteMetadata coverage only exercises ProjectInviteStore.mint directly; add a test covering the delegation mint flow through _auto_approve_delegation and complete_delegation_approval. Configure the app state’s project_invite_store, invoke the delegation approval path, and assert it correctly consumes the mint result and completes approval without contract errors, including the expected invite/approval outcome.tinyagentos/agent_registry_store.py (1)
845-863: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
bump_token_min_iatcan lower the cutoff, silently un-superseding old tokens.The UPDATE unconditionally sets
token_min_iat = ?regardless of the current value. If this is ever called with a timestamp earlier than the existing cutoff (e.g., clock skew, a stale/duplicate rotation call, or a future caller other than the route), tokens that were already superseded become valid again — defeating the security guarantee this feature exists to provide.🔒 Proposed fix: make the cutoff monotonically non-decreasing
await self._db.execute( - "UPDATE agent_registry SET token_min_iat = ? WHERE canonical_id = ?", + "UPDATE agent_registry SET token_min_iat = MAX(token_min_iat, ?) WHERE canonical_id = ?", (ts, canonical_id), )🤖 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 `@tinyagentos/agent_registry_store.py` around lines 845 - 863, Update bump_token_min_iat so the database update only raises token_min_iat, never lowers an existing cutoff; use a monotonic SQL condition or equivalent comparison while preserving the existing missing-record and updated-record behavior.tests/test_token_rotation.py (2)
388-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
tokenbindings.Static analysis flags
tokenas unpacked-but-unused in bothtest_admin_can_rotate(Line 390) andtest_owner_can_rotate(Line 399).🧹 Proposed fix
- cid, token = await _register_and_mint(app, user_id="admin") + cid, _token = await _register_and_mint(app, user_id="admin")(apply to both occurrences)
🤖 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 `@tests/test_token_rotation.py` around lines 388 - 403, Remove the unused token bindings in both test_admin_can_rotate and test_owner_can_rotate by unpacking only the cid value returned from _register_and_mint, while preserving the existing rotation requests and assertions.Source: Linters/SAST tools
22-236: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftLarge duplicated fixture instead of reusing the existing one.
The docstring states this fixture should Reuse the same init + admin-session logic as the main client fixture, but the ~200-line store-init/close sequence appears to be copy-pasted into this new file rather than depending on the existing fixture. If an equivalent fixture (e.g., the one backing the main API test client) already exists in
conftest.py, depending on it directly and layering only the two extraagent_registry/agent_grantsinits on top would avoid this duplication and its maintenance burden (every new store added to the app must now be duplicated here too).#!/bin/bash # Locate the fixture this docstring claims to mirror, to confirm duplication. fd conftest.py --type f rg -n "async def .*client" tests/conftest.py -A5 2>/dev/null | head -50 rg -n "def app\(" tests/conftest.py -A5 2>/dev/null | head -50🤖 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 `@tests/test_token_rotation.py` around lines 22 - 236, Refactor the agent_app fixture to depend on the existing main client fixture from conftest.py, preserving its initialized stores, admin session, and cleanup behavior. Remove the duplicated store initialization and teardown sequence, and retain only the agent_registry and agent_grants initialization needed by this fixture before yielding the reused client.tinyagentos/routes/agent_registry.py (1)
770-777: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAudit entry carries no information about the actual rotation.
_audit_governanceis called withbefore_status/after_statusboth derived fromstatus(unaffected by rotation), so the persisted audit record always showsbefore == afterand doesn't capture what changed (the old vs. newtoken_min_iat). This weakens the "forensic audit-log entry" the PR objective calls for — an investigator can see that a rotation happened and who triggered it, but not the cutoff values involved.Consider extending the audit payload (or
_audit_governance) to includerecord["token_min_iat"](before) andts(after) instead of reusing the status fields.🤖 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 `@tinyagentos/routes/agent_registry.py` around lines 770 - 777, Update the rotate-tokens audit flow around _audit_governance to record the actual cutoff transition: use the pre-rotation record["token_min_iat"] as the before value and ts as the after value, rather than deriving both from status. Extend the audit payload or _audit_governance as needed while preserving the actor and rotation context.
🤖 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 `@tinyagentos/delegation_handler.py`:
- Around line 294-298: Update both _auto_approve_delegation
(tinyagentos/delegation_handler.py#L294-L298) and complete_delegation_approval
(tinyagentos/delegation_handler.py#L378-L382) to read the minted invite
identifier from invite["record"]["invite_id"] instead of invite["id"],
preserving the existing response shape.
- Around line 264-266: Update the invite-store lookups in both
_auto_approve_delegation and complete_delegation_approval to read
request.app.state.project_invites instead of project_invite_store. Preserve the
existing unavailable-store error response and all surrounding delegation
behavior.
- Around line 416-420: Update the project-scoped filtering around project_id and
project_store so a set project_id without a project_store errors out immediately
instead of skipping membership checks. Preserve the existing is_project_member
filtering when project_store is available, preventing revocation of identities
outside the requested project.
- Around line 154-171: Restrict the auto-approval path in the delegation handler
around validate_delegation_scopes and _auto_approve_delegation to grant only
scopes in SPONSORED_DEFAULT_SCOPES; scopes outside that default tier must not be
auto-approved and must continue through explicit per-scope Decisions approval,
while preserving the existing hard-denial behavior.
In `@tinyagentos/projects/project_store.py`:
- Around line 405-417: Update get_project_setting to validate that the project’s
settings value is a dictionary before calling get on it. Return default when
settings is missing or any non-dict value, while preserving keyed lookup
behavior for valid dictionaries and matching the guard used by
set_project_setting.
In `@tinyagentos/routes/agent_auth_requests.py`:
- Around line 460-463: Update the existing-identity reuse branch around
registry.set_sponsor so sponsorship is assigned only when the identity currently
has no sponsor. Load or reuse the existing identity’s current sponsor value,
preserve any existing or different sponsor, and skip set_sponsor when it is
already populated.
In `@tinyagentos/routes/agent_registry.py`:
- Around line 761-778: Handle a None result from store.bump_token_min_iat in the
token-rotation flow before accessing updated.get or auditing; return the same
404 not-found response used when the initial store.get returns None, while
preserving the existing success path for a returned record.
---
Outside diff comments:
In `@tinyagentos/routes/peer.py`:
- Around line 258-273: Move the delegation_request dispatch branch in the peer
message handler above the unrecognised-kind logger so recognized requests are
not logged as unrecognised. Reuse the existing body_data value from the earlier
code instead of reading envelope.get("body", {}) again, while preserving the
process_delegation_request call and response handling.
---
Nitpick comments:
In `@tests/test_collab_d1_delegation.py`:
- Around line 270-300: The existing TestInviteMetadata coverage only exercises
ProjectInviteStore.mint directly; add a test covering the delegation mint flow
through _auto_approve_delegation and complete_delegation_approval. Configure the
app state’s project_invite_store, invoke the delegation approval path, and
assert it correctly consumes the mint result and completes approval without
contract errors, including the expected invite/approval outcome.
In `@tests/test_token_rotation.py`:
- Around line 388-403: Remove the unused token bindings in both
test_admin_can_rotate and test_owner_can_rotate by unpacking only the cid value
returned from _register_and_mint, while preserving the existing rotation
requests and assertions.
- Around line 22-236: Refactor the agent_app fixture to depend on the existing
main client fixture from conftest.py, preserving its initialized stores, admin
session, and cleanup behavior. Remove the duplicated store initialization and
teardown sequence, and retain only the agent_registry and agent_grants
initialization needed by this fixture before yielding the reused client.
In `@tinyagentos/agent_registry_store.py`:
- Around line 845-863: Update bump_token_min_iat so the database update only
raises token_min_iat, never lowers an existing cutoff; use a monotonic SQL
condition or equivalent comparison while preserving the existing missing-record
and updated-record behavior.
In `@tinyagentos/delegation_handler.py`:
- Around line 563-589: The level-3 peer disable state currently exists only in
process-local app state. Update kill_switch_per_instance, kill_switch_reenable,
and is_peer_disabled to persist the flag through the existing shared settings or
database mechanism, and have is_peer_disabled read that persisted value so the
panic survives restarts and is consistent across workers.
- Around line 500-522: Update the task-unassignment loop around
task_store.update_task to log each per-task exception with the task identifier
before continuing, preserving the count behavior for successful updates. Narrow
the AttributeError handling around task_store.list_for_assignee so only a
missing-method condition triggers the list_tasks fallback, while AttributeError
raised during coroutine execution is propagated or handled as a real failure
rather than misrouted.
In `@tinyagentos/projects/project_store.py`:
- Around line 382-404: Update is_project_member to use a single SQL query and
parameter set, making the member_kind predicate optional while preserving
filtering by member_kind when provided and matching any kind when omitted.
Remove the duplicated execute branches and retain the existing boolean result
behavior.
In `@tinyagentos/routes/agent_registry.py`:
- Around line 770-777: Update the rotate-tokens audit flow around
_audit_governance to record the actual cutoff transition: use the pre-rotation
record["token_min_iat"] as the before value and ts as the after value, rather
than deriving both from status. Extend the audit payload or _audit_governance as
needed while preserving the actor and rotation context.
In `@tinyagentos/routes/decisions.py`:
- Around line 476-479: Remove the unnecessary f-string prefix from the error_msg
assignment in the delegation approval error-handling code, since the message
contains no interpolated expressions. Preserve the existing message text
unchanged.
In `@tinyagentos/routes/project_invites.py`:
- Around line 1124-1131: The duplicated sponsor metadata parsing in the invite
handling paths should be centralized and should only return valid non-empty
string IDs. Add a helper such as _sponsor_from_invite, replace both parsing
blocks around the existing extraction sites with calls to it, and preserve None
for invalid, missing, non-dictionary, malformed, or blank sponsor_contact_id
values.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 91988a57-fb4a-4d47-ba8c-18736de70c41
📒 Files selected for processing (13)
tests/test_agent_registry_store.pytests/test_collab_d1_delegation.pytests/test_token_rotation.pytinyagentos/agent_registry_store.pytinyagentos/agent_token_auth.pytinyagentos/delegation_handler.pytinyagentos/projects/invite_store.pytinyagentos/projects/project_store.pytinyagentos/routes/agent_auth_requests.pytinyagentos/routes/agent_registry.pytinyagentos/routes/decisions.pytinyagentos/routes/peer.pytinyagentos/routes/project_invites.py
Comment on lines
+154
to
+171
| granted_scopes, denied_scopes = validate_delegation_scopes(requested_scopes) | ||
|
|
||
| # Check project policy: auto_approve_delegation knob. | ||
| auto_approve = await project_store.get_project_setting( | ||
| project_id, "auto_approve_delegation", default=False | ||
| ) | ||
|
|
||
| if auto_approve: | ||
| # Future dev-swarm path: immediate approval. | ||
| return await _auto_approve_delegation( | ||
| request, | ||
| contact_id=contact_id, | ||
| agent_slug=agent_slug, | ||
| display_name=display_name, | ||
| granted_scopes=granted_scopes, | ||
| denied_scopes=denied_scopes, | ||
| project_id=project_id, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Auto-approve mints every non-denied scope, contradicting the documented tier.
validate_delegation_scopes only strips the two hard-denied scopes, and its own docstring (Lines 48-50) states scopes outside SPONSORED_DEFAULT_SCOPES "require explicit per-scope Decisions approval". On the auto_approve_delegation path no human sees them, so a remote contact can self-grant e.g. canvas_write or files_read by listing them in the envelope.
🔒 Proposed fix — restrict the auto path to the default tier
if auto_approve:
# Future dev-swarm path: immediate approval.
+ auto_scopes = sorted(set(granted_scopes) & SPONSORED_DEFAULT_SCOPES)
+ elevated = sorted(set(granted_scopes) - SPONSORED_DEFAULT_SCOPES)
+ if elevated:
+ logger.warning(
+ "delegation: auto-approve dropped elevated scopes %r for %s",
+ elevated, contact_id,
+ )
return await _auto_approve_delegation(
request,
contact_id=contact_id,
agent_slug=agent_slug,
display_name=display_name,
- granted_scopes=granted_scopes,
- denied_scopes=denied_scopes,
+ granted_scopes=auto_scopes,
+ denied_scopes=denied_scopes + elevated,
project_id=project_id,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| granted_scopes, denied_scopes = validate_delegation_scopes(requested_scopes) | |
| # Check project policy: auto_approve_delegation knob. | |
| auto_approve = await project_store.get_project_setting( | |
| project_id, "auto_approve_delegation", default=False | |
| ) | |
| if auto_approve: | |
| # Future dev-swarm path: immediate approval. | |
| return await _auto_approve_delegation( | |
| request, | |
| contact_id=contact_id, | |
| agent_slug=agent_slug, | |
| display_name=display_name, | |
| granted_scopes=granted_scopes, | |
| denied_scopes=denied_scopes, | |
| project_id=project_id, | |
| ) | |
| granted_scopes, denied_scopes = validate_delegation_scopes(requested_scopes) | |
| # Check project policy: auto_approve_delegation knob. | |
| auto_approve = await project_store.get_project_setting( | |
| project_id, "auto_approve_delegation", default=False | |
| ) | |
| if auto_approve: | |
| # Future dev-swarm path: immediate approval. | |
| auto_scopes = sorted(set(granted_scopes) & SPONSORED_DEFAULT_SCOPES) | |
| elevated = sorted(set(granted_scopes) - SPONSORED_DEFAULT_SCOPES) | |
| if elevated: | |
| logger.warning( | |
| "delegation: auto-approve dropped elevated scopes %r for %s", | |
| elevated, contact_id, | |
| ) | |
| return await _auto_approve_delegation( | |
| request, | |
| contact_id=contact_id, | |
| agent_slug=agent_slug, | |
| display_name=display_name, | |
| granted_scopes=auto_scopes, | |
| denied_scopes=denied_scopes + elevated, | |
| project_id=project_id, | |
| ) |
🤖 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 `@tinyagentos/delegation_handler.py` around lines 154 - 171, Restrict the
auto-approval path in the delegation handler around validate_delegation_scopes
and _auto_approve_delegation to grant only scopes in SPONSORED_DEFAULT_SCOPES;
scopes outside that default tier must not be auto-approved and must continue
through explicit per-scope Decisions approval, while preserving the existing
hard-denial behavior.
Comment on lines
+264
to
+266
| invite_store = getattr(request.app.state, "project_invite_store", None) | ||
| if invite_store is None: | ||
| return {"status": "error", "error": "invite store not available"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Wrong app.state attribute — the invite store is project_invites.
Everywhere else in the codebase the store is registered as request.app.state.project_invites (routes/peer.py Line 511, routes/project_invites.py Line 1092, and the test fixtures). project_invite_store is never set, so both _auto_approve_delegation and complete_delegation_approval (Line 340) always short-circuit with "invite store not available" — delegation can never complete.
🐛 Proposed fix (apply at both sites)
- invite_store = getattr(request.app.state, "project_invite_store", None)
+ invite_store = getattr(request.app.state, "project_invites", None)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| invite_store = getattr(request.app.state, "project_invite_store", None) | |
| if invite_store is None: | |
| return {"status": "error", "error": "invite store not available"} | |
| invite_store = getattr(request.app.state, "project_invites", None) | |
| if invite_store is None: | |
| return {"status": "error", "error": "invite store not available"} |
🤖 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 `@tinyagentos/delegation_handler.py` around lines 264 - 266, Update the
invite-store lookups in both _auto_approve_delegation and
complete_delegation_approval to read request.app.state.project_invites instead
of project_invite_store. Preserve the existing unavailable-store error response
and all surrounding delegation behavior.
Comment on lines
+294
to
+298
| return { | ||
| "status": "approved", | ||
| "invite_id": invite["id"], | ||
| "agent_slug": agent_slug, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
mint return-shape mismatch at both delegation mint sites. ProjectInviteStore.mint returns {"record": {..., "invite_id": ...}, "pin": ...}, so invite["id"] raises KeyError in both handlers; both reads sit outside the surrounding try, so the failure escapes as a 500 / generic "approval failed".
tinyagentos/delegation_handler.py#L294-L298: in_auto_approve_delegation, returninvite["record"]["invite_id"].tinyagentos/delegation_handler.py#L378-L382: incomplete_delegation_approval, returninvite["record"]["invite_id"].
📍 Affects 1 file
tinyagentos/delegation_handler.py#L294-L298(this comment)tinyagentos/delegation_handler.py#L378-L382
🤖 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 `@tinyagentos/delegation_handler.py` around lines 294 - 298, Update both
_auto_approve_delegation (tinyagentos/delegation_handler.py#L294-L298) and
complete_delegation_approval (tinyagentos/delegation_handler.py#L378-L382) to
read the minted invite identifier from invite["record"]["invite_id"] instead of
invite["id"], preserving the existing response shape.
Comment on lines
+416
to
+420
| if project_id is not None and project_store is not None: | ||
| if not await project_store.is_project_member( | ||
| project_id, canonical_id | ||
| ): | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Project-scoped revoke fails open when project_store is missing.
With project_id set but no project_store on app.state, the membership filter is skipped and every sponsored identity for the contact is revoked — a wider blast radius than the caller asked for. Prefer erroring out instead.
🛡️ Proposed fix
+ if project_id is not None and project_store is None:
+ return {"status": "error", "error": "project store not available"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if project_id is not None and project_store is not None: | |
| if not await project_store.is_project_member( | |
| project_id, canonical_id | |
| ): | |
| continue | |
| if project_id is not None and project_store is None: | |
| return {"status": "error", "error": "project store not available"} | |
| if project_id is not None and project_store is not None: | |
| if not await project_store.is_project_member( | |
| project_id, canonical_id | |
| ): | |
| continue |
🤖 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 `@tinyagentos/delegation_handler.py` around lines 416 - 420, Update the
project-scoped filtering around project_id and project_store so a set project_id
without a project_store errors out immediately instead of skipping membership
checks. Preserve the existing is_project_member filtering when project_store is
available, preventing revocation of identities outside the requested project.
Comment on lines
+405
to
+417
| async def get_project_setting( | ||
| self, project_id: str, key: str, default=None | ||
| ): | ||
| """Read a single key from the project's JSON settings dict. | ||
|
|
||
| Returns *default* if the project does not exist, has no settings, or | ||
| the key is absent. | ||
| """ | ||
| project = await self.get_project(project_id) | ||
| if project is None: | ||
| return default | ||
| settings = project.get("settings") or {} | ||
| return settings.get(key, default) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against a non-dict settings here too.
set_project_setting defends with isinstance(settings, dict) (Line 430) but the reader does not; a legacy row whose settings JSON is a list/scalar makes this raise AttributeError instead of returning default.
🛡️ Proposed fix
- settings = project.get("settings") or {}
- return settings.get(key, default)
+ settings = project.get("settings") or {}
+ if not isinstance(settings, dict):
+ return default
+ return settings.get(key, default)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async def get_project_setting( | |
| self, project_id: str, key: str, default=None | |
| ): | |
| """Read a single key from the project's JSON settings dict. | |
| Returns *default* if the project does not exist, has no settings, or | |
| the key is absent. | |
| """ | |
| project = await self.get_project(project_id) | |
| if project is None: | |
| return default | |
| settings = project.get("settings") or {} | |
| return settings.get(key, default) | |
| async def get_project_setting( | |
| self, project_id: str, key: str, default=None | |
| ): | |
| """Read a single key from the project's JSON settings dict. | |
| Returns *default* if the project does not exist, has no settings, or | |
| the key is absent. | |
| """ | |
| project = await self.get_project(project_id) | |
| if project is None: | |
| return default | |
| settings = project.get("settings") or {} | |
| if not isinstance(settings, dict): | |
| return default | |
| return settings.get(key, default) |
🤖 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 `@tinyagentos/projects/project_store.py` around lines 405 - 417, Update
get_project_setting to validate that the project’s settings value is a
dictionary before calling get on it. Return default when settings is missing or
any non-dict value, while preserving keyed lookup behavior for valid
dictionaries and matching the guard used by set_project_setting.
Comment on lines
+460
to
+463
| # For sponsored agents (cross-user collab D1), set the sponsor | ||
| # on the existing identity when reusing it for a new project. | ||
| if sponsor_contact_id: | ||
| await registry.set_sponsor(existing_cid, sponsor_contact_id) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reusing an existing handle can retro-assign sponsorship of a pre-existing identity.
This branch fires when the derived handle already maps to an ACTIVE identity that may have been created by a normal consent flow (or sponsored by a different contact). Setting sponsor_contact_id unconditionally brings that identity under the new contact's cascade_sponsor_revoke authority and overwrites a previous sponsor. Only set it when the row currently has no sponsor.
🔒 Proposed fix
- if sponsor_contact_id:
- await registry.set_sponsor(existing_cid, sponsor_contact_id)
+ if sponsor_contact_id and not existing_active.get("sponsor_contact_id"):
+ await registry.set_sponsor(existing_cid, sponsor_contact_id)
+ elif sponsor_contact_id and existing_active.get("sponsor_contact_id") != sponsor_contact_id:
+ logger.warning(
+ "auth-approve: refusing to re-sponsor %s (existing sponsor %r)",
+ existing_cid, existing_active.get("sponsor_contact_id"),
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # For sponsored agents (cross-user collab D1), set the sponsor | |
| # on the existing identity when reusing it for a new project. | |
| if sponsor_contact_id: | |
| await registry.set_sponsor(existing_cid, sponsor_contact_id) | |
| # For sponsored agents (cross-user collab D1), set the sponsor | |
| # on the existing identity when reusing it for a new project. | |
| if sponsor_contact_id and not existing_active.get("sponsor_contact_id"): | |
| await registry.set_sponsor(existing_cid, sponsor_contact_id) | |
| elif sponsor_contact_id and existing_active.get("sponsor_contact_id") != sponsor_contact_id: | |
| logger.warning( | |
| "auth-approve: refusing to re-sponsor %s (existing sponsor %r)", | |
| existing_cid, existing_active.get("sponsor_contact_id"), | |
| ) |
🤖 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 `@tinyagentos/routes/agent_auth_requests.py` around lines 460 - 463, Update the
existing-identity reuse branch around registry.set_sponsor so sponsorship is
assigned only when the identity currently has no sponsor. Load or reuse the
existing identity’s current sponsor value, preserve any existing or different
sponsor, and skip set_sponsor when it is already populated.
| from tinyagentos.delegation_handler import process_delegation_request | ||
|
|
||
| body_data = envelope.get("body", {}) | ||
| result = await process_delegation_request( |
There was a problem hiding this comment.
WARNING: Misleading log — delegation_request is dispatched but logged as "unrecognised kind"
The logger.info block above marks the kind as unrecognised before the delegation_request dispatch below. This masks real unrecognised kinds in logs.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summary (commit bbbf0ea)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit bbbf0ea)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (13 files)
Reviewed by step-3.7-flash · Input: 95.4K · Output: 21.5K · Cached: 473.1K |
Code Review by Qodo
1. metadata in invites SCHEMA
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+48
to
+49
| status TEXT NOT NULL DEFAULT 'active', | ||
| sponsor_contact_id TEXT |
There was a problem hiding this comment.
1. sponsor_contact_id in schema 📜 Skill insight ≡ Correctness
agent_registry_store.SCHEMA includes sponsor_contact_id, but this column is also added via a migration function. This violates the rule that migration-added columns must not be referenced in SCHEMA, which can break initialization order guarantees on older DBs.
Agent Prompt
## Issue description
`SCHEMA` currently includes `sponsor_contact_id`, while `_migration_v5_add_sponsor_contact_id()` also adds it. Per compliance, migration-added columns must not appear in `SCHEMA`.
## Issue Context
`BaseStore` runs `SCHEMA` before `_post_init()` migrations; `SCHEMA` must therefore only contain columns guaranteed to exist pre-migration.
## Fix Focus Areas
- tinyagentos/agent_registry_store.py[35-50]
- tinyagentos/agent_registry_store.py[209-227]
- tinyagentos/agent_registry_store.py[460-464]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+35
to
37
| contact_id TEXT, | ||
| metadata TEXT NOT NULL DEFAULT '{}' | ||
| ); |
There was a problem hiding this comment.
2. metadata in invites schema 📜 Skill insight ≡ Correctness
ProjectInviteStore.SCHEMA includes metadata, but _post_init() also conditionally adds metadata via ALTER TABLE. This violates the rule that migration-added columns must not appear in SCHEMA.
Agent Prompt
## Issue description
`ProjectInviteStore.SCHEMA` references `metadata`, but `_post_init()` adds `metadata` for older DBs via guarded `ALTER TABLE`. Compliance requires that `SCHEMA` not reference any migration-added columns.
## Issue Context
`SCHEMA` executes before `_post_init()` runs, so it must stay compatible with pre-migration DB assumptions.
## Fix Focus Areas
- tinyagentos/projects/invite_store.py[17-38]
- tinyagentos/projects/invite_store.py[114-118]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+8
| """Agent delegation handler — cross-user collab D1. | ||
|
|
||
| Processes delegation-request envelopes from remote contacts, applies | ||
| scope denylist and policy gates, creates Decisions cards for manual | ||
| approval, and on approval mints project invites for the sponsored agent. | ||
|
|
||
| Also provides cascade revocation and kill-switch machinery. | ||
| """ |
There was a problem hiding this comment.
3. Em dashes in docstrings 📜 Skill insight ✧ Quality
New/modified docstrings and comments include Unicode em dashes (—). This violates the no-em-dashes rule for public-facing text and developer-visible strings.
Agent Prompt
## Issue description
The PR introduces em dash characters (`—`, U+2014) in docstrings/comments/strings, which are disallowed.
## Issue Context
Compliance requires using regular hyphens (`-`) or rephrasing instead of em dashes in any public-facing/developer-facing text.
## Fix Focus Areas
- tinyagentos/delegation_handler.py[1-23]
- tinyagentos/agent_token_auth.py[20-24]
- tinyagentos/routes/agent_registry.py[754-760]
- tests/test_collab_d1_delegation.py[1-1]
- tests/test_agent_registry_store.py[1188-1190]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+264
to
+298
| invite_store = getattr(request.app.state, "project_invite_store", None) | ||
| if invite_store is None: | ||
| return {"status": "error", "error": "invite store not available"} | ||
|
|
||
| # The project-scoped scopes require a non-null project_id for the JWT. | ||
| project_scopes = sorted(set(granted_scopes) & _PROJECT_SCOPES) | ||
| non_project_scopes = sorted(set(granted_scopes) - _PROJECT_SCOPES) | ||
|
|
||
| try: | ||
| invite = await invite_store.mint( | ||
| project_id=project_id, | ||
| scopes=non_project_scopes + project_scopes, | ||
| approval_mode="auto", | ||
| check_interval_secs=1800, | ||
| created_by=contact_id, | ||
| metadata={ | ||
| "kind": "delegation_sponsored", | ||
| "sponsor_contact_id": contact_id, | ||
| "agent_slug": agent_slug, | ||
| "display_name": display_name, | ||
| "pin_required": False, | ||
| }, | ||
| ) | ||
| except Exception: | ||
| logger.warning( | ||
| "delegation: failed to create invite for %s / %s", | ||
| contact_id, agent_slug, exc_info=True, | ||
| ) | ||
| return {"status": "error", "error": "failed to create project invite"} | ||
|
|
||
| return { | ||
| "status": "approved", | ||
| "invite_id": invite["id"], | ||
| "agent_slug": agent_slug, | ||
| } |
There was a problem hiding this comment.
5. Delegation invite mint broken 🐞 Bug ≡ Correctness
delegation_handler looks up the invite store at app.state.project_invite_store and returns
invite['id'], but the app wires the store as app.state.project_invites and
ProjectInviteStore.mint() returns {record:{invite_id,...}, pin}. As a result, delegation
auto-approve and approved Decisions side-effects will fail at runtime (either always “invite store
not available” or raising KeyError).
Agent Prompt
### Issue description
`_auto_approve_delegation()` / `complete_delegation_approval()` are incompatible with the actual app wiring and `ProjectInviteStore.mint()` return shape.
### Issue Context
- The app sets the invite store on `app.state.project_invites`.
- `ProjectInviteStore.mint()` returns a dict with the invite id at `result["record"]["invite_id"]`.
### Fix Focus Areas
- tinyagentos/delegation_handler.py[249-299]
- tinyagentos/delegation_handler.py[340-383]
### Suggested change
- Replace `getattr(request.app.state, "project_invite_store", None)` with `getattr(request.app.state, "project_invites", None)`.
- When minting, treat the return value as `mint_result` and return `mint_result["record"]["invite_id"]` (and optionally return the `pin` if the flow needs it).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+56
to
+59
| requested_set = set(requested_scopes) | ||
| denied = sorted(requested_set & SPONSORED_DENY_SCOPES) | ||
| allowed = sorted(requested_set - SPONSORED_DENY_SCOPES) | ||
| if denied: |
There was a problem hiding this comment.
6. Delegation scopes can crash 🐞 Bug ☼ Reliability
validate_delegation_scopes() / _validate_delegation_envelope_body() build set(requested_scopes) without validating that every element is a string, so a peer can send an unhashable element (e.g. an object) and trigger a TypeError -> 500. This is a new externally reachable crash path via /api/peer/inbox delegation dispatch.
Agent Prompt
### Issue description
Delegation request validation checks only that `requested_scopes` is a list, but it does not validate list *elements* before using `set(...)`. Malformed JSON can trigger `TypeError: unhashable type` and crash the request.
### Issue Context
`/api/peer/inbox` dispatches `kind == "delegation_request"` directly into `process_delegation_request()`, so this error is reachable from authenticated peer traffic.
### Fix Focus Areas
- tinyagentos/delegation_handler.py[42-65]
- tinyagentos/delegation_handler.py[67-105]
- tinyagentos/routes/peer.py[264-273]
### Suggested change
- In `_validate_delegation_envelope_body`, validate that every entry in `requested_scopes` is a non-empty `str` (e.g., `all(isinstance(s, str) and s.strip() for s in value)`), otherwise return `(False, "requested_scopes must be a list of non-empty strings", None)`.
- Optionally defensively wrap `validate_delegation_scopes`’s `set(...)` conversion in a try/except and return a structured error if validation is bypassed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+115
to
+119
| # Reject tokens issued before the identity's token_min_iat cutoff (rotation). | ||
| token_min_iat = record.get("token_min_iat") or 0 | ||
| token_iat = payload.get("iat") or 0 | ||
| if token_iat < token_min_iat: | ||
| raise HTTPException(status_code=401, detail="token superseded") |
There was a problem hiding this comment.
7. Superseded token still authorizes 🐞 Bug ⛨ Security
The new token_min_iat cutoff is enforced only inside _verify_agent_scope, but check_agent_identity() (used to authorize the scope-request creation flow) does not enforce token_min_iat. After rotating tokens, an old token can still authenticate scope-request creation for that identity.
Agent Prompt
### Issue description
Token rotation is implemented in `_verify_agent_scope()`, but other token verification paths still accept superseded tokens.
### Issue Context
`create_scope_request` authorizes via `check_agent_identity()` for agent self-auth, and `check_agent_identity()` currently does not check `token_min_iat`.
### Fix Focus Areas
- tinyagentos/agent_token_auth.py[83-120]
- tinyagentos/agent_token_auth.py[159-198]
- tinyagentos/routes/agent_auth_requests.py[905-929]
### Suggested change
- Add the same `token_min_iat` vs `payload["iat"]` check to `check_agent_identity()`.
- Preferably refactor into a shared helper that:
- verifies signature
- loads registry record
- checks status == active
- enforces token_min_iat cutoff
and is used by both `_verify_agent_scope()` and `check_agent_identity()` to prevent future drift.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+767
to
+768
| ts = int(time.time()) | ||
| updated = await store.bump_token_min_iat(canonical_id, ts) |
There was a problem hiding this comment.
8. Same-second rotation loophole 🐞 Bug ⛨ Security
rotate-tokens stores token_min_iat = int(time.time()) and tokens are minted with `iat = int(time.time()); with the strict <` comparison, tokens minted earlier in the same second as the bump have iat == token_min_iat and remain valid. Rotation therefore does not reliably invalidate existing tokens without waiting for the next second boundary.
Agent Prompt
### Issue description
Token rotation compares second-resolution timestamps and uses a strict `<` comparison, leaving a same-second window where “old” tokens are not invalidated.
### Issue Context
- `mint_registry_token()` sets `iat` as `int(time.time())`.
- `rotate_tokens()` bumps `token_min_iat` to `int(time.time())`.
- Auth rejects only when `token_iat < token_min_iat`.
### Fix Focus Areas
- tinyagentos/routes/agent_registry.py[748-778]
- tinyagentos/agent_token_auth.py[115-120]
- tinyagentos/agent_registry_store.py[322-369]
### Suggested change (one viable approach)
- Switch `iat` and `token_min_iat` to higher precision values:
- set `iat` to `time.time()` (float seconds)
- set rotation cutoff `ts` to `time.time()` (float seconds)
- keep the strict `<` comparison
This avoids same-second collisions while keeping NumericDate semantics (seconds since epoch).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- bump_token_min_iat: use MAX(token_min_iat, ?) so the cutoff is monotonically non-decreasing — a stale/lower timestamp cannot silently un-supersede tokens - rotate-tokens route: handle None from bump_token_min_iat (race condition) with 404 instead of AttributeError on updated.get() - audit entry: capture before_token_min_iat and after_token_min_iat in the governance audit payload so rotation values are traceable - tests: remove unused token bindings in route-level tests
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
1 participant
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.
Closes #2205.
What
Add
token_min_iatINTEGER column (default 0) toagent_registryso registry JWT tokens can be invalidated per-identity without revoking the whole identity.Changes
ALTER TABLEwith default 0 — existing rows keep their tokens valid so the migration cannot lock the fleet outAgentRegistryStore.bump_token_min_iat(canonical_id, ts)— persists the cutoff_verify_agent_scope, reject tokens whoseiatclaim is strictly less than the record'stoken_min_iatwith 401"token superseded"POST /api/agents/registry/{id}/rotate-tokens— session-owner/admin route that bumps the cutoff and writes a forensic audit-log entry (action:rotate-tokens)Tests (9 new, all passing)
Store-level: bump sets cutoff, default zero on registration, nonexistent returns None
Auth-path: old token rejected after bump (401), new token passes after bump, default-zero keeps existing tokens valid
Route: admin rotates, owner rotates, nonexistent returns 404
Rotation flow
No existing test regressions — 138 store + 43 route + 11 auth = all green.
Summary by CodeRabbit
POST /api/agents/registry/{id}/rotate-tokens) that enforces a per-identity cutoff for previously issued tokens.