From f45998b1b36d3edc44fc8dc377c59e4f1522e68c Mon Sep 17 00:00:00 2001 From: Shrey327 Date: Sun, 10 May 2026 19:45:53 +0530 Subject: [PATCH 1/2] feat: enforce absolute path validation in MCP tool handlers and updated tests Signed-off-by: Shrey327 --- pkg/mcp/tools.go | 10 ++++++++++ pkg/mcp/tools_build.go | 3 +++ pkg/mcp/tools_build_test.go | 3 ++- pkg/mcp/tools_config_envs.go | 9 +++++++++ pkg/mcp/tools_config_envs_test.go | 21 ++++++++++++--------- pkg/mcp/tools_config_labels.go | 9 +++++++++ pkg/mcp/tools_config_labels_test.go | 21 ++++++++++++--------- pkg/mcp/tools_config_volumes.go | 9 +++++++++ pkg/mcp/tools_config_volumes_test.go | 21 ++++++++++++--------- pkg/mcp/tools_create.go | 3 +++ pkg/mcp/tools_create_test.go | 8 +++----- pkg/mcp/tools_delete.go | 5 +++++ pkg/mcp/tools_deploy.go | 3 +++ pkg/mcp/tools_deploy_test.go | 3 ++- pkg/mcp/tools_test.go | 13 +++++++++++++ 15 files changed, 107 insertions(+), 34 deletions(-) diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index 0d7ed0fc7d..427ea0b816 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -2,6 +2,7 @@ package mcp import ( "fmt" + "path/filepath" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -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) + } + return nil +} diff --git a/pkg/mcp/tools_build.go b/pkg/mcp/tools_build.go index 52576187aa..0bc20cbf14 100644 --- a/pkg/mcp/tools_build.go +++ b/pkg/mcp/tools_build.go @@ -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)) diff --git a/pkg/mcp/tools_build_test.go b/pkg/mcp/tools_build_test.go index 310fce45e0..c1ff6d4fd9 100644 --- a/pkg/mcp/tools_build_test.go +++ b/pkg/mcp/tools_build_test.go @@ -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"}, diff --git a/pkg/mcp/tools_config_envs.go b/pkg/mcp/tools_config_envs.go index a2d1fceefa..a6ab39d2c6 100644 --- a/pkg/mcp/tools_config_envs.go +++ b/pkg/mcp/tools_config_envs.go @@ -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)) @@ -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)) @@ -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)) diff --git a/pkg/mcp/tools_config_envs_test.go b/pkg/mcp/tools_config_envs_test.go index 16440f701e..194288a72f 100644 --- a/pkg/mcp/tools_config_envs_test.go +++ b/pkg/mcp/tools_config_envs_test.go @@ -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"}, } @@ -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) } @@ -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 @@ -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) @@ -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"}, } @@ -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) @@ -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) @@ -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) diff --git a/pkg/mcp/tools_config_labels.go b/pkg/mcp/tools_config_labels.go index 0c62e87290..579793103c 100644 --- a/pkg/mcp/tools_config_labels.go +++ b/pkg/mcp/tools_config_labels.go @@ -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)) @@ -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)) @@ -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)) diff --git a/pkg/mcp/tools_config_labels_test.go b/pkg/mcp/tools_config_labels_test.go index e3d2a42c83..4584272fae 100644 --- a/pkg/mcp/tools_config_labels_test.go +++ b/pkg/mcp/tools_config_labels_test.go @@ -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"}, } @@ -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) } @@ -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 @@ -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) @@ -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"}, } @@ -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) @@ -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) @@ -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) diff --git a/pkg/mcp/tools_config_volumes.go b/pkg/mcp/tools_config_volumes.go index 232d866d30..009ecd7d58 100644 --- a/pkg/mcp/tools_config_volumes.go +++ b/pkg/mcp/tools_config_volumes.go @@ -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)) @@ -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)) @@ -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)) diff --git a/pkg/mcp/tools_config_volumes_test.go b/pkg/mcp/tools_config_volumes_test.go index bc10965554..43d12d1320 100644 --- a/pkg/mcp/tools_config_volumes_test.go +++ b/pkg/mcp/tools_config_volumes_test.go @@ -11,12 +11,13 @@ import ( // TestTool_ConfigVolumesAdd ensures the config_volumes_add tool executes with all arguments. func TestTool_ConfigVolumesAdd(t *testing.T) { + path := t.TempDir() stringFlags := map[string]struct { jsonKey string flag string value string }{ - "path": {"path", "--path", "."}, + "path": {"path", "--path", path}, "type": {"type", "--type", "secret"}, "mountPath": {"mountPath", "--mount-path", "/workspace/secret"}, "source": {"source", "--source", "my-secret"}, @@ -77,13 +78,14 @@ func TestTool_ConfigVolumesAdd(t *testing.T) { // TestTool_ConfigVolumesList ensures the config_volumes_list tool lists volumes. func TestTool_ConfigVolumesList(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) } - // "volumes" + "--path" + "." = 3 args + // "volumes" + "--path" + path = 3 args if len(args) != 3 { t.Fatalf("expected 3 args, got %d: %v", len(args), args) } @@ -92,8 +94,8 @@ func TestTool_ConfigVolumesList(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("secret:my-secret:/workspace/secret\n"), nil @@ -106,7 +108,7 @@ func TestTool_ConfigVolumesList(t *testing.T) { result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": path}, }) if err != nil { t.Fatal(err) @@ -121,12 +123,13 @@ func TestTool_ConfigVolumesList(t *testing.T) { // TestTool_ConfigVolumesRemove ensures the config_volumes_remove tool removes a volume. func TestTool_ConfigVolumesRemove(t *testing.T) { + path := t.TempDir() stringFlags := map[string]struct { jsonKey string flag string value string }{ - "path": {"path", "--path", "."}, + "path": {"path", "--path", path}, "mountPath": {"mountPath", "--mount-path", "/workspace/secret"}, } @@ -189,7 +192,7 @@ func TestTool_ConfigVolumesList_Error(t *testing.T) { result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": t.TempDir()}, }) if err != nil { t.Fatal(err) @@ -213,7 +216,7 @@ func TestTool_ConfigVolumesAdd_Error(t *testing.T) { result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_add", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": t.TempDir()}, }) if err != nil { t.Fatal(err) @@ -237,7 +240,7 @@ func TestTool_ConfigVolumesRemove_Error(t *testing.T) { result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_remove", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": t.TempDir()}, }) if err != nil { t.Fatal(err) diff --git a/pkg/mcp/tools_create.go b/pkg/mcp/tools_create.go index 28e4b7a03e..982562143b 100644 --- a/pkg/mcp/tools_create.go +++ b/pkg/mcp/tools_create.go @@ -20,6 +20,9 @@ var createTool = &mcp.Tool{ } func (s *Server) createHandler(ctx context.Context, r *mcp.CallToolRequest, input CreateInput) (result *mcp.CallToolResult, output CreateOutput, err error) { + if err = validatePath(input.Path); err != nil { + return + } out, err := s.executor.Execute(ctx, "create", input.Args()...) if err != nil { err = fmt.Errorf("%w\n%s", err, string(out)) diff --git a/pkg/mcp/tools_create_test.go b/pkg/mcp/tools_create_test.go index 737102b8a3..d511e80144 100644 --- a/pkg/mcp/tools_create_test.go +++ b/pkg/mcp/tools_create_test.go @@ -14,12 +14,13 @@ import ( func TestTool_Create_Args(t *testing.T) { // Test data - defined once and used for both input and validation // Note: language (-l) is required and handled separately + path := t.TempDir() stringFlags := map[string]struct { jsonKey string flag string value string }{ - "path": {"path", "--path", "."}, + "path": {"path", "--path", path}, "template": {"template", "--template", "cloudevents"}, "repository": {"repository", "--repository", "https://example.com/repo"}, } @@ -82,9 +83,6 @@ func TestTool_Create_Args(t *testing.T) { } } -// TestCreate_PathValidation is removed - path validation no longer exists -// Create now operates in current working directory - // TestCreate_BinaryFailure ensures errors from the func binary are returned as MCP errors func TestTool_Create_BinaryFailure(t *testing.T) { executor := mock.NewExecutor() @@ -103,7 +101,7 @@ func TestTool_Create_BinaryFailure(t *testing.T) { Name: "create", Arguments: map[string]any{ "language": "go", - "path": ".", + "path": t.TempDir(), }, }) if err != nil { diff --git a/pkg/mcp/tools_delete.go b/pkg/mcp/tools_delete.go index a44df3076f..31088481b8 100644 --- a/pkg/mcp/tools_delete.go +++ b/pkg/mcp/tools_delete.go @@ -30,6 +30,11 @@ func (s *Server) deleteHandler(ctx context.Context, r *mcp.CallToolRequest, inpu err = fmt.Errorf("exactly one of 'path' or 'name' must be provided") return } + if input.Path != nil { + if err = validatePath(*input.Path); err != nil { + return + } + } // Execute out, err := s.executor.Execute(ctx, "delete", input.Args()...) diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index 518052130f..4f3dcbeb45 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -24,6 +24,9 @@ func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, inpu err = fmt.Errorf("the server is currently in readonly mode. Please set FUNC_ENABLE_MCP_WRITE and restart the client") return } + if err = validatePath(input.Path); err != nil { + return + } out, err := s.executor.Execute(ctx, "deploy", input.Args()...) if err != nil { err = fmt.Errorf("%w\n%s", err, string(out)) diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index 4b8d2e83c4..dce826b171 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -11,12 +11,13 @@ import ( // TestTool_Deploy_Args ensures the deploy tool executes with all arguments passed correctly. func TestTool_Deploy_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"}, "image": {"image", "--image", "ghcr.io/user/my-func:latest"}, diff --git a/pkg/mcp/tools_test.go b/pkg/mcp/tools_test.go index a4931ecdbf..109e6c9964 100644 --- a/pkg/mcp/tools_test.go +++ b/pkg/mcp/tools_test.go @@ -62,6 +62,19 @@ func validateBoolFlags(t *testing.T, args []string, boolFlags map[string]string) } } +// TestValidatePath ensures validatePath accepts absolute paths and rejects relative ones. +func TestValidatePath(t *testing.T) { + if err := validatePath("/absolute/path"); err != nil { + t.Fatalf("expected no error for absolute path, got: %v", err) + } + if err := validatePath("relative/path"); err == nil { + t.Fatal("expected error for relative path, got nil") + } + if err := validatePath("."); err == nil { + t.Fatal("expected error for '.', got nil") + } +} + // buildInputArgs constructs the input arguments map for CallTool from test data. func buildInputArgs(stringFlags map[string]struct { jsonKey string From 766fda21ac26f399d7d75dc3de826640d1af545c Mon Sep 17 00:00:00 2001 From: Shrey327 Date: Mon, 11 May 2026 18:38:21 +0530 Subject: [PATCH 2/2] fix: added handler-level tests and updated the error message Signed-off-by: Shrey327 --- pkg/mcp/tools.go | 2 +- pkg/mcp/tools_test.go | 45 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index 427ea0b816..f03a6a1ec7 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -46,7 +46,7 @@ func ptr[T any](v T) *T { // 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) + return fmt.Errorf("path must be absolute (the MCP server's working directory is not the user's project root), got %q", path) } return nil } diff --git a/pkg/mcp/tools_test.go b/pkg/mcp/tools_test.go index 109e6c9964..81ffe787ed 100644 --- a/pkg/mcp/tools_test.go +++ b/pkg/mcp/tools_test.go @@ -3,6 +3,8 @@ package mcp import ( "strings" "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" ) // validateArgLength validates that the args slice has the expected length based on @@ -62,9 +64,50 @@ func validateBoolFlags(t *testing.T, args []string, boolFlags map[string]string) } } +// TestTool_RejectsRelativePath verifies that every tool handler wired to validatePath +// returns an error result when given a relative path, not just the helper itself. +func TestTool_RejectsRelativePath(t *testing.T) { + cases := []struct { + tool string + args map[string]any + }{ + {"build", map[string]any{"path": "."}}, + {"create", map[string]any{"path": ".", "language": "go"}}, + {"deploy", map[string]any{"path": "."}}, + {"delete", map[string]any{"path": "."}}, + {"config_envs_list", map[string]any{"path": "."}}, + {"config_envs_add", map[string]any{"path": "."}}, + {"config_envs_remove", map[string]any{"path": "."}}, + {"config_labels_list", map[string]any{"path": "."}}, + {"config_labels_add", map[string]any{"path": "."}}, + {"config_labels_remove", map[string]any{"path": "."}}, + {"config_volumes_list", map[string]any{"path": "."}}, + {"config_volumes_add", map[string]any{"path": "."}}, + {"config_volumes_remove", map[string]any{"path": "."}}, + } + for _, tc := range cases { + t.Run(tc.tool, func(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: tc.tool, + Arguments: tc.args, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatalf("tool %q: expected error result for relative path, got success", tc.tool) + } + }) + } +} + // TestValidatePath ensures validatePath accepts absolute paths and rejects relative ones. func TestValidatePath(t *testing.T) { - if err := validatePath("/absolute/path"); err != nil { + if err := validatePath(t.TempDir()); err != nil { t.Fatalf("expected no error for absolute path, got: %v", err) } if err := validatePath("relative/path"); err == nil {