diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 53ae5ff53a..9c2df36cab 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -837,6 +837,67 @@ BEGIN END; $$; +CREATE FUNCTION enforce_user_secrets_per_user_limits() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + existing_count int; + existing_total_bytes bigint; + existing_env_bytes bigint; + + new_count int; + new_total_bytes bigint; + new_env_bytes bigint; + + count_limit constant int := 50; + total_bytes_limit constant bigint := 204800; -- 200 KiB + env_bytes_limit constant bigint := 24576; -- 24 KiB +BEGIN + -- Serialize cap checks per user so concurrent inserts cannot all + -- observe the same pre-insert aggregates and exceed the cap. + PERFORM 1 FROM users WHERE id = NEW.user_id FOR UPDATE; + + -- Sum existing rows excluding the row being updated (so UPDATE statements + -- don't double-count NEW). On INSERT, no row matches NEW.id, so + -- the FILTER is a no-op. + SELECT + count(*) FILTER (WHERE id IS DISTINCT FROM NEW.id), + coalesce(sum(octet_length(value)) FILTER (WHERE id IS DISTINCT FROM NEW.id), 0), + coalesce(sum(octet_length(value)) FILTER (WHERE id IS DISTINCT FROM NEW.id AND env_name <> ''), 0) + INTO existing_count, existing_total_bytes, existing_env_bytes + FROM user_secrets + WHERE user_id = NEW.user_id; + + new_count := existing_count + 1; + new_total_bytes := existing_total_bytes + octet_length(NEW.value); + new_env_bytes := existing_env_bytes + + CASE WHEN NEW.env_name <> '' THEN octet_length(NEW.value) ELSE 0 END; + + IF new_count > count_limit THEN + RAISE EXCEPTION 'user has reached the user secrets count limit (% > %)', + new_count, count_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_count_limit'; + END IF; + + IF new_total_bytes > total_bytes_limit THEN + RAISE EXCEPTION 'user has reached the user secrets total value bytes limit (% > %)', + new_total_bytes, total_bytes_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_total_bytes_limit'; + END IF; + + IF new_env_bytes > env_bytes_limit THEN + RAISE EXCEPTION 'user has reached the env-injected user secrets bytes limit (% > %)', + new_env_bytes, env_bytes_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_env_bytes_limit'; + END IF; + + RETURN NEW; +END; +$$; + CREATE FUNCTION enforce_user_skills_per_user_limit() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -4398,6 +4459,8 @@ CREATE TRIGGER trigger_upsert_user_secrets BEFORE INSERT OR UPDATE ON user_secre CREATE TRIGGER trigger_upsert_user_skills BEFORE INSERT OR UPDATE ON user_skills FOR EACH ROW EXECUTE FUNCTION insert_user_skill_fail_if_user_deleted(); +CREATE TRIGGER trigger_user_secrets_per_user_limits BEFORE INSERT OR UPDATE ON user_secrets FOR EACH ROW EXECUTE FUNCTION enforce_user_secrets_per_user_limits(); + CREATE TRIGGER trigger_user_skills_per_user_limit BEFORE INSERT ON user_skills FOR EACH ROW EXECUTE FUNCTION enforce_user_skills_per_user_limit(); CREATE TRIGGER update_notification_message_dedupe_hash BEFORE INSERT OR UPDATE ON notification_messages FOR EACH ROW EXECUTE FUNCTION compute_notification_message_dedupe_hash(); diff --git a/coderd/database/migrations/000509_user_secrets_limits.down.sql b/coderd/database/migrations/000509_user_secrets_limits.down.sql new file mode 100644 index 0000000000..5b103c40dd --- /dev/null +++ b/coderd/database/migrations/000509_user_secrets_limits.down.sql @@ -0,0 +1,2 @@ +DROP TRIGGER IF EXISTS trigger_user_secrets_per_user_limits ON user_secrets; +DROP FUNCTION IF EXISTS enforce_user_secrets_per_user_limits(); diff --git a/coderd/database/migrations/000509_user_secrets_limits.up.sql b/coderd/database/migrations/000509_user_secrets_limits.up.sql new file mode 100644 index 0000000000..b8dbf520d6 --- /dev/null +++ b/coderd/database/migrations/000509_user_secrets_limits.up.sql @@ -0,0 +1,105 @@ +-- Per-user user_secrets caps (count, total stored bytes, env-injected +-- stored bytes), enforced at the schema level. +-- +-- Why: user_secrets is user-scoped; every workspace loads the same +-- set via the agent manifest, and env-injected ones land in the +-- agent's process env. Without a cap the failure surfaces at +-- workspace start (or as a truncated env), not at create-time. +-- +-- What drives each cap: +-- +-- * count_limit = 50: backstop against row-count growth from many +-- small secrets. The total_bytes_limit binds first for large +-- secrets; this binds first for typical-sized ones (~few KB). +-- +-- * total_bytes_limit = 200 KiB: sized to cover realistic +-- credential storage (API keys, SSH keys, kubeconfigs, cert +-- bundles) with headroom. Well under the 4 MiB DRPC manifest +-- budget (codersdk/drpcsdk.MaxMessageSize). +-- +-- * env_bytes_limit = 24 KiB: an approximate budget for the +-- value bytes of env-injected secrets. Leaves ~8 KiB of +-- headroom under the ~32 KiB Windows process env block +-- (CreateProcessW's lpEnvironment is capped at 32,767 +-- characters) for what this aggregate does not count: +-- env_name bytes, per-entry overhead, agent-injected vars +-- (CODER_*, PATH, HOME, ...), and template-defined env. Not +-- a strict overflow guarantee. Linux/macOS ARG_MAX (~2 MiB) +-- is far above this, so the same cap works everywhere. +-- +-- octet_length(value) measures stored bytes. In encrypted +-- deployments stored bytes exceed plaintext (AES-GCM + base64 +-- ~1.33x). The handler's per-value check (UserSecretValueValid) +-- measures plaintext separately, so it can pass while the +-- trigger's stored-bytes aggregate rejects. The trigger is +-- authoritative; the handler is a fast pre-flight. +-- +-- Keep the literals below in sync with codersdk.MaxUserSecret* +-- in codersdk/usersecretvalidation.go. TestUserSecretLimits in +-- coderd/usersecrets_test.go exercises off-by-one for each cap, +-- so any drift between the two layers fails an assertion. +CREATE FUNCTION enforce_user_secrets_per_user_limits() RETURNS trigger + LANGUAGE plpgsql +AS $$ +DECLARE + existing_count int; + existing_total_bytes bigint; + existing_env_bytes bigint; + + new_count int; + new_total_bytes bigint; + new_env_bytes bigint; + + count_limit constant int := 50; + total_bytes_limit constant bigint := 204800; -- 200 KiB + env_bytes_limit constant bigint := 24576; -- 24 KiB +BEGIN + -- Serialize cap checks per user so concurrent inserts cannot all + -- observe the same pre-insert aggregates and exceed the cap. + PERFORM 1 FROM users WHERE id = NEW.user_id FOR UPDATE; + + -- Sum existing rows excluding the row being updated (so UPDATE statements + -- don't double-count NEW). On INSERT, no row matches NEW.id, so + -- the FILTER is a no-op. + SELECT + count(*) FILTER (WHERE id IS DISTINCT FROM NEW.id), + coalesce(sum(octet_length(value)) FILTER (WHERE id IS DISTINCT FROM NEW.id), 0), + coalesce(sum(octet_length(value)) FILTER (WHERE id IS DISTINCT FROM NEW.id AND env_name <> ''), 0) + INTO existing_count, existing_total_bytes, existing_env_bytes + FROM user_secrets + WHERE user_id = NEW.user_id; + + new_count := existing_count + 1; + new_total_bytes := existing_total_bytes + octet_length(NEW.value); + new_env_bytes := existing_env_bytes + + CASE WHEN NEW.env_name <> '' THEN octet_length(NEW.value) ELSE 0 END; + + IF new_count > count_limit THEN + RAISE EXCEPTION 'user has reached the user secrets count limit (% > %)', + new_count, count_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_count_limit'; + END IF; + + IF new_total_bytes > total_bytes_limit THEN + RAISE EXCEPTION 'user has reached the user secrets total value bytes limit (% > %)', + new_total_bytes, total_bytes_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_total_bytes_limit'; + END IF; + + IF new_env_bytes > env_bytes_limit THEN + RAISE EXCEPTION 'user has reached the env-injected user secrets bytes limit (% > %)', + new_env_bytes, env_bytes_limit + USING ERRCODE = 'check_violation', + CONSTRAINT = 'user_secrets_per_user_env_bytes_limit'; + END IF; + + RETURN NEW; +END; +$$; + +CREATE TRIGGER trigger_user_secrets_per_user_limits + BEFORE INSERT OR UPDATE ON user_secrets + FOR EACH ROW +EXECUTE PROCEDURE enforce_user_secrets_per_user_limits(); diff --git a/coderd/usersecrets.go b/coderd/usersecrets.go index 7bcfc5d0f6..eed2570fa5 100644 --- a/coderd/usersecrets.go +++ b/coderd/usersecrets.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "fmt" "net/http" "github.com/go-chi/chi/v5" @@ -23,6 +24,13 @@ const ( userSecretValueField = "value" userSecretEnvNameField = "env_name" userSecretFilePathField = "file_path" + + // These names are raised by the enforce_user_secrets_per_user_limits + // trigger with USING CONSTRAINT. They are not table CHECK + // constraints, so dbgen does not emit them in check_constraint.go. + userSecretsCountLimitConstraint database.CheckConstraint = "user_secrets_per_user_count_limit" + userSecretsTotalBytesLimitConstraint database.CheckConstraint = "user_secrets_per_user_total_bytes_limit" + userSecretsEnvBytesLimitConstraint database.CheckConstraint = "user_secrets_per_user_env_bytes_limit" ) // @Summary Create a new user secret @@ -74,6 +82,10 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) { writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, validations) return } + if resp, ok := userSecretLimitResponse(err); ok { + httpapi.Write(ctx, rw, http.StatusBadRequest, resp) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error creating secret.", Detail: err.Error(), @@ -246,6 +258,10 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) { writeUserSecretValidationErrors(ctx, rw, http.StatusConflict, validations) return } + if resp, ok := userSecretLimitResponse(err); ok { + httpapi.Write(ctx, rw, http.StatusBadRequest, resp) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error updating secret.", Detail: err.Error(), @@ -346,6 +362,44 @@ func appendUserSecretValidationError(validations []codersdk.ValidationError, fie }) } +// userSecretLimitResponse maps a per-user-limits trigger violation +// (raised by enforce_user_secrets_per_user_limits) to a 400. Returns +// ok=false if err is not such a violation. See +// codersdk.MaxUserSecretsPerUserCount for the rationale behind the caps. +func userSecretLimitResponse(err error) (codersdk.Response, bool) { + switch { + case database.IsCheckViolation(err, userSecretsCountLimitConstraint): + return codersdk.Response{ + Message: "User secrets limit reached.", + Detail: fmt.Sprintf( + "Each user can have at most %d secrets.", + codersdk.MaxUserSecretsPerUserCount, + ), + }, true + case database.IsCheckViolation(err, userSecretsTotalBytesLimitConstraint): + return codersdk.Response{ + Message: "User secrets value-bytes limit reached.", + Detail: fmt.Sprintf( + "Stored bytes of your secret values exceed the per-user "+ + "budget (%d bytes after encryption, if applicable). "+ + "Reduce the size or number of your secrets.", + codersdk.MaxUserSecretsTotalValueBytes, + ), + }, true + case database.IsCheckViolation(err, userSecretsEnvBytesLimitConstraint): + return codersdk.Response{ + Message: "Environment-injected user secrets bytes limit reached.", + 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.", + codersdk.MaxUserSecretValueBytes, + ), + }, true + } + return codersdk.Response{}, false +} + func userSecretConflictValidationErrors(err error) []codersdk.ValidationError { switch { case database.IsUniqueViolation(err, database.UniqueUserSecretsUserNameIndex): diff --git a/coderd/usersecrets_test.go b/coderd/usersecrets_test.go index a869d48777..f51cc4b58f 100644 --- a/coderd/usersecrets_test.go +++ b/coderd/usersecrets_test.go @@ -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) diff --git a/codersdk/usersecretvalidation.go b/codersdk/usersecretvalidation.go index 841e7acce1..d43626e8e4 100644 --- a/codersdk/usersecretvalidation.go +++ b/codersdk/usersecretvalidation.go @@ -8,17 +8,6 @@ import ( ) 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. @@ -28,6 +17,94 @@ const ( maxFilePathLength = 4096 ) +// MaxUserSecretsPerUserCount caps the number of secrets a single user +// may own. +// +// Why a cap exists at all: user_secrets is user-scoped, so every +// workspace the user owns loads the same set into its agent +// manifest, and env-injected ones land in the workspace agent's +// process env. Without a cap, a user can overflow one of three +// external limits by accumulating enough secrets, or by making +// them large enough. The failure surfaces at workspace start (or +// as a truncated env), not at create-time. +// +// What drives each cap, and the rough math: +// +// - Count (50): backstops row-count growth from many small +// secrets. The total-bytes cap binds first for large secrets; +// this cap binds first for typical-sized ones (~few KB). +// +// - Total bytes (200 KiB): sized to cover realistic credential +// storage (API keys, SSH keys, kubeconfigs, cert bundles) +// with headroom. Well under the 4 MiB DRPC agent manifest +// budget (codersdk/drpcsdk.MaxMessageSize). +// +// - Env bytes (24 KiB): an approximate budget for the value +// bytes of env-injected secrets. Leaves ~8 KiB of headroom +// under the ~32 KiB Windows process env block +// (CreateProcessW's lpEnvironment is capped at 32,767 +// characters) for what this aggregate does not count: +// env_name bytes, per-entry overhead, agent-injected vars +// (CODER_*, PATH, HOME, ...), and template-defined env. Not +// a strict overflow guarantee. Linux/macOS ARG_MAX (~2 MiB) +// is far above this, so one Windows-safe cap works +// everywhere. +// +// Byte caps measure stored bytes (octet_length of encrypted+base64). +// Plaintext is slightly tighter in encrypted deployments. That is +// fine: the limits we defend all measure transmitted bytes, and +// stored bytes upper-bound those. +// +// The Postgres trigger enforce_user_secrets_per_user_limits is the +// source of truth; the HTTP handler maps its check_violation to a +// 400. TestUserSecretLimits in coderd/usersecrets_test.go exercises +// off-by-one at each cap across POST and PATCH, so any drift +// between these constants and the trigger's literals fails an +// assertion. +const MaxUserSecretsPerUserCount = 50 + +// MaxUserSecretsTotalValueBytes caps the sum of stored value bytes +// per user. See MaxUserSecretsPerUserCount for the full rationale and +// math behind all three caps. +const MaxUserSecretsTotalValueBytes = 200 * 1024 // 200 KiB + +// MaxUserSecretValueBytes is the maximum number of bytes for a +// single secret value. It is enforced in two places: +// +// - The HTTP handler validates the raw (plaintext) value with +// UserSecretValueValid before the row is written. +// - The Postgres trigger enforce_user_secrets_per_user_limits +// enforces the same number as an aggregate on stored bytes +// across a user's env-injected secrets. This defends the +// ~32 KiB Windows process env block. +// +// On deployments with secret encryption enabled, stored bytes +// exceed plaintext by ~1.33x (AES-GCM + base64), so the trigger's +// env-aggregate budget can be reached at less plaintext than the +// handler's per-value check would suggest. The trigger is +// authoritative; the handler's check is a fast pre-flight that +// catches the common "one value is too big" case before the row +// is encrypted and sent to the DB. +// +// One number serves both roles because the per-value cap can't +// usefully exceed the smallest aggregate cap any single row could +// trip: a value bigger than the env aggregate would be rejected +// the moment its env_name was set, so allowing it at the per-value +// layer would just move the failure later. +// +// See MaxUserSecretsPerUserCount for the rationale behind the other +// two caps (count, total bytes). +const MaxUserSecretValueBytes = 24 * 1024 // 24 KiB + +// MaxUserSecretEnvNameLength caps the length of an env_name when one +// is provided. 256 is a generous round number that should allow any +// realistic env name while still bounding inputs. +// +// This is a per-row syntactic check, not an aggregate. It does not +// interact with the env_bytes aggregate (which is itself an +// approximate budget; see MaxUserSecretsPerUserCount). +const MaxUserSecretEnvNameLength = 256 + var ( // posixEnvNameRegex matches valid POSIX environment variable names: // must start with a letter or underscore, followed by letters, @@ -157,6 +234,13 @@ func UserSecretEnvNameValid(s string) error { return nil } + if len(s) > MaxUserSecretEnvNameLength { + return xerrors.Errorf( + "environment variable name must not exceed %d bytes", + MaxUserSecretEnvNameLength, + ) + } + if !posixEnvNameRegex.MatchString(s) { return xerrors.New("must start with a letter or underscore, followed by letters, digits, or underscores") } @@ -204,15 +288,20 @@ func UserSecretFilePathValid(s string) error { return nil } -// UserSecretValueValid validates a user secret value. The value must -// not contain null bytes and must not exceed MaxSecretValueSize. +// UserSecretValueValid validates a user secret value as bytes +// submitted by the user (plaintext). The value must not contain +// null bytes and must not exceed MaxUserSecretValueBytes. The DB +// trigger separately enforces a stored-bytes env aggregate at the +// same numeric cap; under encryption the trigger may reject values +// that pass this check. See MaxUserSecretValueBytes for the +// dual-enforcement explanation. 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) + if len(value) > MaxUserSecretValueBytes { + return xerrors.Errorf("secret value must not exceed %d bytes", MaxUserSecretValueBytes) } return nil diff --git a/codersdk/usersecretvalidation_test.go b/codersdk/usersecretvalidation_test.go index f381bfea6a..fe959d7b5e 100644 --- a/codersdk/usersecretvalidation_test.go +++ b/codersdk/usersecretvalidation_test.go @@ -63,6 +63,10 @@ func TestUserSecretEnvNameValid(t *testing.T) { {name: "WithDigits", input: "A1B2"}, {name: "Empty", input: ""}, + // Length cap. + {name: "ExactlyAtLengthLimit", input: strings.Repeat("A", codersdk.MaxUserSecretEnvNameLength)}, + {name: "OverLengthLimit", input: strings.Repeat("A", codersdk.MaxUserSecretEnvNameLength+1), wantErr: true, errMsg: "256 bytes"}, + // Invalid POSIX names. {name: "StartsWithDigit", input: "1FOO", wantErr: true, errMsg: "must start with"}, {name: "ContainsHyphen", input: "FOO-BAR", wantErr: true, errMsg: "must start with"}, @@ -214,8 +218,8 @@ func TestUserSecretValueValid(t *testing.T) { {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}, + {name: "ExactlyAtLimit", input: strings.Repeat("a", codersdk.MaxUserSecretValueBytes)}, + {name: "OverLimit", input: strings.Repeat("a", codersdk.MaxUserSecretValueBytes+1), wantErr: true}, } for _, tt := range tests { diff --git a/docs/user-guides/user-secrets.md b/docs/user-guides/user-secrets.md index 70e2f2bc78..8cf536528e 100644 --- a/docs/user-guides/user-secrets.md +++ b/docs/user-guides/user-secrets.md @@ -77,6 +77,35 @@ example `~/config` and `/home/coder/config`), only one of them ends up on disk; the workspace agent logs a warning to help spot this. Use distinct paths to avoid the collision. +## Limits + +User secrets are subject to the following limits. Coder enforces these when you +create or update a secret and rejects the request with an explanatory 400 when +you exceed one. Delete or shrink an existing secret to make room. + +| Cap | Value | +|------------------------------------------|-----------| +| Total secrets per user | 50 | +| Combined stored value bytes per user | 200 KiB | +| Combined stored env-injected value bytes | 24 KiB | +| Per-secret value bytes | 24 KiB | +| Env var name length | 256 bytes | + +Only secrets created with `--env` count against the env-injected budget. Coder +injects these into the workspace agent's process environment, which on Windows +has a ~32 KiB total budget. The 24 KiB ceiling leaves room for Coder's own +variables (`CODER_*`, `PATH`, `HOME`, ...) plus any template-defined env. To +inject a value larger than this budget, use `--file` instead; file secrets do +not count against the env budget. + +The per-secret cap matches the env aggregate cap because a value larger than +the env aggregate could never be injected successfully as an environment +variable. + +These caps measure stored bytes, which is what Coder writes to the database. +In deployments with secret encryption enabled, stored bytes exceed the raw +value. + ## Create a secret You can create, edit, and delete user secrets in the Coder dashboard. Click your diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 31369e3e3e..56fbbf6000 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -5230,17 +5230,103 @@ export const MaxChatFileSizeBytes = 10485760; // From codersdk/usersecretvalidation.go /** - * 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. + * MaxUserSecretEnvNameLength caps the length of an env_name when one + * is provided. 256 is a generous round number that should allow any + * realistic env name while still bounding inputs. + * + * This is a per-row syntactic check, not an aggregate. It does not + * interact with the env_bytes aggregate (which is itself an + * approximate budget; see MaxUserSecretsPerUserCount). */ -export const MaxSecretValueSize = 32768; // 32KB +export const MaxUserSecretEnvNameLength = 256; + +// From codersdk/usersecretvalidation.go +/** + * MaxUserSecretValueBytes is the maximum number of bytes for a + * single secret value. It is enforced in two places: + * + * - The HTTP handler validates the raw (plaintext) value with + * UserSecretValueValid before the row is written. + * - The Postgres trigger enforce_user_secrets_per_user_limits + * enforces the same number as an aggregate on stored bytes + * across a user's env-injected secrets. This defends the + * ~32 KiB Windows process env block. + * + * On deployments with secret encryption enabled, stored bytes + * exceed plaintext by ~1.33x (AES-GCM + base64), so the trigger's + * env-aggregate budget can be reached at less plaintext than the + * handler's per-value check would suggest. The trigger is + * authoritative; the handler's check is a fast pre-flight that + * catches the common "one value is too big" case before the row + * is encrypted and sent to the DB. + * + * One number serves both roles because the per-value cap can't + * usefully exceed the smallest aggregate cap any single row could + * trip: a value bigger than the env aggregate would be rejected + * the moment its env_name was set, so allowing it at the per-value + * layer would just move the failure later. + * + * See MaxUserSecretsPerUserCount for the rationale behind the other + * two caps (count, total bytes). + */ +export const MaxUserSecretValueBytes = 24576; // 24 KiB + +// From codersdk/usersecretvalidation.go +/** + * MaxUserSecretsPerUserCount caps the number of secrets a single user + * may own. + * + * Why a cap exists at all: user_secrets is user-scoped, so every + * workspace the user owns loads the same set into its agent + * manifest, and env-injected ones land in the workspace agent's + * process env. Without a cap, a user can overflow one of three + * external limits by accumulating enough secrets, or by making + * them large enough. The failure surfaces at workspace start (or + * as a truncated env), not at create-time. + * + * What drives each cap, and the rough math: + * + * - Count (50): backstops row-count growth from many small + * secrets. The total-bytes cap binds first for large secrets; + * this cap binds first for typical-sized ones (~few KB). + * + * - Total bytes (200 KiB): sized to cover realistic credential + * storage (API keys, SSH keys, kubeconfigs, cert bundles) + * with headroom. Well under the 4 MiB DRPC agent manifest + * budget (codersdk/drpcsdk.MaxMessageSize). + * + * - Env bytes (24 KiB): an approximate budget for the value + * bytes of env-injected secrets. Leaves ~8 KiB of headroom + * under the ~32 KiB Windows process env block + * (CreateProcessW's lpEnvironment is capped at 32,767 + * characters) for what this aggregate does not count: + * env_name bytes, per-entry overhead, agent-injected vars + * (CODER_*, PATH, HOME, ...), and template-defined env. Not + * a strict overflow guarantee. Linux/macOS ARG_MAX (~2 MiB) + * is far above this, so one Windows-safe cap works + * everywhere. + * + * Byte caps measure stored bytes (octet_length of encrypted+base64). + * Plaintext is slightly tighter in encrypted deployments. That is + * fine: the limits we defend all measure transmitted bytes, and + * stored bytes upper-bound those. + * + * The Postgres trigger enforce_user_secrets_per_user_limits is the + * source of truth; the HTTP handler maps its check_violation to a + * 400. TestUserSecretLimits in coderd/usersecrets_test.go exercises + * off-by-one at each cap across POST and PATCH, so any drift + * between these constants and the trigger's literals fails an + * assertion. + */ +export const MaxUserSecretsPerUserCount = 50; + +// From codersdk/usersecretvalidation.go +/** + * MaxUserSecretsTotalValueBytes caps the sum of stored value bytes + * per user. See MaxUserSecretsPerUserCount for the full rationale and + * math behind all three caps. + */ +export const MaxUserSecretsTotalValueBytes = 204800; // 200 KiB // From codersdk/organizations.go export interface MinimalOrganization {