Repository navigation
Commit 78a2eca
authored
🤖 fix: require authentication for the aggregated API and MCP servers (#136)
## Summary
Before this change, the aggregated API server and the MCP HTTP server
accepted requests without authentication, and both acted with the
operator's authority. The aggregated API server used an anonymous
authenticator and an allow-all authorizer, then called Coder with the
control plane's owner token. So any client that could reach port `6443`
directly (pod IP or the `coder-k8s-apiserver` Service) could read
template source and create, change or delete templates and workspaces.
The MCP server on `:8090` also had no authentication, and `--app=all`
(the default) started both servers. Requests through kube-apiserver and
the APIService were already RBAC-checked; direct traffic was not.
A security advisory will be published with the release that contains
this fix.
## What changed
**Aggregated API server** (`fix(apiserver): require delegated
authentication and authorization`)
- Uses the vendored `k8s.io/apiserver` delegated authentication and
authorization: kube-apiserver's front-proxy client certificates (CA from
`kube-system/extension-apiserver-authentication`), cluster client
certificates, TokenReview for bearer tokens, and SubjectAccessReview for
every request.
- Refuses the anonymous identity everywhere except exact `/healthz`,
`/livez` and `/readyz`.
- Fails closed: without read access to the auth ConfigMap it does not
start; if SubjectAccessReview is unavailable, requests are denied.
- One Kubernetes authority for both checks: `KUBECONFIG` (exactly one
valid file, no fallback), else the in-cluster ServiceAccount, else
`~/.kube/config`.
- Keeps the vendored `system:masters` bypass and 10 s caches, and
documents them.
- Docs: RBAC on `aggregation.coder.com` is owner-equivalent inside Coder
(the server still calls Coder with the operator's token for the
namespace's control plane).
**MCP server** (`fix(mcp): require a bearer token and stop serving MCP
by default`)
- `--app=all` no longer starts MCP. `--app=mcp-http` listens on
`127.0.0.1:8090` only.
- `--mcp-token-file` is mandatory for `--app=mcp-http`; every request
except exact `/healthz` and `/readyz` needs `Authorization: Bearer
<token>`, including requests on an existing session. Constant-time
compare.
- Removes `deploy/mcp-service.yaml` and the 8090 container port.
## Upgrade notes
1. Apply `config/rbac/auth-delegator-binding.yaml` and
`config/rbac/authentication-reader-binding.yaml` for the operator
ServiceAccount (both are in every documented aggregated-API install).
2. Clients that called port `6443` directly must send a Kubernetes
bearer token with RBAC on `aggregation.coder.com`, or go through
kube-apiserver (`kubectl`).
3. `--app=all` no longer runs MCP; delete the old `coder-k8s` MCP
Service. To use MCP, run `--app=mcp-http --mcp-token-file=<file>` and
reach it with `kubectl port-forward`. The token file is read once at
startup.
4. Outside a cluster, the aggregated server needs `KUBECONFIG` (one
file) or `~/.kube/config`.
5. Grant RBAC on `codertemplates`/`coderworkspaces` only to subjects you
would trust as Coder owners.
The install bundle `dist/install.yaml` installs controller mode only and
is unaffected.
## Validation
- Unit tests with a fake Kubernetes API and envtest with a real
kube-apiserver (request-header flags, RBAC): forged `X-Remote-*` headers
alone and with a valid unprivileged token, unknown CAs, a disallowed
front-proxy CN, SubjectAccessReview attributes for every verb on both
resources and namespace boundaries, delegation outages,
missing/empty/malformed/forbidden ConfigMap, startup without each
binding. Every denial asserts zero Coder backend calls. MCP tests reuse
a live session without the token (POST, GET with `Last-Event-ID`, DELETE
all get 401).
- 11 mutation checks (restore anonymous, restore allow-all, remove the
anonymous filter, prefix path matching, tolerate lookup failure,
optional kubeconfig, MCP back on `:8090`, MCP back in `--app=all`, token
check removed, filter accepting on error, session ID skipping the
token): all caught.
- Gates on this head: `make verify-vendor`, `make build`, `make lint`,
`make test`, the installer freshness step, markdownlint, cspell and
`make docs-check` pass; `make test-integration` and actionlint passed on
the same change before a docs-free rebase.
- New CI E2E step: RBAC through kube-apiserver for a non-admin identity;
direct requests to `6443` from another pod (anonymous, forged headers,
ServiceAccount token without RBAC) are rejected; `8090` is not served.
- Live KIND run with the fixed image:



Recording of the fixed-build run (rendered from the terminal recording;
idle time capped, so shorter than wall time):
https://github.com/user-attachments/assets/48e11bf0-5eaf-4932-a893-5dc472f75580
Out of scope, tracked separately: `deploy/apiserver-apiservice.yaml`
still sets `insecureSkipTLSVerify: true` (documented as
development-only).
---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-opus-5-5` • Thinking: `medium` • Cost: `$1742.21`_1 parent 8fc3453 commit 78a2eca
28 files changed
Lines changed: 2324 additions & 136 deletions
File tree
- .github/workflows
- config/default
- deploy
- docs
- explanation
- how-to
- examples/cloudnativepg
- internal/app
- allapp
- apiserverapp
- mcpapp
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
56 | 60 | | |
57 | 61 | | |
58 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
372 | 372 | | |
373 | 373 | | |
374 | 374 | | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
375 | 419 | | |
376 | 420 | | |
377 | 421 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
76 | | - | |
| 76 | + | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
| |||
62 | 63 | | |
63 | 64 | | |
64 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
65 | 72 | | |
66 | 73 | | |
67 | 74 | | |
| |||
83 | 90 | | |
84 | 91 | | |
85 | 92 | | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
86 | 97 | | |
87 | 98 | | |
88 | 99 | | |
| |||
97 | 108 | | |
98 | 109 | | |
99 | 110 | | |
100 | | - | |
| 111 | + | |
101 | 112 | | |
102 | 113 | | |
103 | 114 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
18 | | - | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
26 | | - | |
27 | 25 | | |
28 | 26 | | |
29 | 27 | | |
| |||
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
22 | 21 | | |
23 | 22 | | |
24 | 23 | | |
25 | | - | |
26 | 24 | | |
27 | 25 | | |
28 | 26 | | |
29 | 27 | | |
30 | 28 | | |
31 | 29 | | |
32 | 30 | | |
33 | | - | |
| 31 | + | |
34 | 32 | | |
35 | 33 | | |
36 | 34 | | |
| |||
41 | 39 | | |
42 | 40 | | |
43 | 41 | | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
44 | 46 | | |
45 | 47 | | |
46 | 48 | | |
| |||
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
67 | | - | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
15 | 17 | | |
16 | 18 | | |
17 | 19 | | |
| |||
67 | 69 | | |
68 | 70 | | |
69 | 71 | | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
70 | 100 | | |
71 | 101 | | |
72 | 102 | | |
| |||
0 commit comments