shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: audit user secret create, update, and delete (#24756) (#24849)

Browse Source 
Emit user secret audit log entries for create/update/delete operations.
Reads stay un-audited, matching every other resource.

Audit log entries record changes in user secret name, environment
variable name, file path, and value. The secret value column is marked
`ActionSecret` so the diff records the change without showing the
ciphertext or plaintext.

Closes a TOCTOU window on delete to ensure no phantom audit logs for a
delete of a non-existent secret. Secret update accepts a small TOCTOU
window matching the other audited resources (templates, workspaces,
chats). The two-query pattern is wrapped in a transaction so audit state
can't leak from a failed mutation.

(cherry picked from commit 1c30d52b2b)

<!--

If you have used AI to produce some or all of this PR, please ensure you
have read our [AI Contribution
guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING)
before submitting.

-->

Co-authored-by: Zach <3724288+zedkipp@users.noreply.github.com>
This commit is contained in:
Cian Johnston
2026-04-30 21:01:27 +01:00
committed by GitHub
parent feca4c25d8
commit df1bfe6479
24 changed files with 555 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+4 -2
View File
@@ -19766,7 +19766,8 @@ const docTemplate = `{
"workspace_app",
"task",
"ai_seat",
"chat"
"chat",
"user_secret"
],
"x-enum-varnames": [
"ResourceTypeTemplate",
@@ -19796,7 +19797,8 @@ const docTemplate = `{
"ResourceTypeWorkspaceApp",
"ResourceTypeTask",
"ResourceTypeAISeat",
"ResourceTypeChat"
"ResourceTypeChat",
"ResourceTypeUserSecret"
]
},
"codersdk.Response": {
coderd/apidoc/swagger.json
Generated
+4 -2
View File
@@ -18098,7 +18098,8 @@
"workspace_app",
"task",
"ai_seat",
"chat"
"chat",
"user_secret"
],
"x-enum-varnames": [
"ResourceTypeTemplate",
@@ -18128,7 +18129,8 @@
"ResourceTypeWorkspaceApp",
"ResourceTypeTask",
"ResourceTypeAISeat",
"ResourceTypeChat"
"ResourceTypeChat",
"ResourceTypeUserSecret"
]
},
"codersdk.Response": {
coderd/audit.go
+16
View File
@@ -445,6 +445,18 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
api.Logger.Error(ctx, "unable to fetch chat", slog.Error(err))
}
return false
case database.ResourceTypeUserSecret:
_, err := api.Database.GetUserSecretByID(ctx, alog.AuditLog.ResourceID)
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
// Only users have user_secret:read on their own secrets. If dbauthz returns
// ErrUnauthorized, it's not an error worth logging because we have enough
// information to know it's not deleted.
if err != nil && !dbauthz.IsNotAuthorizedError(err) {
api.Logger.Error(ctx, "unable to fetch user secret", slog.Error(err))
}
return false
default:
return false
}
@@ -536,6 +548,10 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
// Chats are surfaced at /agents/{id}. They are owner-scoped but
// not username-scoped in the URL like workspaces or tasks.
return fmt.Sprintf("/agents/%s", alog.AuditLog.ResourceID)
case database.ResourceTypeUserSecret:
// TODO(PLAT-102): point at the user secrets management page once
// it ships. Until then, the audit row links nowhere.
return ""
default:
return ""
}
coderd/audit/diff.go
+2 -1
View File
@@ -34,7 +34,8 @@ type Auditable interface {
idpsync.RoleSyncSettings |
database.TaskTable |
database.AiSeatState |
database.Chat
database.Chat |
database.UserSecret
}
// Map is a map of changed fields in an audited resource. It maps field names to
coderd/audit/request.go
+9
View File
@@ -142,6 +142,8 @@ func ResourceTarget[T Auditable](tgt T) string {
// for display; collisions affect the display label and search
// filter but not the primary resource identifier.
return typed.ID.String()[:8]
case database.UserSecret:
return typed.Name
default:
panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt))
}
@@ -210,6 +212,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.UserID
case database.Chat:
return typed.ID
case database.UserSecret:
return typed.ID
default:
panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt))
}
@@ -269,6 +273,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeAiSeat
case database.Chat:
return database.ResourceTypeChat
case database.UserSecret:
return database.ResourceTypeUserSecret
default:
panic(fmt.Sprintf("unknown resource %T for ResourceType", typed))
}
@@ -333,6 +339,9 @@ func ResourceRequiresOrgID[T Auditable]() bool {
// Chats always have a non-null organization_id (since
// migration 000467).
return true
case database.UserSecret:
// User secrets are global to the user across organizations.
return false
default:
panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt))
}
coderd/database/dbauthz/dbauthz.go
+6 -2
View File
@@ -2223,10 +2223,10 @@ func (q *querier) DeleteUserChatProviderKey(ctx context.Context, arg database.De
return q.db.DeleteUserChatProviderKey(ctx, arg)
}
func (q *querier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (int64, error) {
func (q *querier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String())
if err := q.authorizeContext(ctx, policy.ActionDelete, obj); err != nil {
return 0, err
return database.UserSecret{}, err
}
return q.db.DeleteUserSecretByUserIDAndName(ctx, arg)
}
@@ -4378,6 +4378,10 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui
return q.db.GetUserNotificationPreferences(ctx, userID)
}
func (q *querier) GetUserSecretByID(ctx context.Context, id uuid.UUID) (database.UserSecret, error) {
return fetch(q.log, q.auth, q.db.GetUserSecretByID)(ctx, id)
}
func (q *querier) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
obj := rbac.ResourceUserSecret.WithOwner(arg.UserID.String())
if err := q.authorizeContext(ctx, policy.ActionRead, obj); err != nil {
coderd/database/dbauthz/dbauthz_test.go
+11 -2
View File
@@ -5690,11 +5690,20 @@ func (s *MethodTestSuite) TestUserSecrets() {
}))
s.Run("DeleteUserSecretByUserIDAndName", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
user := testutil.Fake(s.T(), faker, database.User{})
deleted := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID, Name: "test"})
arg := database.DeleteUserSecretByUserIDAndNameParams{UserID: user.ID, Name: "test"}
dbm.EXPECT().DeleteUserSecretByUserIDAndName(gomock.Any(), arg).Return(int64(1), nil).AnyTimes()
dbm.EXPECT().DeleteUserSecretByUserIDAndName(gomock.Any(), arg).Return(deleted, nil).AnyTimes()
check.Args(arg).
Asserts(rbac.ResourceUserSecret.WithOwner(user.ID.String()), policy.ActionDelete).
Returns(int64(1))
Returns(deleted)
}))
s.Run("GetUserSecretByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
user := testutil.Fake(s.T(), faker, database.User{})
secret := testutil.Fake(s.T(), faker, database.UserSecret{UserID: user.ID})
dbm.EXPECT().GetUserSecretByID(gomock.Any(), secret.ID).Return(secret, nil).AnyTimes()
check.Args(secret.ID).
Asserts(secret, policy.ActionRead).
Returns(secret)
}))
}
coderd/database/dbmetrics/querymetrics.go
+9 -1
View File
@@ -768,7 +768,7 @@ func (m queryMetricsStore) DeleteUserChatProviderKey(ctx context.Context, arg da
return r0
}
func (m queryMetricsStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (int64, error) {
func (m queryMetricsStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
start := time.Now()
r0, r1 := m.s.DeleteUserSecretByUserIDAndName(ctx, arg)
m.queryLatencies.WithLabelValues("DeleteUserSecretByUserIDAndName").Observe(time.Since(start).Seconds())
@@ -2848,6 +2848,14 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u
return r0, r1
}
func (m queryMetricsStore) GetUserSecretByID(ctx context.Context, id uuid.UUID) (database.UserSecret, error) {
start := time.Now()
r0, r1 := m.s.GetUserSecretByID(ctx, id)
m.queryLatencies.WithLabelValues("GetUserSecretByID").Observe(time.Since(start).Seconds())
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetUserSecretByID").Inc()
return r0, r1
}
func (m queryMetricsStore) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
start := time.Now()
r0, r1 := m.s.GetUserSecretByUserIDAndName(ctx, arg)
coderd/database/dbmock/dbmock.go
+17 -2
View File
@@ -1303,10 +1303,10 @@ func (mr *MockStoreMockRecorder) DeleteUserChatProviderKey(ctx, arg any) *gomock
}
// DeleteUserSecretByUserIDAndName mocks base method.
func (m *MockStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (int64, error) {
func (m *MockStore) DeleteUserSecretByUserIDAndName(ctx context.Context, arg database.DeleteUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteUserSecretByUserIDAndName", ctx, arg)
ret0, _ := ret[0].(int64)
ret0, _ := ret[0].(database.UserSecret)
ret1, _ := ret[1].(error)
return ret0, ret1
}
@@ -5326,6 +5326,21 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(ctx, userID any)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), ctx, userID)
}
// GetUserSecretByID mocks base method.
func (m *MockStore) GetUserSecretByID(ctx context.Context, id uuid.UUID) (database.UserSecret, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetUserSecretByID", ctx, id)
ret0, _ := ret[0].(database.UserSecret)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetUserSecretByID indicates an expected call of GetUserSecretByID.
func (mr *MockStoreMockRecorder) GetUserSecretByID(ctx, id any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecretByID", reflect.TypeOf((*MockStore)(nil).GetUserSecretByID), ctx, id)
}
// GetUserSecretByUserIDAndName mocks base method.
func (m *MockStore) GetUserSecretByUserIDAndName(ctx context.Context, arg database.GetUserSecretByUserIDAndNameParams) (database.UserSecret, error) {
m.ctrl.T.Helper()
coderd/database/dump.sql
Generated
+2 -1
View File
@@ -526,7 +526,8 @@ CREATE TYPE resource_type AS ENUM (
'prebuilds_settings',
'task',
'ai_seat',
'chat'
'chat',
'user_secret'
);
CREATE TYPE shareable_workspace_owners AS ENUM (
coderd/database/migrations/000481_user_secret_audit.down.sql
+1
View File
@@ -0,0 +1 @@
-- no-op because resource_type enum values cannot be removed safely.
coderd/database/migrations/000481_user_secret_audit.up.sql
+1
View File
@@ -0,0 +1 @@
ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'user_secret';
coderd/database/models.go
+4 -1
View File
@@ -3206,6 +3206,7 @@ const (
ResourceTypeTask ResourceType = "task"
ResourceTypeAiSeat ResourceType = "ai_seat"
ResourceTypeChat ResourceType = "chat"
ResourceTypeUserSecret ResourceType = "user_secret"
)
func (e *ResourceType) Scan(src interface{}) error {
@@ -3272,7 +3273,8 @@ func (e ResourceType) Valid() bool {
ResourceTypePrebuildsSettings,
ResourceTypeTask,
ResourceTypeAiSeat,
ResourceTypeChat:
ResourceTypeChat,
ResourceTypeUserSecret:
return true
}
return false
@@ -3308,6 +3310,7 @@ func AllResourceTypeValues() []ResourceType {
ResourceTypeTask,
ResourceTypeAiSeat,
ResourceTypeChat,
ResourceTypeUserSecret,
}
}
coderd/database/querier.go
+2 -1
View File
@@ -186,7 +186,7 @@ type sqlcQuerier interface {
DeleteTask(ctx context.Context, arg DeleteTaskParams) (uuid.UUID, error)
DeleteUserChatCompactionThreshold(ctx context.Context, arg DeleteUserChatCompactionThresholdParams) error
DeleteUserChatProviderKey(ctx context.Context, arg DeleteUserChatProviderKeyParams) error
DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) (int64, error)
DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) (UserSecret, error)
DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx context.Context, arg DeleteWebpushSubscriptionByUserIDAndEndpointParams) error
DeleteWebpushSubscriptions(ctx context.Context, ids []uuid.UUID) error
DeleteWorkspaceACLByID(ctx context.Context, id uuid.UUID) error
@@ -704,6 +704,7 @@ type sqlcQuerier interface {
GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error)
GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error)
GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error)
GetUserSecretByID(ctx context.Context, id uuid.UUID) (UserSecret, error)
GetUserSecretByUserIDAndName(ctx context.Context, arg GetUserSecretByUserIDAndNameParams) (UserSecret, error)
// GetUserStatusCounts returns the count of users in each status over time.
// The time range is inclusively defined by the start_time and end_time parameters.
coderd/database/queries.sql.go
+42 -7
View File
@@ -24654,9 +24654,10 @@ func (q *sqlQuerier) CreateUserSecret(ctx context.Context, arg CreateUserSecretP
return i, err
}
const deleteUserSecretByUserIDAndName = `-- name: DeleteUserSecretByUserIDAndName :execrows
const deleteUserSecretByUserIDAndName = `-- name: DeleteUserSecretByUserIDAndName :one
DELETE FROM user_secrets
WHERE user_id = $1 AND name = $2
RETURNING id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id
`
type DeleteUserSecretByUserIDAndNameParams struct {
@@ -24664,12 +24665,46 @@ type DeleteUserSecretByUserIDAndNameParams struct {
Name string `db:"name" json:"name"`
}
func (q *sqlQuerier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) (int64, error) {
result, err := q.db.ExecContext(ctx, deleteUserSecretByUserIDAndName, arg.UserID, arg.Name)
if err != nil {
return 0, err
}
return result.RowsAffected()
func (q *sqlQuerier) DeleteUserSecretByUserIDAndName(ctx context.Context, arg DeleteUserSecretByUserIDAndNameParams) (UserSecret, error) {
row := q.db.QueryRowContext(ctx, deleteUserSecretByUserIDAndName, arg.UserID, arg.Name)
var i UserSecret
err := row.Scan(
&i.ID,
&i.UserID,
&i.Name,
&i.Description,
&i.Value,
&i.EnvName,
&i.FilePath,
&i.CreatedAt,
&i.UpdatedAt,
&i.ValueKeyID,
)
return i, err
}
const getUserSecretByID = `-- name: GetUserSecretByID :one
SELECT id, user_id, name, description, value, env_name, file_path, created_at, updated_at, value_key_id
FROM user_secrets
WHERE id = $1
`
func (q *sqlQuerier) GetUserSecretByID(ctx context.Context, id uuid.UUID) (UserSecret, error) {
row := q.db.QueryRowContext(ctx, getUserSecretByID, id)
var i UserSecret
err := row.Scan(
&i.ID,
&i.UserID,
&i.Name,
&i.Description,
&i.Value,
&i.EnvName,
&i.FilePath,
&i.CreatedAt,
&i.UpdatedAt,
&i.ValueKeyID,
)
return i, err
}
const getUserSecretByUserIDAndName = `-- name: GetUserSecretByUserIDAndName :one
coderd/database/queries/user_secrets.sql
+8 -2
View File
@@ -3,6 +3,11 @@ SELECT *
FROM user_secrets
WHERE user_id = @user_id AND name = @name;
-- name: GetUserSecretByID :one
SELECT *
FROM user_secrets
WHERE id = @id;
-- name: ListUserSecrets :many
-- Returns metadata only (no value or value_key_id) for the
-- REST API list and get endpoints.
@@ -56,6 +61,7 @@ SET
WHERE user_id = @user_id AND name = @name
RETURNING *;
-- name: DeleteUserSecretByUserIDAndName :execrows
-- name: DeleteUserSecretByUserIDAndName :one
DELETE FROM user_secrets
WHERE user_id = @user_id AND name = @name;
WHERE user_id = @user_id AND name = @name
RETURNING *;
coderd/usersecrets.go
+83 -21
View File
@@ -7,7 +7,9 @@ import (
"github.com/go-chi/chi/v5"
"github.com/google/uuid"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/httpapi"
@@ -26,8 +28,18 @@ import (
// @Success 201 {object} codersdk.UserSecret
// @Router /users/{user}/secrets [post]
func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
user := httpmw.UserParam(r)
var (
ctx = r.Context()
user = httpmw.UserParam(r)
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.UserSecret](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionCreate,
})
)
defer commitAudit()
var req codersdk.CreateUserSecretRequest
if !httpapi.Read(ctx, rw, r, &req) {
@@ -92,6 +104,7 @@ func (api *API) postUserSecret(rw http.ResponseWriter, r *http.Request) {
})
return
}
aReq.New = secret
httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.UserSecretFromFull(secret))
}
@@ -165,9 +178,19 @@ func (api *API) getUserSecret(rw http.ResponseWriter, r *http.Request) { //nolin
// @Success 200 {object} codersdk.UserSecret
// @Router /users/{user}/secrets/{name} [patch]
func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
user := httpmw.UserParam(r)
name := chi.URLParam(r, "name")
var (
ctx = r.Context()
user = httpmw.UserParam(r)
name = chi.URLParam(r, "name")
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.UserSecret](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
})
)
defer commitAudit()
var req codersdk.UpdateUserSecretRequest
if !httpapi.Read(ctx, rw, r, &req) {
@@ -198,6 +221,15 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) {
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
}
}
params := database.UpdateUserSecretByUserIDAndNameParams{
UserID: user.ID,
@@ -213,13 +245,6 @@ 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 {
@@ -232,7 +257,33 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) {
params.FilePath = *req.FilePath
}
secret, err := api.Database.UpdateUserSecretByUserIDAndName(ctx, params)
// Pre-read the secret inside a transaction so the audit diff has both an
// "old" and "new" snapshot.
//
// Under read committed isolation, a concurrent writer between our SELECT
// and our UPDATE can cause the audit diff to attribute changes to us that
// we did not make. We accept this race to match other audit log diffs
// (templates, workspaces, chats, etc). In practice this should be unlikely
// to hit since a user can only modify their own secrets.
var secret database.UserSecret
err := api.Database.InTx(func(tx database.Store) error {
old, err := tx.GetUserSecretByUserIDAndName(ctx, database.GetUserSecretByUserIDAndNameParams{
UserID: user.ID,
Name: name,
})
if err != nil {
return xerrors.Errorf("fetch user secret: %w", err)
}
aReq.Old = old
updated, err := tx.UpdateUserSecretByUserIDAndName(ctx, params)
if err != nil {
return xerrors.Errorf("update user secret: %w", err)
}
secret = updated
aReq.New = updated
return nil
}, nil)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
@@ -264,25 +315,36 @@ func (api *API) patchUserSecret(rw http.ResponseWriter, r *http.Request) {
// @Success 204
// @Router /users/{user}/secrets/{name} [delete]
func (api *API) deleteUserSecret(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
user := httpmw.UserParam(r)
name := chi.URLParam(r, "name")
var (
ctx = r.Context()
user = httpmw.UserParam(r)
name = chi.URLParam(r, "name")
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.UserSecret](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})
)
defer commitAudit()
rowsAffected, err := api.Database.DeleteUserSecretByUserIDAndName(ctx, database.DeleteUserSecretByUserIDAndNameParams{
deleted, err := api.Database.DeleteUserSecretByUserIDAndName(ctx, database.DeleteUserSecretByUserIDAndNameParams{
UserID: user.ID,
Name: name,
})
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
return
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error deleting secret.",
Detail: err.Error(),
})
return
}
if rowsAffected == 0 {
httpapi.ResourceNotFound(rw)
return
}
aReq.Old = deleted
rw.WriteHeader(http.StatusNoContent)
}
coderd/usersecrets_audit_test.go
+178
View File
@@ -0,0 +1,178 @@
package coderd_test
import (
"net/http"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
//nolint:paralleltest,tparallel // Subtests share one coderdtest.New server and run sequentially.
func TestUserSecretAudit(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
_ = coderdtest.CreateFirstUser(t, client)
ctx := testutil.Context(t, testutil.WaitMedium)
genSecretName := func(t *testing.T) string {
// Use test name derived secret names so subtests cannot
// collide in the shared user's secret namespace.
return strings.ReplaceAll(t.Name(), "/", "-")
}
t.Run("CreateEmitsLog", func(t *testing.T) {
auditor.ResetLogs()
secret, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: genSecretName(t),
Value: "ghp_xxxxxxxxxxxx",
})
require.NoError(t, err)
logs := auditor.AuditLogs()
require.Len(t, logs, 1)
assert.Equal(t, database.AuditActionCreate, logs[0].Action)
assert.Equal(t, secret.ID, logs[0].ResourceID)
assert.Equal(t, secret.Name, logs[0].ResourceTarget)
assert.EqualValues(t, http.StatusCreated, logs[0].StatusCode)
})
t.Run("UpdateEmitsLog", func(t *testing.T) {
auditor.ResetLogs()
secret, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: genSecretName(t),
Value: "old",
})
require.NoError(t, err)
newDescription := "rotated"
newValue := "new-value"
_, err = client.UpdateUserSecret(ctx, codersdk.Me, secret.Name, codersdk.UpdateUserSecretRequest{
Description: &newDescription,
Value: &newValue,
})
require.NoError(t, err)
logs := auditor.AuditLogs()
require.Len(t, logs, 2)
assert.Equal(t, database.AuditActionCreate, logs[0].Action)
assert.Equal(t, database.AuditActionWrite, logs[1].Action)
assert.Equal(t, secret.ID, logs[1].ResourceID)
assert.Equal(t, secret.Name, logs[1].ResourceTarget)
assert.EqualValues(t, http.StatusOK, logs[1].StatusCode)
})
t.Run("DeleteEmitsLog", func(t *testing.T) {
auditor.ResetLogs()
secret, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: genSecretName(t),
Value: "value",
})
require.NoError(t, err)
require.NoError(t, client.DeleteUserSecret(ctx, codersdk.Me, secret.Name))
logs := auditor.AuditLogs()
require.Len(t, logs, 2)
assert.Equal(t, database.AuditActionCreate, logs[0].Action)
assert.Equal(t, database.AuditActionDelete, logs[1].Action)
assert.Equal(t, secret.ID, logs[1].ResourceID)
assert.Equal(t, secret.Name, logs[1].ResourceTarget)
assert.EqualValues(t, http.StatusNoContent, logs[1].StatusCode)
})
t.Run("DeleteOfMissingWritesNoLog", func(t *testing.T) {
auditor.ResetLogs()
err := client.DeleteUserSecret(ctx, codersdk.Me, "does-not-exist")
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr)
assert.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
require.Empty(t, auditor.AuditLogs())
})
t.Run("UpdateOfMissingWritesNoLog", func(t *testing.T) {
auditor.ResetLogs()
desc := "anything"
_, err := client.UpdateUserSecret(ctx, codersdk.Me, "does-not-exist", codersdk.UpdateUserSecretRequest{
Description: &desc,
})
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr)
assert.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
require.Empty(t, auditor.AuditLogs())
})
t.Run("ValidationFailureWritesNoLog", func(t *testing.T) {
auditor.ResetLogs()
_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: genSecretName(t),
Value: "value",
EnvName: "1invalid",
})
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr)
assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
require.Empty(t, auditor.AuditLogs())
})
t.Run("EmptyUpdateWritesNoLog", func(t *testing.T) {
auditor.ResetLogs()
name := genSecretName(t)
_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: name,
Value: "value",
})
require.NoError(t, err)
// Reset to ignore the created log. We are only testing that the
// no-op update does not add a new log.
auditor.ResetLogs()
_, err = client.UpdateUserSecret(ctx, codersdk.Me, name, codersdk.UpdateUserSecretRequest{})
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr)
assert.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
require.Empty(t, auditor.AuditLogs())
})
t.Run("ReadsDoNotAudit", func(t *testing.T) {
auditor.ResetLogs()
secretName := genSecretName(t)
_, err := client.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: secretName,
Value: "value",
})
require.NoError(t, err)
// Discard the create log so the assertion below only sees audit entries
// produced by later reads.
auditor.ResetLogs()
_, err = client.UserSecrets(ctx, codersdk.Me)
require.NoError(t, err)
_, err = client.UserSecretByName(ctx, codersdk.Me, secretName)
require.NoError(t, err)
require.Empty(t, auditor.AuditLogs())
})
}
codersdk/audit.go
+3
View File
@@ -47,6 +47,7 @@ const (
ResourceTypeTask ResourceType = "task"
ResourceTypeAISeat ResourceType = "ai_seat"
ResourceTypeChat ResourceType = "chat"
ResourceTypeUserSecret ResourceType = "user_secret"
)
func (r ResourceType) FriendlyString() string {
@@ -109,6 +110,8 @@ func (r ResourceType) FriendlyString() string {
return "ai seat"
case ResourceTypeChat:
return "chat"
case ResourceTypeUserSecret:
return "user secret"
default:
return "unknown"
}
docs/admin/security/audit-logs.md
+1
View File
@@ -38,6 +38,7 @@ We track the following resources:
| Template<br><i>write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>active_version_id</td><td>true</td></tr><tr><td>activity_bump</td><td>true</td></tr><tr><td>allow_user_autostart</td><td>true</td></tr><tr><td>allow_user_autostop</td><td>true</td></tr><tr><td>allow_user_cancel_workspace_jobs</td><td>true</td></tr><tr><td>autostart_block_days_of_week</td><td>true</td></tr><tr><td>autostop_requirement_days_of_week</td><td>true</td></tr><tr><td>autostop_requirement_weeks</td><td>true</td></tr><tr><td>cors_behavior</td><td>true</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>created_by</td><td>true</td></tr><tr><td>created_by_avatar_url</td><td>false</td></tr><tr><td>created_by_name</td><td>false</td></tr><tr><td>created_by_username</td><td>false</td></tr><tr><td>default_ttl</td><td>true</td></tr><tr><td>deleted</td><td>false</td></tr><tr><td>deprecated</td><td>true</td></tr><tr><td>description</td><td>true</td></tr><tr><td>disable_module_cache</td><td>true</td></tr><tr><td>display_name</td><td>true</td></tr><tr><td>failure_ttl</td><td>true</td></tr><tr><td>group_acl</td><td>true</td></tr><tr><td>icon</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>max_port_sharing_level</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>organization_display_name</td><td>false</td></tr><tr><td>organization_icon</td><td>false</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>organization_name</td><td>false</td></tr><tr><td>provisioner</td><td>true</td></tr><tr><td>require_active_version</td><td>true</td></tr><tr><td>time_til_dormant</td><td>true</td></tr><tr><td>time_til_dormant_autodelete</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>use_classic_parameter_flow</td><td>true</td></tr><tr><td>user_acl</td><td>true</td></tr></tbody></table> |
| TemplateVersion<br><i>create, write</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>archived</td><td>true</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>created_by</td><td>true</td></tr><tr><td>created_by_avatar_url</td><td>false</td></tr><tr><td>created_by_name</td><td>false</td></tr><tr><td>created_by_username</td><td>false</td></tr><tr><td>external_auth_providers</td><td>false</td></tr><tr><td>has_ai_task</td><td>false</td></tr><tr><td>has_external_agent</td><td>false</td></tr><tr><td>id</td><td>true</td></tr><tr><td>job_id</td><td>false</td></tr><tr><td>message</td><td>false</td></tr><tr><td>name</td><td>true</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>readme</td><td>true</td></tr><tr><td>source_example_id</td><td>false</td></tr><tr><td>template_id</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr></tbody></table> |
| User<br><i>create, write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>avatar_url</td><td>false</td></tr><tr><td>chat_spend_limit_micros</td><td>true</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>deleted</td><td>true</td></tr><tr><td>email</td><td>true</td></tr><tr><td>github_com_user_id</td><td>false</td></tr><tr><td>hashed_one_time_passcode</td><td>false</td></tr><tr><td>hashed_password</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>is_service_account</td><td>true</td></tr><tr><td>is_system</td><td>true</td></tr><tr><td>last_seen_at</td><td>false</td></tr><tr><td>login_type</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>one_time_passcode_expires_at</td><td>true</td></tr><tr><td>quiet_hours_schedule</td><td>true</td></tr><tr><td>rbac_roles</td><td>true</td></tr><tr><td>status</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>username</td><td>true</td></tr></tbody></table> |
| UserSecret<br><i>create, write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>created_at</td><td>false</td></tr><tr><td>description</td><td>true</td></tr><tr><td>env_name</td><td>true</td></tr><tr><td>file_path</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>user_id</td><td>true</td></tr><tr><td>value</td><td>true</td></tr><tr><td>value_key_id</td><td>false</td></tr></tbody></table> |
| WorkspaceBuild<br><i>start, stop</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>build_number</td><td>false</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>daily_cost</td><td>false</td></tr><tr><td>deadline</td><td>false</td></tr><tr><td>has_ai_task</td><td>false</td></tr><tr><td>has_external_agent</td><td>false</td></tr><tr><td>id</td><td>false</td></tr><tr><td>initiator_by_avatar_url</td><td>false</td></tr><tr><td>initiator_by_name</td><td>false</td></tr><tr><td>initiator_by_username</td><td>false</td></tr><tr><td>initiator_id</td><td>false</td></tr><tr><td>job_id</td><td>false</td></tr><tr><td>max_deadline</td><td>false</td></tr><tr><td>reason</td><td>false</td></tr><tr><td>template_version_id</td><td>true</td></tr><tr><td>template_version_preset_id</td><td>false</td></tr><tr><td>transition</td><td>false</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>workspace_id</td><td>false</td></tr></tbody></table> |
| WorkspaceProxy<br><i></i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>created_at</td><td>true</td></tr><tr><td>deleted</td><td>false</td></tr><tr><td>derp_enabled</td><td>true</td></tr><tr><td>derp_only</td><td>true</td></tr><tr><td>display_name</td><td>true</td></tr><tr><td>icon</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>region_id</td><td>true</td></tr><tr><td>token_hashed_secret</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>url</td><td>true</td></tr><tr><td>version</td><td>true</td></tr><tr><td>wildcard_hostname</td><td>true</td></tr></tbody></table> |
| WorkspaceTable<br><i></i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody> | <tr><td>automatic_updates</td><td>true</td></tr><tr><td>autostart_schedule</td><td>true</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>deleted</td><td>false</td></tr><tr><td>deleting_at</td><td>true</td></tr><tr><td>dormant_at</td><td>true</td></tr><tr><td>favorite</td><td>true</td></tr><tr><td>group_acl</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>last_used_at</td><td>false</td></tr><tr><td>name</td><td>true</td></tr><tr><td>next_start_at</td><td>true</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>owner_id</td><td>true</td></tr><tr><td>template_id</td><td>true</td></tr><tr><td>ttl</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>user_acl</td><td>true</td></tr></tbody></table> |
docs/reference/api/schemas.md
Generated
+3 -3
View File
@@ -8490,9 +8490,9 @@ Only certain features set these fields: - FeatureManagedAgentLimit|
#### Enumerated Values
| Value(s) |
|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `ai_seat`, `api_key`, `chat`, `convert_login`, `custom_role`, `git_ssh_key`, `group`, `health_settings`, `idp_sync_settings_group`, `idp_sync_settings_organization`, `idp_sync_settings_role`, `license`, `notification_template`, `notifications_settings`, `oauth2_provider_app`, `oauth2_provider_app_secret`, `organization`, `organization_member`, `prebuilds_settings`, `task`, `template`, `template_version`, `user`, `workspace`, `workspace_agent`, `workspace_app`, `workspace_build`, `workspace_proxy` |
| Value(s) |
|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `ai_seat`, `api_key`, `chat`, `convert_login`, `custom_role`, `git_ssh_key`, `group`, `health_settings`, `idp_sync_settings_group`, `idp_sync_settings_organization`, `idp_sync_settings_role`, `license`, `notification_template`, `notifications_settings`, `oauth2_provider_app`, `oauth2_provider_app_secret`, `organization`, `organization_member`, `prebuilds_settings`, `task`, `template`, `template_version`, `user`, `user_secret`, `workspace`, `workspace_agent`, `workspace_app`, `workspace_build`, `workspace_proxy` |
## codersdk.Response
enterprise/audit/table.go
+15
View File
@@ -30,6 +30,7 @@ var AuditActionMap = map[string][]codersdk.AuditAction{
"Task": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete},
"AiSeatState": {codersdk.AuditActionCreate},
"Chat": {codersdk.AuditActionCreate, codersdk.AuditActionWrite}, // chats get 'archived' by users, not deleted.
"UserSecret": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete},
}
type Action string
@@ -408,6 +409,20 @@ var auditableResourcesTypes = map[any]map[string]Action{
"plan_mode": ActionIgnore, // Can flip back and forth during a session.
"client_type": ActionIgnore, // Set at creation.
},
&database.UserSecret{}: {
"id": ActionTrack,
"user_id": ActionTrack,
"name": ActionTrack,
"description": ActionTrack,
"env_name": ActionTrack,
"file_path": ActionTrack,
"value": ActionSecret,
"value_key_id": ActionIgnore,
"created_at": ActionIgnore,
"updated_at": ActionIgnore,
},
}
// auditMap converts a map of struct pointers to a map of struct names as
enterprise/coderd/usersecrets_audit_test.go
+132
View File
@@ -0,0 +1,132 @@
package coderd_test
import (
"context"
"encoding/json"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/codersdk"
entaudit "github.com/coder/coder/v2/enterprise/audit"
"github.com/coder/coder/v2/enterprise/audit/backends"
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
"github.com/coder/coder/v2/enterprise/coderd/license"
"github.com/coder/coder/v2/testutil"
)
func TestUserSecretAuditDiffRedaction(t *testing.T) {
// Ensure secret values never appear in plaintext in audit diffs. The
// enterprise auditor needs to be used because it writes actual diffs.
// We read straight from the audit_logs table to exercise the full
// insert, filter, dbauthz read path.
t.Parallel()
db, ps := dbtestutil.NewDB(t)
auditor := entaudit.NewAuditor(
db,
entaudit.DefaultFilter,
backends.NewPostgres(db, true),
)
ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
Database: db,
Pubsub: ps,
Auditor: auditor,
},
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureAuditLog: 1,
},
},
})
memberClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
defer cancel()
initialDescription := "initial"
initialValue := "initial-secret-value"
secret, err := memberClient.CreateUserSecret(ctx, codersdk.Me, codersdk.CreateUserSecretRequest{
Name: "createDiff-target",
Description: initialDescription,
Value: initialValue,
})
require.NoError(t, err)
newDescription := "after"
newValue := "new-secret-value"
_, err = memberClient.UpdateUserSecret(ctx, codersdk.Me, secret.Name, codersdk.UpdateUserSecretRequest{
Description: &newDescription,
Value: &newValue,
})
require.NoError(t, err)
// Read straight from the database. AsSystemRestricted is necessary because
// the test does not authenticate as an admin when querying the store directly.
rows, err := db.GetAuditLogsOffset(
dbauthz.AsSystemRestricted(ctx),
database.GetAuditLogsOffsetParams{
ResourceType: string(database.ResourceTypeUserSecret),
LimitOpt: 10,
},
)
require.NoError(t, err)
require.Equal(t, len(rows), 2, "expected exactly two rows")
// GetAuditLogsOffset returns entries sorted by time in descending order.
createLog := rows[1].AuditLog
updateLog := rows[0].AuditLog
var createDiff audit.Map
require.NoError(t, json.Unmarshal(createLog.Diff, &createDiff))
// Creation must show both old and new non-secret values verbatim.
if assert.Contains(t, createDiff, "description", "tracked field missing from createDiff") {
assert.Equal(t, "", createDiff["description"].Old)
assert.Equal(t, initialDescription, createDiff["description"].New)
assert.False(t, createDiff["description"].Secret)
}
// Creation must record that it changed but with zero-valued old/new and
// indicate the value is secret.
if assert.Contains(t, createDiff, "value", "value field missing from createDiff") {
assert.True(t, createDiff["value"].Secret, "value field must be marked secret")
assert.Equal(t, "", createDiff["value"].Old)
assert.Equal(t, "", createDiff["value"].New)
}
// Ensure ignored fields are excluded from the create diff.
assert.NotContains(t, createDiff, "value_key_id")
assert.NotContains(t, createDiff, "created_at")
assert.NotContains(t, createDiff, "updated_at")
var updateDiff audit.Map
require.NoError(t, json.Unmarshal(updateLog.Diff, &updateDiff))
// Update must show both old and new non-secret values verbatim.
if assert.Contains(t, updateDiff, "description", "tracked field missing from updateDiff") {
assert.Equal(t, initialDescription, updateDiff["description"].Old)
assert.Equal(t, newDescription, updateDiff["description"].New)
assert.False(t, updateDiff["description"].Secret)
}
// Update must record that it changed but with zero-valued old/new and
// indicate the value is secret.
if assert.Contains(t, updateDiff, "value", "value field missing from updateDiff") {
assert.True(t, updateDiff["value"].Secret, "value field must be marked secret")
assert.Equal(t, "", updateDiff["value"].Old)
assert.Equal(t, "", updateDiff["value"].New)
}
// Ensure ignored fields are excluded from update diff.
assert.NotContains(t, updateDiff, "value_key_id")
assert.NotContains(t, updateDiff, "created_at")
assert.NotContains(t, updateDiff, "updated_at")
}
site/src/api/typesGenerated.ts
Generated
+2
View File
@@ -6351,6 +6351,7 @@ export type ResourceType =
| "template"
| "template_version"
| "user"
| "user_secret"
| "workspace"
| "workspace_agent"
| "workspace_app"
@@ -6381,6 +6382,7 @@ export const ResourceTypes: ResourceType[] = [
"template",
"template_version",
"user",
"user_secret",
"workspace",
"workspace_agent",
"workspace_app",