From 286779ec458b06ecfaf91e56942626b1e6e7d1bd Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 7 Jun 2026 19:35:07 -0700 Subject: [PATCH] feat(ws-server): validate compute.provider against the cloud-provider SSOT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateWorkspaceCompute checked instance_type / volume / display / data_persistence but NOT compute.provider — a typo'd provider flowed to CP and only fail-closed there with a 422. Add an allowlist mirroring the controlplane cloudprovider SSOT (aws|gcp|hetzner) so a bad provider gets a clean 400 before the round-trip. This is the validation seam the switch-provider flow (RFC #622) reuses. PR1 of the switch-existing-workspace-provider series. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/workspace_compute.go | 23 +++++++++++++++ .../handlers/workspace_compute_test.go | 28 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/workspace-server/internal/handlers/workspace_compute.go b/workspace-server/internal/handlers/workspace_compute.go index 3a105d988..a8434f22f 100644 --- a/workspace-server/internal/handlers/workspace_compute.go +++ b/workspace-server/internal/handlers/workspace_compute.go @@ -41,6 +41,20 @@ var workspaceComputeInstanceAllowlist = map[string]struct{}{ "c6i.xlarge": {}, } +// workspaceComputeProviderAllowlist mirrors the controlplane cloud-provider SSOT +// (controlplane internal/cloudprovider.Supported = {aws, hetzner, gcp}). +// ws-server lives in a different repo and cannot import that package, so this is +// a DELIBERATE mirror; TestValidateWorkspaceCompute_Provider pins the exact set +// and this doc-comment names the SSOT, so a CP-side change forces a matching +// change here (and the CP itself fail-closes an unwired provider with a 422). +// "" = default (AWS) and is always accepted. This is the gate the switch-provider +// flow reuses to reject a bad provider with a clean 400 before any CP round-trip. +var workspaceComputeProviderAllowlist = map[string]struct{}{ + "aws": {}, + "gcp": {}, + "hetzner": {}, +} + func validateWorkspaceCompute(compute models.WorkspaceCompute) error { if compute.InstanceType != "" { if _, ok := workspaceComputeInstanceAllowlist[compute.InstanceType]; !ok { @@ -73,6 +87,15 @@ func validateWorkspaceCompute(compute models.WorkspaceCompute) error { default: return fmt.Errorf("unsupported compute.data_persistence (want persist|ephemeral)") } + // Cloud backend for the box (multi-provider). "" = default (AWS). CP fail- + // closes an unwired provider with a 422 (PROVIDER_UNAVAILABLE); validating + // here gives a clean 400 before the round-trip and is the gate reused by the + // switch-provider flow. Mirrors the controlplane cloudprovider SSOT. + if compute.Provider != "" { + if _, ok := workspaceComputeProviderAllowlist[compute.Provider]; !ok { + return fmt.Errorf("unsupported compute.provider (want aws|gcp|hetzner)") + } + } return nil } diff --git a/workspace-server/internal/handlers/workspace_compute_test.go b/workspace-server/internal/handlers/workspace_compute_test.go index ba1c38ab1..26d23ebce 100644 --- a/workspace-server/internal/handlers/workspace_compute_test.go +++ b/workspace-server/internal/handlers/workspace_compute_test.go @@ -36,6 +36,34 @@ func TestValidateWorkspaceCompute_RejectsUnknownInstanceType(t *testing.T) { } } +// Multi-provider: compute.provider must be "" (default AWS) or one of the wired +// cloud backends. Pins the allowlist to the controlplane cloudprovider SSOT +// (Supported = {aws, hetzner, gcp}); if the SSOT changes, update both sides. +func TestValidateWorkspaceCompute_Provider(t *testing.T) { + for _, ok := range []string{"", "aws", "gcp", "hetzner"} { + c := models.WorkspaceCompute{Provider: ok} + if err := validateWorkspaceCompute(c); err != nil { + t.Errorf("provider=%q must be accepted: %v", ok, err) + } + } + for _, bad := range []string{"AWS", "azure", "digitalocean", "ec2", "google", "hetzner-cloud"} { + c := models.WorkspaceCompute{Provider: bad} + if err := validateWorkspaceCompute(c); err == nil { + t.Errorf("provider=%q must be rejected", bad) + } + } + // Pin the exact SSOT-mirrored set so a silent drift fails here. + want := map[string]struct{}{"aws": {}, "gcp": {}, "hetzner": {}} + if len(workspaceComputeProviderAllowlist) != len(want) { + t.Fatalf("provider allowlist drifted from SSOT {aws,gcp,hetzner}: %v", workspaceComputeProviderAllowlist) + } + for p := range want { + if _, ok := workspaceComputeProviderAllowlist[p]; !ok { + t.Fatalf("provider allowlist missing %q (SSOT drift)", p) + } + } +} + // internal#734: data_persistence enum. "" (auto), "persist", "ephemeral" are // the only accepted values; anything else is a clear 400 before the CP call. func TestValidateWorkspaceCompute_DataPersistence(t *testing.T) { -- 2.52.0