Skip to content

Commit a3d8896

Browse files
johnstcnjohnstcn
committed
remove exec command filtering
1 parent a6bd4c5 commit a3d8896

File tree

8 files changed

+19
-209
lines changed

8 files changed

+19
-209
lines changed

‎cli/exp_mcp.go

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,14 @@ import (
1414

1515
func (r *RootCmd) mcpCommand() *serpent.Command {
1616
var (
17-
client = new(codersdk.Client)
18-
instructions string
19-
allowedTools []string
20-
allowedExecCommands []string
17+
client = new(codersdk.Client)
18+
instructions string
19+
allowedTools []string
2120
)
2221
return &serpent.Command{
2322
Use: "mcp",
2423
Handler: func(inv *serpent.Invocation) error {
25-
return mcpHandler(inv, client, instructions, allowedTools, allowedExecCommands)
24+
return mcpHandler(inv, client, instructions, allowedTools)
2625
},
2726
Short: "Start an MCP server that can be used to interact with a Coder depoyment.",
2827
Middleware: serpent.Chain(
@@ -41,17 +40,11 @@ func (r *RootCmd) mcpCommand() *serpent.Command {
4140
Flag: "allowed-tools",
4241
Value: serpent.StringArrayOf(&allowedTools),
4342
},
44-
{
45-
Name: "allowed-exec-commands",
46-
Description: "Comma-separated list of allowed commands for workspace execution. If not specified, all commands are allowed.",
47-
Flag: "allowed-exec-commands",
48-
Value: serpent.StringArrayOf(&allowedExecCommands),
49-
},
5043
},
5144
}
5245
}
5346

54-
func mcpHandler(inv *serpent.Invocation, client *codersdk.Client, instructions string, allowedTools []string, allowedExecCommands []string) error {
47+
func mcpHandler(inv *serpent.Invocation, client *codersdk.Client, instructions string, allowedTools []string) error {
5548
ctx, cancel := context.WithCancel(inv.Context())
5649
defer cancel()
5750

@@ -71,9 +64,6 @@ func mcpHandler(inv *serpent.Invocation, client *codersdk.Client, instructions s
7164
if len(allowedTools) > 0 {
7265
cliui.Infof(inv.Stderr, "Allowed Tools : %v", allowedTools)
7366
}
74-
if len(allowedExecCommands) > 0 {
75-
cliui.Infof(inv.Stderr, "Allowed Exec Commands : %v", allowedExecCommands)
76-
}
7767
cliui.Infof(inv.Stderr, "Press Ctrl+C to stop the server")
7868

7969
// Capture the original stdin, stdout, and stderr.
@@ -98,11 +88,6 @@ func mcpHandler(inv *serpent.Invocation, client *codersdk.Client, instructions s
9888
options = append(options, codermcp.WithAllowedTools(allowedTools))
9989
}
10090

101-
// Add allowed exec commands option if specified
102-
if len(allowedExecCommands) > 0 {
103-
options = append(options, codermcp.WithAllowedExecCommands(allowedExecCommands))
104-
}
105-
10691
closer := codermcp.New(ctx, client, options...)
10792

10893
<-ctx.Done()

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ require (
320320
github.com/google/nftables v0.2.0 // indirect
321321
github.com/google/pprof v0.0.0-20230817174616-7a8ec2ada47b // indirect
322322
github.com/google/s2a-go v0.1.9 // indirect
323-
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
323+
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
324324
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
325325
github.com/googleapis/gax-go/v2 v2.14.1 // indirect
326326
github.com/gorilla/css v1.0.1 // indirect

‎mcp/mcp.go

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,11 @@ import (
1717
)
1818

1919
type mcpOptions struct {
20-
in io.Reader
21-
out io.Writer
22-
instructions string
23-
logger *slog.Logger
24-
allowedTools []string
25-
allowedExecCommands []string
20+
in io.Reader
21+
out io.Writer
22+
instructions string
23+
logger *slog.Logger
24+
allowedTools []string
2625
}
2726

2827
// Option is a function that configures the MCP server.
@@ -63,13 +62,6 @@ func WithAllowedTools(tools []string) Option {
6362
}
6463
}
6564

66-
// WithAllowedExecCommands sets the allowed commands for workspace execution.
67-
func WithAllowedExecCommands(commands []string) Option {
68-
return func(o *mcpOptions) {
69-
o.allowedExecCommands = commands
70-
}
71-
}
72-
7365
// New creates a new MCP server with the given client and options.
7466
func New(ctx context.Context, client *codersdk.Client, opts ...Option) io.Closer {
7567
options := &mcpOptions{
@@ -96,9 +88,8 @@ func New(ctx context.Context, client *codersdk.Client, opts ...Option) io.Closer
9688
reg = reg.WithOnlyAllowed(options.allowedTools...)
9789
}
9890
reg.Register(mcpSrv, mcptools.ToolDeps{
99-
Client: client,
100-
Logger: &logger,
101-
AllowedExecCommands: options.allowedExecCommands,
91+
Client: client,
92+
Logger: &logger,
10293
})
10394

10495
srv := server.NewStdioServer(mcpSrv)

‎mcp/tools/command_validator.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎mcp/tools/command_validator_test.go

Lines changed: 0 additions & 82 deletions
This file was deleted.

‎mcp/tools/tools_coder.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -194,15 +194,6 @@ func handleCoderWorkspaceExec(deps ToolDeps) mcpserver.ToolHandlerFunc {
194194
return nil, xerrors.New("command is required")
195195
}
196196

197-
// Validate the command if allowed commands are specified
198-
allowed, err := IsCommandAllowed(command, deps.AllowedExecCommands)
199-
if err != nil {
200-
return nil, err
201-
}
202-
if !allowed {
203-
return nil, xerrors.Errorf("command not allowed: %s", command)
204-
}
205-
206197
// Attempt to fetch the workspace. We may get a UUID or a name, so try to
207198
// handle both.
208199
ws, err := getWorkspaceByIDOrOwnerName(ctx, deps.Client, wsArg)

‎mcp/tools/tools_coder_test.go

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -217,24 +217,22 @@ func TestCoderTools(t *testing.T) {
217217
})
218218

219219
// NOTE: this test runs after the list_workspaces tool is called.
220-
t.Run("tool_and_command_restrictions", func(t *testing.T) {
220+
t.Run("tool_restrictions", func(t *testing.T) {
221221
// Given: the workspace agent is connected
222222
<-agentStarted
223223

224224
// Given: a restricted MCP server with only allowed tools and commands
225225
restrictedPty := ptytest.New(t)
226226
allowedTools := []string{"coder_workspace_exec"}
227-
allowedCommands := []string{"echo", "ls"}
228227
restrictedMCPSrv, closeRestrictedSrv := startTestMCPServer(ctx, t, restrictedPty.Input(), restrictedPty.Output())
229228
t.Cleanup(func() {
230229
_ = closeRestrictedSrv()
231230
})
232231
mcptools.AllTools().
233232
WithOnlyAllowed(allowedTools...).
234233
Register(restrictedMCPSrv, mcptools.ToolDeps{
235-
Client: memberClient,
236-
Logger: &logger,
237-
AllowedExecCommands: allowedCommands,
234+
Client: memberClient,
235+
Logger: &logger,
238236
})
239237

240238
// When: the tools/list command is called
@@ -256,31 +254,6 @@ func TestCoderTools(t *testing.T) {
256254
disallowedToolResponse := restrictedPty.ReadLine(ctx)
257255
require.Contains(t, disallowedToolResponse, "error")
258256
require.Contains(t, disallowedToolResponse, "not found")
259-
260-
// When: an allowed exec command is called
261-
randString := testutil.GetRandomName(t)
262-
allowedCmd := makeJSONRPCRequest(t, "tools/call", "coder_workspace_exec", map[string]any{
263-
"workspace": r.Workspace.ID.String(),
264-
"command": "echo " + randString,
265-
})
266-
267-
// Then: the response is the output of the command.
268-
restrictedPty.WriteLine(allowedCmd)
269-
_ = restrictedPty.ReadLine(ctx) // skip the echo
270-
actual := restrictedPty.ReadLine(ctx)
271-
require.Contains(t, actual, randString)
272-
273-
// When: a disallowed exec command is called
274-
disallowedCmd := makeJSONRPCRequest(t, "tools/call", "coder_workspace_exec", map[string]any{
275-
"workspace": r.Workspace.ID.String(),
276-
"command": "evil --hax",
277-
})
278-
279-
// Then: the response is an error indicating the command is not allowed.
280-
restrictedPty.WriteLine(disallowedCmd)
281-
_ = restrictedPty.ReadLine(ctx) // skip the echo
282-
errorResponse := restrictedPty.ReadLine(ctx)
283-
require.Contains(t, errorResponse, `command \"evil\" is not allowed`)
284257
})
285258

286259
t.Run("coder_start_workspace", func(t *testing.T) {

‎mcp/tools/tools_registry.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,7 @@ Examples:
150150
- "cat ~/.bashrc" - View a file's contents
151151
- "python -m pip list" - List installed Python packages
152152
153-
Note: Commands are subject to security restrictions and validation.
154-
Very long-running commands may time out.`), mcp.Required()),
153+
Note: Very long-running commands may time out.`), mcp.Required()),
155154
),
156155
MakeHandler: handleCoderWorkspaceExec,
157156
},
@@ -215,9 +214,8 @@ var _ ToolAdder = (*server.MCPServer)(nil)
215214

216215
// ToolDeps contains all dependencies needed by tool handlers
217216
type ToolDeps struct {
218-
Client *codersdk.Client
219-
Logger *slog.Logger
220-
AllowedExecCommands []string
217+
Client *codersdk.Client
218+
Logger *slog.Logger
221219
}
222220

223221
// ToolHandler associates a tool with its handler creation function

0 commit comments

Comments
 (0)