Skip to content

refactor: dedupe revoke cascade triggered by lifecycle CAE signals #128

@rsharath

Description

@rsharath

Background

`IdentityService.runDeactivationCleanup` performs two redundant credential-revocation passes per identity deactivation:

  1. Direct revoke — `credentialSvc.RevokeAllActiveForIdentity(...)`
  2. Signal-cascade revoke — `signalSvc.IngestSignal(...)` at `SignalSeverityHigh`. `signal.go` auto-revokes on every High/Critical signal via `credRepo.RevokeAllActiveForIdentity`.

Both calls are idempotent (the underlying `RevokeAllActiveForIdentity` is filtered `WHERE is_revoked = false`), so there's no correctness bug. But every deactivation path (`UpdateIdentity` status transition, `DeleteIdentity`, the new `DeactivateExpired` from the time-bound authority work) writes two DELETE/UPDATE statements and two audit-log entries where one would suffice.

Surfaced during code review of #22 / PR for time-bound authority. Predates that PR — same pattern has been in place since the signal-cascade landed.

Why not just remove one path?

  • Remove the direct revoke: the signal cascade becomes the only revoke path. If signal emission fails (DB blip, signal service misconfig), credentials remain active after the identity is deactivated → fail-open regression.
  • Remove the cascade: lifecycle signals stop triggering the auto-revoke. Subscribers currently relying on the High-severity contract (e.g., audit pipelines that watch the cascade outcome) would silently lose that signal.

Proposed fix

Lower the severity for lifecycle signals from High → Medium. Lifecycle signals (`SignalTypeRetirement`, `SignalTypeIdentityExpired`) describe state transitions, not active threats. The signal cascade in `signal.go:75` was designed for threat indicators (anomalous behavior, IP change, credential change) where auto-revoke is the urgent response. For lifecycle signals, the direct revoke already handled it.

Drop to Medium:

  • Direct revoke remains the authoritative path (no fail-open regression)
  • Subscribers still see the signal (audit + fan-out unchanged)
  • Cascade auto-revoke skips, eliminating the duplicate write
  • Severity convention now matches the signal's actual semantics (informational, not threat)

Acceptance criteria

  • `runDeactivationCleanup` emits `SignalTypeRetirement` / `SignalTypeIdentityExpired` at `SignalSeverityMedium`
  • Integration test confirms exactly one revoke per deactivation (assert audit-log row count + revoke count)
  • Existing tests still pass (none assert on signal severity for lifecycle paths, but verify)
  • Release notes call out the severity change so subscribers know to update filters if they were keying on High

Risk

  • Behavioral change for any custom subscriber filtering signals on severity. The signal_type is unchanged, so consumers keying on `type = retirement` or `type = identity_expired` are unaffected.
  • Should land in its own PR, not bundled with feature work.

References

  • Discussion: review of PR for feat: Add GetIdentity function #22 in highflame-architecture
  • Code: `internal/service/identity.go:runDeactivationCleanup`, `internal/service/signal.go:75-96`

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions