mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix: honor bypass permission mode (#910)
Make bypassPermissions skip permission-approval ask decisions while preserving explicit denies and tools that require user interaction. Keep desktop permission mode state authoritative by waiting for server/CLI confirmation and persisting CLI-originated mode broadcasts. Tested: - bun test src/utils/permissions/permissions.test.ts - bun test src/server/__tests__/conversations.test.ts -t "permission switch|permission restart|permission-mode broadcasts|permission changes made before|bypass permissions back to default|runtime-only model switch" - bun test src/server/__tests__/ws-memory-events.test.ts - cd desktop && bun run test -- src/stores/chatStore.test.ts --run - bun run check:server Scope-risk: moderate
This commit is contained in:
parent
78d85f0c64
commit
531ce4063d
@ -2448,6 +2448,13 @@ describe('chatStore history mapping', () => {
|
||||
type: 'set_permission_mode',
|
||||
mode: 'acceptEdits',
|
||||
})
|
||||
expect(updateSessionPermissionModeMock).not.toHaveBeenCalled()
|
||||
|
||||
useChatStore.getState().handleServerMessage('session-1', {
|
||||
type: 'permission_mode_changed',
|
||||
mode: 'acceptEdits',
|
||||
})
|
||||
|
||||
expect(updateSessionPermissionModeMock).toHaveBeenCalledWith('session-1', 'acceptEdits')
|
||||
})
|
||||
|
||||
|
||||
@ -1219,7 +1219,6 @@ export const useChatStore = create<ChatStore>((set, get) => ({
|
||||
|
||||
setSessionPermissionMode: (sessionId, mode) => {
|
||||
if (!get().sessions[sessionId]) return
|
||||
useSessionStore.getState().updateSessionPermissionMode(sessionId, mode)
|
||||
wsManager.send(sessionId, { type: 'set_permission_mode', mode })
|
||||
},
|
||||
|
||||
|
||||
@ -3279,6 +3279,8 @@ describe('WebSocket Chat Integration', () => {
|
||||
const ws = new WebSocket(`${wsUrl}/ws/${sessionId}`)
|
||||
let switchTriggered = false
|
||||
let turnComplete = false
|
||||
let modeConfirmedBeforeTurnComplete = false
|
||||
let deferredInspectionChecked = false
|
||||
try {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
@ -3286,7 +3288,7 @@ describe('WebSocket Chat Integration', () => {
|
||||
reject(new Error(`Timed out waiting for active-turn permission switch for session ${sessionId}`))
|
||||
}, 10_000)
|
||||
|
||||
ws.onmessage = (event) => {
|
||||
ws.onmessage = async (event) => {
|
||||
const msg = JSON.parse(event.data as string)
|
||||
|
||||
if (msg.type === 'connected') {
|
||||
@ -3312,9 +3314,29 @@ describe('WebSocket Chat Integration', () => {
|
||||
type: 'set_permission_mode',
|
||||
mode: 'bypassPermissions',
|
||||
}))
|
||||
await new Promise((resolve) => setTimeout(resolve, 25))
|
||||
const inspectionRes = await fetch(`${baseUrl}/api/sessions/${sessionId}/inspection?includeContext=0`)
|
||||
if (!inspectionRes.ok) {
|
||||
clearTimeout(timeout)
|
||||
ws.close()
|
||||
reject(new Error(`Inspection failed while permission switch was deferred: ${inspectionRes.status}`))
|
||||
return
|
||||
}
|
||||
const inspectionBody = await inspectionRes.json() as { status?: { permissionMode?: string } }
|
||||
deferredInspectionChecked = true
|
||||
if (inspectionBody.status?.permissionMode !== 'default') {
|
||||
clearTimeout(timeout)
|
||||
ws.close()
|
||||
reject(new Error(`Deferred permission switch was exposed before restart: ${inspectionBody.status?.permissionMode}`))
|
||||
return
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
if (msg.type === 'permission_mode_changed' && !turnComplete) {
|
||||
modeConfirmedBeforeTurnComplete = true
|
||||
}
|
||||
|
||||
if (
|
||||
msg.type === 'status' &&
|
||||
msg.state === 'idle' &&
|
||||
@ -3349,6 +3371,8 @@ describe('WebSocket Chat Integration', () => {
|
||||
|
||||
expect(switchTriggered).toBe(true)
|
||||
expect(turnComplete).toBe(true)
|
||||
expect(deferredInspectionChecked).toBe(true)
|
||||
expect(modeConfirmedBeforeTurnComplete).toBe(false)
|
||||
expect(startCalls).toHaveLength(2)
|
||||
expect(startCalls[0]).toMatchObject({
|
||||
sessionId,
|
||||
@ -3467,6 +3491,11 @@ describe('WebSocket Chat Integration', () => {
|
||||
.filter((msg) => msg.type === 'status')
|
||||
.map((msg) => msg.state),
|
||||
).toEqual(['idle'])
|
||||
expect(
|
||||
messages
|
||||
.slice(switchStartIndex)
|
||||
.some((msg) => msg.type === 'permission_mode_changed' && msg.mode === 'bypassPermissions'),
|
||||
).toBe(true)
|
||||
expect(messages.slice(switchStartIndex).some((msg) => msg.type === 'error')).toBe(false)
|
||||
} finally {
|
||||
ws.close()
|
||||
@ -3512,6 +3541,7 @@ describe('WebSocket Chat Integration', () => {
|
||||
}) as typeof conversationService.startSession
|
||||
|
||||
const ws = new WebSocket(`${wsUrl}/ws/${sessionId}`)
|
||||
const messages: any[] = []
|
||||
try {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
@ -3519,6 +3549,7 @@ describe('WebSocket Chat Integration', () => {
|
||||
}, 5000)
|
||||
ws.onmessage = (event) => {
|
||||
const msg = JSON.parse(event.data as string)
|
||||
messages.push(msg)
|
||||
if (msg.type === 'connected') {
|
||||
clearTimeout(timeout)
|
||||
ws.send(JSON.stringify({
|
||||
@ -3544,6 +3575,10 @@ describe('WebSocket Chat Integration', () => {
|
||||
const body = await res.json() as { status?: { permissionMode?: string } }
|
||||
return body.status?.permissionMode === 'acceptEdits'
|
||||
}, `persisted inactive permission switch for ${sessionId}`)
|
||||
expect(messages.some((msg) =>
|
||||
msg.type === 'permission_mode_changed' &&
|
||||
msg.mode === 'acceptEdits'
|
||||
)).toBe(true)
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
@ -3666,6 +3701,72 @@ describe('WebSocket Chat Integration', () => {
|
||||
}
|
||||
}, 20_000)
|
||||
|
||||
it('should persist CLI-originated permission-mode broadcasts', async () => {
|
||||
const createRes = await fetch(`${baseUrl}/api/sessions`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workDir: process.cwd(), permissionMode: 'default' }),
|
||||
})
|
||||
expect(createRes.status).toBe(201)
|
||||
const { sessionId } = await createRes.json() as { sessionId: string }
|
||||
|
||||
const ws = new WebSocket(`${wsUrl}/ws/${sessionId}`)
|
||||
const messages: any[] = []
|
||||
try {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
reject(new Error(`Timed out waiting for CLI permission broadcast turn for session ${sessionId}`))
|
||||
}, 10_000)
|
||||
ws.onmessage = (event) => {
|
||||
const msg = JSON.parse(event.data as string)
|
||||
messages.push(msg)
|
||||
if (msg.type === 'connected') {
|
||||
ws.send(JSON.stringify({ type: 'user_message', content: 'turn before CLI permission broadcast' }))
|
||||
return
|
||||
}
|
||||
if (msg.type === 'message_complete') {
|
||||
clearTimeout(timeout)
|
||||
resolve()
|
||||
}
|
||||
if (msg.type === 'error') {
|
||||
clearTimeout(timeout)
|
||||
reject(new Error(msg.message))
|
||||
}
|
||||
}
|
||||
ws.onerror = () => {
|
||||
clearTimeout(timeout)
|
||||
reject(new Error(`WebSocket error for CLI permission broadcast session ${sessionId}`))
|
||||
}
|
||||
})
|
||||
expect(conversationService.hasSession(sessionId)).toBe(true)
|
||||
|
||||
conversationService.handleSdkPayload(sessionId, `${JSON.stringify({
|
||||
type: 'system',
|
||||
subtype: 'status',
|
||||
status: null,
|
||||
permissionMode: 'acceptEdits',
|
||||
})}\n`)
|
||||
|
||||
await waitUntil(
|
||||
() => messages.some((msg) =>
|
||||
msg.type === 'permission_mode_changed' &&
|
||||
msg.mode === 'acceptEdits'
|
||||
),
|
||||
`forwarded CLI permission broadcast for ${sessionId}`,
|
||||
)
|
||||
expect(conversationService.getSessionPermissionMode(sessionId)).toBe('acceptEdits')
|
||||
await waitUntil(async () => {
|
||||
const res = await fetch(`${baseUrl}/api/sessions/${sessionId}/inspection?includeContext=0`)
|
||||
if (!res.ok) return false
|
||||
const body = await res.json() as { status?: { permissionMode?: string } }
|
||||
return body.status?.permissionMode === 'acceptEdits'
|
||||
}, `persisted CLI permission broadcast for ${sessionId}`)
|
||||
} finally {
|
||||
ws.close()
|
||||
conversationService.stopSession(sessionId)
|
||||
}
|
||||
}, 20_000)
|
||||
|
||||
it('should ignore stale persisted runtime provider ids when resuming old sessions', async () => {
|
||||
const providerService = new ProviderService()
|
||||
const activeProvider = await providerService.addProvider({
|
||||
|
||||
@ -526,6 +526,13 @@ export class ConversationService {
|
||||
return sent
|
||||
}
|
||||
|
||||
recordSessionPermissionMode(sessionId: string, mode: string): boolean {
|
||||
const session = this.sessions.get(sessionId)
|
||||
if (!session) return false
|
||||
session.permissionMode = mode
|
||||
return true
|
||||
}
|
||||
|
||||
setMaxThinkingTokens(sessionId: string, maxThinkingTokens: number | null): boolean {
|
||||
return this.sendSdkMessage(sessionId, {
|
||||
type: 'control_request',
|
||||
|
||||
@ -654,7 +654,6 @@ async function handleSetPermissionMode(
|
||||
const pendingStartup = sessionStartupPromises.get(sessionId)
|
||||
|
||||
if (pendingStartup) {
|
||||
await persistSessionPermissionMode(sessionId, message.mode)
|
||||
await enqueueRuntimeTransition(sessionId, async () => {
|
||||
await pendingStartup.catch(() => undefined)
|
||||
if (!conversationService.hasSession(sessionId)) return
|
||||
@ -664,7 +663,9 @@ async function handleSetPermissionMode(
|
||||
}
|
||||
|
||||
if (!conversationService.hasSession(sessionId)) {
|
||||
await persistSessionPermissionMode(sessionId, message.mode)
|
||||
if (await persistSessionPermissionMode(sessionId, message.mode)) {
|
||||
sendMessage(ws, { type: 'permission_mode_changed', mode: message.mode })
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@ -700,11 +701,13 @@ async function applyPermissionModeToActiveSession(
|
||||
const currentMode = conversationService.getSessionPermissionMode(sessionId)
|
||||
if (shouldDeferRuntimeRestartForActiveTurn(sessionId)) {
|
||||
deferredPermissionModes.set(sessionId, mode)
|
||||
await persistSessionPermissionMode(sessionId, mode)
|
||||
return
|
||||
}
|
||||
|
||||
if (currentMode === mode) return
|
||||
if (currentMode === mode) {
|
||||
sendMessage(ws, { type: 'permission_mode_changed', mode })
|
||||
return
|
||||
}
|
||||
const needsRestart = shouldRestartForPermissionMode(currentMode, mode)
|
||||
|
||||
if (needsRestart) {
|
||||
@ -720,6 +723,7 @@ async function applyPermissionModeToActiveSession(
|
||||
return
|
||||
}
|
||||
await persistSessionPermissionMode(sessionId, mode)
|
||||
sendMessage(ws, { type: 'permission_mode_changed', mode })
|
||||
}
|
||||
|
||||
async function handleSetRuntimeConfig(
|
||||
@ -828,6 +832,7 @@ async function restartSessionWithPermissionMode(
|
||||
`?token=${encodeURIComponent(crypto.randomUUID())}`
|
||||
await conversationService.startSession(sessionId, workDir, sdkUrl, runtimeSettings)
|
||||
|
||||
sendMessage(ws, { type: 'permission_mode_changed', mode })
|
||||
sendMessage(ws, { type: 'status', state: 'idle' })
|
||||
console.log(`[WS] Restarted CLI for ${sessionId} with permission mode: ${mode}`)
|
||||
} catch (err) {
|
||||
@ -856,18 +861,19 @@ async function persistSessionPermissionMode(
|
||||
sessionId: string,
|
||||
mode: string,
|
||||
knownWorkDir?: string | null,
|
||||
): Promise<void> {
|
||||
): Promise<boolean> {
|
||||
const workDir =
|
||||
knownWorkDir ||
|
||||
conversationService.getSessionWorkDir(sessionId) ||
|
||||
await sessionService.getSessionWorkDir(sessionId).catch(() => null)
|
||||
|
||||
if (!workDir) return
|
||||
if (!workDir) return false
|
||||
|
||||
await sessionService.appendSessionMetadata(sessionId, {
|
||||
workDir,
|
||||
permissionMode: mode,
|
||||
})
|
||||
return true
|
||||
}
|
||||
|
||||
async function persistSessionRuntimeConfig(
|
||||
@ -2441,6 +2447,7 @@ function bindClientSessionOutput(
|
||||
return
|
||||
}
|
||||
|
||||
handleCliPermissionModeBroadcast(sessionId, cliMsg)
|
||||
const serverMsgs = translateCliMessage(cliMsg, sessionId)
|
||||
for (const msg of serverMsgs) {
|
||||
sendMessage(ws, msg)
|
||||
@ -2452,6 +2459,30 @@ function bindClientSessionOutput(
|
||||
conversationService.onOutput(sessionId, callback)
|
||||
}
|
||||
|
||||
function getCliPermissionModeBroadcast(cliMsg: any): string | null {
|
||||
if (
|
||||
cliMsg?.type === 'system' &&
|
||||
cliMsg.subtype === 'status' &&
|
||||
typeof cliMsg.permissionMode === 'string'
|
||||
) {
|
||||
return cliMsg.permissionMode
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function handleCliPermissionModeBroadcast(sessionId: string, cliMsg: any): void {
|
||||
const mode = getCliPermissionModeBroadcast(cliMsg)
|
||||
if (!mode) return
|
||||
|
||||
const currentMode = conversationService.getSessionPermissionMode(sessionId)
|
||||
if (currentMode === mode) return
|
||||
|
||||
if (!conversationService.recordSessionPermissionMode(sessionId, mode)) return
|
||||
void persistSessionPermissionMode(sessionId, mode).catch((err) => {
|
||||
console.warn(`[WS] Failed to persist CLI permission mode broadcast for ${sessionId}:`, err)
|
||||
})
|
||||
}
|
||||
|
||||
type RuntimeSettings = {
|
||||
permissionMode?: string
|
||||
model?: string
|
||||
|
||||
@ -322,7 +322,8 @@ export async function* runPostToolUseFailureHooks<Input extends AnyObject>(
|
||||
* Resolve a PreToolUse hook's permission result into a final PermissionDecision.
|
||||
*
|
||||
* Encapsulates the invariant that hook 'allow' does NOT bypass settings.json
|
||||
* deny/ask rules — checkRuleBasedPermissions still applies (inc-4788 analog).
|
||||
* deny rules or non-bypass ask rules — checkRuleBasedPermissions still applies
|
||||
* (inc-4788 analog).
|
||||
* Also handles the requiresUserInteraction/requireCanUseTool guards and the
|
||||
* 'ask' forceDecision passthrough.
|
||||
*
|
||||
@ -369,7 +370,8 @@ export async function resolveHookPermissionDecision(
|
||||
}
|
||||
}
|
||||
|
||||
// Hook allow skips the interactive prompt, but deny/ask rules still apply.
|
||||
// Hook allow skips the interactive prompt, but deny rules and non-bypass
|
||||
// ask rules still apply.
|
||||
const ruleCheck = await checkRuleBasedPermissions(
|
||||
tool,
|
||||
hookInput,
|
||||
|
||||
286
src/utils/permissions/permissions.test.ts
Normal file
286
src/utils/permissions/permissions.test.ts
Normal file
@ -0,0 +1,286 @@
|
||||
import { describe, expect, it } from 'bun:test'
|
||||
import type { Tool, ToolPermissionContext, ToolUseContext } from '../../Tool.js'
|
||||
import { getEmptyToolPermissionContext } from '../../Tool.js'
|
||||
import type { PermissionDecision } from './PermissionResult.js'
|
||||
import {
|
||||
checkRuleBasedPermissions,
|
||||
hasPermissionsToUseTool,
|
||||
} from './permissions.js'
|
||||
|
||||
const inputSchema = {
|
||||
parse: (input: Record<string, unknown>) => input,
|
||||
}
|
||||
|
||||
function permissionContext(
|
||||
overrides: Partial<ToolPermissionContext>,
|
||||
): ToolPermissionContext {
|
||||
return {
|
||||
...getEmptyToolPermissionContext(),
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function toolUseContext(
|
||||
toolPermissionContext: ToolPermissionContext,
|
||||
): ToolUseContext {
|
||||
return {
|
||||
abortController: new AbortController(),
|
||||
getAppState: () =>
|
||||
({
|
||||
toolPermissionContext,
|
||||
}) as ReturnType<ToolUseContext['getAppState']>,
|
||||
setAppState: () => {},
|
||||
} as ToolUseContext
|
||||
}
|
||||
|
||||
function fakeTool({
|
||||
permissionDecision,
|
||||
requiresUserInteraction = false,
|
||||
}: {
|
||||
permissionDecision: PermissionDecision | { behavior: 'passthrough'; message: string }
|
||||
requiresUserInteraction?: boolean
|
||||
}): Tool {
|
||||
return {
|
||||
name: 'FakeTool',
|
||||
inputSchema,
|
||||
checkPermissions: async () => permissionDecision,
|
||||
requiresUserInteraction: () => requiresUserInteraction,
|
||||
} as unknown as Tool
|
||||
}
|
||||
|
||||
async function canUseFakeTool(
|
||||
tool: Tool,
|
||||
toolPermissionContext: ToolPermissionContext,
|
||||
): Promise<PermissionDecision> {
|
||||
return hasPermissionsToUseTool(
|
||||
tool,
|
||||
{},
|
||||
toolUseContext(toolPermissionContext),
|
||||
{} as never,
|
||||
'toolu_test',
|
||||
)
|
||||
}
|
||||
|
||||
describe('hasPermissionsToUseTool bypassPermissions mode', () => {
|
||||
it('ignores whole-tool ask rules', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'passthrough',
|
||||
message: 'No tool-specific decision',
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
alwaysAskRules: {
|
||||
session: ['FakeTool'],
|
||||
},
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'allow',
|
||||
decisionReason: {
|
||||
type: 'mode',
|
||||
mode: 'bypassPermissions',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps whole-tool ask rules in default mode', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'passthrough',
|
||||
message: 'No tool-specific decision',
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'default',
|
||||
alwaysAskRules: {
|
||||
session: ['FakeTool'],
|
||||
},
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'ask',
|
||||
decisionReason: {
|
||||
type: 'rule',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('ignores content-specific ask rules returned by tools', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'ask',
|
||||
message: 'Ask rule matched',
|
||||
decisionReason: {
|
||||
type: 'rule',
|
||||
rule: {
|
||||
source: 'session',
|
||||
ruleBehavior: 'ask',
|
||||
ruleValue: {
|
||||
toolName: 'FakeTool',
|
||||
ruleContent: 'sensitive:*',
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'allow',
|
||||
decisionReason: {
|
||||
type: 'mode',
|
||||
mode: 'bypassPermissions',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('ignores safety-check ask decisions', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'ask',
|
||||
message: 'Safety check matched',
|
||||
decisionReason: {
|
||||
type: 'safetyCheck',
|
||||
reason: 'Protected path',
|
||||
},
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'allow',
|
||||
decisionReason: {
|
||||
type: 'mode',
|
||||
mode: 'bypassPermissions',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('preserves explicit deny decisions', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'deny',
|
||||
message: 'Denied by rule',
|
||||
decisionReason: {
|
||||
type: 'rule',
|
||||
rule: {
|
||||
source: 'session',
|
||||
ruleBehavior: 'deny',
|
||||
ruleValue: {
|
||||
toolName: 'FakeTool',
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'deny',
|
||||
message: 'Denied by rule',
|
||||
})
|
||||
})
|
||||
|
||||
it('preserves prompts for tools that require user interaction', async () => {
|
||||
const result = await canUseFakeTool(
|
||||
fakeTool({
|
||||
requiresUserInteraction: true,
|
||||
permissionDecision: {
|
||||
behavior: 'ask',
|
||||
message: 'Needs user input',
|
||||
},
|
||||
}),
|
||||
permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
}),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'ask',
|
||||
message: 'Needs user input',
|
||||
})
|
||||
})
|
||||
|
||||
it('lets hook rule checks skip ask rules in bypass mode', async () => {
|
||||
const result = await checkRuleBasedPermissions(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'ask',
|
||||
message: 'Ask rule matched',
|
||||
decisionReason: {
|
||||
type: 'rule',
|
||||
rule: {
|
||||
source: 'session',
|
||||
ruleBehavior: 'ask',
|
||||
ruleValue: {
|
||||
toolName: 'FakeTool',
|
||||
ruleContent: 'sensitive:*',
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
{},
|
||||
toolUseContext(permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
})),
|
||||
)
|
||||
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps hook rule-check denies in bypass mode', async () => {
|
||||
const result = await checkRuleBasedPermissions(
|
||||
fakeTool({
|
||||
permissionDecision: {
|
||||
behavior: 'deny',
|
||||
message: 'Denied by rule',
|
||||
decisionReason: {
|
||||
type: 'rule',
|
||||
rule: {
|
||||
source: 'session',
|
||||
ruleBehavior: 'deny',
|
||||
ruleValue: {
|
||||
toolName: 'FakeTool',
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
{},
|
||||
toolUseContext(permissionContext({
|
||||
mode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
})),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({
|
||||
behavior: 'deny',
|
||||
message: 'Denied by rule',
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -230,6 +230,16 @@ export function getAskRules(context: ToolPermissionContext): PermissionRule[] {
|
||||
)
|
||||
}
|
||||
|
||||
function shouldBypassToolPermissions(
|
||||
toolPermissionContext: ToolPermissionContext,
|
||||
): boolean {
|
||||
return (
|
||||
toolPermissionContext.mode === 'bypassPermissions' ||
|
||||
(toolPermissionContext.mode === 'plan' &&
|
||||
toolPermissionContext.isBypassPermissionsModeAvailable)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the entire tool matches a rule
|
||||
* For example, this matches "Bash" but not "Bash(prefix:*)" for BashTool
|
||||
@ -1058,11 +1068,11 @@ function handleDenialLimitExceeded(
|
||||
}
|
||||
|
||||
/**
|
||||
* Check only the rule-based steps of the permission pipeline — the subset
|
||||
* that bypassPermissions mode respects (everything that fires before step 2a).
|
||||
* Check only the rule-based steps of the permission pipeline.
|
||||
*
|
||||
* Returns a deny/ask decision if a rule blocks the tool, or null if no rule
|
||||
* objects. Unlike hasPermissionsToUseTool, this does NOT run the auto mode classifier,
|
||||
* objects. In bypassPermissions mode, ask decisions are ignored but denies are
|
||||
* still enforced. Unlike hasPermissionsToUseTool, this does NOT run the auto mode classifier,
|
||||
* mode-based transformations (dontAsk/auto/asyncAgent), PermissionRequest hooks,
|
||||
* or bypassPermissions / always-allowed checks.
|
||||
*
|
||||
@ -1074,6 +1084,9 @@ export async function checkRuleBasedPermissions(
|
||||
context: ToolUseContext,
|
||||
): Promise<PermissionAskDecision | PermissionDenyDecision | null> {
|
||||
const appState = context.getAppState()
|
||||
const shouldBypassPermissions = shouldBypassToolPermissions(
|
||||
appState.toolPermissionContext,
|
||||
)
|
||||
|
||||
// 1a. Entire tool is denied by rule
|
||||
const denyRule = getDenyRuleForTool(appState.toolPermissionContext, tool)
|
||||
@ -1090,7 +1103,7 @@ export async function checkRuleBasedPermissions(
|
||||
|
||||
// 1b. Entire tool has an ask rule
|
||||
const askRule = getAskRuleForTool(appState.toolPermissionContext, tool)
|
||||
if (askRule) {
|
||||
if (askRule && !shouldBypassPermissions) {
|
||||
const canSandboxAutoAllow =
|
||||
tool.name === BASH_TOOL_NAME &&
|
||||
SandboxManager.isSandboxingEnabled() &&
|
||||
@ -1134,6 +1147,7 @@ export async function checkRuleBasedPermissions(
|
||||
// 1f. Content-specific ask rules from tool.checkPermissions
|
||||
// (e.g. Bash(npm publish:*) → {ask, type:'rule', ruleBehavior:'ask'})
|
||||
if (
|
||||
!shouldBypassPermissions &&
|
||||
toolPermissionResult?.behavior === 'ask' &&
|
||||
toolPermissionResult.decisionReason?.type === 'rule' &&
|
||||
toolPermissionResult.decisionReason.rule.ruleBehavior === 'ask'
|
||||
@ -1141,10 +1155,12 @@ export async function checkRuleBasedPermissions(
|
||||
return toolPermissionResult
|
||||
}
|
||||
|
||||
// 1g. Safety checks (e.g. .git/, .claude/, .vscode/, shell configs) are
|
||||
// bypass-immune — they must prompt even when a PreToolUse hook returned
|
||||
// allow. checkPathSafetyForAutoEdit returns {type:'safetyCheck'} for these.
|
||||
// 1g. Safety checks (e.g. .git/, .claude/, .vscode/, shell configs) prompt in
|
||||
// normal modes. Bypass mode means the user already opted into unattended
|
||||
// execution. checkPathSafetyForAutoEdit returns {type:'safetyCheck'} for these
|
||||
// paths.
|
||||
if (
|
||||
!shouldBypassPermissions &&
|
||||
toolPermissionResult?.behavior === 'ask' &&
|
||||
toolPermissionResult.decisionReason?.type === 'safetyCheck'
|
||||
) {
|
||||
@ -1165,6 +1181,9 @@ async function hasPermissionsToUseToolInner(
|
||||
}
|
||||
|
||||
let appState = context.getAppState()
|
||||
let shouldBypassPermissions = shouldBypassToolPermissions(
|
||||
appState.toolPermissionContext,
|
||||
)
|
||||
|
||||
// 1. Check if the tool is denied
|
||||
// 1a. Entire tool is denied
|
||||
@ -1182,7 +1201,7 @@ async function hasPermissionsToUseToolInner(
|
||||
|
||||
// 1b. Check if the entire tool should always ask for permission
|
||||
const askRule = getAskRuleForTool(appState.toolPermissionContext, tool)
|
||||
if (askRule) {
|
||||
if (askRule && !shouldBypassPermissions) {
|
||||
// When autoAllowBashIfSandboxed is on, sandboxed commands skip the ask rule and
|
||||
// auto-allow via Bash's checkPermissions. Commands that won't be sandboxed (excluded
|
||||
// commands, dangerouslyDisableSandbox) still need to respect the ask rule.
|
||||
@ -1235,13 +1254,14 @@ async function hasPermissionsToUseToolInner(
|
||||
return toolPermissionResult
|
||||
}
|
||||
|
||||
// 1f. Content-specific ask rules from tool.checkPermissions take precedence
|
||||
// over bypassPermissions mode. When a user explicitly configures a
|
||||
// 1f. Content-specific ask rules from tool.checkPermissions are honored in
|
||||
// normal modes. When a user explicitly configures a
|
||||
// content-specific ask rule (e.g. Bash(npm publish:*)), the tool's
|
||||
// checkPermissions returns {behavior:'ask', decisionReason:{type:'rule',
|
||||
// rule:{ruleBehavior:'ask'}}}. This must be respected even in bypass mode,
|
||||
// just as deny rules are respected at step 1d.
|
||||
// rule:{ruleBehavior:'ask'}}}. Bypass mode ignores ask decisions while still
|
||||
// preserving deny rules at step 1d.
|
||||
if (
|
||||
!shouldBypassPermissions &&
|
||||
toolPermissionResult?.behavior === 'ask' &&
|
||||
toolPermissionResult.decisionReason?.type === 'rule' &&
|
||||
toolPermissionResult.decisionReason.rule.ruleBehavior === 'ask'
|
||||
@ -1249,10 +1269,12 @@ async function hasPermissionsToUseToolInner(
|
||||
return toolPermissionResult
|
||||
}
|
||||
|
||||
// 1g. Safety checks (e.g. .git/, .claude/, .vscode/, shell configs) are
|
||||
// bypass-immune — they must prompt even in bypassPermissions mode.
|
||||
// checkPathSafetyForAutoEdit returns {type:'safetyCheck'} for these paths.
|
||||
// 1g. Safety checks (e.g. .git/, .claude/, .vscode/, shell configs) prompt in
|
||||
// normal modes. Bypass mode means the user already opted into unattended
|
||||
// execution. checkPathSafetyForAutoEdit returns {type:'safetyCheck'} for these
|
||||
// paths.
|
||||
if (
|
||||
!shouldBypassPermissions &&
|
||||
toolPermissionResult?.behavior === 'ask' &&
|
||||
toolPermissionResult.decisionReason?.type === 'safetyCheck'
|
||||
) {
|
||||
@ -1262,13 +1284,9 @@ async function hasPermissionsToUseToolInner(
|
||||
// 2a. Check if mode allows the tool to run
|
||||
// IMPORTANT: Call getAppState() to get the latest value
|
||||
appState = context.getAppState()
|
||||
// Check if permissions should be bypassed:
|
||||
// - Direct bypassPermissions mode
|
||||
// - Plan mode when the user originally started with bypass mode (isBypassPermissionsModeAvailable)
|
||||
const shouldBypassPermissions =
|
||||
appState.toolPermissionContext.mode === 'bypassPermissions' ||
|
||||
(appState.toolPermissionContext.mode === 'plan' &&
|
||||
appState.toolPermissionContext.isBypassPermissionsModeAvailable)
|
||||
shouldBypassPermissions = shouldBypassToolPermissions(
|
||||
appState.toolPermissionContext,
|
||||
)
|
||||
if (shouldBypassPermissions) {
|
||||
return {
|
||||
behavior: 'allow',
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user