From c352a51b22c01c64d3bd9d5d8aab3624f4ad587e Mon Sep 17 00:00:00 2001 From: George K Date: Tue, 27 Jan 2026 09:08:12 -0800 Subject: [PATCH] fix(coderd): authorize workspace start/stop/delete by transition action (#21691) Use transition-specific actions when authorizing workspace build parameter inserts in the database layer so start/stop/delete do not require workspace.update. Related to: https://github.com/coder/internal/issues/1299 --- coderd/database/dbauthz/dbauthz.go | 28 +++++++++++---- coderd/database/dbauthz/dbauthz_test.go | 46 ++++++++++++++++++++++--- coderd/wsbuilder/wsbuilder.go | 12 +++++-- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index a427d32afb..5de1586c98 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -174,6 +174,19 @@ func (q *querier) authorizePrebuiltWorkspace(ctx context.Context, action policy. return xerrors.Errorf("authorize context: %w", workspaceErr) } +func workspaceTransitionAction(transition database.WorkspaceTransition) (policy.Action, error) { + switch transition { + case database.WorkspaceTransitionStart: + return policy.ActionWorkspaceStart, nil + case database.WorkspaceTransitionStop: + return policy.ActionWorkspaceStop, nil + case database.WorkspaceTransitionDelete: + return policy.ActionDelete, nil + default: + return "", xerrors.Errorf("unsupported workspace transition %q", transition) + } +} + // authorizeAIBridgeInterceptionAction validates that the context's actor matches the initiator of the AIBridgeInterception. // This is used by all of the sub-resources which fall under the [ResourceAibridgeInterception] umbrella. func (q *querier) authorizeAIBridgeInterceptionAction(ctx context.Context, action policy.Action, interceptionID uuid.UUID) error { @@ -4594,11 +4607,9 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW return xerrors.Errorf("get workspace by id: %w", err) } - var action policy.Action = policy.ActionWorkspaceStart - if arg.Transition == database.WorkspaceTransitionDelete { - action = policy.ActionDelete - } else if arg.Transition == database.WorkspaceTransitionStop { - action = policy.ActionWorkspaceStop + action, err := workspaceTransitionAction(arg.Transition) + if err != nil { + return err } // Special handling for prebuilt workspace deletion @@ -4642,8 +4653,13 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa return err } + action, err := workspaceTransitionAction(build.Transition) + if err != nil { + return err + } + // Special handling for prebuilt workspace deletion - if err := q.authorizePrebuiltWorkspace(ctx, policy.ActionUpdate, workspace); err != nil { + if err := q.authorizePrebuiltWorkspace(ctx, action, workspace); err != nil { return err } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 08ce4ef2f3..0646de4214 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2145,9 +2145,12 @@ func (s *MethodTestSuite) TestWorkspace() { dbm.EXPECT().InsertWorkspaceBuild(gomock.Any(), arg).Return(nil).AnyTimes() check.Args(arg).Asserts(w, policy.ActionDelete) })) - s.Run("InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + s.Run("Start/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { w := testutil.Fake(s.T(), faker, database.Workspace{}) - b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{WorkspaceID: w.ID}) + b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{ + WorkspaceID: w.ID, + Transition: database.WorkspaceTransitionStart, + }) arg := database.InsertWorkspaceBuildParametersParams{ WorkspaceBuildID: b.ID, Name: []string{"foo", "bar"}, @@ -2156,7 +2159,39 @@ func (s *MethodTestSuite) TestWorkspace() { dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes() dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes() dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes() - check.Args(arg).Asserts(w, policy.ActionUpdate) + check.Args(arg).Asserts(w, policy.ActionWorkspaceStart) + })) + s.Run("Stop/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + w := testutil.Fake(s.T(), faker, database.Workspace{}) + b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{ + WorkspaceID: w.ID, + Transition: database.WorkspaceTransitionStop, + }) + arg := database.InsertWorkspaceBuildParametersParams{ + WorkspaceBuildID: b.ID, + Name: []string{"foo", "bar"}, + Value: []string{"baz", "qux"}, + } + dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes() + dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes() + dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg).Asserts(w, policy.ActionWorkspaceStop) + })) + s.Run("Delete/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + w := testutil.Fake(s.T(), faker, database.Workspace{}) + b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{ + WorkspaceID: w.ID, + Transition: database.WorkspaceTransitionDelete, + }) + arg := database.InsertWorkspaceBuildParametersParams{ + WorkspaceBuildID: b.ID, + Name: []string{"foo", "bar"}, + Value: []string{"baz", "qux"}, + } + dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes() + dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes() + dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg).Asserts(w, policy.ActionDelete) })) s.Run("UpdateWorkspace", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { w := testutil.Fake(s.T(), faker, database.Workspace{}) @@ -4407,7 +4442,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { return nil }).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete) })) - s.Run("PrebuildUpdate/InsertWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) { + s.Run("PrebuildDelete/InsertWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) o := dbgen.Organization(s.T(), db, database.Organization{}) tpl := dbgen.Template(s.T(), db, database.Template{ @@ -4429,6 +4464,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { }) wb := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{ JobID: pj.ID, + Transition: database.WorkspaceTransitionDelete, WorkspaceID: w.ID, TemplateVersionID: tv.ID, }) @@ -4444,7 +4480,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { return xerrors.Errorf("not authorized for workspace type") } return nil - }).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) + }).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete) })) } diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 9bcd310f7b..d7ace7a4e2 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -1175,8 +1175,16 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje switch b.trans { case database.WorkspaceTransitionDelete: action = policy.ActionDelete - case database.WorkspaceTransitionStart, database.WorkspaceTransitionStop: - action = policy.ActionUpdate + case database.WorkspaceTransitionStart: + action = policy.ActionWorkspaceStart + if b.workspace.DormantAt.Valid { + // Dormant workspaces can't be started directly; they are + // first "woken" by unsetting dormancy, which makes the + // workspace.start permission apply. + action = policy.ActionUpdate + } + case database.WorkspaceTransitionStop: + action = policy.ActionWorkspaceStop default: msg := fmt.Sprintf("Transition %q not supported.", b.trans) return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)}