Skip to content

fix: robust discovery, skip unreadable/non-UTF-8 files (M1) - #5

Merged
ThirdChildren merged 1 commit into
masterfrom
fix/m1-robust-discovery
Jul 1, 2026
Merged

ThirdChildren merged 1 commit into
masterfrom
fix/m1-robust-discovery

Conversation

@ThirdChildren

Copy link
Copy Markdown
Owner

Implements the M1 fix from docs/security-review.md: a single unreadable or non-UTF-8 .nr file must not abort the whole scan. First 0.3.0-line item (robustness/availability).

What

  • zkguard-core: new SkippedFile { path, reason, kind } with SkipKind (non_utf8 | unreadable | other_io). ScanResult gains an additive skipped: Vec<SkippedFile> (#[serde(default)], omitted when empty).
  • zkguard-noir: discovery reads each .nr via read_noir_source; on failure it records a SkippedFile in NoirProject::skipped and keeps scanning the rest of the tree. read_to_string's InvalidData maps to non_utf8, permission/not-found to unreadable, else other_io. Directory-level read errors stay hard errors (out of M1 scope). Still: no execution, no symlink following, no network.
  • CLI: emits a stderr warning: per skipped file and carries them in the result. Exit code still depends only on findings.
  • Reports: human summary skipped: N + a "Warnings (skipped files)" section; Markdown "Skipped files" table + summary line; JSON additive skipped array. SARIF unchanged (skips are not security results).

New JSON shape (additive)

Only present when something was skipped:

"skipped": [
  { "path": "src/bad.nr", "reason": "stream did not contain valid UTF-8", "kind": "non_utf8" }
]

No change when the scan is clean (field omitted).

Example (valid + non-UTF-8 file)

Human:

warning: skipped /tmp/proj/bad.nr: stream did not contain valid UTF-8   # stderr
No findings.

Summary:
  files scanned: 1
  rules run:     6
  ...
  total:     0
  skipped:   1

Warnings (skipped files):
  /tmp/proj/bad.nr [non-utf8]: stream did not contain valid UTF-8

Exit code 0 (driven by findings, not the warning).

Tests

  • discovery: valid + non-UTF-8 file → valid scanned, bad one recorded as non_utf8; clean project has no skips.
  • CLI end-to-end: scan continues, warning on stderr, skipped in JSON, exit driven by findings.
  • core: serde default + omit-when-empty.
  • Full suite: 243 passed, 0 failed, 1 ignored. fmt/clippy -D warnings/fixtures validate (26 ok) green.

Docs

docs/security-review.md M1 marked fixed; docs/architecture.md data flow notes partial-tolerant discovery; CHANGELOG [Unreleased] Fixed entry.

Rule semantics unchanged. Not Circom, not zkVM, not ZK-REPLAY-001.

…can (M1)

A single unreadable or non-UTF-8 .nr file no longer aborts the whole scan
(security-review finding M1).

- zkguard-core: new SkippedFile { path, reason, kind } with SkipKind
  (non_utf8 | unreadable | other_io); ScanResult gains an additive, empty-
  skippable `skipped` field.
- zkguard-noir: discovery reads each .nr via read_noir_source; on failure it
  records a SkippedFile in NoirProject.skipped and keeps scanning. Directory-
  level read errors remain hard errors (out of M1 scope). Still no execution,
  no symlink following, no network.
- CLI: warns on stderr per skipped file and carries them in ScanResult; exit
  code still depends only on findings.
- Reports: human summary count + "Warnings (skipped files)" section;
  Markdown "Skipped files" table + summary line; JSON additive `skipped`
  array. SARIF unchanged (skips are not security results).
- Tests: discovery skip (valid + non-UTF-8), clean-project no-skips, CLI
  end-to-end (scan continues, warning emitted, exit driven by findings),
  core serde-default/omit-when-empty.
- Docs: security-review M1 marked fixed; architecture data flow notes
  partial-tolerant discovery; CHANGELOG [Unreleased] Fixed entry.

Rule semantics unchanged.
@ThirdChildren
ThirdChildren merged commit db501bf into master Jul 1, 2026
4 checks passed
@ThirdChildren
ThirdChildren deleted the fix/m1-robust-discovery branch July 1, 2026 15:27
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