diff --git a/setup/js/mcp_scripts_mcp_server_http.cjs b/setup/js/mcp_scripts_mcp_server_http.cjs index 3da51e2..b61e7b0 100644 --- a/setup/js/mcp_scripts_mcp_server_http.cjs +++ b/setup/js/mcp_scripts_mcp_server_http.cjs @@ -24,7 +24,7 @@ require("./shim.cjs"); const { randomUUID } = require("crypto"); const { MCPServer, MCPHTTPTransport } = require("./mcp_http_transport.cjs"); -const { validateRequiredFields } = require("./mcp_scripts_validation.cjs"); +const { validateRequiredFields, validateStringInputLengths } = require("./mcp_scripts_validation.cjs"); const { generateEnhancedErrorMessage } = require("./mcp_enhanced_errors.cjs"); const { createLogger } = require("./mcp_logger.cjs"); const { bootstrapMCPScriptsServer, cleanupConfigFile } = require("./mcp_scripts_bootstrap.cjs"); @@ -94,6 +94,13 @@ function createMCPServer(configPath, options = {}) { throw new Error(generateEnhancedErrorMessage(missing, tool.name, tool.inputSchema)); } + // SM-IS-01: Validate per-string input length limits (10 KB max per string parameter). + const oversized = validateStringInputLengths(args, tool.inputSchema); + if (oversized.length) { + const details = oversized.map(v => `'${v.field}' (${v.byteLength} bytes)`).join(", "); + throw new Error(`Input string parameter(s) exceed the 10 KB limit for tool '${tool.name}': ${details}`); + } + // Call the handler const result = await Promise.resolve(tool.handler(args)); logger.debug(`Handler returned for tool: ${tool.name}`); diff --git a/setup/js/mcp_scripts_validation.cjs b/setup/js/mcp_scripts_validation.cjs index 7acbe27..2f76f2b 100644 --- a/setup/js/mcp_scripts_validation.cjs +++ b/setup/js/mcp_scripts_validation.cjs @@ -6,6 +6,12 @@ * This module provides validation utilities for mcp-scripts MCP server. */ +/** + * Maximum allowed byte length for any single string-typed input parameter (SM-IS-01). + * 10 KB = 10 * 1024 bytes. + */ +const MAX_STRING_INPUT_BYTES = 10 * 1024; + /** * Validate required fields in tool arguments * @param {Object} args - The arguments object to validate @@ -27,6 +33,48 @@ function validateRequiredFields(args, inputSchema) { return missing; } +/** + * Validate that no string-typed input parameter exceeds the maximum allowed byte length (SM-IS-01). + * Implementations MUST enforce a maximum input string length of at least 10KB for each + * string-typed input parameter. Inputs exceeding the configured maximum MUST be rejected with a + * validation error before the tool script is invoked. Implementations MUST NOT silently truncate + * oversized inputs. + * + * Scope: validates only top-level (direct) properties of the schema where `type === "string"`. + * Nested object/array schemas are not recursively validated, consistent with the SM-IS-01 + * requirement that applies to "input parameters" (top-level tool arguments). + * + * @param {Object} args - The arguments object to validate + * @param {Object} inputSchema - The input schema describing property types + * @param {number} [maxBytes] - Maximum allowed bytes per string (defaults to MAX_STRING_INPUT_BYTES) + * @returns {{ field: string, byteLength: number }[]} Array of violations (empty if all within limit) + */ +function validateStringInputLengths(args, inputSchema, maxBytes) { + const limit = typeof maxBytes === "number" ? maxBytes : MAX_STRING_INPUT_BYTES; + const properties = inputSchema && inputSchema.properties ? inputSchema.properties : {}; + const violations = []; + + for (const [field, schema] of Object.entries(properties)) { + if (schema && schema.type === "string") { + // Skip fields with an explicit maxLength — handler-level validation enforces their limit. + if (typeof schema.maxLength === "number") { + continue; + } + const value = args[field]; + if (typeof value === "string") { + const byteLength = Buffer.byteLength(value, "utf8"); + if (byteLength > limit) { + violations.push({ field, byteLength }); + } + } + } + } + + return violations; +} + module.exports = { validateRequiredFields, + validateStringInputLengths, + MAX_STRING_INPUT_BYTES, }; diff --git a/setup/js/mcp_server_core.cjs b/setup/js/mcp_server_core.cjs index 05b9653..6f40fe7 100644 --- a/setup/js/mcp_server_core.cjs +++ b/setup/js/mcp_server_core.cjs @@ -31,7 +31,7 @@ const fs = require("fs"); const path = require("path"); const { ReadBuffer } = require("./read_buffer.cjs"); -const { validateRequiredFields } = require("./mcp_scripts_validation.cjs"); +const { validateRequiredFields, validateStringInputLengths } = require("./mcp_scripts_validation.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { generateEnhancedErrorMessage } = require("./mcp_enhanced_errors.cjs"); @@ -660,6 +660,16 @@ async function handleRequest(server, request, defaultHandler) { }; } + // SM-IS-01: Validate per-string input length limits (10 KB max per string parameter). + const oversizedFields = validateStringInputLengths(args, tool.inputSchema); + if (oversizedFields.length) { + const details = oversizedFields.map(v => `'${v.field}' (${v.byteLength} bytes)`).join(", "); + throw { + code: -32602, + message: `Input string parameter(s) exceed the 10 KB limit for tool '${name}': ${details}`, + }; + } + // Call handler and await the result (supports both sync and async handlers) const handlerResult = await Promise.resolve(handler(args)); const content = handlerResult && handlerResult.content ? handlerResult.content : []; @@ -799,6 +809,14 @@ async function handleMessage(server, req, defaultHandler) { return; } + // SM-IS-01: Validate per-string input length limits (10 KB max per string parameter). + const oversized = validateStringInputLengths(args, tool.inputSchema); + if (oversized.length) { + const details = oversized.map(v => `'${v.field}' (${v.byteLength} bytes)`).join(", "); + server.replyError(id, -32602, `Input string parameter(s) exceed the 10 KB limit for tool '${name}': ${details}`); + return; + } + // Call handler and await the result (supports both sync and async handlers) server.debug(`Calling handler for tool: ${name}`); const result = await Promise.resolve(handler(args)); diff --git a/setup/js/push_signed_commits.cjs b/setup/js/push_signed_commits.cjs index 2b89cef..dbf3e0f 100644 --- a/setup/js/push_signed_commits.cjs +++ b/setup/js/push_signed_commits.cjs @@ -9,6 +9,7 @@ const { ERR_API } = require("./error_codes.cjs"); const { loadTemporaryIdMapFromResolved, replaceTemporaryIdReferencesInPatch, TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN } = require("./temporary_id.cjs"); +const OID_PATTERN = /^[0-9a-f]{40}$/i; /** Sentinel error class used to signal that the commit range contains a shape * that the GitHub GraphQL `createCommitOnBranch` mutation cannot represent @@ -258,13 +259,45 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c } } + /** @type {string | undefined} */ + let baseRefOid; + try { + const { stdout: baseRefOut } = await exec.getExecOutput("git", ["rev-parse", `${baseRef}^{commit}`], { cwd }); + const trimmedBaseRefOid = baseRefOut.trim(); + if (OID_PATTERN.test(trimmedBaseRefOid)) { + baseRefOid = trimmedBaseRefOid; + } else if (trimmedBaseRefOid) { + core.warning( + `pushSignedCommits: git rev-parse returned an unexpected baseRef OID value for '${baseRef}'; ` + + `boundary-commit filter is disabled for this run. Check that '${baseRef}' resolves to a valid commit in this checkout. ` + + `Observed value: ${JSON.stringify(trimmedBaseRefOid)}` + ); + } + } catch (baseRefResolveError) { + core.warning( + `pushSignedCommits: could not resolve baseRef '${baseRef}' to OID; boundary-commit filter is disabled for this run and parent OID resolution may fall back to per-commit rev-parse: ${baseRefResolveError instanceof Error ? baseRefResolveError.message : String(baseRefResolveError)}` + ); + } // Collect the commits introduced (oldest-first) using topological order to ensure // correct sequencing even when commit dates are out of sync (e.g. after rebase --committer-date-is-author-date). // Using --parents emits each line as " [ ...]", which lets us detect merge commits // (more than one parent) in a single subprocess call without iterating each SHA individually. - const { stdout: revListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${baseRef}..HEAD`], { cwd }); - const revListLines = revListOut.trim().split("\n").filter(Boolean); - const shas = revListLines.map(line => line.split(" ")[0]); + const revListBase = baseRefOid ?? baseRef; + const { stdout: revListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${revListBase}..HEAD`], { cwd }); + const revListEntriesRaw = revListOut + .trim() + .split("\n") + .filter(Boolean) + .map(line => { + const fields = line.split(" "); + return { line, fields, sha: fields[0] }; + }); + const revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw; + const droppedBoundaryCount = revListEntriesRaw.length - revListEntries.length; + if (baseRefOid !== undefined && droppedBoundaryCount > 0) { + core.info(`pushSignedCommits: dropped ${droppedBoundaryCount} baseRef boundary commit(s) from replay set`); + } + const shas = revListEntries.map(entry => entry.sha); if (shas.length === 0) { core.info("pushSignedCommits: no new commits to push via GraphQL"); @@ -278,8 +311,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // A line with 3+ space-separated fields means the commit has 2+ parents (i.e. a merge commit). // The GitHub GraphQL createCommitOnBranch mutation does not support multiple parents, so refuse // the unsigned push fallback if any merge commit is found. - for (const line of revListLines) { - const fields = line.split(" "); + for (const { fields } of revListEntries) { if (fields.length > 2) { const sha = fields[0]; core.warning(`pushSignedCommits: merge commit ${sha} detected, refusing unsigned push fallback`); @@ -426,8 +458,13 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // Resolve the parent OID, create the branch on the remote via the REST API, // then proceed with the signed-commit mutation as normal. core.info(`pushSignedCommits: branch ${branch} not yet on the remote, resolving parent OID for first commit`); - const { stdout: parentOut } = await exec.getExecOutput("git", ["rev-parse", `${sha}^`], { cwd }); - expectedHeadOid = parentOut.trim(); + if (baseRefOid !== undefined) { + expectedHeadOid = baseRefOid; + core.info(`pushSignedCommits: using baseRef OID for initial branch creation: ${expectedHeadOid}`); + } else { + const { stdout: parentOut } = await exec.getExecOutput("git", ["rev-parse", `${sha}^`], { cwd }); + expectedHeadOid = parentOut.trim(); + } if (!expectedHeadOid) { throw new Error(`${ERR_API}: Could not resolve OID for new branch ${branch}`); } @@ -449,6 +486,15 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // GitHub returns 422 "Reference refs/heads/ already exists". Treat that as success and continue. if (status === 422 && /reference.*already exists/i.test(message)) { core.info(`pushSignedCommits: remote branch ${branch} was created concurrently (422 Reference already exists); continuing with signed commits`); + const { stdout: refreshedOidOut } = await exec.getExecOutput("git", ["ls-remote", "origin", `refs/heads/${branch}`], { cwd, env: { ...process.env, ...(gitAuthEnv || {}) } }); + const refreshedHeadOid = refreshedOidOut.trim().split(/\s+/)[0]; + if (!refreshedHeadOid) { + throw new Error(`${ERR_API}: Could not resolve remote branch OID for ${branch} after concurrent creation; ls-remote output was ${JSON.stringify(refreshedOidOut)}`); + } + if (!OID_PATTERN.test(refreshedHeadOid)) { + throw new Error(`${ERR_API}: Invalid remote branch OID for ${branch} after concurrent creation; ls-remote output was ${JSON.stringify(refreshedOidOut)}`); + } + expectedHeadOid = refreshedHeadOid; } else { throw createRefError; } diff --git a/setup/js/run_operation_update_upgrade.cjs b/setup/js/run_operation_update_upgrade.cjs index 1da45d2..8b66d75 100644 --- a/setup/js/run_operation_update_upgrade.cjs +++ b/setup/js/run_operation_update_upgrade.cjs @@ -16,6 +16,7 @@ const KNOWN_FILES_UPDATE = [".github/aw/actions-lock.json"]; */ const KNOWN_FILES_UPGRADE = [ ".github/aw/actions-lock.json", + ".github/skills/agentic-workflows/SKILL.md", ".github/agents/agentic-workflows.agent.md", // Old agent files that may be deleted by deleteOldAgentFiles: ".github/agents/create-agentic-workflow.agent.md", diff --git a/setup/js/safe_outputs_tools.json b/setup/js/safe_outputs_tools.json index 27c34bd..c98e375 100644 --- a/setup/js/safe_outputs_tools.json +++ b/setup/js/safe_outputs_tools.json @@ -252,6 +252,7 @@ "properties": { "body": { "type": "string", + "maxLength": 65536, "description": "The comment text in Markdown format. Must be the final intended comment \u2014 not a placeholder or test value. This is the 'body' field - do not use 'comment_body' or other variations. Provide helpful, relevant information that adds value to the conversation." }, "item_number": { diff --git a/setup/md/agent_failure_issue.md b/setup/md/agent_failure_issue.md index 5bac3ff..7127586 100644 --- a/setup/md/agent_failure_issue.md +++ b/setup/md/agent_failure_issue.md @@ -29,7 +29,7 @@ The failed workflow run is at {run_url} Debug this workflow failure using your favorite Agent CLI and the `agentic-workflows` prompt. - Start your agent -- Load the `agentic-workflows` prompt from `.github/agents/agentic-workflows.agent.md` or +- Load the `agentic-workflows` skill from `.github/skills/agentic-workflows/SKILL.md` or - Type `debug the agentic workflow {workflow_id} failure in {run_url}` diff --git a/setup/md/mcp_cli_tools_prompt.md b/setup/md/mcp_cli_tools_prompt.md index c78a617..fa454e8 100644 --- a/setup/md/mcp_cli_tools_prompt.md +++ b/setup/md/mcp_cli_tools_prompt.md @@ -29,7 +29,7 @@ playwright browser_snapshot # capture page accessibility **Example** — using the `safeoutputs` CLI (safe outputs) when you are ready to emit the final real action: ```bash -safeoutputs add_comment --issue_number 42 --body "Analysis complete" +safeoutputs add_comment --item_number 42 --body "Analysis complete" ``` **Example** — using the `mcpscripts` CLI (mcp-scripts): @@ -44,7 +44,7 @@ mcpscripts mcpscripts-gh --args "pr list --repo owner/repo --limit 5" ```bash # Full argument payload as JSON via printf pipe -printf '{"issue_number":42,"body":"### Title\n\nBody paragraph one.\n\nBody paragraph two."}' \ +printf '{"item_number":42,"body":"### Title\n\nBody paragraph one.\n\nBody paragraph two."}' \ | safeoutputs add_comment . # Works with any tool — just match the parameter names from --help diff --git a/setup/md/missing_data_issue.md b/setup/md/missing_data_issue.md index 08fb654..ca85f3b 100644 --- a/setup/md/missing_data_issue.md +++ b/setup/md/missing_data_issue.md @@ -32,7 +32,7 @@ To help the agent succeed, please: **Steps:** -1. Invoke agent: `/agent agentic-workflows` +1. Invoke skill: `agentic-workflows` 2. Command: "Debug this missing data issue" 3. Analyze what data the agent needs and why 4. Determine the appropriate solution: diff --git a/setup/md/missing_tool_issue.md b/setup/md/missing_tool_issue.md index 837a677..23840a3 100644 --- a/setup/md/missing_tool_issue.md +++ b/setup/md/missing_tool_issue.md @@ -20,7 +20,7 @@ Please investigate why these tools are missing and either: **Steps:** -1. Invoke agent: `/agent agentic-workflows` +1. Invoke skill: `agentic-workflows` 2. Command: "Debug this missing tool issue" 3. Analyze which tools are missing and why they're needed 4. Determine the appropriate solution: