Skip to content

fix(ratchet): reject incomplete detection data - #937

Open
luojiyin1987 wants to merge 7 commits into
OWASP:mainfrom
luojiyin1987:fix/ratchet-incomplete-scan-data
Open

fix(ratchet): reject incomplete detection data#937
luojiyin1987 wants to merge 7 commits into
OWASP:mainfrom
luojiyin1987:fix/ratchet-incomplete-scan-data

Conversation

@luojiyin1987

@luojiyin1987 luojiyin1987 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reject ratchet baseline work when detection data is incomplete.

  • Block single-folder baseline creation and comparison.
  • Block baseline creation and comparison for folders with incomplete detection data.
  • Allow remediation-only diagnostics because they do not change findings.

This PR does not add terminal, JSON, or HTML diagnostic output.

Part of #907.

Validation

  • npm run build
  • npm test -- --runInBand tests/cli-integration.test.ts tests/multi-folder-scan.test.ts

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @luojiyin1987 - the core call is right: gating ratchet on detection-impact gaps (so a failed fix-version lookup does not invalidate a good baseline) is exactly the level to reason at. A few things before it goes in, plus a coordination note with #936.

  • Multi-folder over-block: this blocks ratchet for every subfolder if any single one has a detection gap, but the code just below treats folders independently. In a monorepo, one folder's transient OSV failure would block the baseline for all the clean folders too. Can we scope it per-folder? And a mixed-case test (one clean folder, one incomplete) would lock the behavior in - right now both test folders share the same mock.
  • The block message is a static string - a CI user cannot tell which diagnostic, package, or subfolder tripped it. #936 adds getCompletenessImpact() / formatDiagnosticMessage() for exactly this; can we surface the real diagnostic here? Actionable output is the whole point.
  • Re-run guidance edge case: OSV_DETAIL_CONFIRMED_MISSING is permanent, not transient, so re-running will not help and there is no override - a project that hits a permanently-missing record gets locked out of --ratchet. Fine as a fast-follow (maybe under #907), but let's not tell them to re-run when it cannot fix it.
  • Coordination/DRY: #936 introduces the shared getCompletenessImpact() helper and you duplicate the same detection-gap predicate here. Plan is to merge #936 first, then rebase this onto it and swap the inline .some() checks for that helper.

@luojiyin1987

Copy link
Copy Markdown
Collaborator Author

Thanks, agreed on all points. I’ll wait for #936 to land, then rebase this PR onto main, switch to the shared completeness helpers, scope the multi-folder ratchet check per folder, surface the actual diagnostics, and add a mixed clean/incomplete folder test.

I’ll also avoid the blanket re-run guidance for confirmed-missing diagnostics.
@sonukapoor

@luojiyin1987

Copy link
Copy Markdown
Collaborator Author

Updated after #936 landed.

  • scoped multi-folder ratchet gating per folder, so clean folders still save/evaluate their baselines
  • switched to the shared getCompletenessImpact() / formatDiagnosticMessage() helpers
  • surfaced the actual detection diagnostics
  • removed the blanket re-run guidance and added coverage for OSV_DETAIL_CONFIRMED_MISSING
  • added a mixed clean/incomplete multi-folder test

CI, Self Scan, and CodeQL are passing.

@sonukapoor

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.

2 participants