fix: escape $ and ` in vault secrets to prevent shell command substitution

[stack72]

Summary

Vault secrets containing $(cmd) or `cmd` sequences were passed unescaped through CEL evaluation and executed as shell commands when used in command/shell models via sh -c. This is a shell injection vulnerability.

Background

When vault.get(vault-name, secret-key) appears in a CEL expression, resolveVaultExpressions() fetches the secret value and embeds it as a CEL string literal — for example "my-secret-value" — before cel-js evaluates the expression. The existing escaping (from PR #407) handled CEL string breakout for backticks (```) but did not make the value safe at the shell layer.

Why PR #407's ``` fix was incomplete

PR #407 added .replace(/\/g, "\"). While \` is a valid CEL escape sequence (it maps back to `), this means cel-js strips the backslash during evaluation. The shell then receives an unescaped backtick and executes `cmd` as command substitution. $ was not escaped at all.

Why naive .replace(/\$/g, "\\$") is wrong

\$ is not in cel-js's STRING_ESCAPES map. Inserting it into a CEL string literal throws ParseError: Invalid escape sequence: \$.

The Fix: \\X prefix via "\\\\X" replacement

Inserting \\X (two backslashes + char) into the escaped value causes a two-layer transformation:

Layer	$ example	` example
Secret stored in vault	$(cat /etc/passwd)	`cmd`
escapedValue string	\\$(cat /etc/passwd)	\\`cmd\\`
CEL string literal	"\\$(cat /etc/passwd)"	"\\`cmd\\`"
After CEL evaluation	\$(cat /etc/passwd)	\`cmd\`
Shell output	$(cat /etc/passwd) — literal ✓	`cmd` — literal ✓

• CEL sees \\ → produces one \, then treats $ / ` as plain literal characters (both are valid after \\)
• Shell receives \$ or \` → treats them as literal characters, no substitution

This also replaces PR #407's broken \` line — the new \\\\`` produces the correct shell-safe \ in the CEL string, whereas the old `\\ caused CEL to silently unescape back to `.

What This Can Break for Users

Vault secrets containing $ or ` will now have a \ prefix in their CEL-evaluated value. This is intentional — it is the shell-safe form — but it is a behaviour change in one scenario:

If a user stores a secret like p@ssw0rd!#$%^&* and uses it as a model attribute (not in a shell command), the resolved attribute value will be p@ssw0rd!#\$%^&* (with a backslash before the $).

Practically speaking:

• command/shell model run field — correct and safe: the shell interprets \$ as a literal $, so the command receives the original value.
• Non-shell attributes — the value will contain \$ rather than $. If that attribute is then passed to an external API or compared to a stored value, the extra backslash could cause a mismatch.

The fundamental trade-off is that resolveVaultExpressions() is a shared function with no knowledge of whether the resolved value will reach a shell. Scoping the escaping to shell models only would require a larger architectural change and is out of scope for this fix.

Recommendation for users: Avoid storing secrets with literal $ or ` characters if they are used in non-shell model attributes. Secrets used in command/shell models are fully safe.

Files Changed

File	Change
src/domain/expressions/model_resolver.ts	Replace \$ / \` escaping with \\\\$ / \\\\` double-backslash prefix
src/domain/vaults/vault_expression_test.ts	Update 7 existing dollar/backtick tests to match new escaping; add 2 new shell-injection-prevention tests
integration/vault_cel_integration_test.ts	Update special-characters integration test to expect \$ in CEL-evaluated value
design/expressions.md	Add Shell Safety subsection documenting the escaping contract

Verification

All 1809 tests pass (deno run test). Checked against swamp-uat — no UAT tests use $ or ` in vault secret values, so no UAT impact.

🤖 Generated with Claude Code

[github-actions]

Code Review: PR #419 - Shell injection fix for vault secrets

Verdict: ✅ APPROVED

Summary

This PR fixes a shell injection vulnerability where vault secrets containing $(cmd) or `cmd` sequences could be executed as shell commands when used in command/shell models via sh -c.

Blocking Issues

None.

Review Details

1. Security Fix Assessment ✅

• The double-backslash escaping approach (\\$ and `\``) is correct
• CEL evaluates \\X → \X, then shell treats \$ and \` as literals
• The escape order is correct: backslashes first, then $ and `, then quotes/whitespace

2. Code Quality ✅

• TypeScript strict mode: No any types
• Named exports used throughout
• License headers present
• Code is clean and well-commented

3. Test Coverage ✅

• Updated 7 existing dollar/backtick tests to match new escaping
• Added 2 new shell-injection-prevention test cases (should escape $ to prevent shell command substitution, should escape backticks to prevent shell command substitution)
• Integration test updated with clear comments explaining the expected behavior

4. DDD Principles ✅

• Change is appropriately located in ModelResolver (domain service)
• Escaping is part of resolving vault expressions - single responsibility maintained
• No persistence leaking - vault values remain runtime-only

5. Documentation ✅

• New "Shell Safety" section added to design/expressions.md
• Clear explanation of the escaping contract
• Examples provided

Suggestions (non-blocking)

1. Behavior change awareness: The PR description correctly documents that non-shell attributes will now have \$ instead of $. This is a reasonable security trade-off, but users should be aware. The documentation update captures this well.

2. Future consideration: The PR description mentions that scoping escaping to shell models only would require a larger architectural change. This could be considered as a future enhancement if the backslash-in-non-shell-attributes becomes a pain point for users.

CI Status

• Lint, Test, and Format Check: ✅ Passed

Good security fix with comprehensive test coverage and proper documentation!