feat: plumb user secrets through provisioner chain to terraform (#24542)

This change passes user secrets from coderd to the Terraform process at
workspace build time so the `data.coder_secret` data source in
terraform-provider-coder can resolve values at plan time.

Secrets traverse two proto hops: `provisionerdserver` fetches them
via`ListUserSecretsWithValues`, attaches them to
`AcquiredJob.WorkspaceBuild.user_secrets` on `provisionerd.proto`;
`runner.go` forwards into `PlanRequest.user_secrets` on
`provisioner.proto`; the Terraform provisioner encodes each as
`CODER_SECRET_ENV_<name>` or `CODER_SECRET_FILE_<hex(path)>` before
invoking `terraform plan`. Only plan requests carry secrets; apply runs
with `nil` because values are baked into plan state.

Fetch is gated on a workspace transitioning to start. stop and delete
transitions never carry secrets, so revoking or deleting a stored secret
cannot make a workspace unstoppable. DB errors on the fetch fail the job
outright rather than silently continuing with an empty secret set.

Note that user secrets will be stored in the workspace_builds table in
provisioner_state with other Terraform state (including other sensitive data).
This commit is contained in:
Zach
2026-04-27 08:26:07 -06:00
committed by GitHub
parent 66abd8a271
commit 79735f2d45
15 changed files with 1471 additions and 686 deletions
+17 -2
View File
@@ -2,6 +2,7 @@ package terraform
import (
"context"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
@@ -198,7 +199,7 @@ func (s *server) Plan(
}
}
env, err := provisionEnv(sess.Config, request.Metadata, request.PreviousParameterValues, request.RichParameterValues, request.ExternalAuthProviders)
env, err := provisionEnv(sess.Config, request.Metadata, request.PreviousParameterValues, request.RichParameterValues, request.ExternalAuthProviders, request.UserSecrets)
if err != nil {
return provisionersdk.PlanErrorf("setup env: %s", err)
}
@@ -311,7 +312,7 @@ func (s *server) Apply(
}
}
env, err := provisionEnv(sess.Config, request.Metadata, nil, nil, nil)
env, err := provisionEnv(sess.Config, request.Metadata, nil, nil, nil, nil)
if err != nil {
return provisionersdk.ApplyErrorf("provision env: %s", err)
}
@@ -347,6 +348,7 @@ func planVars(plan *proto.PlanRequest) ([]string, error) {
func provisionEnv(
config *proto.Config, metadata *proto.Metadata,
previousParams, richParams []*proto.RichParameterValue, externalAuth []*proto.ExternalAuthProvider,
userSecrets []*proto.UserSecretValue,
) ([]string, error) {
env := safeEnviron()
ownerGroups, err := json.Marshal(metadata.GetWorkspaceOwnerGroups())
@@ -415,6 +417,19 @@ func provisionEnv(
env = append(env, provider.ExternalAuthAccessTokenEnvironmentVariable(extAuth.Id)+"="+extAuth.AccessToken)
}
for _, secret := range userSecrets {
if secret.EnvName != "" {
env = append(env, fmt.Sprintf("CODER_SECRET_ENV_%s=%s", secret.EnvName, string(secret.Value)))
}
if secret.FilePath != "" {
// Environment variables are used to communicate the file path a
// secret should be written to. The hex encoding is done because
// file paths contain slashes, tildes, and dots that are illegal
// in environment variable names.
env = append(env, fmt.Sprintf("CODER_SECRET_FILE_%s=%s", hex.EncodeToString([]byte(secret.FilePath)), string(secret.Value)))
}
}
if config.ProvisionerLogLevel != "" {
// TF_LOG=JSON enables all kind of logging: trace-debug-info-warn-error.
// The idea behind using TF_LOG=JSON instead of TF_LOG=debug is ensuring the proper log format.
@@ -0,0 +1,119 @@
package terraform
import (
"encoding/hex"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/provisionersdk/proto"
)
func TestProvisionEnv_UserSecrets(t *testing.T) {
t.Parallel()
t.Run("EnvSecret", func(t *testing.T) {
t.Parallel()
secrets := []*proto.UserSecretValue{
{EnvName: "MY_TOKEN", Value: []byte("secret-value")},
}
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, secrets)
require.NoError(t, err)
want := "CODER_SECRET_ENV_MY_TOKEN=secret-value"
assert.Contains(t, env, want)
})
t.Run("FileSecret", func(t *testing.T) {
t.Parallel()
filePath := "~/.ssh/id_rsa"
secrets := []*proto.UserSecretValue{
{FilePath: filePath, Value: []byte("key-data")},
}
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, secrets)
require.NoError(t, err)
hexPath := hex.EncodeToString([]byte(filePath))
want := "CODER_SECRET_FILE_" + hexPath + "=key-data"
assert.Contains(t, env, want)
})
t.Run("BothEnvAndFile", func(t *testing.T) {
t.Parallel()
filePath := "/tmp/secret.txt"
secrets := []*proto.UserSecretValue{
{EnvName: "DUAL", FilePath: filePath, Value: []byte("both-value")},
}
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, secrets)
require.NoError(t, err)
wantEnv := "CODER_SECRET_ENV_DUAL=both-value"
hexPath := hex.EncodeToString([]byte(filePath))
wantFile := "CODER_SECRET_FILE_" + hexPath + "=both-value"
assert.Contains(t, env, wantEnv)
assert.Contains(t, env, wantFile)
})
t.Run("NilSecrets", func(t *testing.T) {
t.Parallel()
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, nil)
require.NoError(t, err)
for _, e := range env {
assert.False(t, strings.HasPrefix(e, "CODER_SECRET_"),
"unexpected secret env var: %s", e)
}
})
t.Run("EmptyEnvAndFile", func(t *testing.T) {
t.Parallel()
secrets := []*proto.UserSecretValue{
{EnvName: "", FilePath: "", Value: []byte("ignored")},
}
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, secrets)
require.NoError(t, err)
for _, e := range env {
assert.False(t, strings.HasPrefix(e, "CODER_SECRET_"),
"unexpected secret env var: %s", e)
}
})
}
// nolint:paralleltest // t.Setenv is incompatible with t.Parallel.
func TestProvisionEnv_HostSecretsStripped(t *testing.T) {
// Host CODER_* env vars must be stripped by safeEnviron before provisionEnv
// appends its own entries. If the order of operations in provisionEnv ever
// changes (e.g. appending before stripping, or adding a post-filter that
// drops CODER_*), this test catches it. The host var below would otherwise
// leak into the terraform environment and could be interpreted as a real
// secret.
t.Setenv("CODER_SECRET_ENV_PREEXISTING", "host-value")
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, nil)
require.NoError(t, err)
for _, e := range env {
assert.False(t, strings.HasPrefix(e, "CODER_SECRET_"),
"host CODER_SECRET_* var leaked into provisioner env: %s", e)
}
}
// nolint:paralleltest // t.Setenv is incompatible with t.Parallel.
func TestProvisionEnv_InputSecretsSurviveHostCollision(t *testing.T) {
// When the host has a CODER_SECRET_ENV_X var set and the caller also passes
// X in the secrets slice, the caller's value must win. This proves secrets
// are appended after safeEnviron strips the host's CODER_* vars, not before.
t.Setenv("CODER_SECRET_ENV_COLLIDE", "host-value-should-not-win")
secrets := []*proto.UserSecretValue{
{EnvName: "COLLIDE", Value: []byte("caller-value")},
}
env, err := provisionEnv(&proto.Config{}, &proto.Metadata{}, nil, nil, nil, secrets)
require.NoError(t, err)
assert.Contains(t, env, "CODER_SECRET_ENV_COLLIDE=caller-value",
"caller-supplied secret must be present")
assert.NotContains(t, env, "CODER_SECRET_ENV_COLLIDE=host-value-should-not-win",
"host value must be stripped before secrets are appended")
}