diff --git a/.changeset/fix-session-approval-special-chars.md b/.changeset/fix-session-approval-special-chars.md new file mode 100644 index 000000000..e1cc22eca --- /dev/null +++ b/.changeset/fix-session-approval-special-chars.md @@ -0,0 +1,7 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code-sdk": patch +"@moonshot-ai/kimi-code": patch +--- + +Fix "Approve for this session" being ignored for Bash commands containing quotes, parentheses, or pipes. The stored approval rule escapes glob metacharacters but was re-matched through picomatch, which dropped the quotes and never matched the original command, so the identical command was prompted again. An exact-literal match now short-circuits before glob matching. diff --git a/packages/agent-core/src/tools/support/rule-match.ts b/packages/agent-core/src/tools/support/rule-match.ts index fe206ce5a..109ba4b9a 100644 --- a/packages/agent-core/src/tools/support/rule-match.ts +++ b/packages/agent-core/src/tools/support/rule-match.ts @@ -5,6 +5,7 @@ import { } from './path-glob-match'; const GLOB_LITERAL_SPECIAL = /[\\*?[\]{}()!+@|]/g; +const ESCAPED_GLOB_LITERAL_SPECIAL = /\\([\\*?[\]{}()!+@|])/g; export function literalRulePattern(toolName: string, subject: string): string { return `${toolName}(${escapeRuleSubjectLiteral(subject)})`; @@ -15,7 +16,9 @@ export function escapeRuleSubjectLiteral(subject: string): string { } export function matchesGlobRuleSubject(ruleArgs: string, subject: string): boolean { - return matchRuleSubjects(ruleArgs, [subject], (pattern, value) => globMatch(value, pattern)); + return matchRuleSubjects(ruleArgs, [subject], (pattern, value) => + unescapeRuleSubjectLiteral(pattern) === value || globMatch(value, pattern), + ); } export function matchesPathRuleSubject( @@ -39,3 +42,7 @@ function matchRuleSubjects( const hit = subjects.some((subject) => matchesPositivePattern(positivePattern, subject)); return negated ? !hit : hit; } + +function unescapeRuleSubjectLiteral(pattern: string): string { + return pattern.replace(ESCAPED_GLOB_LITERAL_SPECIAL, '$1'); +} diff --git a/packages/agent-core/test/agent/permission.test.ts b/packages/agent-core/test/agent/permission.test.ts index 6443cb0cc..6358e5ba9 100644 --- a/packages/agent-core/test/agent/permission.test.ts +++ b/packages/agent-core/test/agent/permission.test.ts @@ -2105,6 +2105,46 @@ describe('Agent-local approve for session', () => { expect(requestApproval).toHaveBeenCalledTimes(2); }); + it.each([ + ['quotes and parens', `python -c "print('1')"`, `python -c "print('2')"`], + ['quotes and pipes', `printf "a|b" | sed "s|a|c|"`, `printf "a|b" | sed "s|a|d|"`], + ] as const)( + 'reuses approve-for-session for literal Bash commands containing %s', + async (_caseName, command, changedCommand) => { + const { manager, requestApproval, telemetryTrack } = makePermissionManager(async () => ({ + decision: 'approved', + scope: 'session', + selectedLabel: 'Approve for this session', + })); + const call = (id: string, commandText: string) => + manager.beforeToolCall( + hookContext({ + id, + args: { command: commandText, timeout: 60 }, + }), + ); + + await expect(call('call_literal_special_1', command)).resolves.toBeUndefined(); + expect(manager.sessionApprovalRulePatterns).toContain(literalRulePattern('Bash', command)); + + await expect(call('call_literal_special_2', command)).resolves.toBeUndefined(); + expect(requestApproval).toHaveBeenCalledTimes(1); + expect(telemetryTrack).toHaveBeenCalledWith( + 'permission_policy_decision', + expect.objectContaining({ + policy_name: 'session-approval-history', + tool_name: 'Bash', + decision: 'approve', + has_rule_args: true, + match_strategy: 'matches_rule', + }), + ); + + await expect(call('call_literal_special_3', changedCommand)).resolves.toBeUndefined(); + expect(requestApproval).toHaveBeenCalledTimes(2); + }, + ); + it('keeps approved once responses one-shot', async () => { const { manager, record, requestApproval } = makePermissionManager(async () => ({ decision: 'approved',