Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
90 changes: 90 additions & 0 deletions .lore.md

Large diffs are not rendered by default.

82 changes: 1 addition & 81 deletions AGENTS.md

Large diffs are not rendered by default.

10 changes: 7 additions & 3 deletions src/commands/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Similar to 'gh api' for GitHub.
*/

import { access, readFile } from "node:fs/promises";
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
import * as Sentry from "@sentry/node-core/light";
import type { SentryContext } from "../context.js";
Expand Down Expand Up @@ -828,11 +829,14 @@ export async function buildBodyFromInput(
if (inputPath === "-") {
content = await readStdin(stdin);
} else {
const file = Bun.file(inputPath);
if (!(await file.exists())) {
const exists = await access(inputPath).then(
() => true,
() => false
);
if (!exists) {
throw new ValidationError(`File not found: ${inputPath}`, "input");
}
content = await file.text();
content = await readFile(inputPath, "utf-8");
}

// Try to parse as JSON for the API client
Expand Down
14 changes: 9 additions & 5 deletions src/commands/cli/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
* so that subsequent bare `sentry cli upgrade` calls use the same channel.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
*/

import { spawn } from "node:child_process";

Check failure on line 17 in src/commands/cli/upgrade.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

Node.js spawn errors swallowed in Promise, making EBUSY retry and ENOENT detection dead code

Unlike `Bun.spawn()`, Node.js `child_process.spawn()` never throws synchronously — ENOENT and EBUSY are delivered as `error` events on the child process. The `proc.on("error", () => resolve(1))` handler silently resolves with exit code 1, so the `catch` block (with its ENOENT→`UpgradeError` translation and EBUSY retry loop) is unreachable dead code. Change the error handler to `reject(err)` so errors propagate into the catch block.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node.js spawn errors swallowed in Promise, making EBUSY retry and ENOENT detection dead code

Unlike Bun.spawn(), Node.js child_process.spawn() never throws synchronously — ENOENT and EBUSY are delivered as error events on the child process. The proc.on("error", () => resolve(1)) handler silently resolves with exit code 1, so the catch block (with its ENOENT→UpgradeError translation and EBUSY retry loop) is unreachable dead code. Change the error handler to reject(err) so errors propagate into the catch block.

Evidence
  • spawn from node:child_process (imported at line 17) returns a ChildProcess immediately without throwing; errors arrive via proc.on('error', …), not as exceptions from the spawn() call itself.
  • spawnWithRetry (line 423) wraps spawn in a try/catch that expects synchronous throws — valid for Bun.spawn() but not for Node.js.
  • proc.on('error', () => resolve(1)) at line 436 converts every spawn error (ENOENT, EBUSY, EACCES) into a successful promise resolution with exit code 1, bypassing the catch block entirely.
  • The EBUSY retry loop (lines 453–460) — the stated purpose of the function and specifically needed for Windows Defender scanning — therefore never executes.
  • The ENOENT→UpgradeError('execution_failed') path (lines 448–453) also never executes, losing the actionable error message in favour of a generic 'Setup failed with exit code 1'.
Also found at 1 additional location
  • src/commands/cli/upgrade.ts:434-437

Identified by Warden find-bugs · HM4-JQ6

import { homedir } from "node:os";
import { dirname } from "node:path";
import { setTimeout } from "node:timers/promises";
import type { SentryContext } from "../../context.js";
import {
determineInstallDir,
Expand Down Expand Up @@ -425,12 +427,14 @@
): Promise<number> {
for (let attempt = 1; attempt <= SPAWN_MAX_ATTEMPTS; attempt++) {
try {
const proc = Bun.spawn([binaryPath, ...args], {
stdout: "inherit",
stderr: "inherit",
const proc = spawn(binaryPath, args, {
stdio: ["ignore", "inherit", "inherit"],
env,
});
return await proc.exited;
return await new Promise<number>((resolve) => {
proc.on("close", (code) => resolve(code ?? 1));
proc.on("error", () => resolve(1));
});

Check failure on line 437 in src/commands/cli/upgrade.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

[HM4-JQ6] Node.js spawn errors swallowed in Promise, making EBUSY retry and ENOENT detection dead code (additional location)

Unlike `Bun.spawn()`, Node.js `child_process.spawn()` never throws synchronously — ENOENT and EBUSY are delivered as `error` events on the child process. The `proc.on("error", () => resolve(1))` handler silently resolves with exit code 1, so the `catch` block (with its ENOENT→`UpgradeError` translation and EBUSY retry loop) is unreachable dead code. Change the error handler to `reject(err)` so errors propagate into the catch block.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spawn errors silently swallowed, breaking retry logic

High Severity

In Node.js, child_process.spawn emits ENOENT and EBUSY as asynchronous 'error' events rather than throwing synchronously (unlike Bun.spawn). The proc.on("error", () => resolve(1)) handler silently resolves the promise with exit code 1, so the catch block's ENOENT-to-UpgradeError translation and EBUSY retry-with-backoff logic are now unreachable dead code. The function will return 1 instead of retrying on Windows antivirus locks or showing a user-friendly message for missing binaries. The error handler needs to reject instead of resolve so the catch block can inspect the error.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 98702ba. Configure here.

} catch (error) {
// Translate the opaque Bun "Executable not found" error into an
// actionable UpgradeError. This path triggers when the binary at
Expand All @@ -454,7 +458,7 @@
log.warn(
`Binary is locked (antivirus scan?), retrying in ${delay}ms... (attempt ${attempt}/${SPAWN_MAX_ATTEMPTS})`
);
await Bun.sleep(delay);
await setTimeout(delay);
}
}
// Unreachable — the loop either returns or throws
Expand Down
9 changes: 5 additions & 4 deletions src/commands/dashboard/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* and optional client-side glob filtering by title.
*/

import picomatch from "picomatch";
import type { SentryContext } from "../../context.js";
import { MAX_PAGINATION_PAGES } from "../../lib/api/infrastructure.js";
import {
Expand Down Expand Up @@ -233,7 +234,7 @@ function processPage(
limit: number;
serverCursor: string | undefined;
afterId: string | undefined;
glob: InstanceType<typeof Bun.Glob> | undefined;
glob: ((input: string) => boolean) | undefined;
}
): PageResult {
// When resuming mid-page, find the afterId and skip everything up to and
Expand All @@ -249,7 +250,7 @@ function processPage(

for (let i = startIdx; i < data.length; i++) {
const item = data[i] as DashboardListItem;
if (!opts.glob || opts.glob.match(item.title.toLowerCase())) {
if (!opts.glob || opts.glob(item.title.toLowerCase())) {
results.push(item);
if (results.length >= opts.limit) {
return {
Expand Down Expand Up @@ -283,7 +284,7 @@ async function fetchDashboards(
perPage: number;
serverCursor: string | undefined;
afterId: string | undefined;
glob: InstanceType<typeof Bun.Glob> | undefined;
glob: ((input: string) => boolean) | undefined;
}
): Promise<FetchResult> {
let { serverCursor, afterId } = opts;
Expand Down Expand Up @@ -434,7 +435,7 @@ export const listCommand = buildListCommand("dashboard", {
const { serverCursor, afterId } = decodeCursor(rawCursor ?? "");

const glob = titleFilter
? new Bun.Glob(titleFilter.toLowerCase())
? picomatch(titleFilter.toLowerCase(), { dot: true })
: undefined;

// When filtering, fetch max-size pages to minimize round trips.
Expand Down
3 changes: 2 additions & 1 deletion src/lib/agent-skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import { accessSync, constants, existsSync, mkdirSync } from "node:fs";
import { writeFile } from "node:fs/promises";
import { dirname, join } from "node:path";
import { captureException } from "@sentry/node-core/light";
import { SKILL_FILES } from "../generated/skill-content.js";
Expand Down Expand Up @@ -68,7 +69,7 @@ async function writeSkillFiles(
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true, mode: 0o755 });
}
await Bun.write(fullPath, content);
await writeFile(fullPath, content, "utf-8");
if (relativePath.startsWith("references/")) {
referenceCount += 1;
}
Expand Down
13 changes: 7 additions & 6 deletions src/lib/binary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@
* Used by both `setup --install` (fresh installs) and `upgrade` (self-updates).
*/

import { spawnSync } from "node:child_process";
import {
existsSync,
readFileSync,
renameSync,
unlinkSync,
writeFileSync,
} from "node:fs";
import { chmod, mkdir, unlink } from "node:fs/promises";
import { chmod, copyFile, mkdir, unlink } from "node:fs/promises";
import { delimiter, join, resolve } from "node:path";
import { compare as semverCompare } from "semver";
import { getUserAgent } from "./constants.js";
import {
buildTlsErrorDetail,
Expand Down Expand Up @@ -56,9 +58,8 @@ export function isMusl(): boolean {

// Heuristic 2: ldd --version output (musl ldd writes "musl libc" to stderr)
try {
const result = Bun.spawnSync(["ldd", "--version"], {
stdout: "pipe",
stderr: "pipe",
const result = spawnSync("ldd", ["--version"], {
stdio: ["ignore", "pipe", "pipe"],
});
const output =
Buffer.from(result.stdout).toString() +
Expand Down Expand Up @@ -130,7 +131,7 @@ export function isNightlyVersion(version: string): boolean {
* @returns 1 if a > b, -1 if a < b, 0 if equal
*/
export function compareVersions(a: string, b: string): -1 | 0 | 1 {
return Bun.semver.order(a, b);
return semverCompare(a, b);
}

/**
Expand Down Expand Up @@ -451,7 +452,7 @@ export async function installBinary(
}

// Copy source binary to temp path next to install location
await Bun.write(tempPath, Bun.file(sourcePath));
await copyFile(sourcePath, tempPath);

// Set executable permission (Unix only)
if (process.platform !== "win32") {
Expand Down
26 changes: 18 additions & 8 deletions src/lib/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@
* Browser utilities
*
* Cross-platform utilities for interacting with the user's browser.
* Uses Bun.spawn and Bun.which for process management.
* Uses child_process.spawn and whichSync for process management.
*/

import { spawn } from "node:child_process";
import { setTimeout } from "node:timers/promises";
import { generateQRCode } from "./qrcode.js";
import { whichSync } from "./which.js";

/** No-op error handler to prevent unhandled error crashes from spawn. */
function noop(): void {
// Intentionally empty — absorbs async spawn errors (e.g., ENOENT)
}

/**
* Open a URL in the user's default browser.
Expand All @@ -20,10 +28,10 @@
let args: string[];

if (platform === "darwin") {
command = Bun.which("open");
command = whichSync("open");
args = [url];
} else if (platform === "win32") {
command = Bun.which("cmd");
command = whichSync("cmd");
args = ["/c", "start", "", url];
} else {
// Linux and other Unix-like systems - try multiple openers
Expand All @@ -35,7 +43,7 @@
"kde-open",
];
for (const opener of linuxOpeners) {
command = Bun.which(opener);
command = whichSync(opener);
if (command) {
break;
}
Expand All @@ -48,16 +56,18 @@
}

try {
const proc = Bun.spawn([command, ...args], {
stdout: "ignore",
stderr: "ignore",
const proc = spawn(command, args, {
stdio: ["ignore", "ignore", "ignore"],
});
// Prevent unhandled error crash if the binary disappears between
// whichSync() and spawn() (TOCTOU window).
proc.on("error", noop);

// Give browser time to open, then detach
await Bun.sleep(500);
await setTimeout(500);
Comment thread
cursor[bot] marked this conversation as resolved.
proc.unref();
return true;
} catch {

Check warning on line 70 in src/lib/browser.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

Spawn errors silently absorbed by `noop` while function still returns `true`

When `spawn` emits an `error` event (e.g., ENOENT due to the TOCTOU window), `noop` silently absorbs it but the function still returns `true`, causing callers like `openOrShowUrl` to print "Opening in browser..." and skip the URL/QR-code fallback even though no browser was opened.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Comment on lines +64 to 70
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spawn errors silently absorbed by noop while function still returns true

When spawn emits an error event (e.g., ENOENT due to the TOCTOU window), noop silently absorbs it but the function still returns true, causing callers like openOrShowUrl to print "Opening in browser..." and skip the URL/QR-code fallback even though no browser was opened.

Evidence
  • proc.on("error", noop) registers a no-op that discards any Error passed to it — the error is never surfaced to the surrounding scope.
  • await setTimeout(500) does not await process exit or observe whether an error was emitted; it simply waits 500 ms unconditionally.
  • return true on line 70 is always reached regardless of whether an error event fired during the 500 ms window.
  • The caller openOrShowUrl branches on the return value: true → prints "Opening in browser..." and returns without showing the QR fallback; the fallback is only shown when openBrowser returns false.
  • Node.js spawn always emits ENOENT/EACCES as an async error event — it never throws from the spawn() call itself — so the surrounding try/catch cannot catch these cases either.

Identified by Warden find-bugs · HK4-FVB

return false;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/bspatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/

import { constants, copyFileSync } from "node:fs";
import { unlink } from "node:fs/promises";
import { readFile, unlink } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";

Expand Down Expand Up @@ -269,7 +269,7 @@ async function loadOldBinary(oldPath: string): Promise<OldFileHandle> {
/* May not exist if copyFileSync failed */
});
return {
data: new Uint8Array(await Bun.file(oldPath).arrayBuffer()),
data: new Uint8Array(await readFile(oldPath)),
cleanup: () => {
// Data is in JS heap — no temp file to clean up
},
Expand Down
28 changes: 17 additions & 11 deletions src/lib/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* Includes both low-level copy function and interactive keyboard-triggered copy.
*/

import { spawn } from "node:child_process";
import { logger } from "./logger.js";
import { whichSync } from "./which.js";

const log = logger.withTag("clipboard");

Expand All @@ -29,18 +31,18 @@ export async function copyToClipboard(text: string): Promise<boolean> {
let args: string[] = [];

if (platform === "darwin") {
command = Bun.which("pbcopy");
command = whichSync("pbcopy");
args = [];
} else if (platform === "win32") {
command = Bun.which("clip");
command = whichSync("clip");
args = [];
} else {
// Linux - try xclip first, then xsel
command = Bun.which("xclip");
command = whichSync("xclip");
if (command) {
args = ["-selection", "clipboard"];
} else {
command = Bun.which("xsel");
command = whichSync("xsel");
if (command) {
args = ["--clipboard", "--input"];
}
Expand All @@ -52,16 +54,20 @@ export async function copyToClipboard(text: string): Promise<boolean> {
}

try {
const proc = Bun.spawn([command, ...args], {
stdin: "pipe",
stdout: "ignore",
stderr: "ignore",
const proc = spawn(command, args, {
stdio: ["pipe", "ignore", "ignore"],
});

proc.stdin.write(text);
proc.stdin.end();
const { stdin } = proc;
if (stdin) {
stdin.write(text);
stdin.end();
}

const exitCode = await proc.exited;
const exitCode = await new Promise<number>((resolve) => {
proc.on("close", (code) => resolve(code ?? 1));
proc.on("error", () => resolve(1));
});
return exitCode === 0;
} catch {
return false;
Expand Down
3 changes: 2 additions & 1 deletion src/lib/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/

import { existsSync, mkdirSync } from "node:fs";
import { writeFile } from "node:fs/promises";
import { dirname, join } from "node:path";
import { routes } from "../app.js";
import { type FlagDef, isCommand, isRouteMap } from "./introspect.js";
Expand Down Expand Up @@ -649,7 +650,7 @@ export async function installCompletions(
}

const alreadyExists = existsSync(path);
await Bun.write(path, script);
await writeFile(path, script, "utf-8");

return {
path,
Expand Down
6 changes: 3 additions & 3 deletions src/lib/db/dsn-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,11 @@ async function validateFileMtime(
cachedMtime: number
): Promise<boolean> {
try {
const file = Bun.file(fullPath);
if (!(await file.exists())) {
const stats = await stat(fullPath);
if (!stats.isFile()) {
return false;
}
return file.lastModified === cachedMtime;
return Math.floor(stats.mtimeMs) === cachedMtime;
} catch {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/db/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Uses UUIDv7 for time-sortable, unique identifiers.
*/

import { uuidv7 } from "uuidv7";
import { getDatabase } from "./index.js";

/**
Expand All @@ -28,8 +29,7 @@ export function getInstanceId(): string {
// Generate and store new instance ID
// Use INSERT OR IGNORE to handle race condition when multiple CLI processes
// start simultaneously - only the first insert succeeds
// Bun.randomUUIDv7() is native in Bun, polyfilled via uuidv7 package for Node.js
const instanceId = Bun.randomUUIDv7();
const instanceId = uuidv7();
const now = Date.now();

db.query(`
Expand Down
7 changes: 4 additions & 3 deletions src/lib/delta-upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
import * as Sentry from "@sentry/node-core/light";
import { compare as semverCompare } from "semver";

import {
GITHUB_RELEASES_URL,
Expand Down Expand Up @@ -465,15 +466,15 @@
const version = tag.slice(PATCH_TAG_PREFIX.length);
// Include tags where: currentVersion < version <= targetVersion
if (
Bun.semver.order(version, currentVersion) === 1 &&
Bun.semver.order(version, targetVersion) !== 1
semverCompare(version, currentVersion) === 1 &&
semverCompare(version, targetVersion) !== 1
) {
chainTags.push({ tag, version });
}
}

// Sort by version (chronological for nightlies)
chainTags.sort((a, b) => Bun.semver.order(a.version, b.version));
chainTags.sort((a, b) => semverCompare(a.version, b.version));

Check warning on line 477 in src/lib/delta-upgrade.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

`semver.compare` throws on invalid version strings where `Bun.semver.order` returned `null`

Replace `semverCompare(version, ...)` calls with a null-safe wrapper (e.g. `semver.valid(version) ? semverCompare(...) : null`) or catch the TypeError, so malformed GHCR tag versions are silently skipped rather than crashing `filterAndSortChainTags`.
Comment on lines +469 to +477
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semver.compare throws on invalid version strings where Bun.semver.order returned null

Replace semverCompare(version, ...) calls with a null-safe wrapper (e.g. semver.valid(version) ? semverCompare(...) : null) or catch the TypeError, so malformed GHCR tag versions are silently skipped rather than crashing filterAndSortChainTags.

Evidence
  • Bun.semver.order(v1, v2) returns null for invalid semver inputs; null === 1 is false, so invalid tags were silently skipped in the loop.
  • The npm semver package's compare(v1, v2) (v7.x) constructs new SemVer(v) internally and throws TypeError: Invalid SemVer version for any non-conforming string.
  • version at line 466 is derived from tag.slice(PATCH_TAG_PREFIX.length) with no prior semver validity guard — any unexpected GHCR tag (e.g. patch-latest, patch-) would produce a non-semver string.
  • currentVersion and targetVersion are caller-supplied strings (from the running binary's version header) and are not validated before being passed in.
  • The .sort() comparator at line 477 also calls semverCompare directly; a throw there would propagate uncaught from filterAndSortChainTags.

Identified by Warden find-bugs · ARU-GUD


return chainTags.map((t) => t.tag);
}
Expand Down
Loading
Loading