SafetyOverride.renew() extends a grant before auditing it, and swallows audit failure

[chenmingwei23]

What

SafetyOverride.renew() — the human-initiated renewal of a time-limited safety override (YOLO) grant — commits the extended deadline and then writes its SEL audit event non-critically. If the audit write fails, the failure is swallowed (_log_sel only re-raises when critical=True) and auto-approval stays extended with no record that it was.

Why

Every other path in this file that creates auto-approval authority already fails closed the other way round:

• _commit_activation audits with critical=True before committing and returns an inactive result if the write fails. Its comment: "Audit BEFORE committing — fail-closed with no race window".
• activate_scoped does the same: "Fail-closed audit before commit — no grant without a trace."

renew() extends the same authority those paths are careful about, so the asymmetry looks unintentional rather than reasoned — an auditor reconstructing "when was auto-approval live?" from the SEL log can be silently missing a renewal that happened.

This was surfaced while reviewing #2443, which added renew_lease() (the machine-issued counterpart). That method was flagged for exactly this weakness and now audits critically before committing, with the commit block re-verifying the guard because the audit runs with the lock released. renew() was deliberately left alone there to keep a focused change reviewable — hence this issue.

Additional Context

• src/kiro_crew/safety_override.py — renew(); compare _commit_activation and activate_scoped for the established pattern, and renew_lease() (post-fix(safety-override): lease the grant to armed loops instead of demoting mid-run #2443) for a worked example including the post-audit re-verify.
• Note the lock discipline that matters here: the SEL write must not happen while holding _lock, so moving the audit before the commit opens a window in which a concurrent deactivate() could be undone. renew_lease() handles this by re-checking _expires_at <= 0 inside the commit lock; renew() would need the same.
• Suggested test shape (mutation-verifiable): patch the SEL sink so log_api_access raises, call renew(), assert it reports not-renewed AND that the grant's deadline did not move. Injecting the failure at the SEL write rather than at _log_sel is what makes the critical=True flag load-bearing in the test — patching _log_sel itself raises regardless of the flag and passes vacuously.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97]: Confirmed against main (10bba9f). SafetyOverride.renew() at src/kiro_crew/safety_override.py:360 extends _expires_at inside the lock and only then writes its safety_override:renew SEL event non-critically, so a failed audit write leaves auto-approval extended with no trace. Both sibling paths that mint the same authority do the opposite: _commit_activation (line 294) and activate_scoped (line 432) audit with critical=True and refuse the grant if the write fails. The asymmetry looks unintentional. Fix: audit critically before committing the new deadline, with the SEL write performed outside _lock, and re-verify the guard (_active/_expires_at) inside the commit lock so a concurrent deactivate() cannot be undone through the released-lock window — and return not-renewed with the deadline unmoved when the audit fails. Note renew_lease() is cited in the issue as the worked example but is not on main yet (it arrives with open PR #2443, which touches this same file), so this lands independently and will need a rebase. Starting an automated fix — this issue is now claimed by bolichen97's instance.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97]

Fix opened: #2484

renew() now audits fail-closed with critical=True BEFORE moving the deadline, with a post-audit re-verify under the re-acquired lock so a concurrent deactivate() during the audit window is not undone. New tests patch the SEL sink (not _log_sel) and fail on unfixed code. Scoped strictly to renew() per the coordination notes (#2443 / #2475 untouched).