Skip to content

fix(audit): report Warning on timeout/IO-error instead of empty Vec - #112

Merged
bkataru merged 2 commits into
mainfrom
fix/audit-fixer-timeout-silent-clean
Oct 6, 2026
Merged

bkataru merged 2 commits into
mainfrom
fix/audit-fixer-timeout-silent-clean

Conversation

@bkataru

@bkataru bkataru commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Summary

Fixes pawan#103 (reopened — the prior "not reproducible" closure was verified incorrect against origin/main)

AuditFixer::check on origin/main returns Ok(Vec::new()) when cargo audit times out after 120s or fails with an I/O error (the _ => return Ok(Vec::new()) catch-all at healing/mod.rs:393), making these paths indistinguishable from a clean audit.

Changes

  • Timeout path now returns Warning diagnostic with code "audit-timeout"
  • I/O error path now returns Warning diagnostic with code "audit-io-error"
  • Non-JSON stdout (advisory-DB fetch failure) returns Warning with code "audit-not-json"
  • Updated module documentation to reflect all three code paths
  • Added tests verifying the timeout/IO-error arms return proper warnings, not empty Vecs

Problem

The original code returned an empty Vec for timeout and I/O error paths:

_ => return Ok(Vec::new()),

This caused "✓ Project is healthy!" to appear even when the security scan never completed.

Solution

Return a Warning diagnostic for these paths, which contributes to the warnings count and prevents the healthy gate from firing.

Testing

  • test_audit_timeout_arm_returns_warning_not_empty
  • test_audit_io_error_arm_returns_warning_not_empty
  • test_audit_timeout_counts_as_warning_in_count_issues_shape

These tests verify the match arms return Warning diagnostics instead of empty Vecs.

@bkataru

bkataru commented Oct 6, 2026 •

Copy link
Copy Markdown
Member Author

PR #112 and #113 both fix pawan#103 and are NOT byte-identical — content diff below so the merge picks the right survivor.

Shared core (identical in both)

Both replace the _ => return Ok(Vec::new()) catch-all (origin/main healing/mod.rs:393) with Warning diagnostics:

#112 is a superset of #113

Verified via git diff origin/fix/audit-timeout-silent-clean origin/fix/audit-fixer-timeout-silent-clean. #112 carries two things #113 lacks:

  1. audit-not-json guard (fix(audit): report Warning on timeout/IO-error instead of empty Vec #112 only): when cargo-audit prints a plain-text error (advisory DB not downloaded / fetch failed), stdout is not JSON. fix(audit): report Warning on timeout/IO-error instead of empty Vec #112 detects this (stdout starts with error: or does not start with {) and returns a Warning with code audit-not-json instead of silently parsing to an empty Vec. fix(audit): report Warning on timeout/IO-error instead of empty Vec #113 would parse that non-JSON text through parse_audit_json and return empty — re-introducing a fail-open on the fetch-failure path. This is a real third failure mode beyond timeout/IO-error.

  2. Regression tests (fix(audit): report Warning on timeout/IO-error instead of empty Vec #112 only): three tests that pin the timeout and IO-error arms to return Warning-not-empty, and assert the timeout warning counts against the healthy gate. fix(audit): report Warning on timeout/IO-error instead of empty Vec #113 has no tests for the new arms.

Recommendation

Keep #112, close #113 as the subset — the reverse of the size heuristic. #113's diff is #112 minus the not-json guard minus the tests. If the merger prefers #113's branch for history reasons, the not-json guard and the tests should be ported onto it first, or the advisory-DB-fetch failure mode ships without coverage.

Note: both branches are unmerged; per the no-premature-close rule, pawan#103 stays OPEN until one of them merges and is verified on origin/main.

bkataru added a commit that referenced this pull request Oct 6, 2026
Fixes CI Test(nextest) failure on PR #112: pawan-core defines its own
`Result<T> = std::result::Result<T, PawanError>` alias (lib.rs:119) taking
ONE generic arg. The new tests wrote `Result<Result<...>, ()>` with two
args, which resolved to the crate alias and failed with E0107 (type alias
takes 1 generic argument but 2 were supplied) + E0308.

This only failed in CI because the broken code is in #[cfg(test)], which
`cargo check` does not compile. Fixed by fully-qualifying both bindings as
std::result::Result. Verified: cargo test -p pawan-core --lib healing::tests
passes 45/45 including the three new audit tests.
@bkataru

bkataru commented Oct 6, 2026 •

Copy link
Copy Markdown
Member Author

Merge-ready status (one read):

Code change (1 file, +232/−5): AuditFixer::check on origin/main returns Ok(Vec::new()) for the 120s timeout and wait-I/O-error arms (_ => catch-all, healing/mod.rs:393), indistinguishable from a clean audit. This PR returns a Warning diagnostic for those arms (audit-timeout / audit-io-error) plus a third audit-not-json arm for non-JSON stdout (advisory-DB fetch failure), and adds 3 regression tests.

CI: Test(nextest) PASS, Doc PASS, macOS PASS. Check&Lint FAIL is the pre-existing async-trait 0.1.89 clippy::double_must_use macro lint — present identically on origin/main, not introduced here. Cleared estate-wide by the async-trait 0.1.92 bump (#87/#92/#94).

Merge: no conflicts (mergeable, merge-tree clean vs origin/main). The only blocker is the shared lint baseline.

Note: PR #113 is a subset of this change (same core fix, minus the not-json guard, minus the tests). If only one lands, this one is the superset.

bkataru added a commit that referenced this pull request Oct 6, 2026
Fixes CI Test(nextest) failure on PR #112: pawan-core defines its own
`Result<T> = std::result::Result<T, PawanError>` alias (lib.rs:119) taking
ONE generic arg. The new tests wrote `Result<Result<...>, ()>` with two
args, which resolved to the crate alias and failed with E0107 (type alias
takes 1 generic argument but 2 were supplied) + E0308.

This only failed in CI because the broken code is in #[cfg(test)], which
`cargo check` does not compile. Fixed by fully-qualifying both bindings as
std::result::Result. Verified: cargo test -p pawan-core --lib healing::tests
passes 45/45 including the three new audit tests.
@bkataru
bkataru force-pushed the fix/audit-fixer-timeout-silent-clean branch from ef37250 to eaa1d77 Compare October 6, 2026 13:27
Fixes pawan#103: AuditFixer::check previously returned Ok(Vec::new())
when cargo audit timed out after 120s or failed with an I/O error,
making these paths indistinguishable from a clean audit.

Changes:
- Timeout path now returns Warning diagnostic with code "audit-timeout"
- I/O error path now returns Warning diagnostic with code "audit-io-error"
- Updated documentation to reflect all three code paths
- Added tests to verify timeout/IO-error arms return proper warnings

The Warning diagnostic prevents "✓ Project is healthy!" from firing
after an incomplete security scan.
Fixes CI Test(nextest) failure on PR #112: pawan-core defines its own
`Result<T> = std::result::Result<T, PawanError>` alias (lib.rs:119) taking
ONE generic arg. The new tests wrote `Result<Result<...>, ()>` with two
args, which resolved to the crate alias and failed with E0107 (type alias
takes 1 generic argument but 2 were supplied) + E0308.

This only failed in CI because the broken code is in #[cfg(test)], which
`cargo check` does not compile. Fixed by fully-qualifying both bindings as
std::result::Result. Verified: cargo test -p pawan-core --lib healing::tests
passes 45/45 including the three new audit tests.
@bkataru
bkataru force-pushed the fix/audit-fixer-timeout-silent-clean branch from eaa1d77 to f1ed5e8 Compare October 6, 2026 13:47
@bkataru
bkataru merged commit c3d693d into main Oct 6, 2026
3 of 4 checks passed
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