fix: salvage malformed optional remediation fields during finding recovery (#159)

A finding whose optional remediationTests or preventiveControls field
arrived in the wrong shape was discarded entirely at finalization, even
when every required field validated. Recover these fields the same way
malformed writeups are recovered: drop the field, keep the finding, and
record a warning.

Also document the array-of-strings shape for both fields in the finding
detail reference and the bundled completed-scan example, since no
reference or example previously showed it.

Fixes #106

Author: alarcritty

sdk/typescript/_bundled_plugin/examples/completed-scan/findings.json

@@ -42,6 +42,12 @@
       "remediation": "Normalize destinations and reject entries that escape the extraction root.",
       "validation": null,
       "attackPath": null,
+      "remediationTests": [
+        "Assert that extracting an archive entry named `../escape.txt` fails without writing outside the extraction root."
+      ],
+      "preventiveControls": [
+        "Route all archive extraction through one helper that normalizes and validates destination paths."
+      ],
       "provenance": {
         "source": "local_plugin"
       },

sdk/typescript/_bundled_plugin/examples/completed-scan/scan-manifest.json

@@ -30,7 +30,7 @@
     "artifacts": [
       {
         "path": "findings.json",
-        "sha256": "db5ce8533c1b6cd9131f9e12e8bb705f63474caa2a3f7dd21207666b3a8da777",
+        "sha256": "a6dc4521d6478828224fbafc33585401a47bfeb0e56e07b5d2886e8a29937f2f",
         "mediaType": "application/json"
       },
       {

sdk/typescript/_bundled_plugin/references/finding-detail-fields.md

@@ -23,7 +23,7 @@ The finding detail view is a decision-focused projection of the canonical findin
 - dataflow source, meaningful transformations, dangerous sink, and concrete outcome;
 - realistic attacker, entry point, access requirements, preconditions, and attacker outcome;
 - severity rationale plus the specific evidence that would raise or lower the rating;
-- the minimal remediation invariant, regression tests, and preventive controls.
+- the minimal remediation invariant (`remediation`, a single string), plus `remediationTests` and `preventiveControls`, each an array of short strings with one regression test or preventive control per entry.
 
 Keep background exposition, alternate exploit research, full PoC instructions, representative command output, and long source walkthroughs in the detailed write-up. Do not copy them into canonical fields merely to make the workspace report longer. The workspace should stay self-contained enough to support triage while avoiding duplicated or speculative prose.
 
@@ -155,7 +155,15 @@ The following shape shows how to encode the `environment/add` reserved-environme
     "limitations": [
       "This overwrite does not directly execute code on the victim host."
     ]
-  }
+  },
+  "remediation": "Reuse the startup reserved-ID check inside `EnvironmentManager::upsert_environment()` so the runtime mutation path rejects the reserved `local` identifier.",
+  "remediationTests": [
+    "Assert that `environment/add` with `environmentId: \"local\"` returns a protocol error.",
+    "Assert that `default_environment()` still resolves the manager-owned local runtime after a rejected upsert."
+  ],
+  "preventiveControls": [
+    "Centralize reserved-identifier validation so every environment mutation path shares one guard."
+  ]
 }
 ```
 

sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py

@@ -796,6 +796,12 @@ def _recover_unsealed_findings(
     writeup_schema = _require_dict(
         finding_properties, "writeup", "findings.schema.properties.findings.items.properties"
     )
+    auxiliary_schemas = {
+        name: _require_dict(
+            finding_properties, name, "findings.schema.properties.findings.items.properties"
+        )
+        for name in ("remediationTests", "preventiveControls")
+    }
     scan = _require_dict(manifest, "scan", "manifest")
     scan_id = _require_str(scan, "id", "manifest.scan")
     if findings.get("scanId") != scan_id:
@@ -854,6 +860,18 @@ def _recover_unsealed_findings(
                 except ContractError as exc:
                     finding.pop("writeup")
                     warnings.append(f"Skipped malformed writeup for finding {index + 1}: {exc}.")
+            for auxiliary, auxiliary_schema in auxiliary_schemas.items():
+                if auxiliary not in finding:
+                    continue
+                try:
+                    _validate_schema_node(
+                        finding[auxiliary], auxiliary_schema, f"{context}.{auxiliary}"
+                    )
+                except ContractError as exc:
+                    finding.pop(auxiliary)
+                    warnings.append(
+                        f"Skipped malformed {auxiliary} for finding {index + 1}: {exc}."
+                    )
             _validate_schema_node(finding, finding_schema, context)
         except ContractError as exc:
             warning = f"Skipped malformed finding {index + 1}: {exc}."

sdk/typescript/tests-ts/scan-recovery.test.ts

@@ -30,6 +30,8 @@ type Finding = Record<string, unknown> & {
     explanation: string;
   }>;
   writeup?: unknown;
+  remediationTests?: unknown;
+  preventiveControls?: unknown;
 };
 
 type FindingsDocument = {
@@ -679,6 +681,74 @@ describe("malformed scan artifact recovery", () => {
     }
   });
 
+  test("keeps findings while removing malformed remediation guidance", async () => {
+    const fixture = await startDraftScan();
+    const path = join(fixture.scanDir, "findings.json");
+    const document = await readJson<FindingsDocument>(path);
+    const valid = document.findings[0]!;
+
+    const cases: Array<
+      [string, "remediationTests" | "preventiveControls", unknown]
+    > = [
+      [
+        "valid-remediation-tests",
+        "remediationTests",
+        ["Add a regression test."],
+      ],
+      ["prose-remediation-tests", "remediationTests", "Add a regression test."],
+      [
+        "object-remediation-tests",
+        "remediationTests",
+        [{ description: "Add a regression test." }],
+      ],
+      [
+        "prose-preventive-controls",
+        "preventiveControls",
+        "Centralize validation.",
+      ],
+    ];
+    for (const [anchor, field, value] of cases) {
+      const finding = structuredClone(valid);
+      finding.identity.anchor = anchor;
+      finding[field] = value;
+      document.findings.push(finding);
+    }
+    await writeJson(path, document);
+
+    const completed = await completeScan(fixture);
+
+    expect(completed.progress.status).toBe("complete");
+    expect(completed.findingCount).toBe(5);
+    expect(completed.warnings).toHaveLength(3);
+    expect(
+      completed.warnings.filter((warning) =>
+        warning.startsWith("Skipped malformed remediationTests for finding"),
+      ),
+    ).toHaveLength(2);
+    expect(
+      completed.warnings.filter((warning) =>
+        warning.startsWith("Skipped malformed preventiveControls for finding"),
+      ),
+    ).toHaveLength(1);
+    const recovered = (await readJson<FindingsDocument>(path)).findings;
+    expect(
+      recovered.find(
+        (finding) => finding?.identity.anchor === "valid-remediation-tests",
+      )?.remediationTests,
+    ).toEqual(["Add a regression test."]);
+    for (const [anchor, field] of [
+      ["prose-remediation-tests", "remediationTests"],
+      ["object-remediation-tests", "remediationTests"],
+      ["prose-preventive-controls", "preventiveControls"],
+    ] as const) {
+      const finding = recovered.find(
+        (candidate) => candidate?.identity.anchor === anchor,
+      );
+      expect(finding).toBeDefined();
+      expect(finding).not.toHaveProperty(field);
+    }
+  });
+
   test("keeps verified coverage receipts and downgrades invalid coverage", async () => {
     const fixture = await startDraftScan();
     const path = join(fixture.scanDir, "coverage.json");