diff --git a/CHANGELOG.md b/CHANGELOG.md index dd5a5ce1b..4339ccd71 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) - Fixed missing workflow permissions in `docs-broken-links.yml` by adding explicit `permissions: {}` to follow least privilege principle. [#1131](https://github.com/sourcebot-dev/sourcebot/pull/1131) - Fixed CodeQL missing-workflow-permissions alert by adding explicit empty permissions to `deploy-railway.yml`. [#1132](https://github.com/sourcebot-dev/sourcebot/pull/1132) 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. diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index 80d5a2f38..433d2b65c 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 { 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"; @@ -30,7 +31,7 @@ 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"); + const reviewAgentLogDir = getReviewAgentLogDir(); if (!fs.existsSync(reviewAgentLogDir)) { fs.mkdirSync(reviewAgentLogDir, { recursive: true }); } 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..a0cbcd9e6 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,23 @@ 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'); +export const getReviewAgentLogDir = (): string => { + return path.join(env.DATA_CACHE_DIR, 'review-agent'); +}; + +const validateLogPath = (logPath: string): void => { + const resolved = path.resolve(logPath); + const logDir = getReviewAgentLogDir(); + if (!resolved.startsWith(logDir + 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 +32,7 @@ export const invokeDiffReviewLlm = async (reviewAgentLogPath: string | undefined }); if (reviewAgentLogPath) { + validateLogPath(reviewAgentLogPath); fs.appendFileSync(reviewAgentLogPath, `\n\nPrompt:\n${prompt}`); } @@ -32,6 +46,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}`); }