From 2f0a4efd109483ba9a32a5133135a75faa18bfc5 Mon Sep 17 00:00:00 2001 From: Factory Date: Sat, 5 Sep 2026 08:56:17 +0000 Subject: [PATCH] fix(auth): impersonation records activity on the identity it acts as MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) Claude-Session: https://claude.ai/code/session_01NmBpW2sFtpHcCWE3f2deCy --- crates/overslash-api/src/extractors.rs | 18 +++ crates/overslash-api/src/impersonation.rs | 8 ++ crates/overslash-api/tests/impersonation.rs | 123 ++++++++++++++++++++ 3 files changed, 149 insertions(+) diff --git a/crates/overslash-api/src/extractors.rs b/crates/overslash-api/src/extractors.rs index 4cfda184..46303354 100644 --- a/crates/overslash-api/src/extractors.rs +++ b/crates/overslash-api/src/extractors.rs @@ -440,6 +440,24 @@ impl FromRequestParts for AuthContext { .await?; } + // Impersonation is the one authentication route that leaves + // no trace on the identity it acts as: the touch above stamps + // the *key's* identity, and an org service key is never a + // sub-agent. Without this, a sub-agent reached only through + // this header has `last_active_at` frozen at creation and the + // idle sweep archives it however busy it actually is. + // + // Only the leaf needs stamping — the sweep skips any identity + // with a live child, so a live leaf holds its ancestors up. + if target.kind == "sub_agent" { + let touch_scope = + OrgScope::new(key_row.org_id, state.db_pool(&parts.extensions)); + let target_id = target.identity_id; + tokio::spawn(async move { + let _ = touch_scope.touch_identity_last_active(target_id).await; + }); + } + (Some(target.identity_id), Some(key_row.identity_id)) } Some(_) => { diff --git a/crates/overslash-api/src/impersonation.rs b/crates/overslash-api/src/impersonation.rs index 3f448382..b4442a3f 100644 --- a/crates/overslash-api/src/impersonation.rs +++ b/crates/overslash-api/src/impersonation.rs @@ -37,6 +37,13 @@ use crate::error::AppError; pub struct ResolvedTarget { /// The effective identity to act as — the leaf of the path. pub identity_id: Uuid, + /// The kind of the effective identity (`user`, `agent`, `sub_agent`). + /// + /// Carried out so the extractor can stamp `last_active_at` on a + /// `sub_agent` target without a second lookup: impersonation is the one + /// authentication route that produces activity on an identity nobody + /// holds a key for, and the idle sweep reaps whatever it cannot see. + pub kind: String, /// The root user whose display name the caller may still refresh, set only /// when a name was supplied for a user root that already existed. /// @@ -194,6 +201,7 @@ pub async fn resolve_target( Ok(ResolvedTarget { identity_id: current.id, + kind: current.kind, renameable_root, }) } diff --git a/crates/overslash-api/tests/impersonation.rs b/crates/overslash-api/tests/impersonation.rs index 36e83791..00284238 100644 --- a/crates/overslash-api/tests/impersonation.rs +++ b/crates/overslash-api/tests/impersonation.rs @@ -1152,3 +1152,126 @@ async fn name_header_rejects_an_agent_target() { .await; assert_eq!(status, 400); } + +// ── Activity tracking ──────────────────────────────────────────────────────── + +/// Create a `sub_agent` under `parent_id` and return its id. +async fn create_subagent( + base: &str, + client: &reqwest::Client, + admin_key: &str, + parent_id: Uuid, + name: &str, +) -> Uuid { + let sub: Value = client + .post(format!("{base}/v1/identities")) + .header("Authorization", format!("Bearer {admin_key}")) + .json(&json!({"name": name, "kind": "sub_agent", "parent_id": parent_id})) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + sub["id"].as_str().unwrap().parse().unwrap() +} + +/// The touch is fire-and-forget (`tokio::spawn`), so poll for it rather than +/// racing it. Returns the observed `last_active_at`. +async fn await_last_active_after( + pool: &PgPool, + id: Uuid, + floor: time::OffsetDateTime, +) -> time::OffsetDateTime { + for _ in 0..50 { + let seen: time::OffsetDateTime = + sqlx::query_scalar("SELECT last_active_at FROM identities WHERE id = $1") + .bind(id) + .fetch_one(pool) + .await + .unwrap(); + if seen > floor { + return seen; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + panic!("last_active_at never advanced past {floor} for {id}"); +} + +#[tokio::test] +async fn impersonation_touches_sub_agent_target_last_active() { + let (base, client, pool, org_id, admin_key, sa_id, _, target_agent_id) = setup().await; + let imp_key = create_impersonation_key(&base, &client, &admin_key, org_id, sa_id).await; + let sub_id = create_subagent(&base, &client, &admin_key, target_agent_id, "worker").await; + + // Backdate activity so any advance is unambiguously ours. + let floor: time::OffsetDateTime = sqlx::query_scalar( + "UPDATE identities SET last_active_at = now() - interval '2 hours' + WHERE id = $1 RETURNING last_active_at", + ) + .bind(sub_id) + .fetch_one(&pool) + .await + .unwrap(); + + let resp = client + .get(format!("{base}/v1/whoami")) + .header("Authorization", format!("Bearer {imp_key}")) + .header("X-Overslash-As", sub_id.to_string()) + .send() + .await + .unwrap(); + assert!( + resp.status().is_success(), + "impersonated call should succeed" + ); + + // Impersonation is the only route that reaches this identity — nobody + // holds a key for it — so if the header does not stamp it, nothing does. + await_last_active_after(&pool, sub_id, floor).await; +} + +#[tokio::test] +async fn impersonated_sub_agent_survives_the_idle_sweep() { + let (base, client, pool, org_id, admin_key, sa_id, _, target_agent_id) = setup().await; + let imp_key = create_impersonation_key(&base, &client, &admin_key, org_id, sa_id).await; + let sub_id = create_subagent(&base, &client, &admin_key, target_agent_id, "busy").await; + + sqlx::query("UPDATE orgs SET subagent_idle_timeout_secs = 60 WHERE id = $1") + .bind(org_id) + .execute(&pool) + .await + .unwrap(); + let floor: time::OffsetDateTime = sqlx::query_scalar( + "UPDATE identities SET last_active_at = now() - interval '2 hours' + WHERE id = $1 RETURNING last_active_at", + ) + .bind(sub_id) + .fetch_one(&pool) + .await + .unwrap(); + + let resp = client + .get(format!("{base}/v1/whoami")) + .header("Authorization", format!("Bearer {imp_key}")) + .header("X-Overslash-As", sub_id.to_string()) + .send() + .await + .unwrap(); + assert!(resp.status().is_success()); + await_last_active_after(&pool, sub_id, floor).await; + + overslash_db::repos::identity::archive_idle_subagents(&pool) + .await + .unwrap(); + + let row = overslash_db::repos::identity::get_by_id(&pool, org_id, sub_id) + .await + .unwrap() + .unwrap(); + assert!( + row.archived_at.is_none(), + "a sub-agent used this second must not be reaped as idle (reason: {:?})", + row.archived_reason + ); +}