fix(gap-assessment): fail loud when a requested framework does not resolve in the crosswalk - #220
Conversation
…solve in the crosswalk
Resolve every requested framework against the SCF crosswalk before any
report is produced. Unresolved frameworks abort with new exit code 7 and
a per-framework reason: the specific no-public-crosswalk explanation
from scf-client's FRAMEWORKS_WITHOUT_SCF_CROSSWALK table for HITRUST /
StateRAMP / PBMM / US-Export, or a not-found hint pointing at
frameworks.js and FRAMEWORK-COVERAGE.md otherwise.
Previously an unresolvable framework rendered a normal green report
("Tier 1 blockers (0) - None") with exit 0, indistinguishable from a
genuine clean pass. Drops HITRUST-CSF and PBMM from the documented
alias list and documents the unsupported set and the new exit code.
Closes #215
Co-authored-by: Ethan Troy <ethanolivertroy@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 22 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesGap-assessment framework resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant GapAssessment
participant SCFCrosswalk
participant Report
Operator->>GapAssessment: request frameworks
GapAssessment->>SCFCrosswalk: resolve each framework
SCFCrosswalk-->>GapAssessment: metadata or unresolved reason
alt all frameworks resolve
GapAssessment->>Report: load findings and generate report
else framework unresolved
GapAssessment-->>Operator: report error and exit 7
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR prevents gap assessments from producing clean reports when requested frameworks cannot be resolved in the SCF crosswalk.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| plugins/grc-engineer/scripts/gap-assessment.js | Adds framework-resolution preflight validation before findings are loaded or reports are generated. |
| plugins/grc-engineer/commands/gap-assessment.md | Documents unsupported framework labels and the new unresolved-framework exit code. |
| tests/gap-assessment-framework-resolution.test.mjs | Adds focused unit cases for successful, unknown, unsupported, and mixed framework resolution. |
| CHANGELOG.md | Records the corrected fail-loud behavior and associated documentation changes. |
Reviews (2): Last reviewed commit: "fix(gap-assessment): resolve frameworks ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/grc-engineer/scripts/gap-assessment.js`:
- Around line 63-75: Move SCF initialization and the resolveRequestedFrameworks
call before discoverSources and its no-source validation, using the existing
resolution and unresolved-framework handling. Ensure unresolved requested
frameworks fail with EXIT_UNRESOLVED_FRAMEWORK and their framework-specific
reasons before any no-source exit, while source validation runs only after all
frameworks resolve.
In `@tests/gap-assessment-framework-resolution.test.mjs`:
- Around line 10-12: Update the HIPAA entry in the test’s known framework
mapping and its corresponding expected result to use the canonical framework ID
usa-federal-law-hipaa-security-rule-2013, while preserving the existing display
name and other mappings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49bda3d3-e4a3-459f-8040-61632278b322
📒 Files selected for processing (4)
CHANGELOG.mdplugins/grc-engineer/commands/gap-assessment.mdplugins/grc-engineer/scripts/gap-assessment.jstests/gap-assessment-framework-resolution.test.mjs
…canonical HIPAA id in test stub An unresolvable framework can never succeed, so it now fails with exit 7 and its specific reason even when the findings cache is empty, instead of being masked by the NO_SOURCES exit. Valid frameworks with an empty cache still exit 3. Test stub now uses the canonical usa-federal-law-hipaa-security-rule-2013 id. Co-authored-by: Ethan Troy <ethanolivertroy@users.noreply.github.com>
2 participants
Summary
Fixes the silent-green failure mode from #215:
gap-assessmentrendered a normal, celebratory report ("Tier 1 blockers (0), None. 🎉") and exited 0 when the requested framework could not be resolved in the SCF crosswalk, so "never evaluated" was visually indistinguishable from a genuine clean pass.Every requested framework is now resolved against the crosswalk before any report is produced. If any fail to resolve, the run aborts with new exit code
7, lists each unresolved framework with a specific reason, and writes no report bundle:scf-client.js'sFRAMEWORKS_WITHOUT_SCF_CROSSWALKtable, which was exported but previously never called fromgap-assessment.js.frameworks.js --search=<term>anddocs/FRAMEWORK-COVERAGE.md.Per the issue's suggestion,
HITRUST-CSF(andPBMM, which has the same problem) are dropped from the documented alias list incommands/gap-assessment.md, with a new "Not supported" note and the exit code documented.Resolves #215
Type of change
Schema impact
schemas/finding.schema.jsonchangedTest plan
New hermetic unit suite
tests/gap-assessment-framework-resolution.test.mjs(stubbed SCF client, no network): resolved/unresolved separation, HITRUST-specific reason, generic not-found reason, distinct exit code. Named*.test.mjsso #216'stests/run-unit-tests.shpicks it up automatically once that lands.End-to-end against the live SCF crosswalk (v2026.1) using the exact reproduction from #215:
Also ran
npm run test:contract(49 fixtures valid) and markdownlint on the changed Markdown files (clean).CHANGELOG
Notes for reviewers
NO_SOURCES(empty cache) still takes precedence, matching existing behavior.resolveRequestedFrameworksandEXITare exported for the test; the CLI guard (invokedFromCLI) already made the module import-safe.Summary by CodeRabbit
7and framework-specific explanations.