fix(mcp): block build --push in read-only MCP mode#3828
fix(mcp): block build --push in read-only MCP mode#3828Ankitsinghsisodya wants to merge 1 commit into
Conversation
|
[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
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. |
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 guard to prevent image pushes when the MCP server is running in read-only mode.
Changes:
- Short-circuits
buildHandlerwheninput.Pushis explicitly enabled whiles.readonlyis true - Returns a user-facing error message instructing how to enable write mode for pushing images
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| 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 { | ||
| 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 { | ||
| err = fmt.Errorf("the server is in read-only mode; set FUNC_ENABLE_MCP_WRITE=true to push images") |
lkingland
added
ok-to-test
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. Co-authored-by: Cursor <cursoragent@cursor.com>
Ankitsinghsisodya
force-pushed
the
fix/mcp-readonly-build-push
branch
from
85c2957 to
dcbf868
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3828 +/- ##
==========================================
+ Coverage 53.44% 53.46% +0.01%
==========================================
Files 200 200
Lines 23450 23454 +4
==========================================
+ Hits 12533 12539 +6
Misses 9662 9662
+ Partials 1255 1253 -2
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