Skip to content

fix(sbom): strip --!> when reading a copyright holder - #1330

Merged
aristarkhovNV merged 1 commit into
mainfrom
aaristarkhov/sbom-bad-tag-regex
Oct 24, 2026
Merged

aristarkhovNV merged 1 commit into
mainfrom
aaristarkhov/sbom-bad-tag-regex

Conversation

@aristarkhovNV

@aristarkhovNV aristarkhovNV commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Resolves code-scanning alert #52 (CodeQL py/bad-tag-filter).

_COMMENT_MARKUP in scripts/sbom/isaaccapture_sbom/discovery.py does not sanitize HTML. It trims comment delimiters from a scraped copyright line so the holder name comes out clean, so the alert's XSS framing doesn't apply. Its point about correctness still holds: HTML also ends a comment at --!>, and a notice closed that way gave the holder Bar Ltd --!>.

  • Accept --!?> as a comment end.
  • Add an html-bang-end case to test_comment_markup_is_not_part_of_a_scraped_holder.

Testing

  • uv run --project scripts/sbom --extra dev pytest scripts/sbom/tests -k comment_markup: all three cases pass.
  • SKIP=check-copyright-year pre-commit run --all-files passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved copyright text extraction by excluding HTML comments that end with --!>, as well as standard --> comments.

CodeQL flagged _COMMENT_MARKUP (py/bad-tag-filter, alert #52) for
accepting only `-->` as an HTML comment end. The regex is not a
sanitizer; it trims comment delimiters from a copyright line so the
holder name comes out clean. HTML also ends a comment at `--!>`, and a
notice closed that way produced the holder "Bar Ltd --!>".

Accept `--!?>` and cover it in the comment-markup test.

Signed-off-by: Andrei Aristarkhov <aaristarkhov@nvidia.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between ba15610 and eefd240.

📒 Files selected for processing: 2
  • scripts/sbom/isaaccapture_sbom/discovery.py
  • scripts/sbom/tests/test_discovery.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

The comment-markup pattern now recognizes --!> as an HTML comment terminator, in addition to -->. The parameterized test includes a case for the new terminator.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to eefd2

No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: stripping the HTML comment terminator --!> from scraped copyright holders.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@aristarkhovNV
aristarkhovNV merged commit 4443a5b into main Oct 24, 2026
37 checks passed
@aristarkhovNV
aristarkhovNV deleted the aaristarkhov/sbom-bad-tag-regex branch October 24, 2026 03:44

This branch was successfully deployed

1 active deployment
dev — eefd2405 Deployed Oct 23, 2026 by aristarkhovNV via publish-wheel #5962
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants