Skip to content

Commit b965269

Browse files
committed
fix(brew): handle root-owned config dir from sudo installs
- Make all post-install setup steps non-fatal using a bestEffort() wrapper so Homebrew's post_install never aborts on permission errors - In tryRepairReadonly(), detect root-owned files (uid 0) and emit a targeted message with the actual username instead of falling through to the generic warning - Add ownership detection and repair to `sentry cli fix`: - Checks config dir / DB / WAL / SHM file ownership - When run as root (sudo sentry cli fix), performs chown to transfer ownership back to the real user (inferred from SUDO_USER env var) - When not root, prints the exact sudo chown command to run
1 parent 14490e7 commit b965269

File tree

5 files changed

+775
-74
lines changed

5 files changed

+775
-74
lines changed

src/commands/cli/fix.ts

Lines changed: 311 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
/**
22
* sentry cli fix
33
*
4-
* Diagnose and repair CLI database issues (schema and permissions).
4+
* Diagnose and repair CLI database issues (schema, permissions, and ownership).
55
*/
66

7-
import { chmod, stat } from "node:fs/promises";
7+
import { execSync } from "node:child_process";
8+
import { chmod, chown, stat } from "node:fs/promises";
9+
import { userInfo } from "node:os";
810
import type { SentryContext } from "../../context.js";
911
import { buildCommand } from "../../lib/command.js";
1012
import { getConfigDir, getDbPath, getRawDatabase } from "../../lib/db/index.js";
@@ -43,6 +45,33 @@ type PermissionIssue = {
4345
expectedMode: number;
4446
};
4547

