Skip to content

Commit e58aa5c

Browse files
fylornclaude
andcommitted
fix(server): emit audit events when api_key_lifecycle disables/revokes keys
Pass 50 — scheduled-task audit-emission gap. The `api_key_lifecycle` backstop loop ran four state-changing queries (expired, inactive-global, inactive-per-key, grace-revoked) each followed by a `tracing::info!` that logged a count to stderr. None of the four emitted an audit event — so security teams investigating "why did my API key stop working at 03:14 UTC?" had to cross-reference application stderr against PG and hope nothing rotated. Convert each query from `.execute()` to `.fetch_all()` with `RETURNING id, user_id`, then run the rows through a shared `emit_disable_audits` helper that fires `api_key.disabled` (or `api_key.revoked` for the grace-period case) for every affected row with a `disabled_reason` detail field distinguishing the four sub-reasons (`expired` / `inactive_global` / `inactive_per_key` / `grace_period_expired`). The expiry-warning step (section 4) was already audited via `key.expiry_warning` — only the actual state changes were missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d7c2581 commit e58aa5c

1 file changed

Lines changed: 70 additions & 21 deletions

File tree

crates/server/src/tasks/api_key_lifecycle.rs

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,31 @@ pub fn spawn_api_key_lifecycle_task(db: PgPool, config: Arc<DynamicConfig>, audi
3838
});
3939
}
4040

