feat: server-side stored scripts for code execution - #990
Merged
Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
5e4f309
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://415a0dce.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://097-stored-scripts.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 31835259705 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
force-pushed
the
096-batched-call-tools
branch
from
August 14, 2026 18:28
882a79a to
51ee14b
Compare
Dumbris
force-pushed
the
097-stored-scripts
branch
from
August 14, 2026 18:28
4888486 to
904f7fd
Compare
…or-driven MCP discovery, active-config-file authority, REST surface pinned Related #986
…scovery bound, listing status model Related #986
…lidated read result Related #986
…ckstart 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.
…OLLOW, explicit config-path authority, handler-only resolution, LimitReader bound, CLI language Changed tracking Related #986
…erywhere symlink tests stated uniformly Related #986
…ries + polish Related #986
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
…tive 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.
…n, 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.
Dumbris
force-pushed
the
097-stored-scripts
branch
from
August 14, 2026 19:52
904f7fd to
5e4f309
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements spec 097 (
specs/097-stored-scripts/): the daemon stores named scripts and callers invoke them by reference —script: "<name>"on the code_execution tool and REST,--scripton the CLI. A 19KB workflow costs a name per run instead of ~4.8k tokens.Related #986
What it does
MCP clients call code_execution with
{"script": "fetch-prs", "input": {...}}. Editing by atomic replace (write + rename) takes effect on the next invocation — no restart. No write/upload API anywhere in v1: the filesystem is the only authoring surface.Design (reviewed plan is binding —
specs/097-stored-scripts/plan.md)internal/codescriptsis the single owner of script semantics. The confinement boundary is the pure name validator ([A-Za-z0-9_-]{1,64}— no separators, no dots, rejected before any filesystem access); on top sits the symlink/non-regular policy: atomic on Unix (O_NOFOLLOW|O_NONBLOCK— a symlink or FIFO inscripts/is rejected, never followed or blocked on), checked best-effort on Windows. 256KB bound viaLimitReader(max+1); one validated read per invocation; directory entries matched by exact byte comparison so case-insensitive filesystems cannot execute a script the listings don't show.--data-dir; empty authority = no scripts, never CWD).mcpproxy code scripts list/GET /api/v1/code/scriptsgive the full listing withok|ambiguous|invalidstatuses.enable_code_execution=falsenow refuses code_execution on REST and direct dispatch too (pre-existing gap for inline code — closed here because stored scripts widened it); the typed refusal maps to HTTP 403, and script-resolution failures map to 4xx instead of 500.Review
Spec: 2 full review rounds (13 findings resolved). Plan: 2 rounds (12 findings — including the discovery that
os.Rootfollows in-root symlinks and silently defeats callerO_NOFOLLOW, which reshaped the confinement design). Implementation: 15-agent adversarial review (3 lenses × verify-per-finding) + scoped opencode review → 9 distinct confirmed findings, all fixed (feature gate, FIFO hang, case-aliasing, CWD leak on empty authority, REST error mapping, CLI fallback notice, 3 test-hardening items). Reviewer: opencode gpt-5.6-sol (Codex quota-locked).Testing
go test -race ./internal/...green; server-edition green; lint v2 (CI config) 0 issues; e2e 65/65; both edition builds + GOOS=linux/windows cross-builds.