Fix CWE tag mapping for IDs with leading zeros#192
Conversation
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
@copilot the ci test failed: https://github.com/advanced-security/codeql-sarif-security-standard-annotator/actions/runs/21215699118/job/61049679966?pr=192 {"text":"org.springframework.web.client.RestTemplate#exchange(String,HttpMethod,HttpEntity,Class,Object\[\])"}}],"semmle.formatSpecifier":"sarif-latest"}}]} Why did you not catch this when you ran the test? Is there anything missing here in the ci testing or the CCA runtime that needs to be fixed so they match? |
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
The CI test failure was expected and has been fixed in commit 869c6d7. Root cause: The fix correctly normalizes CWE IDs with leading zeros (e.g., "cwe-079" → "79"), allowing them to match entries in the XML file (CWE_ID="79"). This means the output now includes OWASP tags for CWE IDs that were previously missed due to the bug. Why local tests didn't catch it: The local unit tests validated the normalization logic but didn't regenerate the integration test's expected output file. The expected file was created with the buggy code and was missing tags for ~12 CWE IDs with leading zeros. Fix: Regenerated |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where CWE tags with leading zeros (e.g., "external/cwe/cwe-079") were not properly mapping to CWE XML documents that store IDs without leading zeros (e.g., "99"). The fix normalizes CWE IDs by converting them to integers and back to strings, effectively removing leading zeros while maintaining proper mapping.
Changes:
- Normalize CWE IDs by parsing as integers to strip leading zeros
- Add validation to skip non-numeric CWE IDs that would result in NaN
- Add comprehensive test coverage for CWE ID normalization including edge cases
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main.ts | Added CWE ID normalization logic to convert IDs to integers (removing leading zeros) and validation to skip invalid non-numeric IDs |
| tests/main.test.ts | Added test suite for CWE ID normalization covering leading zeros, multiple leading zeros, no leading zeros, and invalid non-numeric IDs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot changes made - regenerate output Also, would you consider implementing this feedback - i am not clear this is correct:
|
… dist Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Done in commit 77fb98c. Changes made:
This addresses the AI reviewer's feedback by eliminating code duplication and ensuring tests verify the real implementation. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Fix CWE Leading Zero Mapping Bug
normalizeCweId()function in utils.tsSummary
Issue: CWE tags in SARIF files with leading zeros (like "external/cwe/cwe-079") were not properly mapping to CWE XML documents (which store IDs as "99" without leading zeros). This caused the tool to miss valid CWE mappings.
Solution:
CI Test Fix:
The CI test was failing because the expected output file was generated with the buggy code and was missing tags for CWEs with leading zeros. Regenerated the expected output file using the fixed code, which now correctly includes OWASP tags for all matching CWE IDs (including those with leading zeros like cwe-079, cwe-020, etc.).
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.