From 61affd7498dcad562bb381980426f609be71a9e7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 01:57:15 +0000 Subject: [PATCH 1/7] fix: validate reviewAgentLogPath to prevent path injection Add path validation to invokeDiffReviewLlm to ensure the log path stays within the expected DATA_CACHE_DIR/review-agent directory. This addresses CodeQL alert #19 (js/path-injection) by resolving the path and verifying it does not escape the log directory. The validation is performed before each fs.appendFileSync call to prevent path traversal attacks even if the call chain changes in the future. Co-authored-by: Michael Sukkarieh --- .../agents/review-agent/nodes/invokeDiffReviewLlm.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts index f3f41be80..5cd27cdbd 100644 --- a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts +++ b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts @@ -2,10 +2,20 @@ import OpenAI from "openai"; import { sourcebot_file_diff_review, sourcebot_file_diff_review_schema } from "@/features/agents/review-agent/types"; import { env } from "@sourcebot/shared"; import fs from "fs"; +import path from "path"; import { createLogger } from "@sourcebot/shared"; const logger = createLogger('invoke-diff-review-llm'); +const REVIEW_AGENT_LOG_BASE = path.join(env.DATA_CACHE_DIR, 'review-agent'); + +const validateLogPath = (logPath: string): void => { + const resolved = path.resolve(logPath); + if (!resolved.startsWith(REVIEW_AGENT_LOG_BASE + path.sep)) { + throw new Error('reviewAgentLogPath escapes log directory'); + } +}; + export const invokeDiffReviewLlm = async (reviewAgentLogPath: string | undefined, prompt: string): Promise => { logger.debug("Executing invoke_diff_review_llm"); @@ -19,6 +29,7 @@ export const invokeDiffReviewLlm = async (reviewAgentLogPath: string | undefined }); if (reviewAgentLogPath) { + validateLogPath(reviewAgentLogPath); fs.appendFileSync(reviewAgentLogPath, `\n\nPrompt:\n${prompt}`); } @@ -32,6 +43,7 @@ export const invokeDiffReviewLlm = async (reviewAgentLogPath: string | undefined const openaiResponse = completion.choices[0].message.content; if (reviewAgentLogPath) { + validateLogPath(reviewAgentLogPath); fs.appendFileSync(reviewAgentLogPath, `\n\nResponse:\n${openaiResponse}`); } From 87894f8cc10f6b0dae7bce0eaf6982ab53566d4b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 01:57:49 +0000 Subject: [PATCH 2/7] chore: add CHANGELOG entry for path injection fix Co-authored-by: Michael Sukkarieh --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 454eebecd..3b95a96a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed revision selection so the 64-revision cap prefers the newest matching branches and tags instead of pruning by ref-name order. [#1122](https://github.com/sourcebot-dev/sourcebot/pull/1122) - Fixed infinite pagination loop in Gitea/Forgejo when an API token can only see a subset of org repos (the `x-total-count` header reports org total while token returns fewer items). [#1130](https://github.com/sourcebot-dev/sourcebot/pull/1130) +- Fixed path injection vulnerability (CodeQL js/path-injection) in review agent log writing by validating paths stay within the expected log directory. [#1134](https://github.com/sourcebot-dev/sourcebot/pull/1134) ## [4.16.11] - 2026-04-17 From 9b6e216457c92a1ff279d10c18f0c6eca53f9781 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 02:03:34 +0000 Subject: [PATCH 3/7] refactor: share REVIEW_AGENT_LOG_DIR constant between app.ts and invokeDiffReviewLlm.ts Export the log directory constant from invokeDiffReviewLlm.ts and import it in app.ts to ensure a single source of truth for the review agent log directory path. Co-authored-by: Michael Sukkarieh --- packages/web/src/features/agents/review-agent/app.ts | 8 ++++---- .../agents/review-agent/nodes/invokeDiffReviewLlm.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index 80d5a2f38..1e82eec93 100644 --- a/packages/web/src/features/agents/review-agent/app.ts +++ b/packages/web/src/features/agents/review-agent/app.ts @@ -2,6 +2,7 @@ import { Octokit } from "octokit"; import { generatePrReviews } from "@/features/agents/review-agent/nodes/generatePrReview"; import { githubPushPrReviews } from "@/features/agents/review-agent/nodes/githubPushPrReviews"; import { githubPrParser } from "@/features/agents/review-agent/nodes/githubPrParser"; +import { REVIEW_AGENT_LOG_DIR } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; import { env } from "@sourcebot/shared"; import { GitHubPullRequest } from "@/features/agents/review-agent/types"; import path from "path"; @@ -30,9 +31,8 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi let reviewAgentLogPath: string | undefined; if (env.REVIEW_AGENT_LOGGING_ENABLED) { - const reviewAgentLogDir = path.join(env.DATA_CACHE_DIR, "review-agent"); - if (!fs.existsSync(reviewAgentLogDir)) { - fs.mkdirSync(reviewAgentLogDir, { recursive: true }); + if (!fs.existsSync(REVIEW_AGENT_LOG_DIR)) { + fs.mkdirSync(REVIEW_AGENT_LOG_DIR, { recursive: true }); } const timestamp = new Date().toLocaleString('en-US', { @@ -44,7 +44,7 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi second: '2-digit', hour12: false }).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6'); - reviewAgentLogPath = path.join(reviewAgentLogDir, `review-agent-${pullRequest.number}-${timestamp}.log`); + reviewAgentLogPath = path.join(REVIEW_AGENT_LOG_DIR, `review-agent-${pullRequest.number}-${timestamp}.log`); logger.info(`Review agent logging to ${reviewAgentLogPath}`); } diff --git a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts index 5cd27cdbd..5c6e98032 100644 --- a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts +++ b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts @@ -7,11 +7,11 @@ import { createLogger } from "@sourcebot/shared"; const logger = createLogger('invoke-diff-review-llm'); -const REVIEW_AGENT_LOG_BASE = path.join(env.DATA_CACHE_DIR, 'review-agent'); +export const REVIEW_AGENT_LOG_DIR = path.join(env.DATA_CACHE_DIR, 'review-agent'); const validateLogPath = (logPath: string): void => { const resolved = path.resolve(logPath); - if (!resolved.startsWith(REVIEW_AGENT_LOG_BASE + path.sep)) { + if (!resolved.startsWith(REVIEW_AGENT_LOG_DIR + path.sep)) { throw new Error('reviewAgentLogPath escapes log directory'); } }; From f887d91c17ad965d19de7a72d09ab84e22fea9f0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 02:08:06 +0000 Subject: [PATCH 4/7] docs: add guidance to link PRs to Linear issues in PR description Co-authored-by: Michael Sukkarieh --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 21f044578..bc98950c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -276,6 +276,7 @@ You can optionally include a scope to indicate which package is affected: PR description: - If a GitHub issue number was provided, include `Fixes #` in the PR description +- If a Linear issue ID was provided (e.g., SOU-123), include `Fixes SOU-123` at the top of the PR description to auto-link the PR to the Linear issue After the PR is created: - Update CHANGELOG.md with an entry under `[Unreleased]` linking to the new PR. New entries should be placed at the bottom of their section. From cee5e311023b47b8088ebf17b49f68e6a2b4a6da Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 02:28:35 +0000 Subject: [PATCH 5/7] fix: use lazy evaluation for REVIEW_AGENT_LOG_DIR to fix build Change REVIEW_AGENT_LOG_DIR from a top-level constant to a getReviewAgentLogDir() function to avoid evaluating env.DATA_CACHE_DIR at module load time, which fails during Next.js build when environment variables are not yet available. Co-authored-by: Michael Sukkarieh --- packages/web/src/features/agents/review-agent/app.ts | 9 +++++---- .../agents/review-agent/nodes/invokeDiffReviewLlm.ts | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index 1e82eec93..433d2b65c 100644 --- a/packages/web/src/features/agents/review-agent/app.ts +++ b/packages/web/src/features/agents/review-agent/app.ts @@ -2,7 +2,7 @@ import { Octokit } from "octokit"; import { generatePrReviews } from "@/features/agents/review-agent/nodes/generatePrReview"; import { githubPushPrReviews } from "@/features/agents/review-agent/nodes/githubPushPrReviews"; import { githubPrParser } from "@/features/agents/review-agent/nodes/githubPrParser"; -import { REVIEW_AGENT_LOG_DIR } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; +import { getReviewAgentLogDir } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; import { env } from "@sourcebot/shared"; import { GitHubPullRequest } from "@/features/agents/review-agent/types"; import path from "path"; @@ -31,8 +31,9 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi let reviewAgentLogPath: string | undefined; if (env.REVIEW_AGENT_LOGGING_ENABLED) { - if (!fs.existsSync(REVIEW_AGENT_LOG_DIR)) { - fs.mkdirSync(REVIEW_AGENT_LOG_DIR, { recursive: true }); + const reviewAgentLogDir = getReviewAgentLogDir(); + if (!fs.existsSync(reviewAgentLogDir)) { + fs.mkdirSync(reviewAgentLogDir, { recursive: true }); } const timestamp = new Date().toLocaleString('en-US', { @@ -44,7 +45,7 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi second: '2-digit', hour12: false }).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6'); - reviewAgentLogPath = path.join(REVIEW_AGENT_LOG_DIR, `review-agent-${pullRequest.number}-${timestamp}.log`); + reviewAgentLogPath = path.join(reviewAgentLogDir, `review-agent-${pullRequest.number}-${timestamp}.log`); logger.info(`Review agent logging to ${reviewAgentLogPath}`); } diff --git a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts index 5c6e98032..a0cbcd9e6 100644 --- a/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts +++ b/packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts @@ -7,11 +7,14 @@ import { createLogger } from "@sourcebot/shared"; const logger = createLogger('invoke-diff-review-llm'); -export const REVIEW_AGENT_LOG_DIR = path.join(env.DATA_CACHE_DIR, 'review-agent'); +export const getReviewAgentLogDir = (): string => { + return path.join(env.DATA_CACHE_DIR, 'review-agent'); +}; const validateLogPath = (logPath: string): void => { const resolved = path.resolve(logPath); - if (!resolved.startsWith(REVIEW_AGENT_LOG_DIR + path.sep)) { + const logDir = getReviewAgentLogDir(); + if (!resolved.startsWith(logDir + path.sep)) { throw new Error('reviewAgentLogPath escapes log directory'); } }; From 476081eacdedde96058ccfccd3abdab363c44b0b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 02:30:25 +0000 Subject: [PATCH 6/7] docs: add requirement to run tests and build before pushing Co-authored-by: Michael Sukkarieh --- CLAUDE.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index bc98950c0..3fac41a60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -278,6 +278,10 @@ PR description: - If a GitHub issue number was provided, include `Fixes #` in the PR description - If a Linear issue ID was provided (e.g., SOU-123), include `Fixes SOU-123` at the top of the PR description to auto-link the PR to the Linear issue +Before pushing: +- ALWAYS run `yarn test` and `yarn workspace @sourcebot/web build` before pushing to ensure tests pass and the build succeeds +- Do not push code that fails tests or breaks the build + After the PR is created: - Update CHANGELOG.md with an entry under `[Unreleased]` linking to the new PR. New entries should be placed at the bottom of their section. - Do NOT add a CHANGELOG entry for documentation-only changes (e.g., changes only in `docs/`) From c891f8293f0ff7fe04693e1310d38594e4c648c6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 18 Apr 2026 02:30:42 +0000 Subject: [PATCH 7/7] Revert "docs: add requirement to run tests and build before pushing" This reverts commit 476081eacdedde96058ccfccd3abdab363c44b0b. --- CLAUDE.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3fac41a60..bc98950c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -278,10 +278,6 @@ PR description: - If a GitHub issue number was provided, include `Fixes #` in the PR description - If a Linear issue ID was provided (e.g., SOU-123), include `Fixes SOU-123` at the top of the PR description to auto-link the PR to the Linear issue -Before pushing: -- ALWAYS run `yarn test` and `yarn workspace @sourcebot/web build` before pushing to ensure tests pass and the build succeeds -- Do not push code that fails tests or breaks the build - After the PR is created: - Update CHANGELOG.md with an entry under `[Unreleased]` linking to the new PR. New entries should be placed at the bottom of their section. - Do NOT add a CHANGELOG entry for documentation-only changes (e.g., changes only in `docs/`)