diff --git a/coderd/usersecrets.go b/coderd/usersecrets.go index 57aa6d9f16..09be0a964d 100644 --- a/coderd/usersecrets.go +++ b/coderd/usersecrets.go @@ -20,6 +20,13 @@ import ( "github.com/coder/coder/v2/codersdk" ) +const ( + userSecretNameField = "name" + userSecretValueField = "value" + userSecretEnvNameField = "env_name" + userSecretFilePathField = "file_path" +) + // @Summary Create a new user secret // @ID create-a-new-user-secret // @Security CoderSessionToken @@ -49,37 +56,8 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) { return } - if req.Name == "" { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Name is required.", - }) - return - } - if req.Value == "" { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Value is required.", - }) - return - } - if err := codersdk.UserSecretValueValid(req.Value); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid secret value.", - Detail: err.Error(), - }) - return - } - if err := codersdk.UserSecretEnvNameValid(req.EnvName); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid environment variable name.", - Detail: err.Error(), - }) - return - } - if err := codersdk.UserSecretFilePathValid(req.FilePath); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid file path.", - Detail: err.Error(), - }) + if validations := createUserSecretValidationErrors(req); len(validations) > 0 { + writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, validations) return } @@ -94,11 +72,8 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) { FilePath: req.FilePath, }) if err != nil { - if database.IsUniqueViolation(err) { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "A secret with that name, environment variable, or file path already exists.", - Detail: err.Error(), - }) + if validations := userSecretConflictValidationErrors(err); len(validations) > 0 { + writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, validations) return } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -156,7 +131,7 @@ func (api *API) getUserSecrets(rw http.ResponseWriter, r *http.Request) { //noli func (api *API) getUserSecret(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. ctx := r.Context() user := httpmw.UserParam(r) - name := chi.URLParam(r, "name") + name := chi.URLParam(r, userSecretNameField) secret, err := api.Database.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{ UserID: user.ID, @@ -192,7 +167,7 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() user = httpmw.UserParam(r) - name = chi.URLParam(r, "name") + name = chi.URLParam(r, userSecretNameField) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.UserSecret](rw, &audit.RequestParams{ Audit: *auditor, @@ -214,32 +189,9 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { }) return } - if req.EnvName != nil { - if err := codersdk.UserSecretEnvNameValid(*req.EnvName); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid environment variable name.", - Detail: err.Error(), - }) - return - } - } - if req.FilePath != nil { - if err := codersdk.UserSecretFilePathValid(*req.FilePath); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid file path.", - Detail: err.Error(), - }) - return - } - } - if req.Value != nil { - if err := codersdk.UserSecretValueValid(*req.Value); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid secret value.", - Detail: err.Error(), - }) - return - } + if validations := updateUserSecretValidationErrors(req); len(validations) > 0 { + writeUserSecretValidationErrors(ctx, rw, http.StatusBadRequest, validations) + return } params := database.UpdateUserSecretByUserIDAndNameParams{ @@ -300,11 +252,8 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { httpapi.ResourceNotFound(rw) return } - if database.IsUniqueViolation(err) { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "Update would conflict with an existing secret.", - Detail: err.Error(), - }) + if validations := userSecretConflictValidationErrors(err); len(validations) > 0 { + writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, validations) return } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -337,7 +286,7 @@ func (api *API) deleteUserSecret(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() user = httpmw.UserParam(r) - name = chi.URLParam(r, "name") + name = chi.URLParam(r, userSecretNameField) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.UserSecret](rw, &audit.RequestParams{ Audit: *auditor, @@ -374,6 +323,75 @@ func (api *API) deleteUserSecret(rw http.ResponseWriter, r *http.Request) { rw.WriteHeader(http.StatusNoContent) } +func writeUserSecretValidationErrors(ctx context.Context, rw http.ResponseWriter, status int, validations []codersdk.ValidationError) { + httpapi.Write(ctx, rw, status, codersdk.Response{ + Message: "Validation failed.", + Validations: validations, + }) +} + +func createUserSecretValidationErrors(req codersdk.CreateUserSecretRequest) []codersdk.ValidationError { + var validations []codersdk.ValidationError + validations = appendUserSecretValidationError(validations, userSecretNameField, codersdk.UserSecretNameValid(req.Name)) + if req.Value == "" { + validations = append(validations, codersdk.ValidationError{ + Field: userSecretValueField, + Detail: "Value is required.", + }) + } else { + validations = appendUserSecretValidationError(validations, userSecretValueField, codersdk.UserSecretValueValid(req.Value)) + } + validations = appendUserSecretValidationError(validations, userSecretEnvNameField, codersdk.UserSecretEnvNameValid(req.EnvName)) + validations = appendUserSecretValidationError(validations, userSecretFilePathField, codersdk.UserSecretFilePathValid(req.FilePath)) + return validations +} + +func updateUserSecretValidationErrors(req codersdk.UpdateUserSecretRequest) []codersdk.ValidationError { + var validations []codersdk.ValidationError + if req.Value != nil { + validations = appendUserSecretValidationError(validations, userSecretValueField, codersdk.UserSecretValueValid(*req.Value)) + } + if req.EnvName != nil { + validations = appendUserSecretValidationError(validations, userSecretEnvNameField, codersdk.UserSecretEnvNameValid(*req.EnvName)) + } + if req.FilePath != nil { + validations = appendUserSecretValidationError(validations, userSecretFilePathField, codersdk.UserSecretFilePathValid(*req.FilePath)) + } + return validations +} + +func appendUserSecretValidationError(validations []codersdk.ValidationError, field string, err error) []codersdk.ValidationError { + if err == nil { + return validations + } + return append(validations, codersdk.ValidationError{ + Field: field, + Detail: err.Error(), + }) +} + +func userSecretConflictValidationErrors(err error) []codersdk.ValidationError { + switch { + case database.IsUniqueViolation(err, database.UniqueUserSecretsUserNameIndex): + return []codersdk.ValidationError{{ + Field: userSecretNameField, + Detail: "name already in use", + }} + case database.IsUniqueViolation(err, database.UniqueUserSecretsUserEnvNameIndex): + return []codersdk.ValidationError{{ + Field: userSecretEnvNameField, + Detail: "environment variable already in use", + }} + case database.IsUniqueViolation(err, database.UniqueUserSecretsUserFilePathIndex): + return []codersdk.ValidationError{{ + Field: userSecretFilePathField, + Detail: "file path already in use", + }} + default: + return nil + } +} + func (api *API) publishUserSecretEvent(ctx context.Context, event usersecretspubsub.Event) { if err := usersecretspubsub.Publish(api.Pubsub, event); err != nil { api.Logger.Warn(ctx, "failed to publish user secret event", diff --git a/coderd/usersecrets_test.go b/coderd/usersecrets_test.go index a23316dcd2..a869d48777 100644 --- a/coderd/usersecrets_test.go +++ b/coderd/usersecrets_test.go @@ -45,11 +45,7 @@ func TestPostUserSecret(t *testing.T) { _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ Value: "some-value", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) - assert.Contains(t, sdkErr.Message, "Name is required") + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "name", "required") }) t.Run("MissingValue", func(t *testing.T) { @@ -59,11 +55,29 @@ func TestPostUserSecret(t *testing.T) { _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ Name: "missing-value-secret", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) - assert.Contains(t, sdkErr.Message, "Value is required") + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "value", "required") + }) + + t.Run("InvalidName", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + + _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: "foo/bar", + Value: "some-value", + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "name", "must not contain") + }) + + t.Run("WhitespaceName", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + + _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: " github", + Value: "some-value", + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "name", "whitespace") }) t.Run("DuplicateName", func(t *testing.T) { @@ -80,10 +94,7 @@ func TestPostUserSecret(t *testing.T) { Name: "dup-secret", Value: "value2", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusConflict, sdkErr.StatusCode()) + requireSecretValidationEqualsError(t, err, http.StatusConflict, "name", "name already in use") }) t.Run("DuplicateEnvName", func(t *testing.T) { @@ -102,10 +113,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value2", EnvName: "DUPLICATE_ENV", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusConflict, sdkErr.StatusCode()) + requireSecretValidationEqualsError(t, err, http.StatusConflict, "env_name", "environment variable already in use") }) t.Run("DuplicateFilePath", func(t *testing.T) { @@ -124,10 +132,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value2", FilePath: "/tmp/dup-file", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusConflict, sdkErr.StatusCode()) + requireSecretValidationEqualsError(t, err, http.StatusConflict, "file_path", "file path already in use") }) t.Run("InvalidEnvName", func(t *testing.T) { @@ -139,10 +144,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value", EnvName: "1INVALID", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "env_name", "must start") }) t.Run("ReservedEnvName", func(t *testing.T) { @@ -154,10 +156,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value", EnvName: "PATH", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "env_name", "reserved") }) t.Run("CoderPrefixEnvName", func(t *testing.T) { @@ -169,10 +168,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value", EnvName: "CODER_AGENT_TOKEN", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "env_name", "CODER_") }) t.Run("InvalidFilePath", func(t *testing.T) { @@ -184,10 +180,7 @@ func TestPostUserSecret(t *testing.T) { Value: "value", FilePath: "relative/path", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "file_path", "must start") }) t.Run("NullByteInValue", func(t *testing.T) { @@ -198,11 +191,7 @@ func TestPostUserSecret(t *testing.T) { Name: "null-byte-secret", Value: "before\x00after", }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) - assert.Contains(t, sdkErr.Message, "Invalid secret value") + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "value", "null bytes") }) t.Run("OversizedValue", func(t *testing.T) { @@ -213,11 +202,7 @@ func TestPostUserSecret(t *testing.T) { Name: "oversized-secret", Value: strings.Repeat("a", codersdk.MaxSecretValueSize+1), }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) - assert.Contains(t, sdkErr.Message, "Invalid secret value") + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "value", "must not exceed") }) } @@ -371,10 +356,7 @@ func TestPatchUserSecret(t *testing.T) { _, err = client.UpdateUserSecret(ctx, codersdk.Me, "conflict-env-2", codersdk.UpdateUserSecretRequest{ EnvName: &taken, }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusConflict, sdkErr.StatusCode()) + requireSecretValidationEqualsError(t, err, http.StatusConflict, "env_name", "environment variable already in use") }) t.Run("ConflictFilePath", func(t *testing.T) { @@ -398,10 +380,41 @@ func TestPatchUserSecret(t *testing.T) { _, err = client.UpdateUserSecret(ctx, codersdk.Me, "conflict-fp-2", codersdk.UpdateUserSecretRequest{ FilePath: &taken, }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusConflict, sdkErr.StatusCode()) + requireSecretValidationEqualsError(t, err, http.StatusConflict, "file_path", "file path already in use") + }) + + t.Run("InvalidEnvName", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + + _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: "patch-invalid-env", + Value: "good-value", + }) + require.NoError(t, err) + + badEnvName := "1INVALID" + _, err = client.UpdateUserSecret(ctx, codersdk.Me, "patch-invalid-env", codersdk.UpdateUserSecretRequest{ + EnvName: &badEnvName, + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "env_name", "must start") + }) + + t.Run("InvalidFilePath", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + + _, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{ + Name: "patch-invalid-file-path", + Value: "good-value", + }) + require.NoError(t, err) + + badFilePath := "relative/path" + _, err = client.UpdateUserSecret(ctx, codersdk.Me, "patch-invalid-file-path", codersdk.UpdateUserSecretRequest{ + FilePath: &badFilePath, + }) + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "file_path", "must start") }) t.Run("InvalidValue", func(t *testing.T) { @@ -418,14 +431,38 @@ func TestPatchUserSecret(t *testing.T) { _, err = client.UpdateUserSecret(ctx, codersdk.Me, "patch-invalid-val", codersdk.UpdateUserSecretRequest{ Value: &badVal, }) - require.Error(t, err) - var sdkErr *codersdk.Error - require.ErrorAs(t, err, &sdkErr) - assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) - assert.Contains(t, sdkErr.Message, "Invalid secret value") + requireSecretValidationContainsError(t, err, http.StatusBadRequest, "value", "null bytes") }) } +func requireSecretValidationContainsError(t *testing.T, err error, status int, field string, detailContains string) { + t.Helper() + validation := requireSecretValidation(t, err, status, field) + assert.Contains(t, validation.Detail, detailContains) +} + +func requireSecretValidationEqualsError(t *testing.T, err error, status int, field string, detail string) { + t.Helper() + validation := requireSecretValidation(t, err, status, field) + assert.Equal(t, detail, validation.Detail) +} + +func requireSecretValidation(t *testing.T, err error, status int, field string) codersdk.ValidationError { + t.Helper() + + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + assert.Equal(t, status, sdkErr.StatusCode()) + for _, validation := range sdkErr.Validations { + if validation.Field == field { + return validation + } + } + require.Failf(t, "missing validation", "field %q not found in %#v", field, sdkErr.Validations) + return codersdk.ValidationError{} +} + func TestDeleteUserSecret(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) diff --git a/codersdk/usersecretvalidation.go b/codersdk/usersecretvalidation.go index 7702f95085..841e7acce1 100644 --- a/codersdk/usersecretvalidation.go +++ b/codersdk/usersecretvalidation.go @@ -132,6 +132,24 @@ var ( } ) +// UserSecretNameValid validates a user secret name. Names are used in +// API route path segments, so they must not include route separators. +func UserSecretNameValid(s string) error { + if strings.TrimSpace(s) == "" { + return xerrors.New("Name is required.") + } + + if strings.TrimSpace(s) != s { + return xerrors.New("Name must not have leading or trailing whitespace.") + } + + if strings.ContainsAny(s, "/?#") { + return xerrors.New("Name must not contain /, ?, or #.") + } + + return nil +} + // UserSecretEnvNameValid validates an environment variable name for // a user secret. Empty string is allowed (means no env injection). func UserSecretEnvNameValid(s string) error { diff --git a/codersdk/usersecretvalidation_test.go b/codersdk/usersecretvalidation_test.go index 07c23eda93..f381bfea6a 100644 --- a/codersdk/usersecretvalidation_test.go +++ b/codersdk/usersecretvalidation_test.go @@ -9,6 +9,43 @@ import ( "github.com/coder/coder/v2/codersdk" ) +func TestUserSecretNameValid(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantErr bool + errMsg string + }{ + {name: "Simple", input: "github-token"}, + {name: "WithUnderscore", input: "github_token"}, + {name: "WithDot", input: "github.token"}, + {name: "Empty", input: "", wantErr: true, errMsg: "required"}, + {name: "WhitespaceOnly", input: " ", wantErr: true, errMsg: "required"}, + {name: "LeadingWhitespace", input: " github", wantErr: true, errMsg: "whitespace"}, + {name: "TrailingWhitespace", input: "github ", wantErr: true, errMsg: "whitespace"}, + {name: "Slash", input: "foo/bar", wantErr: true, errMsg: "must not contain"}, + {name: "Question", input: "foo?bar", wantErr: true, errMsg: "must not contain"}, + {name: "Fragment", input: "foo#bar", wantErr: true, errMsg: "must not contain"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := codersdk.UserSecretNameValid(tt.input) + if tt.wantErr { + assert.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + assert.NoError(t, err) + } + }) + } +} + func TestUserSecretEnvNameValid(t *testing.T) { t.Parallel() diff --git a/site/src/api/api.test.ts b/site/src/api/api.test.ts index a966f673e4..729878fb96 100644 --- a/site/src/api/api.test.ts +++ b/site/src/api/api.test.ts @@ -326,4 +326,104 @@ describe("api.ts", () => { expect(axiosInstance.get).toHaveBeenCalledWith(path); }); }); + + describe("user secrets endpoints", () => { + const userId = "me"; + const secretName = "EXAMPLE_TOKEN"; + const secretNameWithPathChars = "foo%2Fbar value"; + const userSecret: TypesGen.UserSecret = { + id: "00000000-0000-0000-0000-000000000001", + name: secretName, + description: "Example token for tests", + env_name: secretName, + file_path: "", + created_at: "2026-05-04T00:00:00Z", + updated_at: "2026-05-04T00:00:00Z", + }; + + it("lists user secrets with the correct method and URL", async () => { + const axiosMockGet = vi.fn().mockResolvedValueOnce({ + data: [userSecret], + }); + axiosInstance.get = axiosMockGet; + + const result = await API.getUserSecrets(userId); + + expect(axiosMockGet).toHaveBeenCalledWith("/api/v2/users/me/secrets"); + expect(result).toStrictEqual([userSecret]); + }); + + it("gets a user secret with the correct method and URL", async () => { + const axiosMockGet = vi.fn().mockResolvedValueOnce({ + data: userSecret, + }); + axiosInstance.get = axiosMockGet; + + const result = await API.getUserSecret(userId, secretNameWithPathChars); + + expect(axiosMockGet).toHaveBeenCalledWith( + "/api/v2/users/me/secrets/foo%252Fbar%20value", + ); + expect(result).toStrictEqual(userSecret); + }); + + it("creates a user secret with the correct method and URL", async () => { + const request: TypesGen.CreateUserSecretRequest = { + name: secretName, + value: "", + description: "Example token for tests", + env_name: secretName, + }; + const axiosMockPost = vi.fn().mockResolvedValueOnce({ + data: userSecret, + }); + axiosInstance.post = axiosMockPost; + + const result = await API.createUserSecret(userId, request); + + expect(axiosMockPost).toHaveBeenCalledWith( + "/api/v2/users/me/secrets", + request, + ); + expect(result).toStrictEqual(userSecret); + }); + + it("updates a user secret with the correct method and URL", async () => { + const request: TypesGen.UpdateUserSecretRequest = { + description: "Updated example token for tests", + }; + const updatedSecret: TypesGen.UserSecret = { + ...userSecret, + description: "Updated example token for tests", + updated_at: "2026-05-04T00:01:00Z", + }; + const axiosMockPatch = vi.fn().mockResolvedValueOnce({ + data: updatedSecret, + }); + axiosInstance.patch = axiosMockPatch; + + const result = await API.updateUserSecret( + userId, + secretNameWithPathChars, + request, + ); + + expect(axiosMockPatch).toHaveBeenCalledWith( + "/api/v2/users/me/secrets/foo%252Fbar%20value", + request, + ); + expect(result).toStrictEqual(updatedSecret); + }); + + it("deletes a user secret with the correct method and URL", async () => { + const axiosMockDelete = vi.fn().mockResolvedValueOnce(undefined); + axiosInstance.delete = axiosMockDelete; + + await API.deleteUserSecret(userId, secretNameWithPathChars); + + expect(axiosMockDelete).toHaveBeenCalledWith( + "/api/v2/users/me/secrets/foo%252Fbar%20value", + ); + }); + }); }); diff --git a/site/src/api/api.ts b/site/src/api/api.ts index eff0e3f957..910a6685d0 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1760,6 +1760,56 @@ class ApiMethods { return response.data; }; + getUserSecrets = async (userId: string): Promise => { + const response = await this.axios.get( + `/api/v2/users/${encodeURIComponent(userId)}/secrets`, + ); + + return response.data; + }; + + getUserSecret = async ( + userId: string, + name: string, + ): Promise => { + const response = await this.axios.get( + `/api/v2/users/${encodeURIComponent(userId)}/secrets/${encodeURIComponent(name)}`, + ); + + return response.data; + }; + + createUserSecret = async ( + userId: string, + request: TypesGen.CreateUserSecretRequest, + ): Promise => { + const response = await this.axios.post( + `/api/v2/users/${encodeURIComponent(userId)}/secrets`, + request, + ); + + return response.data; + }; + + updateUserSecret = async ( + userId: string, + name: string, + request: TypesGen.UpdateUserSecretRequest, + ): Promise => { + const response = await this.axios.patch( + `/api/v2/users/${encodeURIComponent(userId)}/secrets/${encodeURIComponent(name)}`, + request, + ); + + return response.data; + }; + + deleteUserSecret = async (userId: string, name: string): Promise => { + await this.axios.delete( + `/api/v2/users/${encodeURIComponent(userId)}/secrets/${encodeURIComponent(name)}`, + ); + }; + getWorkspaceBuilds = async ( workspaceId: string, req?: TypesGen.WorkspaceBuildsRequest, diff --git a/site/src/pages/UserSettingsPage/SecretsPage/secretForm.test.ts b/site/src/pages/UserSettingsPage/SecretsPage/secretForm.test.ts new file mode 100644 index 0000000000..26f8a1dd19 --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecretsPage/secretForm.test.ts @@ -0,0 +1,156 @@ +import type { UserSecret } from "#/api/typesGenerated"; +import { mockApiError } from "#/testHelpers/entities"; +import { + buildCreateUserSecretRequest, + buildUpdateUserSecretRequest, + getCreateSecretRequiredFieldErrors, + mapSecretApiErrorToFormErrors, +} from "./secretForm"; + +const existingSecrets: UserSecret[] = [ + { + id: "11111111-1111-1111-1111-111111111111", + name: "github", + description: "GitHub token", + env_name: "GITHUB_TOKEN", + file_path: "", + created_at: "2026-05-04T00:00:00Z", + updated_at: "2026-05-04T00:00:00Z", + }, + { + id: "22222222-2222-2222-2222-222222222222", + name: "anthropic", + description: "", + env_name: "ANTHROPIC_API_KEY", + file_path: "~/.config/anthropic/key", + created_at: "2026-05-04T00:00:00Z", + updated_at: "2026-05-04T00:00:00Z", + }, +]; + +describe("getCreateSecretRequiredFieldErrors", () => { + it("requires name and value on create", () => { + expect( + getCreateSecretRequiredFieldErrors({ + name: "", + value: "", + }), + ).toEqual({ + name: "Name is required.", + value: "Value is required.", + }); + }); + + it("requires a non-whitespace name", () => { + expect( + getCreateSecretRequiredFieldErrors({ + name: " ", + value: "some value", + }), + ).toEqual({ + name: "Name is required.", + }); + }); +}); + +describe("payload builders", () => { + it("builds create payloads from form values", () => { + expect( + buildCreateUserSecretRequest({ + name: "github", + value: "example-value", + description: "GitHub token", + env_name: "GITHUB_TOKEN", + file_path: "", + }), + ).toEqual({ + name: "github", + value: "example-value", + description: "GitHub token", + env_name: "GITHUB_TOKEN", + }); + }); + + it("sends only changed update fields", () => { + expect( + buildUpdateUserSecretRequest(existingSecrets[0], { + name: "github", + value: "", + description: "Updated description", + env_name: "GITHUB_TOKEN", + file_path: "~/secrets/github", + }), + ).toEqual({ + description: "Updated description", + file_path: "~/secrets/github", + }); + }); + + it("includes replacement values only when provided", () => { + expect( + buildUpdateUserSecretRequest(existingSecrets[0], { + name: "github", + value: "replacement-value", + description: "GitHub token", + env_name: "GITHUB_TOKEN", + file_path: "", + }), + ).toEqual({ + value: "replacement-value", + }); + }); +}); + +describe("mapSecretApiErrorToFormErrors", () => { + it("maps structured API validation errors to fields", () => { + expect( + mapSecretApiErrorToFormErrors( + mockApiError({ + message: "Validation failed.", + validations: [ + { field: "name", detail: "Name already in use." }, + { field: "env_name", detail: "Use a different variable." }, + { field: "file_path", detail: "Use an absolute path." }, + { field: "unknown", detail: "Ignored." }, + ], + }), + ).fieldErrors, + ).toEqual({ + name: "Name already in use.", + env_name: "Use a different variable.", + file_path: "Use an absolute path.", + }); + }); + + it("maps unstructured API validation errors to a form error", () => { + expect( + mapSecretApiErrorToFormErrors( + mockApiError({ + message: "Invalid environment variable name.", + detail: "Backend detail.", + }), + ), + ).toEqual({ + fieldErrors: {}, + formError: "Backend detail.", + }); + }); + + it("maps generic create conflicts to a form error", () => { + expect( + mapSecretApiErrorToFormErrors({ + isAxiosError: true, + status: 409, + response: { + status: 409, + data: { + message: + "A secret with that name, environment variable, or file path already exists.", + }, + }, + }).formError, + ).toBe( + "A secret with that name, environment variable, or file path already exists.", + ); + }); +}); diff --git a/site/src/pages/UserSettingsPage/SecretsPage/secretForm.ts b/site/src/pages/UserSettingsPage/SecretsPage/secretForm.ts new file mode 100644 index 0000000000..09bf8551fa --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecretsPage/secretForm.ts @@ -0,0 +1,147 @@ +import { + type ApiErrorResponse, + isApiError, + isApiErrorResponse, + mapApiErrorToFieldErrors, +} from "#/api/errors"; +import type { + CreateUserSecretRequest, + UpdateUserSecretRequest, + UserSecret, +} from "#/api/typesGenerated"; + +interface SecretFormValues { + name: string; + value: string; + description: string; + env_name: string; + file_path: string; +} + +type SecretFormField = keyof SecretFormValues; + +type SecretFieldErrors = Partial>; + +interface SecretFormErrors { + fieldErrors: SecretFieldErrors; + formError?: string; +} + +export const getCreateSecretRequiredFieldErrors = ( + values: Pick, +): SecretFieldErrors => { + const errors: SecretFieldErrors = {}; + if (values.name.trim() === "") { + errors.name = "Name is required."; + } + if (values.value === "") { + errors.value = "Value is required."; + } + return errors; +}; + +export const buildCreateUserSecretRequest = ( + values: SecretFormValues, +): CreateUserSecretRequest => { + return stripEmptyOptionalFields({ + name: values.name, + value: values.value, + description: values.description, + env_name: values.env_name, + file_path: values.file_path, + }); +}; + +export const buildUpdateUserSecretRequest = ( + secret: UserSecret, + values: SecretFormValues, +): UpdateUserSecretRequest => { + return { + ...(values.value !== "" ? { value: values.value } : {}), + ...(values.description !== secret.description + ? { description: values.description } + : {}), + ...(values.env_name !== secret.env_name + ? { env_name: values.env_name } + : {}), + ...(values.file_path !== secret.file_path + ? { file_path: values.file_path } + : {}), + }; +}; + +export const mapSecretApiErrorToFormErrors = ( + error: unknown, +): SecretFormErrors => { + const apiError = getApiError(error); + if (!apiError) { + return { + fieldErrors: {}, + formError: "Something went wrong.", + }; + } + + const fieldErrors = getSecretFieldErrors(apiError.response); + if (Object.keys(fieldErrors).length > 0) { + return { fieldErrors }; + } + + return { + fieldErrors: {}, + formError: apiError.response.detail ?? apiError.response.message, + }; +}; + +const secretFormFieldLookup: Record = { + name: true, + value: true, + description: true, + env_name: true, + file_path: true, +}; + +function getSecretFieldErrors(response: ApiErrorResponse): SecretFieldErrors { + const apiFieldErrors = mapApiErrorToFieldErrors(response); + const fieldErrors: SecretFieldErrors = {}; + for (const [field, message] of Object.entries(apiFieldErrors)) { + if (isSecretFormField(field)) { + fieldErrors[field] = message; + } + } + return fieldErrors; +} + +function isSecretFormField(field: string): field is SecretFormField { + return Object.hasOwn(secretFormFieldLookup, field); +} + +function getApiError( + error: unknown, +): { status?: number; response: ApiErrorResponse } | undefined { + if (isApiError(error)) { + return { + status: error.response.status ?? error.status, + response: error.response.data, + }; + } + + if (isApiErrorResponse(error)) { + return { + response: error, + }; + } + + return undefined; +} + +function stripEmptyOptionalFields( + request: CreateUserSecretRequest, +): CreateUserSecretRequest { + return { + name: request.name, + value: request.value, + ...(request.description ? { description: request.description } : {}), + ...(request.env_name ? { env_name: request.env_name } : {}), + ...(request.file_path ? { file_path: request.file_path } : {}), + }; +} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index fcdb91cb35..73348109b1 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -568,6 +568,54 @@ export const MockUserAppearanceSettings: TypesGen.UserAppearanceSettings = { terminal_font: "", }; +export const MockUserSecrets: TypesGen.UserSecret[] = [ + { + id: "secret-env-only", + name: "EXAMPLE_TOKEN", + description: "Used by example templates.", + env_name: "EXAMPLE_TOKEN", + file_path: "", + created_at: "2026-04-28T16:30:00Z", + updated_at: "2026-04-30T16:30:00Z", + }, + { + id: "secret-file-only", + name: "config-json", + description: "Mounted as a workspace file.", + env_name: "", + file_path: "~/.config/example/config.json", + created_at: "2026-04-29T16:30:00Z", + updated_at: "2026-05-01T16:30:00Z", + }, + { + id: "secret-env-and-file", + name: "GITHUB_TOKEN", + description: "Available as an environment variable and file.", + env_name: "GITHUB_TOKEN", + file_path: "/var/run/secrets/github-token", + created_at: "2026-04-30T16:30:00Z", + updated_at: "2026-05-02T16:30:00Z", + }, + { + id: "secret-not-injected", + name: "ANTHROPIC_API_KEY", + description: "", + env_name: "", + file_path: "", + created_at: "2026-05-01T16:30:00Z", + updated_at: "2026-05-03T16:30:00Z", + }, + { + id: "secret-openai", + name: "OPENAI_API_KEY", + description: "Used to exercise duplicate validation.", + env_name: "OPENAI_API_KEY", + file_path: "", + created_at: "2026-05-01T18:30:00Z", + updated_at: "2026-05-03T18:30:00Z", + }, +]; + export const MockTasksTabVisible: boolean = false; export const MockOrganizationMember: TypesGen.OrganizationMemberWithUserData = { diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index df4ab4103d..4365e21834 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -1,7 +1,12 @@ import fs from "node:fs"; import path from "node:path"; import { HttpResponse, http } from "msw"; -import type { CreateWorkspaceBuildRequest } from "#/api/typesGenerated"; +import type { + CreateUserSecretRequest, + CreateWorkspaceBuildRequest, + UpdateUserSecretRequest, + UserSecret, +} from "#/api/typesGenerated"; import { permissionChecks } from "#/modules/permissions"; import * as M from "./entities"; import { MockGroup, MockWorkspaceQuota } from "./entities"; @@ -192,6 +197,46 @@ export const handlers = [ http.get("/api/v2/users/:userId/gitsshkey", () => { return HttpResponse.json(M.MockGitSSHKey); }), + http.get("/api/v2/users/:userId/secrets", () => { + return HttpResponse.json(M.MockUserSecrets); + }), + http.get("/api/v2/users/:userId/secrets/:name", ({ params }) => { + const secret = M.MockUserSecrets.find( + (secret) => secret.name === params.name, + ); + if (!secret) { + return HttpResponse.json( + { message: "Secret not found." }, + { status: 404 }, + ); + } + return HttpResponse.json(secret); + }), + http.post("/api/v2/users/:userId/secrets", async ({ request }) => { + const body = (await request.json()) as CreateUserSecretRequest; + return HttpResponse.json(userSecretFromCreateRequest(body), { + status: 201, + }); + }), + http.patch( + "/api/v2/users/:userId/secrets/:name", + async ({ request, params }) => { + const body = (await request.json()) as UpdateUserSecretRequest; + const existing = M.MockUserSecrets.find( + (secret) => secret.name === params.name, + ); + if (!existing) { + return HttpResponse.json( + { message: "Secret not found." }, + { status: 404 }, + ); + } + return HttpResponse.json(userSecretFromUpdateRequest(existing, body)); + }, + ), + http.delete("/api/v2/users/:userId/secrets/:name", () => { + return new HttpResponse(null, { status: 204 }); + }), http.get("/api/v2/users/:userId/workspace/:workspaceName", () => { return HttpResponse.json(M.MockWorkspace); }), @@ -378,3 +423,31 @@ export const handlers = [ return HttpResponse.json(M.MockListeningPortsResponse); }), ]; + +function userSecretFromCreateRequest( + request: CreateUserSecretRequest, +): UserSecret { + const now = "2026-05-04T00:00:00Z"; + return { + id: `secret-${request.name}`, + name: request.name, + description: request.description ?? "", + env_name: request.env_name ?? "", + file_path: request.file_path ?? "", + created_at: now, + updated_at: now, + }; +} + +function userSecretFromUpdateRequest( + secret: UserSecret, + request: UpdateUserSecretRequest, +): UserSecret { + return { + ...secret, + description: request.description ?? secret.description, + env_name: request.env_name ?? secret.env_name, + file_path: request.file_path ?? secret.file_path, + updated_at: "2026-05-04T00:00:00Z", + }; +}