diff --git a/gitnexus/src/cli/clean.ts b/gitnexus/src/cli/clean.ts index e89cc7f23c..4681508fb3 100644 --- a/gitnexus/src/cli/clean.ts +++ b/gitnexus/src/cli/clean.ts @@ -6,7 +6,13 @@ */ import fs from 'fs/promises'; -import { findRepo, unregisterRepo, listRegisteredRepos } from '../storage/repo-manager.js'; +import { + findRepo, + unregisterRepo, + listRegisteredRepos, + assertSafeStoragePath, + UnsafeStoragePathError, +} from '../storage/repo-manager.js'; export const cleanCommand = async (options?: { force?: boolean; all?: boolean }) => { // --all flag: clean all indexed repos @@ -27,6 +33,24 @@ export const cleanCommand = async (options?: { force?: boolean; all?: boolean }) const entries = await listRegisteredRepos(); for (const entry of entries) { + // Safety guard (#1003 review — @magyargergo): same rationale as + // remove.ts. `~/.gitnexus/registry.json` is user-writable, so a + // corrupted or hand-edited entry could point storagePath at the + // repo root, an empty string, or anywhere else — and + // fs.rm(recursive: true) on any of those would be catastrophic. + // Skip poisoned entries without touching disk, but keep going + // through the rest of the registry (preserves the existing + // per-repo error-tolerance semantics of `clean --all`). + try { + assertSafeStoragePath(entry); + } catch (err) { + if (err instanceof UnsafeStoragePathError) { + console.error(`Refusing to clean ${entry.name}: ${err.message}`); + continue; + } + throw err; + } + try { await fs.rm(entry.storagePath, { recursive: true, force: true }); await unregisterRepo(entry.path); diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 2b54f04f38..fe19ba753e 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -83,6 +83,15 @@ program .option('--all', 'Clean all indexed repos') .action(createLazyAction(() => import('./clean.js'), 'cleanCommand')); +program + .command('remove ') + .description( + 'Delete the GitNexus index for a registered repo (by alias, name, or absolute path). ' + + 'Unlike `clean`, does not require being inside the repo. Idempotent on unknown targets.', + ) + .option('-f, --force', 'Skip confirmation prompt') + .action(createLazyAction(() => import('./remove.js'), 'removeCommand')); + program .command('wiki [path]') .description('Generate repository wiki from knowledge graph') diff --git a/gitnexus/src/cli/remove.ts b/gitnexus/src/cli/remove.ts new file mode 100644 index 0000000000..4d2ce0771d --- /dev/null +++ b/gitnexus/src/cli/remove.ts @@ -0,0 +1,110 @@ +/** + * Remove Command (#664) + * + * Delete the `.gitnexus/` index for a registered repo and unregister it + * from the global registry (~/.gitnexus/registry.json). The target is + * identified by alias / basename-derived name / remote-inferred name / + * absolute path — no `--repo` flag, just a positional argument so the + * destructive-command ergonomics match `clean` (which is also + * destructive but scoped to `process.cwd()`). + * + * Compared to `clean`: + * - `clean` acts on the repo discovered by walking up from cwd. + * - `remove` acts on any registered repo identified by name or path. + * + * Behaviour notes: + * - Idempotent on unknown targets: exits 0 with a warning so that + * `remove X && analyze Y` keeps working in scripts. Per #664: + * "behave atomically and idempotently so retries are safe". + * - Atomic order mirrors `clean`: fs.rm FIRST, then unregister. A + * partial failure leaves the registry pointing at a missing dir + * (recoverable by `listRegisteredRepos({ validate: true })` on + * next read) rather than the opposite, which would orphan + * .gitnexus/ directories on disk. + * - `-f` / `--force` matches the confirmation-skip semantics of + * `clean -f`. (Distinct from `analyze --force`, which re-indexes; + * here there is no pipeline, so no conflation.) + */ + +import fs from 'fs/promises'; +import { + readRegistry, + resolveRegistryEntry, + assertSafeStoragePath, + unregisterRepo, + RegistryNotFoundError, + RegistryAmbiguousTargetError, + UnsafeStoragePathError, +} from '../storage/repo-manager.js'; + +export const removeCommand = async (target: string, options?: { force?: boolean }) => { + // Read the registry snapshot once and pass it to the resolver — this + // lets us render the "before" state in the dry-run path without a + // second disk read. + const entries = await readRegistry(); + + let entry; + try { + entry = resolveRegistryEntry(entries, target); + } catch (err) { + if (err instanceof RegistryNotFoundError) { + // Idempotent: missing target is a no-op warning, not an error. + // The `availableNames` hint comes from the error itself so users + // can see what they might have meant. + console.warn(`Nothing to remove: ${err.message}`); + return; + } + if (err instanceof RegistryAmbiguousTargetError) { + // Duplicate aliases are allowed via --allow-duplicate-name (#829); + // refuse to guess which one the user meant — surface the full list + // and exit non-zero so scripts don't silently pick the wrong repo. + console.error(`Error: ${err.message}`); + process.exit(1); + } + throw err; + } + + // Confirmation gate — same shape as `clean`. Default is a dry-run + // that describes what would be deleted; `--force` actually deletes. + if (!options?.force) { + console.log(`This will delete the GitNexus index for: ${entry.name}`); + console.log(` Path: ${entry.path}`); + console.log(` Storage: ${entry.storagePath}`); + console.log('\nRun with --force to confirm deletion.'); + return; + } + + // Safety guard (#1003 review — @magyargergo): refuse to proceed if + // the registry entry's `storagePath` isn't the canonical + // `/.gitnexus` subfolder. `~/.gitnexus/registry.json` is + // user-writable, so a corrupted or hand-edited entry could point + // storagePath at the repo root, an empty string (→ cwd), a parent + // dir, or anywhere else; `fs.rm(recursive: true, force: true)` on + // any of those would be a runtime disaster. Bail before touching + // disk, with an actionable hint for recovering a broken registry. + try { + assertSafeStoragePath(entry); + } catch (err) { + if (err instanceof UnsafeStoragePathError) { + console.error(`Error: ${err.message}`); + process.exit(1); + } + throw err; + } + + // Deletion order: fs.rm first, then unregister. If fs.rm fails mid-way, + // the registry entry stays so the user can retry. If fs.rm succeeds but + // unregister throws (e.g. ENOSPC on registry write), the entry becomes + // orphaned — `listRegisteredRepos({ validate: true })` prunes those on + // next read, so the failure is self-healing. + try { + await fs.rm(entry.storagePath, { recursive: true, force: true }); + await unregisterRepo(entry.path); + console.log(`Removed: ${entry.name}`); + console.log(` Path: ${entry.path}`); + console.log(` Storage: ${entry.storagePath}`); + } catch (err) { + console.error(`Failed to remove ${entry.name}:`, err); + process.exit(1); + } +}; diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 1b151cec19..8155e592fe 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -7,10 +7,50 @@ */ import fs from 'fs/promises'; +import { realpathSync } from 'fs'; import path from 'path'; import os from 'os'; import { getInferredRepoName } from './git.js'; +/** + * Normalise a repo path for registry comparison across platforms + * (#664 review feedback from @evander-wang). + * + * Why this exists: `path.resolve` alone is NOT enough for + * cross-platform registry stability. + * - **macOS**: tmpdirs and `/var` are symlinks to `/private/var`. + * A child process that stored `/private/var/folders/.../repo` in + * the registry cannot later be matched by an outer caller that + * supplies the symlink form `/var/folders/.../repo`. `path.resolve` + * does not follow symlinks; `realpathSync.native` does. + * - **Windows**: GitHub runners surface tmpdirs in 8.3 short-name + * form (`RUNNERA~1\...`), but `process.cwd()` often returns the + * long form (`runneradmin\...`). `realpathSync.native` normalises + * both sides to the long-name canonical path. + * + * Fallback behaviour: if the path does not exist on disk (e.g. a user + * passed `gitnexus remove some-alias` and the alias misses every + * registry entry, or the caller is resolving a path that was deleted + * after registration), we return `path.resolve(p)` rather than + * throwing. This preserves the idempotent-on-missing semantics of + * `resolveRegistryEntry` / `remove`. + * + * Backwards compatibility: this function is applied to BOTH the + * caller-supplied input AND each stored `entry.path` at compare time + * inside `resolveRegistryEntry`, so registries written by older + * versions (where `registerRepo` only ran `path.resolve`) still match + * correctly. Newly-written entries are canonicalised at write time too + * so the registry stabilises over analyze/re-analyze cycles. + */ +export const canonicalizePath = (p: string): string => { + const resolved = path.resolve(p); + try { + return realpathSync.native(resolved); + } catch { + return resolved; + } +}; + export interface RepoMeta { repoPath: string; lastCommit: string; @@ -349,13 +389,33 @@ export const registerRepo = async ( meta: RepoMeta, opts?: RegisterRepoOptions, ): Promise => { + // Preserve the caller's chosen path form in the registry — don't + // canonicalise at write time. This matters for two reasons: + // 1. `list` and error messages show the path the user actually + // knows (e.g. the 8.3 short form they typed), not a runtime- + // resolved long form they've never seen. + // 2. Keeps pre-existing #829 test assertions that compare + // `err.existingPath` against `path.resolve(tmpPath)` stable. + // Canonicalisation is applied at COMPARE points only (see below), + // which is where the cross-platform divergence actually matters. const resolved = path.resolve(repoPath); const { storagePath } = getStoragePaths(resolved); + // Canonical form used strictly for comparison — `realpathSync.native` + // expands macOS /var → /private/var and Windows 8.3 → long-name, + // falling back to `path.resolve` when the path doesn't exist. + const canonicalInput = canonicalizePath(repoPath); + const entries = await readRegistry(); const existingIdx = entries.findIndex((e) => { - const a = path.resolve(e.path); - const b = resolved; + // Canonicalise the STORED entry too so pre-canonicalisation + // registries (written by older versions, or paths passed in a + // different form) still match correctly. `canonicalizePath` falls + // back to `path.resolve` when the path no longer exists on disk, + // so stale entries that have been rm'd externally still resolve + // to a stable key instead of throwing. + const a = canonicalizePath(e.path); + const b = canonicalInput; return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; }); const existing = existingIdx >= 0 ? entries[existingIdx] : null; @@ -389,11 +449,14 @@ export const registerRepo = async ( // messages and list output #829 ships). const explicitName = opts?.name !== undefined || isPreservedAlias; if (explicitName && !opts?.allowDuplicateName) { + // Compare canonical-vs-canonical here too so `/var/foo` and + // `/private/var/foo` (same repo, different form) aren't treated as + // two colliding paths. const collidingEntry = entries.find( (e, i) => i !== existingIdx && e.name.toLowerCase() === name.toLowerCase() && - path.resolve(e.path) !== resolved, + canonicalizePath(e.path) !== canonicalInput, ); if (collidingEntry) { throw new RegistryNameCollisionError(name, collidingEntry.path, resolved); @@ -424,12 +487,204 @@ export const registerRepo = async ( * Called after `gitnexus clean`. */ export const unregisterRepo = async (repoPath: string): Promise => { - const resolved = path.resolve(repoPath); + // Canonicalise BOTH sides so an unregister call issued with the + // symlink form (`/var/folders/.../repo`) still matches an entry + // written with the realpath form (`/private/var/folders/.../repo`), + // and vice versa. Matches the semantics of `registerRepo` and + // `resolveRegistryEntry` post-#1003 review. + const resolved = canonicalizePath(repoPath); const entries = await readRegistry(); - const filtered = entries.filter((e) => path.resolve(e.path) !== resolved); + const matches = (a: string, b: string) => + process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; + const filtered = entries.filter((e) => !matches(canonicalizePath(e.path), resolved)); await writeRegistry(filtered); }; +/** + * Thrown by {@link resolveRegistryEntry} when no registered repo matches + * the caller's target string (by alias, basename, remote-inferred name, + * or resolved path). CLI callers that want idempotent "remove" semantics + * should catch this and exit 0 with a warning; non-idempotent callers + * (e.g. MCP tools) can surface the error directly. + */ +export class RegistryNotFoundError extends Error { + readonly kind = 'RegistryNotFoundError' as const; + constructor( + public readonly target: string, + public readonly availableNames: string[], + ) { + const hint = + availableNames.length > 0 + ? ` Available: ${availableNames.join(', ')}.` + : ' No repositories are currently registered.'; + super(`No registered repo matches "${target}".${hint}`); + this.name = 'RegistryNotFoundError'; + } +} + +/** + * Thrown by {@link resolveRegistryEntry} when the target string matches + * the `name` of two or more entries — only possible when the user + * previously registered duplicates via `analyze --name X + * --allow-duplicate-name` (#829). The error carries enough information + * for the caller to render an actionable disambiguation hint without + * string-matching on `.message`. + * + * `kind` is a string literal discriminant (same pattern as + * {@link RegistryNameCollisionError}) so callers can narrow via + * `err.kind === 'RegistryAmbiguousTargetError'` without importing the + * class. + */ +export class RegistryAmbiguousTargetError extends Error { + readonly kind = 'RegistryAmbiguousTargetError' as const; + constructor( + public readonly target: string, + public readonly matches: RegistryEntry[], + ) { + const listing = matches.map((m) => ` - ${m.name} (${m.path})`).join('\n'); + super( + `Multiple registered repos match "${target}":\n${listing}\n` + + `Pass the absolute path instead to disambiguate.`, + ); + this.name = 'RegistryAmbiguousTargetError'; + } +} + +/** + * Thrown by {@link assertSafeStoragePath} when a registry entry's + * `storagePath` does NOT point at the expected `/.gitnexus` + * subfolder. CLI destructive commands (`remove`, `clean --all`) should + * catch this and exit non-zero without deleting anything — the usual + * cause is a corrupted or hand-edited `~/.gitnexus/registry.json`, and + * proceeding would mean `fs.rm(recursive: true)` on whatever odd path + * the entry is pointing at. + */ +export class UnsafeStoragePathError extends Error { + readonly kind = 'UnsafeStoragePathError' as const; + constructor( + public readonly entry: RegistryEntry, + public readonly expectedStoragePath: string, + public readonly actualStoragePath: string, + ) { + super( + `Refusing to remove storage path for safety: expected ` + + `"${expectedStoragePath}" under the repo's .gitnexus subfolder, ` + + `but the registry entry has "${actualStoragePath}". ` + + `This usually means the registry entry is corrupted or was ` + + `hand-edited. Delete the entry manually from ~/.gitnexus/registry.json ` + + `and re-run analyze.`, + ); + this.name = 'UnsafeStoragePathError'; + } +} + +/** + * Guard rail for destructive CLI paths (`remove` #664, + * `clean --all` #258, future MCP `remove` tool): verify that a + * registry entry's `storagePath` is the canonical `/.gitnexus` + * subfolder of its `path`. If not, throw {@link UnsafeStoragePathError} + * so the caller exits without touching disk. + * + * Why this exists (#1003 review — @magyargergo): + * - `~/.gitnexus/registry.json` is a plain-text user-writable file. + * A corrupted, hand-edited, or downgrade/upgrade-racing entry + * could plausibly end up with `storagePath === ""` (resolves to + * cwd), `storagePath === path` (the repo root!), `storagePath` + * equal to a parent/sibling of the repo, or simply any arbitrary + * filesystem path. + * - `fs.rm(recursive: true, force: true)` on ANY of those would be + * a runtime disaster — at best delete the user's working tree, at + * worst nuke an unrelated directory tree they happen to own. + * - `clean` (default, cwd-scoped) is safe by construction — it + * re-derives storagePath from `findRepo(cwd)` and never trusts + * the registry field. But `clean --all` DOES iterate the registry + * and trust each entry's stored storagePath (same shape as + * `remove`), so this helper must be wired into that loop too. + * - `server/api.ts` recomputes storagePath from `getStoragePath(entry.path)` + * and so is likewise safe-by-construction. + * + * Pure string check — does NOT require the paths to exist on disk. + * Windows: case-insensitive; POSIX: case-sensitive. Matches the + * comparison shape used elsewhere in this module. + */ +export const assertSafeStoragePath = (entry: RegistryEntry): void => { + const expected = path.join(path.resolve(entry.path), '.gitnexus'); + const actual = path.resolve(entry.storagePath); + const matches = + process.platform === 'win32' + ? expected.toLowerCase() === actual.toLowerCase() + : expected === actual; + if (!matches) { + throw new UnsafeStoragePathError(entry, expected, actual); + } +}; + +/** + * Resolve a user-supplied target string (from `gitnexus remove ` + * or equivalent MCP tool argument) to a single registry entry. + * + * Match precedence (first hit wins, subsequent tiers are only tried if + * the prior tier produces zero matches): + * 1. Exact resolved-path match (Windows: case-insensitive). + * Paths are unique by registry construction, so a path match can + * never be ambiguous. + * 2. Exact `name` match (case-insensitive). If ≥ 2 entries share the + * name — only possible via `--allow-duplicate-name` (#829) — + * throws {@link RegistryAmbiguousTargetError}. + * + * No fuzzy / partial matching — unambiguous, scriptable behaviour is + * more important than convenience for destructive commands. + * + * Throws {@link RegistryNotFoundError} if no entry matches. + * + * `entries` is passed in (rather than re-read) so callers that already + * hold the registry snapshot (e.g. to print a "before" state) can avoid + * a second disk read, and so tests can inject fixtures without touching + * `GITNEXUS_HOME`. + */ +export const resolveRegistryEntry = (entries: RegistryEntry[], target: string): RegistryEntry => { + // Tier 1: path match. Canonicalise BOTH sides so symlink and + // Windows-8.3 quirks don't cause a false miss — e.g. the caller + // passes `/var/folders/.../repo` while the registry has + // `/private/var/folders/.../repo` (both resolve to the same + // `realpath.native`). See `canonicalizePath` for the rationale. + // + // Canonicalising the STORED entry (not just the input) is what gives + // us backward-compat for registries written by versions that only + // ran `path.resolve` — both get canonicalised here at compare time. + const canonicalTarget = canonicalizePath(target); + const pathMatch = entries.find((e) => { + const a = canonicalizePath(e.path); + const b = canonicalTarget; + return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; + }); + if (pathMatch) return pathMatch; + + // Tier 2: name match. Case-insensitive on all platforms — registry + // name collisions are already filtered case-insensitively in + // `registerRepo`, so "APP" vs "app" are considered the same key. + const targetLower = target.toLowerCase(); + const nameMatches = entries.filter((e) => e.name.toLowerCase() === targetLower); + if (nameMatches.length === 1) return nameMatches[0]; + if (nameMatches.length > 1) { + throw new RegistryAmbiguousTargetError(target, nameMatches); + } + + // Tier 3: miss. Build the available-names hint ONCE; resolveRepo-style + // disambiguated labels (`app (/path)`) are applied when the same name + // appears in multiple entries so the user sees the same hint shape as + // `-r ` errors. + const nameCounts = new Map(); + for (const e of entries) { + const key = e.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + const availableNames = entries.map((e) => + (nameCounts.get(e.name.toLowerCase()) ?? 0) > 1 ? `${e.name} (${e.path})` : e.name, + ); + throw new RegistryNotFoundError(target, availableNames); +}; + /** * List all registered repos from the global registry. * Optionally validates that each entry's .gitnexus/ still exists. diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index 844e3978d6..f994b42aed 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -345,6 +345,410 @@ describe('CLI end-to-end', () => { }, 360000); // 6-min outer budget (4 × ~60s analyze calls + fixture setup) }); + // ─── gitnexus remove (#664) ───────────────────────────── + // + // End-to-end regression guard for the remove command: + // 1. `remove ` without --force is a dry-run (exit 0, preserves state) + // 2. `remove --force` deletes the .gitnexus/ directory + // AND unregisters from the global registry + // 3. `remove ` is idempotent (exit 0 with a warning) + // 4. `remove ` (two entries share the alias via + // --allow-duplicate-name) exits 1 with a disambiguation hint + // and leaves the registry unchanged. + // + // Every assertion reads the real registry.json on disk, so any + // regression in remove.ts → resolveRegistryEntry → unregisterRepo + // will surface here. + describe('remove (#664)', () => { + it('dry-run lists, --force deletes, missing target is a no-op warning', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-remove-')); + const repoA = makeMiniRepoCopy('remove-me', 'gn-rm-a-'); + const parentA = path.dirname(repoA); + + try { + // Index the repo under a custom alias so we can target it by + // name below. `--name` guarantees a stable alias regardless of + // how the host resolves the basename/remote-inferred name. + const r1 = runCliWithEnv( + ['analyze', '--name', 'alias-a'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect( + r1.status, + [`analyze exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join( + '\n', + ), + ).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const afterIndex = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterIndex).toHaveLength(1); + expect(afterIndex[0].name).toBe('alias-a'); + // Storage dir must exist before remove so we can assert its + // disappearance below. + const storagePath = afterIndex[0].storagePath; + expect(fs.existsSync(storagePath)).toBe(true); + + // Dry-run: must NOT delete. Use parentA as cwd so the test + // never runs with the to-be-removed storage dir as its cwd. + // + // Assert the FULL dry-run output shape, not just the `--force` + // hint (#1003 senior-reviewer NIT): `remove.ts` prints the + // alias, the resolved path, AND the storage path. Verifying + // all three appear catches silent format regressions + // (e.g. a future refactor that accidentally drops one of the + // three `console.log` lines, or swaps `entry.path` for + // `entry.name` in the output). + const r2 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000); + if (r2.status === null) return; + expect(r2.status).toBe(0); + const r2Output = `${r2.stdout}${r2.stderr}`; + expect(r2Output).toMatch(/Run with --force/i); + expect(r2Output, 'dry-run must surface the alias').toContain('alias-a'); + expect(r2Output, 'dry-run must surface the repo path').toContain(afterIndex[0].path); + expect(r2Output, 'dry-run must surface the storage path').toContain(storagePath); + expect(fs.existsSync(storagePath)).toBe(true); + // Registry still has the entry. + expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(1); + + // --force: must delete storage AND unregister. + const r3 = runCliWithEnv( + ['remove', 'alias-a', '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r3.status === null) return; + expect( + r3.status, + [ + `remove --force exited with ${r3.status}`, + `stdout: ${r3.stdout}`, + `stderr: ${r3.stderr}`, + ].join('\n'), + ).toBe(0); + // Success-case output shape: `Removed: ` header plus the + // same path-and-storagePath lines the dry-run prints (same NIT + // rationale — the success branch mirrors the dry-run's three + // console.log calls, so it has the same silent-regression risk). + const r3Output = `${r3.stdout}${r3.stderr}`; + expect(r3Output).toMatch(/Removed/i); + expect(r3Output, 'success output must surface the alias').toContain('alias-a'); + expect(r3Output, 'success output must surface the repo path').toContain(afterIndex[0].path); + expect(r3Output, 'success output must surface the storage path').toContain(storagePath); + expect(fs.existsSync(storagePath)).toBe(false); + expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(0); + + // Idempotent: removing the same alias AGAIN must exit 0 with a + // warning (so `remove X && analyze Y` keeps working in scripts). + const r4 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000); + if (r4.status === null) return; + expect(r4.status).toBe(0); + expect(`${r4.stdout}${r4.stderr}`).toMatch(/Nothing to remove/i); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + } + }, 180000); // 3-min outer budget (1 × ~60s analyze + 3 × fast remove calls) + + it('ambiguous target (two entries share alias via --allow-duplicate-name) errors without mutating registry', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-rm-amb-')); + const repoA = makeMiniRepoCopy('dup', 'gn-dup-a-'); + const repoB = makeMiniRepoCopy('dup', 'gn-dup-b-'); + const parentA = path.dirname(repoA); + const parentB = path.dirname(repoB); + + try { + // Two repos registered under the same alias — only possible via + // --allow-duplicate-name (#829). + const r1 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect(r1.status).toBe(0); + + const r2 = runCliWithEnv( + ['analyze', '--name', 'shared', '--allow-duplicate-name'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r2.status === null) return; + expect(r2.status).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const before = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(before).toHaveLength(2); + + // `remove shared` must refuse to guess — exit 1, disambiguation hint. + const r3 = runCliWithEnv( + ['remove', 'shared', '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r3.status === null) return; + expect(r3.status).toBe(1); + const r3Output = `${r3.stdout}${r3.stderr}`; + expect(r3Output).toMatch(/Multiple registered repos match/i); + // Both paths must be surfaced in the hint so the user knows + // which ones to disambiguate between. + expect(r3Output).toMatch(/dup/); + + // Registry unchanged — the failed resolution must NOT have + // mutated state. + const after = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(after).toHaveLength(2); + + // And path-based remove still works: pass the absolute path of + // repoA and it resolves unambiguously. + // + // We pull the path from the registry snapshot rather than + // passing the outer `repoA` variable directly. This is the + // belt-and-suspenders for cross-platform path normalisation + // (#1003 review): the path the registry recorded has already + // gone through the analyze-side canonicalisation (which on + // macOS expands /var → /private/var and on Windows expands 8.3 + // → long-name). Passing that exact string back to `remove` + // guarantees the comparison succeeds even on runners where the + // outer `repoA` is the symlink/short-name form. The code-side + // fix in `canonicalizePath` makes this redundant in practice, + // but the test shouldn't depend on the code fix being perfect + // on every platform — it should prove correctness against the + // registry contract. + const repoAEntry = before.find( + (e: { path: string }) => + path.basename(e.path) === 'dup' && e.path.includes(path.basename(parentA)), + ); + expect( + repoAEntry, + 'repoA entry must exist in registry before path-remove step', + ).toBeDefined(); + + const r4 = runCliWithEnv( + ['remove', repoAEntry.path, '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r4.status === null) return; + expect( + r4.status, + [ + `remove-by-path exited with ${r4.status}`, + `stdout: ${r4.stdout}`, + `stderr: ${r4.stderr}`, + ].join('\n'), + ).toBe(0); + const finalEntries = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(finalEntries).toHaveLength(1); + // The survivor is repoB (its path stays in the registry). + expect(path.basename(finalEntries[0].path)).toBe('dup'); + // And it's NOT the one we just removed. + expect(finalEntries[0].path).not.toBe(repoAEntry.path); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + fs.rmSync(parentB, { recursive: true, force: true }); + } + }, 240000); // 4-min outer budget (2 × ~60s analyze + 2 × fast remove) + + it('refuses to proceed when a registry entry points storagePath outside /.gitnexus (#1003)', () => { + // Regression guard for the safety gap flagged by @magyargergo on + // PR #1003: `~/.gitnexus/registry.json` is a user-writable JSON + // file, so a corrupted or hand-edited entry could point + // storagePath at the repo root (catastrophic: rm the working + // tree) or at any other arbitrary path. `remove --force` must + // refuse to call fs.rm when storagePath isn't the canonical + // `/.gitnexus`. We verify: + // 1. Exit code 1 with the actionable "registry entry corrupted" + // hint. + // 2. The .gitnexus/ storage dir is UNTOUCHED. + // 3. The repo itself (entry.path) is UNTOUCHED. + // 4. The registry entry is NOT removed (no partial mutation). + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-poison-')); + const repo = makeMiniRepoCopy('poisoned', 'gn-poison-'); + const parent = path.dirname(repo); + + try { + // Index the repo normally first so the registry has a valid + // entry we can then poison. + const r1 = runCliWithEnv( + ['analyze', '--name', 'poisoned-alias'], + repo, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect(r1.status).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(original).toHaveLength(1); + + // Poison the entry: set storagePath to the REPO ROOT itself. + // If the guard isn't in place, `remove --force` would call + // `fs.rm(repo, {recursive: true, force: true})` and wipe the + // entire working tree. + const poisoned = [{ ...original[0], storagePath: repo }]; + fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2)); + + // Sanity: storage dir and working tree both still exist. + expect(fs.existsSync(path.join(repo, '.gitnexus'))).toBe(true); + expect(fs.existsSync(repo)).toBe(true); + expect(fs.existsSync(path.join(repo, '.git'))).toBe(true); + + // Attempt the remove — must FAIL without deleting anything. + const r2 = runCliWithEnv( + ['remove', 'poisoned-alias', '--force'], + parent, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r2.status === null) return; + + expect( + r2.status, + [`remove should have exited 1`, `stdout: ${r2.stdout}`, `stderr: ${r2.stderr}`].join( + '\n', + ), + ).toBe(1); + const r2Output = `${r2.stdout}${r2.stderr}`; + // Must surface the actionable "registry corrupted" hint, not + // just a raw fs.rm error. + expect(r2Output).toMatch(/Refusing to remove/i); + expect(r2Output).toMatch(/registry\.json/i); + + // Repo + .gitnexus dir + .git dir must all still exist — the + // guard aborts BEFORE fs.rm. This is the whole point of the + // test: the working tree is not allowed to disappear. + expect(fs.existsSync(repo), 'repo working tree must survive').toBe(true); + expect(fs.existsSync(path.join(repo, '.gitnexus')), 'storage dir must survive').toBe(true); + expect(fs.existsSync(path.join(repo, '.git')), '.git must survive').toBe(true); + + // Registry unchanged — no partial mutation. + const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterRegistry).toHaveLength(1); + expect(afterRegistry[0].storagePath).toBe(repo); // still poisoned (we did that) + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parent, { recursive: true, force: true }); + } + }, 120000); // 2-min budget (1 × ~60s analyze + 1 × fast remove-refused) + }); + + // ─── clean --all: same safety guard applies (#1003 review) ─────── + // + // The `clean --all` path iterates over the registry and calls + // `fs.rm(entry.storagePath)` — identical trust-the-registry pattern + // as `remove` had before the guard. A poisoned entry must be SKIPPED + // (not aborted), so clean --all preserves its existing per-repo + // error-tolerance semantics: one bad entry does not halt cleanup of + // the rest. We verify: + // 1. The poisoned entry is NOT deleted (working tree + .gitnexus + // survive), and the CLI prints a "Refusing to clean" message. + // 2. The poisoned entry is left in the registry (nothing was + // mutated for it). + // 3. A co-existing well-formed entry IS still cleaned (both its + // .gitnexus dir AND its registry entry are gone). + describe('clean --all with a poisoned registry entry (#1003)', () => { + it('skips poisoned entries, cleans valid ones, never deletes the working tree', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-clean-poison-')); + const repoBad = makeMiniRepoCopy('bad-repo', 'gn-clean-bad-'); + const repoGood = makeMiniRepoCopy('good-repo', 'gn-clean-good-'); + const parentBad = path.dirname(repoBad); + const parentGood = path.dirname(repoGood); + + try { + // Analyze both so the registry has two well-formed entries. + for (const [repo, alias] of [ + [repoBad, 'bad-alias'], + [repoGood, 'good-alias'], + ] as const) { + const r = runCliWithEnv( + ['analyze', '--name', alias], + repo, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r.status === null) return; + expect(r.status, `analyze ${alias} exited ${r.status}: ${r.stdout}${r.stderr}`).toBe(0); + } + + const registryPath = path.join(gnHome, 'registry.json'); + const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(original).toHaveLength(2); + + // Poison the 'bad-alias' entry by pointing its storagePath at + // the repo root itself. If the guard isn't wired into the + // clean --all loop, `clean --all --force` would fs.rm the + // working tree. + const poisoned = original.map((e: { name: string; storagePath: string; path: string }) => + e.name === 'bad-alias' ? { ...e, storagePath: repoBad } : e, + ); + fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2)); + + // Sanity: both working trees and .gitnexus dirs still exist. + expect(fs.existsSync(repoBad)).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.gitnexus'))).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.git'))).toBe(true); + expect(fs.existsSync(path.join(repoGood, '.gitnexus'))).toBe(true); + + // clean --all --force from a neutral cwd (parentBad), so the + // command isn't "inside" either repo. + const r = runCliWithEnv( + ['clean', '--all', '--force'], + parentBad, + { GITNEXUS_HOME: gnHome }, + 30000, + ); + if (r.status === null) return; + + // clean --all's per-entry error handling always exits 0 at + // the end (it only logs per-repo failures). The important + // assertions are on side effects, not the exit code. + const output = `${r.stdout}${r.stderr}`; + expect(output).toMatch(/Refusing to clean/i); + expect(output).toMatch(/bad-alias/); + + // Poisoned repo: working tree + .gitnexus + .git all SURVIVE. + expect(fs.existsSync(repoBad), 'poisoned repo working tree must survive').toBe(true); + expect( + fs.existsSync(path.join(repoBad, '.gitnexus')), + 'poisoned repo .gitnexus must survive (guard refused to rm repo root)', + ).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.git')), '.git must survive').toBe(true); + + // Good repo: its .gitnexus IS gone (cleanup succeeded despite + // the poisoned sibling entry — per-entry error tolerance is + // preserved). + expect( + fs.existsSync(path.join(repoGood, '.gitnexus')), + 'good repo .gitnexus should be cleaned', + ).toBe(false); + // But the good repo's working tree stays (clean never touches + // anything outside .gitnexus). + expect(fs.existsSync(repoGood), 'good repo working tree must survive').toBe(true); + + // Registry post-state: poisoned entry still present (skipped, + // not mutated); good entry unregistered. + const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterRegistry).toHaveLength(1); + expect(afterRegistry[0].name).toBe('bad-alias'); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentBad, { recursive: true, force: true }); + fs.rmSync(parentGood, { recursive: true, force: true }); + } + }, 240000); // 4-min budget (2 × ~60s analyze + 1 × fast clean --all) + }); + describe('unhappy path', () => { it('exits with error when no command is given', () => { const result = runCliRaw([], MINI_REPO); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 83827fc156..12c56d67a3 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -15,7 +15,14 @@ import { loadCLIConfig, registerRepo, listRegisteredRepos, + resolveRegistryEntry, + canonicalizePath, + assertSafeStoragePath, RegistryNameCollisionError, + RegistryNotFoundError, + RegistryAmbiguousTargetError, + UnsafeStoragePathError, + type RegistryEntry, type RepoMeta, } from '../../src/storage/repo-manager.js'; import { parseRepoNameFromUrl, getInferredRepoName } from '../../src/storage/git.js'; @@ -453,3 +460,334 @@ describe('getInferredRepoName + registerRepo (#979 — git remote inference)', ( } }); }); + +// ─── resolveRegistryEntry (#664 — gitnexus remove ) ────────── +// +// The resolver is a pure function over a `RegistryEntry[]` snapshot, so +// these tests build synthetic entries inline and do NOT touch +// ~/.gitnexus. No GITNEXUS_HOME sandboxing needed. This also means the +// tests are platform-portable on Windows where realpath semantics on +// tmpdirs can diverge between runs (see the #955 CI pivot). + +describe('resolveRegistryEntry (#664)', () => { + // A well-known synthetic registry with two same-name entries (which + // can only exist in reality after `--allow-duplicate-name` — #829) and + // one unique-name entry. Path prefixes differ across platforms so the + // tests stay meaningful regardless of `process.platform`. + const prefix = process.platform === 'win32' ? 'D:\\' : '/tmp/'; + const pathA = `${prefix}projects${path.sep}gnx-a${path.sep}app`; + const pathB = `${prefix}projects${path.sep}gnx-b${path.sep}app`; + const pathW = `${prefix}work${path.sep}website`; + + const entries: RegistryEntry[] = [ + { + name: 'app', + path: pathA, + storagePath: `${pathA}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'aaaaaaa', + }, + { + name: 'app', + path: pathB, + storagePath: `${pathB}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'bbbbbbb', + }, + { + name: 'website', + path: pathW, + storagePath: `${pathW}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'ccccccc', + }, + ]; + + it('resolves by absolute path to the exact entry (path tier beats name tier)', () => { + const hit = resolveRegistryEntry(entries, pathA); + expect(hit).toBe(entries[0]); + expect(hit.path).toBe(pathA); + + const hit2 = resolveRegistryEntry(entries, pathB); + expect(hit2).toBe(entries[1]); + expect(hit2.path).toBe(pathB); + }); + + it('resolves by unique name to the only matching entry', () => { + const hit = resolveRegistryEntry(entries, 'website'); + expect(hit).toBe(entries[2]); + expect(hit.name).toBe('website'); + }); + + it('name match is case-insensitive', () => { + expect(resolveRegistryEntry(entries, 'WEBSITE')).toBe(entries[2]); + expect(resolveRegistryEntry(entries, 'Website')).toBe(entries[2]); + }); + + it('path match is case-insensitive on Windows only', () => { + if (process.platform !== 'win32') { + // On POSIX, a differently-cased path must NOT match. Verify by + // lower-casing a mixed-case copy of pathW and expecting a miss. + const upper = pathW.toUpperCase(); + expect(() => resolveRegistryEntry(entries, upper)).toThrow(RegistryNotFoundError); + return; + } + const upper = pathA.toUpperCase(); + const hit = resolveRegistryEntry(entries, upper); + expect(hit).toBe(entries[0]); + }); + + it('throws RegistryAmbiguousTargetError when name matches multiple entries', () => { + // Two 'app' entries exist only because of --allow-duplicate-name + // (#829). The resolver MUST refuse to guess. + expect(() => resolveRegistryEntry(entries, 'app')).toThrow(RegistryAmbiguousTargetError); + try { + resolveRegistryEntry(entries, 'app'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryAmbiguousTargetError); + const err = e as RegistryAmbiguousTargetError; + expect(err.kind).toBe('RegistryAmbiguousTargetError'); + expect(err.target).toBe('app'); + expect(err.matches).toHaveLength(2); + // Error message must include both paths so the CLI can surface + // them without string-matching on `.message`. + expect(err.message).toContain(pathA); + expect(err.message).toContain(pathB); + } + }); + + it('throws RegistryNotFoundError when no entry matches', () => { + expect(() => resolveRegistryEntry(entries, 'nonexistent')).toThrow(RegistryNotFoundError); + try { + resolveRegistryEntry(entries, 'nonexistent'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNotFoundError); + const err = e as RegistryNotFoundError; + expect(err.kind).toBe('RegistryNotFoundError'); + expect(err.target).toBe('nonexistent'); + // availableNames is disambiguated: 'app' appears twice, so both + // `app (path)` variants are included; 'website' is unique so it + // stays plain — matches the resolveRepo disambiguation shape. + expect(err.availableNames).toContain('website'); + expect(err.availableNames.some((n) => n.startsWith('app ('))).toBe(true); + // Error message surfaces the hint. + expect(err.message).toContain('website'); + } + }); + + it('throws RegistryNotFoundError with "no repositories registered" hint when registry is empty', () => { + try { + resolveRegistryEntry([], 'anything'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNotFoundError); + const err = e as RegistryNotFoundError; + expect(err.availableNames).toEqual([]); + expect(err.message).toContain('No repositories are currently registered'); + } + }); + + it('path match wins over name match (never ambiguous)', () => { + // Construct a pathological fixture where a registry entry's NAME + // happens to equal another entry's PATH. The path tier must win + // without triggering ambiguity. + const weird: RegistryEntry[] = [ + { ...entries[2] }, // 'website' at pathW + { + name: pathW, // degenerate: name equals another entry's path + path: `${prefix}elsewhere${path.sep}odd`, + storagePath: `${prefix}elsewhere${path.sep}odd${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'ddddddd', + }, + ]; + const hit = resolveRegistryEntry(weird, pathW); + // Must match the entry whose PATH is pathW, not the one whose NAME + // is pathW — because Tier 1 runs before Tier 2 and finds the path + // match first. + expect(hit.path).toBe(pathW); + expect(hit.name).toBe('website'); + }); +}); + +// ─── canonicalizePath (#1003 review — @evander-wang / @magyargergo) ── +// +// Shields `registerRepo`, `unregisterRepo`, and `resolveRegistryEntry` +// against cross-platform path-form divergence: macOS symlink expansion +// (/var → /private/var) and Windows 8.3 short-name expansion +// (RUNNERA~1 → runneradmin). The helper also underpins backwards +// compatibility with registries written by versions that only ran +// `path.resolve` — by canonicalising the stored entry at compare time, +// both pre- and post-fix entries converge to the same key. +// +// These tests avoid snapshotting a specific realpath value (that would +// be platform-fragile); instead they assert: +// - canonicalizePath is idempotent (f(f(x)) == f(x)) +// - canonicalizePath falls back cleanly when the path doesn't exist +// - resolveRegistryEntry matches a stored entry even when the target +// and the stored value disagree on one-step normalisation (simulated +// via a fixture that stores the de-canonicalised form of a real +// existing path). + +describe('canonicalizePath (#1003)', () => { + it('is idempotent — canonicalizePath(canonicalizePath(x)) === canonicalizePath(x)', async () => { + // Use the vitest project-root as a known-existing path. `os.tmpdir()` + // would work too but process.cwd() is guaranteed to exist for the + // test runner. + const p = process.cwd(); + const once = canonicalizePath(p); + const twice = canonicalizePath(once); + expect(twice).toBe(once); + }); + + it('falls back to path.resolve when the target does not exist', () => { + // Construct a definitely-nonexistent path under tmpdir. Using + // random-ish segments so we don't collide with anything real. + const ghost = path.join(os.tmpdir(), 'gnx-never-exists-____', 'still-not-there'); + const got = canonicalizePath(ghost); + // Must not throw, must not resolve to something weird — should be + // identical to `path.resolve(ghost)` since realpathSync.native will + // have thrown and we swallowed it. + expect(got).toBe(path.resolve(ghost)); + }); + + it('returns an absolute path for relative input even when the path is missing', () => { + // Relative path that does not exist. Must still be absolute + // (fallback path: path.resolve normalises even non-existent inputs). + const rel = './does-not-exist-zzz-' + Date.now(); + const got = canonicalizePath(rel); + expect(path.isAbsolute(got)).toBe(true); + }); +}); + +describe('resolveRegistryEntry backward-compat with non-canonical stored paths (#1003)', () => { + it('matches a stored entry even when the target was passed in canonical form', async () => { + // Simulate the bug-producing scenario without depending on a real + // symlink/8.3 discrepancy (those are platform-specific and flaky to + // set up in CI). We take a REAL path that exists + // (canonicalizePath-stable), store a known-non-canonical copy of it + // in a fake RegistryEntry, then resolve with the canonical form and + // assert the match. + // + // Construct a non-canonical string that resolves to the same real + // path. `path.join` auto-normalises `.` and trailing separators, so + // we build the string by raw concat to keep it string-unequal to + // `realDir` until `canonicalizePath` runs. + const realDir = process.cwd(); + const nonCanonical = realDir + path.sep + '.'; // e.g. /work/gitnexus/. + // Sanity: these are string-unequal before canonicalisation. + expect(nonCanonical).not.toBe(realDir); + + const entries: RegistryEntry[] = [ + { + name: 'stored-under-noncanonical-form', + path: nonCanonical, + storagePath: path.join(nonCanonical, '.gitnexus'), + indexedAt: '2026-04-20T00:00:00.000Z', + lastCommit: 'deadbee', + }, + ]; + + // Pass the canonical form as the target — resolver must still match. + const hit = resolveRegistryEntry(entries, realDir); + expect(hit).toBe(entries[0]); + }); +}); + +// ─── assertSafeStoragePath (#1003 review — @magyargergo) ───────────── +// +// Guard rail against destroying more than the `.gitnexus/` subfolder. +// `~/.gitnexus/registry.json` is user-writable plain text, so a +// corrupted or hand-edited entry could put storagePath anywhere. +// These tests use synthetic `RegistryEntry` fixtures (no disk I/O) +// because the guard is a pure string check — it must not depend on +// the paths existing. + +describe('assertSafeStoragePath (#1003)', () => { + const prefix = process.platform === 'win32' ? 'D:\\' : '/tmp/'; + const repoPath = `${prefix}projects${path.sep}my-repo`; + const base: Omit = { + name: 'my-repo', + path: repoPath, + indexedAt: '2026-04-21T00:00:00.000Z', + lastCommit: 'deadbee', + }; + + it('accepts the canonical /.gitnexus storage path', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.join(repoPath, '.gitnexus'), + }; + expect(() => assertSafeStoragePath(entry)).not.toThrow(); + }); + + it('rejects when storagePath equals the repo path itself (would delete the code)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: repoPath, // catastrophic: rm the working tree + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is a parent of the repo path', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.dirname(repoPath), // also catastrophic + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is empty (path.resolve falls back to cwd)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: '', // path.resolve('') === process.cwd() — would rm cwd + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath points somewhere totally unrelated', () => { + const entry: RegistryEntry = { + ...base, + storagePath: `${prefix}some${path.sep}other${path.sep}place`, + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is a sibling .gitnexus (right basename, wrong parent)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.join(`${prefix}different${path.sep}repo`, '.gitnexus'), + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('UnsafeStoragePathError carries the original entry + expected + actual paths', () => { + const entry: RegistryEntry = { + ...base, + storagePath: `${prefix}evil${path.sep}path`, + }; + try { + assertSafeStoragePath(entry); + } catch (e) { + expect(e).toBeInstanceOf(UnsafeStoragePathError); + const err = e as UnsafeStoragePathError; + expect(err.kind).toBe('UnsafeStoragePathError'); + expect(err.entry).toBe(entry); + // Expected path is the canonical `/.gitnexus`. + expect(err.expectedStoragePath).toBe(path.join(path.resolve(repoPath), '.gitnexus')); + // Actual path is the corrupted value (resolved). + expect(err.actualStoragePath).toBe(path.resolve(entry.storagePath)); + // Message must suggest the recovery action. + expect(err.message).toContain('registry.json'); + } + }); + + it('Windows: storagePath match is case-insensitive to match register/unregister semantics', () => { + if (process.platform !== 'win32') return; + const entry: RegistryEntry = { + ...base, + storagePath: path.join(repoPath.toUpperCase(), '.GITNEXUS'), + }; + // Should accept because Windows paths are case-insensitive. + expect(() => assertSafeStoragePath(entry)).not.toThrow(); + }); +});