mirror of
https://github.com/SikongJueluo/pi-extensions.git
synced 2026-10-05 11:52:55 +08:00
refactor(pi-permission-inner-cmd): dispatch commands through a handler registry
- replace the hardcoded timeout switch with an engine that iterates registered handlers - extract the timeout logic into handlers/timeout.ts and add handlers/env.ts that defers env as non-transparent - thread a partial-evidence bag so the engine exception log retains handler-derived values like innerCommand - add CONTEXT.md with the transparent vs non-transparent wrapper glossary - record ADR 0002: env always defers to the AI judge and is never unwrapped
This commit is contained in:
+21
-46
@@ -1,53 +1,28 @@
|
||||
# Permission Authorization
|
||||
# pi-extensions
|
||||
|
||||
This context describes how ambiguous coding-agent operations are reviewed before they may execute.
|
||||
Personal `pi` coding-agent extensions. This context covers the permission
|
||||
extensions that inspect and re-evaluate Bash commands before they are allowed.
|
||||
|
||||
## Language
|
||||
|
||||
**Deterministic Permission Policy**:
|
||||
The rule-based authority that classifies an operation as allowed, denied, or requiring a decision.
|
||||
_Avoid_: Static judge
|
||||
### Wrappers
|
||||
|
||||
**Authorization Judge**:
|
||||
An independent reviewer that proposes a verdict for an operation the Deterministic Permission Policy could not decide.
|
||||
_Avoid_: Bash parser, safety classifier
|
||||
**Wrapper**:
|
||||
A Bash command of the form `<program> [modifier-args] <inner-command>`, where the
|
||||
authorization question is "what does the inner command do?". Whether a wrapper
|
||||
may be unwrapped depends on whether its modifier args are transparent.
|
||||
_Avoid_: command type, prefix command
|
||||
|
||||
**Shadow Mode**:
|
||||
An observation mode in which an Authorization Judge records a verdict without changing whether the operation executes.
|
||||
_Avoid_: Dry run
|
||||
**Transparent wrapper**:
|
||||
A wrapper whose modifier args do not change which program the inner command
|
||||
resolves to or its trust boundary (e.g. `timeout`). Stripping the modifiers and
|
||||
re-evaluating the inner command is sound: the verdict applies to the same
|
||||
program that actually runs.
|
||||
_Avoid_: safe wrapper
|
||||
|
||||
**Enforce Mode**:
|
||||
An authority mode in which an Authorization Judge's allow verdict may approve an operation. In the initial rollout, deny and defer still pass to the next authority.
|
||||
_Avoid_: Production mode
|
||||
|
||||
**Judge Participation**:
|
||||
The presence of an Authorization Judge in the configured authorizer chain. Participation determines whether the Judge is consulted, independently of whether it is in Shadow Mode or Enforce Mode.
|
||||
_Avoid_: Enabled, installed
|
||||
|
||||
**Defer**:
|
||||
A verdict stating that the available information or the judge itself is insufficient to decide, leaving the decision to the next authority.
|
||||
_Avoid_: Deny, error
|
||||
|
||||
**False Allow**:
|
||||
A Shadow Mode outcome in which the Authorization Judge proposes approval and the human reviewer rejects the same permission request.
|
||||
_Avoid_: False positive
|
||||
|
||||
**Requesting Session**:
|
||||
The session in which the operation requiring authorization originated. For a forwarded request, this is the child session.
|
||||
_Avoid_: Current session
|
||||
|
||||
**Serving Session**:
|
||||
The authority-bearing session that resolves a forwarded request and runs its configured Authorization Judge before the terminal human authority.
|
||||
_Avoid_: Parent context, current session
|
||||
|
||||
**Conversation Owner**:
|
||||
The session whose conversation entries are supplied to an Authorization Judge. It may differ from the Requesting Session for forwarded requests.
|
||||
_Avoid_: Requester
|
||||
|
||||
**Judgment Evidence**:
|
||||
The minimal operation, execution, user-intent, and forwarded-provenance facts supplied to an Authorization Judge for one verdict. Adapter validation state and review-log metadata are not Judgment Evidence.
|
||||
_Avoid_: Judge request, full request context
|
||||
|
||||
**Execution Working Directory**:
|
||||
The working directory of the Requesting Session in which an operation will execute and relative paths are resolved. For a forwarded request, it must not be replaced with the Serving Session's working directory.
|
||||
_Avoid_: Current cwd, parent cwd
|
||||
**Non-transparent wrapper**:
|
||||
A wrapper whose modifier args change the inner command's resolution or effect
|
||||
(e.g. `env`, whose `PATH=` or `-i` can make the inner name resolve to a
|
||||
different binary). Stripping the modifiers and re-evaluating the inner command
|
||||
is unsound: the verdict may apply to a different program than the one that runs.
|
||||
_Avoid_: unsafe wrapper
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
---
|
||||
status: accepted
|
||||
---
|
||||
|
||||
# Defer `env` to the AI judge; inner-cmd never unwraps it
|
||||
|
||||
`env` is the second command type considered for `pi-permission-inner-cmd`'s
|
||||
deterministic unwrapping. We decided inner-cmd **always defers** `env` — it
|
||||
never strips modifiers or re-evaluates an inner command — and lets the AI judge
|
||||
authority handle `env` commands together with their full environment context.
|
||||
|
||||
## Why
|
||||
|
||||
`env` is a *non-transparent wrapper*: its modifier args (`NAME=VALUE`, `-i`,
|
||||
`-u`, `-C`, `-S`) form an unbounded, implicit input channel that the
|
||||
command-string policy cannot see. Stripping them and re-evaluating the inner
|
||||
command is unsound, because the modifiers can change what actually runs without
|
||||
changing the inner command string:
|
||||
|
||||
- `PATH=/evil` — resolve the inner name to a different binary;
|
||||
- `LD_PRELOAD=/x.so`, `LD_LIBRARY_PATH` — inject arbitrary native code into the
|
||||
inner binary before its `main()`;
|
||||
- `BASH_ENV`, `PYTHONPATH`, `NODE_OPTIONS`, `PERL5OPT`, … — source scripts or
|
||||
inject code into the inner command's runtime;
|
||||
- `-i`, `-u NAME`, `-C DIR` — alter the execution context.
|
||||
|
||||
A "detect dangerous variables and defer" denylist is infeasible: the dangerous
|
||||
set is open-ended (new runtimes keep introducing new `*_OPT` / `*_PATH`
|
||||
injection variables), so any denylist is permanently incomplete, and one miss is
|
||||
an RCE. Detecting "does *any* modifier exist" is trivial and complete, but the
|
||||
only sound unwrap case is bare `env <cmd>` (zero modifiers), which is too rare in
|
||||
practice to justify the shell parsing it would require (assignment/flag
|
||||
detection, `-S`, `--`, `/usr/bin/env` basename, quoting).
|
||||
|
||||
## Considered options
|
||||
|
||||
- **Detect a `PATH=` override, otherwise unwrap** — rejected as false security:
|
||||
`LD_PRELOAD` and the interpreter-injection variables defeat it without
|
||||
touching `PATH`.
|
||||
- **Unwrap only bare `env <cmd>` (zero modifiers)** — sound, but bare `env` is
|
||||
rarely written and the boundary parsing adds real complexity for near-zero
|
||||
coverage. Not worth it.
|
||||
- **Always defer (chosen)** — zero parsing risk, zero soundness surface;
|
||||
non-transparent commands get the semantic judgement they require from the AI
|
||||
judge.
|
||||
|
||||
## Consequences
|
||||
|
||||
This establishes a clean division between the two permission authorities:
|
||||
|
||||
- **inner-cmd (deterministic)** unwraps only *transparent* wrappers (`timeout`)
|
||||
— sound, fast, narrow. Every uncertain or non-transparent command defers
|
||||
fail-closed.
|
||||
- **The AI judge authority** receives deferred commands (including `env`) and
|
||||
reasons about the full command *with* its environment, where non-transparency
|
||||
can be understood semantically rather than stripped.
|
||||
|
||||
This is why the AI judge consumes the full recovered command from the shared
|
||||
recovery module (ADR 0001) — it needs the complete input, modifiers included, to
|
||||
judge non-transparent wrappers. See `CONTEXT.md` for the transparent /
|
||||
non-transparent wrapper distinction.
|
||||
@@ -5,14 +5,11 @@ import type {
|
||||
PermissionQuery,
|
||||
PromptPermissionDetails,
|
||||
} from "@gotgenes/pi-permission-system";
|
||||
import { classifyWrapper, isRecognizedWrapper } from "./recognizer";
|
||||
import {
|
||||
NATIVE_BASH_TOOL_NAME,
|
||||
recoverNativeBashCommand,
|
||||
} from "@sikongjueluo/pi-permission-shared";
|
||||
|
||||
/** Bash permission surface queried when re-evaluating the inner command. */
|
||||
const BASH_SURFACE = "bash";
|
||||
import { handlers } from "./handlers";
|
||||
|
||||
/** Convert a thrown value into a short, log-safe string. */
|
||||
function toErrorString(error: unknown): string {
|
||||
@@ -49,24 +46,22 @@ export interface InnerCommandAuthorizerDeps {
|
||||
}
|
||||
|
||||
/**
|
||||
* V0.1 inner-command Authorizer decision (ADR 0001).
|
||||
* Inner-command Authorizer decision (ADR 0001).
|
||||
*
|
||||
* Recovers the complete native Bash command for `details.toolCallId` from the
|
||||
* captured session, unwraps one strict `timeout` level, and re-evaluates the
|
||||
* inner command through the deterministic permission policy. Every uncertain
|
||||
* path — forwarded requests, a session-identity mismatch, non-Bash tools,
|
||||
* missing/duplicate/malformed session evidence, unsupported or nested wrapper
|
||||
* syntax, parse failures, and exceptions — defers to the next authority.
|
||||
* Revalidates root ownership, recovers the complete native Bash command for
|
||||
* `details.toolCallId` from the captured session, then hands it to the first
|
||||
* registered handler that claims it. Each handler owns its own recognition and
|
||||
* verdict logic: the timeout handler unwraps one level and re-evaluates the
|
||||
* inner command; the env handler defers as non-transparent.
|
||||
*
|
||||
* Logging contract:
|
||||
* - `review` only for a recognized wrapper whose inner command resolves to a
|
||||
* decisive `allow`/`deny`.
|
||||
* - `debug` for a recognized inner `ask`, unsupported timeout syntax, nested
|
||||
* wrappers, a session-identity mismatch, and exceptions.
|
||||
* - ordinary non-timeout commands defer silently.
|
||||
* - recognized logs carry both `command` and `innerCommand`; an exception after
|
||||
* recognition retains both alongside `error`, while an earlier exception logs
|
||||
* only the safe data available at that point.
|
||||
* Every uncertain path — forwarded requests, a session-identity mismatch,
|
||||
* non-Bash tools, missing session evidence, an unrecognized command, or any
|
||||
* exception — defers to the next authority (fail-closed).
|
||||
*
|
||||
* Logging: handlers emit their own review/debug events for decisive and
|
||||
* notable-defer outcomes; silent deferrals log nothing. Exceptions are logged
|
||||
* by this engine as `inner_cmd.exception`, retaining the recovered command and
|
||||
* any partial evidence the active handler recorded before throwing.
|
||||
*/
|
||||
export async function authorizeInnerCommand(
|
||||
deps: InnerCommandAuthorizerDeps,
|
||||
@@ -75,18 +70,17 @@ export async function authorizeInnerCommand(
|
||||
|
||||
// Track recovered evidence so an exception after recognition can retain it.
|
||||
let command: string | undefined;
|
||||
let innerCommand: string | undefined;
|
||||
let evidence: Record<string, unknown> = {};
|
||||
|
||||
try {
|
||||
// Forwarded subagent asks are out of scope for v0.1: the captured
|
||||
// session is the serving root's conversation, not the requester's.
|
||||
// Forwarded subagent asks are out of scope: the captured session is the
|
||||
// serving root's conversation, not the requester's.
|
||||
if (details.forwarding) {
|
||||
return { kind: "defer" };
|
||||
}
|
||||
|
||||
// Revalidate root ownership: the live session must still be the one we
|
||||
// registered for. A mismatch (or a session id that cannot be read)
|
||||
// means the captured conversation can no longer be attributed safely.
|
||||
// registered for.
|
||||
const currentSessionId = session.getSessionId();
|
||||
if (currentSessionId !== expectedSessionId) {
|
||||
log.debug("inner_cmd.session_mismatch", {
|
||||
@@ -113,59 +107,15 @@ export async function authorizeInnerCommand(
|
||||
return { kind: "defer" };
|
||||
}
|
||||
|
||||
const classification = classifyWrapper(command);
|
||||
|
||||
switch (classification.kind) {
|
||||
case "nonTimeout":
|
||||
// An ordinary Bash command this authorizer does not unwrap.
|
||||
return { kind: "defer" };
|
||||
|
||||
case "unsupportedTimeout":
|
||||
log.debug("inner_cmd.unsupported_timeout_syntax", { command });
|
||||
return { kind: "defer" };
|
||||
|
||||
case "recognized": {
|
||||
innerCommand = classification.match.innerCommand;
|
||||
|
||||
// Never unwrap more than one level in v0.1.
|
||||
if (isRecognizedWrapper(innerCommand)) {
|
||||
log.debug("inner_cmd.nested_timeout", {
|
||||
command,
|
||||
innerCommand,
|
||||
});
|
||||
return { kind: "defer" };
|
||||
}
|
||||
|
||||
const result = query.checkPermission(
|
||||
BASH_SURFACE,
|
||||
innerCommand,
|
||||
details.agentName ?? undefined,
|
||||
);
|
||||
|
||||
switch (result.state) {
|
||||
case "allow":
|
||||
log.review("inner_cmd.allow", {
|
||||
command,
|
||||
innerCommand,
|
||||
});
|
||||
return { kind: "allow" };
|
||||
case "deny":
|
||||
log.review("inner_cmd.deny", {
|
||||
command,
|
||||
innerCommand,
|
||||
});
|
||||
return { kind: "deny" };
|
||||
case "ask":
|
||||
default:
|
||||
// Treat any unexpected state as a safe defer.
|
||||
log.debug("inner_cmd.inner_ask", {
|
||||
command,
|
||||
innerCommand,
|
||||
});
|
||||
return { kind: "defer" };
|
||||
}
|
||||
// Dispatch to the first registered handler that claims the command.
|
||||
for (const handler of handlers) {
|
||||
evidence = {};
|
||||
const verdict = handler.decide({ command, details, query, log, evidence });
|
||||
if (verdict !== undefined) {
|
||||
return verdict;
|
||||
}
|
||||
}
|
||||
return { kind: "defer" };
|
||||
} catch (error) {
|
||||
const exceptionDetails: Record<string, unknown> = {
|
||||
error: toErrorString(error),
|
||||
@@ -173,9 +123,7 @@ export async function authorizeInnerCommand(
|
||||
if (command !== undefined) {
|
||||
exceptionDetails.command = command;
|
||||
}
|
||||
if (innerCommand !== undefined) {
|
||||
exceptionDetails.innerCommand = innerCommand;
|
||||
}
|
||||
Object.assign(exceptionDetails, evidence);
|
||||
log.debug("inner_cmd.exception", exceptionDetails);
|
||||
return { kind: "defer" };
|
||||
}
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import type { CommandHandler } from "./types";
|
||||
|
||||
/** Matches a command whose leading program is `env`. */
|
||||
const ENV_PREFIX = /^env(?:[ \t]|$)/;
|
||||
|
||||
/**
|
||||
* The `env` wrapper handler.
|
||||
*
|
||||
* `env` is non-transparent: its modifier args (`NAME=VALUE`, `-i`, `-u`) can
|
||||
* change which binary the inner command resolves to (e.g. a `PATH=` override),
|
||||
* so stripping them and re-evaluating the inner command is unsound. `env` is
|
||||
* therefore claimed but always deferred — it never unwraps. Commands not
|
||||
* starting with `env` return `undefined` so the engine can try the next
|
||||
* handler.
|
||||
*/
|
||||
export const envHandler: CommandHandler = {
|
||||
id: "env",
|
||||
decide({ command, log }) {
|
||||
if (!ENV_PREFIX.test(command)) {
|
||||
return undefined;
|
||||
}
|
||||
log.debug("inner_cmd.env_non_transparent", { command });
|
||||
return { kind: "defer" };
|
||||
},
|
||||
};
|
||||
@@ -0,0 +1,10 @@
|
||||
import { envHandler } from "./env";
|
||||
import { timeoutHandler } from "./timeout";
|
||||
import type { CommandHandler } from "./types";
|
||||
|
||||
/**
|
||||
* Registered command handlers, tried in order. The first to return a verdict
|
||||
* claims the command; the rest are not consulted. Add a handler here (and a
|
||||
* new file under `handlers/`) to support a new command type.
|
||||
*/
|
||||
export const handlers: readonly CommandHandler[] = [timeoutHandler, envHandler];
|
||||
@@ -0,0 +1,56 @@
|
||||
import { classifyWrapper, isRecognizedWrapper } 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).
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
export const timeoutHandler: CommandHandler = {
|
||||
id: "timeout",
|
||||
decide(ctx) {
|
||||
const { command, details, query, log, evidence } = ctx;
|
||||
const classification = classifyWrapper(command);
|
||||
switch (classification.kind) {
|
||||
case "nonTimeout":
|
||||
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 });
|
||||
return { kind: "defer" };
|
||||
}
|
||||
const result = query.checkPermission(
|
||||
BASH_SURFACE,
|
||||
innerCommand,
|
||||
details.agentName ?? undefined,
|
||||
);
|
||||
switch (result.state) {
|
||||
case "allow":
|
||||
log.review("inner_cmd.allow", { command, innerCommand });
|
||||
return { kind: "allow" };
|
||||
case "deny":
|
||||
log.review("inner_cmd.deny", { command, innerCommand });
|
||||
return { kind: "deny" };
|
||||
case "ask":
|
||||
default:
|
||||
log.debug("inner_cmd.inner_ask", { command, innerCommand });
|
||||
return { kind: "defer" };
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
@@ -0,0 +1,39 @@
|
||||
import type {
|
||||
AuthorizerLog,
|
||||
AuthorizerVerdict,
|
||||
PermissionQuery,
|
||||
PromptPermissionDetails,
|
||||
} from "@gotgenes/pi-permission-system";
|
||||
|
||||
/** Context handed to a handler for one recovered command. */
|
||||
export interface HandlerContext {
|
||||
/** The full recovered Bash command. */
|
||||
readonly command: string;
|
||||
readonly details: PromptPermissionDetails;
|
||||
readonly query: PermissionQuery;
|
||||
readonly log: AuthorizerLog;
|
||||
/**
|
||||
* Partial-result bag. A handler that recognizes the command records derived
|
||||
* values here (e.g. `ctx.evidence.innerCommand = innerCommand`) so the
|
||||
* engine's exception log retains them if `decide` later throws. The engine
|
||||
* resets this bag per handler.
|
||||
*/
|
||||
readonly evidence: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* A self-contained verdict strategy for one kind of Bash command.
|
||||
*
|
||||
* The engine walks registered handlers in order; the first that returns a
|
||||
* verdict claims the command. Returning `undefined` means "not mine" and lets
|
||||
* the engine try the next handler.
|
||||
*/
|
||||
export interface CommandHandler {
|
||||
/** Stable id for logs, e.g. "timeout". */
|
||||
readonly id: string;
|
||||
/**
|
||||
* Inspect the command: return a verdict to claim it (the engine stops), or
|
||||
* `undefined` to pass to the next handler.
|
||||
*/
|
||||
decide(ctx: HandlerContext): AuthorizerVerdict | undefined;
|
||||
}
|
||||
@@ -402,6 +402,36 @@ describe("authorizeInnerCommand — fail-closed deferrals", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("authorizeInnerCommand — env wrapper", () => {
|
||||
it("defers on an env wrapper with a debug log (non-transparent)", async () => {
|
||||
const { verdict, log, check } = await run({
|
||||
recoveredCommand: "env FOO=bar pnpm test",
|
||||
states: { "pnpm test": "allow" },
|
||||
});
|
||||
expect(verdict.kind).toBe("defer");
|
||||
// env is non-transparent: never unwrapped, so the inner command is not
|
||||
// re-evaluated through the deterministic policy.
|
||||
expect(check).toEqual([]);
|
||||
expect(log).toEqual([
|
||||
{
|
||||
level: "debug",
|
||||
event: "inner_cmd.env_non_transparent",
|
||||
details: { command: "env FOO=bar pnpm test" },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not claim a command that only contains env later", async () => {
|
||||
// Starts with printf, not env -> no handler claims it -> silent defer.
|
||||
const { verdict, log } = await run({
|
||||
recoveredCommand: "printf hi; env | sort",
|
||||
states: {},
|
||||
});
|
||||
expect(verdict.kind).toBe("defer");
|
||||
expect(log).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
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({
|
||||
|
||||
Reference in New Issue
Block a user