From 71742f8eb19da8324f560055f063453a30d429bd Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 11 Feb 2026 19:56:11 +0100 Subject: [PATCH 01/11] fix(db): handle readonly database gracefully with user warning When the local database file is read-only (wrong permissions, read-only filesystem, etc.), DB writes now fail silently instead of crashing the CLI. A one-time warning is printed to stderr pointing users to `sentry cli fix`. The fix is centralized in the traced DB proxy (createTracedStatement), so all ~14 DB write paths are protected without touching individual modules. Reads continue working normally. Resolves CLI-4E: SQLiteError: attempt to write a readonly database --- src/lib/db/schema.ts | 16 ++++++ src/lib/telemetry.ts | 48 ++++++++++++++++- test/lib/db/schema.test.ts | 32 ++++++++++++ test/lib/telemetry.test.ts | 103 +++++++++++++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 1 deletion(-) diff --git a/src/lib/db/schema.ts b/src/lib/db/schema.ts index 8f72a0ab..f5c11d6a 100644 --- a/src/lib/db/schema.ts +++ b/src/lib/db/schema.ts @@ -438,6 +438,22 @@ function isSchemaError(error: unknown): boolean { return false; } +/** + * Check if an error is a SQLite "readonly database" error. + * + * This happens when the CLI's local database file or its containing directory + * lacks write permissions (e.g., installed globally in a protected path, + * read-only filesystem, or changed permissions). + */ +export function isReadonlyError(error: unknown): boolean { + if (error instanceof Error && error.name === "SQLiteError") { + return error.message + .toLowerCase() + .includes("attempt to write a readonly database"); + } + return false; +} + /** Result of a repair attempt */ export type RepairAttemptResult = | { attempted: false } diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index e74bd6d9..ed68832b 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -12,7 +12,7 @@ // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import import * as Sentry from "@sentry/bun"; import { CLI_VERSION, SENTRY_CLI_DSN } from "./constants.js"; -import { tryRepairAndRetry } from "./db/schema.js"; +import { isReadonlyError, tryRepairAndRetry } from "./db/schema.js"; import { ApiError, AuthError } from "./errors.js"; import { getSentryBaseUrl, isSentrySaasUrl } from "./sentry-urls.js"; @@ -586,6 +586,44 @@ export function withDbSpan(operation: string, fn: () => T): T { ); } +/** Whether the readonly database warning has been shown this process */ +let readonlyWarningShown = false; + +/** + * Print a one-time warning when the local database is read-only. + * Uses lazy require to avoid circular dependency with db/index.ts. + */ +function warnReadonlyDatabaseOnce(): void { + if (readonlyWarningShown) { + return; + } + readonlyWarningShown = true; + + let dbPath = "~/.sentry/cli.db"; + try { + const { getDbPath } = require("./db/index.js") as { + getDbPath: () => string; + }; + dbPath = getDbPath(); + } catch { + // Fall back to default path if import fails + } + + process.stderr.write( + `\nWarning: Sentry CLI local database is read-only. Caching and preferences won't persist.\n` + + ` Path: ${dbPath}\n` + + " Fix: sentry cli fix\n\n" + ); +} + +/** + * Reset the readonly warning flag (for testing). + * @internal + */ +export function resetReadonlyWarning(): void { + readonlyWarningShown = false; +} + /** Methods on SQLite Statement that execute queries and should be traced */ const TRACED_STATEMENT_METHODS = ["get", "run", "all", "values"] as const; @@ -644,6 +682,14 @@ function createTracedStatement(stmt: T, sql: string): T { if (repairResult.attempted) { return repairResult.result; } + + // Handle readonly database gracefully: warn once, skip the write. + // The CLI still works — reads succeed, only caching/persistence is lost. + if (isReadonlyError(error)) { + warnReadonlyDatabaseOnce(); + return; + } + // Re-throw if repair didn't help or wasn't applicable throw error; } diff --git a/test/lib/db/schema.test.ts b/test/lib/db/schema.test.ts index 6b322edd..5d86c3eb 100644 --- a/test/lib/db/schema.test.ts +++ b/test/lib/db/schema.test.ts @@ -13,6 +13,7 @@ import { getSchemaIssues, hasColumn, initSchema, + isReadonlyError, repairSchema, tableExists, } from "../../../src/lib/db/schema.js"; @@ -259,3 +260,34 @@ describe("EXPECTED_COLUMNS", () => { expect(columnNames).toContain("name"); }); }); + +describe("isReadonlyError", () => { + test("returns true for SQLiteError with readonly message", () => { + const error = new Error("attempt to write a readonly database"); + error.name = "SQLiteError"; + expect(isReadonlyError(error)).toBe(true); + }); + + test("returns true for mixed-case readonly message", () => { + const error = new Error("Attempt to Write a Readonly Database"); + error.name = "SQLiteError"; + expect(isReadonlyError(error)).toBe(true); + }); + + test("returns false for schema errors", () => { + const error = new Error("no such table: foo"); + error.name = "SQLiteError"; + expect(isReadonlyError(error)).toBe(false); + }); + + test("returns false for non-SQLiteError", () => { + const error = new Error("attempt to write a readonly database"); + expect(isReadonlyError(error)).toBe(false); + }); + + test("returns false for non-Error values", () => { + expect(isReadonlyError("attempt to write a readonly database")).toBe(false); + expect(isReadonlyError(null)).toBe(false); + expect(isReadonlyError(undefined)).toBe(false); + }); +}); diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index 00279d05..bc6123df 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -6,6 +6,7 @@ import { Database } from "bun:sqlite"; import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { chmodSync, mkdirSync, rmSync } from "node:fs"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as Sentry from "@sentry/bun"; import { ApiError, AuthError } from "../../src/lib/errors.js"; @@ -14,6 +15,7 @@ import { initSentry, isClientApiError, recordApiErrorOnSpan, + resetReadonlyWarning, setArgsContext, setCommandSpanName, setFlagContext, @@ -727,4 +729,105 @@ describe("createTracedDatabase", () => { db.close(); }); + + describe("readonly database handling", () => { + let tmpDir: string; + let dbPath: string; + + beforeEach(() => { + resetReadonlyWarning(); + // Create a temp directory for the test database + tmpDir = `${import.meta.dir}/tmp-readonly-${Date.now()}`; + mkdirSync(tmpDir, { recursive: true }); + dbPath = `${tmpDir}/test.db`; + + // Create a database with a table and some data + const setupDb = new Database(dbPath); + setupDb.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, name TEXT)"); + setupDb.exec("INSERT INTO test (id, name) VALUES (1, 'Alice')"); + setupDb.close(); + + // Make the database file read-only + chmodSync(dbPath, 0o444); + }); + + afterEach(() => { + // Restore permissions so we can clean up + try { + chmodSync(dbPath, 0o644); + } catch { + // May already be removed + } + rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("does not throw on write to readonly database", () => { + // Open in readonly mode (bun:sqlite won't fail on open, fails on write) + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + // Write should be silently swallowed + expect(() => { + tracedDb + .query("INSERT INTO test (id, name) VALUES (?, ?)") + .run(2, "Bob"); + }).not.toThrow(); + + db.close(); + }); + + test("reads still work on readonly database", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const row = tracedDb.query("SELECT * FROM test WHERE id = ?").get(1) as { + id: number; + name: string; + }; + expect(row).toEqual({ id: 1, name: "Alice" }); + + db.close(); + }); + + test("warns to stderr only once across multiple writes", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const stderrSpy = spyOn(process.stderr, "write"); + + // Multiple writes should only produce one warning + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); + tracedDb + .query("INSERT INTO test (id, name) VALUES (?, ?)") + .run(3, "Charlie"); + tracedDb + .query("INSERT INTO test (id, name) VALUES (?, ?)") + .run(4, "Dave"); + + const warningCalls = stderrSpy.mock.calls.filter((call) => + String(call[0]).includes("read-only") + ); + expect(warningCalls.length).toBe(1); + + stderrSpy.mockRestore(); + db.close(); + }); + + test("warning message includes helpful instructions", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const stderrSpy = spyOn(process.stderr, "write"); + + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); + + const warning = String(stderrSpy.mock.calls[0]?.[0] ?? ""); + expect(warning).toContain("local database"); + expect(warning).toContain("read-only"); + expect(warning).toContain("sentry cli fix"); + + stderrSpy.mockRestore(); + db.close(); + }); + }); }); From dbf23ec483265cf60d411f8d33afe715099fa04f Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 11 Feb 2026 19:56:23 +0100 Subject: [PATCH 02/11] feat(cli): add permission repair to sentry cli fix Extends `sentry cli fix` to detect and repair file permission issues on the local database. Checks the config directory (expects 0700), DB file, and WAL/SHM journals (expect 0600). Attempts chmod repair automatically; shows manual fix commands if that fails. This is the recommended fix path shown in the readonly DB warning. --- src/commands/cli/fix.ts | 307 ++++++++++++++++++++++++++++------ test/commands/cli/fix.test.ts | 2 +- 2 files changed, 255 insertions(+), 54 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index c0c2ca7a..b6d42038 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -1,12 +1,14 @@ /** * sentry cli fix * - * Diagnose and repair CLI database issues. + * Diagnose and repair CLI database issues (schema and permissions). */ +import { chmodSync, statSync } from "node:fs"; +import { dirname } from "node:path"; import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; -import { getDbPath, getRawDatabase } from "../../lib/db/index.js"; +import { getConfigDir, getDbPath, getRawDatabase } from "../../lib/db/index.js"; import { CURRENT_SCHEMA_VERSION, getSchemaIssues, @@ -25,15 +27,260 @@ function formatIssue(issue: SchemaIssue): string { return `Missing column: ${issue.table}.${issue.column}`; } +/** Expected permissions for the config directory (owner rwx) */ +const EXPECTED_DIR_MODE = 0o700; +/** Expected permissions for the database file (owner rw) */ +const EXPECTED_FILE_MODE = 0o600; + +type PermissionIssue = { + path: string; + /** What kind of file this is (for display) */ + kind: "directory" | "database" | "journal"; + currentMode: number; + expectedMode: number; +}; + +/** + * Check if a path has the required permission bits set. + * Returns the actual mode if it lacks the expected bits, or null if OK. + */ +function checkMode( + path: string, + expectedMode: number +): { actualMode: number } | null { + try { + const st = statSync(path); + // biome-ignore lint/suspicious/noBitwiseOperators: extracting permission bits with bitmask + const mode = st.mode & 0o777; + // biome-ignore lint/suspicious/noBitwiseOperators: checking required permission bits are set + if ((mode & expectedMode) !== expectedMode) { + return { actualMode: mode }; + } + } catch { + // File/dir doesn't exist — not a permission issue + } + return null; +} + +/** + * Check if the database file and its directory have correct permissions. + * Returns a list of permission issues found. + */ +function checkPermissions(dbPath: string): PermissionIssue[] { + const issues: PermissionIssue[] = []; + const configDir = dirname(dbPath); + + // Check config directory permissions + const dirCheck = checkMode(configDir, EXPECTED_DIR_MODE); + if (dirCheck) { + issues.push({ + path: configDir, + kind: "directory", + currentMode: dirCheck.actualMode, + expectedMode: EXPECTED_DIR_MODE, + }); + } + + // Check database file and associated WAL/SHM files + const filesToCheck: Array<{ path: string; kind: "database" | "journal" }> = [ + { path: dbPath, kind: "database" }, + { path: `${dbPath}-wal`, kind: "journal" }, + { path: `${dbPath}-shm`, kind: "journal" }, + ]; + + for (const { path, kind } of filesToCheck) { + const fileCheck = checkMode(path, EXPECTED_FILE_MODE); + if (fileCheck) { + issues.push({ + path, + kind, + currentMode: fileCheck.actualMode, + expectedMode: EXPECTED_FILE_MODE, + }); + } + } + + return issues; +} + +/** + * Format a permission mode as an octal string (e.g., "0644"). + */ +function formatMode(mode: number): string { + return `0${mode.toString(8)}`; +} + +/** + * Attempt to fix file/directory permissions. + * Returns lists of what was fixed and what failed. + */ +function repairPermissions(issues: PermissionIssue[]): { + fixed: string[]; + failed: string[]; +} { + const fixed: string[] = []; + const failed: string[] = []; + + for (const issue of issues) { + try { + chmodSync(issue.path, issue.expectedMode); + fixed.push( + `${issue.kind} ${issue.path}: ${formatMode(issue.currentMode)} -> ${formatMode(issue.expectedMode)}` + ); + } catch (error) { + const reason = + error instanceof Error ? error.message : "permission denied"; + failed.push(`${issue.kind} ${issue.path}: ${reason}`); + } + } + + return { fixed, failed }; +} + +type Output = { + stdout: { write(s: string): void }; + stderr: { write(s: string): void }; +}; + +/** Result of diagnosing a category of issues */ +type DiagnoseResult = { + /** Number of issues found */ + found: number; + /** Whether any repairs failed (only meaningful when not dry-run) */ + repairFailed: boolean; +}; + +/** + * Report permission issues and optionally repair them. + */ +function handlePermissionIssues( + dbPath: string, + dryRun: boolean, + { stdout, stderr }: Output +): DiagnoseResult { + const permIssues = checkPermissions(dbPath); + if (permIssues.length === 0) { + return { found: 0, repairFailed: false }; + } + + stdout.write(`Found ${permIssues.length} permission issue(s):\n`); + for (const issue of permIssues) { + stdout.write( + ` - ${issue.kind} ${issue.path}: ${formatMode(issue.currentMode)} (expected ${formatMode(issue.expectedMode)})\n` + ); + } + stdout.write("\n"); + + if (dryRun) { + return { found: permIssues.length, repairFailed: false }; + } + + stdout.write("Repairing permissions...\n"); + const { fixed, failed } = repairPermissions(permIssues); + for (const fix of fixed) { + stdout.write(` + ${fix}\n`); + } + if (failed.length > 0) { + stderr.write("\nSome permission repairs failed:\n"); + for (const fail of failed) { + stderr.write(` ! ${fail}\n`); + } + stderr.write( + "\nYou may need to fix permissions manually:\n" + + ` chmod 700 "${getConfigDir()}"\n` + + ` chmod 600 "${dbPath}"\n` + ); + } + stdout.write("\n"); + + return { found: permIssues.length, repairFailed: failed.length > 0 }; +} + +/** + * Report schema issues and optionally repair them. + */ +function handleSchemaIssues( + dbPath: string, + dryRun: boolean, + { stdout, stderr }: Output +): DiagnoseResult { + const db = getRawDatabase(); + const issues = getSchemaIssues(db); + if (issues.length === 0) { + return { found: 0, repairFailed: false }; + } + + stdout.write(`Found ${issues.length} schema issue(s):\n`); + for (const issue of issues) { + stdout.write(` - ${formatIssue(issue)}\n`); + } + stdout.write("\n"); + + if (dryRun) { + return { found: issues.length, repairFailed: false }; + } + + stdout.write("Repairing schema...\n"); + const { fixed, failed } = repairSchema(db); + for (const fix of fixed) { + stdout.write(` + ${fix}\n`); + } + if (failed.length > 0) { + stderr.write("\nSome schema repairs failed:\n"); + for (const fail of failed) { + stderr.write(` ! ${fail}\n`); + } + stderr.write( + `\nTry deleting the database and restarting: rm "${dbPath}"\n` + ); + } + stdout.write("\n"); + + return { found: issues.length, repairFailed: failed.length > 0 }; +} + +function fixFunc(this: SentryContext, flags: FixFlags): void { + const { stdout, process: proc } = this; + const dbPath = getDbPath(); + const dryRun = flags["dry-run"]; + const out = { stdout, stderr: this.stderr }; + + stdout.write(`Database: ${dbPath}\n`); + stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`); + + const perm = handlePermissionIssues(dbPath, dryRun, out); + const schema = handleSchemaIssues(dbPath, dryRun, out); + const totalFound = perm.found + schema.found; + + if (totalFound === 0) { + stdout.write( + "No issues found. Database schema and permissions are correct.\n" + ); + return; + } + + if (dryRun) { + stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + return; + } + + if (perm.repairFailed || schema.repairFailed) { + proc.exitCode = 1; + } else { + stdout.write("All issues repaired successfully.\n"); + } +} + export const fixCommand = buildCommand({ docs: { brief: "Diagnose and repair CLI database issues", fullDescription: - "Check the CLI's local SQLite database for schema issues and repair them.\n\n" + - "This is useful when upgrading from older CLI versions or if the database\n" + - "becomes inconsistent due to interrupted operations.\n\n" + + "Check the CLI's local SQLite database for schema and permission issues and repair them.\n\n" + + "This is useful when upgrading from older CLI versions, if the database\n" + + "becomes inconsistent due to interrupted operations, or if file permissions\n" + + "prevent the CLI from writing to its local database.\n\n" + "The command performs non-destructive repairs only - it adds missing tables\n" + - "and columns but never deletes data.\n\n" + + "and columns, and fixes file permissions, but never deletes data.\n\n" + "Examples:\n" + " sentry cli fix # Fix database issues\n" + " sentry cli fix --dry-run # Show what would be fixed without making changes", @@ -47,51 +294,5 @@ export const fixCommand = buildCommand({ }, }, }, - func(this: SentryContext, flags: FixFlags): void { - const { stdout, stderr, process: proc } = this; - const dbPath = getDbPath(); - - stdout.write(`Database: ${dbPath}\n`); - stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`); - - const db = getRawDatabase(); - const issues = getSchemaIssues(db); - - if (issues.length === 0) { - stdout.write("No issues found. Database schema is up to date.\n"); - return; - } - - stdout.write(`Found ${issues.length} issue(s):\n`); - for (const issue of issues) { - stdout.write(` - ${formatIssue(issue)}\n`); - } - stdout.write("\n"); - - if (flags["dry-run"]) { - stdout.write("Run 'sentry cli fix' to apply fixes.\n"); - return; - } - - stdout.write("Repairing...\n"); - const { fixed, failed } = repairSchema(db); - - for (const fix of fixed) { - stdout.write(` + ${fix}\n`); - } - - if (failed.length > 0) { - stderr.write("\nSome repairs failed:\n"); - for (const fail of failed) { - stderr.write(` ! ${fail}\n`); - } - stderr.write( - `\nTry deleting the database and restarting: rm ${dbPath}\n` - ); - proc.exitCode = 1; - return; - } - - stdout.write("\nDatabase repaired successfully.\n"); - }, + func: fixFunc, }); diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 029949fd..149c1f8c 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -82,7 +82,7 @@ describe("sentry cli fix", () => { const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("No issues found"); - expect(output).toContain("up to date"); + expect(output).toContain("permissions are correct"); }); test("detects and reports missing columns in dry-run mode", async () => { From 4b14c66a584e31a468f5e3099842e7f485420c62 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 11 Feb 2026 20:25:08 +0100 Subject: [PATCH 03/11] refactor(cli): cleanup JSDoc, remove redundant comments, use getConfigDir - Use getConfigDir() instead of dirname(dbPath) for consistency with the canonical config path source - Remove node:path dirname import (no longer needed) - Add @param/@returns JSDoc to fix.ts helpers - Tighten warnReadonlyDatabaseOnce JSDoc (document no-op behavior) - Use explicit return undefined in proxy readonly handler - Trim obvious 'what' comments from test setup/teardown --- src/commands/cli/fix.ts | 46 +++++++++++++++++++++++++++++++------- src/lib/telemetry.ts | 7 ++++-- test/lib/telemetry.test.ts | 11 +++------ 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index b6d42038..afeca031 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -5,7 +5,6 @@ */ import { chmodSync, statSync } from "node:fs"; -import { dirname } from "node:path"; import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; import { getConfigDir, getDbPath, getRawDatabase } from "../../lib/db/index.js"; @@ -20,6 +19,7 @@ type FixFlags = { readonly "dry-run": boolean; }; +/** Format a schema issue as a human-readable string for display. */ function formatIssue(issue: SchemaIssue): string { if (issue.type === "missing_table") { return `Missing table: ${issue.table}`; @@ -42,7 +42,11 @@ type PermissionIssue = { /** * Check if a path has the required permission bits set. - * Returns the actual mode if it lacks the expected bits, or null if OK. + * + * @param path - Filesystem path to check + * @param expectedMode - Bitmask of required permission bits (e.g., 0o700) + * @returns Object with the actual mode if permissions are insufficient, or null if OK. + * Returns null if the path doesn't exist (missing files aren't a permission problem). */ function checkMode( path: string, @@ -64,11 +68,16 @@ function checkMode( /** * Check if the database file and its directory have correct permissions. - * Returns a list of permission issues found. + * Inspects the config directory (needs rwx), the DB file, and SQLite's + * WAL/SHM journal files (need rw). Missing files are silently skipped + * since WAL/SHM are created on demand. + * + * @param dbPath - Absolute path to the database file + * @returns List of permission issues found (empty if everything is OK) */ function checkPermissions(dbPath: string): PermissionIssue[] { const issues: PermissionIssue[] = []; - const configDir = dirname(dbPath); + const configDir = getConfigDir(); // Check config directory permissions const dirCheck = checkMode(configDir, EXPECTED_DIR_MODE); @@ -105,14 +114,19 @@ function checkPermissions(dbPath: string): PermissionIssue[] { /** * Format a permission mode as an octal string (e.g., "0644"). + * + * @param mode - Unix permission bits (0-0o777) */ function formatMode(mode: number): string { return `0${mode.toString(8)}`; } /** - * Attempt to fix file/directory permissions. - * Returns lists of what was fixed and what failed. + * Attempt to fix file/directory permissions via chmod. + * Repairs may fail if the current user doesn't own the file. + * + * @param issues - Permission issues to repair + * @returns Separate lists of human-readable repair successes and failures */ function repairPermissions(issues: PermissionIssue[]): { fixed: string[]; @@ -151,7 +165,13 @@ type DiagnoseResult = { }; /** - * Report permission issues and optionally repair them. + * Diagnose permission issues and optionally repair them. + * Writes findings and repair results to stdout/stderr as a side effect. + * + * @param dbPath - Absolute path to the database file + * @param dryRun - If true, report issues without repairing + * @param output - Streams for user-facing output + * @returns Count of issues found and whether any repairs failed */ function handlePermissionIssues( dbPath: string, @@ -197,7 +217,13 @@ function handlePermissionIssues( } /** - * Report schema issues and optionally repair them. + * Diagnose schema issues (missing tables/columns) and optionally repair them. + * Writes findings and repair results to stdout/stderr as a side effect. + * + * @param dbPath - Absolute path to the database file (used in error messages) + * @param dryRun - If true, report issues without repairing + * @param output - Streams for user-facing output + * @returns Count of issues found and whether any repairs failed */ function handleSchemaIssues( dbPath: string, @@ -239,6 +265,10 @@ function handleSchemaIssues( return { found: issues.length, repairFailed: failed.length > 0 }; } +/** + * Entry point for `sentry cli fix`. Runs permission and schema checks + * in sequence, repairs what it can, and reports results. + */ function fixFunc(this: SentryContext, flags: FixFlags): void { const { stdout, process: proc } = this; const dbPath = getDbPath(); diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index ed68832b..26ccba59 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -590,8 +590,11 @@ export function withDbSpan(operation: string, fn: () => T): T { let readonlyWarningShown = false; /** - * Print a one-time warning when the local database is read-only. - * Uses lazy require to avoid circular dependency with db/index.ts. + * Print a one-time warning to stderr when the local database is read-only. + * Does nothing if the warning was already shown this process. + * + * Uses lazy require for db/index.js to avoid a circular dependency + * (db/index.ts imports createTracedDatabase from this module). */ function warnReadonlyDatabaseOnce(): void { if (readonlyWarningShown) { diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index bc6123df..973cd113 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -736,37 +736,33 @@ describe("createTracedDatabase", () => { beforeEach(() => { resetReadonlyWarning(); - // Create a temp directory for the test database + tmpDir = `${import.meta.dir}/tmp-readonly-${Date.now()}`; mkdirSync(tmpDir, { recursive: true }); dbPath = `${tmpDir}/test.db`; - // Create a database with a table and some data const setupDb = new Database(dbPath); setupDb.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, name TEXT)"); setupDb.exec("INSERT INTO test (id, name) VALUES (1, 'Alice')"); setupDb.close(); - // Make the database file read-only chmodSync(dbPath, 0o444); }); afterEach(() => { - // Restore permissions so we can clean up try { chmodSync(dbPath, 0o644); } catch { - // May already be removed + // May already be cleaned up } rmSync(tmpDir, { recursive: true, force: true }); }); test("does not throw on write to readonly database", () => { - // Open in readonly mode (bun:sqlite won't fail on open, fails on write) + // bun:sqlite opens the file successfully — the error only surfaces on write const db = new Database(dbPath); const tracedDb = createTracedDatabase(db); - // Write should be silently swallowed expect(() => { tracedDb .query("INSERT INTO test (id, name) VALUES (?, ?)") @@ -795,7 +791,6 @@ describe("createTracedDatabase", () => { const stderrSpy = spyOn(process.stderr, "write"); - // Multiple writes should only produce one warning tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); tracedDb .query("INSERT INTO test (id, name) VALUES (?, ?)") From b80b6d9dd00f388370384580c692b32793a19c64 Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 11 Feb 2026 20:44:06 +0100 Subject: [PATCH 04/11] test(cli): add permission detection and repair tests for sentry cli fix Add 5 tests covering file/directory permission checking and repair: - Detect readonly database file permissions - Repair database file permissions (chmod 0444 -> 0600) - Detect directory permission issues - Dry-run reports issues without repairing - Handle combined permission and schema issues Also extract runFix() helper to reduce test boilerplate. Coverage for src/commands/cli/fix.ts: 100% functions, 88% lines. --- test/commands/cli/fix.test.ts | 144 ++++++++++++++++++++++++++++------ 1 file changed, 118 insertions(+), 26 deletions(-) diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 149c1f8c..7137a895 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -4,9 +4,10 @@ import { Database } from "bun:sqlite"; import { describe, expect, mock, test } from "bun:test"; +import { chmodSync, statSync } from "node:fs"; import { join } from "node:path"; import { fixCommand } from "../../../src/commands/cli/fix.js"; -import { closeDatabase } from "../../../src/lib/db/index.js"; +import { closeDatabase, getDatabase } from "../../../src/lib/db/index.js"; import { EXPECTED_TABLES, generatePreMigrationTableDDL, @@ -63,26 +64,38 @@ function createDatabaseWithMissingTables( const getTestDir = useTestConfigDir("fix-test-"); +/** + * Run the fix command with the given flags and return captured output. + * Reduces boilerplate across test cases. + */ +async function runFix(dryRun: boolean) { + const stdoutWrite = mock(() => true); + const stderrWrite = mock(() => true); + const mockContext = { + stdout: { write: stdoutWrite }, + stderr: { write: stderrWrite }, + process: { exitCode: 0 }, + }; + + const func = await fixCommand.loader(); + func.call(mockContext, { "dry-run": dryRun }); + + return { + stdout: stdoutWrite.mock.calls.map((c) => c[0]).join(""), + stderr: stderrWrite.mock.calls.map((c) => c[0]).join(""), + exitCode: mockContext.process.exitCode, + }; +} + describe("sentry cli fix", () => { test("reports no issues for healthy database", async () => { - // Create healthy database const db = new Database(join(getTestDir(), "cli.db")); initSchema(db); db.close(); - const stdoutWrite = mock(() => true); - const mockContext = { - stdout: { write: stdoutWrite }, - stderr: { write: mock(() => true) }, - process: { exitCode: 0 }, - }; - - const func = await fixCommand.loader(); - func.call(mockContext, { "dry-run": false }); - - const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); - expect(output).toContain("No issues found"); - expect(output).toContain("permissions are correct"); + const { stdout } = await runFix(false); + expect(stdout).toContain("No issues found"); + expect(stdout).toContain("permissions are correct"); }); test("detects and reports missing columns in dry-run mode", async () => { @@ -188,18 +201,97 @@ describe("sentry cli fix", () => { initSchema(db); db.close(); - const stdoutWrite = mock(() => true); - const mockContext = { - stdout: { write: stdoutWrite }, - stderr: { write: mock(() => true) }, - process: { exitCode: 0 }, - }; + const { stdout } = await runFix(false); + expect(stdout).toContain("Database:"); + expect(stdout).toContain(getTestDir()); + }); - const func = await fixCommand.loader(); - func.call(mockContext, { "dry-run": false }); + test("detects permission issues on readonly database file", async () => { + // Warm the DB cache so getRawDatabase() won't try to reinitialize + // after we break permissions (PRAGMAs like WAL need write access) + getDatabase(); + const dbPath = join(getTestDir(), "cli.db"); - const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); - expect(output).toContain("Database:"); - expect(output).toContain(getTestDir()); + chmodSync(dbPath, 0o444); + const { stdout } = await runFix(true); + + expect(stdout).toContain("permission issue(s)"); + expect(stdout).toContain("0444"); + expect(stdout).toContain("Run 'sentry cli fix' to apply fixes"); + + chmodSync(dbPath, 0o644); + }); + + test("repairs database file permissions", async () => { + getDatabase(); + const dbPath = join(getTestDir(), "cli.db"); + + chmodSync(dbPath, 0o444); + const { stdout, exitCode } = await runFix(false); + + expect(stdout).toContain("Repairing permissions"); + expect(stdout).toContain("0444"); + expect(stdout).toContain("0600"); + expect(stdout).toContain("repaired successfully"); + expect(exitCode).toBe(0); + + // biome-ignore lint/suspicious/noBitwiseOperators: verifying permission bits + const repairedMode = statSync(dbPath).mode & 0o777; + // biome-ignore lint/suspicious/noBitwiseOperators: verifying permission bits + expect(repairedMode & 0o600).toBe(0o600); + }); + + test("detects directory permission issues", async () => { + getDatabase(); + + // Remove write bit from config directory — WAL/SHM files can't be created + chmodSync(getTestDir(), 0o500); + const { stdout } = await runFix(true); + + expect(stdout).toContain("permission issue(s)"); + expect(stdout).toContain("directory"); + expect(stdout).toContain(getTestDir()); + + chmodSync(getTestDir(), 0o700); + }); + + test("dry-run reports permission issues without repairing", async () => { + getDatabase(); + const dbPath = join(getTestDir(), "cli.db"); + + chmodSync(dbPath, 0o444); + const { stdout } = await runFix(true); + + expect(stdout).toContain("permission issue(s)"); + expect(stdout).not.toContain("Repairing"); + + // File should still be readonly — dry-run didn't touch it + // biome-ignore lint/suspicious/noBitwiseOperators: verifying permission bits + const mode = statSync(dbPath).mode & 0o777; + expect(mode).toBe(0o444); + + chmodSync(dbPath, 0o644); + }); + + test("handles both permission and schema issues together", async () => { + // Create a pre-migration DB (missing columns) then break permissions. + // The fix command repairs permissions first, which unblocks schema repair. + const dbPath = join(getTestDir(), "cli.db"); + const db = new Database(dbPath); + createPreMigrationDatabase(db); + db.close(); + + // Warm the cache with this pre-migration DB so getRawDatabase() works + getDatabase(); + + chmodSync(dbPath, 0o444); + const { stdout, exitCode } = await runFix(false); + + expect(stdout).toContain("permission issue(s)"); + expect(stdout).toContain("Repairing permissions"); + expect(stdout).toContain("schema issue(s)"); + expect(stdout).toContain("Repairing schema"); + expect(stdout).toContain("repaired successfully"); + expect(exitCode).toBe(0); }); }); From 300ceb8b2f166e788b8eeacbc869c932beffb2de Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 11 Feb 2026 21:02:46 +0100 Subject: [PATCH 05/11] fix(db): return empty arrays from all()/values() on readonly database The readonly error handler used a bare `return` for all traced methods, yielding `undefined` even for all() and values() which callers expect to return arrays. This would crash on .map()/.length if a write query ever used these methods (e.g. DELETE ... RETURNING *). Extract handleReadonlyError() helper that returns [] for all/values and undefined for run/get. Also reduces complexity in the proxy handler. --- src/lib/telemetry.ts | 16 ++++++++++++++-- test/lib/telemetry.test.ts | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 26ccba59..a8702ee6 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -630,6 +630,19 @@ export function resetReadonlyWarning(): void { /** Methods on SQLite Statement that execute queries and should be traced */ const TRACED_STATEMENT_METHODS = ["get", "run", "all", "values"] as const; +/** + * Handle a readonly database error by warning the user and returning a + * type-appropriate no-op value. Returns `undefined` for run/get (void / no-row) + * and `[]` for all/values (empty result set). + */ +function handleReadonlyError(method: string | symbol): unknown { + warnReadonlyDatabaseOnce(); + if (method === "all" || method === "values") { + return []; + } + return; +} + /** * Wrap a SQLite Statement to automatically trace query execution. * @@ -689,8 +702,7 @@ function createTracedStatement(stmt: T, sql: string): T { // Handle readonly database gracefully: warn once, skip the write. // The CLI still works — reads succeed, only caching/persistence is lost. if (isReadonlyError(error)) { - warnReadonlyDatabaseOnce(); - return; + return handleReadonlyError(prop); } // Re-throw if repair didn't help or wasn't applicable diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index 973cd113..fe2ed601 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -808,6 +808,23 @@ describe("createTracedDatabase", () => { db.close(); }); + test("all() and values() return empty arrays on readonly write", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const allResult = tracedDb + .query("INSERT INTO test (id, name) VALUES (?, ?)") + .all(2, "Bob"); + const valuesResult = tracedDb + .query("INSERT INTO test (id, name) VALUES (?, ?)") + .values(3, "Charlie"); + + expect(allResult).toEqual([]); + expect(valuesResult).toEqual([]); + + db.close(); + }); + test("warning message includes helpful instructions", () => { const db = new Database(dbPath); const tracedDb = createTracedDatabase(db); From c683e885a44d85b7d614bac045c9bd29cb7af670 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 16 Feb 2026 23:40:14 +0000 Subject: [PATCH 06/11] refactor: address PR review comments for readonly db handling - fix.ts: convert to async (stat/chmod from node:fs/promises), parallel checks with Promise.all/Promise.allSettled, inline fixFunc into buildCommand, explicit ENOENT handling - telemetry.ts: use let+noop self-replacing pattern for one-shot warning, add tryRepairReadonly() that chmods files for future commands before falling back to user warning - fix.test.ts: await async func.call() invocations - telemetry.test.ts: update readonly tests for auto-repair behavior --- src/commands/cli/fix.ts | 200 ++++++++++++++++++---------------- src/lib/telemetry.ts | 111 +++++++++++++++---- test/commands/cli/fix.test.ts | 8 +- test/lib/telemetry.test.ts | 72 ++++++++---- 4 files changed, 250 insertions(+), 141 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index afeca031..e715c67d 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -4,7 +4,7 @@ * Diagnose and repair CLI database issues (schema and permissions). */ -import { chmodSync, statSync } from "node:fs"; +import { chmod, stat } from "node:fs/promises"; import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; import { getConfigDir, getDbPath, getRawDatabase } from "../../lib/db/index.js"; @@ -46,22 +46,32 @@ type PermissionIssue = { * @param path - Filesystem path to check * @param expectedMode - Bitmask of required permission bits (e.g., 0o700) * @returns Object with the actual mode if permissions are insufficient, or null if OK. - * Returns null if the path doesn't exist (missing files aren't a permission problem). + * Returns null if the path doesn't exist (ENOENT). Re-throws unexpected errors + * so they propagate to the user and get captured by Sentry's error handling. */ -function checkMode( +async function checkMode( path: string, expectedMode: number -): { actualMode: number } | null { +): Promise<{ actualMode: number } | null> { try { - const st = statSync(path); + const st = await stat(path); // biome-ignore lint/suspicious/noBitwiseOperators: extracting permission bits with bitmask const mode = st.mode & 0o777; // biome-ignore lint/suspicious/noBitwiseOperators: checking required permission bits are set if ((mode & expectedMode) !== expectedMode) { return { actualMode: mode }; } - } catch { - // File/dir doesn't exist — not a permission issue + } catch (error: unknown) { + // Missing files aren't a permission problem (WAL/SHM created on demand) + if ( + error instanceof Error && + (error as NodeJS.ErrnoException).code === "ENOENT" + ) { + return null; + } + // Unexpected filesystem error — re-throw so it surfaces to the user + // and gets captured by the top-level Sentry error handler in bin.ts + throw error; } return null; } @@ -69,47 +79,50 @@ function checkMode( /** * Check if the database file and its directory have correct permissions. * Inspects the config directory (needs rwx), the DB file, and SQLite's - * WAL/SHM journal files (need rw). Missing files are silently skipped - * since WAL/SHM are created on demand. + * WAL/SHM journal files (need rw) in parallel. Missing files are silently + * skipped since WAL/SHM are created on demand. * * @param dbPath - Absolute path to the database file * @returns List of permission issues found (empty if everything is OK) */ -function checkPermissions(dbPath: string): PermissionIssue[] { - const issues: PermissionIssue[] = []; +async function checkPermissions(dbPath: string): Promise { const configDir = getConfigDir(); - // Check config directory permissions - const dirCheck = checkMode(configDir, EXPECTED_DIR_MODE); - if (dirCheck) { - issues.push({ - path: configDir, - kind: "directory", - currentMode: dirCheck.actualMode, - expectedMode: EXPECTED_DIR_MODE, - }); - } - - // Check database file and associated WAL/SHM files - const filesToCheck: Array<{ path: string; kind: "database" | "journal" }> = [ - { path: dbPath, kind: "database" }, - { path: `${dbPath}-wal`, kind: "journal" }, - { path: `${dbPath}-shm`, kind: "journal" }, + const checks: Array<{ + path: string; + kind: PermissionIssue["kind"]; + expectedMode: number; + }> = [ + { path: configDir, kind: "directory", expectedMode: EXPECTED_DIR_MODE }, + { path: dbPath, kind: "database", expectedMode: EXPECTED_FILE_MODE }, + { + path: `${dbPath}-wal`, + kind: "journal", + expectedMode: EXPECTED_FILE_MODE, + }, + { + path: `${dbPath}-shm`, + kind: "journal", + expectedMode: EXPECTED_FILE_MODE, + }, ]; - for (const { path, kind } of filesToCheck) { - const fileCheck = checkMode(path, EXPECTED_FILE_MODE); - if (fileCheck) { - issues.push({ - path, - kind, - currentMode: fileCheck.actualMode, - expectedMode: EXPECTED_FILE_MODE, - }); - } - } - - return issues; + const results = await Promise.all( + checks.map(async ({ path, kind, expectedMode }) => { + const result = await checkMode(path, expectedMode); + if (result) { + return { + path, + kind, + currentMode: result.actualMode, + expectedMode, + } satisfies PermissionIssue; + } + return null; + }) + ); + + return results.filter((r): r is PermissionIssue => r !== null); } /** @@ -122,28 +135,35 @@ function formatMode(mode: number): string { } /** - * Attempt to fix file/directory permissions via chmod. + * Attempt to fix file/directory permissions via chmod in parallel. * Repairs may fail if the current user doesn't own the file. * * @param issues - Permission issues to repair * @returns Separate lists of human-readable repair successes and failures */ -function repairPermissions(issues: PermissionIssue[]): { +async function repairPermissions(issues: PermissionIssue[]): Promise<{ fixed: string[]; failed: string[]; -} { +}> { + const results = await Promise.allSettled( + issues.map(async (issue) => { + await chmod(issue.path, issue.expectedMode); + return `${issue.kind} ${issue.path}: ${formatMode(issue.currentMode)} -> ${formatMode(issue.expectedMode)}`; + }) + ); + const fixed: string[] = []; const failed: string[] = []; - - for (const issue of issues) { - try { - chmodSync(issue.path, issue.expectedMode); - fixed.push( - `${issue.kind} ${issue.path}: ${formatMode(issue.currentMode)} -> ${formatMode(issue.expectedMode)}` - ); - } catch (error) { + for (let i = 0; i < results.length; i++) { + const result = results[i] as PromiseSettledResult; + if (result.status === "fulfilled") { + fixed.push(result.value); + } else { + const issue = issues[i] as PermissionIssue; const reason = - error instanceof Error ? error.message : "permission denied"; + result.reason instanceof Error + ? result.reason.message + : "permission denied"; failed.push(`${issue.kind} ${issue.path}: ${reason}`); } } @@ -173,12 +193,12 @@ type DiagnoseResult = { * @param output - Streams for user-facing output * @returns Count of issues found and whether any repairs failed */ -function handlePermissionIssues( +async function handlePermissionIssues( dbPath: string, dryRun: boolean, { stdout, stderr }: Output -): DiagnoseResult { - const permIssues = checkPermissions(dbPath); +): Promise { + const permIssues = await checkPermissions(dbPath); if (permIssues.length === 0) { return { found: 0, repairFailed: false }; } @@ -196,7 +216,7 @@ function handlePermissionIssues( } stdout.write("Repairing permissions...\n"); - const { fixed, failed } = repairPermissions(permIssues); + const { fixed, failed } = await repairPermissions(permIssues); for (const fix of fixed) { stdout.write(` + ${fix}\n`); } @@ -265,42 +285,6 @@ function handleSchemaIssues( return { found: issues.length, repairFailed: failed.length > 0 }; } -/** - * Entry point for `sentry cli fix`. Runs permission and schema checks - * in sequence, repairs what it can, and reports results. - */ -function fixFunc(this: SentryContext, flags: FixFlags): void { - const { stdout, process: proc } = this; - const dbPath = getDbPath(); - const dryRun = flags["dry-run"]; - const out = { stdout, stderr: this.stderr }; - - stdout.write(`Database: ${dbPath}\n`); - stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`); - - const perm = handlePermissionIssues(dbPath, dryRun, out); - const schema = handleSchemaIssues(dbPath, dryRun, out); - const totalFound = perm.found + schema.found; - - if (totalFound === 0) { - stdout.write( - "No issues found. Database schema and permissions are correct.\n" - ); - return; - } - - if (dryRun) { - stdout.write("Run 'sentry cli fix' to apply fixes.\n"); - return; - } - - if (perm.repairFailed || schema.repairFailed) { - proc.exitCode = 1; - } else { - stdout.write("All issues repaired successfully.\n"); - } -} - export const fixCommand = buildCommand({ docs: { brief: "Diagnose and repair CLI database issues", @@ -324,5 +308,35 @@ export const fixCommand = buildCommand({ }, }, }, - func: fixFunc, + async func(this: SentryContext, flags: FixFlags): Promise { + const { stdout, process: proc } = this; + const dbPath = getDbPath(); + const dryRun = flags["dry-run"]; + const out = { stdout, stderr: this.stderr }; + + stdout.write(`Database: ${dbPath}\n`); + stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`); + + const perm = await handlePermissionIssues(dbPath, dryRun, out); + const schema = handleSchemaIssues(dbPath, dryRun, out); + const totalFound = perm.found + schema.found; + + if (totalFound === 0) { + stdout.write( + "No issues found. Database schema and permissions are correct.\n" + ); + return; + } + + if (dryRun) { + stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + return; + } + + if (perm.repairFailed || schema.repairFailed) { + proc.exitCode = 1; + } else { + stdout.write("All issues repaired successfully.\n"); + } + }, }); diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index a8702ee6..4c399853 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -9,6 +9,7 @@ * No PII is collected. Opt-out via SENTRY_CLI_NO_TELEMETRY=1 environment variable. */ +import { chmodSync } from "node:fs"; // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import import * as Sentry from "@sentry/bun"; import { CLI_VERSION, SENTRY_CLI_DSN } from "./constants.js"; @@ -586,57 +587,123 @@ export function withDbSpan(operation: string, fn: () => T): T { ); } -/** Whether the readonly database warning has been shown this process */ -let readonlyWarningShown = false; +/** Intentional no-op used as a self-replacement target for one-shot functions. */ +// biome-ignore lint/suspicious/noEmptyBlockStatements: intentional noop +const noop = (): void => {}; -/** - * Print a one-time warning to stderr when the local database is read-only. - * Does nothing if the warning was already shown this process. - * - * Uses lazy require for db/index.js to avoid a circular dependency - * (db/index.ts imports createTracedDatabase from this module). - */ -function warnReadonlyDatabaseOnce(): void { - if (readonlyWarningShown) { - return; - } - readonlyWarningShown = true; - - let dbPath = "~/.sentry/cli.db"; +/** Resolves the database path, falling back to a default if the import fails. */ +function resolveDbPath(): string { try { const { getDbPath } = require("./db/index.js") as { getDbPath: () => string; }; - dbPath = getDbPath(); + return getDbPath(); } catch { - // Fall back to default path if import fails + return "~/.sentry/cli.db"; } +} +/** + * Print a one-time warning to stderr when the local database is read-only. + * Replaces itself with a noop after the first call so subsequent invocations + * are free. Assigned via `let` so the binding can be swapped. + * + * Uses lazy require for db/index.js to avoid a circular dependency + * (db/index.ts imports createTracedDatabase from this module). + */ +let warnReadonlyDatabaseOnce = (): void => { + warnReadonlyDatabaseOnce = noop; + + const dbPath = resolveDbPath(); process.stderr.write( `\nWarning: Sentry CLI local database is read-only. Caching and preferences won't persist.\n` + ` Path: ${dbPath}\n` + " Fix: sentry cli fix\n\n" ); +}; + +/** Whether we already attempted a permission repair this process. */ +let repairAttempted = false; + +/** + * Attempt to repair database file permissions so future commands can write. + * + * SQLite caches the readonly state at connection open time, so even after a + * successful chmod the *current* connection remains readonly. This function + * repairs permissions for the NEXT command and prints a differentiated message. + * If the repair fails (e.g., file owned by another user) we fall through to + * {@link warnReadonlyDatabaseOnce} which tells the user to run `sentry cli fix`. + * + * Replaces itself with a noop after the first call via the `repairAttempted` + * guard so we only try once per process. + */ +function tryRepairReadonly(): boolean { + if (repairAttempted) { + return false; + } + repairAttempted = true; + + try { + const dbPath = resolveDbPath(); + chmodSync(dbPath, 0o600); + try { + chmodSync(`${dbPath}-wal`, 0o600); + } catch { + // WAL file may not exist + } + try { + chmodSync(`${dbPath}-shm`, 0o600); + } catch { + // SHM file may not exist + } + + // Disable the fallback warning — repair succeeded + warnReadonlyDatabaseOnce = noop; + + process.stderr.write( + "\nNote: Database permissions were auto-repaired. Caching will resume on next command.\n\n" + ); + return true; + } catch { + // chmod failed — fall through so warnReadonlyDatabaseOnce fires + return false; + } } /** - * Reset the readonly warning flag (for testing). + * Reset all readonly-related state (for testing). * @internal */ export function resetReadonlyWarning(): void { - readonlyWarningShown = false; + repairAttempted = false; + warnReadonlyDatabaseOnce = (): void => { + warnReadonlyDatabaseOnce = noop; + + const dbPath = resolveDbPath(); + process.stderr.write( + `\nWarning: Sentry CLI local database is read-only. Caching and preferences won't persist.\n` + + ` Path: ${dbPath}\n` + + " Fix: sentry cli fix\n\n" + ); + }; } /** Methods on SQLite Statement that execute queries and should be traced */ const TRACED_STATEMENT_METHODS = ["get", "run", "all", "values"] as const; /** - * Handle a readonly database error by warning the user and returning a + * Handle a readonly database error by attempting auto-repair and returning a * type-appropriate no-op value. Returns `undefined` for run/get (void / no-row) * and `[]` for all/values (empty result set). + * + * First tries to repair file permissions via {@link tryRepairReadonly}. If that + * fails (or was already attempted), falls back to a one-shot warning directing + * the user to `sentry cli fix`. */ function handleReadonlyError(method: string | symbol): unknown { - warnReadonlyDatabaseOnce(); + if (!tryRepairReadonly()) { + warnReadonlyDatabaseOnce(); + } if (method === "all" || method === "values") { return []; } diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 7137a895..885614d5 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -78,7 +78,7 @@ async function runFix(dryRun: boolean) { }; const func = await fixCommand.loader(); - func.call(mockContext, { "dry-run": dryRun }); + await func.call(mockContext, { "dry-run": dryRun }); return { stdout: stdoutWrite.mock.calls.map((c) => c[0]).join(""), @@ -112,7 +112,7 @@ describe("sentry cli fix", () => { }; const func = await fixCommand.loader(); - func.call(mockContext, { "dry-run": true }); + await func.call(mockContext, { "dry-run": true }); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Found"); @@ -136,7 +136,7 @@ describe("sentry cli fix", () => { }; const func = await fixCommand.loader(); - func.call(mockContext, { "dry-run": false }); + await func.call(mockContext, { "dry-run": false }); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); expect(output).toContain("Repairing"); @@ -178,7 +178,7 @@ describe("sentry cli fix", () => { const func = await fixCommand.loader(); // When getRawDatabase() is called, it triggers getDatabase() which runs initSchema() // This auto-creates the missing dsn_cache table, so the fix command sees no issues - func.call(mockContext, { "dry-run": false }); + await func.call(mockContext, { "dry-run": false }); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); // Auto-repair at startup means command sees healthy database diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index fe2ed601..a13b7758 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -785,7 +785,23 @@ describe("createTracedDatabase", () => { db.close(); }); - test("warns to stderr only once across multiple writes", () => { + test("auto-repairs permissions on first readonly write", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const stderrSpy = spyOn(process.stderr, "write"); + + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); + + const output = stderrSpy.mock.calls.map((c) => String(c[0])).join(""); + expect(output).toContain("auto-repaired"); + expect(output).toContain("next command"); + + stderrSpy.mockRestore(); + db.close(); + }); + + test("shows only one message across multiple writes", () => { const db = new Database(dbPath); const tracedDb = createTracedDatabase(db); @@ -799,10 +815,39 @@ describe("createTracedDatabase", () => { .query("INSERT INTO test (id, name) VALUES (?, ?)") .run(4, "Dave"); - const warningCalls = stderrSpy.mock.calls.filter((call) => - String(call[0]).includes("read-only") - ); - expect(warningCalls.length).toBe(1); + // Only one message total (the auto-repair note) + expect(stderrSpy.mock.calls.length).toBe(1); + + stderrSpy.mockRestore(); + db.close(); + }); + + test("resetReadonlyWarning allows auto-repair to trigger again", () => { + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const stderrSpy = spyOn(process.stderr, "write"); + + // First write triggers auto-repair + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); + expect(stderrSpy.mock.calls.length).toBe(1); + expect(String(stderrSpy.mock.calls[0]?.[0])).toContain("auto-repaired"); + + // Second write is silent (one-shot guard) + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(3, "X"); + expect(stderrSpy.mock.calls.length).toBe(1); + + // Reset all state + resetReadonlyWarning(); + stderrSpy.mockClear(); + + // Re-break permissions so SQLite errors again + chmodSync(dbPath, 0o444); + + // Next write triggers auto-repair again after reset + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(4, "Y"); + expect(stderrSpy.mock.calls.length).toBe(1); + expect(String(stderrSpy.mock.calls[0]?.[0])).toContain("auto-repaired"); stderrSpy.mockRestore(); db.close(); @@ -824,22 +869,5 @@ describe("createTracedDatabase", () => { db.close(); }); - - test("warning message includes helpful instructions", () => { - const db = new Database(dbPath); - const tracedDb = createTracedDatabase(db); - - const stderrSpy = spyOn(process.stderr, "write"); - - tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); - - const warning = String(stderrSpy.mock.calls[0]?.[0] ?? ""); - expect(warning).toContain("local database"); - expect(warning).toContain("read-only"); - expect(warning).toContain("sentry cli fix"); - - stderrSpy.mockRestore(); - db.close(); - }); }); }); From 2dcf8524d62413ad5ccb0d560547a8840349bf06 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 16 Feb 2026 23:49:19 +0000 Subject: [PATCH 07/11] fix: address BugBot review comments for readonly db handling - Use exact mode match instead of bitmask in checkMode (prevents extra permission bits from passing validation) - Wrap handleSchemaIssues in try/catch so --dry-run doesn't crash on readonly databases - Repair config directory permissions in tryRepairReadonly alongside database files - Extract chmodIfExists helper that only catches ENOENT, re-throws other errors instead of swallowing them - Update tests: chmod db files to 0o600 after creation to match production setDbPermissions behavior --- src/commands/cli/fix.ts | 34 +++++++++++++++++++++++++------ src/lib/telemetry.ts | 38 ++++++++++++++++++++++++++--------- test/commands/cli/fix.test.ts | 9 +++++++-- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index e715c67d..dcf9954c 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -41,11 +41,15 @@ type PermissionIssue = { }; /** - * Check if a path has the required permission bits set. + * Check if a path has the exact expected permission mode. + * + * Uses exact match (not bitmask) so extra bits like group/other read (e.g., + * 0o644 instead of 0o600) are flagged as issues — the CLI's local database + * may contain auth tokens and should not be accessible to other users. * * @param path - Filesystem path to check - * @param expectedMode - Bitmask of required permission bits (e.g., 0o700) - * @returns Object with the actual mode if permissions are insufficient, or null if OK. + * @param expectedMode - Exact permission mode (e.g., 0o700, 0o600) + * @returns Object with the actual mode if permissions differ, or null if OK. * Returns null if the path doesn't exist (ENOENT). Re-throws unexpected errors * so they propagate to the user and get captured by Sentry's error handling. */ @@ -57,8 +61,7 @@ async function checkMode( const st = await stat(path); // biome-ignore lint/suspicious/noBitwiseOperators: extracting permission bits with bitmask const mode = st.mode & 0o777; - // biome-ignore lint/suspicious/noBitwiseOperators: checking required permission bits are set - if ((mode & expectedMode) !== expectedMode) { + if (mode !== expectedMode) { return { actualMode: mode }; } } catch (error: unknown) { @@ -318,7 +321,26 @@ export const fixCommand = buildCommand({ stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`); const perm = await handlePermissionIssues(dbPath, dryRun, out); - const schema = handleSchemaIssues(dbPath, dryRun, out); + + // Schema check opens the database, which can throw if the DB or config + // directory is readonly. Guard with try/catch so --dry-run can finish + // diagnostics even when the filesystem is broken. + let schema: DiagnoseResult; + try { + schema = handleSchemaIssues(dbPath, dryRun, out); + } catch { + // If we already found permission issues, the schema check failure is + // expected — don't obscure the permission report with an unrelated crash. + // If no permission issues were found, this is unexpected so re-report it. + if (perm.found === 0) { + out.stderr.write("Could not open database to check schema.\n"); + out.stderr.write( + `Try deleting the database and restarting: rm "${dbPath}"\n` + ); + } + schema = { found: 0, repairFailed: true }; + } + const totalFound = perm.found + schema.found; if (totalFound === 0) { diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 4c399853..36faa53d 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -637,6 +637,24 @@ let repairAttempted = false; * Replaces itself with a noop after the first call via the `repairAttempted` * guard so we only try once per process. */ +/** + * Chmod a path, ignoring ENOENT (file doesn't exist yet). + * Re-throws any other error so permission failures aren't silently masked. + */ +function chmodIfExists(filePath: string, mode: number): void { + try { + chmodSync(filePath, mode); + } catch (error: unknown) { + if ( + error instanceof Error && + (error as NodeJS.ErrnoException).code === "ENOENT" + ) { + return; + } + throw error; + } +} + function tryRepairReadonly(): boolean { if (repairAttempted) { return false; @@ -645,17 +663,17 @@ function tryRepairReadonly(): boolean { try { const dbPath = resolveDbPath(); + + // Repair config directory (needs rwx for WAL/SHM creation) + const { dirname } = require("node:path") as { + dirname: (p: string) => string; + }; + chmodSync(dirname(dbPath), 0o700); + + // Repair database file and journal files chmodSync(dbPath, 0o600); - try { - chmodSync(`${dbPath}-wal`, 0o600); - } catch { - // WAL file may not exist - } - try { - chmodSync(`${dbPath}-shm`, 0o600); - } catch { - // SHM file may not exist - } + chmodIfExists(`${dbPath}-wal`, 0o600); + chmodIfExists(`${dbPath}-shm`, 0o600); // Disable the fallback warning — repair succeeded warnReadonlyDatabaseOnce = noop; diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 885614d5..5fa60eb9 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -89,9 +89,12 @@ async function runFix(dryRun: boolean) { describe("sentry cli fix", () => { test("reports no issues for healthy database", async () => { - const db = new Database(join(getTestDir(), "cli.db")); + const dbPath = join(getTestDir(), "cli.db"); + const db = new Database(dbPath); initSchema(db); db.close(); + // Match the permissions that setDbPermissions() applies in production + chmodSync(dbPath, 0o600); const { stdout } = await runFix(false); expect(stdout).toContain("No issues found"); @@ -164,9 +167,11 @@ describe("sentry cli fix", () => { // that was previously missing tables (now fixed by auto-repair at startup). test("handles database that was auto-repaired at startup", async () => { // Create database missing dsn_cache - initSchema will create it when command runs - const db = new Database(join(getTestDir(), "cli.db")); + const dbPath = join(getTestDir(), "cli.db"); + const db = new Database(dbPath); createDatabaseWithMissingTables(db, ["dsn_cache"]); db.close(); + chmodSync(dbPath, 0o600); const stdoutWrite = mock(() => true); const mockContext = { From 587d8fddb66c0761ec4d21437147191c2118b10c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 16 Feb 2026 23:55:09 +0000 Subject: [PATCH 08/11] fix(cli): set exitCode=1 when schema check fails with no permission issues Previously, when handleSchemaIssues threw and no permission issues were detected, totalFound was 0 and the command reported 'No issues found' despite the schema check failure. Now anyFailed is checked alongside totalFound to ensure failures are always surfaced with a non-zero exit. --- src/commands/cli/fix.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index dcf9954c..916855c2 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -342,8 +342,9 @@ export const fixCommand = buildCommand({ } const totalFound = perm.found + schema.found; + const anyFailed = perm.repairFailed || schema.repairFailed; - if (totalFound === 0) { + if (totalFound === 0 && !anyFailed) { stdout.write( "No issues found. Database schema and permissions are correct.\n" ); @@ -351,11 +352,16 @@ export const fixCommand = buildCommand({ } if (dryRun) { - stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + if (totalFound > 0) { + stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + } + if (anyFailed) { + proc.exitCode = 1; + } return; } - if (perm.repairFailed || schema.repairFailed) { + if (anyFailed) { proc.exitCode = 1; } else { stdout.write("All issues repaired successfully.\n"); From 6887a60c26b9b474b2809717efd9715e9fdb48f8 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Feb 2026 00:02:15 +0000 Subject: [PATCH 09/11] fix(cli): handle EACCES gracefully in checkMode during permission scan When the config directory lacks execute permission, stat on child files (db, wal, shm) fails with EACCES instead of returning mode bits. Treat EACCES like ENOENT (skip the file) since the directory permission check will catch the root cause. --- src/commands/cli/fix.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index 916855c2..440ee5f0 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -65,15 +65,16 @@ async function checkMode( return { actualMode: mode }; } } catch (error: unknown) { - // Missing files aren't a permission problem (WAL/SHM created on demand) - if ( - error instanceof Error && - (error as NodeJS.ErrnoException).code === "ENOENT" - ) { + const code = + error instanceof Error + ? (error as NodeJS.ErrnoException).code + : undefined; + // Missing files aren't a permission problem (WAL/SHM created on demand). + // EACCES means the parent directory blocks stat — the directory check + // will catch the root cause, so skip the individual file here. + if (code === "ENOENT" || code === "EACCES") { return null; } - // Unexpected filesystem error — re-throw so it surfaces to the user - // and gets captured by the top-level Sentry error handler in bin.ts throw error; } return null; From c245be52f05e406ee04197866e6be12a350d1cb1 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Feb 2026 00:07:49 +0000 Subject: [PATCH 10/11] fix(cli): repair directory permissions before files to avoid EACCES race Directory chmod must complete before child file chmod calls, otherwise file repairs can fail with EACCES when the parent directory lacks execute permission. Split repairPermissions into directory-first (sequential) then files (parallel) to prevent the race. --- src/commands/cli/fix.ts | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index 440ee5f0..80270c2a 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -139,8 +139,10 @@ function formatMode(mode: number): string { } /** - * Attempt to fix file/directory permissions via chmod in parallel. - * Repairs may fail if the current user doesn't own the file. + * Attempt to fix file/directory permissions via chmod. + * Directory issues are repaired first (sequentially) because child file + * chmod calls will fail with EACCES if the parent directory lacks execute + * permission. File issues are then repaired in parallel. * * @param issues - Permission issues to repair * @returns Separate lists of human-readable repair successes and failures @@ -149,6 +151,31 @@ async function repairPermissions(issues: PermissionIssue[]): Promise<{ fixed: string[]; failed: string[]; }> { + const fixed: string[] = []; + const failed: string[] = []; + + // Repair directories first so child file chmod calls don't fail with EACCES + const dirIssues = issues.filter((i) => i.kind === "directory"); + const fileIssues = issues.filter((i) => i.kind !== "directory"); + + await collectResults(dirIssues, fixed, failed); + await collectResults(fileIssues, fixed, failed); + + return { fixed, failed }; +} + +/** + * Run chmod for each issue in parallel, collecting successes and failures. + * + * @param issues - Permission issues to repair + * @param fixed - Accumulator for successful repair messages + * @param failed - Accumulator for failed repair messages + */ +async function collectResults( + issues: PermissionIssue[], + fixed: string[], + failed: string[] +): Promise { const results = await Promise.allSettled( issues.map(async (issue) => { await chmod(issue.path, issue.expectedMode); @@ -156,8 +183,6 @@ async function repairPermissions(issues: PermissionIssue[]): Promise<{ }) ); - const fixed: string[] = []; - const failed: string[] = []; for (let i = 0; i < results.length; i++) { const result = results[i] as PromiseSettledResult; if (result.status === "fulfilled") { @@ -171,8 +196,6 @@ async function repairPermissions(issues: PermissionIssue[]): Promise<{ failed.push(`${issue.kind} ${issue.path}: ${reason}`); } } - - return { fixed, failed }; } type Output = { From 9617a9b56b61f5fe68d856f30284ada0faab8473 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 17 Feb 2026 00:50:08 +0000 Subject: [PATCH 11/11] test: improve patch coverage for readonly db handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add fix command tests: - Schema check failure with no permission issues sets exitCode=1 - Dry-run with schema failure sets exitCode=1 - Schema check suppresses error when permission issues explain failure - Schema repair success path (missing columns → repaired) Add telemetry test: - Readonly warning fallback when auto-repair fails (mock chmodSync to simulate unowned files) --- test/commands/cli/fix.test.ts | 69 +++++++++++++++++++++++++++++++++++ test/lib/telemetry.test.ts | 35 ++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 5fa60eb9..4573bc3b 100644 --- a/test/commands/cli/fix.test.ts +++ b/test/commands/cli/fix.test.ts @@ -299,4 +299,73 @@ describe("sentry cli fix", () => { expect(stdout).toContain("repaired successfully"); expect(exitCode).toBe(0); }); + + test("repairs missing columns and reports success", async () => { + // Create database with pre-migration tables then repair (non-dry-run) + // This exercises the schema repair success path + const db = new Database(join(getTestDir(), "cli.db")); + createPreMigrationDatabase(db); + db.close(); + + const { stdout, exitCode } = await runFix(false); + + expect(stdout).toContain("schema issue(s)"); + expect(stdout).toContain("Missing column"); + expect(stdout).toContain("Repairing schema"); + expect(stdout).toContain("Added column"); + expect(stdout).toContain("repaired successfully"); + expect(exitCode).toBe(0); + }); + + test("sets exitCode=1 when schema check throws with no permission issues", async () => { + // Create a DB file that cannot be opened by getRawDatabase. + // Write garbage so SQLite cannot parse it — getRawDatabase will throw. + const dbPath = join(getTestDir(), "cli.db"); + closeDatabase(); + await Bun.write(dbPath, "not a sqlite database"); + chmodSync(dbPath, 0o600); + chmodSync(getTestDir(), 0o700); + + const { stdout, stderr, exitCode } = await runFix(false); + + // No permission issues found, so schema failure should be reported + expect(stderr).toContain("Could not open database to check schema"); + expect(stderr).toContain("Try deleting the database"); + expect(exitCode).toBe(1); + // Should NOT say "No issues found" + expect(stdout).not.toContain("No issues found"); + }); + + test("dry-run sets exitCode=1 when schema check throws", async () => { + // Same corrupt DB scenario, but in dry-run mode + const dbPath = join(getTestDir(), "cli.db"); + closeDatabase(); + await Bun.write(dbPath, "not a sqlite database"); + chmodSync(dbPath, 0o600); + chmodSync(getTestDir(), 0o700); + + const { stdout, stderr, exitCode } = await runFix(true); + + expect(stderr).toContain("Could not open database to check schema"); + expect(exitCode).toBe(1); + // Should NOT suggest running fix (no fixable issues found) + expect(stdout).not.toContain("Run 'sentry cli fix' to apply fixes"); + }); + + test("schema check failure with permission issues does not print schema error", async () => { + // When permissions are broken AND schema can't be opened, the schema error + // is suppressed because permission issues are the likely root cause. + getDatabase(); + const dbPath = join(getTestDir(), "cli.db"); + + // Make DB readonly — will cause permission issue AND potentially schema failure + chmodSync(dbPath, 0o444); + + // The schema catch block should suppress the error message when perm.found > 0 + const { stdout, stderr } = await runFix(true); + + expect(stdout).toContain("permission issue(s)"); + // Should NOT print "Could not open database" since permission issues explain it + expect(stderr).not.toContain("Could not open database"); + }); }); diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index a13b7758..aa1f06b2 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -869,5 +869,40 @@ describe("createTracedDatabase", () => { db.close(); }); + + test("shows readonly warning when auto-repair fails", () => { + // Mock chmodSync to always throw, simulating a file owned by another user. + // This makes tryRepairReadonly fail and fall through to warnReadonlyDatabaseOnce. + const { mock: mockFn } = require("bun:test"); + mockFn.module("node:fs", () => { + const realFs = require("node:fs"); + return { + ...realFs, + chmodSync: () => { + throw Object.assign(new Error("EPERM: operation not permitted"), { + code: "EPERM", + }); + }, + }; + }); + + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + const stderrSpy = spyOn(process.stderr, "write"); + + tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(2, "Bob"); + + const output = stderrSpy.mock.calls.map((c) => String(c[0])).join(""); + // When repair fails, the warning message should appear instead + expect(output).toContain("read-only"); + expect(output).toContain("sentry cli fix"); + + stderrSpy.mockRestore(); + db.close(); + + // Restore mock + mockFn.module("node:fs", () => require("node:fs")); + }); }); });