Skip to content

Commit d242d38

Browse files
jerm-droclaude
andcommitted
Add per-script execution timeout and fix chain order comment
Add a default 30s per-script execution timeout that bounds total wall-clock time including all tool calls. Configurable via scriptEngine.scriptTimeout in the vMCP config. The timeout is applied via context.WithTimeout in executor.Execute, so tool Call closures that respect context cancellation will abort when the deadline is exceeded. Fix the middleware chain order comment in server.go to accurately reflect that script middleware runs before authz in the execution path: auth establishes identity, but authz does not see the execute_tool_script call itself. Inner tool calls from scripts do flow through authz. Part of #4742 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a490ef1 commit d242d38

8 files changed

Lines changed: 62 additions & 7 deletions

File tree

cmd/vmcp/app/commands.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,8 @@ func createScriptMiddleware(cfg *config.Config) func(http.Handler) http.Handler
669669

670670
slog.Info("code mode enabled — execute_tool_script virtual tool will be available")
671671
return script.NewMiddleware(&script.Config{
672-
StepLimit: cfg.ScriptEngine.StepLimit,
673-
ParallelMax: cfg.ScriptEngine.ParallelMax,
672+
StepLimit: cfg.ScriptEngine.StepLimit,
673+
ParallelMax: cfg.ScriptEngine.ParallelMax,
674+
ScriptTimeout: time.Duration(cfg.ScriptEngine.ScriptTimeout),
674675
})
675676
}

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,6 +1666,15 @@ spec:
16661666
ParallelMax is the maximum number of concurrent worker goroutines that
16671667
parallel() can use. Zero means the worker count matches the task count.
16681668
type: integer
1669+
scriptTimeout:
1670+
default: 30s
1671+
description: |-
1672+
ScriptTimeout is the maximum wall-clock duration for a single script
1673+
execution, including all tool calls. Bounds total execution time so a
1674+
script with many slow tool calls cannot block indefinitely.
1675+
Defaults to 30s if not specified or zero.
1676+
pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
1677+
type: string
16691678
stepLimit:
16701679
description: |-
16711680
StepLimit is the maximum number of Starlark execution steps per script.

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,6 +1669,15 @@ spec:
16691669
ParallelMax is the maximum number of concurrent worker goroutines that
16701670
parallel() can use. Zero means the worker count matches the task count.
16711671
type: integer
1672+
scriptTimeout:
1673+
default: 30s
1674+
description: |-
1675+
ScriptTimeout is the maximum wall-clock duration for a single script
1676+
execution, including all tool calls. Bounds total execution time so a
1677+
script with many slow tool calls cannot block indefinitely.
1678+
Defaults to 30s if not specified or zero.
1679+
pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
1680+
type: string
16721681
stepLimit:
16731682
description: |-
16741683
StepLimit is the maximum number of Starlark execution steps per script.

docs/operator/crd-api.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/script/executor.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ type executor struct {
2525

2626
// Execute runs a Starlark script against the bound tools.
2727
func (e *executor) Execute(ctx context.Context, script string, data map[string]interface{}) (*mcp.CallToolResult, error) {
28+
// Apply per-script execution timeout to bound total wall-clock time
29+
// including all tool calls.
30+
ctx, cancel := context.WithTimeout(ctx, e.config.ScriptTimeout)
31+
defer cancel()
32+
2833
globals := e.buildGlobals(ctx)
2934

3035
// Inject data arguments, rejecting any that shadow builtins or tools

pkg/script/script.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package script
88

99
import (
1010
"context"
11+
"time"
1112

1213
"github.com/mark3labs/mcp-go/mcp"
1314
)
@@ -55,6 +56,10 @@ type Tool struct {
5556
Call func(ctx context.Context, arguments map[string]interface{}) (*mcp.CallToolResult, error)
5657
}
5758

59+
// DefaultScriptTimeout is the default maximum wall-clock duration for a
60+
// single script execution, including all tool calls.
61+
const DefaultScriptTimeout = 30 * time.Second
62+
5863
// Config holds script execution parameters. A nil Config passed to New
5964
// uses sensible defaults for all fields.
6065
type Config struct {
@@ -66,6 +71,12 @@ type Config struct {
6671
// ParallelMax is the maximum number of concurrent goroutines that
6772
// parallel() can spawn. Zero means unlimited.
6873
ParallelMax int
74+
75+
// ScriptTimeout is the maximum wall-clock duration for a single script
76+
// execution, including all tool calls. Bounds total execution time so
77+
// a script with many slow tool calls cannot block indefinitely.
78+
// Zero uses DefaultScriptTimeout (30s).
79+
ScriptTimeout time.Duration
6980
}
7081

7182
// New creates an Executor bound to the given tools and configuration.
@@ -80,7 +91,10 @@ func New(tools []Tool, cfg *Config) Executor {
8091

8192
func resolveConfig(cfg *Config) Config {
8293
if cfg == nil {
83-
return Config{StepLimit: DefaultStepLimit}
94+
return Config{
95+
StepLimit: DefaultStepLimit,
96+
ScriptTimeout: DefaultScriptTimeout,
97+
}
8498
}
8599
c := *cfg
86100
if c.StepLimit == 0 {
@@ -89,5 +103,8 @@ func resolveConfig(cfg *Config) Config {
89103
if c.ParallelMax < 0 {
90104
c.ParallelMax = 0
91105
}
106+
if c.ScriptTimeout == 0 {
107+
c.ScriptTimeout = DefaultScriptTimeout
108+
}
92109
return c
93110
}

pkg/vmcp/config/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,14 @@ type ScriptEngineConfig struct {
958958
// parallel() can use. Zero means the worker count matches the task count.
959959
// +optional
960960
ParallelMax int `json:"parallelMax,omitempty" yaml:"parallelMax,omitempty"`
961+
962+
// ScriptTimeout is the maximum wall-clock duration for a single script
963+
// execution, including all tool calls. Bounds total execution time so a
964+
// script with many slow tool calls cannot block indefinitely.
965+
// Defaults to 30s if not specified or zero.
966+
// +kubebuilder:default="30s"
967+
// +optional
968+
ScriptTimeout Duration `json:"scriptTimeout,omitempty" yaml:"scriptTimeout,omitempty"`
961969
}
962970

963971
// Validator validates configuration.

pkg/vmcp/server/server.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -559,11 +559,16 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) {
559559

560560
// MCP endpoint - apply middleware chain (wrapping order, execution happens in reverse):
561561
// Code wraps: auth+parser → audit → discovery → annotation-enrichment →
562-
// authz → script(+parser) → backend-enrichment → MCP-parsing → telemetry
562+
// script(+parser) → authz → backend-enrichment → MCP-parsing → telemetry
563563
// Execution order: recovery → header-val → auth+parser → audit →
564-
// discovery → annotation-enrichment → authz → parser → script →
565-
// backend-enrichment → MCP-parsing → telemetry → handler
566-
// Note: script middleware composes its own parser internally.
564+
// discovery → annotation-enrichment → parser → script →
565+
// authz → backend-enrichment → MCP-parsing → telemetry → handler
566+
//
567+
// Script middleware sits between annotation-enrichment and authz:
568+
// - Auth (identity) runs before script — requests are authenticated
569+
// - Authz runs AFTER script — execute_tool_script itself is not authorized,
570+
// but inner tool calls dispatched from scripts flow through authz
571+
// - Script middleware composes its own parser internally
567572

568573
var mcpHandler http.Handler = streamableServer
569574

0 commit comments

Comments
 (0)