Skip to content

[CLAUDE ROUTINE]: Security enhancement — bring findCodexTranscript cache lookup up to the same containment-check standard as Cursor / Copilot, and add the matching path-traversal test #272

@NiveditJain

Description

@NiveditJain

Summary

findCopilotTranscript and findCursorTranscript already do a defense-in-depth containment check that rejects any sessionId whose resolved path escapes the integration's session-state root — and both have a rejects path-traversal session ids unit test pinning that behaviour. findCodexTranscript is the only one of the three that doesn't yet have that same belt-and-braces validation, and it has one extra wrinkle: it looks up paths from a JSON cache file at ~/.failproofai/cache/codex-session-paths.json and trusts whatever path it reads there.

Bringing the Codex path to parity with the other two integrations is a small, friendly polish — it costs almost nothing at runtime and gives us the same guarantee for every CLI we support.

Where

lib/codex-sessions.ts:39-121

const CACHE_PATH = join(homedir(), ".failproofai", "cache", "codex-session-paths.json");

function readCache(): Record<string, string> {
  try {
    if (!existsSync(CACHE_PATH)) return {};
    return JSON.parse(readFileSync(CACHE_PATH, "utf-8")) as Record<string, string>;
  } catch { return {}; }
}

export function findCodexTranscript(sessionId: string): string | null {
  const cache = readCache();
  const cached = cache[sessionId];
  if (cached && existsSync(cached)) return cached;   // ← trusts cache value verbatim
  // ...
}

For comparison, lib/copilot-sessions.ts:66-74 and lib/cursor-sessions.ts:52-63 both do:

const root = resolve(getCopilotSessionStateRoot());
const candidate = resolve(root, sessionId);
if (candidate === root || !candidate.startsWith(`${root}${sep}`)) return null;

…and __tests__/lib/copilot-sessions.test.ts:268 and __tests__/lib/cursor-sessions.test.ts:225 both have a rejects path-traversal session ids test. __tests__/lib/codex-sessions.test.ts does not.

Why this matters

flowchart LR
    Caller[Dashboard /<br/>session viewer] -->|sessionId| Codex[findCodexTranscript]
    Codex -->|reads| Cache[(codex-session-paths.json)]
    Codex -->|fallback scan| Sessions[(~/.codex/sessions/<br/>YYYY/MM/DD/)]
    Cache -. trusted as-is .-> Read[readFile]
    Sessions -. trusted as-is .-> Read
    Read --> Out[Returned to dashboard]
    style Cache fill:#fff5d6,stroke:#d6a900
    style Sessions fill:#fff5d6,stroke:#d6a900
Loading

Today the dashboard accepts whatever cached[sessionId] resolves to. The threat model here is narrow — the cache lives in the user's own ~/.failproofai/cache/, so anyone who can write to it already has the user's privileges — but every other CLI integration in this codebase enforces "the resolved path must live under the session root" anyway, because:

  • It catches accidental cache corruption or stale entries from an older layout.
  • It makes the file-discovery contract uniform across CLIs (one mental model, one helper, one test pattern).
  • It blocks any future code path that lets a less-trusted source seed sessionId (URL params, env vars, etc.) from being upgraded into an arbitrary read.

Suggested approach

  1. Add a containment helper that mirrors getCopilotSessionDir/getCursorSessionDir:
const CODEX_SESSIONS_ROOT = resolve(homedir(), ".codex", "sessions");

function isUnderCodexSessions(p: string): boolean {
  const r = resolve(p);
  return r === CODEX_SESSIONS_ROOT || r.startsWith(CODEX_SESSIONS_ROOT + sep);
}
  1. Apply it both ways in findCodexTranscript:
const cached = cache[sessionId];
if (cached && isUnderCodexSessions(cached) && existsSync(cached)) return cached;
// ...also wrap each `dirSearch` hit before writeCacheEntry / return:
if (hit && isUnderCodexSessions(hit)) { writeCacheEntry(sessionId, hit); return hit; }
  1. Optionally validate sessionId itself (UUID-shape) before joining — Codex sessionIds are always UUIDs and the existing SESSION_ID_RE in lib/codex-projects.ts:23 is already there to reuse.

  2. Add a __tests__/lib/codex-sessions.test.ts case that mirrors the Copilot/Cursor test:

it("rejects cached paths that escape ~/.codex/sessions/", () => {
  // seed the cache with a poisoned path
  // assert findCodexTranscript returns null instead of returning the poisoned path
});

Customer impact

  • Same protection model as Copilot and Cursor — no new behaviour, just consistency.
  • No effect on the happy path: a normal ~/.codex/sessions/2025/05/01/<file>.jsonl path stays under the root and continues to work.
  • Cache file gracefully self-heals: a poisoned/stale entry is simply ignored on read, and the next discovery pass re-populates a clean entry.

Acceptance criteria

  • findCodexTranscript rejects cache entries that don't resolve under ~/.codex/sessions/.
  • dirSearch results pass the same check before being cached or returned.
  • __tests__/lib/codex-sessions.test.ts has a rejects path-traversal / poisoned cache entries test.
  • No regressions in getCachedCodexSessionLog / getCodexSessionLog happy path.

Related

https://claude.ai/code/session_01SAwaAnE9bTuLujnvksoCYN

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions