fix(mcp): block build --push in read-only MCP mode#3741
fix(mcp): block build --push in read-only MCP mode#3741Ankitsinghsisodya wants to merge 1 commit into
Conversation
Added a check in the buildHandler function to return an error if an attempt is made to push images while the server is in read-only mode. This ensures that users are informed of the correct server state and can adjust their configurations accordingly.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
knative-prow
Bot
added
the
size/XS
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
knative-prow
Bot
added
the
needs-ok-to-test
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a read-only mode guard to the MCP build tool that blocks image pushes unless write mode is explicitly enabled.
Changes:
- Reject
buildinvocations withPush=truewhen the server is in read-only mode. - Surface a clear error mentioning the
FUNC_ENABLE_MCP_WRITE=truetoggle.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if s.readonly && input.Push != nil && *input.Push { | ||
| err = fmt.Errorf("the server is in read-only mode; set FUNC_ENABLE_MCP_WRITE=true to push images") |
| if s.readonly && input.Push != nil && *input.Push { | ||
| err = fmt.Errorf("the server is in read-only mode; set FUNC_ENABLE_MCP_WRITE=true to push images") |
| } | ||
|
|
||
| func (s *Server) buildHandler(ctx context.Context, r *mcp.CallToolRequest, input BuildInput) (result *mcp.CallToolResult, output BuildOutput, err error) { | ||
| if s.readonly && input.Push != nil && *input.Push { |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3741 +/- ##
=======================================
Coverage 56.95% 56.96%
=======================================
Files 181 181
Lines 21116 21120 +4
=======================================
+ Hits 12026 12030 +4
+ Misses 7866 7865 -1
- Partials 1224 1225 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
When the MCP server runs in read-only mode (default unless
FUNC_ENABLE_MCP_WRITEenables writes),deployanddeleteare already rejected.buildwithpush: truecould still push images to a registry. This change rejects that path with a clear error.Motivation
Read-only mode is meant to avoid mutating external state. Pushing a built image to a registry is such a mutation and should align with the same policy as deploy/delete.
Changes
pkg/mcp/tools_build.go: inbuildHandler, return an error whens.readonlyis true and the tool input requests push (Push != nil && *input.Push).Testing
go test ./pkg/mcp/...ormake test