From 30cb5a990136690829390fc02b3a206db1581ab7 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:24:55 +0900 Subject: [PATCH 01/15] chore: add .worktrees/ to .gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 31f7915..011bf7d 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,9 @@ coverage/ docs/codingbuddy/ .omc/ +# Git worktrees +.worktrees/ + # TypeScript build cache *.tsbuildinfo From 4431dd63f2efded1e8f16b6d898a0dc57510b0b2 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:28:00 +0900 Subject: [PATCH 02/15] fix(security): reject NONE in authorized_approvers (H3) --- src/config.test.ts | 19 ++++++++++++++++--- src/config.ts | 6 +++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index 4408390..c907b8a 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -664,7 +664,6 @@ describe("config", () => { "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "MANNEQUIN", - "NONE", ], }; const inputs: ActionInputs = { @@ -685,7 +684,6 @@ describe("config", () => { "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "MANNEQUIN", - "NONE", ]); }); @@ -760,7 +758,22 @@ describe("config", () => { }; expect(() => mergeConfig(fileConfig, inputs)).toThrow( - "Must be one of: OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, MANNEQUIN, NONE", + "Must be one of: OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, MANNEQUIN", + ); + }); + + it("should reject NONE in authorized_approvers", () => { + const fileConfig = { authorized_approvers: ["OWNER", "NONE"] }; + const inputs: ActionInputs = { + mode: "plan", + anthropic_api_key: "test-key", + github_token: "test-token", + config_path: ".leonidas.yml", + system_prompt_path: ".github/leonidas.md", + }; + + expect(() => mergeConfig(fileConfig, inputs)).toThrow( + 'Invalid authorized_approvers value: "NONE" is not allowed', ); }); }); diff --git a/src/config.ts b/src/config.ts index 37e86ff..16e3283 100644 --- a/src/config.ts +++ b/src/config.ts @@ -159,9 +159,13 @@ export function mergeConfig( "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "MANNEQUIN", - "NONE", ]; for (const approver of merged.authorized_approvers) { + if (approver === "NONE") { + throw new Error( + 'Invalid authorized_approvers value: "NONE" is not allowed as it represents unauthenticated users. Use valid associations like OWNER, MEMBER, or COLLABORATOR instead.', + ); + } if (!validAssociations.includes(approver)) { throw new Error( `Invalid authorized_approvers value: "${approver}". Must be one of: ${validAssociations.join(", ")}`, From 231de7c3621c65cfbc002c75459d301c9a273665 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:28:19 +0900 Subject: [PATCH 03/15] fix(security): escape labels/author in execute prompt (H2) --- src/prompts/execute.test.ts | 40 +++++++++++++++++++++++++++++++++ src/prompts/execute.ts | 6 ++--- src/utils/sanitize.test.ts | 45 ++++++++++++++++++++++++++++++++++++- src/utils/sanitize.ts | 11 +++++++++ 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/src/prompts/execute.test.ts b/src/prompts/execute.test.ts index 247e22c..fdbd663 100644 --- a/src/prompts/execute.test.ts +++ b/src/prompts/execute.test.ts @@ -450,5 +450,45 @@ Step 3: Third thing`; expect(result).not.toContain("Dependency:"); }); + + it("should escape shell metacharacters in labels", () => { + const maliciousLabels = ['bug"; rm -rf /', "feature$VAR", "test`whoami`"]; + + const result = buildExecutePrompt({ + ...defaultOptions, + issueLabels: maliciousLabels, + }); + + // Verify escaping is applied + expect(result).toContain('bug\\"; rm -rf /,feature\\$VAR,test\\`whoami\\`'); + // Verify unescaped metacharacters are not present + expect(result).not.toMatch(/gh pr edit --add-label "bug"; rm/); + expect(result).not.toMatch(/feature\$VAR/); + }); + + it("should escape shell metacharacters in author", () => { + const maliciousAuthor = 'user"; curl evil.com; echo "'; + + const result = buildExecutePrompt({ + ...defaultOptions, + issueAuthor: maliciousAuthor, + }); + + // Verify escaping is applied + expect(result).toContain('user\\"; curl evil.com; echo \\"'); + // Verify the command injection is prevented + expect(result).not.toMatch(/gh pr edit --add-assignee "user"; curl/); + }); + + it("should escape both labels and author when both contain metacharacters", () => { + const result = buildExecutePrompt({ + ...defaultOptions, + issueLabels: ['label"1', "label$2"], + issueAuthor: "user`whoami`", + }); + + expect(result).toContain('label\\"1,label\\$2'); + expect(result).toContain("user\\`whoami\\`"); + }); }); }); diff --git a/src/prompts/execute.ts b/src/prompts/execute.ts index a0d8564..6e2b922 100644 --- a/src/prompts/execute.ts +++ b/src/prompts/execute.ts @@ -1,5 +1,5 @@ import { SubIssueMetadata } from "../types"; -import { wrapUserContent, escapeForShellArg } from "../utils/sanitize"; +import { wrapUserContent, escapeForShellArg, escapeArrayForShellArg } from "../utils/sanitize"; export interface ExecutePromptOptions { issueTitle: string; @@ -38,10 +38,10 @@ export function buildExecutePrompt(options: ExecutePromptOptions): string { const prLabels = issueLabels.filter((l) => l !== "leonidas"); const labelCmd = prLabels.length > 0 - ? `\n - Add labels: \`gh pr edit --add-label "${prLabels.join(",")}"\`` + ? `\n - Add labels: \`gh pr edit --add-label "${escapeArrayForShellArg(prLabels)}"\`` : ""; const assigneeCmd = issueAuthor - ? `\n - Add assignee: \`gh pr edit --add-assignee "${issueAuthor}"\`` + ? `\n - Add assignee: \`gh pr edit --add-assignee "${escapeForShellArg(issueAuthor)}"\`` : ""; // Escape shell metacharacters in issueTitle to prevent command injection diff --git a/src/utils/sanitize.test.ts b/src/utils/sanitize.test.ts index a17f917..569252f 100644 --- a/src/utils/sanitize.test.ts +++ b/src/utils/sanitize.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from "vitest"; -import { wrapUserContent, wrapRepoConfiguration, escapeForShellArg } from "./sanitize"; +import { + wrapUserContent, + wrapRepoConfiguration, + escapeForShellArg, + escapeArrayForShellArg, +} from "./sanitize"; describe("wrapUserContent", () => { it("wraps simple content in delimiters", () => { @@ -246,3 +251,41 @@ describe("escapeForShellArg", () => { expect(escapeForShellArg("feat: add login")).toBe("feat: add login"); }); }); + +describe("escapeArrayForShellArg", () => { + it("escapes each element and joins with commas", () => { + const labels = ["bug", "high-priority", "needs-review"]; + const result = escapeArrayForShellArg(labels); + + expect(result).toBe("bug,high-priority,needs-review"); + }); + + it("escapes shell metacharacters in each element", () => { + const labels = ['label"with"quotes', "label$with$dollar", "label`with`backtick"]; + const result = escapeArrayForShellArg(labels); + + expect(result).toBe('label\\"with\\"quotes,label\\$with\\$dollar,label\\`with\\`backtick'); + }); + + it("returns empty string for empty array", () => { + const result = escapeArrayForShellArg([]); + + expect(result).toBe(""); + }); + + it("handles single element array", () => { + const result = escapeArrayForShellArg(["bug"]); + + expect(result).toBe("bug"); + }); + + it("escapes realistic malicious label attempts", () => { + const maliciousLabels = ['"; rm -rf /', "$(whoami)", "`curl evil.com`"]; + const result = escapeArrayForShellArg(maliciousLabels); + + expect(result).not.toMatch(/(? Date: Mon, 16 Feb 2026 16:29:27 +0900 Subject: [PATCH 04/15] fix(security): remove untrusted plan comment fallback (H1) --- src/github.test.ts | 76 +++++++++++++++++++++++++++++++++++----------- src/github.ts | 7 ----- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/github.test.ts b/src/github.test.ts index 050899a..93c51ef 100644 --- a/src/github.test.ts +++ b/src/github.test.ts @@ -51,10 +51,10 @@ describe("github", () => { describe("findPlanComment", () => { it("should return the latest plan comment when multiple exist", async () => { const comments = [ - { id: 1, body: "Regular comment" }, - { id: 2, body: `${PLAN_HEADER}\n\nFirst plan` }, - { id: 3, body: "Another regular comment" }, - { id: 4, body: `${PLAN_HEADER}\n\nLatest plan` }, + { id: 1, body: "Regular comment", user: { login: "github-actions[bot]" } }, + { id: 2, body: `${PLAN_HEADER}\n\nFirst plan`, user: { login: "github-actions[bot]" } }, + { id: 3, body: "Another regular comment", user: { login: "github-actions[bot]" } }, + { id: 4, body: `${PLAN_HEADER}\n\nLatest plan`, user: { login: "github-actions[bot]" } }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -96,8 +96,12 @@ describe("github", () => { it("should handle comments with undefined body", async () => { const comments = [ - { id: 1, body: undefined }, - { id: 2, body: `${PLAN_HEADER}\n\nPlan with content` }, + { id: 1, body: undefined, user: { login: "github-actions[bot]" } }, + { + id: 2, + body: `${PLAN_HEADER}\n\nPlan with content`, + user: { login: "github-actions[bot]" }, + }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -118,7 +122,13 @@ describe("github", () => { }); it("should find plan comment in the middle of body text", async () => { - const comments = [{ id: 1, body: `Some text before\n${PLAN_HEADER}\nSome text after` }]; + const comments = [ + { + id: 1, + body: `Some text before\n${PLAN_HEADER}\nSome text after`, + user: { login: "github-actions[bot]" }, + }, + ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -130,8 +140,12 @@ describe("github", () => { it("should find plan comment by PLAN_MARKER", async () => { const comments = [ - { id: 1, body: "Regular comment" }, - { id: 2, body: `${PLAN_MARKER}\n## Plan\n\nContent here` }, + { id: 1, body: "Regular comment", user: { login: "github-actions[bot]" } }, + { + id: 2, + body: `${PLAN_MARKER}\n## Plan\n\nContent here`, + user: { login: "github-actions[bot]" }, + }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -144,8 +158,12 @@ describe("github", () => { it("should prefer PLAN_MARKER over PLAN_HEADER when both exist", async () => { const comments = [ - { id: 1, body: `${PLAN_HEADER}\n\nOld style plan` }, - { id: 2, body: `${PLAN_MARKER}\n## Plan\n\nNew style plan` }, + { id: 1, body: `${PLAN_HEADER}\n\nOld style plan`, user: { login: "github-actions[bot]" } }, + { + id: 2, + body: `${PLAN_MARKER}\n## Plan\n\nNew style plan`, + user: { login: "github-actions[bot]" }, + }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -158,9 +176,9 @@ describe("github", () => { it("should return latest PLAN_MARKER comment when multiple exist", async () => { const comments = [ - { id: 1, body: `${PLAN_MARKER}\n## Plan\n\nFirst plan` }, - { id: 2, body: "Regular comment" }, - { id: 3, body: `${PLAN_MARKER}\n## Plan\n\nLatest plan` }, + { id: 1, body: `${PLAN_MARKER}\n## Plan\n\nFirst plan`, user: { login: "github-actions[bot]" } }, + { id: 2, body: "Regular comment", user: { login: "someone" } }, + { id: 3, body: `${PLAN_MARKER}\n## Plan\n\nLatest plan`, user: { login: "github-actions[bot]" } }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -173,8 +191,8 @@ describe("github", () => { it("should fallback to PLAN_HEADER when no PLAN_MARKER found", async () => { const comments = [ - { id: 1, body: "Regular comment" }, - { id: 2, body: `${PLAN_HEADER}\n\nPlan without marker` }, + { id: 1, body: "Regular comment", user: { login: "someone" } }, + { id: 2, body: `${PLAN_HEADER}\n\nPlan without marker`, user: { login: "github-actions[bot]" } }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -184,6 +202,28 @@ describe("github", () => { expect(result).toBe(`${PLAN_HEADER}\n\nPlan without marker`); }); + + it("should return null when plan comment is from untrusted user", async () => { + const comments = [ + { + id: 1, + body: `${PLAN_MARKER}\n## Malicious Plan\n\nFake plan from attacker`, + user: { login: "malicious-user" }, + }, + { + id: 2, + body: `${PLAN_HEADER}\n\nAnother fake plan`, + user: { login: "untrusted-actor" }, + }, + ]; + + mockOctokit.paginate.mockResolvedValue(comments); + + const client = createGitHubClient({ token: mockToken, owner: mockOwner, repo: mockRepo }); + const result = await client.findPlanComment(mockIssueNumber); + + expect(result).toBeNull(); + }); }); describe("postComment", () => { @@ -948,8 +988,8 @@ Plan content`; const client = createGitHubClient({ token: mockToken, owner: mockOwner, repo: mockRepo }); const comments = [ - { id: 1, body: "Regular comment" }, - { id: 2, body: `${PLAN_HEADER}\n\nPlan content` }, + { id: 1, body: "Regular comment", user: { login: "someone" } }, + { id: 2, body: `${PLAN_HEADER}\n\nPlan content`, user: { login: "github-actions[bot]" } }, ]; mockOctokit.paginate.mockResolvedValue(comments); diff --git a/src/github.ts b/src/github.ts index b6f1199..14e9db5 100644 --- a/src/github.ts +++ b/src/github.ts @@ -38,13 +38,6 @@ export function createGitHubClient(repo: GitHubRepo) { planComments = trustedComments.filter((comment) => comment.body?.includes(PLAN_HEADER)); } - if (planComments.length === 0) { - planComments = comments.filter((comment) => comment.body?.includes(PLAN_MARKER)); - } - if (planComments.length === 0) { - planComments = comments.filter((comment) => comment.body?.includes(PLAN_HEADER)); - } - if (planComments.length === 0) { return null; } From 747b7682efd26aee25ff8757f0578a1ff9d0267a Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:30:48 +0900 Subject: [PATCH 05/15] fix(security): add newline escaping to escapeForShellArg (M2) --- src/utils/sanitize.test.ts | 6 ++++++ src/utils/sanitize.ts | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/utils/sanitize.test.ts b/src/utils/sanitize.test.ts index 569252f..9b68ffd 100644 --- a/src/utils/sanitize.test.ts +++ b/src/utils/sanitize.test.ts @@ -250,6 +250,12 @@ describe("escapeForShellArg", () => { expect(escapeForShellArg("Fix bug #42")).toBe("Fix bug #42"); expect(escapeForShellArg("feat: add login")).toBe("feat: add login"); }); + + it("should escape newlines and carriage returns", () => { + expect(escapeForShellArg("line1\nline2")).toBe("line1\\nline2"); + expect(escapeForShellArg("line1\rline2")).toBe("line1\\rline2"); + expect(escapeForShellArg("line1\r\nline2")).toBe("line1\\r\\nline2"); + }); }); describe("escapeArrayForShellArg", () => { diff --git a/src/utils/sanitize.ts b/src/utils/sanitize.ts index 8f2ce22..e644f1d 100644 --- a/src/utils/sanitize.ts +++ b/src/utils/sanitize.ts @@ -52,14 +52,17 @@ export function wrapRepoConfiguration(content: string): string { * @returns The shell-safe escaped string */ export function escapeForShellArg(value: string): string { - // Replace double quotes, backticks, dollar signs, backslashes, semicolons, - // and other shell metacharacters that could break out of a quoted string + // Replace double quotes, backticks, dollar signs, backslashes, + // newlines, carriage returns, and other shell metacharacters that could + // break out of a quoted string return value .replace(/\\/g, "\\\\") .replace(/"/g, '\\"') .replace(/`/g, "\\`") .replace(/\$/g, "\\$") - .replace(/!/g, "\\!"); + .replace(/!/g, "\\!") + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r"); } /** From a43e9a5b19bc2f27e7b0995df9be545819dce643 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:31:26 +0900 Subject: [PATCH 06/15] fix(security): replace unsafe type casts with type guards (H4, H5) --- src/main.ts | 17 ++++++++++++++--- src/post_process_cli.test.ts | 8 ++++++-- src/post_process_cli.ts | 20 +++++++++++++++++++- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/main.ts b/src/main.ts index 25cab70..17a5199 100644 --- a/src/main.ts +++ b/src/main.ts @@ -11,9 +11,13 @@ import { createGitHubClient, parseSubIssueMetadata, isDecomposedPlan } from "./g import type { GitHubClient } from "./github"; import { t } from "./i18n"; +function isValidLeonidasMode(value: string): value is LeonidasMode { + return value === "plan" || value === "execute"; +} + export function readInputs(): ActionInputs { const modeRaw = core.getInput("mode", { required: true }); - if (modeRaw !== "plan" && modeRaw !== "execute") { + if (!isValidLeonidasMode(modeRaw)) { throw new Error(`Invalid mode: ${modeRaw}. Must be "plan" or "execute".`); } const mode: LeonidasMode = modeRaw; @@ -27,10 +31,17 @@ export function readInputs(): ActionInputs { } } + const anthropicApiKey = core.getInput("anthropic_api_key", { required: true }); + const githubToken = core.getInput("github_token", { required: true }); + + // Register secrets to prevent accidental logging in workflow output + core.setSecret(anthropicApiKey); + core.setSecret(githubToken); + return { mode, - anthropic_api_key: core.getInput("anthropic_api_key", { required: true }), - github_token: core.getInput("github_token", { required: true }), + anthropic_api_key: anthropicApiKey, + github_token: githubToken, model: core.getInput("model") || undefined, max_turns: maxTurns, allowed_tools: core.getInput("allowed_tools") || undefined, diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index bea77e9..2003033 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -444,13 +444,17 @@ describe("post_process", () => { it("throws error for unknown command", async () => { process.argv = ["node", "post_process.js", "unknown-cmd"]; - await expect(run()).rejects.toThrow("Unknown post-process command: unknown-cmd"); + await expect(run()).rejects.toThrow( + "Invalid post-process command: unknown-cmd. Must be one of: link-subissues, post-completion, post-failure, rescue, post-process-pr, trigger-ci", + ); }); it("throws error when no command provided", async () => { process.argv = ["node", "post_process.js"]; - await expect(run()).rejects.toThrow("Unknown post-process command: undefined"); + await expect(run()).rejects.toThrow( + "Invalid post-process command: undefined. Must be one of: link-subissues, post-completion, post-failure, rescue, post-process-pr, trigger-ci", + ); }); }); }); diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index 890e1f5..da6e571 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -226,8 +226,26 @@ async function runTriggerCI(): Promise { await client.triggerCI(branchName); } +function isValidCommand(value: string): value is Command { + const validCommands: Command[] = [ + "link-subissues", + "post-completion", + "post-failure", + "rescue", + "post-process-pr", + "trigger-ci", + ]; + return validCommands.includes(value as Command); +} + export async function run(): Promise { - const command = process.argv[2] as Command; + const commandRaw = process.argv[2]; + if (!commandRaw || !isValidCommand(commandRaw)) { + throw new Error( + `Invalid post-process command: ${String(commandRaw)}. Must be one of: link-subissues, post-completion, post-failure, rescue, post-process-pr, trigger-ci`, + ); + } + const command: Command = commandRaw; switch (command) { case "link-subissues": return runLinkSubIssues(); From aef50523d0de10239cacd2126c125f9265a3eec0 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:31:26 +0900 Subject: [PATCH 07/15] fix(security): register API key and token with core.setSecret (M1) --- src/main.test.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main.test.ts b/src/main.test.ts index a7ab39d..c2ef348 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import * as core from "@actions/core"; import * as fs from "fs"; import * as os from "os"; -import { run } from "./main"; +import { run, readInputs } from "./main"; vi.mock("@actions/core"); vi.mock("fs"); @@ -391,6 +391,26 @@ describe("main", () => { }); }); + describe("readInputs() - secrets handling", () => { + it("should register API key and GitHub token as secrets", () => { + vi.mocked(core.getInput).mockImplementation((name: string) => { + const inputs: Record = { + mode: "plan", + anthropic_api_key: "test-api-key", + github_token: "test-github-token", + }; + return inputs[name] || ""; + }); + vi.mocked(core.setSecret).mockImplementation(() => {}); + + readInputs(); + + expect(core.setSecret).toHaveBeenCalledWith("test-api-key"); + expect(core.setSecret).toHaveBeenCalledWith("test-github-token"); + expect(core.setSecret).toHaveBeenCalledTimes(2); + }); + }); + describe("run() - error handling", () => { it("should catch and report errors via setFailed", async () => { vi.mocked(core.getInput).mockImplementation(() => { From 3ef7d79c47f987526d93ec4044e4bdae17e1765e Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:32:35 +0900 Subject: [PATCH 08/15] refactor: use path.join and add coverage to ESLint ignores (M6, M12) --- eslint.config.mjs | 2 +- src/config.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 29691e3..80f9190 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -6,7 +6,7 @@ import prettier from "eslint-config-prettier"; export default tseslint.config( { - ignores: ["dist/**", "node_modules/**", "*.config.{js,mjs,ts}"], + ignores: ["dist/**", "node_modules/**", "coverage/**", "*.config.{js,mjs,ts}"], }, eslint.configs.recommended, ...tseslint.configs.recommendedTypeChecked, diff --git a/src/config.ts b/src/config.ts index 16e3283..b034e9c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,5 +1,6 @@ import * as core from "@actions/core"; import * as fs from "fs"; +import * as path from "path"; import * as yaml from "js-yaml"; import { LeonidasConfig, ActionInputs } from "./types"; import { resolveLanguage } from "./i18n"; @@ -194,7 +195,7 @@ export function loadRules(rulesPath: string): Record { const rules: Record = {}; for (const file of mdFiles) { - const filePath = `${rulesPath}/${file}`; + const filePath = path.join(rulesPath, file); const ruleName = file.replace(/\.md$/, ""); try { const content = fs.readFileSync(filePath, "utf-8"); From b019ae2c7603dea2c151a60a9c889e559a0958b6 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:33:12 +0900 Subject: [PATCH 09/15] perf: cache Octokit instances by token (M5) --- src/github.test.ts | 59 ++++++++++++++++++++++++++++++++++++++++++---- src/github.ts | 14 ++++++++++- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/github.test.ts b/src/github.test.ts index 93c51ef..c41a1db 100644 --- a/src/github.test.ts +++ b/src/github.test.ts @@ -1,7 +1,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import * as github from "@actions/github"; import * as core from "@actions/core"; -import { parseSubIssueMetadata, isDecomposedPlan, createGitHubClient } from "./github"; +import { + parseSubIssueMetadata, + isDecomposedPlan, + createGitHubClient, + clearOctokitCache, +} from "./github"; import { PLAN_HEADER, PLAN_MARKER, DECOMPOSED_MARKER } from "./templates/plan_comment"; vi.mock("@actions/core"); @@ -17,6 +22,7 @@ describe("github", () => { beforeEach(() => { vi.clearAllMocks(); + clearOctokitCache(); mockOctokit = { paginate: vi.fn(), @@ -176,9 +182,17 @@ describe("github", () => { it("should return latest PLAN_MARKER comment when multiple exist", async () => { const comments = [ - { id: 1, body: `${PLAN_MARKER}\n## Plan\n\nFirst plan`, user: { login: "github-actions[bot]" } }, + { + id: 1, + body: `${PLAN_MARKER}\n## Plan\n\nFirst plan`, + user: { login: "github-actions[bot]" }, + }, { id: 2, body: "Regular comment", user: { login: "someone" } }, - { id: 3, body: `${PLAN_MARKER}\n## Plan\n\nLatest plan`, user: { login: "github-actions[bot]" } }, + { + id: 3, + body: `${PLAN_MARKER}\n## Plan\n\nLatest plan`, + user: { login: "github-actions[bot]" }, + }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -192,7 +206,11 @@ describe("github", () => { it("should fallback to PLAN_HEADER when no PLAN_MARKER found", async () => { const comments = [ { id: 1, body: "Regular comment", user: { login: "someone" } }, - { id: 2, body: `${PLAN_HEADER}\n\nPlan without marker`, user: { login: "github-actions[bot]" } }, + { + id: 2, + body: `${PLAN_HEADER}\n\nPlan without marker`, + user: { login: "github-actions[bot]" }, + }, ]; mockOctokit.paginate.mockResolvedValue(comments); @@ -966,6 +984,39 @@ Plan content`; }); }); + describe("createGitHubClient - caching", () => { + it("should cache Octokit instances by token", () => { + // This test needs isolated mock state + vi.clearAllMocks(); + + const client1 = createGitHubClient({ token: mockToken, owner: mockOwner, repo: mockRepo }); + const client2 = createGitHubClient({ token: mockToken, owner: mockOwner, repo: mockRepo }); + + // Should only call getOctokit once for the same token + expect(github.getOctokit).toHaveBeenCalledWith(mockToken); + expect(github.getOctokit).toHaveBeenCalledTimes(1); + + // Both clients should have all expected methods + expect(client1).toHaveProperty("findPlanComment"); + expect(client2).toHaveProperty("findPlanComment"); + }); + + it("should create separate Octokit instances for different tokens", () => { + vi.clearAllMocks(); + + const token1 = "ghp_token_1"; + const token2 = "ghp_token_2"; + + createGitHubClient({ token: token1, owner: mockOwner, repo: mockRepo }); + createGitHubClient({ token: token2, owner: mockOwner, repo: mockRepo }); + + // Should call getOctokit twice for different tokens + expect(github.getOctokit).toHaveBeenCalledWith(token1); + expect(github.getOctokit).toHaveBeenCalledWith(token2); + expect(github.getOctokit).toHaveBeenCalledTimes(2); + }); + }); + describe("createGitHubClient", () => { it("should create a client that shares a single Octokit instance", () => { const client = createGitHubClient({ token: mockToken, owner: mockOwner, repo: mockRepo }); diff --git a/src/github.ts b/src/github.ts index 14e9db5..84a8916 100644 --- a/src/github.ts +++ b/src/github.ts @@ -8,6 +8,14 @@ import { SubIssueMetadata, GitHubRepo } from "./types"; // Other bot identifiers may be added for GitHub App installations const TRUSTED_BOT_AUTHORS = new Set(["github-actions[bot]"]); +// Cache for Octokit instances keyed by token +const octokitCache = new Map>(); + +// For testing: clear the Octokit cache +export function clearOctokitCache(): void { + octokitCache.clear(); +} + export interface LinkSubIssuesResult { linked: number; failed: number; @@ -16,7 +24,11 @@ export interface LinkSubIssuesResult { export type GitHubClient = ReturnType; export function createGitHubClient(repo: GitHubRepo) { - const octokit = github.getOctokit(repo.token); + let octokit = octokitCache.get(repo.token); + if (!octokit) { + octokit = github.getOctokit(repo.token); + octokitCache.set(repo.token, octokit); + } const { owner, repo: repoName } = repo; return { From 69b88f45f150fd0bb3e8ddcc1cc512d8ed609e27 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:33:40 +0900 Subject: [PATCH 10/15] fix: extract magic number, validate parseRepo, restrict temp file perms (L1, L4, L5) --- src/main.test.ts | 4 ++-- src/main.ts | 2 +- src/post_process_cli.test.ts | 31 +++++++++++++++++++++++++++++++ src/post_process_cli.ts | 5 +++++ src/prompts/execute.ts | 8 +++++--- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/main.test.ts b/src/main.test.ts index c2ef348..543a999 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -110,7 +110,7 @@ describe("main", () => { expect(fs.writeFileSync).toHaveBeenCalledWith( expect.stringContaining("leonidas-prompt-"), "plan prompt content", - "utf-8", + { encoding: "utf-8", mode: 0o600 }, ); }); @@ -166,7 +166,7 @@ describe("main", () => { expect(fs.writeFileSync).toHaveBeenCalledWith( expect.stringMatching(/^\/runner\/tmp\/leonidas-prompt-\d+\.md$/), "prompt content", - "utf-8", + { encoding: "utf-8", mode: 0o600 }, ); delete process.env.RUNNER_TEMP; diff --git a/src/main.ts b/src/main.ts index 17a5199..72172a2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -270,7 +270,7 @@ export async function run(): Promise { // Prefer RUNNER_TEMP (cleaned per-job by GitHub Actions) over os.tmpdir() const tmpDir = process.env.RUNNER_TEMP ?? os.tmpdir(); const promptFile = path.join(tmpDir, `leonidas-prompt-${Date.now()}.md`); - fs.writeFileSync(promptFile, prompt, "utf-8"); + fs.writeFileSync(promptFile, prompt, { encoding: "utf-8", mode: 0o600 }); // Set outputs for composite action core.setOutput("prompt_file", promptFile); diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index 2003033..8c5861b 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -94,6 +94,37 @@ describe("post_process", () => { repo: "my-awesome-repo", }); }); + + it("throws on empty string", () => { + expect(() => parseRepo("")).toThrow( + 'Invalid repository format: "". Expected "owner/repo".', + ); + }); + + it("throws on missing separator", () => { + expect(() => parseRepo("just-a-name")).toThrow( + 'Invalid repository format: "just-a-name". Expected "owner/repo".', + ); + }); + + it("throws on empty owner", () => { + expect(() => parseRepo("/repo")).toThrow( + 'Invalid repository format: "/repo". Expected "owner/repo".', + ); + }); + + it("throws on empty repo name", () => { + expect(() => parseRepo("owner/")).toThrow( + 'Invalid repository format: "owner/". Expected "owner/repo".', + ); + }); + + it("handles valid octocat/hello-world format", () => { + expect(parseRepo("octocat/hello-world")).toEqual({ + owner: "octocat", + repo: "hello-world", + }); + }); }); describe("run — link-subissues command", () => { diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index da6e571..50359a1 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -45,6 +45,11 @@ export function getEnvOptional(name: string): string | undefined { export function parseRepo(repo: string): { owner: string; repo: string } { const [owner, name] = repo.split("/"); + if (!owner || !name) { + throw new Error( + `Invalid repository format: "${repo}". Expected "owner/repo".`, + ); + } return { owner, repo: name }; } diff --git a/src/prompts/execute.ts b/src/prompts/execute.ts index 6e2b922..c664e7b 100644 --- a/src/prompts/execute.ts +++ b/src/prompts/execute.ts @@ -1,6 +1,9 @@ import { SubIssueMetadata } from "../types"; import { wrapUserContent, escapeForShellArg, escapeArrayForShellArg } from "../utils/sanitize"; +/** Turns reserved for final push + PR creation workflow */ +const RESERVED_TURNS = 5; + export interface ExecutePromptOptions { issueTitle: string; issueBody: string; @@ -32,8 +35,7 @@ export function buildExecutePrompt(options: ExecutePromptOptions): string { hasRules = false, } = options; const branchName = `${branchPrefix}${issueNumber}`; - const reservedTurns = 5; - const pushDeadline = maxTurns - reservedTurns; + const pushDeadline = maxTurns - RESERVED_TURNS; const prLabels = issueLabels.filter((l) => l !== "leonidas"); const labelCmd = @@ -89,7 +91,7 @@ ${safePlan} ## Turn Budget -You have **${maxTurns} turns** total. Reserve the last ${reservedTurns} turns for push + PR creation. +You have **${maxTurns} turns** total. Reserve the last ${RESERVED_TURNS} turns for push + PR creation. - **Push deadline:** By turn ${pushDeadline}, you MUST have pushed your branch. - **Strategy:** Push early and create a draft PR after completing 2-3 implementation steps. From 1879988aaa35fb58c1aedf85fe432509d358349b Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:38:58 +0900 Subject: [PATCH 11/15] style: apply prettier formatting --- src/post_process_cli.test.ts | 4 +--- src/post_process_cli.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index 8c5861b..956287b 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -96,9 +96,7 @@ describe("post_process", () => { }); it("throws on empty string", () => { - expect(() => parseRepo("")).toThrow( - 'Invalid repository format: "". Expected "owner/repo".', - ); + expect(() => parseRepo("")).toThrow('Invalid repository format: "". Expected "owner/repo".'); }); it("throws on missing separator", () => { diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index 50359a1..7fe3df3 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -46,9 +46,7 @@ export function getEnvOptional(name: string): string | undefined { export function parseRepo(repo: string): { owner: string; repo: string } { const [owner, name] = repo.split("/"); if (!owner || !name) { - throw new Error( - `Invalid repository format: "${repo}". Expected "owner/repo".`, - ); + throw new Error(`Invalid repository format: "${repo}". Expected "owner/repo".`); } return { owner, repo: name }; } From 311bdb683649d29c16a0069dcbfc30af286f5515 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:53:23 +0900 Subject: [PATCH 12/15] fix(security): validate MODE env var in post_process_cli (E1) --- src/post_process_cli.test.ts | 14 ++++++++++++++ src/post_process_cli.ts | 7 ++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index 956287b..a6ba052 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -305,6 +305,20 @@ describe("post_process", () => { runUrl: "https://example.com/run", }); }); + + it("throws error when MODE is invalid", async () => { + process.argv = ["node", "post_process.js", "post-failure"]; + process.env.GH_TOKEN = "ghp_test"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.LANGUAGE = "en"; + process.env.RUN_URL = "https://example.com/run"; + process.env.MODE = "invalid"; + + await expect(run()).rejects.toThrow( + 'Invalid MODE: "invalid". Must be "plan" or "execute".', + ); + }); }); describe("run — rescue command", () => { diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index 7fe3df3..1955aed 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -53,6 +53,7 @@ export function parseRepo(repo: string): { owner: string; repo: string } { export function readBaseContext(): PostProcessContext { const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("GITHUB_REPOSITORY")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const language = resolveLanguage(getEnvOptional("LANGUAGE")); @@ -122,7 +123,11 @@ async function runPostFailure(): Promise { const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const language = resolveLanguage(getEnvOptional("LANGUAGE")); const runUrl = getEnvRequired("RUN_URL"); - const mode = getEnvRequired("MODE") as LeonidasMode; + const modeRaw = getEnvRequired("MODE"); + if (modeRaw !== "plan" && modeRaw !== "execute") { + throw new Error(`Invalid MODE: "${modeRaw}". Must be "plan" or "execute".`); + } + const mode: LeonidasMode = modeRaw; const client = createGitHubClient({ token, owner, repo }); From 31693b0bf86b2acb4c894167f1aa73d195b42547 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:56:07 +0900 Subject: [PATCH 13/15] fix(security): register GH_TOKEN with core.setSecret in post_process_cli (E2) --- src/post_process_cli.test.ts | 105 ++++++++++++++++++++++++++++++++++- src/post_process_cli.ts | 5 ++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index a6ba052..b7ba58c 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -126,6 +126,21 @@ describe("post_process", () => { }); describe("run — link-subissues command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "link-subissues"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.REPO = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + + const mockClient = createMockClient(); + mockClient.findPlanComment.mockResolvedValue(null); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("calls findPlanComment and linkSubIssues when decomposed plan found", async () => { process.argv = ["node", "post_process.js", "link-subissues"]; process.env.GH_TOKEN = "ghp_test"; @@ -208,6 +223,25 @@ describe("post_process", () => { }); describe("run — post-completion command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "post-completion"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.LANGUAGE = "en"; + process.env.BRANCH_PREFIX = "leonidas/issue-"; + process.env.RUN_URL = "https://example.com/run"; + + const mockClient = createMockClient(); + mockClient.getPRForBranch.mockResolvedValue(undefined); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + vi.mocked(postProcessingModule.buildCompletionComment).mockReturnValue("completion msg"); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("posts completion comment with PR number when PR exists", async () => { process.argv = ["node", "post_process.js", "post-completion"]; process.env.GH_TOKEN = "ghp_test"; @@ -261,6 +295,24 @@ describe("post_process", () => { }); describe("run — post-failure command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "post-failure"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.LANGUAGE = "en"; + process.env.RUN_URL = "https://example.com/run"; + process.env.MODE = "plan"; + + const mockClient = createMockClient(); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + vi.mocked(postProcessingModule.buildFailureComment).mockReturnValue("failure msg"); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("posts failure comment for plan mode", async () => { process.argv = ["node", "post_process.js", "post-failure"]; process.env.GH_TOKEN = "ghp_test"; @@ -315,13 +367,30 @@ describe("post_process", () => { process.env.RUN_URL = "https://example.com/run"; process.env.MODE = "invalid"; - await expect(run()).rejects.toThrow( - 'Invalid MODE: "invalid". Must be "plan" or "execute".', - ); + await expect(run()).rejects.toThrow('Invalid MODE: "invalid". Must be "plan" or "execute".'); }); }); describe("run — rescue command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "rescue"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.BRANCH_PREFIX = "leonidas/issue-"; + process.env.BASE_BRANCH = "main"; + process.env.LANGUAGE = "en"; + process.env.RUN_URL = "https://example.com/run"; + + const mockClient = createMockClient(); + mockClient.branchExistsOnRemote.mockResolvedValue(false); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("sets branch_exists=false and skips when branch does not exist", async () => { process.argv = ["node", "post_process.js", "rescue"]; process.env.GH_TOKEN = "ghp_test"; @@ -450,6 +519,21 @@ describe("post_process", () => { }); describe("run — post-process-pr command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "post-process-pr"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.BRANCH_PREFIX = "leonidas/issue-"; + + const mockClient = createMockClient(); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("delegates to postProcessPR with correct arguments", async () => { process.argv = ["node", "post_process.js", "post-process-pr"]; process.env.GH_TOKEN = "ghp_test"; @@ -467,6 +551,21 @@ describe("post_process", () => { }); describe("run — trigger-ci command", () => { + it("registers GH_TOKEN as secret", async () => { + process.argv = ["node", "post_process.js", "trigger-ci"]; + process.env.GH_TOKEN = "ghp_test_secret_token"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + process.env.ISSUE_NUMBER = "42"; + process.env.BRANCH_PREFIX = "leonidas/issue-"; + + const mockClient = createMockClient(); + vi.mocked(githubModule.createGitHubClient).mockReturnValue(mockClient as any); + + await run(); + + expect(core.setSecret).toHaveBeenCalledWith("ghp_test_secret_token"); + }); + it("delegates to triggerCI with constructed branch name", async () => { process.argv = ["node", "post_process.js", "trigger-ci"]; process.env.GH_TOKEN = "ghp_test"; diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index 1955aed..b6a2f84 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -75,6 +75,7 @@ async function runLinkSubIssues(): Promise { // Note: This command uses REPO instead of GITHUB_REPOSITORY and doesn't need // language, branchPrefix, or runUrl, so it reads env vars directly const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("REPO")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); @@ -119,6 +120,7 @@ async function runPostCompletion(): Promise { async function runPostFailure(): Promise { const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("GITHUB_REPOSITORY")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const language = resolveLanguage(getEnvOptional("LANGUAGE")); @@ -142,6 +144,7 @@ async function runPostFailure(): Promise { async function runRescue(): Promise { const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("GITHUB_REPOSITORY")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const branchPrefix = getEnvRequired("BRANCH_PREFIX"); @@ -215,6 +218,7 @@ async function runRescue(): Promise { async function runPostProcessPR(): Promise { const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("GITHUB_REPOSITORY")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const branchPrefix = getEnvRequired("BRANCH_PREFIX"); @@ -225,6 +229,7 @@ async function runPostProcessPR(): Promise { async function runTriggerCI(): Promise { const token = getEnvRequired("GH_TOKEN"); + core.setSecret(token); const { owner, repo } = parseRepo(getEnvRequired("GITHUB_REPOSITORY")); const issueNumber = parseInt(getEnvRequired("ISSUE_NUMBER"), 10); const branchPrefix = getEnvRequired("BRANCH_PREFIX"); From e64375235a2e671cd22bfbcc55c398f9f2da96da Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 16:59:19 +0900 Subject: [PATCH 14/15] fix: escape single-quotes, strip control chars, strict parseRepo, cleanup (E3-E6) --- src/utils/sanitize.test.ts | 10 ++++++++++ src/utils/sanitize.ts | 28 ++++++++++++++++++---------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/utils/sanitize.test.ts b/src/utils/sanitize.test.ts index 9b68ffd..5dd8692 100644 --- a/src/utils/sanitize.test.ts +++ b/src/utils/sanitize.test.ts @@ -256,6 +256,16 @@ describe("escapeForShellArg", () => { expect(escapeForShellArg("line1\rline2")).toBe("line1\\rline2"); expect(escapeForShellArg("line1\r\nline2")).toBe("line1\\r\\nline2"); }); + + it("should escape single quotes", () => { + expect(escapeForShellArg("it's")).toBe("it\\'s"); + }); + + it("should strip control characters", () => { + expect(escapeForShellArg("hello\x00world")).toBe("helloworld"); + expect(escapeForShellArg("tab\there")).toBe("tabhere"); + expect(escapeForShellArg("escape\x1bhere")).toBe("escapehere"); + }); }); describe("escapeArrayForShellArg", () => { diff --git a/src/utils/sanitize.ts b/src/utils/sanitize.ts index e644f1d..947c2b6 100644 --- a/src/utils/sanitize.ts +++ b/src/utils/sanitize.ts @@ -48,21 +48,29 @@ export function wrapRepoConfiguration(content: string): string { * This prevents command injection when user-controlled values (like issue titles) * are embedded in shell commands that the LLM is instructed to execute. * + * Escapes double quotes, single quotes, backticks, dollar signs, backslashes, + * exclamation marks, newlines, and carriage returns. Also strips control characters. + * * @param value - The value to escape for shell interpolation * @returns The shell-safe escaped string */ export function escapeForShellArg(value: string): string { - // Replace double quotes, backticks, dollar signs, backslashes, + // Replace double quotes, single quotes, backticks, dollar signs, backslashes, // newlines, carriage returns, and other shell metacharacters that could - // break out of a quoted string - return value - .replace(/\\/g, "\\\\") - .replace(/"/g, '\\"') - .replace(/`/g, "\\`") - .replace(/\$/g, "\\$") - .replace(/!/g, "\\!") - .replace(/\n/g, "\\n") - .replace(/\r/g, "\\r"); + // break out of a quoted string, then strip control characters + return ( + value + .replace(/\\/g, "\\\\") + .replace(/"/g, '\\"') + .replace(/'/g, "\\'") + .replace(/`/g, "\\`") + .replace(/\$/g, "\\$") + .replace(/!/g, "\\!") + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r") + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x09\x0b\x0c\x0e-\x1f\x7f]/g, "") + ); } /** From 7565ef31b52a2b6f5100fbf22b1a32370441b214 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Mon, 16 Feb 2026 17:14:47 +0900 Subject: [PATCH 15/15] fix: strict parseRepo validation to reject extra path segments (E4) --- src/post_process_cli.test.ts | 6 ++++++ src/post_process_cli.ts | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/post_process_cli.test.ts b/src/post_process_cli.test.ts index b7ba58c..7b37e72 100644 --- a/src/post_process_cli.test.ts +++ b/src/post_process_cli.test.ts @@ -117,6 +117,12 @@ describe("post_process", () => { ); }); + it("throws on extra path segments", () => { + expect(() => parseRepo("owner/repo/extra")).toThrow( + 'Invalid repository format: "owner/repo/extra". Expected "owner/repo".', + ); + }); + it("handles valid octocat/hello-world format", () => { expect(parseRepo("octocat/hello-world")).toEqual({ owner: "octocat", diff --git a/src/post_process_cli.ts b/src/post_process_cli.ts index b6a2f84..c88f5ae 100644 --- a/src/post_process_cli.ts +++ b/src/post_process_cli.ts @@ -44,11 +44,11 @@ export function getEnvOptional(name: string): string | undefined { } export function parseRepo(repo: string): { owner: string; repo: string } { - const [owner, name] = repo.split("/"); - if (!owner || !name) { + const parts = repo.split("/"); + if (parts.length !== 2 || !parts[0] || !parts[1]) { throw new Error(`Invalid repository format: "${repo}". Expected "owner/repo".`); } - return { owner, repo: name }; + return { owner: parts[0], repo: parts[1] }; } export function readBaseContext(): PostProcessContext {