feat: enforce per-user limits on user_secrets (#25588)

Add a Postgres trigger and matching codersdk constants that cap each user's secrets in four dimensions: count (50), total stored value bytes (200 KiB), env-injected stored value bytes (24 KiB), and env name length (256 bytes). Without these caps a user could overflow the 4 MiB DRPC agent manifest, the ~32 KiB Windows process env block, or Linux/macOS ARG_MAX at workspace start. The trigger is the source of truth on aggregates; the handler maps its check_violation error into a 400 that names the per-user budget in stored (post-encryption) bytes. A handler test exercises off-by-one at each cap across POST and PATCH, plus per-user budget isolation. Generated with help from Coder Agents.
2026-09-24 15:04:27 +08:00 · 2026-05-26 14:42:31 -06:00
parent d3155e1cab
commit 47ac4b309a
9 changed files with 672 additions and 28 deletions
@@ -1,6 +1,7 @@
 package coderd_test

 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"testing"
@@ -200,7 +201,7 @@ func TestPostUserSecret(t *testing.T) {

 		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
 			Name:  "oversized-secret",
-			Value: strings.Repeat("a", codersdk.MaxSecretValueSize+1),
+			Value: strings.Repeat("a", codersdk.MaxUserSecretValueBytes+1),
 		})
 		requireSecretValidationContainsError(t, err, http.StatusBadRequest, "value", "must not exceed")
 	})
@@ -463,6 +464,217 @@ func requireSecretValidation(t *testing.T, err error, status int, field string)
 	return codersdk.ValidationError{}
 }

