Skip to content

fix: resolve Preflight security findings from PR #2 review - #3

Merged
jsingh6 merged 1 commit into
mainfrom
fix/preflight-security-findings
Jun 8, 2026
Merged

fix: resolve Preflight security findings from PR #2 review#3
jsingh6 merged 1 commit into
mainfrom
fix/preflight-security-findings

Conversation

@jsingh6

@jsingh6 jsingh6 commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Fixes all findings posted by Preflight on PR #2.

Changes

  • log-review.js — validate repo and pr args against allowlist patterns before use; switch execSync string to execFileSync array to eliminate shell interpretation; wrap readFileSync/JSON.parse in try/catch with empty-array fallback
  • preflight-agent.sh — validate $REPO and $PR against expected patterns before any gh calls
  • preflight.sh — handle gh auth token failure with a clear error message instead of silent empty token

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preflight Review

This PR hardens the input handling and error resilience across the Preflight review scripts. The core change replaces execSync with execFileSync in log-review.js, which prevents shell injection by passing arguments as an array rather than interpolating them into a shell command string. Input validation was added to both the Node script and the agent shell script to reject malformed repo names and PR numbers before they reach any external commands. The log-review.js script was also made more resilient by gracefully handling a missing or corrupted reviews.json file rather than crashing, and preflight.sh now properly surfaces a failure from gh auth token instead of silently continuing. Taken together, the changes close an injection risk that existed when user-supplied repo and PR arguments were passed through shell string interpolation, and improve operational robustness when the data file doesn't yet exist.

Findings: 0 high, 0 medium — see inline comments

@jsingh6
jsingh6 merged commit cfb5887 into main Jun 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant