diff --git a/src/main/ipc/repos/proven-absence.test.ts b/src/main/ipc/repos/proven-absence.test.ts index 157e3cb6522..10bfbcd9abf 100644 --- a/src/main/ipc/repos/proven-absence.test.ts +++ b/src/main/ipc/repos/proven-absence.test.ts @@ -43,4 +43,43 @@ describe('proven-absence', () => { expect(isNotADirectory(Object.assign(new Error('x'), { code: 'ENOTDIR' }))).toBe(true) expect(isNotADirectory(Object.assign(new Error('x'), { code: 'ENOENT' }))).toBe(false) }) + + it('does not throw when an Error instance has a throwing code getter', () => { + // R5: the old version delegated to isENOENT, which read `.code` a SECOND time unguarded. + const e = new Error('probe') + Object.defineProperty(e, 'code', { + get() { + throw new Error('boom') + } + }) + expect(() => isProvenAbsent(e)).not.toThrow() + expect(isProvenAbsent(e)).toBe(false) + expect(() => isNotADirectory(e)).not.toThrow() + }) + + it('recognises the relay ENOTDIR shape, where the string errno did not survive', () => { + expect( + isNotADirectory( + Object.assign(new Error("ENOTDIR: not a directory, stat '/x/.git'"), { code: -32000 }) + ) + ).toBe(true) + }) + + it('still consults the message when a wrapper attached a non-errno string code', () => { + expect( + isProvenAbsent( + Object.assign(new Error("ENOENT: no such file or directory, stat '/x'"), { + code: 'REMOTE_FS_ERROR' + }) + ) + ).toBe(true) + }) + + it('does not match an errno quoted later in the message', () => { + expect( + isProvenAbsent( + new Error("EACCES: denied, access '/a/ENOENT: no such file or directory/.git'") + ) + ).toBe(false) + }) }) diff --git a/src/main/ipc/repos/proven-absence.ts b/src/main/ipc/repos/proven-absence.ts index 99e431809b4..30e464743fe 100644 --- a/src/main/ipc/repos/proven-absence.ts +++ b/src/main/ipc/repos/proven-absence.ts @@ -1,44 +1,57 @@ -import { isENOENT } from '../filesystem-path-containment' +// Why anchored messages rather than `isENOENT`: over SSH the relay replaces a string errno with +// -32000 and forwards only the message (`relay/dispatcher-rpc-routing.ts`, +// `ssh/ssh-channel-multiplexer.ts`), so the message is the ONLY classification path remotely. +// Anchoring at the start stops a path that merely quotes an errno from matching. +const ENOENT_MESSAGE = /^ENOENT: no such file or directory\b/ +const ENOTDIR_MESSAGE = /^ENOTDIR: not a directory\b/ -/** - * Whether a failed existence probe proves the path is absent. - * - * Why not `isENOENT` alone: its message fallback is unanchored, so an `EACCES` whose *path* - * happens to contain "ENOENT: no such file or directory" would read as absent. A string `code` is - * a definitive errno and settles the question by itself. Only when there is no string code — the - * SSH relay replaces string errnos with a numeric one — do we fall back to the message. - * - * The message fallback is not a convenience: over SSH it is the ONLY path. The relay forwards a - * handler's message verbatim but replaces a string errno with -32000 - * (`relay/dispatcher-rpc-routing.ts`), and the multiplexer rebuilds the error from that pair - * (`ssh/ssh-channel-multiplexer.ts`). So every remote ENOENT is classified by its text. Do not - * "simplify" this to a code-only check — it would refuse every legitimate remote creation. - */ +// Why a vocabulary: only real errno names carry errno meaning. A wrapper attaching a domain string +// (`REMOTE_FS_ERROR`) while keeping the canonical message must not suppress the message fallback. +const AUTHORITATIVE_ERRNO = new Set(['ENOENT', 'ENOTDIR']) + +/** Whether a failed probe proves the path is absent. Never throws. */ export function isProvenAbsent(error: unknown): boolean { const code = readErrnoCode(error) - if (typeof code === 'string') { + if (code !== undefined && AUTHORITATIVE_ERRNO.has(code)) { return code === 'ENOENT' } - return isENOENT(error) + return ENOENT_MESSAGE.test(readMessage(error)) } /** - * Whether the probe proved the *parent* is not a directory — a definite answer about the target, + * Whether the probe proved the parent is not a directory — a definite answer about the target, * not an indeterminate probe, so callers should say so rather than "could not check". */ export function isNotADirectory(error: unknown): boolean { - return readErrnoCode(error) === 'ENOTDIR' + const code = readErrnoCode(error) + if (code !== undefined && AUTHORITATIVE_ERRNO.has(code)) { + return code === 'ENOTDIR' + } + return ENOTDIR_MESSAGE.test(readMessage(error)) } -// Why: `?.` only guards a nullish base — it still invokes an accessor, so an error-like rejection -// with a throwing `code` getter would escape a fail-closed path as an unhandled rejection. -function readErrnoCode(error: unknown): unknown { +// Why guarded: `?.` only protects a nullish base — it still invokes an accessor, so a throwing +// `code` getter would escape a fail-closed path as an unhandled rejection. +function readErrnoCode(error: unknown): string | undefined { if (typeof error !== 'object' || error === null) { return undefined } try { - return (error as { code?: unknown }).code + const code = (error as { code?: unknown }).code + return typeof code === 'string' ? code : undefined } catch { return undefined } } + +function readMessage(error: unknown): string { + if (typeof error !== 'object' || error === null) { + return '' + } + try { + const message = (error as { message?: unknown }).message + return typeof message === 'string' ? message : '' + } catch { + return '' + } +} diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 449659d7446..db2651c8270 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -23916,16 +23916,20 @@ export class OrcaRuntimeService { // Why: refuse an existing repository on positive evidence, before `git init` can silently // reinitialize it. A `.git` FILE counts — that is how linked worktrees and submodules point. const gitPath = join(targetPath, '.git') - let gitProbeFailure: string | null = null + let gitProbeFailed = false + let gitProbeFailure = '' const gitStat = await stat(gitPath).catch((error: unknown) => { // Why: an indeterminate probe must surface as the shared "cannot check" message, not as // the generic prepare-directory error the outer catch would produce. + // Why a flag, not a truthy string: an error with an empty message would otherwise read + // as "no failure" and fail open into `git init`. if (!isProvenAbsent(error)) { + gitProbeFailed = true gitProbeFailure = error instanceof Error ? error.message : String(error) } return null }) - if (gitProbeFailure) { + if (gitProbeFailed) { return { error: repositoryCheckUnavailableError(trimmedName, gitProbeFailure) } } if (gitStat) {