diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 7d0ecca6c3f92..7de4e0a806e15 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -347,10 +347,18 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req dbAutostartSchedule.String = *createWorkspace.AutostartSchedule } - var dbTTL sql.NullInt64 - if createWorkspace.TTL != nil && *createWorkspace.TTL > 0 { - dbTTL.Valid = true - dbTTL.Int64 = int64(*createWorkspace.TTL) + dbTTL, err := validWorkspaceTTL(createWorkspace.TTL) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "validate workspace ttl", + Errors: []httpapi.Error{ + { + Field: "ttl", + Detail: err.Error(), + }, + }, + }) + return } workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ @@ -559,14 +567,21 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { return } - var dbTTL sql.NullInt64 - if req.TTL != nil && *req.TTL > 0 { - truncated := req.TTL.Truncate(time.Minute) - dbTTL.Int64 = int64(truncated) - dbTTL.Valid = true + dbTTL, err := validWorkspaceTTL(req.TTL) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "validate workspace ttl", + Errors: []httpapi.Error{ + { + Field: "ttl", + Detail: err.Error(), + }, + }, + }) + return } - err := api.Database.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{ + err = api.Database.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{ ID: workspace.ID, Ttl: dbTTL, }) @@ -590,36 +605,29 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { return } - var code = http.StatusOK + code := http.StatusOK + resp := httpapi.Response{} err := api.Database.InTx(func(s database.Store) error { build, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if err != nil { code = http.StatusInternalServerError + resp.Message = "workspace not found" return xerrors.Errorf("get latest workspace build: %w", err) } if build.Transition != database.WorkspaceTransitionStart { code = http.StatusConflict + resp.Message = "workspace must be started, current status: " + string(build.Transition) return xerrors.Errorf("workspace must be started, current status: %s", build.Transition) } newDeadline := req.Deadline.UTC() - if newDeadline.IsZero() { - // This should not be possible because the struct validation field enforces a non-zero value. - code = http.StatusBadRequest - return xerrors.New("new deadline cannot be zero") - } - - if newDeadline.Before(build.Deadline) || newDeadline.Before(time.Now()) { + if err := validWorkspaceDeadline(build.Deadline, newDeadline); err != nil { code = http.StatusBadRequest - return xerrors.Errorf("new deadline %q must be after existing deadline %q", newDeadline.Format(time.RFC3339), build.Deadline.Format(time.RFC3339)) - } - - // Disallow updates within less than one minute - if withinDuration(newDeadline, build.Deadline, time.Minute) { - code = http.StatusNotModified - return nil + resp.Message = "bad extend workspace request" + resp.Errors = append(resp.Errors, httpapi.Error{Field: "deadline", Detail: err.Error()}) + return err } if err := s.UpdateWorkspaceBuildByID(r.Context(), database.UpdateWorkspaceBuildByIDParams{ @@ -628,15 +636,17 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { ProvisionerState: build.ProvisionerState, Deadline: newDeadline, }); err != nil { + code = http.StatusInternalServerError + resp.Message = "failed to extend workspace deadline" return xerrors.Errorf("update workspace build: %w", err) } + resp.Message = "deadline updated to " + newDeadline.Format(time.RFC3339) return nil }) - var resp = httpapi.Response{} if err != nil { - resp.Message = err.Error() + api.Logger.Info(r.Context(), "extending workspace", slog.Error(err)) } httpapi.Write(rw, code, resp) } @@ -850,11 +860,44 @@ func convertSQLNullInt64(i sql.NullInt64) *time.Duration { return (*time.Duration)(&i.Int64) } -func withinDuration(t1, t2 time.Time, d time.Duration) bool { - dt := t1.Sub(t2) - if dt < -d || dt > d { - return false +func validWorkspaceTTL(ttl *time.Duration) (sql.NullInt64, error) { + if ttl == nil { + return sql.NullInt64{}, nil + } + + truncated := ttl.Truncate(time.Minute) + if truncated < time.Minute { + return sql.NullInt64{}, xerrors.New("ttl must be at least one minute") + } + + if truncated > 24*7*time.Hour { + return sql.NullInt64{}, xerrors.New("ttl must be less than 7 days") + } + + return sql.NullInt64{ + Valid: true, + Int64: int64(truncated), + }, nil +} + +func validWorkspaceDeadline(old, new time.Time) error { + if old.IsZero() { + return xerrors.New("nothing to do: no existing deadline set") + } + + now := time.Now() + if new.Before(now) { + return xerrors.New("new deadline must be in the future") + } + + delta := new.Sub(old) + if delta < time.Minute { + return xerrors.New("minimum extension is one minute") + } + + if delta > 24*time.Hour { + return xerrors.New("maximum extension is 24 hours") } - return true + return nil } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index ed898d1e1dc51..3b0bfe79c728f 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -164,6 +164,49 @@ func TestPostWorkspacesByOrganization(t *testing.T) { coderdtest.AwaitTemplateVersionJob(t, client, version.ID) _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) }) + + t.Run("InvalidTTL", func(t *testing.T) { + t.Parallel() + t.Run("BelowMin", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + req := codersdk.CreateWorkspaceRequest{ + TemplateID: template.ID, + Name: "testing", + AutostartSchedule: ptr("CRON_TZ=US/Central * * * * *"), + TTL: ptr(59 * time.Second), + } + _, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req) + require.Error(t, err) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + }) + + t.Run("AboveMax", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + req := codersdk.CreateWorkspaceRequest{ + TemplateID: template.ID, + Name: "testing", + AutostartSchedule: ptr("CRON_TZ=US/Central * * * * *"), + TTL: ptr(24*7*time.Hour + time.Minute), + } + _, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req) + require.Error(t, err) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + }) + }) } func TestWorkspacesByOrganization(t *testing.T) { @@ -552,10 +595,25 @@ func TestWorkspaceUpdateTTL(t *testing.T) { expectedError: "", }, { - name: "enable ttl", - ttl: ptr(time.Hour), + name: "below minimum ttl", + ttl: ptr(30 * time.Second), + expectedError: "ttl must be at least one minute", + }, + { + name: "minimum ttl", + ttl: ptr(time.Minute), + expectedError: "", + }, + { + name: "maximum ttl", + ttl: ptr(24 * 7 * time.Hour), expectedError: "", }, + { + name: "above maximum ttl", + ttl: ptr(24*7*time.Hour + time.Minute), + expectedError: "ttl must be less than 7 days", + }, } for _, testCase := range testCases { @@ -583,7 +641,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) { }) if testCase.expectedError != "" { - require.EqualError(t, err, testCase.expectedError, "unexpected error when setting workspace autostop schedule") + require.ErrorContains(t, err, testCase.expectedError, "unexpected error when setting workspace autostop schedule") return } @@ -657,7 +715,13 @@ func TestWorkspaceExtend(t *testing.T) { err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ Deadline: oldDeadline, }) - require.ErrorContains(t, err, "must be after existing deadline", "setting an earlier deadline should fail") + require.ErrorContains(t, err, "deadline: minimum extension is one minute", "setting an earlier deadline should fail") + + // Updating with a time far in the future should also fail + err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ + Deadline: oldDeadline.AddDate(1, 0, 0), + }) + require.ErrorContains(t, err, "deadline: maximum extension is 24 hours", "setting an earlier deadline should fail") // Ensure deadline still set correctly updated, err = client.Workspace(ctx, workspace.ID)