From 16415a858e39fc32a748d2636d6641d328046ddf Mon Sep 17 00:00:00 2001 From: Test Date: Sat, 4 Apr 2026 22:52:59 -0300 Subject: [PATCH] fix(workspace): canonicalize paths in isTempPath, use dynamic config path in serve warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #1056 addressing two review comments from gemini-code-assist. 1. isTempPath() now uses realpathSync to canonicalize both tmpdir() and the candidate root before prefix-comparing. On macOS /tmp is a symlink to /private/tmp and tmpdir() returns /var/folders/..., so the previous path.resolve()-only check would miss any literal /tmp/... path and bypass the guard against poisoning the global workspaceRoot config. Fails CLOSED on canonicalization errors (treats as temp → refuses to persist) since the whole purpose of this guard is to be conservative. 2. startAgentSync() now builds its "no workspace found" warning using genieHome() (newly exported) instead of hardcoding ~/.genie/config.json, so users who set GENIE_HOME see the actual path in the fix guidance. Also adds: - A regression test using a real symlink within the test's tmpdir to verify canonicalization works end-to-end (skips gracefully on CI environments that disallow symlinks). - A sanity test that genieHome() honors the GENIE_HOME env override. --- src/lib/workspace.test.ts | 39 ++++++++++++++++++++++++++++++++++++-- src/lib/workspace.ts | 26 +++++++++++++++++++------ src/term-commands/serve.ts | 6 ++++-- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/src/lib/workspace.test.ts b/src/lib/workspace.test.ts index 269842b17..5bd0686e1 100644 --- a/src/lib/workspace.test.ts +++ b/src/lib/workspace.test.ts @@ -1,8 +1,8 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { findWorkspace, getWorkspaceConfig, scanAgents } from './workspace.js'; +import { findWorkspace, genieHome, getWorkspaceConfig, scanAgents } from './workspace.js'; let testDir: string; let fakeGenieHome: string; @@ -122,6 +122,41 @@ describe('workspaceRoot persistence', () => { // (if the file doesn't exist at all, that's also a pass — nothing was persisted) }); + test('canonicalizes symlinked tmp paths so the guard cannot be bypassed', () => { + // Regression guard for the macOS /tmp → /private/tmp case. path.resolve() + // does NOT follow symlinks, so a naive prefix check against a non-canonical + // path would miss. realpathSync() canonicalizes both sides. + const real = join(testDir, 'real-workspace'); + const link = join(testDir, 'linked-workspace'); + mkdirSync(real, { recursive: true }); + makeWorkspace(real); + try { + symlinkSync(real, link, 'dir'); + } catch { + // Some CI environments disallow symlink creation; skip gracefully. + return; + } + + // findWorkspace called via the symlink path — without realpathSync the + // tmp-guard would fail to match, with realpathSync it matches correctly. + const result = findWorkspace(link); + expect(result).not.toBeNull(); + + // Either the config file doesn't exist at all, or it exists with no workspaceRoot. + // Both mean "nothing was persisted" — which is what we want. + const configPath = join(fakeGenieHome, 'config.json'); + if (existsSync(configPath)) { + const config = JSON.parse(readFileSync(configPath, 'utf-8')); + expect(config.workspaceRoot).toBeUndefined(); + } + }); + + test('genieHome() reflects GENIE_HOME env override', () => { + // Sanity check: the exported helper resolves dynamically so callers in other + // modules (e.g. serve.ts warning messages) see the same value as workspace.ts. + expect(genieHome()).toBe(fakeGenieHome); + }); + test('clears stale workspaceRoot from config when the saved path is gone', () => { // Simulate a prior run that persisted a path which has since vanished. const vanished = join(testDir, 'was-here-now-gone'); diff --git a/src/lib/workspace.ts b/src/lib/workspace.ts index 629724339..b021faffb 100644 --- a/src/lib/workspace.ts +++ b/src/lib/workspace.ts @@ -5,7 +5,7 @@ * If the cwd passes through `agents//`, the agent name is extracted. */ -import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, writeFileSync } from 'node:fs'; import { homedir, tmpdir } from 'node:os'; import { dirname, join, resolve, sep } from 'node:path'; @@ -63,15 +63,29 @@ export function findWorkspace(cwd?: string): WorkspaceInfo | null { } /** Resolved lazily so GENIE_HOME env overrides in tests take effect. */ -function genieHome(): string { +export function genieHome(): string { return process.env.GENIE_HOME ?? join(homedir(), '.genie'); } -/** True if `root` is under the OS temp directory (avoids persisting test workspaces). */ +/** + * True if `root` is under the OS temp directory (avoids persisting test workspaces). + * + * Uses `realpathSync` to canonicalize both paths before comparing — on macOS + * `/tmp` is a symlink to `/private/tmp`, and `tmpdir()` returns `/var/folders/…`, + * so a plain string prefix check against non-canonical paths would bypass the guard. + * + * Fails CLOSED: if canonicalization fails (path doesn't exist, permission error), + * treat as temp and refuse to persist — the saveWorkspaceRoot caller's goal is + * to be conservative about what lands in the global config. + */ function isTempPath(root: string): boolean { - const normalizedTmp = resolve(tmpdir()); - const normalizedRoot = resolve(root); - return normalizedRoot === normalizedTmp || normalizedRoot.startsWith(normalizedTmp + sep); + try { + const canonicalTmp = realpathSync(tmpdir()); + const canonicalRoot = realpathSync(root); + return canonicalRoot === canonicalTmp || canonicalRoot.startsWith(canonicalTmp + sep); + } catch { + return true; + } } function saveWorkspaceRoot(root: string): void { diff --git a/src/term-commands/serve.ts b/src/term-commands/serve.ts index f2a1129f1..e16a35ebd 100644 --- a/src/term-commands/serve.ts +++ b/src/term-commands/serve.ts @@ -369,12 +369,14 @@ const handles: DaemonHandles = { schedulerHandle: null, agentWatcher: null }; /** Sync agent directory from workspace and start file watcher. */ async function startAgentSync(): Promise<{ close: () => void } | null> { try { - const { findWorkspace } = require('../lib/workspace.js') as typeof import('../lib/workspace.js'); + const { findWorkspace, genieHome } = require('../lib/workspace.js') as typeof import('../lib/workspace.js'); const ws = findWorkspace(); if (!ws) { // Loud failure — silent return used to hide the whole discovery subsystem // when serve booted from outside a workspace (or with a stale saved root). - console.warn(' Agent sync: DISABLED — no workspace found from cwd or ~/.genie/config.json'); + const { join } = require('node:path') as typeof import('node:path'); + const configPath = join(genieHome(), 'config.json'); + console.warn(` Agent sync: DISABLED — no workspace found from cwd or ${configPath}`); console.warn(' Fix: `cd && genie serve restart`, or run `genie init` to bootstrap one'); return null; }