From fbe886f3b9b21738771f2b801ed4f43ba4428ef9 Mon Sep 17 00:00:00 2001 From: "SREDevOps.org Agent" Date: Sun, 1 Nov 2026 21:38:49 -0300 Subject: [PATCH] fix(gateway): grant sandboxclaims patch and map API errors to 503 (#45) * fix(gateway): grant patch on sandboxclaims for token rotation The SSO landing path calls K8sClient.patch_token(), which patches the `dsh-` SandboxClaim's token-hash annotation whenever the browser has no valid gateway session: first visit, an expired or foreign cookie, or a gateway restart that drops the in-memory session store. The gateway Role only granted get/list/create/delete on sandboxclaims, so the patch returned 403 and, because ApiException is not handled, every such GET / answered a bare 500 "Internal Server Error". Grant the missing `patch` verb on extensions.agents.x-k8s.io/sandboxclaims, and update the least-privilege comment plus the two docs that enumerate the Role verbs. Verified with `make check` (versions, example layout, manifests) and `yamllint`; the 403 and the resulting 500 were reproduced from a running install and cleared by applying the equivalent Role patch (RBAC is evaluated per request, so no gateway restart is needed). * fix(gateway): return 503 instead of a bare 500 on Kubernetes API errors Every gateway route talks to the Kubernetes API, and ApiException used to be unhandled: Starlette's ServerErrorMiddleware turned it into a bare 500 "Internal Server Error" plus an ERROR traceback. That masked a missing RBAC verb as a client-visible crash on GET / (the SSO landing path), and it would equally hide an API outage or throttling. Register an ApiException handler that logs one credential-free line (status/reason) and answers 503 + Retry-After with a fixed body, so the Kubernetes Status payload (which can name cluster objects) is never proxied. Guard the first get_claim() in the WebSocket route the same way: WebSocket scopes bypass Starlette's HTTP exception handlers, so it now closes with 1011 instead of escaping as a traceback. Tests assert the 503 + Retry-After response, cover the SSO landing path that produced the reported 500 (forbidden claim patch), and assert the WebSocket close code. The failure-semantics tables document the new behaviour. --------- Co-authored-by: SREDevOpsAgents --- .../docs/architecture.md | 3 +- .../dsh-agents-as-a-service/gateway/README.md | 5 ++- .../gateway/gateway.py | 32 +++++++++++++++++-- .../gateway/test_gateway.py | 31 ++++++++++++++++-- .../manifests/50-gateway.yaml | 5 +-- 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/examples/dsh-agents-as-a-service/docs/architecture.md b/examples/dsh-agents-as-a-service/docs/architecture.md index a9fe76e..38a63ed 100644 --- a/examples/dsh-agents-as-a-service/docs/architecture.md +++ b/examples/dsh-agents-as-a-service/docs/architecture.md @@ -339,7 +339,7 @@ The gateway runs with a namespace-scoped ServiceAccount (`manifests/50-gateway.y | API group | Resource | Verbs | Why | | --- | --- | --- | --- | -| `extensions.agents.x-k8s.io` | `sandboxclaims` | `get`, `list`, `create`, `delete` | signup, state, account deletion | +| `extensions.agents.x-k8s.io` | `sandboxclaims` | `get`, `list`, `create`, `patch`, `delete` | signup, state, account deletion, token rotation | | `agents.x-k8s.io` | `sandboxes` | `get`, `list`, `patch` | resolve DNS/pod, wake/suspend | | `""` (core) | `pods` | `get`, `list` | resolve the pod behind a sandbox | | `""` (core) | `pods/log` | `get` | launch-token handshake | @@ -404,6 +404,7 @@ Notes and limits: | DSH log line changed or rotated away | Proxy returns `503` (missing launch token) | Fix the parser or restart the pod so the token line is re-emitted | | Probe on `/` | Pod `CrashLoopBackOff`, `401` in probe logs | Use `/favicon.svg` for readiness/liveness (shipped templates do) | | Warm pool exhausted | JSON signup returns `202` (state `Waking`); form signup is slow | Increase `replicas` in the warm pool | +| Kubernetes API error (API down or throttled, missing RBAC verb) | HTTP `503` + `Retry-After`; WebSocket close `1011` | One credential-free log line with the API `status`/`reason`, plus a fixed body; the Kubernetes `Status` payload is never proxied. Check the ServiceAccount Role and the API server — RBAC is evaluated per request, so a Role fix needs no restart | | Gateway restart | Idle sandboxes stop suspending; handshake cache and browser sessions lost | Idle clocks and handshake cache rebuild lazily; sessions do not, so browsers must sign in again after a rollout. Use a shared session store for production | | Gateway configured `COOKIE_SECURE=false` without `ALLOW_INSECURE_COOKIES=1` | Gateway pod exits at startup / `CrashLoopBackOff` | Intentional guard against an insecure cookie on an HTTPS deployment; set `ALLOW_INSECURE_COOKIES=1` only for the local plain-HTTP demo, or set `COOKIE_SECURE=true` | | Auth rate limit hit | `429` + `Retry-After` on signup/login | Per-client in-memory bucket (`AUTH_RATE_LIMIT`/`AUTH_RATE_WINDOW`); wait for the window or tune it. Behind a proxy, keying uses the rightmost `X-Forwarded-For` entry | diff --git a/examples/dsh-agents-as-a-service/gateway/README.md b/examples/dsh-agents-as-a-service/gateway/README.md index 6dfb84d..9bd773b 100644 --- a/examples/dsh-agents-as-a-service/gateway/README.md +++ b/examples/dsh-agents-as-a-service/gateway/README.md @@ -175,9 +175,11 @@ oauth2-proxy's egress to the gateway, DNS and public HTTPS. | Sandbox not provisioned, wake timeout, handshake failure | `503` + `Retry-After: 10`. The handshake error body is a fixed message: it never contains the launch token, the handshake URL or pod/credential details. | | User over `MAX_IN_FLIGHT_PER_USER` | `503` + `Retry-After` (HTTP); the existing in-flight requests are untouched | | Upstream connection/protocol failure | `502` | +| Kubernetes API error (API unreachable/throttled, RBAC `403`, `5xx`) | `503` + `Retry-After: 10`. The body is a fixed message; the Kubernetes `Status` payload (which can name cluster objects) is never proxied, and the API `status`/`reason` is logged once | | WS auth failure (unknown user, bad token, cross-origin) | accept, then close `4401` | | WS per-user concurrency cap | accept, then close `1011` | | WS upstream/handshake failure | accept, then close `1011` | +| WS Kubernetes API error | accept, then close `1011` | | WS message from DSH over `WS_MAX_MESSAGE_BYTES` | the upstream/relay closes the connection (websockets' message-too-big path) | A close code can only be delivered after the WebSocket upgrade is accepted; @@ -230,7 +232,8 @@ would never be sent). HTTPS deployments must set `COOKIE_SECURE=true`. The gateway's ServiceAccount needs, in its namespace: -* `sandboxclaims` (`extensions.agents.x-k8s.io`): get, list, create, delete +* `sandboxclaims` (`extensions.agents.x-k8s.io`): get, list, create, patch, delete + (patch rotates the token-hash annotation when a session is minted) * `sandboxes` (`agents.x-k8s.io`): get, list, patch * `pods`: get, list * `pods/log`: get diff --git a/examples/dsh-agents-as-a-service/gateway/gateway.py b/examples/dsh-agents-as-a-service/gateway/gateway.py index 30fbfb1..9335a98 100644 --- a/examples/dsh-agents-as-a-service/gateway/gateway.py +++ b/examples/dsh-agents-as-a-service/gateway/gateway.py @@ -1510,7 +1510,13 @@ against the JSON API. The browser session is an HttpOnly cookie.

user = websocket.path_params["user"] if not valid_user(user): return await self._ws_reject(websocket, 4401) - claim = await self.k8s.get_claim(user) + try: + claim = await self.k8s.get_claim(user) + except ApiException: + # Starlette has no exception handler for WebSocket scopes, so an + # unhandled API error would escape as a traceback and an unclean + # close. 1011 keeps the HTTP 503 meaning on the WS channel. + return await self._ws_reject(websocket, 1011) if claim is None: return await self._ws_reject(websocket, 4401) token, cookie_user = self.credentials(websocket.headers, websocket.cookies) @@ -1746,6 +1752,28 @@ async def client_gone(request: Request, exc: ClientDisconnect) -> Response: return Response(status_code=CLIENT_CLOSED_REQUEST) +async def k8s_api_error(request: Request, exc: ApiException) -> Response: + """Answer a Kubernetes API failure with 503 instead of a bare 500. + + The gateway is the only path between the browser and the cluster, so an + API outage, a throttle or a missing RBAC verb is a server-side + availability problem, not a client error. A 503 keeps ingress and + browser retry semantics honest, and the body is a fixed message: the + Kubernetes ``Status`` payload can name cluster objects and is never + proxied. The API ``status``/``reason`` is logged once, credential-free. + """ + print( + "gateway: kubernetes API error: " + f"status={getattr(exc, 'status', None)} reason={getattr(exc, 'reason', None)}", + flush=True, + ) + return JSONResponse( + {"error": "cluster API unavailable, retry shortly"}, + status_code=503, + headers={"Retry-After": "10"}, + ) + + def create_app( settings: Settings | None = None, k8s=None, @@ -1778,7 +1806,7 @@ def create_app( ] app = Starlette( routes=routes, - exception_handlers={ClientDisconnect: client_gone}, + exception_handlers={ClientDisconnect: client_gone, ApiException: k8s_api_error}, lifespan=_lifespan(gateway, http_client if owns_http_client else None), ) app.state.gateway = gateway diff --git a/examples/dsh-agents-as-a-service/gateway/test_gateway.py b/examples/dsh-agents-as-a-service/gateway/test_gateway.py index 9bddb65..f7ec72c 100644 --- a/examples/dsh-agents-as-a-service/gateway/test_gateway.py +++ b/examples/dsh-agents-as-a-service/gateway/test_gateway.py @@ -1176,12 +1176,14 @@ async def test_delete_missing_claim_is_404(env): @pytest.mark.asyncio -async def test_unexpected_api_error_propagates(env): +async def test_unexpected_api_error_returns_503(env): add_user(env.k8s) env.k8s.get_claim_error = ApiException(status=500) async with asgi_client(env) as client: - with pytest.raises(ApiException): - await client.get("/users/alice") + response = await client.get("/users/alice") + assert response.status_code == 503 + assert response.headers["retry-after"] == "10" + assert response.json() == {"error": "cluster API unavailable, retry shortly"} @pytest.mark.asyncio @@ -1831,6 +1833,16 @@ def test_ws_unauthenticated_closes_4401(env): assert exc.value.code == 4401 +def test_ws_kubernetes_api_error_closes_1011(env): + add_user(env.k8s) + env.k8s.get_claim_error = ApiException(status=503) + with TestClient(env.app) as client: + with client.websocket_connect("/u/alice/api/remote.mux") as ws: + with pytest.raises(WebSocketDisconnect) as exc: + ws.receive_text() + assert exc.value.code == 1011 + + def test_ws_cookie_cross_origin_closes_4401(env): add_user(env.k8s) with TestClient(env.app) as client: @@ -2149,6 +2161,19 @@ def test_remote_user_returns_valid_and_rejects_invalid(env): assert g.remote_user({}) is None +@pytest.mark.asyncio +async def test_sso_landing_returns_503_when_claim_patch_is_forbidden(env): + """A missing RBAC verb on ``patch sandboxclaims`` used to escape as a 500.""" + enable_sso(env) + env.k8s.claims["alice"] = {"metadata": {"name": claim_name("alice")}} + env.k8s.patch_error = ApiException(status=403) + async with asgi_client(env) as client: + response = await client.get("/", headers={"X-Forwarded-User": "alice"}) + assert response.status_code == 503 + assert response.headers["retry-after"] == "10" + assert response.json() == {"error": "cluster API unavailable, retry shortly"} + + @pytest.mark.asyncio async def test_sso_landing_new_user_shows_provisioning(env): enable_sso(env) diff --git a/examples/dsh-agents-as-a-service/manifests/50-gateway.yaml b/examples/dsh-agents-as-a-service/manifests/50-gateway.yaml index 024e52e..943a1fa 100644 --- a/examples/dsh-agents-as-a-service/manifests/50-gateway.yaml +++ b/examples/dsh-agents-as-a-service/manifests/50-gateway.yaml @@ -26,7 +26,8 @@ metadata: namespace: dsh-demo --- # Least privilege: -# - claims are created/deleted per user (signup/delete); +# - claims are created/deleted per user (signup/delete) and patched only to +# rotate the token-hash annotation when a session is minted; # - sandboxes are read, and patched only to flip operatingMode (suspend/resume); # - pods are read, and pods/log is read to capture DSH's launch token from # the startup log line (`dsh web: ...?token=...`). @@ -39,7 +40,7 @@ metadata: rules: - apiGroups: ["extensions.agents.x-k8s.io"] resources: ["sandboxclaims"] - verbs: ["get", "list", "create", "delete"] + verbs: ["get", "list", "create", "patch", "delete"] - apiGroups: ["agents.x-k8s.io"] resources: ["sandboxes"] verbs: ["get", "list", "patch"]