From e0be9bf213376633ff254b30cbb7b582c7f551a4 Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Wed, 13 May 2026 13:20:02 -0700 Subject: [PATCH] feat: surface missing coder_secret requirements on resolve-autostart (#25081) Adds `dynamicparameters.EvaluateSecretMismatch` as a shared helper on top of the existing renderer, then wires it into the resolve-autostart handler so the UI can surface unsatisfied `coder_secret` requirements in a template alongside parameter mismatch for autostart. The lifecycle executor changes will land in a follow-up that depend on this helper. The UI changes that consume the new `secret_mismatch` field is also a follow-up. Generated with assistance from Coder Agents. --- coderd/apidoc/docs.go | 4 + coderd/apidoc/swagger.json | 4 + coderd/dynamicparameters/secrets.go | 100 +++++++++ .../secrets_internal_test.go | 112 ++++++++++ coderd/parameters_test.go | 192 ++++++++++++++++++ coderd/workspaces.go | 31 +++ codersdk/workspaces.go | 4 + docs/reference/api/schemas.md | 10 +- docs/reference/api/workspaces.md | 3 +- site/src/api/typesGenerated.ts | 6 + 10 files changed, 461 insertions(+), 5 deletions(-) create mode 100644 coderd/dynamicparameters/secrets.go create mode 100644 coderd/dynamicparameters/secrets_internal_test.go diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b4b2a60160..b0a0f2b45e 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -21557,6 +21557,10 @@ const docTemplate = `{ "properties": { "parameter_mismatch": { "type": "boolean" + }, + "secret_mismatch": { + "description": "SecretMismatch is true when the active template version declares\n` + "`" + `coder_secret` + "`" + ` requirements that the workspace owner's secrets do not\nsatisfy.", + "type": "boolean" } } }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a77910edd7..9dbe821356 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -19775,6 +19775,10 @@ "properties": { "parameter_mismatch": { "type": "boolean" + }, + "secret_mismatch": { + "description": "SecretMismatch is true when the active template version declares\n`coder_secret` requirements that the workspace owner's secrets do not\nsatisfy.", + "type": "boolean" } } }, diff --git a/coderd/dynamicparameters/secrets.go b/coderd/dynamicparameters/secrets.go new file mode 100644 index 0000000000..42cfcbe704 --- /dev/null +++ b/coderd/dynamicparameters/secrets.go @@ -0,0 +1,100 @@ +package dynamicparameters + +import ( + "context" + "slices" + + "github.com/google/uuid" + "github.com/hashicorp/hcl/v2" + + "cdr.dev/slog/v3" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/files" + "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/coder/v2/codersdk" + previewtypes "github.com/coder/preview/types" +) + +// EvaluateSecretMismatch reports whether the given template version +// declares coder_secret requirements that the workspace owner's secrets +// do not satisfy. Returns false (no mismatch) when the renderer cannot +// authoritatively evaluate the requirements; the reason is logged at the +// appropriate level so operators can distinguish a forbidden caller +// (expected for template admins) from a genuine renderer or DB failure. +// Returns ErrTemplateVersionNotReady when the version's provisioner job +// has not yet completed; callers should treat that as "unknown" and +// leave SecretMismatch false. +func EvaluateSecretMismatch( + ctx context.Context, + logger slog.Logger, + db database.Store, + cache files.FileAcquirer, + version database.TemplateVersion, + ownerID uuid.UUID, + buildParams []database.WorkspaceBuildParameter, +) (bool, error) { + paramValues := slice.ToMapFunc(buildParams, func(p database.WorkspaceBuildParameter) (string, string) { + return p.Name, p.Value + }) + renderer, err := Prepare(ctx, db, cache, version.ID, + WithTemplateVersion(version), + WithLogger(logger)) + if err != nil { + return false, err + } + defer renderer.Close() + + result, diags := renderer.Render(ctx, ownerID, paramValues, IncludeSecretRequirements()) + + // Three distinct "unknown" cases. Returning false from any of them + // matches the resolve-autostart handler's semantics, but they have + // very different operator implications, so we log accordingly. The + // renderer already logs its own diagnostics through the same logger, + // so we omit them here to avoid duplication. + if result.Output == nil { + logger.Warn(ctx, + "secret requirement evaluation produced no preview output; treating as unknown", + slog.F("template_version_id", version.ID), + ) + return false, nil + } + switch secretValidationBlockerCode(diags) { + case DiagCodeOwnerSecretsFetchFailed: + logger.Warn(ctx, + "failed to fetch owner secrets during requirement evaluation; treating as unknown", + slog.F("template_version_id", version.ID), + ) + return false, nil + case DiagCodeSecretValidationForbidden: + // Expected when a caller without user_secret:read on the owner + // hits the renderer, e.g. a template admin viewing another user's + // workspace. Debug-level keeps production volume sane while + // preserving visibility under trace logging. + logger.Debug(ctx, + "secret requirement evaluation forbidden for caller; treating as unknown", + slog.F("template_version_id", version.ID), + ) + return false, nil + } + + return slices.ContainsFunc(result.SecretRequirements, + func(s codersdk.SecretRequirementStatus) bool { return !s.Satisfied }), nil +} + +// secretValidationBlockerCode returns the first diagnostic code among the +// codes that indicate secret-requirement evaluation could not be +// performed. Returns the empty string if no such diagnostic is present. +// +// ExtractDiagnosticExtra walks the wrapped-extra chain so we still +// detect our marker when another extra has been chained on top by +// preview's SetDiagnosticExtra. +func secretValidationBlockerCode(diags hcl.Diagnostics) string { + for _, d := range diags { + extra := previewtypes.ExtractDiagnosticExtra(d) + switch extra.Code { + case DiagCodeOwnerSecretsFetchFailed, DiagCodeSecretValidationForbidden: + return extra.Code + } + } + return "" +} diff --git a/coderd/dynamicparameters/secrets_internal_test.go b/coderd/dynamicparameters/secrets_internal_test.go new file mode 100644 index 0000000000..cc799aca3b --- /dev/null +++ b/coderd/dynamicparameters/secrets_internal_test.go @@ -0,0 +1,112 @@ +package dynamicparameters + +import ( + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/stretchr/testify/require" + + previewtypes "github.com/coder/preview/types" +) + +func TestSecretValidationBlockerCode(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in hcl.Diagnostics + want string + }{ + { + name: "Empty", + in: hcl.Diagnostics{}, + want: "", + }, + { + name: "MissingSecretIsNotBlocking", + in: hcl.Diagnostics{{ + Severity: hcl.DiagError, + Summary: "Missing required secrets", + Extra: previewtypes.DiagnosticExtra{ + Code: DiagCodeMissingSecret, + }, + }}, + want: "", + }, + { + name: "Forbidden", + in: hcl.Diagnostics{{ + Severity: hcl.DiagWarning, + Summary: "Cannot validate secret requirements", + Extra: previewtypes.DiagnosticExtra{ + Code: DiagCodeSecretValidationForbidden, + }, + }}, + want: DiagCodeSecretValidationForbidden, + }, + { + name: "FetchFailed", + in: hcl.Diagnostics{{ + Severity: hcl.DiagError, + Summary: "Failed to fetch owner secrets", + Extra: previewtypes.DiagnosticExtra{ + Code: DiagCodeOwnerSecretsFetchFailed, + }, + }}, + want: DiagCodeOwnerSecretsFetchFailed, + }, + { + name: "DiagnosticWithNoExtraIsIgnored", + in: hcl.Diagnostics{{ + Severity: hcl.DiagError, + Summary: "Some other error", + }}, + want: "", + }, + { + name: "MixedKeepsLookingUntilMatch", + in: hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Missing required secrets", + Extra: previewtypes.DiagnosticExtra{ + Code: DiagCodeMissingSecret, + }, + }, + { + Severity: hcl.DiagError, + Summary: "Failed to fetch owner secrets", + Extra: previewtypes.DiagnosticExtra{ + Code: DiagCodeOwnerSecretsFetchFailed, + }, + }, + }, + want: DiagCodeOwnerSecretsFetchFailed, + }, + { + // SetDiagnosticExtra wraps any pre-existing extra into + // previewtypes.DiagnosticExtra.Wrapped. ExtractDiagnosticExtra + // walks that chain. A naive type assertion would miss it. + name: "WrappedExtraIsDetected", + in: func() hcl.Diagnostics { + d := &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Cannot validate secret requirements", + Extra: "some other extra", + } + previewtypes.SetDiagnosticExtra(d, previewtypes.DiagnosticExtra{ + Code: DiagCodeSecretValidationForbidden, + }) + return hcl.Diagnostics{d} + }(), + want: DiagCodeSecretValidationForbidden, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.want, secretValidationBlockerCode(tc.in)) + }) + } +} diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index 442682b5e0..a41af06dbb 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -649,6 +649,167 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { }) } +// TestResolveAutostartPreservesParameterMismatchOnSecretEvalError exercises +// the handler's default switch arm: when EvaluateSecretMismatch returns a +// non-ErrTemplateVersionNotReady error, the handler must log and treat +// SecretMismatch as "unknown" without dropping the already-computed +// ParameterMismatch signal. +func TestResolveAutostartPreservesParameterMismatchOnSecretEvalError(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + + // Wrap the DB so we can fail the renderer's GetTemplateVersionTerraformValues + // call. Toggle is flipped on after setup so initial template version + // processing succeeds. + reject := &dbRejectTemplateVersionTerraformValues{Store: db} + + noRequirementsTF := []byte(`terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} +`) + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + db: reject, + ps: ps, + provisionerDaemonVersion: provProto.CurrentVersion.String(), + mainTF: noRequirementsTF, + }) + _ = setup.stream.Close(websocket.StatusGoingAway) + + wrk := coderdtest.CreateWorkspace(t, setup.client, setup.template.ID, + func(req *codersdk.CreateWorkspaceRequest) { + req.AutomaticUpdates = codersdk.AutomaticUpdatesAlways + }) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, setup.client, wrk.LatestBuild.ID) + + // Push a v2 that adds a required-no-default parameter so resolve-autostart + // computes ParameterMismatch=true. The new version becomes active via + // DynamicParameterTemplate's UpdateActiveTemplateVersion call. + paramRequiredTF := []byte(`terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} + +data "coder_parameter" "required_param" { + name = "required_param" + type = "string" +} +`) + // StaticParams populates the legacy template_version_parameters table via + // the GraphComplete response. resolve-autostart reads from this table to + // determine ParameterMismatch. + _, _ = coderdtest.DynamicParameterTemplate(t, setup.dynamicParamsClient, + wrk.OrganizationID, + coderdtest.DynamicParameterTemplateParams{ + MainTF: string(paramRequiredTF), + TemplateID: setup.template.ID, + StaticParams: []*proto.RichParameter{{ + Name: "required_param", + Type: "string", + Required: true, + }}, + }) + + // Arm the rejection only for the resolve-autostart call. Setup has + // already completed, so all earlier calls passed through. + reject.SetReject(true) + + ctx := testutil.Context(t, testutil.WaitLong) + resp, err := setup.client.ResolveAutostart(ctx, wrk.ID.String()) + require.NoError(t, err, "resolve-autostart must not 500 when secret evaluation fails") + require.True(t, resp.ParameterMismatch, "ParameterMismatch should be preserved across secret evaluation failure") + require.False(t, resp.SecretMismatch, "SecretMismatch should be unknown (false) when evaluation fails") +} + +// TestResolveAutostartSecretRequirements is the PLAT-81 backend coverage: +// resolve-autostart must surface coder_secret requirements declared by the +// active template version that the workspace owner's secrets do not +// satisfy. The dashboard banner uses this to tell the user autostart +// cannot run the auto-update build until they create the missing secrets. +func TestResolveAutostartSecretRequirements(t *testing.T) { + t.Parallel() + + noRequirementsTF := []byte(`terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} +`) + secretRequiredTF, err := os.ReadFile("testdata/parameters/secret_required/main.tf") + require.NoError(t, err) + + // v1 has no secret requirements; we need a workspace to exist so + // resolve-autostart enters its version-mismatch branch. + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + provisionerDaemonVersion: provProto.CurrentVersion.String(), + mainTF: noRequirementsTF, + }) + _ = setup.stream.Close(websocket.StatusGoingAway) + + wrk := coderdtest.CreateWorkspace(t, setup.client, setup.template.ID, + func(req *codersdk.CreateWorkspaceRequest) { + req.AutomaticUpdates = codersdk.AutomaticUpdatesAlways + }) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, setup.client, wrk.LatestBuild.ID) + + // Push v2 with a coder_secret requirement and make it active. + _, _ = coderdtest.DynamicParameterTemplate(t, setup.dynamicParamsClient, + wrk.OrganizationID, + coderdtest.DynamicParameterTemplateParams{ + MainTF: string(secretRequiredTF), + TemplateID: setup.template.ID, + }) + + t.Run("OwnerSeesMismatchThenSatisfies", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + // Owner has no GITHUB_TOKEN secret; resolve-autostart must surface + // the unsatisfied requirement. + resp, err := setup.client.ResolveAutostart(ctx, wrk.ID.String()) + require.NoError(t, err) + require.False(t, resp.ParameterMismatch) + require.True(t, resp.SecretMismatch) + + // Creating the matching secret must clear the entry without further + // template changes. + _, err = setup.client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: "github-token", + Value: "ghp_test", + EnvName: "GITHUB_TOKEN", + }) + require.NoError(t, err) + + resp, err = setup.client.ResolveAutostart(ctx, wrk.ID.String()) + require.NoError(t, err) + require.False(t, resp.ParameterMismatch) + require.False(t, resp.SecretMismatch) + }) + + t.Run("ForbiddenCallerSeesNoMismatch", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + // The template-admin client can read the workspace but lacks + // user_secret:read on the workspace owner. The renderer's secret + // fetch produces a forbidden diagnostic, and the handler must + // treat that as "unknown" and report SecretMismatch=false rather + // than leaking a 500 or a stale true value. + resp, err := setup.dynamicParamsClient.ResolveAutostart(ctx, wrk.ID.String()) + require.NoError(t, err) + require.False(t, resp.SecretMismatch) + }) +} + type setupDynamicParamsTestParams struct { db database.Store ps pubsub.Pubsub @@ -749,3 +910,34 @@ func (d *dbRejectGitSSHKey) GetGitSSHKey(ctx context.Context, userID uuid.UUID) return d.Store.GetGitSSHKey(ctx, userID) } + +// dbRejectTemplateVersionTerraformValues wraps a Store so the dynamic +// parameter renderer's GetTemplateVersionTerraformValues call can be made +// to fail on demand. This forces the resolve-autostart handler into the +// default switch arm where EvaluateSecretMismatch returns a non- +// ErrTemplateVersionNotReady error. +type dbRejectTemplateVersionTerraformValues struct { + database.Store + rejectMu sync.RWMutex + reject bool +} + +// SetReject toggles whether GetTemplateVersionTerraformValues should +// return an error or passthrough to the underlying store. +func (d *dbRejectTemplateVersionTerraformValues) SetReject(reject bool) { + d.rejectMu.Lock() + defer d.rejectMu.Unlock() + d.reject = reject +} + +func (d *dbRejectTemplateVersionTerraformValues) GetTemplateVersionTerraformValues(ctx context.Context, templateVersionID uuid.UUID) (database.TemplateVersionTerraformValue, error) { + d.rejectMu.RLock() + reject := d.reject + d.rejectMu.RUnlock() + + if reject { + return database.TemplateVersionTerraformValue{}, xerrors.New("forcing a fake error") + } + + return d.Store.GetTemplateVersionTerraformValues(ctx, templateVersionID) +} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a39f70cdaa..4742189d2f 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -25,6 +25,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" + "github.com/coder/coder/v2/coderd/dynamicparameters" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpapi/httperror" "github.com/coder/coder/v2/coderd/httpmw" @@ -2009,6 +2010,36 @@ func (api *API) resolveAutostart(rw http.ResponseWriter, r *http.Request) { break } } + + // Surface whether the active template version declares coder_secret + // requirements that the workspace owner's secrets do not satisfy. The + // intention is for this information to inform the workspace update + // requirement so the user knows autostart will not run an auto-update + // build until the missing secrets are satisfied. + // + // Callers without user_secret:read on the workspace owner produce a + // forbidden warning diagnostic. This is treated as "unknown" and + // no mismatch is reported rather than returning a partial answer. + secretMismatch, err := dynamicparameters.EvaluateSecretMismatch( + ctx, + api.Logger.Named("dynamicparameters"), + api.Database, api.FileCache, version, workspace.OwnerID, dbBuildParams, + ) + switch { + case err == nil: + response.SecretMismatch = secretMismatch + case xerrors.Is(err, dynamicparameters.ErrTemplateVersionNotReady): + // Active version's provisioner job hasn't completed yet. Leave + // SecretMismatch false. + default: + // Don't drop the already-computed ParameterMismatch signal on a + // renderer infrastructure error. Log and treat as "unknown." + api.Logger.Warn(ctx, "failed to evaluate secret requirements", + slog.F("workspace_id", workspace.ID), + slog.Error(err), + ) + } + httpapi.Write(ctx, rw, http.StatusOK, response) } diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index b520f27e4f..e1fbbd859b 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -680,6 +680,10 @@ func (c *Client) WorkspaceQuota(ctx context.Context, organizationID string, user type ResolveAutostartResponse struct { ParameterMismatch bool `json:"parameter_mismatch"` + // SecretMismatch is true when the active template version declares + // `coder_secret` requirements that the workspace owner's secrets do not + // satisfy. + SecretMismatch bool `json:"secret_mismatch"` } func (c *Client) ResolveAutostart(ctx context.Context, workspaceID string) (ResolveAutostartResponse, error) { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 30328f7d02..21f4030928 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -10625,15 +10625,17 @@ Only certain features set these fields: - FeatureManagedAgentLimit| ```json { - "parameter_mismatch": true + "parameter_mismatch": true, + "secret_mismatch": true } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -|----------------------|---------|----------|--------------|-------------| -| `parameter_mismatch` | boolean | false | | | +| Name | Type | Required | Restrictions | Description | +|----------------------|---------|----------|--------------|--------------------------------------------------------------------------------------------------------------------------------------------------| +| `parameter_mismatch` | boolean | false | | | +| `secret_mismatch` | boolean | false | | Secret mismatch is true when the active template version declares `coder_secret` requirements that the workspace owner's secrets do not satisfy. | ## codersdk.ResourceType diff --git a/docs/reference/api/workspaces.md b/docs/reference/api/workspaces.md index 758005578c..b1faf32448 100644 --- a/docs/reference/api/workspaces.md +++ b/docs/reference/api/workspaces.md @@ -2336,7 +2336,8 @@ curl -X GET http://coder-server:8080/api/v2/workspaces/{workspace}/resolve-autos ```json { - "parameter_mismatch": true + "parameter_mismatch": true, + "secret_mismatch": true } ``` diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 431ef9cbdf..9a2d698c3d 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -6593,6 +6593,12 @@ export interface RequestOneTimePasscodeRequest { // From codersdk/workspaces.go export interface ResolveAutostartResponse { readonly parameter_mismatch: boolean; + /** + * SecretMismatch is true when the active template version declares + * `coder_secret` requirements that the workspace owner's secrets do not + * satisfy. + */ + readonly secret_mismatch: boolean; } // From codersdk/audit.go