fix(codeql): close remaining 6 alerts — fs races + dead code#54
Merged
Manavarya09 merged 1 commit intomainfrom May 3, 2026
Merged
fix(codeql): close remaining 6 alerts — fs races + dead code#54Manavarya09 merged 1 commit intomainfrom
Manavarya09 merged 1 commit intomainfrom
Conversation
Closes the last 6 open CodeQL alerts on main. HIGH — js/file-system-race (two TOCTOU windows): - src/sync.js #70 — updateIfExists() previously did `if (statSync(path).isFile()) writeFileSync(...)`. The window between stat and write was exploitable on shared filesystems. Replaced with `openSync(path, 'r+')` which atomically requires-existing-file + acquires a write descriptor in one syscall, then ftruncate + writeSync through that fd. Verified ENOENT semantics with a quick repl test. - src/studio.js #71 — the local studio's static-file handler did `if (statSync(p).isFile()) readFileSync(p)`. The stat was redundant — readFileSync surfaces ENOENT / EISDIR / EACCES on its own and we catch all of them. Dropped the stat; now one syscall, no race. NOTE — js/unused-local-variable (dead code): - src/chat.js #76, #77 — hexToRgb / rgbToHex helpers were defined but never called. Color ops in chat.js work directly on hex strings. - website/app/components/HeroExtractor.js #74, #75 — `copied` state and `handleCopyMarkdown` callback were a never-wired feature (no JSX call site). Deleted both. If we want a "Copy markdown" button later, that's feature work, not cleanup. 336/336 tests pass. All touched modules import-load cleanly.
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 aims to clear the remaining CodeQL alerts by removing dead code and hardening two filesystem access paths against check-then-use races. In the broader codebase, these changes affect the CLI sync workflow, the local studio server’s static file serving, and the website extractor UI component.
Changes:
- Reworked
src/sync.jsto update existing output files through a single opened file descriptor instead of astat+ write sequence. - Simplified
src/studio.jsstatic file serving by removing a redundantstatSyncbeforereadFileSync. - Removed unused helper/state code from
src/chat.jsandwebsite/app/components/HeroExtractor.js.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
website/app/components/HeroExtractor.js |
Removes unused clipboard-related state and callback from the extractor UI. |
src/sync.js |
Replaces the previous existence-check/write flow with fd-based truncate/write logic for sync output files. |
src/studio.js |
Simplifies static file reads to avoid a stat/read race window. |
src/chat.js |
Deletes unused color conversion helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+21
| ftruncateSync(fd, 0); | ||
| writeSync(fd, content, 0, 'utf-8'); |
Comment on lines
+11
to
+15
| // Race-free "update only if file exists" — open with 'r+' atomically | ||
| // requires an existing file (throws ENOENT otherwise) and gives us a | ||
| // write-capable descriptor in one syscall, eliminating the toctou window | ||
| // that statSync→writeFileSync would have. Truncate then write through the | ||
| // same fd so no other process can sneak between check and write. |
akasaj-uet
pushed a commit
to akasaj-uet/design-extract
that referenced
this pull request
May 4, 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 the last 6 open CodeQL alerts on `main`.
High — `js/file-system-race`
Note — `js/unused-local-variable` (dead code)
Test plan
After this lands, the open CodeQL alert count should drop from 6 → 0.
🤖 Generated with Claude Code