fix: support spaces in quoted vault.get() arguments#906
Conversation
The vault.get() regex in model_resolver.ts used character classes that excluded whitespace even inside quoted arguments. This broke vault expressions referencing secrets with spaces in their names, such as 1Password fields like "Client ID" or paths like "Tailscale K8s Operator/Client ID". Update the regex to use alternation: quoted arguments now match any character except the closing quote, while unquoted arguments preserve current no-space behavior. Update match extraction for the new capture group positions. Fixes #902 Co-authored-by: Dan McClain <danmcclain@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
Blocking Issues
None.
Suggestions
None — this is a clean, well-scoped fix. The regex alternation correctly handles quoted arguments (with spaces) vs unquoted arguments, the group extraction logic is correct, and test coverage is thorough with 7 new tests covering the key scenarios (spaces in keys, vault names, both, mixed quoting, and 1Password-style paths). The containsVaultExpression detection function already works correctly since it only checks for the presence of vault.get( without parsing arguments.
There was a problem hiding this comment.
Adversarial Review
Critical / High
None.
Medium
None.
Low
-
model_resolver.ts:835—.+?does not handle escaped quotes inside quoted arguments.
If a secret key ever contains the same quote character used to delimit the argument (e.g.,vault.get("infra", "API Key \"prod\"")), the non-greedy.+?would stop at the backslash-escaped quote, producing an incorrect key likeAPI Key \. This is a pre-existing limitation (the old regex had the same restriction via[^'"\x60\s,]+) and is unlikely in practice since vault key names rarely contain quotes. Mentioning for completeness only — not a regression. -
model_resolver.ts:835— Unquoted branch[^\s,)]+will match stray quote characters.
If a user writes malformed input likevault.get("foo, bar)(missing closing quote), the quoted branch fails and the unquoted branch matches"foo(since"is not excluded from[^\s,)]+). This would produce a vault name of"foowith a literal quote. Again, not a regression from the old pattern and only triggers on syntactically invalid input.
Verdict
PASS — The regex change correctly extends vault expression parsing to support spaces in quoted arguments. The alternation structure is sound, group numbering is correct, the containsVaultExpression detection pattern needed no change (it's a simpler presence check), and the test coverage is thorough with 6 new test cases covering the key scenarios (spaces in key, vault name, both, mixed quoted/unquoted, 1Password-style paths). No regressions to existing behavior.
Summary
vault.get()regex inmodel_resolver.tsto allow spaces insidequoted arguments (e.g.
vault.get("infra", "Client ID"))unquoted args preserve current no-space behavior
Fixes #902
Test Plan
vault_expression_test.ts:containsVaultExpressiondetection test for quoted args with spacesdeno check,deno lint,deno fmtall pass🤖 Generated with Claude Code