Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 77 additions & 4 deletions actions/setup/js/push_signed_commits.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,79 @@ const fs = require("fs");
const path = require("path");
const { ERR_API } = require("./error_codes.cjs");

/**
* Unescape a C-quoted path returned by `git diff-tree --raw`.
*
* git wraps paths that contain special characters (spaces, non-ASCII bytes,
* control characters, etc.) in double-quotes and encodes each "unusual" byte
* as a C-style escape sequence. This function strips the surrounding quotes
* and decodes the escape sequences back to the original byte sequence, then
* interprets the result as UTF-8.
*
* Supported escape sequences: `\\`, `\"`, `\a`, `\b`, `\f`, `\n`, `\r`,
* `\t`, `\v`, and octal `\NNN` (1–3 octal digits).
*
* @param {string} s - Raw path token from git output (may or may not be quoted)
* @returns {string} Unescaped path
*/
function unquoteCPath(s) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot add test suite for this function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a dedicated unquoteCPath test suite in commit f01052f. The 9 tests cover: plain strings returned unchanged, quote stripping, all standard C escape sequences (\\, \", \a/\b/\f/\n/\r/\t/\v), octal sequences decoding to UTF-8 bytes (e.g. \303\251é), filenames with spaces, unknown escapes (backslash preserved), edge cases (single char, empty string, 1/2/3-digit octals). The function is also now exported from the module to make it directly importable in tests.

if (!s.startsWith('"')) return s;
// Strip surrounding double-quotes
const inner = s.slice(1, s.endsWith('"') ? s.length - 1 : s.length);
const bytes = [];
let i = 0;
while (i < inner.length) {
if (inner[i] === "\\") {
i++;
if (i < inner.length && inner[i] >= "0" && inner[i] <= "7") {
// Octal sequence – collect up to 3 octal digits
let oct = "";
while (i < inner.length && inner[i] >= "0" && inner[i] <= "7" && oct.length < 3) {
oct += inner[i++];
}
bytes.push(parseInt(oct, 8));
} else {
const esc = inner[i++];
switch (esc) {
Comment on lines +41 to +43
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the quoted token is malformed (e.g. starts with " but is missing a closing quote) and the inner string ends with a single trailing backslash, const esc = inner[i++] evaluates to undefined and later esc.charCodeAt(0) will throw. Add an i >= inner.length check after consuming \\ to treat a trailing backslash as a literal instead of crashing.

Copilot uses AI. Check for mistakes.
case "\\":
bytes.push(0x5c);
break;
case '"':
bytes.push(0x22);
break;
case "a":
bytes.push(0x07);
break;
case "b":
bytes.push(0x08);
break;
case "f":
bytes.push(0x0c);
break;
case "n":
bytes.push(0x0a);
break;
case "r":
bytes.push(0x0d);
break;
case "t":
bytes.push(0x09);
break;
case "v":
bytes.push(0x0b);
break;
default:
// Unknown escape: preserve backslash and the character as-is
bytes.push(0x5c, esc.charCodeAt(0));
}
}
} else {
bytes.push(inner.charCodeAt(i++));
}
}
return Buffer.from(bytes).toString("utf8");
}

/**
* @fileoverview Signed Commit Push Helper
*
Expand Down Expand Up @@ -88,13 +161,13 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c
const status = modeFields[4]; // A=Added, M=Modified, D=Deleted, R=Renamed, C=Copied

const paths = line.slice(tabIdx + 1).split("\t");
const filePath = paths[0];
const filePath = unquoteCPath(paths[0]);

if (status === "D") {
deletions.push({ path: filePath });
} else if (status && status.startsWith("R")) {
// Rename: source path is deleted, destination path is added
const renamedPath = paths[1];
const renamedPath = unquoteCPath(paths[1]);
if (!renamedPath) {
core.warning(`pushSignedCommits: rename entry missing destination path, skipping: ${line}`);
Comment on lines 168 to 172
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paths[1] can be undefined for malformed/edge-case git diff-tree --raw output (the code already anticipates this via the if (!renamedPath) guard). Calling unquoteCPath(paths[1]) before checking existence will throw a TypeError and skip the warning/continue behavior. Guard paths[1] before passing it to unquoteCPath (or make unquoteCPath tolerate non-string inputs) so missing destination paths are handled gracefully.

This issue also appears on line 185 of the same file.

Copilot uses AI. Check for mistakes.
continue;
Expand All @@ -111,7 +184,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c
additions.push({ path: renamedPath, contents: content.toString("base64") });
} else if (status && status.startsWith("C")) {
// Copy: source path is kept (no deletion), only the destination path is added
const copiedPath = paths[1];
const copiedPath = unquoteCPath(paths[1]);
if (!copiedPath) {
core.warning(`pushSignedCommits: copy entry missing destination path, skipping: ${line}`);
continue;
Expand Down Expand Up @@ -242,4 +315,4 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c
}
}

module.exports = { pushSignedCommits };
module.exports = { pushSignedCommits, unquoteCPath };
207 changes: 206 additions & 1 deletion actions/setup/js/push_signed_commits.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,71 @@ import os from "os";
const require = createRequire(import.meta.url);

// Import module once – globals are resolved at call time, not import time.
const { pushSignedCommits } = require("./push_signed_commits.cjs");
const { pushSignedCommits, unquoteCPath } = require("./push_signed_commits.cjs");

// ──────────────────────────────────────────────────────────────────────────────
// Unit tests for unquoteCPath
// ──────────────────────────────────────────────────────────────────────────────

describe("unquoteCPath", () => {
it("should return unquoted strings unchanged", () => {
expect(unquoteCPath("simple.txt")).toBe("simple.txt");
expect(unquoteCPath("path/to/file.txt")).toBe("path/to/file.txt");
expect(unquoteCPath("")).toBe("");
});

it("should strip surrounding double-quotes from plain filenames", () => {
expect(unquoteCPath('"hello.txt"')).toBe("hello.txt");
expect(unquoteCPath('"path/to/file.txt"')).toBe("path/to/file.txt");
});

it("should unescape standard C escape sequences", () => {
expect(unquoteCPath('"back\\\\slash"')).toBe("back\\slash");
expect(unquoteCPath('"double\\"quote"')).toBe('double"quote');
expect(unquoteCPath('"new\\nline"')).toBe("new\nline");
expect(unquoteCPath('"tab\\there"')).toBe("tab\there");
expect(unquoteCPath('"carriage\\rreturn"')).toBe("carriage\rreturn");
expect(unquoteCPath('"form\\ffeed"')).toBe("form\ffeed");
expect(unquoteCPath('"bell\\achar"')).toBe("bell\x07char");
expect(unquoteCPath('"back\\bspace"')).toBe("back\bspace");
expect(unquoteCPath('"vertical\\vtab"')).toBe("vertical\x0btab");
});

it("should decode octal sequences as UTF-8 bytes (unicode filenames)", () => {
// é = U+00E9 → UTF-8: 0xC3 0xA9 → octal \303\251
expect(unquoteCPath('"h\\303\\251llo.txt"')).toBe("héllo.txt");
// ö = U+00F6 → UTF-8: 0xC3 0xB6 → octal \303\266
expect(unquoteCPath('"w\\303\\266rld.txt"')).toBe("wörld.txt");
});

it("should decode filenames with spaces (git quotes when core.quotePath=true)", () => {
// git does NOT actually quote spaces alone (only non-ASCII), but the function
// must correctly pass through quoted strings that happen to contain spaces.
expect(unquoteCPath('"hello world.txt"')).toBe("hello world.txt");
});

it("should preserve unknown escape sequences with backslash intact", () => {
// '\x' is not a known escape – backslash is kept
expect(unquoteCPath('"foo\\xbar"')).toBe("foo\\xbar");
});

it("should handle a quoted string with only one character", () => {
expect(unquoteCPath('"a"')).toBe("a");
});

it("should handle a quoted empty string", () => {
expect(unquoteCPath('""')).toBe("");
});

it("should handle 1-, 2-, and 3-digit octal sequences", () => {
// \0 = 0x00 (NUL – unusual but valid)
expect(unquoteCPath('"\\0"')).toBe("\x00");
// \77 = 0x3F = '?'
expect(unquoteCPath('"\\77"')).toBe("?");
// \101 = 0x41 = 'A'
expect(unquoteCPath('"\\101"')).toBe("A");
});
});

// ──────────────────────────────────────────────────────────────────────────────
// Git helpers (real subprocess – no mocking)
Expand Down Expand Up @@ -678,4 +742,145 @@ describe("push_signed_commits integration tests", () => {
expect(mockCore.warning).not.toHaveBeenCalled();
});
});

// ──────────────────────────────────────────────────────
// C-quoted (special character) filenames
// ──────────────────────────────────────────────────────

describe("C-quoted filenames (spaces and unicode)", () => {
it("should handle filenames with spaces", async () => {
execGit(["checkout", "-b", "spaces-branch"], { cwd: workDir });

const spacedName = "hello world.txt";
fs.writeFileSync(path.join(workDir, spacedName), "spaced content\n");
execGit(["add", spacedName], { cwd: workDir });
execGit(["commit", "-m", "Add file with spaces"], { cwd: workDir });
execGit(["push", "-u", "origin", "spaces-branch"], { cwd: workDir });

global.exec = makeRealExec(workDir);
const githubClient = makeMockGithubClient();

await pushSignedCommits({
githubClient,
owner: "test-owner",
repo: "test-repo",
branch: "spaces-branch",
baseRef: "origin/main",
cwd: workDir,
});

expect(githubClient.graphql).toHaveBeenCalledTimes(1);
const callArg = githubClient.graphql.mock.calls[0][1].input;
expect(callArg.fileChanges.additions).toHaveLength(1);
expect(callArg.fileChanges.additions[0].path).toBe(spacedName);
expect(Buffer.from(callArg.fileChanges.additions[0].contents, "base64").toString()).toBe("spaced content\n");
});

it("should handle filenames with unicode characters", async () => {
execGit(["checkout", "-b", "unicode-branch"], { cwd: workDir });

const unicodeName = "héllo_wörld.txt";
fs.writeFileSync(path.join(workDir, unicodeName), "unicode content\n");
execGit(["add", unicodeName], { cwd: workDir });
execGit(["commit", "-m", "Add file with unicode name"], { cwd: workDir });
execGit(["push", "-u", "origin", "unicode-branch"], { cwd: workDir });

global.exec = makeRealExec(workDir);
const githubClient = makeMockGithubClient();

await pushSignedCommits({
githubClient,
owner: "test-owner",
repo: "test-repo",
branch: "unicode-branch",
baseRef: "origin/main",
cwd: workDir,
});

expect(githubClient.graphql).toHaveBeenCalledTimes(1);
const callArg = githubClient.graphql.mock.calls[0][1].input;
expect(callArg.fileChanges.additions).toHaveLength(1);
expect(callArg.fileChanges.additions[0].path).toBe(unicodeName);
expect(Buffer.from(callArg.fileChanges.additions[0].contents, "base64").toString()).toBe("unicode content\n");
});
});

// ──────────────────────────────────────────────────────
// Rename and copy file handling
// ──────────────────────────────────────────────────────

describe("rename and copy file handling", () => {
it("should add old path to deletions and new path to additions on rename", async () => {
execGit(["checkout", "-b", "rename-branch"], { cwd: workDir });

// Add a file that will be renamed
fs.writeFileSync(path.join(workDir, "original.txt"), "rename me\n");
execGit(["add", "original.txt"], { cwd: workDir });
execGit(["commit", "-m", "Add original.txt"], { cwd: workDir });
execGit(["push", "-u", "origin", "rename-branch"], { cwd: workDir });

// Rename the file
fs.renameSync(path.join(workDir, "original.txt"), path.join(workDir, "renamed.txt"));
execGit(["add", "-A"], { cwd: workDir });
execGit(["commit", "-m", "Rename original.txt to renamed.txt"], { cwd: workDir });
execGit(["push", "origin", "rename-branch"], { cwd: workDir });

global.exec = makeRealExec(workDir);
const githubClient = makeMockGithubClient();

await pushSignedCommits({
githubClient,
owner: "test-owner",
repo: "test-repo",
branch: "rename-branch",
baseRef: "rename-branch^",
cwd: workDir,
});

expect(githubClient.graphql).toHaveBeenCalledTimes(1);
const callArg = githubClient.graphql.mock.calls[0][1].input;
// Old path must be deleted, new path must be in additions
expect(callArg.fileChanges.deletions).toEqual([{ path: "original.txt" }]);
expect(callArg.fileChanges.additions).toHaveLength(1);
expect(callArg.fileChanges.additions[0].path).toBe("renamed.txt");
expect(Buffer.from(callArg.fileChanges.additions[0].contents, "base64").toString()).toBe("rename me\n");
});

it("should not add source to deletions on copy (only destination in additions)", async () => {
execGit(["checkout", "-b", "copy-branch"], { cwd: workDir });

// Add a file that will be copied
fs.writeFileSync(path.join(workDir, "source.txt"), "copy source\n");
execGit(["add", "source.txt"], { cwd: workDir });
execGit(["commit", "-m", "Add source.txt"], { cwd: workDir });
execGit(["push", "-u", "origin", "copy-branch"], { cwd: workDir });

// Copy the file (source kept, destination added)
fs.copyFileSync(path.join(workDir, "source.txt"), path.join(workDir, "destination.txt"));
execGit(["add", "destination.txt"], { cwd: workDir });
execGit(["commit", "-m", "Copy source.txt to destination.txt"], { cwd: workDir });
execGit(["push", "origin", "copy-branch"], { cwd: workDir });

global.exec = makeRealExec(workDir);
const githubClient = makeMockGithubClient();

await pushSignedCommits({
githubClient,
owner: "test-owner",
repo: "test-repo",
branch: "copy-branch",
baseRef: "copy-branch^",
cwd: workDir,
});

expect(githubClient.graphql).toHaveBeenCalledTimes(1);
const callArg = githubClient.graphql.mock.calls[0][1].input;
// Source file must NOT appear in deletions
expect(callArg.fileChanges.deletions).toHaveLength(0);
// Destination file must appear in additions
expect(callArg.fileChanges.additions).toHaveLength(1);
expect(callArg.fileChanges.additions[0].path).toBe("destination.txt");
expect(Buffer.from(callArg.fileChanges.additions[0].contents, "base64").toString()).toBe("copy source\n");
});
});
});
Loading