diff --git a/internal/api/handler/auth_session_oidc.go b/internal/api/handler/auth_session_oidc.go index 37e58be..16b9938 100644 --- a/internal/api/handler/auth_session_oidc.go +++ b/internal/api/handler/auth_session_oidc.go @@ -31,6 +31,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "net/http" "strings" "time" @@ -1002,8 +1003,17 @@ func (h *AuthSessionOIDCHandler) recordAudit(ctx context.Context, action, actor if h.audit == nil { return } - _ = h.audit.RecordEventWithCategory(ctx, actor, actorType, action, - domain.EventCategoryAuth, "session", resourceID, details) + // Audit 2026-05-10 HIGH-6 partial closure — emit WARN on audit-write + // failure so the silent row-miss is observable. The transactional- + // leg WithinTx refactor is a v3 follow-on. + if err := h.audit.RecordEventWithCategory(ctx, actor, actorType, action, + domain.EventCategoryAuth, "session", resourceID, details); err != nil { + slog.WarnContext(ctx, "oidc handler audit write failed (action committed; audit row may be missing)", + "action", action, + "actor_id", actor, + "resource_id", resourceID, + "err", err) + } } func (h *AuthSessionOIDCHandler) clearPreLoginCookie(w http.ResponseWriter) { diff --git a/internal/auth/bootstrap/service.go b/internal/auth/bootstrap/service.go index 47df576..e8ddd68 100644 --- a/internal/auth/bootstrap/service.go +++ b/internal/auth/bootstrap/service.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "log/slog" "regexp" "time" @@ -182,12 +183,20 @@ func (s *Service) ValidateAndMint(ctx context.Context, token, actorName string) // already landed in the DB. The audit-row gap is detectable // in monitoring (every successful mint should have a paired // bootstrap.consume row). - _ = s.audit.RecordEventWithCategory(ctx, "bootstrap-token", domain.ActorTypeSystem, + // Audit 2026-05-10 HIGH-6 partial closure — emit WARN on audit- + // write failure so the silent-row-miss is observable. The + // transactional-leg WithinTx refactor is a v3 follow-on. + if err := s.audit.RecordEventWithCategory(ctx, "bootstrap-token", domain.ActorTypeSystem, "bootstrap.consume", domain.EventCategoryAuth, "api_key", apiKey.ID, map[string]interface{}{ "actor_name": actorName, "role_id": authdomain.RoleIDAdmin, - }) + }); err != nil { + slog.WarnContext(ctx, "bootstrap.consume audit write failed (admin key minted; audit row may be missing)", + "actor_name", actorName, + "api_key_id", apiKey.ID, + "err", err) + } } return &MintResult{APIKey: apiKey, KeyValue: keyValue}, nil } diff --git a/internal/auth/breakglass/service.go b/internal/auth/breakglass/service.go index f06420a..a3be349 100644 --- a/internal/auth/breakglass/service.go +++ b/internal/auth/breakglass/service.go @@ -555,8 +555,17 @@ func (s *Service) recordAudit(ctx context.Context, action, actor string, actorTy if s.audit == nil { return } - _ = s.audit.RecordEventWithCategory(ctx, actor, actorType, action, - domain.EventCategoryAuth, "breakglass_credential", resourceID, details) + // Audit 2026-05-10 HIGH-6 partial closure — emit WARN on audit-write + // failure so a silent row-miss is observable. The transactional-leg + // WithinTx refactor (action + audit row atomic) is a v3 follow-on. + if err := s.audit.RecordEventWithCategory(ctx, actor, actorType, action, + domain.EventCategoryAuth, "breakglass_credential", resourceID, details); err != nil { + slog.WarnContext(ctx, "breakglass audit write failed (action committed; audit row may be missing)", + "action", action, + "actor_id", actor, + "resource_id", resourceID, + "err", err) + } } // _ ensures authdomain import is live in case future service code needs diff --git a/internal/auth/session/service.go b/internal/auth/session/service.go index b15e10a..18847b8 100644 --- a/internal/auth/session/service.go +++ b/internal/auth/session/service.go @@ -917,6 +917,15 @@ func (s *Service) recordAudit(ctx context.Context, action, actor string, actorTy if s.audit == nil { return } - _ = s.audit.RecordEventWithCategory(ctx, actor, actorType, action, - "auth", "session", resourceID, details) + // Audit 2026-05-10 HIGH-6 partial closure — emit WARN on audit-write + // failure so the silent row-miss is observable. The transactional- + // leg WithinTx refactor (action + audit row atomic) is a v3 follow-on. + if err := s.audit.RecordEventWithCategory(ctx, actor, actorType, action, + "auth", "session", resourceID, details); err != nil { + slog.WarnContext(ctx, "session audit write failed (action committed; audit row may be missing)", + "action", action, + "actor_id", actor, + "resource_id", resourceID, + "err", err) + } } diff --git a/internal/service/audit.go b/internal/service/audit.go index b5bdaea..0848764 100644 --- a/internal/service/audit.go +++ b/internal/service/audit.go @@ -106,6 +106,45 @@ func (s *AuditService) RecordEventWithTx(ctx context.Context, q repository.Queri return nil } +// RecordEventWithCategoryWithTx records a categorized audit event using +// the supplied repository.Querier so the row is committed in the same +// transaction as the underlying action. Mirrors RecordEventWithCategory +// but takes the Querier (typically *sql.Tx from postgres.WithinTx). +// +// Audit 2026-05-10 HIGH-6 closure — closes the gap where Bundle-1+2 +// auth-mutation paths emitted the audit row via a separate, non- +// transactional connection. A DB hiccup or connection reset between +// the action and the audit-row INSERT used to leave the action +// committed with no audit trail (CWE-778). With this method, the +// audit row participates in the action's transaction: rollback on +// any failure removes both the action row AND any audit row that the +// caller wrote inside the tx. +func (s *AuditService) RecordEventWithCategoryWithTx(ctx context.Context, q repository.Querier, actor string, actorType domain.ActorType, action, eventCategory, resourceType, resourceID string, details map[string]interface{}) error { + redacted := RedactDetailsForAudit(details) + detailsJSON, err := json.Marshal(redacted) + if err != nil { + detailsJSON = []byte("{}") + } + + event := &domain.AuditEvent{ + ID: generateID("audit"), + Timestamp: time.Now(), + Actor: actor, + ActorType: actorType, + Action: action, + ResourceType: resourceType, + ResourceID: resourceID, + Details: json.RawMessage(detailsJSON), + EventCategory: eventCategory, + } + + if err := s.auditRepo.CreateWithTx(ctx, q, event); err != nil { + return fmt.Errorf("failed to record audit event: %w", err) + } + + return nil +} + // List returns audit events matching filter criteria. func (s *AuditService) List(ctx context.Context, filter *repository.AuditFilter) ([]*domain.AuditEvent, error) { events, err := s.auditRepo.List(ctx, filter) diff --git a/internal/service/auth/actor_role_service.go b/internal/service/auth/actor_role_service.go index 3851eab..a483528 100644 --- a/internal/service/auth/actor_role_service.go +++ b/internal/service/auth/actor_role_service.go @@ -3,6 +3,7 @@ package auth import ( "context" "fmt" + "log/slog" "github.com/certctl-io/certctl/internal/domain" authdomain "github.com/certctl-io/certctl/internal/domain/auth" @@ -173,5 +174,21 @@ func (s *ActorRoleService) recordAudit(ctx context.Context, caller *Caller, acti // authentication / authorization event. The auditor role queries // /v1/audit?category=auth to surface this slice without // also pulling in cert.* events. - _ = s.audit.RecordEventWithCategory(ctx, caller.ActorID, caller.ActorType, action, domain.EventCategoryAuth, resourceType, resourceID, details) + // + // Audit 2026-05-10 HIGH-6 partial closure: the audit emit is still + // best-effort relative to the action transaction (the transactional- + // leg WithinTx refactor is a v3 follow-on; see + // cowork/auth-bundles-fixes-2026-05-10/10-high-6-atomic-audit-commit.md). + // What this commit closes is the *silence* leg — swap the discarded + // `_ = ...` pattern for an explicit WARN log so a DB hiccup or + // connection reset between action and audit is observable to the + // operator instead of going unnoticed (CWE-778). + if err := s.audit.RecordEventWithCategory(ctx, caller.ActorID, caller.ActorType, action, domain.EventCategoryAuth, resourceType, resourceID, details); err != nil { + slog.WarnContext(ctx, "audit write failed (action committed; audit row may be missing)", + "action", action, + "resource_type", resourceType, + "resource_id", resourceID, + "actor_id", caller.ActorID, + "err", err) + } } diff --git a/internal/service/auth/auth.go b/internal/service/auth/auth.go index c5b54fa..4a1723d 100644 --- a/internal/service/auth/auth.go +++ b/internal/service/auth/auth.go @@ -22,6 +22,7 @@ import ( "github.com/certctl-io/certctl/internal/domain" authdomain "github.com/certctl-io/certctl/internal/domain/auth" + "github.com/certctl-io/certctl/internal/repository" ) // Sentinel errors for the service layer. Handler / middleware code @@ -68,6 +69,19 @@ type AuditService interface { action, eventCategory, resourceType, resourceID string, details map[string]interface{}, ) error + // RecordEventWithCategoryWithTx records the audit row using the + // supplied repository.Querier so it commits atomically with the + // caller's transaction. Audit 2026-05-10 HIGH-6 closure — closes + // the gap where auth-mutation paths used a non-transactional audit + // emit, leaving orphan action rows on partial failure. + RecordEventWithCategoryWithTx( + ctx context.Context, + q repository.Querier, + actor string, + actorType domain.ActorType, + action, eventCategory, resourceType, resourceID string, + details map[string]interface{}, + ) error } // Caller describes the actor performing a service operation. Bundle 1 diff --git a/internal/service/auth/role_service.go b/internal/service/auth/role_service.go index 41e1995..fe6dca8 100644 --- a/internal/service/auth/role_service.go +++ b/internal/service/auth/role_service.go @@ -3,6 +3,7 @@ package auth import ( "context" "fmt" + "log/slog" "github.com/certctl-io/certctl/internal/domain" authdomain "github.com/certctl-io/certctl/internal/domain/auth" @@ -199,7 +200,18 @@ func (s *RoleService) recordAudit(ctx context.Context, caller *Caller, action, r if s.audit == nil || caller == nil { return } - _ = s.audit.RecordEventWithCategory(ctx, caller.ActorID, caller.ActorType, action, domain.EventCategoryAuth, resourceType, resourceID, details) + // Audit 2026-05-10 HIGH-6 partial closure — see + // actor_role_service.go::recordAudit for the rationale. Silence-leg + // closed by emitting WARN on audit-write failure; transactional-leg + // (action + audit atomic via WithinTx) is a v3 follow-on. + if err := s.audit.RecordEventWithCategory(ctx, caller.ActorID, caller.ActorType, action, domain.EventCategoryAuth, resourceType, resourceID, details); err != nil { + slog.WarnContext(ctx, "audit write failed (action committed; audit row may be missing)", + "action", action, + "resource_type", resourceType, + "resource_id", resourceID, + "actor_id", caller.ActorID, + "err", err) + } } // Ensure the compile-time pin: domain.ActorType is convertible to diff --git a/internal/service/auth/service_test.go b/internal/service/auth/service_test.go index 4cd378c..09e6dfe 100644 --- a/internal/service/auth/service_test.go +++ b/internal/service/auth/service_test.go @@ -221,6 +221,16 @@ func (f *fakeAudit) RecordEventWithCategory(_ context.Context, actor string, act return nil } +// RecordEventWithCategoryWithTx satisfies the Audit 2026-05-10 HIGH-6 +// interface extension. The test stub stores into the same calls slice; +// no transactional semantics needed because the fake doesn't have a DB. +func (f *fakeAudit) RecordEventWithCategoryWithTx(_ context.Context, _ repository.Querier, actor string, actorType domain.ActorType, action, eventCategory, resourceType, resourceID string, _ map[string]interface{}) error { + f.calls = append(f.calls, struct{ Actor, ActorType, Action, Category, ResourceID string }{ + actor, string(actorType), action, eventCategory, resourceID, + }) + return nil +} + // ============================================================================= // Authorizer tests // ============================================================================= diff --git a/internal/service/profile.go b/internal/service/profile.go index f11889a..ed54a67 100644 --- a/internal/service/profile.go +++ b/internal/service/profile.go @@ -165,13 +165,21 @@ func (s *ProfileService) UpdateProfile(ctx context.Context, id string, profile d return nil, fmt.Errorf("approval gate: %w", gerr) } if s.auditService != nil { - _ = s.auditService.RecordEventWithCategory( + // Audit 2026-05-10 HIGH-6 partial closure — emit WARN on + // audit-write failure so the silent row-miss is observable. + if err := s.auditService.RecordEventWithCategory( context.WithoutCancel(ctx), requester, domain.ActorTypeUser, "profile.edit_request", domain.EventCategoryAuth, "certificate_profile", id, map[string]interface{}{"approval_id": approvalID}, - ) + ); err != nil { + slog.WarnContext(ctx, "profile.edit_request audit write failed (approval requested; audit row may be missing)", + "profile_id", id, + "approval_id", approvalID, + "requester", requester, + "err", err) + } } return nil, fmt.Errorf("%w: approval=%s", ErrProfileEditPendingApproval, approvalID) }