diff --git a/.github/workflows/mcp-inspector.lock.yml b/.github/workflows/mcp-inspector.lock.yml index a6c476a93e1..ad1d4a64f61 100644 --- a/.github/workflows/mcp-inspector.lock.yml +++ b/.github/workflows/mcp-inspector.lock.yml @@ -996,7 +996,7 @@ jobs: "url": "https://mcp.datadoghq.com/api/unstable/mcp-server/mcp?toolsets=core", "headers": { "DD_API_KEY": "\${DD_API_KEY}", - "DD_APPLICATION_KEY": "\${DD_APP_KEY}", + "DD_APPLICATION_KEY": "\${DD_APPLICATION_KEY}", "DD_SITE": "\${DD_SITE}" }, "tools": [ diff --git a/.github/workflows/smoke-otel-backends.lock.yml b/.github/workflows/smoke-otel-backends.lock.yml index 878ea3a0c79..59f8342c061 100644 --- a/.github/workflows/smoke-otel-backends.lock.yml +++ b/.github/workflows/smoke-otel-backends.lock.yml @@ -769,7 +769,7 @@ jobs: "url": "https://mcp.datadoghq.com/api/unstable/mcp-server/mcp?toolsets=core", "headers": { "DD_API_KEY": "\${DD_API_KEY}", - "DD_APPLICATION_KEY": "\${DD_APP_KEY}", + "DD_APPLICATION_KEY": "\${DD_APPLICATION_KEY}", "DD_SITE": "\${DD_SITE}" }, "tools": [ diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 3807b012bd0..a1d0453844c 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -1507,6 +1507,8 @@ async function main(config = {}) { baseRef: `origin/${baseBranch}`, cwd: process.cwd(), signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, }); core.info("Changes pushed to branch (from bundle)"); @@ -1537,6 +1539,8 @@ async function main(config = {}) { baseRef: `origin/${baseBranch}`, cwd: process.cwd(), signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, }); core.info("Changes pushed to branch after bundle rewrite retry"); @@ -1765,6 +1769,8 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo baseRef: `origin/${baseBranch}`, cwd: process.cwd(), signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, }); core.info("Changes pushed to branch"); @@ -1911,6 +1917,8 @@ ${patchPreview}`; baseRef: `origin/${baseBranch}`, cwd: process.cwd(), signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, }); core.info("Empty branch pushed successfully"); diff --git a/actions/setup/js/push_signed_commits.cjs b/actions/setup/js/push_signed_commits.cjs index 31643b5d842..ed6168544a7 100644 --- a/actions/setup/js/push_signed_commits.cjs +++ b/actions/setup/js/push_signed_commits.cjs @@ -8,6 +8,7 @@ */ const { ERR_API } = require("./error_codes.cjs"); +const { loadTemporaryIdMapFromResolved, replaceTemporaryIdReferencesInPatch, TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN } = require("./temporary_id.cjs"); /** Sentinel error class used to signal that the commit range contains a shape * that the GitHub GraphQL `createCommitOnBranch` mutation cannot represent @@ -124,6 +125,45 @@ async function readBlobAsBase64(blobHash, cwd) { return Buffer.concat(chunks).toString("base64"); } +/** + * Replace temporary ID references in base64-encoded UTF-8 text content. + * Returns original content unchanged for: + * - binary / non-UTF8 blobs + * - UTF-8 text with no temporary ID matches + * Returns rewritten base64 content when UTF-8 text contains resolvable temporary IDs. + * + * @param {string} base64Content + * @param {Map} temporaryIdMap + * @param {string} currentRepo + * @param {string} filePath + * @returns {string} + */ +function maybeReplaceTemporaryIdsInBase64Content(base64Content, temporaryIdMap, currentRepo, filePath) { + if (!(temporaryIdMap instanceof Map) || temporaryIdMap.size === 0) { + return base64Content; + } + + const rawBytes = Buffer.from(base64Content, "base64"); + const utf8Text = rawBytes.toString("utf8"); + + // Treat only clean UTF-8 round-trippable content as text. + if (!Buffer.from(utf8Text, "utf8").equals(rawBytes)) { + return base64Content; + } + + if (!TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN.test(utf8Text)) { + return base64Content; + } + + const replaced = replaceTemporaryIdReferencesInPatch(utf8Text, temporaryIdMap, currentRepo); + if (replaced === utf8Text) { + return base64Content; + } + + core.info(`pushSignedCommits: resolved temporary ID references in file content: ${filePath}`); + return Buffer.from(replaced, "utf8").toString("base64"); +} + /** * Push the local branch to origin using git directly and return the local HEAD * SHA after the push succeeds. @@ -167,9 +207,20 @@ async function resolveLocalHeadSha(cwd) { * @param {string} opts.cwd - Working directory of the local git checkout * @param {object} [opts.gitAuthEnv] - Environment variables for git push fallback auth * @param {boolean} [opts.signedCommits=true] - When false, skip GraphQL signed commits and use git push directly + * @param {Record} [opts.resolvedTemporaryIds] - Resolved temporary IDs map + * @param {string} [opts.currentRepo] - Repository slug used for same-repo temporary ID resolution * @returns {Promise} SHA of the commit that landed on the target branch */ -async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true }) { +async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true, resolvedTemporaryIds, currentRepo }) { + const effectiveCurrentRepo = currentRepo || `${owner}/${repo}`; + const temporaryIdMap = loadTemporaryIdMapFromResolved(resolvedTemporaryIds, { + defaultRepo: effectiveCurrentRepo, + validatePositiveIntegers: true, + onInvalidNumber: (normalizedKey, rawValue) => { + core.warning(`pushSignedCommits: ignoring invalid resolved temporary ID number for '${normalizedKey}': ${String(rawValue)}`); + }, + }); + // The default parameter value converts undefined to true; this check tests only the explicit false value. if (signedCommits === false) { core.info(`pushSignedCommits: signed-commits disabled (using direct git push) for branch ${branch}`); @@ -303,7 +354,8 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${renamedPath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - additions.push({ path: renamedPath, contents: await readBlobAsBase64(dstHash, cwd) }); + const blobContents = await readBlobAsBase64(dstHash, cwd); + additions.push({ path: renamedPath, contents: maybeReplaceTemporaryIdsInBase64Content(blobContents, temporaryIdMap, effectiveCurrentRepo, renamedPath) }); } else if (status && status.startsWith("C")) { // Copy: source path is kept (no deletion), only the destination path is added const copiedPath = unquoteCPath(paths[1]); @@ -322,7 +374,8 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${copiedPath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - additions.push({ path: copiedPath, contents: await readBlobAsBase64(dstHash, cwd) }); + const blobContents = await readBlobAsBase64(dstHash, cwd); + additions.push({ path: copiedPath, contents: maybeReplaceTemporaryIdsInBase64Content(blobContents, temporaryIdMap, effectiveCurrentRepo, copiedPath) }); } else { // Added or Modified if (dstMode === "160000") { @@ -336,7 +389,8 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${filePath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - additions.push({ path: filePath, contents: await readBlobAsBase64(dstHash, cwd) }); + const blobContents = await readBlobAsBase64(dstHash, cwd); + additions.push({ path: filePath, contents: maybeReplaceTemporaryIdsInBase64Content(blobContents, temporaryIdMap, effectiveCurrentRepo, filePath) }); } } diff --git a/actions/setup/js/push_signed_commits.test.cjs b/actions/setup/js/push_signed_commits.test.cjs index f07d65bd27b..0edea1900c9 100644 --- a/actions/setup/js/push_signed_commits.test.cjs +++ b/actions/setup/js/push_signed_commits.test.cjs @@ -307,6 +307,99 @@ describe("push_signed_commits integration tests", () => { expect(Buffer.from(variables.input.fileChanges.additions[0].contents, "base64").toString()).toBe("Hello World\n"); }); + it("should resolve temporary ID references in text file contents before GraphQL replay", async () => { + execGit(["checkout", "-b", "temp-id-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "quarantine.cs"), '[QuarantinedTest("https://github.com/test-owner/test-repo/issues/#aw_test1")]\n// linked: #aw_test1\n'); + execGit(["add", "quarantine.cs"], { cwd: workDir }); + execGit(["commit", "-m", "Add quarantine reference"], { cwd: workDir }); + execGit(["push", "-u", "origin", "temp-id-branch"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "temp-id-branch", + baseRef: "origin/main", + cwd: workDir, + resolvedTemporaryIds: { + aw_test1: { repo: "test-owner/test-repo", number: 66708 }, + }, + currentRepo: "test-owner/test-repo", + }); + + expect(githubClient.graphql).toHaveBeenCalledTimes(1); + const additions = githubClient.graphql.mock.calls[0][1].input.fileChanges.additions; + expect(additions).toHaveLength(1); + const resolvedContent = Buffer.from(additions[0].contents, "base64").toString(); + expect(resolvedContent).toContain("https://github.com/test-owner/test-repo/issues/66708"); + expect(resolvedContent).toContain("#66708"); + expect(resolvedContent).not.toContain("#aw_test1"); + }); + + it("should still run replacement logic for malformed temporary ID candidates and emit warning", async () => { + execGit(["checkout", "-b", "temp-id-malformed-candidate-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "quarantine.cs"), "// malformed link: #aw_test-id\n"); + execGit(["add", "quarantine.cs"], { cwd: workDir }); + execGit(["commit", "-m", "Add malformed temporary ID reference"], { cwd: workDir }); + execGit(["push", "-u", "origin", "temp-id-malformed-candidate-branch"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "temp-id-malformed-candidate-branch", + baseRef: "origin/main", + cwd: workDir, + resolvedTemporaryIds: { + aw_test: { repo: "test-owner/test-repo", number: 66708 }, + }, + currentRepo: "test-owner/test-repo", + }); + + expect(githubClient.graphql).toHaveBeenCalledTimes(1); + const additions = githubClient.graphql.mock.calls[0][1].input.fileChanges.additions; + const replayedContent = Buffer.from(additions[0].contents, "base64").toString(); + expect(replayedContent).toContain("#66708-id"); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Malformed temporary ID reference '#aw_test-id'")); + }); + + it("should ignore invalid resolved temporary ID numbers instead of replacing with NaN", async () => { + execGit(["checkout", "-b", "temp-id-invalid-number-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "quarantine.cs"), "// linked: #aw_test2\n"); + execGit(["add", "quarantine.cs"], { cwd: workDir }); + execGit(["commit", "-m", "Add temporary ID with invalid map entry"], { cwd: workDir }); + execGit(["push", "-u", "origin", "temp-id-invalid-number-branch"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "temp-id-invalid-number-branch", + baseRef: "origin/main", + cwd: workDir, + resolvedTemporaryIds: { + aw_test2: { repo: "test-owner/test-repo", number: "not-a-number" }, + }, + currentRepo: "test-owner/test-repo", + }); + + expect(githubClient.graphql).toHaveBeenCalledTimes(1); + const additions = githubClient.graphql.mock.calls[0][1].input.fileChanges.additions; + const replayedContent = Buffer.from(additions[0].contents, "base64").toString(); + expect(replayedContent).toContain("#aw_test2"); + expect(replayedContent).not.toContain("#NaN"); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("ignoring invalid resolved temporary ID number for 'aw_test2'")); + }); + it("should call GraphQL once per commit for multiple new commits", async () => { execGit(["checkout", "-b", "multi-commit-branch"], { cwd: workDir }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index c4a71de9aa0..af649afc76b 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -921,6 +921,8 @@ async function main(config = {}) { cwd: repoCwd || process.cwd(), gitAuthEnv, signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, }); if (pushedSha) { pushedCommitSha = pushedSha; diff --git a/actions/setup/js/temporary_id.cjs b/actions/setup/js/temporary_id.cjs index 04a517093a7..4fb10083e67 100644 --- a/actions/setup/js/temporary_id.cjs +++ b/actions/setup/js/temporary_id.cjs @@ -37,6 +37,12 @@ const TEMPORARY_ID_PATTERN = /#(aw_[A-Za-z0-9_]{3,12})\b/gi; */ const TEMPORARY_ID_CANDIDATE_PATTERN = /#aw_([A-Za-z0-9_-]+)/gi; +/** + * Regex pattern for quick candidate detection of temporary ID references. + * Non-global on purpose so repeated `.test()` calls are state-free. + */ +const TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN = /#aw_/i; + /** * @typedef {Object} RepoIssuePair * @property {string} repo - Repository slug in "owner/repo" format @@ -256,9 +262,13 @@ function loadTemporaryIdMap() { * - { repo, number } * * @param {any} resolvedTemporaryIds - Object or Map of temporary IDs to resolved values + * @param {object} [options] + * @param {string} [options.defaultRepo] - Fallback repo for legacy number-only values; when null/undefined, uses GitHub Action context repo when available, else "" + * @param {boolean} [options.validatePositiveIntegers] - When true, ignore non-positive-integer numbers + * @param {(normalizedKey: string, rawValue: unknown) => void} [options.onInvalidNumber] - Callback invoked when a value is skipped for non-finite parsing, or for non-positive/non-integer values when `validatePositiveIntegers` is true * @returns {Map} Map of normalized temporary_id to {repo, number} */ -function loadTemporaryIdMapFromResolved(resolvedTemporaryIds) { +function loadTemporaryIdMapFromResolved(resolvedTemporaryIds, options = {}) { /** @type {Map} */ const result = new Map(); @@ -266,22 +276,59 @@ function loadTemporaryIdMapFromResolved(resolvedTemporaryIds) { return result; } - const contextRepo = typeof context !== "undefined" ? `${context.repo.owner}/${context.repo.repo}` : ""; + const contextRepo = options.defaultRepo ?? (typeof context !== "undefined" ? `${context.repo.owner}/${context.repo.repo}` : ""); + + /** + * @param {string} normalizedKey + * @param {unknown} rawValue + * @returns {number | null} + */ + const toNumber = (normalizedKey, rawValue) => { + const number = Number(rawValue); + if (!Number.isFinite(number)) { + if (typeof options.onInvalidNumber === "function") { + options.onInvalidNumber(normalizedKey, rawValue); + } + return null; + } + if (!options.validatePositiveIntegers) { + return number; + } + if (!Number.isInteger(number) || number < 1) { + if (typeof options.onInvalidNumber === "function") { + options.onInvalidNumber(normalizedKey, rawValue); + } + return null; + } + return number; + }; const entries = resolvedTemporaryIds instanceof Map ? Array.from(resolvedTemporaryIds.entries()) : Object.entries(resolvedTemporaryIds); for (const [key, value] of entries) { const normalizedKey = normalizeTemporaryId(key); if (typeof value === "number") { - result.set(normalizedKey, { repo: contextRepo, number: value }); + const number = toNumber(normalizedKey, value); + if (number === null) { + continue; + } + result.set(normalizedKey, { repo: contextRepo, number }); continue; } if (typeof value === "object" && value !== null) { if ("repo" in value && "number" in value) { - result.set(normalizedKey, { repo: String(value.repo), number: Number(value.number) }); + const number = toNumber(normalizedKey, value.number); + if (number === null) { + continue; + } + result.set(normalizedKey, { repo: String(value.repo), number }); continue; } if ("number" in value) { - result.set(normalizedKey, { repo: contextRepo, number: Number(value.number) }); + const number = toNumber(normalizedKey, value.number); + if (number === null) { + continue; + } + result.set(normalizedKey, { repo: contextRepo, number }); continue; } } @@ -673,6 +720,7 @@ function resolveNumberFromTemporaryId(value, resolvedTemporaryIds) { module.exports = { TEMPORARY_ID_PATTERN, TEMPORARY_ID_CANDIDATE_PATTERN, + TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN, generateTemporaryId, isTemporaryId, normalizeTemporaryId,