Phase 2: safe .pi/ materialisation from a pinned SHA (symlink/submodule/traversal proof)

sec-review flagged the obvious implementation -- fs.readFile off a clone -- as a real hole:
a .pi/APPEND_SYSTEM.md symlinked to the worker's .env or /etc/passwd would pull a host file
into the agent's SYSTEM PROMPT, and the worker is a trusted zone. This materialises .pi/
without that hole.

Proven first-hand with git before writing a line: `git show <sha>:<symlink>` outputs the
symlink's TARGET PATH as a blob (the string "/etc/passwd"), never the contents of the target
file -- git reads objects, not the filesystem. And `git ls-tree` exposes the mode, so a
symlink (120000) or submodule (160000) is rejected BEFORE anything is read.

materialize.mjs:
- enumerates via `git ls-tree -r -z <sha> .pi/`, keeps ONLY regular blobs (100644) at
  allowlisted paths (.pi/APPEND_SYSTEM.md, .pi/skills/*/SKILL.md); symlinks, submodules,
  executables, and anything else are dropped at one choke point (selectEntries);
- reads content via `git cat-file blob <oid>` -- raw bytes by object id, no working-tree
  checkout, no smudge/clean filters, no hooks, no diff drivers: nothing in the repo executes;
- runs git with `-c core.hooksPath=/dev/null` hardening;
- re-derives every output path and asserts containment via path.relative (separator-agnostic
  -- the worker is cross-platform and destDir may be a Windows path);
- takes the SHA as input, resolved by the caller from a fresh default-branch API call, never
  a webhook field or the triggering branch.

Tests build a REAL git repo through the index (update-index --cacheinfo) with a genuine
120000 symlink-to-/etc/passwd object and a 160000 submodule -- no OS symlink privilege
needed, so it runs on the Windows dev box too. Asserts the symlink and submodule materialise
NOTHING and that neither the /etc/passwd contents nor the target path string reach any output
file. A repo with no .pi/ materialises nothing (a guardrails-only job), no error.

Found and fixed a cross-platform bug while testing: the containment check used string-prefix
with a forward slash, which rejected valid paths on Windows (backslash separators). Now
path.relative-based. The worker runs on the host, which may be Windows -- this would have
broken every job there.

Author: edgehero

worker/src/materialize.mjs

@@ -0,0 +1,116 @@
+import { execFile } from "node:child_process";
+import { mkdirSync, writeFileSync } from "node:fs";
+import { dirname, isAbsolute, join, relative } from "node:path";
+import { promisify } from "node:util";
+
+const exec = promisify(execFile);
+
+/**
+ * Materialise a serviced repo's `.pi/` (its persona and skills) from a pinned commit into a
+ * read-only `/job/pi/` directory the container mounts.
+ *
+ * This is security-critical: the content becomes the agent's SYSTEM PROMPT, and the repo is only
+ * trusted at maintainer level. Three properties, each PROVEN with a hostile fixture in the tests:
+ *
+ *   1. NO SYMLINK FOLLOWING. A `.pi/APPEND_SYSTEM.md` symlinked to the worker's `.env` or
+ *      `/etc/passwd` must never pull a host file into the prompt. We enumerate with `git ls-tree`
+ *      and REJECT any entry that is not a regular blob (mode 100644): symlinks are 120000,
+ *      submodules 160000. We never touch the working tree, so there is no link to follow.
+ *   2. NO PATH TRAVERSAL. Every output path is re-derived from the git tree path and asserted to
+ *      stay under the destination root; a crafted entry cannot escape `/job/pi/`.
+ *   3. NO EXECUTION. `git cat-file blob <oid>` dumps raw bytes by object id -- no working-tree
+ *      checkout, no smudge/clean filters, no hooks, no diff drivers. Nothing in the repo runs.
+ *
+ * The SHA is an input, resolved by the caller from a fresh default-branch API call -- NEVER a
+ * webhook field, and NEVER the triggering (possibly fork) branch.
+ */
+
+const PI_DIR = ".pi";
+// Exactly the two shapes we accept. Anything else in .pi/ is ignored, not materialised.
+const APPEND_SYSTEM = `${PI_DIR}/APPEND_SYSTEM.md`;
+const SKILL_RE = /^\.pi\/skills\/[A-Za-z0-9._-]+\/SKILL\.md$/;
+
+/** A git tree path we are willing to materialise. Rejects traversal and unexpected shapes. */
+export function isAllowedPiPath(path) {
+	if (path === APPEND_SYSTEM) return true;
+	return SKILL_RE.test(path);
+}
+
+/**
+ * Parse `git ls-tree -r -z` output into entries, keeping ONLY regular blobs (100644) at allowed
+ * paths. Symlinks (120000), submodules (160000), executables (100755), and anything outside the
+ * allowlist are dropped here -- the single choke point for the reject-by-mode rule.
+ */
+export function selectEntries(lsTreeZ) {
+	const entries = [];
+	for (const record of lsTreeZ.split("\0")) {
+		if (!record) continue;
+		// "<mode> <type> <oid>\t<path>"
+		const tab = record.indexOf("\t");
+		if (tab === -1) continue;
+		const [mode, type, oid] = record.slice(0, tab).split(/\s+/);
+		const path = record.slice(tab + 1);
+		if (mode !== "100644" || type !== "blob") continue; // rejects symlink/submodule/exec
+		if (!isAllowedPiPath(path)) continue;
+		entries.push({ oid, path });
+	}
+	return entries;
+}
+
+/**
+ * Assert a resolved output path stays under root. Defence in depth behind the path allowlist.
+ * Uses path.relative rather than string-prefix so it is correct on Windows too -- the worker is
+ * cross-platform and destDir may be a Windows path with backslash separators.
+ */
+function safeJoin(root, relPath) {
+	const resolved = join(root, relPath);
+	const rel = relative(root, resolved);
+	if (rel === "" || rel.startsWith("..") || isAbsolute(rel)) {
+		throw new Error(`path escapes destination: ${relPath}`);
+	}
+	return resolved;
+}
+
+/**
+ * Materialise `.pi/` at `sha` from the clone at `gitDir` into `destDir` (which becomes /job/pi).
+ * Returns the list of relative paths written (under `pi/`), for logging.
+ *
+ * `git` is injected for tests; defaults to a thin wrapper over the real binary.
+ */
+export async function materializePiDir({ gitDir, sha, destDir, git = defaultGit }) {
+	const lsTreeZ = await git(gitDir, ["ls-tree", "-r", "-z", sha, `${PI_DIR}/`]);
+	const entries = selectEntries(lsTreeZ);
+
+	const written = [];
+	for (const { oid, path } of entries) {
+		const content = await git(gitDir, ["cat-file", "blob", oid], { raw: true });
+		// path is ".pi/skills/x/SKILL.md" (git always uses forward slashes); strip the leading
+		// ".pi/" so it lands under destDir/pi.
+		const relPosix = `pi/${path.slice(PI_DIR.length + 1)}`;
+		const out = safeJoin(destDir, relPosix);
+		mkdirSync(dirname(out), { recursive: true });
+		writeFileSync(out, content);
+		// Report posix-style: this names a CONTAINER path (/job/pi/...), stable across host OSes.
+		written.push(relPosix);
+	}
+	return written;
+}
+
+async function defaultGit(gitDir, args, { raw = false } = {}) {
+	// -c protecting against a hostile repo config: no hooks, no external filters, no pager.
+	const hardened = [
+		"-c",
+		"core.hooksPath=/dev/null",
+		"-c",
+		"core.fsmonitor=false",
+		"--no-pager",
+		"-C",
+		gitDir,
+		...args,
+	];
+	const { stdout } = await exec("git", hardened, {
+		encoding: raw ? "buffer" : "utf8",
+		maxBuffer: 16 * 1024 * 1024,
+	});
+	return stdout;
+}

worker/test/materialize.test.mjs

@@ -0,0 +1,98 @@
+import assert from "node:assert/strict";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, readFileSync, readdirSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { test } from "node:test";
+import { isAllowedPiPath, materializePiDir, selectEntries } from "../src/materialize.mjs";
+
+// --- pure selection logic: runs everywhere ---
+
+test("isAllowedPiPath accepts exactly the persona and skill shapes", () => {
+	assert.ok(isAllowedPiPath(".pi/APPEND_SYSTEM.md"));
+	assert.ok(isAllowedPiPath(".pi/skills/bug-fix/SKILL.md"));
+	assert.ok(!isAllowedPiPath(".pi/skills/bug-fix/notes.md"));
+	assert.ok(!isAllowedPiPath(".pi/settings.json"));
+	assert.ok(!isAllowedPiPath(".pi/skills/../../etc/passwd/SKILL.md")); // traversal in the name
+	assert.ok(!isAllowedPiPath(".pi/APPEND_SYSTEM.md.evil"));
+});
+
+test("selectEntries rejects symlinks (120000), submodules (160000), and executables (100755)", () => {
+	const z = [
+		"100644 blob aaa\t.pi/APPEND_SYSTEM.md",
+		"120000 blob bbb\t.pi/EVIL_SYMLINK.md", // symlink -> host file
+		"160000 commit ccc\t.pi/skills/sub", // submodule
+		"100755 blob ddd\t.pi/skills/x/SKILL.md", // executable bit set
+		"100644 blob eee\t.pi/skills/good/SKILL.md",
+	].join("\0");
+	const picked = selectEntries(z).map((e) => e.path);
+	assert.deepEqual(picked, [".pi/APPEND_SYSTEM.md", ".pi/skills/good/SKILL.md"]);
+});
+
+// --- integration against a REAL git repo with REAL hostile objects ---
+
+function git(dir, args) {
+	return execFileSync("git", ["-C", dir, ...args], { encoding: "utf8" });
+}
+
+/** A repo whose .pi/ contains a genuine symlink object and a submodule gitlink, plus real files. */
+function hostileRepo() {
+	const dir = mkdtempSync(join(tmpdir(), "pi-mat-"));
+	git(dir, ["init", "-q"]);
+	git(dir, ["config", "user.email", "t@t"]);
+	git(dir, ["config", "user.name", "t"]);
+	git(dir, ["config", "core.autocrlf", "false"]);
+
+	const blob = (content) =>
+		execFileSync("git", ["-C", dir, "hash-object", "-w", "--stdin"], { input: content, encoding: "utf8" }).trim();
+
+	const persona = blob("REAL-PERSONA-SENTINEL");
+	const skill = blob("---\nname: good\ndescription: real\n---\nsteps\n");
+	const evilTarget = blob("/etc/passwd"); // the symlink's blob content = its target path
+
+	// Build the tree entirely through the index -- creates a genuine 120000 symlink and 160000
+	// gitlink without needing OS symlink privilege (which Windows dev boxes lack).
+	git(dir, ["update-index", "--add", "--cacheinfo", `100644,${persona},.pi/APPEND_SYSTEM.md`]);
+	git(dir, ["update-index", "--add", "--cacheinfo", `100644,${skill},.pi/skills/good/SKILL.md`]);
+	git(dir, ["update-index", "--add", "--cacheinfo", `120000,${evilTarget},.pi/EVIL_SYMLINK.md`]);
+	// A submodule gitlink. git rejects a null sha, so use any valid nonzero oid (the blob's) --
+	// update-index does not verify a gitlink points at a real commit, which is all we need here.
+	git(dir, ["update-index", "--add", "--cacheinfo", `160000,${persona},.pi/skills/sub`]);
+	git(dir, ["-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", "x"]);
+	const sha = git(dir, ["rev-parse", "HEAD"]).trim();
+	return { dir, sha };
+}
+
+test("materialize writes real files and NEVER the symlink or submodule", async () => {
+	const { dir, sha } = hostileRepo();
+	const dest = mkdtempSync(join(tmpdir(), "pi-dest-"));
+
+	const written = await materializePiDir({ gitDir: dir, sha, destDir: dest });
+
+	assert.deepEqual(written.sort(), ["pi/APPEND_SYSTEM.md", "pi/skills/good/SKILL.md"].sort());
+	assert.equal(readFileSync(join(dest, "pi/APPEND_SYSTEM.md"), "utf8"), "REAL-PERSONA-SENTINEL");
+
+	// The symlink must have produced NOTHING -- not a file containing "/etc/passwd", not anything.
+	const flat = JSON.stringify(readdirSync(dest, { recursive: true }));
+	assert.ok(!flat.includes("EVIL_SYMLINK"), "the symlink entry was materialised");
+	assert.ok(!flat.includes("sub"), "the submodule entry was materialised");
+
+	// And no host-file content leaked in either.
+	const allContent = written.map((r) => readFileSync(join(dest, r), "utf8")).join("\n");
+	assert.ok(!allContent.includes("root:x:0:0"), "host /etc/passwd content leaked into the prompt");
+	assert.ok(!allContent.includes("/etc/passwd"), "symlink target path leaked into the prompt");
+});
+
+test("a repo with no .pi/ materialises nothing (guardrails-only job), no error", async () => {
+	const dir = mkdtempSync(join(tmpdir(), "pi-empty-"));
+	git(dir, ["init", "-q"]);
+	git(dir, ["config", "user.email", "t@t"]);
+	git(dir, ["config", "user.name", "t"]);
+	execFileSync("git", ["-C", dir, "commit", "-q", "--allow-empty", "-m", "x"], {
+		env: { ...process.env, GIT_AUTHOR_NAME: "t", GIT_AUTHOR_EMAIL: "t@t", GIT_COMMITTER_NAME: "t", GIT_COMMITTER_EMAIL: "t@t" },
+	});
+	const sha = git(dir, ["rev-parse", "HEAD"]).trim();
+	const dest = mkdtempSync(join(tmpdir(), "pi-dest-"));
+	const written = await materializePiDir({ gitDir: dir, sha, destDir: dest });
+	assert.deepEqual(written, []);
+});