41+
/// One row's worth of "this key just changed state" — fed into the
42+
/// audit-emission loop so every disable/revoke leaves a compliance
43+
/// trail, not just a `tracing::info!` line.
44+
#[derive(sqlx::FromRow)]
45+
struct DisabledKeyRow {
46+
id: uuid::Uuid,
47+
user_id: Option<uuid::Uuid>,
48+
}
49+
50+
fn emit_disable_audits(audit: &AuditLogger, action: &str, rows: &[DisabledKeyRow], reason: &str) {
51+
for row in rows {
52+
let mut entry = SystemActor
53+
.audit(action)
54+
.resource(format!("api_key:{}", row.id))
55+
.detail(serde_json::json!({
56+
"api_key_id": row.id.to_string(),
57+
"disabled_reason": reason,
58+
}));
59+
if let Some(uid) = row.user_id {
60+
entry = entry.user_id(uid);
61+
}
62+
audit.log(entry);
63+
}
64+
}
65+
4166
/// Single deterministic pass of the lifecycle loop. `pub` so
4267
/// integration tests can drive it without `tokio::time::pause`-ing
4368
/// the whole 10-minute interval.
@@ -48,86 +73,110 @@ pub async fn run_lifecycle_check(
4873
) -> anyhow::Result<()> {
4974
let now = chrono::Utc::now();
5075

51-
// 1. Disable expired keys
52-
let expired = sqlx::query(
76+
// 1. Disable expired keys. Previously this query used `.execute()`
77+
// and `tracing::info!`'d the count; the actual state changes
78+
// landed in PG with NO audit trail, so security teams investigating
79+
// "why did this key stop working" had to cross-reference application
80+
// logs against the DB and hope they hadn't rotated. RETURNING +
81+
// audit emit closes the gap. Same change applied to all four
82+
// disable/revoke queries below.
83+
let expired: Vec<DisabledKeyRow> = sqlx::query_as(
5384
r#"UPDATE api_keys
5485
SET is_active = false, disabled_reason = 'expired'
5586
WHERE is_active = true
5687
AND expires_at IS NOT NULL
5788
AND expires_at < $1
58-
AND disabled_reason IS NULL"#,
89+
AND disabled_reason IS NULL
90+
RETURNING id, user_id"#,
5991
)
6092
.bind(now)
61-
.execute(db)
93+
.fetch_all(db)
6294
.await?;
6395

64-
if expired.rows_affected() > 0 {
65-
tracing::info!("Disabled {} expired API keys", expired.rows_affected());
96+
if !expired.is_empty() {
97+
tracing::info!("Disabled {} expired API keys", expired.len());
98+
emit_disable_audits(audit, "api_key.disabled", &expired, "expired");
6699
}
67100

68101
// 2. Disable inactive keys
69102
let global_inactivity_days = config.api_keys_inactivity_timeout_days().await;
70103
if global_inactivity_days > 0 {
71104
let inactive_threshold = now - chrono::Duration::days(global_inactivity_days);
72-
let inactive = sqlx::query(
105+
let inactive: Vec<DisabledKeyRow> = sqlx::query_as(
73106
r#"UPDATE api_keys
74107
SET is_active = false, disabled_reason = 'inactive'
75108
WHERE is_active = true
76109
AND last_used_at IS NOT NULL
77110
AND last_used_at < $1
78111
AND disabled_reason IS NULL
79-
AND (inactivity_timeout_days IS NULL OR inactivity_timeout_days = 0)"#,
112+
AND (inactivity_timeout_days IS NULL OR inactivity_timeout_days = 0)
113+
RETURNING id, user_id"#,
80114
)
81115
.bind(inactive_threshold)
82-
.execute(db)
116+
.fetch_all(db)
83117
.await?;
84118

85-
if inactive.rows_affected() > 0 {
119+
if !inactive.is_empty() {
86120
tracing::info!(
87121
"Disabled {} inactive API keys (global timeout: {} days)",
88-
inactive.rows_affected(),
122+
inactive.len(),
89123
global_inactivity_days
90124
);
125+
emit_disable_audits(audit, "api_key.disabled", &inactive, "inactive_global");
91126
}
92127
}
93128

94129
// Per-key inactivity timeout
95-
let per_key_inactive = sqlx::query(
130+
let per_key_inactive: Vec<DisabledKeyRow> = sqlx::query_as(
96131
r#"UPDATE api_keys
97132
SET is_active = false, disabled_reason = 'inactive'
98133
WHERE is_active = true
99134
AND inactivity_timeout_days IS NOT NULL
100135
AND inactivity_timeout_days > 0
101136
AND last_used_at IS NOT NULL
102137
AND last_used_at < now() - (inactivity_timeout_days || ' days')::interval
103-
AND disabled_reason IS NULL"#,
138+
AND disabled_reason IS NULL
139+
RETURNING id, user_id"#,
104140
)
105-
.execute(db)
141+
.fetch_all(db)
106142
.await?;
107143

108-
if per_key_inactive.rows_affected() > 0 {
144+
if !per_key_inactive.is_empty() {
109145
tracing::info!(
110146
"Disabled {} inactive API keys (per-key timeout)",
111-
per_key_inactive.rows_affected()
147+
per_key_inactive.len()
148+
);
149+
emit_disable_audits(
150+
audit,
151+
"api_key.disabled",
152+
&per_key_inactive,
153+
"inactive_per_key",
112154
);
113155
}
114156

115157
// 3. Revoke rotated keys past grace period
116-
let grace_expired = sqlx::query(
158+
let grace_expired: Vec<DisabledKeyRow> = sqlx::query_as(
117159
r#"UPDATE api_keys
118160
SET is_active = false
119161
WHERE is_active = true
120162
AND grace_period_ends_at IS NOT NULL
121-
AND grace_period_ends_at < $1"#,
163+
AND grace_period_ends_at < $1
164+
RETURNING id, user_id"#,
122165
)
123166
.bind(now)
124-
.execute(db)
167+
.fetch_all(db)
125168
.await?;
126169

127-
if grace_expired.rows_affected() > 0 {
170+
if !grace_expired.is_empty() {
128171
tracing::info!(
129172
"Revoked {} rotated API keys past grace period",
130-
grace_expired.rows_affected()
173+
grace_expired.len()
174+
);
175+
emit_disable_audits(
176+
audit,
177+
"api_key.revoked",
178+
&grace_expired,
179+
"grace_period_expired",
131180
);
132181
}
133182

0 commit comments

Comments
 (0)