From d324cf7fa8eb443ab0c25a868b059d59a612da81 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Tue, 12 Aug 2025 22:31:07 +1000 Subject: [PATCH 1/6] ci: fix gcp service accounts (#19312) (#19315) Backport of #19312 --- .github/workflows/ci.yaml | 16 ++++++++-------- .github/workflows/dogfood.yaml | 4 ++-- .github/workflows/pr-deploy.yaml | 2 +- .github/workflows/release.yaml | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fbc34b0dfb6ec..a7fad297a8304 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -256,8 +256,8 @@ jobs: pushd /tmp/proto curl -L -o protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v23.4/protoc-23.4-linux-x86_64.zip unzip protoc.zip - cp -r ./bin/* /usr/local/bin - cp -r ./include /usr/local/bin/include + sudo cp -r ./bin/* /usr/local/bin + sudo cp -r ./include /usr/local/bin/include popd - name: make gen @@ -869,8 +869,8 @@ jobs: pushd /tmp/proto curl -L -o protoc.zip https://github.com/protocolbuffers/protobuf/releases/download/v23.4/protoc-23.4-linux-x86_64.zip unzip protoc.zip - cp -r ./bin/* /usr/local/bin - cp -r ./include /usr/local/bin/include + sudo cp -r ./bin/* /usr/local/bin + sudo cp -r ./include /usr/local/bin/include popd - name: Setup Go @@ -1123,8 +1123,8 @@ jobs: id: gcloud_auth uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11 with: - workload_identity_provider: ${{ secrets.GCP_CODE_SIGNING_WORKLOAD_ID_PROVIDER }} - service_account: ${{ secrets.GCP_CODE_SIGNING_SERVICE_ACCOUNT }} + workload_identity_provider: ${{ vars.GCP_CODE_SIGNING_WORKLOAD_ID_PROVIDER }} + service_account: ${{ vars.GCP_CODE_SIGNING_SERVICE_ACCOUNT }} token_format: "access_token" - name: Setup GCloud SDK @@ -1427,8 +1427,8 @@ jobs: - name: Authenticate to Google Cloud uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11 with: - workload_identity_provider: projects/573722524737/locations/global/workloadIdentityPools/github/providers/github - service_account: coder-ci@coder-dogfood.iam.gserviceaccount.com + workload_identity_provider: ${{ vars.GCP_WORKLOAD_ID_PROVIDER }} + service_account: ${{ vars.GCP_SERVICE_ACCOUNT }} - name: Set up Google Cloud SDK uses: google-github-actions/setup-gcloud@6a7c903a70c8625ed6700fa299f5ddb4ca6022e9 # v2.1.5 diff --git a/.github/workflows/dogfood.yaml b/.github/workflows/dogfood.yaml index bafdb5fb19767..9fcb286b8539e 100644 --- a/.github/workflows/dogfood.yaml +++ b/.github/workflows/dogfood.yaml @@ -131,8 +131,8 @@ jobs: - name: Authenticate to Google Cloud uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11 with: - workload_identity_provider: projects/573722524737/locations/global/workloadIdentityPools/github/providers/github - service_account: coder-ci@coder-dogfood.iam.gserviceaccount.com + workload_identity_provider: ${{ vars.GCP_WORKLOAD_ID_PROVIDER }} + service_account: ${{ vars.GCP_SERVICE_ACCOUNT }} - name: Terraform init and validate run: | diff --git a/.github/workflows/pr-deploy.yaml b/.github/workflows/pr-deploy.yaml index c82861db22094..7c9f78ca4d1a3 100644 --- a/.github/workflows/pr-deploy.yaml +++ b/.github/workflows/pr-deploy.yaml @@ -420,7 +420,7 @@ jobs: curl -fsSL "$URL" -o "${DEST}" chmod +x "${DEST}" "${DEST}" version - mv "${DEST}" /usr/local/bin/coder + sudo mv "${DEST}" /usr/local/bin/coder - name: Create first user if: needs.get_info.outputs.NEW == 'true' || github.event.inputs.deploy == 'true' diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 6ea28ad87a90c..da18f1b312dba 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -288,8 +288,8 @@ jobs: id: gcloud_auth uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11 with: - workload_identity_provider: ${{ secrets.GCP_CODE_SIGNING_WORKLOAD_ID_PROVIDER }} - service_account: ${{ secrets.GCP_CODE_SIGNING_SERVICE_ACCOUNT }} + workload_identity_provider: ${{ vars.GCP_CODE_SIGNING_WORKLOAD_ID_PROVIDER }} + service_account: ${{ vars.GCP_CODE_SIGNING_SERVICE_ACCOUNT }} token_format: "access_token" - name: Setup GCloud SDK @@ -698,8 +698,8 @@ jobs: - name: Authenticate to Google Cloud uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11 with: - workload_identity_provider: ${{ secrets.GCP_WORKLOAD_ID_PROVIDER }} - service_account: ${{ secrets.GCP_SERVICE_ACCOUNT }} + workload_identity_provider: ${{ vars.GCP_WORKLOAD_ID_PROVIDER }} + service_account: ${{ vars.GCP_SERVICE_ACCOUNT }} - name: Setup GCloud SDK uses: google-github-actions/setup-gcloud@6a7c903a70c8625ed6700fa299f5ddb4ca6022e9 # 2.1.5 From ed39f4c92ca84a1d8b5f03f85c81d17959120ed8 Mon Sep 17 00:00:00 2001 From: Rowan Smith Date: Fri, 22 Aug 2025 13:38:02 +1000 Subject: [PATCH 2/6] chore: fix typo in clientNetcheckSummary for support bundle command (#19482) (cherry picked from commit 33708413b847550b6d1c51e5044414eeffe9c481) bringing in https://github.com/coder/coder/pull/19441 to the 2.25 release branch to fix a bug in the `support bundle` command. --- cli/support.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/support.go b/cli/support.go index 70fadc3994580..c55bab92cd6ff 100644 --- a/cli/support.go +++ b/cli/support.go @@ -251,7 +251,7 @@ func summarizeBundle(inv *serpent.Invocation, bun *support.Bundle) { clientNetcheckSummary := bun.Network.Netcheck.Summarize("Client netcheck:", docsURL) if len(clientNetcheckSummary) > 0 { - cliui.Warn(inv.Stdout, "Networking issues detected:", deployHealthSummary...) + cliui.Warn(inv.Stdout, "Networking issues detected:", clientNetcheckSummary...) } } From ee8050986d49d007ab69bafb089b29477629d06c Mon Sep 17 00:00:00 2001 From: Jakub Domeracki Date: Mon, 25 Aug 2025 14:58:14 +0200 Subject: [PATCH 3/6] chore: update the slim binaries upload from the build directory to the GCS bucket (#19521) Updated the upload script to copy the slim binaries from the ./build directory to the GCS bucket (instead of the ./site/out/bin directory) --- .github/workflows/release.yaml | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index da18f1b312dba..6bdf094b3c9ea 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -641,20 +641,21 @@ jobs: version="$(./scripts/version.sh)" - binaries=( - "coder-darwin-amd64" - "coder-darwin-arm64" - "coder-linux-amd64" - "coder-linux-arm64" - "coder-linux-armv7" - "coder-windows-amd64.exe" - "coder-windows-arm64.exe" - ) - - for binary in "${binaries[@]}"; do - detached_signature="${binary}.asc" - gcloud storage cp "./site/out/bin/${binary}" "gs://releases.coder.com/coder-cli/${version}/${binary}" - gcloud storage cp "./site/out/bin/${detached_signature}" "gs://releases.coder.com/coder-cli/${version}/${detached_signature}" + # Source array of slim binaries + declare -A binaries + binaries["coder-darwin-amd64"]="coder-slim_${version}_darwin_amd64" + binaries["coder-darwin-arm64"]="coder-slim_${version}_darwin_arm64" + binaries["coder-linux-amd64"]="coder-slim_${version}_linux_amd64" + binaries["coder-linux-arm64"]="coder-slim_${version}_linux_arm64" + binaries["coder-linux-armv7"]="coder-slim_${version}_linux_armv7" + binaries["coder-windows-amd64.exe"]="coder-slim_${version}_windows_amd64.exe" + binaries["coder-windows-arm64.exe"]="coder-slim_${version}_windows_arm64.exe" + + for cli_name in "${!binaries[@]}"; do + slim_binary="${binaries[$cli_name]}" + detached_signature="${slim_binary}.asc" + gcloud storage cp "./build/${slim_binary}" "gs://releases.coder.com/coder-cli/${version}/${cli_name}" + gcloud storage cp "./build/${detached_signature}" "gs://releases.coder.com/coder-cli/${version}/${cli_name}.asc" done - name: Publish release From ec660907faa0b0eae20fa2ba58ce1733f5f4b35a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 3 Sep 2025 09:23:02 +0100 Subject: [PATCH 4/6] fix: expire token for prebuilds user when regenerating session token (#19667) (#19668) * provisionerdserver: Expires prebuild user token for workspace, if it exists, when regenerating session token. * dbauthz: disallow prebuilds user from creating api keys * dbpurge: added functionality to expire stale api keys owned by the prebuilds user (cherry picked from commit 06cbb2890f453cd522bb2158a6549afa3419c276) --- coderd/apikey.go | 18 +++++ coderd/apikey_test.go | 33 ++++++++++ coderd/database/dbauthz/dbauthz.go | 15 +++++ coderd/database/dbauthz/dbauthz_test.go | 21 ++++++ coderd/database/dbgen/dbgen.go | 10 ++- coderd/database/dbmetrics/querymetrics.go | 7 ++ coderd/database/dbmock/dbmock.go | 14 ++++ coderd/database/dbpurge/dbpurge.go | 3 + coderd/database/dbpurge/dbpurge_test.go | 66 +++++++++++++++++++ coderd/database/querier.go | 5 ++ coderd/database/queries.sql.go | 40 +++++++++++ coderd/database/queries/apikeys.sql | 34 ++++++++++ .../provisionerdserver/provisionerdserver.go | 22 +++++-- .../provisionerdserver_test.go | 64 ++++++++++++++++++ 14 files changed, 344 insertions(+), 8 deletions(-) diff --git a/coderd/apikey.go b/coderd/apikey.go index 895be440ef930..12646d627a212 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -12,6 +12,8 @@ import ( "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -56,6 +58,14 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { return } + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + scope := database.APIKeyScopeAll if scope != "" { scope = database.APIKeyScope(createToken.Scope) @@ -124,6 +134,14 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(), diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index dbf5a3520a6f0..1509aa2e2f402 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -13,8 +13,10 @@ import ( "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/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/serpent" @@ -351,3 +353,34 @@ func TestAPIKey_SetDefault(t *testing.T) { require.NoError(t, err) require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds) } + +func TestAPIKey_PrebuildsNotAllowed(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + DeploymentValues: dc, + }) + + ctx := testutil.Context(t, testutil.WaitLong) + + // Given: an existing api token for the prebuilds user + _, prebuildsToken := dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + }) + client.SetSessionToken(prebuildsToken) + + // When: the prebuilds user tries to create an API key + _, err := client.CreateAPIKey(ctx, database.PrebuildsSystemUserID.String()) + // Then: denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) + + // When: the prebuilds user tries to create a token + _, err = client.CreateToken(ctx, database.PrebuildsSystemUserID.String(), codersdk.CreateTokenRequest{}) + // Then: also denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 257cbc6e6b142..401bea0bd75c5 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1725,6 +1725,13 @@ func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.E return q.db.EnqueueNotificationMessage(ctx, arg) } +func (q *querier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil { + return err + } + return q.db.ExpirePrebuildsAPIKeys(ctx, now) +} + func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) { return q.db.GetWorkspaceByID(ctx, id) @@ -3623,6 +3630,14 @@ func (q *querier) HasTemplateVersionsWithAITask(ctx context.Context) (bool, erro } func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) { + // TODO(Cian): ideally this would be encoded in the policy, but system users are just members and we + // don't currently have a capability to conditionally deny creating resources by owner ID in a role. + // We also need to enrich rbac.Actor with IsSystem so that we can distinguish all system users. + // For now, there is only one system user (prebuilds). + if act, ok := ActorFromContext(ctx); ok && act.ID == database.PrebuildsSystemUserID.String() { + return database.APIKey{}, logNotAuthorizedError(ctx, q.log, NotAuthorizedError{Err: xerrors.Errorf("prebuild user may not create api keys")}) + } + return insert(q.log, q.auth, rbac.ResourceApiKey.WithOwner(arg.UserID.String()), q.db.InsertAPIKey)(ctx, arg) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index bcf0caa95c365..9a7333165655f 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -14,14 +14,17 @@ import ( "github.com/google/uuid" "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/xerrors" "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" @@ -1681,6 +1684,9 @@ func (s *MethodTestSuite) TestUser() { u := dbgen.User(s.T(), db, database.User{}) check.Args(u.ID).Asserts(rbac.ResourceApiKey.WithOwner(u.ID.String()), policy.ActionDelete).Returns() })) + s.Run("ExpirePrebuildsAPIKeys", s.Subtest(func(db database.Store, check *expects) { + check.Args(dbtime.Now()).Asserts(rbac.ResourceApiKey, policy.ActionDelete).Returns() + })) s.Run("GetQuotaAllowanceForUser", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) check.Args(database.GetQuotaAllowanceForUserParams{ @@ -5845,3 +5851,18 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { }).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) })) } + +// Ensures that the prebuilds actor may never insert an api key. +func TestInsertAPIKey_AsPrebuildsUser(t *testing.T) { + t.Parallel() + prebuildsSubj := rbac.Subject{ + ID: database.PrebuildsSystemUserID.String(), + } + ctx := dbauthz.As(testutil.Context(t, testutil.WaitShort), prebuildsSubj) + mDB := dbmock.NewMockStore(gomock.NewController(t)) + log := slogtest.Make(t, nil) + mDB.EXPECT().Wrappers().Times(1).Return([]string{}) + dbz := dbauthz.New(mDB, nil, log, nil) + _, err := dbz.InsertAPIKey(ctx, database.InsertAPIKeyParams{}) + require.True(t, dbauthz.IsNotAuthorizedError(err)) +} diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 0ccefff2c42a9..0d98ec8fc64a1 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -156,7 +156,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. return template } -func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database.APIKey, token string) { +func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func(*database.InsertAPIKeyParams)) (key database.APIKey, token string) { id, _ := cryptorand.String(10) secret, _ := cryptorand.String(22) hashed := sha256.Sum256([]byte(secret)) @@ -172,7 +172,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database } } - key, err := db.InsertAPIKey(genCtx, database.InsertAPIKeyParams{ + params := database.InsertAPIKeyParams{ ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), @@ -186,7 +186,11 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database LoginType: takeFirst(seed.LoginType, database.LoginTypePassword), Scope: takeFirst(seed.Scope, database.APIKeyScopeAll), TokenName: takeFirst(seed.TokenName), - }) + } + for _, fn := range munge { + fn(¶ms) + } + key, err := db.InsertAPIKey(genCtx, params) require.NoError(t, err, "insert api key") return key, fmt.Sprintf("%s-%s", key.ID, secret) } diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 811d945ac7da9..25545083ea5fe 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -509,6 +509,13 @@ func (m queryMetricsStore) EnqueueNotificationMessage(ctx context.Context, arg d return r0 } +func (m queryMetricsStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + start := time.Now() + r0 := m.s.ExpirePrebuildsAPIKeys(ctx, now) + m.queryLatencies.WithLabelValues("ExpirePrebuildsAPIKeys").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) FavoriteWorkspace(ctx context.Context, arg uuid.UUID) error { start := time.Now() r0 := m.s.FavoriteWorkspace(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b20c3d06209b5..bebc15616ca05 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -933,6 +933,20 @@ func (mr *MockStoreMockRecorder) EnqueueNotificationMessage(ctx, arg any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnqueueNotificationMessage", reflect.TypeOf((*MockStore)(nil).EnqueueNotificationMessage), ctx, arg) } +// ExpirePrebuildsAPIKeys mocks base method. +func (m *MockStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExpirePrebuildsAPIKeys", ctx, now) + ret0, _ := ret[0].(error) + return ret0 +} + +// ExpirePrebuildsAPIKeys indicates an expected call of ExpirePrebuildsAPIKeys. +func (mr *MockStoreMockRecorder) ExpirePrebuildsAPIKeys(ctx, now any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExpirePrebuildsAPIKeys", reflect.TypeOf((*MockStore)(nil).ExpirePrebuildsAPIKeys), ctx, now) +} + // FavoriteWorkspace mocks base method. func (m *MockStore) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { m.ctrl.T.Helper() diff --git a/coderd/database/dbpurge/dbpurge.go b/coderd/database/dbpurge/dbpurge.go index 135d7f40b05dd..5b7346c36fe56 100644 --- a/coderd/database/dbpurge/dbpurge.go +++ b/coderd/database/dbpurge/dbpurge.go @@ -67,6 +67,9 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz. if err := tx.DeleteOldNotificationMessages(ctx); err != nil { return xerrors.Errorf("failed to delete old notification messages: %w", err) } + if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil { + return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err) + } deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge) if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{ diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 1d57a87e68f48..547b9cee098b9 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -25,6 +25,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbrollup" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionerd/proto" "github.com/coder/coder/v2/provisionersdk" @@ -635,3 +636,68 @@ func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) { require.Len(t, logs, 0) } + +func TestExpireOldAPIKeys(t *testing.T) { + t.Parallel() + + // Given: a number of workspaces and API keys owned by a regular user and the prebuilds system user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + now = dbtime.Now() + db, _ = dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + tpl = dbgen.Template(t, db, database.Template{OrganizationID: org.ID, CreatedBy: user.ID}) + userWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: tpl.ID, + }) + prebuildsWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + TemplateID: tpl.ID, + }) + createAPIKey = func(userID uuid.UUID, name string) database.APIKey { + k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) { + iap.TokenName = name + }) + return k + } + assertKeyActive = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.After(now)) + } + assertKeyExpired = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.Equal(now)) + } + unnamedUserAPIKey = createAPIKey(user.ID, "") + unnamedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "") + namedUserAPIKey = createAPIKey(user.ID, "my-token") + namedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "also-my-token") + userWorkspaceAPIKey1 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, userWs.ID)) + userWorkspaceAPIKey2 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey1 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey2 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, userWs.ID)) + ) + + // When: we call ExpirePrebuildsAPIKeys + err := db.ExpirePrebuildsAPIKeys(ctx, now) + // Then: no errors is reported. + require.NoError(t, err) + + // We do not touch user API keys. + assertKeyActive(unnamedUserAPIKey.ID) + assertKeyActive(namedUserAPIKey.ID) + assertKeyActive(userWorkspaceAPIKey1.ID) + assertKeyActive(userWorkspaceAPIKey2.ID) + // Unnamed prebuilds API keys get expired. + assertKeyExpired(unnamedPrebuildsAPIKey.ID) + // API keys for workspaces still owned by prebuilds user remain active until claimed. + assertKeyActive(prebuildsWorkspaceAPIKey1.ID) + // API keys for workspaces no longer owned by prebuilds user get expired. + assertKeyExpired(prebuildsWorkspaceAPIKey2.ID) + // Out of an abundance of caution, we do not expire explicitly named prebuilds API keys. + assertKeyActive(namedPrebuildsAPIKey.ID) +} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index baa5d8590b1d7..e622c94c54b3e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -128,6 +128,11 @@ type sqlcQuerier interface { // of the test-only in-memory database. Do not use this in new code. DisableForeignKeysAndTriggers(ctx context.Context) error EnqueueNotificationMessage(ctx context.Context, arg EnqueueNotificationMessageParams) error + // Firstly, collect api_keys owned by the prebuilds user that correlate + // to workspaces no longer owned by the prebuilds user. + // Next, collect api_keys that belong to the prebuilds user but have no token name. + // These were most likely created via 'coder login' as the prebuilds user. + ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error FavoriteWorkspace(ctx context.Context, id uuid.UUID) error FetchMemoryResourceMonitorsByAgentID(ctx context.Context, agentID uuid.UUID) (WorkspaceAgentMemoryResourceMonitor, error) FetchMemoryResourceMonitorsUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]WorkspaceAgentMemoryResourceMonitor, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 80357b3731874..7e36f2c037d3c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -144,6 +144,46 @@ func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context return err } +const expirePrebuildsAPIKeys = `-- name: ExpirePrebuildsAPIKeys :exec +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > $1::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > $1::timestamptz +) +UPDATE api_keys +SET expires_at = $1::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +) +` + +// Firstly, collect api_keys owned by the prebuilds user that correlate +// to workspaces no longer owned by the prebuilds user. +// Next, collect api_keys that belong to the prebuilds user but have no token name. +// These were most likely created via 'coder login' as the prebuilds user. +func (q *sqlQuerier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + _, err := q.db.ExecContext(ctx, expirePrebuildsAPIKeys, now) + return err +} + const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope, token_name diff --git a/coderd/database/queries/apikeys.sql b/coderd/database/queries/apikeys.sql index 4ff77cb469cd5..98be411ca65ea 100644 --- a/coderd/database/queries/apikeys.sql +++ b/coderd/database/queries/apikeys.sql @@ -83,3 +83,37 @@ DELETE FROM api_keys WHERE user_id = $1; + +-- name: ExpirePrebuildsAPIKeys :exec +-- Firstly, collect api_keys owned by the prebuilds user that correlate +-- to workspaces no longer owned by the prebuilds user. +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > @now::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +-- Next, collect api_keys that belong to the prebuilds user but have no token name. +-- These were most likely created via 'coder login' as the prebuilds user. +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > @now::timestamptz +) +UPDATE api_keys +SET expires_at = @now::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +); diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 518b48d2fe04b..43ae5e7b5da96 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -2711,15 +2711,23 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. return nil } -func workspaceSessionTokenName(workspace database.Workspace) string { - return fmt.Sprintf("%s_%s_session_token", workspace.OwnerID, workspace.ID) +func WorkspaceSessionTokenName(ownerID, workspaceID uuid.UUID) string { + return fmt.Sprintf("%s_%s_session_token", ownerID, workspaceID) } func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) { + // NOTE(Cian): Once a workspace is claimed, there's no reason for the session token to be valid any longer. + // Not generating any session token at all for a system user may unintentionally break existing templates, + // which we want to avoid. If there's no session token for the workspace belonging to the prebuilds user, + // then there's nothing for us to worry about here. + // TODO(Cian): Update this to handle _all_ system users. At the time of writing, only one system user exists. + if err := deleteSessionTokenForUserAndWorkspace(ctx, s.Database, database.PrebuildsSystemUserID, workspace.ID); err != nil && !errors.Is(err, sql.ErrNoRows) { + s.Logger.Error(ctx, "failed to delete prebuilds session token", slog.Error(err), slog.F("workspace_id", workspace.ID)) + } newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{ UserID: user.ID, LoginType: user.LoginType, - TokenName: workspaceSessionTokenName(workspace), + TokenName: WorkspaceSessionTokenName(workspace.OwnerID, workspace.ID), DefaultLifetime: s.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()), }) @@ -2747,10 +2755,14 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User, } func deleteSessionToken(ctx context.Context, db database.Store, workspace database.Workspace) error { + return deleteSessionTokenForUserAndWorkspace(ctx, db, workspace.OwnerID, workspace.ID) +} + +func deleteSessionTokenForUserAndWorkspace(ctx context.Context, db database.Store, userID, workspaceID uuid.UUID) error { err := db.InTx(func(tx database.Store) error { key, err := tx.GetAPIKeyByName(ctx, database.GetAPIKeyByNameParams{ - UserID: workspace.OwnerID, - TokenName: workspaceSessionTokenName(workspace), + UserID: userID, + TokenName: WorkspaceSessionTokenName(userID, workspaceID), }) if err == nil { err = tx.DeleteAPIKeyByID(ctx, key.ID) diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 66684835650a8..dbec8b4052acd 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -3576,6 +3576,70 @@ func TestNotifications(t *testing.T) { }) } +func TestServer_ExpirePrebuildsSessionToken(t *testing.T) { + t.Parallel() + + // Given: a prebuilt workspace where an API key was previously created for the prebuilds user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + srv, db, ps, pd = setup(t, false, nil) + user = dbgen.User(t, db, database.User{}) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + version = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + workspace = dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: pd.OrganizationID, + TemplateID: template.ID, + OwnerID: database.PrebuildsSystemUserID, + }) + workspaceBuildID = uuid.New() + buildJob = dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + OrganizationID: pd.OrganizationID, + FileID: dbgen.File(t, db, database.File{CreatedBy: user.ID}).ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{ + WorkspaceBuildID: workspaceBuildID, + })), + InitiatorID: database.PrebuildsSystemUserID, + Tags: pd.Tags, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + ID: workspaceBuildID, + WorkspaceID: workspace.ID, + TemplateVersionID: version.ID, + JobID: buildJob.ID, + Transition: database.WorkspaceTransitionStart, + InitiatorID: database.PrebuildsSystemUserID, + }) + existingKey, _ = dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + TokenName: provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, workspace.ID), + }) + ) + + // When: the prebuild claim job is acquired + fs := newFakeStream(ctx) + err := srv.AcquireJobWithCancel(fs) + require.NoError(t, err) + job, err := fs.waitForJob() + require.NoError(t, err) + require.NotNil(t, job) + workspaceBuildJob := job.Type.(*proto.AcquiredJob_WorkspaceBuild_).WorkspaceBuild + require.NotNil(t, workspaceBuildJob.Metadata) + + // Assert test invariant: we acquired the expected build job + require.Equal(t, workspaceBuildID.String(), workspaceBuildJob.WorkspaceBuildId) + // Then: The session token should be deleted + _, err = db.GetAPIKeyByID(ctx, existingKey.ID) + require.ErrorIs(t, err, sql.ErrNoRows, "api key for prebuilds user should be deleted") +} + type overrides struct { ctx context.Context deploymentValues *codersdk.DeploymentValues From a79adb155816c2a71e23c0cb1827b47177682bf2 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 3 Sep 2025 14:33:45 +0100 Subject: [PATCH 5/6] fix(coderd): add audit log on creating a new session key (#19672) (#19684) Fixes https://github.com/coder/coder/issues/19671 (re-?)Adds an audit log entry when an API key is created via `coder login`. NOTE: This does _not_ backfill audit logs. Screenshot 2025-09-02 at 14 16 24 (cherry picked from commit bd6e91eeab56db906272d45f790ac0ba92040f6c) --- coderd/apikey.go | 18 +++++++++++++++--- coderd/apikey_test.go | 23 +++++++++++++++++++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/coderd/apikey.go b/coderd/apikey.go index 12646d627a212..91ab0ad957624 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -131,8 +131,19 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { // @Success 201 {object} codersdk.GenerateAPIKeyResponse // @Router /users/{user}/keys [post] func (api *API) postAPIKey(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.APIKey](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + }) + ) + aReq.Old = database.APIKey{} + defer commitAudit() // TODO(Cian): System users technically just have the 'member' role // and we don't want to disallow all members from creating API keys. @@ -142,7 +153,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { return } - cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ + cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LoginType: database.LoginTypePassword, @@ -156,6 +167,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { return } + aReq.New = *key // We intentionally do not set the cookie on the response here. // Setting the cookie will couple the browser session to the API // key we return here, meaning logging out of the website would diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index 1509aa2e2f402..73655754a8fb3 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -2,6 +2,7 @@ package coderd_test import ( "context" + "encoding/json" "net/http" "strings" "testing" @@ -303,14 +304,32 @@ func TestSessionExpiry(t *testing.T) { func TestAPIKey_OK(t *testing.T) { t.Parallel() + + // Given: a deployment with auditing enabled ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - _ = coderdtest.CreateFirstUser(t, client) + auditor := audit.NewMock() + client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor}) + owner := coderdtest.CreateFirstUser(t, client) + auditor.ResetLogs() + // When: an API key is created res, err := client.CreateAPIKey(ctx, codersdk.Me) require.NoError(t, err) require.Greater(t, len(res.Key), 2) + + // Then: an audit log is generated + als := auditor.AuditLogs() + require.Len(t, als, 1) + al := als[0] + assert.Equal(t, owner.UserID, al.UserID) + assert.Equal(t, database.AuditActionCreate, al.Action) + assert.Equal(t, database.ResourceTypeApiKey, al.ResourceType) + + // Then: the diff MUST NOT contain the generated key. + raw, err := json.Marshal(al) + require.NoError(t, err) + require.NotContains(t, res.Key, string(raw)) } func TestAPIKey_Deleted(t *testing.T) { From f51e22da5caeae6be27dfe08e8ef98f9e1e838fa Mon Sep 17 00:00:00 2001 From: Stephen Kirby <58410745+stirby@users.noreply.github.com> Date: Wed, 3 Sep 2025 23:11:45 -0500 Subject: [PATCH 6/6] fix: pin pg_dump version when generating schema (#19696) (#19697) Co-authored-by: Ethan <39577870+ethanndickson@users.noreply.github.com> --- coderd/database/dbtestutil/db.go | 38 ++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index f67e3206b09d1..b70d00e4eb97b 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -10,7 +10,6 @@ import ( "os/exec" "path/filepath" "regexp" - "strconv" "strings" "testing" "time" @@ -251,26 +250,31 @@ func PGDump(dbURL string) ([]byte, error) { return stdout.Bytes(), nil } -const minimumPostgreSQLVersion = 13 +const ( + minimumPostgreSQLVersion = 13 + postgresImageSha = "sha256:467e7f2fb97b2f29d616e0be1d02218a7bbdfb94eb3cda7461fd80165edfd1f7" +) // PGDumpSchemaOnly is for use by gen/dump only. // It runs pg_dump against dbURL and sets a consistent timezone and encoding. func PGDumpSchemaOnly(dbURL string) ([]byte, error) { hasPGDump := false - if _, err := exec.LookPath("pg_dump"); err == nil { - out, err := exec.Command("pg_dump", "--version").Output() - if err == nil { - // Parse output: - // pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1) - parts := strings.Split(string(out), " ") - if len(parts) > 2 { - version, err := strconv.Atoi(strings.Split(parts[2], ".")[0]) - if err == nil && version >= minimumPostgreSQLVersion { - hasPGDump = true - } - } - } - } + // TODO: Temporarily pin pg_dump to the docker image until + // https://github.com/sqlc-dev/sqlc/issues/4065 is resolved. + // if _, err := exec.LookPath("pg_dump"); err == nil { + // out, err := exec.Command("pg_dump", "--version").Output() + // if err == nil { + // // Parse output: + // // pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1) + // parts := strings.Split(string(out), " ") + // if len(parts) > 2 { + // version, err := strconv.Atoi(strings.Split(parts[2], ".")[0]) + // if err == nil && version >= minimumPostgreSQLVersion { + // hasPGDump = true + // } + // } + // } + // } cmdArgs := []string{ "pg_dump", @@ -295,7 +299,7 @@ func PGDumpSchemaOnly(dbURL string) ([]byte, error) { "run", "--rm", "--network=host", - fmt.Sprintf("%s:%d", postgresImage, minimumPostgreSQLVersion), + fmt.Sprintf("%s:%d@%s", postgresImage, minimumPostgreSQLVersion, postgresImageSha), }, cmdArgs...) } cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...) //#nosec