Skip to content

Conversation

@sestinj
Copy link
Contributor

@sestinj sestinj commented Nov 24, 2025

Summary by cubic

Add POST /pr endpoint to create GitHub pull requests via the GitHub CLI. Validates repo state, auto-fills title/body, and returns the PR URL.

  • New Features

    • POST /pr with options: { title?, body?, base?, draft?, web? }.
    • Uses gh pr create; checks gh is installed.
    • Validates: in git repo, non-main/master branch, GitHub remote (HTTPS or SSH).
    • Defaults: base=main, title from branch name, body from recent commits; uses --fill when needed.
    • Supports --draft and --web flags; returns { success, message, prUrl } with 400/500 on errors.
    • Server help output updated; comprehensive tests cover validation, URL parsing, and options.
  • Refactors

    • Use execFile for gh invocations to reduce command injection risk.

Written for commit d3cba54. Summary will update automatically on new commits.

@sestinj sestinj requested a review from a team as a code owner November 24, 2025 19:24
@sestinj sestinj requested review from tingwai and removed request for a team November 24, 2025 19:24
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Nov 24, 2025
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

✅ Review Complete

Code Review Summary

⚠️ Continue configuration error. Please verify that the assistant exists in Continue Hub.


Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 3 files

Prompt for AI agents (all 3 issues)

Understand the root cause of the following 3 issues and fix them.


<file name="extensions/cli/src/commands/serve.ts">

<violation number="1" location="extensions/cli/src/commands/serve.ts:329">
`/pr` accepts untrusted JSON and feeds it into createPullRequest, which constructs `gh pr create` via string concatenation and executes it with `child_process.exec`, allowing remote command injection via crafted title/body/base values.</violation>

<violation number="2" location="extensions/cli/src/commands/serve.ts:419">
Rule violated: **Don&#39;t use console.log**

The newly added POST /pr endpoint announcement uses console.log instead of the project logger, violating the &quot;Don&#39;t use console.log&quot; rule. Use logger.info/logger.warn for all runtime logging to keep output centralized and configurable.</violation>
</file>

<file name="extensions/cli/src/commands/pr.ts">

<violation number="1" location="extensions/cli/src/commands/pr.ts:91">
User-supplied PR metadata is interpolated into a shell command (`execAsync(&quot;gh …&quot;)`), allowing command injection and even breaking when titles/bodies contain spaces. Use spawn/execFile with an argument array instead of building a shell string.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

state.lastActivity = Date.now();

try {
const options: PrOptions = req.body;
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 24, 2025

Choose a reason for hiding this comment

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

/pr accepts untrusted JSON and feeds it into createPullRequest, which constructs gh pr create via string concatenation and executes it with child_process.exec, allowing remote command injection via crafted title/body/base values.

Prompt for AI agents
Address the following comment on extensions/cli/src/commands/serve.ts at line 329:

<comment>`/pr` accepts untrusted JSON and feeds it into createPullRequest, which constructs `gh pr create` via string concatenation and executes it with `child_process.exec`, allowing remote command injection via crafted title/body/base values.</comment>

<file context>
@@ -320,6 +321,40 @@ export async function serve(prompt?: string, options: ServeOptions = {}) {
+    state.lastActivity = Date.now();
+
+    try {
+      const options: PrOptions = req.body;
+      logger.info(&quot;Creating pull request&quot;, { options });
+
</file context>
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

4 issues found across 3 files

Prompt for AI agents (all 4 issues)

Understand the root cause of the following 4 issues and fix them.


<file name="extensions/cli/src/commands/pr.ts">

<violation number="1" location="extensions/cli/src/commands/pr.ts:116">
User-controlled base is interpolated into a git log shell command executed via execAsync, enabling command injection. Use execFile with argument arrays or strictly validate allowed branch names before executing git.</violation>
</file>

<file name="extensions/cli/src/commands/pr.test.ts">

<violation number="1" location="extensions/cli/src/commands/pr.test.ts:17">
Mocked `child_process` never exposes `execFile`, so importing the module under test throws before any test runs.</violation>

<violation number="2" location="extensions/cli/src/commands/pr.test.ts:121">
The `execMock` implementations use an `exec`-style signature, but the code under test calls `execFile(file, args, options, callback)`, so callbacks are never invoked and the command matching logic never triggers.</violation>
</file>

<file name="extensions/cli/src/commands/serve.ts">

<violation number="1" location="extensions/cli/src/commands/serve.ts:419">
Rule violated: **Don&#39;t use console.log**

Replace this new console.log with the shared logger (e.g., logger.info) to comply with the “Don’t use console.log” rule for server output.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

sestinj and others added 3 commits November 24, 2025 16:05
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants