Conversation
Both validators refuse all input where the OS does not provide O_NOFOLLOW and O_NONBLOCK, which is intentional. Every CLI-invoking test in both suites is guarded for that, by option or inline, except "does not reflect controls from a failed CLI input path". That test is unguarded in both suites, and it passes there anyway: the safe-open check fires before the missing path is ever opened, and its three assertions -- exit 1, the "Failed to read" prefix, and no control bytes -- hold for the safe-open rejection just as they do for a missing file. So it reports green without exercising the missing-path branch it is named for. Guard it with the same option the sibling CLI tests use, so it skips honestly where it cannot run. No change on platforms that provide the flags, where it continues to run and pass.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Both validators intentionally refuse all input where the OS does not provide
O_NOFOLLOW/O_NONBLOCK. Every CLI-invoking test in both suites is guarded for that — by the{ skip: !HAS_SAFE_INPUT_OPEN }option or by an inline early return — with exactly one exception in each file:does not reflect controls from a failed CLI input path.It is unguarded in both suites, and it passes there regardless. The safe-open check fires before the missing path is ever opened, and all three of its assertions hold for that rejection just as they do for a missing file:
1✅ (safe-open rejection also exits 1)/Failed to read .../✅ (both paths share the prefix)So it reports green without ever exercising the missing-path branch it is named for.
Fix
Apply the same guard the sibling CLI tests already use, in both files, so it skips honestly where it cannot run.
Test plan
node --test skills/security-audit/validate-coverage-ledger.test.cjs— 0 failures; skips go 7 → 8node --test skills/security-audit/validate-findings.test.cjs— this test now skips instead of passing vacuouslyHAS_SAFE_INPUT_OPEN = truein a scratch copy: both tests are then attempted and pass rather than skipped, so nothing is disabled where the flags existRelationship to #31
Independent and non-overlapping. #31 guards 7 other tests in
validate-findings.test.cjs; it does not touch this test or the coverage-ledger suite. These merge in either order.