Skip to content

Securely flow developer secrets into direct-module recipe parameters#12249

Closed
willdavsmith wants to merge 3 commits into
mainfrom
willdavsmith-recipe-secret-injection
Closed

Securely flow developer secrets into direct-module recipe parameters#12249
willdavsmith wants to merge 3 commits into
mainfrom
willdavsmith-recipe-secret-injection

Conversation

@willdavsmith

@willdavsmith willdavsmith commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Description

Implements Phase 1 of #12244: securely flow developer-authored secrets into the parameters of direct-module recipes. Secret material is loaded for secret-typed connections, resolved into recipe parameters only as whole values, tagged as secure, and routed through each driver's native secret-handling mechanism.

Changes by layer

  • Types (pkg/recipes/types.go): adds a tainted Secrets map[string]string (json:"-") lane to recipes.ConnectedResource, so secret material is never serialized alongside regular connection data.
  • Engine (pkg/recipes/engine): enrichConnectionSecrets loads secret material for secret-typed connections (Applications.Core/secretStores, Radius.Security/secrets) using the engine's existing SecretsLoader. Loading is best-effort and non-fatal.
  • Param resolver (pkg/recipes/paramresolver): ResolveParameterExpressions now returns ResolvedParameters{Values, SecureKeys}, error. It resolves context.resource.connections.<name>.secrets.<key> from a separate secret lookup, enforces whole-value-only usage (a secret interpolated into a surrounding string is rejected with an error), and records secret-sourced parameters in SecureKeys.
  • Bicep driver (pkg/recipes/driver/bicep): passes the resolved values to ARM, relying on @secure() scrubbing; Radius never logs the parameter map.
  • Terraform driver (pkg/recipes/terraform): routes SecureKeys through a sensitive = true Terraform variable referenced as ${var.<name>}.

This change is rebased onto main's recursive nested-ternary implementation (#12239) and preserves all of its ternary helpers.

Testing

  • Resolver secret cases (Test_SecretExpressions): whole-value resolution + secure tagging, surrounding-whitespace handling, nested-object tagging, non-secret params untagged, interpolation-into-a-string rejection, and missing-key passthrough.
  • Engine enrichment cases (Test_enrichConnectionSecrets): secrets populated for secret-typed connections only, non-fatal load error, and nil-loader no-op.
  • Terraform Test_SetModuleParams: secure params routed through sensitive variables while others are set inline, plus an unknown-module no-op.
  • Existing recursive nested-ternary resolver tests (Test_TernaryExpressions) continue to pass.

Verified locally: gofmt -l (clean), go build ./..., go vet ./pkg/recipes/..., and go test ./pkg/recipes/... all pass.

Out of scope / hard to test

  • End-to-end secret flow into a live AVM @secure() / Terraform sensitive parameter needs a real secret store plus a cloud deploy; existing direct-module cloud tests are already manual/deferred.
  • ARM @secure() and Terraform sensitive redaction are provider behaviors and aren't assertable in unit tests.
  • Cleartext in the temporary main.tf.json working directory is inherent to Phase 1; Phase 2 (reference passthrough) avoids it.

Addresses #12244 (Phase 2 reference-passthrough remains open, so this does not close the issue).


Note: A second commit explored the secretName / Radius.Security/secrets reference path (context.resource.secrets.<key>) but has been reverted in this PR — the #12244 owner parked that path pending a scope decision (Phase 1 vs deferred Phase 2 reference passthrough). The reverted work remains recoverable in this branch's history. This PR's effective change is the connection-based Phase 1 deliverable described above.

Implements Phase 1 of #12244: securely flow developer-authored secrets into direct-module recipe parameters. The engine enriches secret-typed connections with secret material, the param resolver resolves whole-value secret references and tags them, and the Bicep and Terraform drivers route secret values through @secure() / sensitive variables.

Docs are a planned follow-up.

Signed-off-by: willdavsmith <[email protected]>
Copilot AI review requested due to automatic review settings June 25, 2026 16:16
@willdavsmith willdavsmith requested review from a team as code owners June 25, 2026 16:16
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements Phase 1 of #12244 by introducing an in-memory (non-serialized) “secret lane” for connected resources, enabling {{context.resource.connections.<name>.secrets.<key>}} expressions to resolve developer-authored secret values into direct-module recipe parameters, and propagating “secure” taint metadata to drivers so Terraform can route those values via sensitive = true variables.

Changes:

  • Add ConnectedResource.Secrets (json:"-") and enrich secret-typed connections in the recipe engine via the existing SecretsLoader.
  • Extend the parameter resolver to (a) resolve secret expressions from a separate secret lookup, (b) reject secret interpolation into surrounding strings, and (c) return {Values, SecureKeys} plus an error.
  • Update Terraform direct-module execution to use SecureKeys and emit sensitive variables, while Bicep continues passing resolved parameters through ARM.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/recipes/types.go Adds a non-serialized Secrets map to ConnectedResource for secret material.
