Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions docs/path-consent-safety.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Path-consent safety

The path-tool fence from PR #1610 requires per-target, per-session confirmation for `read`, `write`, `edit`, `grep`, `find`, and `ls` outside the session worktree or registered same-clone worktrees. These follow-ups depend on that PR; they do not add Bash path fencing.

## Resolution and boundary inputs

- Dangling symlinks resolve to their destination before consent is checked. Inaccessible or cyclic links block rather than inventing an internal target (#1659, already corrected in the dependency).
- Durable worktree roots must be canonical, absolute, existing paths. Empty, relative, and malformed roots cannot widen the boundary (#1660, already corrected in the dependency).
- Consent is ephemeral and cannot carry across sessions.

## Confirmation failures

Shell-command and outside-path confirmation have the same failure semantics:

| Outcome | Result |
| --- | --- |
| Confirmation returns `true` | The guarded call is allowed. |
| Confirmation returns anything else, or the UI/confirm capability is absent | The call is denied. |
| Confirmation throws or rejects | The permission lifecycle settles as denied, then the original error propagates. |

Missing confirmation never grants a path target. A subsequent attempt still requires consent. Headless confirmation-required calls remain blocked.

## Tool-schema coverage

`tests/path-consent-fence.test.ts` compares the installed Pi tools' complete input-key shapes, including nested properties, with an explicit snapshot for all six fenced tools. An added field such as `backupPath` fails this assertion instead of silently escaping review. On a Pi upgrade, review which new fields carry paths and update fence collection and its regressions before accepting a new snapshot. The test also injects top-level and nested schema drift to prove the assertion rejects it.

## Focused checks

```sh
node --experimental-strip-types --test tests/path-consent-fence.test.ts tests/session-worktree-registry.test.ts
node --experimental-strip-types --test --test-name-pattern='missing confirmation capability|guarded command confirmation' tests/gentle-ai.test.ts
```

All filesystem witnesses use disposable fixtures, not user files. Existing dependency regressions establish the dangling-link and invalid-root protections; the complementary diff only changes missing-confirmation behavior and adds schema-drift coverage.
2 changes: 1 addition & 1 deletion extensions/gentle-ai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1871,7 +1871,7 @@ async function confirmCommand(
emitPermissionRequest("waiting");
herdrLifecycle.begin();
try {
approved = (await ctx.ui.confirm(title, preview)) === true;
approved = (await ctx.ui?.confirm?.(title, preview)) === true;
} catch (error) {
confirmationFailed = true;
confirmationError = error;
Expand Down
10 changes: 6 additions & 4 deletions odd/tasks/path-fence-1659-1660.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ S12. External-temp permission: "驴autoriz谩s usar 煤nicamente la carpeta descart

## Tasks
- T0 | S6 | delegated read-only investigation: three causes proven, remainder unclassified | dropped: superseded by authorized scoped fixture correction, not claimed fully classified | commit: N/A (investigation only).
- T3 | S7-S9 | two parallel bounded writers with disjoint launcher vs permission/Git edit surfaces | in_progress: independent verification complete including targeted absence/PATH recheck; two separate work-unit commits pending | two work-unit commits pending; permission/Git and targeted launcher checks pass, full launcher remains partial.
- T1 | S1-S5 | inline implementation: shared confirmation flow, no independent disjoint writer units; independent delegated verify (security trigger), then RDD | pending: fixture prerequisite T3 | commit: pending; inherited primary regressions pass.
- T2 | S1-S3 | inline delivery: publish complementary branch/PR and record evidence | pending | commit: pending.
- T3 | S7-S12 | two parallel bounded writers; bounded inline SIGINT adaptation; independent verification per unit | done: authorized fixture outcomes independently verified | commits: `5cccc9c0eef42ef7af84b75ab67073e29c14dfa3` filesystem81 lines; `2916bbec15389558af12582bba36a119930d79be` launcher149 lines; no production changes.
- T1 | S1-S5 | inline implementation: shared confirmation flow, no independent disjoint writer units; independent delegated verify (security trigger), then RDD | done: self/independent security checks pass; native RDD approved and exact acknowledgement burned authority | commit: `f0e44094719105b4fa47963712cb745d912828da`114 lines; inherited primary regressions pass.
- T2 | S1-S3 | inline delivery: publish pinned mirror, draft tracker and three focused child PRs; record evidence | in_progress | commit: tracking evidence pending; no merge.

## Log
L1. Original request: "que sigue ac谩? hacer el analisis y luego validar los problemas y hacer la implementacion ,verify, rdd y PR?"
Expand Down Expand Up @@ -61,4 +61,6 @@ L33. Needs user decision: authorize one disposable external temp target `C:/User
L34. User replies "sip, hagamos eso" to exact S12 permission. Continue the same independent launcher verifier for only its one blocked case; no source correction, no full sweep. External directory must be created exclusively, never overwrite existing contents; remove only owned disposable state.
L35. Targeted independent recheck PASS: `node --experimental-strip-types --test --test-timeout=60000 --test-name-pattern='^a genuinely absent adjacent peer falls back to PATH$' tests/gentle-shell-bin.test.ts`; exit0, exactly1 selected/1 pass/0 fail/0 skips, runner stderr empty. Child status0 and stdout/Pi0.99.2 assertion pass; child stderr not asserted. Exclusive external target, scratch and dependency junction removed; no source change. Earlier122/1/8 run remains separate, not rewritten as full-file-green.
L36. Verification blocker resolved. Delivery boundaries: pinned mirror `feat/path-fence-1610-base`, draft tracker `feat/path-fence-safety`, filesystem child `test/path-fence-filesystem-fixtures`, launcher child `test/path-fence-launcher-fixtures`, security child `fix/path-fence-confirmation`. Tracker initially targets mirror (not main) to exclude inherited #1610 diff. Each child <=400 lines; no merge or base reconciliation authorized implicitly. RDD global on; review security work-unit only against preceding boundary, with separate candidate consent.
L37. Fork `danielgap/gentle-pi` permission is READ. Complementary upstream PR requires publishing a new dependency branch pinned exactly to #1610 head, never modifying the contributor branch. Retarget after #1610 merges; do not claim inherited primary fixes as this candidate's diff.
L37. Created tracker doc commit `f76901bcf`; filesystem unit `5cccc9c0eef42ef7af84b75ab67073e29c14dfa3`81 lines and launcher unit `2916bbec15389558af12582bba36a119930d79be`149 lines close T3. Each carries its own tests/docs and rollback boundary; real launcher black-box proof covers Windows fixture scope. Parent seam178/0/8 remains separate from independent launcher122/1/8 plus single-case correction1/1. No source/public CLI changes in fixture units; provider/runtime harness checks already pass; no POSIX/console/full-corpus-green claim. T1 resumes.
L38. Security unit `f0e44094719105b4fa47963712cb745d912828da`114 lines is the exact reviewed source candidate against `2916bbec15389558af12582bba36a119930d79be`. Native lineage `review-7d6d3e35350d1ce1`: all4 lens results admitted, approved; exact acknowledgement returned `native-approved-acknowledgement-completed`, authority burned. No blocking finding/correction. Nonblocking R2-001(readability,:47) and R3-001(reliability,:50) in path-fence test are separate later work, not another correction or review. T1 done; T2 publication begins under original PR authorization. Subsequent tracking-only doc commits are not part of this reviewed source candidate.
L39. Fork `danielgap/gentle-pi` permission is READ. Complementary upstream PR requires publishing a new dependency branch pinned exactly to #1610 head, never modifying the contributor branch. Retarget after #1610 merges; do not claim inherited primary fixes as this candidate's diff.
38 changes: 38 additions & 0 deletions tests/gentle-ai.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,44 @@ test("delivery commands bypass RDD under every mode outcome while command safety
}
});

test("missing confirmation capability denies shell and path calls uniformly", async (t) => {
const cwd = realpathSync(mkdtempSync(join(tmpdir(), "gentle-pi-missing-confirm-")));
t.after(() => rmSync(cwd, { recursive: true, force: true }));
execFileSync("git", ["init", "--quiet", cwd], { stdio: "pipe" });
const outside = join(dirname(cwd), "not-created-by-confirm-test.txt");
const session = SessionManager.create(cwd, join(cwd, "sessions"));
for (const ui of [undefined, {}]) {
type Handler = (event: { toolName: string; input: unknown }, ctx: ExtensionContext) => Promise<ToolCallEventResult | undefined>;
const handlers = new Map<string, Handler>();
const emitted: Array<{ channel: string; data: any }> = [];
const pi = {
on(name: string, handler: Handler) { handlers.set(name, handler); },
events: { emit(channel: string, data: any) { emitted.push({ channel, data }); } },
registerCommand() {}, registerTool() {},
} as unknown as ExtensionAPI;
createGentleAiExtension({ processEnv: { GENTLE_PI_AGENTS_CHILD: "0" }, nativeReviewCli: null })(pi);
const ctx = { cwd, hasUI: true, ui, sessionManager: session } as unknown as ExtensionContext;
for (const event of [
{ toolName: "bash", input: { command: "git rebase main" }, reason: "Gentle AI safety policy blocked the command because it was not confirmed." },
{ toolName: "write", input: { path: outside, content: "must not be written" }, reason: "Gentle AI safety policy blocked the path access because it was not confirmed." },
]) {
emitted.length = 0;
assert.deepEqual(await handlers.get("tool_call")!(event, ctx), { block: true, reason: event.reason });
const permissions = emitted.filter(({ channel }) => channel === "pi-permission-system:permission-request");
assert.deepEqual(permissions.map(({ data }) => data.state), ["waiting", "denied"]);
assert.equal(permissions[0].data.requestId, permissions[1].data.requestId);
assert.equal(permissions[1].data.toolName, event.toolName);
assert.deepEqual(emitted.filter(({ channel }) => channel === "herdr:blocked").map(({ data }) => data.active), [true, false]);
assert.equal(existsSync(outside), false, "a denied path call never executes the write");
}
let prompts = 0;
ctx.ui = { confirm: async () => { prompts++; return false; } } as ExtensionContext["ui"];
assert.equal((await handlers.get("tool_call")!({ toolName: "write", input: { path: outside, content: "must not be written" } }, ctx))?.block, true);
assert.equal(prompts, 1, "missing confirmation cannot cache a grant");
assert.equal(existsSync(outside), false);
}
});

test("guarded command confirmation emits a generic correlated permission lifecycle", async () => {
type ToolCallHandler = (
event: { toolName: string; input: unknown },
Expand Down
40 changes: 38 additions & 2 deletions tests/path-consent-fence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSyn
import { homedir, tmpdir } from "node:os";
import { join } from "node:path";
import test from "node:test";
import { SessionManager } from "@earendil-works/pi-coding-agent";
import { PathTargetGrants, canonicalizeTarget, evaluatePathFence, resolveOutsidePaths } from "../lib/path-consent-fence.ts";
import { SessionManager, createReadTool, createWriteTool, createEditTool, createGrepTool, createFindTool, createLsTool } from "@earendil-works/pi-coding-agent";
import { PathTargetGrants, canonicalizeTarget, collectStringPaths, evaluatePathFence, PATH_FENCE_TOOL_NAMES, resolveOutsidePaths } from "../lib/path-consent-fence.ts";
import { registeredRootsForSession, SESSION_WORKTREE_ENTRY } from "../lib/session-worktree-registry.ts";

// All writes belong to unique fixtures, never the live clone.
Expand All @@ -21,6 +21,42 @@ function fixture(t: test.TestContext) {

const grants = () => new PathTargetGrants();

// Pin every input key, not just known path spellings: an added backupPath (or
// any nested input change) must force review of fence coverage on a Pi upgrade.
test("installed fenced tool schemas cannot silently drift beyond path coverage", () => {
const tools = [createReadTool, createWriteTool, createEditTool, createGrepTool, createFindTool, createLsTool].map(create => create(process.cwd()));
const expected: Record<string, string[]> = {
read: ["limit", "offset", "path"],
write: ["content", "path"],
edit: ["edits", "edits[].newText", "edits[].oldText", "path"],
grep: ["context", "glob", "ignoreCase", "limit", "literal", "path", "pattern"],
find: ["limit", "path", "pattern"],
ls: ["limit", "path"],
};
assert.deepEqual(tools.map(tool => tool.name).sort(), [...PATH_FENCE_TOOL_NAMES].sort());
const schemaKeys = (schema: any, prefix = ""): string[] => {
const keys = Object.entries(schema.properties ?? {}).flatMap(([key, child]) => {
const path = prefix ? `${prefix}.${key}` : key;
return [path, ...schemaKeys(child, path)];
});
if (schema.items) keys.push(...schemaKeys(schema.items, `${prefix}[]`));
for (const branch of [...(schema.anyOf ?? []), ...(schema.oneOf ?? []), ...(schema.allOf ?? [])]) keys.push(...schemaKeys(branch, prefix));
return [...new Set(keys)].sort();
};
const assertCoverage = (name: string, properties: Record<string, unknown>) => {
assert.deepEqual(schemaKeys({ properties }), expected[name], `${name}: review path-fence coverage when tool inputs change`);
};
for (const tool of tools) {
assertCoverage(tool.name, tool.parameters.properties);
assert.deepEqual(collectStringPaths({ path: "/outside/target" }), ["/outside/target"]);
}
const edit = tools.find(tool => tool.name === "edit")!;
assert.throws(() => assertCoverage("edit", { ...edit.parameters.properties, backupPath: { type: "string" } }), /review path-fence coverage/);
const nested = structuredClone(edit.parameters.properties) as Record<string, any>;
nested.edits.items.properties.backupPath = { type: "string" };
assert.throws(() => assertCoverage("edit", nested), /review path-fence coverage/);
});

function fileLink(t: test.TestContext, target: string, link: string): boolean {
try {
symlinkSync(target, link, "file");
Expand Down
Loading