mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-07-09 17:28:58 +00:00
fix(security): block git ref option injection (#112)
This commit is contained in:
parent
9f81cbe486
commit
64552aee9b
10 changed files with 119 additions and 9 deletions
|
|
@ -13,7 +13,7 @@ func runGitCmd(repoDir string, args ...string) ([]byte, error) {
|
|||
}
|
||||
|
||||
func getCommitMessage(repoDir, commit string) (string, error) {
|
||||
out, err := runGitCmd(repoDir, "log", "-1", "--format=%B", commit)
|
||||
out, err := runGitCmd(repoDir, "log", "-1", "--format=%B", "--end-of-options", commit)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("git log failed: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/agent"
|
||||
|
|
@ -48,6 +49,9 @@ func runReview(args []string) error {
|
|||
if err != nil {
|
||||
return fmt.Errorf("resolve repo: %w", err)
|
||||
}
|
||||
if err := validateReviewRefs(repoDir, opts); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if opts.commit != "" && opts.background == "" {
|
||||
if msg, err := getCommitMessage(repoDir, opts.commit); err == nil && msg != "" {
|
||||
|
|
@ -216,6 +220,33 @@ func requireGitRepo(dir string) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func validateReviewRefs(repoDir string, opts reviewOptions) error {
|
||||
refs := []struct {
|
||||
flag string
|
||||
ref string
|
||||
}{
|
||||
{"--from", opts.from},
|
||||
{"--to", opts.to},
|
||||
{"--commit", opts.commit},
|
||||
}
|
||||
for _, item := range refs {
|
||||
if item.ref == "" {
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(item.ref, "-") {
|
||||
return fmt.Errorf("%s value %q is not a valid git ref: refs must not start with '-'", item.flag, item.ref)
|
||||
}
|
||||
if out, err := runGitCmd(repoDir, "rev-parse", "--verify", "--end-of-options", item.ref+"^{commit}"); err != nil {
|
||||
msg := strings.TrimSpace(string(out))
|
||||
if msg != "" {
|
||||
return fmt.Errorf("%s value %q is not a valid commit ref: %s", item.flag, item.ref, msg)
|
||||
}
|
||||
return fmt.Errorf("%s value %q is not a valid commit ref", item.flag, item.ref)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func runPreview(repoDir string, opts reviewOptions, fileFilter *rules.FileFilter) error {
|
||||
gitRunner := gitcmd.New(opts.maxGitProcs)
|
||||
ag := agent.New(agent.Args{
|
||||
|
|
|
|||
26
cmd/opencodereview/review_cmd_test.go
Normal file
26
cmd/opencodereview/review_cmd_test.go
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestValidateReviewRefsRejectsOptionLikeCommit(t *testing.T) {
|
||||
err := validateReviewRefs(t.TempDir(), reviewOptions{commit: "-O./pwn.sh"})
|
||||
if err == nil {
|
||||
t.Fatal("expected option-like --commit ref to be rejected")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "--commit") || !strings.Contains(err.Error(), "must not start with '-'") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateReviewRefsRejectsOptionLikeRangeRef(t *testing.T) {
|
||||
err := validateReviewRefs(t.TempDir(), reviewOptions{to: "-O./pwn.sh"})
|
||||
if err == nil {
|
||||
t.Fatal("expected option-like --to ref to be rejected")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "--to") || !strings.Contains(err.Error(), "must not start with '-'") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue