Skip to content

mcp: reject relative paths in tool handlers #3694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shrey327 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

mcp: reject relative paths in tool handlers #3694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/mcp/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mcp

import (
"fmt"
"path/filepath"

"github.com/modelcontextprotocol/go-sdk/mcp"
)
Expand Down Expand Up @@ -40,3 +41,12 @@ func appendBoolFlag(args []string, flag string, value *bool) []string {
func ptr[T any](v T) *T {
return &v
}

// validatePath rejects relative paths. Since cmd.Dir is not set in the executor,
// relative paths resolve against the MCP server's CWD, not the user's project root.
func validatePath(path string) error {
if !filepath.IsAbs(path) {
return fmt.Errorf("path must be absolute, got %q", path)
}
Copy link
Copy Markdown
Member

@lkingland lkingland May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a more agent-friendly error message — the client is usually an LLM. Something like:

return fmt.Errorf("path must be absolute (the MCP server's working directory is not the user's project root), got %q", path)

The parenthetical hints at the why, which helps the agent fix its call instead of just retrying with a different relative path.

return nil
}
3 changes: 3 additions & 0 deletions pkg/mcp/tools_build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ var buildTool = &mcp.Tool{
}

func (s *Server) buildHandler(ctx context.Context, r *mcp.CallToolRequest, input BuildInput) (result *mcp.CallToolResult, output BuildOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "build", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down
3 changes: 2 additions & 1 deletion pkg/mcp/tools_build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import (
// TestTool_Build_Args ensures the build tool executes with all arguments passed correctly.
func TestTool_Build_Args(t *testing.T) {
// Test data - defined once and used for both input and validation
path := t.TempDir()
stringFlags := map[string]struct {
jsonKey string
flag string
value string
}{
"path": {"path", "--path", "."},
"path": {"path", "--path", path},
"builder": {"builder", "--builder", "pack"},
"registry": {"registry", "--registry", "ghcr.io/user"},
"builderImage": {"builderImage", "--builder-image", "custom-builder:latest"},
Expand Down
9 changes: 9 additions & 0 deletions pkg/mcp/tools_config_envs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type ConfigEnvsListOutput struct {
}

func (s *Server) configEnvsListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsListInput) (result *mcp.CallToolResult, output ConfigEnvsListOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -79,6 +82,9 @@ type ConfigEnvsAddOutput struct {
}

func (s *Server) configEnvsAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsAddInput) (result *mcp.CallToolResult, output ConfigEnvsAddOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -120,6 +126,9 @@ type ConfigEnvsRemoveOutput struct {
}

func (s *Server) configEnvsRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsRemoveInput) (result *mcp.CallToolResult, output ConfigEnvsRemoveOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down
21 changes: 12 additions & 9 deletions pkg/mcp/tools_config_envs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import (

// TestTool_ConfigEnvsAdd ensures the config_envs_add tool executes with all arguments.
func TestTool_ConfigEnvsAdd(t *testing.T) {
path := t.TempDir()
stringFlags := map[string]struct {
jsonKey string
flag string
value string
}{
"path": {"path", "--path", "."},
"path": {"path", "--path", path},
"name": {"name", "--name", "API_KEY"},
"value": {"value", "--value", "secret123"},
}
Expand Down Expand Up @@ -74,13 +75,14 @@ func TestTool_ConfigEnvsAdd(t *testing.T) {

// TestTool_ConfigEnvsList ensures the config_envs_list tool lists environment variables.
func TestTool_ConfigEnvsList(t *testing.T) {
path := t.TempDir()
executor := mock.NewExecutor()
executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) {
if subcommand != "config" {
t.Fatalf("expected subcommand 'config', got %q", subcommand)
}

// "envs" + "--path" + "." = 3 args
// "envs" + "--path" + path = 3 args
if len(args) != 3 {
t.Fatalf("expected 3 args, got %d: %v", len(args), args)
}
Expand All @@ -89,8 +91,8 @@ func TestTool_ConfigEnvsList(t *testing.T) {
}

argsMap := argsToMap(args[1:])
if val, ok := argsMap["--path"]; !ok || val != "." {
t.Fatalf("expected --path='.', got %q", val)
if val, ok := argsMap["--path"]; !ok || val != path {
t.Fatalf("expected --path=%q, got %q", path, val)
}

return []byte("DATABASE_URL=postgres://localhost\nAPI_KEY=secret\n"), nil
Expand All @@ -103,7 +105,7 @@ func TestTool_ConfigEnvsList(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_envs_list",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": path},
})
if err != nil {
t.Fatal(err)
Expand All @@ -118,12 +120,13 @@ func TestTool_ConfigEnvsList(t *testing.T) {

// TestTool_ConfigEnvsRemove ensures the config_envs_remove tool removes an environment variable.
func TestTool_ConfigEnvsRemove(t *testing.T) {
path := t.TempDir()
stringFlags := map[string]struct {
jsonKey string
flag string
value string
}{
"path": {"path", "--path", "."},
"path": {"path", "--path", path},
"name": {"name", "--name", "API_KEY"},
}

Expand Down Expand Up @@ -186,7 +189,7 @@ func TestTool_ConfigEnvsList_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_envs_list",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand All @@ -210,7 +213,7 @@ func TestTool_ConfigEnvsAdd_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_envs_add",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand All @@ -234,7 +237,7 @@ func TestTool_ConfigEnvsRemove_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_envs_remove",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand Down
9 changes: 9 additions & 0 deletions pkg/mcp/tools_config_labels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type ConfigLabelsListOutput struct {
}

func (s *Server) configLabelsListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsListInput) (result *mcp.CallToolResult, output ConfigLabelsListOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -79,6 +82,9 @@ type ConfigLabelsAddOutput struct {
}

func (s *Server) configLabelsAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsAddInput) (result *mcp.CallToolResult, output ConfigLabelsAddOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -120,6 +126,9 @@ type ConfigLabelsRemoveOutput struct {
}

func (s *Server) configLabelsRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsRemoveInput) (result *mcp.CallToolResult, output ConfigLabelsRemoveOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down
21 changes: 12 additions & 9 deletions pkg/mcp/tools_config_labels_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import (

// TestTool_ConfigLabelsAdd ensures the config_labels_add tool executes with all arguments.
func TestTool_ConfigLabelsAdd(t *testing.T) {
path := t.TempDir()
stringFlags := map[string]struct {
jsonKey string
flag string
value string
}{
"path": {"path", "--path", "."},
"path": {"path", "--path", path},
"name": {"name", "--name", "environment"},
"value": {"value", "--value", "prod"},
}
Expand Down Expand Up @@ -73,13 +74,14 @@ func TestTool_ConfigLabelsAdd(t *testing.T) {

// TestTool_ConfigLabelsList ensures the config_labels_list tool lists labels.
func TestTool_ConfigLabelsList(t *testing.T) {
path := t.TempDir()
executor := mock.NewExecutor()
executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) {
if subcommand != "config" {
t.Fatalf("expected subcommand 'config', got %q", subcommand)
}

// "labels" + "--path" + "." = 3 args
// "labels" + "--path" + path = 3 args
if len(args) != 3 {
t.Fatalf("expected 3 args, got %d: %v", len(args), args)
}
Expand All @@ -88,8 +90,8 @@ func TestTool_ConfigLabelsList(t *testing.T) {
}

argsMap := argsToMap(args[1:])
if val, ok := argsMap["--path"]; !ok || val != "." {
t.Fatalf("expected --path='.', got %q", val)
if val, ok := argsMap["--path"]; !ok || val != path {
t.Fatalf("expected --path=%q, got %q", path, val)
}

return []byte("app=my-function\nenvironment=prod\n"), nil
Expand All @@ -102,7 +104,7 @@ func TestTool_ConfigLabelsList(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_labels_list",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": path},
})
if err != nil {
t.Fatal(err)
Expand All @@ -117,12 +119,13 @@ func TestTool_ConfigLabelsList(t *testing.T) {

// TestTool_ConfigLabelsRemove ensures the config_labels_remove tool removes a label.
func TestTool_ConfigLabelsRemove(t *testing.T) {
path := t.TempDir()
stringFlags := map[string]struct {
jsonKey string
flag string
value string
}{
"path": {"path", "--path", "."},
"path": {"path", "--path", path},
"name": {"name", "--name", "environment"},
}

Expand Down Expand Up @@ -185,7 +188,7 @@ func TestTool_ConfigLabelsList_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_labels_list",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand All @@ -209,7 +212,7 @@ func TestTool_ConfigLabelsAdd_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_labels_add",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand All @@ -233,7 +236,7 @@ func TestTool_ConfigLabelsRemove_Error(t *testing.T) {

result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
Name: "config_labels_remove",
Arguments: map[string]any{"path": "."},
Arguments: map[string]any{"path": t.TempDir()},
})
if err != nil {
t.Fatal(err)
Expand Down
9 changes: 9 additions & 0 deletions pkg/mcp/tools_config_volumes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type ConfigVolumesListOutput struct {
}

func (s *Server) configVolumesListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesListInput) (result *mcp.CallToolResult, output ConfigVolumesListOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -87,6 +90,9 @@ type ConfigVolumesAddOutput struct {
}

func (s *Server) configVolumesAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesAddInput) (result *mcp.CallToolResult, output ConfigVolumesAddOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down Expand Up @@ -128,6 +134,9 @@ type ConfigVolumesRemoveOutput struct {
}

func (s *Server) configVolumesRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesRemoveInput) (result *mcp.CallToolResult, output ConfigVolumesRemoveOutput, err error) {
if err = validatePath(input.Path); err != nil {
return
}
out, err := s.executor.Execute(ctx, "config", input.Args()...)
if err != nil {
err = fmt.Errorf("%w\n%s", err, string(out))
Expand Down
Loading
Loading