Compare commits

...

5 Commits

Author SHA1 Message Date
Abhijit Balaji 92acd0de56 test: fix policy tests for argName logic 2026-02-20 15:59:28 -08:00
Abhijit Balaji 95d687a7dd fix(policy): secure and robust regex matching for commandRegex
- Implemented targeted argument matching by adding 'argName' to PolicyRule.
- Updated PolicyEngine to match against a specific argument (e.g., 'command')
  when argName is specified, preventing nested property injection bypasses.
- Simplified anchor handling by matching against raw argument values instead
  of JSON strings, enabling standard ^ and $ behavior.
- Added a security regression test to verify bypasses are blocked.
- Resolved type errors in config.ts and various test files.
- Cleaned up TDD-related notes and updated unit tests.
2026-02-20 15:59:16 -08:00
Adam Weidman af5b523a62 feat(core): migrate read_file to 1-based start_line/end_line parameters (#19526) 2026-02-20 15:58:53 -08:00
Abhijit Balaji b7b6773f9a docs: remove TDD notes and irrelevant comments from policy tests 2026-02-20 15:47:21 -08:00
Abhijit Balaji 7ff6d563f1 fix(policy): support regex anchors in commandRegex
Modified buildArgsPatterns to correctly transform ^ and $ anchors
into JSON-aware patterns, allowing precise matching of command values.

Closes #19688
2026-02-20 15:13:01 -08:00
22 changed files with 427 additions and 221 deletions
@@ -549,7 +549,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -705,7 +705,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -827,7 +827,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1422,7 +1422,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1574,7 +1574,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1717,7 +1717,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1860,7 +1860,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1999,7 +1999,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -2138,7 +2138,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -2276,7 +2276,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -2656,7 +2656,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -2795,7 +2795,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -3046,7 +3046,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -3185,7 +3185,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like grep_search and glob with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like grep_search with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like grep_search and/or read_file called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
+7 -11
View File
@@ -1169,17 +1169,13 @@ modes = ["plan"]
r.argsPattern,
);
expect(shellRules).toHaveLength(2);
expect(
shellRules?.some((r) => r.argsPattern?.test('{"command":"git status"}')),
).toBe(true);
expect(
shellRules?.some((r) => r.argsPattern?.test('{"command":"git diff"}')),
).toBe(true);
expect(
shellRules?.every(
(r) => !r.argsPattern?.test('{"command":"git commit"}'),
),
).toBe(true);
expect(shellRules?.some((r) => r.argsPattern?.test('git status'))).toBe(
true,
);
expect(shellRules?.some((r) => r.argsPattern?.test('git diff'))).toBe(true);
expect(shellRules?.every((r) => !r.argsPattern?.test('git commit'))).toBe(
true,
);
const subagentRule = config.rules?.find(
(r) =>
+6 -4
View File
@@ -307,12 +307,13 @@ export async function createPolicyEngineConfig(
if (toolName === SHELL_TOOL_NAME) {
const patterns = buildArgsPatterns(undefined, args);
for (const pattern of patterns) {
if (pattern) {
if (pattern.pattern) {
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
argsPattern: new RegExp(pattern),
argsPattern: new RegExp(pattern.pattern),
argName: pattern.argName,
source: 'Settings (Tools Allowed)',
});
}
@@ -410,14 +411,15 @@ export function createPolicyUpdater(
// Convert commandPrefix(es) to argsPatterns for in-memory rules
const patterns = buildArgsPatterns(undefined, message.commandPrefix);
for (const pattern of patterns) {
if (pattern) {
if (pattern.pattern) {
// Note: patterns from buildArgsPatterns are derived from escapeRegex,
// which is safe and won't contain ReDoS patterns.
policyEngine.addRule({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp(pattern),
argsPattern: new RegExp(pattern.pattern),
argName: pattern.argName,
source: 'Dynamic (Confirmed)',
});
}
+2 -1
View File
@@ -154,8 +154,9 @@ describe('createPolicyUpdater', () => {
expect(addedRule).toBeDefined();
expect(addedRule?.priority).toBe(ALWAYS_ALLOW_PRIORITY);
expect(addedRule?.argsPattern).toEqual(
new RegExp(`"command":"git\\ status(?:[\\s"]|\\\\")`),
new RegExp(`^git\\ status(?:\\s|$)`),
);
expect(addedRule?.argName).toBe('command');
// Verify file written
expect(fs.open).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), 'wx');
+86 -1
View File
@@ -607,7 +607,8 @@ describe('PolicyEngine', () => {
const rules: PolicyRule[] = [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(patterns[0]!),
argsPattern: new RegExp(patterns[0].pattern!),
argName: patterns[0].argName,
decision: PolicyDecision.ALLOW,
},
];
@@ -1482,6 +1483,90 @@ describe('PolicyEngine', () => {
});
});
describe('commandRegex anchors', () => {
it('should ALLOW tmux command with $ anchor', async () => {
const patterns = buildArgsPatterns(
undefined,
undefined,
'tmux send-keys -t [a-z0-9:]+ (C-c|Up|Enter|Up Enter)$',
);
const regex = new RegExp(patterns[0].pattern!);
engine.addRule({
toolName: 'run_shell_command',
decision: PolicyDecision.ALLOW,
priority: 100,
argsPattern: regex,
argName: patterns[0].argName,
});
const toolCall = {
name: 'run_shell_command',
args: {
command: 'tmux send-keys -t superpowers:6 C-c',
},
};
const result = await engine.check(toolCall, undefined);
expect(result.decision).toBe(PolicyDecision.ALLOW);
});
it('should ALLOW git status with ^ anchor', async () => {
const patterns = buildArgsPatterns(undefined, undefined, '^git status');
const regex = new RegExp(patterns[0].pattern!);
engine.addRule({
toolName: 'run_shell_command',
decision: PolicyDecision.ALLOW,
priority: 100,
argsPattern: regex,
argName: patterns[0].argName,
});
const toolCall = {
name: 'run_shell_command',
args: {
command: 'git status',
},
};
const result = await engine.check(toolCall, undefined);
expect(result.decision).toBe(PolicyDecision.ALLOW);
});
it('should NOT match nested command property (security bypass check)', async () => {
// Rule allowing only 'git status'
const patterns = buildArgsPatterns(undefined, undefined, '^git status$');
const regex = new RegExp(patterns[0].pattern!);
engine.addRule({
toolName: 'run_shell_command',
decision: PolicyDecision.ALLOW,
priority: 100,
argsPattern: regex,
argName: patterns[0].argName,
});
// Malicious tool call attempting to bypass using nested property
const toolCall = {
name: 'run_shell_command',
args: {
command: 'rm -rf /',
dummy: {
command: 'git status',
},
},
};
const result = await engine.check(toolCall, undefined);
// Should be ASK_USER because 'rm -rf /' doesn't match '^git status$'
expect(result.decision).toBe(PolicyDecision.ASK_USER);
});
});
describe('Plan Mode vs Subagent Priority (Regression)', () => {
it('should DENY subagents in Plan Mode despite dynamic allow rules', async () => {
// Plan Mode Deny (1.06) > Subagent Allow (1.05)
+17 -6
View File
@@ -83,12 +83,23 @@ function ruleMatches(
if (!toolCall.args) {
return false;
}
// Use stable JSON stringification with sorted keys to ensure consistent matching
if (
stringifiedArgs === undefined ||
!rule.argsPattern.test(stringifiedArgs)
) {
return false;
if (rule.argName) {
// Match against a specific named argument (e.g., 'command' for shell)
const val = toolCall.args[rule.argName];
// We only support matching string arguments for now
if (typeof val !== 'string' || !rule.argsPattern.test(val)) {
return false;
}
} else {
// Use stable JSON stringification with sorted keys to ensure consistent matching
// against the entire arguments object.
if (
stringifiedArgs === undefined ||
!rule.argsPattern.test(stringifiedArgs)
) {
return false;
}
}
}
@@ -75,7 +75,8 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"echo(?:[\\s"]|\\\\")'),
argsPattern: new RegExp('^echo(?:\\s|$)'),
argName: 'command',
}),
);
expect(policyEngine.addRule).toHaveBeenNthCalledWith(
@@ -83,7 +84,8 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"ls(?:[\\s"]|\\\\")'),
argsPattern: new RegExp('^ls(?:\\s|$)'),
argName: 'command',
}),
);
});
@@ -103,7 +105,8 @@ describe('createPolicyUpdater', () => {
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"git(?:[\\s"]|\\\\")'),
argsPattern: new RegExp('^git(?:\\s|$)'),
argName: 'command',
}),
);
});
+23 -12
View File
@@ -85,15 +85,16 @@ describe('Shell Safety Policy', () => {
// Helper to create a policy engine with a simple command prefix rule
function createPolicyEngineWithPrefix(prefix: string) {
const argsPatterns = buildArgsPatterns(undefined, prefix, undefined);
// Since buildArgsPatterns returns array of patterns (strings), we pick the first one
// Since buildArgsPatterns returns array of ArgsPatternInfo, we pick the first one
// and compile it.
const argsPattern = new RegExp(argsPatterns[0]!);
const argsPattern = new RegExp(argsPatterns[0].pattern!);
return new PolicyEngine({
rules: [
{
toolName: 'run_shell_command',
argsPattern,
argName: argsPatterns[0].argName,
decision: PolicyDecision.ALLOW,
priority: 1.01,
},
@@ -201,13 +202,15 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsEcho[0]!),
argsPattern: new RegExp(argsPatternsEcho[0].pattern!),
argName: argsPatternsEcho[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
},
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGit[0]!),
argsPattern: new RegExp(argsPatternsGit[0].pattern!),
argName: argsPatternsGit[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
},
@@ -287,14 +290,16 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsEcho[0]!),
argsPattern: new RegExp(argsPatternsEcho[0].pattern!),
argName: argsPatternsEcho[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
},
{
toolName: 'run_shell_command',
// Matches "git" at start of *subcommand*
argsPattern: new RegExp(argsPatternsGit[0]!),
argsPattern: new RegExp(argsPatternsGit[0].pattern!),
argName: argsPatternsGit[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
},
@@ -332,7 +337,8 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGitLog[0]!),
argsPattern: new RegExp(argsPatternsGitLog[0].pattern!),
argName: argsPatternsGitLog[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
allowRedirection: true,
@@ -375,7 +381,8 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsPush[0]!),
argsPattern: new RegExp(argsPatternsPush[0].pattern!),
argName: argsPatternsPush[0].argName,
decision: PolicyDecision.DENY,
priority: 2,
},
@@ -406,7 +413,8 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsGitStatus[0]!),
argsPattern: new RegExp(argsPatternsGitStatus[0].pattern!),
argName: argsPatternsGitStatus[0].argName,
decision: PolicyDecision.ALLOW,
priority: 2,
name: 'allow_git_status_rule', // Give a name to easily identify
@@ -443,7 +451,8 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAnotherUnknown[0]!),
argsPattern: new RegExp(argsPatternsAnotherUnknown[0].pattern!),
argName: argsPatternsAnotherUnknown[0].argName,
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_another_unknown_command_rule',
@@ -486,14 +495,16 @@ describe('Shell Safety Policy', () => {
rules: [
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAsk1[0]!),
argsPattern: new RegExp(argsPatternsAsk1[0].pattern!),
argName: argsPatternsAsk1[0].argName,
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_rule_1',
},
{
toolName: 'run_shell_command',
argsPattern: new RegExp(argsPatternsAsk2[0]!),
argsPattern: new RegExp(argsPatternsAsk2[0].pattern!),
argName: argsPatternsAsk2[0].argName,
decision: PolicyDecision.ASK_USER,
priority: 2,
name: 'ask_rule_2',
+16 -27
View File
@@ -80,12 +80,10 @@ priority = 100
expect(result.rules).toHaveLength(2);
expect(result.rules[0].toolName).toBe('run_shell_command');
expect(result.rules[1].toolName).toBe('run_shell_command');
expect(
result.rules[0].argsPattern?.test('{"command":"git status"}'),
).toBe(true);
expect(result.rules[1].argsPattern?.test('{"command":"git log"}')).toBe(
true,
);
expect(result.rules[0].argName).toBe('command');
expect(result.rules[1].argName).toBe('command');
expect(result.rules[0].argsPattern?.test('git status')).toBe(true);
expect(result.rules[1].argsPattern?.test('git log')).toBe(true);
expect(result.errors).toHaveLength(0);
});
@@ -99,19 +97,14 @@ priority = 100
`);
expect(result.rules).toHaveLength(1);
expect(
result.rules[0].argsPattern?.test('{"command":"git status"}'),
).toBe(true);
expect(
result.rules[0].argsPattern?.test('{"command":"git log --all"}'),
).toBe(true);
expect(
result.rules[0].argsPattern?.test('{"command":"git branch"}'),
).toBe(false);
expect(result.rules[0].argName).toBe('command');
expect(result.rules[0].argsPattern?.test('git status')).toBe(true);
expect(result.rules[0].argsPattern?.test('git log --all')).toBe(true);
expect(result.rules[0].argsPattern?.test('git branch')).toBe(false);
expect(result.errors).toHaveLength(0);
});
it('should NOT match if ^ is used in commandRegex because it matches against full JSON', async () => {
it('should match if ^ is used in commandRegex', async () => {
const result = await runLoadPoliciesFromToml(`
[[rule]]
toolName = "run_shell_command"
@@ -121,11 +114,10 @@ priority = 100
`);
expect(result.rules).toHaveLength(1);
// The generated pattern is "command":"^git status
// This will NOT match '{"command":"git status"}' because of the '{"' at the start.
expect(
result.rules[0].argsPattern?.test('{"command":"git status"}'),
).toBe(false);
const rule = result.rules[0];
expect(rule.argName).toBe('command');
expect(rule.argsPattern?.test('git status')).toBe(true);
expect(rule.argsPattern?.test('prefix git status')).toBe(false);
expect(result.errors).toHaveLength(0);
});
@@ -323,13 +315,10 @@ priority = 100
`);
expect(result.rules).toHaveLength(1);
expect(result.rules[0].argName).toBe('command');
// The regex should have escaped the * and .
expect(
result.rules[0].argsPattern?.test('{"command":"git log file.txt"}'),
).toBe(false);
expect(
result.rules[0].argsPattern?.test('{"command":"git log *.txt"}'),
).toBe(true);
expect(result.rules[0].argsPattern?.test('git log file.txt')).toBe(false);
expect(result.rules[0].argsPattern?.test('git log *.txt')).toBe(true);
expect(result.errors).toHaveLength(0);
});
+8 -4
View File
@@ -352,14 +352,15 @@ export async function loadPoliciesFromToml(
// Transform rules
const parsedRules: PolicyRule[] = (validationResult.data.rule ?? [])
.flatMap((rule) => {
const argsPatterns = buildArgsPatterns(
const argsPatternInfos = buildArgsPatterns(
rule.argsPattern,
rule.commandPrefix,
rule.commandRegex,
);
// For each argsPattern, expand toolName arrays
return argsPatterns.flatMap((argsPattern) => {
return argsPatternInfos.flatMap((info) => {
const { pattern: argsPattern, argName } = info;
const toolNames: Array<string | undefined> = rule.toolName
? Array.isArray(rule.toolName)
? rule.toolName
@@ -383,6 +384,7 @@ export async function loadPoliciesFromToml(
decision: rule.decision,
priority: transformPriority(rule.priority, tier),
modes: rule.modes,
argName,
allowRedirection: rule.allow_redirection,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
denyMessage: rule.deny_message,
@@ -438,13 +440,14 @@ export async function loadPoliciesFromToml(
validationResult.data.safety_checker ?? []
)
.flatMap((checker) => {
const argsPatterns = buildArgsPatterns(
const argsPatternInfos = buildArgsPatterns(
checker.argsPattern,
checker.commandPrefix,
checker.commandRegex,
);
return argsPatterns.flatMap((argsPattern) => {
return argsPatternInfos.flatMap((info) => {
const { pattern: argsPattern, argName } = info;
const toolNames: Array<string | undefined> = checker.toolName
? Array.isArray(checker.toolName)
? checker.toolName
@@ -467,6 +470,7 @@ export async function loadPoliciesFromToml(
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
checker: checker.checker as SafetyCheckerConfig,
modes: checker.modes,
argName,
source: `${tierName.charAt(0).toUpperCase() + tierName.slice(1)}: ${file}`,
};
+14
View File
@@ -115,6 +115,13 @@ export interface PolicyRule {
*/
argsPattern?: RegExp;
/**
* Optional name of a specific argument to match the argsPattern against.
* If provided, the argsPattern is tested against the value of this argument
* (as a string) rather than the full JSON-stringified arguments object.
*/
argName?: string;
/**
* The decision to make when this rule matches.
*/
@@ -165,6 +172,13 @@ export interface SafetyCheckerRule {
*/
argsPattern?: RegExp;
/**
* Optional name of a specific argument to match the argsPattern against.
* If provided, the argsPattern is tested against the value of this argument
* (as a string) rather than the full JSON-stringified arguments object.
*/
argName?: string;
/**
* Priority of this checker. Higher numbers run first.
* Default is 0.
+66 -34
View File
@@ -64,85 +64,117 @@ describe('policy/utils', () => {
describe('buildArgsPatterns', () => {
it('should return argsPattern if provided and no commandPrefix/regex', () => {
const result = buildArgsPatterns('my-pattern', undefined, undefined);
expect(result).toEqual(['my-pattern']);
expect(result).toEqual([{ pattern: 'my-pattern' }]);
});
it('should build pattern from a single commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'ls', undefined);
expect(result).toEqual(['"command":"ls(?:[\\s"]|\\\\")']);
expect(result).toEqual([{ pattern: '^ls(?:\\s|$)', argName: 'command' }]);
});
it('should build patterns from an array of commandPrefixes', () => {
const result = buildArgsPatterns(undefined, ['ls', 'cd'], undefined);
expect(result).toEqual([
'"command":"ls(?:[\\s"]|\\\\")',
'"command":"cd(?:[\\s"]|\\\\")',
{ pattern: '^ls(?:\\s|$)', argName: 'command' },
{ pattern: '^cd(?:\\s|$)', argName: 'command' },
]);
});
it('should build pattern from commandRegex', () => {
const result = buildArgsPatterns(undefined, undefined, 'rm -rf .*');
expect(result).toEqual(['"command":"rm -rf .*']);
expect(result).toEqual([{ pattern: 'rm -rf .*', argName: 'command' }]);
});
it('should prioritize commandPrefix over commandRegex and argsPattern', () => {
const result = buildArgsPatterns('raw', 'prefix', 'regex');
expect(result).toEqual(['"command":"prefix(?:[\\s"]|\\\\")']);
expect(result).toEqual([
{ pattern: '^prefix(?:\\s|$)', argName: 'command' },
]);
});
it('should prioritize commandRegex over argsPattern if no commandPrefix', () => {
const result = buildArgsPatterns('raw', undefined, 'regex');
expect(result).toEqual(['"command":"regex']);
expect(result).toEqual([{ pattern: 'regex', argName: 'command' }]);
});
it('should escape characters in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git checkout -b', undefined);
expect(result).toEqual([
'"command":"git\\ checkout\\ \\-b(?:[\\s"]|\\\\")',
{ pattern: '^git\\ checkout\\ \\-b(?:\\s|$)', argName: 'command' },
]);
});
it('should correctly escape quotes in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git "fix"', undefined);
it('should correctly escape special characters in commandPrefix', () => {
const result = buildArgsPatterns(undefined, 'git*', undefined);
expect(result).toEqual([
'"command":"git\\ \\\\\\"fix\\\\\\"(?:[\\s"]|\\\\")',
{ pattern: '^git\\*(?:\\s|$)', argName: 'command' },
]);
});
it('should handle undefined correctly when no inputs are provided', () => {
const result = buildArgsPatterns(undefined, undefined, undefined);
expect(result).toEqual([undefined]);
expect(result).toEqual([{ pattern: undefined }]);
});
it('should match prefixes followed by JSON escaped quotes', () => {
// Testing the security fix logic: allowing "echo \"foo\""
it('should match prefixes correctly', () => {
const prefix = 'echo ';
const patterns = buildArgsPatterns(undefined, prefix, undefined);
const regex = new RegExp(patterns[0]!);
expect(patterns[0].argName).toBe('command');
const regex = new RegExp(patterns[0].pattern!);
// Mimic JSON stringified args
// echo "foo" -> {"command":"echo \"foo\""}
const validJsonArgs = '{"command":"echo \\"foo\\""}';
expect(regex.test(validJsonArgs)).toBe(true);
expect(regex.test('echo hello')).toBe(true);
expect(regex.test('echo')).toBe(true);
expect(regex.test('echonop')).toBe(false);
});
it('should NOT match prefixes followed by raw backslashes (security check)', () => {
// Testing that we blocked the hole: "echo\foo"
const prefix = 'echo ';
const patterns = buildArgsPatterns(undefined, prefix, undefined);
const regex = new RegExp(patterns[0]!);
describe('commandRegex anchors', () => {
it('should transform ^ anchor correctly', () => {
const patterns = buildArgsPatterns(undefined, undefined, '^git status');
expect(patterns[0].argName).toBe('command');
const regex = new RegExp(patterns[0].pattern!);
// We match against the command string directly now
const command = 'git status';
expect(regex.test(command)).toBe(true);
});
// echo\foo -> {"command":"echo\\foo"}
// In regex matching: "echo " is followed by "\" which is NOT in [\s"] and is not \"
const attackJsonArgs = '{"command":"echo\\\\foo"}';
expect(regex.test(attackJsonArgs)).toBe(false);
it('should transform $ anchor correctly', () => {
const patterns = buildArgsPatterns(
undefined,
undefined,
'tmux send-keys -t [a-z0-9:]+ (C-c|Up|Enter|Up Enter)$',
);
expect(patterns[0].argName).toBe('command');
const regex = new RegExp(patterns[0].pattern!);
const command = 'tmux send-keys -t superpowers:6 C-c';
expect(regex.test(command)).toBe(true);
});
// Also validation for "git " matching "git\status"
const gitPatterns = buildArgsPatterns(undefined, 'git ', undefined);
const gitRegex = new RegExp(gitPatterns[0]!);
// git\status -> {"command":"git\\status"}
const gitAttack = '{"command":"git\\\\status"}';
expect(gitRegex.test(gitAttack)).toBe(false);
it('should handle $ anchor correctly', () => {
const patterns = buildArgsPatterns(undefined, undefined, 'git status$');
expect(patterns[0].argName).toBe('command');
const regex = new RegExp(patterns[0].pattern!);
const command = 'git status';
expect(regex.test(command)).toBe(true);
});
it('should NOT match if $ anchor is used and more text follows in command', () => {
const patterns = buildArgsPatterns(undefined, undefined, 'git status$');
const regex = new RegExp(patterns[0].pattern!);
const command = 'git status --porcelain';
expect(regex.test(command)).toBe(false);
});
it('should handle escaped anchors as literals', () => {
const patterns = buildArgsPatterns(
undefined,
undefined,
'git status\\$',
);
const regex = new RegExp(patterns[0].pattern!);
// Literal $ in command: git status$
const command = 'git status$';
expect(regex.test(command)).toBe(true);
});
});
});
});
+36 -12
View File
@@ -42,6 +42,22 @@ export function isSafeRegExp(pattern: string): boolean {
return true;
}
/**
* Metadata about an arguments pattern for policy matching.
*/
export interface ArgsPatternInfo {
/**
* The regular expression pattern string.
*/
pattern?: string;
/**
* Optional name of a specific argument to match the pattern against.
* If undefined, the pattern matches against the full JSON arguments string.
*/
argName?: string;
}
/**
* Builds a list of args patterns for policy matching.
*
@@ -51,34 +67,42 @@ export function isSafeRegExp(pattern: string): boolean {
* @param argsPattern An optional raw regex string for arguments.
* @param commandPrefix An optional command prefix (or list of prefixes) to allow.
* @param commandRegex An optional command regex string to allow.
* @returns An array of string patterns (or undefined) for the PolicyEngine.
* @returns An array of pattern info objects for the PolicyEngine.
*/
export function buildArgsPatterns(
argsPattern?: string,
commandPrefix?: string | string[],
commandRegex?: string,
): Array<string | undefined> {
): ArgsPatternInfo[] {
if (commandPrefix) {
const prefixes = Array.isArray(commandPrefix)
? commandPrefix
: [commandPrefix];
// Expand command prefixes to multiple patterns.
// We append [\\s"] to ensure we match whole words only (e.g., "git" but not
// "github"). Since we match against JSON stringified args, the value is
// always followed by a space or a closing quote.
// We now match against the 'command' argument directly.
return prefixes.map((prefix) => {
const jsonPrefix = JSON.stringify(prefix).slice(1, -1);
// We allow [\s], ["], or the specific sequence [\"] (for escaped quotes
// in JSON). We do NOT allow generic [\\], which would match "git\status"
// -> "gitstatus".
return `"command":"${escapeRegex(jsonPrefix)}(?:[\\s"]|\\\\")`;
// For prefixes, we match the string followed by whitespace or end-of-string.
// We trim the prefix and then ensure it's followed by a separator to
// match whole words (e.g. 'git' matches 'git status' but not 'github').
const trimmedPrefix = prefix.trim();
return {
pattern: `^${escapeRegex(trimmedPrefix)}(?:\\s|$)`,
argName: 'command',
};
});
}
if (commandRegex) {
return [`"command":"${commandRegex}`];
// For commandRegex, we match against the 'command' argument directly.
// Standard anchors (^, $) work as expected relative to the command string.
return [
{
pattern: commandRegex,
argName: 'command',
},
];
}
return [argsPattern];
return [{ pattern: argsPattern }];
}
+1 -1
View File
@@ -192,7 +192,7 @@ Use the following guidelines to optimize your search and read patterns.
- **Searching:** utilize search tools like ${GREP_TOOL_NAME} and ${GLOB_TOOL_NAME} with a conservative result count (\`total_max_matches\`) and a narrow scope (\`include\` and \`exclude\` parameters).
- **Searching and editing:** utilize search tools like ${GREP_TOOL_NAME} with a conservative result count and a narrow scope. Use \`context\`, \`before\`, and/or \`after\` to request enough context to avoid the need to read the file before editing matches.
- **Understanding:** minimize turns needed to understand a file. It's most efficient to read small files in their entirety.
- **Large files:** utilize search tools like ${GREP_TOOL_NAME} and/or ${READ_FILE_TOOL_NAME} called in parallel with an offset and a limit to reduce the impact on context. Minmize extra turns, unless unavoidable due to the file being too large.
- **Large files:** utilize search tools like ${GREP_TOOL_NAME} and/or ${READ_FILE_TOOL_NAME} called in parallel with 'start_line' and 'end_line' to reduce the impact on context. Minimize extra turns, unless unavoidable due to the file being too large.
- **Navigating:** read the minimum required to not require additional turns spent reading the file.
</examples>
@@ -1,5 +1,5 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`ReadFileTool > getSchema > should return the base schema when no modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the base schema when no modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`;
@@ -411,20 +411,20 @@ exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snaps
exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snapshot for tool: read_file 1`] = `
{
"description": "Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.",
"description": "Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.",
"name": "read_file",
"parametersJsonSchema": {
"properties": {
"end_line": {
"description": "Optional: The 1-based line number to end reading at (inclusive).",
"type": "number",
},
"file_path": {
"description": "The path to the file to read.",
"type": "string",
},
"limit": {
"description": "Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
"type": "number",
},
"offset": {
"description": "Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
"start_line": {
"description": "Optional: The 1-based line number to start reading from.",
"type": "number",
},
},
@@ -1200,20 +1200,20 @@ exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview >
exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview > snapshot for tool: read_file 1`] = `
{
"description": "Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.",
"description": "Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.",
"name": "read_file",
"parametersJsonSchema": {
"properties": {
"end_line": {
"description": "Optional: The 1-based line number to end reading at (inclusive).",
"type": "number",
},
"file_path": {
"description": "The path to the file to read.",
"type": "string",
},
"limit": {
"description": "Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
"type": "number",
},
"offset": {
"description": "Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
"start_line": {
"description": "Optional: The 1-based line number to start reading from.",
"type": "number",
},
},
@@ -35,7 +35,7 @@ import {
export const DEFAULT_LEGACY_SET: CoreToolSet = {
read_file: {
name: READ_FILE_TOOL_NAME,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.`,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.`,
parametersJsonSchema: {
type: 'object',
properties: {
@@ -43,14 +43,14 @@ export const DEFAULT_LEGACY_SET: CoreToolSet = {
description: 'The path to the file to read.',
type: 'string',
},
offset: {
start_line: {
description:
"Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
'Optional: The 1-based line number to start reading from.',
type: 'number',
},
limit: {
end_line: {
description:
"Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
'Optional: The 1-based line number to end reading at (inclusive).',
type: 'number',
},
},
@@ -38,7 +38,7 @@ import {
export const GEMINI_3_SET: CoreToolSet = {
read_file: {
name: READ_FILE_TOOL_NAME,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.`,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.`,
parametersJsonSchema: {
type: 'object',
properties: {
@@ -46,14 +46,14 @@ export const GEMINI_3_SET: CoreToolSet = {
description: 'The path to the file to read.',
type: 'string',
},
offset: {
start_line: {
description:
"Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
'Optional: The 1-based line number to start reading from.',
type: 'number',
},
limit: {
end_line: {
description:
"Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
'Optional: The 1-based line number to end reading at (inclusive).',
type: 'number',
},
},
+23 -12
View File
@@ -130,29 +130,36 @@ describe('ReadFileTool', () => {
);
});
it('should throw error if offset is negative', () => {
it('should throw error if start_line is less than 1', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
offset: -1,
start_line: 0,
};
expect(() => tool.build(params)).toThrow(
'Offset must be a non-negative number',
);
expect(() => tool.build(params)).toThrow('start_line must be at least 1');
});
it('should throw error if limit is zero or negative', () => {
it('should throw error if end_line is less than 1', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
limit: 0,
end_line: 0,
};
expect(() => tool.build(params)).toThrow('end_line must be at least 1');
});
it('should throw error if start_line is greater than end_line', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
start_line: 10,
end_line: 5,
};
expect(() => tool.build(params)).toThrow(
'Limit must be a positive number',
'start_line cannot be greater than end_line',
);
});
});
describe('getDescription', () => {
it('should return relative path without limit/offset', () => {
it('should return relative path without ranges', () => {
const subDir = path.join(tempRootDir, 'sub', 'dir');
const params: ReadFileToolParams = {
file_path: path.join(subDir, 'file.txt'),
@@ -393,7 +400,7 @@ describe('ReadFileTool', () => {
expect(result.returnDisplay).toBe('');
});
it('should support offset and limit for text files', async () => {
it('should support start_line and end_line for text files', async () => {
const filePath = path.join(tempRootDir, 'paginated.txt');
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
const fileContent = lines.join('\n');
@@ -401,8 +408,8 @@ describe('ReadFileTool', () => {
const params: ReadFileToolParams = {
file_path: filePath,
offset: 5, // Start from line 6
limit: 3,
start_line: 6,
end_line: 8,
};
const invocation = tool.build(params);
@@ -569,6 +576,10 @@ describe('ReadFileTool', () => {
const schema = tool.getSchema();
expect(schema.name).toBe(ReadFileTool.Name);
expect(schema.description).toMatchSnapshot();
expect(
(schema.parametersJsonSchema as { properties: Record<string, unknown> })
.properties,
).not.toHaveProperty('offset');
});
it('should return the schema from the resolver when modelId is provided', () => {
+25 -15
View File
@@ -36,14 +36,14 @@ export interface ReadFileToolParams {
file_path: string;
/**
* The line number to start reading from (optional)
* The line number to start reading from (optional, 1-based)
*/
offset?: number;
start_line?: number;
/**
* The number of lines to read (optional)
* The line number to end reading at (optional, 1-based, inclusive)
*/
limit?: number;
end_line?: number;
}
class ReadFileToolInvocation extends BaseToolInvocation<
@@ -74,7 +74,12 @@ class ReadFileToolInvocation extends BaseToolInvocation<
}
override toolLocations(): ToolLocation[] {
return [{ path: this.resolvedPath, line: this.params.offset }];
return [
{
path: this.resolvedPath,
line: this.params.start_line,
},
];
}
async execute(): Promise<ToolResult> {
@@ -97,8 +102,8 @@ class ReadFileToolInvocation extends BaseToolInvocation<
this.resolvedPath,
this.config.getTargetDir(),
this.config.getFileSystemService(),
this.params.offset,
this.params.limit,
this.params.start_line,
this.params.end_line,
);
if (result.error) {
@@ -116,13 +121,11 @@ class ReadFileToolInvocation extends BaseToolInvocation<
if (result.isTruncated) {
const [start, end] = result.linesShown!;
const total = result.originalLineCount!;
const nextOffset = this.params.offset
? this.params.offset + end - start + 1
: end;
llmContent = `
IMPORTANT: The file content has been truncated.
Status: Showing lines ${start}-${end} of ${total} total lines.
Action: To read more of the file, you can use the 'offset' and 'limit' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use offset: ${nextOffset}.
Action: To read more of the file, you can use the 'start_line' and 'end_line' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use start_line: ${end + 1}.
--- FILE CONTENT (truncated) ---
${result.llmContent}`;
@@ -207,11 +210,18 @@ export class ReadFileTool extends BaseDeclarativeTool<
return validationError;
}
if (params.offset !== undefined && params.offset < 0) {
return 'Offset must be a non-negative number';
if (params.start_line !== undefined && params.start_line < 1) {
return 'start_line must be at least 1';
}
if (params.limit !== undefined && params.limit <= 0) {
return 'Limit must be a positive number';
if (params.end_line !== undefined && params.end_line < 1) {
return 'end_line must be at least 1';
}
if (
params.start_line !== undefined &&
params.end_line !== undefined &&
params.start_line > params.end_line
) {
return 'start_line cannot be greater than end_line';
}
const fileFilteringOptions = this.config.getFileFilteringOptions();
+18 -17
View File
@@ -930,7 +930,7 @@ describe('fileUtils', () => {
expect(result.returnDisplay).toContain('Path is a directory');
});
it('should paginate text files correctly (offset and limit)', async () => {
it('should paginate text files correctly (startLine and endLine)', async () => {
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
@@ -938,9 +938,9 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
5,
5,
); // Read lines 6-10
6,
10,
); // Read lines 6-10 (1-based)
const expectedContent = lines.slice(5, 10).join('\n');
expect(result.llmContent).toBe(expectedContent);
@@ -954,13 +954,13 @@ describe('fileUtils', () => {
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
// Read from line 11 to 20. The start is not 0, so it's truncated.
// Read from line 11 to 20. The start is not 1, so it's truncated.
const result = await processSingleFileContent(
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
10,
10,
11,
20,
);
const expectedContent = lines.slice(10, 20).join('\n');
@@ -971,7 +971,7 @@ describe('fileUtils', () => {
expect(result.linesShown).toEqual([11, 20]);
});
it('should handle limit exceeding file length', async () => {
it('should handle endLine exceeding file length', async () => {
const lines = ['Line 1', 'Line 2'];
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
@@ -979,7 +979,7 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
0,
1,
10,
);
const expectedContent = lines.join('\n');
@@ -1015,21 +1015,22 @@ describe('fileUtils', () => {
expect(result.isTruncated).toBe(true);
});
it('should truncate when line count exceeds the limit', async () => {
const lines = Array.from({ length: 11 }, (_, i) => `Line ${i + 1}`);
it('should truncate when line count exceeds the default limit', async () => {
const lines = Array.from({ length: 2500 }, (_, i) => `Line ${i + 1}`);
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
// Read 5 lines, but there are 11 total
// No ranges provided, should use default limit (2000)
const result = await processSingleFileContent(
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
0,
5,
);
expect(result.isTruncated).toBe(true);
expect(result.returnDisplay).toBe('Read lines 1-5 of 11 from test.txt');
expect(result.returnDisplay).toBe(
'Read lines 1-2000 of 2500 from test.txt',
);
expect(result.linesShown).toEqual([1, 2000]);
});
it('should truncate when a line length exceeds the character limit', async () => {
@@ -1043,7 +1044,7 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
0,
1,
11,
);
@@ -1069,7 +1070,7 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
0,
1,
10,
);
expect(result.isTruncated).toBe(true);
+33 -21
View File
@@ -53,7 +53,7 @@ export async function loadWasmBinary(
}
// Constants for text file processing
const DEFAULT_MAX_LINES_TEXT_FILE = 2000;
export const DEFAULT_MAX_LINES_TEXT_FILE = 2000;
const MAX_LINE_LENGTH_TEXT_FILE = 2000;
// Default values for encoding and separator format
@@ -399,16 +399,17 @@ export interface ProcessedFileReadResult {
* Reads and processes a single file, handling text, images, and PDFs.
* @param filePath Absolute path to the file.
* @param rootDirectory Absolute path to the project root for relative path display.
* @param offset Optional offset for text files (0-based line number).
* @param limit Optional limit for text files (number of lines to read).
* @param _fileSystemService Currently unused in this function; kept for signature stability.
* @param startLine Optional 1-based line number to start reading from.
* @param endLine Optional 1-based line number to end reading at (inclusive).
* @returns ProcessedFileReadResult object.
*/
export async function processSingleFileContent(
filePath: string,
rootDirectory: string,
fileSystemService: FileSystemService,
offset?: number,
limit?: number,
_fileSystemService: FileSystemService,
startLine?: number,
endLine?: number,
): Promise<ProcessedFileReadResult> {
try {
if (!fs.existsSync(filePath)) {
@@ -474,14 +475,24 @@ export async function processSingleFileContent(
const lines = content.split('\n');
const originalLineCount = lines.length;
const startLine = offset || 0;
const effectiveLimit =
limit === undefined ? DEFAULT_MAX_LINES_TEXT_FILE : limit;
// Ensure endLine does not exceed originalLineCount
const endLine = Math.min(startLine + effectiveLimit, originalLineCount);
// Ensure selectedLines doesn't try to slice beyond array bounds if startLine is too high
const actualStartLine = Math.min(startLine, originalLineCount);
const selectedLines = lines.slice(actualStartLine, endLine);
let sliceStart = 0;
let sliceEnd = originalLineCount;
if (startLine !== undefined || endLine !== undefined) {
sliceStart = startLine ? startLine - 1 : 0;
sliceEnd = endLine
? Math.min(endLine, originalLineCount)
: Math.min(
sliceStart + DEFAULT_MAX_LINES_TEXT_FILE,
originalLineCount,
);
} else {
sliceEnd = Math.min(DEFAULT_MAX_LINES_TEXT_FILE, originalLineCount);
}
// Ensure selectedLines doesn't try to slice beyond array bounds
const actualStart = Math.min(sliceStart, originalLineCount);
const selectedLines = lines.slice(actualStart, sliceEnd);
let linesWereTruncatedInLength = false;
const formattedLines = selectedLines.map((line) => {
@@ -494,17 +505,18 @@ export async function processSingleFileContent(
return line;
});
const contentRangeTruncated =
startLine > 0 || endLine < originalLineCount;
const isTruncated = contentRangeTruncated || linesWereTruncatedInLength;
const isTruncated =
actualStart > 0 ||
sliceEnd < originalLineCount ||
linesWereTruncatedInLength;
const llmContent = formattedLines.join('\n');
// By default, return nothing to streamline the common case of a successful read_file.
let returnDisplay = '';
if (contentRangeTruncated) {
if (actualStart > 0 || sliceEnd < originalLineCount) {
returnDisplay = `Read lines ${
actualStartLine + 1
}-${endLine} of ${originalLineCount} from ${relativePathForDisplay}`;
actualStart + 1
}-${sliceEnd} of ${originalLineCount} from ${relativePathForDisplay}`;
if (linesWereTruncatedInLength) {
returnDisplay += ' (some lines were shortened)';
}
@@ -517,7 +529,7 @@ export async function processSingleFileContent(
returnDisplay,
isTruncated,
originalLineCount,
linesShown: [actualStartLine + 1, endLine],
linesShown: [actualStart + 1, sliceEnd],
};
}
case 'image':