SikongJueluo/pi-extensions
1
0
Fork 0
mirror of https://github.com/SikongJueluo/pi-extensions.git synced 2026-10-05 11:52:55 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(pi-permission-inner-cmd): unwrap timeout in real-world command forms

Browse Source 
- accept GNU timeout durations without a unit suffix and with decimals (timeout 240 …)
- detect the wrapper on details.command and strip it from the full command so scaffolded inputs (cd … && timeout … | tail) unwrap
- re-evaluate the full de-wrapped compound so sibling commands cannot hide behind the wrapper allow
- defer fail-closed when the unit is not a unique substring of the full command
- amend ADR 0001 with the relaxed grammar and the scaffolded-command handling
This commit is contained in:
SikongJueluo
2026-08-12 11:20:46 +08:00
parent 43ae2db90b
commit 6008c9e817
5 changed files with 240 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/adr/0001-recover-full-bash-command-from-session.md
+26 -3
View File
@@ -15,17 +15,40 @@ Proceed only when all evidence agrees:
- the request and recovered tool call both identify the native `bash` tool;
- 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;
- the complete input matches the strict wrapper grammar;
- the ask-triggering unit (`details.command`) matches the wrapper grammar;
- the inner command is not another recognized wrapper.
V0.1 recognizes only:
V0.1 recognizes:
```regex
^timeout[ \t]+([1-9][0-9]*[smhd])[ \t]+(.+)$
^timeout[ \t]+([1-9][0-9]*(?:\.[0-9]+)?[smhd]?)[ \t]+(.+)$
```
The duration follows GNU timeout's grammar: a positive number (integer or
decimal) with an optional unit `s`/`m`/`h`/`d` (default seconds), so a bare
integer such as `timeout 240 cmd` is accepted. The duration format is irrelevant
to unwrap soundness — the duration is discarded and only the inner command is
re-evaluated — so the full numeric grammar is allowed. `0`/leading-zero, `ms`,
and flags remain excluded. (Amended from the original unit-required grammar
after `timeout 240 …` was seen in real usage.)
The complete inner command is re-evaluated through the Deterministic Permission Policy. Map `allow` to `allow`, `deny` to `deny`, and `ask` to `defer`. Missing or ambiguous session evidence, forwarded requests, shell aliases, unsupported `timeout` options, nested wrappers, parse failures, and exceptions all produce `defer`.
**Scaffolded commands (amended).** The recovered full command may be a compound
that does not start with the wrapper — e.g. a subagent scaffold
`cd … && timeout … | tail; echo EXIT`. Detection therefore runs on
`details.command` (the unit the permission system isolated, always
wrapper-leading). The wrapper is stripped from the *full* command (the unit
text is replaced by its unwrapped inner, required to occur exactly once) and the
entire de-wrapped compound is re-evaluated through the Deterministic Permission
Policy, which decomposes it into units and keeps the most restrictive — so
sibling commands are still judged and cannot hide behind the wrapper's allow. If
the unit is not a recognized wrapper, cannot be located exactly once, or the
re-evaluation is not `allow`, the authorizer defers fail-closed. (The original
design detected on the full command only, which missed every scaffolded command;
`details.command` is now used for *detection and locating* only — judgment still
runs on the full command, preserving the "never trust a truncated unit" rule.)
## Consequences
This avoids changing permission-system and avoids parsing authorization evidence from the human-facing `details.message`. It deliberately supports fewer contexts: forwarded and non-native shell calls continue through the existing authority chain.
packages/pi-permission-inner-cmd/src/handlers/timeout.ts
+99 -27
View File
@@ -1,56 +1,128 @@
import { classifyWrapper, isRecognizedWrapper } from "../recognizer";
import {
isRecognizedWrapper,
parseTimeoutWrapper,
TIMEOUT_PREFIX,
} from "../recognizer";
import type { CommandHandler } from "./types";
/** Bash permission surface queried when re-evaluating the inner command. */
const BASH_SURFACE = "bash";
/**
* The strict simple-timeout wrapper handler (ADR 0001).
* Replace the wrapper unit with its unwrapped inner inside the full command,
* exactly once. Returns `undefined` when the unit is not a unique substring
* (absent, or appears more than once), so the caller defers fail-closed rather
* than guess where to strip.
*/
function stripWrapperUnit(
fullCommand: string,
unit: string,
inner: string,
): string | undefined {
const first = fullCommand.indexOf(unit);
if (first === -1) {
return undefined;
}
if (fullCommand.indexOf(unit, first + unit.length) !== -1) {
return undefined;
}
return (
fullCommand.slice(0, first) +
inner +
fullCommand.slice(first + unit.length)
);
}
/**
* The simple-timeout wrapper handler (ADR 0001).
*
* Unwraps `timeout <duration> <command>`, rejects nested wrappers, and
* re-evaluates the complete inner program through the deterministic policy.
* Unsupported timeout syntax and nested wrappers defer with a debug log.
* Commands that are not timeout at all return `undefined` so the engine can try
* the next handler.
* Detection runs on `details.command` — the command unit the permission system
* isolated as the ask trigger — which is always wrapper-leading even when the
* full recovered command is a scaffold that starts with `cd`/`echo`/…. The
* wrapper is then stripped from the FULL command and the whole de-wrapped
* compound is re-evaluated, so sibling commands (including dangerous ones) are
* still judged and cannot hide behind the wrapper's allow.
*
* Unsupported timeout syntax, a nested wrapper, a unit that cannot be located
* exactly once in the full command, and any non-allowing re-evaluation all
* defer fail-closed.
*/
export const timeoutHandler: CommandHandler = {
id: "timeout",
decide(ctx) {
const { command, details, query, log, evidence } = ctx;
const classification = classifyWrapper(command);
switch (classification.kind) {
case "nonTimeout":
const { command: fullCommand, details, query, log, evidence } = ctx;
const unit = details.command;
if (unit === undefined) {
return undefined;
case "unsupportedTimeout":
log.debug("inner_cmd.unsupported_timeout_syntax", { command });
return { kind: "defer" };
case "recognized": {
const innerCommand = classification.match.innerCommand;
// Record the derived inner command so the engine's exception
// log retains it if the re-evaluation below throws.
evidence.innerCommand = innerCommand;
if (isRecognizedWrapper(innerCommand)) {
log.debug("inner_cmd.nested_timeout", { command, innerCommand });
}
const unitMatch = parseTimeoutWrapper(unit);
if (unitMatch === undefined) {
// Not the recognized form. If it still names `timeout`, surface it
// as unsupported; otherwise this unit is not ours.
if (TIMEOUT_PREFIX.test(unit)) {
log.debug("inner_cmd.unsupported_timeout_syntax", {
command: fullCommand,
});
return { kind: "defer" };
}
return undefined;
}
const innerCommand = unitMatch.innerCommand;
evidence.innerCommand = innerCommand;
// Never unwrap into another wrapper.
if (isRecognizedWrapper(innerCommand)) {
log.debug("inner_cmd.nested_timeout", {
command: fullCommand,
innerCommand,
});
return { kind: "defer" };
}
// Strip the wrapper from the full command (handles scaffolds). Defer
// fail-closed if the unit is not a unique substring.
const unwrappedFull = stripWrapperUnit(
fullCommand,
unit,
innerCommand,
);
if (unwrappedFull === undefined) {
log.debug("inner_cmd.wrapper_not_located", {
command: fullCommand,
});
return { kind: "defer" };
}
// Authoritative: re-evaluate the full de-wrapped compound. The
// permission system decomposes it into units and keeps the most
// restrictive, so any non-allowing sibling defers here.
const result = query.checkPermission(
BASH_SURFACE,
innerCommand,
unwrappedFull,
details.agentName ?? undefined,
);
switch (result.state) {
case "allow":
log.review("inner_cmd.allow", { command, innerCommand });
log.review("inner_cmd.allow", {
command: fullCommand,
innerCommand,
});
return { kind: "allow" };
case "deny":
log.review("inner_cmd.deny", { command, innerCommand });
log.review("inner_cmd.deny", {
command: fullCommand,
innerCommand,
});
return { kind: "deny" };
case "ask":
default:
log.debug("inner_cmd.inner_ask", { command, innerCommand });
log.debug("inner_cmd.inner_ask", {
command: fullCommand,
innerCommand,
});
return { kind: "defer" };
}
}
}
},
};
packages/pi-permission-inner-cmd/src/recognizer.ts
+13 -8
View File
@@ -1,23 +1,28 @@
/**
* V0.1 wrapper recognizer.
*
* The strict simple-timeout grammar from ADR 0001. V0.1 unwraps exactly
* The simple-timeout grammar from ADR 0001. V0.1 unwraps exactly
* `timeout <duration> <command>`; every other `timeout` invocation is left to
* the next authority.
*/
/**
* Matches `timeout <duration> <command>` where `<duration>` is a positive
* integer (no leading zero) followed by a single unit `s`/`m`/`h`/`d`.
* Matches `timeout <duration> <command>` where `<duration>` follows GNU
* timeout's grammar: a positive number (integer or decimal, no leading zero)
* with an optional unit `s`/`m`/`h`/`d` (default seconds). A bare integer such
* as `timeout 240 cmd` is therefore accepted (240 seconds).
*
* Flags (`-k`, `--preserve-status`, GNU `--`), compound durations, and the
* bare form are intentionally excluded so v0.1 never unwraps a wrapper it
* cannot re-evaluate safely.
* The duration format is irrelevant to unwrap soundness — the duration is
* discarded and only the inner command is re-evaluated — so GNU's full numeric
* grammar is accepted. Still excluded: `0`/leading-zero durations, `ms` (not a
* timeout unit), multi-letter units, and flags (`-k`, `--preserve-status`, GNU
* `--`), so a wrapper that cannot be re-evaluated safely is never unwrapped.
*/
const TIMEOUT_WRAPPER_PATTERN = /^timeout[ \t]+([1-9][0-9]*[smhd])[ \t]+(.+)$/;
const TIMEOUT_WRAPPER_PATTERN =
/^timeout[ \t]+([1-9][0-9]*(?:\.[0-9]+)?[smhd]?)[ \t]+(.+)$/;
/** A command that begins with the bare `timeout` wrapper program. */
const TIMEOUT_PREFIX = /^timeout(?:[ \t]|$)/;
export const TIMEOUT_PREFIX = /^timeout(?:[ \t]|$)/;
export interface TimeoutWrapperMatch {
readonly duration: string;
packages/pi-permission-inner-cmd/test/authorizer.test.ts
+70 -3
View File
@@ -79,6 +79,7 @@ function entriesRecovering(command: string, toolCallId = "call_1"): SessionEntry
function bashDetails(
toolCallId = "call_1",
agentName: string | null = null,
command?: string,
): PromptPermissionDetails {
return {
requestId: "req-1",
@@ -87,8 +88,8 @@ function bashDetails(
message: "May I run bash?",
toolCallId,
toolName: "bash",
// details.command is intentionally the winning unit, not the full input.
command: "ignored-winning-unit",
// details.command is the winning unit the permission system isolated.
command,
};
}
@@ -117,6 +118,8 @@ function makeSessionProbe(args: {
async function run(args: {
recoveredCommand: string;
/** details.command — the ask-triggering unit; defaults to recoveredCommand. */
unitCommand?: string;
states?: Record<string, PermissionState>;
details?: Partial<PromptPermissionDetails>;
getEntriesThrows?: boolean;
@@ -141,8 +144,12 @@ async function run(args: {
getSessionIdThrows: args.getSessionIdThrows,
sessionId: args.sessionMismatch ? "session-changed" : ROOT_SESSION_ID,
});
const unitCommand = args.unitCommand ?? args.recoveredCommand;
const verdict = await authorizeInnerCommand({
details: { ...bashDetails(toolCallId), ...args.details } as PromptPermissionDetails,
details: {
...bashDetails(toolCallId, null, unitCommand),
...args.details,
} as PromptPermissionDetails,
query,
log,
session,
@@ -462,6 +469,66 @@ describe("authorizeInnerCommand — xargs wrapper", () => {
});
});
describe("authorizeInnerCommand — scaffolded commands", () => {
it("unwraps a timeout buried in a cd/echo/|/tail scaffold", async () => {
const full =
"cd /repo && echo go && timeout 240 pnpm install --frozen-lockfile 2>&1 | tail -30; echo EXIT";
const deWrapped =
"cd /repo && echo go && pnpm install --frozen-lockfile 2>&1 | tail -30; echo EXIT";
const { verdict, log, check } = await run({
recoveredCommand: full,
unitCommand: "timeout 240 pnpm install --frozen-lockfile",
states: { [deWrapped]: "allow" },
});
expect(verdict.kind).toBe("allow");
// re-evaluated the full de-wrapped compound, not just the unit's inner
expect(check).toEqual([
{ surface: "bash", value: deWrapped, agentName: undefined },
]);
expect(log).toEqual([
{
level: "review",
event: "inner_cmd.allow",
details: {
command: full,
innerCommand: "pnpm install --frozen-lockfile",
},
},
]);
});
it("defers when the de-wrapped compound is not fully allowing (sibling)", async () => {
const full = "cd /repo && timeout 30s pnpm test && git push origin";
const deWrapped = "cd /repo && pnpm test && git push origin";
const { verdict, check } = await run({
recoveredCommand: full,
unitCommand: "timeout 30s pnpm test",
states: {},
});
expect(verdict.kind).toBe("defer");
// the de-wrapped compound still contains the git push sibling
expect(check).toEqual([
{ surface: "bash", value: deWrapped, agentName: undefined },
]);
});
it("defers fail-closed when the unit is not a unique substring", async () => {
const { verdict, log } = await run({
recoveredCommand: "timeout 30s pnpm test",
unitCommand: "timeout 30s pnpm test",
states: { "pnpm test": "allow" },
});
expect(verdict.kind).toBe("defer");
expect(log).toEqual([
{
level: "debug",
event: "inner_cmd.wrapper_not_located",
details: { command: "timeout 30s pnpm test" },
},
]);
});
});
describe("authorizeInnerCommand — exceptions defer with a debug log", () => {
it("defers when reading the session id throws (logs only safe data)", async () => {
const { verdict, log } = await run({
packages/pi-permission-inner-cmd/test/recognizer.test.ts
+19 -2
View File
@@ -6,7 +6,7 @@ import {
} from "../src/recognizer";
describe("parseTimeoutWrapper", () => {
it("matches the strict simple-timeout form", () => {
it("matches the simple-timeout form", () => {
expect(parseTimeoutWrapper("timeout 30s pnpm test")).toEqual({
duration: "30s",
innerCommand: "pnpm test",
@@ -25,6 +25,21 @@ describe("parseTimeoutWrapper", () => {
});
});
it("accepts GNU durations: bare integer (seconds) and decimals", () => {
expect(parseTimeoutWrapper("timeout 240 pnpm test")).toEqual({
duration: "240",
innerCommand: "pnpm test",
});
expect(parseTimeoutWrapper("timeout 1.5h deploy")).toEqual({
duration: "1.5h",
innerCommand: "deploy",
});
expect(parseTimeoutWrapper("timeout 2.5s build")).toEqual({
duration: "2.5s",
innerCommand: "build",
});
});
it("preserves compound inner programs as the inner command", () => {
expect(parseTimeoutWrapper("timeout 60s pnpm test && git push")).toEqual({
duration: "60s",
@@ -47,8 +62,10 @@ describe("parseTimeoutWrapper", () => {
});
});
it("rejects leading-zero and multi-letter durations", () => {
it("rejects zero, leading-zero, ms, and multi-letter durations", () => {
expect(parseTimeoutWrapper("timeout 0 pnpm test")).toBeUndefined();
expect(parseTimeoutWrapper("timeout 0s pnpm test")).toBeUndefined();
expect(parseTimeoutWrapper("timeout 0.5s pnpm test")).toBeUndefined();
expect(parseTimeoutWrapper("timeout 030s pnpm test")).toBeUndefined();
expect(parseTimeoutWrapper("timeout 30ms pnpm test")).toBeUndefined();
expect(parseTimeoutWrapper("timeout 30sec pnpm test")).toBeUndefined();