From ac5b595e977acec8c83ac5484eff9291e4dcfb26 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sat, 25 Jul 2026 02:17:36 +0100 Subject: [PATCH] Cover the alert-spec matchers and the audit, email and update guards A third pass on partially covered functions, led by the alert evaluation predicates where a wrong arm means a missed or spurious alert. - internal/alerts/specs: matches 47.9 to 100, and all six matches helpers (severity threshold, change threshold, baseline anomaly, health assessment, posture threshold, and the severity latch) from 50 to 75 percent up to 100. Each threshold is pinned at, just below and just above, and the latch arm is exercised both latched and unlatched with concrete verdicts. - pkg/audit: exportCSV 76 to 88 with commas, quotes and newlines in the detail field asserted through a parsed round-trip; NewSigner error arms, both IsPersistent predicates and VerifySignature against a tampered payload and a wrong key. - internal/agentupdate: retryBackoffDelay, sleepWithContext, Snapshot and writeSelfTestTokenFile to 100, the token file exercised under t.TempDir including the unwritable-directory arm. - internal/notifications: writeMultipartBodyPart and alertNodeDisplay to 100, attachment handling to 69, all asserted on the produced MIME text. No test opens a network or SMTP connection. - internal/unifiedresources: the three pure action-dispatch helpers to 100. - internal/alerts/config: CanonicalResourceTypeKeys 34.3 to 78.4. Five targets deliberately did not move and are recorded rather than faked: the error arms of writeEmailThreadingHeaders, buildMultipartEmailMessage and copyWebhookConfig are unreachable because those functions write only into a local bytes.Buffer, which never errors; exportJSON's only gap is a json.MarshalIndent failure that its event struct cannot produce; and verifyBinaryMagic's remaining gap is a deferred close-error handler. No source file is modified. Adversarial review returned no rejects and flagged seven near-duplicate subtests; all were removed and every target function re-measured at an identical percentage. PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change --- .../update_branchcov0724pm_test.go | 435 +++++++++ .../config/identity_branchcov0724pm_test.go | 132 +++ .../specs/evaluator_branchcov0724pm_test.go | 825 ++++++++++++++++++ .../email_message_branchcov0724pm_test.go | 631 ++++++++++++++ .../action_dispatch_branchcov0724pm_test.go | 364 ++++++++ pkg/audit/export_branchcov0724pm_test.go | 338 +++++++ pkg/audit/signer_branchcov0724pm_test.go | 403 +++++++++ 7 files changed, 3128 insertions(+) create mode 100644 internal/agentupdate/update_branchcov0724pm_test.go create mode 100644 internal/alerts/config/identity_branchcov0724pm_test.go create mode 100644 internal/alerts/specs/evaluator_branchcov0724pm_test.go create mode 100644 internal/notifications/email_message_branchcov0724pm_test.go create mode 100644 internal/unifiedresources/action_dispatch_branchcov0724pm_test.go create mode 100644 pkg/audit/export_branchcov0724pm_test.go create mode 100644 pkg/audit/signer_branchcov0724pm_test.go diff --git a/internal/agentupdate/update_branchcov0724pm_test.go b/internal/agentupdate/update_branchcov0724pm_test.go new file mode 100644 index 000000000..6daa42071 --- /dev/null +++ b/internal/agentupdate/update_branchcov0724pm_test.go @@ -0,0 +1,435 @@ +package agentupdate + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestBranchcov0724pmSnapshot covers the nil-receiver arm of Snapshot (which +// returns a disabled status without dereferencing the receiver) and proves that +// the returned Status is an independent deep copy: mutating a cloned *time.Time +// pointer obtained from one Snapshot must not affect a subsequent Snapshot. +func TestBranchcov0724pmSnapshot(t *testing.T) { + t.Run("NilReceiverReturnsDisabled", func(t *testing.T) { + var u *Updater + got := u.Snapshot() + if got.State != UpdateStateDisabled { + t.Fatalf("nil Snapshot State = %q, want %q", got.State, UpdateStateDisabled) + } + if got.AutoUpdate { + t.Fatalf("nil Snapshot AutoUpdate = true, want false") + } + }) + + t.Run("ReturnsIndependentCopy", func(t *testing.T) { + u := New(Config{PulseURL: "https://pulse.example.com", CurrentVersion: "1.0.0"}) + + original := time.Date(2025, 1, 2, 3, 4, 5, 0, time.UTC) + u.updateStatus(func(s *Status) { + s.State = UpdateStateError + s.LastError = "boom" + s.LastCheckedAt = &original + s.LastAttemptAt = &original + s.LastSuccessAt = &original + }) + + first := u.Snapshot() + if first.State != UpdateStateError { + t.Fatalf("first State = %q, want %q", first.State, UpdateStateError) + } + if first.LastError != "boom" { + t.Fatalf("first LastError = %q, want %q", first.LastError, "boom") + } + if first.LastCheckedAt == nil || !first.LastCheckedAt.Equal(original) { + t.Fatalf("first LastCheckedAt = %v, want %v", first.LastCheckedAt, original) + } + + // Mutate the value behind the returned pointer and a value field. The + // internal status must be unaffected because Snapshot returns a copy. + sabotaged := original.Add(99 * time.Hour) + if first.LastCheckedAt != nil { + *first.LastCheckedAt = sabotaged + } + *first.LastAttemptAt = sabotaged + first.State = UpdateStateIdle + first.LastError = "mutated" + + second := u.Snapshot() + if second.State != UpdateStateError { + t.Fatalf("second State = %q, want %q (internal copy was mutated)", second.State, UpdateStateError) + } + if second.LastError != "boom" { + t.Fatalf("second LastError = %q, want %q", second.LastError, "boom") + } + if second.LastCheckedAt == nil || !second.LastCheckedAt.Equal(original) { + t.Fatalf("second LastCheckedAt = %v, want %v (time pointer was shared, not copied)", second.LastCheckedAt, original) + } + if second.LastAttemptAt == nil || !second.LastAttemptAt.Equal(original) { + t.Fatalf("second LastAttemptAt = %v, want %v", second.LastAttemptAt, original) + } + if second.LastSuccessAt == nil || !second.LastSuccessAt.Equal(original) { + t.Fatalf("second LastSuccessAt = %v, want %v", second.LastSuccessAt, original) + } + }) +} + +// TestBranchcov0724pmRetryBackoffDelay asserts the exact backoff durations, +// including the two arms the existing suite misses: attempt <= 0 (base delay) +// and the cap boundary where the exponential delay exceeds updateRetryMaxDelay. +func TestBranchcov0724pmRetryBackoffDelay(t *testing.T) { + cases := []struct { + name string + attempt int + want time.Duration + }{ + {"NegativeReturnsBase", -3, updateRetryBaseDelay}, + {"ZeroReturnsBase", 0, updateRetryBaseDelay}, + {"AttemptOne", 1, updateRetryBaseDelay}, + {"AttemptTwo", 2, 2 * updateRetryBaseDelay}, + {"AttemptThree", 3, 4 * updateRetryBaseDelay}, + {"AttemptFour", 4, 8 * updateRetryBaseDelay}, + {"AttemptFive", 5, 16 * updateRetryBaseDelay}, + // attempt 6 is the boundary: base<<5 (32s) > updateRetryMaxDelay (30s), + // so the cap arm returns updateRetryMaxDelay. + {"AttemptSixCapped", 6, updateRetryMaxDelay}, + {"AttemptSevenStillCapped", 7, updateRetryMaxDelay}, + // Well beyond the cap but still within int64 range (no overflow), so the + // cap arm still applies. attempt 50 is intentionally excluded here: it + // overflows the shift/multiply (see TestBranchcov0724pmRetryBackoffDelayOverflow). + {"AttemptTwentyCapped", 20, updateRetryMaxDelay}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := retryBackoffDelay(tc.attempt) + if got != tc.want { + t.Fatalf("retryBackoffDelay(%d) = %s, want %s", tc.attempt, got, tc.want) + } + }) + } + + // Explicit boundary check: the largest non-capped delay must be below the + // cap, and the very next step must equal the cap. + if got := retryBackoffDelay(5); got >= updateRetryMaxDelay { + t.Fatalf("retryBackoffDelay(5) = %s, expected below cap %s", got, updateRetryMaxDelay) + } + if got := retryBackoffDelay(6); got != updateRetryMaxDelay { + t.Fatalf("retryBackoffDelay(6) = %s, expected cap %s", got, updateRetryMaxDelay) + } +} + +// TestBranchcov0724pmRetryBackoffDelayOverflow is a characterization test that +// documents a SUSPECTED SOURCE BUG (reported, not fixed): for very large +// attempt values, the expression +// +// updateRetryBaseDelay * time.Duration(1<<(attempt-1)) +// +// overflows int64 (1s * 2^49 far exceeds int64 max). The wrapped result is +// negative, so the subsequent `delay > updateRetryMaxDelay` guard is false and +// the cap is bypassed, returning a bogus (negative) duration instead of the +// cap. This is not reachable through the real retry loop (updateRequestMaxAttempts +// == 3), but it is a latent defect of retryBackoffDelay. +func TestBranchcov0724pmRetryBackoffDelayOverflow(t *testing.T) { + got := retryBackoffDelay(50) + if got >= 0 { + t.Fatalf("retryBackoffDelay(50) = %s, expected a negative duration due to int64 overflow (bug)", got) + } + if got == updateRetryMaxDelay { + t.Fatalf("retryBackoffDelay(50) returned the cap; overflow bypass should produce a negative value (bug)") + } +} + +// TestBranchcov0724pmSleepWithContext covers both select arms: an already +// cancelled context returns immediately with context.Canceled, and a normal +// short sleep completes with nil. +func TestBranchcov0724pmSleepWithContext(t *testing.T) { + t.Run("CancelledContextReturnsImmediately", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + err := sleepWithContext(ctx, 30*time.Second) + elapsed := time.Since(start) + + if !errors.Is(err, context.Canceled) { + t.Fatalf("sleepWithContext err = %v, want context.Canceled", err) + } + if elapsed > 100*time.Millisecond { + t.Fatalf("sleepWithContext took %s on cancelled ctx, want immediate return", elapsed) + } + }) + + t.Run("ShortDurationCompletes", func(t *testing.T) { + d := 50 * time.Millisecond + start := time.Now() + err := sleepWithContext(context.Background(), d) + elapsed := time.Since(start) + + if err != nil { + t.Fatalf("sleepWithContext err = %v, want nil", err) + } + if elapsed < d { + t.Fatalf("sleepWithContext returned early after %s, want >= %s", elapsed, d) + } + if elapsed > 2*time.Second { + t.Fatalf("sleepWithContext took %s, want close to %s", elapsed, d) + } + }) +} + +// TestBranchcov0724pmWriteSelfTestTokenFile asserts the success-path file +// contents/mode and covers every error arm reachable through the package's +// injectable seams: createTemp failure (unwritable directory), write failure +// (pre-closed file), close failure, and chmod failure. +func TestBranchcov0724pmWriteSelfTestTokenFile(t *testing.T) { + t.Run("EmptyTokenReturnsEmptyAndNoFile", func(t *testing.T) { + gotPath, err := writeSelfTestTokenFile(" \t\n") + if err != nil { + t.Fatalf("writeSelfTestTokenFile err = %v, want nil", err) + } + if gotPath != "" { + t.Fatalf("writeSelfTestTokenFile path = %q, want empty", gotPath) + } + }) + + t.Run("SuccessWritesTrimmedContentsWithMode0600", func(t *testing.T) { + gotPath, err := writeSelfTestTokenFile(" my-secret-token \n") + if err != nil { + t.Fatalf("writeSelfTestTokenFile err = %v, want nil", err) + } + if gotPath == "" { + t.Fatalf("writeSelfTestTokenFile path empty, want a temp path") + } + t.Cleanup(func() { _ = os.Remove(gotPath) }) + + data, err := os.ReadFile(gotPath) + if err != nil { + t.Fatalf("read token file: %v", err) + } + if string(data) != "my-secret-token" { + t.Fatalf("token file contents = %q, want %q", string(data), "my-secret-token") + } + info, err := os.Stat(gotPath) + if err != nil { + t.Fatalf("stat token file: %v", err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("token file mode = %o, want 0600", info.Mode().Perm()) + } + }) + + t.Run("CreateTempFailureMissingDir", func(t *testing.T) { + // Deterministic createTemp failure regardless of privileges: point at a + // directory that does not exist on disk. + missingDir := filepath.Join(t.TempDir(), "does-not-exist") + + origCreateTemp := createTempFn + t.Cleanup(func() { createTempFn = origCreateTemp }) + createTempFn = func(string, string) (*os.File, error) { + return os.CreateTemp(missingDir, "pulse-agent-selftest-token-*") + } + + gotPath, err := writeSelfTestTokenFile("token") + if err == nil { + t.Fatalf("writeSelfTestTokenFile err = nil, want create error") + } + if !strings.Contains(err.Error(), "create self-test token file") { + t.Fatalf("err = %v, want it to contain %q", err, "create self-test token file") + } + if gotPath != "" { + t.Fatalf("gotPath = %q, want empty on create failure", gotPath) + } + }) + + t.Run("WriteFailureOnPreClosedFile", func(t *testing.T) { + workDir := t.TempDir() + + origCreateTemp := createTempFn + t.Cleanup(func() { createTempFn = origCreateTemp }) + createTempFn = func(string, string) (*os.File, error) { + f, err := os.CreateTemp(workDir, "pulse-agent-selftest-token-*") + if err != nil { + return nil, err + } + // Close before returning so the subsequent WriteString fails. + _ = f.Close() + return f, nil + } + + gotPath, err := writeSelfTestTokenFile("token") + if err == nil { + t.Fatalf("writeSelfTestTokenFile err = nil, want write error") + } + if !strings.Contains(err.Error(), "write self-test token file") { + t.Fatalf("err = %v, want it to contain %q", err, "write self-test token file") + } + if gotPath != "" { + t.Fatalf("gotPath = %q, want empty on write failure", gotPath) + } + // The on-disk file must have been cleaned up by the error-path defer. + matches, _ := filepath.Glob(filepath.Join(workDir, "pulse-agent-selftest-token-*")) + if len(matches) != 0 { + t.Fatalf("expected cleanup of token file, still present: %v", matches) + } + }) + + t.Run("CloseFailure", func(t *testing.T) { + origClose := closeFileFn + t.Cleanup(func() { closeFileFn = origClose }) + closeFileFn = func(f *os.File) error { + _ = f.Close() + return errors.New("close denied") + } + + gotPath, err := writeSelfTestTokenFile("token") + if err == nil { + t.Fatalf("writeSelfTestTokenFile err = nil, want close error") + } + if !strings.Contains(err.Error(), "close self-test token file") { + t.Fatalf("err = %v, want it to contain %q", err, "close self-test token file") + } + if gotPath != "" { + t.Fatalf("gotPath = %q, want empty on close failure", gotPath) + } + }) + + t.Run("ChmodFailure", func(t *testing.T) { + origChmod := chmodFn + t.Cleanup(func() { chmodFn = origChmod }) + chmodFn = func(string, os.FileMode) error { return errors.New("chmod denied") } + + gotPath, err := writeSelfTestTokenFile("token") + if err == nil { + t.Fatalf("writeSelfTestTokenFile err = nil, want chmod error") + } + if !strings.Contains(err.Error(), "chmod self-test token file") { + t.Fatalf("err = %v, want it to contain %q", err, "chmod self-test token file") + } + if gotPath != "" { + t.Fatalf("gotPath = %q, want empty on chmod failure", gotPath) + } + }) +} + +// TestBranchcov0724pmVerifyBinaryMagic exercises the magic-byte contract for +// each platform with concrete fixtures built from byte slices in t.TempDir: +// valid headers pass, truncated/empty files fail to read magic, wrong magic +// fails closed, and a missing path fails at open. +// +// NOTE: these arms are already exercised indirectly by update_test.go and via +// performUpdateWithExecPath, so this function's coverage percentage is not +// expected to rise. The only genuinely uncovered arms are the deferred +// close-error handling (update.go:679-685), which call f.Close() directly with +// no injectable seam and therefore cannot be reached from a test without a +// source change. +func TestBranchcov0724pmVerifyBinaryMagic(t *testing.T) { + origOS := runtimeGOOS + t.Cleanup(func() { runtimeGOOS = origOS }) + + tmpDir := t.TempDir() + + writeFixture := func(name string, data []byte) string { + t.Helper() + p := filepath.Join(tmpDir, name) + if err := os.WriteFile(p, data, 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + return p + } + + elf := []byte{0x7f, 'E', 'L', 'F', 0x02, 0x01, 0x01, 0x00} + macho64 := []byte{0xcf, 0xfa, 0xed, 0xfe, 0x07, 0x00, 0x00, 0x00} + macho32 := []byte{0xce, 0xfa, 0xed, 0xfe, 0x07, 0x00, 0x00, 0x00} + machoFat := []byte{0xca, 0xfe, 0xba, 0xbe, 0x00, 0x00, 0x00, 0x08} + pe := []byte{'M', 'Z', 0x90, 0x00, 0x03, 0x00, 0x00, 0x00} + wrong := []byte{0xde, 0xad, 0xbe, 0xef, 0x12, 0x34} + + t.Run("LinuxValidELF", func(t *testing.T) { + runtimeGOOS = goOSLinux + p := writeFixture("elf", elf) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected ELF to validate, got %v", err) + } + }) + + t.Run("FreeBSDValidELF", func(t *testing.T) { + runtimeGOOS = goOSFreeBSD + p := writeFixture("elf-fbsd", elf) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected FreeBSD ELF to validate, got %v", err) + } + }) + + t.Run("LinuxWrongMagic", func(t *testing.T) { + runtimeGOOS = goOSLinux + p := writeFixture("wrong-elf", wrong) + err := verifyBinaryMagic(p) + if err == nil || !strings.Contains(err.Error(), "ELF") { + t.Fatalf("expected ELF magic error, got %v", err) + } + }) + + t.Run("DarwinValidMachO64", func(t *testing.T) { + runtimeGOOS = goOSDarwin + p := writeFixture("macho64", macho64) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected Mach-O 64-bit to validate, got %v", err) + } + }) + + t.Run("DarwinValidMachO32", func(t *testing.T) { + runtimeGOOS = goOSDarwin + p := writeFixture("macho32", macho32) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected Mach-O 32-bit to validate, got %v", err) + } + }) + + t.Run("DarwinValidUniversal", func(t *testing.T) { + runtimeGOOS = goOSDarwin + p := writeFixture("macho-fat", machoFat) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected universal/fat Mach-O to validate, got %v", err) + } + }) + + t.Run("DarwinWrongMagic", func(t *testing.T) { + runtimeGOOS = goOSDarwin + p := writeFixture("wrong-macho", wrong) + err := verifyBinaryMagic(p) + if err == nil || !strings.Contains(err.Error(), "Mach-O") { + t.Fatalf("expected Mach-O magic error, got %v", err) + } + }) + + t.Run("WindowsValidPE", func(t *testing.T) { + runtimeGOOS = goOSWindows + p := writeFixture("pe", pe) + if err := verifyBinaryMagic(p); err != nil { + t.Fatalf("expected PE to validate, got %v", err) + } + }) + + t.Run("WindowsWrongMagic", func(t *testing.T) { + runtimeGOOS = goOSWindows + p := writeFixture("wrong-pe", wrong) + err := verifyBinaryMagic(p) + if err == nil || !strings.Contains(err.Error(), "PE") { + t.Fatalf("expected PE magic error, got %v", err) + } + }) + + t.Run("UnsupportedOSFailsClosed", func(t *testing.T) { + runtimeGOOS = "plan9" + p := writeFixture("plan9", []byte{0x00, 0x01, 0x02, 0x03}) + err := verifyBinaryMagic(p) + if err == nil || !strings.Contains(err.Error(), "unsupported") { + t.Fatalf("expected unsupported OS error, got %v", err) + } + }) +} diff --git a/internal/alerts/config/identity_branchcov0724pm_test.go b/internal/alerts/config/identity_branchcov0724pm_test.go new file mode 100644 index 000000000..bc1922708 --- /dev/null +++ b/internal/alerts/config/identity_branchcov0724pm_test.go @@ -0,0 +1,132 @@ +package config + +import ( + "reflect" + "testing" +) + +// This file is a purpose-built branch-coverage test set (selected via +// `-run "^TestBranchcov0724pm"`) for CanonicalResourceTypeKeys (identity.go:9), +// which had 34.3% coverage. The existing alerts_config_identity_branchcov0716 +// test only reaches a handful of switch arms (vm, node, agent-disk, storage, +// k8s-cluster + the default/legacy-nil paths); the majority of the resource- +// type enumeration in the switch was never executed. +// +// These tests drive EVERY reachable case arm of the switch directly and assert +// the exact, ordered key slice each produces, including the multi-key ancestry +// chains (e.g. oci-container -> system-container -> guest). They also pin two +// observable invariants of the addUnique closure: ordering is append-order +// (not sorted), and each call returns a fresh, independent allocation. +// +// Conventions match sibling in-package tests in this directory (see +// validsignal_branchcov0724pm_test.go): stdlib `testing` only, table-driven +// subtests with `tc := tc`, reflect.DeepEqual assertions, no testify. +// +// Purity: CanonicalResourceTypeKeys is a pure function over its string argument; +// no network, daemon, database, or filesystem is touched. + +// TestBranchcov0724pmCanonicalResourceTypeKeys exercises every reachable case +// arm of the CanonicalResourceTypeKeys switch. Inputs are the canonical +// (post-normalization) type keys, since CanonicalAlertResourceType folds any +// spaced display alias ("kubernetes cluster", "truenas disk", ...) into its +// hyphenated form before this switch is consulted. +func TestBranchcov0724pmCanonicalResourceTypeKeys(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + // Single-key leaf types and their ancestry chains. Order is append-order, + // which these assertions pin against accidental reordering. + {"guest", "guest", []string{"guest"}}, + {"vm", "vm", []string{"vm", "guest"}}, + {"system-container", "system-container", []string{"system-container", "guest"}}, + {"oci-container", "oci-container", []string{"oci-container", "system-container", "guest"}}, + {"app-container", "app-container", []string{"app-container", "guest"}}, + {"docker-host", "docker-host", []string{"docker-host", "node"}}, + {"docker-service", "docker-service", []string{"docker-service", "app-container", "guest"}}, + {"node", "node", []string{"node"}}, + {"agent", "agent", []string{"agent", "node"}}, + {"agent-disk", "agent-disk", []string{"agent-disk", "agent", "storage"}}, + {"pbs", "pbs", []string{"pbs", "node"}}, + {"pmg", "pmg", []string{"pmg", "node"}}, + {"k8s-cluster", "k8s-cluster", []string{"k8s-cluster", "node"}}, + {"k8s-node", "k8s-node", []string{"k8s-node", "node"}}, + {"k8s-deployment", "k8s-deployment", []string{"k8s-deployment", "guest"}}, + {"k8s-namespace", "k8s-namespace", []string{"k8s-namespace"}}, + {"pod", "pod", []string{"pod", "guest"}}, + + // TrueNAS family. + {"truenas-system", "truenas-system", []string{"truenas-system", "agent", "node"}}, + {"truenas-pool", "truenas-pool", []string{"truenas-pool", "storage"}}, + {"truenas-dataset", "truenas-dataset", []string{"truenas-dataset", "storage"}}, + {"truenas-disk", "truenas-disk", []string{"truenas-disk", "physical_disk", "disk", "storage"}}, + + // VMware family. + {"vmware-host", "vmware-host", []string{"vmware-host", "agent", "node"}}, + {"vmware-vm", "vmware-vm", []string{"vmware-vm", "vm", "guest"}}, + {"vmware-datastore", "vmware-datastore", []string{"vmware-datastore", "storage"}}, + {"vmware-network", "vmware-network", []string{"vmware-network", "network"}}, + + // Generic storage-shaped types. + {"storage", "storage", []string{"storage"}}, + {"disk", "disk", []string{"disk", "storage"}}, + {"datastore", "datastore", []string{"datastore", "storage", "pbs"}}, + {"pool", "pool", []string{"pool", "storage"}}, + {"dataset", "dataset", []string{"dataset", "storage"}}, + {"ceph", "ceph", []string{"ceph", "storage"}}, + {"physical_disk", "physical_disk", []string{"physical_disk", "disk", "storage"}}, + + // Unknown type -> default arm: the type key is the sole element. + {"unknown custom type default passthrough", "widget", []string{"widget"}}, + {"unknown hyphenated type default passthrough", "foo-bar-baz", []string{"foo-bar-baz"}}, + + // Empty / blank -> early nil return (typeKey == "" branch). Asserts a + // true nil rather than an empty non-nil slice. + {"empty string returns nil", "", nil}, + {"whitespace only returns nil", " ", nil}, + + // Legacy unsupported types that survive CanonicalAlertResourceType + // unchanged but are rejected by isUnsupportedLegacyAlertResourceType -> + // early nil return. These reach the second operand of the early-return + // conjunction (the local switch arms). + {"legacy host returns nil", "host", nil}, + {"legacy qemu returns nil", "qemu", nil}, + {"legacy lxc returns nil", "lxc", nil}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := CanonicalResourceTypeKeys(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("CanonicalResourceTypeKeys(%q) = %v, want %v", tc.in, got, tc.want) + } + // A nil want must be a TRUE nil slice, not a non-nil empty one: the + // function's contract is `return nil`. + if tc.want == nil && got != nil { + t.Fatalf("CanonicalResourceTypeKeys(%q) = %#v, want true nil", tc.in, got) + } + }) + } +} + +// TestBranchcov0724pmCanonicalResourceTypeKeysIndependentCopy proves each call +// returns a fresh, independent allocation: mutating one result must not affect a +// subsequent call's result (no shared backing array). +func TestBranchcov0724pmCanonicalResourceTypeKeysIndependentCopy(t *testing.T) { + first := CanonicalResourceTypeKeys("vm") + if len(first) != 2 { + t.Fatalf("expected two keys for vm, got %v", first) + } + // Mutate the returned slice in place and via append. + first[0] = "MUTATED" + first = append(first, "EXTRA") + + second := CanonicalResourceTypeKeys("vm") + want := []string{"vm", "guest"} + if !reflect.DeepEqual(second, want) { + t.Fatalf("second call returned %v, want %v (result must be an independent copy)", second, want) + } +} diff --git a/internal/alerts/specs/evaluator_branchcov0724pm_test.go b/internal/alerts/specs/evaluator_branchcov0724pm_test.go new file mode 100644 index 000000000..ae09873b8 --- /dev/null +++ b/internal/alerts/specs/evaluator_branchcov0724pm_test.go @@ -0,0 +1,825 @@ +package specs + +import ( + "testing" + "time" +) + +// This file is a purpose-built branch-coverage test set (selected via +// `-run "^TestBranchcov0724pm"`) for the alert-spec matching predicates in +// evaluator.go that the existing evaluator_test.go suite only reaches +// indirectly (and partially) through Evaluate. The functions are unexported, so +// these tests live in-package and call the predicates directly, which lets them +// exercise arms that Evaluate's validation/dispatch makes hard to reach: +// +// - matches (evaluator.go:435) baseline 47.9% +// - matchesSeverityThreshold (evaluator.go:518) baseline 50.0% +// - severityThresholdStillLatched (evaluator.go:547) baseline 62.5% +// - matchesChangeThreshold (evaluator.go:565) baseline 58.8% +// - matchesBaselineAnomaly (evaluator.go:598) baseline 63.2% +// - matchesHealthAssessment (evaluator.go:636) baseline 75.0% +// - matchesPostureThreshold (evaluator.go:658) baseline 66.7% +// +// Conventions match sibling in-package tests in this directory (see +// rollup_evidence_branchcov0723am_test.go and evaluator_test.go): stdlib +// `testing` only, table-driven subtests, t.Fatalf assertions on concrete +// expected (matched, severity, reason) triples, no testify. +// +// Purity: every target is a pure function over its struct arguments; no +// network, daemon, database, or filesystem is touched. + +// matchTriple is the (matched, severity, reason) shape returned by every +// matches* predicate, used to keep table assertions uniform and tautology-free. +type matchTriple struct { + matched bool + severity AlertSeverity + reason string +} + +func assertMatchTriple(t *testing.T, got, want matchTriple) { + t.Helper() + if got.matched != want.matched || got.severity != want.severity || got.reason != want.reason { + t.Fatalf("got (matched=%v severity=%q reason=%q), want (matched=%v severity=%q reason=%q)", + got.matched, got.severity, got.reason, want.matched, want.severity, want.reason) + } +} + +// TestBranchcov0724pmMatches drives the matches() dispatcher (evaluator.go:435) +// across every spec kind, focusing on the arms the existing suite does not +// reach when going through Evaluate: the nil/missing-evidence early return of +// each payload kind, the unknown-kind default arm, the ProviderIncident and +// ResourceIncidentRollup kinds (entirely uncovered), the ServiceGap duration +// fallback + negative-missing clamp, and the no-match arms of DiscreteState and +// Connectivity/PoweredState. +func TestBranchcov0724pmMatches(t *testing.T) { + // Shared, fully-populated payloads used by both the "matched" and + // "nil-evidence" variants of each kind so the only variable is the + // evidence/spec presence. + sevSpec := &SeverityThresholdSpec{Metric: "queue", Direction: ThresholdDirectionAbove, Warning: 500, Critical: 1000} + changeSpec := &ChangeThresholdSpec{Metric: "spam", WarningCurrent: 2000, CriticalCurrent: 5000, WarningDelta: 250, CriticalDelta: 500, WarningPercent: 25, CriticalPercent: 50} + baselineSpec := &BaselineAnomalySpec{Metric: "spamIn", QuietBaseline: 40, WarningRatio: 1.8, CriticalRatio: 2.5, WarningDelta: 150, CriticalDelta: 300, QuietWarningDelta: 60, QuietCriticalDelta: 120} + healthSpec := &HealthAssessmentSpec{Signal: "host-raid", Codes: []string{"raid_degraded"}} + postureSpec := &PostureThresholdSpec{AgeMetric: "age", WarningAge: 7, CriticalAge: 14, SizeMetric: "size", WarningSize: 50, CriticalSize: 100} + discreteSpec := &DiscreteStateSpec{StateKey: "state", TriggerStates: []string{"paused"}} + svcGapPercentSpec := &ServiceGapSpec{Service: "web", WarningPercent: 10, CriticalPercent: 50, GapAfter: 5 * time.Minute} + svcGapDurationSpec := &ServiceGapSpec{Service: "web", GapAfter: 5 * time.Minute} + providerSpec := &ProviderIncidentSpec{Provider: "aws", Codes: []string{"aws_ec2_degraded"}, NativeIDs: []string{"inc-1"}} + rollupSpec := &ResourceIncidentRollupSpec{Code: "cap_low", IncidentCount: 3} + + cases := []struct { + name string + spec ResourceAlertSpec + evidence AlertEvidence + want matchTriple + }{ + // ---- nil / missing-evidence early returns (one per payload kind) ---- + { + name: "severity threshold nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindSeverityThreshold, Severity: AlertSeverityWarning, SeverityThreshold: sevSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "severity threshold nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindSeverityThreshold, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{SeverityThreshold: &SeverityThresholdEvidence{Metric: "queue", Direction: ThresholdDirectionAbove, Observed: 700}}, + want: matchTriple{false, "", ""}, + }, + { + // SeverityThreshold is normally dispatched to evaluateSeverityThreshold + // by Evaluate and never reaches matches(); calling matches() directly + // with both payloads present exercises the delegation return to + // matchesSeverityThreshold. + name: "severity threshold delegated to matchesSeverityThreshold", + spec: ResourceAlertSpec{Kind: AlertSpecKindSeverityThreshold, Severity: AlertSeverityWarning, SeverityThreshold: sevSpec}, + evidence: AlertEvidence{SeverityThreshold: &SeverityThresholdEvidence{Metric: "queue", Direction: ThresholdDirectionAbove, Observed: 1200}}, + want: matchTriple{true, AlertSeverityCritical, "severity-threshold-critical"}, + }, + { + name: "change threshold nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindChangeThreshold, Severity: AlertSeverityWarning, ChangeThreshold: changeSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "change threshold nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindChangeThreshold, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{ChangeThreshold: &ChangeThresholdEvidence{Metric: "spam", Observed: 2500}}, + want: matchTriple{false, "", ""}, + }, + { + name: "baseline anomaly nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindBaselineAnomaly, Severity: AlertSeverityWarning, BaselineAnomaly: baselineSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "baseline anomaly nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindBaselineAnomaly, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{BaselineAnomaly: &BaselineAnomalyEvidence{Metric: "spamIn", Observed: 420, Baseline: 100}}, + want: matchTriple{false, "", ""}, + }, + { + name: "health assessment nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindHealthAssessment, Severity: AlertSeverityWarning, HealthAssessment: healthSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "health assessment nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindHealthAssessment, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{HealthAssessment: &HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityWarning, Codes: []string{"raid_degraded"}}}, + want: matchTriple{false, "", ""}, + }, + { + name: "posture threshold nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindPostureThreshold, Severity: AlertSeverityWarning, PostureThreshold: postureSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "posture threshold nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindPostureThreshold, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{PostureThreshold: &PostureThresholdEvidence{AgeMetric: "age", AgeValue: 20}}, + want: matchTriple{false, "", ""}, + }, + { + name: "discrete state nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindDiscreteState, Severity: AlertSeverityWarning, DiscreteState: discreteSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "discrete state nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindDiscreteState, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{DiscreteState: &DiscreteStateEvidence{StateKey: "state", Observed: "paused"}}, + want: matchTriple{false, "", ""}, + }, + { + name: "service gap nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapPercentSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "service gap nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 10, Running: 8}}, + want: matchTriple{false, "", ""}, + }, + { + name: "provider incident nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityWarning, ProviderIncident: providerSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "provider incident nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "aws", Code: "aws_ec2_degraded"}}, + want: matchTriple{false, "", ""}, + }, + { + name: "resource incident rollup nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindResourceIncidentRollup, Severity: AlertSeverityWarning, ResourceIncidentRollup: rollupSpec}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "resource incident rollup nil spec payload returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindResourceIncidentRollup, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{ResourceIncidentRollup: &ResourceIncidentRollupEvidence{Code: "cap_low", IncidentCount: 3}}, + want: matchTriple{false, "", ""}, + }, + + // ---- Connectivity: nil evidence vs disconnected vs connected ---- + { + name: "connectivity nil evidence returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindConnectivity, Severity: AlertSeverityCritical}, + evidence: AlertEvidence{}, + want: matchTriple{false, AlertSeverityCritical, "connectivity-lost"}, + }, + { + name: "connectivity connected returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindConnectivity, Severity: AlertSeverityCritical}, + evidence: AlertEvidence{Connectivity: &ConnectivityEvidence{Signal: "heartbeat", Connected: true}}, + want: matchTriple{false, AlertSeverityCritical, "connectivity-lost"}, + }, + { + name: "connectivity disconnected returns true critical", + spec: ResourceAlertSpec{Kind: AlertSpecKindConnectivity, Severity: AlertSeverityCritical}, + evidence: AlertEvidence{Connectivity: &ConnectivityEvidence{Signal: "heartbeat", Connected: false}}, + want: matchTriple{true, AlertSeverityCritical, "connectivity-lost"}, + }, + + // ---- PoweredState: nil evidence vs match vs match ---- + { + name: "powered state nil evidence returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKindPoweredState, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + name: "powered state matching observed equals expected returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindPoweredState, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{PoweredState: &PoweredStateEvidence{Expected: PowerStateOn, Observed: PowerStateOn}}, + want: matchTriple{false, AlertSeverityWarning, "powered-state-mismatch"}, + }, + { + name: "powered state mismatch returns true", + spec: ResourceAlertSpec{Kind: AlertSpecKindPoweredState, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{PoweredState: &PoweredStateEvidence{Expected: PowerStateOn, Observed: PowerStateOff}}, + want: matchTriple{true, AlertSeverityWarning, "powered-state-mismatch"}, + }, + + // ---- DiscreteState: in-set vs not-in-set ---- + { + name: "discrete state observed not in trigger set returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindDiscreteState, Severity: AlertSeverityWarning, DiscreteState: discreteSpec}, + evidence: AlertEvidence{DiscreteState: &DiscreteStateEvidence{StateKey: "state", Observed: "running"}}, + want: matchTriple{false, AlertSeverityWarning, "discrete-state-match"}, + }, + { + name: "discrete state observed in trigger set returns true", + spec: ResourceAlertSpec{Kind: AlertSpecKindDiscreteState, Severity: AlertSeverityWarning, DiscreteState: discreteSpec}, + evidence: AlertEvidence{DiscreteState: &DiscreteStateEvidence{StateKey: "state", Observed: "paused"}}, + want: matchTriple{true, AlertSeverityWarning, "discrete-state-match"}, + }, + + // ---- ServiceGap: percent critical/warning/normal + duration + clamp ---- + { + name: "service gap critical percent returns critical", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapPercentSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 10, Running: 4}}, + want: matchTriple{true, AlertSeverityCritical, "service-gap-critical"}, + }, + { + name: "service gap warning percent returns warning", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapPercentSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 10, Running: 8}}, + want: matchTriple{true, AlertSeverityWarning, "service-gap-warning"}, + }, + { + name: "service gap percent below thresholds returns normal", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapPercentSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 10, Running: 10}}, + want: matchTriple{false, "", "service-gap-normal"}, + }, + { + // Running exceeds Desired: missing goes negative and is clamped to 0, + // yielding percent 0 -> normal. Exercises the `if missing < 0` arm. + name: "service gap running exceeds desired clamps missing to zero", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapPercentSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 10, Running: 12}}, + want: matchTriple{false, "", "service-gap-normal"}, + }, + { + // Desired == 0 falls through to the duration branch; MissingFor past + // GapAfter with GapAfter > 0 -> true at spec severity. + name: "service gap duration beyond gapAfter returns true", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapDurationSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 0, MissingFor: 6 * time.Minute}}, + want: matchTriple{true, AlertSeverityWarning, "service-gap-duration"}, + }, + { + // Duration branch, MissingFor below GapAfter -> false. + name: "service gap duration below gapAfter returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: svcGapDurationSpec}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 0, MissingFor: 1 * time.Minute}}, + want: matchTriple{false, AlertSeverityWarning, "service-gap-duration"}, + }, + { + // Duration branch, GapAfter == 0 short-circuits to false even when + // MissingFor is positive (covers the `&& spec.ServiceGap.GapAfter > 0` + // right operand of the conjunction). + name: "service gap duration with zero gapAfter returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindServiceGap, Severity: AlertSeverityWarning, ServiceGap: &ServiceGapSpec{Service: "web"}}, + evidence: AlertEvidence{ServiceGap: &ServiceGapEvidence{Service: "web", Desired: 0, MissingFor: time.Hour}}, + want: matchTriple{false, AlertSeverityWarning, "service-gap-duration"}, + }, + + // ---- ProviderIncident: provider/code/native-id mismatch + happy ---- + { + name: "provider incident provider mismatch returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityWarning, ProviderIncident: providerSpec}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "gcp", Code: "aws_ec2_degraded"}}, + want: matchTriple{false, "", "provider-mismatch"}, + }, + { + name: "provider incident code mismatch returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityWarning, ProviderIncident: providerSpec}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "aws", Code: "other_code", NativeID: "inc-1"}}, + want: matchTriple{false, "", "provider-code-mismatch"}, + }, + { + name: "provider incident native id mismatch returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityWarning, ProviderIncident: providerSpec}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "aws", Code: "aws_ec2_degraded", NativeID: "other"}}, + want: matchTriple{false, "", "provider-native-id-mismatch"}, + }, + { + // Provider matches, code is in the allowed set, no native-id filter + // restriction violated -> incident reported at spec severity. + name: "provider incident happy path returns true", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityCritical, ProviderIncident: providerSpec}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "aws", Code: "aws_ec2_degraded", NativeID: "inc-1"}}, + want: matchTriple{true, AlertSeverityCritical, "provider-incident"}, + }, + { + // No Codes filter and no NativeIDs filter: only the provider needs to + // match (both `len(...) > 0` guards are false). + name: "provider incident no filters matches on provider only", + spec: ResourceAlertSpec{Kind: AlertSpecKindProviderIncident, Severity: AlertSeverityCritical, ProviderIncident: &ProviderIncidentSpec{Provider: "aws", NativeIDs: []string{"inc-1"}}}, + evidence: AlertEvidence{ProviderIncident: &ProviderIncidentEvidence{Provider: "aws", NativeID: "inc-1", Code: "anything"}}, + want: matchTriple{true, AlertSeverityCritical, "provider-incident"}, + }, + + // ---- ResourceIncidentRollup: count/code mismatch + happy ---- + { + name: "resource incident rollup zero count returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindResourceIncidentRollup, Severity: AlertSeverityWarning, ResourceIncidentRollup: rollupSpec}, + evidence: AlertEvidence{ResourceIncidentRollup: &ResourceIncidentRollupEvidence{Code: "cap_low", IncidentCount: 0}}, + want: matchTriple{false, AlertSeverityWarning, "resource-incident-rollup"}, + }, + { + name: "resource incident rollup code mismatch returns false", + spec: ResourceAlertSpec{Kind: AlertSpecKindResourceIncidentRollup, Severity: AlertSeverityWarning, ResourceIncidentRollup: rollupSpec}, + evidence: AlertEvidence{ResourceIncidentRollup: &ResourceIncidentRollupEvidence{Code: "other", IncidentCount: 5}}, + want: matchTriple{false, AlertSeverityWarning, "resource-incident-rollup"}, + }, + { + name: "resource incident rollup happy path returns true", + spec: ResourceAlertSpec{Kind: AlertSpecKindResourceIncidentRollup, Severity: AlertSeverityCritical, ResourceIncidentRollup: rollupSpec}, + evidence: AlertEvidence{ResourceIncidentRollup: &ResourceIncidentRollupEvidence{Code: "cap_low", IncidentCount: 2}}, + want: matchTriple{true, AlertSeverityCritical, "resource-incident-rollup"}, + }, + + // ---- default / unknown kind ---- + { + name: "unknown kind returns false empty", + spec: ResourceAlertSpec{Kind: AlertSpecKind("nope"), Severity: AlertSeverityWarning}, + evidence: AlertEvidence{}, + want: matchTriple{false, "", ""}, + }, + { + // MetricThreshold is intentionally NOT a case in matches() (it is + // dispatched to evaluateMetricThreshold by Evaluate, never matches()), + // so it must fall through to the default arm and return false. + name: "metric threshold kind falls through to default", + spec: ResourceAlertSpec{Kind: AlertSpecKindMetricThreshold, Severity: AlertSeverityWarning}, + evidence: AlertEvidence{MetricThreshold: &MetricThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 95, Trigger: 80}}, + want: matchTriple{false, "", ""}, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matches(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} + +// TestBranchcov0724pmMatchesSeverityThreshold covers every branch of +// matchesSeverityThreshold (evaluator.go:518). The existing suite only reaches +// the ThresholdDirectionAbove arm (critical/warning/normal) and only the metric- +// and direction-matching path; this adds the direction/metric mismatch early +// return, the entire ThresholdDirectionBelow arm, and the unknown-direction +// default arm. +func TestBranchcov0724pmMatchesSeverityThreshold(t *testing.T) { + spec := SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90} + + cases := []struct { + name string + spec SeverityThresholdSpec + evidence SeverityThresholdEvidence + want matchTriple + }{ + // Direction mismatch short-circuits before any threshold comparison. + {"direction mismatch returns false empty", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionBelow, Observed: 95}, matchTriple{false, "", ""}}, + // Metric mismatch (direction equal) short-circuits via the || operand. + {"metric mismatch returns false empty", + spec, SeverityThresholdEvidence{Metric: "mem", Direction: ThresholdDirectionAbove, Observed: 95}, matchTriple{false, "", ""}}, + + // ---- Above direction: at / above critical, warning, normal ---- + {"above at critical boundary", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 90}, matchTriple{true, AlertSeverityCritical, "severity-threshold-critical"}}, + {"above above critical", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 99}, matchTriple{true, AlertSeverityCritical, "severity-threshold-critical"}}, + {"above just below critical hits warning", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 89}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + {"above at warning boundary", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 80}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + {"above below warning is normal", + spec, SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 79}, matchTriple{false, "", "severity-threshold-normal"}}, + // Critical threshold disabled (<=0) -> only warning is consulted. + {"above with no critical warning only", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 85}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + // Both thresholds disabled (<=0) -> default normal within Above. + {"above with no thresholds is normal", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 999}, matchTriple{false, "", "severity-threshold-normal"}}, + + // ---- Below direction: at / below critical, warning, normal ---- + {"below at critical boundary", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 10}, matchTriple{true, AlertSeverityCritical, "severity-threshold-critical"}}, + {"below under critical", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 5}, matchTriple{true, AlertSeverityCritical, "severity-threshold-critical"}}, + {"below just above critical hits warning", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 11}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + {"below at warning boundary", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 20}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + {"below above warning is normal", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 21}, matchTriple{false, "", "severity-threshold-normal"}}, + // Below with no critical -> warning only. + {"below with no critical warning only", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 15}, matchTriple{true, AlertSeverityWarning, "severity-threshold-warning"}}, + + // ---- Unknown direction default arm ---- + {"unknown direction returns false empty", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirection("sideways"), Warning: 80, Critical: 90}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirection("sideways"), Observed: 95}, matchTriple{false, "", ""}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matchesSeverityThreshold(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} + +// TestBranchcov0724pmSeverityThresholdStillLatched covers every branch of +// severityThresholdStillLatched (evaluator.go:547). The existing suite only +// reaches the Above arm of the switch; this adds the nil-Recovery early return, +// the direction/metric mismatch return, the Below arm (latched and unlatched), +// and the unknown-direction default arm. +func TestBranchcov0724pmSeverityThresholdStillLatched(t *testing.T) { + aboveRecovery := 85.0 + belowRecovery := 15.0 + + cases := []struct { + name string + spec SeverityThresholdSpec + evidence SeverityThresholdEvidence + want bool + }{ + // nil Recovery -> immediate false (no hysteresis configured). + {"nil recovery returns false", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 88}, false}, + + // Direction mismatch -> false. + {"direction mismatch returns false", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionBelow, Observed: 88}, false}, + // Metric mismatch -> false. + {"metric mismatch returns false", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "mem", Direction: ThresholdDirectionAbove, Observed: 88}, false}, + + // Above: observed >= recovery latches. + {"above observed at recovery latches", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 85}, true}, + {"above observed above recovery latches", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 88}, true}, + {"above observed below recovery unlatches", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirectionAbove, Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirectionAbove, Observed: 84}, false}, + + // Below: observed <= recovery latches. + {"below observed at recovery latches", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10, Recovery: &belowRecovery}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 15}, true}, + {"below observed under recovery latches", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10, Recovery: &belowRecovery}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 12}, true}, + {"below observed above recovery unlatches", + SeverityThresholdSpec{Metric: "disk", Direction: ThresholdDirectionBelow, Warning: 20, Critical: 10, Recovery: &belowRecovery}, + SeverityThresholdEvidence{Metric: "disk", Direction: ThresholdDirectionBelow, Observed: 16}, false}, + + // Unknown direction -> default false. + {"unknown direction returns false", + SeverityThresholdSpec{Metric: "cpu", Direction: ThresholdDirection("sideways"), Warning: 80, Critical: 90, Recovery: &aboveRecovery}, + SeverityThresholdEvidence{Metric: "cpu", Direction: ThresholdDirection("sideways"), Observed: 88}, false}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := severityThresholdStillLatched(tc.spec, tc.evidence); got != tc.want { + t.Fatalf("severityThresholdStillLatched() = %v, want %v", got, tc.want) + } + }) + } +} + +// TestBranchcov0724pmMatchesChangeThreshold covers every return arm of +// matchesChangeThreshold (evaluator.go:565). The existing suite reaches the +// warning-current and growth-critical arms; this adds the metric mismatch, the +// critical-current arm, the nil/non-positive PreviousObserved early normal, the +// growth-warning arm, the percent-gate-skip arms (CriticalPercent/WarningPercent +// <= 0), the percent-not-met fall-throughs, and the final normal return. +func TestBranchcov0724pmMatchesChangeThreshold(t *testing.T) { + fullSpec := ChangeThresholdSpec{ + Metric: "spam", + WarningCurrent: 2000, + CriticalCurrent: 5000, + WarningDelta: 250, + CriticalDelta: 500, + WarningPercent: 25, + CriticalPercent: 50, + } + prev1000 := 1000.0 + prev2000 := 2000.0 + prev100 := 100.0 + prev0 := 0.0 + + cases := []struct { + name string + spec ChangeThresholdSpec + evidence ChangeThresholdEvidence + want matchTriple + }{ + // Metric mismatch short-circuits. + {"metric mismatch returns false empty", + fullSpec, ChangeThresholdEvidence{Metric: "virus", Observed: 2500}, matchTriple{false, "", ""}}, + + // Absolute current thresholds. + {"critical current at boundary", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 5000}, matchTriple{true, AlertSeverityCritical, "change-threshold-current-critical"}}, + {"warning current at boundary", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 2000}, matchTriple{true, AlertSeverityWarning, "change-threshold-current-warning"}}, + + // PreviousObserved absent or non-positive -> normal (both || operands). + // Observed is kept below the current thresholds so the current checks do + // not short-circuit before the PreviousObserved guard runs. + {"nil previous observed returns normal", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 100}, matchTriple{false, "", "change-threshold-normal"}}, + {"zero previous observed returns normal", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 100, PreviousObserved: &prev0}, matchTriple{false, "", "change-threshold-normal"}}, + {"negative previous observed returns normal", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 100, PreviousObserved: &[]float64{-5}[0]}, matchTriple{false, "", "change-threshold-normal"}}, + + // Growth delta + percent met. + {"growth critical delta and percent met", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 1600, PreviousObserved: &prev1000}, matchTriple{true, AlertSeverityCritical, "change-threshold-growth-critical"}}, + {"growth warning delta and percent met", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 1300, PreviousObserved: &prev1000}, matchTriple{true, AlertSeverityWarning, "change-threshold-growth-warning"}}, + + // Delta met but percent gate not met -> falls through to next check / normal. + // Uses dedicated specs with no current thresholds so the current guards + // cannot mask the percent-gate behaviour. + {"critical delta met but percent below gate falls to warning", + ChangeThresholdSpec{Metric: "spam", WarningDelta: 250, CriticalDelta: 500, WarningPercent: 25, CriticalPercent: 50}, + ChangeThresholdEvidence{Metric: "spam", Observed: 2500, PreviousObserved: &prev2000}, matchTriple{true, AlertSeverityWarning, "change-threshold-growth-warning"}}, + {"warning delta met but percent below gate falls to normal", + ChangeThresholdSpec{Metric: "spam", WarningDelta: 250, WarningPercent: 25}, + ChangeThresholdEvidence{Metric: "spam", Observed: 2250, PreviousObserved: &prev2000}, matchTriple{false, "", "change-threshold-normal"}}, + {"no delta met returns normal", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 1050, PreviousObserved: &prev1000}, matchTriple{false, "", "change-threshold-normal"}}, + + // Percent gate disabled (<=0) -> any delta meeting the threshold fires. + {"critical with no percent gate fires on delta only", + ChangeThresholdSpec{Metric: "spam", CriticalDelta: 500, CriticalPercent: 0}, + ChangeThresholdEvidence{Metric: "spam", Observed: 600, PreviousObserved: &prev100}, matchTriple{true, AlertSeverityCritical, "change-threshold-growth-critical"}}, + {"warning with no percent gate fires on delta only", + ChangeThresholdSpec{Metric: "spam", WarningDelta: 50, WarningPercent: 0}, + ChangeThresholdEvidence{Metric: "spam", Observed: 200, PreviousObserved: &prev100}, matchTriple{true, AlertSeverityWarning, "change-threshold-growth-warning"}}, + + // Small previous observed to exercise large percent swings without + // tripping any current threshold. + {"large percent growth but delta under thresholds returns normal", + fullSpec, ChangeThresholdEvidence{Metric: "spam", Observed: 130, PreviousObserved: &prev100}, matchTriple{false, "", "change-threshold-normal"}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matchesChangeThreshold(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} + +// TestBranchcov0724pmMatchesBaselineAnomaly covers every return arm of +// matchesBaselineAnomaly (evaluator.go:598). The existing suite reaches the +// quiet-critical and normal-baseline critical arms; this adds the metric +// mismatch, the baseline==0 clamp, the quiet-warning and quiet-normal arms, the +// baseline<=0 arm (reachable only when QuietBaseline==0), and the normal-baseline +// warning and normal arms. +func TestBranchcov0724pmMatchesBaselineAnomaly(t *testing.T) { + spec := BaselineAnomalySpec{ + Metric: "spamIn", + QuietBaseline: 40, + WarningRatio: 1.8, + CriticalRatio: 2.5, + WarningDelta: 150, + CriticalDelta: 300, + QuietWarningDelta: 60, + QuietCriticalDelta: 120, + } + + cases := []struct { + name string + spec BaselineAnomalySpec + evidence BaselineAnomalyEvidence + want matchTriple + }{ + // Metric mismatch short-circuits. + {"metric mismatch returns false empty", + spec, BaselineAnomalyEvidence{Metric: "virus", Observed: 420, Baseline: 100}, matchTriple{false, "", ""}}, + + // ---- Quiet site (baseline < QuietBaseline) ---- + {"quiet critical delta", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 140, Baseline: 10}, matchTriple{true, AlertSeverityCritical, "baseline-anomaly-quiet-critical"}}, + {"quiet warning delta", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 80, Baseline: 10}, matchTriple{true, AlertSeverityWarning, "baseline-anomaly-quiet-warning"}}, + {"quiet below warning delta is normal", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 50, Baseline: 10}, matchTriple{false, "", "baseline-anomaly-normal"}}, + + // baseline == 0 with observed > 0 is clamped to 1, then treated as quiet. + // delta = observed - 1; pick observed so delta meets quiet warning. + {"baseline zero clamps to one and enters quiet branch", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 61, Baseline: 0}, matchTriple{true, AlertSeverityWarning, "baseline-anomaly-quiet-warning"}}, + + // ---- Normal site (baseline >= QuietBaseline) ---- + {"normal critical ratio and delta", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 420, Baseline: 100}, matchTriple{true, AlertSeverityCritical, "baseline-anomaly-critical"}}, + {"normal warning ratio and delta", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 250, Baseline: 100}, matchTriple{true, AlertSeverityWarning, "baseline-anomaly-warning"}}, + {"normal ratio below thresholds is normal", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 150, Baseline: 100}, matchTriple{false, "", "baseline-anomaly-normal"}}, + // Ratio meets critical but delta below critical; ratio also meets warning + // but delta below warning -> normal (proves the `&& delta >=` is required). + {"ratio high but delta below thresholds is normal", + spec, BaselineAnomalyEvidence{Metric: "spamIn", Observed: 240, Baseline: 100}, matchTriple{false, "", "baseline-anomaly-normal"}}, + + // QuietBaseline == 0 with baseline 0 and observed 0: the quiet branch is + // skipped (0 < 0 is false) and the `baseline <= 0` arm fires. + {"zero baseline zero observed with zero quiet threshold returns normal", + BaselineAnomalySpec{Metric: "spamIn", QuietBaseline: 0, QuietWarningDelta: 60, QuietCriticalDelta: 120, WarningDelta: 150, CriticalDelta: 300, WarningRatio: 1.8, CriticalRatio: 2.5}, + BaselineAnomalyEvidence{Metric: "spamIn", Observed: 0, Baseline: 0}, matchTriple{false, "", "baseline-anomaly-normal"}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matchesBaselineAnomaly(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} + +// TestBranchcov0724pmMatchesHealthAssessment covers every return arm of +// matchesHealthAssessment (evaluator.go:636). The existing suite reaches the +// match arm and the empty-codes/severity normal arm; this adds the signal- +// mismatch arm, the empty-spec-codes wildcard match arm, and the no-overlap +// normal arm. +func TestBranchcov0724pmMatchesHealthAssessment(t *testing.T) { + spec := HealthAssessmentSpec{Signal: "host-raid", Codes: []string{"raid_degraded", "raid_rebuilding"}} + + cases := []struct { + name string + spec HealthAssessmentSpec + evidence HealthAssessmentEvidence + want matchTriple + }{ + // Signal mismatch short-circuits with its own reason. + {"signal mismatch returns false with mismatch reason", + spec, HealthAssessmentEvidence{Signal: "host-disk", Severity: AlertSeverityWarning, Codes: []string{"raid_degraded"}}, + matchTriple{false, "", "health-assessment-signal-mismatch"}}, + + // Empty observed codes -> normal. + {"empty evidence codes returns normal", + spec, HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityWarning}, matchTriple{false, "", "health-assessment-normal"}}, + // Codes present but severity empty -> normal. (Reachable by direct call; + // Evaluate's evidence validation forbids codes-without-severity, so this + // arm is otherwise unreachable through the public path.) + {"codes present but empty severity returns normal", + spec, HealthAssessmentEvidence{Signal: "host-raid", Codes: []string{"raid_degraded"}}, matchTriple{false, "", "health-assessment-normal"}}, + + // Spec with no code filter -> any evidence code/severity matches. + {"empty spec codes matches any evidence severity", + HealthAssessmentSpec{Signal: "host-raid"}, + HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityCritical, Codes: []string{"anything"}}, matchTriple{true, AlertSeverityCritical, "health-assessment-match"}}, + + // Overlap present -> match at evidence severity. + {"observed code in spec set matches", + spec, HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityWarning, Codes: []string{"raid_rebuilding"}}, matchTriple{true, AlertSeverityWarning, "health-assessment-match"}}, + // First observed code misses but a later one hits -> still matches. + {"later observed code in spec set matches", + spec, HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityCritical, Codes: []string{"raid_unknown", "raid_degraded"}}, matchTriple{true, AlertSeverityCritical, "health-assessment-match"}}, + // No overlap -> normal. + {"no observed code in spec set returns normal", + spec, HealthAssessmentEvidence{Signal: "host-raid", Severity: AlertSeverityWarning, Codes: []string{"disk_worn", "psu_fail"}}, matchTriple{false, "", "health-assessment-normal"}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matchesHealthAssessment(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} + +// TestBranchcov0724pmMatchesPostureThreshold covers every return arm of +// matchesPostureThreshold (evaluator.go:658). The existing suite reaches the +// size-critical and age-warning arms; this adds the age/size metric-mismatch +// returns, the size-missing return, every severity combination in the final +// switch (age+size critical, age-only critical, age+size warning, size-only +// warning, default normal), and the per-dimension disabled-metric guards. +func TestBranchcov0724pmMatchesPostureThreshold(t *testing.T) { + fullSpec := PostureThresholdSpec{AgeMetric: "snapshot-age-days", WarningAge: 7, CriticalAge: 14, SizeMetric: "snapshot-size-gib", WarningSize: 50, CriticalSize: 100} + small := 20.0 + large := 120.0 + mid := 60.0 + + cases := []struct { + name string + spec PostureThresholdSpec + evidence PostureThresholdEvidence + want matchTriple + }{ + // ---- Metric mismatches (each dimension independently) ---- + {"age metric mismatch returns false", + fullSpec, PostureThresholdEvidence{AgeMetric: "wrong-age", AgeValue: 20, SizeMetric: "snapshot-size-gib", SizeValue: &small}, + matchTriple{false, "", "posture-threshold-age-metric-mismatch"}}, + {"size metric mismatch returns false", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 20, SizeMetric: "wrong-size", SizeValue: &small}, + matchTriple{false, "", "posture-threshold-size-metric-mismatch"}}, + {"size metric set but size value nil returns false", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 20, SizeMetric: "snapshot-size-gib", SizeValue: nil}, + matchTriple{false, "", "posture-threshold-size-missing"}}, + + // ---- Final switch: every severity combination ---- + {"age and size both critical", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 20, SizeMetric: "snapshot-size-gib", SizeValue: &large}, + matchTriple{true, AlertSeverityCritical, "posture-threshold-critical"}}, + {"age critical size not", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 20, SizeMetric: "snapshot-size-gib", SizeValue: &small}, + matchTriple{true, AlertSeverityCritical, "posture-threshold-age-critical"}}, + {"size critical age not", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 2, SizeMetric: "snapshot-size-gib", SizeValue: &large}, + matchTriple{true, AlertSeverityCritical, "posture-threshold-size-critical"}}, + {"age and size both warning", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 10, SizeMetric: "snapshot-size-gib", SizeValue: &mid}, + matchTriple{true, AlertSeverityWarning, "posture-threshold-warning"}}, + {"age warning size not", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 10, SizeMetric: "snapshot-size-gib", SizeValue: &small}, + matchTriple{true, AlertSeverityWarning, "posture-threshold-age-warning"}}, + {"size warning age not", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 2, SizeMetric: "snapshot-size-gib", SizeValue: &mid}, + matchTriple{true, AlertSeverityWarning, "posture-threshold-size-warning"}}, + {"neither age nor size breach is normal", + fullSpec, PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 2, SizeMetric: "snapshot-size-gib", SizeValue: &small}, + matchTriple{false, "", "posture-threshold-normal"}}, + + // ---- Per-dimension disabled guards ---- + // Age metric empty -> age dimension skipped entirely; only size evaluated. + {"only size dimension configured size warning", + PostureThresholdSpec{SizeMetric: "snapshot-size-gib", WarningSize: 50, CriticalSize: 100}, + PostureThresholdEvidence{SizeMetric: "snapshot-size-gib", SizeValue: &mid}, matchTriple{true, AlertSeverityWarning, "posture-threshold-size-warning"}}, + // Size metric empty -> size dimension skipped; only age evaluated. + {"only age dimension configured age critical", + PostureThresholdSpec{AgeMetric: "snapshot-age-days", WarningAge: 7, CriticalAge: 14}, + PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 20}, matchTriple{true, AlertSeverityCritical, "posture-threshold-age-critical"}}, + // Size metric empty -> the size branch is not entered, so a nil SizeValue + // must NOT trigger the size-missing return. + {"no size metric tolerates nil size value", + PostureThresholdSpec{AgeMetric: "snapshot-age-days", WarningAge: 7, CriticalAge: 14}, + PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 3, SizeValue: nil}, matchTriple{false, "", "posture-threshold-normal"}}, + // Critical threshold disabled (<=0) -> warning is consulted for that dim. + {"age warning when critical age disabled", + PostureThresholdSpec{AgeMetric: "snapshot-age-days", WarningAge: 7}, + PostureThresholdEvidence{AgeMetric: "snapshot-age-days", AgeValue: 10}, matchTriple{true, AlertSeverityWarning, "posture-threshold-age-warning"}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + matched, severity, reason := matchesPostureThreshold(tc.spec, tc.evidence) + assertMatchTriple(t, matchTriple{matched, severity, reason}, tc.want) + }) + } +} diff --git a/internal/notifications/email_message_branchcov0724pm_test.go b/internal/notifications/email_message_branchcov0724pm_test.go new file mode 100644 index 000000000..961927ce6 --- /dev/null +++ b/internal/notifications/email_message_branchcov0724pm_test.go @@ -0,0 +1,631 @@ +package notifications + +import ( + "bytes" + "errors" + "io" + "mime" + "mime/multipart" + "mime/quotedprintable" + "net/mail" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" +) + +// branchcovFailWriter is an io.Writer that allows the first `ok` Write calls to +// succeed and then fails every subsequent Write. It lets us exercise the error +// arms of writeMultipartBodyPart that are unreachable when the underlying writer +// is a *bytes.Buffer (whose Write never errors). +type branchcovFailWriter struct { + ok int + written int +} + +func (w *branchcovFailWriter) Write(p []byte) (int, error) { + w.written++ + if w.written > w.ok { + return 0, errors.New("branchcov: write blocked") + } + return len(p), nil +} + +func branchcovBasicAddresses() resolvedEmailAddresses { + return resolvedEmailAddresses{ + from: &mail.Address{Address: "sender@example.com"}, + to: []*mail.Address{{Address: "recipient@example.com"}}, + } +} + +func branchcovAddressesWithReplyTo() resolvedEmailAddresses { + return resolvedEmailAddresses{ + from: &mail.Address{Address: "sender@example.com"}, + to: []*mail.Address{{Address: "recipient@example.com"}}, + replyTo: &mail.Address{Address: "replies@example.com"}, + } +} + +// --- writeMultipartBodyPart (email_enhanced.go:171) --- + +func TestBranchcov0724pmWriteMultipartBodyPart(t *testing.T) { + t.Run("CreatePartError", func(t *testing.T) { + // A writer that fails on the very first Write so multipart.Writer.CreatePart + // cannot even emit the part boundary/headers. + fw := &branchcovFailWriter{ok: 0} + mw := multipart.NewWriter(fw) + + err := writeMultipartBodyPart(mw, "text/plain", "body") + if err == nil { + t.Fatal("expected create-part error, got nil") + } + if !strings.Contains(err.Error(), "create text/plain part") { + t.Fatalf("error should mention create text/plain part, got %v", err) + } + if !strings.Contains(err.Error(), "write blocked") { + t.Fatalf("error should wrap underlying write failure, got %v", err) + } + }) + + t.Run("EncodeError", func(t *testing.T) { + // Allow exactly one Write (the CreatePart header block) to succeed, then + // fail. A body longer than one quoted-printable line (76 chars) forces a + // flush during encoder.Write, hitting the encode-error arm. + fw := &branchcovFailWriter{ok: 1} + mw := multipart.NewWriter(fw) + largeBody := strings.Repeat("x", 200) + + err := writeMultipartBodyPart(mw, "text/html", largeBody) + if err == nil { + t.Fatal("expected encode error, got nil") + } + if !strings.Contains(err.Error(), "encode text/html part") { + t.Fatalf("error should mention encode text/html part, got %v", err) + } + }) + + t.Run("FinalizeError", func(t *testing.T) { + // Allow exactly one Write (CreatePart headers) to succeed. A short body + // (2 chars) stays buffered inside the quoted-printable encoder during + // Write; the buffer is only flushed on Close, hitting the finalize arm. + fw := &branchcovFailWriter{ok: 1} + mw := multipart.NewWriter(fw) + + err := writeMultipartBodyPart(mw, "text/plain", "hi") + if err == nil { + t.Fatal("expected finalize error, got nil") + } + if !strings.Contains(err.Error(), "finalize text/plain part") { + t.Fatalf("error should mention finalize text/plain part, got %v", err) + } + }) + + t.Run("SuccessSetsHeadersAndEncodesBody", func(t *testing.T) { + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + + if err := writeMultipartBodyPart(mw, "text/plain", "hello world"); err != nil { + t.Fatalf("writeMultipartBodyPart() error = %v", err) + } + if err := mw.Close(); err != nil { + t.Fatalf("close multipart writer: %v", err) + } + + reader := multipart.NewReader(&buf, mw.Boundary()) + part, err := reader.NextRawPart() + if err != nil { + t.Fatalf("NextRawPart() error = %v", err) + } + ct := part.Header.Get("Content-Type") + if !strings.Contains(ct, "text/plain") || !strings.Contains(ct, "charset=UTF-8") { + t.Errorf("Content-Type = %q, want text/plain with charset=UTF-8", ct) + } + if cte := part.Header.Get("Content-Transfer-Encoding"); cte != "quoted-printable" { + t.Errorf("Content-Transfer-Encoding = %q, want quoted-printable", cte) + } + decoded, err := io.ReadAll(quotedprintable.NewReader(part)) + if err != nil { + t.Fatalf("ReadAll(quotedprintable) error = %v", err) + } + if string(decoded) != normalizeEmailBodyLineEndings("hello world") { + t.Errorf("decoded body = %q, want %q", decoded, normalizeEmailBodyLineEndings("hello world")) + } + }) +} + +// --- writeEmailThreadingHeaders (email_enhanced.go:204) --- +// +// The two fmt.Fprintf error arms (lines 210 and 213) are provably unreachable: +// the function signature accepts *bytes.Buffer, and bytes.Buffer.Write / +// WriteString never return an error. These subtests cover every reachable arm. + +func TestBranchcov0724pmWriteEmailThreadingHeaders(t *testing.T) { + t.Run("EmptyThreadIDWritesNothing", func(t *testing.T) { + var buf bytes.Buffer + if err := writeEmailThreadingHeaders(&buf, ""); err != nil { + t.Fatalf("writeEmailThreadingHeaders() error = %v", err) + } + if buf.Len() != 0 { + t.Fatalf("buffer should be empty for blank threadID, got %q", buf.String()) + } + }) + + t.Run("WhitespaceOnlyThreadIDSanitizedToEmpty", func(t *testing.T) { + var buf bytes.Buffer + // CR/LF/spaces are all sanitized away, leaving an empty threadID. + if err := writeEmailThreadingHeaders(&buf, " \r\n "); err != nil { + t.Fatalf("writeEmailThreadingHeaders() error = %v", err) + } + if buf.Len() != 0 { + t.Fatalf("buffer should be empty for whitespace-only threadID, got %q", buf.String()) + } + }) + + t.Run("ValidThreadIDWritesBothHeaders", func(t *testing.T) { + var buf bytes.Buffer + threadID := "" + if err := writeEmailThreadingHeaders(&buf, threadID); err != nil { + t.Fatalf("writeEmailThreadingHeaders() error = %v", err) + } + raw := buf.String() + if !strings.Contains(raw, "In-Reply-To: "+threadID+"\r\n") { + t.Errorf("missing In-Reply-To header in:\n%s", raw) + } + if !strings.Contains(raw, "References: "+threadID+"\r\n") { + t.Errorf("missing References header in:\n%s", raw) + } + }) + + t.Run("ThreadIDCRLFSanitized", func(t *testing.T) { + var buf bytes.Buffer + // Embedded CRLF must be collapsed to spaces so it cannot inject headers. + if err := writeEmailThreadingHeaders(&buf, ""); err != nil { + t.Fatalf("writeEmailThreadingHeaders() error = %v", err) + } + raw := buf.String() + if strings.Contains(raw, "\r\nBcc:") { + t.Fatalf("CRLF injection produced a Bcc header:\n%s", raw) + } + if !strings.Contains(raw, "") { + t.Fatalf("threadID CR/LF should be replaced with spaces:\n%s", raw) + } + }) +} + +// --- buildMultipartEmailMessage (email_enhanced.go:218) --- +// +// Every uncovered statement in this function is a fmt.Fprintf / WriteString +// error-return arm. The function allocates its own bytes.Buffer locally and +// writes all headers to it; bytes.Buffer never errors, so these arms are +// provably unreachable. The subtests below assert observable MIME behaviour +// for the scenarios requested (empty alternatives, header escaping, threading). + +func TestBranchcov0724pmBuildMultipartEmailMessage(t *testing.T) { + now := time.Unix(1711711711, 1234).UTC() + + t.Run("EmptyTextBody", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessage(addr, "Subject", "

HTML

", "", "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessage() error = %v", err) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + mediaType, params, err := mime.ParseMediaType(parsed.Header.Get("Content-Type")) + if err != nil { + t.Fatalf("ParseMediaType() error = %v", err) + } + if mediaType != "multipart/alternative" { + t.Fatalf("Content-Type = %q, want multipart/alternative", mediaType) + } + reader := multipart.NewReader(parsed.Body, params["boundary"]) + + textPart, err := reader.NextRawPart() + if err != nil { + t.Fatalf("text NextRawPart() error = %v", err) + } + decoded, err := io.ReadAll(quotedprintable.NewReader(textPart)) + if err != nil { + t.Fatalf("ReadAll(text) error = %v", err) + } + if string(decoded) != "" { + t.Errorf("empty text body should decode to empty string, got %q", decoded) + } + + htmlPart, err := reader.NextRawPart() + if err != nil { + t.Fatalf("html NextRawPart() error = %v", err) + } + decodedHTML, err := io.ReadAll(quotedprintable.NewReader(htmlPart)) + if err != nil { + t.Fatalf("ReadAll(html) error = %v", err) + } + if string(decodedHTML) != normalizeEmailBodyLineEndings("

HTML

") { + t.Errorf("html body = %q, want %q", decodedHTML, normalizeEmailBodyLineEndings("

HTML

")) + } + }) + + t.Run("EmptyHTMLBody", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessage(addr, "Subject", "", "plain text body", "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessage() error = %v", err) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + _, params, err := mime.ParseMediaType(parsed.Header.Get("Content-Type")) + if err != nil { + t.Fatalf("ParseMediaType() error = %v", err) + } + reader := multipart.NewReader(parsed.Body, params["boundary"]) + + textPart, err := reader.NextRawPart() + if err != nil { + t.Fatalf("text NextRawPart() error = %v", err) + } + decodedText, err := io.ReadAll(quotedprintable.NewReader(textPart)) + if err != nil { + t.Fatalf("ReadAll(text) error = %v", err) + } + if string(decodedText) != normalizeEmailBodyLineEndings("plain text body") { + t.Errorf("text body = %q, want %q", decodedText, normalizeEmailBodyLineEndings("plain text body")) + } + + htmlPart, err := reader.NextRawPart() + if err != nil { + t.Fatalf("html NextRawPart() error = %v", err) + } + decodedHTML, err := io.ReadAll(quotedprintable.NewReader(htmlPart)) + if err != nil { + t.Fatalf("ReadAll(html) error = %v", err) + } + if string(decodedHTML) != "" { + t.Errorf("empty html body should decode to empty string, got %q", decodedHTML) + } + }) + + t.Run("SubjectCRLFHeaderInjectionSanitized", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessage(addr, "Safe\r\nBcc: evil@example.com", "

hi

", "hi", "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessage() error = %v", err) + } + raw := string(msg) + if strings.Contains(raw, "\r\nBcc: ") { + t.Fatalf("CRLF injection in subject produced a Bcc header:\n%s", raw) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + wantSubject := "Safe Bcc: evil@example.com" + if got := parsed.Header.Get("Subject"); got != wantSubject { + t.Errorf("Subject = %q, want %q (CR/LF replaced with spaces)", got, wantSubject) + } + }) + + t.Run("MissingThreadingReferencesProduceNoHeaders", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessage(addr, "Alert", "

hi

", "hi", "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessage() error = %v", err) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + if got := parsed.Header.Get("In-Reply-To"); got != "" { + t.Errorf("In-Reply-To should be absent for empty threadID, got %q", got) + } + if got := parsed.Header.Get("References"); got != "" { + t.Errorf("References should be absent for empty threadID, got %q", got) + } + if got := parsed.Header.Get("Message-ID"); got == "" { + t.Error("Message-ID should always be present") + } + }) + + t.Run("NoReplyToOmitsHeader", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessage(addr, "Subject", "

hi

", "hi", "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessage() error = %v", err) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + if got := parsed.Header.Get("Reply-To"); got != "" { + t.Errorf("Reply-To should be absent when replyTo is nil, got %q", got) + } + }) +} + +// --- buildMultipartEmailMessageWithAttachments (email_enhanced.go:274) --- + +func TestBranchcov0724pmBuildMultipartEmailMessageWithAttachments(t *testing.T) { + now := time.Unix(1711711711, 1234).UTC() + + t.Run("ZeroAttachmentsDelegatesToAlternative", func(t *testing.T) { + addr := branchcovBasicAddresses() + msg, err := buildMultipartEmailMessageWithAttachments(addr, "Subject", "

hi

", "hi", nil, "", now) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if !strings.Contains(raw, "multipart/alternative") { + t.Errorf("zero-attachment message should use multipart/alternative:\n%s", raw) + } + if strings.Contains(raw, "multipart/mixed") { + t.Errorf("zero-attachment message should not use multipart/mixed:\n%s", raw) + } + if strings.Contains(raw, "Content-Disposition: attachment") { + t.Errorf("zero-attachment message should not contain attachment disposition:\n%s", raw) + } + }) + + t.Run("EmptyAttachmentDataSkipped", func(t *testing.T) { + addr := branchcovBasicAddresses() + empty := EmailAttachment{Filename: "empty.bin", ContentType: "application/octet-stream", Data: nil} + real := EmailAttachment{Filename: "real.txt", ContentType: "text/plain", Data: []byte("payload")} + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{empty, real}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if strings.Contains(raw, `filename="empty.bin"`) { + t.Errorf("attachment with empty data should be skipped:\n%s", raw) + } + if !strings.Contains(raw, `filename="real.txt"`) { + t.Errorf("real attachment should be present:\n%s", raw) + } + // Exactly one Content-Disposition header (for the real attachment only). + if c := strings.Count(raw, "Content-Disposition: attachment"); c != 1 { + t.Errorf("expected 1 attachment disposition, got %d:\n%s", c, raw) + } + }) + + t.Run("EmptyFilenameDefaultsToAttachment", func(t *testing.T) { + addr := branchcovBasicAddresses() + att := EmailAttachment{Filename: "", ContentType: "text/plain", Data: []byte("data")} + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{att}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if !strings.Contains(raw, `filename="attachment"`) { + t.Errorf("empty filename should default to \"attachment\":\n%s", raw) + } + if !strings.Contains(raw, `name="attachment"`) { + t.Errorf("Content-Type name should also default to \"attachment\":\n%s", raw) + } + }) + + t.Run("WhitespaceFilenameDefaultsToAttachment", func(t *testing.T) { + addr := branchcovBasicAddresses() + att := EmailAttachment{Filename: " \r\n ", ContentType: "text/plain", Data: []byte("data")} + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{att}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if !strings.Contains(raw, `filename="attachment"`) { + t.Errorf("whitespace-only filename should default to \"attachment\":\n%s", raw) + } + }) + + t.Run("EmptyContentTypeDefaultsToOctetStream", func(t *testing.T) { + addr := branchcovBasicAddresses() + att := EmailAttachment{Filename: "mystery.bin", ContentType: "", Data: []byte("data")} + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{att}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if !strings.Contains(raw, "application/octet-stream") { + t.Errorf("empty content type should default to application/octet-stream:\n%s", raw) + } + }) + + t.Run("ReplyToHeaderPresent", func(t *testing.T) { + addr := branchcovAddressesWithReplyTo() + att := EmailAttachment{Filename: "file.txt", ContentType: "text/plain", Data: []byte("data")} + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{att}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + parsed, err := mail.ReadMessage(bytes.NewReader(msg)) + if err != nil { + t.Fatalf("mail.ReadMessage() error = %v", err) + } + if got := parsed.Header.Get("Reply-To"); got != addr.replyTo.String() { + t.Errorf("Reply-To = %q, want %q", got, addr.replyTo.String()) + } + }) + + t.Run("FilenameCRLFSanitized", func(t *testing.T) { + addr := branchcovBasicAddresses() + att := EmailAttachment{ + Filename: "report\r\nBcc: evil.pdf", + ContentType: "application/pdf", + Data: []byte("%PDF-1.7"), + } + msg, err := buildMultipartEmailMessageWithAttachments( + addr, "Subject", "

hi

", "hi", + []EmailAttachment{att}, "", now, + ) + if err != nil { + t.Fatalf("buildMultipartEmailMessageWithAttachments() error = %v", err) + } + raw := string(msg) + if strings.Contains(raw, "\r\nBcc:") { + t.Fatalf("CRLF in filename injected a Bcc header:\n%s", raw) + } + if !strings.Contains(raw, `filename="report Bcc: evil.pdf"`) { + t.Errorf("filename CR/LF should be replaced with spaces:\n%s", raw) + } + }) +} + +// --- alertNodeDisplay (email_template.go:33) --- + +func TestBranchcov0724pmAlertNodeDisplay(t *testing.T) { + t.Run("DisplayNameTakesPrecedence", func(t *testing.T) { + alert := &alerts.Alert{Node: "node-raw", NodeDisplayName: "Pretty Name"} + got := alertNodeDisplay(alert) + if got != "Pretty Name" { + t.Errorf("alertNodeDisplay() = %q, want %q (display name should win)", got, "Pretty Name") + } + }) + + t.Run("FallsBackToNodeWhenDisplayNameEmpty", func(t *testing.T) { + alert := &alerts.Alert{Node: "node-raw", NodeDisplayName: ""} + got := alertNodeDisplay(alert) + if got != "node-raw" { + t.Errorf("alertNodeDisplay() = %q, want %q (should fall back to Node)", got, "node-raw") + } + }) + + t.Run("BothBlankReturnsEmpty", func(t *testing.T) { + alert := &alerts.Alert{Node: "", NodeDisplayName: ""} + got := alertNodeDisplay(alert) + if got != "" { + t.Errorf("alertNodeDisplay() = %q, want empty string", got) + } + }) +} + +// --- copyWebhookConfig (notifications.go:324) --- +// +// The `len(clones) == 0` early-return arm is provably unreachable: +// copyWebhookConfig always passes a one-element slice to copyWebhookConfigs, +// which always returns a one-element slice for non-empty input. + +func TestBranchcov0724pmCopyWebhookConfig(t *testing.T) { + t.Run("HeadersAndCustomFieldsIndependent", func(t *testing.T) { + src := WebhookConfig{ + ID: "wh-1", + Name: "original", + URL: "https://example.com/hook", + Method: "POST", + Headers: map[string]string{"Authorization": "Bearer token", "X-Custom": "val"}, + CustomFields: map[string]string{"channel": "general"}, + SigningSecret: "secret", + } + clone := copyWebhookConfig(src) + + // Mutate the clone in every map-bearing field. + clone.Name = "modified" + clone.Headers["Authorization"] = "Bearer changed" + delete(clone.Headers, "X-Custom") + clone.Headers["New-Key"] = "added" + clone.CustomFields["channel"] = "changed" + clone.CustomFields["new-key"] = "added" + clone.SigningSecret = "leaked" + + // Assert the original is completely unchanged. + if src.Name != "original" { + t.Errorf("src.Name = %q, want %q", src.Name, "original") + } + if src.Headers["Authorization"] != "Bearer token" { + t.Errorf("src.Headers[Authorization] = %q, want %q", src.Headers["Authorization"], "Bearer token") + } + if _, ok := src.Headers["X-Custom"]; !ok { + t.Error("src.Headers[X-Custom] was deleted through the clone (shared map)") + } + if _, ok := src.Headers["New-Key"]; ok { + t.Error("src.Headers[New-Key] was added through the clone (shared map)") + } + if src.CustomFields["channel"] != "general" { + t.Errorf("src.CustomFields[channel] = %q, want %q", src.CustomFields["channel"], "general") + } + if _, ok := src.CustomFields["new-key"]; ok { + t.Error("src.CustomFields[new-key] was added through the clone (shared map)") + } + if src.SigningSecret != "secret" { + t.Errorf("src.SigningSecret = %q, want %q", src.SigningSecret, "secret") + } + + // Assert the clone carries the modified values. + if clone.Name != "modified" { + t.Errorf("clone.Name = %q, want %q", clone.Name, "modified") + } + if clone.Headers["Authorization"] != "Bearer changed" { + t.Errorf("clone.Headers[Authorization] = %q, want %q", clone.Headers["Authorization"], "Bearer changed") + } + if clone.CustomFields["channel"] != "changed" { + t.Errorf("clone.CustomFields[channel] = %q, want %q", clone.CustomFields["channel"], "changed") + } + }) + + t.Run("AllScalarFieldsPreserved", func(t *testing.T) { + src := WebhookConfig{ + ID: "wh-2", + Name: "test", + URL: "https://example.com", + Method: "PUT", + Enabled: true, + Service: "slack", + Template: "tmpl", + Mention: "@here", + SigningSecret: "abc123", + Headers: map[string]string{"X-Test": "1"}, + CustomFields: map[string]string{"key": "val"}, + } + clone := copyWebhookConfig(src) + + if clone.ID != src.ID { + t.Errorf("ID = %q, want %q", clone.ID, src.ID) + } + if clone.Name != src.Name { + t.Errorf("Name = %q, want %q", clone.Name, src.Name) + } + if clone.URL != src.URL { + t.Errorf("URL = %q, want %q", clone.URL, src.URL) + } + if clone.Method != src.Method { + t.Errorf("Method = %q, want %q", clone.Method, src.Method) + } + if clone.Enabled != src.Enabled { + t.Errorf("Enabled = %v, want %v", clone.Enabled, src.Enabled) + } + if clone.Service != src.Service { + t.Errorf("Service = %q, want %q", clone.Service, src.Service) + } + if clone.Template != src.Template { + t.Errorf("Template = %q, want %q", clone.Template, src.Template) + } + if clone.Mention != src.Mention { + t.Errorf("Mention = %q, want %q", clone.Mention, src.Mention) + } + if clone.SigningSecret != src.SigningSecret { + t.Errorf("SigningSecret = %q, want %q", clone.SigningSecret, src.SigningSecret) + } + if clone.Headers["X-Test"] != "1" { + t.Errorf("Headers[X-Test] = %q, want %q", clone.Headers["X-Test"], "1") + } + if clone.CustomFields["key"] != "val" { + t.Errorf("CustomFields[key] = %q, want %q", clone.CustomFields["key"], "val") + } + }) +} diff --git a/internal/unifiedresources/action_dispatch_branchcov0724pm_test.go b/internal/unifiedresources/action_dispatch_branchcov0724pm_test.go new file mode 100644 index 000000000..b951db729 --- /dev/null +++ b/internal/unifiedresources/action_dispatch_branchcov0724pm_test.go @@ -0,0 +1,364 @@ +package unifiedresources + +import ( + "strings" + "testing" + "time" +) + +// Branch/function coverage tests for the three PURE helpers in +// action_dispatch.go that the existing suite reaches only partially: +// - ActionDispatchAttemptID(string) string [action_dispatch.go:65] was 75.0% +// - NewActionDispatchAttempt(string, time.Time) (...) [action_dispatch.go:73] was 75.0% +// - NormalizeActionDispatchReceipt(ActionDispatchReceipt) (ActionDispatchReceipt, error) +// [action_dispatch.go:140] was 72.7% +// +// Each subtest drives a concrete branch/return path the existing tests miss: +// the empty/whitespace identifier arm of ActionDispatchAttemptID, the +// zero-time arm of NewActionDispatchAttempt, and the identity-error, +// absent-optional-field and zero-timestamp arms of NormalizeActionDispatchReceipt, +// plus determinism and idempotence. +// +// Conventions (package clause, table-driven subtests, in-package construction +// of inputs, t.Fatalf/t.Errorf assertions) mirror the sibling +// action_dispatch_bind_branchcov0723am_test.go. No source file or pre-existing +// test is modified; the SQLite-backed store methods in action_dispatch_store.go +// are explicitly out of scope. + +// --------------------------------------------------------------------------- +// ActionDispatchAttemptID [action_dispatch.go:65] +// --------------------------------------------------------------------------- + +// TestBranchcov0724pmActionDispatchAttemptID drives both arms of the if/else +// (empty-after-trim returns ""; otherwise returns actionID+".dispatch.1"), +// exercises strings.TrimSpace on the input, and pins determinism plus input +// sensitivity. +func TestBranchcov0724pmActionDispatchAttemptID(t *testing.T) { + cases := []struct { + name string + actionID string + want string + }{ + { + // Empty arm of `if actionID == ""`: the bare zero input. + name: "EmptyReturnsEmpty", + actionID: "", + want: "", + }, + { + // Empty arm reached via TrimSpace: whitespace-only input must + // collapse to "" and hit the early return, not the suffix path. + name: "WhitespaceOnlyTrimsToEmpty", + actionID: " \t\n", + want: "", + }, + { + // Non-empty arm: concrete canonical id for a real action id. + name: "NonEmptyAppendsDispatchSuffix", + actionID: "vm:7", + want: "vm:7.dispatch.1", + }, + { + // Non-empty arm reached after TrimSpace: surrounding whitespace + // is stripped before the suffix is appended, proving the trim + // runs unconditionally and feeds the suffix path. + name: "WhitespacePaddedIsTrimmedBeforeSuffix", + actionID: " vm:7\t", + want: "vm:7.dispatch.1", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + if got := ActionDispatchAttemptID(tc.actionID); got != tc.want { + t.Fatalf("ActionDispatchAttemptID(%q) = %q, want %q", tc.actionID, got, tc.want) + } + }) + } +} + +// --------------------------------------------------------------------------- +// NewActionDispatchAttempt [action_dispatch.go:73] +// --------------------------------------------------------------------------- + +// TestBranchcov0724pmNewActionDispatchAttempt drives both arms of the +// `if now.IsZero()` conditional: the zero-time arm (now is replaced with +// time.Now().UTC()) and the non-zero arm (now is converted to UTC). It also +// pins the generated id and the action-id trim. +func TestBranchcov0724pmNewActionDispatchAttempt(t *testing.T) { + t.Run("ZeroNowReplacedWithCurrentUTCTime", func(t *testing.T) { + // The zero-time arm: passing time.Time{} must NOT leave CreatedAt at + // the zero value; the constructor substitutes time.Now().UTC(). + before := time.Now().UTC().Add(-time.Second) + got, err := NewActionDispatchAttempt("act-zeronow", time.Time{}) + if err != nil { + t.Fatalf("NewActionDispatchAttempt with zero now unexpected error: %v", err) + } + after := time.Now().UTC().Add(time.Second) + + if got.CreatedAt.IsZero() { + t.Fatal("CreatedAt is zero; zero-time arm did not substitute time.Now().UTC()") + } + if got.CreatedAt.Location() != time.UTC { + t.Fatalf("CreatedAt location = %v, want UTC", got.CreatedAt.Location()) + } + // The substituted instant must fall in the [before, after] window + // around the real clock — a concrete, non-tautological check. + if got.CreatedAt.Before(before) || got.CreatedAt.After(after) { + t.Fatalf("CreatedAt = %v, expected within [%v, %v]", got.CreatedAt, before, after) + } + // UpdatedAt mirrors CreatedAt in the constructor. + if !got.UpdatedAt.Equal(got.CreatedAt) { + t.Fatalf("UpdatedAt = %v, want equal to CreatedAt %v", got.UpdatedAt, got.CreatedAt) + } + // The generated id is the canonical dispatch id for this action. + if got.ID != "act-zeronow.dispatch.1" { + t.Fatalf("ID = %q, want act-zeronow.dispatch.1", got.ID) + } + if got.State != ActionDispatchQueued { + t.Fatalf("State = %q, want %q", got.State, ActionDispatchQueued) + } + }) + + t.Run("NonZeroNowIsConvertedToUTC", func(t *testing.T) { + // The non-zero arm: a non-UTC wall-clock must be normalised to UTC + // without changing the instant. + zone := time.FixedZone("PST", -8*3600) + local := time.Date(2026, 7, 24, 9, 30, 0, 0, zone) // 09:30 PST == 17:30 UTC + got, err := NewActionDispatchAttempt("act-utcnorm", local) + if err != nil { + t.Fatalf("NewActionDispatchAttempt unexpected error: %v", err) + } + wantUTC := local.UTC() + if !got.CreatedAt.Equal(wantUTC) { + t.Fatalf("CreatedAt = %v, want %v (UTC of input)", got.CreatedAt, wantUTC) + } + if got.CreatedAt.Location() != time.UTC { + t.Fatalf("CreatedAt location = %v, want UTC", got.CreatedAt.Location()) + } + if got.CreatedAt.Hour() != 17 { + t.Fatalf("CreatedAt hour = %d, want 17 (09:30 PST -> 17:30 UTC)", got.CreatedAt.Hour()) + } + }) + + t.Run("WhitespaceActionIDIsTrimmed", func(t *testing.T) { + got, err := NewActionDispatchAttempt(" act-trim ", time.Date(2026, 7, 24, 12, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("NewActionDispatchAttempt unexpected error: %v", err) + } + if got.ActionID != "act-trim" { + t.Fatalf("ActionID = %q, want act-trim (trimmed)", got.ActionID) + } + if got.ID != "act-trim.dispatch.1" { + t.Fatalf("ID = %q, want act-trim.dispatch.1", got.ID) + } + }) + + t.Run("EmptyActionIDWithZeroNowErrorsAfterClockSubstitution", func(t *testing.T) { + // Zero now still triggers the clock substitution, but the empty + // action id then fails inside NormalizeActionDispatchAttempt. This + // proves the zero-time arm runs even on the error path. + _, err := NewActionDispatchAttempt("", time.Time{}) + if err == nil { + t.Fatal("expected error for empty action id, got nil") + } + if !strings.Contains(err.Error(), "action dispatch action id required") { + t.Fatalf("error = %q, want substring %q", err.Error(), "action dispatch action id required") + } + }) +} + +// --------------------------------------------------------------------------- +// NormalizeActionDispatchReceipt [action_dispatch.go:140] +// --------------------------------------------------------------------------- + +// TestBranchcov0724pmNormalizeActionDispatchReceipt covers every arm the +// existing store-backed tests miss: both identity-invalid error conditions +// (empty/mismatched), the absent-optional-field defaults (empty +// TransportRequestID -> AttemptID; zero ReceivedAt -> time.Now().UTC()), the +// UTC conversion of a supplied non-zero ReceivedAt, whitespace trimming, and +// idempotence of a second normalization. +func TestBranchcov0724pmNormalizeActionDispatchReceipt(t *testing.T) { + t.Run("Error/EmptyActionIDIsInvalid", func(t *testing.T) { + // First clause of the identity guard: ActionID trims to empty. + _, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + AttemptID: "x.dispatch.1", // value irrelevant; empty ActionID short-circuits + }) + if err == nil { + t.Fatal("expected error for empty ActionID, got nil") + } + if !strings.Contains(err.Error(), "action dispatch receipt identity is invalid") { + t.Fatalf("error = %q, want substring %q", err.Error(), "action dispatch receipt identity is invalid") + } + }) + + t.Run("Error/WhitespaceActionIDTrimsToEmpty", func(t *testing.T) { + _, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: " ", AttemptID: "x.dispatch.1", + }) + if err == nil { + t.Fatal("expected error for whitespace-only ActionID, got nil") + } + if !strings.Contains(err.Error(), "action dispatch receipt identity is invalid") { + t.Fatalf("error = %q, want substring %q", err.Error(), "action dispatch receipt identity is invalid") + } + }) + + t.Run("Error/MismatchedAttemptIDIsInvalid", func(t *testing.T) { + // Second clause: ActionID is valid but AttemptID is not its + // canonical ActionDispatchAttemptID form. + _, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-mismatch", AttemptID: "not-the-canonical-id", + }) + if err == nil { + t.Fatal("expected error for mismatched AttemptID, got nil") + } + if !strings.Contains(err.Error(), "action dispatch receipt identity is invalid") { + t.Fatalf("error = %q, want substring %q", err.Error(), "action dispatch receipt identity is invalid") + } + }) + + t.Run("Error/EmptyAttemptIDWithValidActionIDIsMismatch", func(t *testing.T) { + // Empty AttemptID != ActionDispatchAttemptID(ActionID), so the + // mismatch clause fires (not the empty-ActionID clause). + _, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-noattempt", AttemptID: "", + }) + if err == nil { + t.Fatal("expected error for empty AttemptID, got nil") + } + if !strings.Contains(err.Error(), "action dispatch receipt identity is invalid") { + t.Fatalf("error = %q, want substring %q", err.Error(), "action dispatch receipt identity is invalid") + } + }) + + t.Run("Success/AbsentOptionalFieldsDefaulted", func(t *testing.T) { + // Both optional fields absent: TransportRequestID empty -> AttemptID, + // ReceivedAt zero -> time.Now().UTC(). + before := time.Now().UTC().Add(-time.Second) + got, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-absent", AttemptID: ActionDispatchAttemptID("act-absent"), + // TransportRequestID and ReceivedAt intentionally zero. + }) + if err != nil { + t.Fatalf("NormalizeActionDispatchReceipt unexpected error: %v", err) + } + after := time.Now().UTC().Add(time.Second) + + // Empty TransportRequestID must be defaulted to the AttemptID. + if got.TransportRequestID != got.AttemptID { + t.Fatalf("TransportRequestID = %q, want defaulted to AttemptID %q", got.TransportRequestID, got.AttemptID) + } + if got.TransportRequestID != "act-absent.dispatch.1" { + t.Fatalf("TransportRequestID = %q, want act-absent.dispatch.1", got.TransportRequestID) + } + // Zero ReceivedAt must be replaced with the current UTC instant. + if got.ReceivedAt.IsZero() { + t.Fatal("ReceivedAt is zero; zero-time arm did not substitute time.Now().UTC()") + } + if got.ReceivedAt.Location() != time.UTC { + t.Fatalf("ReceivedAt location = %v, want UTC", got.ReceivedAt.Location()) + } + if got.ReceivedAt.Before(before) || got.ReceivedAt.After(after) { + t.Fatalf("ReceivedAt = %v, expected within [%v, %v]", got.ReceivedAt, before, after) + } + }) + + t.Run("Success/SuppliedFieldsPreservedAndConvertedToUTC", func(t *testing.T) { + // Non-empty TransportRequestID is kept; non-zero ReceivedAt in a + // non-UTC zone is converted to UTC without changing the instant. + zone := time.FixedZone("JST", 9*3600) + local := time.Date(2026, 7, 24, 23, 0, 0, 0, zone) // 23:00 JST == 14:00 UTC + got, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-supplied", + AttemptID: ActionDispatchAttemptID("act-supplied"), + TransportRequestID: "transport-xyz", + ReceivedAt: local, + }) + if err != nil { + t.Fatalf("NormalizeActionDispatchReceipt unexpected error: %v", err) + } + if got.TransportRequestID != "transport-xyz" { + t.Fatalf("TransportRequestID = %q, want transport-xyz (preserved)", got.TransportRequestID) + } + wantUTC := local.UTC() + if !got.ReceivedAt.Equal(wantUTC) { + t.Fatalf("ReceivedAt = %v, want %v (UTC of input)", got.ReceivedAt, wantUTC) + } + if got.ReceivedAt.Location() != time.UTC { + t.Fatalf("ReceivedAt location = %v, want UTC", got.ReceivedAt.Location()) + } + if got.ReceivedAt.Hour() != 14 { + t.Fatalf("ReceivedAt hour = %d, want 14 (23:00 JST -> 14:00 UTC)", got.ReceivedAt.Hour()) + } + }) + + t.Run("Success/WhitespaceFieldsTrimmedAndStillValid", func(t *testing.T) { + // All three string fields are padded; after trim the identity still + // matches and the transport id is preserved (trimmed). + got, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: " act-ws ", + AttemptID: "\tact-ws.dispatch.1\t", + TransportRequestID: " transport-ws ", + ReceivedAt: time.Date(2026, 7, 24, 12, 0, 0, 0, time.UTC), + }) + if err != nil { + t.Fatalf("NormalizeActionDispatchReceipt unexpected error: %v", err) + } + if got.ActionID != "act-ws" { + t.Fatalf("ActionID = %q, want act-ws (trimmed)", got.ActionID) + } + if got.AttemptID != "act-ws.dispatch.1" { + t.Fatalf("AttemptID = %q, want act-ws.dispatch.1 (trimmed)", got.AttemptID) + } + if got.TransportRequestID != "transport-ws" { + t.Fatalf("TransportRequestID = %q, want transport-ws (trimmed)", got.TransportRequestID) + } + }) + + t.Run("Idempotent/AlreadyNormalizedReceiptPassesThroughUnchanged", func(t *testing.T) { + // A receipt that is already canonical must normalize to an identical + // value: TransportRequestID already set runs the keep arm, ReceivedAt + // already set runs the UTC arm (a no-op on an already-UTC instant). + fixedReceived := time.Date(2026, 7, 24, 12, 0, 0, 0, time.UTC) + once, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-idem", + AttemptID: ActionDispatchAttemptID("act-idem"), + TransportRequestID: "transport-idem", + ReceivedAt: fixedReceived, + }) + if err != nil { + t.Fatalf("first normalize unexpected error: %v", err) + } + // Normalizing the already-normalized receipt again must not drift. + twice, err := NormalizeActionDispatchReceipt(once) + if err != nil { + t.Fatalf("second normalize unexpected error: %v", err) + } + if twice != once { + t.Fatalf("idempotence broken: second normalization drifted\nonce = %+v\ntwice= %+v", once, twice) + } + }) + + t.Run("Idempotent/AbsentFieldsStabilizeOnSecondNormalization", func(t *testing.T) { + // First normalization defaults the absent optional fields; the + // second normalization (now over a fully-populated receipt) must be + // a fixed point, proving the defaults are themselves canonical. + once, err := NormalizeActionDispatchReceipt(ActionDispatchReceipt{ + ActionID: "act-stable", + AttemptID: ActionDispatchAttemptID("act-stable"), + // both optional fields absent on first pass + }) + if err != nil { + t.Fatalf("first normalize unexpected error: %v", err) + } + twice, err := NormalizeActionDispatchReceipt(once) + if err != nil { + t.Fatalf("second normalize unexpected error: %v", err) + } + if twice != once { + t.Fatalf("idempotence broken after defaulting:\nonce = %+v\ntwice= %+v", once, twice) + } + }) +} diff --git a/pkg/audit/export_branchcov0724pm_test.go b/pkg/audit/export_branchcov0724pm_test.go new file mode 100644 index 000000000..d9c12542d --- /dev/null +++ b/pkg/audit/export_branchcov0724pm_test.go @@ -0,0 +1,338 @@ +package audit + +import ( + "bytes" + "encoding/csv" + "encoding/json" + "strconv" + "testing" + "time" +) + +// boolPtr returns the address of b so we can populate ExportEvent.SignatureValid. +func boolPtr(b bool) *bool { return &b } + +// TestBranchcov0724pmExportCSV_EmptyEventsAndVerificationArms exercises the arms +// the existing Export test never reaches: an empty event slice, the +// includeVerification=false header branch, and every state of +// ExportEvent.SignatureValid (nil, true, false) inside the per-row block. +func TestBranchcov0724pmExportCSV_EmptyEventsAndVerificationArms(t *testing.T) { + exporter := &Exporter{} + + // Arm 1: empty event list with includeVerification=false. The for-loop body + // is skipped entirely; output must be a header-only CSV with NO + // "Signature Valid" column. + res, err := exporter.exportCSV(nil, "20240101-120000", false) + if err != nil { + t.Fatalf("exportCSV(nil,false): %v", err) + } + if res.EventCount != 0 { + t.Fatalf("EventCount = %d, want 0", res.EventCount) + } + if res.ContentType != "text/csv; charset=utf-8" { + t.Fatalf("ContentType = %q", res.ContentType) + } + if res.Filename != "audit-log-20240101-120000.csv" { + t.Fatalf("Filename = %q", res.Filename) + } + // Header only, and it must NOT carry the verification column. + reader := csv.NewReader(bytes.NewReader(res.Data)) + records, err := reader.ReadAll() + if err != nil { + t.Fatalf("parse empty csv: %v", err) + } + if len(records) != 1 { + t.Fatalf("want exactly 1 header record, got %d", len(records)) + } + wantHeader := []string{"ID", "Timestamp", "Event Type", "User", "IP", "Path", "Success", "Details", "Signature"} + if len(records[0]) != len(wantHeader) { + t.Fatalf("header has %d cols, want %d (%v)", len(records[0]), len(wantHeader), records[0]) + } + for i, h := range wantHeader { + if records[0][i] != h { + t.Fatalf("header[%d] = %q, want %q", i, records[0][i], h) + } + } + + // Arm 2: includeVerification=true with all three SignatureValid states on a + // single batch. This is the only way to reach the inner + // `if event.SignatureValid != nil` block and both of its sub-arms. + ts := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC) + events := []ExportEvent{ + {ID: "nil-verdict", Timestamp: ts, EventType: "login", SignatureValid: nil}, + {ID: "true-verdict", Timestamp: ts, EventType: "login", SignatureValid: boolPtr(true)}, + {ID: "false-verdict", Timestamp: ts, EventType: "login", SignatureValid: boolPtr(false)}, + } + res, err = exporter.exportCSV(events, "20240101-120000", true) + if err != nil { + t.Fatalf("exportCSV(verification,true): %v", err) + } + if res.EventCount != 3 { + t.Fatalf("EventCount = %d, want 3", res.EventCount) + } + reader = csv.NewReader(bytes.NewReader(res.Data)) + records, err = reader.ReadAll() + if err != nil { + t.Fatalf("parse verification csv: %v", err) + } + if len(records) != 4 { // header + 3 rows + t.Fatalf("want 4 records, got %d", len(records)) + } + // Header must now carry the appended verification column. + if records[0][len(records[0])-1] != "Signature Valid" { + t.Fatalf("last header = %q, want %q", records[0][len(records[0])-1], "Signature Valid") + } + // Per-row verdict column (last column) must reflect each sub-arm exactly. + wantVerdicts := map[string]string{ + "nil-verdict": "", + "true-verdict": "true", + "false-verdict": "false", + } + for _, row := range records[1:] { + id := row[0] + verdict := row[len(row)-1] + if wantVerdicts[id] != verdict { + t.Fatalf("row %q verdict col = %q, want %q", id, verdict, wantVerdicts[id]) + } + } +} + +// TestBranchcov0724pmExportCSV_SuccessFlagAndFieldRoundTrip pins the two Success +// arms (true/false) and confirms optional fields round-trip through a real CSV +// parser rather than merely that no error was returned. +func TestBranchcov0724pmExportCSV_SuccessFlagAndFieldRoundTrip(t *testing.T) { + exporter := &Exporter{} + ts := time.Date(2024, 6, 15, 9, 30, 0, 0, time.UTC) + events := []ExportEvent{ + { + ID: "ok", Timestamp: ts, EventType: "login", User: "alice", IP: "10.0.0.1", + Path: "/login", Success: true, Details: "ok", Signature: "sig-ok", + }, + { + ID: "fail", Timestamp: ts, EventType: "login", User: "bob", IP: "10.0.0.2", + Path: "/login", Success: false, Details: "bad", Signature: "sig-bad", + }, + } + + res, err := exporter.exportCSV(events, "20240101-120000", false) + if err != nil { + t.Fatalf("exportCSV: %v", err) + } + reader := csv.NewReader(bytes.NewReader(res.Data)) + records, err := reader.ReadAll() + if err != nil { + t.Fatalf("parse csv: %v", err) + } + if len(records) != 3 { + t.Fatalf("want header+2 rows, got %d", len(records)) + } + // Success column index 6 must be "true"/"false" (the only two arms). + if records[1][6] != "true" { + t.Fatalf("success col for ok row = %q, want \"true\"", records[1][6]) + } + if records[2][6] != "false" { + t.Fatalf("success col for fail row = %q, want \"false\"", records[2][6]) + } + // Round-trip every populated field of the first row. + if records[1][0] != "ok" || records[1][3] != "alice" || records[1][4] != "10.0.0.1" || + records[1][5] != "/login" || records[1][7] != "ok" || records[1][8] != "sig-ok" { + t.Fatalf("row fields did not round-trip: %v", records[1]) + } + // Timestamp column must be RFC3339 formatted. + if _, perr := time.Parse(time.RFC3339, records[1][1]); perr != nil { + t.Fatalf("timestamp %q not RFC3339: %v", records[1][1], perr) + } +} + +// TestBranchcov0724pmExportCSV_CSVEscaping verifies that detail fields +// containing commas, double quotes and embedded newlines survive a CSV +// write/parse round-trip byte-for-byte (RFC 4180 quoting). +func TestBranchcov0724pmExportCSV_CSVEscaping(t *testing.T) { + exporter := &Exporter{} + ts := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC) + specials := []string{ + "plain", + "has,comma", + `has"quote`, + "has\nnewline", + `mix"d, and\nall`, + "", // empty detail must also survive + } + events := make([]ExportEvent, 0, len(specials)) + for i, d := range specials { + events = append(events, ExportEvent{ + ID: "e" + strconv.Itoa(i), Timestamp: ts, EventType: "t", Details: d, + }) + } + + res, err := exporter.exportCSV(events, "ts", false) + if err != nil { + t.Fatalf("exportCSV: %v", err) + } + reader := csv.NewReader(bytes.NewReader(res.Data)) + // Allow variable number of fields per record (relaxed) just in case, but we + // expect consistent columns here. + records, err := reader.ReadAll() + if err != nil { + t.Fatalf("parse csv: %v", err) + } + if len(records) != len(specials)+1 { + t.Fatalf("want %d records, got %d", len(specials)+1, len(records)) + } + for i, want := range specials { + got := records[i+1][7] // Details column + if got != want { + t.Fatalf("detail[%d] round-trip = %q, want %q", i, got, want) + } + } + // Prove the raw bytes actually contain quoting for the comma case, so a + // naive split on "," would have broken (defends against a future regression + // to unescaped output). + if !bytes.Contains(res.Data, []byte(`"has,comma"`)) { + t.Fatalf("expected quoted comma field in output: %q", res.Data) + } + if !bytes.Contains(res.Data, []byte(`"has""quote"`)) { + t.Fatalf("expected doubled-quote escaping in output: %q", res.Data) + } +} + +// TestBranchcov0724pmExportJSON_RoundTripAndOmitempty covers the empty slice, +// single event, special-character detail, and the omitempty behaviour of +// ExportEvent. It parses the produced JSON back rather than asserting on a +// brittle timestamped blob. +func TestBranchcov0724pmExportJSON_RoundTripAndOmitempty(t *testing.T) { + exporter := &Exporter{} + + // Empty (nil) slice: EventCount must be 0 and the wrapper still well-formed. + res, err := exporter.exportJSON(nil, "20240101-120000") + if err != nil { + t.Fatalf("exportJSON(nil): %v", err) + } + if res.EventCount != 0 { + t.Fatalf("EventCount = %d, want 0", res.EventCount) + } + if res.ContentType != "application/json; charset=utf-8" { + t.Fatalf("ContentType = %q", res.ContentType) + } + if res.Filename != "audit-log-20240101-120000.json" { + t.Fatalf("Filename = %q", res.Filename) + } + var empty struct { + EventCount int `json:"event_count"` + Events []ExportEvent `json:"events"` + ExportedAt time.Time `json:"exported_at"` + } + if err := json.Unmarshal(res.Data, &empty); err != nil { + t.Fatalf("unmarshal empty json: %v", err) + } + if empty.EventCount != 0 || len(empty.Events) != 0 { + t.Fatalf("empty json = %+v", empty) + } + if empty.ExportedAt.IsZero() { + t.Fatal("ExportedAt should be populated") + } + + // omitempty: an event with only required fields must omit every optional key. + ts := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC) + bareEvents := []ExportEvent{{ID: "bare", Timestamp: ts, EventType: "login", Success: false}} + res, err = exporter.exportJSON(bareEvents, "ts") + if err != nil { + t.Fatalf("exportJSON(bare): %v", err) + } + if !bytes.Contains(res.Data, []byte(`"event_count": 1`)) { + t.Fatalf("expected event_count=1 in %s", res.Data) + } + // Parse the first event into a generic map so we can reason about the exact + // key set rather than matching against the indented (space-padded) bytes. + var wrapper struct { + Events []map[string]any `json:"events"` + } + if err := json.Unmarshal(res.Data, &wrapper); err != nil { + t.Fatalf("unmarshal bare json: %v", err) + } + if len(wrapper.Events) != 1 { + t.Fatalf("want 1 bare event, got %d", len(wrapper.Events)) + } + ev := wrapper.Events[0] + // The optional string fields must NOT appear as keys. + for _, key := range []string{"user", "ip", "path", "details", "signature", "signature_valid"} { + if _, ok := ev[key]; ok { + t.Fatalf("omitempty leaked key %q into output: %s", key, res.Data) + } + } + // Required keys must appear with their concrete values. + for k, want := range map[string]any{"id": "bare", "event_type": "login", "success": false} { + got, ok := ev[k] + if !ok { + t.Fatalf("missing required key %q in %s", k, res.Data) + } + if got != want { + t.Fatalf("key %q = %v, want %v", k, got, want) + } + } + + // Full event with special chars in Details round-trips byte-for-byte. + fullEvents := []ExportEvent{{ + ID: "full", Timestamp: ts, EventType: "config", User: "a,b", IP: "10.0.0.9", + Path: "/p", Success: true, Details: "line1\nline2\ttab", Signature: "0dead", + SignatureValid: boolPtr(true), + }} + res, err = exporter.exportJSON(fullEvents, "ts") + if err != nil { + t.Fatalf("exportJSON(full): %v", err) + } + var parsed struct { + Events []ExportEvent `json:"events"` + } + if err := json.Unmarshal(res.Data, &parsed); err != nil { + t.Fatalf("unmarshal full json: %v", err) + } + if len(parsed.Events) != 1 { + t.Fatalf("want 1 event, got %d", len(parsed.Events)) + } + got := parsed.Events[0] + if got.Details != "line1\nline2\ttab" || got.User != "a,b" || got.Signature != "0dead" { + t.Fatalf("full event did not round-trip: %+v", got) + } + if got.SignatureValid == nil || *got.SignatureValid != true { + t.Fatalf("SignatureValid did not round-trip: %v", got.SignatureValid) + } +} + +// TestBranchcov0724pmExportJSON_NilVsEmptySlice distinguishes the two zero-event +// shapes a caller can pass: a nil slice encodes as JSON null, a non-nil empty +// slice encodes as []. Both must marshal without error (the error arm of +// exportJSON is only reachable with an unmarshalable type, which ExportEvent +// cannot hold - see report). +func TestBranchcov0724pmExportJSON_NilVsEmptySlice(t *testing.T) { + exporter := &Exporter{} + + nilRes, err := exporter.exportJSON(nil, "ts") + if err != nil { + t.Fatalf("exportJSON(nil): %v", err) + } + // Decode into a RawMessage map so we can distinguish JSON null from []; + // both decode to a nil []ExportEvent, so the typed round-trip cannot tell + // them apart. + var nilRaw map[string]json.RawMessage + if err := json.Unmarshal(nilRes.Data, &nilRaw); err != nil { + t.Fatalf("unmarshal nil json: %v", err) + } + // A nil []ExportEvent serialises to the JSON literal `null`. + if string(nilRaw["events"]) != "null" { + t.Fatalf("nil slice should encode events as null, got %q: %s", nilRaw["events"], nilRes.Data) + } + + emptyRes, err := exporter.exportJSON([]ExportEvent{}, "ts") + if err != nil { + t.Fatalf("exportJSON(empty): %v", err) + } + var emptyRaw map[string]json.RawMessage + if err := json.Unmarshal(emptyRes.Data, &emptyRaw); err != nil { + t.Fatalf("unmarshal empty json: %v", err) + } + // A non-nil empty slice serialises to `[]`. + if string(emptyRaw["events"]) != "[]" { + t.Fatalf("empty slice should encode events as [], got %q: %s", emptyRaw["events"], emptyRes.Data) + } +} diff --git a/pkg/audit/signer_branchcov0724pm_test.go b/pkg/audit/signer_branchcov0724pm_test.go new file mode 100644 index 000000000..6b9f6bca9 --- /dev/null +++ b/pkg/audit/signer_branchcov0724pm_test.go @@ -0,0 +1,403 @@ +package audit + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// --- crypto doubles for the NewSigner error arms --- + +// failingEncryptCrypto fails both Encrypt and Decrypt; used to hit the +// "failed to encrypt audit signing key" arm on the generate-new-key path. +type failingEncryptCrypto struct{} + +func (failingEncryptCrypto) Encrypt([]byte) ([]byte, error) { + return nil, errors.New("encrypt unavailable") +} + +func (failingEncryptCrypto) Decrypt([]byte) ([]byte, error) { + return nil, errors.New("decrypt unavailable") +} + +// decryptOnlyCrypto mirrors taggedMockCryptoManager's Decrypt (requires the +// "enc:" prefix) so a plaintext key file triggers migration, but its Encrypt +// always fails. This is the only way to reach the +// "failed to encrypt migrated audit signing key" arm. +type decryptOnlyCrypto struct{} + +func (decryptOnlyCrypto) Encrypt([]byte) ([]byte, error) { + return nil, errors.New("encrypt unavailable during migration") +} + +func (decryptOnlyCrypto) Decrypt(b []byte) ([]byte, error) { + if len(b) < 4 || string(b[:4]) != "enc:" { + return nil, os.ErrInvalid + } + return append([]byte(nil), b[4:]...), nil +} + +// --- NewSigner: uncovered error arms of the load-and-generate state machine --- + +// TestBranchcov0724pmNewSigner_LoadKeyError hits the raw `return nil, err` +// arm by feeding a key file that neither decrypts nor matches the legacy +// 32-/64-byte plaintext shapes. +func TestBranchcov0724pmNewSigner_LoadKeyError(t *testing.T) { + dir := t.TempDir() + keyPath := filepath.Join(dir, ".audit-signing.key") + // 12 bytes: not decryptable by taggedMockCryptoManager (no "enc:" prefix), + // not 32 bytes, not 64 bytes -> loadAuditSigningKey returns an error. + if err := os.WriteFile(keyPath, []byte("garbage_key"), 0o600); err != nil { + t.Fatalf("seed key file: %v", err) + } + + signer, err := NewSigner(dir, taggedMockCryptoManager{}) + if err == nil { + t.Fatalf("expected load error, got signer=%v", signer) + } + if !strings.Contains(err.Error(), "failed to decrypt audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } + if signer != nil { + t.Fatalf("expected nil signer on error, got %v", signer) + } +} + +// TestBranchcov0724pmNewSigner_ShortDecryptedKey hits the +// "invalid audit signing key length" arm: the file decrypts successfully but +// yields fewer than 32 bytes. +func TestBranchcov0724pmNewSigner_ShortDecryptedKey(t *testing.T) { + dir := t.TempDir() + keyPath := filepath.Join(dir, ".audit-signing.key") + // "enc:" + "short" decrypts (via taggedMockCryptoManager) to a 5-byte key. + if err := os.WriteFile(keyPath, []byte("enc:short"), 0o600); err != nil { + t.Fatalf("seed short key file: %v", err) + } + + signer, err := NewSigner(dir, taggedMockCryptoManager{}) + if err == nil { + t.Fatalf("expected length error, got signer=%v", signer) + } + if !strings.Contains(err.Error(), "invalid audit signing key length") { + t.Fatalf("unexpected error text: %v", err) + } + if !strings.Contains(err.Error(), "got 5") { + t.Fatalf("error should report decoded length 5: %v", err) + } +} + +// TestBranchcov0724pmNewSigner_MigrationEncryptError reaches the +// "failed to encrypt migrated audit signing key" arm: a plaintext 32-byte key +// triggers migration, but Encrypt then fails. +func TestBranchcov0724pmNewSigner_MigrationEncryptError(t *testing.T) { + dir := t.TempDir() + keyPath := filepath.Join(dir, ".audit-signing.key") + plaintext := []byte("0123456789abcdef0123456789abcdef") // exactly 32 bytes + if err := os.WriteFile(keyPath, plaintext, 0o600); err != nil { + t.Fatalf("seed plaintext key: %v", err) + } + + _, err := NewSigner(dir, decryptOnlyCrypto{}) + if err == nil { + t.Fatal("expected migration encrypt error") + } + if !strings.Contains(err.Error(), "failed to encrypt migrated audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } + + // The original plaintext file must be untouched when encryption fails + // (migration must not corrupt the source on the failure path). + got, readErr := os.ReadFile(keyPath) + if readErr != nil { + t.Fatalf("re-read key: %v", readErr) + } + if string(got) != string(plaintext) { + t.Fatalf("plaintext key corrupted on encrypt-failure path: %q", got) + } +} + +// TestBranchcov0724pmNewSigner_MigrationWriteError reaches the +// "failed to rewrite audit signing key" arm by making an otherwise-migratable +// key file unwritable. Skipped when running as root (root bypasses file mode). +func TestBranchcov0724pmNewSigner_MigrationWriteError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("migration WriteFile failure cannot be triggered as root") + } + dir := t.TempDir() + keyPath := filepath.Join(dir, ".audit-signing.key") + // 32-byte plaintext -> migration triggers with the (succeeding) + // taggedMockCryptoManager.Encrypt; the subsequent rewrite hits the + // read-only file and fails. + if err := os.WriteFile(keyPath, []byte("0123456789abcdef0123456789abcdef"), 0o600); err != nil { + t.Fatalf("seed plaintext key: %v", err) + } + if err := os.Chmod(keyPath, 0o444); err != nil { + t.Fatalf("chmod read-only: %v", err) + } + + _, err := NewSigner(dir, taggedMockCryptoManager{}) + if err == nil { + t.Fatal("expected migration rewrite error") + } + if !strings.Contains(err.Error(), "failed to rewrite audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } +} + +// TestBranchcov0724pmNewSigner_GenerateEncryptError reaches the +// "failed to encrypt audit signing key" arm on the generate-new-key path +// (no existing key file present). +func TestBranchcov0724pmNewSigner_GenerateEncryptError(t *testing.T) { + dir := t.TempDir() + // No key file present -> generate path; failingEncryptCrypto.Encrypt fails. + _, err := NewSigner(dir, failingEncryptCrypto{}) + if err == nil { + t.Fatal("expected generate encrypt error") + } + if !strings.Contains(err.Error(), "failed to encrypt audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } + + // On this failure path no key file should have been written. + if _, statErr := os.Stat(filepath.Join(dir, ".audit-signing.key")); !os.IsNotExist(statErr) { + t.Fatalf("expected no key file after encrypt failure, stat err=%v", statErr) + } +} + +// TestBranchcov0724pmNewSigner_MkdirAllError reaches the +// "failed to create directory for audit signing key" arm by pointing dataDir +// at a regular file, so MkdirAll(parent) fails with "not a directory". +func TestBranchcov0724pmNewSigner_MkdirAllError(t *testing.T) { + // dataDir is a regular file: ReadFile(file/.audit-signing.key) fails + // (ENOTDIR) -> skip load -> generate -> MkdirAll(file) fails. + file, err := os.CreateTemp("", "audit-data-is-file") + if err != nil { + t.Fatalf("create temp file: %v", err) + } + defer os.Remove(file.Name()) + if err := file.Close(); err != nil { + t.Fatalf("close temp file: %v", err) + } + + _, err = NewSigner(file.Name(), taggedMockCryptoManager{}) + if err == nil { + t.Fatal("expected mkdir error") + } + if !strings.Contains(err.Error(), "failed to create directory for audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } +} + +// TestBranchcov0724pmNewSigner_GenerateWriteError reaches the +// "failed to save audit signing key" arm by pre-creating the key path as a +// directory, so the final WriteFile fails with EISDIR. +func TestBranchcov0724pmNewSigner_GenerateWriteError(t *testing.T) { + dir := t.TempDir() + // Pre-create the destination path as a directory. ReadFile on a directory + // fails ("is a directory") -> skip load -> generate -> MkdirAll(dir) ok -> + // WriteFile(dir) fails with EISDIR. + keyPath := filepath.Join(dir, ".audit-signing.key") + if err := os.Mkdir(keyPath, 0o700); err != nil { + t.Fatalf("seed key-as-dir: %v", err) + } + + _, err := NewSigner(dir, taggedMockCryptoManager{}) + if err == nil { + t.Fatal("expected save error") + } + if !strings.Contains(err.Error(), "failed to save audit signing key") { + t.Fatalf("unexpected error text: %v", err) + } +} + +// TestBranchcov0724pmNewSigner_NilCryptoYieldsDisabledSigner covers the +// cryptoMgr==nil fast path explicitly and asserts the resulting signer signs +// nothing ("empty key") and verifies nothing. +func TestBranchcov0724pmNewSigner_NilCryptoYieldsDisabledSigner(t *testing.T) { + signer, err := NewSigner(t.TempDir(), nil) + if err != nil { + t.Fatalf("NewSigner(nil crypto): %v", err) + } + if signer == nil { + t.Fatal("expected non-nil disabled signer") + } + if signer.SigningEnabled() { + t.Fatal("signing must be disabled with nil crypto manager") + } + if signer.ExportKey() != "" { + t.Fatalf("ExportKey should be empty for disabled signer, got %q", signer.ExportKey()) + } + event := Event{ID: "x", Timestamp: time.Now(), EventType: "e"} + if sig := signer.Sign(event); sig != "" { + t.Fatalf("disabled signer produced signature %q", sig) + } + // A disabled signer must reject verification regardless of signature. + if signer.Verify(Event{ID: "x", Signature: "deadbeef"}) { + t.Fatal("disabled signer must not verify any event") + } +} + +// TestBranchcov0724pmNewSigner_WrongKeyDoesNotCrossVerify pins the "wrong key" +// behaviour: a signature produced under one key must not verify under another. +func TestBranchcov0724pmNewSigner_WrongKeyDoesNotCrossVerify(t *testing.T) { + a, err := NewSignerWithKey([]byte("0123456789abcdef0123456789abcdef")) + if err != nil { + t.Fatalf("NewSignerWithKey A: %v", err) + } + b, err := NewSignerWithKey([]byte("fedcba9876543210fedcba9876543210")) + if err != nil { + t.Fatalf("NewSignerWithKey B: %v", err) + } + event := Event{ + ID: "cross", Timestamp: time.Date(2026, 7, 25, 0, 0, 0, 0, time.UTC), + EventType: "login", User: "u", IP: "1.2.3.4", Path: "/", Success: true, + } + event.Signature = a.Sign(event) + if !a.Verify(event) { + t.Fatal("signer A should verify its own signature") + } + if b.Verify(event) { + t.Fatal("signer B must reject A's signature (wrong key)") + } +} + +// --- async_logger.go: VerifySignature (75%) and IsPersistentAuditLogger (66.7%) --- + +// TestBranchcov0724pmAsyncLogger_VerifySignatureArms covers the two arms the +// existing async test misses: a nil receiver and a backend that does not +// implement VerifySignature (ConsoleLogger). It also re-confirms the happy +// delegation path with a real signed event and a tampered payload. +func TestBranchcov0724pmAsyncLogger_VerifySignatureArms(t *testing.T) { + // Arm 1: nil receiver must return false without panicking. + var nilLogger *AsyncLogger + if nilLogger.VerifySignature(Event{}) { + t.Fatal("nil AsyncLogger.VerifySignature must return false") + } + + // Arm 2: backend without a VerifySignature method (ConsoleLogger) -> + // type assertion fails -> ok==false -> returns false. + console := NewAsyncLogger(NewConsoleLogger(), AsyncLoggerConfig{BufferSize: 4}) + defer console.Close() + if console.VerifySignature(Event{ID: "x", Signature: "sig"}) { + t.Fatal("ConsoleLogger-backed AsyncLogger must report false (no verifier)") + } + + // Arm 3 (happy path, for the prose: "valid signature"): a genuinely signed + // event verifies true through the async wrapper; a tampered payload false. + backend, err := NewSQLiteLogger(SQLiteLoggerConfig{ + DataDir: t.TempDir(), + CryptoMgr: newMockCryptoManager(), + }) + if err != nil { + t.Fatalf("NewSQLiteLogger: %v", err) + } + defer backend.Close() + + signer, err := NewSigner(t.TempDir(), newMockCryptoManager()) + if err != nil { + t.Fatalf("NewSigner: %v", err) + } + ts := time.Date(2026, 7, 25, 10, 0, 0, 0, time.UTC) + event := Event{ + ID: "verify-good", Timestamp: ts, EventType: "login", + User: "admin", IP: "10.0.0.1", Path: "/login", Success: true, Details: "ok", + } + event.Signature = signer.Sign(event) + if err := backend.Record(event); err != nil { + t.Fatalf("record event: %v", err) + } + + async := NewAsyncLogger(backend, AsyncLoggerConfig{BufferSize: 8}) + defer async.Close() + stored, _, qerr := async.QueryPage(QueryFilter{ID: event.ID, Limit: 1}) + if qerr != nil || len(stored) != 1 { + t.Fatalf("QueryPage: qerr=%v len=%d", qerr, len(stored)) + } + if !async.VerifySignature(stored[0]) { + t.Fatal("async wrapper must verify a validly signed stored event") + } + // Tamper the payload: same signature, changed content -> must fail. + tampered := stored[0] + tampered.Details = "tampered" + if async.VerifySignature(tampered) { + t.Fatal("async wrapper must reject a tampered payload") + } +} + +// TestBranchcov0724pmAsyncLogger_IsPersistentAuditLoggerArms covers both +// verdicts plus the nil receiver, which the existing suite never exercises. +func TestBranchcov0724pmAsyncLogger_IsPersistentAuditLoggerArms(t *testing.T) { + // Nil receiver -> false. + var nilLogger *AsyncLogger + if nilLogger.IsPersistentAuditLogger() { + t.Fatal("nil AsyncLogger.IsPersistentAuditLogger must return false") + } + + // Non-persistent backend (ConsoleLogger) -> false via delegation. + console := NewAsyncLogger(NewConsoleLogger(), AsyncLoggerConfig{BufferSize: 2}) + defer console.Close() + if console.IsPersistentAuditLogger() { + t.Fatal("console-backed async logger must not be persistent") + } + + // Persistent backend (SQLiteLogger) -> true. + backend, err := NewSQLiteLogger(SQLiteLoggerConfig{DataDir: t.TempDir()}) + if err != nil { + t.Fatalf("NewSQLiteLogger: %v", err) + } + defer backend.Close() + persistent := NewAsyncLogger(backend, AsyncLoggerConfig{BufferSize: 2}) + defer persistent.Close() + if !persistent.IsPersistentAuditLogger() { + t.Fatal("sqlite-backed async logger must be persistent") + } +} + +// --- audit.go: IsPersistentLogger (50%) --- + +// stubBareLogger satisfies Logger but deliberately does NOT implement +// persistentAuditLogger, so IsPersistentLogger falls through to the +// isConsole branch. +type stubBareLogger struct{} + +func (stubBareLogger) Record(Event) error { return nil } +func (stubBareLogger) Query(QueryFilter) ([]Event, error) { return nil, nil } +func (stubBareLogger) Count(QueryFilter) (int, error) { return 0, nil } +func (stubBareLogger) GetWebhookURLs() []string { return nil } +func (stubBareLogger) UpdateWebhookURLs([]string) error { return nil } +func (stubBareLogger) Close() error { return nil } + +// TestBranchcov0724pmIsPersistentLogger_AllArms covers the nil fast path and +// the fall-through isConsole branch (both verdicts). +func TestBranchcov0724pmIsPersistentLogger_AllArms(t *testing.T) { + // Arm 1: nil logger -> false. + if IsPersistentLogger(nil) { + t.Fatal("IsPersistentLogger(nil) must be false") + } + + // Arm 2: a Logger that does not declare persistence and is NOT a + // *ConsoleLogger falls through to `return !isConsole` == true. This is the + // fallback's "treat unknown backends as persistent" contract for enterprise + // implementations. + if !IsPersistentLogger(stubBareLogger{}) { + t.Fatal("IsPersistentLogger should treat a non-console bare Logger as persistent") + } + + // Arm 3: ConsoleLogger declares persistence explicitly and reports false. + if IsPersistentLogger(NewConsoleLogger()) { + t.Fatal("IsPersistentLogger(ConsoleLogger) must be false") + } + + // Arm 4: SQLiteLogger declares persistence and reports true. + backend, err := NewSQLiteLogger(SQLiteLoggerConfig{DataDir: t.TempDir()}) + if err != nil { + t.Fatalf("NewSQLiteLogger: %v", err) + } + defer backend.Close() + if !IsPersistentLogger(backend) { + t.Fatal("IsPersistentLogger(SQLiteLogger) must be true") + } +}