fix(config): expand env vars inside global-block heredocs#284
Open
fix(config): expand env vars inside global-block heredocs#284
Conversation
The OPA policy is configured via a heredoc inside the global `opa { policy
= <<EOF ... EOF }` block. Operators are expected to template caller-identity
values into that policy with `${MY_PRINCIPAL}` placeholders that get
substituted from the environment at startup, mirroring how strategy/cache
blocks handle env vars.
That substitution did not happen for the global block. `Load()` calls
`expandVars()` (which walks the AST and runs `os.Expand` on both
`*hcl.String` and `*hcl.Heredoc` attribute values) on the providers AST,
but `loadGlobalConfig` decoded the global AST directly via
`hcl.UnmarshalAST` with `hcl.WithDefaultTransformer`. That transformer is
only applied to struct-tag `default:` values inside `valueFromTag`, never
to live AST node strings — so `${MY_PRINCIPAL}` reached the OPA compiler
verbatim and no SPIFFE id ever matched the resulting comparison.
Export `expandVars` as `ExpandVars` and call it from `loadGlobalConfig`
before either UnmarshalAST pass, so global heredoc and string attribute
values get the same treatment as provider blocks. Adds `TestExpandVars`
covering both node kinds plus the os.Expand "unset → empty" case.
alecthomas
approved these changes
May 1, 2026
| // struct-tag default values, not against live AST node strings, so without | ||
| // this pass `opa { policy = <<EOF ... ${CACHEW_X} ... EOF }` would reach | ||
| // the OPA compiler with the placeholder intact. | ||
| config.ExpandVars(ast, envars) |
Collaborator
There was a problem hiding this comment.
Wouldn't it make more sense, given the line above is InjectEnvars(), that we just refactor that function to do whatever is missing?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
${VAR}placeholders inside theopa { policy = <<EOF ... EOF }global block (and any other heredoc on a global block such aslog,scheduler,bind) were never substituted at startup. The placeholder reached the OPA Rego compiler verbatim, and thecaller_principal == "${MY_WARMER_PRINCIPAL}"style guard could never match any real caller — every write got 403, regardless of the env var value.Root cause
There are two AST decode paths in the codebase, with different env-var handling:
config.Load->expandVarsstrategy { },cache { },metadata { }os.Expandon*hcl.Stringand*hcl.HeredocloadGlobalConfig->hcl.UnmarshalASTwithhcl.WithDefaultTransformeropa { },log { },scheduler { },bind,state,url, ...default:strings viavalueFromTag; live*hcl.Heredocvalues reachunmarshalValueand callrv.SetString(v.GetHeredoc())with no transformerSo the comment on the OPA block (
"interpolated at startup via os.Expand") was true for non-heredoc fields elsewhere in the config, but silently false for the heredoc that needed it most.Fix
expandVarsasExpandVarsso it can be used by both decode paths.config.ExpandVars(ast, envars)fromloadGlobalConfigincmd/cachewd/main.goonce, before eitherhcl.UnmarshalASTpass. This in-place mutation makes the live AST consistent with whatLoadalready does for provider blocks, so subsequent unmarshal passes (and the OPA compiler) see the expanded values.TestExpandVarscovering both*hcl.Stringand*hcl.Heredocattribute values plus theos.Expand"unset -> empty string" case, so the heredoc path will not silently regress again.Verification
just lintpasses.just test— all 594 tests pass, including the newTestExpandVars.CACHEW_WARMER_PRINCIPALwas being denied even though the env var value matched what the policy text would compare against after substitution. With this change, OPA receives the substituted comparison and the same caller is allowed.Backwards compatibility
No HCL behaviour changes for configurations that do not use
${VAR}placeholders inside global heredocs — the placeholder simply was not being expanded before, so nothing could have relied on the previous behaviour.