Add secure Wails project MCP server - #5896
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request adds the ChangesProject MCP server
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPHost
participant wails3
participant commandsMCP
participant ProjectProcess
MCPHost->>wails3: Launch wails3 mcp
wails3->>commandsMCP: Start project MCP server
MCPHost->>commandsMCP: Send authenticated tool request
commandsMCP->>ProjectProcess: Run validated Wails command
ProjectProcess-->>commandsMCP: Return bounded output or job state
commandsMCP-->>MCPHost: Return MCP response
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
Documentation
leaanthony
marked this pull request as ready for review
August 4, 2026 12:25
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
v3/internal/commands/mcp.go (6)
550-561: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAccumulate job output in a byte slice.
j.output += string(p[:n])reallocates and copies the whole buffer on every child write. For a verbose build that fills the 2 MiB cap through many small writes, the total copying is quadratic.cappedBufferat Line 521 already uses the[]byteappend form. Reuse that type or switchoutputto[]byteand convert only insnapshot.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 550 - 561, Update mcpJob.Write to accumulate output as []byte using cappedBuffer or append semantics instead of string concatenation, while preserving the maxMCPOutput cap and io.Writer return behavior. Adjust snapshot to convert the byte slice to a string only when producing its result.
403-460: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared job-tool preamble.
build,dev,bindings, andtaskrepeat the same four steps:authorize,projectPath,validateMCPArgs,startJob. Only the leading command words differ. Extract one helper so a future change to the authorization or validation order applies to every tool.♻️ Proposed refactor
+func (s *mcpServer) startCommandJob(ctx context.Context, in mcpCommandInput, command ...string) (*mcp.CallToolResult, mcpJobOutput, error) { + if err := s.authorize(in.Token); err != nil { + return nil, mcpJobOutput{}, err + } + if err := validateMCPArgs(in.Args); err != nil { + return nil, mcpJobOutput{}, err + } + path, err := s.projectPath(in.Path) + if err != nil { + return nil, mcpJobOutput{}, err + } + return s.startJob(ctx, path, append(command, in.Args...)) +} + func (s *mcpServer) build(ctx context.Context, _ *mcp.CallToolRequest, in mcpCommandInput) (*mcp.CallToolResult, mcpJobOutput, error) { - if err := s.authorize(in.Token); err != nil { - return nil, mcpJobOutput{}, err - } - path, err := s.projectPath(in.Path) - if err != nil { - return nil, mcpJobOutput{}, err - } - if err := validateMCPArgs(in.Args); err != nil { - return nil, mcpJobOutput{}, err - } - return s.startJob(ctx, path, append([]string{"build"}, in.Args...)) + return s.startCommandJob(ctx, in, "build") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 403 - 460, Extract the repeated authorization, project-path resolution, argument validation, and job startup flow from mcpServer.build, dev, bindings, and task into a shared helper. Have each command method supply its command words and retain task-specific validation for in.Task before invoking the helper, preserving the current validation order and error behavior.
383-401: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReturn parsed JSON rather than a raw string.
The tool descriptions state that these tools return machine-readable JSON, but
resultis the raw child stdout and is embedded as a JSON string. The client receives a doubly encoded payload and must parse it again. Unmarshal the child output and return the decoded value, and fall back to the raw text when unmarshalling fails.♻️ Proposed refactor
result, err := s.runSync(ctx, s.root, "doctor", "--json") if err != nil { return nil, nil, fmt.Errorf("doctor: %w: %s", err, result) } - return nil, map[string]any{"root": s.root, "report": result}, nil + var report any + if jsonErr := json.Unmarshal([]byte(result), &report); jsonErr != nil { + report = result + } + return nil, map[string]any{"root": s.root, "report": report}, nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 383 - 401, Update doctor and taskList to unmarshal the successful runSync result from JSON and return the decoded value in the report or tasks field instead of the raw string. If unmarshalling fails, preserve the raw result as the field value; keep existing command-error handling unchanged.
84-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the transport flags before doing work.
The mutual-exclusion check runs after
resolveMCPRoot, token generation, and signal-handler installation. Move it to the top ofMCPso an invalid flag combination fails immediately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 84 - 86, The mutual-exclusion validation for HTTP and Stdio flags in the MCP function currently runs after resolveMCPRoot, token generation, and signal-handler installation. Move the check that returns the error for choosing both --http and --stdio flags to the beginning of the MCP function so invalid flag combinations fail immediately before any initialization work is performed.
37-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the stale transport comment.
The comment states that stdio is the only transport and that no network listener is opened.
runHTTPopens a loopback TCP listener, and the struct exposesHTTPandPort. Correct the comment so the documented security model matches the code.📝 Proposed comment update
-// MCPOptions configures the local project MCP server. Stdio is intentionally -// the only transport exposed by this command: a caller that can launch the -// process already owns the pipe, and no network listener is opened. +// MCPOptions configures the local project MCP server. Two transports are +// supported: stdio, where the caller that launched the process owns the pipe, +// and Streamable HTTP, which binds only to loopback and requires a bearer +// token.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 37 - 46, Update the comment block for the MCPOptions struct to accurately reflect the supported transports. The current comment states that stdio is the only transport and no network listener is opened, but the struct fields HTTP and Port along with runHTTP functionality contradict this. Revise the comment to document that both stdio and HTTP transports are supported, and clarify that when HTTP transport is selected, a loopback TCP listener is opened on the configured port.
132-139: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSecurity Misconfiguration (CWE-350)
Reachability path
● Entry v3/cmd/wails3/main.go:49 MCP │ ▼ ● Sink v3/internal/commands/mcp.goAdd
Origin/Hostvalidation and anIdleTimeoutto the loopback HTTP server.
auth.RequireBearerTokenreturns401 no bearer tokenfor requests without a validBearer ...header, and the bearer verifier rejects bad tokens before tools are called. Harden DNS rebinding for Streamable HTTP by rejecting non-loopbackOriginvalues and non-loopbackHostvalues on loopback transport, and add anIdleTimeoutso abandoned keep-alive connections do not exhaust resources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/mcp.go` around lines 132 - 139, Update the loopback HTTP server setup around NewStreamableHTTPHandler and http.Server to validate Origin and Host headers, rejecting values that are not loopback addresses before requests reach the MCP handler or bearer authentication path. Preserve the existing token verifier, and configure the server’s IdleTimeout to close abandoned keep-alive connections.docs/src/content/docs/guides/mcp-service.mdx (1)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a root-relative Starlight link for the CLI MCP section.
The target heading exists at
docs/src/content/docs/guides/cli.mdx:210, but this docs tree uses root-relative links with locale prefixes, such as/guides/build/obfuscationand/id/guides/build/obfuscation. Change./cli/#mcpto/guides/cli/#mcpso it resolves on every locale page consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/content/docs/guides/mcp-service.mdx` around lines 21 - 26, Update the CLI MCP documentation link in the project lifecycle paragraph to use the root-relative `/guides/cli/#mcp` target instead of `./cli/#mcp`, preserving the existing link text and surrounding content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/guides/cli.mdx`:
- Around line 230-243: Update the MCP flags documentation to include
WAILS_MCP_TOKEN as the supported token source, matching the existing README
documentation. Also align the documented -http and -stdio flag notation with the
runtime error wording in the MCP command, consistently using the double-dash
style throughout this section.
In `@v3/cmd/wails3/README.md`:
- Around line 45-50: Update the README’s available-tools description to replace
“bounded job status/log/stop operations” with wording that names only the
registered tools wails_job_status and wails_job_stop, noting that status returns
bounded output.
In `@v3/internal/commands/mcp_test.go`:
- Around line 21-32: In TestMCPProjectPathRejectsEscapes, resolve the root path
using filepath.EvalSymlinks before assigning it to the mcpServer initialization
so the test root matches how projectPath resolves paths in production (via
resolveMCPRoot). Additionally, add a positive assertion after the existing error
checks to confirm that projectPath accepts a legitimate nested path within the
root directory, ensuring the escape checks do not incorrectly reject valid
paths.
In `@v3/internal/commands/mcp.go`:
- Around line 331-342: Update the marker comparisons in the file-scanning logic
to use the already computed slash-normalized path value used for Files instead
of platform-specific rel. Ensure the build/config.yml and frontend/package.json
checks match correctly on Windows while preserving the existing Taskfile and
go.mod detection behavior.
- Around line 584-599: Update mcpJobs.add and related job lifecycle handling so
completed jobs are reaped when the map reaches maxMCPJobs, while preserving the
cap for active jobs. Record completion time in mcpJob.finish and expose a small
accessor; use a reapLocked helper to remove the oldest finished jobs without
reading mcpJob fields outside its lock, then admit the new job.
- Around line 474-479: Update the job stop flow around job.cancel() and
job.snapshot() to mark the job as stopping before taking the snapshot. Ensure
wails_job_stop returns the transitional stopping state immediately, while
preserving the existing cancellation and unknown-job behavior.
- Around line 283-291: The projectPath method skips symlink resolution for
non-existent paths because the EvalSymlinks call is gated behind the os.Stat
check, allowing a symlinked ancestor to bypass root confinement. Refactor the
symlink validation to find the nearest existing ancestor directory before
applying EvalSymlinks, then verify that the resolved ancestor path remains
within cleanRoot and its prefix. Apply this resolution logic regardless of
whether cleanPath itself exists, so symlinked parents of missing leaf paths are
caught before being passed to child processes or used in paths like wails3 init.
---
Nitpick comments:
In `@docs/src/content/docs/guides/mcp-service.mdx`:
- Around line 21-26: Update the CLI MCP documentation link in the project
lifecycle paragraph to use the root-relative `/guides/cli/#mcp` target instead
of `./cli/#mcp`, preserving the existing link text and surrounding content.
In `@v3/internal/commands/mcp.go`:
- Around line 550-561: Update mcpJob.Write to accumulate output as []byte using
cappedBuffer or append semantics instead of string concatenation, while
preserving the maxMCPOutput cap and io.Writer return behavior. Adjust snapshot
to convert the byte slice to a string only when producing its result.
- Around line 403-460: Extract the repeated authorization, project-path
resolution, argument validation, and job startup flow from mcpServer.build, dev,
bindings, and task into a shared helper. Have each command method supply its
command words and retain task-specific validation for in.Task before invoking
the helper, preserving the current validation order and error behavior.
- Around line 383-401: Update doctor and taskList to unmarshal the successful
runSync result from JSON and return the decoded value in the report or tasks
field instead of the raw string. If unmarshalling fails, preserve the raw result
as the field value; keep existing command-error handling unchanged.
- Around line 84-86: The mutual-exclusion validation for HTTP and Stdio flags in
the MCP function currently runs after resolveMCPRoot, token generation, and
signal-handler installation. Move the check that returns the error for choosing
both --http and --stdio flags to the beginning of the MCP function so invalid
flag combinations fail immediately before any initialization work is performed.
- Around line 37-46: Update the comment block for the MCPOptions struct to
accurately reflect the supported transports. The current comment states that
stdio is the only transport and no network listener is opened, but the struct
fields HTTP and Port along with runHTTP functionality contradict this. Revise
the comment to document that both stdio and HTTP transports are supported, and
clarify that when HTTP transport is selected, a loopback TCP listener is opened
on the configured port.
- Around line 132-139: Update the loopback HTTP server setup around
NewStreamableHTTPHandler and http.Server to validate Origin and Host headers,
rejecting values that are not loopback addresses before requests reach the MCP
handler or bearer authentication path. Preserve the existing token verifier, and
configure the server’s IdleTimeout to close abandoned keep-alive connections.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8462ff2d-8038-4209-87da-77727a7d1636
⛔ Files ignored due to path filters (1)
v3/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
docs/src/content/docs/guides/cli.mdxdocs/src/content/docs/guides/mcp-service.mdxdocs/src/content/docs/reference/cli.mdxv3/cmd/wails3/README.mdv3/cmd/wails3/main.gov3/go.modv3/internal/commands/mcp.gov3/internal/commands/mcp_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new wails3 mcp subcommand that runs a local Model Context Protocol (MCP) server for agent-assisted Wails project lifecycle management, with automatic transport selection (stdio vs loopback Streamable HTTP), job management, and documentation updates so the capability is discoverable from the CLI guides.
Changes:
- Introduces a new CLI-side MCP server with project inspection/init, doctor, task listing/execution, build/dev/bindings jobs, plus job status/stop.
- Adds a child-process mode (
WAILS_MCP_CHILD=1) to suppress CLI footer output so spawned CLI subprocesses don’t corrupt protocol/log output. - Documents the new
wails3 mcpcommand across CLI docs and adds the required Go module dependencies.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/internal/commands/mcp.go | New MCP server implementation: transport selection, root confinement checks, token auth, job orchestration, and tool registration. |
| v3/internal/commands/mcp_test.go | Adds unit tests for root resolution, token generation, token auth, and basic path escape checks. |
| v3/cmd/wails3/main.go | Registers the mcp subcommand and disables the footer for MCP child subprocesses. |
| v3/cmd/wails3/README.md | Documents the project MCP server usage, transport behavior, and security constraints. |
| docs/src/content/docs/reference/cli.mdx | Adds wails3 mcp to the CLI reference table. |
| docs/src/content/docs/guides/mcp-service.mdx | Points readers to the CLI MCP server for project lifecycle automation (separate from app MCP server). |
| docs/src/content/docs/guides/cli.mdx | Adds a dedicated mcp command section and flag documentation. |
| v3/go.mod | Adds MCP SDK dependency (and indirect deps). |
| v3/go.sum | Adds checksums for new dependencies. |
Suppressed comments (1)
v3/internal/commands/mcp.go:58
- The function doc for MCP states it starts a local stdio-only server, but the implementation can also run Streamable HTTP (auto-selected in terminal mode or via --http). Keeping this comment accurate matters because stdout/stderr handling and security assumptions differ by transport.
// MCP starts a local, stdio MCP server. The token is returned in the MCP
// initialize instructions so an agent can use it without contaminating the
// JSON-RPC stdout stream. It is also written to stderr for humans debugging a
// manually launched server.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
…ymlink, path flags) Co-authored-by: leaanthony <1943904+leaanthony@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
v3/internal/commands/mcp.go:628
validateMCPArgsblocks--output, butwails3 taskuses--outputto select the output style (not a filesystem path). This prevents legitimatewails_project_task_runusage. At the same time, the path-valued--taskfileflag is not blocked, which can bypass the intended root confinement by pointing at a Taskfile outside the allowed root.
pathFlags := map[string]bool{
"-d": true, "--dir": true,
"-o": true, "--output": true,
"--config": true,
}
v3/internal/commands/mcp.go:293
- The symlink-escape check repeats the same string prefix comparison against
realPath. For the same reasons as above (case-insensitive filesystems and general robustness), prefer afilepath.Rel-based containment check here too.
if _, err := os.Lstat(check); err == nil {
realPath, err := filepath.EvalSymlinks(check)
if err != nil || (realPath != cleanRoot && !strings.HasPrefix(realPath, cleanRoot+string(filepath.Separator))) {
return "", fmt.Errorf("path %q resolves outside the allowed MCP root", input)
}
v3/internal/commands/mcp_test.go:34
- The test name claims the token check is "constant time", but
crypto/subtle.ConstantTimeComparereturns immediately when slice lengths differ. Consider renaming the test to assert the actual property being validated (exact match required).
func TestMCPAuthorizeUsesExactConstantTimeToken(t *testing.T) {
v3/internal/commands/mcp.go:283
- Root confinement uses a string prefix check on cleaned absolute paths. On case-insensitive filesystems (notably Windows), this can incorrectly reject paths that are actually within the root when the path casing differs. Using
filepath.Relavoids case/prefix pitfalls and keeps the intent clear.
This issue also appears on line 289 of the same file.
cleanRoot := filepath.Clean(s.root)
cleanPath := filepath.Clean(p)
if cleanPath != cleanRoot && !strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) {
return "", fmt.Errorf("path %q is outside the allowed MCP root", input)
}
27 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/internal/commands/mcp.go`:
- Around line 142-153: Add a finite ReadTimeout to the http.Server configuration
used around the protectedHandler and loopback validation, ensuring request body
reads cannot continue indefinitely. Keep the existing ReadHeaderTimeout and
IdleTimeout values, and do not add a global WriteTimeout so long-lived
Streamable HTTP responses remain supported.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e3cce99d-ff36-4b34-b64a-490ad86fdb61
📒 Files selected for processing (3)
docs/src/content/docs/guides/mcp-service.mdxv3/internal/commands/mcp.gov3/internal/commands/mcp_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/content/docs/guides/mcp-service.mdx
6 tasks
overlordtm
pushed a commit
to overlordtm/wails
that referenced
this pull request
Aug 18, 2026
* Add secure Wails project MCP server * fix: address MCP review feedback (comments, init dir, job ID error, symlink, path flags) Co-authored-by: leaanthony <1943904+leaanthony@users.noreply.github.com> * fix: address MCP PR review feedback * fix: address remaining MCP review feedback * fix: bound MCP HTTP request reads --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: taliesin-ai <bot@taliesin.ai>
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
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
wails3 mcpfor agent-assisted Wails project managementValidation
go test ./...go vet ./internal/commands ./cmd/wails3git diff --checkThe documentation build was attempted but is blocked in this checkout because the available Node.js version is 20.20.2 while Astro requires Node.js >=22.12.0.
Summary by CodeRabbit
New Features
wails3 mcpcommand for agent-assisted project management.Documentation