From 2e71f9355862611078a4ae96429c1cfc5e8e59b7 Mon Sep 17 00:00:00 2001 From: Matt Boyle Date: Thu, 12 Mar 2026 15:58:35 +0000 Subject: [PATCH] Enforce org inactivity policy on workspace timeout changes Users could bypass the organization's inactivity timeout policy by calling setWorkspaceTimeout at runtime with a value exceeding the org limit. The org policy was only applied at workspace start time. Changes: - setWorkspaceTimeout now rejects durations exceeding the org's inactivity policy - Workspace starter no longer lets user default timeout override the org inactivity policy - updateWorkspace API properly rejects inactivity field with seconds=0 (was silently ignored due to falsy check) - Add WorkspaceTimeoutDuration.toMs helper for duration comparison - Add tests for org policy enforcement Co-authored-by: Ona --- .../src/service/json-rpc-workspace-client.ts | 2 +- .../gitpod-protocol/src/gitpod-service.ts | 16 ++++++ .../src/timeout-validation.spec.ts | 20 ++++++++ .../server/src/api/workspace-service-api.ts | 2 +- .../workspace/workspace-service.spec.db.ts | 49 +++++++++++++++++++ .../server/src/workspace/workspace-service.ts | 12 +++++ .../server/src/workspace/workspace-starter.ts | 15 +++++- 7 files changed, 112 insertions(+), 4 deletions(-) diff --git a/components/dashboard/src/service/json-rpc-workspace-client.ts b/components/dashboard/src/service/json-rpc-workspace-client.ts index 4b8ddfb4e6fea4..7ff2b0dfec6ea7 100644 --- a/components/dashboard/src/service/json-rpc-workspace-client.ts +++ b/components/dashboard/src/service/json-rpc-workspace-client.ts @@ -275,7 +275,7 @@ export class JsonRpcWorkspaceClient implements PromiseClient 0) ) { throw new ApplicationError(ErrorCodes.UNIMPLEMENTED, "not implemented"); diff --git a/components/gitpod-protocol/src/gitpod-service.ts b/components/gitpod-protocol/src/gitpod-service.ts index ad68bf984162a1..4b6266fe3a4d8d 100644 --- a/components/gitpod-protocol/src/gitpod-service.ts +++ b/components/gitpod-protocol/src/gitpod-service.ts @@ -394,6 +394,22 @@ export namespace WorkspaceTimeoutDuration { throw new Error(`Invalid timeout format: ${duration}. Use Go duration format (e.g., "30m", "1h30m", "2h")`); } } + + /** + * Parses a Go-style duration string to milliseconds. + * Returns undefined if the duration is invalid. + */ + export function toMs(duration: string): number | undefined { + try { + const ms = parse(duration.toLowerCase()); + if (ms === undefined || ms === null) { + return undefined; + } + return ms; + } catch { + return undefined; + } + } } export const WORKSPACE_TIMEOUT_DEFAULT_SHORT: WorkspaceTimeoutDuration = "30m"; diff --git a/components/gitpod-protocol/src/timeout-validation.spec.ts b/components/gitpod-protocol/src/timeout-validation.spec.ts index 44357307c8aec8..3e6bcc9849350f 100644 --- a/components/gitpod-protocol/src/timeout-validation.spec.ts +++ b/components/gitpod-protocol/src/timeout-validation.spec.ts @@ -67,6 +67,26 @@ describe("WorkspaceTimeoutDuration", () => { // Zero duration components are handled by the totalMinutes > 0 check expect(() => WorkspaceTimeoutDuration.validate("0m")).to.throw("Invalid timeout value"); expect(() => WorkspaceTimeoutDuration.validate("0h")).to.throw("Invalid timeout value"); + expect(() => WorkspaceTimeoutDuration.validate("0s")).to.throw("Invalid timeout value"); + }); + }); + + describe("toMs", () => { + it("should parse valid durations to milliseconds", () => { + expect(WorkspaceTimeoutDuration.toMs("30m")).to.equal(30 * 60 * 1000); + expect(WorkspaceTimeoutDuration.toMs("1h")).to.equal(60 * 60 * 1000); + expect(WorkspaceTimeoutDuration.toMs("1h30m")).to.equal(90 * 60 * 1000); + expect(WorkspaceTimeoutDuration.toMs("2h15m")).to.equal(135 * 60 * 1000); + }); + + it("should return undefined for invalid durations", () => { + expect(WorkspaceTimeoutDuration.toMs("invalid")).to.be.undefined; + expect(WorkspaceTimeoutDuration.toMs("")).to.be.undefined; + }); + + it("should handle zero durations", () => { + expect(WorkspaceTimeoutDuration.toMs("0s")).to.equal(0); + expect(WorkspaceTimeoutDuration.toMs("0m")).to.equal(0); }); }); }); diff --git a/components/server/src/api/workspace-service-api.ts b/components/server/src/api/workspace-service-api.ts index febe968c558134..659468a457770d 100644 --- a/components/server/src/api/workspace-service-api.ts +++ b/components/server/src/api/workspace-service-api.ts @@ -349,7 +349,7 @@ export class WorkspaceServiceAPI implements ServiceImpl 0)) { + if (req.spec?.timeout?.inactivity !== undefined || (req.spec?.sshPublicKeys && req.spec?.sshPublicKeys.length > 0)) { throw new ApplicationError(ErrorCodes.UNIMPLEMENTED, "not implemented"); } const userId = ctxUserId(); diff --git a/components/server/src/workspace/workspace-service.spec.db.ts b/components/server/src/workspace/workspace-service.spec.db.ts index 33869f3bb6818c..b83d620b6925ea 100644 --- a/components/server/src/workspace/workspace-service.spec.db.ts +++ b/components/server/src/workspace/workspace-service.spec.db.ts @@ -473,6 +473,55 @@ describe("WorkspaceService", async () => { ); }); + it("should reject setWorkspaceTimeout when denyUserTimeouts is set", async () => { + const svc = container.get(WorkspaceService); + const orgService = container.get(OrganizationService); + const ws = await createTestWorkspace(svc, org, owner, project); + + await orgService.updateSettings(owner.id, org.id, { + timeoutSettings: { denyUserTimeouts: true }, + }); + + await expectError( + ErrorCodes.PRECONDITION_FAILED, + svc.setWorkspaceTimeout(owner.id, ws.id, "60m"), + "should fail when user timeouts are denied", + ); + }); + + it("should reject setWorkspaceTimeout exceeding org inactivity policy", async () => { + const svc = container.get(WorkspaceService); + const orgService = container.get(OrganizationService); + const ws = await createTestWorkspace(svc, org, owner, project); + + await orgService.updateSettings(owner.id, org.id, { + timeoutSettings: { inactivity: "30m" }, + }); + + await expectError( + ErrorCodes.PRECONDITION_FAILED, + svc.setWorkspaceTimeout(owner.id, ws.id, "60m"), + "should fail when timeout exceeds org inactivity policy", + ); + }); + + it("should allow setWorkspaceTimeout within org inactivity policy", async () => { + const svc = container.get(WorkspaceService); + const orgService = container.get(OrganizationService); + const ws = await createTestWorkspace(svc, org, owner, project); + + await orgService.updateSettings(owner.id, org.id, { + timeoutSettings: { inactivity: "60m" }, + }); + + // This should pass the org policy check but fail on non-running workspace + await expectError( + ErrorCodes.NOT_FOUND, + svc.setWorkspaceTimeout(owner.id, ws.id, "30m"), + "should pass org policy check but fail on non-running workspace", + ); + }); + it("should getHeadlessLog", async () => { const svc = container.get(WorkspaceService); await createTestWorkspace(svc, org, owner, project); diff --git a/components/server/src/workspace/workspace-service.ts b/components/server/src/workspace/workspace-service.ts index c44657d8e0fac7..becaf6000d97d2 100644 --- a/components/server/src/workspace/workspace-service.ts +++ b/components/server/src/workspace/workspace-service.ts @@ -1076,6 +1076,18 @@ export class WorkspaceService { ); } + // Enforce org inactivity policy as a maximum: user-set timeout must not exceed it + if (orgSettings.timeoutSettings?.inactivity) { + const orgTimeoutMs = WorkspaceTimeoutDuration.toMs(orgSettings.timeoutSettings.inactivity); + const requestedMs = WorkspaceTimeoutDuration.toMs(validatedDuration); + if (orgTimeoutMs !== undefined && requestedMs !== undefined && requestedMs > orgTimeoutMs) { + throw new ApplicationError( + ErrorCodes.PRECONDITION_FAILED, + `Timeout exceeds the organization's inactivity limit of ${orgSettings.timeoutSettings.inactivity}`, + ); + } + } + const instance = await this.getCurrentInstance(userId, workspaceId); if (instance.status.phase !== "running" || workspace.type !== "regular") { throw new ApplicationError(ErrorCodes.NOT_FOUND, "Can only set keep-alive for regular, running workspaces"); diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index 38655f6c2612a7..9db34453eaf590 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -1673,11 +1673,22 @@ export class WorkspaceStarter { } catch (err) {} } - // Users can optionally override the organization-wide timeout default if the organization allows it + // Users can optionally override the organization-wide timeout default if the organization allows it, + // but the user timeout must not exceed the org's inactivity policy. if (!organizationSettings.timeoutSettings?.denyUserTimeouts && user.additionalData?.workspaceTimeout) { try { const timeout = WorkspaceTimeoutDuration.validate(user.additionalData?.workspaceTimeout); - spec.setTimeout(timeout); + const orgTimeoutMs = organizationSettings.timeoutSettings?.inactivity + ? WorkspaceTimeoutDuration.toMs(organizationSettings.timeoutSettings.inactivity) + : undefined; + const userTimeoutMs = WorkspaceTimeoutDuration.toMs(timeout); + if ( + orgTimeoutMs === undefined || + userTimeoutMs === undefined || + userTimeoutMs <= orgTimeoutMs + ) { + spec.setTimeout(timeout); + } } catch (err) {} }