diff --git a/forge/comms/platformAutomation.js b/forge/comms/platformAutomation.js index 2cadbf858d..c6155ffc62 100644 --- a/forge/comms/platformAutomation.js +++ b/forge/comms/platformAutomation.js @@ -204,9 +204,9 @@ class PlatformAutomationHandler { }) } - const { formatResponse } = require('../ee/lib/mcp/toolLoader') + const { formatResponse, invokeTool } = require('../ee/lib/mcp/toolLoader') const args = applyInputDefaults(tool, data?.input || {}) - const response = await tool.handler(args, { inject, app: this.app, user, mcpSessionId, scope: callerScope }) + const response = await invokeTool(tool, args, { inject, app: this.app, user, mcpSessionId, scope: callerScope }) result = formatResponse(response) break } diff --git a/forge/ee/lib/mcp/schemas.js b/forge/ee/lib/mcp/schemas.js index 4fb9867580..2ee1b3a6a4 100644 --- a/forge/ee/lib/mcp/schemas.js +++ b/forge/ee/lib/mcp/schemas.js @@ -1,11 +1,16 @@ const { z } = require('zod') +// Ids are interpolated into the URL path handed to app.inject(), which resolves dot segments, so +// an id must be one plain path segment: hashid, UUID, or the 'team-broker' literal. +const PATH_ID_PATTERN = /^[A-Za-z0-9_-]+$/ +const pathId = z.string().regex(PATH_ID_PATTERN, 'must contain only letters, digits, hyphen and underscore') + // Hosted instances are Projects (UUID primary key); the other entities use hashids. -const teamId = z.string().describe('The hashid of the team') -const applicationId = z.string().describe('The hashid of the application') +const teamId = pathId.describe('The hashid of the team') +const applicationId = pathId.describe('The hashid of the application') const hostedInstanceId = z.string().uuid().describe('The id (UUID) of the hosted instance') -const remoteInstanceId = z.string().describe('The hashid of the remote instance') -const snapshotId = z.string().describe('The hashid of the snapshot') +const remoteInstanceId = pathId.describe('The hashid of the remote instance') +const snapshotId = pathId.describe('The hashid of the snapshot') // Shared by the pipeline stage add and update tools: both routes take the same // git-repo fields. forUpdate carries the one real difference between them, that @@ -14,7 +19,7 @@ const snapshotId = z.string().describe('The hashid of the snapshot') const gitStageFields = ({ forUpdate = false } = {}) => { const resendNote = forUpdate ? '. Resent on every git update; reset to empty when omitted' : '' return { - gitTokenId: z.string().optional().describe(forUpdate + gitTokenId: pathId.optional().describe(forUpdate ? 'Hashid of a team git token. Required on every update of a git-repo stage, together with the full git settings, since the git settings are only applied when it is present and are applied as a set' : 'Hashid of a team git token, making this a git-repository stage. Pass exactly one target, and include url'), url: z.string().optional().describe(`Git repository URL for a git-repo stage${resendNote}`), @@ -111,6 +116,8 @@ function appendQuery (url, args, keys) { } module.exports = { + PATH_ID_PATTERN, + pathId, teamId, applicationId, hostedInstanceId, diff --git a/forge/ee/lib/mcp/tool-schemas/tables.js b/forge/ee/lib/mcp/tool-schemas/tables.js index 179c790273..a7376aadf7 100644 --- a/forge/ee/lib/mcp/tool-schemas/tables.js +++ b/forge/ee/lib/mcp/tool-schemas/tables.js @@ -1,9 +1,11 @@ const { z } = require('zod') +const { pathId } = require('../schemas') + // Shared field schemas reused across the FlowFuse Tables tools, since the same // database/table identifiers appear in both the input and output schemas. -const teamIdSchema = z.string().describe('The hashid of the team') -const databaseIdSchema = z.string().describe('The hashid of the FlowFuse Tables database') +const teamIdSchema = pathId.describe('The hashid of the team') +const databaseIdSchema = pathId.describe('The hashid of the FlowFuse Tables database') const tableNameSchema = z.string().describe('Name of the database table') const schemaNameSchema = z.string().describe('Schema the table lives in, as returned by platform_list_database_tables') const databaseSchema = z.object({ diff --git a/forge/ee/lib/mcp/toolLoader.js b/forge/ee/lib/mcp/toolLoader.js index e9b4d1cf74..b209ca318b 100644 --- a/forge/ee/lib/mcp/toolLoader.js +++ b/forge/ee/lib/mcp/toolLoader.js @@ -1,6 +1,8 @@ const fs = require('fs') const path = require('path') +const { validateIdArgs, guardInject } = require('./utils') + const toolsDir = path.join(__dirname, 'tools') /** @@ -39,4 +41,17 @@ function formatResponse (response) { return body } -module.exports = { formatResponse, loadToolDefinitions } +/** + * Runs a tool handler behind the shared path guard: rejects id arguments that are not a plain + * path segment, and hands the handler an inject that refuses dot-segment paths. Every dispatch + * of a tool must go through here so individual tools never have to remember the checks. + */ +async function invokeTool (tool, args, context) { + const invalid = validateIdArgs(args) + if (invalid) { + return invalid + } + return tool.handler(args, { ...context, inject: guardInject(context.inject) }) +} + +module.exports = { formatResponse, loadToolDefinitions, invokeTool } diff --git a/forge/ee/lib/mcp/tools/broker.js b/forge/ee/lib/mcp/tools/broker.js index f079c48bcd..f6a12195c7 100644 --- a/forge/ee/lib/mcp/tools/broker.js +++ b/forge/ee/lib/mcp/tools/broker.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { teamId, basePagination, basePaginationKeys, searchQuery, appendQuery, toolError } = require('../schemas') +const { pathId, teamId, basePagination, basePaginationKeys, searchQuery, appendQuery, toolError } = require('../schemas') const isHashid = (id) => /^[A-Za-z0-9]+$/.test(id) @@ -38,7 +38,7 @@ module.exports = [ username: z.string().describe('Username of the broker client to fetch') }, handler: async (args, { inject }) => { - const response = await inject({ method: 'GET', url: `/api/v1/teams/${args.teamId}/broker/client/${args.username}` }) + const response = await inject({ method: 'GET', url: `/api/v1/teams/${args.teamId}/broker/client/${encodeURIComponent(args.username)}` }) return response } }, @@ -76,7 +76,7 @@ module.exports = [ annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/teams/${args.teamId}/brokers/${args.brokerId}` }) @@ -95,7 +95,7 @@ module.exports = [ annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/teams/${args.teamId}/brokers/${args.brokerId}/topics` }) @@ -115,7 +115,7 @@ module.exports = [ annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid") }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/teams/${args.teamId}/broker/${args.brokerId}/schema` }) @@ -136,7 +136,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), action: z.enum(['start', 'stop', 'suspend']).describe('Lifecycle transition to apply to the broker topic-collection agent') }, handler: async (args, { inject }) => { @@ -154,7 +154,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), topics: z.array(z.object({ topic: z.string().describe('MQTT topic string, e.g. "factory/line1/temperature"'), metadata: z.record(z.string(), z.any()).optional().describe('Arbitrary topic metadata, e.g. { description: "..." }') @@ -176,8 +176,8 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), - topicId: z.string().describe('The hashid of the topic to update, as returned by platform_list_broker_topics'), + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), + topicId: pathId.describe('The hashid of the topic to update, as returned by platform_list_broker_topics'), metadata: z.record(z.string(), z.any()).describe('Replacement metadata object for the topic, e.g. { description: "..." }') }, handler: async (args, { inject }) => { @@ -199,8 +199,8 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - brokerId: z.string().describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), - topicId: z.string().describe('The hashid of the topic to delete, as returned by platform_list_broker_topics') + brokerId: pathId.describe("broker id: either the literal 'team-broker' or a 3rd-party broker hashid"), + topicId: pathId.describe('The hashid of the topic to delete, as returned by platform_list_broker_topics') }, handler: async (args, { inject }) => { // The ids go into the URL path and inject resolves dot segments, so anything but a plain diff --git a/forge/ee/lib/mcp/tools/deviceGroups.js b/forge/ee/lib/mcp/tools/deviceGroups.js index 8348e5e061..3a30f3f56e 100644 --- a/forge/ee/lib/mcp/tools/deviceGroups.js +++ b/forge/ee/lib/mcp/tools/deviceGroups.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { appendQuery, applicationId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, teamId, toolError } = require('../schemas') +const { pathId, appendQuery, applicationId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, teamId, toolError } = require('../schemas') module.exports = [ { @@ -59,7 +59,7 @@ module.exports = [ annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { applicationId, - groupId: z.string().describe('Device group hashid to fetch') + groupId: pathId.describe('Device group hashid to fetch') }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/applications/${args.applicationId}/device-groups/${args.groupId}` }) @@ -104,10 +104,10 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { applicationId: applicationId.describe('The hashid of the application the group belongs to'), - groupId: z.string().describe('The hashid of the device group to update'), + groupId: pathId.describe('The hashid of the device group to update'), name: z.string().min(1).optional().describe('New name for the group'), description: z.string().optional().describe('New description for the group'), - targetSnapshotId: z.string().nullable().optional().describe('The hashid of the snapshot to pin as the group target (deployed to all member devices immediately), or null to clear the pin. The snapshot must belong to the same application') + targetSnapshotId: pathId.nullable().optional().describe('The hashid of the snapshot to pin as the group target (deployed to all member devices immediately), or null to clear the pin. The snapshot must belong to the same application') }, handler: async (args, { inject }) => { const payload = {} @@ -139,7 +139,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { applicationId: applicationId.describe('The hashid of the application the group belongs to'), - groupId: z.string().describe('The hashid of the device group whose membership to change'), + groupId: pathId.describe('The hashid of the device group whose membership to change'), add: z.array(z.string()).optional().describe('Hashids of devices to add to the group'), remove: z.array(z.string()).optional().describe('Hashids of devices to remove from the group'), set: z.array(z.string()).optional().describe('Hashids of devices to set as the exact membership, replacing the current list. Overrides add/remove') @@ -172,7 +172,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { applicationId: applicationId.describe('The hashid of the application the group belongs to'), - groupId: z.string().describe('The hashid of the device group whose settings to update'), + groupId: pathId.describe('The hashid of the device group whose settings to update'), env: z.array(z.object({ name: z.string().describe('Environment variable name: a letter or underscore followed by letters, digits or underscores'), value: z.string().describe('Environment variable value. For an existing hidden variable, pass an empty string (with hidden true) to keep the stored value'), @@ -199,7 +199,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { applicationId: applicationId.describe('The hashid of the application the group belongs to'), - groupId: z.string().describe('The hashid of the device group to delete') + groupId: pathId.describe('The hashid of the device group to delete') }, handler: async (args, { inject }) => { // Both ids go into the URL path and inject resolves dot segments, so anything but a plain diff --git a/forge/ee/lib/mcp/tools/instances.js b/forge/ee/lib/mcp/tools/instances.js index 37bf1f92f7..b65dfbdc1b 100644 --- a/forge/ee/lib/mcp/tools/instances.js +++ b/forge/ee/lib/mcp/tools/instances.js @@ -2,7 +2,7 @@ const { randomUUID } = require('crypto') const { z } = require('zod') -const { teamId, applicationId, hostedInstanceId, searchQuery, sortParams, limitParam, pageParam, toolError } = require('../schemas') +const { pathId, teamId, applicationId, hostedInstanceId, searchQuery, sortParams, limitParam, pageParam, toolError } = require('../schemas') const { emptySuccessAsOkay } = require('../utils') // Mirrors the runningStates/errorStates/stoppedStates groups in frontend/src/composables/InstanceStates.js, @@ -201,7 +201,7 @@ module.exports = [ projectType: z.string().describe('The ID of the hosted instance type (use platform_list_hosted_instance_types to find valid values)'), stack: z.string().describe('The ID of the stack (use platform_list_hosted_instance_types to find valid values)'), template: z.string().describe('The ID of the template (use platform_list_templates to find valid values)'), - flowBlueprintId: z.string().optional().describe('Optional blueprint ID to initialize the hosted instance with starter flows (use platform_list_blueprints to find valid values)') + flowBlueprintId: pathId.optional().describe('Optional blueprint ID to initialize the hosted instance with starter flows (use platform_list_blueprints to find valid values)') }, handler: async (args, { inject }) => { const payload = { @@ -639,7 +639,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, inputSchema: { instanceId: hostedInstanceId.describe('The id (UUID) of the hosted instance to roll back'), - snapshotId: z.string().describe('The hashid of the snapshot to roll back to. Must be a snapshot of this hosted instance') + snapshotId: pathId.describe('The hashid of the snapshot to roll back to. Must be a snapshot of this hosted instance') }, handler: async (args, { inject }) => { const response = await inject({ method: 'POST', url: `/api/v1/projects/${args.instanceId}/actions/rollback`, payload: { snapshot: args.snapshotId } }) diff --git a/forge/ee/lib/mcp/tools/pipelines.js b/forge/ee/lib/mcp/tools/pipelines.js index 924148cad9..63e5b4a7b0 100644 --- a/forge/ee/lib/mcp/tools/pipelines.js +++ b/forge/ee/lib/mcp/tools/pipelines.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { teamId, applicationId, toolError, gitStageFields, gitStageFieldKeys } = require('../schemas') +const { pathId, teamId, applicationId, toolError, gitStageFields, gitStageFieldKeys } = require('../schemas') const isHashid = (id) => /^[A-Za-z0-9]+$/.test(id) @@ -49,8 +49,8 @@ module.exports = [ Use this once you have a pipeline ID and a stage ID and need to inspect that stage.`, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('Pipeline hashid the stage belongs to'), - stageId: z.string().describe('Pipeline stage hashid to fetch') + pipelineId: pathId.describe('Pipeline hashid the stage belongs to'), + stageId: pathId.describe('Pipeline stage hashid to fetch') }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/pipelines/${args.pipelineId}/stages/${args.stageId}` }) @@ -80,7 +80,7 @@ module.exports = [ Renames a pipeline. The name is the only property a pipeline has beyond its stages; use the stage tools to change stages.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline to rename'), + pipelineId: pathId.describe('The hashid of the pipeline to rename'), name: z.string().min(1).describe('New name for the pipeline') }, handler: async (args, { inject }) => { @@ -98,11 +98,11 @@ module.exports = [ Ordering rules enforced by the API: a device group cannot be the first stage, and an instance or device cannot be added after a device group.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline to add the stage to'), + pipelineId: pathId.describe('The hashid of the pipeline to add the stage to'), name: z.string().min(1).describe('Name for the stage'), instanceId: z.string().uuid().optional().describe('UUID of the hosted instance this stage deploys to. Pass exactly one target'), - deviceId: z.string().optional().describe('Hashid of the remote instance (device) this stage deploys to. Pass exactly one target'), - deviceGroupId: z.string().optional().describe('Hashid of the device group this stage deploys to. Cannot be the first stage. Pass exactly one target'), + deviceId: pathId.optional().describe('Hashid of the remote instance (device) this stage deploys to. Pass exactly one target'), + deviceGroupId: pathId.optional().describe('Hashid of the device group this stage deploys to. Cannot be the first stage. Pass exactly one target'), ...gitStageFields(), deployToDevices, action: stageAction, @@ -127,12 +127,12 @@ module.exports = [ Rebinding the target replaces the previous binding, and the same ordering rules as adding apply: a device group cannot be the first stage, and an instance or device cannot come after a device group.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline the stage belongs to'), - stageId: z.string().describe('The hashid of the stage to update'), + pipelineId: pathId.describe('The hashid of the pipeline the stage belongs to'), + stageId: pathId.describe('The hashid of the stage to update'), name: z.string().min(1).optional().describe('New name for the stage'), instanceId: z.string().uuid().optional().describe('UUID of the hosted instance to rebind this stage to. Pass at most one target'), - deviceId: z.string().optional().describe('Hashid of the remote instance (device) to rebind this stage to. Pass at most one target'), - deviceGroupId: z.string().optional().describe('Hashid of the device group to rebind this stage to. Pass at most one target'), + deviceId: pathId.optional().describe('Hashid of the remote instance (device) to rebind this stage to. Pass at most one target'), + deviceGroupId: pathId.optional().describe('Hashid of the device group to rebind this stage to. Pass at most one target'), ...gitStageFields({ forUpdate: true }), deployToDevices, action: stageAction @@ -169,9 +169,9 @@ module.exports = [ // platform_set_instance_device_target, so it belongs behind destructive access. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline'), - stageId: z.string().describe('The hashid of the SOURCE stage to deploy from; the deploy lands on the stage after it'), - sourceSnapshotId: z.string().optional().describe('The hashid of the snapshot to deploy. Only used (and required) when the source stage action is prompt') + pipelineId: pathId.describe('The hashid of the pipeline'), + stageId: pathId.describe('The hashid of the SOURCE stage to deploy from; the deploy lands on the stage after it'), + sourceSnapshotId: pathId.optional().describe('The hashid of the snapshot to deploy. Only used (and required) when the source stage action is prompt') }, handler: async (args, { inject }) => { const payload = {} @@ -195,7 +195,7 @@ module.exports = [ // idempotentHint: a repeat call has no further effect, it just answers 404. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline to delete') + pipelineId: pathId.describe('The hashid of the pipeline to delete') }, handler: async (args, { inject }) => { // pipelineId goes into the URL path and inject resolves dot segments, so anything but @@ -221,8 +221,8 @@ module.exports = [ // idempotentHint: a repeat call has no further effect, it just answers 404. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - pipelineId: z.string().describe('The hashid of the pipeline the stage belongs to'), - stageId: z.string().describe('The hashid of the stage to delete') + pipelineId: pathId.describe('The hashid of the pipeline the stage belongs to'), + stageId: pathId.describe('The hashid of the stage to delete') }, handler: async (args, { inject }) => { // Same as above, for both ids. diff --git a/forge/ee/lib/mcp/tools/platform.js b/forge/ee/lib/mcp/tools/platform.js index 3a136635a2..fa6d97cc42 100644 --- a/forge/ee/lib/mcp/tools/platform.js +++ b/forge/ee/lib/mcp/tools/platform.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { teamId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, appendQuery } = require('../schemas') +const { pathId, teamId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, appendQuery } = require('../schemas') const { PLATFORM_UI_TOOL_NAMES, SESSION_GATED_GROUPS, noBrowserSessionGuidance } = require('../sessionGatedTools') function getProperty (properties, key) { @@ -128,7 +128,7 @@ module.exports = [ description: 'FlowFuse platform automation tool: Get a single template by id. Hidden environment variable values are blanked in the response; visible ones are returned as set.', annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - templateId: z.string().describe('Template hashid') + templateId: pathId.describe('Template hashid') }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/templates/${args.templateId}` }) @@ -141,7 +141,7 @@ module.exports = [ description: 'FlowFuse platform automation tool: Get a single flow blueprint by id. By default the flow content is omitted from the response.', annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - flowBlueprintId: z.string().describe('Flow blueprint hashid'), + flowBlueprintId: pathId.describe('Flow blueprint hashid'), includeFlow: z.boolean().default(false).optional().describe('Whether to include the full flow JSON in the response. Omitted by default to keep the response small; set true to fetch it, e.g. to deploy it.') }, handler: async (args, { inject }) => { @@ -184,7 +184,7 @@ module.exports = [ Use this to inspect the tier/plan a team is on, or to check a team type before assigning it to a new team.`, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - teamTypeId: z.string().describe('Team type hashid') + teamTypeId: pathId.describe('Team type hashid') }, handler: async (args, { inject }) => { const response = await inject({ method: 'GET', url: `/api/v1/team-types/${args.teamTypeId}` }) diff --git a/forge/ee/lib/mcp/tools/shared-instances-devices.js b/forge/ee/lib/mcp/tools/shared-instances-devices.js index 2dd555a37a..eba7de449a 100644 --- a/forge/ee/lib/mcp/tools/shared-instances-devices.js +++ b/forge/ee/lib/mcp/tools/shared-instances-devices.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { basePagination, basePaginationKeys, searchQuery, searchQueryKeys, auditLogFilters, auditLogFilterKeys, appendQuery, toolError, hostedInstanceId } = require('../schemas') +const { pathId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, auditLogFilters, auditLogFilterKeys, appendQuery, toolError, hostedInstanceId } = require('../schemas') const { emptySuccessAsOkay } = require('../utils') // Tools that work against both hosted instances and remote instances (devices), @@ -16,7 +16,7 @@ module.exports = [ HTTP bearer tokens are a plan-gated feature: a team without it enabled gets a 404 error.`, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")') }, handler: async (args, { inject }) => { @@ -34,7 +34,7 @@ module.exports = [ Use this when the user wants a chronological view of what changed on an instance.`, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), ...basePagination }, @@ -55,7 +55,7 @@ module.exports = [ scope and includeChildren apply only to hosted instances: by default only the instance's own ("project") entries are returned; set scope to "device" to read the entries for its assigned devices instead, and set includeChildren to also include entries from child entities within the chosen scope. A remote instance has no child entities, so these two parameters are rejected when instanceType is "remote".`, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), ...basePagination, ...searchQuery, @@ -88,7 +88,7 @@ module.exports = [ // destructiveHint: stop and suspend take the instance down, and restart interrupts whatever it is running. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), action: z.enum(['start', 'stop', 'restart', 'suspend', 'restartStack']).describe('Lifecycle action to apply. Remote instances accept only restart') }, @@ -112,7 +112,7 @@ module.exports = [ HTTP bearer tokens are a plan-gated feature (the same gate as FlowFuse User Authentication); a team without it enabled gets a 404 error.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), name: z.string().describe('Human-readable name for the token'), expiresAt: z.string().optional().describe('Token expiry as an ISO 8601 timestamp. Omit for a token that never expires') @@ -138,9 +138,9 @@ module.exports = [ // destructiveHint: omitting expiresAt clears the expiry, removing a security control from an existing token. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), - tokenId: z.string().describe('The hashid of the token to update, as returned by platform_list_instance_http_tokens'), + tokenId: pathId.describe('The hashid of the token to update, as returned by platform_list_instance_http_tokens'), expiresAt: z.string().optional().describe('New expiry as an ISO 8601 timestamp. OMITTING THIS CLEARS THE EXPIRY, making the token never expire') }, handler: async (args, { inject }) => { @@ -166,9 +166,9 @@ module.exports = [ // idempotentHint: a repeat call has no further effect, it just answers 404. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")'), - tokenId: z.string().describe('The hashid of the token to delete, as returned by platform_list_instance_http_tokens') + tokenId: pathId.describe('The hashid of the token to delete, as returned by platform_list_instance_http_tokens') }, handler: async (args, { inject }) => { // Both ids go into the URL path and inject resolves dot segments, so anything but @@ -199,7 +199,7 @@ module.exports = [ // idempotentHint: a repeat call has no further effect, it just answers 404. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - instanceId: z.string().describe('The ID of the instance to delete (hosted instance UUID, or remote instance/device hashid)'), + instanceId: pathId.describe('The ID of the instance to delete (hosted instance UUID, or remote instance/device hashid)'), instanceType: z.enum(['hosted', 'remote']).describe('Whether instanceId refers to a hosted instance ("hosted") or a remote instance/device ("remote")') }, handler: async (args, { inject }) => { diff --git a/forge/ee/lib/mcp/tools/snapshots.js b/forge/ee/lib/mcp/tools/snapshots.js index 4c0de23f46..fad2d3ca72 100644 --- a/forge/ee/lib/mcp/tools/snapshots.js +++ b/forge/ee/lib/mcp/tools/snapshots.js @@ -2,7 +2,7 @@ const SemVer = require('semver') const { z } = require('zod') const { MIN_DEVICE_AGENT_VERSIONS } = require('../constants') -const { basePaginationKeys, limitParam, appendQuery, hostedInstanceId, remoteInstanceId, snapshotId, snapshotComponents, toolError } = require('../schemas') +const { pathId, basePaginationKeys, limitParam, appendQuery, hostedInstanceId, remoteInstanceId, snapshotId, snapshotComponents, toolError } = require('../schemas') const { blankHiddenEnvValues } = require('../utils') // Width of ProjectSnapshot.name, a DataTypes.STRING column. @@ -33,7 +33,7 @@ module.exports = [ annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { instanceType: z.enum(['hosted', 'remote']).describe('Which kind of instance instanceId refers to: "hosted" for a hosted instance, "remote" for a remote instance (device)'), - instanceId: z.string().describe('The ID of the instance whose snapshots to list (UUID for a hosted instance, hashid for a remote instance)'), + instanceId: pathId.describe('The ID of the instance whose snapshots to list (UUID for a hosted instance, hashid for a remote instance)'), cursor: z.string().optional().describe('Cursor for pagination (the hashid of the last item from the previous page)'), ...limitParam }, @@ -59,7 +59,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, inputSchema: { instanceType: z.enum(['hosted', 'remote']).describe('Which kind of instance instanceId refers to: "hosted" for a hosted instance, "remote" for a remote instance (device)'), - instanceId: z.string().describe('The ID of the instance to snapshot (UUID for a hosted instance, hashid for a remote instance)'), + instanceId: pathId.describe('The ID of the instance to snapshot (UUID for a hosted instance, hashid for a remote instance)'), name: z.string().trim().min(1).max(SNAPSHOT_NAME_MAX_LENGTH).describe('Name for the snapshot'), description: z.string().optional().describe('Description of the snapshot') }, @@ -226,7 +226,7 @@ module.exports = [ Use components to import selectively, for example envVars: "keys" to import env var names without their values.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, inputSchema: { - ownerId: z.string().describe('Target owner id: hosted instance (project) UUID when ownerType is "instance", or device hashid when ownerType is "device"'), + ownerId: pathId.describe('Target owner id: hosted instance (project) UUID when ownerType is "instance", or device hashid when ownerType is "device"'), ownerType: z.enum(['instance', 'device']).describe('Type of resource that will own the imported snapshot'), snapshot: z.object({ name: z.string().describe('Name for the imported snapshot'), diff --git a/forge/ee/lib/mcp/tools/teams.js b/forge/ee/lib/mcp/tools/teams.js index ca9ca92f17..78ee0cd4bd 100644 --- a/forge/ee/lib/mcp/tools/teams.js +++ b/forge/ee/lib/mcp/tools/teams.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { teamId, applicationId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, auditLogFilters, auditLogFilterKeys, appendQuery, toolError } = require('../schemas') +const { pathId, teamId, applicationId, basePagination, basePaginationKeys, searchQuery, searchQueryKeys, auditLogFilters, auditLogFilterKeys, appendQuery, toolError } = require('../schemas') const isHashid = (id) => /^[A-Za-z0-9]+$/.test(id) @@ -53,7 +53,7 @@ module.exports = [ } const url = hasId ? `/api/v1/teams/${args.teamId}` - : `/api/v1/teams/slug/${args.teamSlug}` + : `/api/v1/teams/slug/${encodeURIComponent(args.teamSlug)}` const response = await inject({ method: 'GET', url }) return response } @@ -252,7 +252,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - userId: z.string().describe('The hashid of the team member whose role is changing'), + userId: pathId.describe('The hashid of the team member whose role is changing'), role: z.union([z.literal(5), z.literal(10), z.literal(30), z.literal(50)]).describe('New team role: 5=Dashboard, 10=Viewer, 30=Member, 50=Owner') }, handler: async (args, { inject }) => { @@ -293,7 +293,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - invitationId: z.string().describe('The hashid of the invitation to resend, as returned by platform_list_team_invitations') + invitationId: pathId.describe('The hashid of the invitation to resend, as returned by platform_list_team_invitations') }, handler: async (args, { inject }) => { const response = await inject({ method: 'POST', url: `/api/v1/teams/${args.teamId}/invitations/${args.invitationId}` }) @@ -315,7 +315,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - userId: z.string().describe('The hashid of the member to remove, as returned by platform_list_team_members') + userId: pathId.describe('The hashid of the member to remove, as returned by platform_list_team_members') }, handler: async (args, { inject }) => { // Both ids go into the URL path and inject resolves dot segments, so anything but @@ -345,7 +345,7 @@ module.exports = [ annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { teamId, - invitationId: z.string().describe('The hashid of the invitation to revoke, as returned by platform_list_team_invitations') + invitationId: pathId.describe('The hashid of the invitation to revoke, as returned by platform_list_team_invitations') }, handler: async (args, { inject }) => { // Same as above: a crafted invitationId like "../../../applications/" reaches another route. diff --git a/forge/ee/lib/mcp/tools/user.js b/forge/ee/lib/mcp/tools/user.js index 4588ec18ef..033e563785 100644 --- a/forge/ee/lib/mcp/tools/user.js +++ b/forge/ee/lib/mcp/tools/user.js @@ -1,6 +1,6 @@ const { z } = require('zod') -const { basePagination, basePaginationKeys, appendQuery, teamId, toolError } = require('../schemas') +const { pathId, basePagination, basePaginationKeys, appendQuery, teamId, toolError } = require('../schemas') module.exports = [ { @@ -40,7 +40,7 @@ module.exports = [ The single-notification call replies { status: "okay" }; the bulk call replies with the refreshed notification list.`, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, inputSchema: { - notificationId: z.string().optional().describe('The hashid of a single notification to change. Provide exactly one of notificationId or ids'), + notificationId: pathId.optional().describe('The hashid of a single notification to change. Provide exactly one of notificationId or ids'), ids: z.array(z.string()).min(1).optional().describe('Hashids of the notifications to change in bulk, at least one. Provide exactly one of notificationId or ids'), read: z.boolean().describe('Read state to set: true marks read, false marks unread') }, @@ -69,7 +69,7 @@ module.exports = [ // idempotentHint: a repeat call has no further effect, it just answers 404. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, inputSchema: { - notificationId: z.string().describe('The hashid of the notification to delete, as returned by platform_list_notifications') + notificationId: pathId.describe('The hashid of the notification to delete, as returned by platform_list_notifications') }, handler: async (args, { inject }) => { // notificationId goes into the URL path and inject resolves dot segments, so anything but @@ -92,7 +92,7 @@ module.exports = [ // original sender can issue another, so this removes rather than adds. annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, inputSchema: { - invitationId: z.string().describe('The hashid of an invitation the current user received, as returned by platform_list_own_invitations'), + invitationId: pathId.describe('The hashid of an invitation the current user received, as returned by platform_list_own_invitations'), action: z.enum(['accept', 'reject']).describe('Whether to accept the invitation (join the team) or reject it (decline)') }, handler: async (args, { inject }) => { diff --git a/forge/ee/lib/mcp/utils.js b/forge/ee/lib/mcp/utils.js index efd93a5c72..59f3625f70 100644 --- a/forge/ee/lib/mcp/utils.js +++ b/forge/ee/lib/mcp/utils.js @@ -1,3 +1,5 @@ +const { PATH_ID_PATTERN, toolError } = require('./schemas') + // Strips credentials (including the password) before returning results to the caller. function redactDatabaseCredentials (database) { if (!database) { @@ -29,4 +31,62 @@ function emptySuccessAsOkay (response) { return response } -module.exports = { redactDatabaseCredentials, blankHiddenEnvValues, emptySuccessAsOkay } +function hasControlChars (value) { + return [...value].some((char) => { + const code = char.charCodeAt(0) + return code < 0x20 || code === 0x7f + }) +} + +function decodeSegment (segment) { + try { + return decodeURIComponent(segment) + } catch (err) { + return segment + } +} + +// Every `...Id` argument is interpolated into a URL path, so it must be a single plain segment. +// Returns an error response for the first offender, or null when all are fine. +function validateIdArgs (args) { + for (const [key, value] of Object.entries(args || {})) { + if (/Id$/.test(key) && typeof value === 'string' && !PATH_ID_PATTERN.test(value)) { + return toolError(400, 'invalid_request', `Input validation error: ${key} must contain only letters, digits, hyphen and underscore`) + } + } + return null +} + +// Backstop for values that are not ids (usernames, names, file paths): app.inject() resolves +// dot segments, so a path that reaches the URL as "..", or as "a%2F..%2Fb" once decoded, must +// not get through. File paths legitimately contain "/", which callers percent-encode into one +// segment, so only "..", "." and control characters are refused, never the separator itself. +function unsafePathSegment (url) { + const path = url.split(/[?#]/)[0] + for (const raw of path.split('/')) { + const decoded = decodeSegment(raw) + if (hasControlChars(decoded)) { + return decoded + } + if (decoded === '.' || decoded === '..') { + return decoded + } + if (decoded.split(/[/\\]/).includes('..')) { + return decoded + } + } + return null +} + +// Wraps an inject function so that no request whose path could be re-routed by dot segments +// or control characters reaches the app. +function guardInject (inject) { + return (opts) => { + if (unsafePathSegment(opts.url) !== null) { + return Promise.resolve(toolError(400, 'invalid_request', 'Input validation error: a value used in the request path contains a ".." segment or control characters')) + } + return inject(opts) + } +} + +module.exports = { redactDatabaseCredentials, blankHiddenEnvValues, emptySuccessAsOkay, validateIdArgs, guardInject } diff --git a/test/unit/forge/ee/lib/mcp/tools/pathGuard_spec.js b/test/unit/forge/ee/lib/mcp/tools/pathGuard_spec.js new file mode 100644 index 0000000000..ff592f4a67 --- /dev/null +++ b/test/unit/forge/ee/lib/mcp/tools/pathGuard_spec.js @@ -0,0 +1,119 @@ +const should = require('should') // eslint-disable-line no-unused-vars +const sinon = require('sinon') +const { z } = require('zod') + +const { invokeTool, loadToolDefinitions } = require('../../../../../../../forge/ee/lib/mcp/toolLoader') + +const tools = loadToolDefinitions() + +function getTool (name) { + return tools.find(tool => tool.name === name) +} + +const hostedInstanceId = '11111111-1111-1111-1111-111111111111' +const traversalIds = ['../applications/abc', '../teams/abc', 'abc/snapshots', 'abc?x=1', 'abc#', '..\\teams\\abc', '%2e%2e', 'abc\n'] + +describe('MCP tool path guard', function () { + let inject + + beforeEach(function () { + inject = sinon.stub().resolves({ statusCode: 200, json: () => ({}) }) + }) + + async function assertRejected (toolName, args, field) { + const response = await invokeTool(getTool(toolName), args, { inject }) + inject.called.should.be.false() + response.statusCode.should.equal(400) + response.json().error.should.match(/^Input validation error: /) + if (field) { + response.json().error.should.containEql(field) + } + } + + describe('id arguments', function () { + const cases = [ + ['platform_delete_instance', { instanceType: 'remote', instanceId: '../applications/abc' }, 'instanceId'], + ['platform_get_hosted_instance', { hostedInstanceId: '../applications/abc' }, 'hostedInstanceId'], + ['platform_update_hosted_instance_env', { instanceId: '../teams/abc', env: [] }, 'instanceId'], + ['platform_delete_application', { applicationId: '../teams/abc' }, 'applicationId'], + ['platform_get_application', { applicationId: '../teams/abc' }, 'applicationId'], + ['platform_delete_pipeline', { pipelineId: '../applications/abc' }, 'pipelineId'], + ['platform_get_pipeline_stage', { pipelineId: 'abc', stageId: '../../teams/abc' }, 'stageId'], + ['platform_get_remote_instance', { remoteInstanceId: '../applications/abc' }, 'remoteInstanceId'], + ['platform_get_broker', { teamId: 'abc', brokerId: '../../applications/abc' }, 'brokerId'], + ['platform_get_snapshot', { snapshotId: '../teams/abc' }, 'snapshotId'], + ['platform_delete_snapshot', { snapshotId: '../applications/abc' }, 'snapshotId'], + ['platform_list_instance_snapshots', { instanceType: 'remote', instanceId: '../applications/abc' }, 'instanceId'], + ['platform_get_team', { teamId: '../teams/abc' }, 'teamId'] + ] + + for (const [toolName, args, field] of cases) { + it(`${toolName} rejects ${field} "${Object.values(args).find(v => typeof v === 'string' && v.includes('..'))}"`, async function () { + getTool(toolName).should.be.ok() + await assertRejected(toolName, args, field) + }) + } + + it('rejects other path-breaking values', async function () { + for (const id of traversalIds) { + inject.resetHistory() + await assertRejected('platform_get_snapshot', { snapshotId: id }, 'snapshotId') + } + }) + + it('passes hashids, UUIDs and the team-broker literal through unchanged', async function () { + await invokeTool(getTool('platform_get_snapshot'), { snapshotId: 'abc123' }, { inject }) + inject.calledOnceWith({ method: 'GET', url: '/api/v1/snapshots/abc123' }).should.be.true() + + inject.resetHistory() + await invokeTool(getTool('platform_get_hosted_instance'), { hostedInstanceId }, { inject }) + inject.calledOnceWith({ method: 'GET', url: `/api/v1/projects/${hostedInstanceId}` }).should.be.true() + + inject.resetHistory() + await invokeTool(getTool('platform_get_broker'), { teamId: 'abc', brokerId: 'team-broker' }, { inject }) + inject.calledOnceWith({ method: 'GET', url: '/api/v1/teams/abc/brokers/team-broker' }).should.be.true() + }) + }) + + describe('non-id values', function () { + it('rejects a broker client username that is a ".." segment', async function () { + await assertRejected('platform_get_broker_client', { teamId: 'abc', username: '..' }) + }) + + it('encodes a broker client username containing a slash into one segment', async function () { + await invokeTool(getTool('platform_get_broker_client'), { teamId: 'abc', username: 'a/b' }, { inject }) + inject.calledOnceWith({ method: 'GET', url: '/api/v1/teams/abc/broker/client/a%2Fb' }).should.be.true() + }) + + it('rejects a broker client username that smuggles a ".." segment through encoding', async function () { + await assertRejected('platform_get_broker_client', { teamId: 'abc', username: 'a/../b' }) + }) + + it('rejects a file path with a ".." segment', async function () { + await assertRejected('platform_update_instance_file', { instanceId: hostedInstanceId, path: 'logs/../../etc', newPath: 'x' }) + await assertRejected('platform_list_hosted_instance_files', { hostedInstanceId, path: '..' }) + }) + + it('still accepts file paths containing "/"', async function () { + await invokeTool(getTool('platform_list_hosted_instance_files'), { hostedInstanceId, path: 'logs/archive' }, { inject }) + inject.calledOnceWith({ method: 'GET', url: `/api/v1/projects/${hostedInstanceId}/files/_/logs%2Farchive` }).should.be.true() + }) + }) + + describe('declared id schemas', function () { + it('every string id parameter rejects path traversal at the schema level', function () { + const checked = [] + for (const tool of tools) { + for (const [key, schema] of Object.entries(tool.inputSchema || {})) { + if (!/Id$/.test(key)) { + continue + } + const result = z.object({ [key]: schema }).safeParse({ [key]: '../applications/abc' }) + result.success.should.be.false(`${tool.name}.${key} accepts a traversal id`) + checked.push(`${tool.name}.${key}`) + } + } + checked.length.should.be.greaterThan(50) + }) + }) +})