reach a member's transcripts by listing them, not just by reading them
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/<slug>) -> false
statSync(<transcript>) -> 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/<id> 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 <noreply@anthropic.com>
This commit is contained in:
+153
-13
@@ -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/<slug>) -> false (no `x` on projects, so it cannot even be reached)
|
||||
// statSync(<transcript>) -> 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/<id> 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/<slug>/<id>.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/<slug>`. 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;
|
||||
|
||||
Reference in New Issue
Block a user