pkg/recipes/engine/engine.go Best-effort enrichment of secret-typed connections via SecretsLoader.
pkg/recipes/engine/engine_test.go Tests secret enrichment behavior (typed-only, non-fatal errors, nil-loader).
pkg/recipes/paramresolver/resolver.go Returns (ResolvedParameters, error) and adds secret resolution + interpolation rejection.
pkg/recipes/paramresolver/resolver_test.go Adds Test_SecretExpressions and updates existing tests for new signature.
pkg/recipes/driver/bicep/bicep.go Updates for new resolver signature; passes resolved values into ARM parameters.
pkg/recipes/terraform/execute.go Uses resolver {Values, SecureKeys} and calls tfConfig.SetModuleParams(...).
pkg/recipes/terraform/config/types.go Adds TerraformConfig.Variable for declaring sensitive input variables.
pkg/recipes/terraform/config/config.go Implements SetModuleParams to route secure params via sensitive Terraform variables.
pkg/recipes/terraform/config/config_test.go Adds unit tests for SetModuleParams secure/non-secure routing.

Comment on lines +223 to +227
cfg.Variable[varName] = map[string]any{
"type": "string",
"sensitive": true,
"default": v,
}
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Unit Tests

    2 files  ± 0    450 suites  ±0   7m 42s ⏱️ +5s
5 605 tests +14  5 603 ✅ +14  2 💤 ±0  0 ❌ ±0 
6 802 runs  +14  6 800 ✅ +14  2 💤 ±0  0 ❌ ±0 

Results for commit db64e90. ± Comparison against base commit 98f5623.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.85047% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.95%. Comparing base (98f5623) to head (db64e90).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/recipes/terraform/execute.go 0.00% 5 Missing ⚠️
pkg/recipes/driver/bicep/bicep.go 0.00% 4 Missing ⚠️
pkg/recipes/paramresolver/resolver.go 92.45% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12249      +/-   ##
==========================================
+ Coverage   52.88%   52.95%   +0.06%     
==========================================
  Files         751      751              
  Lines       48353    48440      +87     
==========================================
+ Hits        25570    25649      +79     
- Misses      20385    20391       +6     
- Partials     2398     2400       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@willdavsmith willdavsmith marked this pull request as draft June 25, 2026 16:53
Extends Phase 1 of #12244 so a developer-authored Radius.Security/secrets
resource referenced by a resource property (x-radius-secret-reference, e.g.
secretName) flows its real cleartext into direct-module recipe parameters via
the context.resource.secrets.<key> expression, tagged for secure routing
(ARM @secure() / Terraform sensitive = true).

Because a secret's sensitive data is redacted from the database before recipe
execution, the new dynamic-RP secrets loader reads cleartext on demand from the
secret's deployed Kubernetes Secret via status.outputResources (never the
redacted Properties.Data), scoped to Kubernetes-backed secrets. Enrichment is
fail-closed: a referenced secret that cannot be loaded fails the deployment
rather than passing a literal placeholder to the module.

Docs are a planned follow-up.

Signed-off-by: willdavsmith <[email protected]>
…ters"

This reverts commit 3876f0a.

The #12244 owner has parked the secretName / Radius.Security/secrets path
pending reconciliation of its scope (Phase 1 vs deferred Phase 2 reference
passthrough, the latter potentially needing security-owner sign-off). This
restores PR #12249 to the merge-ready connection-based Phase 1 deliverable.
The reverted work remains in history at 3876f0a and can be re-applied when
the owner picks it back up.

Signed-off-by: willdavsmith <[email protected]>
@radius-functional-tests

radius-functional-tests Bot commented Jun 25, 2026

Copy link
Copy Markdown

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details
Name Value
Repository radius-project/radius
Commit ref db64e90
Unique ID funcac933fd605
Image tag pr-funcac933fd605
  • KinD: v0.29.0
  • Dapr: 1.14.4
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.3.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funcac933fd605
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funcac933fd605
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-funcac933fd605
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-funcac933fd605
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funcac933fd605
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
❌ Test recipe publishing failed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment on lines +223 to +227
cfg.Variable[varName] = map[string]any{
"type": "string",
"sensitive": true,
"default": v,
}
Comment on lines +789 to +793
t.Run("routes secure params through sensitive variables and sets others inline", func(t *testing.T) {
tfconfig, err := New(context.Background(), testRecipeName, &envRecipe, &resourceRecipe)
require.NoError(t, err)

params := RecipeParams{
@willdavsmith

Copy link
Copy Markdown
Contributor Author

Superseded by #12286, which consolidates this connection-based secrets work, the #12254 secret-binding references, and the new retain-encrypted-at-rest changes into a single draft PR against main. The branch history is preserved. Closing in favor of #12286.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants