mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-13 21:07:00 -07:00
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.
This commit is contained in:
@@ -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)',
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
];
|
||||
@@ -1489,13 +1490,14 @@ describe('PolicyEngine', () => {
|
||||
undefined,
|
||||
'tmux send-keys -t [a-z0-9:]+ (C-c|Up|Enter|Up Enter)$',
|
||||
);
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
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 = {
|
||||
@@ -1512,13 +1514,14 @@ describe('PolicyEngine', () => {
|
||||
|
||||
it('should ALLOW git status with ^ anchor', async () => {
|
||||
const patterns = buildArgsPatterns(undefined, undefined, '^git status');
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
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 = {
|
||||
@@ -1532,6 +1535,36 @@ describe('PolicyEngine', () => {
|
||||
|
||||
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)', () => {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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,15 +97,10 @@ 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);
|
||||
});
|
||||
|
||||
@@ -121,10 +114,10 @@ priority = 100
|
||||
`);
|
||||
|
||||
expect(result.rules).toHaveLength(1);
|
||||
// The generated pattern is "command":"^git status
|
||||
expect(
|
||||
result.rules[0].argsPattern?.test('{"command":"git status"}'),
|
||||
).toBe(true);
|
||||
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);
|
||||
});
|
||||
|
||||
@@ -322,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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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}`,
|
||||
};
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -64,93 +64,77 @@ 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);
|
||||
});
|
||||
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]!);
|
||||
|
||||
// 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);
|
||||
|
||||
// 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);
|
||||
expect(regex.test('echo hello')).toBe(true);
|
||||
expect(regex.test('echo')).toBe(true);
|
||||
expect(regex.test('echonop')).toBe(false);
|
||||
});
|
||||
|
||||
describe('commandRegex anchors', () => {
|
||||
it('should transform ^ anchor correctly', () => {
|
||||
const patterns = buildArgsPatterns(undefined, undefined, '^git status');
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
// JSON stringified command: {"command":"git status"}
|
||||
const json = '{"command":"git status"}';
|
||||
expect(regex.test(json)).toBe(true);
|
||||
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);
|
||||
});
|
||||
|
||||
it('should transform $ anchor correctly', () => {
|
||||
@@ -159,23 +143,25 @@ describe('policy/utils', () => {
|
||||
undefined,
|
||||
'tmux send-keys -t [a-z0-9:]+ (C-c|Up|Enter|Up Enter)$',
|
||||
);
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
const json = '{"command":"tmux send-keys -t superpowers:6 C-c"}';
|
||||
expect(regex.test(json)).toBe(true);
|
||||
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);
|
||||
});
|
||||
|
||||
it('should handle $ anchor when other fields follow', () => {
|
||||
it('should handle $ anchor correctly', () => {
|
||||
const patterns = buildArgsPatterns(undefined, undefined, 'git status$');
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
const json = '{"command":"git status","dir_path":"/tmp"}';
|
||||
expect(regex.test(json)).toBe(true);
|
||||
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]!);
|
||||
const json = '{"command":"git status --porcelain"}';
|
||||
expect(regex.test(json)).toBe(false);
|
||||
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', () => {
|
||||
@@ -184,11 +170,10 @@ describe('policy/utils', () => {
|
||||
undefined,
|
||||
'git status\\$',
|
||||
);
|
||||
const regex = new RegExp(patterns[0]!);
|
||||
const regex = new RegExp(patterns[0].pattern!);
|
||||
// Literal $ in command: git status$
|
||||
// JSON: {"command":"git status$"}
|
||||
const json = '{"command":"git status$"}';
|
||||
expect(regex.test(json)).toBe(true);
|
||||
const command = 'git status$';
|
||||
expect(regex.test(command)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,64 +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) {
|
||||
let pattern = commandRegex;
|
||||
|
||||
// 1. Handle ^ (Start Anchor)
|
||||
// If the regex starts with ^, we remove it because the pattern is already
|
||||
// implicitly anchored to the start of the command value by the prepended
|
||||
// "command":" prefix. We only do this if it's not escaped.
|
||||
if (pattern.startsWith('^')) {
|
||||
pattern = pattern.slice(1);
|
||||
}
|
||||
|
||||
// 2. Handle $ (End Anchor)
|
||||
// If the regex ends with $, we replace it with "(?:,|\\}) to match the
|
||||
// closing quote of the command value in the JSON-stringified arguments.
|
||||
// We only do this if the $ is not escaped by an odd number of backslashes.
|
||||
if (pattern.endsWith('$')) {
|
||||
let backslashCount = 0;
|
||||
for (let i = pattern.length - 2; i >= 0; i--) {
|
||||
if (pattern[i] === '\\') {
|
||||
backslashCount++;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (backslashCount % 2 === 0) {
|
||||
// Anchor to the end of the JSON string value: " followed by , or }
|
||||
pattern = pattern.slice(0, -1) + '"(?:,|\\})';
|
||||
}
|
||||
}
|
||||
|
||||
return [`"command":"${pattern}`];
|
||||
// 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 }];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user