Skip to content

Commit a574f34

Browse files
Copilotpelikhan
andauthored
Enforce non-empty dispatch_workflow names across safe-output schema and MCP registration (#40315)
* Plan dispatch workflow name validation fix Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Enforce non-empty dispatch workflow names in schema and MCP handlers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent a5c78cd commit a574f34

13 files changed

Lines changed: 243 additions & 13 deletions

.github/workflows/smoke-copilot-aoai-apikey.lock.yml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-copilot-aoai-entra.lock.yml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-copilot-arm.lock.yml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-copilot.lock.yml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/test-dispatcher.lock.yml

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

actions/setup/js/dispatch_workflow.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ async function main(config = {}) {
144144

145145
processedCount++;
146146

147-
const workflowName = message.workflow_name;
147+
const workflowName = typeof message.workflow_name === "string" ? message.workflow_name.trim() : "";
148148

149149
if (!workflowName || workflowName.trim() === "") {
150150
core.warning("Workflow name is empty, skipping");

actions/setup/js/dispatch_workflow.test.cjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,22 @@ describe("dispatch_workflow handler factory", () => {
217217
expect(github.rest.actions.createWorkflowDispatch).not.toHaveBeenCalled();
218218
});
219219

220+
it("should handle whitespace-only workflow name", async () => {
221+
const handler = await main({});
222+
223+
const message = {
224+
type: "dispatch_workflow",
225+
workflow_name: " ",
226+
inputs: {},
227+
};
228+
229+
const result = await handler(message, {});
230+
231+
expect(result.success).toBe(false);
232+
expect(result.error).toContain("empty");
233+
expect(github.rest.actions.createWorkflowDispatch).not.toHaveBeenCalled();
234+
});
235+
220236
it("should handle dispatch errors", async () => {
221237
const handler = await main({
222238
workflows: ["missing-workflow"],

actions/setup/js/safe_output_type_validator.test.cjs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,21 @@ const SAMPLE_VALIDATION_CONFIG = {
149149
body: { required: true, type: "string", sanitize: true, maxLength: 65000, minLength: 20 },
150150
},
151151
},
152+
dispatch_workflow: {
153+
defaultMax: 1,
154+
fields: {
155+
workflow_name: {
156+
required: true,
157+
type: "string",
158+
sanitize: true,
159+
minLength: 1,
160+
maxLength: 256,
161+
pattern: ".*\\S.*",
162+
patternError: "must not be empty",
163+
},
164+
inputs: { type: "object" },
165+
},
166+
},
152167
};
153168

154169
const ISSUE_CLOSING_KEYWORDS = ["fix", "fixes", "fixed", "close", "closes", "closed", "resolve", "resolves", "resolved"];
@@ -210,6 +225,23 @@ describe("safe_output_type_validator", () => {
210225
expect(result.normalizedItem).toBeDefined();
211226
});
212227

228+
it("should validate dispatch_workflow with non-empty workflow_name", async () => {
229+
const { validateItem } = await import("./safe_output_type_validator.cjs");
230+
231+
const result = validateItem({ type: "dispatch_workflow", workflow_name: "haiku-printer", inputs: {} }, "dispatch_workflow", 1);
232+
233+
expect(result.isValid).toBe(true);
234+
});
235+
236+
it("should fail dispatch_workflow when workflow_name is whitespace-only", async () => {
237+
const { validateItem } = await import("./safe_output_type_validator.cjs");
238+
239+
const result = validateItem({ type: "dispatch_workflow", workflow_name: " ", inputs: {} }, "dispatch_workflow", 1);
240+
241+
expect(result.isValid).toBe(false);
242+
expect(result.error).toContain("workflow_name");
243+
});
244+
213245
it("should fail validation when required title is missing", async () => {
214246
const { validateItem } = await import("./safe_output_type_validator.cjs");
215247

actions/setup/js/safe_outputs_mcp_server_http.cjs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ function normalizeMcpToolResult(result) {
6363
return { content, isError };
6464
}
6565

66+
/**
67+
* Check whether workflow metadata name is a non-empty string after trimming.
68+
* @param {any} workflowName
69+
* @returns {boolean}
70+
*/
71+
function hasValidWorkflowMetadataName(workflowName) {
72+
return typeof workflowName === "string" && workflowName.trim().length > 0;
73+
}
74+
6675
/**
6776
* Create and configure the MCP server with tools
6877
* @param {Object} [options] - Additional options
@@ -135,7 +144,7 @@ function createMCPServer(options = {}) {
135144
// Check if this is a dispatch_workflow tool (has _workflow_name metadata)
136145
// These tools are dynamically generated with workflow-specific names
137146
// The _workflow_name should be a non-empty string
138-
const isDispatchWorkflowTool = tool._workflow_name && typeof tool._workflow_name === "string" && tool._workflow_name.length > 0;
147+
const isDispatchWorkflowTool = hasValidWorkflowMetadataName(tool._workflow_name);
139148

140149
// Check if this is a dispatch_repository tool (has _dispatch_repository_tool metadata)
141150
// These tools are dynamically generated with tool-specific names
@@ -144,7 +153,7 @@ function createMCPServer(options = {}) {
144153
// Check if this is a call_workflow tool (has _call_workflow_name metadata)
145154
// These tools are dynamically generated with workflow-specific names
146155
// The _call_workflow_name should be a non-empty string
147-
const isCallWorkflowTool = tool._call_workflow_name && typeof tool._call_workflow_name === "string" && tool._call_workflow_name.length > 0;
156+
const isCallWorkflowTool = hasValidWorkflowMetadataName(tool._call_workflow_name);
148157

149158
if (isDispatchWorkflowTool) {
150159
logger.debug(`Found dispatch_workflow tool: ${tool.name} (_workflow_name: ${tool._workflow_name})`);

actions/setup/js/safe_outputs_mcp_server_http_dispatch.test.cjs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
5454
}
5555

5656
// Test the registration logic (extracted from safe_outputs_mcp_server_http.cjs)
57-
const isDispatchWorkflowTool = !!tool._workflow_name;
57+
const isDispatchWorkflowTool = typeof tool._workflow_name === "string" && tool._workflow_name.trim().length > 0;
5858
let shouldRegister = false;
5959

6060
if (isDispatchWorkflowTool) {
@@ -98,7 +98,7 @@ describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
9898
}
9999
}
100100

101-
const isDispatchWorkflowTool = !!tool._workflow_name;
101+
const isDispatchWorkflowTool = typeof tool._workflow_name === "string" && tool._workflow_name.trim().length > 0;
102102
let shouldRegister = false;
103103

104104
if (isDispatchWorkflowTool) {
@@ -117,6 +117,42 @@ describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
117117
expect(config.dispatch_workflow).toBeFalsy();
118118
});
119119

120+
it("should NOT treat whitespace-only _workflow_name as dispatch_workflow metadata", () => {
121+
const tool = {
122+
name: "test_workflow",
123+
_workflow_name: " ",
124+
description: "Dispatch workflow",
125+
inputSchema: { type: "object", properties: {} },
126+
};
127+
128+
const config = {
129+
dispatch_workflow: {
130+
max: 1,
131+
},
132+
};
133+
134+
const enabledTools = new Set();
135+
for (const [toolName, enabled] of Object.entries(config)) {
136+
if (enabled) {
137+
enabledTools.add(toolName);
138+
}
139+
}
140+
141+
const isDispatchWorkflowTool = typeof tool._workflow_name === "string" && tool._workflow_name.trim().length > 0;
142+
let shouldRegister = false;
143+
144+
if (isDispatchWorkflowTool) {
145+
if (config.dispatch_workflow) {
146+
shouldRegister = true;
147+
}
148+
} else if (enabledTools.has(tool.name)) {
149+
shouldRegister = true;
150+
}
151+
152+
expect(isDispatchWorkflowTool).toBe(false);
153+
expect(shouldRegister).toBe(false);
154+
});
155+
120156
it("should register regular tools when their name is in config", () => {
121157
// Regular tool (no _workflow_name)
122158
const tool = {
@@ -137,7 +173,7 @@ describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
137173
}
138174
}
139175

140-
const isDispatchWorkflowTool = !!tool._workflow_name;
176+
const isDispatchWorkflowTool = typeof tool._workflow_name === "string" && tool._workflow_name.trim().length > 0;
141177
let shouldRegister = false;
142178

143179
if (isDispatchWorkflowTool) {
@@ -175,7 +211,7 @@ describe("safe_outputs_mcp_server_http dispatch_workflow registration", () => {
175211
}
176212
}
177213

178-
const isDispatchWorkflowTool = !!tool._workflow_name;
214+
const isDispatchWorkflowTool = typeof tool._workflow_name === "string" && tool._workflow_name.trim().length > 0;
179215
let shouldRegister = false;
180216

181217
if (isDispatchWorkflowTool) {

0 commit comments

Comments
 (0)