Skip to content

Commit c78f36a

Browse files
authored
refactor: typed event payload structs + roundtrip tests (PR F of cleanup) (#192)
* refactor: typed event payload structs + roundtrip tests (PR F of cleanup) Server-only refactor; no SDK or proto changes. Promotes the projector decoder Raw structs to exported payload structs in a shared internal/eventtypes/payloads/ package. Handler emit sites now construct typed structs instead of map[string]any literals. Catches handler/projector schema drift at compile time — exactly the bug class behind issue #135's partial-write hazard. One enabler change: store.Event.Data type is broadened from map[string]any to any so call sites can pass typed structs. AppendEvent's existing JSON marshal accepts any JSON-marshalable value; legacy map-literal call sites continue to compile unchanged. Files created: - internal/eventtypes/payloads/{user,device,action,assignment, execution,lps_password,luks_key}.go — 36 typed payload structs across 7 stream types - internal/eventtypes/payloads/doc.go — package docs - internal/eventtypes/payloads/payloads_test.go — 32 roundtrip tests (one per payload), each marshal → unmarshal → assert.Equal Files modified: - internal/store/store.go — Event.Data: any - internal/projectors/{user,device,action,assignment,execution, lps_password,luks_key}.go — decoders use payloads.X structs - ~30 handler emit sites converted to typed payloads: - internal/api/user_handler.go — 11 sites - internal/api/device_handler.go — 6 sites - internal/api/registration_handler.go — DeviceRegistered - internal/api/certificate_handler.go — DeviceCertRenewed - internal/api/system_actions.go — AssignmentCreated + UserSystemActionLinked - internal/control/inbox_worker.go — 5 execution sites + introduced commandOutputPayload helper Deferred to PR G (final tech-debt audit): ~50 emit sites still use map[string]any. They continue to work because Event.Data is now any, but they don't get the compile-time schema-drift guarantee. Mechanical follow-up: add payload structs in remaining files (action.go, internal_handler.go, idp/sso/auth handlers, SCIM, role/group handlers, settings, totp, terminal, compliance) and update emit sites. Note: lps_password and luks_key payload structs preserve the masked LogValue() method from the projector originals so neither password nor passphrase can leak into slog output. Part of the 2026.06 cleanup-release sweep. PR G (final audit) follows. * test: redaction + RawCommandOutput contract tests (CR catches on PR #192) Three nitpick test additions from CR review: - TestLpsPasswordRotated_LogValueMasksPassword: locks the security guarantee — routing the payload through slog (e.g. slog.Warn("...", "payload", payload)) must NEVER emit the raw Password value. Asserts both 'NotContains(secret)' and 'Contains([REDACTED])'. - TestLuksKeyRotated_LogValueMasksPassphrase: sibling for LUKS. - TestRawCommandOutput_NilOmitted + TestRawCommandOutput_ProducesObject: locks the wire contract used by ExecutionTerminal / ExecutionTimedOut. Nil input produces nil so omitempty fires; non-nil input produces a JSON object (NOT a quoted string), with the exact legacy keys (stdout, stderr, exit_code), and round-trips cleanly through ExecutionTerminal. * test: assert exact legacy key count + omitempty parent contract (CR catches on PR #192) Two more nitpick test tightenings from CR review: - TestRoundtrip_CommandOutput + TestRawCommandOutput_ProducesObject: added assert.Len(asMap, 3) so an extra key (regression where a new field gets added to CommandOutput) would fail the test. - TestRawCommandOutput_NilOmitted: added end-to-end omitempty contract assertion via ExecutionTerminal — marshal a payload with Output: RawCommandOutput(nil) and assert the wire JSON does NOT contain the 'output' key. Without this the helper could regress to []byte('null') and we'd only catch it via downstream projector failures.
1 parent 6630773 commit c78f36a

23 files changed

Lines changed: 1321 additions & 431 deletions

internal/api/certificate_handler.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
1313
"github.com/manchtools/power-manage/server/internal/ca"
1414
"github.com/manchtools/power-manage/server/internal/eventtypes"
15+
"github.com/manchtools/power-manage/server/internal/eventtypes/payloads"
1516
"github.com/manchtools/power-manage/server/internal/store"
1617
db "github.com/manchtools/power-manage/server/internal/store/generated"
1718
)
@@ -82,13 +83,15 @@ func (h *CertificateHandler) RenewCertificate(ctx context.Context, req *connect.
8283
}
8384

