Skip to content

Commit fd8a30e

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 30faf6f commit fd8a30e

File tree

3 files changed

+468
-73
lines changed

3 files changed

+468
-73
lines changed

src/commands/cli/fix.ts

Lines changed: 297 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
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 { chmod, chown, stat } from "node:fs/promises";
8+
import { userInfo } from "node:os";
89
import type { SentryContext } from "../../context.js";
910
import { buildCommand } from "../../lib/command.js";
1011
import { getConfigDir, getDbPath, getRawDatabase } from "../../lib/db/index.js";
@@ -43,6 +44,33 @@ type PermissionIssue = {
4344
expectedMode: number;
4445
};
4546

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

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

548+
/**
549+
* Run schema diagnostics, guarding against DB open failures.
550+
*
551+
* The schema check opens the database, which can throw if the DB or config
552+
* directory is inaccessible. This wrapper catches those errors so `--dry-run`
553+
* can finish all diagnostics even when the filesystem is broken.
554+
*
555+
* @param priorIssuesFound - Total ownership+permission issues already found.
556+
* If non-zero, a schema open failure is expected and we stay quiet about it.
557+
*/
558+
function safeHandleSchemaIssues(
559+
dbPath: string,
560+
dryRun: boolean,
561+
out: Output,
562+
priorIssuesFound: number
563+
): DiagnoseResult {
564+
try {
565+
return handleSchemaIssues(dbPath, dryRun, out);
566+
} catch {
567+
if (priorIssuesFound === 0) {
568+
out.stderr.write("Could not open database to check schema.\n");
569+
out.stderr.write(
570+
`Try deleting the database and restarting: rm "${dbPath}"\n`
571+
);
572+
}
573+
return { found: 0, repairFailed: true };
574+
}
575+
}
576+
318577
export const fixCommand = buildCommand({
319578
docs: {
320579
brief: "Diagnose and repair CLI database issues",
321580
fullDescription:
322-
"Check the CLI's local SQLite database for schema and permission issues and repair them.\n\n" +
581+
"Check the CLI's local SQLite database for schema, permission, and ownership\n" +
582+
"issues and repair them.\n\n" +
323583
"This is useful when upgrading from older CLI versions, if the database\n" +
324584
"becomes inconsistent due to interrupted operations, or if file permissions\n" +
325585
"prevent the CLI from writing to its local database.\n\n" +
326586
"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" +
587+
"and columns, fixes file permissions, and transfers ownership — but never\n" +
588+
"deletes data.\n\n" +
589+
"If files are owned by root (e.g. after `sudo brew install`), run with sudo\n" +
590+
"to transfer ownership back to the current user:\n\n" +
591+
" sudo sentry cli fix\n\n" +
328592
"Examples:\n" +
329593
" sentry cli fix # Fix database issues\n" +
594+
" sudo sentry cli fix # Fix root-owned files\n" +
330595
" sentry cli fix --dry-run # Show what would be fixed without making changes",
331596
},
332597
parameters: {
@@ -344,32 +609,38 @@ export const fixCommand = buildCommand({
344609
const dryRun = flags["dry-run"];
345610
const out = { stdout, stderr: this.stderr };
346611

612+
// process.getuid() is undefined on Windows
613+
const currentUid =
614+
typeof process.getuid === "function" ? process.getuid() : -1;
615+
347616
stdout.write(`Database: ${dbPath}\n`);
348617
stdout.write(`Expected schema version: ${CURRENT_SCHEMA_VERSION}\n\n`);
349618

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-
}
619+
// 1. Check ownership first — if files are root-owned, chmod will fail anyway.
620+
// On Windows (currentUid === -1), skip the ownership check entirely.
621+
const ownership: DiagnoseResult =
622+
currentUid >= 0
623+
? await handleOwnershipIssues(dbPath, currentUid, dryRun, out)
624+
: { found: 0, repairFailed: false };
625+
626+
// 2. Check permissions (skip if ownership issues already reported failures —
627+
// chmod will fail on root-owned files so the output would be misleading).
628+
const skipPerm = !dryRun && ownership.repairFailed;
629+
const perm: DiagnoseResult = skipPerm
630+
? { found: 0, repairFailed: false }
631+
: await handlePermissionIssues(dbPath, dryRun, out);
632+
633+
// 3. Schema check — guarded so filesystem errors don't hide earlier reports.
634+
const schema = safeHandleSchemaIssues(
635+
dbPath,
636+
dryRun,
637+
out,
638+
ownership.found + perm.found
639+
);
370640

371-
const totalFound = perm.found + schema.found;
372-
const anyFailed = perm.repairFailed || schema.repairFailed;
641+
const totalFound = ownership.found + perm.found + schema.found;
642+
const anyFailed =
643+
ownership.repairFailed || perm.repairFailed || schema.repairFailed;
373644

374645
if (totalFound === 0 && !anyFailed) {
375646
stdout.write(

0 commit comments

Comments
 (0)