From fa967aa97dc6b8404befac5f45dec6b2218e1168 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Tue, 18 Aug 2026 21:36:12 -0300 Subject: [PATCH 1/3] feat(preview): optional hosted sandboxes, off by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A preview cannot drive agents today: SANDBOX_START creates a SandboxClaim through the Kubernetes API and the preview's ServiceAccount has no rights in the sandbox namespace, so it fails with a 403 that surfaces as an unhandled rejection. This adds the binding, and keeps it off by default, because what it grants is not narrow. The Role lives in a namespace shared by every environment on the cluster, production included, and Kubernetes RBAC has no label scoping — so a preview bound there reaches production sandboxes too. A preview runs un-merged code from any PR carrying a label. That is the whole trade, and it is written at the top of the template rather than buried in a values comment. It is shaped this way because the sandbox namespace is hardcoded in the application and across the sandbox-env chart, on the premise that multiple envs share it. A namespace-per-environment, or the move to a controller API with a scoped token, removes the need for this entirely. Two guards, because both failures render something that looks correct: serviceAccount.create must be on (otherwise the binding lands on and every pod in the preview gets sandbox access, Postgres and MinIO included), and roleName must be set (an empty roleRef binds nothing). Permission and configuration stay separate: enabling this grants rights but changes no behaviour until the STUDIO_SANDBOX_* block is pointed at an existing environment, which values-preview.yaml now spells out. --- .github/workflows/helm-test.yml | 12 +++++ deploy/helm/studio/templates/_helpers.tpl | 15 ++++++ .../templates/preview-sandbox-rbac.yaml | 49 +++++++++++++++++++ deploy/helm/studio/templates/validations.yaml | 1 + deploy/helm/studio/values-preview.yaml | 25 +++++++++- deploy/helm/studio/values.yaml | 17 +++++++ 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 deploy/helm/studio/templates/preview-sandbox-rbac.yaml diff --git a/.github/workflows/helm-test.yml b/.github/workflows/helm-test.yml index 378ec64210..b62b282d95 100644 --- a/.github/workflows/helm-test.yml +++ b/.github/workflows/helm-test.yml @@ -496,6 +496,18 @@ jobs: --set preview.postgres.enabled=false \ --set externalSecret.enabled=false + # Binding sandbox access to the namespace's `default` ServiceAccount + # would hand it to every pod in the preview — Postgres and MinIO + # included — and the render would look perfectly healthy. + expect_fail "sandbox without a named ServiceAccount" "requires serviceAccount.create=true" \ + --set preview.host=h.example.com \ + --set preview.sandbox.enabled=true \ + --set preview.sandbox.roleName=some-role + expect_fail "sandbox without a role" "preview.sandbox.roleName is required" \ + --set preview.host=h.example.com \ + --set preview.sandbox.enabled=true \ + --set serviceAccount.create=true + # image.command is a list, which --set cannot express cleanly. printf 'image:\n command: ["bun","run","deco"]\n' > /tmp/no-skip.yaml expect_fail "missing --skip-migrations" "requires --skip-migrations" \ diff --git a/deploy/helm/studio/templates/_helpers.tpl b/deploy/helm/studio/templates/_helpers.tpl index 43ace27ba8..9afb1d2fbf 100644 --- a/deploy/helm/studio/templates/_helpers.tpl +++ b/deploy/helm/studio/templates/_helpers.tpl @@ -341,6 +341,21 @@ Service/Deployment name for the preview's own MinIO. {{- printf "%s-minio" (include "chart-deco-studio.fullname" .) | trunc 63 | trimSuffix "-" -}} {{- end }} +{{/* +Guards the sandbox binding. Both checks stop a render that would look correct +and grant the wrong thing. +*/}} +{{- define "chart-deco-studio.validatePreviewSandbox" -}} +{{- if and .Values.preview.enabled .Values.preview.sandbox.enabled }} +{{- if not .Values.serviceAccount.create }} +{{- fail "chart-deco-studio: preview.sandbox.enabled=true requires serviceAccount.create=true — otherwise the binding lands on the namespace's `default` ServiceAccount, handing sandbox access to every pod in the preview, Postgres and MinIO included" -}} +{{- end }} +{{- if not .Values.preview.sandbox.roleName }} +{{- fail "chart-deco-studio: preview.sandbox.roleName is required when preview.sandbox.enabled=true" -}} +{{- end }} +{{- end }} +{{- end }} + {{/* Validates per-PR preview releases. Every failure here is something that would otherwise render a healthy-looking object that silently does nothing: an diff --git a/deploy/helm/studio/templates/preview-sandbox-rbac.yaml b/deploy/helm/studio/templates/preview-sandbox-rbac.yaml new file mode 100644 index 0000000000..1be8841f7d --- /dev/null +++ b/deploy/helm/studio/templates/preview-sandbox-rbac.yaml @@ -0,0 +1,49 @@ +{{- if and .Values.preview.enabled .Values.preview.sandbox.enabled }} +{{- /* +Lets a preview drive hosted agent sandboxes. + +READ THIS BEFORE ENABLING. The Role bound here lives in the sandbox namespace, +and that namespace holds the sandboxes of EVERY environment on the cluster — +production included. Kubernetes RBAC has no label scoping, so this grants reach +over all of them, not only the ones a preview creates. The housekeeper's +CLAIM_SELECTOR separates what it sweeps; it does not separate what a +ServiceAccount can touch. + +A preview runs un-merged code from any pull request carrying the preview label. +Binding it here means a label on a PR is enough to obtain pods/portforward and +HTTPRoute delete against production sandboxes. + +It is written this way because the sandbox namespace is not configurable — it is +hardcoded in the application and throughout the sandbox-env chart, on the stated +premise that "multiple envs share agent-sandbox-system". Until that changes, or +until the application talks to a controller API instead of the Kubernetes API +(which would replace this with a scoped token), there is no way to give previews +sandboxes without also giving them this reach. + +Off by default. Turn it on deliberately, and for as long as you actually need it. +*/ -}} +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: {{ include "chart-deco-studio.fullname" . }}-sandbox + namespace: {{ required "preview.sandbox.namespace is required when preview.sandbox.enabled=true" .Values.preview.sandbox.namespace }} + labels: + {{- include "chart-deco-studio.labels" . | nindent 4 }} + annotations: + # Ahead of the pods, so the ServiceAccount can already claim by the time + # anything tries to. + argocd.argoproj.io/sync-wave: "-30" + # Deliberately loud: this object is the one place where a preview reaches + # outside its own namespace, and into a shared production one. + decocms.com/grants-access-to: {{ .Values.preview.sandbox.namespace | quote }} +subjects: + - kind: ServiceAccount + name: {{ include "chart-deco-studio.serviceAccountName" . }} + namespace: {{ .Release.Namespace }} +roleRef: + kind: Role + # An existing Role, not one this chart defines: the sandbox-env chart owns the + # verb set, and a copy here would drift from it silently. + name: {{ required "preview.sandbox.roleName is required when preview.sandbox.enabled=true" .Values.preview.sandbox.roleName }} + apiGroup: rbac.authorization.k8s.io +{{- end }} diff --git a/deploy/helm/studio/templates/validations.yaml b/deploy/helm/studio/templates/validations.yaml index c5238cdeae..b30587f11a 100644 --- a/deploy/helm/studio/templates/validations.yaml +++ b/deploy/helm/studio/templates/validations.yaml @@ -9,3 +9,4 @@ This file only runs chart-level validations and renders no resources. {{- include "chart-deco-studio.validateClickhouse" . -}} {{- include "chart-deco-studio.validateIngress" . -}} {{- include "chart-deco-studio.validatePreview" . -}} +{{- include "chart-deco-studio.validatePreviewSandbox" . -}} diff --git a/deploy/helm/studio/values-preview.yaml b/deploy/helm/studio/values-preview.yaml index d8faf64030..be70e0497e 100644 --- a/deploy/helm/studio/values-preview.yaml +++ b/deploy/helm/studio/values-preview.yaml @@ -164,6 +164,12 @@ topologySpreadConstraints: [] # Spot is safe here in a way it is nowhere else on this cluster: the database # is an emptyDir Postgres that the PreSync Job re-migrates on the next sync, # and a preview holds no PVC at all. A reclaim costs a reviewer a restart. +# A named ServiceAccount rather than the namespace's `default`. Costs nothing +# when sandboxes are off, and is required when they are on: binding sandbox +# access to `default` would hand it to every pod here, Postgres and MinIO too. +serviceAccount: + create: true + nodeSelector: decocms.com/nodepool: studio-preview tolerations: @@ -207,8 +213,25 @@ configMap: AUTH_EMAIL_PASSWORD_ENABLED: "true" # Several reviewers behind one office IP trip the auth limiter otherwise. DISABLE_RATE_LIMIT: "true" - # No hosted sandboxes in a preview. Agent dispatch returns 409 link_offline + # No hosted sandboxes by default. Agent dispatch returns 409 link_offline # with no local daemon attached — a documented limitation, not a bug. + # + # To test agent behaviour in a preview, set preview.sandbox.enabled=true + # (see values.yaml — it grants reach over a shared production namespace) and + # replace this block with the settings below, pointed at an existing + # environment whose templates and warm pool the preview borrows: + # + # STUDIO_SANDBOX_PROVIDER: "cluster" + # STUDIO_SANDBOX_RUNNER: "agent-sandbox" + # STUDIO_SANDBOX_TEMPLATE_NAME: "" + # STUDIO_ENV: "" + # STUDIO_SANDBOX_PREVIEW_GATEWAY_NAME: "" + # STUDIO_SANDBOX_PREVIEW_GATEWAY_NAMESPACE: "agent-sandbox-system" + # STUDIO_SANDBOX_PREVIEW_URL_PATTERN: "https://{handle}./" + # + # Without STUDIO_SANDBOX_SENTINEL_TOKEN the claim takes the cold-start path + # instead of the warm pool — slower per sandbox, and it borrows no capacity + # from the environment being shared. STUDIO_SANDBOX_PROVIDER: "user-desktop" DISABLE_ORGFS_MOUNTS: "true" # Never let a preview provision real gateway keys. STUDIO_PROVISION_SECRET_KEY diff --git a/deploy/helm/studio/values.yaml b/deploy/helm/studio/values.yaml index 2bec0e2444..5cc3bb7af2 100644 --- a/deploy/helm/studio/values.yaml +++ b/deploy/helm/studio/values.yaml @@ -147,6 +147,23 @@ preview: limits: memory: "768Mi" + # Hosted agent sandboxes for a preview. OFF by default, and it should stay off + # unless someone is actively testing agent behaviour. + # + # Enabling binds this release's ServiceAccount to a Role in a namespace shared + # with production sandboxes — see templates/preview-sandbox-rbac.yaml for why + # that cannot currently be narrowed, and what would remove the need. + # + # The application also needs the matching STUDIO_SANDBOX_* configuration + # (provider, runner, template name, preview gateway) pointed at an existing + # environment; enabling this alone grants permission but changes no behaviour. + sandbox: + enabled: false + # Namespace holding the SandboxTemplates, warm pools and claims. + namespace: agent-sandbox-system + # An existing Role in that namespace, owned by the sandbox-env chart. + roleName: "" + # Resources for the short-lived migration Job. jobResources: requests: From be2408184a6d80af68e64b7539ed3abb582c66c2 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Tue, 18 Aug 2026 22:01:42 -0300 Subject: [PATCH 2/3] chore(chart): 0.14.2 for the optional preview sandbox binding --- deploy/helm/studio/Chart.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/deploy/helm/studio/Chart.yaml b/deploy/helm/studio/Chart.yaml index 1fb5c299fa..bb5b01487e 100644 --- a/deploy/helm/studio/Chart.yaml +++ b/deploy/helm/studio/Chart.yaml @@ -2,6 +2,12 @@ apiVersion: v2 name: chart-deco-studio description: Helm chart for deco Studio — supports inline secrets, external secretName, and AWS Secrets Manager via ExternalSecret type: application +# 0.14.2: optional hosted sandboxes for previews (preview.sandbox.enabled, +# default false). Renders one RoleBinding when on, nothing when off. Read the +# note in templates/preview-sandbox-rbac.yaml before enabling: the namespace it +# binds into is shared with production sandboxes and RBAC has no label scoping. +# Previews now also run under a named ServiceAccount rather than `default`. +# # 0.14.1: the preview ConfigMap had no sync-wave, so it landed at 0 — after the # migrate Job at -10 that consumes it via envFrom. The Job sat in # CreateContainerConfigError and the sync stalled on the hook with nothing on @@ -26,7 +32,7 @@ type: application # when disabled, so existing releases are unaffected. # 0.12.4: chart-managed API/worker dispatch roles now use # STUDIO_DISPATCH_ROLE; legacy overrides are rejected. -version: 0.14.1 +version: 0.14.2 appVersion: "latest" dependencies: From 4cdbe21c9374a7f6439952f8a8a7522677281642 Mon Sep 17 00:00:00 2001 From: Nicacio Oliveira Date: Tue, 18 Aug 2026 22:12:52 -0300 Subject: [PATCH 3/3] fix(ci): the sandbox guard case has to disable what values-preview enables values-preview.yaml now sets serviceAccount.create=true, so the negative case rendered successfully and the assertion reported a missing guard. --- .github/workflows/helm-test.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/helm-test.yml b/.github/workflows/helm-test.yml index b62b282d95..1e4b80e5b7 100644 --- a/.github/workflows/helm-test.yml +++ b/.github/workflows/helm-test.yml @@ -499,10 +499,13 @@ jobs: # Binding sandbox access to the namespace's `default` ServiceAccount # would hand it to every pod in the preview — Postgres and MinIO # included — and the render would look perfectly healthy. + # values-preview.yaml turns serviceAccount.create on, so this case has + # to turn it back off — the point is the guard, not the default. expect_fail "sandbox without a named ServiceAccount" "requires serviceAccount.create=true" \ --set preview.host=h.example.com \ --set preview.sandbox.enabled=true \ - --set preview.sandbox.roleName=some-role + --set preview.sandbox.roleName=some-role \ + --set serviceAccount.create=false expect_fail "sandbox without a role" "preview.sandbox.roleName is required" \ --set preview.host=h.example.com \ --set preview.sandbox.enabled=true \