Skip to content

Potential fix for code scanning alert no. 14: Incomplete multi-character sanitization #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
KiidxAtlas wants to merge 1 commit into from
Closed

Potential fix for code scanning alert no. 14: Incomplete multi-character sanitization #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs-engine/src/scraping/sphinxScraper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,15 @@
md = md.replace(/[ \t]+\n/g, '\n');
md = md.replace(/\n{3,}/g, '\n\n');

// As a final safety step, strip any script tags that might have been
// reintroduced by HTML entity decoding above.
let prev: string;
const scriptTagPattern = /<script\b[^>]*>[\s\S]*?<\/script>/gi;

Check failure

Code scanning / CodeQL

Bad HTML filtering regexp High

This regular expression does not match script end tags like </script >.

Copilot Autofix

AI 6 months ago

In general, the robust way to fix this is to avoid hand-written HTML-stripping regexes entirely and instead rely on a well-tested HTML parser or sanitization library that understands browser quirks (for example, ignoring attributes on closing tags, handling malformed tags, etc.). For this specific case, we want to keep existing behavior and minimize changes, while making the "final safety step" actually remove any remaining script content that a browser might interpret as a script, even if the closing tag is malformed.

The single best minimal change here is to stop trying to match well-formed <script>...</script> blocks with a regex that insists on a precise </script> end tag. Instead, we can: (1) remove any remaining <script start tags in a loop, regardless of how they are closed, and (2) optionally also remove standalone </script-like closing tags. Since this function already strips all other tags, losing only the script tag text (or even the script contents) is acceptable and aligns with the comment that this is a "final safety step". Concretely, within htmlToMarkdown near lines 1011–1018 in docs-engine/src/scraping/sphinxScraper.ts, replace the current scriptTagPattern and do/while loop with a simpler pattern that matches any <script tag (opening or malformed), such as /<\/?script\b[^>]*>/gi, and run it until no more matches remain. That avoids reliance on a specific </script> shape, and covers odd cases like </script foo="bar">. No additional imports or helper functions are required.

Suggested changeset 1
docs-engine/src/scraping/sphinxScraper.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/docs-engine/src/scraping/sphinxScraper.ts b/docs-engine/src/scraping/sphinxScraper.ts
--- a/docs-engine/src/scraping/sphinxScraper.ts
+++ b/docs-engine/src/scraping/sphinxScraper.ts
@@ -1009,9 +1009,11 @@
         md = md.replace(/\n{3,}/g, '\n\n');
 
         // As a final safety step, strip any script tags that might have been
-        // reintroduced by HTML entity decoding above.
+        // reintroduced by HTML entity decoding above. We deliberately match
+        // both opening and closing tags with optional attributes to handle
+        // malformed but browser-accepted variants like "</script foo="bar">".
         let prev: string;
-        const scriptTagPattern = /<script\b[^>]*>[\s\S]*?<\/script>/gi;
+        const scriptTagPattern = /<\/?script\b[^>]*>/gi;
         do {
             prev = md;
             md = md.replace(scriptTagPattern, '');
EOF
@@ -1009,9 +1009,11 @@
md = md.replace(/\n{3,}/g, '\n\n');

// As a final safety step, strip any script tags that might have been
// reintroduced by HTML entity decoding above.
// reintroduced by HTML entity decoding above. We deliberately match
// both opening and closing tags with optional attributes to handle
// malformed but browser-accepted variants like "</script foo="bar">".
let prev: string;
const scriptTagPattern = /<script\b[^>]*>[\s\S]*?<\/script>/gi;
const scriptTagPattern = /<\/?script\b[^>]*>/gi;
do {
prev = md;
md = md.replace(scriptTagPattern, '');
Copilot is powered by AI and may make mistakes. Always verify output.
do {
prev = md;
md = md.replace(scriptTagPattern, '');
} while (md !== prev);

return md.trim();
}
}
Expand Down
Loading