Environment variables exposed to CEL expressions via buildEnvContext()

[umag]

Description

buildEnvContext() in src/domain/expressions/model_resolver.ts (line 32-34) exposes ALL environment variables to CEL expressions via Deno.env.toObject(). Any definition using ${{ env.AWS_SECRET_ACCESS_KEY }} or similar will have the secret value resolved and persisted as plaintext YAML in .swamp/definitions-evaluated/.

Steps to Reproduce

1. Set sensitive environment variables (e.g., AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN)
2. Create a model definition using ${{ env.AWS_SECRET_ACCESS_KEY }} in globalArguments
3. Run any command that evaluates expressions
4. Inspect .swamp/definitions-evaluated/ — secrets are in plaintext YAML

Expected Behavior

Either restrict which env vars are accessible to CEL expressions (allowlist), or ensure evaluated definitions with secrets are never persisted to disk.

Actual Behavior

All env vars from the process environment are exposed and persisted as plaintext after expression evaluation.

Summary

This affects the expression resolver in model_resolver.ts. The fix would involve either restricting env var access via an allowlist pattern, or ensuring that evaluated definitions containing env references are marked as sensitive and excluded from disk persistence.

[stack72]

Analysis & Fix Plan

After investigating the codebase, we confirmed there is strong existing precedent for this pattern — vault expressions (${{ vault.get(...) }}) are already deferred to runtime and never persisted. The fix applies the same two-phase evaluation pattern to env expressions.

Root Cause

buildEnvContext() in model_resolver.ts calls Deno.env.toObject() and passes ALL env vars into the CEL evaluation context during the persist phase. Env expressions like ${{ env.AWS_SECRET_ACCESS_KEY }} are eagerly resolved and the plaintext values are written to .swamp/definitions-evaluated/.

Fix: Defer env expressions to runtime (matching vault pattern)

1. Add detection functions (expression_evaluation_service.ts)

• containsEnvExpression() — pattern /\benv\./ detects env references
• containsRuntimeOnlyExpression() — combines vault + env checks

2. Skip env expressions at all persist-phase evaluation sites

Replace containsVaultExpression() with containsRuntimeOnlyExpression() at all 7 persist-phase skip locations:

• expression_evaluation_service.ts — evaluateData() (line 124), evaluateDefinition() (line 179)
• execution_service.ts — lines 634, 1477
• workflow_evaluate.ts — lines 209, 384, 405

3. Update runtime resolution methods

• Rename resolveVaultExpressionsInDefinition() → resolveRuntimeExpressionsInDefinition()
• Filter for both vault AND env expressions (not just vault)
• Change env: {} to env: buildEnvContext() in the runtime CEL evaluation context
• Same for resolveVaultExpressionsInData() → resolveRuntimeExpressionsInData()

4. Defense-in-depth: remove env from persist-phase context

In model_resolver.ts buildContext() (line 396), change env: buildEnvContext() to env: {}. Even if skip logic is bypassed, env values won't leak.

5. Update callers

• model_method_run.ts line 240
• execution_service.ts line 402
• vault_cel_integration_test.ts (5 call sites)

6. Update existing tests

Two integration tests currently expect env expressions to resolve during evaluateDefinition():

• integration/cel_data_access_test.ts (lines 567-601)
• integration/definition_lifecycle_test.ts (lines 748-782)

These will be updated to expect deferred resolution: raw expression after evaluate, resolved value after runtime resolution.

7. New integration tests (integration/env_cel_integration_test.ts)

• Env expressions preserved during persist phase
• Env expressions resolved at runtime
• Persisted YAML does NOT contain resolved env values

Edge Cases

• Mixed vault+env (${{ vault.get(a,b) + env.PREFIX }}): vault resolved by string substitution first, then CEL evaluated with env context. Works naturally.
• Mixed model+env (${{ model.foo.name + env.SUFFIX }}): skipped at persist time. At runtime, model context is empty so model ref fails. Same known limitation as mixed model+vault (documented in vault_cel_integration_test.ts line 292-295). Users should use separate expressions.

Files Affected

