fix(commands): escape named-argument names before building the regex (#1914)

substituteArguments builds a dynamic RegExp from each frontmatter-defined
argument name without escaping regex metacharacters:
new RegExp(`\$${name}(?![\[\w])`, 'g'). parseArgumentNames only rejects
empty and numeric-only names, so an author-defined name that contains a regex
special char reaches the constructor. A name with an unbalanced '(' / '[' (e.g.
'pattern)') throws a SyntaxError on every invocation of that skill/command, and
a name like 'a.' silently over-matches ('.' turns $ab into the arg value).
Escape the name with the existing escapeRegExp helper so it is matched
literally.

Consumers: src/skills/loadSkillsDir.ts and src/utils/plugins/loadPluginCommands.ts
feed frontmatter argument names into substituteArguments.
This commit is contained in:
0xfandom
2026-07-09 09:17:55 +08:00
committed by GitHub
parent 6602076b53
commit 3f85b255dd
2 changed files with 41 additions and 2 deletions
+34
View File
@@ -0,0 +1,34 @@
import { describe, expect, test } from 'bun:test'
import { substituteArguments } from './argumentSubstitution.js'
describe('substituteArguments named-argument regex safety', () => {
test('substitutes a normal named argument', () => {
expect(substituteArguments('hello $name', 'world', false, ['name'])).toBe(
'hello world',
)
})
// A frontmatter argument name is author-defined and unrestricted beyond
// rejecting empty/numeric-only names, so a name with regex metacharacters must
// be treated literally rather than compiled into a live pattern.
test('does not over-match when the name contains a regex wildcard', () => {
// Name `a.` must not let `.` match the `b` in `$ab`.
expect(substituteArguments('$ab', 'X', false, ['a.'])).toBe('$ab')
// The literal `$a.` placeholder still substitutes.
expect(substituteArguments('$a.', 'X', false, ['a.'])).toBe('X')
})
test('does not throw when the name contains unbalanced regex characters', () => {
for (const name of ['a)', 'a(', 'a[', 'a+', 'a*', 'a{2']) {
expect(() =>
substituteArguments('body has no placeholder', 'x', true, [name]),
).not.toThrow()
}
})
test('substitutes a literal placeholder whose name has metacharacters', () => {
expect(substituteArguments('run $a+b now', 'VAL', false, ['a+b'])).toBe(
'run VAL now',
)
})
})
+7 -2
View File
@@ -11,6 +11,7 @@
*/
import { tryParseShellCommand } from './bash/shellQuote.js'
import { escapeRegExp } from './stringUtils.js'
/**
* Parse an arguments string into an array of individual arguments.
@@ -113,9 +114,13 @@ export function substituteArguments(
if (!name) continue
// Match $name but not $name[...] or $nameXxx (word chars)
// Also ensure we match word boundaries to avoid partial matches
// Also ensure we match word boundaries to avoid partial matches.
// escapeRegExp: the name comes from author-defined frontmatter, so a name
// containing regex metacharacters would otherwise throw (unbalanced `(`/`[`)
// or over-match (`a.` matching `$ab`) — parseArgumentNames does not restrict
// the character set beyond rejecting empty/numeric-only names.
content = content.replace(
new RegExp(`\\$${name}(?![\\[\\w])`, 'g'),
new RegExp(`\\$${escapeRegExp(name)}(?![\\[\\w])`, 'g'),
parsedArgs[i] ?? '',
)
}