fix: resolve issue #42 — Fix WEIGHT_PER_POINT constant value#1
Open
fix: resolve issue #42 — Fix WEIGHT_PER_POINT constant value#1
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA single public constant Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
echobt
force-pushed
the
fix/bounty-issue-42
branch
from
db46240 to
0265bcb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/git-manager.ts`:
- Around line 36-49: The commitAndPush function currently interpolates the
user-supplied message into a shell string (exec(`git commit -m
"${message...}"`)) which risks command injection; replace the shell-invoking
exec calls that include user input with a safe child process call that accepts
an argument array (e.g., use execFileSync or spawnSync) so the commit message is
passed as an argument rather than embedded in a shell string, update the git
commit invocation in commitAndPush to call git with args ["commit","-m",
message] and likewise ensure any other exec calls that take untrusted input are
invoked with argument arrays, and preserve the existing error handling (catching
push failures) while logging failures appropriately.
Comment on lines
36
to
49
| export function commitAndPush(repoPath: string, branchName: string, message: string): void { | ||
| exec("git add -A", repoPath); | ||
| const status = exec("git status --porcelain", repoPath); | ||
| if (!status) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| execGit(`git checkout ${branchName}`); | ||
|
|
||
| for (const file of files) { | ||
| execGit(`git add -- "${file}"`); | ||
| } | ||
|
|
||
| execGit(`git commit -m "${message.replace(/"/g, '\\"')}"`); | ||
| } catch (error) { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| throw new Error( | ||
| `Failed to commit changes on branch '${branchName}': ${msg}` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export function pushBranch(branchName: string): void { | ||
| const token = getGHToken(); | ||
| const originUrl = getOriginUrl(); | ||
|
|
||
| exec(`git commit -m "${message.replace(/"/g, '\\"')}"`, repoPath); | ||
| try { | ||
| execGit(`git checkout ${branchName}`); | ||
|
|
||
| const authenticatedUrl = originUrl.replace( | ||
| /https:\/\/(.*@)?github\.com/, | ||
| `https://${token}@github.com` | ||
| ); | ||
| execGit(`git remote set-url origin ${authenticatedUrl}`); | ||
|
|
||
| try { | ||
| execGit(`git push -u origin ${branchName}`); | ||
| } finally { | ||
| execGit(`git remote set-url origin ${originUrl}`); | ||
| } | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| throw new Error(`Failed to push branch '${branchName}': ${message}`); | ||
| exec(`git push origin ${branchName} --force`, repoPath); | ||
| } catch { | ||
| // push may fail without remote configured; log but don't throw | ||
| console.error(`Warning: git push to origin/${branchName} failed. Commit is local only.`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid shell interpolation of commit messages (command injection risk).
Line 42 interpolates message into a shell command. Issue titles can contain $() or backticks, which the shell expands even inside double quotes, enabling command injection. Prefer execFileSync/spawnSync with argument arrays.
🛡️ Proposed fix (use execFileSync for git commands with user input)
-import { execSync } from "child_process";
+import { execSync, execFileSync } from "child_process";
function exec(cmd: string, cwd: string): string {
return execSync(cmd, { cwd, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
}
+
+function execGit(args: string[], cwd: string): string {
+ return execFileSync("git", args, { cwd, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
+}
@@
- exec(`git commit -m "${message.replace(/"/g, '\\"')}"`, repoPath);
+ execGit(["commit", "-m", message], repoPath);
try {
- exec(`git push origin ${branchName} --force`, repoPath);
+ execGit(["push", "origin", branchName, "--force"], repoPath);
} catch {#!/bin/bash
# Verify where commitAndPush is called and how the message is built.
rg -n "commitAndPush\\(" -S
rg -n "commitMessage|spec\\.title|issue_number" -S src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/git-manager.ts` around lines 36 - 49, The commitAndPush function
currently interpolates the user-supplied message into a shell string (exec(`git
commit -m "${message...}"`)) which risks command injection; replace the
shell-invoking exec calls that include user input with a safe child process call
that accepts an argument array (e.g., use execFileSync or spawnSync) so the
commit message is passed as an argument rather than embedded in a shell string,
update the git commit invocation in commitAndPush to call git with args
["commit","-m", message] and likewise ensure any other exec calls that take
untrusted input are invoked with argument arrays, and preserve the existing
error handling (catching push failures) while logging failures appropriately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix for Issue PlatformNetwork#42
Issue: Fix WEIGHT_PER_POINT constant value
Automated Fixes Applied
Files Analyzed
src/scoring.rs— 1 suggestion(s)Patch File
A detailed patch file has been generated at
output/patches/issue-42.patchGenerated by bounty-challenge worker
Summary by CodeRabbit