Public: anyone can read this; the crew can change it.
A builder's sandbox runs `bun install` and `bun x tsc --noEmit`: bun gets a writable temp directory #4151
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4136-a-builder-s-sandbox-runs-bun-install-and → main 2 commits fork 3 threads · 3 unresolved
src/crew/sandbox.ts +17
| @@ -60,6 +60,16 @@ export interface SandboxPolicy { | ||
| 60 | 60 | readonly readAllow: readonly string[]; |
| 61 | 61 | } |
| 62 | 62 | |
| 63 | /** | |
| 64 | * The one directory under the run's temp directory the sandboxed shell may | |
| 65 | * write, and the `TMPDIR` it re-exports: `<tmpDir>/claude-<uid>`. | |
| 66 | */ | |
| 67 | export function shellTmpDir(policy: SandboxPolicy): string | undefined { | |
| 68 | return policy.tmpDir === undefined | |
| 69 | ? undefined | |
| 70 | : join(policy.tmpDir, `claude-${process.getuid?.() ?? 0}`); | |
| 71 | } | |
| 72 | ||
| 63 | 73 | /** |
| 64 | 74 | * The run's env for its repository's install cache: the directory made, and |
| 65 | 75 | * bun told to use it. A policy with no cache (no repository) changes nothing. |
outdated
High: inside the sandbox, |
||
| @@ -68,6 +78,13 @@ export function installCacheEnv( | ||
| 68 | 78 | policy: SandboxPolicy, |
| 69 | 79 | env: Record<string, string>, |
| 70 | 80 | ): void { |
| 81 | // Bun reads BUN_TMPDIR before TMPDIR. The run's temp directory is read-only | |
| 82 | // to the shell; only `shellTmpDir` under it is writable. | |
| 83 | const tmp = shellTmpDir(policy); | |
| 84 | if (tmp !== undefined) { | |
| 85 | mkdirSync(tmp, { recursive: true }); | |
| 86 | env["BUN_TMPDIR"] = tmp; | |
| 87 | } | |
| 71 | 88 | if (policy.installCache === undefined) return; |
| 72 | 89 | mkdirSync(policy.installCache, { recursive: true }); |
| 73 | 90 | env["BUN_INSTALL_CACHE_DIR"] = policy.installCache; |
test/crew-install-cache.test.ts +13
| @@ -70,6 +70,19 @@ describe("the shared install cache", () => { | ||
| 70 | 70 | expect(none).toEqual({}); |
| 71 | 71 | }); |
| 72 | 72 | |
| 73 | test("bun's temp directory is the one the sandboxed shell can write", () => { | |
| 74 | const p = policy(data, "r1"); | |
| 75 | const env: Record<string, string> = {}; | |
| 76 | installCacheEnv(p, env); | |
| 77 | const writable = join(p.tmpDir!, `claude-${process.getuid?.() ?? 0}`); | |
| 78 | // The run's temp directory itself is read-only in the shell; only the | |
| 79 | // claude-<uid> directory under it is bound writable. | |
| 80 | expect(env["BUN_TMPDIR"]).toBe(writable); | |
outdated
This is a tautology: it is always true once line 77 passes. The test needs to assert that |
||
| 81 | expect(env["BUN_TMPDIR"]).not.toBe(p.tmpDir!); | |
| 82 | expect(existsSync(writable)).toBe(true); | |
| 83 | expect(p.denyRead).not.toContain(p.tmpDir!); | |
| 84 | }); | |
| 85 | ||
| 73 | 86 | test("removing a repository clears its cache and only its", async () => { |
| 74 | 87 | await removeInstallCache(data, "r1"); |
| 75 | 88 | expect(existsSync(installCacheDir(data, "r1"))).toBe(false); |
Nit, not blocking:
tempDeniedbuilds the sameclaude-${process.getuid?.() ?? 0}name a few lines below. A smallclaudeTmpName()helper shared by both would keep the two from drifting.