fix(write): evaluate function-valued xd:// device approvals

The write approval gate discarded a mounted tool's function-valued
approval and never decoded the device JSON payload, defaulting the tier
to exec. Read/write xd:// operations then prompted in non-yolo modes
that permit them.

Now decode valid object payloads and resolve the mounted tool's normal
approval decision via resolveToolTier; malformed JSON, non-object
payloads, and unknown devices still fall back to exec and prompt.

Fixes #5727
This commit is contained in:
roboomp
2026-07-16 17:24:06 +00:00
parent c0d0ad7629
commit 5445beb6f5
4 changed files with 76 additions and 7 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed the `write` approval gate misclassifying `xd://` device writes as `exec` when the mounted tool declared a function-valued (argument-dependent) `approval`: the gate discarded the function and never decoded the device JSON payload, so read/write device operations prompted in non-yolo modes their approval mode permits. It now parses valid object payloads and evaluates the mounted tool's normal approval decision, while malformed JSON, non-object payloads, and unknown devices still fall back to `exec` and prompt ([#5727](https://github.com/can1357/oh-my-pi/issues/5727)).
## [17.0.1] - 2026-07-16
### Changed
@@ -75,6 +75,17 @@ function getToolDecision(tool: ApprovalSubject, args: unknown): Omit<ResolvedApp
return normalizeDecision(decision);
}
/**
* Evaluate a tool's own approval declaration against `args` and return the
* resulting capability tier, defaulting to `exec` when the tool omits an
* approval. Unlike reading `tool.approval` directly, this runs function-valued
* approvals — the write tool's `xd://` gate uses it to take a mounted device's
* argument-dependent tier instead of falling back to `exec`.
*/
export function resolveToolTier(tool: ApprovalSubject, args: unknown): ToolTier {
return getToolDecision(tool, args).tier;
}
function modeApprovesTier(mode: ApprovalMode, tier: ToolTier): boolean {
return TIER_RANK[tier] <= TIER_RANK[APPROVAL_MODE_MAX_TIER[mode]];
}
+17 -4
View File
@@ -36,7 +36,7 @@ import {
writeArchive,
} from "../utils/zip";
import { routeWriteThroughBridge } from "./acp-bridge";
import { truncateForPrompt } from "./approval";
import { resolveToolTier, truncateForPrompt } from "./approval";
import { assertEditableFile } from "./auto-generated-guard";
import {
type ConflictEntry,
@@ -398,9 +398,22 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
if (xdevTarget.name === REPORT_ISSUE_DEVICE_NAME) return "write";
if (xdevTarget.name && isResolutionDeviceName(xdevTarget.name)) return "read";
const inst = xdevTarget.name ? this.session.xdevRegistry?.get(xdevTarget.name) : undefined;
const decision = typeof inst?.approval === "function" ? undefined : inst?.approval;
const tier = typeof decision === "object" ? decision?.tier : decision;
return tier ?? "exec";
if (!inst) return "exec";
// Decode the device JSON payload and evaluate the mounted tool's own
// approval (which may be argument-dependent, e.g. ast_edit is read-tier
// for internal-URL paths, debug is read-tier for inspection actions).
// Malformed JSON, non-object payloads, and missing content stay exec so
// the gate fails closed — the dispatch itself rejects them too.
const rawContent = (args as Partial<WriteParams>).content;
if (typeof rawContent !== "string") return "exec";
let parsed: unknown;
try {
parsed = JSON.parse(rawContent);
} catch {
return "exec";
}
if (!isRecord(parsed)) return "exec";
return resolveToolTier(inst, parsed);
}
// Remote SSH writes open an outbound connection and run a remote shell —
// gate them like the exec-tier `ssh` tool, ahead of the handler-write
@@ -59,12 +59,12 @@ describe("read and write route xd:// device URLs", () => {
paths: [filePath],
});
// Approval resolves a tier instead of throwing. A mounted tool whose own
// approval is a function (unresolvable statically) falls back to exec.
// The write gate decodes the device payload and evaluates the mounted
// tool's own approval. ast_edit is write-tier for a filesystem path.
const approval = write!.approval;
expect(typeof approval).toBe("function");
if (typeof approval === "function") {
expect(approval({ path: "xd://ast_edit", content })).toBe("exec");
expect(approval({ path: "xd://ast_edit", content })).toBe("write");
}
// Execute dispatches through the xdev registry to the mounted ast_edit,
@@ -86,6 +86,47 @@ describe("read and write route xd:// device URLs", () => {
}
});
it("resolves function-valued device approvals per payload and fails closed on bad content", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-approval-"));
try {
const filePath = path.join(tempDir, "target.ts");
await Bun.write(filePath, "legacyWrap(x, value)\n");
const tools = await createTools(xdevSession(tempDir));
const write = tools.find(entry => entry.name === "write");
expect(write).toBeDefined();
const approval = write!.approval;
expect(typeof approval).toBe("function");
if (typeof approval !== "function") throw new Error("expected a function approval");
const tier = (path: string, content: string) => approval({ path, content });
// ast_edit on a filesystem path → write; on internal URLs only → read.
const astFsPath = JSON.stringify({
ops: [{ pat: "legacyWrap($A, $B)", out: "modernWrap($A, $B)" }],
paths: [filePath],
});
const astInternalPath = JSON.stringify({
ops: [{ pat: "a", out: "b" }],
paths: ["artifact://abc"],
});
expect(tier("xd://ast_edit", astFsPath)).toBe("write");
expect(tier("xd://ast_edit", astInternalPath)).toBe("read");
// debug: inspection action → read; a real launch → exec (control).
expect(tier("xd://debug", JSON.stringify({ action: "sessions" }))).toBe("read");
expect(tier("xd://debug", JSON.stringify({ action: "launch", program: "./app" }))).toBe("exec");
// Fail closed: malformed JSON, non-object payloads, missing content,
// and unknown devices all stay exec so the gate never under-prompts.
expect(tier("xd://ast_edit", "{ not json")).toBe("exec");
expect(tier("xd://ast_edit", "[1,2,3]")).toBe("exec");
expect(tier("xd://ast_edit", '"a string"')).toBe("exec");
expect(approval({ path: "xd://ast_edit" })).toBe("exec");
expect(tier("xd://no_such_device", "{}")).toBe("exec");
} finally {
await removeWithRetries(tempDir);
}
});
it("renderCall withholds a partial xd:// URL, then delegates once settled", async () => {
await themeModule.initTheme();
const uiTheme = (await themeModule.getThemeByName("dark")) ?? (await themeModule.getThemeByName("light"));