Skip to content

Commit aa9a5a5

Browse files
jcanizalezjcanizalez
authored
fix: add MCP input validation and filter env vars in script-runner (#31)
* fix: use getSafeEnv() in script-runner and ide-detector script-runner.ts passed raw process.env to spawned scripts, leaking all environment variables (API keys, tokens, credentials) to user scripts. ide-detector.ts spawned IDE processes without specifying env, which defaults to inheriting process.env. Both now use getSafeEnv() which filters out sensitive prefixes (AWS_SECRET, GITHUB_TOKEN, ANTHROPIC_API, STRIPE_, etc.) matching the same pattern already used by pty-manager, headless-manager, and agent-detector. * fix: add input validation to all MCP tool parameters Previously all MCP tool inputs used bare z.string() with no length limits or content validation. This allowed: - Unbounded strings (title, description) causing resource exhaustion - Path traversal in project_name (../../../etc/passwd) - Arbitrary-length prompts and terminal writes - Invalid hex colors and non-absolute paths Add shared validation module (packages/mcp/src/validation.ts) with bounded schemas for all input types: - name: 1-200 chars, no .. / \ (path traversal protection) - title: 1-500 chars - description: 0-5000 chars - prompt: 0-10000 chars - absolutePath: must start with / - hexColor: validated regex - id: 1-100 chars - shortText: 0-200 chars (branches, display names, icons) Applied across all 6 tool files: tasks, projects, sessions, workflows, git, and config. * fix: use getSafeEnv() in commandExists() for consistent env filtering The commandExists() helper used execFileSync without an explicit env, inheriting process.env with sensitive variables. Now uses getSafeEnv() to match the pattern already used in openInIDE().
1 parent 28015e8 commit aa9a5a5

File tree

8 files changed

+144
-76
lines changed

8 files changed

+144
-76
lines changed

‎packages/mcp/src/tools/git.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { z } from 'zod'
21
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
32
import { listBranches, getGitBranch, getGitDiffFull } from '@vibegrid/server/git-utils'
3+
import { V } from '../validation'
44

55
export function registerGitTools(server: McpServer): void {
66
server.tool(
77
'list_branches',
88
'List git branches for a project',
9-
{ project_path: z.string().describe('Absolute path to project directory') },
9+
{ project_path: V.absolutePath.describe('Absolute path to project directory') },
1010
async (args) => {
1111
try {
1212
const local = listBranches(args.project_path)
@@ -26,7 +26,7 @@ export function registerGitTools(server: McpServer): void {
2626
server.tool(
2727
'get_diff',
2828
'Get git diff for a project (staged and unstaged changes)',
29-
{ project_path: z.string().describe('Absolute path to project directory') },
29+
{ project_path: V.absolutePath.describe('Absolute path to project directory') },
3030
async (args) => {
3131
try {
3232
const result = getGitDiffFull(args.project_path)

‎packages/mcp/src/tools/projects.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from 'zod'
22
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
33
import type { configManager as ConfigManagerInstance } from '@vibegrid/server/config-manager'
44
import type { AgentType } from '@vibegrid/shared/types'
5+
import { V } from '../validation'
56
import {
67
dbListProjects,
78
dbGetProject,
@@ -35,11 +36,11 @@ export function registerProjectTools(
3536
'create_project',
3637
'Create a new project',
3738
{
38-
name: z.string().describe('Project name (unique identifier)'),
39-
path: z.string().describe('Absolute path to project directory'),
39+
name: V.name.describe('Project name (unique identifier)'),
40+
path: V.absolutePath.describe('Absolute path to project directory'),
4041
preferred_agents: z.array(z.enum(AGENT_TYPES)).optional().describe('Preferred agent types'),
41-
icon: z.string().optional().describe('Lucide icon name'),
42-
icon_color: z.string().optional().describe('Hex color for icon')
42+
icon: V.shortText.optional().describe('Lucide icon name'),
43+
icon_color: V.hexColor.optional().describe('Hex color for icon')
4344
},
4445
async (args) => {
4546
if (dbGetProject(args.name)) {
@@ -68,11 +69,11 @@ export function registerProjectTools(
6869
'update_project',
6970
"Update a project's properties",
7071
{
71-
name: z.string().describe('Project name (identifier, cannot be changed)'),
72-
path: z.string().optional().describe('New project path'),
72+
name: V.name.describe('Project name (identifier, cannot be changed)'),
73+
path: V.absolutePath.optional().describe('New project path'),
7374
preferred_agents: z.array(z.enum(AGENT_TYPES)).optional().describe('Preferred agent types'),
74-
icon: z.string().optional().describe('Lucide icon name'),
75-
icon_color: z.string().optional().describe('Hex color for icon')
75+
icon: V.shortText.optional().describe('Lucide icon name'),
76+
icon_color: V.hexColor.optional().describe('Hex color for icon')
7677
},
7778
async (args) => {
7879
if (!dbGetProject(args.name)) {
@@ -100,7 +101,7 @@ export function registerProjectTools(
100101
server.tool(
101102
'delete_project',
102103
'Delete a project and all its tasks',
103-
{ name: z.string().describe('Project name') },
104+
{ name: V.name.describe('Project name') },
104105
async (args) => {
105106
if (!dbGetProject(args.name)) {
106107
return {

‎packages/mcp/src/tools/sessions.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from 'zod'
22
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
33
import type { ptyManager as PtyManagerInstance } from '@vibegrid/server/pty-manager'
44
import type { AgentType, CreateTerminalPayload } from '@vibegrid/shared/types'
5+
import { V } from '../validation'
56

67
type PtyManager = typeof PtyManagerInstance
78

@@ -35,12 +36,12 @@ export function registerSessionTools(server: McpServer, deps: { ptyManager: PtyM
3536
'Launch an AI agent in a new terminal session',
3637
{
3738
agent_type: z.enum(AGENT_TYPES).describe('Agent type to launch'),
38-
project_name: z.string().describe('Project name'),
39-
project_path: z.string().describe('Absolute path to project directory'),
40-
prompt: z.string().optional().describe('Initial prompt to send to the agent'),
41-
branch: z.string().optional().describe('Git branch to checkout'),
39+
project_name: V.name.describe('Project name'),
40+
project_path: V.absolutePath.describe('Absolute path to project directory'),
41+
prompt: V.prompt.optional().describe('Initial prompt to send to the agent'),
42+
branch: V.shortText.optional().describe('Git branch to checkout'),
4243
use_worktree: z.boolean().optional().describe('Create a git worktree'),
43-
display_name: z.string().optional().describe('Display name for the session')
44+
display_name: V.shortText.optional().describe('Display name for the session')
4445
},
4546
async (args) => {
4647
const payload: CreateTerminalPayload = {
@@ -82,7 +83,7 @@ export function registerSessionTools(server: McpServer, deps: { ptyManager: PtyM
8283
server.tool(
8384
'kill_session',
8485
'Kill a terminal session',
85-
{ id: z.string().describe('Session ID to kill') },
86+
{ id: V.id.describe('Session ID to kill') },
8687
async (args) => {
8788
try {
8889
ptyManager.killPty(args.id)
@@ -97,8 +98,11 @@ export function registerSessionTools(server: McpServer, deps: { ptyManager: PtyM
9798
'write_to_terminal',
9899
'Send input to a running terminal session',
99100
{
100-
id: z.string().describe('Session ID'),
101-
data: z.string().describe('Data to write (text input to send to the agent)')
101+
id: V.id.describe('Session ID'),
102+
data: z
103+
.string()
104+
.max(50000, 'Data must be 50000 characters or less')
105+
.describe('Data to write (text input to send to the agent)')
102106
},
103107
async (args) => {
104108
try {

‎packages/mcp/src/tools/tasks.ts‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { z } from 'zod'
44
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
55
import type { configManager as ConfigManagerInstance } from '@vibegrid/server/config-manager'
66
import type { TaskConfig, TaskStatus, AgentType } from '@vibegrid/shared/types'
7+
import { V } from '../validation'
78
import {
89
dbListTasks,
910
dbGetTask,
@@ -33,7 +34,7 @@ export function registerTaskTools(server: McpServer, deps: { configManager: Conf
3334
'list_tasks',
3435
'List tasks, optionally filtered by project and/or status',
3536
{
36-
project_name: z.string().optional().describe('Filter by project name'),
37+
project_name: V.name.optional().describe('Filter by project name'),
3738
status: z
3839
.enum(TASK_STATUSES as [string, ...string[]])
3940
.optional()
@@ -49,14 +50,14 @@ export function registerTaskTools(server: McpServer, deps: { configManager: Conf
4950
'create_task',
5051
'Create a new task in a project',
5152
{
52-
project_name: z.string().describe('Project name (must match existing project)'),
53-
title: z.string().describe('Task title'),
54-
description: z.string().optional().describe('Task description (markdown)'),
53+
project_name: V.name.describe('Project name (must match existing project)'),
54+
title: V.title.describe('Task title'),
55+
description: V.description.optional().describe('Task description (markdown)'),
5556
status: z
5657
.enum(TASK_STATUSES as [string, ...string[]])
5758
.optional()
5859
.describe('Task status (default: todo)'),
59-
branch: z.string().optional().describe('Git branch for this task'),
60+
branch: V.shortText.optional().describe('Git branch for this task'),
6061
use_worktree: z.boolean().optional().describe('Create a git worktree for this task'),
6162
assigned_agent: z.enum(AGENT_TYPES).optional().describe('Assign to an agent type')
6263
},
@@ -95,34 +96,29 @@ export function registerTaskTools(server: McpServer, deps: { configManager: Conf
9596
}
9697
)
9798

98-
server.tool(
99-
'get_task',
100-
'Get a task by ID',
101-
{ id: z.string().describe('Task ID') },
102-
async (args) => {
103-
const task = dbGetTask(args.id)
104-
if (!task) {
105-
return {
106-
content: [{ type: 'text', text: `Error: task "${args.id}" not found` }],
107-
isError: true
108-
}
99+
server.tool('get_task', 'Get a task by ID', { id: V.id.describe('Task ID') }, async (args) => {
100+
const task = dbGetTask(args.id)
101+
if (!task) {
102+
return {
103+
content: [{ type: 'text', text: `Error: task "${args.id}" not found` }],
104+
isError: true
109105
}
110-
return { content: [{ type: 'text', text: JSON.stringify(task, null, 2) }] }
111106
}
112-
)
107+
return { content: [{ type: 'text', text: JSON.stringify(task, null, 2) }] }
108+
})
113109

114110
server.tool(
115111
'update_task',
116112
"Update a task's properties",
117113
{
118-
id: z.string().describe('Task ID'),
119-
title: z.string().optional().describe('New title'),
120-
description: z.string().optional().describe('New description'),
114+
id: V.id.describe('Task ID'),
115+
title: V.title.optional().describe('New title'),
116+
description: V.description.optional().describe('New description'),
121117
status: z
122118
.enum(TASK_STATUSES as [string, ...string[]])
123119
.optional()
124120
.describe('New status'),
125-
branch: z.string().optional().describe('Git branch'),
121+
branch: V.shortText.optional().describe('Git branch'),
126122
use_worktree: z.boolean().optional().describe('Use git worktree'),
127123
assigned_agent: z.enum(AGENT_TYPES).optional().describe('Assigned agent type'),
128124
order: z.number().optional().describe('Queue order')
@@ -165,7 +161,7 @@ export function registerTaskTools(server: McpServer, deps: { configManager: Conf
165161
server.tool(
166162
'delete_task',
167163
'Delete a task by ID',
168-
{ id: z.string().describe('Task ID') },
164+
{ id: V.id.describe('Task ID') },
169165
async (args) => {
170166
const task = dbGetTask(args.id)
171167
if (!task) {
@@ -186,15 +182,13 @@ export function registerTaskTools(server: McpServer, deps: { configManager: Conf
186182
'Get your current task and project context. Auto-detects based on your working directory. ' +
187183
'Call this at the start of a session to understand what you are working on.',
188184
{
189-
cwd: z
190-
.string()
185+
cwd: V.absolutePath
191186
.optional()
192187
.describe(
193188
'Your current working directory (auto-detected if omitted). ' +
194189
'Used to match against known projects and task worktrees.'
195190
),
196-
task_id: z
197-
.string()
191+
task_id: V.id
198192
.optional()
199193
.describe('Specific task ID to get context for (overrides auto-detection)')
200194
},

‎packages/mcp/src/tools/workflows.ts‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { z } from 'zod'
33
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
44
import type { configManager as ConfigManagerInstance } from '@vibegrid/server/config-manager'
55
import type { scheduler as SchedulerInstance } from '@vibegrid/server/scheduler'
6+
import { V } from '../validation'
67
import type {
78
WorkflowDefinition,
89
WorkflowNode,
@@ -22,48 +23,48 @@ type Scheduler = typeof SchedulerInstance
2223

2324
const launchAgentConfigSchema = z.object({
2425
agentType: z.enum(['claude', 'copilot', 'codex', 'opencode', 'gemini']),
25-
projectName: z.string(),
26-
projectPath: z.string(),
27-
args: z.array(z.string()).optional(),
28-
displayName: z.string().optional(),
29-
branch: z.string().optional(),
26+
projectName: V.name,
27+
projectPath: V.absolutePath,
28+
args: z.array(V.shortText).optional(),
29+
displayName: V.shortText.optional(),
30+
branch: V.shortText.optional(),
3031
useWorktree: z.boolean().optional(),
31-
remoteHostId: z.string().optional(),
32-
prompt: z.string().optional(),
32+
remoteHostId: V.id.optional(),
33+
prompt: V.prompt.optional(),
3334
promptDelayMs: z.number().optional(),
34-
taskId: z.string().optional(),
35+
taskId: V.id.optional(),
3536
taskFromQueue: z.boolean().optional()
3637
})
3738

3839
const triggerConfigSchema = z.union([
3940
z.object({ triggerType: z.literal('manual') }),
40-
z.object({ triggerType: z.literal('once'), runAt: z.string() }),
41+
z.object({ triggerType: z.literal('once'), runAt: V.shortText }),
4142
z.object({
4243
triggerType: z.literal('recurring'),
43-
cron: z.string(),
44-
timezone: z.string().optional()
44+
cron: V.shortText,
45+
timezone: V.shortText.optional()
4546
}),
46-
z.object({ triggerType: z.literal('taskCreated'), projectFilter: z.string().optional() }),
47+
z.object({ triggerType: z.literal('taskCreated'), projectFilter: V.name.optional() }),
4748
z.object({
4849
triggerType: z.literal('taskStatusChanged'),
49-
projectFilter: z.string().optional(),
50+
projectFilter: V.name.optional(),
5051
fromStatus: z.enum(['todo', 'in_progress', 'in_review', 'done', 'cancelled']).optional(),
5152
toStatus: z.enum(['todo', 'in_progress', 'in_review', 'done', 'cancelled']).optional()
5253
})
5354
])
5455

5556
const nodeSchema = z.object({
56-
id: z.string(),
57+
id: V.id,
5758
type: z.enum(['trigger', 'launchAgent']),
58-
label: z.string(),
59+
label: V.shortText,
5960
config: z.record(z.unknown()),
6061
position: z.object({ x: z.number(), y: z.number() })
6162
})
6263

6364
const edgeSchema = z.object({
64-
id: z.string(),
65-
source: z.string(),
66-
target: z.string()
65+
id: V.id,
66+
source: V.id,
67+
target: V.id
6768
})
6869

6970
/**
@@ -135,7 +136,7 @@ export function registerWorkflowTools(
135136
'create_workflow',
136137
'Create a new workflow. Accepts either full nodes/edges or a convenience flat format (trigger + actions array).',
137138
z.object({
138-
name: z.string().describe('Workflow name'),
139+
name: V.title.describe('Workflow name'),
139140
trigger: triggerConfigSchema
140141
.optional()
141142
.describe('Trigger config (convenience mode). Defaults to manual.'),
@@ -145,8 +146,8 @@ export function registerWorkflowTools(
145146
.describe('Actions to execute (convenience mode). Auto-generates graph.'),
146147
nodes: z.array(nodeSchema).optional().describe('Full graph nodes (advanced mode)'),
147148
edges: z.array(edgeSchema).optional().describe('Full graph edges (advanced mode)'),
148-
icon: z.string().optional().describe('Lucide icon name (default: zap)'),
149-
icon_color: z.string().optional().describe('Hex color (default: #6366f1)'),
149+
icon: V.shortText.optional().describe('Lucide icon name (default: zap)'),
150+
icon_color: V.hexColor.optional().describe('Hex color (default: #6366f1)'),
150151
enabled: z.boolean().optional().describe('Whether workflow is enabled (default: true)'),
151152
stagger_delay_ms: z.number().optional().describe('Delay in ms between actions')
152153
}),
@@ -188,12 +189,12 @@ export function registerWorkflowTools(
188189
'update_workflow',
189190
"Update a workflow's properties",
190191
z.object({
191-
id: z.string().describe('Workflow ID'),
192-
name: z.string().optional(),
192+
id: V.id.describe('Workflow ID'),
193+
name: V.title.optional(),
193194
nodes: z.array(nodeSchema).optional(),
194195
edges: z.array(edgeSchema).optional(),
195-
icon: z.string().optional(),
196-
icon_color: z.string().optional(),
196+
icon: V.shortText.optional(),
197+
icon_color: V.hexColor.optional(),
197198
enabled: z.boolean().optional(),
198199
stagger_delay_ms: z.number().optional()
199200
}),
@@ -229,7 +230,7 @@ export function registerWorkflowTools(
229230
server.tool(
230231
'delete_workflow',
231232
'Delete a workflow',
232-
{ id: z.string().describe('Workflow ID') },
233+
{ id: V.id.describe('Workflow ID') },
233234
async (args) => {
234235
const workflows = dbListWorkflows()
235236
const workflow = workflows.find((w) => w.id === args.id)

0 commit comments

Comments
 (0)