fix: address workspace build secrets listing review feedback

agent/agent_test.go

@@ -1806,6 +1806,19 @@ func TestAgent_CoderWorkspaceBuildIDEnvVar(t *testing.T) {
 	output, err := session.Output(command)
 	require.NoError(t, err)
 	require.Equal(t, buildID.String(), strings.TrimSpace(string(output)))
+
+	// A manifest from a server that predates the build ID leaves it unset
+	// rather than exporting the nil UUID.
+	t.Run("Unset", func(t *testing.T) {
+		t.Parallel()
+		if runtime.GOOS == "windows" {
+			t.Skip("uses POSIX parameter expansion")
+		}
+		session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
+		output, err := session.Output("sh -c 'echo ${CODER_WORKSPACE_BUILD_ID-unset}'")
+		require.NoError(t, err)
+		require.Equal(t, "unset", strings.TrimSpace(string(output)))
+	})
 }
 
 func TestAgent_SSHConnectionEnvVars(t *testing.T) {

cli/secret.go

@@ -493,9 +493,15 @@ type secretListRow struct {
 	File        string `json:"-" table:"file"`
 	Enabled     string `json:"-" table:"enabled"`
 	ReplacedBy  string `json:"-" table:"replaced by"`
+	Ephemeral   string `json:"-" table:"ephemeral"`
 	Description string `json:"-" table:"description"`
 }
 
+var (
+	secretListColumns      = []string{"name", "created", "updated", "env", "file", "enabled", "description"}
+	secretListBuildColumns = []string{"name", "source", "created", "updated", "env", "file", "enabled", "replaced by", "description"}
+)
+
 // secretListRowFromUserSecret builds a table row for a user secret.
 // replacedBy lists the build secrets that take its targets.
 func secretListRowFromUserSecret(secret codersdk.UserSecret, replacedBy []string) secretListRow {
@@ -509,6 +515,7 @@ func secretListRowFromUserSecret(secret codersdk.UserSecret, replacedBy []string
 		File:        secret.FilePath,
 		Enabled:     strconv.FormatBool(secret.Enabled),
 		ReplacedBy:  strings.Join(replacedBy, ", "),
+		Ephemeral:   "false",
 		Description: secret.Description,
 	}
 }
@@ -517,6 +524,7 @@ func secretListRowFromBuildSecret(secret codersdk.WorkspaceSecret) secretListRow
 	row := secretListRowFromUserSecret(secret.UserSecret, nil)
 	row.Secret = secret
 	row.Source = "build"
+	row.Ephemeral = strconv.FormatBool(secret.Ephemeral)
 	return row
 }
 
