Skip to content

Commit

Permalink
eval: fix showSecrets in CheckEnvironment (#364)
Browse files Browse the repository at this point in the history
eval: fix showSecrets in CheckEnvironment

These changes fix a bug in `CheckEnvironment` when `showSecrets` is
`false`. In this case, if the environment loader supplied a decrypter
for an imported environment, the imported environment's secrets would be
decrypted, violating the `showSecrets` contract. In order to address
this issue, these changes store and propagate the value of `showSecrets`
in the evaluator's context.
  • Loading branch information
pgavlin authored Jul 25, 2024
1 parent c61d9d3 commit b345967
Show file tree
Hide file tree
Showing 4 changed files with 671 additions and 8 deletions.
17 changes: 9 additions & 8 deletions eval/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func EvalEnvironment(
environments EnvironmentLoader,
execContext *esc.ExecContext,
) (*esc.Environment, syntax.Diagnostics) {
return evalEnvironment(ctx, false, name, env, decrypter, providers, environments, execContext)
return evalEnvironment(ctx, false, name, env, decrypter, providers, environments, execContext, true)
}

// CheckEnvironment symbolically evaluates the given environment. Calls to fn::open are not invoked, and instead
Expand All @@ -94,10 +94,7 @@ func CheckEnvironment(
execContext *esc.ExecContext,
showSecrets bool,
) (*esc.Environment, syntax.Diagnostics) {
if !showSecrets {
decrypter = nil
}
return evalEnvironment(ctx, true, name, env, decrypter, providers, environments, execContext)
return evalEnvironment(ctx, true, name, env, decrypter, providers, environments, execContext, showSecrets)
}

// evalEnvironment evaluates an environment and exports the result of evaluation.
Expand All @@ -110,12 +107,13 @@ func evalEnvironment(
providers ProviderLoader,
envs EnvironmentLoader,
execContext *esc.ExecContext,
showSecrets bool,
) (*esc.Environment, syntax.Diagnostics) {
if env == nil || (len(env.Values.GetEntries()) == 0 && len(env.Imports.GetElements()) == 0) {
return nil, nil
}

ec := newEvalContext(ctx, validating, name, env, decrypter, providers, envs, map[string]*imported{}, execContext)
ec := newEvalContext(ctx, validating, name, env, decrypter, providers, envs, map[string]*imported{}, execContext, showSecrets)
v, diags := ec.evaluate()

s := schema.Never().Schema()
Expand Down Expand Up @@ -149,6 +147,7 @@ type imported struct {
type evalContext struct {
ctx context.Context // the cancellation context for evaluation
validating bool // true if we are only checking the environment
showSecrets bool // true if secrets should be decrypted during validation
name string // the name of the environment
env *ast.EnvironmentDecl // the root of the environment AST
decrypter Decrypter // the decrypter to use for the environment
Expand All @@ -175,10 +174,12 @@ func newEvalContext(
environments EnvironmentLoader,
imports map[string]*imported,
execContext *esc.ExecContext,
showSecrets bool,
) *evalContext {
return &evalContext{
ctx: ctx,
validating: validating,
showSecrets: showSecrets,
name: name,
env: env,
decrypter: decrypter,
Expand All @@ -191,7 +192,7 @@ func newEvalContext(

// decryptSecrets returns true if static secrets should be decrypted.
func (e *evalContext) decryptSecrets() bool {
return !e.validating || e.decrypter != nil
return !e.validating || e.showSecrets
}

// error records an evaluation error associated with an expression.
Expand Down Expand Up @@ -457,7 +458,7 @@ func (e *evalContext) evaluateImport(myImports map[string]*value, decl *ast.Impo
return
}

imp := newEvalContext(e.ctx, e.validating, name, env, dec, e.providers, e.environments, e.imports, e.execContext)
imp := newEvalContext(e.ctx, e.validating, name, env, dec, e.providers, e.environments, e.imports, e.execContext, e.showSecrets)
v, diags := imp.evaluate()
e.diags.Extend(diags...)

Expand Down
6 changes: 6 additions & 0 deletions eval/testdata/eval/ciphertext-import/env.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
imports:
- test-base
values:
password:
fn::secret:
ciphertext: ZXNjeAAAAAHo9e705fKyKo30VQ==
Loading

0 comments on commit b345967

Please sign in to comment.