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,9 +5,12 @@
  * 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
- * normalized path variants, so `./a`, `dir/../a`, and Windows separator or
- * case variants can match the same rule. Pure functions; no scoped service.
+ * `matchesRule` closures and canonical rule strings. Glob matching accepts a
+ * subject under path-glob semantics or as opaque text where `*` also crosses
+ * `/`; NUL-bearing subjects match under path-glob semantics only. 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.
  */
 
 import { isAbsolute, join, parse } from 'pathe';
@@ -27,19 +30,46 @@ 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;
+  if (pathSegmentGlobMatch(value, pattern, options)) return true;
+  if (value.includes(SLASH_PLACEHOLDER) || pattern.includes(SLASH_PLACEHOLDER)) return false;
+
+  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 {
+  return value.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 +78,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,44 @@ 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);
+  });
+
+  it('keeps NUL-bearing subjects distinct instead of conflating them', () => {
+    expect(matchesGlobRuleSubject('ab', 'a\u0000b')).toBe(false);
+    expect(matchesGlobRuleSubject('a/b', 'a\u0000b')).toBe(false);
+    expect(matchesGlobRuleSubject('a\u0000b', 'a\u0000b')).toBe(true);
+    expect(matchesGlobRuleSubject('a*', 'a\u0000b')).toBe(true);
+  });
+
+  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-v2/test/lint/header-comments.test.ts

@@ -0,0 +1,37 @@
+/**
+ * Header-only-comment gate — the package AGENTS.md "Comment conventions"
+ * section requires comments to live solely in the top-of-file block, never
+ * beside functions, methods, or statements. This probe guards files that
+ * previously attracted review findings for inline implementation narration;
+ * extend the list when a file newly trades in subtle encoding or matching
+ * rules that tempt an explanatory comment.
+ */
+
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+import { describe, expect, it } from 'vitest';
+
+const SRC_ROOT = join(import.meta.dirname, '..', '..', 'src');
+
+const HEADER_ONLY_FILES = ['tool/rule-match.ts'];
+
+function commentLinesOutsideHeader(source: string): string[] {
+  const headerEnd = source.indexOf('*/');
+  const body = headerEnd === -1 ? source : source.slice(headerEnd + 2);
+  return body
+    .split('\n')
+    .filter(
+      (line) =>
+        line.includes('//') || line.includes('/*') || line.trimStart().startsWith('*'),
+    );
+}
+
+describe('header-only comments', () => {
+  for (const file of HEADER_ONLY_FILES) {
+    it(`${file} keeps comments inside the top-of-file header`, () => {
+      const source = readFileSync(join(SRC_ROOT, file), 'utf8');
+      expect(source.startsWith('/**')).toBe(true);
+      expect(commentLinesOutsideHeader(source)).toEqual([]);
+    });
+  }
+});

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

@@ -15,23 +15,58 @@ 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. NUL-bearing subjects skip this:
+  // the historical match above already compares them literally, and the
+  // rewrite must stay injective so distinct subjects cannot collide.
+  if (value.includes(SLASH_PLACEHOLDER) || pattern.includes(SLASH_PLACEHOLDER)) return false;
+
+  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 {
+  return value.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 +80,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,44 @@ 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);
+  });
+
+  it('keeps NUL-bearing subjects distinct instead of conflating them', () => {
+    expect(matchesGlobRuleSubject('ab', 'a\u0000b')).toBe(false);
+    expect(matchesGlobRuleSubject('a/b', 'a\u0000b')).toBe(false);
+    expect(matchesGlobRuleSubject('a\u0000b', 'a\u0000b')).toBe(true);
+    expect(matchesGlobRuleSubject('a*', 'a\u0000b')).toBe(true);
+  });
+
+  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', {