Hunt - #1
Conversation
…ter and fix settings footer hints
…s default spinner
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
ThreatCrush Security Scan2 finding(s) MEDIUM: 2
Snippets are redacted; ThreatCrush never prints matched credential material. |
📝 WalkthroughWalkthroughThe application adds persistent bookmarks and search history, bookmark navigation, search-history input controls, configurable queue-completion actions, cross-platform power management, updated settings, and project metadata. ChangesApplication feature integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Storage
User->>App: bookmark result or submit search
App->>Storage: save bookmark or search history
Storage-->>App: persisted state
App-->>User: updated view and notice
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/ui/components/Results.tsx (1)
338-345: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
bhint to the detail footer.The bookmark action works in detail mode, but the detail legend at lines 97-115 lists only
d,y,e, andesc. Users cannot discover the bookmark key in this view. The logic itself matches the list-mode payload.💡 Proposed hint addition (outside the selected range, near line 106)
<Text color={theme.colors.text}> Copy</Text> <Text dimColor>{` ${ICON.dot} `}</Text> + <Text color={theme.colors.accent} bold> + b + </Text> + <Text color={theme.colors.text}> Bookmark</Text> + <Text dimColor>{` ${ICON.dot} `}</Text> <Text color={theme.colors.accent} bold> e </Text>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/components/Results.tsx` around lines 338 - 345, Add a visible “b” bookmark hint to the detail-mode footer legend near the existing d, y, e, and esc hints in Results,tsx, while leaving the existing detail bookmark handler and payload unchanged.src/ui/components/BookmarksView.test.tsx (1)
9-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the keyboard actions.
This test renders the list and asserts the names only. The
useInputhandler inBookmarksView.tsxincludes destructive actions (b/cremove,Cclear all) and the download paths. A test that writes keys through the harness and asserts the spied store actions would protect those paths and would also pin the rendered size and date columns.Do you want me to generate the additional test cases?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/components/BookmarksView.test.tsx` around lines 9 - 31, Extend the BookmarksView test suite beyond name rendering by using the harness to send the useInput keyboard actions, including b/c removal, C clear-all, and download keys, and assert the corresponding store actions via spies. Also assert the rendered size and date columns for the Alpha and Beta bookmarks while preserving the existing list-rendering coverage.src/ui/components/TextField.tsx (1)
98-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the shared history-selection logic.
The
upArrowanddownArrowbranches repeat the same four steps: set the index, set the value, set the cursor, and callonChange. A single helper reduces the risk that the two paths diverge later.♻️ Proposed refactor
+ function selectHistory(idx: number, text: string): void { + setHistoryIdx(idx); + setValue(text); + setCursor(text.length); + onChange?.(text); + } + useInput( (input, key) => { if (key.upArrow) { if (history && history.length > 0) { const nextIdx = historyIdx === -1 ? 0 : Math.min(history.length - 1, historyIdx + 1); if (historyIdx === -1) { draftRef.current = value; } const item = history[nextIdx]; - if (item !== undefined) { - setHistoryIdx(nextIdx); - setValue(item); - setCursor(item.length); - onChange?.(item); - } + if (item !== undefined) selectHistory(nextIdx, item); } return; } if (key.downArrow) { if (history && history.length > 0 && historyIdx >= 0) { if (historyIdx === 0) { - setHistoryIdx(-1); - setValue(draftRef.current); - setCursor(draftRef.current.length); - onChange?.(draftRef.current); + selectHistory(-1, draftRef.current); return; } const nextIdx = historyIdx - 1; const item = history[nextIdx]; if (item !== undefined) { - setHistoryIdx(nextIdx); - setValue(item); - setCursor(item.length); - onChange?.(item); + selectHistory(nextIdx, item); return; } } onExitDown?.(); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/components/TextField.tsx` around lines 98 - 138, Extract the repeated history-item selection steps from the upArrow and downArrow branches in useInput into a shared helper within TextField. Have the helper update the history index, value, cursor position, and invoke onChange, while preserving the existing draft restoration and navigation behavior.src/ui/App.tsx (1)
506-555: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep React state updater callbacks pure.
Move
saveBookmarks,saveSearchHistory, andsetNoticeoutside functional state updaters. This prevents duplicate or stale writes if React re-runs an updater under Strict Mode or concurrent rendering. Use an effect or reducer-controlled action path for persistence and notices.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/App.tsx` around lines 506 - 555, Refactor the bookmark and search-history callbacks so the functional updaters passed to setBookmarks and setSearchHistory only compute and return state, without calling saveBookmarks, saveSearchHistory, or setNotice. Move persistence and notice updates into an effect or reducer-controlled action path, preserving duplicate-bookmark handling and the existing add, remove, clear, and history behaviors.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/download/bookmarks.ts`:
- Around line 33-36: Update isBookmarkItem to validate every required
BookmarkItem field, including sizeBytes and bookmarkedAt as numbers, before
loadBookmarks accepts persisted records. Preserve the existing string checks for
id, name, and magnet.
In `@src/sources/searchHistory.ts`:
- Around line 33-38: Update loadSearchHistory to filter valid non-empty strings
first, then limit the resulting entries to MAX_SEARCH_HISTORY before returning
them. Preserve the existing empty-array behavior for invalid or non-array JSON.
In `@src/ui/App.tsx`:
- Around line 506-524: Update the bookmark handling in the setBookmarks callback
to truncate the newly constructed next list to BOOKMARKS_CAP before using it for
both setBookmarks and saveBookmarks. Preserve duplicate detection and ensure the
UI state and persisted data use the same capped list.
In `@src/ui/components/BookmarksView.tsx`:
- Around line 59-67: The bookmarks UI does not expose the destructive clear-all
action or the alternate single-removal key. Update the bookmarks footer hints in
the keymap configuration to include C for clear-all and c alongside b for single
removal, keeping the existing key behavior unchanged.
In `@src/ui/store.ts`:
- Around line 99-102: Update isBookmarkItem and the bookmark-loading flow in
bookmarks.ts to ensure persisted records include valid sizeBytes and
bookmarkedAt fields before returning them as BookmarkItem[]. Either reject
records missing these required fields or normalize legacy records with
appropriate defaults, matching addBookmark’s sizeBytes behavior.
In `@src/util/power.test.ts`:
- Around line 9-20: Update the power-tool tests and keep-awake process handling
around acquireKeepAwake to mock os.platform and spawn, emit an asynchronous
ENOENT error for an unavailable power tool, and verify the failure is handled
without throwing. Ensure the spawn error handler clears keepAwakeProc only when
it still references the failed active process, preserving an independently
replaced process reference.
In `@src/util/power.ts`:
- Around line 25-42: Update each spawned process in the keep-awake platform
branches to attach an error handler before unref(), and clear keepAwakeProc on
both error and exit only if it still references that specific child process.
Apply this consistently to the PowerShell, caffeinate, and systemd-inhibit spawn
paths.
---
Nitpick comments:
In `@src/ui/App.tsx`:
- Around line 506-555: Refactor the bookmark and search-history callbacks so the
functional updaters passed to setBookmarks and setSearchHistory only compute and
return state, without calling saveBookmarks, saveSearchHistory, or setNotice.
Move persistence and notice updates into an effect or reducer-controlled action
path, preserving duplicate-bookmark handling and the existing add, remove,
clear, and history behaviors.
In `@src/ui/components/BookmarksView.test.tsx`:
- Around line 9-31: Extend the BookmarksView test suite beyond name rendering by
using the harness to send the useInput keyboard actions, including b/c removal,
C clear-all, and download keys, and assert the corresponding store actions via
spies. Also assert the rendered size and date columns for the Alpha and Beta
bookmarks while preserving the existing list-rendering coverage.
In `@src/ui/components/Results.tsx`:
- Around line 338-345: Add a visible “b” bookmark hint to the detail-mode footer
legend near the existing d, y, e, and esc hints in Results,tsx, while leaving
the existing detail bookmark handler and payload unchanged.
In `@src/ui/components/TextField.tsx`:
- Around line 98-138: Extract the repeated history-item selection steps from the
upArrow and downArrow branches in useInput into a shared helper within
TextField. Have the helper update the history index, value, cursor position, and
invoke onChange, while preserving the existing draft restoration and navigation
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e9291ed-9fce-4b3c-9e3b-f2dd02c38448
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
package.jsonscripts/render-previews-impl.tsxsrc/config/config.tssrc/config/paths.tssrc/download/bookmarks.test.tssrc/download/bookmarks.tssrc/sources/searchHistory.test.tssrc/sources/searchHistory.tssrc/ui/App.tsxsrc/ui/components/BookmarksView.test.tsxsrc/ui/components/BookmarksView.tsxsrc/ui/components/Footer.tsxsrc/ui/components/HeaderBar.tsxsrc/ui/components/HelpOverlay.tsxsrc/ui/components/Results.tsxsrc/ui/components/SearchBar.tsxsrc/ui/components/SettingsView.test.tsxsrc/ui/components/SettingsView.tsxsrc/ui/components/Sidebar.tsxsrc/ui/components/TextField.tsxsrc/ui/helpLayout.test.tssrc/ui/keymap.tssrc/ui/spinnerPresets.tssrc/ui/store.tssrc/ui/testHarness.tssrc/ui/views/Splash.tsxsrc/update/run.tssrc/util/power.test.tssrc/util/power.ts
💤 Files with no reviewable changes (1)
- src/ui/components/HeaderBar.tsx
| function isBookmarkItem(v: unknown): v is BookmarkItem { | ||
| if (!v || typeof v !== "object") return false; | ||
| const r = v as Record<string, unknown>; | ||
| return typeof r.id === "string" && typeof r.name === "string" && typeof r.magnet === "string"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate all required BookmarkItem fields.
loadBookmarks returns values as BookmarkItem, but this guard accepts records without sizeBytes or bookmarkedAt. A malformed persistence file can then place undefined values in consumers that expect numbers.
Proposed fix
- return typeof r.id === "string" && typeof r.name === "string" && typeof r.magnet === "string";
+ return (
+ typeof r.id === "string" &&
+ typeof r.name === "string" &&
+ typeof r.magnet === "string" &&
+ typeof r.sizeBytes === "number" &&
+ Number.isFinite(r.sizeBytes) &&
+ r.sizeBytes >= 0 &&
+ typeof r.bookmarkedAt === "number" &&
+ Number.isFinite(r.bookmarkedAt)
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isBookmarkItem(v: unknown): v is BookmarkItem { | |
| if (!v || typeof v !== "object") return false; | |
| const r = v as Record<string, unknown>; | |
| return typeof r.id === "string" && typeof r.name === "string" && typeof r.magnet === "string"; | |
| function isBookmarkItem(v: unknown): v is BookmarkItem { | |
| if (!v || typeof v !== "object") return false; | |
| const r = v as Record<string, unknown>; | |
| return ( | |
| typeof r.id === "string" && | |
| typeof r.name === "string" && | |
| typeof r.magnet === "string" && | |
| typeof r.sizeBytes === "number" && | |
| Number.isFinite(r.sizeBytes) && | |
| r.sizeBytes >= 0 && | |
| typeof r.bookmarkedAt === "number" && | |
| Number.isFinite(r.bookmarkedAt) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/download/bookmarks.ts` around lines 33 - 36, Update isBookmarkItem to
validate every required BookmarkItem field, including sizeBytes and bookmarkedAt
as numbers, before loadBookmarks accepts persisted records. Preserve the
existing string checks for id, name, and magnet.
| export async function loadSearchHistory(): Promise<string[]> { | ||
| try { | ||
| const raw = await fs.readFile(searchHistoryFile, "utf8"); | ||
| const parsed = JSON.parse(raw); | ||
| if (!Array.isArray(parsed)) return []; | ||
| return parsed.filter((item): item is string => typeof item === "string" && item.trim().length > 0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Enforce the history cap during load.
loadSearchHistory returns every valid entry from disk. A file with more than 50 entries bypasses MAX_SEARCH_HISTORY until another search occurs. Apply the cap after filtering.
Proposed fix
- return parsed.filter((item): item is string => typeof item === "string" && item.trim().length > 0);
+ return parsed
+ .filter((item): item is string => typeof item === "string" && item.trim().length > 0)
+ .slice(0, MAX_SEARCH_HISTORY);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function loadSearchHistory(): Promise<string[]> { | |
| try { | |
| const raw = await fs.readFile(searchHistoryFile, "utf8"); | |
| const parsed = JSON.parse(raw); | |
| if (!Array.isArray(parsed)) return []; | |
| return parsed.filter((item): item is string => typeof item === "string" && item.trim().length > 0); | |
| export async function loadSearchHistory(): Promise<string[]> { | |
| try { | |
| const raw = await fs.readFile(searchHistoryFile, "utf8"); | |
| const parsed = JSON.parse(raw); | |
| if (!Array.isArray(parsed)) return []; | |
| return parsed | |
| .filter((item): item is string => typeof item === "string" && item.trim().length > 0) | |
| .slice(0, MAX_SEARCH_HISTORY); |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 34-34: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFile(searchHistoryFile, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sources/searchHistory.ts` around lines 33 - 38, Update loadSearchHistory
to filter valid non-empty strings first, then limit the resulting entries to
MAX_SEARCH_HISTORY before returning them. Preserve the existing empty-array
behavior for invalid or non-array JSON.
| setBookmarks((prev) => { | ||
| if (prev.some((b) => b.id === input.id)) { | ||
| setNotice(`Already bookmarked: ${truncate(cleanText(input.name), 40)}`); | ||
| return prev; | ||
| } | ||
| const next: BookmarkItem[] = [ | ||
| { | ||
| id: input.id, | ||
| name: input.name, | ||
| magnet: input.magnet, | ||
| source: input.source, | ||
| sizeBytes: input.sizeBytes ?? 0, | ||
| bookmarkedAt: Date.now(), | ||
| }, | ||
| ...prev, | ||
| ]; | ||
| void saveBookmarks(next); | ||
| setNotice(`★ Bookmarked: ${truncate(cleanText(input.name), 40)}`); | ||
| return next; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply the bookmark cap before updating UI state.
saveBookmarks writes only BOOKMARKS_CAP items, but next remains unbounded in memory. After the cap is exceeded, a bookmark remains visible in this session and disappears after restart.
Limit next with the same cap before calling setBookmarks and saveBookmarks.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 506-506: This state updater performs the nested state update "setNotice()". React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
(no-impure-state-updater)
[error] 508-508: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
[error] 522-522: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
[error] 523-523: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/App.tsx` around lines 506 - 524, Update the bookmark handling in the
setBookmarks callback to truncate the newly constructed next list to
BOOKMARKS_CAP before using it for both setBookmarks and saveBookmarks. Preserve
duplicate detection and ensure the UI state and persisted data use the same
capped list.
| } else if (input === "b" || input === "c") { | ||
| const b = bookmarks[clamped]; | ||
| if (b) removeBookmark(b.id); | ||
| } else if (input === "y") { | ||
| const b = bookmarks[clamped]; | ||
| if (b) copyMagnet({ name: b.name, magnet: b.magnet }); | ||
| } else if (input === "C") { | ||
| clearBookmarks(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Advertise or confirm the clear-all key.
C deletes every bookmark, and the deletion is persisted. The footer hints for the bookmarks section in src/ui/keymap.ts list only ↵/d, b, and y, so users cannot discover C and cannot undo it. Add C to the footer hints, or require a confirmation step before clearBookmarks runs.
Also note that b and c both map to single removal. That duplication is intentional for muscle memory, but only b appears in the hints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/components/BookmarksView.tsx` around lines 59 - 67, The bookmarks UI
does not expose the destructive clear-all action or the alternate single-removal
key. Update the bookmarks footer hints in the keymap configuration to include C
for clear-all and c alongside b for single removal, keeping the existing key
behavior unchanged.
| bookmarks: BookmarkItem[]; | ||
| addBookmark: (input: { id: string; name: string; magnet: string; source?: SourceId; sizeBytes?: number }) => void; | ||
| removeBookmark: (id: string) => void; | ||
| clearBookmarks: () => void; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the addBookmark implementation and the bookmark persistence path.
fd -t f 'App.tsx' src | xargs -r rg -n -C 12 'addBookmark'
fd -t f 'bookmarks.ts' src | xargs -r rg -n -C 6 'sizeBytes|saveBookmarks|loadBookmarks'Repository: pseudoshell/torhunt
Length of output: 3474
🏁 Script executed:
#!/bin/bash
set -e
app_file=$(fd -t f 'App.tsx' src | head -n 1)
bookmarks_file=$(fd -t f 'bookmarks.ts' src | head -n 1)
printf '%s\n' "== $app_file =="
sed -n '490,530p' "$app_file"
printf '%s\n' "== bookmark persistence =="
sed -n '1,110p' "$bookmarks_file"
printf '%s\n' "== BookmarkItem construction and persistence call sites =="
rg -n -C 8 'BookmarkItem|saveBookmarks|saveBookmarksSync|loadBookmarks|sizeBytes' srcRepository: pseudoshell/torhunt
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== bookmark loading and state initialization =='
rg -n -C 10 'loadBookmarks|setBookmarks|saveBookmarks' src/ui/App.tsx
printf '%s\n' '== bookmark-related tests =='
sed -n '1,120p' src/download/bookmarks.test.ts
printf '%s\n' '== deterministic contract probe =='
python3 - <<'PY'
import json
from pathlib import Path
app = Path("src/ui/App.tsx").read_text()
bookmarks = Path("src/download/bookmarks.ts").read_text()
assert "sizeBytes: input.sizeBytes ?? 0" in app
assert "typeof r.sizeBytes ===" not in bookmarks
assert "typeof r.bookmarkedAt ===" not in bookmarks
record = {
"id": "legacy",
"name": "Legacy bookmark",
"magnet": "magnet:?xt=urn:btih:legacy",
}
accepted_fields = ("id", "name", "magnet")
accepted = all(isinstance(record[k], str) for k in accepted_fields)
print({"addBookmark_defaults_sizeBytes": True, "missing_size_record_accepted_by_current_guard": accepted})
PYRepository: pseudoshell/torhunt
Length of output: 6274
Validate required fields when loading bookmarks.
addBookmark already stores sizeBytes: input.sizeBytes ?? 0. However, isBookmarkItem accepts persisted records without sizeBytes or bookmarkedAt, then returns them as BookmarkItem[]. Require these fields or normalize legacy records in src/download/bookmarks.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/store.ts` around lines 99 - 102, Update isBookmarkItem and the
bookmark-loading flow in bookmarks.ts to ensure persisted records include valid
sizeBytes and bookmarkedAt fields before returning them as BookmarkItem[].
Either reject records missing these required fields or normalize legacy records
with appropriate defaults, matching addBookmark’s sizeBytes behavior.
| it("acquires and releases keep-awake process lock safely", () => { | ||
| expect(isKeepAwakeActive()).toBe(false); | ||
| acquireKeepAwake(); | ||
| expect(isKeepAwakeActive()).toBe(true); | ||
| // Double acquire should be a no-op | ||
| acquireKeepAwake(); | ||
| expect(isKeepAwakeActive()).toBe(true); | ||
| releaseKeepAwake(); | ||
| expect(isKeepAwakeActive()).toBe(false); | ||
| // Double release should be a no-op | ||
| releaseKeepAwake(); | ||
| expect(isKeepAwakeActive()).toBe(false); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In the Node.js child_process.spawnAPI, when the executable cannot be found, such as ENOENT, doesspawnthrow synchronously or emit anerror event on the returned ChildProcess? Cite the official Node.js documentation.
💡 Result:
In the Node.js child_process.spawn API, when the executable cannot be found or a specified path does not exist, the spawn method does not throw synchronously. Instead, it emits an 'error' event on the returned ChildProcess instance [1][2]. According to the official Node.js documentation, the 'error' event is emitted whenever the process cannot be spawned, which includes scenarios where the command does not exist or the provided working directory (cwd) does not exist [1][3]. Furthermore, the documentation clarifies that if the child process does not spawn successfully, the 'spawn' event is not emitted and the 'error' event is emitted instead [1][2].
Citations:
- 1: https://nodejs.org/api/child_process.html
- 2: https://nodejs.org/dist/latest/docs/api/child_process.html
- 3: http://nodejs.org/docs/latest/api/child_process.html
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'src/util/power.ts' 'src/util/power.test.ts'
printf '%s\n' '--- power.ts ---'
cat -n src/util/power.ts
printf '%s\n' '--- power.test.ts ---'
cat -n src/util/power.test.ts
printf '%s\n' '--- related usages ---'
rg -n 'acquireKeepAwake|releaseKeepAwake|isKeepAwakeActive|spawn\\(' src test .github 2>/dev/null | head -200Repository: pseudoshell/torhunt
Length of output: 5599
🏁 Script executed:
node - <<'JS'
const { spawn } = require("node:child_process");
let returned = false;
let syncThrow = false;
const child = (() => {
try {
const proc = spawn("__torhunt_missing_power_tool__", [], { stdio: "ignore" });
returned = true;
return proc;
} catch {
syncThrow = true;
return null;
}
})();
console.log(JSON.stringify({ returned, syncThrow }));
if (child) {
child.once("error", (error) => {
console.log(JSON.stringify({
event: "error",
code: error.code,
messageIncludesMissingTool: error.message.includes("__torhunt_missing_power_tool__"),
}));
});
}
setTimeout(() => {}, 25);
JSRepository: pseudoshell/torhunt
Length of output: 261
Test unavailable power tools and handle child-process errors.
spawn emits error with ENOENT when a power tool is missing. The surrounding try/catch does not handle this asynchronous failure. Mock os.platform and spawn, test this event, and clear keepAwakeProc only if the failed process is still active.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/power.test.ts` around lines 9 - 20, Update the power-tool tests and
keep-awake process handling around acquireKeepAwake to mock os.platform and
spawn, emit an asynchronous ENOENT error for an unavailable power tool, and
verify the failure is handled without throwing. Ensure the spawn error handler
clears keepAwakeProc only when it still references the failed active process,
preserving an independently replaced process reference.
| keepAwakeProc = spawn("powershell", ["-NoProfile", "-NonInteractive", "-Command", script], { | ||
| windowsHide: true, | ||
| stdio: "ignore", | ||
| }); | ||
| keepAwakeProc.unref(); | ||
| } else if (platform === "darwin") { | ||
| keepAwakeProc = spawn("caffeinate", ["-d", "-i", "-m"], { | ||
| stdio: "ignore", | ||
| }); | ||
| keepAwakeProc.unref(); | ||
| } else if (platform === "linux") { | ||
| keepAwakeProc = spawn( | ||
| "systemd-inhibit", | ||
| ["--what=idle:sleep", "--why=torhunt downloading", "sleep", "infinity"], | ||
| { stdio: "ignore" }, | ||
| ); | ||
| keepAwakeProc.unref(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect every power-command spawn and its lifecycle handlers.
rg -n -C 4 'spawn\(|\.once?\("error"|\.once?\("exit"|keepAwakeProc' src/util/power.tsRepository: pseudoshell/torhunt
Length of output: 3471
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- source ---'
cat -n src/util/power.ts
printf '%s\n' '--- lifecycle call sites ---'
rg -n -C 3 'acquireKeepAwake|releaseKeepAwake|triggerSleep|triggerShutdown|isKeepAwakeActive' --glob '!node_modules' .
printf '%s\n' '--- Node child-process behavior ---'
node - <<'JS'
const { spawn } = require('node:child_process');
let caught = false;
try {
const child = spawn('__torhunt_command_that_does_not_exist__', [], {
stdio: 'ignore',
});
child.on('error', (error) => {
console.log(JSON.stringify({
event: 'error',
code: error.code,
caughtByTryCatch: caught,
}));
});
} catch (error) {
console.log(JSON.stringify({ event: 'throw', message: error.message }));
}
caught = true;
setTimeout(() => {}, 25);
JSRepository: pseudoshell/torhunt
Length of output: 9556
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- unhandled spawn error ---'
set +e
node -e 'require("node:child_process").spawn("__torhunt_command_that_does_not_exist__").unref(); setTimeout(() => {}, 100)' >/tmp/torhunt-spawn-stdout 2>/tmp/torhunt-spawn-stderr
status=$?
set -e
printf 'exit_status=%s\n' "$status"
printf 'stderr_contains_unhandled_error=%s\n' "$(
grep -q 'Error: spawn __torhunt_command_that_does_not_exist__ ENOENT' /tmp/torhunt-spawn-stderr &&
printf true || printf false
)"
printf '%s\n' '--- keep-awake lifecycle event behavior ---'
node - <<'JS'
const { spawn } = require('node:child_process');
const missing = spawn('__torhunt_command_that_does_not_exist__', [], { stdio: 'ignore' });
const missingEvents = [];
missing.on('error', (error) => missingEvents.push(`error:${error.code}`));
missing.on('exit', (code, signal) => missingEvents.push(`exit:${code}:${signal}`));
const shortLived = spawn(process.execPath, ['-e', 'process.exit(7)'], { stdio: 'ignore' });
const shortLivedEvents = [];
shortLived.on('error', (error) => shortLivedEvents.push(`error:${error.code}`));
shortLived.on('exit', (code, signal) => {
shortLivedEvents.push(`exit:${code}:${signal}`);
setImmediate(() => {
console.log(JSON.stringify({ missingEvents, shortLivedEvents }));
});
});
setTimeout(() => {}, 100);
JSRepository: pseudoshell/torhunt
Length of output: 352
Handle asynchronous child-process failures.
Attach an error handler before calling unref() for every spawned process. Clear keepAwakeProc on error and exit, but only when it still references that child.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/power.ts` around lines 25 - 42, Update each spawned process in the
keep-awake platform branches to attach an error handler before unref(), and
clear keepAwakeProc on both error and exit only if it still references that
specific child process. Apply this consistently to the PowerShell, caffeinate,
and systemd-inhibit spawn paths.
2 participants
Summary by CodeRabbit