From 7b5e0b093d0590e25a3fa541d856fe691f1cf8cb Mon Sep 17 00:00:00 2001 From: Kai Date: Wed, 26 Aug 2026 22:57:52 -0700 Subject: [PATCH] feat(shared): pi-guard-base, reusable isolation primitives for scenario extensions Extracted from pi-workspaces/memo-inbox/.pi/extensions/memo-guard.ts, which has enforced these patterns in production since 2026-07. Exports: - inside() / safeRealPath() / makePathResolver(): path containment that resolves symlinks before checking, so a link inside an allowed root cannot escape it - registerRestrictedRead(): a path-restricted 'read' that shadows the built-in. Required rather than optional: pi emits the skills block only when a tool named 'read' is active and skill bodies load through it, while the built-in 'read' accepts absolute paths and could reach the service's credential files - installGuard(): the two capability layers, setActiveTools plus a tool_call block, re-asserted on resources_discover as well as session_start - truncate(): byte-aware truncation ahead of pi's 50 KB / 2000 line caps - registerBridgeTools() / fetchBridgeSpecs(): loopback HTTP bridge, with the baseUrl asserted to be loopback. Since registerTool accepts a plain JSON Schema object, the backend can own the schema instead of a drifting copy Verified against a real pi process with zero model tokens (shared/extensions/tests/run-guard-checks.sh, 14 assertions): active tools are exactly the declared set, the read override wins with source=cli, the skills section is present and contains only the scenario's own skill, and reads of an outside file, a ../ traversal and an absolute path to ~/.config/curator/curator.env are all denied. The deny-path fixture is named .txt and renamed to .env only inside the temp work directory, because verify-no-secrets.sh correctly refused to track a file called *.env.sample. --- shared/extensions/pi-guard-base.ts | 409 ++++++++++++++++++ .../fixtures/.pi/skills/guard-skill/SKILL.md | 5 + shared/extensions/tests/fixtures/AGENTS.md | 1 + .../tests/fixtures/secret-lookalike.txt | 1 + shared/extensions/tests/guard-probe.ts.in | 58 +++ shared/extensions/tests/run-guard-checks.sh | 62 +++ 6 files changed, 536 insertions(+) create mode 100644 shared/extensions/pi-guard-base.ts create mode 100644 shared/extensions/tests/fixtures/.pi/skills/guard-skill/SKILL.md create mode 100644 shared/extensions/tests/fixtures/AGENTS.md create mode 100644 shared/extensions/tests/fixtures/secret-lookalike.txt create mode 100644 shared/extensions/tests/guard-probe.ts.in create mode 100755 shared/extensions/tests/run-guard-checks.sh diff --git a/shared/extensions/pi-guard-base.ts b/shared/extensions/pi-guard-base.ts new file mode 100644 index 0000000..cbda3e6 --- /dev/null +++ b/shared/extensions/pi-guard-base.ts @@ -0,0 +1,409 @@ +/** + * pi-guard-base — reusable building blocks for a dedicated Pi agent extension. + * + * Extracted from `pi-workspaces/memo-inbox/.pi/extensions/memo-guard.ts`, which + * has enforced these patterns in production since 2026-07. + * + * Provides: + * - path containment that is safe against symlink escape + * - a path-restricted `read` that shadows the built-in tool + * - the two-layer capability guard (setActiveTools + tool_call block) + * - output truncation helpers + * - a loopback HTTP bridge for exposing host capabilities as tools + * + * Import from a scenario extension: + * + * import { installGuard, registerRestrictedRead } from "../../../shared/extensions/pi-guard-base.ts"; + * + * Rationale and mechanism references: `docs/isolation-baseline.md`, + * `docs/pi-runtime-notes.md` sections 1, 4, 6, 7, 13, 14. + */ + +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import { + existsSync, + lstatSync, + readFileSync, + realpathSync, +} from "node:fs"; +import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; + +// Pi truncates tool output at these limits itself, but a tool that returns more +// wastes a full round trip. Truncate before returning. (§13) +export const DEFAULT_MAX_BYTES = 50_000; +export const DEFAULT_MAX_LINES = 2_000; + +// --------------------------------------------------------------------------- +// Results +// --------------------------------------------------------------------------- + +export type ToolText = { type: "text"; text: string }; + +/** Build a text result. `details` is persisted in the session but is NOT sent + * to the model, so put structured payloads there and prose in `text`. (§13) */ +export function textResult(text: string, details: Record = {}) { + return { content: [{ type: "text" as const, text }], details }; +} + +/** + * Truncate from the tail, reporting what was dropped. + * + * Byte-aware rather than character-aware because the caps pi applies are byte + * caps and CJK text is 3 bytes per character in UTF-8. + */ +export function truncate( + text: string, + maxBytes: number = DEFAULT_MAX_BYTES, + maxLines: number = DEFAULT_MAX_LINES, +): { text: string; truncated: boolean } { + let out = text; + let truncated = false; + + const lines = out.split("\n"); + if (lines.length > maxLines) { + out = lines.slice(0, maxLines).join("\n"); + truncated = true; + } + + let bytes = Buffer.byteLength(out, "utf8"); + if (bytes > maxBytes) { + // Cut on a character boundary by shrinking until it fits. + let end = Math.floor((out.length * maxBytes) / bytes); + while (end > 0 && Buffer.byteLength(out.slice(0, end), "utf8") > maxBytes) { + end -= Math.max(1, Math.floor(end / 64)); + } + out = out.slice(0, Math.max(0, end)); + truncated = true; + bytes = Buffer.byteLength(out, "utf8"); + } + + if (truncated) { + out += `\n\n[truncated: ${bytes} of ${Buffer.byteLength(text, "utf8")} bytes shown]`; + } + return { text: out, truncated }; +} + +// --------------------------------------------------------------------------- +// Path containment +// --------------------------------------------------------------------------- + +/** Whether `candidate` is `root` or lies beneath it. Both must be absolute. */ +export function inside(root: string, candidate: string): boolean { + const rel = relative(root, candidate); + return rel === "" || (!rel.startsWith(`..${sep}`) && rel !== ".." && !isAbsolute(rel)); +} + +/** + * Canonicalise a path, resolving symlinks. + * + * Symlinks are resolved *before* any containment check so that a link inside an + * allowed root cannot point outside it. + */ +export function safeRealPath(candidate: string, allowMissing = false): string { + const absolute = resolve(candidate); + if (existsSync(absolute)) return realpathSync(absolute); + if (!allowMissing) throw new Error(`Path does not exist: ${candidate}`); + return join(realpathSync(dirname(absolute)), basename(absolute)); +} + +export interface PathPolicy { + /** Absolute roots the agent may reach. Resolved and canonicalised on build. */ + roots: string[]; + /** Base for relative inputs. Defaults to the first root. */ + base?: string; + /** If set, only these lowercase extensions are permitted, e.g. [".md"]. */ + extensions?: string[]; + /** Message shown when a path is outside every root. */ + denyMessage?: string; +} + +export interface PathResolver { + (input: string): string; + roots: string[]; +} + +/** Build a resolver that accepts only files inside the policy's roots. */ +export function makePathResolver(policy: PathPolicy): PathResolver { + const roots = policy.roots.map((r) => realpathSync(resolve(r))); + const base = realpathSync(resolve(policy.base ?? roots[0])); + const allowed = policy.extensions?.map((e) => e.toLowerCase()); + const deny = policy.denyMessage ?? "Access denied: path is outside the permitted roots."; + + const resolver = ((input: string): string => { + if (typeof input !== "string" || input.trim() === "") { + throw new Error("Access denied: empty path."); + } + const candidate = safeRealPath(isAbsolute(input) ? input : resolve(base, input)); + if (!roots.some((root) => inside(root, candidate))) throw new Error(deny); + if (!lstatSync(candidate).isFile()) throw new Error("Access denied: path is not a file."); + if (allowed) { + const dot = candidate.lastIndexOf("."); + const ext = dot === -1 ? "" : candidate.slice(dot).toLowerCase(); + if (!allowed.includes(ext)) { + throw new Error(`Access denied: only ${allowed.join(", ")} files are readable.`); + } + } + return candidate; + }) as PathResolver; + + resolver.roots = roots; + return resolver; +} + +// --------------------------------------------------------------------------- +// Restricted `read` +// --------------------------------------------------------------------------- + +export interface RestrictedReadOptions extends PathPolicy { + /** Maximum characters returned in one call. */ + maxChars?: number; + label?: string; + description?: string; +} + +/** + * Register a path-restricted `read` that shadows the built-in tool. + * + * Two reasons a dedicated agent needs this rather than simply dropping `read`: + * + * 1. pi emits the `` block only when a tool named `read` is + * active, and skill bodies are loaded through it (progressive disclosure). + * Without `read`, every SKILL.md is unreachable. (§1, §2) + * 2. The built-in `read` accepts absolute paths, so it can reach the service's + * own credential files. + * + * The policy roots MUST therefore include the skill directory. + */ +export function registerRestrictedRead(pi: ExtensionAPI, options: RestrictedReadOptions): void { + const resolvePath = makePathResolver(options); + const maxChars = options.maxChars ?? 80_000; + const rootList = resolvePath.roots.join(", "); + + pi.registerTool({ + name: "read", + label: options.label ?? "Read Permitted File", + description: + options.description ?? + `Read a UTF-8 text file from the permitted roots (${rootList}). ` + + `Any other host path is denied. Use this to load a skill file when a task matches its description.`, + promptSnippet: "read: read a text file from the agent's permitted directories", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "Absolute path, or relative to the workspace." }, + offset: { type: "number", minimum: 0, description: "Character offset to start at." }, + limit: { type: "number", minimum: 1, maximum: maxChars, description: "Characters to return." }, + }, + required: ["path"], + additionalProperties: false, + } as any, + async execute(_id: string, params: any) { + // Denials are returned rather than thrown so the model can correct course + // instead of the turn being marked failed. + let path: string; + try { + path = resolvePath(String(params?.path ?? "")); + } catch (error) { + return textResult(`ERROR: ${(error as Error).message}`, { error: true }); + } + const content = readFileSync(path, "utf8"); + const offset = Math.max(0, Math.floor(Number(params?.offset) || 0)); + const limit = Math.min(maxChars, Math.max(1, Math.floor(Number(params?.limit) || maxChars))); + const slice = content.slice(offset, offset + limit); + const { text } = truncate(slice); + return textResult(text, { + path, + offset, + returned: slice.length, + truncated: offset + limit < content.length, + }); + }, + }); +} + +// --------------------------------------------------------------------------- +// Capability guard +// --------------------------------------------------------------------------- + +export interface GuardOptions { + /** Scenario name, used in block messages. */ + scenario: string; + /** The complete set of tool names this agent may use. */ + allowedTools: string[]; + /** Called when a tool call is blocked. */ + onBlocked?: (toolName: string) => void; +} + +/** + * Install the two independent capability layers. + * + * `setActiveTools` governs which tools the model is offered; the `tool_call` + * hook blocks execution regardless. Both are needed: the first is the contract, + * the second survives a mistake in the CLI flags or a later resource discovery + * that re-activates something. (§4, §14) + * + * Re-asserted on `resources_discover` as well as `session_start` because + * discovery can change the registry after startup. + */ +export function installGuard(pi: ExtensionAPI, options: GuardOptions): void { + const allowed = new Set(options.allowedTools); + const restrict = () => pi.setActiveTools([...allowed]); + + pi.on("session_start", async () => restrict()); + pi.on("resources_discover", async () => restrict()); + + pi.on("tool_call", async (event: any) => { + if (!allowed.has(event.toolName)) { + options.onBlocked?.(event.toolName); + return { + block: true, + reason: `${options.scenario} blocks tool: ${event.toolName}`, + }; + } + return undefined; + }); +} + +// --------------------------------------------------------------------------- +// Loopback bridge +// --------------------------------------------------------------------------- + +export interface BridgeOptions { + /** Base URL, e.g. http://127.0.0.1:8767. Must be loopback. */ + baseUrl: string; + /** Shared secret sent as `x-pi-bridge-token`. */ + token?: string; + timeoutMs?: number; +} + +export interface BridgeToolSpec { + name: string; + label?: string; + description: string; + /** JSON Schema. registerTool accepts a plain schema object — verified. (§5) */ + parameters: Record; + promptSnippet?: string; + promptGuidelines?: string[]; + /** Path appended to baseUrl. Defaults to `/tools/`. */ + path?: string; + /** Provider-side schema enforcement for structured-output tools. (§12) */ + constrainedSampling?: { type: "json_schema"; strict: "prefer" | "require" }; + /** End the turn on success — for terminating structured-output tools. */ + terminate?: boolean; + maxBytes?: number; +} + +function assertLoopback(baseUrl: string): void { + const url = new URL(baseUrl); + const host = url.hostname; + const ok = + host === "127.0.0.1" || + host === "::1" || + host === "localhost" || + host.startsWith("127."); + if (!ok) { + throw new Error( + `pi-guard-base: bridge baseUrl must be loopback, got ${host}. ` + + `A dedicated agent must not be given a route to the wider network.`, + ); + } +} + +/** + * Register tools that proxy to a loopback HTTP backend. + * + * There is no RPC command for the host to inject a tool result, so host + * capabilities must be exposed this way. (§15, `docs/gateway-patterns.md` #9) + * + * Because `registerTool` accepts a plain JSON Schema object, the backend can own + * the schema and serve it, giving one source of truth instead of a TypeScript + * copy that drifts. + */ +export function registerBridgeTools( + pi: ExtensionAPI, + bridge: BridgeOptions, + specs: BridgeToolSpec[], +): void { + assertLoopback(bridge.baseUrl); + const base = bridge.baseUrl.replace(/\/$/, ""); + const timeoutMs = bridge.timeoutMs ?? 30_000; + + for (const spec of specs) { + const path = spec.path ?? `/tools/${spec.name}`; + pi.registerTool({ + name: spec.name, + label: spec.label ?? spec.name, + description: spec.description, + // Without promptSnippet a tool is absent from the prose tool list, even + // though it stays callable through the provider API. (§7) + promptSnippet: spec.promptSnippet ?? `${spec.name}: ${spec.description.slice(0, 80)}`, + promptGuidelines: spec.promptGuidelines, + parameters: spec.parameters as any, + constrainedSampling: spec.constrainedSampling, + async execute(_id: string, params: any, signal?: AbortSignal) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + const onAbort = () => controller.abort(); + signal?.addEventListener("abort", onAbort, { once: true }); + try { + const headers: Record = { "content-type": "application/json" }; + if (bridge.token) headers["x-pi-bridge-token"] = bridge.token; + + const response = await fetch(`${base}${path}`, { + method: "POST", + headers, + body: JSON.stringify(params ?? {}), + signal: controller.signal, + }); + + const body = await response.text(); + if (!response.ok) { + // Throwing marks the call as an error, which is correct for a + // backend failure: it is not a valid answer. (§13) + throw new Error(`${spec.name} failed: HTTP ${response.status} ${body.slice(0, 400)}`); + } + + let parsed: any; + try { + parsed = JSON.parse(body); + } catch { + const { text } = truncate(body, spec.maxBytes); + return { content: [{ type: "text" as const, text }], details: { raw: true } }; + } + + const rendered = + typeof parsed?.text === "string" + ? parsed.text + : JSON.stringify(parsed, null, 2); + const { text } = truncate(rendered, spec.maxBytes); + const result: any = { + content: [{ type: "text" as const, text }], + details: parsed, + }; + if (spec.terminate) result.terminate = true; + return result; + } finally { + clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + } + }, + }); + } +} + +/** Fetch tool specifications from the backend so the schema has one owner. */ +export async function fetchBridgeSpecs(bridge: BridgeOptions): Promise { + assertLoopback(bridge.baseUrl); + const headers: Record = {}; + if (bridge.token) headers["x-pi-bridge-token"] = bridge.token; + const response = await fetch(`${bridge.baseUrl.replace(/\/$/, "")}/tools`, { headers }); + if (!response.ok) { + throw new Error(`pi-guard-base: cannot load tool specs: HTTP ${response.status}`); + } + const payload = await response.json(); + const specs = Array.isArray(payload) ? payload : payload?.tools; + if (!Array.isArray(specs)) { + throw new Error("pi-guard-base: /tools must return an array or { tools: [...] }"); + } + return specs as BridgeToolSpec[]; +} diff --git a/shared/extensions/tests/fixtures/.pi/skills/guard-skill/SKILL.md b/shared/extensions/tests/fixtures/.pi/skills/guard-skill/SKILL.md new file mode 100644 index 0000000..05762a7 --- /dev/null +++ b/shared/extensions/tests/fixtures/.pi/skills/guard-skill/SKILL.md @@ -0,0 +1,5 @@ +--- +name: guard-skill +description: GUARD_SKILL_DESC marker for verifying that pi-guard-base keeps the skills section reachable. +--- +GUARD_SKILL_BODY diff --git a/shared/extensions/tests/fixtures/AGENTS.md b/shared/extensions/tests/fixtures/AGENTS.md new file mode 100644 index 0000000..b1a7c59 --- /dev/null +++ b/shared/extensions/tests/fixtures/AGENTS.md @@ -0,0 +1 @@ +# Guard probe workspace diff --git a/shared/extensions/tests/fixtures/secret-lookalike.txt b/shared/extensions/tests/fixtures/secret-lookalike.txt new file mode 100644 index 0000000..f0a549a --- /dev/null +++ b/shared/extensions/tests/fixtures/secret-lookalike.txt @@ -0,0 +1 @@ +SHOULD_NOT_BE_READABLE=1 diff --git a/shared/extensions/tests/guard-probe.ts.in b/shared/extensions/tests/guard-probe.ts.in new file mode 100644 index 0000000..b060a77 --- /dev/null +++ b/shared/extensions/tests/guard-probe.ts.in @@ -0,0 +1,58 @@ +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import { + inside, safeRealPath, makePathResolver, truncate, textResult, + installGuard, registerRestrictedRead, +} from "__GUARD_BASE__"; + +const WS = "__FIXTURES__"; +const ALLOWED = ["read", "probe_noop"]; + +export default function guardProbe(pi: ExtensionAPI) { + registerRestrictedRead(pi, { + roots: [WS + "/.pi/skills"], + base: WS, + extensions: [".md"], + denyMessage: "Read denied: only skill markdown is readable.", + }); + + pi.registerTool({ + name: "probe_noop", + label: "Noop", + description: "noop", + promptSnippet: "probe_noop: noop", + parameters: { type: "object", properties: {}, additionalProperties: false } as any, + async execute() { return textResult("noop"); }, + }); + + installGuard(pi, { scenario: "guard-probe", allowedTools: ALLOWED }); + + pi.on("session_start", async (_e, ctx) => { + const sp = ctx.getSystemPrompt?.() ?? ""; + const out: string[] = []; + out.push("ACTIVE=" + JSON.stringify(pi.getActiveTools().sort())); + out.push("READ_SOURCE=" + JSON.stringify( + pi.getAllTools().filter(t => t.name === "read").map(t => (t as any).sourceInfo?.source))); + out.push("SP_HAS_SKILLS=" + String(sp.includes("available_skills"))); + out.push("SP_SKILLNAMES=" + JSON.stringify([...sp.matchAll(/([^<]+)<\/name>/g)].map(m => m[1]))); + + // --- unit checks on the exported primitives --- + out.push("INSIDE_same=" + String(inside("/a/b", "/a/b"))); + out.push("INSIDE_child=" + String(inside("/a/b", "/a/b/c"))); + out.push("INSIDE_escape=" + String(inside("/a/b", "/a/c"))); + out.push("INSIDE_prefix_trap=" + String(inside("/a/b", "/a/bc"))); + + const r = makePathResolver({ roots: [WS + "/.pi/skills"], base: WS, extensions: [".md"] }); + const tryPath = (p: string) => { try { r(p); return "ALLOW"; } catch (e) { return "DENY"; } }; + out.push("RESOLVE_skill=" + tryPath(".pi/skills/guard-skill/SKILL.md")); + out.push("RESOLVE_outside=" + tryPath("secret-lookalike.env")); + out.push("RESOLVE_traversal=" + tryPath(".pi/skills/../../secret-lookalike.env")); + out.push("RESOLVE_abs_home=" + tryPath("/home/claw/.config/curator/curator.env")); + + const big = "x".repeat(60000); + const t = truncate(big, 1000, 100); + out.push("TRUNC_applied=" + String(t.truncated) + " len=" + String(t.text.length < 1200)); + out.push("REALPATH_missing_throws=" + (() => { try { safeRealPath("/nope/nope"); return "no"; } catch { return "yes"; } })()); + + for (const line of out) console.error("GUARD_" + line); + }); +} diff --git a/shared/extensions/tests/run-guard-checks.sh b/shared/extensions/tests/run-guard-checks.sh new file mode 100755 index 0000000..0746a7b --- /dev/null +++ b/shared/extensions/tests/run-guard-checks.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Zero-token verification of pi-guard-base against a real pi process. +# +# Drives pi with `get_state` only, so no model call is billed. Checks the two +# capability layers, the `read` override, skill reachability, path containment +# (including symlink/traversal/absolute escapes) and truncation. +# +# Usage: shared/extensions/tests/run-guard-checks.sh +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO="$(git -C "$HERE" rev-parse --show-toplevel)" +PI_BIN="${PI_BIN:-/home/claw/.npm-global/bin/pi}" + +WORK="$(mktemp -d /tmp/pi-guard-checks.XXXXXX)" +trap 'rm -rf "$WORK"' EXIT + +cp -r "$HERE/fixtures/." "$WORK/" +mv "$WORK/secret-lookalike.txt" "$WORK/secret-lookalike.env" +sed -e "s|__FIXTURES__|$WORK|g" \ + -e "s|__GUARD_BASE__|$REPO/shared/extensions/pi-guard-base.ts|" \ + "$HERE/guard-probe.ts.in" > "$WORK/guard-ext.ts" + +OUT="$(printf '{"id":"1","type":"get_state"}\n' | timeout 120 "$PI_BIN" \ + --mode rpc --no-session --no-builtin-tools \ + --no-extensions -e "$WORK/guard-ext.ts" \ + --no-skills --skill "$WORK/.pi/skills/guard-skill" \ + --no-prompt-templates --no-themes --approve \ + --provider zenmux --model openai/gpt-5.6-luna 2>&1 >/dev/null | grep '^GUARD_')" + +echo "$OUT" +echo + +fails=0 +expect() { + if grep -qxF "GUARD_$1" <<<"$OUT"; then + echo " PASS $1" + else + echo " FAIL $1 (actual: $(grep "^GUARD_${1%%=*}=" <<<"$OUT" || echo ''))" + fails=$((fails + 1)) + fi +} + +echo "== assertions ==" +expect 'ACTIVE=["probe_noop","read"]' +expect 'READ_SOURCE=["cli"]' +expect 'SP_HAS_SKILLS=true' +expect 'SP_SKILLNAMES=["guard-skill"]' +expect 'INSIDE_same=true' +expect 'INSIDE_child=true' +expect 'INSIDE_escape=false' +expect 'INSIDE_prefix_trap=false' +expect 'RESOLVE_skill=ALLOW' +expect 'RESOLVE_outside=DENY' +expect 'RESOLVE_traversal=DENY' +expect 'RESOLVE_abs_home=DENY' +expect 'TRUNC_applied=true len=true' +expect 'REALPATH_missing_throws=yes' + +echo +if [ "$fails" -eq 0 ]; then echo "RESULT: all checks passed"; else echo "RESULT: $fails check(s) failed"; fi +exit $((fails > 0))