Break-glass records custody but not conduct: config changes made while elevated are never audited

Problem/Motivation

Dual-venue record for GitHub #94. Found while reviewing the 2.2.0 break-glass seal — the seal itself is sound; this is the gap it leaves.

The break-glass mechanism proves who held the role and for how long. It does not record what they did with it.

McpBreakGlassManager audits mcp_admin_granted and mcp_admin_revoked, and the grants screen shows current holders. That is custody. But actions taken while elevated produce no audit rows, because of this gate in McpConfigSaveSubscriber::onConfigSave():

// Only governed (policy-resolved agent) requests are subject to this gate.
// Non-agent traffic (cron, drush deploys, human admins) is never touched.
if (!$this->policyResolver->isGoverned()) {
  return;
}

McpPolicyResolver::isGoverned() keys on the OAuth agent channel. A break-glass holder is a human on a cookie session, so they are never governed, so nothing they change is audited.

Why this is not simply "admins are ungoverned by design"

That early return is correct for ordinary administrators, and the resolver documents it deliberately: an admin's cookie-session UI stays ungoverned while their token-bearing agent traffic is governed and audited.

Break-glass is built on the opposite premise. It is time-boxed, approval-gated, recorded as an mcp_admin_grant entity, surfaced on a "who holds break-glass right now" screen, and reaped on cron. Every one of those exists to make the elevation accountable — and then the elevation itself is the one path where nothing is recorded.

Blast radius

The role's operative permission is administer mcp sentinel, which is the editor for both mcp_sentinel.settings and audit_chain.settings. While elevated, with no audit row produced, a holder can set:

• audit_enabled — turns the audit log off entirely
• enabled — turns governance off
• audit_hash_key — repoints the audit chain's signing key
• audit_retention_days — shrinks retention so existing history prunes on cron
• audit_encryption_profile — orphans metadata readability for prior rows
• governed_roles, agent_scopes, agent_oauth_clients — removes what is governed at all
• webhook_secret_key, dlp_enabled — repoints webhook signing, disables PII scanning

audit_enabled is the sharpest: switching off the audit log is itself unaudited. So is enabled. Both are self-concealing — the action removes the evidence of the action, and no prior record says it happened.

This is not an argument for removing administer mcp sentinel from the role. Without it, break-glass can look but not fix, which makes it a diagnostic role rather than a break-glass role. The capability is right; the silence is not.

Proposed resolution

Audit configuration changes while a break-glass grant is active, regardless of channel.

Deliberately narrow: the path engages only when mcp_admin_grant has a live row (not revoked, not expired — the same predicate reapExpired() uses, and mindful of the SQLite boolean binding documented on McpBreakGlassManager::NOT_REVOKED). Ordinary admin traffic, cron and drush deploys stay untouched, so the deliberate ungoverned-admin design is preserved.

• Extend the ConfigEvents::SAVE listener so that when isGoverned() is false it still checks for an active grant before returning.
• When one exists and the saving account holds mcp_admin, write an audit row — suggested operation config_save_break_glass, with metadata naming the config object, the changed keys, the grant id and the acting uid.
• Record changed key names, not values. Values would put webhook_secret_key and audit_hash_key references into the log, and an audit trail should not become somewhere secrets accumulate.

Points that need deciding

• Scope: all config, or only governance-relevant objects? All config cannot be gamed by making the change somewhere unexpected; a prefix allowlist is cheaper and quieter. Leaning toward all config while elevated — the grant is time-boxed and rare, so volume is bounded by construction, and an exhaustive record is the point.
• Non-config actions. This covers configuration only; content and entity operations performed while elevated are still unrecorded. Decide whether that is in scope now or a follow-up.
• Failure mode. If the audit write fails while a grant is active, does the save proceed? Consistent with the module's doctrine elsewhere it should fail closed — refuse rather than let a privileged change through unrecorded.

Acceptance criteria

• A config save made while a grant is active produces an audit row naming the object, changed keys, grant id and acting uid.
• Setting audit_enabled: false while elevated is itself audited — the self-concealing case covered explicitly by a test.
• An identical save with no active grant produces no row, so the ungoverned-admin design is unchanged.
• Values are not recorded; only key names.
• The active-grant check uses the integer-bound predicate rather than FALSE — the same bug silently disabled the reaper on SQLite.
• Documented in the approval submodule README as part of what break-glass guarantees.

Related

The 2.2.0 seal (allowlist + status-report drift) is what this does not cover. #3614165 is adjacent but distinct: that is about the role drifting under a live grant, this is about conduct during a perfectly valid one.