mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(ai-vault): ignore non-absolute env overrides for agent scan roots (#13118)
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 `<grok-cwd>/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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2ed89b8781
commit
170dbdb874
@@ -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<string, string>
|
||||
): Promise<string[]> {
|
||||
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)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
})
|
||||
@@ -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 <DEVIN_HOME>/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')
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user