48+
/**
49+
* A file or directory that is owned by a different user (typically root),
50+
* preventing the current process from writing to it.
51+
*/
52+
type OwnershipIssue = {
53+
path: string;
54+
kind: "directory" | "database" | "journal";
55+
/** UID of the file's current owner */
56+
ownerUid: number;
57+
};
58+
59+
/**
60+
* Determine the real username for display in chown instructions.
61+
*
62+
* When running under `sudo`, `SUDO_USER` holds the original user's login name.
63+
* Falls back to `USER` / `USERNAME` env vars, then `os.userInfo()`.
64+
*/
65+
function getRealUsername(): string {
66+
return (
67+
process.env.SUDO_USER ||
68+
process.env.USER ||
69+
process.env.USERNAME ||
70+
userInfo().username ||
71+
"$(whoami)"
72+
);
73+
}
74+
4675
/**
4776
* Check if a path has the exact expected permission mode.
4877
*
@@ -132,6 +161,221 @@ async function checkPermissions(dbPath: string): Promise<PermissionIssue[]> {
132161
return results.filter((r): r is PermissionIssue => r !== null);
133162
}
134163

164+
/**
165+
* Check whether any config dir files or the DB are owned by a different user
166+
* (typically root after a `sudo` install).
167+
*
168+
* We only check the config directory and the DB file — those are the gating
169+
* items. If they are owned by root, chmod will fail and is pointless to attempt.
170+
*
171+
* @param dbPath - Absolute path to the database file
172+
* @param currentUid - The UID of the running process
173+
* @returns List of paths owned by a different user (empty = all owned by us)
174+
*/
175+
async function checkOwnership(
176+
dbPath: string,
177+
currentUid: number
178+
): Promise<OwnershipIssue[]> {
179+
const configDir = getConfigDir();
180+
181+
const checks: Array<{ path: string; kind: OwnershipIssue["kind"] }> = [
182+
{ path: configDir, kind: "directory" },
183+
{ path: dbPath, kind: "database" },
184+
{ path: `${dbPath}-wal`, kind: "journal" },
185+
{ path: `${dbPath}-shm`, kind: "journal" },
186+
];
187+
188+
const settled = await Promise.allSettled(checks.map((c) => stat(c.path)));
189+
const issues: OwnershipIssue[] = [];
190+
191+
for (let i = 0; i < settled.length; i++) {
192+
const result = settled[i] as PromiseSettledResult<
193+
Awaited<ReturnType<typeof stat>>
194+
>;
195+
const check = checks[i] as (typeof checks)[number];
196+
197+
if (result.status === "fulfilled") {
198+
const ownerUid = Number(result.value.uid);
199+
if (ownerUid !== currentUid) {
200+
issues.push({ path: check.path, kind: check.kind, ownerUid });
201+
}
202+
continue;
203+
}
204+
205+
// Missing files are fine (WAL/SHM created on demand).
206+
// EACCES on a child file means the directory already blocks access — the
207+
// directory check above will surface the real issue.
208+
const code =
209+
result.reason instanceof Error
210+
? (result.reason as NodeJS.ErrnoException).code
211+
: undefined;
212+
if (code !== "ENOENT" && code !== "EACCES") {
213+
throw result.reason;
214+
}
215+
}
216+
217+
return issues;
218+
}
219+
220+
/**
221+
* Resolve the numeric UID for a username by running `id -u <username>`.
222+
* Returns null if the lookup fails or returns a non-numeric result.
223+
*
224+
* Uses `execSync` from `node:child_process` so this works on both the Bun
225+
* binary and the npm/node distribution.
226+
*/
227+
function resolveUid(username: string): number | null {
228+
try {
229+
const result = execSync(`id -u ${username}`, {
230+
encoding: "utf-8",
231+
stdio: ["pipe", "pipe", "ignore"],
232+
});
233+
const uid = Number(result.trim());
234+
return Number.isNaN(uid) ? null : uid;
235+
} catch {
236+
return null;
237+
}
238+
}
239+
240+
/**
241+
* Perform chown on the given ownership issues, transferring files to
242+
* `username`. Called only when the current process is already root.
243+
*
244+
* @returns Object with lists of human-readable success and failure messages
245+
*/
246+
async function repairOwnership(
247+
issues: OwnershipIssue[],
248+
username: string,
249+
targetUid: number
250+
): Promise<{ fixed: string[]; failed: string[] }> {
251+
const fixed: string[] = [];
252+
const failed: string[] = [];
253+
254+
const results = await Promise.allSettled(
255+
issues.map((issue) => chown(issue.path, targetUid, -1))
256+
);
257+
258+
for (let i = 0; i < results.length; i++) {
259+
const result = results[i] as PromiseSettledResult<void>;
260+
const issue = issues[i] as OwnershipIssue;
261+
if (result.status === "fulfilled") {
262+
fixed.push(`${issue.kind} ${issue.path}: transferred to ${username}`);
263+
} else {
264+
const reason =
265+
result.reason instanceof Error
266+
? result.reason.message
267+
: "unknown error";
268+
failed.push(`${issue.kind} ${issue.path}: ${reason}`);
269+
}
270+
}
271+
272+
return { fixed, failed };
273+
}
274+
275+
/**
276+
* Diagnose ownership issues and optionally repair them.
277+
*
278+
* When the running process is root (`currentUid === 0`), we can perform chown
279+
* to transfer ownership back to the real user. The real username is inferred
280+
* from `SUDO_USER` / `USER` / `USERNAME` env vars (set by sudo).
281+
*
282+
* When not root, we print the exact `sudo chown` command the user must run.
283+
*
284+
* @param dbPath - Absolute path to the database file
285+
* @param currentUid - UID of the running process
286+
* @param dryRun - If true, report issues without repairing
287+
* @param output - Streams for user-facing output
288+
* @returns Count of issues found and whether any repairs failed
289+
*/
290+
async function handleOwnershipIssues(
291+
dbPath: string,
292+
currentUid: number,
293+
dryRun: boolean,
294+
{ stdout, stderr }: Output
295+
): Promise<DiagnoseResult> {
296+
const issues = await checkOwnership(dbPath, currentUid);
297+
if (issues.length === 0) {
298+
return { found: 0, repairFailed: false };
299+
}
300+
301+
stdout.write(`Found ${issues.length} ownership issue(s):\n`);
302+
for (const issue of issues) {
303+
stdout.write(
304+
` - ${issue.kind} ${issue.path}: owned by uid ${issue.ownerUid}\n`
305+
);
306+
}
307+
stdout.write("\n");
308+
309+
const configDir = getConfigDir();
310+
const username = getRealUsername();
311+
312+
if (dryRun) {
313+
stdout.write(
314+
printOwnershipInstructions(currentUid, username, configDir, true)
315+
);
316+
return { found: issues.length, repairFailed: false };
317+
}
318+
319+
if (currentUid !== 0) {
320+
// Not root — can't chown, print instructions.
321+
stderr.write(
322+
printOwnershipInstructions(currentUid, username, configDir, false)
323+
);
324+
return { found: issues.length, repairFailed: true };
325+
}
326+
327+
// Running as root (e.g. via sudo sentry cli fix) — perform chown.
328+
const targetUid = resolveUid(username);
329+
if (targetUid === null) {
330+
stderr.write(
331+
`Warning: Could not determine UID for user "${username}".\n` +
332+
"Run the following command manually:\n" +
333+
` chown -R ${username} "${configDir}"\n\n`
334+
);
335+
return { found: issues.length, repairFailed: true };
336+
}
337+
338+
stdout.write(`Transferring ownership to ${username} (uid ${targetUid})...\n`);
339+
const { fixed, failed } = await repairOwnership(issues, username, targetUid);
340+
for (const fix of fixed) {
341+
stdout.write(` + ${fix}\n`);
342+
}
343+
if (failed.length > 0) {
344+
stderr.write("\nSome ownership repairs failed:\n");
345+
for (const fail of failed) {
346+
stderr.write(` ! ${fail}\n`);
347+
}
348+
return { found: issues.length, repairFailed: true };
349+
}
350+
stdout.write("\n");
351+
return { found: issues.length, repairFailed: false };
352+
}
353+
354+
/**
355+
* Return the ownership fix instructions string.
356+
*
357+
* @param currentUid - UID of the running process
358+
* @param username - The real user's login name
359+
* @param configDir - The config directory path
360+
* @param dryRun - Whether this is a dry-run preview
361+
*/
362+
function printOwnershipInstructions(
363+
currentUid: number,
364+
username: string,
365+
configDir: string,
366+
dryRun: boolean
367+
): string {
368+
if (dryRun && currentUid === 0) {
369+
return `Would transfer ownership of "${configDir}" to ${username}.\n`;
370+
}
371+
return (
372+
"To fix ownership, run one of:\n\n" +
373+
` sudo chown -R ${username} "${configDir}"\n\n` +
374+
"Or let sentry fix it automatically:\n\n" +
375+
" sudo sentry cli fix\n\n"
376+
);
377+
}
378+
135379
/**
136380
* Format a permission mode as an octal string (e.g., "0644").
137381
*
@@ -315,18 +559,53 @@ function handleSchemaIssues(
315559
return { found: issues.length, repairFailed: failed.length > 0 };
316560
}
317561

562+
/**
563+
* Run schema diagnostics, guarding against DB open failures.
564+
*
565+
* The schema check opens the database, which can throw if the DB or config
566+
* directory is inaccessible. This wrapper catches those errors so `--dry-run`
567+
* can finish all diagnostics even when the filesystem is broken.
568+
*
569+
* @param priorIssuesFound - Total ownership+permission issues already found.
570+
* If non-zero, a schema open failure is expected and we stay quiet about it.
571+
*/
572+
function safeHandleSchemaIssues(
573+
dbPath: string,
574+
dryRun: boolean,
575+
out: Output,
576+
priorIssuesFound: number
577+
): DiagnoseResult {
578+
try {
579+
return handleSchemaIssues(dbPath, dryRun, out);
580+
} catch {
581+
if (priorIssuesFound === 0) {
582+
out.stderr.write("Could not open database to check schema.\n");
583+
out.stderr.write(
584+
`Try deleting the database and restarting: rm "${dbPath}"\n`
585+
);
586+
}
587+
return { found: 0, repairFailed: true };
588+
}
589+
}
590+
318591
export const fixCommand = buildCommand({
319592
docs: {
320593
brief: "Diagnose and repair CLI database issues",
321594
fullDescription:
322-
"Check the CLI's local SQLite database for schema and permission issues and repair them.\n\n" +
595+
"Check the CLI's local SQLite database for schema, permission, and ownership\n" +
596+
"issues and repair them.\n\n" +
323597
"This is useful when upgrading from older CLI versions, if the database\n" +
324598
"becomes inconsistent due to interrupted operations, or if file permissions\n" +
325599
"prevent the CLI from writing to its local database.\n\n" +
326600
"The command performs non-destructive repairs only - it adds missing tables\n" +
327-
"and columns, and fixes file permissions, but never deletes data.\n\n" +
601+
"and columns, fixes file permissions, and transfers ownership — but never\n" +
602+
"deletes data.\n\n" +
603+
"If files are owned by root (e.g. after `sudo brew install`), run with sudo\n" +
604+
"to transfer ownership back to the current user:\n\n" +
605+
" sudo sentry cli fix\n\n" +
328606
"Examples:\n" +
329607
" sentry cli fix # Fix database issues\n" +
608+
" sudo sentry cli fix # Fix root-owned files\n" +
330609
" sentry cli fix --dry-run # Show what would be fixed without making changes",
331610
},
332611
parameters: {
@@ -344,32 +623,38 @@ export const fixCommand = buildCommand({
344623
const dryRun = flags["dry-run"];
345624
const out = { stdout, stderr: this.stderr };
346625

626+
// process.getuid() is undefined on Windows
627+
const currentUid =
628+
typeof process.getuid === "function" ? process.getuid() : -1;
629+
347630
stdout.write(`Database: ${dbPath}\n`);
348631
stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`);
349632

350-
const perm = await handlePermissionIssues(dbPath, dryRun, out);
351-
352-
// Schema check opens the database, which can throw if the DB or config
353-
// directory is readonly. Guard with try/catch so --dry-run can finish
354-
// diagnostics even when the filesystem is broken.
355-
let schema: DiagnoseResult;
356-
try {
357-
schema = handleSchemaIssues(dbPath, dryRun, out);
358-
} catch {
359-
// If we already found permission issues, the schema check failure is
360-
// expected — don't obscure the permission report with an unrelated crash.
361-
// If no permission issues were found, this is unexpected so re-report it.
362-
if (perm.found === 0) {
363-
out.stderr.write("Could not open database to check schema.\n");
364-
out.stderr.write(
365-
`Try deleting the database and restarting: rm "${dbPath}"\n`
366-
);
367-
}
368-
schema = { found: 0, repairFailed: true };
369-
}
633+
// 1. Check ownership first — if files are root-owned, chmod will fail anyway.
634+
// On Windows (currentUid === -1), skip the ownership check entirely.
635+
const ownership: DiagnoseResult =
636+
currentUid >= 0
637+
? await handleOwnershipIssues(dbPath, currentUid, dryRun, out)
638+
: { found: 0, repairFailed: false };
639+
640+
// 2. Check permissions (skip if ownership issues already reported failures —
641+
// chmod will fail on root-owned files so the output would be misleading).
642+
const skipPerm = !dryRun && ownership.repairFailed;
643+
const perm: DiagnoseResult = skipPerm
644+
? { found: 0, repairFailed: false }
645+
: await handlePermissionIssues(dbPath, dryRun, out);
646+
647+
// 3. Schema check — guarded so filesystem errors don't hide earlier reports.
648+
const schema = safeHandleSchemaIssues(
649+
dbPath,
650+
dryRun,
651+
out,
652+
ownership.found + perm.found
653+
);
370654

371-
const totalFound = perm.found + schema.found;
372-
const anyFailed = perm.repairFailed || schema.repairFailed;
655+
const totalFound = ownership.found + perm.found + schema.found;
656+
const anyFailed =
657+
ownership.repairFailed || perm.repairFailed || schema.repairFailed;
373658

374659
if (totalFound === 0 && !anyFailed) {
375660
stdout.write(

0 commit comments

Comments
 (0)