fix(auth): impersonation records activity on the identity it acts as - #595
Merged
Conversation
`last_active_at` was stamped in exactly one place — on the API key's own identity, and only when that identity is a sub-agent. The impersonation path never touched the identity it resolved. So an identity reached only through `X-Overslash-As` — which is every agent identity a white-label caller like overfolder creates — had `last_active_at` frozen at its creation timestamp forever, however much traffic flowed through it. `archive_idle_subagents` then did exactly what it is meant to do, to identities that were in daily use. In the `overfolder-dev` org, 0 of 14 identities had ever had `last_active_at` advance, and all 6 sub-agents ever created there are archived with reason `idle_timeout` — including the two built-in agents that had been serving a live user, archived 4h 0m 44s after creation, the org's timeout to the second. Every call afterwards 403s with "impersonation target is archived", and restore is the only way back. Stamp the resolved target too, after the ACL cap has agreed the caller may act as it. Only the leaf needs it: the sweep skips any identity with a live child, so a live leaf holds its ancestors up. `ResolvedTarget` carries the kind out so the extractor can make that sub-agent check without a second lookup. Two tests, both of which fail without the fix: an impersonated call advances the target's `last_active_at`, and a sub-agent used this second survives a sweep it would otherwise be reaped by. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmBpW2sFtpHcCWE3f2deCy
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Code diff size+26 / −0 across 2 files (net +26)
Source files under |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
The bug
last_active_atis written in exactly one place —extractors.rs, on the API key's own identity, and only when that identity is asub_agent. The impersonation path resolves a target and never touches it.So an identity reached only through
X-Overslash-Ashaslast_active_atfrozen at its creation timestamp forever, however much traffic flows through it. An org service key is not a sub-agent, so the existing touch never fires for it either. Impersonation is the one authentication route that produces activity without recording it.archive_idle_subagentsthen does exactly what it is meant to do, to identities that are in daily use.What it cost
In the
overfolder-devorg: 0 of 14 identities have ever hadlast_active_atadvance pastcreated_at, and all 6 sub-agents ever created there are archived with reasonidle_timeout. Two of them are a live user's built-in agents, archived 4h 0m 44s after creation —subagent_idle_timeout_secsto the second. Every call afterwards 403s withimpersonation target is archived, andPOST /v1/identities/{id}/restoreis the only way back (re-creating collides on theUNIQUE (org_id, external_id)an archived row still holds).Downstream, that surfaced to a real user as their assistant reporting their calendar and email as "down this morning". They were not.
The fix
Stamp the resolved target as well, after the ACL cap has agreed the caller may act as it, with the same fire-and-forget spawn the key's own identity already gets.
Only the leaf needs stamping: the sweep's
NOT EXISTS (live child)guard means a live leaf already holds its ancestors up.ResolvedTargetnow carrieskindso the extractor can make thesub_agentcheck without a second lookup.Tests
Two, in
tests/impersonation.rs, which had no activity coverage at all. Both fail without the fix (verified by reverting the hunk):impersonation_touches_sub_agent_target_last_active— an impersonated call advances the target'slast_active_at.impersonated_sub_agent_survives_the_idle_sweep— a sub-agent used this second survives a sweep that would otherwise reap it.cargo fmt --all --checkandcargo clippy --workspace --all-targets -- -D warningsare clean;impersonation(33) andidentity_idle_cleanup(22) pass.Two adjacent things, not fixed here
POST /v1/identitieswith an(org_id, external_id)that collides with an archived row returns 500, not 409 —routes/identities/crud.rshas nois_unique_violationcatch, unlike ~10 sibling routes. A caller-caused collision surfacing as a 5xx is both misleading and unactionable, and it is the reason "just re-provision" is not a recovery path for a client.restoreis route-gated tosub_agentthough the repo function has no such limit, so akind='agent'identity archived by any path has no API recovery at all.Downstream counterpart: overfolder#679 (recover from an already-archived identity instead of wedging).
🤖 Generated with Claude Code
https://claude.ai/code/session_01NmBpW2sFtpHcCWE3f2deCy