mirror of
https://github.com/SikongJueluo/pi-extensions.git
synced 2026-10-05 11:52:55 +08:00
refactor(pi-permission-inner-cmd): narrow bash recovery uniqueness to one message
- walk entries in reverse and stop at the latest assistant message containing the id - require the id to match exactly one block within that message rather than across the whole session - resolve a cross-message id reuse to the latest call being authorized - update ADR 0001 wording for the narrowed scope - add a regression test for cross-message id reuse
This commit is contained in:
@@ -8,12 +8,12 @@ status: accepted
|
|||||||
|
|
||||||
## Decision
|
## Decision
|
||||||
|
|
||||||
For direct calls to Pi's native `bash` tool, capture the session manager at `session_start`. During authorization, use `details.toolCallId` to find exactly one matching assistant `toolCall` block in the current session and read its structured `arguments.command` value.
|
For direct calls to Pi's native `bash` tool, capture the session manager at `session_start`. During authorization, walk the session in reverse to the latest assistant message containing a `toolCall` block whose `id` equals `details.toolCallId`, require exactly one such block *within that message*, and read its structured `arguments.command` value.
|
||||||
|
|
||||||
Proceed only when all evidence agrees:
|
Proceed only when all evidence agrees:
|
||||||
|
|
||||||
- the request and recovered tool call both identify the native `bash` tool;
|
- the request and recovered tool call both identify the native `bash` tool;
|
||||||
- the tool-call ID has exactly one match;
|
- the tool-call ID has exactly one match within the latest assistant message that contains it (a cross-message reuse resolves to the latest, which is the call being authorized);
|
||||||
- `arguments.command` is a string;
|
- `arguments.command` is a string;
|
||||||
- the complete input matches the strict wrapper grammar;
|
- the complete input matches the strict wrapper grammar;
|
||||||
- the inner command is not another recognized wrapper.
|
- the inner command is not another recognized wrapper.
|
||||||
@@ -35,7 +35,8 @@ The implementation must retain regression tests for:
|
|||||||
- inner `allow`, `ask`, and `deny` mapping;
|
- inner `allow`, `ask`, and `deny` mapping;
|
||||||
- compound input such as `timeout 60s pnpm test && git push`;
|
- compound input such as `timeout 60s pnpm test && git push`;
|
||||||
- unsupported and nested wrapper syntax;
|
- unsupported and nested wrapper syntax;
|
||||||
- missing, duplicate, malformed, and forwarded session evidence;
|
- missing, within-message duplicate, malformed, and forwarded session evidence;
|
||||||
|
- a tool-call id reused across messages (resolved to the latest);
|
||||||
- Authorizer registration and disposal.
|
- Authorizer registration and disposal.
|
||||||
|
|
||||||
The end-to-end experiment confirmed that permission-system checks every Bash command unit but invokes the Authorizer once for the aggregated `ask`; an Authorizer `allow` then releases the complete tool call. See [context ownership](../research/ai-bash-context-ownership.md) and [minimal Bash judgment evidence](../research/ai-bash-judge-input-minimality.md) for the underlying permission and evidence boundaries.
|
The end-to-end experiment confirmed that permission-system checks every Bash command unit but invokes the Authorizer once for the aggregated `ask`; an Authorizer `allow` then releases the complete tool call. See [context ownership](../research/ai-bash-context-ownership.md) and [minimal Bash judgment evidence](../research/ai-bash-judge-input-minimality.md) for the underlying permission and evidence boundaries.
|
||||||
|
|||||||
@@ -33,17 +33,21 @@ function extractBashCommand(block: ToolCallBlock): string | undefined {
|
|||||||
/**
|
/**
|
||||||
* Recover the complete native Bash command for one tool call.
|
* Recover the complete native Bash command for one tool call.
|
||||||
*
|
*
|
||||||
* Walks session entries, finds the assistant `toolCall` block whose `id` equals
|
* The tool call being authorized is always the most recent one, so entries are
|
||||||
* `toolCallId`, and reads its structured `arguments.command`. Proceeds only
|
* walked in reverse and the search stops at the first (latest) assistant
|
||||||
* when, per ADR 0001:
|
* message that contains a `toolCall` block whose `id` equals `toolCallId`.
|
||||||
*
|
*
|
||||||
* - exactly one block matches the id (no duplicate),
|
* Per ADR 0001, the id must match exactly one block *within that single
|
||||||
* - that block names the native Bash tool,
|
* message*. An earlier message reusing the same id is a stale, already-resolved
|
||||||
* - `arguments.command` is a string.
|
* call and is irrelevant to the current authorization; but two matching blocks
|
||||||
|
* inside one message cannot be disambiguated (we cannot tell which one
|
||||||
|
* `details.toolCallId` refers to), so that case stays fail-closed. The matched
|
||||||
|
* block must then name the native Bash tool and carry a string
|
||||||
|
* `arguments.command`.
|
||||||
*
|
*
|
||||||
* Any other outcome — no match, duplicate id, a non-Bash tool call, a
|
* Any other outcome — no match, a within-message duplicate id, a non-Bash tool
|
||||||
* non-string command, or malformed entries — returns `undefined` so the caller
|
* call, a non-string command, or malformed entries — returns `undefined` so the
|
||||||
* defers fail-closed.
|
* caller defers fail-closed.
|
||||||
*
|
*
|
||||||
* @returns the complete Bash command, or `undefined`.
|
* @returns the complete Bash command, or `undefined`.
|
||||||
*/
|
*/
|
||||||
@@ -51,33 +55,34 @@ export function recoverNativeBashCommand(
|
|||||||
entries: ReadonlyArray<SessionEntry>,
|
entries: ReadonlyArray<SessionEntry>,
|
||||||
toolCallId: string,
|
toolCallId: string,
|
||||||
): string | undefined {
|
): string | undefined {
|
||||||
let matches = 0;
|
for (let i = entries.length - 1; i >= 0; i--) {
|
||||||
let command: string | undefined;
|
const entry = entries[i];
|
||||||
|
|
||||||
for (const entry of entries) {
|
|
||||||
if (entry.type !== "message") {
|
if (entry.type !== "message") {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
const message = entry.message as { role?: unknown; content?: unknown };
|
const message = entry.message;
|
||||||
if (message.role !== "assistant") {
|
if (message.role !== "assistant") {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
const content = message.content;
|
|
||||||
if (!Array.isArray(content)) {
|
const matches: ToolCallBlock[] = [];
|
||||||
continue;
|
for (const block of message.content) {
|
||||||
}
|
if (isToolCallBlock(block) && block.id === toolCallId) {
|
||||||
for (const block of content) {
|
matches.push(block);
|
||||||
if (!isToolCallBlock(block) || block.id !== toolCallId) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
matches += 1;
|
|
||||||
// Keep walking the whole session so a duplicate id (two matching
|
|
||||||
// blocks) is detected even when the first match was unusable.
|
|
||||||
if (matches === 1) {
|
|
||||||
command = extractBashCommand(block);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return matches === 1 ? command : undefined;
|
if (matches.length === 0) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Latest message containing the id. Uniqueness only has to hold within
|
||||||
|
// this one message (see above); a cross-message reuse resolves to the
|
||||||
|
// latest, which is the call currently being authorized.
|
||||||
|
return matches.length === 1
|
||||||
|
? extractBashCommand(matches[0])
|
||||||
|
: undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,10 +3,10 @@ import type { SessionEntry } from "@earendil-works/pi-coding-agent";
|
|||||||
import { recoverNativeBashCommand } from "../src/recovery";
|
import { recoverNativeBashCommand } from "../src/recovery";
|
||||||
|
|
||||||
/** Build a minimal assistant message entry carrying the given content blocks. */
|
/** Build a minimal assistant message entry carrying the given content blocks. */
|
||||||
function assistantEntry(content: unknown[]): SessionEntry {
|
function assistantEntry(content: unknown[], id = "entry-1"): SessionEntry {
|
||||||
return {
|
return {
|
||||||
type: "message",
|
type: "message",
|
||||||
id: "entry-1",
|
id,
|
||||||
parentId: null,
|
parentId: null,
|
||||||
timestamp: "2026-08-08T00:00:00.000Z",
|
timestamp: "2026-08-08T00:00:00.000Z",
|
||||||
message: {
|
message: {
|
||||||
@@ -156,6 +156,25 @@ describe("recoverNativeBashCommand", () => {
|
|||||||
expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined();
|
expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("returns the latest command when the id recurs across messages", () => {
|
||||||
|
// A cross-message id reuse resolves to the latest block, which is the
|
||||||
|
// call currently being authorized; the earlier block is already-
|
||||||
|
// resolved history and must not fail-closed the recovery.
|
||||||
|
const entries = [
|
||||||
|
assistantEntry(
|
||||||
|
[bashToolCall("call_1", "timeout 30s rm -rf /")],
|
||||||
|
"entry-a",
|
||||||
|
),
|
||||||
|
assistantEntry(
|
||||||
|
[bashToolCall("call_1", "timeout 30s pnpm test")],
|
||||||
|
"entry-b",
|
||||||
|
),
|
||||||
|
];
|
||||||
|
expect(recoverNativeBashCommand(entries, "call_1")).toBe(
|
||||||
|
"timeout 30s pnpm test",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("tolerates a malformed content block that is not a tool call", () => {
|
it("tolerates a malformed content block that is not a tool call", () => {
|
||||||
const entries = [
|
const entries = [
|
||||||
assistantEntry([
|
assistantEntry([
|
||||||
|
|||||||
Reference in New Issue
Block a user