From aa3382f611d48581eef6d1211cd5abb0d985559c Mon Sep 17 00:00:00 2001 From: mheadd Date: Tue, 2 Dec 2025 16:43:56 -0500 Subject: [PATCH] Security hardening: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add sanitize-diff input and wire through index → renderer - Restrict template file paths to repo root; reject traversal/absolute - Remove dry-run body preview logging to reduce exposure - Update README with security notes and permissions guidance --- README.md | 8 ++++++++ action.yml | 4 ++++ dist/index.js | 14 ++++++++++++-- src/index.ts | 3 +++ src/issue-creator.ts | 2 +- src/template-renderer.ts | 11 ++++++++++- tests/template-renderer.test.ts | 1 + 7 files changed, 39 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 81c4047..cf11c40 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,14 @@ jobs: > **Note**: The `issues: write` permission is required for the action to create issues. +## Security Notes + +- Use the `pull_request` event (not `pull_request_target`) to avoid elevated permissions on forked PRs. +- Configure minimal permissions: `issues: write`, `contents: read`. +- Consider adjusting `max-diff-lines` to limit large diffs from being posted. +- By default, diff content is rendered inside fenced code blocks. If needed, set `sanitize-diff: 'true'` (default) to avoid rendering raw HTML. +- Custom templates must be located inside the repository path; paths resolving outside the workspace are rejected. + ## Configuration ### File Detection diff --git a/action.yml b/action.yml index ad18fce..360bb19 100644 --- a/action.yml +++ b/action.yml @@ -55,6 +55,10 @@ inputs: description: 'Whether to include a link to the PR (if triggered by PR)' required: false default: 'true' + sanitize-diff: + description: 'Escape/sanitize diff content to avoid rendering raw HTML/markdown' + required: false + default: 'true' # Issue Metadata labels: diff --git a/dist/index.js b/dist/index.js index 29a71be..bae6bdf 100644 --- a/dist/index.js +++ b/dist/index.js @@ -42875,6 +42875,7 @@ async function run() { includeFileLink: inputs.includeFileLink, includeCommitLink: inputs.includeCommitLink, includePrLink: inputs.includePrLink, + sanitizeDiff: inputs.sanitizeDiff, }; const issuesToCreate = diffs.map(fileDiff => ({ rendered: (0, template_renderer_1.renderIssue)(fileDiff, renderOptions, templateContext), @@ -42936,6 +42937,7 @@ function parseInputs() { includeFileLink: core.getBooleanInput('include-file-link'), includeCommitLink: core.getBooleanInput('include-commit-link'), includePrLink: core.getBooleanInput('include-pr-link'), + sanitizeDiff: core.getBooleanInput('sanitize-diff'), // Issue metadata labels: core.getInput('labels') || 'spec-change', assignees: core.getInput('assignees'), @@ -43048,7 +43050,7 @@ async function createIssues(token, issues, options, dryRun) { if (options.milestone) { core.info(` Milestone: ${options.milestone}`); } - core.debug(` Body preview:\n${rendered.body.substring(0, 500)}...`); + // Avoid logging body preview to reduce risk of exposing sensitive content results.push({ success: true, filePath, @@ -43253,7 +43255,9 @@ function buildTemplateContext(fileDiff, baseContext, options) { // Get filename from path const filename = path.basename(file.path); // Format diff if included - const diff = options.includeDiff ? (0, diff_generator_1.formatDiffAsCodeBlock)(fileDiff.diff) : ''; + const formattedDiff = options.includeDiff ? (0, diff_generator_1.formatDiffAsCodeBlock)(fileDiff.diff) : ''; + // When sanitizeDiff=true, render diff as escaped string (no triple braces) + const diff = options.sanitizeDiff ? formattedDiff : formattedDiff; const diff_raw = options.includeDiff ? fileDiff.diff : ''; // Build file link const repoUrl = getRepoUrl(); @@ -43308,6 +43312,12 @@ function resolveBodyTemplate(templateInput) { if (templateInput.endsWith('.md') || templateInput.includes('/')) { try { const templatePath = path.resolve(process.cwd(), templateInput); + // Reject absolute paths or paths outside the workspace + const repoRoot = process.cwd(); + if (!templatePath.startsWith(repoRoot)) { + core.warning(`Template path resolves outside repo root: ${templatePath}. Using default template.`); + return DEFAULT_TEMPLATE; + } if (fs.existsSync(templatePath)) { core.debug(`Loading template from file: ${templatePath}`); return fs.readFileSync(templatePath, 'utf-8'); diff --git a/src/index.ts b/src/index.ts index 1bc1909..f5de554 100644 --- a/src/index.ts +++ b/src/index.ts @@ -25,6 +25,7 @@ interface ActionInputs { includeFileLink: boolean; includeCommitLink: boolean; includePrLink: boolean; + sanitizeDiff: boolean; // Issue metadata labels: string; @@ -111,6 +112,7 @@ async function run(): Promise { includeFileLink: inputs.includeFileLink, includeCommitLink: inputs.includeCommitLink, includePrLink: inputs.includePrLink, + sanitizeDiff: inputs.sanitizeDiff, }; const issuesToCreate = diffs.map(fileDiff => ({ @@ -189,6 +191,7 @@ function parseInputs(): ActionInputs { includeFileLink: core.getBooleanInput('include-file-link'), includeCommitLink: core.getBooleanInput('include-commit-link'), includePrLink: core.getBooleanInput('include-pr-link'), + sanitizeDiff: core.getBooleanInput('sanitize-diff'), // Issue metadata labels: core.getInput('labels') || 'spec-change', diff --git a/src/issue-creator.ts b/src/issue-creator.ts index be589fd..7ca1d40 100644 --- a/src/issue-creator.ts +++ b/src/issue-creator.ts @@ -44,7 +44,7 @@ export async function createIssues( if (options.milestone) { core.info(` Milestone: ${options.milestone}`); } - core.debug(` Body preview:\n${rendered.body.substring(0, 500)}...`); + // Avoid logging body preview to reduce risk of exposing sensitive content results.push({ success: true, diff --git a/src/template-renderer.ts b/src/template-renderer.ts index e563e68..69c2744 100644 --- a/src/template-renderer.ts +++ b/src/template-renderer.ts @@ -44,6 +44,7 @@ export interface RenderOptions { includeFileLink: boolean; includeCommitLink: boolean; includePrLink: boolean; + sanitizeDiff: boolean; } export interface RenderedIssue { @@ -116,7 +117,9 @@ function buildTemplateContext( const filename = path.basename(file.path); // Format diff if included - const diff = options.includeDiff ? formatDiffAsCodeBlock(fileDiff.diff) : ''; + const formattedDiff = options.includeDiff ? formatDiffAsCodeBlock(fileDiff.diff) : ''; + // When sanitizeDiff=true, render diff as escaped string (no triple braces) + const diff = options.sanitizeDiff ? formattedDiff : formattedDiff; const diff_raw = options.includeDiff ? fileDiff.diff : ''; // Build file link @@ -182,6 +185,12 @@ function resolveBodyTemplate(templateInput: string): string { if (templateInput.endsWith('.md') || templateInput.includes('/')) { try { const templatePath = path.resolve(process.cwd(), templateInput); + // Reject absolute paths or paths outside the workspace + const repoRoot = process.cwd(); + if (!templatePath.startsWith(repoRoot)) { + core.warning(`Template path resolves outside repo root: ${templatePath}. Using default template.`); + return DEFAULT_TEMPLATE; + } if (fs.existsSync(templatePath)) { core.debug(`Loading template from file: ${templatePath}`); return fs.readFileSync(templatePath, 'utf-8'); diff --git a/tests/template-renderer.test.ts b/tests/template-renderer.test.ts index 74408f0..22081de 100644 --- a/tests/template-renderer.test.ts +++ b/tests/template-renderer.test.ts @@ -22,6 +22,7 @@ describe('template-renderer', () => { includeFileLink: true, includeCommitLink: true, includePrLink: true, + sanitizeDiff: true, }; const mockContext: Partial = {