File	Change
src/domain/expressions/expression_evaluation_service.ts	Add detection functions, update skips, rename + update runtime methods
src/domain/expressions/model_resolver.ts	Remove env from persist-phase context
src/domain/workflows/execution_service.ts	Update 3 call sites
src/cli/commands/workflow_evaluate.ts	Update 3 skip sites
src/cli/commands/model_method_run.ts	Update 1 runtime call
src/domain/expressions/expression_evaluation_service_test.ts	Add unit tests
integration/vault_cel_integration_test.ts	Update 5 method name calls
integration/cel_data_access_test.ts	Update test assertions
integration/definition_lifecycle_test.ts	Update test assertions
integration/env_cel_integration_test.ts	New integration test file

[stack72]

Why deferring env expressions to runtime is the correct fix

After further analysis, we're confident the plan above (deferring all env expressions to runtime, matching the vault pattern) is the right approach. Here's the reasoning:

Env vars frequently contain secrets — and that's by design

Tools like the AWS CLI, Terraform, and other CLIs consume credentials via environment variables (AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN, DATABASE_URL, etc.). When a swamp model wraps these tools, using ${{ env.AWS_SECRET_ACCESS_KEY }} is the natural way to pass credentials through. Forcing users to duplicate their environment credentials into a swamp vault adds friction for no benefit — the secrets are already configured in their shell environment and "just work."

This means env expressions must be treated as potentially sensitive, just like vault expressions.

The vault pattern already proves this approach works

Swamp's two-phase evaluation model for vault expressions is well-tested and battle-hardened:

• Persist phase: expressions containing vault.get() are skipped, preserved as raw strings
• Runtime phase: resolved to actual values in memory, never written to disk

Applying the same pattern to env expressions is a natural extension with known, predictable behavior.

Concerns we evaluated and dismissed

1. "Not all env vars are secrets" — True, but deferring non-sensitive env vars (like AWS_REGION) to runtime is harmless. They still resolve correctly; they just aren't persisted to definitions-evaluated/. This is actually better behavior — the evaluated definition reflects the environment at execution time, not at evaluation time.

2. "Mixed model+env expressions would break" — Expressions like ${{ model.foo.name + env.REGION }} would need both contexts at runtime. But this is the same existing limitation as mixed model+vault expressions (documented in vault_cel_integration_test.ts line 292-295). It's consistent behavior, not a new regression. Users can split these into separate expressions.

3. "Changes --last-evaluated behavior" — Yes, and correctly so. --last-evaluated caches evaluated definitions to disk. You don't want it caching AWS_SECRET_ACCESS_KEY=AKIA... in plaintext YAML either. The cached definition preserves the raw ${{ env.* }} expression and re-resolves it at runtime.

4. "Just document that users should use vault instead" — This adds unnecessary friction. Users' AWS credentials are already in their environment because that's how AWS tooling works. Telling them to also configure a vault is an extra step that provides no security benefit when the real issue is just disk persistence.

Summary

Env expressions and vault expressions share the same security requirement: resolve at runtime, never persist. The vault infrastructure already handles this correctly. Extending it to env is a straightforward, consistent, and minimal change.

[stack72]

What changes for swamp users:

1. Env expressions resolve at execution time, not creation time. Today, when a user runs swamp model create with ${{ env.AWS_REGION }}, the current value of AWS_REGION gets baked in permanently. After this
   change, the expression stays raw — AWS_REGION is looked up each time the model method runs. This means:
   - If a user changes an env var, they no longer need to re-evaluate their definitions — the new value is picked up automatically on the next run
   - Definitions are portable across environments (dev → staging → prod) without re-evaluation
2. --last-evaluated with env. Currently --last-evaluated replays with baked-in env values from the evaluation run. After this change, env vars are resolved fresh each time (matching how vault secrets
   already work). The "fast replay" still skips expensive CEL evaluation and forEach expansion — it just looks up env vars fresh.
3. Invalid static fields are now caught at runtime. Today, if a user has port: "not-a-number" alongside an expression field, validation silently passes. After this change, they'll get a clear error at
   method execution time: Global arguments validation failed: Expected number at "port". This is a new error users might see for definitions that were previously silently invalid.
4. No changes to workflow/definition YAML syntax. No migration needed — existing definitions with env expressions will work, they'll just be re-resolved at runtime instead of using baked-in values.

The main risk: users who intentionally relied on env being captured at evaluation time (snapshotting env vars) would see different behavior. But this is an edge case and the new behavior is arguably more
correct.