mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add enable/disable support for user secrets (#27537)
Users can now disable a secret to stop it from being injected into workspaces without deleting it, and re-enable it later. Disabled secrets stay visible and editable everywhere they already appear. An enabled secret must have at least one injection target; a secret with no target can be stored only while disabled. Existing target-less secrets are migrated to disabled to preserve current behavior. Support spans the REST API, SDK, CLI, dashboard, and audit log.
This commit is contained in:
@@ -0,0 +1,2 @@
|
||||
ALTER TABLE user_secrets
|
||||
DROP COLUMN enabled;
|
||||
@@ -0,0 +1,30 @@
|
||||
-- Add an explicit enabled flag to user_secrets.
|
||||
--
|
||||
-- A disabled secret stays visible and editable in the management UI, CLI,
|
||||
-- and API, but is not injected into workspaces and does not satisfy any
|
||||
-- "secret present" predicate. This is the single source of truth for
|
||||
-- "not injected"; the agent manifest layer no longer skips rows based
|
||||
-- on having both env_name and file_path empty.
|
||||
--
|
||||
-- Existing rows whose env_name and file_path are both empty are flipped
|
||||
-- to enabled = false. Today those rows are silently skipped during agent
|
||||
-- manifest assembly, so flipping them preserves observable behavior
|
||||
-- while letting the manifest stop encoding the both-empty special case.
|
||||
ALTER TABLE user_secrets
|
||||
ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT true;
|
||||
|
||||
UPDATE user_secrets
|
||||
SET enabled = false
|
||||
WHERE env_name = '' AND file_path = '';
|
||||
|
||||
-- Enforce the injection-target invariant in the database: an enabled
|
||||
-- secret must have at least one of env_name / file_path non-empty.
|
||||
-- Disabled secrets may have no targets (bulk imports use that state for
|
||||
-- keys that cannot be env-injected). The API also checks this on write,
|
||||
-- but the constraint is the source of truth: it closes a read-modify-write
|
||||
-- race where two concurrent PATCHes each clear a different target, both
|
||||
-- pass the API's post-state check, and serialize to an enabled row with
|
||||
-- no targets.
|
||||
ALTER TABLE user_secrets
|
||||
ADD CONSTRAINT user_secrets_enabled_requires_target
|
||||
CHECK (NOT enabled OR env_name <> '' OR file_path <> '');
|
||||
@@ -2251,3 +2251,100 @@ func TestMigration000543ChatSearchSchemaBehavior(t *testing.T) {
|
||||
"search must exclude deleted, model-only, and tool-role rows (%d %d %d)",
|
||||
toolMsg.ID, modelOnly.ID, deletedMsg.ID)
|
||||
}
|
||||
|
||||
func TestMigration000556UserSecretsEnabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const migrationVersion = 556
|
||||
|
||||
sqlDB := testSQLDB(t)
|
||||
|
||||
// Migrate up to the migration before the one that adds the enabled
|
||||
// column.
|
||||
next, err := migrations.Stepper(sqlDB)
|
||||
require.NoError(t, err)
|
||||
for {
|
||||
version, more, err := next()
|
||||
require.NoError(t, err)
|
||||
if !more {
|
||||
t.Fatalf("migration %d not found", migrationVersion)
|
||||
}
|
||||
if version == migrationVersion-1 {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitSuperLong)
|
||||
|
||||
userID := uuid.New()
|
||||
envSecretID := uuid.New()
|
||||
fileSecretID := uuid.New()
|
||||
bothEmptySecretID := uuid.New()
|
||||
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
|
||||
tx, err := sqlDB.BeginTx(ctx, nil)
|
||||
require.NoError(t, err)
|
||||
defer tx.Rollback()
|
||||
|
||||
fixtures := []struct {
|
||||
query string
|
||||
args []any
|
||||
}{
|
||||
{
|
||||
`INSERT INTO users (id, username, email, hashed_password, created_at, updated_at, status, rbac_roles, login_type)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||
[]any{userID, "user-secrets-enabled", "user-secrets-enabled@test.com", []byte{}, now, now, "active", pq.StringArray{}, "password"},
|
||||
},
|
||||
// env-only secret: should remain enabled after migration.
|
||||
{
|
||||
`INSERT INTO user_secrets (id, user_id, name, description, value, env_name, file_path, created_at, updated_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||
[]any{envSecretID, userID, "env-secret", "", "v1", "ENV_SECRET", "", now, now},
|
||||
},
|
||||
// file-only secret: should remain enabled after migration.
|
||||
{
|
||||
`INSERT INTO user_secrets (id, user_id, name, description, value, env_name, file_path, created_at, updated_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||
[]any{fileSecretID, userID, "file-secret", "", "v2", "", "/tmp/file-secret", now, now},
|
||||
},
|
||||
// Both env_name and file_path empty: silently skipped today by
|
||||
// the agent manifest layer. Should be flipped to enabled=false
|
||||
// by the migration so the behavior is preserved exactly under
|
||||
// the new "always inject when enabled" rule.
|
||||
{
|
||||
`INSERT INTO user_secrets (id, user_id, name, description, value, env_name, file_path, created_at, updated_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||
[]any{bothEmptySecretID, userID, "both-empty", "", "v3", "", "", now, now},
|
||||
},
|
||||
}
|
||||
|
||||
for i, f := range fixtures {
|
||||
_, err := tx.ExecContext(ctx, f.query, f.args...)
|
||||
require.NoError(t, err, "fixture %d", i)
|
||||
}
|
||||
require.NoError(t, tx.Commit())
|
||||
|
||||
// Run the migration.
|
||||
version, _, err := next()
|
||||
require.NoError(t, err)
|
||||
require.EqualValues(t, migrationVersion, version)
|
||||
|
||||
getEnabled := func(t *testing.T, id uuid.UUID) bool {
|
||||
t.Helper()
|
||||
var enabled bool
|
||||
err := sqlDB.QueryRowContext(ctx,
|
||||
"SELECT enabled FROM user_secrets WHERE id = $1", id,
|
||||
).Scan(&enabled)
|
||||
require.NoError(t, err)
|
||||
return enabled
|
||||
}
|
||||
|
||||
require.True(t, getEnabled(t, envSecretID),
|
||||
"env-only secret should remain enabled")
|
||||
require.True(t, getEnabled(t, fileSecretID),
|
||||
"file-only secret should remain enabled")
|
||||
require.False(t, getEnabled(t, bothEmptySecretID),
|
||||
"secret with both targets empty should be flipped to disabled "+
|
||||
"to preserve the previous implicit-skip behavior")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user