Repository navigation
Grant pods/attach only for stdio transport - #6841
Merged
JAORMX merged 2 commits intoOct 16, 2026
Merged
Conversation
The proxy-runner Role created for every MCPServer carried pods/attach (create, get) regardless of transport. Only stdio uses it: the runner reaches a stdio MCP server by attaching to its pod through Client.AttachToWorkload, which is called only from pkg/transport/stdio.go. A streamable-http or sse server talks to its pod over the network and never attaches. Security auditors flag pods/attach as equivalent to pod exec, so clusters with strict policies saw a finding on every MCPServer deployment for a permission that could not be used. Split the rule out of defaultRBACRules into stdioRBACRules and choose between them in rbacRulesForTransport. An absent transport is treated as stdio, matching the CRD default, so an object stored before that default existed keeps working. Already-deployed servers are corrected without operator action: EnsureRBACResources replaces Rules wholesale, so the next reconcile rewrites the Role. Closes stacklok#6230 Signed-off-by: hellouz818 <hellouz818@gmail.com>
hellouz818
requested review from
ChrisJBurns,
JAORMX,
blkt,
jerm-dro,
jhrozek,
rdimitrov,
reyortiz3 and
tgrunnagle
as code owners
October 16, 2026 05:42
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6841 +/- ##
==========================================
- Coverage 79.69% 79.67% -0.02%
==========================================
Files 817 817
Lines 83421 83425 +4
==========================================
- Hits 66480 66467 -13
- Misses 16936 16953 +17
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The chainsaw operator E2E suites asserted one shared proxyrunner-role.yaml for the stdio, sse, streamable-http and stdio-streamable-http scenarios. That stopped holding once pods/attach became transport-dependent: chainsaw matches the rules array positionally and on length, so the stdio scenarios failed on the rule having moved to the end, and the HTTP ones would fail on it being absent. Replace the shared file with proxyrunner-role-stdio.yaml (six rules, pods/attach last) and proxyrunner-role-http.yaml (the five shared rules), and point each scenario at the one matching the transport in its mcpserver.yaml. Both files still spell out the complete expected Role, so an over-granted rule fails the assertion rather than slipping through. Only the multi-tenancy run reached CI: chainsaw runs with FailFast and both suites share one shell step, so the single-tenancy scenarios never executed. Refs stacklok#6230 Signed-off-by: hellouz818 <hellouz818@gmail.com>
JAORMX
approved these changes
Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The operator currently grants
pods/attach(create,get) to every MCPServer proxy-runner Role. Only stdio transport needs this permission;streamable-httpandssecommunicate with the backend over the network. Unnecessary attach permissions can trigger security audit findings, as reported with Polaris.This change separates the attach rule from the shared RBAC rules and includes it only for stdio transport. An empty transport is treated as stdio, consistent with the CRD default.
Closes #6230
Type of change
Test plan
task test)task test-e2e)task lint)Also ran
task docs-verify,task operator-manifests, andtask operator-generate; all completed without generated artifact changes.New coverage:
TestEnsureRBACResources_Role_PodsAttachScopedToStdiochecks Role creation forstdio,streamable-http,sse, and an empty transport, verifying the expected attach rule and preservation of shared rules.TestRBACRulesForTransport_ReturnsFreshSlicechecks that replacing an element in a returned slice does not affect subsequent calls.Restoring unconditional attach permissions causes both new tests to fail, while the pre-existing RBAC tests still pass.
The new tests cover Role creation and slice isolation. They do not directly test transport changes on an existing Role.
API Compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.No CRD schema changes; this changes reconcile-time RBAC construction only.
Does this introduce a user-facing change?
Yes, for operators.
Before: Every MCPServer proxy-runner Role grants
pods/attach(create,get).After: The rule is included only when
spec.transportisstdioor empty. Shared permissions remain unchanged for all transports.Existing Roles are updated on the next reconcile: the RBAC helper uses
CreateOrUpdateto replace the Role's rules with the desired set. HTTP/SSE servers therefore lose the attach permission without requiring a workload restart. Changing the transport to stdio restores it on reconciliation.Special notes for reviewers
+kubebuilder:rbacmarker forpods/attachremains unchanged because the operator still needs permission to grant it to stdio servers.stdiodefault.slices.Clonegives each call its own outer slice, so replacing or appending rules does not modify the shared rule slice. NestedPolicyRulefields remain shared and are treated as immutable.