diff --git a/src/application/ClientRegistrationStatus.ts b/src/application/ClientRegistrationStatus.ts index b8c041105d..5c9e4e218f 100644 --- a/src/application/ClientRegistrationStatus.ts +++ b/src/application/ClientRegistrationStatus.ts @@ -6,7 +6,8 @@ import { effectiveClientServer, type ClientConfigurationDocument, } from "./ClientConfigurationDocument.js"; -import { access } from "node:fs/promises"; +import { lstat } from "node:fs/promises"; +import { err, ok, type Result } from "../domain/result.js"; import { isAbsolute } from "node:path"; import { z } from "zod"; @@ -146,7 +147,18 @@ export const readClientRegistrationInspections = async ( }); continue; } - if (!(await clientEvidencePresent(client))) continue; + const presence = await clientEvidencePresent(client); + if (!presence.ok) { + record({ + client: client.name, + config_path: presence.error.path, + command: [], + state: "invalid", + remediation: presence.error.detail, + }); + continue; + } + if (!presence.value) continue; const manualRemediation = manualRegistrationRemediation(client.name); if (client.format === "unsupported") { if (manualRemediation !== undefined) @@ -559,22 +571,28 @@ const unavailableStatus = ( remediation: remediation(client), }); -const clientEvidencePresent = async (client: SetupClient): Promise => { - for (const path of clientEvidencePaths(client)) - if (await exists(path)) return true; - return false; -}; - -const exists = async (path: string | undefined): Promise => { - if (path === undefined) return false; - try { - await access(path); - return true; - } catch (cause: unknown) { - // best-effort cleanup: optional registration probing; absence means unregistered. - void cause; - return false; +/** Entry presence is independent of whether its target can currently be read. */ +const clientEvidencePresent = async ( + client: SetupClient, +): Promise< + Result +> => { + let unavailable: + | { readonly path: string; readonly detail: string } + | undefined; + for (const path of clientEvidencePaths(client)) { + try { + await lstat(path); + return ok(true); + } catch (cause: unknown) { + if (!isMissing(cause)) + unavailable ??= { + path, + detail: `Client configuration evidence at ${path} could not be inspected: ${cause instanceof Error ? cause.message : String(cause)}. Check file permissions and rerun rea doctor.`, + }; + } } + return unavailable === undefined ? ok(false) : err(unavailable); }; const isMissing = (cause: unknown): boolean => diff --git a/tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts b/tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts index 425e0e610f..e5bba24ee2 100644 --- a/tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts +++ b/tests/acceptance/setup/clientConfigurationDiagnosticsCli.test.ts @@ -1,4 +1,11 @@ -import { mkdir, readFile, readdir, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + readFile, + readdir, + symlink, + writeFile, +} from "node:fs/promises"; import { dirname, join } from "node:path"; import { expect } from "vitest"; @@ -165,3 +172,56 @@ for (const scenario of cases) ]); }, ); + +cliTest.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "doctor reports an unreadable configuration symlink without changing it", + async ({ cli }) => { + const home = await createTestTempDirectory( + "rea-doctor-denied-registration-", + ); + const locked = join(home, "owned-denied"); + const target = join(locked, "client.json"); + await mkdir(locked); + const original = + '{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}\n'; + await writeFile(target, original); + const config = join(home, ".claude.json"); + await symlink(target, config); + await chmod(locked, 0); + try { + const result = await cli.run({ + arguments: [ + "doctor", + "--client", + "claude_code", + "--skill=false", + "--json", + ], + cwd: home, + environment: { + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: join(home, ".config"), + }, + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + healthy: false, + identity: { + registrations: [ + expect.objectContaining({ + client: "claude_code", + config_path: config, + state: "invalid", + command: [], + }), + ], + }, + }); + } finally { + await chmod(locked, 0o700); + } + expect(await readFile(target, "utf8")).toBe(original); + expect(await readdir(home)).toEqual([".claude.json", "owned-denied"]); + }, +); diff --git a/tests/boundary/filesystem/clientRegistrationStatus.test.ts b/tests/boundary/filesystem/clientRegistrationStatus.test.ts index c713851f5c..83c7fc1852 100644 --- a/tests/boundary/filesystem/clientRegistrationStatus.test.ts +++ b/tests/boundary/filesystem/clientRegistrationStatus.test.ts @@ -1,4 +1,4 @@ -import { mkdir, writeFile } from "node:fs/promises"; +import { chmod, mkdir, symlink, writeFile } from "node:fs/promises"; import { join, relative, resolve } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -7,6 +7,7 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js"; import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js"; +import { existingMaintenanceScope } from "../../../src/application/UpdateMaintenance.js"; import { PRODUCT_IDENTITY } from "../../../src/identity.js"; beforeEach(clearClientLocationEnvironment); @@ -439,3 +440,86 @@ describe("Node-wrapped registration policy", () => { }, ); }); + +it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports an inaccessible configuration target instead of omitting its registration", + async () => { + const home = await createTestTempDirectory( + "rea-registration-denied-target-", + ); + const locked = join(home, "owned-denied"); + await mkdir(locked); + const target = join(locked, "client.json"); + await writeFile( + target, + JSON.stringify({ + mcpServers: { rea: { command: "rea", args: ["mcp"] } }, + }), + ); + const path = join(home, ".claude.json"); + await symlink(target, path); + await chmod(locked, 0); + try { + const statuses = await readClientRegistrationStatuses( + home, + "/current/rea", + ); + expect( + statuses.find((status) => status.client === "claude_code"), + ).toMatchObject({ + config_path: path, + state: "invalid", + command: [], + remediation: expect.any(String), + }); + expect( + (await existingMaintenanceScope(home, "/current/rea", process.env)) + .clients, + ).not.toContain("claude_code"); + } finally { + await chmod(locked, 0o700); + } + }, +); + +it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports an unresolved configuration location when metadata lookup is denied", + async () => { + const home = await createTestTempDirectory( + "rea-registration-denied-metadata-", + ); + const locked = join(home, "owned-denied"); + const directory = join(locked, "claude"); + await mkdir(directory, { recursive: true }); + vi.stubEnv("CLAUDE_CONFIG_DIR", directory); + await chmod(locked, 0); + try { + const statuses = await readClientRegistrationStatuses( + home, + "/current/rea", + ); + expect( + statuses.find((status) => status.client === "claude_code"), + ).toMatchObject({ + state: "invalid", + command: [], + remediation: expect.stringContaining("EACCES"), + }); + } finally { + await chmod(locked, 0o700); + } + }, +); + +it("reports a dangling configuration entry as missing without losing the detected client", async () => { + const home = await createTestTempDirectory( + "rea-registration-dangling-config-", + ); + const path = join(home, ".claude.json"); + await symlink(join(home, "absent.json"), path); + expect( + (await readClientRegistrationStatuses(home)).find( + (status) => status.client === "claude_code", + ), + ).toMatchObject({ config_path: path, state: "missing", command: [] }); +});