diff --git a/internal/infra/http/handler/command_handler.go b/internal/infra/http/handler/command_handler.go index 40fb9183..9ec1b91e 100644 --- a/internal/infra/http/handler/command_handler.go +++ b/internal/infra/http/handler/command_handler.go @@ -614,10 +614,33 @@ func (h *CommandHandler) triggerValidationEvidence(cmd *commanddom.Command) { } tenantID := cmd.TenantID + // Carry the simulation link onto the evidence. It was passed as nil here, + // so every row this path wrote had simulation_run_id NULL — on the live + // database all 5, all executor_kind=safe-check, i.e. all produced BY a + // simulation and none traceable back to one. The API exposes the field, so + // "which evidence did this run produce?" answered empty. + // + // The value comes from the command payload, not from the agent: it is the + // same field the sibling triggerSimulationFinalize already reads to decide + // which run to finalize, so the two paths now agree by construction instead + // of by an agent remembering to echo it back. + var simRunID *shared.ID + if payload.SimulationRunID != "" { + if id, sErr := shared.IDFromString(payload.SimulationRunID); sErr == nil { + simRunID = &id + } else { + // Don't fail the evidence over it — a malformed id costs the link, + // not the finding update. + h.logger.Warn("validate command carries an unparseable simulation_run_id", + "command_id", cmd.ID.String(), + "simulation_run_id", payload.SimulationRunID) + } + } + go func() { bgCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - if _, err := h.validationIngest.Ingest(bgCtx, tenantID, findingID, nil, ev); err != nil { + if _, err := h.validationIngest.Ingest(bgCtx, tenantID, findingID, simRunID, ev); err != nil { h.logger.Error("failed to record validation evidence", "command_id", cmd.ID.String(), "finding_id", payload.FindingID, diff --git a/internal/infra/http/handler/command_simulation_link_test.go b/internal/infra/http/handler/command_simulation_link_test.go new file mode 100644 index 00000000..53795955 --- /dev/null +++ b/internal/infra/http/handler/command_simulation_link_test.go @@ -0,0 +1,118 @@ +package handler + +import ( + "encoding/json" + "testing" + + "github.com/openctemio/api/internal/app/validation" + commanddom "github.com/openctemio/api/pkg/domain/command" + "github.com/openctemio/api/pkg/domain/shared" + "github.com/openctemio/api/pkg/logger" +) + +// triggerValidationEvidence passed nil for the simulation run id, so every row +// this path wrote had simulation_run_id NULL. On the live database that was all +// 5 of them — every one executor_kind=safe-check, i.e. every one produced BY a +// simulation and none traceable back to one. The API exposes the field, so +// "which evidence did this run produce?" answered empty. +// +// The value was never missing: payload.SimulationRunID is the same field the +// sibling triggerSimulationFinalize already reads to decide which run to +// finalize. The two paths simply disagreed about whether it existed. +// +// Note the old mock signature was `_ *shared.ID` — it discarded the argument, +// which is exactly why no test could see this. A mock that ignores a parameter +// cannot fail when the parameter is wrong. + +func simulationValidateCommand(t *testing.T, tenantID, findingID shared.ID, simRunID string) *commanddom.Command { + t.Helper() + + payload, err := json.Marshal(validation.ValidateCommandPayload{ + FindingID: findingID.String(), + SimulationRunID: simRunID, + ExecutorKind: "safe-check", + Technique: "T1046", + Target: validation.ValidateTargetPayload{Type: "domain", Address: "example.com"}, + }) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + + cmd, err := commanddom.NewCommand(tenantID, commanddom.CommandTypeValidate, + commanddom.CommandPriorityNormal, payload) + if err != nil { + t.Fatalf("new command: %v", err) + } + result, err := json.Marshal(validation.ValidateResultPayload{ + Outcome: "not_detected", Summary: "port closed", + }) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + cmd.Complete(result) + return cmd +} + +func TestTriggerValidationEvidence_CarriesTheSimulationLink(t *testing.T) { + ing := &captureIngester{} + h := &CommandHandler{logger: logger.NewNop()} + h.SetValidationIngest(ing) + + simRunID := shared.NewID() + cmd := simulationValidateCommand(t, shared.NewID(), shared.NewID(), simRunID.String()) + + h.triggerValidationEvidence(cmd) + waitFor(t, func() bool { calls, _, _, _ := ing.snapshot(); return calls == 1 }) + + got := ing.simRun() + if got == nil { + t.Fatal("evidence was ingested with a nil simulation run id even though the " + + "command payload carries one. The row lands with simulation_run_id NULL " + + "and the run it came from can never be traced to it") + } + if *got != simRunID { + t.Errorf("simulation_run_id = %s, want %s", got, simRunID) + } +} + +// Validation that is not part of a simulation must stay unlinked — inventing a +// link would be worse than missing one. +func TestTriggerValidationEvidence_NoSimulationStaysNil(t *testing.T) { + ing := &captureIngester{} + h := &CommandHandler{logger: logger.NewNop()} + h.SetValidationIngest(ing) + + cmd := simulationValidateCommand(t, shared.NewID(), shared.NewID(), "") + + h.triggerValidationEvidence(cmd) + waitFor(t, func() bool { calls, _, _, _ := ing.snapshot(); return calls == 1 }) + + if got := ing.simRun(); got != nil { + t.Errorf("simulation_run_id = %s for a command with no simulation; a "+ + "fabricated link is worse than an absent one", got) + } +} + +// A malformed id costs the link, not the finding update. Dropping the evidence +// would turn a cosmetic provenance problem into a lost validation result. +func TestTriggerValidationEvidence_MalformedSimulationIDStillIngests(t *testing.T) { + ing := &captureIngester{} + h := &CommandHandler{logger: logger.NewNop()} + h.SetValidationIngest(ing) + + cmd := simulationValidateCommand(t, shared.NewID(), shared.NewID(), "not-a-uuid") + + h.triggerValidationEvidence(cmd) + waitFor(t, func() bool { calls, _, _, _ := ing.snapshot(); return calls == 1 }) + + if got := ing.simRun(); got != nil { + t.Errorf("an unparseable simulation_run_id produced a link: %s", got) + } + calls, _, _, ev := ing.snapshot() + if calls != 1 { + t.Fatalf("ingest calls = %d, want 1: the evidence must survive a bad link", calls) + } + if ev.Outcome != validation.Outcome("not_detected") { + t.Errorf("outcome = %q, want not_detected — the verdict must be unaffected", ev.Outcome) + } +} diff --git a/internal/infra/http/handler/command_validation_hook_test.go b/internal/infra/http/handler/command_validation_hook_test.go index 12fdaa47..cd8c18ea 100644 --- a/internal/infra/http/handler/command_validation_hook_test.go +++ b/internal/infra/http/handler/command_validation_hook_test.go @@ -19,18 +19,28 @@ type captureIngester struct { tenantID shared.ID finding shared.ID ev validation.Evidence + // simRunID was previously discarded by this mock, which is why it could not + // see that the handler always passed nil. + simRunID *shared.ID } -func (c *captureIngester) Ingest(_ context.Context, tenantID, findingID shared.ID, _ *shared.ID, ev validation.Evidence) (validation.IngestResult, error) { +func (c *captureIngester) Ingest(_ context.Context, tenantID, findingID shared.ID, simRunID *shared.ID, ev validation.Evidence) (validation.IngestResult, error) { c.mu.Lock() defer c.mu.Unlock() c.calls++ c.tenantID = tenantID c.finding = findingID + c.simRunID = simRunID c.ev = ev return validation.IngestResult{StatusChanged: true}, nil } +func (c *captureIngester) simRun() *shared.ID { + c.mu.Lock() + defer c.mu.Unlock() + return c.simRunID +} + func (c *captureIngester) snapshot() (int, shared.ID, shared.ID, validation.Evidence) { c.mu.Lock() defer c.mu.Unlock()