a member's background job can be read and stopped
A member started `bun dev`, watched it run, and got no logs at all — then
`{"stopped":false,"reason":"no such task"}` from the Stop button. Two symptoms,
one cause, and it is the same one `read-as-user.ts` was written for.
`findTaskOutput` globbed `/tmp/claude-*/…` as the platform's own uid.
`/tmp/claude-<uid>` is `drwx------` (checked, not assumed), so for a member that
directory cannot even be traversed. A glob that cannot traverse does not throw —
it returns an empty iterator. Both callers read that as "no such task": the log
view showed "Nothing written yet" forever, and Stop refused an id that was
perfectly real.
Every other filesystem access in that module already goes through the as-user
helpers. This one path never did, and it was invisible for as long as only the
owner used chat.
So the lookup moves into `findPathAs`, and Stop runs as the member too — both
halves of it. `fuser` reads `/proc/<pid>/fd`, which only the process's own user
and root may open, so as the platform it would have found no pids and reported
the job already stopped; the kill after it would have been cross-user EPERM,
swallowed by a catch. Neither would have said anything.
── The exit code is ignored, and that is the point ──
The first version of this fix located the file and threw it away. `find` exits 1
when it cannot descend into something, and searching `/tmp` from ANY identity
hits another account's `drwx------` — it prints the match on stdout first, then
returns 1. Treating that as failure reproduced the original bug through a new
mechanism. Caught by testing the owner path against a real task file rather than
assuming the refactor was behaviour-preserving; a missing root still exits 1 with
empty stdout, so absence still reads as absence.
Owner and member now take the same `find`, deliberately: one set of matching
semantics, rather than a glob for us and a find for them, which is how a bug ends
up reproducing on one account and not the other.
Verified on the owner path here against a live task file. The member path is
covered by `read-as-user.live.test.ts` — same opt-in shape as
`spawn-as-member.live.test.ts`, asserting the two identities DISAGREE, which a
world-readable tree would not satisfy. It cannot run on a machine with one
account, so it skips here and needs OFFICER_TEST_MEMBER on the host.
tsgo clean, frontend builds, 865 pass / 7 fail unchanged, 3 new skips.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@ import { dirname, join } from 'node:path';
|
|||||||
import { DATA_PATH } from '../../data-path';
|
import { DATA_PATH } from '../../data-path';
|
||||||
import {
|
import {
|
||||||
appendTextAs,
|
appendTextAs,
|
||||||
|
findPathAs,
|
||||||
listTranscriptsAs,
|
listTranscriptsAs,
|
||||||
readHeadAs,
|
readHeadAs,
|
||||||
readTailAs,
|
readTailAs,
|
||||||
@@ -10,6 +11,7 @@ import {
|
|||||||
removeAs,
|
removeAs,
|
||||||
type AsUser,
|
type AsUser,
|
||||||
} from '../../read-as-user';
|
} from '../../read-as-user';
|
||||||
|
import { runAsArgv } from '../../os-user';
|
||||||
|
|
||||||
// ── Claude session store (source of truth) ──
|
// ── Claude session store (source of truth) ──
|
||||||
// The `claude` CLI persists every session as a JSONL transcript at
|
// The `claude` CLI persists every session as a JSONL transcript at
|
||||||
@@ -694,20 +696,32 @@ export type BackgroundTaskDetail =
|
|||||||
| { kind: 'agent'; messages: ClaudeChatMessage[] }
|
| { kind: 'agent'; messages: ClaudeChatMessage[] }
|
||||||
| { kind: 'log'; text: string; truncated: boolean };
|
| { kind: 'log'; text: string; truncated: boolean };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Where a task's file is, looked for AS the account that owns it.
|
||||||
|
*
|
||||||
|
* This used to be a `Bun.Glob` scan running as the platform's own uid, and it worked for exactly one
|
||||||
|
* account: the owner. `/tmp/claude-<uid>` is `drwx------`, so for a member the scan matched nothing — and
|
||||||
|
* a glob that cannot traverse does not throw, it returns an empty iterator. Both callers read that as "no
|
||||||
|
* such task", which is why a member saw a job that ran fine, streamed no logs at all, and answered
|
||||||
|
* `{"stopped":false,"reason":"no such task"}` when they pressed Stop. One unreachable directory, two
|
||||||
|
* symptoms that look unrelated.
|
||||||
|
*
|
||||||
|
* The depths are the old glob's shape, kept as explicit bounds so neither `find` wanders: a task output is
|
||||||
|
* `<tmp>/claude-<uid>/<project-slug>/<session>/tasks/<id>.output` (5), a subagent transcript is
|
||||||
|
* `<projects>/<slug>/<session>/subagents/agent-<id>.jsonl` (4). `taskId` is `TASK_ID_RE`-checked above, so
|
||||||
|
* it carries no glob or shell metacharacter into either.
|
||||||
|
*/
|
||||||
function findTaskOutput(who: ChatIdentity, taskId: string): string | null {
|
function findTaskOutput(who: ChatIdentity, taskId: string): string | null {
|
||||||
|
// The member's `claude` runs under `--reset-env`, so its TMPDIR is the passwd default, not ours. Both
|
||||||
|
// resolve to /tmp on Linux; a server started with a TMPDIR of its own would look in the wrong place.
|
||||||
const tmpRoot = process.env.TMPDIR ?? '/tmp';
|
const tmpRoot = process.env.TMPDIR ?? '/tmp';
|
||||||
const candidates: [string, string][] = [
|
const candidates: [string, string[]][] = [
|
||||||
[tmpRoot, `claude-*/*/*/tasks/${taskId}.output`],
|
[tmpRoot, ['-mindepth', '5', '-maxdepth', '5', '-name', `${taskId}.output`]],
|
||||||
[claudeProjectsDir(who.home), `*/*/subagents/agent-${taskId}.jsonl`],
|
[claudeProjectsDir(who.home), ['-mindepth', '4', '-maxdepth', '4', '-name', `agent-${taskId}.jsonl`]],
|
||||||
];
|
];
|
||||||
for (const [root, pattern] of candidates) {
|
for (const [root, predicate] of candidates) {
|
||||||
try {
|
const hit = findPathAs(who.osUser, root, predicate);
|
||||||
for (const rel of new Bun.Glob(pattern).scanSync({ cwd: root, followSymlinks: false, onlyFiles: false })) {
|
if (hit) return hit;
|
||||||
return join(root, rel);
|
|
||||||
}
|
|
||||||
} catch {
|
|
||||||
/* root doesn't exist — try the next one */
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -743,7 +757,12 @@ export async function stopBackgroundTask(
|
|||||||
if (!found) return { stopped: false, reason: 'no such task' };
|
if (!found) return { stopped: false, reason: 'no such task' };
|
||||||
if (found.endsWith('.jsonl')) return { stopped: false, reason: 'subagents cannot be stopped this way' };
|
if (found.endsWith('.jsonl')) return { stopped: false, reason: 'subagents cannot be stopped this way' };
|
||||||
|
|
||||||
const proc = Bun.spawn(['fuser', found], { stdout: 'pipe', stderr: 'ignore' });
|
// As the member, for both halves. `fuser` reads `/proc/<pid>/fd`, which only the process's own user and
|
||||||
|
// root may open, so run as the platform it would find no pids for a member's job and report it already
|
||||||
|
// stopped. And the kill would then be cross-user: EPERM, silently swallowed by the catch below.
|
||||||
|
const asUser = (command: string[]) => (who.osUser ? runAsArgv(who.osUser, command) : command);
|
||||||
|
|
||||||
|
const proc = Bun.spawn(asUser(['fuser', found]), { stdout: 'pipe', stderr: 'ignore' });
|
||||||
const out = await new Response(proc.stdout).text();
|
const out = await new Response(proc.stdout).text();
|
||||||
await proc.exited;
|
await proc.exited;
|
||||||
|
|
||||||
@@ -754,6 +773,14 @@ export async function stopBackgroundTask(
|
|||||||
|
|
||||||
if (pids.length === 0) return { stopped: false, reason: 'already stopped' };
|
if (pids.length === 0) return { stopped: false, reason: 'already stopped' };
|
||||||
|
|
||||||
|
if (who.osUser) {
|
||||||
|
// One fork for the whole set rather than one per pid, and the kernel checks the member's own identity
|
||||||
|
// against processes that are already theirs.
|
||||||
|
const kill = Bun.spawn(asUser(['kill', '-TERM', ...pids.map(String)]), { stdout: 'ignore', stderr: 'ignore' });
|
||||||
|
await kill.exited;
|
||||||
|
return { stopped: true };
|
||||||
|
}
|
||||||
|
|
||||||
for (const pid of pids) {
|
for (const pid of pids) {
|
||||||
try {
|
try {
|
||||||
process.kill(pid, 'SIGTERM');
|
process.kill(pid, 'SIGTERM');
|
||||||
|
|||||||
@@ -0,0 +1,80 @@
|
|||||||
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
import { join } from 'node:path';
|
||||||
|
import { runAsArgv } from './os-user';
|
||||||
|
import { findPathAs } from './read-as-user';
|
||||||
|
|
||||||
|
// Can the platform LOCATE a member's file? Same shape as `spawn-as-member.live.test.ts`, and for the same
|
||||||
|
// reason: the failure only exists where a second Linux account does, which is the production host and no
|
||||||
|
// developer machine.
|
||||||
|
//
|
||||||
|
// ── The bug this pins ──
|
||||||
|
//
|
||||||
|
// `findTaskOutput` globbed `/tmp/claude-*/…` as the platform's own uid. `/tmp/claude-<uid>` is `drwx------`,
|
||||||
|
// so for a member the glob matched nothing — and a glob that cannot traverse does not throw, it returns an
|
||||||
|
// empty iterator. Downstream that read as "no such task", which a member saw as a background job that
|
||||||
|
// streamed no logs and answered `{"stopped":false,"reason":"no such task"}` on Stop.
|
||||||
|
//
|
||||||
|
// The assertion is the two identities DISAGREEING. A test that only checked `findPathAs(member, …)` finds
|
||||||
|
// the file would pass just as happily on a world-readable tree, which is not the situation that broke.
|
||||||
|
//
|
||||||
|
// ── Running it ──
|
||||||
|
//
|
||||||
|
// OFFICER_TEST_MEMBER=green bun test src/servers/read-as-user.live.test.ts
|
||||||
|
//
|
||||||
|
// Skips without it. A skipped run is not a pass; the log says which it was.
|
||||||
|
|
||||||
|
const osUser = process.env.OFFICER_TEST_MEMBER;
|
||||||
|
const live = Boolean(osUser);
|
||||||
|
|
||||||
|
const runAs = (user: string, command: string[]) => {
|
||||||
|
const result = Bun.spawnSync(runAsArgv(user, command), { stdout: 'pipe', stderr: 'pipe' });
|
||||||
|
if (result.exitCode !== 0) {
|
||||||
|
throw new Error(`as ${user}: ${new TextDecoder().decode(result.stderr).trim() || `exit ${result.exitCode}`}`);
|
||||||
|
}
|
||||||
|
return new TextDecoder().decode(result.stdout).trim();
|
||||||
|
};
|
||||||
|
|
||||||
|
describe.skipIf(!live)(`findPathAs (live, as ${osUser ?? 'nobody'})`, () => {
|
||||||
|
const user = osUser!;
|
||||||
|
// The real layout, at the real depth: <tmp>/claude-<uid>/<slug>/<session>/tasks/<id>.output
|
||||||
|
const uid = live ? runAs(user, ['id', '-u']) : '';
|
||||||
|
const taskId = 'blivetest01';
|
||||||
|
const dir = join('/tmp', `claude-${uid}`, '-live-test-slug', 'live-test-session', 'tasks');
|
||||||
|
const file = join(dir, `${taskId}.output`);
|
||||||
|
|
||||||
|
test('the member can be found where the platform is blind', () => {
|
||||||
|
// Created BY the member, so its modes are the ones `claude` would produce rather than ones we chose.
|
||||||
|
runAs(user, ['mkdir', '-p', dir]);
|
||||||
|
runAs(user, ['sh', '-c', 'printf "live\\n" > "$1"', '_', file]);
|
||||||
|
runAs(user, ['chmod', '700', join('/tmp', `claude-${uid}`)]);
|
||||||
|
|
||||||
|
try {
|
||||||
|
const predicate = ['-mindepth', '5', '-maxdepth', '5', '-name', `${taskId}.output`];
|
||||||
|
|
||||||
|
// As the member: found.
|
||||||
|
expect(findPathAs(user, '/tmp', predicate)).toBe(file);
|
||||||
|
|
||||||
|
// As the platform: NOT found — this is the half that used to be the only half.
|
||||||
|
expect(findPathAs(null, '/tmp', predicate)).toBeNull();
|
||||||
|
} finally {
|
||||||
|
runAs(user, ['rm', '-rf', join('/tmp', `claude-${uid}`, '-live-test-slug')]);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a task id that does not exist is null, not a throw', () => {
|
||||||
|
expect(findPathAs(user, '/tmp', ['-mindepth', '5', '-maxdepth', '5', '-name', 'no-such-task.output'])).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an unreadable directory in the walk does not discard a real match', () => {
|
||||||
|
// `find /tmp` from any identity hits another account's `drwx------` and exits 1 having already printed
|
||||||
|
// what it found. This is the regression that cost the first version of the fix: the match was located
|
||||||
|
// and then thrown away with the exit code.
|
||||||
|
runAs(user, ['mkdir', '-p', dir]);
|
||||||
|
runAs(user, ['sh', '-c', 'printf "live\\n" > "$1"', '_', file]);
|
||||||
|
try {
|
||||||
|
expect(findPathAs(user, '/tmp', ['-mindepth', '5', '-maxdepth', '5', '-name', `${taskId}.output`])).toBe(file);
|
||||||
|
} finally {
|
||||||
|
runAs(user, ['rm', '-rf', join('/tmp', `claude-${uid}`, '-live-test-slug')]);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -165,6 +165,38 @@ function runSync(osUser: string, command: string[]): string {
|
|||||||
return new TextDecoder().decode(result.stdout);
|
return new TextDecoder().decode(result.stdout);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The first path under `root` matching `predicate`, as the owner of the tree.
|
||||||
|
*
|
||||||
|
* Locating a file needs `x` on every directory above it, which the service user does not have on a
|
||||||
|
* member's tree — the header explains why the ACL cannot give it. `/tmp/claude-<uid>` is `drwx------`,
|
||||||
|
* so a glob run as the platform's own uid does not fail: it silently matches NOTHING, which reads
|
||||||
|
* downstream as "no such task".
|
||||||
|
*
|
||||||
|
* `predicate` is `find` arguments, so callers state their own depth bounds — an unbounded walk of `/tmp`
|
||||||
|
* or a member's home is not something to leave to chance.
|
||||||
|
*
|
||||||
|
* Owner and member take the same `find`, deliberately. One set of matching semantics beats a glob for us
|
||||||
|
* and a `find` for them, which is exactly the kind of split that makes a bug reproduce on one account and
|
||||||
|
* not the other.
|
||||||
|
*/
|
||||||
|
export function findPathAs(osUser: AsUser, root: string, predicate: string[]): string | null {
|
||||||
|
const command = ['find', root, ...predicate];
|
||||||
|
const result = Bun.spawnSync(osUser ? runAsArgv(osUser, command) : command, { stdout: 'pipe', stderr: 'ignore' });
|
||||||
|
|
||||||
|
// The EXIT CODE IS IGNORED, and that is the whole trick. `find` exits 1 when it could not descend into
|
||||||
|
// something — and searching `/tmp` from any identity is guaranteed to hit another account's
|
||||||
|
// `drwx------`. It still prints what it DID find, on stdout, before returning 1. Treating that as
|
||||||
|
// failure threw away the match: verified here, where the file was located and then discarded.
|
||||||
|
//
|
||||||
|
// A genuinely missing root also exits 1, with nothing on stdout, so absence still reads as absence.
|
||||||
|
const first = new TextDecoder()
|
||||||
|
.decode(result.stdout)
|
||||||
|
.split('\n')
|
||||||
|
.find((line) => line.length > 0);
|
||||||
|
return first ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
/** The whole file, as text. Throws on any failure, so existing `try`/`catch` around reads keeps working. */
|
/** The whole file, as text. Throws on any failure, so existing `try`/`catch` around reads keeps working. */
|
||||||
export function readTextAs(osUser: AsUser, path: string): string {
|
export function readTextAs(osUser: AsUser, path: string): string {
|
||||||
if (!osUser) return readFileSync(path, 'utf-8');
|
if (!osUser) return readFileSync(path, 'utf-8');
|
||||||
|
|||||||
Reference in New Issue
Block a user