mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
fix(sandbox): PR-B fold-ins — allow read ~/.claude + skip wrap under test mock
Live PI231 verification of d0dcd28 surfaced two real-runtime issues:
1. ~/.claude was denyRead, but claude CLI MUST read its own OAuth
credentials file to authenticate. Result: 100% of anthropic requests
on sandboxed PI231 failed with "Not logged in · Please run /login".
Live transcript:
{"event":"fallback_hop_error", "provider":"anthropic",
"error":"Not logged in · Please run /login"}
The cross-tenant risk for ~/.claude was a false security trade-off:
(a) ~/.claude is the spawn's OWN auth, not cross-tenant material
(all OLP clients share the same Anthropic OAuth — the file is
not multi-tenant secret)
(b) Real protection for the model accidentally exfiltrating creds
via tool_use is Phase 6c's --system-prompt suppressing tool
descriptions; sandbox-runtime can only blanket-deny a path, not
distinguish "claude reads creds" from "model emits Read tool"
Fix: remove ~/.claude from denyRead. Keep ~/.olp, ~/.ssh, ~/.config,
~/.codex (all genuinely cross-tenant).
Suite 44a (cat ~/.olp/keys.json MUST fail) unchanged — still proves
the core acceptance.
Suite 44c added (regression guard): in-sandbox cat ~/.claude/.credentials.json
MUST succeed when the file exists.
2. anthropic.mjs::_spawnAndStream called wrapSpawn() unconditionally,
including when a test had set __setSpawnImpl(mockFn). The wrap
rewrites bin to '/bin/sh -c <wrapped-string>' which breaks every HTTP
integration test that asserts on spawn args. Result on PI231 (sandbox
active): 16 production-shape tests failed because their mocks were
never called with the expected bin/args.
Fix: skip wrapSpawn when spawnImpl !== defaultSpawn (mock active).
Mocks don't exec, so isolation is meaningless there anyway. Real-CLI
spawns (production + Suite 44) still get wrapped.
Tests: 813 → 813 (44c is also PI231-gated, skips on Mac with file
absent; the fold-in does not change Mac test count).
Authority:
- @anthropic-ai/sandbox-runtime v0.0.52 — wrapWithSandbox() shell-string
return shape is the proximate cause of (2)
- Live PI231 2026-05-28 transcript: /v1/chat/completions returning
content=null + "Not logged in" from anthropic; OLP_E2E_SANDBOX=1 npm
test showing 16 prior pass tests now fail
- ADR 0014 § 4.1 PR-B acceptance criteria — the negative test (44a)
continues to pass; the false-positive denyRead (~/.claude) is corrected
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -928,15 +928,26 @@ async function* _spawnAndStream(irRequest, authContext, spawnImpl) {
|
|||||||
// ADR 0009 Amendment 1 § unchanged spawn args: only the spawn execution is
|
// ADR 0009 Amendment 1 § unchanged spawn args: only the spawn execution is
|
||||||
// wrapped — bin/args/env/NDJSON parsing are all unchanged from pre-PR-B.
|
// wrapped — bin/args/env/NDJSON parsing are all unchanged from pre-PR-B.
|
||||||
//
|
//
|
||||||
|
// 2026-05-28 PR-B fold-in: skip sandbox wrap when a custom spawnImpl is in
|
||||||
|
// use (test mode — __setSpawnImpl was called). Test mocks do not actually
|
||||||
|
// exec a binary, so sandbox isolation provides no protection there; the
|
||||||
|
// wrap only obscures the original bin/args from the mock's assertions and
|
||||||
|
// breaks every HTTP integration test that uses __setSpawnImpl + asserts
|
||||||
|
// on spawn args. wrapSpawn is for real-CLI spawns; Suite 44 exercises that
|
||||||
|
// path directly without going through this provider.
|
||||||
|
//
|
||||||
// Authority: @anthropic-ai/sandbox-runtime v0.0.52 wrapWithSandbox() API,
|
// Authority: @anthropic-ai/sandbox-runtime v0.0.52 wrapWithSandbox() API,
|
||||||
// ADR 0014 § PR-B, spike-anthropic.mjs (PI231 2026-05-28).
|
// ADR 0014 § PR-B, spike-anthropic.mjs (PI231 2026-05-28).
|
||||||
const wrapped = await wrapSpawn({
|
const usingMockSpawn = spawnImpl !== defaultSpawn;
|
||||||
bin,
|
const wrapped = usingMockSpawn
|
||||||
args,
|
? { bin, args, env, cwd: undefined, sandboxed: false }
|
||||||
env,
|
: await wrapSpawn({
|
||||||
cwd: undefined, // let manager assign ephemeral cwd
|
bin,
|
||||||
allowedDomains: ['api.anthropic.com', 'statsig.anthropic.com'],
|
args,
|
||||||
});
|
env,
|
||||||
|
cwd: undefined, // let manager assign ephemeral cwd
|
||||||
|
allowedDomains: ['api.anthropic.com', 'statsig.anthropic.com'],
|
||||||
|
});
|
||||||
|
|
||||||
// OCP server.mjs:542: spawn(CLAUDE, cliArgs, { env, stdio: ["pipe", "pipe", "pipe"] })
|
// OCP server.mjs:542: spawn(CLAUDE, cliArgs, { env, stdio: ["pipe", "pipe", "pipe"] })
|
||||||
const proc = spawnImpl(wrapped.bin, wrapped.args, {
|
const proc = spawnImpl(wrapped.bin, wrapped.args, {
|
||||||
|
|||||||
+22
-4
@@ -167,12 +167,30 @@ export async function bootstrapSandbox(opts = {}) {
|
|||||||
// denyRead: Absolute literal Linux paths per ADR 0014 § Pitfalls #2.
|
// denyRead: Absolute literal Linux paths per ADR 0014 § Pitfalls #2.
|
||||||
// No ~ or glob — ripgrep glob expansion is not used here to stay safe on
|
// No ~ or glob — ripgrep glob expansion is not used here to stay safe on
|
||||||
// both Linux (bwrap) and macOS (sandbox-exec profile).
|
// both Linux (bwrap) and macOS (sandbox-exec profile).
|
||||||
|
//
|
||||||
|
// 2026-05-28 PR-B fold-in: ~/.claude is NOT in denyRead. It contains the
|
||||||
|
// spawn's own OAuth credentials — claude CLI must read its own auth file
|
||||||
|
// to function. Denying read here causes "Not logged in" failures even
|
||||||
|
// though the operator has valid credentials present.
|
||||||
|
//
|
||||||
|
// The cross-tenant risk for ~/.claude is mitigated by Phase 6c's
|
||||||
|
// --system-prompt flag (ADR 0009 Amendment 1): the system prompt is
|
||||||
|
// fully replaced, suppressing the default tool descriptions that would
|
||||||
|
// otherwise tell the model it has Read/Bash. Without tool descriptions,
|
||||||
|
// the model is highly unlikely to emit tool_use even under prompt
|
||||||
|
// injection. Sandbox's contribution here is protecting OTHER auth
|
||||||
|
// material (other clients' OLP keys, SSH identity, other providers'
|
||||||
|
// tokens) — files claude CLI does NOT legitimately need.
|
||||||
|
//
|
||||||
|
// If we ever switch to a CLI that requires reading credentials.json
|
||||||
|
// AND also legitimately offers tool execution that surfaces those files
|
||||||
|
// (no known case today), this trade-off needs revisiting.
|
||||||
const denyRead = [
|
const denyRead = [
|
||||||
join(home, '.olp'), // OLP API keys + config
|
join(home, '.olp'), // OLP API keys + config — cross-tenant
|
||||||
join(home, '.claude'), // Claude OAuth credentials
|
join(home, '.ssh'), // SSH identity material — lateral movement
|
||||||
join(home, '.ssh'), // SSH identity material
|
|
||||||
join(home, '.config'), // Generic config dir (may contain tokens)
|
join(home, '.config'), // Generic config dir (may contain tokens)
|
||||||
join(home, '.codex'), // Codex config (PR-C will wrap codex)
|
join(home, '.codex'), // Codex config — other-provider auth (PR-C will wrap codex)
|
||||||
|
// NOT denied: ~/.claude — this spawn's own auth, breaks claude CLI if denied
|
||||||
];
|
];
|
||||||
|
|
||||||
// allowWrite: ephemeral spawn workspace only. mkdirSync at bootstrap.
|
// allowWrite: ephemeral spawn workspace only. mkdirSync at bootstrap.
|
||||||
|
|||||||
@@ -18222,6 +18222,11 @@ describe('Suite 43 — Phase 7 PR-B: lib/sandbox/manager.mjs', () => {
|
|||||||
|
|
||||||
const _RUN_SANDBOX_E2E = Boolean(process.env.OLP_E2E_SANDBOX);
|
const _RUN_SANDBOX_E2E = Boolean(process.env.OLP_E2E_SANDBOX);
|
||||||
|
|
||||||
|
// Suite 44c (fold-in 2026-05-28) needs `join`. statSync already imported at
|
||||||
|
// the Suite 17 boundary above. We just need a local `join` alias here since
|
||||||
|
// the file-top `join` was bound as `_pathJoinForSetup`.
|
||||||
|
const join = _pathJoinForSetup;
|
||||||
|
|
||||||
describe('Suite 44 — sandbox negative security test (PI231 only)', { skip: !_RUN_SANDBOX_E2E }, () => {
|
describe('Suite 44 — sandbox negative security test (PI231 only)', { skip: !_RUN_SANDBOX_E2E }, () => {
|
||||||
|
|
||||||
before(async () => {
|
before(async () => {
|
||||||
@@ -18325,4 +18330,48 @@ describe('Suite 44 — sandbox negative security test (PI231 only)', { skip: !_R
|
|||||||
assert.ok(stdout.includes('SANDBOX_PROOF'),
|
assert.ok(stdout.includes('SANDBOX_PROOF'),
|
||||||
`stdout must contain SANDBOX_PROOF; got: ${stdout.slice(0, 100)}`);
|
`stdout must contain SANDBOX_PROOF; got: ${stdout.slice(0, 100)}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('44c: in-sandbox spawn CAN read ~/.claude/.credentials.json (regression guard for fold-in 2026-05-28)', async () => {
|
||||||
|
// Phase 7 PR-B fold-in (commit pending): ~/.claude removed from denyRead.
|
||||||
|
// The spawn's own OAuth file MUST be readable, otherwise claude CLI fails
|
||||||
|
// with "Not logged in" — and live PI231 verification produces empty
|
||||||
|
// response bodies via the anthropic fallback path.
|
||||||
|
//
|
||||||
|
// The cross-tenant protection for ~/.claude relies on Phase 6c
|
||||||
|
// --system-prompt suppressing tool descriptions, not on sandbox denyRead.
|
||||||
|
// See manager.mjs comment block above denyRead for full rationale.
|
||||||
|
const { spawn: realSpawn } = await import('node:child_process');
|
||||||
|
const credPath = join(homedir(), '.claude', '.credentials.json');
|
||||||
|
|
||||||
|
// If the credentials file isn't present (e.g. dev machine without OAuth),
|
||||||
|
// this test is meaningless — skip the assertion but log.
|
||||||
|
let credStat;
|
||||||
|
try { credStat = statSync(credPath); } catch { credStat = null; }
|
||||||
|
if (!credStat) {
|
||||||
|
// No OAuth file present; cannot test read. Pass with note.
|
||||||
|
assert.ok(true, `No ${credPath} on this host — skipping read-allowed verification`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const wrapped = await wrapSpawn({
|
||||||
|
bin: 'cat',
|
||||||
|
args: [credPath],
|
||||||
|
env: { ...process.env },
|
||||||
|
cwd: undefined,
|
||||||
|
allowedDomains: [],
|
||||||
|
});
|
||||||
|
|
||||||
|
const { exitCode } = await new Promise((resolve) => {
|
||||||
|
const child = realSpawn(wrapped.bin, wrapped.args, {
|
||||||
|
env: wrapped.env,
|
||||||
|
cwd: wrapped.cwd,
|
||||||
|
stdio: ['ignore', 'pipe', 'pipe'],
|
||||||
|
});
|
||||||
|
child.on('exit', code => resolve({ exitCode: code }));
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.equal(exitCode, 0,
|
||||||
|
`cat ${credPath} inside sandbox exited with code ${exitCode} — sandbox is denying read on a path the spawn legitimately needs. ` +
|
||||||
|
`~/.claude must NOT be in denyRead per the 2026-05-28 fold-in.`);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user