feat: add secret value and file path validation (#24269)

zedkipp · web-flow · commit 4854f33678aa · 2026-04-13T07:24:34.000-06:00
Add secret value validation to reject null bytes and values exceeding 32KB.
The 32KB limit applies uniformly to both env var and file secrets because the
value field is shared and the destination can change after creation.

Add file path validation to also reject null bytes and paths exceeding 4096
bytes.

Wire up secret value validation into both POST and PATCH handlers.
diff --git a/coderd/usersecrets.go b/coderd/usersecrets.go
@@ -46,6 +46,13 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) {
 		})
 		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
+	}
 	envOpts := codersdk.UserSecretEnvValidationOptions{
 		AIGatewayEnabled: api.DeploymentValues.AI.BridgeConfig.Enabled.Value(),
 	}
@@ -212,6 +219,13 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) {
 		FilePath:          "",
 	}
 	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
+		}
 		params.Value = *req.Value
 	}
 	if req.Description != nil {
diff --git a/coderd/usersecrets_test.go b/coderd/usersecrets_test.go
@@ -2,6 +2,7 @@ package coderd_test
 
 import (
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -188,6 +189,36 @@ func TestPostUserSecret(t *testing.T) {
 		require.ErrorAs(t, err, &sdkErr)
 		assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
 	})
+
+	t.Run("NullByteInValue", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			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")
+	})
+
+	t.Run("OversizedValue", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			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")
+	})
 }
 
 func TestGetUserSecrets(t *testing.T) {
@@ -372,6 +403,27 @@ func TestPatchUserSecret(t *testing.T) {
 		require.ErrorAs(t, err, &sdkErr)
 		assert.Equal(t, http.StatusConflict, sdkErr.StatusCode())
 	})
+
+	t.Run("InvalidValue", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:  "patch-invalid-val",
+			Value: "good-value",
+		})
+		require.NoError(t, err)
+
+		badVal := "before\x00after"
+		_, 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")
+	})
 }
 
 func TestDeleteUserSecret(t *testing.T) {
diff --git a/codersdk/usersecretvalidation.go b/codersdk/usersecretvalidation.go
@@ -7,6 +7,27 @@ import (
 	"golang.org/x/xerrors"
 )
 
+const (
+	// MaxSecretValueSize is the maximum size of a user secret value
+	// in bytes. This limit applies uniformly to both env var and
+	// file-destined secrets because the value field is shared and
+	// the destination can change after creation. 32KB is generous
+	// for env vars (most are under 1KB) but necessary for file
+	// content like SSH keys, TLS certificate chains, and JSON
+	// configs. We are not trying to be overly restrictive here;
+	// users can use the full 32KB for env var values even though
+	// it would be unusual.
+	MaxSecretValueSize = 32 * 1024 // 32KB
+
+	// maxFilePathLength is the maximum length of a file path for
+	// a user secret. Matches Linux PATH_MAX, which is the common
+	// case since workspace agents almost always run on Linux.
+	// This does not catch all Windows path length edge cases
+	// (legacy MAX_PATH is 260), but the agent will surface a
+	// runtime error if the write fails.
+	maxFilePathLength = 4096
+)
+
 // UserSecretEnvValidationOptions controls deployment-aware behavior
 // in environment variable name validation.
 type UserSecretEnvValidationOptions struct {
@@ -177,15 +198,38 @@ func UserSecretEnvNameValid(s string, opts UserSecretEnvValidationOptions) error
 
 // UserSecretFilePathValid validates a file path for a user secret.
 // Empty string is allowed (means no file injection). Non-empty paths
-// must start with ~/ or /.
+// must start with ~/ or /, must not contain null bytes, and must not
+// exceed 4096 bytes.
 func UserSecretFilePathValid(s string) error {
 	if s == "" {
 		return nil
 	}
 
-	if strings.HasPrefix(s, "~/") || strings.HasPrefix(s, "/") {
-		return nil
+	if !strings.HasPrefix(s, "~/") && !strings.HasPrefix(s, "/") {
+		return xerrors.New("file path must start with ~/ or /")
+	}
+
+	if strings.Contains(s, "\x00") {
+		return xerrors.New("file path must not contain null bytes")
+	}
+
+	if len(s) > maxFilePathLength {
+		return xerrors.Errorf("file path must not exceed %d bytes", maxFilePathLength)
 	}
 
-	return xerrors.New("file path must start with ~/ or /")
+	return nil
+}
+
+// UserSecretValueValid validates a user secret value. The value must
+// not contain null bytes and must not exceed MaxSecretValueSize.
+func UserSecretValueValid(value string) error {
+	if strings.Contains(value, "\x00") {
+		return xerrors.New("secret value must not contain null bytes")
+	}
+
+	if len(value) > MaxSecretValueSize {
+		return xerrors.Errorf("secret value must not exceed %d bytes", MaxSecretValueSize)
+	}
+
+	return nil
 }
diff --git a/codersdk/usersecretvalidation_test.go b/codersdk/usersecretvalidation_test.go
@@ -1,6 +1,7 @@
 package codersdk_test
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -166,6 +167,8 @@ func TestUserSecretFilePathValid(t *testing.T) {
 		{name: "DotRelative", input: ".ssh/id_rsa", wantErr: true},
 		{name: "JustFilename", input: "credentials", wantErr: true},
 		{name: "TildeNoSlash", input: "~foo", wantErr: true},
+		{name: "NullByte", input: "/home/\x00coder", wantErr: true},
+		{name: "TooLong", input: "/" + strings.Repeat("a", 4096), wantErr: true},
 	}
 
 	for _, tt := range tests {
@@ -174,7 +177,36 @@ func TestUserSecretFilePathValid(t *testing.T) {
 			err := codersdk.UserSecretFilePathValid(tt.input)
 			if tt.wantErr {
 				assert.Error(t, err)
-				assert.Contains(t, err.Error(), "must start with")
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestUserSecretValueValid(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		input   string
+		wantErr bool
+	}{
+		{name: "NormalString", input: "my-secret-token"},
+		{name: "Empty", input: ""},
+		{name: "WithNewlines", input: "line1\nline2\nline3"},
+		{name: "WithTabs", input: "key\tvalue"},
+		{name: "NullByte", input: "before\x00after", wantErr: true},
+		{name: "ExactlyAtLimit", input: strings.Repeat("a", codersdk.MaxSecretValueSize)},
+		{name: "OverLimit", input: strings.Repeat("a", codersdk.MaxSecretValueSize+1), wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			err := codersdk.UserSecretValueValid(tt.input)
+			if tt.wantErr {
+				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 			}
diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts
