Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions apps/x/apps/renderer/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1527,9 +1527,15 @@ function App() {
}
}, [runId, isStopping, stopClickedAt])

const handlePermissionResponse = useCallback(async (toolCallId: string, subflow: string[], response: 'approve' | 'deny') => {
const handlePermissionResponse = useCallback(async (
toolCallId: string,
subflow: string[],
response: 'approve' | 'deny',
scope?: 'once' | 'session' | 'always',
command?: string,
) => {
if (!runId) return

// Optimistically update the UI immediately
setPermissionResponses(prev => {
const next = new Map(prev)
Expand All @@ -1541,11 +1547,11 @@ function App() {
next.delete(toolCallId)
return next
})

try {
await window.ipc.invoke('runs:authorizePermission', {
runId,
authorization: { subflow, toolCallId, response }
authorization: { subflow, toolCallId, response, scope, command }
})
} catch (error) {
console.error('Failed to authorize permission:', error)
Expand Down Expand Up @@ -2945,6 +2951,14 @@ function App() {
<PermissionRequest
toolCall={permRequest.toolCall}
onApprove={() => handlePermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve')}
onApproveSession={() => {
const cmd = permRequest.toolCall.toolName === 'executeCommand' && typeof permRequest.toolCall.arguments === 'object' && permRequest.toolCall.arguments !== null && 'command' in permRequest.toolCall.arguments ? String(permRequest.toolCall.arguments.command) : undefined
handlePermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve', 'session', cmd)
}}
onApproveAlways={() => {
const cmd = permRequest.toolCall.toolName === 'executeCommand' && typeof permRequest.toolCall.arguments === 'object' && permRequest.toolCall.arguments !== null && 'command' in permRequest.toolCall.arguments ? String(permRequest.toolCall.arguments.command) : undefined
handlePermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve', 'always', cmd)
}}
onDeny={() => handlePermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'deny')}
isProcessing={isActive && isProcessing}
response={response}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,23 @@

import { Badge } from "@/components/ui/badge";
import { Button } from "@/components/ui/button";
import {
DropdownMenu,
DropdownMenuTrigger,
DropdownMenuContent,
DropdownMenuItem,
} from "@/components/ui/dropdown-menu";
import { cn } from "@/lib/utils";
import { AlertTriangleIcon, CheckCircleIcon, CheckIcon, XCircleIcon, XIcon } from "lucide-react";
import { AlertTriangleIcon, CheckCircleIcon, CheckIcon, ChevronDownIcon, XCircleIcon, XIcon } from "lucide-react";
import type { ComponentProps } from "react";
import { ToolCallPart } from "@x/shared/dist/message.js";
import z from "zod";

export type PermissionRequestProps = ComponentProps<"div"> & {
toolCall: z.infer<typeof ToolCallPart>;
onApprove?: () => void;
onApproveSession?: () => void;
onApproveAlways?: () => void;
onDeny?: () => void;
isProcessing?: boolean;
response?: 'approve' | 'deny' | null;
Expand All @@ -20,6 +28,8 @@ export const PermissionRequest = ({
className,
toolCall,
onApprove,
onApproveSession,
onApproveAlways,
onDeny,
isProcessing = false,
response = null,
Expand Down Expand Up @@ -117,16 +127,40 @@ export const PermissionRequest = ({
</div>
{!isResponded && (
<div className="flex items-center gap-2 pt-2">
<Button
variant="default"
size="sm"
onClick={onApprove}
disabled={isProcessing}
className="flex-1"
>
<CheckIcon className="size-4" />
Approve
</Button>
<div className="flex flex-1 items-center">
<Button
variant="default"
size="sm"
onClick={onApprove}
disabled={isProcessing}
className={cn("flex-1", command && "rounded-r-none")}
>
<CheckIcon className="size-4" />
Approve
</Button>
{command && (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
variant="default"
size="sm"
disabled={isProcessing}
className="rounded-l-none border-l border-l-primary-foreground/20 px-1.5"
>
<ChevronDownIcon className="size-3.5" />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuItem onClick={onApproveSession}>
Allow for Session
</DropdownMenuItem>
<DropdownMenuItem onClick={onApproveAlways}>
Always Allow
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
)}
</div>
<Button
variant="destructive"
size="sm"
Expand Down
10 changes: 9 additions & 1 deletion apps/x/apps/renderer/src/components/chat-sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ interface ChatSidebarProps {
pendingAskHumanRequests?: ChatTabViewState['pendingAskHumanRequests']
allPermissionRequests?: ChatTabViewState['allPermissionRequests']
permissionResponses?: ChatTabViewState['permissionResponses']
onPermissionResponse?: (toolCallId: string, subflow: string[], response: PermissionResponse) => void
onPermissionResponse?: (toolCallId: string, subflow: string[], response: PermissionResponse, scope?: 'once' | 'session' | 'always', command?: string) => void
onAskHumanResponse?: (toolCallId: string, subflow: string[], response: string) => void
isToolOpenForTab?: (tabId: string, toolId: string) => boolean
onToolOpenChangeForTab?: (tabId: string, toolId: string, open: boolean) => void
Expand Down Expand Up @@ -444,6 +444,14 @@ export function ChatSidebar({
<PermissionRequest
toolCall={permRequest.toolCall}
onApprove={() => onPermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve')}
onApproveSession={() => {
const cmd = permRequest.toolCall.toolName === 'executeCommand' && typeof permRequest.toolCall.arguments === 'object' && permRequest.toolCall.arguments !== null && 'command' in permRequest.toolCall.arguments ? String(permRequest.toolCall.arguments.command) : undefined
onPermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve', 'session', cmd)
}}
onApproveAlways={() => {
const cmd = permRequest.toolCall.toolName === 'executeCommand' && typeof permRequest.toolCall.arguments === 'object' && permRequest.toolCall.arguments !== null && 'command' in permRequest.toolCall.arguments ? String(permRequest.toolCall.arguments.command) : undefined
onPermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'approve', 'always', cmd)
}}
onDeny={() => onPermissionResponse(permRequest.toolCall.toolCallId, permRequest.subflow, 'deny')}
isProcessing={isActive && isProcessing}
response={response}
Expand Down
69 changes: 65 additions & 4 deletions apps/x/packages/core/src/application/lib/command-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,78 @@ import { promisify } from 'util';
import { getSecurityAllowList } from '../../config/security.js';

const execPromise = promisify(exec);
const COMMAND_SPLIT_REGEX = /(?:\|\||&&|;|\||\n)/;
const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=.*/;
const WRAPPER_COMMANDS = new Set(['sudo', 'env', 'time', 'command']);

function sanitizeToken(token: string): string {
return token.trim().replace(/^['"]+|['"]+$/g, '');
return token.trim().replace(/^['"()]+|['"()]+$/g, '');
}

function extractCommandNames(command: string): string[] {
/**
* Split a shell command string on command separators (||, &&, ;, |, \n)
* while respecting single and double quotes.
*/
function splitCommandSegments(command: string): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this approach following any documented practices for parsing commands? If so, can we include a reference to that? If not, can we use a proven parsing-based solution here?

const segments: string[] = [];
let current = '';
let inSingle = false;
let inDouble = false;

for (let i = 0; i < command.length; i++) {
const ch = command[i];

if (ch === "'" && !inDouble) {
inSingle = !inSingle;
current += ch;
continue;
}
if (ch === '"' && !inSingle) {
inDouble = !inDouble;
current += ch;
continue;
}

if (inSingle || inDouble) {
current += ch;
continue;
}

// Outside quotes — check for separators
if (ch === '\n' || ch === ';') {
segments.push(current);
current = '';
continue;
}
if (ch === '|') {
if (command[i + 1] === '|') {
// ||
segments.push(current);
current = '';
i++; // skip second |
} else {
// single pipe
segments.push(current);
current = '';
}
continue;
}
if (ch === '&' && command[i + 1] === '&') {
segments.push(current);
current = '';
i++; // skip second &
continue;
}

current += ch;
}

if (current) segments.push(current);
return segments;
}

export function extractCommandNames(command: string): string[] {
const discovered = new Set<string>();
const segments = command.split(COMMAND_SPLIT_REGEX);
const segments = splitCommandSegments(command);

for (const segment of segments) {
const tokens = segment.trim().split(/\s+/).filter(Boolean);
Expand Down
49 changes: 43 additions & 6 deletions apps/x/packages/core/src/config/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,33 @@ const DEFAULT_ALLOW_LIST = [
let cachedAllowList: string[] | null = null;
let cachedMtimeMs: number | null = null;

/** In-memory session allowlist — resets on app restart */
const sessionAllowSet = new Set<string>();

export function addToSessionAllowList(commands: string[]): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

session means a specific run. This implementation assumes session to be the app-session, which is incorrect. Additionally, it is storing the session prefs in memory, which will not survive app-restarts as documented in comments above.

Instead, the agent runtime should simply incorporate session-wide permissions in its state (the state-builder can take care of this). This way, when the state is rebuilt from the run.jsonl file, the session-specific permission grants are restored automatically

for (const cmd of commands) {
const normalized = cmd.trim().toLowerCase();
if (normalized) sessionAllowSet.add(normalized);
}
}

export async function addToSecurityConfig(commands: string[]): Promise<void> {
ensureSecurityConfigSync();
const current = readAllowList();
const merged = new Set(current);
for (const cmd of commands) {
const normalized = cmd.trim().toLowerCase();
if (normalized) merged.add(normalized);
}
await fsPromises.writeFile(
SECURITY_CONFIG_PATH,
JSON.stringify(Array.from(merged).sort(), null, 2) + "\n",
"utf8",
);
// Reset cache so next read picks up the new file
resetSecurityAllowListCache();
}

/**
* Async function to ensure security config file exists.
* Called explicitly at app startup via initConfigs().
Expand Down Expand Up @@ -99,18 +126,28 @@ export function getSecurityAllowList(): string[] {
ensureSecurityConfigSync();
try {
const stats = fs.statSync(SECURITY_CONFIG_PATH);
let fileList: string[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we don't expect any code changes in this part of security.ts since no session-related data needs to be picked up from here.

if (cachedAllowList && cachedMtimeMs === stats.mtimeMs) {
return cachedAllowList;
fileList = cachedAllowList;
} else {
fileList = readAllowList();
cachedAllowList = fileList;
cachedMtimeMs = stats.mtimeMs;
}

const allowList = readAllowList();
cachedAllowList = allowList;
cachedMtimeMs = stats.mtimeMs;
return allowList;
// Merge session allowlist
if (sessionAllowSet.size === 0) return fileList;
const merged = new Set(fileList);
for (const cmd of sessionAllowSet) merged.add(cmd);
return Array.from(merged);
} catch {
cachedAllowList = null;
cachedMtimeMs = null;
return readAllowList();
const fileList = readAllowList();
if (sessionAllowSet.size === 0) return fileList;
const merged = new Set(fileList);
for (const cmd of sessionAllowSet) merged.add(cmd);
return Array.from(merged);
}
}

Expand Down
18 changes: 17 additions & 1 deletion apps/x/packages/core/src/runs/runs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { IBus } from "../application/lib/bus.js";
import { IAbortRegistry } from "./abort-registry.js";
import { IRunsLock } from "./lock.js";
import { forceCloseAllMcpClients } from "../mcp/mcp.js";
import { extractCommandNames } from "../application/lib/command-executor.js";
import { addToSessionAllowList, addToSecurityConfig } from "../config/security.js";

export async function createRun(opts: z.infer<typeof CreateRunOptions>): Promise<z.infer<typeof Run>> {
const repo = container.resolve<IRunsRepo>('runsRepo');
Expand All @@ -26,9 +28,23 @@ export async function createMessage(runId: string, message: string): Promise<str
}

export async function authorizePermission(runId: string, ev: z.infer<typeof ToolPermissionAuthorizePayload>): Promise<void> {
const { scope, command, ...rest } = ev;

// Handle scope side-effects when approving
if (rest.response === "approve" && command && scope && scope !== "once") {
const commandNames = extractCommandNames(command);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

command name(s) should be extracted from the original tool call, not from the auth response

if (commandNames.length > 0) {
if (scope === "session") {
addToSessionAllowList(commandNames);
} else if (scope === "always") {
await addToSecurityConfig(commandNames);
}
}
}

const repo = container.resolve<IRunsRepo>('runsRepo');
const event: z.infer<typeof ToolPermissionResponseEvent> = {
...ev,
...rest,
runId,
type: "tool-permission-response",
};
Expand Down
3 changes: 3 additions & 0 deletions apps/x/packages/shared/src/runs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ export const ToolPermissionAuthorizePayload = ToolPermissionResponseEvent.pick({
subflow: true,
toolCallId: true,
response: true,
}).extend({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues

  • why extend the shape instead of adding these fields to the original shape?
  • why do we need to add command here? The response is already linked to a permission request through toolCallId, so the backend would be aware of the original command in the tool-call

scope: z.enum(["once", "session", "always"]).optional(),
command: z.string().optional(),
});

export const AskHumanResponsePayload = AskHumanResponseEvent.pick({
Expand Down