Path-jailed live custom-node dev tools
Status: implemented (this PR)Safety-gates note: The original draft depended on a general safety-gates framework (spec PR #172). That framework was rejected as won’t-do — see issue #168’s closing rationale: comfyui-mcp’s deployment reality is single-user (own panel, own box/pod), so permissive-by-default is correct and we are not building a gate system on spec (ROADMAP Theme G records the thinking should a shared-deployment need ever materialize). The path jail is not a gate feature — it is an inherent boundary of these tools and stands on its own. Beyond it:node_pack(action: "write") /node_pack(action: "patch") are UNGATED (like the other mutating tools in this repo today), andnode_pack (action:"git") commit/pushare guarded by a single narrow inline env flag,COMFYUI_MCP_ALLOW_GIT_WRITES("1"/"true"; default OFF). When off, commit/push return anisErrorresult with the structuredDISABLED_BY_CONFIGbody ({ "error": "DISABLED_BY_CONFIG", "disabled_by_config": true, "required_flag": "COMFYUI_MCP_ALLOW_GIT_WRITES=1", "message": … });status/diff/logare always allowed. The flag exists because push has off-machine effects, not as a placeholder for a gate system. Wherever the text below says “gatednode-writes”/“gatedgit-writes”, read it as: node-writes → ungated; git-writes → theCOMFYUI_MCP_ALLOW_GIT_WRITESflag.
Prior art: filliptm/ComfyUI_FL-MCPbackend/coding_tools.py— file read/search/write/patch/git tools hard-jailed tocustom_nodes/with bounded output. We port the shape and the output-bounding constants, and add Windows symlink/junction safety, and reuse of our existing containment code.
Motivation
The bisect/fix flow (bisect_* in src/tools/node-bisect.ts, install_custom_node (action: "fix") in src/tools/node-management.ts:148) can isolate a broken pack, and node_pack (action: "scaffold")/node_pack (action: "verify")/node_pack (action: "publish") (src/services/node-authoring.ts, node-verify.ts) cover create/verify/ship — but there is no way for an agent to actually read, search, edit, and commit pack source in between. Users currently need a second coding agent with unrestricted filesystem access. This closes the loop: bisect → search/read → patch → node_pack (action: "verify") → restart_comfyui → git commit → node_pack (action: "publish").
Tool API
Servicesrc/services/node-dev.ts, category custom-nodes, LOCAL-only (no config.comfyuiPath in remote/cloud mode → the clear “requires a local ComfyUI install” refusal already used at node-management.ts:804-810). Naming is snake_case verb-noun like the rest of the surface; note list_packs is taken (installer packs, skills-access.ts:273) and install_custom_node (action: "list") already lists packs, so there is no new list-packs tool.
-
node_pack(action: "list_files") — read-only, ungated. Params:pack: string(folder undercustom_nodes),glob?: string,max_entries?: number(default 500, max 2000). Result:{ pack, root, entries: [{ path, size, dir }], truncated, is_git_repo, has_pyproject }. Skips.git/,__pycache__/,node_modules/. -
node_pack(action: "read") — read-only, ungated. Params:path: string(pack-relative, e.g.MyPack/nodes.py),start_line?: number(default 1),line_count?: number(default 240, max 800),max_chars?: number(default 12000, max 24000). Result:{ path, content, start_line, end_line, total_lines, size, truncated }with a truncation notice appended when clipped (“request a narrower line range…”). Long lines chunked at 1000 chars. (Bounds are FL-MCP’s proven constants.) -
node_pack(action: "search") — read-only, ungated. Params:query: string(regex),path?: string(default.= all packs),glob?: string,max_results?: number(default 50, max 100),case_sensitive?: boolean. Result:{ engine: "ripgrep" | "builtin", matches: [{ file, line, text }], truncated }; match lines capped at 600 chars. Ripgrep is not a current dependency (verified). Strategy: probergon PATH once (spawnSync("rg", ["--version"])); if absent, fall back to a bounded pure-JS scanner (recursive walk, skip dot dirs/__pycache__, skip files > 1 MiB or containing NUL bytes,RegExpper line, hard cap on scanned files). Optional follow-up:@vscode/ripgrepinoptionalDependenciesviarequireOptionalDep(src/utils/optional-dep.ts), consistent with how cloud SDKs are handled. -
node_pack(action: "write") — gatednode-writes. Params:path: string,content: string,overwrite?: boolean(default false; refuse existing file without it),create_dirs?: boolean(default true). Result:{ path, bytes, created: boolean }. -
node_pack(action: "patch") — gatednode-writes. Params:patch: string(unified diff;a/–b/prefixes accepted). Behavior (FL-MCP’s two-phase apply): parse+++/---headers, jail-check every touched path before any git call, thengit apply --checkfollowed bygit apply, run from the pack directory;--unsafe-pathsnever allowed. Works on non-repo packs too (git applyfunctions outside a repository for plain file patching). Result:{ success, stage: "check" | "apply", touched: [...], stdout, stderr }(output bounded at 12000 chars). -
node_pack(action: "git") — one tool, action enum (keeps the catalog small; compact-router agents discover subactions viadescribe_tool). Params:pack: string,action: "status" | "diff" | "log" | "commit" | "push",message?: string(required for commit),paths?: string[](jail-checked, staged selectively; default all pack changes),max_chars?: number. Gating:status/diff/logungated (reads);commit/pushgatedgit-writes(default closed per the gates RFC) via an in-handlerisGateOpen/gateRefusalcheck — the one sanctioned exception, since registration-time wrapping can’t see actions. Refusal is the standardDISABLED_BY_CONFIGshape withrequired_flag: "COMFYUI_MCP_ALLOW_GIT_WRITES=1". Execution:execFileSync("git", [...], { cwd: packDir })withnonInteractiveGitEnv()reused (exported) fromsrc/services/node-management.ts(prevents credential prompts hanging the server), timeouts (60 s; 180 s for push), bounded output,--end-of-optionswherever args are user-derived (same discipline asnode-management.ts:836-838).
class NodeDevError extends ComfyUIError (code NODE_DEV_ERROR) through the standard errorToToolResult path.
Path-jail mechanism
src/services/node-dev.ts exports one auditable function used by every tool:
- Root =
resolve(config.comfyuiPath, "custom_nodes")(identical tonode-management.ts:821). - Candidate = absolute input taken as-is, else
resolve(root, input). - Lexical containment:
relative(root, candidate)must be non-empty, not start with.., notisAbsolute(existing pattern atnode-management.ts:823-828, mirroringmanifest.ts’sisWithinRoot). - Symlink safety: realpath the deepest existing ancestor of the candidate (
fs.realpathSync.native) and realpath the root, then re-run containment on the realpaths. Defeats a symlinked pack dir — and Windows junctions/dir-symlinks, whichrealpathSync.nativeresolves. Not-yet-existing files (node_pack(action: "write")) are checked via their existing parent. - Windows extras: reject NTFS alternate data streams (
:in any segment past the drive letter), reserved device names (CON,NUL,COM1…), and trailing dots/spaces in segments; comparisons viarelative()(platform-correct case sensitivity). - Write/patch/git never operate on the root itself (
rel === ""rejected; list/search may use.).
pack params additionally pass the existing assertSafeRepoName-style validation (exported from node-management.ts).
Integration with existing flows
- Tool descriptions cross-reference the closed loop (scaffold → write/patch → verify → restart → commit → publish);
install_custom_node(action: "fix") and the bisect tools mention the diagnose-then-patch follow-up in their descriptions (descriptions are the agent UX). - Compact router: the
["custom-nodes", registerNodePackTools]entry inTOOL_GROUPSis captured bycollectToolCatalogautomatically, andsearchCorpus(compact.ts:35-40) indexes the param descriptions, solist_tools search:"patch"finds it. TheCOMFYUI_MCP_ALLOW_GIT_WRITEScheck lives in the handler, so it applies identically through the compact router.
Implementation plan
src/services/node-dev.ts— jail resolver, bounded-text helpers (boundText,chunkLongLines; FL constants: read 12k/24k chars, 240/800 lines, search lines 600 chars, command output 12k), git runner, builtin search fallback, ripgrep probe. Inject fs/exec seams likeAuthoringDeps(node-authoring.ts:104-120) so tests need no real disk/subprocess.src/tools/node-pack.ts— thenode_packregistration with a flat zod schema; handlerstry/catch errorToToolResult.src/tools/index.ts—["custom-nodes", registerNodePackTools](registration order is observable).— no gates framework in this PR (deferred to ROADMAP Theme G). Instead,src/tools/gates.tsnode_pack (action:"git") commit/pushdo an in-handlergitWritesEnabled()(COMFYUI_MCP_ALLOW_GIT_WRITES) check returning theDISABLED_BY_CONFIGrefusal;node_pack(action: "write")/node_pack(action: "patch") ship ungated.- Export
nonInteractiveGitEnv(andassertSafeRepoName) fromsrc/services/node-management.ts. - Docs page + README section; call out
git-writesdefault-closed prominently.
Test plan (vitest, src/__tests__/node-dev.test.ts)
- Jail: table-driven —
../x,..\x, absolute outside root, drive-relative (C:x), UNC, ADS (nodes.py:zone), reserved names,./..pack names, and a junction-escape fixture (fs.symlinkSync(target, link, "junction")in a tmpdir — junctions need no admin on Windows; skip POSIX-symlink variant on unprivileged win32 CI). - read/list bounding: line-range math, char-truncation notice, long-line chunking,
total_lineson CRLF files. - search: builtin fallback correctness + caps; ripgrep path via exec seam; binary/large-file skip.
- patch: temp git-repo fixture — clean apply; check-stage failure surfaces stderr; patch touching a path outside
custom_nodesrefused before any git call. - git: seam-mocked exec asserting exact argv (no option injection); commit requires message; commit/push return the
DISABLED_BY_CONFIGrefusal whenCOMFYUI_MCP_ALLOW_GIT_WRITESis unset. - Integration (opt-in,
COMFYUI_INTEGRATION=true): scaffold → write → verify → git status roundtrip in a temp workspace.
Rollout / compat
- Six new tools appended; no existing tool changes.
- Superseded by 0.50.0 slice 12. The six tools this document specifies shipped as six names and were then folded, with
scaffold/verify/publish, into the single action-parameterizednode_packtool (src/tools/node-pack.ts,registerNodePackTools);src/tools/node-dev.tsno longer exists. Nothing about the SERVICE layer, the jail, the bounding or theCOMFYUI_MCP_ALLOW_GIT_WRITESdefault-closed flag changed — only the surface. Read the numbered tool sections above as the six ACTIONS they are today. Reads work day one; writes are ungated by design (single-user deployment reality, see #168);node_pack (action:"git") commit/pushrequire explicitCOMFYUI_MCP_ALLOW_GIT_WRITES=1— safe-by-default for the only genuinely new blast radius. - Remote/cloud modes: all six refuse cleanly (no
comfyuiPath), consistent with node-authoring’s LOCAL-ONLY contract. - Ships independently: no gate-system dependency (safety gates closed as won’t-do per issue #168; ROADMAP Theme G archives the design).