Repository navigation
Conversation
|
Warning Review limit reachedNext included review available in 39 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a SAST scanner endpoint. It generates JSON rows from cataloged vulnerability methods, registers the endpoint, and adds workflow checks for direct and aggregated scanner responses. ChangesSAST endpoint
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Router
participant SASTScanner
Workflow->>Router: GET /VulnerableApp-php/scanner/sast
Router->>SASTScanner: Invoke sast()
SASTScanner-->>Router: Return SAST rows as JSON
Router-->>Workflow: Return scanner response
Workflow->>Workflow: Validate response fields and VulnerableApp-php key
Merge Risk: ⚪ Minimal · up to The new endpoint returns the expected JSON row shape and is registered for facade consumption. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 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 |
|
|
||
| class SASTScanner | ||
| { | ||
| private const CATALOG = [ |
There was a problem hiding this comment.
I think it would be better if we have a csv file listing all the sast vulnerabilities and sast endpoint exposes it as Json file. you can look at https://github.com/SasanLabs/VulnerableApp/blob/master/src/main/resources/scanner/sast/expectedIssues.csv for more information.
| "filePath" => "src/MagicHashVulnerability/MagicHash.php", | ||
| "methods" => ["level1", "level2"], | ||
| "cwe" => "CWE-704", | ||
| "type" => "Magic Hash Exploitation", |
There was a problem hiding this comment.
Sast has line number as wel. please follow the format mentioned at https://github.com/SasanLabs/VulnerableApp/blob/master/src/main/resources/scanner/sast/expectedIssues.csv
Summary
GET /VulnerableApp-php/scanner/sastusing the same response shape as VulnerableAppBootstrapThe catalog keeps the vulnerable level/type mapping explicit while deriving each level's current method line with reflection, so line numbers stay aligned when source files move.
Validation
php -lon the changed PHP filesVulnerableApp-phpkeyCloses #24
Summary by CodeRabbit
New Features
Tests