diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index df40f2e050..4fa2ec46b7 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -48,6 +48,11 @@ OPTIONS: the workspace serves malicious JavaScript. This is recommended for security purposes if a --wildcard-access-url is configured. + --disable-user-secret-file-path bool, $CODER_DISABLE_USER_SECRET_FILE_PATH + Disable Coder-managed file path delivery for user secrets. Stored + paths remain until users clear them and resume if this setting is + turned off. + --disable-workspace-agent-context-sync bool, $CODER_DISABLE_WORKSPACE_AGENT_CONTEXT_SYNC Stop persisting workspace agent context snapshots (instructions, skills, and MCP state used for pinned chat context). When set, coderd diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 2d3fbda816..b631ac69c1 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -580,6 +580,10 @@ disableChatSharing: false # deployments. # (default: , type: bool) disableWorkspaceAgentContextSync: false +# Disable Coder-managed file path delivery for user secrets. Stored paths remain +# until users clear them and resume if this setting is turned off. +# (default: , type: bool) +disableUserSecretFilePath: false # These options change the behavior of how clients interact with the Coder. # Clients include the Coder CLI, Coder Desktop, IDE extensions, and the web UI. client: diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index bf10c41e5a..9951c17e96 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3885,6 +3885,31 @@ const docTemplate = `{ ] } }, + "/api/v2/deployment/user-secrets/capabilities": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "General" + ], + "summary": "Get user secrets capabilities", + "operationId": "get-user-secrets-capabilities", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.UserSecretsCapabilities" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ] + } + }, "/api/v2/derp-map": { "get": { "tags": [ @@ -13248,6 +13273,18 @@ const docTemplate = `{ "schema": { "$ref": "#/definitions/codersdk.UserSecret" } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } } }, "security": [ @@ -13440,6 +13477,18 @@ const docTemplate = `{ "schema": { "$ref": "#/definitions/codersdk.UserSecret" } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } } }, "security": [ @@ -22930,6 +22979,9 @@ const docTemplate = `{ "disable_path_apps": { "type": "boolean" }, + "disable_user_secret_file_path": { + "type": "boolean" + }, "disable_workspace_agent_context_sync": { "type": "boolean" }, @@ -30391,6 +30443,15 @@ const docTemplate = `{ } } }, + "codersdk.UserSecretsCapabilities": { + "type": "object", + "properties": { + "file_path_delivery_enabled": { + "description": "FilePathDeliveryEnabled reports whether Coder writes stored file paths\ninto workspaces. Stored paths are preserved either way.", + "type": "boolean" + } + } + }, "codersdk.UserSkill": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 514bbd0a75..2664c97faf 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -3428,6 +3428,27 @@ ] } }, + "/api/v2/deployment/user-secrets/capabilities": { + "get": { + "produces": ["application/json"], + "tags": ["General"], + "summary": "Get user secrets capabilities", + "operationId": "get-user-secrets-capabilities", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.UserSecretsCapabilities" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ] + } + }, "/api/v2/derp-map": { "get": { "tags": ["Agents"], @@ -11767,6 +11788,18 @@ "schema": { "$ref": "#/definitions/codersdk.UserSecret" } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } } }, "security": [ @@ -11941,6 +11974,18 @@ "schema": { "$ref": "#/definitions/codersdk.UserSecret" } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } } }, "security": [ @@ -20845,6 +20890,9 @@ "disable_path_apps": { "type": "boolean" }, + "disable_user_secret_file_path": { + "type": "boolean" + }, "disable_workspace_agent_context_sync": { "type": "boolean" }, @@ -28002,6 +28050,15 @@ } } }, + "codersdk.UserSecretsCapabilities": { + "type": "object", + "properties": { + "file_path_delivery_enabled": { + "description": "FilePathDeliveryEnabled reports whether Coder writes stored file paths\ninto workspaces. Stored paths are preserved either way.", + "type": "boolean" + } + } + }, "codersdk.UserSkill": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index 5eb6549ed9..7ad9f0e662 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1441,6 +1441,7 @@ func New(options *Options) *API { r.Get("/config", api.deploymentValues) r.Get("/stats", api.deploymentStats) r.Get("/ssh", api.sshConfig) + r.Get("/user-secrets/capabilities", api.userSecretsCapabilities) r.Post("/premium-funnel-events", api.postPremiumFunnelEvent) }) r.Route("/experiments", func(r chi.Router) { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 13c28cbedb..84f52631aa 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -5399,6 +5399,15 @@ func (q *querier) GetUserSecretByUserIDAndName(ctx context.Context, arg database return q.db.GetUserSecretByUserIDAndName(ctx, arg) } +func (q *querier) GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg database.GetUserSecretByUserIDAndNameForUpdateParams) (database.UserSecret, error) { + obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String()) + if err := q.authorizeContext(ctx, policy.ActionUpdate, obj); err != nil { + return database.UserSecret{}, err + } + + return q.db.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) +} + func (q *querier) GetUserSecretsTelemetrySummary(ctx context.Context) (database.GetUserSecretsTelemetrySummaryRow, error) { // Telemetry queries are called from system contexts only. The // query reads aggregate counts across all users' secrets, so diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 7d4fa9845c..af6bc0357e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -6661,6 +6661,15 @@ func (s *MethodTestSuite) TestUserSecrets() { Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionRead). Returns(secret) })) + s.Run("GetUserSecretByUserIDAndNameForUpdate", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + user := testutil.Fake(s.T(), faker, database.User{}) + secret := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID}) + arg := database.GetUserSecretByUserIDAndNameForUpdateParams{UserID: user.ID, Name: secret.Name} + dbm.EXPECT().GetUserSecretByUserIDAndNameForUpdate(gomock.Any(), arg).Return(secret, nil).AnyTimes() + check.Args(arg). + Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionUpdate). + Returns(secret) + })) s.Run("ListUserSecrets", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { user := testutil.Fake(s.T(), faker, database.User{}) row := testutil.Fake(s.T(), faker, database.ListUserSecretsRow{UserID: user.ID}) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 39dfda8122..cd8072e84a 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -3464,6 +3464,14 @@ func (m queryMetricsStore) GetUserSecretByUserIDAndName(ctx context.Context, arg return r0, r1 } +func (m queryMetricsStore) GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg database.GetUserSecretByUserIDAndNameForUpdateParams) (database.UserSecret, error) { + start := time.Now() + r0, r1 := m.s.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserSecretByUserIDAndNameForUpdate").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetUserSecretByUserIDAndNameForUpdate").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetUserSecretsTelemetrySummary(ctx context.Context) (database.GetUserSecretsTelemetrySummaryRow, error) { start := time.Now() r0, r1 := m.s.GetUserSecretsTelemetrySummary(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index d237d3124d..0d6eebe193 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -6524,6 +6524,21 @@ func (mr *MockStoreMockRecorder) GetUserSecretByUserIDAndName(ctx, arg any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecretByUserIDAndName", reflect.TypeOf((*MockStore)(nil).GetUserSecretByUserIDAndName), ctx, arg) } +// GetUserSecretByUserIDAndNameForUpdate mocks base method. +func (m *MockStore) GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg database.GetUserSecretByUserIDAndNameForUpdateParams) (database.UserSecret, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserSecretByUserIDAndNameForUpdate", ctx, arg) + ret0, _ := ret[0].(database.UserSecret) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserSecretByUserIDAndNameForUpdate indicates an expected call of GetUserSecretByUserIDAndNameForUpdate. +func (mr *MockStoreMockRecorder) GetUserSecretByUserIDAndNameForUpdate(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecretByUserIDAndNameForUpdate", reflect.TypeOf((*MockStore)(nil).GetUserSecretByUserIDAndNameForUpdate), ctx, arg) +} + // GetUserSecretsTelemetrySummary mocks base method. func (m *MockStore) GetUserSecretsTelemetrySummary(ctx context.Context) (database.GetUserSecretsTelemetrySummaryRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 17a05ed18e..23e8a8db5e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -973,6 +973,7 @@ type sqlcQuerier interface { GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) GetUserSecretByID(ctx context.Context, id uuid.UUID) (UserSecret, error) GetUserSecretByUserIDAndName(ctx context.Context, arg GetUserSecretByUserIDAndNameParams) (UserSecret, error) + GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg GetUserSecretByUserIDAndNameForUpdateParams) (UserSecret, error) // Returns deployment-wide aggregates for the telemetry snapshot. // // The denominator for both user-level counts and the per-user diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5e62cc6984..9307957ad8 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -30811,6 +30811,37 @@ func (q *sqlQuerier) GetUserSecretByUserIDAndName(ctx context.Context, arg GetUs return i, err } +const getUserSecretByUserIDAndNameForUpdate = `-- name: GetUserSecretByUserIDAndNameForUpdate :one +SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id, enabled +FROM user_secrets +WHERE user_id = $1 AND name = $2 +FOR UPDATE +` + +type GetUserSecretByUserIDAndNameForUpdateParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg GetUserSecretByUserIDAndNameForUpdateParams) (UserSecret, error) { + row := q.db.QueryRowContext(ctx, getUserSecretByUserIDAndNameForUpdate, arg.UserID, arg.Name) + var i UserSecret + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Value, + &i.EnvName, + &i.FilePath, + &i.CreatedAt, + &i.UpdatedAt, + &i.ValueKeyID, + &i.Enabled, + ) + return i, err +} + const getUserSecretsTelemetrySummary = `-- name: GetUserSecretsTelemetrySummary :one WITH active_users AS ( SELECT id AS user_id diff --git a/coderd/database/queries/user_secrets.sql b/coderd/database/queries/user_secrets.sql index 56e07b57ad..3b552386e9 100644 --- a/coderd/database/queries/user_secrets.sql +++ b/coderd/database/queries/user_secrets.sql @@ -8,6 +8,12 @@ SELECT * FROM user_secrets WHERE id = @id; +-- name: GetUserSecretByUserIDAndNameForUpdate :one +SELECT * +FROM user_secrets +WHERE user_id = @user_id AND name = @name +FOR UPDATE; + -- name: ListUserSecrets :many -- Returns metadata only (no value or value_key_id) for the -- REST API list and get endpoints. diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go index ec1e9c3992..292cdf255a 100644 --- a/coderd/telemetry/telemetry.go +++ b/coderd/telemetry/telemetry.go @@ -2649,7 +2649,9 @@ type ChatDiffStatusSummary struct { // UsersWithSecrets is the count of active non-system users that have // at least one secret. TotalSecrets is the count of secrets owned by // those users. EnvNameOnly, FilePathOnly, Both, and Neither break -// TotalSecrets down by which injection fields are populated. +// TotalSecrets down by which target fields are stored. They do not +// describe effective delivery because deployment policy can block a +// stored target. // // The SecretsPerUser* fields describe the distribution of secrets per // user across the entire active non-system user base, including users diff --git a/coderd/usersecrets.go b/coderd/usersecrets.go index 9f4c8aad53..3aed0e2c16 100644 --- a/coderd/usersecrets.go +++ b/coderd/usersecrets.go @@ -35,6 +35,104 @@ const ( // codersdk.ValidateCreateUserSecretRequest. var errUserSecretInjectionTargetRequired = xerrors.New("enabled user secret must have at least one of env_name or file_path set") +var ( + errUserSecretFilePathDisabled = xerrors.New("file-based user secrets are disabled") + errUserSecretEnvTargetRequired = xerrors.New("enabled user secret requires env_name") +) + +const ( + //nolint:gosec // User-facing validation text. + userSecretFilePathDisabledDetail = "File path delivery for user secrets is disabled by the deployment administrator. Clear file_path or use env_name instead." + //nolint:gosec // User-facing validation text. + userSecretEnvTargetRequiredDetail = "File path delivery for user secrets is disabled by the deployment administrator, so an enabled secret must have an env_name. Add env_name, or set enabled to false." +) + +// userSecretFilePathBlocked reports whether the deployment forbids file path +// delivery for user secrets. +func (api *API) userSecretFilePathBlocked() bool { + return api.DeploymentValues != nil && api.DeploymentValues.DisableUserSecretFilePath.Value() +} + +//nolint:revive // blocked is deployment configuration, not caller control coupling. +func userSecretCreateValidationErrors(req codersdk.CreateUserSecretRequest, blocked bool) []codersdk.ValidationError { + validations := codersdk.ValidateCreateUserSecretRequest(req) + if !blocked { + return validations + } + + if req.FilePath != "" { + validations = append(validations, codersdk.ValidationError{ + Field: codersdk.UserSecretFilePathField, + Detail: userSecretFilePathDisabledDetail, + }) + } + if (req.Enabled != nil && !*req.Enabled) || req.EnvName != "" { + return validations + } + + for i := range validations { + if validations[i].Field == codersdk.UserSecretEnvNameField && validations[i].Detail == codersdk.UserSecretInjectionTargetRequiredDetail { + validations[i].Detail = userSecretEnvTargetRequiredDetail + } + } + return validations +} + +func prefixUserSecretValidationErrors(index int, validations []codersdk.ValidationError) []codersdk.ValidationError { + if index < 0 { + return validations + } + prefixed := make([]codersdk.ValidationError, 0, len(validations)) + for _, v := range validations { + prefixed = append(prefixed, codersdk.ValidationError{ + Field: fmt.Sprintf("secrets[%d].%s", index, v.Field), + Detail: v.Detail, + }) + } + return prefixed +} + +// Existing enabled file-only rows may be edited or disabled, but a PATCH +// cannot add or change a path or enter a new enabled state without env_name. +func userSecretFilePathPolicyError(old database.UserSecret, req codersdk.UpdateUserSecretRequest) error { + if req.FilePath != nil && *req.FilePath != "" && *req.FilePath != old.FilePath { + return errUserSecretFilePathDisabled + } + + postEnvName := old.EnvName + if req.EnvName != nil { + postEnvName = *req.EnvName + } + postEnabled := old.Enabled + if req.Enabled != nil { + postEnabled = *req.Enabled + } + if postEnabled && postEnvName == "" && req.FilePath != nil && *req.FilePath == "" { + return errUserSecretEnvTargetRequired + } + + predatesPolicy := old.Enabled && old.EnvName == "" + if postEnabled && postEnvName == "" && !predatesPolicy { + return errUserSecretEnvTargetRequired + } + return nil +} + +// @Summary Get user secrets capabilities +// @ID get-user-secrets-capabilities +// @Security CoderSessionToken +// @Produce json +// @Tags General +// @Success 200 {object} codersdk.UserSecretsCapabilities +// @Router /api/v2/deployment/user-secrets/capabilities [get] +func (api *API) userSecretsCapabilities(rw http.ResponseWriter, r *http.Request) { + // Any authenticated user may read this. It exposes only the delivery + // policy that shapes their own secrets, not the deployment config. + httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.UserSecretsCapabilities{ + FilePathDeliveryEnabled: !api.userSecretFilePathBlocked(), + }) +} + // @Summary Create a new user secret // @ID create-a-new-user-secret // @Security CoderSessionToken @@ -44,6 +142,8 @@ var errUserSecretInjectionTargetRequired = xerrors.New("enabled user secret must // @Param user path string true "User ID, username, or me" // @Param request body codersdk.CreateUserSecretRequest true "Create secret request" // @Success 201 {object} codersdk.UserSecret +// @Failure 400 {object} codersdk.Response +// @Failure 409 {object} codersdk.Response // @Router /api/v2/users/{user}/secrets [post] func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) { var ( @@ -64,7 +164,7 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) { return } - if validations := codersdk.ValidateCreateUserSecretRequest(req); len(validations) > 0 { + if validations := userSecretCreateValidationErrors(req, api.userSecretFilePathBlocked()); len(validations) > 0 { writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, validations) return } @@ -151,14 +251,11 @@ func (api *API) postUserSecretsBatch(rw http.ResponseWriter, r *http.Request) { // Validate every entry and accumulate all errors so the caller can // fix the whole file in one round-trip. Each field is prefixed with // the entry index, e.g. "secrets[2].env_name". + filePathBlocked := api.userSecretFilePathBlocked() var validations []codersdk.ValidationError for i, sreq := range reqs { - for _, v := range codersdk.ValidateCreateUserSecretRequest(sreq) { - validations = append(validations, codersdk.ValidationError{ - Field: fmt.Sprintf("secrets[%d].%s", i, v.Field), - Detail: v.Detail, - }) - } + entryValidations := userSecretCreateValidationErrors(sreq, filePathBlocked) + validations = append(validations, prefixUserSecretValidationErrors(i, entryValidations)...) } if len(validations) > 0 { writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, validations) @@ -200,21 +297,11 @@ func (api *API) postUserSecretsBatch(rw http.ResponseWriter, r *http.Request) { index := failedIndex if conflicts := userSecretConflictValidationErrors(err); len(conflicts) > 0 { - if index >= 0 { - for i := range conflicts { - conflicts[i].Field = fmt.Sprintf("secrets[%d].%s", index, conflicts[i].Field) - } - } - writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, conflicts) + writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, prefixUserSecretValidationErrors(index, conflicts)) return } if validations := userSecretInjectionTargetValidationErrors(err); len(validations) > 0 { - if index >= 0 { - for i := range validations { - validations[i].Field = fmt.Sprintf("secrets[%d].%s", index, validations[i].Field) - } - } - writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, validations) + writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, prefixUserSecretValidationErrors(index, validations)) return } if resp, ok := userSecretLimitResponse(err); ok { @@ -330,6 +417,8 @@ func (api *API) getUserSecret(rw http.ResponseWriter, r *http.Request) { //nolin // @Param name path string true "Secret name" // @Param request body codersdk.UpdateUserSecretRequest true "Update secret request" // @Success 200 {object} codersdk.UserSecret +// @Failure 400 {object} codersdk.Response +// @Failure 409 {object} codersdk.Response // @Router /api/v2/users/{user}/secrets/{name} [patch] func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { var ( @@ -393,17 +482,11 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { params.Enabled = *req.Enabled } - // Pre-read the secret inside a transaction so the audit diff has both an - // "old" and "new" snapshot. - // - // Under read committed isolation, a concurrent writer between our SELECT - // and our UPDATE can cause the audit diff to attribute changes to us that - // we did not make. We accept this race to match other audit log diffs - // (templates, workspaces, chats, etc). In practice this should be unlikely - // to hit since a user can only modify their own secrets. + // Lock before computing post-state so concurrent PATCHes serialize. var secret database.UserSecret + filePathBlocked := api.userSecretFilePathBlocked() err := api.Database.InTx(func(tx database.Store) error { - old, err := tx.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ + old, err := tx.GetUserSecretByUserIDAndNameForUpdate(ctx, database.GetUserSecretByUserIDAndNameForUpdateParams{ UserID: user.ID, Name: name, }) @@ -428,6 +511,12 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { if req.Enabled != nil { postEnabled = *req.Enabled } + if filePathBlocked { + if err := userSecretFilePathPolicyError(old, req); err != nil { + return err + } + } + if postEnabled && postEnvName == "" && postFilePath == "" { return errUserSecretInjectionTargetRequired } @@ -445,10 +534,19 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { httpapi.ResourceNotFound(rw) return } - if errors.Is(err, errUserSecretInjectionTargetRequired) { + var field, detail string + switch { + case errors.Is(err, errUserSecretInjectionTargetRequired): + field, detail = codersdk.UserSecretEnvNameField, codersdk.UserSecretInjectionTargetRequiredDetail + case errors.Is(err, errUserSecretFilePathDisabled): + field, detail = codersdk.UserSecretFilePathField, userSecretFilePathDisabledDetail + case errors.Is(err, errUserSecretEnvTargetRequired): + field, detail = codersdk.UserSecretEnvNameField, userSecretEnvTargetRequiredDetail + } + if field != "" { writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, []codersdk.ValidationError{{ - Field: codersdk.UserSecretEnvNameField, - Detail: codersdk.UserSecretInjectionTargetRequiredDetail, + Field: field, + Detail: detail, }}) return } @@ -464,6 +562,10 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusBadRequest, resp) return } + if httpapi.IsUnauthorizedError(err) { + httpapi.Forbidden(rw) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error updating secret.", Detail: err.Error(), @@ -578,7 +680,7 @@ func userSecretLimitResponse(err error) (codersdk.Response, bool) { Detail: fmt.Sprintf( "Stored bytes of env-injected secret values exceed the "+ "per-user budget (%d bytes after encryption, if applicable). "+ - "Clear env_name on large secrets or use file_path instead.", + "Reduce the size or number of env-injected secrets. If your deployment permits file path delivery, clear env_name and use file_path instead.", codersdk.MaxUserSecretValueBytes, ), }, true diff --git a/coderd/usersecrets_concurrency_test.go b/coderd/usersecrets_concurrency_test.go new file mode 100644 index 0000000000..602790d836 --- /dev/null +++ b/coderd/usersecrets_concurrency_test.go @@ -0,0 +1,89 @@ +package coderd_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/testutil" +) + +func TestGetUserSecretByUserIDAndNameForUpdateLocks(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitLong) + + user := dbgen.User(t, db, database.User{}) + secret := dbgen.UserSecret(t, db, database.UserSecret{ + UserID: user.ID, Name: "lock-me", EnvName: "LOCK_ME", + }) + arg := database.GetUserSecretByUserIDAndNameForUpdateParams{ + UserID: user.ID, Name: secret.Name, + } + + // The first transaction holds the row lock until released. + locked, release := make(chan struct{}), make(chan struct{}) + first := make(chan error, 1) + go func() { + first <- db.InTx(func(tx database.Store) error { + if _, err := tx.GetUserSecretByUserIDAndNameForUpdate(ctx, arg); err != nil { + return err + } + close(locked) + <-release + return nil + }, nil) + }() + testutil.TryReceive(ctx, t, locked) + + second := make(chan error, 1) + go func() { + second <- db.InTx(func(tx database.Store) error { + _, err := tx.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) + return err + }, nil) + }() + + // Wait until the second transaction is observably blocked on the row lock, + // rather than inferring it from a fixed delay. A waiter for SELECT ... FOR + // UPDATE takes a granted "tuple" lock on the relation and then waits on the + // holder's transaction ID, so it appears in pg_locks as a backend holding + // both rows. Asserting the wait directly stops this from passing vacuously + // when the second goroutine is merely slow to reach the query. + require.Eventually(t, func() bool { + locks, err := db.PGLocks(ctx) + if err != nil { + return false + } + waitingOnRow := make(map[int]bool) + for _, l := range locks { + if l.LockType != nil && *l.LockType == "tuple" && + l.RelationName != nil && *l.RelationName == "user_secrets" { + waitingOnRow[l.PID] = true + } + } + for _, l := range locks { + if !l.Granted && waitingOnRow[l.PID] && + l.LockType != nil && *l.LockType == "transactionid" { + return true + } + } + return false + }, testutil.WaitShort, testutil.IntervalFast, "second transaction must block on the user_secrets row lock") + + // With the second transaction proven blocked, it must not have completed + // while the row lock is still held. + select { + case err := <-second: + t.Fatalf("second transaction acquired the row lock while it was held: %v", err) + default: + } + + close(release) + require.NoError(t, testutil.TryReceive(ctx, t, first)) + require.NoError(t, testutil.TryReceive(ctx, t, second)) +} diff --git a/coderd/usersecrets_filepath_policy_test.go b/coderd/usersecrets_filepath_policy_test.go new file mode 100644 index 0000000000..86b19007e3 --- /dev/null +++ b/coderd/usersecrets_filepath_policy_test.go @@ -0,0 +1,112 @@ +package coderd_test + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" + "github.com/coder/serpent" +) + +func TestUserSecretsCapabilities(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + disableFilePath bool + filePathDeliveryEnabled bool + }{ + {name: "FilePathDeliveryEnabled", disableFilePath: false, filePathDeliveryEnabled: true}, + {name: "FilePathDeliveryDisabled", disableFilePath: true, filePathDeliveryEnabled: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) { + dv.DisableUserSecretFilePath = serpent.Bool(tt.disableFilePath) + }) + ownerClient := coderdtest.New(t, &coderdtest.Options{DeploymentValues: dv}) + owner := coderdtest.CreateFirstUser(t, ownerClient) + memberClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) + ctx := testutil.Context(t, testutil.WaitMedium) + + capabilities, err := memberClient.UserSecretsCapabilities(ctx) + require.NoError(t, err) + assert.Equal(t, tt.filePathDeliveryEnabled, capabilities.FilePathDeliveryEnabled) + + // The same member may not read the deployment configuration that + // holds the underlying option. + _, err = memberClient.DeploymentConfig(ctx) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + assert.Equal(t, http.StatusForbidden, sdkErr.StatusCode()) + }) + } +} + +func TestUserSecretFilePathDisabledHandlers(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + 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, Auditor: auditor, + }) + owner := coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitMedium) + + _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: "file-secret", Value: "value", EnvName: "FILE_SECRET", FilePath: "/tmp/file-secret", + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "file_path", "disabled") + + legacy := dbgen.UserSecret(t, db, database.UserSecret{UserID: owner.UserID, Name: "legacy"}, + func(p *database.CreateUserSecretParams) { + p.EnvName, p.FilePath, p.Enabled = "", "/tmp/legacy", true + }) + + description := "updated" + updated, err := client.UpdateUserSecret(ctx, codersdk.Me, legacy.Name, codersdk.UpdateUserSecretRequest{ + Description: &description, + }) + require.NoError(t, err) + assert.Equal(t, "/tmp/legacy", updated.FilePath) + assert.True(t, updated.Enabled) + + empty := "" + _, err = client.UpdateUserSecret(ctx, codersdk.Me, legacy.Name, codersdk.UpdateUserSecretRequest{FilePath: &empty}) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "env_name", "Add env_name") + + // Changing the path of the still-enabled legacy row is rejected. + auditor.ResetLogs() + newPath := "/tmp/other" + _, err = client.UpdateUserSecret(ctx, codersdk.Me, legacy.Name, codersdk.UpdateUserSecretRequest{ + FilePath: &newPath, + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "file_path", "disabled") + + logs := auditor.AuditLogs() + require.Len(t, logs, 1) + assert.Equal(t, database.AuditActionWrite, logs[0].Action) + assert.EqualValues(t, http.StatusBadRequest, logs[0].StatusCode) + assert.Equal(t, legacy.ID, logs[0].ResourceID) + assert.JSONEq(t, "{}", string(logs[0].Diff)) + + disabled := false + updated, err = client.UpdateUserSecret(ctx, codersdk.Me, legacy.Name, codersdk.UpdateUserSecretRequest{Enabled: &disabled}) + require.NoError(t, err) + assert.Equal(t, "/tmp/legacy", updated.FilePath) +} diff --git a/coderd/usersecrets_internal_test.go b/coderd/usersecrets_internal_test.go new file mode 100644 index 0000000000..0951ba049d --- /dev/null +++ b/coderd/usersecrets_internal_test.go @@ -0,0 +1,124 @@ +package coderd + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/codersdk" +) + +func TestUserSecretCreateValidationErrors(t *testing.T) { + t.Parallel() + + req := func(envName, filePath string, enabled *bool) codersdk.CreateUserSecretRequest { + return codersdk.CreateUserSecretRequest{ + Name: "n", Value: "v", EnvName: envName, FilePath: filePath, Enabled: enabled, + } + } + const ( + envField = codersdk.UserSecretEnvNameField + pathField = codersdk.UserSecretFilePathField + ) + + tests := []struct { + name string + blocked bool + req codersdk.CreateUserSecretRequest + wantField string + wantDetail string + }{ + {name: "PolicyOffAllowsFilePath", blocked: false, req: req("X", "/tmp/x", nil)}, + {name: "PolicyOnAllowsEnvOnly", blocked: true, req: req("X", "", nil)}, + {name: "PolicyOnAllowsDisabledTargetless", blocked: true, req: req("", "", ptr.Ref(false))}, + {name: "PolicyOnRejectsFilePath", blocked: true, req: req("X", "/tmp/x", nil), wantField: pathField, wantDetail: userSecretFilePathDisabledDetail}, + {name: "PolicyOnRejectsFilePathOnDisabled", blocked: true, req: req("", "/tmp/x", ptr.Ref(false)), wantField: pathField, wantDetail: userSecretFilePathDisabledDetail}, + {name: "PolicyOnRequiresEnvTarget", blocked: true, req: req("", "", nil), wantField: envField, wantDetail: userSecretEnvTargetRequiredDetail}, + {name: "PolicyOffKeepsSharedMessage", blocked: false, req: req("", "", nil), wantField: envField, wantDetail: codersdk.UserSecretInjectionTargetRequiredDetail}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := userSecretCreateValidationErrors(tt.req, tt.blocked) + if tt.wantField == "" { + require.Empty(t, got) + return + } + require.Len(t, got, 1) + assert.Equal(t, tt.wantField, got[0].Field) + assert.Equal(t, tt.wantDetail, got[0].Detail) + }) + } +} + +func TestPrefixUserSecretValidationErrors(t *testing.T) { + t.Parallel() + + got := prefixUserSecretValidationErrors(2, []codersdk.ValidationError{ + {Field: codersdk.UserSecretFilePathField, Detail: "nope"}, + {Field: codersdk.UserSecretEnvNameField, Detail: "also nope"}, + }) + require.Len(t, got, 2) + assert.Equal(t, "secrets[2].file_path", got[0].Field) + assert.Equal(t, "nope", got[0].Detail) + assert.Equal(t, "secrets[2].env_name", got[1].Field) +} + +func TestPrefixUserSecretValidationErrorsNegativeIndex(t *testing.T) { + t.Parallel() + + // The batch handler passes failedIndex=-1 when no entry can be blamed. + in := []codersdk.ValidationError{{Field: codersdk.UserSecretEnvNameField, Detail: "nope"}} + got := prefixUserSecretValidationErrors(-1, in) + require.Len(t, got, 1) + assert.Equal(t, codersdk.UserSecretEnvNameField, got[0].Field) + assert.Equal(t, "nope", got[0].Detail) +} + +func TestUserSecretFilePathPolicyError(t *testing.T) { + t.Parallel() + + var ( + legacyFileOnly = database.UserSecret{FilePath: "/tmp/legacy", Enabled: true} + bothTargets = database.UserSecret{EnvName: "ENV", FilePath: "/tmp/legacy", Enabled: true} + disabledFileOnly = database.UserSecret{FilePath: "/tmp/legacy"} + envOnly = database.UserSecret{EnvName: "ENV", Enabled: true} + ) + + tests := []struct { + name string + old database.UserSecret + req codersdk.UpdateUserSecretRequest + wantErr error + }{ + {name: "UnrelatedEdit", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{Description: ptr.Ref("new")}}, + {name: "ResubmitSamePath", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref("/tmp/legacy")}}, + {name: "Disable", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{Enabled: ptr.Ref(false)}}, + {name: "ClearOnlyPath", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref("")}, wantErr: errUserSecretEnvTargetRequired}, + {name: "ClearPathAndDisable", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref(""), Enabled: ptr.Ref(false)}}, + {name: "MigrateToEnv", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{EnvName: ptr.Ref("ENV"), FilePath: ptr.Ref("")}}, + {name: "CleanupPathWithEnvTarget", old: bothTargets, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref("")}}, + {name: "AddPath", old: envOnly, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref("/tmp/new")}, wantErr: errUserSecretFilePathDisabled}, + {name: "ChangePath", old: legacyFileOnly, req: codersdk.UpdateUserSecretRequest{FilePath: ptr.Ref("/tmp/other")}, wantErr: errUserSecretFilePathDisabled}, + {name: "ClearEnvLeavesOnlyPath", old: bothTargets, req: codersdk.UpdateUserSecretRequest{EnvName: ptr.Ref("")}, wantErr: errUserSecretEnvTargetRequired}, + {name: "ReEnableFileOnly", old: disabledFileOnly, req: codersdk.UpdateUserSecretRequest{Enabled: ptr.Ref(true)}, wantErr: errUserSecretEnvTargetRequired}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := userSecretFilePathPolicyError(tt.old, tt.req) + if tt.wantErr == nil { + require.NoError(t, err) + return + } + require.ErrorIs(t, err, tt.wantErr) + }) + } +} diff --git a/codersdk/deployment.go b/codersdk/deployment.go index f42258c885..e79b94774e 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -738,6 +738,7 @@ type DeploymentValues struct { DisableWorkspaceSharing serpent.Bool `json:"disable_workspace_sharing,omitempty" typescript:",notnull"` DisableChatSharing serpent.Bool `json:"disable_chat_sharing,omitempty" typescript:",notnull"` DisableWorkspaceAgentContextSync serpent.Bool `json:"disable_workspace_agent_context_sync,omitempty" typescript:",notnull"` + DisableUserSecretFilePath serpent.Bool `json:"disable_user_secret_file_path,omitempty" typescript:",notnull"` ProxyHealthStatusInterval serpent.Duration `json:"proxy_health_status_interval,omitempty" typescript:",notnull"` EnableTerraformDebugMode serpent.Bool `json:"enable_terraform_debug_mode,omitempty" typescript:",notnull"` UserQuietHoursSchedule UserQuietHoursScheduleConfig `json:"user_quiet_hours_schedule,omitempty" typescript:",notnull"` @@ -3809,6 +3810,15 @@ communicating directly.`, Value: &c.DisableWorkspaceAgentContextSync, YAML: "disableWorkspaceAgentContextSync", }, + { + Name: "Disable User Secret File Path", + Description: "Disable Coder-managed file path delivery for user secrets. Stored paths remain until users clear them and resume if this setting is turned off.", + Flag: "disable-user-secret-file-path", + Env: "CODER_DISABLE_USER_SECRET_FILE_PATH", + + Value: &c.DisableUserSecretFilePath, + YAML: "disableUserSecretFilePath", + }, { Name: "Session Duration", Description: "The token expiry duration for browser sessions. Sessions may last longer if they are actively making requests, but this functionality can be disabled via --disable-session-expiry-refresh.", diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 5ff75b7f5d..ce163b95ce 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -1428,6 +1428,46 @@ func TestRetentionConfigParsing(t *testing.T) { } } +func TestDisableUserSecretFilePath(t *testing.T) { + t.Parallel() + + dv := codersdk.DeploymentValues{} + opts := dv.Options() + require.NoError(t, opts.SetDefaults()) + require.False(t, dv.DisableUserSecretFilePath.Value(), "must default to false") + + var opt serpent.Option + for _, o := range opts { + if o.Value == &dv.DisableUserSecretFilePath { + opt = o + break + } + } + require.NotEmpty(t, opt.Flag, "option must be registered") + assert.Equal(t, "disable-user-secret-file-path", opt.Flag) + assert.Equal(t, "CODER_DISABLE_USER_SECRET_FILE_PATH", opt.Env) + assert.Equal(t, "disableUserSecretFilePath", opt.YAML) + + require.NoError(t, opts.ParseEnv([]serpent.EnvVar{ + {Name: "CODER_DISABLE_USER_SECRET_FILE_PATH", Value: "true"}, + })) + require.True(t, dv.DisableUserSecretFilePath.Value(), "env must set the value") + + yamlDV := codersdk.DeploymentValues{} + yamlOpts := yamlDV.Options() + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("disableUserSecretFilePath: true\n"), &node)) + require.NoError(t, node.Decode(&yamlOpts)) + require.True(t, yamlDV.DisableUserSecretFilePath.Value(), "yaml must set the value") + + // The option is not a secret, so telemetry and the config endpoint + // must keep reporting it after sanitization. + full := codersdk.DeploymentValues{DisableUserSecretFilePath: true} + sanitized, err := full.WithoutSecrets() + require.NoError(t, err) + require.True(t, sanitized.DisableUserSecretFilePath.Value()) +} + func TestChatAIGatewayRoutingEnabledDefault(t *testing.T) { t.Parallel() diff --git a/codersdk/usersecrets.go b/codersdk/usersecrets.go index a340d8e638..a0a3e2ee20 100644 --- a/codersdk/usersecrets.go +++ b/codersdk/usersecrets.go @@ -30,8 +30,9 @@ type UserSecret struct { // secret. Name and Value are required. An enabled secret must have at // least one of EnvName or FilePath non-empty so it has an injection // target; to keep a secret without injecting it, set Enabled to false. -// All other fields are optional and default to empty string. Enabled -// defaults to true when omitted. +// A deployment may disable file path delivery, which rejects a +// non-empty FilePath. All other fields are optional and default to +// empty string. Enabled defaults to true when omitted. type CreateUserSecretRequest struct { Name string `json:"name"` Value string `json:"value"` @@ -47,6 +48,8 @@ type CreateUserSecretRequest struct { // to empty string). If the post-update row is enabled it must still // have at least one of EnvName or FilePath non-empty; clearing both // targets is only allowed when the secret is (or becomes) disabled. +// When a deployment disables file path delivery, an enabled row also +// requires EnvName. type UpdateUserSecretRequest struct { Value *string `json:"value,omitempty"` Description *string `json:"description,omitempty"` @@ -55,6 +58,31 @@ type UpdateUserSecretRequest struct { Enabled *bool `json:"enabled,omitempty"` } +// UserSecretsCapabilities reports which user secret delivery targets the +// deployment allows. Any authenticated user can read it, unlike the full +// deployment configuration. +type UserSecretsCapabilities struct { + // FilePathDeliveryEnabled reports whether Coder writes stored file paths + // into workspaces. Stored paths are preserved either way. + FilePathDeliveryEnabled bool `json:"file_path_delivery_enabled"` +} + +// UserSecretsCapabilities reads the deployment's user secret delivery policy. +// Deployments that predate this endpoint return a 404 error, which callers can +// treat as "policy unknown" rather than a failure. +func (c *Client) UserSecretsCapabilities(ctx context.Context) (UserSecretsCapabilities, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/deployment/user-secrets/capabilities", nil) + if err != nil { + return UserSecretsCapabilities{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return UserSecretsCapabilities{}, ReadBodyAsError(res) + } + var capabilities UserSecretsCapabilities + return capabilities, ReadBodyAsJSON(res, &capabilities) +} + func (c *Client) CreateUserSecret(ctx context.Context, user string, req CreateUserSecretRequest) (UserSecret, error) { res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/secrets", user), req) if err != nil { diff --git a/docs/admin/setup/configuration-reference.md b/docs/admin/setup/configuration-reference.md index b97f648f20..a442a10e8d 100644 --- a/docs/admin/setup/configuration-reference.md +++ b/docs/admin/setup/configuration-reference.md @@ -73,6 +73,14 @@ Disable workspace apps that are not served from subdomains. Path-based apps can - CLI flag: [`--disable-path-apps`](../../reference/cli/server.md#--disable-path-apps) - YAML key: `disablePathApps` +### Disable user secret file path + +Disable Coder-managed file path delivery for user secrets. Stored paths remain until users clear them and resume if this setting is turned off. + +- Environment variable: `CODER_DISABLE_USER_SECRET_FILE_PATH` +- CLI flag: [`--disable-user-secret-file-path`](../../reference/cli/server.md#--disable-user-secret-file-path) +- YAML key: `disableUserSecretFilePath` + ### Disable workspace agent context sync Stop persisting workspace agent context snapshots (instructions, skills, and MCP state used for pinned chat context). When set, coderd rejects agent context pushes as unimplemented and agents stop sending them; chats cannot pin workspace context. Use this to shed the database write load of context sync on large deployments. diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index 70b6179d99..0c0634aa3c 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -311,6 +311,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, + "disable_user_secret_file_path": true, "disable_workspace_agent_context_sync": true, "disable_workspace_sharing": true, "docs_url": { @@ -816,6 +817,37 @@ curl -X GET http://coder-server:8080/api/v2/deployment/stats \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get user secrets capabilities + +### Code samples + +```sh +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/deployment/user-secrets/capabilities \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /api/v2/deployment/user-secrets/capabilities` + +### Example responses + +> 200 Response + +```json +{ + "file_path_delivery_enabled": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|--------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UserSecretsCapabilities](schemas.md#codersdkusersecretscapabilities) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get enabled experiments ### Code samples diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index bd6df6802e..1ff247cb87 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -7503,6 +7503,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, + "disable_user_secret_file_path": true, "disable_workspace_agent_context_sync": true, "disable_workspace_sharing": true, "docs_url": { @@ -8135,6 +8136,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, + "disable_user_secret_file_path": true, "disable_workspace_agent_context_sync": true, "disable_workspace_sharing": true, "docs_url": { @@ -8534,6 +8536,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `disable_owner_workspace_exec` | boolean | false | | | | `disable_password_auth` | boolean | false | | | | `disable_path_apps` | boolean | false | | | +| `disable_user_secret_file_path` | boolean | false | | | | `disable_workspace_agent_context_sync` | boolean | false | | | | `disable_workspace_sharing` | boolean | false | | | | `docs_url` | [serpent.URL](#serpenturl) | false | | | @@ -16936,6 +16939,20 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `name` | string | false | | | | `updated_at` | string | false | | | +## codersdk.UserSecretsCapabilities + +```json +{ + "file_path_delivery_enabled": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|------------------------------|---------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------| +| `file_path_delivery_enabled` | boolean | false | | File path delivery enabled reports whether Coder writes stored file paths into workspaces. Stored paths are preserved either way. | + ## codersdk.UserSkill ```json diff --git a/docs/reference/api/secrets.md b/docs/reference/api/secrets.md index f64bcaa091..2156bb8162 100644 --- a/docs/reference/api/secrets.md +++ b/docs/reference/api/secrets.md @@ -120,9 +120,11 @@ curl -X POST http://coder-server:8080/api/v2/users/{user}/secrets \ ### Responses -| Status | Meaning | Description | Schema | -|--------|--------------------------------------------------------------|-------------|------------------------------------------------------| -| 201 | [Created](https://tools.ietf.org/html/rfc7231#section-6.3.2) | Created | [codersdk.UserSecret](schemas.md#codersdkusersecret) | +| Status | Meaning | Description | Schema | +|--------|------------------------------------------------------------------|-------------|------------------------------------------------------| +| 201 | [Created](https://tools.ietf.org/html/rfc7231#section-6.3.2) | Created | [codersdk.UserSecret](schemas.md#codersdkusersecret) | +| 400 | [Bad Request](https://tools.ietf.org/html/rfc7231#section-6.5.1) | Bad Request | [codersdk.Response](schemas.md#codersdkresponse) | +| 409 | [Conflict](https://tools.ietf.org/html/rfc7231#section-6.5.8) | Conflict | [codersdk.Response](schemas.md#codersdkresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -327,8 +329,10 @@ curl -X PATCH http://coder-server:8080/api/v2/users/{user}/secrets/{name} \ ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|------------------------------------------------------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UserSecret](schemas.md#codersdkusersecret) | +| Status | Meaning | Description | Schema | +|--------|------------------------------------------------------------------|-------------|------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UserSecret](schemas.md#codersdkusersecret) | +| 400 | [Bad Request](https://tools.ietf.org/html/rfc7231#section-6.5.1) | Bad Request | [codersdk.Response](schemas.md#codersdkresponse) | +| 409 | [Conflict](https://tools.ietf.org/html/rfc7231#section-6.5.8) | Conflict | [codersdk.Response](schemas.md#codersdkresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 4805d96574..de888dce67 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1236,6 +1236,16 @@ Disable chat sharing. Chat ACL checking is disabled and only owners can access t Stop persisting workspace agent context snapshots (instructions, skills, and MCP state used for pinned chat context). When set, coderd rejects agent context pushes as unimplemented and agents stop sending them; chats cannot pin workspace context. Use this to shed the database write load of context sync on large deployments. +### --disable-user-secret-file-path + +| | | +|-------------|---------------------------------------------------| +| Type | bool | +| Environment | $CODER_DISABLE_USER_SECRET_FILE_PATH | +| YAML | disableUserSecretFilePath | + +Disable Coder-managed file path delivery for user secrets. Stored paths remain until users clear them and resume if this setting is turned off. + ### --session-duration | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index a74fc0a5f1..15bee916a7 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -49,6 +49,11 @@ OPTIONS: the workspace serves malicious JavaScript. This is recommended for security purposes if a --wildcard-access-url is configured. + --disable-user-secret-file-path bool, $CODER_DISABLE_USER_SECRET_FILE_PATH + Disable Coder-managed file path delivery for user secrets. Stored + paths remain until users clear them and resume if this setting is + turned off. + --disable-workspace-agent-context-sync bool, $CODER_DISABLE_WORKSPACE_AGENT_CONTEXT_SYNC Stop persisting workspace agent context snapshots (instructions, skills, and MCP state used for pinned chat context). When set, coderd diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 44b109d83c..fd089256dc 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -939,6 +939,17 @@ func (db *dbCrypt) GetUserSecretByUserIDAndName(ctx context.Context, arg databas return secret, nil } +func (db *dbCrypt) GetUserSecretByUserIDAndNameForUpdate(ctx context.Context, arg database.GetUserSecretByUserIDAndNameForUpdateParams) (database.UserSecret, error) { + secret, err := db.Store.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) + if err != nil { + return database.UserSecret{}, err + } + if err := db.decryptField(&secret.Value, secret.ValueKeyID); err != nil { + return database.UserSecret{}, err + } + return secret, nil +} + func (db *dbCrypt) ListUserSecretsWithValues(ctx context.Context, userID uuid.UUID) ([]database.UserSecret, error) { secrets, err := db.Store.ListUserSecretsWithValues(ctx, userID) if err != nil { diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index 47fe8d9439..4a9532569c 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -1886,6 +1886,23 @@ func TestUserSecrets(t *testing.T) { require.Equal(t, rawBefore.ValueKeyID, rawAfter.ValueKeyID) }) + t.Run("GetUserSecretForUpdateDecryptsValue", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + secret := insertUserSecret(t, crypt, ciphers) + arg := database.GetUserSecretByUserIDAndNameForUpdateParams{ + UserID: secret.UserID, Name: secret.Name, + } + + got, err := crypt.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) + require.NoError(t, err) + require.Equal(t, initialValue, got.Value) + + raw, err := db.GetUserSecretByUserIDAndNameForUpdate(ctx, arg) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], raw.Value, initialValue) + }) + t.Run("GetUserSecretDecryptErr", func(t *testing.T) { t.Parallel() db, crypt, ciphers := setup(t) @@ -1906,6 +1923,26 @@ func TestUserSecrets(t *testing.T) { require.ErrorAs(t, err, &derr) }) + t.Run("GetUserSecretForUpdateDecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + dbgen.UserSecret(t, db, database.UserSecret{ + UserID: user.ID, + Name: "corrupt-secret-for-update", + Value: fakeBase64RandomData(t, 32), + ValueKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.GetUserSecretByUserIDAndNameForUpdate(ctx, database.GetUserSecretByUserIDAndNameForUpdateParams{ + UserID: user.ID, + Name: "corrupt-secret-for-update", + }) + require.Error(t, err) + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr) + }) + t.Run("ListUserSecretsWithValuesDecryptErr", func(t *testing.T) { t.Parallel() db, crypt, ciphers := setup(t) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index bd59ec2fed..9bce83a5a7 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -4336,8 +4336,9 @@ export interface CreateUserRequestWithOrgs { * secret. Name and Value are required. An enabled secret must have at * least one of EnvName or FilePath non-empty so it has an injection * target; to keep a secret without injecting it, set Enabled to false. - * All other fields are optional and default to empty string. Enabled - * defaults to true when omitted. + * A deployment may disable file path delivery, which rejects a + * non-empty FilePath. All other fields are optional and default to + * empty string. Enabled defaults to true when omitted. */ export interface CreateUserSecretRequest { readonly name: string; @@ -4839,6 +4840,7 @@ export interface DeploymentValues { readonly disable_workspace_sharing?: boolean; readonly disable_chat_sharing?: boolean; readonly disable_workspace_agent_context_sync?: boolean; + readonly disable_user_secret_file_path?: boolean; readonly proxy_health_status_interval?: number; readonly enable_terraform_debug_mode?: boolean; readonly user_quiet_hours_schedule?: UserQuietHoursScheduleConfig; @@ -10295,6 +10297,8 @@ export interface UpdateUserQuietHoursScheduleRequest { * to empty string). If the post-update row is enabled it must still * have at least one of EnvName or FilePath non-empty; clearing both * targets is only allowed when the secret is (or becomes) disabled. + * When a deployment disables file path delivery, an enabled row also + * requires EnvName. */ export interface UpdateUserSecretRequest { readonly value?: string; @@ -10851,6 +10855,20 @@ export const UserSecretNameField = "name"; */ export const UserSecretValueField = "value"; +// From codersdk/usersecrets.go +/** + * UserSecretsCapabilities reports which user secret delivery targets the + * deployment allows. Any authenticated user can read it, unlike the full + * deployment configuration. + */ +export interface UserSecretsCapabilities { + /** + * FilePathDeliveryEnabled reports whether Coder writes stored file paths + * into workspaces. Stored paths are preserved either way. + */ + readonly file_path_delivery_enabled: boolean; +} + // From codersdk/userskills.go /** * UserSkill represents a user skill with its raw Markdown content.