+// TestUserSecretLimits exercises the per-user count and byte caps
+// enforced by enforce_user_secrets_per_user_limits across both POST
+// (creating a new secret) and PATCH (updating an existing one).
+// Each subtest spins up its own server so it can burn the budget
+// without affecting other tests.
+//
+// Each subtest checks three things per cap:
+//
+//   - POST past the cap is rejected with a 400.
+//   - PATCH of an existing row at the cap is accepted; the trigger
+//     uses FILTER (WHERE id IS DISTINCT FROM NEW.id) so an UPDATE
+//     does not double-count its own row.
+//   - A different user's budget is independent; the trigger groups
+//     by user_id.
+func TestUserSecretLimits(t *testing.T) {
+	t.Parallel()
+
+	t.Run("CountLimit", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		client := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, client)
+		otherClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
+
+		// Fill the count budget exactly to the cap.
+		var firstSecret codersdk.UserSecret
+		for i := 0; i < codersdk.MaxUserSecretsPerUserCount; i++ {
+			s, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+				Name:  fmt.Sprintf("count-limit-%03d", i),
+				Value: "x",
+			})
+			require.NoError(t, err)
+			if i == 0 {
+				firstSecret = s
+			}
+		}
+
+		// POST: the 51st secret is rejected.
+		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:  "one-too-many",
+			Value: "x",
+		})
+		requireSecretAPIError(t, err, http.StatusBadRequest, "at most")
+
+		// PATCH at the cap: changing the description must succeed.
+		// Without the FILTER clause the trigger would re-count
+		// firstSecret and reject this UPDATE.
+		newDescription := "renamed"
+		_, err = client.UpdateUserSecret(ctx, codersdk.Me, firstSecret.Name, codersdk.UpdateUserSecretRequest{
+			Description: &newDescription,
+		})
+		require.NoError(t, err)
+
+		// Other-user isolation: the second user's budget is independent.
+		_, err = otherClient.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:  "other-user-secret",
+			Value: "x",
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("TotalBytesLimit", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		client := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, client)
+		otherClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
+
+		// Pre-fill the total-bytes budget exactly to the cap using
+		// max-sized file-only secrets (which don't count against env
+		// bytes).
+		big := strings.Repeat("a", codersdk.MaxUserSecretValueBytes)
+		numBig := codersdk.MaxUserSecretsTotalValueBytes / codersdk.MaxUserSecretValueBytes
+		remainder := codersdk.MaxUserSecretsTotalValueBytes % codersdk.MaxUserSecretValueBytes
+		var firstSecret codersdk.UserSecret
+		for i := 0; i < numBig; i++ {
+			s, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+				Name:     fmt.Sprintf("big-%03d", i),
+				Value:    big,
+				FilePath: fmt.Sprintf("/tmp/big-%03d", i),
+			})
+			require.NoError(t, err)
+			if i == 0 {
+				firstSecret = s
+			}
+		}
+		if remainder > 0 {
+			_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+				Name:     "big-pad",
+				Value:    strings.Repeat("a", remainder),
+				FilePath: "/tmp/big-pad",
+			})
+			require.NoError(t, err)
+		}
+
+		// POST: one more byte pushes past the total budget.
+		_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:     "overflow",
+			Value:    "x",
+			FilePath: "/tmp/overflow",
+		})
+		requireSecretAPIError(t, err, http.StatusBadRequest, "per-user budget")
+
+		// PATCH at the cap: rewriting the existing row with a value
+		// of the same size must succeed. The FILTER clause excludes
+		// firstSecret's old bytes from the aggregate so the trigger
+		// computes (cap - old) + new = cap, not cap + new.
+		_, err = client.UpdateUserSecret(ctx, codersdk.Me, firstSecret.Name, codersdk.UpdateUserSecretRequest{
+			Value: &big,
+		})
+		require.NoError(t, err)
+
+		// Other-user isolation: a fresh user can fill their own
+		// total-bytes budget without interference.
+		for i := 0; i < numBig; i++ {
+			_, err := otherClient.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+				Name:     fmt.Sprintf("other-big-%03d", i),
+				Value:    big,
+				FilePath: fmt.Sprintf("/tmp/other-big-%03d", i),
+			})
+			require.NoError(t, err)
+		}
+		if remainder > 0 {
+			_, err := otherClient.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+				Name:     "other-big-pad",
+				Value:    strings.Repeat("a", remainder),
+				FilePath: "/tmp/other-big-pad",
+			})
+			require.NoError(t, err)
+		}
+	})
+
+	t.Run("EnvBytesLimit", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		client := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, client)
+		otherClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
+
+		// One env-injected secret consumes nearly the whole env budget.
+		envBig, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:    "env-big",
+			Value:   strings.Repeat("a", codersdk.MaxUserSecretValueBytes-16),
+			EnvName: "ENV_BIG",
+		})
+		require.NoError(t, err)
+
+		// POST: another env-injected secret pushes us over the env budget.
+		_, err = client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:    "env-overflow",
+			Value:   strings.Repeat("a", 1024),
+			EnvName: "ENV_OVERFLOW",
+		})
+		requireSecretAPIError(t, err, http.StatusBadRequest, "env_name")
+
+		// A same-sized value used purely as a file is fine because
+		// file_path secrets do not count against the env budget.
+		fileOK, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:     "file-ok",
+			Value:    strings.Repeat("a", 1024),
+			FilePath: "/tmp/file-ok",
+		})
+		require.NoError(t, err)
+
+		// PATCH at the cap: updating envBig's description must
+		// succeed. Without FILTER, the trigger would re-add envBig's
+		// 24 KiB to itself and reject the UPDATE.
+		newDescription := "renamed"
+		_, err = client.UpdateUserSecret(ctx, codersdk.Me, envBig.Name, codersdk.UpdateUserSecretRequest{
+			Description: &newDescription,
+		})
+		require.NoError(t, err)
+
+		// PATCH a file_path secret to env mode: moves its 1 KiB into
+		// the env budget, which already holds envBig's 24 KiB - 16.
+		// new_env_bytes = 24560 + 1024 = 25584 > 24576, rejected.
+		envName := "ENV_LATE"
+		_, err = client.UpdateUserSecret(ctx, codersdk.Me, fileOK.Name, codersdk.UpdateUserSecretRequest{
+			EnvName: &envName,
+		})
+		requireSecretAPIError(t, err, http.StatusBadRequest, "env_name")
+
+		// Other-user isolation: a fresh user can create their own
+		// near-cap env secret.
+		_, err = otherClient.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
+			Name:    "other-env-big",
+			Value:   strings.Repeat("a", codersdk.MaxUserSecretValueBytes-16),
+			EnvName: "OTHER_ENV_BIG",
+		})
+		require.NoError(t, err)
+	})
+}
+
+// requireSecretAPIError asserts a non-validation user-facing error.
+// Used for trigger-driven failures (per-user limits) whose responses
+// are plain codersdk.Response without ValidationError entries.
+func requireSecretAPIError(t *testing.T, err error, status int, detailContains string) {
+	t.Helper()
+	require.Error(t, err)
+	var sdkErr *codersdk.Error
+	require.ErrorAs(t, err, &sdkErr)
+	assert.Equal(t, status, sdkErr.StatusCode())
+	combined := sdkErr.Message + " " + sdkErr.Response.Detail
+	assert.Containsf(t, combined, detailContains,
+		"expected response to contain %q; got Message=%q Detail=%q",
+		detailContains, sdkErr.Message, sdkErr.Response.Detail)
+}
+
 func TestDeleteUserSecret(t *testing.T) {
 	t.Parallel()
 	client := coderdtest.New(t, nil)