fix: match permission rule glob subjects as opaque text

.changeset/glob-rule-subjects-cross-slashes.md

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/kimi-code": patch
+---
+
+Fix permission rule argument patterns such as Bash(rm -rf*) not matching commands, URLs, or search text that contain slashes or dot segments. Patterns negated with `!` now exclude those subjects as well.

packages/agent-core-v2/src/tool/rule-match.ts

@@ -5,7 +5,8 @@
  * and the rule-subject helpers (`literalRulePattern`,
  * `escapeRuleSubjectLiteral`, `matchesGlobRuleSubject`,
  * `matchesPathRuleSubject`) that tool implementations use to build their
- * `matchesRule` closures and canonical rule strings. Path matching compares
+ * `matchesRule` closures and canonical rule strings. Glob matching treats
+ * subjects as opaque text, so `*` crosses `/`. Path matching compares
  * normalized path variants, so `./a`, `dir/../a`, and Windows separator or
  * case variants can match the same rule. Pure functions; no scoped service.
  */
@@ -27,19 +28,51 @@ interface PathMatchSemantics {
   readonly pathClass: PathClass;
 }
 
+const SLASH_PLACEHOLDER = '\0';
+
 export function globMatch(value: string, pattern: string, options?: { nocase?: boolean }): boolean {
-  if (picomatch.isMatch(value, pattern, options)) return true;
+  // Try the historical path-semantics match first so rules that matched
+  // before keep matching (e.g. `a/**/b` still matches `a/b`).
+  if (pathSegmentGlobMatch(value, pattern, options)) return true;
+
+  // Then match the subject as opaque text: picomatch gives wildcards path
+  // semantics (`*` stops at `/` and refuses dot segments), so rewrite `/` to
+  // a placeholder and allow dots instead.
+  const opaqueOptions = { ...options, dot: true };
+  if (picomatch.isMatch(asOpaqueText(value), asOpaqueText(pattern), opaqueOptions)) return true;
 
   const normalizedValue = stripLeadingDotSlash(value);
   const normalizedPattern = stripLeadingDotSlash(pattern);
   if (normalizedValue === value && normalizedPattern === pattern) return false;
-  return picomatch.isMatch(normalizedValue, normalizedPattern, options);
+  return picomatch.isMatch(
+    asOpaqueText(normalizedValue),
+    asOpaqueText(normalizedPattern),
+    opaqueOptions,
+  );
+}
+
+function asOpaqueText(value: string): string {
+  // Strip real NUL bytes first so one cannot be mistaken for a rewritten `/`.
+  return value.replaceAll(SLASH_PLACEHOLDER, '').replaceAll('/', SLASH_PLACEHOLDER);
 }
 
 function stripLeadingDotSlash(value: string): string {
   return value.startsWith('./') ? value.slice(2) : value;
 }
 
+function pathSegmentGlobMatch(
+  value: string,
+  pattern: string,
+  options?: { nocase?: boolean },
+): boolean {
+  if (picomatch.isMatch(value, pattern, options)) return true;
+
+  const normalizedValue = stripLeadingDotSlash(value);
+  const normalizedPattern = stripLeadingDotSlash(pattern);
+  if (normalizedValue === value && normalizedPattern === pattern) return false;
+  return picomatch.isMatch(normalizedValue, normalizedPattern, options);
+}
+
 export function pathGlobMatch(
   value: string,
   pattern: string,
@@ -48,11 +81,11 @@ export function pathGlobMatch(
   const semantics = pathMatchSemantics(value, pattern, pathOptions);
   const nocase = pathOptions?.caseInsensitivePaths ?? true;
 
-  if (globMatch(value, pattern, { nocase })) return true;
+  if (pathSegmentGlobMatch(value, pattern, { nocase })) return true;
 
   for (const valueVariant of pathVariants(value, semantics, pathOptions)) {
     for (const patternVariant of pathVariants(pattern, semantics, pathOptions)) {
-      if (globMatch(valueVariant, patternVariant, { nocase })) return true;
+      if (pathSegmentGlobMatch(valueVariant, patternVariant, { nocase })) return true;
     }
   }
   return false;

packages/agent-core-v2/test/agent/permissionRules/matchesRule.test.ts

@@ -132,6 +132,38 @@ describe('permissionRules/matchPermissionRule', () => {
     expect(matches(rule('Bad(unclosed'), 'Bad', noArgs)).toBe(false);
   });
 
+  it('matches glob rule subjects as opaque text rather than as paths', () => {
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf x')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf /tmp/x')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git commit -m "fix src/a.ts"')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf ./build')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf ~/.ssh')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf /home/u/.ssh')).toBe(true);
+    expect(matchesGlobRuleSubject('https://example.com/*', 'https://example.com/a/b')).toBe(true);
+    expect(matchesGlobRuleSubject('*acme corp*', 'news about acme corp / rivals')).toBe(true);
+    expect(matchesGlobRuleSubject('**rm**', 'rm -rf /tmp/x')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git status')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git2 status')).toBe(false);
+    expect(matchesGlobRuleSubject('rm -rf*', 'git status')).toBe(false);
+    expect(matchesGlobRuleSubject('git log -- src/*.ts', 'git log -- srcXx.ts')).toBe(false);
+    expect(matchesGlobRuleSubject('https://example.com/a', 'https://example.com/b')).toBe(false);
+    expect(matchesGlobRuleSubject('!git *', 'git commit -m "fix src/a.ts"')).toBe(false);
+    expect(matchesGlobRuleSubject('!git *', 'npm test')).toBe(true);
+  });
+
+  it('keeps historical glob matches that opaque-text semantics alone would drop', () => {
+    expect(matchesGlobRuleSubject('**/*.ts', 'a.ts')).toBe(true);
+    expect(matchesGlobRuleSubject('a/**/b', 'a/b')).toBe(true);
+    expect(matchesGlobRuleSubject('a/**/b', 'a/x/y/b')).toBe(true);
+    expect(matchesGlobRuleSubject('a/b', 'a\u0000b')).toBe(false);
+  });
+
+  it('keeps path rule subjects on path semantics where * does not cross /', () => {
+    expect(matchesPathRuleSubject('src/*', 'src/a.ts')).toBe(true);
+    expect(matchesPathRuleSubject('src/**', 'src/sub/a.ts')).toBe(true);
+    expect(matchesPathRuleSubject('src/*', 'src/sub/a.ts')).toBe(false);
+  });
+
   it('does not match rule arguments without an execution matcher', () => {
     expect(matches(rule('Custom("query":"a.b")'), 'Custom', noArgs)).toBe(false);
     expect(matches(rule('Bash("command":"git status")'), 'Bash', noArgs)).toBe(false);

packages/agent-core/src/tools/support/path-glob-match.ts

@@ -15,23 +15,55 @@ interface PathMatchSemantics {
   readonly pathClass: PathClass;
 }
 
+const SLASH_PLACEHOLDER = '\0';
+
 /**
  * Match ordinary string fields, like command text or search patterns.
  * `*` and `**` work as wildcards, but the value is not treated as a file path.
  */
 export function globMatch(value: string, pattern: string, options?: { nocase?: boolean }): boolean {
-  if (picomatch.isMatch(value, pattern, options)) return true;
+  // Try the historical path-semantics match first so rules that matched
+  // before keep matching (e.g. `a/**/b` still matches `a/b`).
+  if (pathSegmentGlobMatch(value, pattern, options)) return true;
+
+  // Then match the subject as opaque text: picomatch gives wildcards path
+  // semantics (`*` stops at `/` and refuses dot segments), so rewrite `/` to
+  // a placeholder and allow dots instead.
+  const opaqueOptions = { ...options, dot: true };
+  if (picomatch.isMatch(asOpaqueText(value), asOpaqueText(pattern), opaqueOptions)) return true;
 
   const normalizedValue = stripLeadingDotSlash(value);
   const normalizedPattern = stripLeadingDotSlash(pattern);
   if (normalizedValue === value && normalizedPattern === pattern) return false;
-  return picomatch.isMatch(normalizedValue, normalizedPattern, options);
+  return picomatch.isMatch(
+    asOpaqueText(normalizedValue),
+    asOpaqueText(normalizedPattern),
+    opaqueOptions,
+  );
+}
+
+function asOpaqueText(value: string): string {
+  // Strip real NUL bytes first so one cannot be mistaken for a rewritten `/`.
+  return value.replaceAll(SLASH_PLACEHOLDER, '').replaceAll('/', SLASH_PLACEHOLDER);
 }
 
 function stripLeadingDotSlash(value: string): string {
   return value.startsWith('./') ? value.slice(2) : value;
 }
 
+function pathSegmentGlobMatch(
+  value: string,
+  pattern: string,
+  options?: { nocase?: boolean },
+): boolean {
+  if (picomatch.isMatch(value, pattern, options)) return true;
+
+  const normalizedValue = stripLeadingDotSlash(value);
+  const normalizedPattern = stripLeadingDotSlash(pattern);
+  if (normalizedValue === value && normalizedPattern === pattern) return false;
+  return picomatch.isMatch(normalizedValue, normalizedPattern, options);
+}
+
 /**
  * Match file path fields, like Read/Write/Edit `path`.
  * Also compares normalized forms, so `./a`, `dir/../a`, and Windows
@@ -45,11 +77,11 @@ export function pathGlobMatch(
   const semantics = pathMatchSemantics(value, pattern, pathOptions);
   const nocase = pathOptions?.caseInsensitivePaths ?? true;
 
-  if (globMatch(value, pattern, { nocase })) return true;
+  if (pathSegmentGlobMatch(value, pattern, { nocase })) return true;
 
   for (const valueVariant of pathVariants(value, semantics, pathOptions)) {
     for (const patternVariant of pathVariants(pattern, semantics, pathOptions)) {
-      if (globMatch(valueVariant, patternVariant, { nocase })) return true;
+      if (pathSegmentGlobMatch(valueVariant, patternVariant, { nocase })) return true;
     }
   }
   return false;

packages/agent-core/test/agent/permission.test.ts

@@ -3799,6 +3799,38 @@ describe('Permission rule helpers', () => {
     expect(ruleMatches(permissionRule('Bad(unclosed'), 'Bad', {})).toBe(false);
   });
 
+  it('matches glob rule subjects as opaque text rather than as paths', () => {
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf x')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf /tmp/x')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git commit -m "fix src/a.ts"')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf ./build')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf ~/.ssh')).toBe(true);
+    expect(matchesGlobRuleSubject('rm -rf*', 'rm -rf /home/u/.ssh')).toBe(true);
+    expect(matchesGlobRuleSubject('https://example.com/*', 'https://example.com/a/b')).toBe(true);
+    expect(matchesGlobRuleSubject('*acme corp*', 'news about acme corp / rivals')).toBe(true);
+    expect(matchesGlobRuleSubject('**rm**', 'rm -rf /tmp/x')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git status')).toBe(true);
+    expect(matchesGlobRuleSubject('git *', 'git2 status')).toBe(false);
+    expect(matchesGlobRuleSubject('rm -rf*', 'git status')).toBe(false);
+    expect(matchesGlobRuleSubject('git log -- src/*.ts', 'git log -- srcXx.ts')).toBe(false);
+    expect(matchesGlobRuleSubject('https://example.com/a', 'https://example.com/b')).toBe(false);
+    expect(matchesGlobRuleSubject('!git *', 'git commit -m "fix src/a.ts"')).toBe(false);
+    expect(matchesGlobRuleSubject('!git *', 'npm test')).toBe(true);
+  });
+
+  it('keeps historical glob matches that opaque-text semantics alone would drop', () => {
+    expect(matchesGlobRuleSubject('**/*.ts', 'a.ts')).toBe(true);
+    expect(matchesGlobRuleSubject('a/**/b', 'a/b')).toBe(true);
+    expect(matchesGlobRuleSubject('a/**/b', 'a/x/y/b')).toBe(true);
+    expect(matchesGlobRuleSubject('a/b', 'a\u0000b')).toBe(false);
+  });
+
+  it('keeps path rule subjects on path semantics where * does not cross /', () => {
+    expect(matchesPathRuleSubject('src/*', 'src/a.ts')).toBe(true);
+    expect(matchesPathRuleSubject('src/**', 'src/sub/a.ts')).toBe(true);
+    expect(matchesPathRuleSubject('src/*', 'src/sub/a.ts')).toBe(false);
+  });
+
   it('does not match rule arguments without an execution matcher', () => {
     expect(
       ruleMatches(permissionRule('Custom("query":"a.b")'), 'Custom', {