8485
// Emit DeviceCertRenewed event (projection handler already exists in migration 001)
86+
fingerprint := newCert.Fingerprint
87+
notAfterStr := newCert.NotAfter.Format(time.RFC3339)
8588
if err := h.store.AppendEvent(ctx, store.Event{
8689
StreamType: "device",
8790
StreamID: deviceID,
8891
EventType: string(eventtypes.DeviceCertRenewed),
89-
Data: map[string]any{
90-
"cert_fingerprint": newCert.Fingerprint,
91-
"cert_not_after": newCert.NotAfter.Format(time.RFC3339),
92+
Data: payloads.DeviceCertRenewed{
93+
CertFingerprint: &fingerprint,
94+
CertNotAfter: &notAfterStr,
9295
},
9396
ActorType: "device",
9497
ActorID: deviceID,

internal/api/device_handler.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/manchtools/power-manage/server/internal/auth"
2323
"github.com/manchtools/power-manage/server/internal/crypto"
2424
"github.com/manchtools/power-manage/server/internal/eventtypes"
25+
"github.com/manchtools/power-manage/server/internal/eventtypes/payloads"
2526
"github.com/manchtools/power-manage/server/internal/middleware"
2627
"github.com/manchtools/power-manage/server/internal/search"
2728
"github.com/manchtools/power-manage/server/internal/store"
@@ -215,9 +216,9 @@ func (h *DeviceHandler) SetDeviceLabel(ctx context.Context, req *connect.Request
215216
StreamType: "device",
216217
StreamID: req.Msg.Id,
217218
EventType: string(eventtypes.DeviceLabelSet),
218-
Data: map[string]any{
219-
"key": req.Msg.Key,
220-
"value": req.Msg.Value,
219+
Data: payloads.DeviceLabelSet{
220+
Key: &req.Msg.Key,
221+
Value: &req.Msg.Value,
221222
},
222223
ActorType: "user",
223224
ActorID: userCtx.ID,
@@ -260,8 +261,8 @@ func (h *DeviceHandler) RemoveDeviceLabel(ctx context.Context, req *connect.Requ
260261
StreamType: "device",
261262
StreamID: req.Msg.Id,
262263
EventType: string(eventtypes.DeviceLabelRemoved),
263-
Data: map[string]any{
264-
"key": req.Msg.Key,
264+
Data: payloads.DeviceLabelRemoved{
265+
Key: &req.Msg.Key,
265266
},
266267
ActorType: "user",
267268
ActorID: userCtx.ID,
@@ -375,12 +376,13 @@ func (h *DeviceHandler) AssignDevice(ctx context.Context, req *connect.Request[p
375376
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user")
376377
}
377378

379+
uid := userID
378380
if err := appendEvent(ctx, h.store, h.logger, store.Event{
379381
StreamType: "device",
380382
StreamID: req.Msg.DeviceId,
381383
EventType: string(eventtypes.DeviceAssigned),
382-
Data: map[string]any{
383-
"user_id": userID,
384+
Data: payloads.DeviceUserAssignment{
385+
UserID: &uid,
384386
},
385387
ActorType: "user",
386388
ActorID: userCtx.ID,
@@ -403,12 +405,13 @@ func (h *DeviceHandler) AssignDevice(ctx context.Context, req *connect.Request[p
403405
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to get user group")
404406
}
405407

408+
gid := groupID
406409
if err := appendEvent(ctx, h.store, h.logger, store.Event{
407410
StreamType: "device",
408411
StreamID: req.Msg.DeviceId,
409412
EventType: string(eventtypes.DeviceGroupAssigned),
410-
Data: map[string]any{
411-
"group_id": groupID,
413+
Data: payloads.DeviceGroupAssignment{
414+
GroupID: &gid,
412415
},
413416
ActorType: "user",
414417
ActorID: userCtx.ID,
@@ -480,8 +483,8 @@ func (h *DeviceHandler) UnassignDevice(ctx context.Context, req *connect.Request
480483
StreamType: "device",
481484
StreamID: req.Msg.DeviceId,
482485
EventType: string(eventtypes.DeviceUnassigned),
483-
Data: map[string]any{
484-
"user_id": req.Msg.UserId,
486+
Data: payloads.DeviceUserAssignment{
487+
UserID: &req.Msg.UserId,
485488
},
486489
ActorType: "user",
487490
ActorID: userCtx.ID,
@@ -494,8 +497,8 @@ func (h *DeviceHandler) UnassignDevice(ctx context.Context, req *connect.Request
494497
StreamType: "device",
495498
StreamID: req.Msg.DeviceId,
496499
EventType: string(eventtypes.DeviceGroupUnassigned),
497-
Data: map[string]any{
498-
"group_id": req.Msg.GroupId,
500+
Data: payloads.DeviceGroupAssignment{
501+
GroupID: &req.Msg.GroupId,
499502
},
500503
ActorType: "user",
501504
ActorID: userCtx.ID,
@@ -533,12 +536,13 @@ func (h *DeviceHandler) SetDeviceSyncInterval(ctx context.Context, req *connect.
533536
}
534537

535538
// Emit DeviceSyncIntervalSet event
539+
syncInterval := req.Msg.SyncIntervalMinutes
536540
if err := appendEvent(ctx, h.store, h.logger, store.Event{
537541
StreamType: "device",
538542
StreamID: req.Msg.Id,
539543
EventType: string(eventtypes.DeviceSyncIntervalSet),
540-
Data: map[string]any{
541-
"sync_interval_minutes": req.Msg.SyncIntervalMinutes,
544+
Data: payloads.DeviceSyncIntervalSet{
545+
SyncIntervalMinutes: &syncInterval,
542546
},
543547
ActorType: "user",
544548
ActorID: userCtx.ID,

internal/api/registration_handler.go

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
pm "github.com/manchtools/power-manage/sdk/gen/go/pm/v1"
1818
"github.com/manchtools/power-manage/server/internal/ca"
1919
"github.com/manchtools/power-manage/server/internal/eventtypes"
20+
"github.com/manchtools/power-manage/server/internal/eventtypes/payloads"
2021
"github.com/manchtools/power-manage/server/internal/store"
2122
)
2223

@@ -193,21 +194,36 @@ func (h *RegistrationHandler) Register(ctx context.Context, req *connect.Request
193194
return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to issue certificate")
194195
}
195196

196-
// Build event data for device registration
197-
eventData := map[string]any{
198-
"hostname": req.Msg.Hostname,
199-
"agent_version": req.Msg.AgentVersion,
200-
"cert_fingerprint": cert.Fingerprint,
201-
"cert_not_after": cert.NotAfter.Format(time.RFC3339),
202-
"registration_token_id": token.ID,
203-
"cert_pem": string(cert.CertPEM),
204-
"ca_cert_pem": string(h.ca.CACertPEM()),
197+
// Build event data for device registration. CertPEM + CACertPEM
198+
// ride along so future replays can recover the cert bytes from
199+
// the event log; the projector ignores them.
200+
hostname := req.Msg.Hostname
201+
agentVersion := req.Msg.AgentVersion
202+
certFingerprint := cert.Fingerprint
203+
// Preserve the legacy RFC 3339-string serialisation (no
204+
// sub-second precision) so wire bytes stay identical to the
205+
// pre-typed-payload emission. The projector parses the string
206+
// back into a time.Time via parseOptionalRFC3339, which accepts
207+
// both RFC 3339 and RFC 3339Nano.
208+
certNotAfterStr := cert.NotAfter.Format(time.RFC3339)
209+
registrationTokenID := token.ID
210+
certPEM := string(cert.CertPEM)
211+
caCertPEM := string(h.ca.CACertPEM())
212+
deviceData := payloads.DeviceRegistered{
213+
Hostname: &hostname,
214+
AgentVersion: &agentVersion,
215+
CertFingerprint: &certFingerprint,
216+
CertNotAfter: &certNotAfterStr,
217+
RegistrationTokenID: &registrationTokenID,
218+
CertPEM: &certPEM,
219+
CACertPEM: &caCertPEM,
205220
}
206221

207222
// Auto-assign device to token owner if the token has an owner
208223
if token.OwnerID != nil && *token.OwnerID != "" {
209-
eventData["assigned_user_id"] = *token.OwnerID
210-
logger.Info("auto-assigning device to token owner", "owner_id", *token.OwnerID)
224+
ownerID := *token.OwnerID
225+
deviceData.AssignedUserID = &ownerID
226+
logger.Info("auto-assigning device to token owner", "owner_id", ownerID)
211227
}
212228

213229
// Consume the token FIRST to prevent race conditions with one-time tokens.
@@ -238,7 +254,7 @@ func (h *RegistrationHandler) Register(ctx context.Context, req *connect.Request
238254
StreamType: "device",
239255
StreamID: deviceID,
240256
EventType: string(eventtypes.DeviceRegistered),
241-
Data: eventData,
257+
Data: deviceData,
242258
ActorType: "system",
243259
ActorID: "registration",
244260
}); err != nil {

internal/api/system_actions.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/manchtools/power-manage/server/internal/actionparams"
1313
"github.com/manchtools/power-manage/server/internal/ca"
1414
"github.com/manchtools/power-manage/server/internal/eventtypes"
15+
"github.com/manchtools/power-manage/server/internal/eventtypes/payloads"
1516
"github.com/manchtools/power-manage/server/internal/store"
1617
db "github.com/manchtools/power-manage/server/internal/store/generated"
1718
)
@@ -529,18 +530,19 @@ func (m *SystemActionManager) createSystemAction(ctx context.Context, name strin
529530
// assignActionToUser emits an AssignmentCreated event.
530531
func (m *SystemActionManager) assignActionToUser(ctx context.Context, actionID, userID string) error {
531532
assignmentID := newULID()
532-
533+
mode := int32(0) // REQUIRED
534+
sortOrder := int32(0)
533535
return m.store.AppendEvent(ctx, store.Event{
534536
StreamType: "assignment",
535537
StreamID: assignmentID,
536538
EventType: string(eventtypes.AssignmentCreated),
537-
Data: map[string]any{
538-
"source_type": "action",
539-
"source_id": actionID,
540-
"target_type": "user",
541-
"target_id": userID,
542-
"mode": 0, // REQUIRED
543-
"sort_order": 0,
539+
Data: payloads.AssignmentCreated{
540+
SourceType: "action",
541+
SourceID: actionID,
542+
TargetType: "user",
543+
TargetID: userID,
544+
Mode: &mode,
545+
SortOrder: &sortOrder,
544546
},
545547
ActorType: "system",
546548
ActorID: "system",
@@ -586,9 +588,9 @@ func (m *SystemActionManager) linkSystemAction(ctx context.Context, userID, fiel
586588
StreamType: "user",
587589
StreamID: userID,
588590
EventType: string(eventtypes.UserSystemActionLinked),
589-
Data: map[string]any{
590-
"field": field,
591-
"action_id": actionID,
591+
Data: payloads.UserSystemActionLinked{
592+
Field: &field,
593+
ActionID: &actionID,
592594
},
593595
ActorType: "system",
594596
ActorID: "system",

0 commit comments

Comments
 (0)