fix(codeql): tighten host assertions + drop unused import#52
Merged
Manavarya09 merged 1 commit intomainfrom May 2, 2026
Merged
fix(codeql): tighten host assertions + drop unused import#52Manavarya09 merged 1 commit intomainfrom
Manavarya09 merged 1 commit intomainfrom
Conversation
Three CodeQL alerts (#78, #80, #81 — js/incomplete-url-substring-sanitization)
flagged `html.includes('example.com')` patterns in tests as potential URL
sanitization gaps. They are *test* assertions, not security checks, but the
same patterns are dangerous in real validators — replacing with regex
anchored inside rendered HTML markup ('>host<') makes the intent explicit
and silences the rule.
Also removes the unused `diffDarkMode` import in bin/design-extract.js
(alert #79 — js/unused-local-variable). The CLI uses `diffDesigns` from
src/diff.js, not the dark-mode-only `diffDarkMode` helper.
No behavior change. 336/336 tests still pass.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR makes a small, surgical cleanup to address CodeQL findings without changing runtime behavior: it tightens a few formatter test assertions so they clearly target rendered markup, and removes an unused CLI import. In the broader codebase, this keeps the formatter test suite aligned with static-analysis expectations and trims dead code from the command entrypoint.
Changes:
- Replaced broad hostname substring assertions in formatter HTML tests with regex assertions anchored to rendered markup.
- Added explanatory comments clarifying that the updated assertions are test-only markup checks, not URL validation logic.
- Removed the unused
diffDarkModeimport from the CLI entrypoint.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/formatters.test.js |
Tightens host-rendering assertions in formatGrade and formatBattle tests to avoid CodeQL false positives. |
bin/design-extract.js |
Removes an unused diffDarkMode import from the CLI bootstrap file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 tasks
sathvikc
pushed a commit
to sathvikc/design-extract
that referenced
this pull request
May 3, 2026
Closes 8 CodeQL js/unused-local-variable alerts. All deletions are pure import-list trims — no runtime behavior changes: src/history.js -- existsSync (Manavarya09#72) src/sync.js -- readFileSync (#73) src/visual-diff.js -- basename (Manavarya09#62) src/index.js -- formatAnatomyStubs (Manavarya09#57; still re-exported) src/index.js -- formatMotionTokens (Manavarya09#58; still re-exported) src/formatters/tailwind.js -- rgbToHex (Manavarya09#54) src/formatters/vue-theme.js -- rgbToHsl (Manavarya09#55) src/formatters/css-vars.js -- pxToRem (Manavarya09#52) tests/wide-gamut.test.js -- oklchToSrgb, oklabToSrgb (Manavarya09#64) Confirmed via grep that none of the removed names had call sites in their source file. The two src/index.js entries still appear as `export { … } from '…'` re-exports on lines 201–202 — those are self-contained re-exports, not affected by the removed imports. Skipped intentionally: - Unused functions (Manavarya09#66–Manavarya09#69, Manavarya09#61, #76–#77) — may be retained as internal API surface; need per-call review. - File-system race conditions (Manavarya09#70, Manavarya09#71) in sync.js / studio.js — real bugs, but need a dedicated atomic-write fix, not in scope. 336/336 tests pass; all touched modules load cleanly.
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
Closes 4 CodeQL alerts on `main`:
The "incomplete URL substring sanitization" alerts are CodeQL false positives — the patterns are test assertions, not URL validators. But the same patterns are dangerous in real validators, so swapping `html.includes('example.com')` → `html.match(/>example.com</)` is both:
The `diffDarkMode` import was dead — the CLI uses `diffDesigns` from `src/diff.js`.
Out of scope
Skipped pre-existing alerts to keep this PR surgical:
Test plan
🤖 Generated with Claude Code