diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index c0c2ca7a..80270c2a 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -1,12 +1,13 @@ /** * sentry cli fix * - * Diagnose and repair CLI database issues. + * Diagnose and repair CLI database issues (schema and permissions). */ +import { chmod, stat } from "node:fs/promises"; 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, @@ -18,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}`; @@ -25,15 +27,301 @@ 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 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 - 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. + */ +async function checkMode( + path: string, + expectedMode: number +): Promise<{ actualMode: number } | null> { + try { + const st = await stat(path); + // biome-ignore lint/suspicious/noBitwiseOperators: extracting permission bits with bitmask + const mode = st.mode & 0o777; + if (mode !== expectedMode) { + return { actualMode: mode }; + } + } catch (error: unknown) { + 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; + } + throw error; + } + return null; +} + +/** + * 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) 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) + */ +async function checkPermissions(dbPath: string): Promise { + const configDir = getConfigDir(); + + 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, + }, + ]; + + 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); +} + +/** + * 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 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 + */ +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); + return `${issue.kind} ${issue.path}: ${formatMode(issue.currentMode)} -> ${formatMode(issue.expectedMode)}`; + }) + ); + + 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 = + result.reason instanceof Error + ? result.reason.message + : "permission denied"; + failed.push(`${issue.kind} ${issue.path}: ${reason}`); + } + } +} + +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; +}; + +/** + * 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 + */ +async function handlePermissionIssues( + dbPath: string, + dryRun: boolean, + { stdout, stderr }: Output +): Promise { + const permIssues = await 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 } = await 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 }; +} + +/** + * 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, + 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 }; +} + 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 +335,60 @@ export const fixCommand = buildCommand({ }, }, }, - func(this: SentryContext, flags: FixFlags): void { - const { stdout, stderr, process: proc } = this; + 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 db = getRawDatabase(); - const issues = getSchemaIssues(db); + const perm = await handlePermissionIssues(dbPath, dryRun, out); - if (issues.length === 0) { - stdout.write("No issues found. Database schema is up to date.\n"); - return; + // 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 }; } - stdout.write(`Found ${issues.length} issue(s):\n`); - for (const issue of issues) { - stdout.write(` - ${formatIssue(issue)}\n`); - } - stdout.write("\n"); + const totalFound = perm.found + schema.found; + const anyFailed = perm.repairFailed || schema.repairFailed; - if (flags["dry-run"]) { - stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + if (totalFound === 0 && !anyFailed) { + stdout.write( + "No issues found. Database schema and permissions are correct.\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`); + if (dryRun) { + if (totalFound > 0) { + stdout.write("Run 'sentry cli fix' to apply fixes.\n"); + } + if (anyFailed) { + proc.exitCode = 1; } - stderr.write( - `\nTry deleting the database and restarting: rm ${dbPath}\n` - ); - proc.exitCode = 1; return; } - stdout.write("\nDatabase repaired successfully.\n"); + if (anyFailed) { + proc.exitCode = 1; + } else { + stdout.write("All issues repaired successfully.\n"); + } }, }); 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..36faa53d 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -9,10 +9,11 @@ * 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"; -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,9 +587,147 @@ export function withDbSpan(operation: string, fn: () => T): T { ); } +/** Intentional no-op used as a self-replacement target for one-shot functions. */ +// biome-ignore lint/suspicious/noEmptyBlockStatements: intentional noop +const noop = (): void => {}; + +/** 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; + }; + return getDbPath(); + } catch { + 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. + */ +/** + * 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; + } + repairAttempted = true; + + 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); + chmodIfExists(`${dbPath}-wal`, 0o600); + chmodIfExists(`${dbPath}-shm`, 0o600); + + // 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 all readonly-related state (for testing). + * @internal + */ +export function resetReadonlyWarning(): void { + 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 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 { + if (!tryRepairReadonly()) { + warnReadonlyDatabaseOnce(); + } + if (method === "all" || method === "values") { + return []; + } + return; +} + /** * Wrap a SQLite Statement to automatically trace query execution. * @@ -644,6 +783,13 @@ 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)) { + return handleReadonlyError(prop); + } + // Re-throw if repair didn't help or wasn't applicable throw error; } diff --git a/test/commands/cli/fix.test.ts b/test/commands/cli/fix.test.ts index 029949fd..4573bc3b 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,41 @@ 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(); + await 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")); + 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 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("up to date"); + 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 () => { @@ -99,7 +115,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"); @@ -123,7 +139,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"); @@ -151,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 = { @@ -165,7 +183,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 @@ -188,18 +206,166 @@ 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); + }); + + 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/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..aa1f06b2 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,180 @@ describe("createTracedDatabase", () => { db.close(); }); + + describe("readonly database handling", () => { + let tmpDir: string; + let dbPath: string; + + beforeEach(() => { + resetReadonlyWarning(); + + tmpDir = `${import.meta.dir}/tmp-readonly-${Date.now()}`; + mkdirSync(tmpDir, { recursive: true }); + dbPath = `${tmpDir}/test.db`; + + 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(); + + chmodSync(dbPath, 0o444); + }); + + afterEach(() => { + try { + chmodSync(dbPath, 0o644); + } catch { + // May already be cleaned up + } + rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("does not throw on write to readonly database", () => { + // bun:sqlite opens the file successfully — the error only surfaces on write + const db = new Database(dbPath); + const tracedDb = createTracedDatabase(db); + + 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("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); + + const stderrSpy = spyOn(process.stderr, "write"); + + 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"); + + // 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(); + }); + + 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("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")); + }); + }); });