Test/trigger ai review - #2
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Preflight Review
This PR adds two new ways to run Preflight reviews locally against any GitHub PR without needing CI. The first is an agent mode (preflight-agent.sh) that uses the Claude Code CLI directly as the reviewer, issuing gh API calls autonomously to fetch diffs, analyze changed lines, and post a structured review without requiring an Anthropic API key in the environment. The second is a dry-run mode added to the existing review.js pipeline (preflight.sh), which runs the full API-based review logic but prints findings to stdout instead of posting them to GitHub. Both scripts share a new log-review.js helper that fetches the posted review after the fact and appends structured metadata to data/reviews.json for tracking review history over time.
Findings: 2 high, 1 medium — see inline comments
| } | ||
|
|
||
| function gh(endpoint) { | ||
| return JSON.parse(execSync(`gh api ${endpoint}`, { encoding: 'utf-8' })); |
There was a problem hiding this comment.
[HIGH] Shell injection via unsanitized repo and pr arguments in gh helper
The gh function builds a shell command by string-interpolating endpoint directly: execSync(gh api ${endpoint}). The values repo and pr come from process.argv without any sanitisation. A malicious caller could pass repo = 'x/y --method DELETE' or a PR number containing shell metacharacters, causing arbitrary gh API calls or command execution. The arguments should be passed as an array with execSync('gh', ['api', endpoint], …) or at minimum validated to match expected patterns before use.
Category: security issues
| }; | ||
|
|
||
| const dataFile = path.join(__dirname, '..', 'data', 'reviews.json'); | ||
| const log = JSON.parse(fs.readFileSync(dataFile, 'utf-8')); |
There was a problem hiding this comment.
[HIGH] readFileSync throws if data/reviews.json does not exist; no error handling
Line 46 calls fs.readFileSync(dataFile, 'utf-8') with no try/catch. If data/reviews.json is missing (e.g. first run, CI clean checkout), the process crashes with an unhandled exception and the review is never logged. The file should be initialised to [] when absent, or the read should be wrapped in a try/catch that falls back to an empty array.
Category: edge cases
| fi | ||
|
|
||
| if ! gh api "repos/$REPO" --silent 2>/dev/null; then | ||
| echo "Error: gh cannot access $REPO. Run 'gh auth login' and ensure you have repo access." |
There was a problem hiding this comment.
[MEDIUM] Unquoted $REPO interpolated into gh api URL allows argument injection
Line 24 uses gh api "repos/$REPO" where $REPO is taken directly from the first positional argument without validation. A value such as x/y --method DELETE would inject extra flags into the gh invocation. The variable should be validated to match the pattern [A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+ before use, and/or the gh api call should use -- to separate options from the path.
Category: security issues
There was a problem hiding this comment.
Preflight Review
This PR adds two new ways to run the Preflight code reviewer locally against any GitHub PR without needing a CI environment. The first mode (preflight.sh) is an API-based dry run that calls the existing review.js pipeline but suppresses all GitHub posts, printing findings to stdout instead. The second mode (preflight-agent.sh) uses the Claude Code CLI directly as an autonomous agent, instructing it to fetch PR files, analyze the diff, and post inline review comments entirely on its own using the gh CLI, requiring no Anthropic API key. A shared log-review.js script is added to record review metadata and finding counts to a local data/reviews.json file after either mode runs. The sample.js example file is also updated to fix the intentional bugs that were there as test targets, suggesting the tooling has been validated against them.
Findings: 2 high, 1 medium — see inline comments
| } | ||
|
|
||
| function gh(endpoint) { | ||
| return JSON.parse(execSync(`gh api ${endpoint}`, { encoding: 'utf-8' })); |
There was a problem hiding this comment.
[HIGH] Command injection via unsanitized repo and pr arguments
The gh function builds a shell command by directly interpolating repo and pr into a template string passed to execSync. Since repo and pr come from process.argv without any validation or escaping, a malicious value such as owner/repo; rm -rf / would be executed as a shell command. execSync uses /bin/sh by default when given a string, so shell metacharacters are interpreted. The values should be validated (e.g. /^[\w.-]+\/[\w.-]+$/ for repo and /^\d+$/ for pr) before use, or the command should be passed as an array with execFileSync to avoid shell interpretation.
Category: security issues
| merged: prData.merged, | ||
| pr_state: prData.state, | ||
| }; | ||
|
|
There was a problem hiding this comment.
[HIGH] data/reviews.json read failure crashes the process with no error handling
On line 44, fs.readFileSync(dataFile, 'utf-8') and JSON.parse(...) are called without any try/catch. If the file does not exist, is empty, or contains malformed JSON, the process throws an unhandled exception and the log entry is never written. There is also no guarantee the data/ directory exists. These calls should be wrapped in a try/catch, defaulting to an empty array [] when the file is missing or unparseable.
Category: edge cases
|
|
||
| # Pull a token from gh CLI — works for both public and private repos | ||
| # as long as the user is authenticated with sufficient scope (repo). | ||
| GITHUB_TOKEN=$(gh auth token) |
There was a problem hiding this comment.
[MEDIUM] gh auth token failure is not handled — token silently empty under set -e
With set -euo pipefail, if gh auth token fails (user not logged in, token expired) the script exits immediately at line 17 without printing a meaningful error message. GITHUB_TOKEN is never set. While set -e prevents the bad token from propagating, the user sees no actionable error. The call should be wrapped: GITHUB_TOKEN=$(gh auth token) || { echo 'Error: gh auth token failed. Run gh auth login.'; exit 1; }.
Category: unhandled errors
fix: resolve Preflight security findings from PR #2 review
1 participant
No description provided.