From 170dbdb8748af31d0472472b0587734baa2ca0ba Mon Sep 17 00:00:00 2001 From: manuaudio Date: Mon, 14 Sep 2026 04:44:53 -0400 Subject: [PATCH] fix(ai-vault): ignore non-absolute env overrides for agent scan roots (#13118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six scan roots took a directory from an environment variable and used it verbatim. A relative value is resolved by whichever Orca process reads it — main sits at `/` when Finder-launched, the terminal daemon chdirs itself to the user data dir, the AI Vault service inherits main's cwd — so one value names a different directory in each, and walkSessionFiles walks it with no depth cap, no entry cap and no time budget, about once a minute per the session-list cache TTL. The agent CLIs do accept a relative home (verified against real Grok 1.0.30: `GROK_HOME=myhome grok du` creates `/myhome`), but they resolve it against their own per-terminal cwd, which no Orca reader shares. Falling back to the default home is therefore not a lost configuration — it replaces an unbounded walk of an arbitrary tree with a bounded read of a known one, and matches what readGrokHomeEnvelope, skill-provider normalizedRoot and absoluteConfiguredDir already do with the same values. Add resolveAbsoluteDirOverride and apply it to CODEX_HOME, COPILOT_HOME, OPENCLAW_STATE_DIR, DEVIN_HOME, KIMI_CODE_HOME and GROK_HOME. It takes an explicit platform so the Windows shapes are provable from a POSIX CI box: `C:\...`, `C:/...` and UNC roots are kept, while the drive-relative `C:foo` and bare `C:` fall back. Tilde expansion stays out of it — Grok creates a literal `~` directory rather than expanding one — so absoluteConfiguredDir keeps its own Pi/Prime-specific expansion and delegates the absolute check. isAbsolute is syntactic only, so `/..` still collapses to `/`. That is fine for read-only discovery; these roots never gate renderer-supplied paths. Tests assert at the call sites, not just on the helper: the four session-scanner-agent-sources roots are module-level consts evaluated at import time, so they are exercised through AI_VAULT_AGENT_SOURCES with vi.stubEnv plus vi.resetModules. Reverting any one of the six call sites fails them (11-33 cases each). Closes #13082 Co-authored-by: Claude Opus 5 (1M context) --- ...ssion-scanner-agent-root-overrides.test.ts | 117 ++++++++++++++++++ .../ai-vault/session-scanner-agent-sources.ts | 15 ++- .../ai-vault/session-scanner-kimi-paths.ts | 3 +- src/main/ai-vault/session-scanner-values.ts | 13 +- src/shared/absolute-dir-override.test.ts | 62 ++++++++++ src/shared/absolute-dir-override.ts | 19 +++ src/shared/grok-session-paths.test.ts | 17 +++ src/shared/grok-session-paths.ts | 4 +- 8 files changed, 237 insertions(+), 13 deletions(-) create mode 100644 src/main/ai-vault/session-scanner-agent-root-overrides.test.ts create mode 100644 src/shared/absolute-dir-override.test.ts create mode 100644 src/shared/absolute-dir-override.ts diff --git a/src/main/ai-vault/session-scanner-agent-root-overrides.test.ts b/src/main/ai-vault/session-scanner-agent-root-overrides.test.ts new file mode 100644 index 00000000000..a75a437611a --- /dev/null +++ b/src/main/ai-vault/session-scanner-agent-root-overrides.test.ts @@ -0,0 +1,117 @@ +import { homedir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AiVaultScanOptions } from './session-scanner-types' + +/** + * The six env-derived scan roots must ignore a non-absolute value (#13082). + * + * These assert at the *call sites*, not on the shared helper: four of the roots are module-level + * consts evaluated at import time, so a helper that exists but is no longer wired into one of them + * is exactly the regression a helper-only test cannot see. + */ + +const NO_OPTIONS: AiVaultScanOptions = {} +const NO_WSL: readonly string[] = [] + +async function rootDirsFor( + agent: 'codex' | 'copilot' | 'devin' | 'openclaw' | 'kimi' | 'grok', + env: Record +): Promise { + vi.resetModules() + for (const [key, value] of Object.entries(env)) { + vi.stubEnv(key, value) + } + const sources = await import('./session-scanner-agent-sources.js') + const source = sources.AI_VAULT_AGENT_SOURCES[agent] + if (!source) { + throw new Error(`no source table entry for ${agent}`) + } + return source.rootDirs(NO_OPTIONS, NO_WSL) +} + +// Every shape a relative value can take, including the two Windows drive-relative ones. +const RELATIVE_VALUES = ['.', '..', 'rel/path', '~/sessions', 'C:foo', 'C:'] as const + +const CASES = [ + { + agent: 'codex', + envVar: 'CODEX_HOME', + absolute: '/srv/codex', + absoluteRoot: join('/srv/codex', 'sessions'), + defaultRoot: () => join(homedir(), '.codex', 'sessions') + }, + { + agent: 'copilot', + envVar: 'COPILOT_HOME', + absolute: '/srv/copilot', + absoluteRoot: join('/srv/copilot', 'session-state'), + defaultRoot: () => join(homedir(), '.copilot', 'session-state') + }, + { + agent: 'devin', + envVar: 'DEVIN_HOME', + absolute: '/srv/devin', + absoluteRoot: join('/srv/devin', 'transcripts'), + defaultRoot: () => join(homedir(), '.local', 'share', 'devin', 'cli', 'transcripts') + }, + { + agent: 'openclaw', + envVar: 'OPENCLAW_STATE_DIR', + absolute: '/srv/openclaw', + absoluteRoot: join('/srv/openclaw', 'agents'), + defaultRoot: () => join(homedir(), '.openclaw', 'agents') + }, + { + agent: 'kimi', + envVar: 'KIMI_CODE_HOME', + absolute: '/srv/kimi', + absoluteRoot: join('/srv/kimi', 'sessions'), + defaultRoot: () => join(homedir(), '.kimi-code', 'sessions') + }, + { + agent: 'grok', + envVar: 'GROK_HOME', + absolute: '/srv/grok', + absoluteRoot: join('/srv/grok', 'sessions'), + defaultRoot: () => join(homedir(), '.grok', 'sessions') + } +] as const + +describe('agent scan roots from environment overrides', () => { + afterEach(() => { + vi.unstubAllEnvs() + vi.resetModules() + }) + + for (const testCase of CASES) { + describe(testCase.envVar, () => { + it('uses an absolute override', async () => { + const roots = await rootDirsFor(testCase.agent, { [testCase.envVar]: testCase.absolute }) + expect(roots[0]).toBe(testCase.absoluteRoot) + }) + + it('tolerates whitespace around an absolute override', async () => { + const roots = await rootDirsFor(testCase.agent, { + [testCase.envVar]: ` ${testCase.absolute} ` + }) + expect(roots[0]).toBe(testCase.absoluteRoot) + }) + + it.each(RELATIVE_VALUES)('falls back to the default root for %j', async (value) => { + const roots = await rootDirsFor(testCase.agent, { [testCase.envVar]: value }) + expect(roots[0]).toBe(testCase.defaultRoot()) + }) + + // A relative root is the actual #13082 failure: it resolves against whichever Orca process + // reads it, so the walk starts somewhere arbitrary and has no depth, entry or time cap. + it.each(RELATIVE_VALUES)('never yields a relative root for %j', async (value) => { + const roots = await rootDirsFor(testCase.agent, { [testCase.envVar]: value }) + for (const root of roots) { + expect(root).toBe(join(root)) + expect(root.startsWith('/') || /^[A-Za-z]:[\\/]/.test(root)).toBe(true) + } + }) + }) + } +}) diff --git a/src/main/ai-vault/session-scanner-agent-sources.ts b/src/main/ai-vault/session-scanner-agent-sources.ts index 957d8d680a6..ff3abaddae9 100644 --- a/src/main/ai-vault/session-scanner-agent-sources.ts +++ b/src/main/ai-vault/session-scanner-agent-sources.ts @@ -1,5 +1,6 @@ import { homedir } from 'node:os' import { basename, dirname, extname, join, relative } from 'node:path' +import { resolveAbsoluteDirOverride } from '../../shared/absolute-dir-override' import type { AiVaultAgent } from '../../shared/ai-vault-types' import type { AiVaultDeletableAgent } from '../../shared/ai-vault-session-deletion' import { resolveGrokSessionsDir } from '../../shared/grok-session-paths' @@ -18,18 +19,21 @@ import { normalizeAgentSessionsDir, primeAgentSessionsDirFromEnv } from './sessi export const DEFAULT_CODEX_HOME_DIR = join(homedir(), '.codex') const CODEX_SESSIONS_DIR = join( - process.env.CODEX_HOME?.trim() || DEFAULT_CODEX_HOME_DIR, + resolveAbsoluteDirOverride(process.env.CODEX_HOME, DEFAULT_CODEX_HOME_DIR), 'sessions' ) const GEMINI_SESSIONS_DIR = join(homedir(), '.gemini', 'tmp') const COPILOT_SESSIONS_DIR = join( - process.env.COPILOT_HOME?.trim() || join(homedir(), '.copilot'), + resolveAbsoluteDirOverride(process.env.COPILOT_HOME, join(homedir(), '.copilot')), 'session-state' ) const CURSOR_PROJECTS_DIR = join(homedir(), '.cursor', 'projects') const HERMES_SESSIONS_DIR = join(homedir(), '.hermes', 'sessions') const ROVO_SESSIONS_DIR = join(homedir(), '.rovodev', 'sessions') -const OPENCLAW_STATE_DIR = process.env.OPENCLAW_STATE_DIR?.trim() || join(homedir(), '.openclaw') +const OPENCLAW_STATE_DIR = resolveAbsoluteDirOverride( + process.env.OPENCLAW_STATE_DIR, + join(homedir(), '.openclaw') +) const PI_SESSIONS_DIR = normalizeAgentSessionsDir( process.env.PI_CODING_AGENT_DIR?.trim() || join(homedir(), '.pi', 'agent', 'sessions'), '.pi' @@ -40,7 +44,10 @@ const PI_SESSIONS_DIR = normalizeAgentSessionsDir( const PRIME_AGENT_SESSIONS_DIR = primeAgentSessionsDirFromEnv() // Why: Devin ATIF transcripts are stored under /transcripts. const DEVIN_TRANSCRIPTS_DIR = join( - process.env.DEVIN_HOME?.trim() || join(homedir(), '.local', 'share', 'devin', 'cli'), + resolveAbsoluteDirOverride( + process.env.DEVIN_HOME, + join(homedir(), '.local', 'share', 'devin', 'cli') + ), 'transcripts' ) const DROID_SESSIONS_DIR = join(homedir(), '.factory', 'sessions') diff --git a/src/main/ai-vault/session-scanner-kimi-paths.ts b/src/main/ai-vault/session-scanner-kimi-paths.ts index 590e4aee814..d55061eac68 100644 --- a/src/main/ai-vault/session-scanner-kimi-paths.ts +++ b/src/main/ai-vault/session-scanner-kimi-paths.ts @@ -1,6 +1,7 @@ import { homedir } from 'node:os' import { basename, dirname, join } from 'node:path' import { createInterface } from 'node:readline' +import { resolveAbsoluteDirOverride } from '../../shared/absolute-dir-override' import { openTranscriptReadStream, wslGatedStat } from '../native-chat/wsl-transcript-fs-access' import { WslTranscriptFsError } from '../native-chat/wsl-transcript-fs-gate' import { asRecord, extractString } from './session-scanner-values' @@ -18,7 +19,7 @@ export function resolveKimiSessionsDir(override?: string): string { if (override?.trim()) { return override.trim() } - const home = process.env.KIMI_CODE_HOME?.trim() || join(homedir(), '.kimi-code') + const home = resolveAbsoluteDirOverride(process.env.KIMI_CODE_HOME, join(homedir(), '.kimi-code')) return join(home, 'sessions') } diff --git a/src/main/ai-vault/session-scanner-values.ts b/src/main/ai-vault/session-scanner-values.ts index f7d62611adb..a2f3b633be5 100644 --- a/src/main/ai-vault/session-scanner-values.ts +++ b/src/main/ai-vault/session-scanner-values.ts @@ -1,5 +1,6 @@ import { homedir } from 'node:os' -import { basename, dirname, isAbsolute, join } from 'node:path' +import { basename, dirname, join } from 'node:path' +import { resolveAbsoluteDirOverride } from '../../shared/absolute-dir-override' import { wslGatedReadFile } from '../native-chat/wsl-transcript-fs-access' import { WslTranscriptFsError } from '../native-chat/wsl-transcript-fs-gate' import { asRecord } from './session-scanner-record-value' @@ -165,14 +166,14 @@ function defaultPrimeAgentSessionsDir(): string { return join(homedir(), '.prime', 'agent', 'sessions') } -// Why: the CLI expands a leading `~` itself, so a value set outside a shell -// (config file, plist, quoted assignment) still resolves against the home dir. -// Returns null for anything that is not an absolute root, since a relative value -// ('', '.', '..', 'sessions') would resolve against the main-process cwd. +// Why: the Pi/Prime CLIs expand a leading `~` themselves, so a value set outside a +// shell (config file, plist, quoted assignment) still resolves against the home dir. +// That expansion is per-CLI and deliberately not in the shared absolute check — Grok, +// for one, creates a literal `~` directory instead. function absoluteConfiguredDir(rawValue: string): string | null { const expanded = rawValue === '~' ? homedir() : rawValue.replace(/^~(?=[\\/])/, homedir()) const normalized = expanded.replace(/[\\/]+$/, '') - return normalized && isAbsolute(normalized) ? normalized : null + return resolveAbsoluteDirOverride(normalized, '') || null } // Prime Agent takes PRIME_AGENT_CODING_AGENT_DIR verbatim as its agent config dir diff --git a/src/shared/absolute-dir-override.test.ts b/src/shared/absolute-dir-override.test.ts new file mode 100644 index 00000000000..26b2049a673 --- /dev/null +++ b/src/shared/absolute-dir-override.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest' +import { resolveAbsoluteDirOverride } from './absolute-dir-override' + +const FALLBACK = '/home/user/.agent' + +describe('resolveAbsoluteDirOverride', () => { + it('keeps an absolute override, trimming first', () => { + expect(resolveAbsoluteDirOverride('/srv/sessions', FALLBACK, 'linux')).toBe('/srv/sessions') + expect(resolveAbsoluteDirOverride(' /srv/sessions ', FALLBACK, 'linux')).toBe('/srv/sessions') + }) + + it.each([ + ['undefined', undefined], + ['null', null], + ['empty', ''], + ['whitespace only', ' '] + ])('falls back for %s', (_label, value) => { + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'linux')).toBe(FALLBACK) + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'win32')).toBe(FALLBACK) + }) + + it.each([ + ['a bare dot', '.'], + ['a parent reference', '..'], + ['a relative path', 'rel/path'], + // Grok 1.0.30 does not expand `~` — `GROK_HOME=~/x` makes it create a literal `~` dir under + // its own cwd — so expanding one here would point Orca at a directory no agent writes to. + ['an unexpanded tilde', '~/sessions'], + // Drive-*relative*: both resolve against that drive's current directory, not its root. + ['a drive-relative path', 'C:foo'], + ['a bare drive letter', 'C:'] + ])('falls back for %s on every platform', (_label, value) => { + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'linux')).toBe(FALLBACK) + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'darwin')).toBe(FALLBACK) + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'win32')).toBe(FALLBACK) + }) + + // Why: the check is platform-bound, so a POSIX CI box would silently "reject" every real + // Windows root if it ran the POSIX predicate. These pin the Windows shapes users actually set. + it.each([ + ['a drive-rooted path', 'C:\\Users\\ada\\.grok'], + ['a forward-slash drive root', 'C:/Users/ada/.grok'], + ['a UNC share', '\\\\server\\share\\grok'], + // Rooted but drive-relative; `path.resolve` still bounds it to the current drive. + ['a drive-current-root path', '\\grok'] + ])('keeps %s on Windows', (_label, value) => { + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'win32')).toBe(value) + }) + + it.each([['C:\\Users\\ada\\.grok'], ['\\\\server\\share\\grok'], ['\\grok']])( + 'falls back for the Windows path %j on POSIX', + (value) => { + expect(resolveAbsoluteDirOverride(value, FALLBACK, 'linux')).toBe(FALLBACK) + } + ) + + it('defaults to the host platform', () => { + const rooted = process.platform === 'win32' ? 'C:\\srv\\sessions' : '/srv/sessions' + expect(resolveAbsoluteDirOverride(rooted, FALLBACK)).toBe(rooted) + expect(resolveAbsoluteDirOverride('rel/path', FALLBACK)).toBe(FALLBACK) + }) +}) diff --git a/src/shared/absolute-dir-override.ts b/src/shared/absolute-dir-override.ts new file mode 100644 index 00000000000..a67f8ea0bde --- /dev/null +++ b/src/shared/absolute-dir-override.ts @@ -0,0 +1,19 @@ +import { posix, win32 } from 'node:path' + +/** + * An env-provided directory override, kept only when absolute (#13082). + * + * A relative value resolves against the *reading* process's cwd — `/` for a Finder-launched app, + * the user data dir for the terminal daemon — never against the cwd the agent CLI used to write + * it, so it names a different directory in every Orca process. Syntactic only: `/..` passes and + * collapses to `/`, so this is not a containment check. + */ +export function resolveAbsoluteDirOverride( + value: string | undefined | null, + fallback: string, + platform: NodeJS.Platform = process.platform +): string { + const trimmed = value?.trim() ?? '' + const isAbsolutePath = platform === 'win32' ? win32.isAbsolute : posix.isAbsolute + return trimmed && isAbsolutePath(trimmed) ? trimmed : fallback +} diff --git a/src/shared/grok-session-paths.test.ts b/src/shared/grok-session-paths.test.ts index 90d4cf69e1e..78b0a1fd52c 100644 --- a/src/shared/grok-session-paths.test.ts +++ b/src/shared/grok-session-paths.test.ts @@ -57,6 +57,23 @@ describe('grok-session-paths', () => { expect(resolveGrokHomeDir({}, '/home/ada')).toBe(join('/home/ada', '.grok')) }) + // Why: Grok itself accepts a relative GROK_HOME, but resolves it against *its own* cwd — a + // different directory per terminal. Orca's readers (main at `/` when Finder-launched, the + // daemon at the user data dir, the scan service inheriting main) would each resolve the same + // value somewhere else and walk it with no depth, entry or time cap (#13082). `~/…` is in the + // list because Grok 1.0.30 does not expand a tilde — it creates a literal `~` dir under its cwd. + it.each(['.', '..', 'rel/path', '~/grok', '~', 'C:foo', 'C:'])( + 'ignores the non-absolute GROK_HOME %j', + (relativeHome) => { + expect(resolveGrokHomeDir({ GROK_HOME: relativeHome }, '/home/ada')).toBe( + join('/home/ada', '.grok') + ) + expect(resolveGrokSessionsDir({ GROK_HOME: relativeHome }, '/home/ada')).toBe( + join('/home/ada', '.grok', 'sessions') + ) + } + ) + it('refuses to invent encodeURIComponent names longer than 255 bytes', () => { const longCwd = `/${'a'.repeat(200)}/${'b'.repeat(200)}` expect(Buffer.byteLength(encodeURIComponent(longCwd), 'utf8')).toBeGreaterThan( diff --git a/src/shared/grok-session-paths.ts b/src/shared/grok-session-paths.ts index e4d59c61f09..b40db0174b4 100644 --- a/src/shared/grok-session-paths.ts +++ b/src/shared/grok-session-paths.ts @@ -2,6 +2,7 @@ import { lstatSync } from 'node:fs' import { lstat, opendir } from 'node:fs/promises' import { homedir } from 'node:os' import { basename, dirname, isAbsolute, join, relative, resolve, sep } from 'node:path' +import { resolveAbsoluteDirOverride } from './absolute-dir-override' import { GrokSessionPathLookupQueue, type GrokSessionPathScanner @@ -41,8 +42,7 @@ export function resolveGrokHomeDir( env: GrokSessionPathEnv = process.env, homeDir: string = homedir() ): string { - const fromEnv = env.GROK_HOME?.trim() - return fromEnv || join(homeDir, '.grok') + return resolveAbsoluteDirOverride(env.GROK_HOME, join(homeDir, '.grok')) } export function resolveGrokSessionsDir(