@@ -650,7 +658,7 @@ func secretListBuildID(inv *serpent.Invocation, client *codersdk.Client, workspa
 		}
 		ws, err := client.Workspace(inv.Context(), id)
 		if isNotFound(err) {
-			warnCurrentWorkspaceNotFound(inv)
+			warnCurrentWorkspaceNotFound(inv, client)
 			return uuid.Nil, false, nil
 		}
 		if err != nil {
@@ -661,11 +669,8 @@ func secretListBuildID(inv *serpent.Invocation, client *codersdk.Client, workspa
 	return uuid.Nil, false, nil
 }
 
-// warnCurrentWorkspaceNotFound explains why only user secrets are listed
-// when the CLI is logged in to a deployment or user that cannot see the
-// workspace it runs in.
-func warnCurrentWorkspaceNotFound(inv *serpent.Invocation) {
-	cliui.Warn(inv.Stderr, "The current workspace was not found on this deployment, so only user secrets are listed.",
+func warnCurrentWorkspaceNotFound(inv *serpent.Invocation, client *codersdk.Client) {
+	cliui.Warn(inv.Stderr, fmt.Sprintf("The current workspace is not visible to the logged-in user on %s, so only that user's secrets are listed.", client.URL),
 		"Use --workspace or --build-id to list a workspace's secrets.")
 }
 
@@ -675,37 +680,38 @@ func isNotFound(err error) bool {
 }
 
 // secretListBuildSecrets returns the secrets of the workspace build chosen by
-// secretListBuildID, or nil when only user secrets are listed. Secrets of
+// secretListBuildID. included is false when only user secrets are listed.
+// Secrets of
 // another user's build are never mixed into the logged-in user's listing: a
 // build from the workspace environment is skipped, and an explicit one fails.
-func secretListBuildSecrets(inv *serpent.Invocation, client *codersdk.Client, workspace, buildID string) ([]codersdk.WorkspaceSecret, error) {
+func secretListBuildSecrets(inv *serpent.Invocation, client *codersdk.Client, workspace, buildID string) (secrets []codersdk.WorkspaceSecret, included bool, err error) {
 	id, fromEnv, err := secretListBuildID(inv, client, workspace, buildID)
 	if err != nil || id == uuid.Nil {
-		return nil, err
+		return nil, false, err
 	}
 	build, err := client.WorkspaceBuild(inv.Context(), id)
 	if fromEnv && isNotFound(err) {
-		warnCurrentWorkspaceNotFound(inv)
-		return nil, nil
+		warnCurrentWorkspaceNotFound(inv, client)
+		return nil, false, nil
 	}
 	if err != nil {
-		return nil, xerrors.Errorf("get workspace build %s: %w", id, err)
+		return nil, false, xerrors.Errorf("get workspace build %s: %w", id, err)
 	}
 	me, err := client.User(inv.Context(), codersdk.Me)
 	if err != nil {
-		return nil, xerrors.Errorf("get logged-in user: %w", err)
+		return nil, false, xerrors.Errorf("get logged-in user: %w", err)
 	}
 	if build.WorkspaceOwnerID != me.ID {
 		if fromEnv {
-			return nil, nil
+			return nil, false, nil
 		}
-		return nil, xerrors.Errorf("workspace build %s belongs to %q, not the logged-in user %q", id, build.WorkspaceOwnerName, me.Username)
+		return nil, false, xerrors.Errorf("workspace build %s belongs to %q, not the logged-in user %q", id, build.WorkspaceOwnerName, me.Username)
 	}
-	secrets, err := client.WorkspaceBuildSecrets(inv.Context(), id)
+	secrets, err = client.WorkspaceBuildSecrets(inv.Context(), id)
 	if err != nil {
-		return nil, xerrors.Errorf("list workspace build secrets: %w", err)
+		return nil, false, xerrors.Errorf("list workspace build secrets: %w", err)
 	}
-	return secrets, nil
+	return secrets, true, nil
 }
 
 func (r *RootCmd) secretList() *serpent.Command {
@@ -714,7 +720,7 @@ func (r *RootCmd) secretList() *serpent.Command {
 		cliui.ChangeFormatterData(
 			cliui.TableFormat(
 				[]secretListRow{},
-				[]string{"name", "source", "created", "updated", "env", "file", "enabled", "replaced by", "description"},
+				secretListColumns,
 			),
 			func(data any) (any, error) {
 				switch rows := data.(type) {
@@ -752,8 +758,10 @@ func (r *RootCmd) secretList() *serpent.Command {
 		Short:   "List secrets, or show one by name",
 		Long: "Secret values are omitted from the output.\n\n" +
 			"When listing all secrets inside a workspace, or with --workspace or --build-id, " +
-			"the workspace build's secrets are included and the \"replaced by\" column shows " +
-			"user secrets whose env var or file is taken by a workspace secret.",
+			"the workspace build's secrets are included and the source and \"replaced by\" columns " +
+			"are shown by default. If a build secret uses the same env var " +
+			"or file as a user secret, the user secret is not delivered on that target and its " +
+			"\"replaced by\" column names the build secret, for example \"env: github-token\".",
 		Middleware: serpent.RequireRangeArgs(0, 1),
 		Handler: func(inv *serpent.Invocation) error {
 			client, err := r.InitClient(inv)
@@ -772,10 +780,21 @@ func (r *RootCmd) secretList() *serpent.Command {
 				}
 				data = secretListRowFromUserSecret(secret, nil)
 			} else {
-				buildSecrets, err := secretListBuildSecrets(inv, client, workspace, buildID)
+				buildSecrets, included, err := secretListBuildSecrets(inv, client, workspace, buildID)
 				if err != nil {
 					return err
 				}
+				if included && !inv.ParsedFlags().Changed("column") {
+					// Show where each secret comes from only when a build's
+					// secrets are listed.
+					if opt := inv.Command.Options.ByFlag("column"); opt != nil {
+						if columns, ok := opt.Value.(*serpent.EnumArray); ok {
+							if err := columns.Replace(secretListBuildColumns); err != nil {
+								return xerrors.Errorf("set columns: %w", err)
+							}
+						}
+					}
+				}
 				userSecrets, err := client.UserSecrets(inv.Context(), codersdk.Me)
 				if err != nil {
 					return xerrors.Errorf("list secrets: %w", err)
@@ -798,7 +817,7 @@ func (r *RootCmd) secretList() *serpent.Command {
 		Options: serpent.OptionSet{
 			{
 				Flag:        "workspace",
-				Description: "Include the secrets of this workspace's latest build. Defaults to the current workspace when run inside one.",
+				Description: "Include the secrets of this workspace's latest build. Inside a workspace, the default is the build the workspace agent was started from.",
 				Value:       serpent.StringOf(&workspace),
 			},
 			{

cli/secret_test.go

@@ -601,9 +601,38 @@ func TestSecretListWorkspaceBuild(t *testing.T) {
 			assert.Contains(t, out, "REPLACED BY")
 			assert.Contains(t, out, "build-token")
 			assert.Contains(t, out, "env: build-token")
+			assert.NotContains(t, out, "EPHEMERAL", "ephemeral is hidden by default")
 		})
 	}
 
+	// Without a build, the build-only columns are not shown.
+	t.Run("NoBuildColumns", func(t *testing.T) {
+		t.Parallel()
+
+		inv, root := clitest.New(t, "secret", "list")
+		output := clitest.Capture(inv)
+		clitest.SetupConfig(t, client, root)
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		require.NoError(t, inv.WithContext(ctx).Run())
+		assert.Contains(t, output.Stdout(), "user-token")
+		assert.NotContains(t, output.Stdout(), "SOURCE")
+		assert.NotContains(t, output.Stdout(), "REPLACED BY")
+	})
+
+	t.Run("EphemeralColumn", func(t *testing.T) {
+		t.Parallel()
+
+		inv, root := clitest.New(t, "secret", "list", "--build-id", workspace.LatestBuild.ID.String(), "-c", "name,source,ephemeral")
+		output := clitest.Capture(inv)
+		clitest.SetupConfig(t, client, root)
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		require.NoError(t, inv.WithContext(ctx).Run())
+		assert.Contains(t, output.Stdout(), "EPHEMERAL")
+		assert.NotContains(t, output.Stdout(), "REPLACED BY", "explicit columns are kept")
+	})
+
 	// Inside a workspace that the logged-in deployment or user cannot see,
 	// only user secrets are listed.
 	for _, envName := range []string{"CODER_WORKSPACE_BUILD_ID", "CODER_WORKSPACE_ID"} {
@@ -617,7 +646,7 @@ func TestSecretListWorkspaceBuild(t *testing.T) {
 
 			ctx := testutil.Context(t, testutil.WaitMedium)
 			require.NoError(t, inv.WithContext(ctx).Run())
-			assert.Contains(t, output.Stderr(), "only user secrets are listed")
+			assert.Contains(t, output.Stderr(), "only that user's secrets are listed")
 			assert.Contains(t, output.Stdout(), "user-token")
 			assert.NotContains(t, output.Stdout(), "build-token")
 		})

cli/testdata/coder_secret_list_--help.golden

@@ -10,24 +10,27 @@ USAGE:
   Secret values are omitted from the output.
   
   When listing all secrets inside a workspace, or with --workspace or
-  --build-id, the workspace build's secrets are included and the "replaced by"
-  column shows user secrets whose env var or file is taken by a workspace
-  secret.
+  --build-id, the workspace build's secrets are included and the source and
+  "replaced by" columns are shown by default. If a build secret uses the same
+  env var or file as a user secret, the user secret is not delivered on that
+  target and its "replaced by" column names the build secret, for example "env:
+  github-token".
 
 OPTIONS:
       --build-id string
           Include the secrets of this workspace build. Takes precedence over
           --workspace.
 
-  -c, --column [created|name|updated|source|env|file|enabled|replaced by|description] (default: name,source,created,updated,env,file,enabled,replaced by,description)
+  -c, --column [created|name|updated|source|env|file|enabled|replaced by|ephemeral|description] (default: name,created,updated,env,file,enabled,description)
           Columns to display in table output.
 
   -o, --output table|json (default: table)
           Output format.
 
       --workspace string
-          Include the secrets of this workspace's latest build. Defaults to the
-          current workspace when run inside one.
+          Include the secrets of this workspace's latest build. Inside a
+          workspace, the default is the build the workspace agent was started
+          from.
 
 ———
 Run `coder --help` for a list of global options.

coderd/agentapi/manifest.go

@@ -340,37 +340,34 @@ func dbAgentDevcontainersToProto(devcontainers []database.WorkspaceAgentDevconta
 // dbSecretsToProto builds the manifest secrets from the user's and the
 // agent build's secrets, delivering each on the targets Resolve assigns.
 func dbSecretsToProto(userSecrets []database.UserSecret, workspaceSecrets []database.WorkspaceSecret, policy workspacesecrets.FilePathPolicy) []*agentproto.WorkspaceSecret {
-	candidates := make([]workspacesecrets.Secret, 0, len(userSecrets)+len(workspaceSecrets))
+	user := make([]workspacesecrets.Secret, 0, len(userSecrets))
 	for _, s := range userSecrets {
-		candidates = append(candidates, workspacesecrets.Secret{
+		user = append(user, workspacesecrets.Secret{
 			ID:       s.ID,
-			Source:   workspacesecrets.SourceUser,
-			Name:     s.Name,
 			EnvName:  s.EnvName,
 			FilePath: s.FilePath,
 			Enabled:  s.Enabled,
 			Value:    s.Value,
 		})
 	}
+	build := make([]workspacesecrets.Secret, 0, len(workspaceSecrets))
 	for _, s := range workspaceSecrets {
 		// ListActiveWorkspaceSecrets only returns rows that still hold a
 		// value; the check guards against a cleared row slipping through.
 		if !s.Value.Valid {
 			continue
 		}
-		candidates = append(candidates, workspacesecrets.Secret{
+		build = append(build, workspacesecrets.Secret{
 			ID:       s.ID,
-			Source:   workspacesecrets.SourceBuild,
-			Name:     s.Name,
 			EnvName:  s.EnvName,
 			FilePath: s.FilePath,
-			Enabled:  true,
 			Value:    s.Value.String,
 		})
 	}
 
-	ret := make([]*agentproto.WorkspaceSecret, 0, len(candidates))
-	for _, r := range workspacesecrets.Resolve(candidates, policy) {
+	resolvedUser, resolvedBuild := workspacesecrets.Resolve(user, build, policy)
+	ret := make([]*agentproto.WorkspaceSecret, 0, len(user)+len(build))
+	for _, r := range append(resolvedUser, resolvedBuild...) {
 		if !r.Delivered() {
 			continue
 		}

coderd/agentapi/manifest_internal_test.go

@@ -22,6 +22,7 @@ func Test_dbSecretsToProto(t *testing.T) {
 		{Name: "file-only", FilePath: "~/.ssh/id_rsa", Value: "file-val", Enabled: true},
 		{Name: "dual", EnvName: "DUAL_ENV", FilePath: "/etc/dual", Value: "dual-val", Enabled: true},
 		{Name: "disabled", EnvName: "DISABLED_ENV", FilePath: "/etc/disabled", Value: "disabled-val"},
+		// ws-shared takes this secret's env var; its file is still written.
 		{Name: "overridden", EnvName: "SHARED_ENV", FilePath: "/etc/overridden", Value: "user-val", Enabled: true},
 	}
 	workspaceSecrets := []database.WorkspaceSecret{
@@ -31,8 +32,6 @@ func Test_dbSecretsToProto(t *testing.T) {
 		{Name: "ws-shared", EnvName: "SHARED_ENV", Value: sql.NullString{String: "ws-shared-val", Valid: true}},
 	}
 
-	// A workspace secret on the same env var replaces the user secret's env
-	// delivery; the user secret's file is still written.
 	cases := []struct {
 		name     string
 		policy   workspacesecrets.FilePathPolicy

coderd/database/dbmetrics/querymetrics.go

@@ -4192,9 +4192,9 @@ func (m queryMetricsStore) GetWorkspaceSecrets(ctx context.Context, arg database
 	return r0, r1
 }
 
-func (m queryMetricsStore) GetWorkspaceSecretsHistory(ctx context.Context, workspaceID database.GetWorkspaceSecretsHistoryParams) ([]database.GetWorkspaceSecretsHistoryRow, error) {
+func (m queryMetricsStore) GetWorkspaceSecretsHistory(ctx context.Context, arg database.GetWorkspaceSecretsHistoryParams) ([]database.GetWorkspaceSecretsHistoryRow, error) {
 	start := time.Now()
-	r0, r1 := m.s.GetWorkspaceSecretsHistory(ctx, workspaceID)
+	r0, r1 := m.s.GetWorkspaceSecretsHistory(ctx, arg)
 	m.queryLatencies.WithLabelValues("GetWorkspaceSecretsHistory").Observe(time.Since(start).Seconds())
 	m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetWorkspaceSecretsHistory").Inc()
 	return r0, r1

coderd/workspaces_test.go

@@ -7075,3 +7075,47 @@ func TestWorkspaceSecretsClearedOnCompletion(t *testing.T) {
 	require.Empty(t, live(build1.ID), "the previous build's secret is cleared once the new build completes")
 	require.Len(t, live(build2.ID), 1)
 }
+
+func TestWorkspaceBuildSecretsFilePathBlocked(t *testing.T) {
+	t.Parallel()
+
+	dv := coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) {
+		dv.DisableUserSecretFilePath = true
+	})
+	db, ps := dbtestutil.NewDB(t)
+	client := coderdtest.New(t, &coderdtest.Options{Database: db, Pubsub: ps, DeploymentValues: dv})
+	owner := coderdtest.CreateFirstUser(t, client)
+	ctx := testutil.Context(t, testutil.WaitLong)
+
+	// The API rejects file paths under this policy, so seed rows that were
+	// written before it was enabled.
+	ws := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
+		OrganizationID: owner.OrganizationID,
+		OwnerID:        owner.UserID,
+	}).Do()
+	userSecret := dbgen.UserSecret(t, db, database.UserSecret{
+		UserID: owner.UserID, EnvName: "TOKEN", FilePath: "~/.token",
+	})
+	buildEnv := dbgen.WorkspaceSecret(t, db, database.WorkspaceSecret{
+		WorkspaceID: ws.Workspace.ID, WorkspaceBuildID: ws.Build.ID, Name: "build-env", EnvName: "TOKEN",
+	})
+	buildFile := dbgen.WorkspaceSecret(t, db, database.WorkspaceSecret{
+		WorkspaceID: ws.Workspace.ID, WorkspaceBuildID: ws.Build.ID, Name: "build-file", EnvName: "OTHER", FilePath: "~/.token",
+	})
+
+	secrets, err := client.WorkspaceBuildSecrets(ctx, ws.Build.ID)
+	require.NoError(t, err)
+	byID := make(map[uuid.UUID]codersdk.WorkspaceSecret, len(secrets))
+	for _, s := range secrets {
+		byID[s.ID] = s
+	}
+	require.Len(t, byID, 2)
+	require.Equal(t, &userSecret.ID, byID[buildEnv.ID].EnvReplaces, "env targets are still delivered")
+	require.Nil(t, byID[buildFile.ID].FileReplaces, "no file is delivered, so none is replaced")
+
+	// A build ID that is not a UUID is rejected.
+	res, err := client.Request(ctx, http.MethodGet, "/api/v2/workspacebuilds/not-a-uuid/secrets", nil)
+	require.NoError(t, err)
+	defer res.Body.Close()
+	require.Equal(t, http.StatusBadRequest, res.StatusCode)
+}

coderd/workspacesecrets.go

@@ -136,36 +136,30 @@ func (api *API) applySecretReplacements(secrets []codersdk.WorkspaceSecret, user
 	if api.userSecretFilePathBlocked() {
 		policy = workspacesecrets.FilePathBlocked
 	}
-	candidates := make([]workspacesecrets.Secret, 0, len(secrets)+len(userSecrets))
+	user := make([]workspacesecrets.Secret, 0, len(userSecrets))
 	for _, s := range userSecrets {
-		candidates = append(candidates, workspacesecrets.Secret{
+		user = append(user, workspacesecrets.Secret{
 			ID:       s.ID,
-			Source:   workspacesecrets.SourceUser,
-			Name:     s.Name,
 			EnvName:  s.EnvName,
 			FilePath: s.FilePath,
 			Enabled:  s.Enabled,
 		})
 	}
+	build := make([]workspacesecrets.Secret, 0, len(secrets))
 	byID := make(map[uuid.UUID]*codersdk.WorkspaceSecret, len(secrets))
 	for i, s := range secrets {
 		if s.ClearedAt != nil {
 			continue
 		}
 		byID[s.ID] = &secrets[i]
-		candidates = append(candidates, workspacesecrets.Secret{
+		build = append(build, workspacesecrets.Secret{
 			ID:       s.ID,
-			Source:   workspacesecrets.SourceBuild,
-			Name:     s.Name,
 			EnvName:  s.EnvName,
 			FilePath: s.FilePath,
-			Enabled:  true,
 		})
 	}
-	for _, r := range workspacesecrets.Resolve(candidates, policy) {
-		if r.Source != workspacesecrets.SourceUser {
-			continue
-		}
+	resolvedUser, _ := workspacesecrets.Resolve(user, build, policy)
+	for _, r := range resolvedUser {
 		if s, ok := byID[r.EnvReplacedBy.UUID]; r.EnvReplacedBy.Valid && ok {
 			s.EnvReplaces = &r.ID
 		}

coderd/workspacesecrets/workspacesecrets.go

@@ -1,9 +1,11 @@
 // Package workspacesecrets decides which secrets are delivered to a
-// workspace and on which targets. The agent manifest and the secrets API
-// both use Resolve so they always agree.
+// workspace and on which targets. The agent manifest and the workspace build
+// secrets API both use Resolve so they always agree.
 package workspacesecrets
 
 import (
+	"path"
+
 	"github.com/google/uuid"
 )
 
@@ -11,30 +13,22 @@ import (
 type FilePathPolicy int
 
 const (
+	// FilePathAllowed delivers file_path targets.
 	FilePathAllowed FilePathPolicy = iota
+	// FilePathBlocked drops every secret's file_path target, as set by the
+	// deployment's user secret file path option. Env targets are still
+	// delivered.
 	FilePathBlocked
 )
 
-// Source identifies where a secret delivered to a workspace comes from.
-type Source int
-
-const (
-	// SourceUser is a user secret, shared by all of the user's workspaces.
-	SourceUser Source = iota
-	// SourceBuild is a workspace secret set on a workspace build.
-	SourceBuild
-)
-
 // Secret is a candidate for delivery to a workspace. Value may be empty when
 // only metadata is needed.
 type Secret struct {
 	ID       uuid.UUID
-	Source   Source
-	Name     string
 	EnvName  string
 	FilePath string
-	// Enabled is false for disabled user secrets. Workspace secrets are
-	// always enabled.
+	// Enabled is false for disabled user secrets. It is ignored for build
+	// secrets, which cannot be disabled.
 	Enabled bool
 	Value   string
 }
@@ -47,8 +41,8 @@ type Resolved struct {
 	// target is unset, blocked by policy, or taken by another secret.
 	DeliveredEnvName  string
 	DeliveredFilePath string
-	// EnvReplacedBy and FileReplacedBy name the secret delivered on this
-	// secret's target instead of it.
+	// EnvReplacedBy and FileReplacedBy hold the ID of the secret delivered
+	// on this secret's target instead of it.
 	EnvReplacedBy  uuid.NullUUID
 	FileReplacedBy uuid.NullUUID
 }
@@ -58,43 +52,53 @@ func (r Resolved) Delivered() bool {
 	return r.DeliveredEnvName != "" || r.DeliveredFilePath != ""
 }
 
-// Resolve returns the delivery outcome of each secret, in input order.
+// Resolve returns the delivery outcome of the user secrets and the workspace
+// build secrets, each in input order.
 //
-// Disabled secrets are never delivered and never replace or get replaced.
-// When secrets share an env_name or file_path, a workspace build secret wins
-// over a user secret. Targets are compared exactly as written.
-func Resolve(secrets []Secret, policy FilePathPolicy) []Resolved {
-	out := make([]Resolved, len(secrets))
+// Build secrets win over user secrets that share an env_name or file_path.
+// Disabled user secrets are never delivered and never replace or get
+// replaced. Env names are compared exactly. File paths are compared after
+// path.Clean, so "~/a//b" and "~/a/b" collide, but "~/a" and "/home/<user>/a"
+// cannot be matched on the server and do not.
+func Resolve(user, build []Secret, policy FilePathPolicy) (resolvedUser, resolvedBuild []Resolved) {
 	envOwner := map[string]uuid.UUID{}
 	fileOwner := map[string]uuid.UUID{}
-
-	// Claim targets in precedence order: build secrets first.
-	for _, pass := range []Source{SourceBuild, SourceUser} {
-		for i, s := range secrets {
-			if s.Source != pass {
-				continue
-			}
-			r := Resolved{Secret: s}
-			if s.Enabled {
-				r.DeliveredEnvName, r.EnvReplacedBy = claim(envOwner, s.EnvName, s.ID)
-				if policy == FilePathAllowed {
-					r.DeliveredFilePath, r.FileReplacedBy = claim(fileOwner, s.FilePath, s.ID)
-				}
-			}
-			out[i] = r
+	resolve := func(s Secret, enabled bool) Resolved {
+		r := Resolved{Secret: s}
+		if !enabled {
+			return r
 		}
+		if claim(envOwner, s.EnvName, s.ID, &r.EnvReplacedBy) {
+			r.DeliveredEnvName = s.EnvName
+		}
+		if policy == FilePathAllowed && s.FilePath != "" && claim(fileOwner, path.Clean(s.FilePath), s.ID, &r.FileReplacedBy) {
+			r.DeliveredFilePath = s.FilePath
+		}
+		return r
 	}
-	return out
+
+	// Build secrets claim targets first, so they take precedence.
+	resolvedBuild = make([]Resolved, len(build))
+	for i, s := range build {
+		resolvedBuild[i] = resolve(s, true)
+	}
+	resolvedUser = make([]Resolved, len(user))
+	for i, s := range user {
+		resolvedUser[i] = resolve(s, s.Enabled)
+	}
+	return resolvedUser, resolvedBuild
 }
 
-// claim assigns target to id unless another secret already holds it.
-func claim(owners map[string]uuid.UUID, target string, id uuid.UUID) (string, uuid.NullUUID) {
-	if target == "" {
-		return "", uuid.NullUUID{}
+// claim assigns key to id unless another secret already holds it, in which
+// case that secret's ID is stored in replacedBy.
+func claim(owners map[string]uuid.UUID, key string, id uuid.UUID, replacedBy *uuid.NullUUID) bool {
+	if key == "" {
+		return false
 	}
-	if owner, taken := owners[target]; taken {
-		return "", uuid.NullUUID{UUID: owner, Valid: true}
+	if owner, taken := owners[key]; taken {
+		*replacedBy = uuid.NullUUID{UUID: owner, Valid: true}
+		return false
 	}
-	owners[target] = id
-	return target, uuid.NullUUID{}
+	owners[key] = id
+	return true
 }

coderd/workspacesecrets/workspacesecrets_test.go

@@ -12,59 +12,59 @@ import (
 func TestResolve(t *testing.T) {
 	t.Parallel()
 
-	user := func(name, env, file string, enabled bool) workspacesecrets.Secret {
-		return workspacesecrets.Secret{ID: uuid.New(), Source: workspacesecrets.SourceUser, Name: name, EnvName: env, FilePath: file, Enabled: enabled}
-	}
-	build := func(name, env, file string) workspacesecrets.Secret {
-		return workspacesecrets.Secret{ID: uuid.New(), Source: workspacesecrets.SourceBuild, Name: name, EnvName: env, FilePath: file, Enabled: true}
+	secret := func(env, file string, enabled bool) workspacesecrets.Secret {
+		return workspacesecrets.Secret{ID: uuid.New(), EnvName: env, FilePath: file, Enabled: enabled}
 	}
 
 	var (
-		userBoth     = user("both", "TOKEN", "/etc/token", true)
-		userFileOnly = user("file-only", "", "/etc/cert", true)
-		userDisabled = user("disabled", "DISABLED", "", false)
-		userPlain    = user("plain", "PLAIN", "", true)
-		buildEnv     = build("build-env", "TOKEN", "")
-		buildFile    = build("build-file", "", "/etc/cert")
-		buildOnDis   = build("build-on-disabled", "DISABLED", "")
+		userBoth     = secret("TOKEN", "/etc/token", true)
+		userFileOnly = secret("", "~/a/cert", true)
+		userDisabled = secret("DISABLED", "", false)
+		userPlain    = secret("PLAIN", "", true)
+		buildEnv     = secret("TOKEN", "", true)
+		// Written differently from userFileOnly's path, but the same file.
+		buildFile = secret("", "~/a//cert", true)
+		// Enabled is ignored for build secrets.
+		buildOnDis = secret("DISABLED", "", false)
 	)
-	// Build secrets listed last still win, so input order does not matter.
-	secrets := []workspacesecrets.Secret{userBoth, userFileOnly, userDisabled, userPlain, buildEnv, buildFile, buildOnDis}
+	user := []workspacesecrets.Secret{userBoth, userFileOnly, userDisabled, userPlain}
+	build := []workspacesecrets.Secret{buildEnv, buildFile, buildOnDis}
 
 	t.Run("FilePathAllowed", func(t *testing.T) {
 		t.Parallel()
-		got := workspacesecrets.Resolve(secrets, workspacesecrets.FilePathAllowed)
-		require.Len(t, got, len(secrets))
+		gotUser, gotBuild := workspacesecrets.Resolve(user, build, workspacesecrets.FilePathAllowed)
+		require.Len(t, gotUser, len(user))
+		require.Len(t, gotBuild, len(build))
 
 		// Partial override: the env var is taken, the file is still delivered.
-		require.Empty(t, got[0].DeliveredEnvName)
-		require.Equal(t, uuid.NullUUID{UUID: buildEnv.ID, Valid: true}, got[0].EnvReplacedBy)
-		require.Equal(t, "/etc/token", got[0].DeliveredFilePath)
-		require.False(t, got[0].FileReplacedBy.Valid)
-		require.True(t, got[0].Delivered())
+		require.Empty(t, gotUser[0].DeliveredEnvName)
+		require.Equal(t, uuid.NullUUID{UUID: buildEnv.ID, Valid: true}, gotUser[0].EnvReplacedBy)
+		require.Equal(t, "/etc/token", gotUser[0].DeliveredFilePath)
+		require.False(t, gotUser[0].FileReplacedBy.Valid)
+		require.True(t, gotUser[0].Delivered())
 
-		// Full override.
-		require.False(t, got[1].Delivered())
-		require.Equal(t, uuid.NullUUID{UUID: buildFile.ID, Valid: true}, got[1].FileReplacedBy)
+		// Full override, matched on the cleaned path.
+		require.False(t, gotUser[1].Delivered())
+		require.Equal(t, uuid.NullUUID{UUID: buildFile.ID, Valid: true}, gotUser[1].FileReplacedBy)
 
-		// Disabled secrets are neither delivered nor replaced.
-		require.False(t, got[2].Delivered())
-		require.False(t, got[2].EnvReplacedBy.Valid)
+		// Disabled user secrets are neither delivered nor replaced.
+		require.False(t, gotUser[2].Delivered())
+		require.False(t, gotUser[2].EnvReplacedBy.Valid)
 
-		require.Equal(t, "PLAIN", got[3].DeliveredEnvName)
-		require.Equal(t, "TOKEN", got[4].DeliveredEnvName)
-		require.Equal(t, "/etc/cert", got[5].DeliveredFilePath)
-		require.Equal(t, "DISABLED", got[6].DeliveredEnvName)
+		require.Equal(t, "PLAIN", gotUser[3].DeliveredEnvName)
+		require.Equal(t, "TOKEN", gotBuild[0].DeliveredEnvName)
+		require.Equal(t, "~/a//cert", gotBuild[1].DeliveredFilePath, "the path is delivered as written")
+		require.Equal(t, "DISABLED", gotBuild[2].DeliveredEnvName)
 	})
 
 	t.Run("FilePathBlocked", func(t *testing.T) {
 		t.Parallel()
-		got := workspacesecrets.Resolve(secrets, workspacesecrets.FilePathBlocked)
+		gotUser, gotBuild := workspacesecrets.Resolve(user, build, workspacesecrets.FilePathBlocked)
 
 		// No file is delivered, so no file target is reported as replaced.
-		require.False(t, got[0].Delivered())
-		require.False(t, got[1].Delivered())
-		require.False(t, got[1].FileReplacedBy.Valid)
-		require.False(t, got[5].Delivered())
+		require.False(t, gotUser[0].Delivered())
+		require.False(t, gotUser[1].Delivered())
+		require.False(t, gotUser[1].FileReplacedBy.Valid)
+		require.False(t, gotBuild[1].Delivered())
 	})
 }

docs/reference/cli/secret/list.md

@@ -23,7 +23,7 @@ coder secret list [flags] [name]
 ```console
 Secret values are omitted from the output.
 
-When listing all secrets inside a workspace, or with --workspace or --build-id, the workspace build's secrets are included and the "replaced by" column shows user secrets whose env var or file is taken by a workspace secret.
+When listing all secrets inside a workspace, or with --workspace or --build-id, the workspace build's secrets are included and the source and "replaced by" columns are shown by default. If a build secret uses the same env var or file as a user secret, the user secret is not delivered on that target and its "replaced by" column names the build secret, for example "env: github-token".
 ```
 
 ## Options
@@ -34,7 +34,7 @@ When listing all secrets inside a workspace, or with --workspace or --build-id,
 |------|---------------------|
 | Type | <code>string</code> |
 
-Include the secrets of this workspace's latest build. Defaults to the current workspace when run inside one.
+Include the secrets of this workspace's latest build. Inside a workspace, the default is the build the workspace agent was started from.
 
 ### --build-id
 
@@ -46,10 +46,10 @@ Include the secrets of this workspace build. Takes precedence over --workspace.
 
 ### -c, --column
 
-|         |                                                                                             |
-|---------|---------------------------------------------------------------------------------------------|
-| Type    | <code>[created\|name\|updated\|source\|env\|file\|enabled\|replaced by\|description]</code> |
-| Default | <code>name,source,created,updated,env,file,enabled,replaced by,description</code>           |
+|         |                                                                                                        |
+|---------|--------------------------------------------------------------------------------------------------------|
+| Type    | <code>[created\|name\|updated\|source\|env\|file\|enabled\|replaced by\|ephemeral\|description]</code> |
+| Default | <code>name,created,updated,env,file,enabled,description</code>                                         |
 
 Columns to display in table output.