Skip to content

refactor: re-check validated run inputs through one shared helper - #173

Merged
Nagi-Inaba merged 2 commits into
mainfrom
refactor/centralize-validated-run-recheck
Oct 12, 2026
Merged

Nagi-Inaba merged 2 commits into
mainfrom
refactor/centralize-validated-run-recheck

Conversation

@Nagi-Inaba

Copy link
Copy Markdown
Owner

Problem

A command that validates a run and then writes must confirm that every file the validation read still holds the validated bytes. Otherwise a file can be swapped between the check and the write. validateAuditRun returns three such collections: registered artifacts, saved evidence, and the historical resources used to validate older run formats.

Each command carried its own copy of the loops, and the copies disagreed. Only render-report.mjs and validate-audit-run.mjs also covered the historical resources, so every other command could write after one of those files changed.

Change

  • lib/audit-run-validation.mjs now owns the single list of those inputs and exports assertValidatedRunStable(validation, labels) and validatedRunSnapshots(validation) (for commands that collect snapshots and re-check them in a write hook). Both are re-exported by lib/audit-run.mjs.
  • The 18 hand-written re-check blocks and 5 hand-written collections in 20 files are replaced one for one, keeping the labels that appear in failure messages (saved evidence, sibling registered artifact, predecessor evidence and so on). A command that renders or writes now also covers the historical resources, which closes the gap above. The helper accepts the shapes of an invalid validation, so status reporting is unaffected.
  • tests/validated-run-stability.test.mjs covers each of the three collections refusing a change with its label, custom labels, tolerance of a validation without snapshots, and a guard that fails when a script reintroduces a hand-written loop over validateAuditRun's collections.
  • Add the test to release-files.json and a CHANGELOG entry.
  • Left as they are on purpose: the hand-written argument parsers, the BOM-stripping JSON read idiom and the usage text. The parsers check their arguments in at least six different orders, so sharing one would change some commands' error precedence or need an ordering parameter, and none of them is a safety property.

Verification

  • node scripts/verify-all.mjs on Windows / Node.js 22.19.0: PASS, 92 test files
  • Codex / Claude parity remains valid
  • Canonical files were edited and node scripts/sync-distributions.mjs --write was followed by --check
  • Relevant focused regression test added: tests/validated-run-stability.test.mjs
  • Remote CI is reported separately. This PR touches render-report.mjs, lib/network-cli.mjs and import-scanner-results.mjs, so the path-filtered report and Web evidence workflows are expected to run.

Compatibility

  • Schema/profile/procedure version impact: none
  • Migration required: no
  • Behavior change: commands that render or write now also refuse when a historical resource changed after validation. Failure messages keep their labels.
  • CHANGELOG updated. docs/version-support.md: not applicable.

Security and privacy

  • No credentials, private URLs, local paths, or real audit evidence are included
  • Boundaries are stricter, never looser: more commands re-check more inputs before writing
  • Release preparation, publication and remote CI results are reported separately

Merge note

Sibling PRs from the same batch also add one bullet at the same place in CHANGELOG.md. After one of them merges, the others need a trivial conflict resolution that keeps both bullets.

Issue

Part of the follow-ups listed after PR #169. Closes no issue.

🤖 Generated with Claude Code

A command that validates a run and then writes must confirm that every file the
validation read still holds the validated bytes, otherwise the file can be
swapped between the check and the write. validateAuditRun returns three such
collections: registered artifacts, saved evidence and the historical resources
used to validate older run formats. Each command carried its own copy of the
loops, and the copies disagreed: only render-report.mjs and validate-audit-run.mjs
also covered the historical resources, so every other command could write after
one of those files changed.

lib/audit-run-validation.mjs now owns the single list of those inputs and exports

- assertValidatedRunStable(validation, labels) to re-check them, and
- validatedRunSnapshots(validation) for commands that collect snapshots and
  re-check them in a write hook.

Both are re-exported by lib/audit-run.mjs. The 18 hand-written re-check blocks
and 5 hand-written collections in 20 files are replaced one for one, keeping the
labels that appear in failure messages (saved evidence, sibling registered
artifact, predecessor evidence and so on). A command that renders or writes now
also covers the historical resources, which closes the gap above; the same
helper accepts the shapes of an invalid validation so status reporting is
unaffected.

tests/validated-run-stability.test.mjs covers each of the three collections
refusing a change with its label, custom labels, tolerance of a validation
without snapshots, and a guard that fails when a script reintroduces a
hand-written loop over validateAuditRun's collections.

The hand-written argument parsers, the BOM-stripping JSON read idiom and the
usage text are intentionally left as they are: the parsers check their
arguments in at least six different orders, so sharing one would change some
commands' error precedence or need an ordering parameter, and none of them is a
safety property.

Verified: verify-all passes on Node.js 22.19.0 (Windows), 92 test files.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T05:38:27.679426Z 7ef351a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Keep all CHANGELOG entries. The Node.js 22 baseline (pull request 171), the
HTTP observer retirement (pull request 172) and this change each add one
bullet at the same place in the Unreleased section; no other file conflicts.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Nagi-Inaba
Nagi-Inaba merged commit bd13368 into main Oct 12, 2026
9 checks passed
@github-actions
github-actions Bot deleted the refactor/centralize-validated-run-recheck branch October 12, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant