fix: auto-allow safe read-only commands in acceptEdits mode (#341)
* fix: auto-allow safe read-only commands in acceptEdits mode In acceptEdits mode, read-only commands like grep, cat, ls, find, head, tail were still prompting for approval. This created unnecessary friction since these commands cannot modify or delete files. Add safe read-only commands to ACCEPT_EDITS_ALLOWED_COMMANDS: grep, cat, ls, find, head, tail, echo, pwd, wc, sort, uniq, diff These are all read-only — they cannot cause data loss or modify the filesystem. Auto-allowing them reduces approval fatigue in acceptEdits mode without introducing any safety risk. Write commands (rm, rmdir, mv, cp, sed, mkdir, touch) are unchanged. The dangerous path guard for rm/rmdir remains in place. Fixes #251. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(bash): block unsafe acceptEdits auto-allow Keep the new read-only acceptEdits commands behind the existing read-only validator and block shell redirection based on the original command text. This prevents commands like echo > file and find -delete from being silently auto-approved while preserving safe read-only commands. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
44
src/tools/BashTool/modeValidation.test.ts
Normal file
44
src/tools/BashTool/modeValidation.test.ts
Normal file
@@ -0,0 +1,44 @@
|
|||||||
|
import { expect, test } from 'bun:test'
|
||||||
|
import { getEmptyToolPermissionContext } from '../../Tool.js'
|
||||||
|
import { checkPermissionMode } from './modeValidation.js'
|
||||||
|
|
||||||
|
const acceptEditsContext = {
|
||||||
|
...getEmptyToolPermissionContext(),
|
||||||
|
mode: 'acceptEdits' as const,
|
||||||
|
}
|
||||||
|
|
||||||
|
test('acceptEdits does not auto-allow read commands with output redirection', () => {
|
||||||
|
const result = checkPermissionMode(
|
||||||
|
{ command: 'echo hello > output.txt' } as never,
|
||||||
|
acceptEditsContext,
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result.behavior).toBe('passthrough')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('acceptEdits does not auto-allow mutating find invocations', () => {
|
||||||
|
const result = checkPermissionMode(
|
||||||
|
{ command: 'find . -delete' } as never,
|
||||||
|
acceptEditsContext,
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result.behavior).toBe('passthrough')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('acceptEdits still auto-allows safe read-only commands', () => {
|
||||||
|
const result = checkPermissionMode(
|
||||||
|
{ command: 'grep foo package.json' } as never,
|
||||||
|
acceptEditsContext,
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result.behavior).toBe('allow')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('acceptEdits still blocks dangerous rm paths even in auto-allow mode', () => {
|
||||||
|
const result = checkPermissionMode(
|
||||||
|
{ command: 'rm -rf ~' } as never,
|
||||||
|
acceptEditsContext,
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result.behavior).toBe('ask')
|
||||||
|
})
|
||||||
@@ -1,12 +1,15 @@
|
|||||||
import type { z } from 'zod/v4'
|
import type { z } from 'zod/v4'
|
||||||
import type { ToolPermissionContext } from '../../Tool.js'
|
import type { ToolPermissionContext } from '../../Tool.js'
|
||||||
import { splitCommand_DEPRECATED } from '../../utils/bash/commands.js'
|
import { splitCommand_DEPRECATED } from '../../utils/bash/commands.js'
|
||||||
|
import { tryParseShellCommand } from '../../utils/bash/shellQuote.js'
|
||||||
import { getCwd } from '../../utils/cwd.js'
|
import { getCwd } from '../../utils/cwd.js'
|
||||||
import type { PermissionResult } from '../../utils/permissions/PermissionResult.js'
|
import type { PermissionResult } from '../../utils/permissions/PermissionResult.js'
|
||||||
import type { BashTool } from './BashTool.js'
|
import type { BashTool } from './BashTool.js'
|
||||||
|
import { checkReadOnlyConstraints } from './readOnlyValidation.js'
|
||||||
import { checkDangerousRemovalPaths } from './pathValidation.js'
|
import { checkDangerousRemovalPaths } from './pathValidation.js'
|
||||||
|
|
||||||
const ACCEPT_EDITS_ALLOWED_COMMANDS = [
|
const ACCEPT_EDITS_WRITE_COMMANDS = [
|
||||||
|
// Filesystem write commands
|
||||||
'mkdir',
|
'mkdir',
|
||||||
'touch',
|
'touch',
|
||||||
'rm',
|
'rm',
|
||||||
@@ -16,15 +19,64 @@ const ACCEPT_EDITS_ALLOWED_COMMANDS = [
|
|||||||
'sed',
|
'sed',
|
||||||
] as const
|
] as const
|
||||||
|
|
||||||
type FilesystemCommand = (typeof ACCEPT_EDITS_ALLOWED_COMMANDS)[number]
|
const ACCEPT_EDITS_READ_ONLY_COMMANDS = [
|
||||||
|
// Safe read-only commands — cannot modify files or cause data loss.
|
||||||
|
// These still need to pass the existing read-only validator so redirects and
|
||||||
|
// dangerous flags fall through to the normal permission flow.
|
||||||
|
'grep',
|
||||||
|
'cat',
|
||||||
|
'ls',
|
||||||
|
'find',
|
||||||
|
'head',
|
||||||
|
'tail',
|
||||||
|
'echo',
|
||||||
|
'pwd',
|
||||||
|
'wc',
|
||||||
|
'sort',
|
||||||
|
'uniq',
|
||||||
|
'diff',
|
||||||
|
] as const
|
||||||
|
|
||||||
function isFilesystemCommand(command: string): command is FilesystemCommand {
|
type AcceptEditsWriteCommand = (typeof ACCEPT_EDITS_WRITE_COMMANDS)[number]
|
||||||
return ACCEPT_EDITS_ALLOWED_COMMANDS.includes(command as FilesystemCommand)
|
type AcceptEditsReadOnlyCommand =
|
||||||
|
(typeof ACCEPT_EDITS_READ_ONLY_COMMANDS)[number]
|
||||||
|
|
||||||
|
function isAcceptEditsWriteCommand(
|
||||||
|
command: string,
|
||||||
|
): command is AcceptEditsWriteCommand {
|
||||||
|
return ACCEPT_EDITS_WRITE_COMMANDS.includes(command as AcceptEditsWriteCommand)
|
||||||
|
}
|
||||||
|
|
||||||
|
function isAcceptEditsReadOnlyCommand(
|
||||||
|
command: string,
|
||||||
|
): command is AcceptEditsReadOnlyCommand {
|
||||||
|
return ACCEPT_EDITS_READ_ONLY_COMMANDS.includes(
|
||||||
|
command as AcceptEditsReadOnlyCommand,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasShellRedirection(cmd: string): boolean {
|
||||||
|
const parsed = tryParseShellCommand(cmd, env => `$${env}`)
|
||||||
|
if (!parsed.success) {
|
||||||
|
// Fail closed: unparseable commands should go through the normal prompt flow.
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
return parsed.tokens.some(
|
||||||
|
token =>
|
||||||
|
typeof token === 'object' &&
|
||||||
|
token !== null &&
|
||||||
|
'op' in token &&
|
||||||
|
['>', '>>', '>|', '&>', '&>>', '1>', '1>>', '2>', '2>>'].includes(
|
||||||
|
String(token.op),
|
||||||
|
),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
function validateCommandForMode(
|
function validateCommandForMode(
|
||||||
cmd: string,
|
cmd: string,
|
||||||
toolPermissionContext: ToolPermissionContext,
|
toolPermissionContext: ToolPermissionContext,
|
||||||
|
originalInput: string,
|
||||||
): PermissionResult {
|
): PermissionResult {
|
||||||
const trimmedCmd = cmd.trim()
|
const trimmedCmd = cmd.trim()
|
||||||
const [baseCmd] = trimmedCmd.split(/\s+/)
|
const [baseCmd] = trimmedCmd.split(/\s+/)
|
||||||
@@ -36,10 +88,10 @@ function validateCommandForMode(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// In Accept Edits mode, auto-allow filesystem operations
|
// In Accept Edits mode, auto-allow filesystem write operations.
|
||||||
if (
|
if (
|
||||||
toolPermissionContext.mode === 'acceptEdits' &&
|
toolPermissionContext.mode === 'acceptEdits' &&
|
||||||
isFilesystemCommand(baseCmd)
|
isAcceptEditsWriteCommand(baseCmd)
|
||||||
) {
|
) {
|
||||||
// Guard: always run dangerous path check for rm/rmdir before auto-allowing.
|
// Guard: always run dangerous path check for rm/rmdir before auto-allowing.
|
||||||
// This prevents rm -rf ~ / rm -rf / from bypassing checkDangerousRemovalPaths
|
// This prevents rm -rf ~ / rm -rf / from bypassing checkDangerousRemovalPaths
|
||||||
@@ -62,6 +114,37 @@ function validateCommandForMode(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// In Accept Edits mode, only auto-allow read-only commands if they still
|
||||||
|
// pass the full read-only validator. This prevents redirects and mutating
|
||||||
|
// find forms from being silently auto-approved.
|
||||||
|
if (
|
||||||
|
toolPermissionContext.mode === 'acceptEdits' &&
|
||||||
|
isAcceptEditsReadOnlyCommand(baseCmd)
|
||||||
|
) {
|
||||||
|
if (hasShellRedirection(originalInput)) {
|
||||||
|
return {
|
||||||
|
behavior: 'passthrough',
|
||||||
|
message:
|
||||||
|
'Read-only commands with shell redirection require normal permission checks',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const readOnlyResult = checkReadOnlyConstraints(
|
||||||
|
{ command: cmd } as z.infer<typeof BashTool.inputSchema>,
|
||||||
|
false,
|
||||||
|
)
|
||||||
|
if (readOnlyResult.behavior === 'allow') {
|
||||||
|
return {
|
||||||
|
behavior: 'allow',
|
||||||
|
updatedInput: { command: cmd },
|
||||||
|
decisionReason: {
|
||||||
|
type: 'mode',
|
||||||
|
mode: 'acceptEdits',
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
behavior: 'passthrough',
|
behavior: 'passthrough',
|
||||||
message: `No mode-specific handling for '${baseCmd}' in ${toolPermissionContext.mode} mode`,
|
message: `No mode-specific handling for '${baseCmd}' in ${toolPermissionContext.mode} mode`,
|
||||||
@@ -106,7 +189,7 @@ export function checkPermissionMode(
|
|||||||
|
|
||||||
// Check each subcommand
|
// Check each subcommand
|
||||||
for (const cmd of commands) {
|
for (const cmd of commands) {
|
||||||
const result = validateCommandForMode(cmd, toolPermissionContext)
|
const result = validateCommandForMode(cmd, toolPermissionContext, input.command)
|
||||||
|
|
||||||
// If any command triggers mode-specific behavior, return that result
|
// If any command triggers mode-specific behavior, return that result
|
||||||
if (result.behavior !== 'passthrough') {
|
if (result.behavior !== 'passthrough') {
|
||||||
@@ -124,5 +207,7 @@ export function checkPermissionMode(
|
|||||||
export function getAutoAllowedCommands(
|
export function getAutoAllowedCommands(
|
||||||
mode: ToolPermissionContext['mode'],
|
mode: ToolPermissionContext['mode'],
|
||||||
): readonly string[] {
|
): readonly string[] {
|
||||||
return mode === 'acceptEdits' ? ACCEPT_EDITS_ALLOWED_COMMANDS : []
|
return mode === 'acceptEdits'
|
||||||
|
? [...ACCEPT_EDITS_WRITE_COMMANDS, ...ACCEPT_EDITS_READ_ONLY_COMMANDS]
|
||||||
|
: []
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user