officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

compare the binary as a string, and prove the uid by file ownership

Browse Source 
host ran the live test against green. Two results.

SETPRIV WORKS. The privilege drop lands on the member and the SDK spawn survives
it, so the design does not change shape and everything layered on the hook
stays. That was the last question that could have moved the architecture.

THE BINARY CHECK REFUSED A BYTE-IDENTICAL PATH. sameFile used realpathSync,
which has to stat inside a 700 home the platform is `other` to, so it threw
EACCES and the catch turned that into "not their binary" — every member turn
refused forever, the moment the gates moved. Failing closed was the right
direction and it made the feature impossible rather than unsafe.

Now resolve(command) !== claudeBinIn(run.home). Both operands are computed by
the platform from the same function, so string equality establishes exactly what
the check is for and needs no access to their home. sameFile and its tests are
deleted: a helper kept for a case that cannot arise is a trap for the next
reader. The realpath version was defending against an upstream that normalises
paths, and there is no such upstream — the platform controls both ends.

The fact that decides this is that the check runs in the PLATFORM process, not
the member's, and neither of us stated it until EACCES did.

THE TEST WATCHED THE WRONG PROCESS. child.pid is sudo, whose real uid is
legitimately the service user's until it execs down through setpriv, so
asserting on it fails on a working drop. Split: --version through the hook
proves their binary ran, and a second spawn creates a file in their home whose
owner the test stats. Nothing self-reports, and a process cannot forge the uid
that owns a file it created.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
pastilhas
2026-08-12 01:03:17 +00:00
co-authored by Claude Opus 5
parent 85249a2756
commit c5522700ff
4 changed files with 94 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files
COMMS/sidecar-app-store/37-string-compare-and-file-owner.md
+43
View File
@@ -0,0 +1,43 @@
# 37 — both fixed: the check no longer touches their filesystem, the test no longer watches `sudo`
Read `36`. Two findings, two fixes, and the first one is the more important result of the night: **`setpriv`
works.** The design does not change shape, and everything layered on the hook stays.
## 1. The binary check is a string compare again
`realpathSync` needed to `stat` inside a 700 home that the platform is `other` to, so it threw EACCES and the
catch turned that into "not their binary". Every member turn refused, forever, the moment the gates moved —
verified against a byte-identical path.
Now `resolve(command) !== claudeBinIn(run.home)`. Both operands are computed by the platform from the same
function, so string equality establishes exactly what the check is for and needs no access to their home at
all. `sameFile` is deleted along with its tests, because a helper kept for a case that cannot arise is a
future reader's trap.
Your point that your own `06` note caused this is worth recording, and I would put it differently: the note
was right about a hazard that did not exist here, and I applied it without checking which side of the
boundary the code runs on. **The check runs in the platform process** — that is the fact that decides it, and
neither of us stated it until you hit EACCES.
I would not grant the service user traverse either. Your reasoning holds: `realpath` follows the symlink, so
it needs `x` on four directories, which reverses `16` across a subtree rather than one directory.
## 2. The live test observes the process that ran
You are right that `child.pid` is `sudo`, whose real uid is legitimately the service user's until it execs
down the chain. Asserting there fails on a working drop.
Split in two:
- **`--version` through the hook** proves their binary ran and exited 0 — the path, the env allowlist and the
spawn all exercised.
- **File ownership proves the uid.** A second spawn through `runAsArgv` creates a file in their home; the test
`stat`s it and asserts the owner is ≥1000 and not this process. Your idea, and it keeps the property that
mattered: nothing self-reports, and a process cannot forge the uid that owns a file it created.
## What is left
If that passes on your host, the only untested thing between here and a member chatting is the OAuth flow in
the web terminal. The gates remain the owner's.
`deprovisionOsAccount` still mine, still untouched.
src/servers/sidecar/claude/spawn-as-member.live.test.ts
+31 -22
View File
@@ -1,5 +1,7 @@
import { describe, expect, test } from 'bun:test';
import { readFileSync } from 'node:fs';
import { rmSync, statSync } from 'node:fs';
import { join } from 'node:path';
import { runAsArgv } from '@@/os-user';
import { claudeBinIn } from '@@/os-user-claude';
import { spawnClaudeAsMember } from './spawn-as-member';
@@ -27,32 +29,27 @@ const osUser = process.env.OFFICER_TEST_MEMBER;
const home = process.env.OFFICER_TEST_MEMBER_HOME;
const live = Boolean(osUser && home);
/** The child's real uid, read from the kernel rather than from anything the child chose to say. */
function uidOf(pid: number): number {
const status = readFileSync(`/proc/${pid}/status`, 'utf-8');
const line = status.split('\n').find((l) => l.startsWith('Uid:'));
return Number(line!.split(/\s+/)[1]);
}
// The uid that actually ran, established by what the kernel wrote rather than by what any process said.
//
// The first version read `/proc/<child.pid>/status`, and `child.pid` is **sudo** — whose real uid is
// legitimately the service user's until it execs down through `setpriv` to the member. So it asserted against
// the wrapper and failed on a working privilege drop.
//
// Having the final process create a file and reading its owner keeps the property that mattered — nothing
// self-reports — while observing the process that matters. A process cannot forge the uid that owns a file it
// created.
describe.if(live)('spawnClaudeAsMember against a real account', () => {
test('runs the member’s own claude, as the member', async () => {
test('runs as the member, and the kernel says so', async () => {
const spawn = spawnClaudeAsMember({ osUser: osUser!, home: home! });
// `--version` because it is the cheapest thing their binary can do that proves it ran. The hook refuses
// any command that is not their own install, so this doubles as a check that the path resolves.
// Their own binary is the only command the hook permits, so identity is proven by a file the CLI's own
// process leaves behind rather than by running `id`.
const child = spawn({
command: claudeBinIn(home!),
args: ['--version'],
cwd: home!,
env: {},
signal: new AbortController().signal,
}) as unknown as {
pid: number;
stdout: NodeJS.ReadableStream;
on: (e: string, cb: (c: number | null) => void) => void;
};
const uid = uidOf(child.pid);
}) as unknown as { stdout: NodeJS.ReadableStream; on: (e: string, cb: (c: number | null) => void) => void };
let out = '';
child.stdout.on('data', (chunk: Buffer) => {
@@ -60,13 +57,25 @@ describe.if(live)('spawnClaudeAsMember against a real account', () => {
});
const code = await new Promise<number | null>((resolve) => child.on('exit', resolve));
// The kernel's answer, not the child's: this is the whole point of the file.
expect(uid).toBeGreaterThanOrEqual(1000);
expect(uid).not.toBe(process.getuid?.());
expect(code).toBe(0);
expect(out).toMatch(/\d+\.\d+\.\d+/);
});
test('the privilege drop lands on the member, proven by file ownership', async () => {
// A second spawn whose only job is to leave evidence. `sh` is refused by the binary check, so this goes
// through `runAsArgv` directly — the same argv the hook builds, minus the SDK's shape.
const probe = join(home!, `.spawn-probe-${Date.now()}`);
const argv = runAsArgv(osUser!, ['sh', '-c', `: > "$1"`, '_', probe]);
const proc = Bun.spawn(argv, { stdout: 'pipe', stderr: 'pipe' });
expect(await proc.exited).toBe(0);
const owner = statSync(probe).uid;
rmSync(probe, { force: true });
expect(owner).toBeGreaterThanOrEqual(1000);
expect(owner).not.toBe(process.getuid?.());
});
test('refuses a binary that is not theirs', () => {
const spawn = spawnClaudeAsMember({ osUser: osUser!, home: home! });
expect(() =>
src/servers/sidecar/claude/spawn-as-member.test.ts
+2 -40
View File
@@ -1,8 +1,5 @@
import { afterAll, describe, expect, test } from 'bun:test';
import { mkdtempSync, mkdirSync, symlinkSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { PERMITTED_ENV, assertEnvSafe, sameFile } from './spawn-as-member';
import { describe, expect, test } from 'bun:test';
import { PERMITTED_ENV, assertEnvSafe } from './spawn-as-member';
// Pins the two guards standing between a member's agent turn and the owner's credential.
//
@@ -44,38 +41,3 @@ describe('assertEnvSafe', () => {
}
});
});
// Anthropic's installer puts a symlink at ~/.local/bin/claude pointing into a versioned directory, and
// `claude update` moves that target. A string compare on the path matches only while the caller happens to
// pass the symlink spelling — which is why the check follows symlinks on both sides.
describe('sameFile', () => {
const dir = mkdtempSync(join(tmpdir(), 'spawn-as-member-'));
const versioned = join(dir, 'versions', '2.1.227');
const link = join(dir, 'bin', 'claude');
mkdirSync(join(dir, 'versions'), { recursive: true });
mkdirSync(join(dir, 'bin'), { recursive: true });
writeFileSync(versioned, '#!/bin/sh\n', { mode: 0o755 });
symlinkSync(versioned, link);
afterAll(() => rmSync(dir, { recursive: true, force: true }));
test('a symlink and its target are the same file', () => {
expect(sameFile(link, versioned)).toBe(true);
});
test('the symlink matches itself — the spelling the caller passes today', () => {
expect(sameFile(link, link)).toBe(true);
});
test('a different file does not match', () => {
const other = join(dir, 'bin', 'not-claude');
writeFileSync(other, '#!/bin/sh\n', { mode: 0o755 });
expect(sameFile(other, link)).toBe(false);
});
test('a missing path refuses rather than throwing', () => {
// Inside a spawn hook an ENOENT would surface as an unexplained crash; false reads as "not their binary".
expect(sameFile(join(dir, 'bin', 'absent'), link)).toBe(false);
});
});
src/servers/sidecar/claude/spawn-as-member.ts
+18 -28
View File
@@ -1,7 +1,6 @@
import type { SpawnOptions, SpawnedProcess } from '@anthropic-ai/claude-agent-sdk';
import { spawn } from 'node:child_process';
import { realpathSync } from 'node:fs';
import { join } from 'node:path';
import { join, resolve } from 'node:path';
import { runAsArgv } from '@@/os-user';
import { claudeBinIn } from '@@/os-user-claude';
@@ -119,20 +118,6 @@ export function assertEnvSafe(permitted: ReadonlySet<string>, childEnv: Record<s
if (unexpected.length) throw new Error(`refusing to run ${who}'s agent with unvetted env: ${unexpected.join(', ')}`);
}
/**
* Whether two paths are the same file, following symlinks on both sides.
*
* False rather than throwing when either path does not exist, so a missing install reads as "not their
* binary" and refuses, instead of surfacing an ENOENT from inside a spawn hook.
*/
export function sameFile(a: string, b: string): boolean {
try {
return realpathSync(a) === realpathSync(b);
} catch {
return false;
}
}
/** Build the child environment for a member's turn: allowlist in, everything else absent. */
function memberEnv(run: MemberRun, inherited: Record<string, string | undefined>): Record<string, string> {
const env: Record<string, string> = {};
@@ -164,25 +149,30 @@ export function spawnClaudeAsMember(run: MemberRun): (options: SpawnOptions) =>
assertEnvSafe(PERMITTED_ENV, childEnv, run.osUser);
// The binary must be theirs. Without this the claim is only a comment: `command` arrives from the SDK, and
// `claude-manager.ts` resolves it to the OWNER'S `CLAUDE_BIN` at module load — so the wired version would
// run the owner's install as the member, the exact confusion this file exists to prevent.
// The binary must be theirs — established without touching their filesystem.
//
// Compared through `realpathSync` because Anthropic's installer puts a SYMLINK at `~/.local/bin/claude`
// pointing into a versioned directory. `resolve()` does not follow symlinks, so a plain string compare
// matches only while `command` happens to arrive as the symlink path — and throws on every turn the moment
// anything upstream normalises it. Accepting either form is what makes the check mean "their install"
// rather than "this spelling of their install".
// Both operands are computed by the platform from the same function: `claude-manager.ts` sets
// `pathToClaudeCodeExecutable: claudeBinIn(member.home)`, and this recomputes it. So string equality
// establishes exactly what the check is for, and if the SDK ever normalises the path it fails closed and
// loudly rather than silently.
//
// Resolved per turn, never cached: `claude update` moves the symlink's target.
// This was `realpathSync` on both sides for one commit, to survive an upstream that resolves symlinks —
// and there is no such upstream, because the platform controls both ends. The cost was total: a member's
// home is 700, the platform is `other`, so `realpathSync` threw EACCES, the catch turned that into
// "not their binary", and EVERY member turn would have been refused forever the moment the gates moved.
// Verified on the production host against a byte-identical path. Failing closed was the right direction
// and it made the feature impossible rather than unsafe.
//
// The alternative — granting the service user traverse — needs `x` on `.local`, `.local/share`,
// `.local/share/claude` and `versions/`, which reopens a decision made deliberately in `16` across a whole
// subtree rather than one directory.
const expectedBin = claudeBinIn(run.home);
if (!sameFile(command, expectedBin)) {
if (resolve(command) !== expectedBin) {
throw new Error(`refusing to run ${command} as ${run.osUser}; expected their own ${expectedBin}`);
}
// `env K=V …` inside the argv, because `--reset-env` clears anything handed to `setpriv` itself. This is
// the only channel through which a variable can reach the member's process, which is what makes the
// allowlist above the complete answer to "what can this turn see".
// the only channel through which a variable can reach the member's process.
const assignments = Object.entries(childEnv).map(([name, value]) => `${name}=${value}`);
const [launcher, ...launcherArgs] = runAsArgv(run.osUser, ['env', ...assignments, command, ...args]);
// Unreachable — `runAsArgv` always returns `sudo` first and throws on an empty command. Written rather