From 327783532e70824606f698b1ab9320904c2b8a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Al=C3=A7ada=20Padez?= Date: Fri, 14 Aug 2026 19:48:06 +0000 Subject: [PATCH] reach a member's transcripts by listing them, not just by reading them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yesterday's fix routed transcript CONTENT through the member's identity and stopped there, on the strength of a comment in ChatIdentity saying enumeration never needed it — "their directories are 775 and the platform holds an ACL entry, so readdirSync and statSync have always worked". there are no 775 directories on this path. claude creates ~/.claude/projects/ and every project group at mode 700, and a 700 directory clamps the ACL mask to --- exactly as a 600 file does: user:officer:rwx #effective:--- mask::--- measured against a real member home: existsSync(projects) -> true (stat only needs traverse on .claude) readdirSync(projects) -> EACCES existsSync(projects/) -> false statSync() -> EACCES existsSync answering false rather than throwing is why this was invisible: every caller read it as "no such session". one root cause, three reported symptoms — an empty conversation list, no title on a new chat, and a /chat/ deep link that never restored the conversation. a fourth nobody had reported yet: delete removed nothing and still answered ok, because unlink needs w+x on the group directory too. so enumeration goes through the same door as content, as ONE call rather than a spawn per entry: listTranscriptsAs runs a single `find` as the member and returns every transcript with its mtime, which readdir+stat could not do without dozens of setpriv forks per request and a matching pile of auth.log lines. the owner keeps a fork-free path — that process already IS the owner. removeAs does the same for unlink, and readTailAs no longer stats a file it cannot stat. summarizeTranscript now takes the mtime it is given instead of stat'ing again, which is both the fix and one less syscall per file. verified against jg@pertento.ai on this machine: 6 conversations listed with titles from their first prompts, and a deep link by id alone loads 71 messages. owner path re-checked and unchanged. Co-Authored-By: Claude Opus 5 --- src/servers/api/chat/claude-sessions.ts | 211 +++++++++++------------- src/servers/read-as-user.ts | 166 +++++++++++++++++-- 2 files changed, 253 insertions(+), 124 deletions(-) diff --git a/src/servers/api/chat/claude-sessions.ts b/src/servers/api/chat/claude-sessions.ts index a1d53a4c..d01a1e29 100644 --- a/src/servers/api/chat/claude-sessions.ts +++ b/src/servers/api/chat/claude-sessions.ts @@ -1,7 +1,15 @@ -import { readdirSync, existsSync, statSync, mkdirSync, rmSync, realpathSync } from 'node:fs'; +import { realpathSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { DATA_PATH } from '../../data-path'; -import { appendTextAs, readHeadAs, readTailAs, readTextAs, type AsUser } from '../../read-as-user'; +import { + appendTextAs, + listTranscriptsAs, + readHeadAs, + readTailAs, + readTextAs, + removeAs, + type AsUser, +} from '../../read-as-user'; // ── Claude session store (source of truth) ── // The `claude` CLI persists every session as a JSONL transcript at @@ -31,12 +39,13 @@ export type ChatIdentity = { /** From `resolveHomeDir`. Never `getOwnerHomeDir`, which ignores its argument. */ home: string; /** - * Whose identity to read transcript CONTENT as — `null` for the owner. From `resolveHomeDir`. + * Whose identity to read a member's transcripts as — `null` for the owner. From `resolveHomeDir`. * - * Locating a member's transcripts never needed this: their directories are 775 and the platform holds an - * ACL entry, so `readdirSync` and `statSync` have always worked. Reading one does, because `claude` writes - * every transcript at mode 600 and that clamps the ACL mask to `---`. See `read-as-user.ts` — this field - * is the whole reason a member's chat list was empty while their chat worked. + * This said locating them "never needed this: their directories are 775". There are no 775 directories on + * this path — `claude` creates `projects/` and every group at 700, which clamps the platform's ACL entry + * to nothing exactly as mode 600 does for the files. So `readdirSync`, `statSync` and unlink all fail too, + * and `existsSync` answers **false** rather than throwing, which is why it read as "no such session" + * everywhere instead of as an error. See `read-as-user.ts`; the listing goes through `listTranscriptsAs`. */ osUser: string | null; /** @@ -51,6 +60,28 @@ export type ChatIdentity = { const claudeProjectsDir = (home: string): string => join(home, '.claude', 'projects'); +/** + * Where a session's transcript actually is, or null. + * + * One listing as the transcripts' owner, then a lookup — never `existsSync` on a candidate path. For a + * member `existsSync` answers **false** on a file that is plainly there, because the group directory is + * mode 700 and the service user cannot traverse it, and every caller read that false as "no such session". + * + * `cwd` names the group to prefer, not the group to trust: a session's group and the caller's current one + * disagree routinely (a deep link has not resolved its group yet, or the list is showing another). Reads + * have always fallen back like this; writes did not, so delete and rename returned "not found" for a + * session that was on screen. + */ +function locateTranscript(who: ChatIdentity, sessionId: string, cwd?: string): string | null { + const projectsDir = claudeProjectsDir(who.home); + const files = listTranscriptsAs(who.osUser, projectsDir); + const preferred = cwd ? projectSlug(cwd) : null; + const hit = + (preferred && files.find((f) => f.id === sessionId && f.slug === preferred)) || + files.find((f) => f.id === sessionId); + return hit ? join(projectsDir, hit.slug, `${sessionId}.jsonl`) : null; +} + /** Claude's folder name for a working directory. */ export const projectSlug = (cwd: string): string => cwd.replace(/[^a-zA-Z0-9]/g, '-'); @@ -132,16 +163,11 @@ type Entry = { */ const summaryCache = new Map(); -function summarizeTranscript(osUser: AsUser, filePath: string, id: string): TranscriptSummary | null { - let mtimeMs: number; - let mtime: string; - try { - const stat = statSync(filePath); - mtimeMs = stat.mtimeMs; - mtime = stat.mtime.toISOString(); - } catch { - return null; - } +// `mtimeMs` is passed in rather than stat'ed here: `statSync` is EACCES on a member's transcript, and the +// listing that found the file already carries its mtime. Stat'ing again would be a second fork per file AND +// would fail for exactly the accounts this exists to serve. +function summarizeTranscript(osUser: AsUser, filePath: string, id: string, mtimeMs: number): TranscriptSummary | null { + const mtime = new Date(mtimeMs).toISOString(); const cached = summaryCache.get(filePath); if (cached && cached.mtimeMs === mtimeMs) return cached.summary; @@ -430,8 +456,9 @@ function parseClaudeTranscript( sessionId: string, fallbackCwd = '', ): ClaudeSessionDetail | null { - if (!existsSync(filePath)) return null; - + // No `existsSync` guard: it is redundant with the catch below (a missing file throws there just the same) + // and it is actively WRONG for a member, answering false on a transcript that exists — which is how a + // /chat/ deep link 404'd on a session the member was looking at. const messages: ClaudeChatMessage[] = []; const toolById = new Map>(); let model = ''; @@ -570,20 +597,10 @@ export function loadClaudeSession(who: ChatIdentity, cwd: string, sessionId: str * refresh to /chat/, when the cwd isn't known yet; the transcript records the real cwd, which the * caller uses to scope the list + cwd picker. */ export function loadClaudeSessionById(who: ChatIdentity, sessionId: string): ClaudeSessionDetail | null { - const projectsDir = claudeProjectsDir(who.home); - let slugs: string[]; - try { - slugs = readdirSync(projectsDir); - } catch { - return null; - } - for (const slug of slugs) { - const filePath = join(projectsDir, slug, `${sessionId}.jsonl`); - if (!existsSync(filePath)) continue; - const detail = parseClaudeTranscript(who.osUser, filePath, sessionId); - return detail && loadChainTranscript(who, detail); - } - return null; + const filePath = locateTranscript(who, sessionId); + if (!filePath) return null; + const detail = parseClaudeTranscript(who.osUser, filePath, sessionId); + return detail && loadChainTranscript(who, detail); } /** @@ -593,23 +610,8 @@ export function loadClaudeSessionById(who: ChatIdentity, sessionId: string): Cla * or a deep link hasn't resolved its group yet). Reads have always fallen back like this; writes did * not, so delete and rename returned "not found" for a session that was plainly on screen. */ -function findTranscript(who: ChatIdentity, cwd: string, sessionId: string): string | null { - const preferred = join(claudeProjectsDir(who.home), projectSlug(cwd), `${sessionId}.jsonl`); - if (existsSync(preferred)) return preferred; - - const projectsDir = claudeProjectsDir(who.home); - let slugs: string[]; - try { - slugs = readdirSync(projectsDir); - } catch { - return null; - } - for (const slug of slugs) { - const filePath = join(projectsDir, slug, `${sessionId}.jsonl`); - if (existsSync(filePath)) return filePath; - } - return null; -} +const findTranscript = (who: ChatIdentity, cwd: string, sessionId: string): string | null => + locateTranscript(who, sessionId, cwd); /** * Delete a conversation by removing its transcript file — and, when it is a `/clear` chain, the files @@ -625,11 +627,11 @@ export function deleteClaudeSession(who: ChatIdentity, cwd: string, sessionId: s const ownCwd = firstCwd(who.osUser, filePath); const ids = ownCwd ? chainFileIds(who, ownCwd, sessionId) : [sessionId]; + // `removeAs`, not `rmSync`: unlinking needs `w`+`x` on the DIRECTORY, which the service user does not have + // on a member's group. `rmSync` guarded by `existsSync` therefore deleted nothing and still reported + // success — the conversation reappeared on the next refresh. const dir = dirname(filePath); - for (const id of ids) { - const partPath = join(dir, `${id}.jsonl`); - if (existsSync(partPath)) rmSync(partPath); - } + for (const id of ids) removeAs(who.osUser, join(dir, `${id}.jsonl`)); return true; } @@ -773,30 +775,30 @@ export function listClaudePwds(who: ChatIdentity): ClaudePwd[] { const defaultCwd = who.home; const byCwd = new Map(); - if (existsSync(projectsDir)) { - for (const group of readdirSync(projectsDir)) { - const groupDir = join(projectsDir, group); - let files: string[]; - try { - files = readdirSync(groupDir).filter((f) => f.endsWith('.jsonl')); - } catch { - continue; // not a directory - } - if (files.length === 0) continue; + // One listing for the whole tree, grouped here. This used to be `readdirSync` + a `statSync` per file, + // and BOTH are EACCES for a member — the readdir threw uncaught, so this endpoint answered 500 rather + // than answering wrongly. That 500 was the visible half of the empty conversation list. + const byGroup = new Map(); + for (const file of listTranscriptsAs(who.osUser, projectsDir)) { + const prev = byGroup.get(file.slug); + byGroup.set(file.slug, { + count: (prev?.count ?? 0) + 1, + newest: Math.max(prev?.newest ?? 0, file.mtimeMs), + first: prev?.first ?? file.id, + }); + } - const cwd = firstCwd(who.osUser, join(groupDir, files[0]!)); - if (!cwd) continue; - let updatedAt = ''; - for (const f of files) { - const m = statSync(join(groupDir, f)).mtime.toISOString(); - if (m > updatedAt) updatedAt = m; - } - const prev = byCwd.get(cwd); - byCwd.set(cwd, { - count: (prev?.count ?? 0) + files.length, - updatedAt: prev && prev.updatedAt > updatedAt ? prev.updatedAt : updatedAt, - }); - } + for (const [slug, group] of byGroup) { + // The cwd is a property of the transcript's entries, not of the slug, which is lossy and cannot be + // reversed. Any file in the group answers it. + const cwd = firstCwd(who.osUser, join(projectsDir, slug, `${group.first}.jsonl`)); + if (!cwd) continue; + const updatedAt = new Date(group.newest).toISOString(); + const prev = byCwd.get(cwd); + byCwd.set(cwd, { + count: (prev?.count ?? 0) + group.count, + updatedAt: prev && prev.updatedAt > updatedAt ? prev.updatedAt : updatedAt, + }); } if (!byCwd.has(defaultCwd)) byCwd.set(defaultCwd, { count: 0, updatedAt: '' }); @@ -814,13 +816,12 @@ export function listClaudePwds(who: ChatIdentity): ClaudePwd[] { * mtime-cached, which is what makes calling this on every request cheap. */ function scanGroup(who: ChatIdentity, cwd: string): TranscriptSummary[] { - const dir = join(claudeProjectsDir(who.home), projectSlug(cwd)); - if (!existsSync(dir)) return []; + const slug = projectSlug(cwd); + const dir = join(claudeProjectsDir(who.home), slug); const sessions: TranscriptSummary[] = []; - for (const file of readdirSync(dir)) { - if (!file.endsWith('.jsonl')) continue; - const summary = summarizeTranscript(who.osUser, join(dir, file), file.replace(/\.jsonl$/, '')); + for (const file of listTranscriptsAs(who.osUser, claudeProjectsDir(who.home), slug)) { + const summary = summarizeTranscript(who.osUser, join(dir, `${file.id}.jsonl`), file.id, file.mtimeMs); if (summary) sessions.push(summary); } return applyLineage(sessions); @@ -869,40 +870,28 @@ export function claudeSessionContext( * anywhere hotter. */ export function liveSessionTitle(who: ChatIdentity, sessionId: string): { title: string; cwd: string } | null { - const projectsDir = claudeProjectsDir(who.home); - let slugs: string[]; + const filePath = locateTranscript(who, sessionId); + if (!filePath) return null; + + // The cwd is a property of the transcript's entries, so the first one carrying it settles which group + // this session belongs to — no need to reverse the slug, which is lossy. + let cwd: string | null = null; try { - slugs = readdirSync(projectsDir); + for (const line of readTextAs(who.osUser, filePath).split('\n')) { + if (!line.trim()) continue; + const entry = JSON.parse(line) as { cwd?: string }; + if (entry.cwd) { + cwd = entry.cwd; + break; + } + } } catch { return null; } + if (!cwd) return null; - for (const slug of slugs) { - const filePath = join(projectsDir, slug, `${sessionId}.jsonl`); - if (!existsSync(filePath)) continue; - - // The cwd is a property of the transcript's entries, so the first one carrying it settles which - // group this session belongs to — no need to reverse the slug, which is lossy. - let cwd: string | null = null; - try { - for (const line of readTextAs(who.osUser, filePath).split('\n')) { - if (!line.trim()) continue; - const entry = JSON.parse(line) as { cwd?: string }; - if (entry.cwd) { - cwd = entry.cwd; - break; - } - } - } catch { - return null; - } - if (!cwd) return null; - - const context = claudeSessionContext(who, cwd, sessionId); - return context ? { title: context.title, cwd } : null; - } - - return null; + const context = claudeSessionContext(who, cwd, sessionId); + return context ? { title: context.title, cwd } : null; } /** diff --git a/src/servers/read-as-user.ts b/src/servers/read-as-user.ts index a8dd91fa..58d1bdf2 100644 --- a/src/servers/read-as-user.ts +++ b/src/servers/read-as-user.ts @@ -1,4 +1,15 @@ -import { appendFileSync, closeSync, openSync, readFileSync, readSync, statSync } from 'node:fs'; +import { + appendFileSync, + closeSync, + existsSync, + openSync, + readdirSync, + readFileSync, + readSync, + rmSync, + statSync, +} from 'node:fs'; +import { basename, join } from 'node:path'; import { runAsArgv } from './os-user'; // Reading a file that belongs to a member. @@ -35,10 +46,111 @@ import { runAsArgv } from './os-user'; // for a subprocess that takes a millisecond. The alternative was an `await` ripple through every caller for // no behavioural gain. // -// Only file CONTENT needs this. `statSync` needs traverse on the parent, `readdirSync` needs read on it, and -// both are satisfied by the 775 directories; creating and deleting entries inside them works too, because a -// directory's mask is `rwx`. So the privileged surface is small on purpose — see the call sites, not this -// file, for what actually needed it. +// ── That last paragraph used to say only CONTENT needed this. It was wrong ── +// +// It claimed `statSync` and `readdirSync` were satisfied by "the 775 directories". They are not, because +// there are no 775 directories on this path: `claude` creates `~/.claude/projects/` and each project group +// at mode **700**, and the same rule that clamps a 600 file clamps a 700 directory — +// +// $ getfacl .../.claude/projects +// user:officer:rwx #effective:--- +// mask::--- +// +// so the service user has neither `r` nor `x` on it. Measured, not reasoned: +// +// existsSync(projects) -> true (stat needs traverse on `.claude`, which IS permissive) +// readdirSync(projects) -> EACCES +// existsSync(projects/) -> false (no `x` on projects, so it cannot even be reached) +// statSync() -> EACCES +// +// `existsSync` returning **false** rather than throwing is what made this invisible: every caller read it as +// "no such session" and returned an empty list or a 404. One bug, three symptoms — an empty conversation +// list, no title on a new chat, and a /chat/ deep link that never restored. The content fix landed +// without it because content reads were already funnelled through this file; enumeration never was. +// +// So enumeration is here too, and as ONE call rather than a spawn per entry: `readdir` + `stat` per file +// would be dozens of `sudo setpriv` forks per request, each writing a line to `/var/log/auth.log`. A single +// `find` answers the whole tree — which paths exist AND their mtimes — in one fork. + +/** One transcript on disk. `slug` is the project-group directory; `id` the session uuid. */ +export type TranscriptFile = { slug: string; id: string; mtimeMs: number }; + +/** + * Every `*.jsonl` under `projectsDir`, with mtimes, as the owner of the files. + * + * Replaces `readdirSync` + `statSync`, both of which fail for a member. Returns `[]` for a tree that does + * not exist or cannot be read — the callers all treat "no transcripts" and "cannot look" the same way, and + * there is no useful third answer to give a list endpoint. + * + * `onlySlug` narrows to one project group; it bounds the owner's syscalls and the member's `find`, and the + * result is identical either way. + */ +export function listTranscriptsAs(osUser: AsUser, projectsDir: string, onlySlug?: string): TranscriptFile[] { + if (!osUser) return listTranscriptsAsSelf(projectsDir, onlySlug); + + // GNU `-printf` is safe here: the member path exists only where `sudo setpriv` does, which is Linux. An + // owner on macOS takes the branch above. + // Depth follows the root: transcripts are `projects//.jsonl`, so scanning the whole tree is two + // levels down and scanning one group is one. Pinning both bounds keeps `find` off the rest of the home. + const root = onlySlug ? join(projectsDir, onlySlug) : projectsDir; + const depth = onlySlug ? '1' : '2'; + let out: string; + try { + out = runSync(osUser, [ + 'find', + root, + '-mindepth', + depth, + '-maxdepth', + depth, + '-name', + '*.jsonl', + '-printf', + '%h\t%f\t%T@\n', + ]); + } catch { + // A missing tree exits non-zero, which is the same nothing as an empty one. + return []; + } + + const files: TranscriptFile[] = []; + for (const line of out.split('\n')) { + if (!line) continue; + const [dir, name, mtime] = line.split('\t'); + if (!dir || !name || !mtime) continue; + files.push({ slug: basename(dir), id: name.replace(/\.jsonl$/, ''), mtimeMs: Math.round(Number(mtime) * 1000) }); + } + return files; +} + +/** The owner's own files — no fork, because this process already IS them. */ +function listTranscriptsAsSelf(projectsDir: string, onlySlug?: string): TranscriptFile[] { + const slugs = onlySlug ? [onlySlug] : safeReaddir(projectsDir); + const files: TranscriptFile[] = []; + for (const slug of slugs) { + for (const name of safeReaddir(join(projectsDir, slug))) { + if (!name.endsWith('.jsonl')) continue; + try { + files.push({ + slug, + id: name.replace(/\.jsonl$/, ''), + mtimeMs: statSync(join(projectsDir, slug, name)).mtimeMs, + }); + } catch { + // Raced with a delete, or not a regular file. Either way it is not a transcript we can offer. + } + } + } + return files; +} + +const safeReaddir = (dir: string): string[] => { + try { + return readdirSync(dir); + } catch { + return []; + } +}; /** Whose identity to read as. `null` is this process's own uid — the owner, and the common case. */ export type AsUser = string | null; @@ -67,14 +179,18 @@ export function readHeadAs(osUser: AsUser, path: string, bytes: number): string /** The last `bytes` bytes. `truncated` reports whether anything was left off the front. */ export function readTailAs(osUser: AsUser, path: string, bytes: number): { text: string; truncated: boolean } { - // `statSync` needs traverse on the parent directory, not read on the file, so it works unprivileged even - // when the content does not — which is also what keeps the mtime cache in `summarizeTranscript` honest. - const size = statSync(path).size; - const truncated = size > bytes; - const text = osUser - ? runSync(osUser, ['tail', '-c', String(bytes), '--', path]) - : readRange(path, Math.max(0, size - bytes), bytes); - return { text, truncated }; + if (!osUser) { + const size = statSync(path).size; + return { text: readRange(path, Math.max(0, size - bytes), bytes), truncated: size > bytes }; + } + + // `statSync` is EACCES on a member's transcript — the header explains why — so the size has to come back + // from the same identity as the bytes. One fork for both: `wc -c` writes the size on the first line, then + // `tail` writes the window. Splitting them would double the forks and could straddle an append. + const out = runSync(osUser, ['sh', '-c', 'wc -c < "$1"; tail -c "$2" -- "$1"', '_', path, String(bytes)]); + const firstBreak = out.indexOf('\n'); + const size = Number(out.slice(0, firstBreak).trim()); + return { text: out.slice(firstBreak + 1), truncated: Number.isFinite(size) && size > bytes }; } /** Append one line, as its owner. The rename path writes a `summary` entry into the member's transcript. */ @@ -95,6 +211,30 @@ export function appendTextAs(osUser: AsUser, path: string, text: string): void { } } +/** + * Delete a file as its owner. Returns false if it was not there. + * + * Needed for the same reason the listing is: removing an entry needs `w`+`x` on the DIRECTORY, and the + * service user has neither on a member's `projects/`. Without this, deleting a member's conversation + * silently removed nothing and still answered `{ ok: true }`. + */ +export function removeAs(osUser: AsUser, path: string): boolean { + if (!osUser) { + if (!existsSync(path)) return false; + rmSync(path); + return true; + } + // `rm -f` exits 0 on a missing file, so absence is reported by testing first — in the same fork. + const out = runSync(osUser, [ + 'sh', + '-c', + 'if test -e "$1"; then rm -f -- "$1" && printf 1; else printf 0; fi', + '_', + path, + ]); + return out.trim() === '1'; +} + /** Owner fast path for a byte window — the same positional read the callers used before this file existed. */ function readRange(path: string, start: number, bytes: number): string { let fd: number | undefined;