Skip to content

Finding write-ups and the hardening portfolio are referenced by the manifest but left outside the seal #230

Description

@rohanpoudel2

Summary

Every entry in manifest.scan.artifacts is digest-verified when a scan is loaded, and every coverage.surfaces[].receiptRefs entry is required to be one of those sealed artifacts. finding.writeup.reportPath and manifest.scan.hardening.portfolioPath get neither treatment — they are opened and immediately closed, an existence check only.

The consequence is that the human-readable write-up, which is the artifact a person actually reads, can be replaced wholesale after sealing and the scan still loads as intact.

Environment

  • @openai/codex-security 0.1.5, current main at a8fc009
  • macOS 15 (Darwin 25.5.0), Node.js 24.11.1, Bun 1.3.14

Where the asymmetry is

SDK side, sdk/typescript/src/contract.ts in validateSeal. Coverage receipts are checked for membership in the sealed set:

if (!artifactPaths.has(normalized)) {
  throw new ContractValidationError(
    `Coverage receipt is missing from sealed artifacts: ${receipt}`,
  );
}

Write-ups are not:

for (const [index, finding] of findings.findings.entries()) {
  const writeup = finding.writeup;
  if (writeup === undefined) continue;
  const file = await openCheckedScanFile(
    scanDir, writeup.reportPath, `findings[${index}].writeup.reportPath`, signal, expectedRoot,
  );
  await file.close();
}

and neither is the hardening portfolio, a few lines below.

Plugin side confirms it is by construction, not an oversight in one layer. _bundled_plugin/scripts/finalize_scan_contract.py builds the sealed set from three sources only:

scan["artifacts"] = [
    _artifact_record(scan_dir, "findings.json", "application/json", findings_bytes),
    _artifact_record(scan_dir, "coverage.json", "application/json", coverage_bytes),
    *[
        _artifact_record(scan_dir, ref, "application/octet-stream")
        for ref in _coverage_receipt_refs(coverage)
    ],
]

Write-ups are handled separately by _require_derived_writeup_files, which only asserts the files exist.

Reproduction

Seal a scan whose finding carries writeup.reportPath, load it (passes), replace that markdown file's entire contents, then load again:

sealed scan with writeup loads OK; reportPath = findings/path-traversal.archive-extraction/path-traversal.archive-extraction.md
WRITEUP TAMPERING NOT DETECTED by loadContract
CONTROL: sealed-document tampering detected: manifest.scan.artifacts[1]: sealed artifact changed or is missing.

The control line shows the seal is otherwise working — tampering with a sealed document is caught.

Not a bug, for the record

While investigating this I checked whether a manifest could simply omit findings.json or coverage.json from scan.artifacts to dodge verification. It cannot: the schema's allOf / contains / maxContains: 1 clauses force both into the array. That part is sound.

Suggested direction

In validateSeal, apply the rule that already governs coverage receipts — require safeRelativePath(writeup.reportPath) and safeRelativePath(hardening.portfolioPath) to be members of artifactPaths — and have finalize_scan_contract.py emit _artifact_record(...) entries for them.

The two halves have to land together. Tightening the SDK alone would reject every scan sealed by a current plugin, since those manifests do not list the write-ups. That is why I am filing this rather than opening a PR: the change spans the bundled plugin, and the migration story for already-sealed scans is yours to choose.

Happy to implement whichever shape you prefer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions