bug: Vault secret shell escaping incomplete — $(), semicolons, and pipes are interpreted

[stack72]

Summary

The shell escaping fix in commit 878e8de for vault secrets is incomplete. While backtick command substitution is properly escaped, three other shell metacharacter classes are still interpreted when vault secrets are injected into sh -c commands:

Metacharacter	Vault secret value	Expected output	Actual behavior
$()	value$(whoami)end	literal value$(whoami)end	$(whoami) is expanded
;	value; echo injected	literal value; echo injected	; splits into two commands
|	value| cat /etc/passwd	literal value| cat /etc/passwd	| pipes to second command
`	value`whoami`end	literal value`whoami`end	correctly escaped (passes)

Reproduction

For each case:

1. Create a vault and store a secret with metacharacters:

   swamp vault create local_encryption test-vault
   swamp vault put test-vault "METACHAR_SECRET=value$(whoami)end"
   

2. Install a model that echoes the secret:

   globalArguments:
     run: "echo ${{ vault.get('test-vault', 'METACHAR_SECRET') }}"

3. Execute the model:

   swamp model method run vault-echo-metachar execute
   

4. The output contains the expanded/injected result instead of the literal string.

Expected behavior

All shell metacharacters in vault secret values should be properly escaped before being passed to sh -c. The resolved secret should appear literally in the output, regardless of what characters it contains.

Root cause

The escaping fix from 878e8de handles backticks but does not escape $, ;, or |. The secret value likely needs to be single-quoted or fully shell-escaped before interpolation into the sh -c command string.

Security impact

An attacker who can write to a vault (or a user who stores a password containing shell metacharacters) can achieve arbitrary command execution through the shell interpolation path. This is particularly dangerous because vault secrets are expected to contain arbitrary values.

References

• Original escaping fix: 878e8de
• Related: security: validate version string in HttpSourceDownloader to prevent unexpected URL construction #413, fix: validate version string in HttpSourceDownloader #418

[stack72]

Investigation & Fix Plan

Findings

The issue is confirmed legitimate for ;, |, and & (plus (, ), <, > for completeness). However, $() is already properly handled — the existing $ escaping produces \$ in the shell, which prevents $(cmd) from executing. The existing test at line 339-348 of vault_expression_test.ts verifies this.

Root Cause

The escaping in resolveVaultExpressions() (model_resolver.ts:795-803) uses a \\X technique that works through two interpretation layers (CEL → shell). It currently covers $ and ` but not other POSIX shell metacharacters that enable command injection.

Fix Approach

Extend the existing \\\\X escaping technique to cover all dangerous shell metacharacters. Add 7 new .replace() calls for:

Character	Attack vector	Escaped form
;	Command separator (cmd1; cmd2)	\;
|	Pipe (cmd1 | cmd2)	|
&	Background/AND (cmd1 & cmd2)	\&
( )	Subshell execution	\( \)
< >	I/O redirection	\< \>

Files to Change

1. src/domain/expressions/model_resolver.ts — Add 7 new .replace() calls after the existing backtick escape, update comments
2. src/domain/vaults/vault_expression_test.ts — Add 6 new test cases (semicolon, pipe, ampersand, parentheses, redirects, combined), update 2 existing tests (dollar-amp and multi-dollar) whose expected values change because & is now escaped
3. integration/vault_cel_integration_test.ts — Update expectedResolved for the special characters test to include backslashes before &, (, ), |, ;, <, >
4. design/expressions.md — Expand Shell Safety section to document all escaped metacharacters

Trade-offs

This uses the same approach already accepted for $ escaping:

• Non-shell contexts: Vault secrets used outside shell commands (e.g., keyData) will have backslashes before these characters — same as existing $ behavior
• Quoted shell contexts: Inside double quotes, \; stays as two characters (cosmetic issue only, no injection risk since quotes already protect)

[stack72]

Analysis: Fix Options

The Problem

When a vault secret is used in a shell command via run: "echo ${{ vault.get(v, s) }}", the secret value is string-interpolated directly into the sh -c command. Shell metacharacters in the secret are interpreted as shell syntax, enabling command injection.

What We Have Today

Commit 878e8de added escaping for $ and ` to prevent command substitution ($(cmd) and `cmd`). The technique: double-backslash in TypeScript → single backslash after CEL evaluation → shell treats \$ and \` as literal characters.

Currently escaped: $, `, \, ", ', \n, \r, \t

Not escaped (vulnerable): ; (command separator), | (pipe), & (background/AND), ( ) (subshell), < > (redirects)

Example: secret pass;rm -rf / → shell runs echo pass then rm -rf /.

Note: The issue reports $() as vulnerable, but it is already properly handled — the existing $ escaping produces \$ in the shell, which prevents $(cmd) from executing.

Option A: Extend Character Escaping

Add the same \\X technique for ;, |, &, (, ), <, >.

Effort: ~100 lines, 4 files, ships in a day.

Side effect: The escaping is applied to ALL vault secrets, not just those used in shell commands. A secret like host=db;port=5432 used as a non-shell attribute becomes host=db\;port=5432. This same trade-off already exists for $ — a secret price=$100 becomes price=\$100 everywhere. Extending to ; and & makes it more likely to hit real users (these characters appear in connection strings, URLs, etc.).

Option B: Environment Variable Approach

Instead of interpolating secrets into the command string, pass them as environment variables to the shell process. The shell expands $__SWAMP_VAULT_0 at runtime — variable values are never parsed as shell syntax, so injection is impossible regardless of secret content.

Effort: ~2-3 weeks. Requires threading model type information through the expression evaluation pipeline so it can distinguish shell run fields from non-shell attributes. Currently ExpressionEvaluationService has no concept of "this field will be used in a shell command."

Benefit: Non-shell contexts get raw secret values with zero corruption. Eliminates the entire class of shell injection bugs — no more maintaining a list of dangerous characters.

The Core Tension

The escaping happens in resolveVaultExpressions() which is context-unaware — it doesn't know if the secret will end up in a shell command or a database connection attribute. Making it context-aware requires significant plumbing (the model type isn't available during expression evaluation today).