Fix CodeQL comment tag filter finding - #1180
Closed
brunoborges wants to merge 1 commit into
Closed
Conversation
Patch is-unsafe's XML comment-close detector during builds so generated bundles recognize both HTML comment end forms and satisfy CodeQL until the dependency publishes a fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 277302b1-aa95-4012-817b-9752cdaee14e
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates a CodeQL finding by patching a transitive dependency (is-unsafe) during builds so its XML comment-close detector recognizes both --> and --!> when generating the action bundles.
Changes:
- Add a build-time patch script that rewrites the
is-unsafeXML detector pattern from/-->/to/--!?>/with a guard against unexpected source changes. - Run the patch script as part of
npm run buildbeforenccbundling. - Regenerate
dist/setupanddist/cleanupbundles to include the patched pattern.
Show a summary per file
| File | Description |
|---|---|
| scripts/patch-is-unsafe.mjs | New build-time script to patch the transitive dependency’s XML comment-close pattern. |
| package.json | Updates the build script to run the patch step before bundling. |
| dist/setup/index.js | Updates bundled dependency code to use /--!?>/ for XML comment close detection. |
| dist/cleanup/index.js | Updates bundled dependency code to use /--!?>/ for XML comment close detection. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/4 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
+8
to
+20
| // CodeQL treats this XML detector as an incomplete HTML comment-end filter. | ||
| if (source.includes(safePattern)) { | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const occurrences = source.split(vulnerablePattern).length - 1; | ||
| if (occurrences !== 1) { | ||
| throw new Error( | ||
| `Expected one ${JSON.stringify(vulnerablePattern)} in ${sourcePath.pathname}, found ${occurrences}` | ||
| ); | ||
| } | ||
|
|
||
| await writeFile(sourcePath, source.replace(vulnerablePattern, safePattern)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
is-unsafeXML comment-close detector during builds to recognize--!>as well as-->Dependency path
@actions/cache→@azure/storage-blob→@azure/core-xml→fast-xml-parser→is-unsafeValidation
npm run check/--!?>/and no longer contain/-->/