🤖 fix: require authentication for the aggregated API and MCP servers - #136
Conversation
The MCP HTTP server acted with the operator's Kubernetes authority but accepted unauthenticated requests on :8090, and --app=all started it by default. - --app=all no longer starts the MCP server; it runs only with --app=mcp-http. - --app=mcp-http listens on 127.0.0.1:8090 only. - --mcp-token-file is mandatory for --app=mcp-http. A missing, unreadable, empty, short, or multi-token file fails MCP startup before any cluster access; other modes reject the flag. - Every request except exact /healthz and /readyz must send "Authorization: Bearer <token>" (SHA-256 digest, constant-time compare). This covers initialize, requests on an existing session (POST, GET stream reconnect/resume, DELETE), and unknown paths. - Remove deploy/mcp-service.yaml and the 8090 container port; document the trust boundary (the token is a shared administrative credential for the server's tool powers, not per-caller RBAC). Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ide3808b6ba40c83dce6df2379ef55ca30f7b6b7d
The aggregated API server used an anonymous authenticator and an allow-all authorizer, so any client that reached port 6443 directly was served with the control plane's Coder credentials. - Use the vendored k8s.io/apiserver DelegatingAuthenticationOptions and DelegatingAuthorizationOptions: front-proxy request-header client certificates from kube-system/extension-apiserver-authentication, cluster client certificates, TokenReview for bearer tokens, and SubjectAccessReview for every request. Lookup failures other than NotFound abort startup; delegation outages deny requests. - Refuse the anonymous identity except on exact /healthz, /livez and /readyz. The vendored options always add an anonymous fallback, so without this "no data for anonymous callers" would depend on cluster RBAC. - Resolve one Kubernetes API authority for both authentication and authorization: KUBECONFIG (exactly one valid file, never a fallback), otherwise the in-cluster ServiceAccount, otherwise ~/.kube/config. Missing configuration fails startup. - Keep the vendored system:masters bypass and 10s caches; document them, and the retained trust of the last loaded front-proxy CA. - Tests: fake Kubernetes API (TokenReview, SAR, ConfigMap watch-list) and a real envtest kube-apiserver with request-header flags and RBAC cover forged headers, unknown CAs, SAR attributes for every verb on both resources, fail-closed outages and missing bindings, with zero Coder backend calls asserted on every denial. - CI E2E: check RBAC through kube-apiserver for a non-admin identity, and that direct requests to 6443 from another pod (anonymous, forged headers, ServiceAccount token without RBAC) are rejected and 8090 is not served. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: If7a288381565727f87f07bc37d678d741d31b6d2
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex security review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73046cea3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…E delete check Addresses the PR #136 review and the E2E failure. - validateKubeconfigFile now calls clientcmd.ResolveLocalPaths, so a relative tokenFile or certificate path is read relative to the kubeconfig's directory, as the vendored options' loader does, instead of the process working directory. Regression test with a relative tokenFile (fails without the fix). - The E2E RBAC step checked delete with "delete --all --dry-run=server", which succeeds without any delete request when the namespace has no templates yet. It now deletes a named template, so kube-apiserver's authorization of the delete verb is always exercised. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `medium`_ Change-Id: I1cb37dd205dc1eed0a60d4817dafd14e19eba470
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
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
6443directly (pod IP or thecoder-k8s-apiserverService) could read template source and create, change or delete templates and workspaces. The MCP server on:8090also 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)k8s.io/apiserverdelegated authentication and authorization: kube-apiserver's front-proxy client certificates (CA fromkube-system/extension-apiserver-authentication), cluster client certificates, TokenReview for bearer tokens, and SubjectAccessReview for every request./healthz,/livezand/readyz.KUBECONFIG(exactly one valid file, no fallback), else the in-cluster ServiceAccount, else~/.kube/config.system:mastersbypass and 10 s caches, and documents them.aggregation.coder.comis 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=allno longer starts MCP.--app=mcp-httplistens on127.0.0.1:8090only.--mcp-token-fileis mandatory for--app=mcp-http; every request except exact/healthzand/readyzneedsAuthorization: Bearer <token>, including requests on an existing session. Constant-time compare.deploy/mcp-service.yamland the 8090 container port.Upgrade notes
config/rbac/auth-delegator-binding.yamlandconfig/rbac/authentication-reader-binding.yamlfor the operator ServiceAccount (both are in every documented aggregated-API install).6443directly must send a Kubernetes bearer token with RBAC onaggregation.coder.com, or go through kube-apiserver (kubectl).--app=allno longer runs MCP; delete the oldcoder-k8sMCP Service. To use MCP, run--app=mcp-http --mcp-token-file=<file>and reach it withkubectl port-forward. The token file is read once at startup.KUBECONFIG(one file) or~/.kube/config.codertemplates/coderworkspacesonly to subjects you would trust as Coder owners.The install bundle
dist/install.yamlinstalls controller mode only and is unaffected.Validation
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 withLast-Event-ID, DELETE all get 401).:8090, MCP back in--app=all, token check removed, filter accepting on error, session ID skipping the token): all caught.make verify-vendor,make build,make lint,make test, the installer freshness step, markdownlint, cspell andmake docs-checkpass;make test-integrationand actionlint passed on the same change before a docs-free rebase.6443from another pod (anonymous, forged headers, ServiceAccount token without RBAC) are rejected;8090is not served.Recording of the fixed-build run (rendered from the terminal recording; idle time capped, so shorter than wall time):
02-fixed.mp4
Out of scope, tracked separately:
deploy/apiserver-apiservice.yamlstill setsinsecureSkipTLSVerify: true(documented as development-only).Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:medium• Cost:$1742.21