Skip to content

feat(check): SARIF 2.1.0 output mode for clad check (closes #195)#201

Closed
yuyu04 wants to merge 1 commit into
qwerfunch:developfrom
yuyu04:feature/sarif-export
Closed

feat(check): SARIF 2.1.0 output mode for clad check (closes #195)#201
yuyu04 wants to merge 1 commit into
qwerfunch:developfrom
yuyu04:feature/sarif-export

Conversation

@yuyu04

@yuyu04 yuyu04 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds clad check --format sarif — a SARIF 2.1.0 output mode for the gate, so every drift finding surfaces inline on the PR diff, in the GitHub Security tab (github/codeql-action/upload-sarif), and in any SARIF viewer, instead of living only in the terminal/--json view. The mapping is a near 1:1 re-projection of the existing DriftFinding shape — no new analysis.

Linked issue

Closes #195

PR contract (GOVERNANCE.md §4.3)

  • npm run typecheck clean
  • npm run lint clean
  • npm test — all tests pass (147 files / 1509 tests)
  • npm run stage:drift — zero error-severity findings (strict pre-push gate GREEN)
  • npm run conformance — fixtures matched (33/33 in the current suite)
  • node bin/clad check — 15-stage gate green
  • Touches a shipped feature → spec/features/sarif-export-acedface.yaml (F-acedface) authored; spec.yaml/spec/index.yaml inventory synced; attestation stamped
  • CHANGELOG.md entry added under [Unreleased] (Added + Fixed)

Scope of change

  • Minor — additive CLI output mode + a feature shard; default text/--json output unchanged

Out-of-scope checklist (must all be no)

  • Does not regress Iron Law conformance — if anything it strengthens it (a RED gate can no longer serialize to a falsely-clean SARIF)
  • Does not bypass the anti-self-cert guard
  • Does not fork the Ironclad spec

What's in the box

  • src/stages/sarif.ts — pure, deterministic serializer. detector → rule, severity (error/warn/info) → level (error/warning/note), path+line → physicalLocation. Each result carries a partialFingerprints value (sha256 over the finding's stable identity) so re-runs de-duplicate code-scanning alerts. A blocking stage with no findings (type/test failure) still emits a result keyed on the stage id — a failing gate is never represented as clean. No clock/PRNG → byte-identical output across runs.
  • src/cli/clad.ts--format <fmt> wiring (unknown value → clear error, non-zero exit). The tool version is threaded from commander's program.version() rather than a new literal, to avoid adding a 10th version-bump/HARNESS_INTEGRITY site.

Notes for the reviewer

  • Latent bug fixed alongside: clad check's machine-output modes previously called process.exit(worst), which can terminate before a buffered stdout pipe (>64 KB) flushes — SARIF is ~150 KB and was being truncated when piped to another process (file redirects were already safe because those writes are synchronous). Changed to set process.exitCode and let the event loop drain. This also touched 6 existing runCheckCommand tests that spied on process.exit; they now assert process.exitCode (with a reset in beforeEach/afterEach so a recorded code can't leak into vitest's own exit status).
  • Anti-self-cert note: I authored the implementation and its tests in the same pass; this PR relies on the human-in-the-loop reviewer (per §4.3) for the independent sign-off rather than claiming self-certification.
  • Follow-up candidates (separate issues, not in this PR): GitHub Actions annotations as a GHAS-free complement (#... TBD), and emitting the gate result as JUnit XML.

`clad check --format sarif` serializes the gate result as a SARIF 2.1.0 log
so every drift finding surfaces inline on the PR diff, in the GitHub Security
tab (github/codeql-action/upload-sarif), and in any SARIF viewer — instead of
living only in the terminal/`--json` view.

Mapping is 1:1 with the existing finding shape: detector → rule, severity
(error/warn/info) → level (error/warning/note), path+line → physicalLocation.
Each result carries a deterministic partialFingerprints value so re-runs
de-duplicate alerts. A blocking stage that produced no findings (type/test
failure) still surfaces as a result, so a RED gate never serializes to a
falsely-clean SARIF. Output is deterministic (no clock/PRNG); default text
and --json outputs are unchanged.

Also fixes a latent truncation: clad check's machine-output modes now set
process.exitCode and let stdout drain instead of calling process.exit(),
which could terminate before a buffered stdout pipe (>64KB) flushed — SARIF
is ~150KB and was truncated when piped (file redirects were already safe).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yuyu04

yuyu04 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Reducing #201 to the pipe-safety fix only, as decided. That fix is now extracted into its own PR — #221 (process.exit → process.exitCode, value independent of SARIF). The SARIF 2.1.0 body remains parked here; reopen this PR if/when GitHub code-scanning adoption is decided. Closing in favor of #221.

@yuyu04 yuyu04 closed this Jun 30, 2026
yuyu04 added a commit to yuyu04/cladding that referenced this pull request Jul 2, 2026
…exit()

`clad check`'s machine-output mode (--json) can write >64KB to stdout. Calling
process.exit(worst) terminates Node before a buffered stdout PIPE flushes, so a
consumer that pipes the output (`clad check --json | cat`, vs. redirecting to a
file) gets a TRUNCATED document. Setting process.exitCode and letting the event
loop drain guarantees the full payload is emitted, then Node exits with the
worst stage code.

This is the pipe-safety fix extracted from qwerfunch#201 (SARIF) on its own — it has
value independent of SARIF: it fixes the existing --json path. The SARIF body
stays in qwerfunch#201 for when GitHub code-scanning adoption is decided.

- runCheckCommand: process.exit(...).worst → process.exitCode = ...worst
- tests/cli/clad.test.ts: the six runCheckCommand tests now assert
  process.exitCode (process.exit is no longer called for that path); beforeEach
  resets process.exitCode, afterEach restores 0 so a recorded failure code
  can't leak into vitest's own exit status. Other commands (sync/status/init/
  checkpoint/rollback) still use process.exit and keep their exitCalls asserts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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