Repository navigation
Commit bfd43e7
authored
feat: server-side stored scripts for code execution (#990)
* spec: 097-stored-scripts — server-side stored scripts for code execution
Related #986
* spec: 097 review pass — open-time confinement, fixed 256KB bound, error-driven MCP discovery, active-config-file authority, REST surface pinned
Related #986
* spec: 097 — pin one-read semantics vs atomic replacement, 20+count discovery bound, listing status model
Related #986
* spec: 097 — freshness guarantees scoped to atomic replacement; one validated read result
Related #986
* plan: 097-stored-scripts — research, plan, data model, contracts, quickstart
Related #986
Key decisions from code research: os.Root confinement with Lstat regular-file
policy (os.Root follows in-root symlinks; caller O_NOFOLLOW is silently
defeated); new leaf package internal/codescripts as single owner of script
semantics; scripts-dir authority per surface incl. the mainServer-nil and
loadCodeConfig path traps; three registration sites (incl. disabled stub);
GetConfigPath already on ServerController so the REST listing needs no
interface/mock churn; discovery is error-driven with static registrations.
* plan: 097 review pass — validation-as-boundary with atomic Unix O_NOFOLLOW, explicit config-path authority, handler-only resolution, LimitReader bound, CLI language Changed tracking
Related #986
* plan: 097 consistency pass — handler-only resolution and attempted-everywhere symlink tests stated uniformly
Related #986
* tasks: 097-stored-scripts — 14 tasks across foundational + 4 user stories + polish
Related #986
* feat: server-side stored scripts for code execution
Related #986
code_execution (MCP + REST) accepts script: "<name>" as an alternative to
inline code, resolving from <active-config-dir>/scripts/. The new
internal/codescripts package is the single owner of script semantics: pure
token name validation (the confinement boundary — invalid names are
rejected before any filesystem access), platform no-follow open (Unix
O_NOFOLLOW atomic; Windows Lstat best-effort), 256KB bound via
LimitReader(max+1), one validated read per invocation, listing with
ok/ambiguous/invalid statuses, and extension-derived language with
explicit-contradiction rejection.
## Changes
- internal/codescripts: new package + traversal-corpus/symlink/freshness
tests (symlink cases attempted on every platform, skipped only on
privilege failure)
- internal/server: script XOR code handler seam (handler is the only
execution-time resolver on every surface), explicit config-file-path
authority via WithConfigFilePath at construction, shared
codeExecRecordArguments (history/activity carry resolved source as code
plus additive script name), registrations on all three sites incl. the
disabled stub, error-driven discovery (not-found lists <=20 names +
total) documented in the tool description
- internal/httpapi: Script field with XOR 400 INVALID_REQUEST; new
GET /api/v1/code/scripts listing endpoint (auth inherited)
- cmd/mcpproxy + internal/cliclient: --script flag (3-way exclusion),
language sent only when the flag was explicitly set, name-over-the-wire
in both daemon and standalone modes, mcpproxy code scripts list
- oas: swagger regen; docs: stored-scripts sections sourced from the
implementation's actual strings
## Testing
- TDD throughout (red observed first per task)
- go test -race ./internal/... green; server-edition green; lint v2 0
issues; e2e 65/65; both edition builds + GOOS=linux/windows cross-builds
* fix(097): empty scripts-dir authority never falls through to CWD-relative resolution
Related #986
Review finding: with no explicit path, no mainServer path, and a nil
config, filepath.Join("", name+ext) produced a bare relative path
resolved against the process working directory. An empty authority now
means no scripts, reported as the standard not-found error.
* fix(097): review round — feature gate on every surface, FIFO-safe open, case-exact resolution, typed 4xx REST mapping, test hardening
Related #986
- enable_code_execution=false now refuses code_execution on EVERY surface
(REST /code/exec, /tools/call, CallToolDirect) via a gate at the shared
handler chokepoint, with one message everywhere; the typed refusal
travels through a context capture box (Spec-093 shed pattern) so REST
answers 403 FEATURE_DISABLED without string matching. Pre-existing gap
for inline code; closed here because stored scripts widened it.
- Unix open gains O_NONBLOCK so a FIFO dropped into scripts/ fails as
non-regular instead of blocking the handler until a writer appears.
- Resolve now matches directory entries by exact byte comparison
(os.ReadDir) before opening, so case-insensitive filesystems can no
longer execute a script (backdoor.JS) that every listing and discovery
surface reports as nonexistent.
- Stored-script resolution failures surface as 4xx over REST (404
not-found carrying the discovery list, 400 invalid/ambiguous) instead
of 500 EXECUTION_FAILED.
- code scripts list prints a stderr notice when it falls back to a local
listing because the daemon is unreachable.
- Tests: traversal corpus extended with existing-target escape cases,
stored-vs-inline parity deepened (stub upstream, options, scope), -o
json listing branch covered, FIFO regression test.1 parent d3ad9c2 commit bfd43e7
42 files changed
Lines changed: 5084 additions & 109 deletions
File tree
- cmd/mcpproxy
- docs
- code_execution
- features
- internal
- cliclient
- codescripts
- config
- httpapi
- server
- oas
- specs/097-stored-scripts
- checklists
- contracts
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
158 | 158 | | |
159 | 159 | | |
160 | 160 | | |
| 161 | + | |
161 | 162 | | |
162 | 163 | | |
163 | | - | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
798 | 798 | | |
799 | 799 | | |
800 | 800 | | |
| 801 | + | |
0 commit comments