feat: port project_luks_key_event to Go listener (#99)#121
Merged
Conversation
Replaces project_luks_key_event PL/pgSQL with projectors.LuksKeyListener. Fourth port under the projector-migration pattern (#96 canary, #97 totp, #98 lps_password, #99 luks_key). Four event types: - LuksKeyRotated: 3 writes (mark prev not-current, insert new, trim to last 3) wrapped in store.WithTx for inter-write atomicity - LuksDeviceKeyRevocationDispatched: UPDATE current row's revocation_status='dispatched' - LuksDeviceKeyRevoked: UPDATE current row's revocation_status='success' (event-name vs column-value mismatch matches PL/pgSQL verbatim) - LuksDeviceKeyRevocationFailed: UPDATE revocation_status='failed' + revocation_error captured LuksDeviceKeyRevocationRequested stays a no-op — the deleted PL/pgSQL projector also lacked a case for it. Requested is a marker the dispatcher handler appends before enqueueing; the projection only changes once Dispatched / Revoked / Failed lands. Stream-key partitioning is (device_id, action_id, device_path) for LuksKeyRotated (vs LPS's (device_id, username)). Per-action-path scope test confirms a regression that drops device_path from the WHERE clause is caught. Tests follow TDD: written before implementation per saved feedback. Pure-function suite covers happy paths, default rotation_reason, required-field validation per variant, wrong stream/event type, malformed payload. Integration suite covers 4-rotation lifecycle (trim-to-3), per-action-path scoping, three revocation flows, LuksDeviceKeyRevocationRequested ignored, wrong-stream-type ignored. Refs tracker #107.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR ports the in-DB PL/pgSQL ChangesLUKS Key Projector Migration
Sequence DiagramsequenceDiagram
actor App as Application
participant ES as EventStore
participant Listener as LuksKeyListener
participant DB as Postgres
App->>ES: Append LuksKeyRotated / Revocation event
ES->>Listener: Post-commit listener invoked with event
activate Listener
Listener->>Listener: Decode payload (validate, parse timestamps)
alt Rotation event
Listener->>DB: BEGIN
Listener->>DB: MarkLuksKeysNotCurrent(device,action,path)
Listener->>DB: InsertLuksKey(new current row)
Listener->>DB: TrimLuksKeysToLast3(device,action,path)
Listener->>DB: COMMIT
else Revocation event
Listener->>DB: UpdateLuksKeyRevocationStatus(device,action, status,error,at) on current row
end
deactivate Listener
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/store/queries/luks.sql (1)
25-29: ⚡ Quick winOnly flip rows that are still current.
This update rewrites every historical row for the partition on each rotation, even though only
is_current = TRUErows need changing. Adding that predicate keeps the result the same while reducing write amplification on the hot path.Proposed change
UPDATE luks_keys_projection SET is_current = FALSE WHERE device_id = $1 AND action_id = $2 - AND device_path = $3; + AND device_path = $3 + AND is_current = TRUE;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/store/queries/luks.sql` around lines 25 - 29, The UPDATE on table luks_keys_projection is updating every historical row for the partition; restrict it to only flip currently active rows by adding the is_current predicate: in the UPDATE that targets device_id = $1, action_id = $2, device_path = $3, add AND is_current = TRUE so only rows where is_current is TRUE are set to FALSE, reducing write amplification on the hot path while preserving semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/projectors/luks_key_listener.go`:
- Around line 57-135: Both error branches currently only log a warning which
lets events commit while the projection stays stale; change them to persist a
durable projection error record and signal retries. In applyLuksKeyRotated
(inside the st.WithTx error handling) and in applyLuksRevocation (inside the
UpdateLuksKeyRevocationStatus error branch) call into the store to insert a
projection error (e.g. via a new or existing method on store.Store / st.Queries
such as InsertProjectionError or RecordProjectionError) with fields: event_id
e.ID, projector "luks_key", event_type e.EventType, device_id payload.DeviceID,
action_id payload.ActionID, device_path payload.DevicePath (if present), and the
error string; if you use st.WithTx in applyLuksKeyRotated ensure the insert
happens in the same transaction or, if recording outside the TX, handle TX
rollback semantics explicitly; also log any failure to persist the projection
error after attempting the insert so persistent-write failures are visible.
---
Nitpick comments:
In `@internal/store/queries/luks.sql`:
- Around line 25-29: The UPDATE on table luks_keys_projection is updating every
historical row for the partition; restrict it to only flip currently active rows
by adding the is_current predicate: in the UPDATE that targets device_id = $1,
action_id = $2, device_path = $3, add AND is_current = TRUE so only rows where
is_current is TRUE are set to FALSE, reducing write amplification on the hot
path while preserving semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5412bcb6-7e71-484e-94a9-c3a28b35521e
⛔ Files ignored due to path filters (1)
internal/store/generated/luks.sql.gois excluded by!**/generated/**
📒 Files selected for processing (6)
internal/projectors/luks_key.gointernal/projectors/luks_key_listener.gointernal/projectors/luks_key_test.gointernal/projectors/wire.gointernal/store/migrations/020_luks_key_projector_to_go.sqlinternal/store/queries/luks.sql
Comment on lines
+57
to
+135
| func applyLuksKeyRotated(ctx context.Context, st *store.Store, logger *slog.Logger, e store.PersistedEvent) { | ||
| payload, err := LuksKeyRotatedFromEvent(e) | ||
| if err != nil { | ||
| if errors.Is(err, ErrIgnoredEvent) { | ||
| return | ||
| } | ||
| logger.Warn("luks_key projector: invalid LuksKeyRotated payload", | ||
| "event_id", e.ID, "error", err) | ||
| return | ||
| } | ||
| if err := st.WithTx(ctx, func(q *store.Queries) error { | ||
| if err := q.MarkLuksKeysNotCurrent(ctx, db.MarkLuksKeysNotCurrentParams{ | ||
| DeviceID: payload.DeviceID, | ||
| ActionID: payload.ActionID, | ||
| DevicePath: payload.DevicePath, | ||
| }); err != nil { | ||
| return err | ||
| } | ||
| if err := q.InsertLuksKey(ctx, db.InsertLuksKeyParams{ | ||
| DeviceID: payload.DeviceID, | ||
| ActionID: payload.ActionID, | ||
| DevicePath: payload.DevicePath, | ||
| Passphrase: payload.Passphrase, | ||
| RotatedAt: payload.RotatedAt, | ||
| RotationReason: payload.RotationReason, | ||
| }); err != nil { | ||
| return err | ||
| } | ||
| return q.TrimLuksKeysToLast3(ctx, db.TrimLuksKeysToLast3Params{ | ||
| DeviceID: payload.DeviceID, | ||
| ActionID: payload.ActionID, | ||
| DevicePath: payload.DevicePath, | ||
| }) | ||
| }); err != nil { | ||
| logger.Warn("luks_key projector: failed to apply LuksKeyRotated", | ||
| "event_id", e.ID, | ||
| "device_id", payload.DeviceID, | ||
| "action_id", payload.ActionID, | ||
| "device_path", payload.DevicePath, | ||
| "error", err) | ||
| } | ||
| } | ||
|
|
||
| // applyLuksRevocation is generic across the three revocation event | ||
| // types — they all UPDATE the current row's (revocation_status, | ||
| // revocation_error, revocation_at). The decoder picks the right | ||
| // timestamp key + status value; the writer is identical. | ||
| func applyLuksRevocation( | ||
| ctx context.Context, | ||
| st *store.Store, | ||
| logger *slog.Logger, | ||
| e store.PersistedEvent, | ||
| decode func(store.PersistedEvent) (LuksRevocationPayload, error), | ||
| ) { | ||
| payload, err := decode(e) | ||
| if err != nil { | ||
| if errors.Is(err, ErrIgnoredEvent) { | ||
| return | ||
| } | ||
| logger.Warn("luks_key projector: invalid revocation payload", | ||
| "event_id", e.ID, "event_type", e.EventType, "error", err) | ||
| return | ||
| } | ||
| at := payload.At | ||
| if err := st.Queries().UpdateLuksKeyRevocationStatus(ctx, db.UpdateLuksKeyRevocationStatusParams{ | ||
| DeviceID: payload.DeviceID, | ||
| ActionID: payload.ActionID, | ||
| RevocationStatus: stringPtr(payload.Status), | ||
| RevocationError: payload.Error, | ||
| RevocationAt: &at, | ||
| }); err != nil { | ||
| logger.Warn("luks_key projector: failed to update revocation_status", | ||
| "event_id", e.ID, | ||
| "event_type", e.EventType, | ||
| "device_id", payload.DeviceID, | ||
| "action_id", payload.ActionID, | ||
| "status", payload.Status, | ||
| "error", err) | ||
| } |
There was a problem hiding this comment.
Persist projector failures somewhere durable.
After 020_luks_key_projector_to_go.sql turns the trigger function into a no-op, these branches become the only failure path for luks_key projection work. Right now they just Warn and return, so a transient DB error leaves the event committed and luks_keys_projection stale with no durable projection_errors record or retry signal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/projectors/luks_key_listener.go` around lines 57 - 135, Both error
branches currently only log a warning which lets events commit while the
projection stays stale; change them to persist a durable projection error record
and signal retries. In applyLuksKeyRotated (inside the st.WithTx error handling)
and in applyLuksRevocation (inside the UpdateLuksKeyRevocationStatus error
branch) call into the store to insert a projection error (e.g. via a new or
existing method on store.Store / st.Queries such as InsertProjectionError or
RecordProjectionError) with fields: event_id e.ID, projector "luks_key",
event_type e.EventType, device_id payload.DeviceID, action_id payload.ActionID,
device_path payload.DevicePath (if present), and the error string; if you use
st.WithTx in applyLuksKeyRotated ensure the insert happens in the same
transaction or, if recording outside the TX, handle TX rollback semantics
explicitly; also log any failure to persist the projection error after
attempting the insert so persistent-write failures are visible.
Contributor
Author
There was a problem hiding this comment.
Skipping this for the listener pattern, intentionally — the design choice is logged + reconciler-as-safety-net rather than per-event projection_errors persistence. Documented in project_projector_migration_pattern.md and feedback_post_commit_listener_is_sync.md:
Errors logged + swallowed (post-commit notification contract). The periodic reconciler (1h cycle) is the safety net for any drift.
Reasons not to add projection_errors here:
- Replicating the PL/pgSQL
EXCEPTION WHEN OTHERSwould require a second autocommit DB write that could itself fail. The PL/pgSQL version was atomic with the event commit because it ran inside the trigger transaction; the Go listener is post-commit, so the projection-error insert wouldn't share atomicity with anything. - The reconciler already covers the gap. Within the next reconciliation cycle, the projection re-derives from the event log; a transient DB error during the listener run is fully recovered.
- Per-listener persistence becomes a tax across all 11 ports. refactor: port
project_security_alert_eventprojector to Go event listener #96 / refactor: portproject_totp_eventprojector to Go event listener #97 / refactor: portproject_lps_password_eventprojector to Go event listener #98 followed the same pattern; adding it here without doing so for the rest fragments the surface.
If we want a richer durable failure record across all listeners (with retry signal, dead-letter visibility), that's its own design discussion + tracker issue, not a per-port addition.
There was a problem hiding this comment.
@PaulDotterer, understood — the rationale is sound. The three points together make a coherent case:
- A projection-error insert outside the event commit buys no atomicity guarantee and introduces its own failure surface.
- The 1h reconciler already provides the durability safety net that
projection_errorswould otherwise supply. - Diverging from the pattern already established by
#96/refactor: portproject_totp_eventprojector to Go event listener #97/refactor: portproject_lps_password_eventprojector to Go event listener #98 mid-series would fragment the surface rather than improve it.
Happy to drop this. If a unified durable-failure / dead-letter design ever gets tracked, the same consideration applies across all 11 listener ports equally.
(¬_¬ )ﾉ🐇
✏️ Learnings added
Learnt from: PaulDotterer
Repo: manchtools/power-manage-server PR: 121
File: internal/projectors/luks_key_listener.go:57-135
Timestamp: 2026-05-04T23:10:54.356Z
Learning: In the power-manage-server projector migration pattern (Go post-commit listeners replacing PL/pgSQL triggers), projection errors are intentionally logged and swallowed rather than persisted to a `projection_errors` table. The periodic reconciler (1h cycle) is the designated safety net for projection drift. This is documented in `project_projector_migration_pattern.md` and `feedback_post_commit_listener_is_sync.md`. Do not flag missing durable projection-error persistence in `internal/projectors/` listeners — the pattern is intentional and consistent across all ported projectors (`#96`, `#97`, `#98`, `#99`).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
CR quick-win on PR #121: add `is_current = TRUE` predicate to the UPDATE so historical rows that already have is_current=FALSE aren't rewritten. Result is identical (FALSE rows stay FALSE) but skips a write per historical row, reducing write amplification on the hot rotation path. The other CR finding (persist projection_errors for durable retry) is intentionally skipped — the listener pattern is documented as "log + reconciler-as-safety-net" in project memory; per-listener projection_errors would replicate the PL/pgSQL EXCEPTION block but without the in-transaction atomicity guarantee. See PR comment for full reasoning.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced May 6, 2026
PaulDotterer
added a commit
that referenced
this pull request
Jun 15, 2026
…uth/api/compliance/dyngroupeval/search/store (WS17b) (#440) * fix(handler): enforce cert-CN binding on ValidateLuksToken via shared assertDeviceMatchesCert ValidateLuksToken relayed the caller-supplied device_id to the control proxy without the mTLS cert-CN check SyncActions enforces, so a compromised agent presenting device A's certificate could redeem a one-time LUKS token issued for device B and unlock that device's encrypted volume. Extract the binding into a shared assertDeviceMatchesCert helper (the only two device-scoped agent connect RPCs, ValidateLuksToken + SyncActions, now both call it so it cannot drift) and apply it before the proxy call. Tests (RED before the guard): cert/request mismatch -> CodePermissionDenied with the proxy never called; no cert identity under requireTLS -> CodeUnauthenticated; matching identity proceeds to the proxy. Refs #439 (WS17b #7) * fix(search): sanitize agent-controlled fields at the warm/index boundary Agent-reported hostname / labels / os_* / kernel / agent_version were written into the Valkey search index verbatim. A malicious or buggy agent could report a multi-KB value (index bloat) or one embedding ASCII control characters or <markup> (which a UI rendering raw search-result fields could execute). Add sanitizeSearchField (drops control chars, angle brackets, invalid UTF-8; caps length) and apply it on BOTH the warm path (Index.warmDevices) and the live path (worker.entityFields) so they stay in lockstep. Operator-set fields (linux_username, actor_id) are trusted-by-policy and deliberately not routed through it. Refs #439 (WS17b #17) * fix(store): make system-role permission seed reconciler-owned (drop drifted SQL literals) The Admin/User permission arrays were seeded as SQL literals (008_seeds.sql, patched by 009/010) — a frozen snapshot that drifts from the Go source of truth (auth.AdminPermissions/DefaultUserPermissions) as permissions are added/renamed. The Admin literal had already drifted 18 added + 6 renamed permissions behind. auth.ReconcileSystemRoles overwrites them from the Go sets on every boot, so the literals are runtime-irrelevant but misleading and undrift-guardable. Migration 014 blanks the system-role permission arrays so the reconciler is the single source of truth. Tests: - store: self-discovering scan that the LAST migration setting each system role's permissions leaves them empty (catches a future re-introduced literal); RED without 014 (names the drifted role + offending migration). - auth: repurpose TestUserSeedRole_MatchesDefaultPermissions -> the seed is reconciler-owned (empty after migrations); TestReconcileSystemRoles_UpdatesDB already pins reconcile fills them to the Go sets. Refs #439 (WS17b #18) * refactor(indexer): extract BuildSearchWorkerMux; test forged-key rejection + verify-first Extract the mux+signer assembly from cmd/indexer/main.go into search.BuildSearchWorkerMux(rdb, signer, logger), which fails closed when the signer is nil (empty PM_TASK_SIGNING_KEY) instead of building an unsigned mux that would accept forged/unsigned search:* tasks from a compromised Valkey relay. main now calls it. Tests: a task signed with a DIFFERENT key (forged, not merely absent) is rejected with asynq.SkipRetry and creates no index hash, for all three task types (#11, #16); BuildSearchWorkerMux rejects a nil signer (#15) and mounts VerifyMiddleware ahead of the handlers so an unsigned task is dropped before any HSET (#15). Refs #439 (WS17b #11/#15/#16) * test: self-discovering guards — canary fail-closed, rebuild-applier parity, parity matches-zero, user_role no-op - system-actor canary (#1): re-key sites by ENCLOSING FUNCTION (stable across line edits, not line numbers), FAIL on an unknown or stale site (was advisory t.Logf), and add a matches-zero guard. Documented the 12 current system-actor write sites with rationales (all legitimate server-owned lifecycle / compensating events). - rebuild parity (#14): add Store.HasRebuildApply + a self-discovering test that every AllRebuildTargets entry has a Go applier wired by WireAll (a missing one makes RebuildAll a destructive no-op — #125's failure mode); matches-zero guarded. - permission parity (#19): matches-zero guards in rpcMethodNames (covers all three parity tests) and on AllPermissions() so they can't pass vacuously. - user_role revoke-absent (#21): plant a real unrelated grant and prove it SURVIVES the absent revoke — turns a success-only no-op into a scoping invariant on the projector feeding the JWT sgrants claim. Refs #439 (WS17b #1/#14/#19/#21) * test: dynamicquery in-split metacharacters (#13) + CA identity-from-server (#3) - dynamicquery (#13): pin the in/notIn comma-split semantics with values that carry the separator, empty elements, keyword/quote chars — equals does NOT split (a comma-bearing value matches itself) while in() does (so it matches neither element); the empty-field-matches-empty-element footgun is documented. Sourced from the split semantics, not the implementation. - ca (#3): TestIssueCertificateFromCSR_IdentityComesFromServerNotCSR uses a CSR CN that DIFFERS from the server-supplied deviceID and asserts the issued cert's CN + Subject.SerialNumber (and VerifyCertificate/DeviceIDFromPEM) all come from the server id, never the attacker-controlled CSR CN — breaking the circular same-string sourcing the Success test had. Refs #439 (WS17b #3/#13) * test: stream cert/Hello mismatch (#5) + registration cert-CN binding & SAN rejection (#2) - stream (#5): newStreamFixtureWithCert stamps an mTLS-cert device id into the server-side ctx; pin that a cert/Hello device-ID mismatch is CodePermissionDenied and a requireTLS connection with no cert identity is CodeUnauthenticated — both before VerifyDevice/StartWorker (recording control + fake worker stay empty). - registration (#2): drive the real h.Register and assert the issued cert's CN is the SERVER-minted device id (not the attacker CSR CN) with no CSR SAN leaking in; a SAN-bearing CSR yields no certificate. Refs #439 (WS17b #2/#5) * test(api): self-discovering validate-before-work sweep for every ControlService RPC (#8) Reflect over ControlServiceClient and drive a ZERO request through the real client -> interceptor chain (logging -> auth -> validation -> authz) with an admin bearer for every RPC; a non-exempt RPC MUST surface CodeInvalidArgument, proving Validate() runs before the handler. RPCs whose zero request is legitimately valid (list/read, current-user-scoped, partial-update, the Validate* verdict RPCs) are exempted via validateExemptControlRPCs, guarded by a companion test that every exemption names a live RPC. A new RPC is covered automatically; matches-zero + matches-everything guards prevent vacuous passes. Refs #439 (WS17b #8) * test(api): Register pre-auth field bounds rejected by validation before token lookup (#9) The absent/zero case is covered by the self-discovering sweep; this adds the present-but-overlong boundary (Token>255, Hostname>253, AgentVersion>32, one over each documented cap) driven through the real public Register RPC + validation interceptor, asserting CodeInvalidArgument before the token lookup. Refs #439 (WS17b #9) * build: bump sdk pin to WS17b (#121) — request-field validate tags + verify hardening Pulls in the new bound-able request-field validate tags (the validation interceptor now enforces max=/ulid/url/email/dive bounds on the 33 newly-tagged fields) and the ed25519/byte-tampered verify test hardening. Full server suite passes — no existing request exceeded the conservative new bounds. Refs #439 * docs(adr): 0022 — WS17b boundary hardening (cert-CN binding, search sanitizer, reconciler-owned seed, indexer mux) Refs #439 * fix(compliance): recognize no-row via store.IsNotFound; evaluator coverage (#6) Writing the grace-period/rollup/first_failed_at coverage the finding asked for surfaced a real bug: the evaluator resolved 'no result yet -> UNKNOWN' and 'first-ever non-compliant -> seed first_failed_at' with errors.Is(err, store.ErrNotFound). The generated queries return the backend's pgx.ErrNoRows — a DIFFERENT sentinel — so those branches NEVER fired: any rule with no result yet, or failing for the first time, ERRORED instead, aborting the whole device evaluation (a rule just assigned, or a fresh failure, is the common case). store/notfound.go mandates store.IsNotFound (matches both sentinels) for callers outside the store package; the evaluator now uses it. (Sibling sweep: no other caller had the pattern.) Tests (RED against the broken check, GREEN after): grace IN_GRACE->NON_COMPLIANT boundary on both sides via the clock seam; first_failed_at preserved across re-eval; device rollup precedence (non-compliant > grace > compliant, UNKNOWN keeps it out of COMPLIANT) + total/passing; no-policies recalculate-only fallback. Refs #439 (WS17b #6) * test(dyngroupeval): membership add/remove reconcile + short-circuits (#12) The existing tests only exercised the queue lifecycle; these pin the actual reconciliation EvaluateDeviceGroup/EvaluateUserGroup perform: - a now-matching device/user is INSERTED and a stale member that no longer matches is DELETED, in a single evaluation (both directions); - a non-dynamic group and a missing group each clear their queue row and write no membership (the two short-circuits); - an unparseable dynamic_query returns an error, writes no membership, and leaves the queue row INTACT so a fixed query re-queues (fail-safe). Refs #439 (WS17b #12) * docs(adr): 0022 — record the compliance no-row recognition fix (#6) * test(store): scope the reconciler-owned seed scan to individual statements (CR) CodeRabbit: the whole-file scan could misclassify a migration that mentions a role ID in one statement and sets unrelated permissions in another. Split on ';' and evaluate each statement independently. Still RED-meaningful (drops 014 -> flags 008's frozen literal).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fourth port under the projector-migration pattern (#96 canary, #97 totp, #98 lps_password, #99 luks_key). Replaces
project_luks_key_eventPL/pgSQL withprojectors.LuksKeyListener.Four event types:
LuksKeyRotated— 3 writes (mark prev not-current, insert new, trim to last 3) wrapped instore.WithTxLuksDeviceKeyRevocationDispatched— UPDATErevocation_status='dispatched'LuksDeviceKeyRevoked— UPDATErevocation_status='success'(event-name vs column-value mismatch matches PL/pgSQL verbatim)LuksDeviceKeyRevocationFailed— UPDATErevocation_status='failed'+revocation_errorcapturedLuksDeviceKeyRevocationRequestedstays a no-op — the deleted PL/pgSQL projector also lacked a case for it.Behavioural delta
LuksKeyRotated's 3 writes remain atomic with each other (store.WithTx); revocation events are single UPDATEs and don't need tx wrap. LUKS reads are operator-driven (recover passphrase, audit revocation status) — not hot-path RPCs — so the post-commit gap closes before any human-driven query.Test plan
Tests written FIRST per saved feedback (TDD).
rotation_reason, required-field validation per variant, wrong stream/event type →ErrIgnoredEvent, malformed payload bytesis_current=TRUErow on the same deviceLuksDeviceKeyRevocationRequestedis ignored (preserves PL/pgSQL no-op behaviour)go test ./server/internal/projectors/...)Refs tracker #107. Closes #99.
Summary by CodeRabbit