diff --git a/modules/git/command.go b/modules/git/command.go
index b24d32dbe8..ed06dd9b08 100644
--- a/modules/git/command.go
+++ b/modules/git/command.go
@@ -40,6 +40,7 @@ type Command struct {
parentContext context.Context
desc string
globalArgsLength int
+ brokenArgs []string
}
func (c *Command) String() string {
@@ -50,6 +51,7 @@ func (c *Command) String() string {
}
// NewCommand creates and returns a new Git Command based on given command and arguments.
+// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead.
func NewCommand(ctx context.Context, args ...string) *Command {
// Make an explicit copy of globalCommandArgs, otherwise append might overwrite it
cargs := make([]string, len(globalCommandArgs))
@@ -63,11 +65,13 @@ func NewCommand(ctx context.Context, args ...string) *Command {
}
// NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args
+// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead.
func NewCommandNoGlobals(args ...string) *Command {
return NewCommandContextNoGlobals(DefaultContext, args...)
}
// NewCommandContextNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args
+// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead.
func NewCommandContextNoGlobals(ctx context.Context, args ...string) *Command {
return &Command{
name: GitExecutable,
@@ -89,12 +93,28 @@ func (c *Command) SetDescription(desc string) *Command {
return c
}
-// AddArguments adds new argument(s) to the command.
+// AddArguments adds new argument(s) to the command. Each argument must be safe to be trusted.
+// User-provided arguments should be passed to AddDynamicArguments instead.
func (c *Command) AddArguments(args ...string) *Command {
c.args = append(c.args, args...)
return c
}
+// AddDynamicArguments adds new dynamic argument(s) to the command.
+// The arguments may come from user input and can not be trusted, so no leading '-' is allowed to avoid passing options
+func (c *Command) AddDynamicArguments(args ...string) *Command {
+ for _, arg := range args {
+ if arg != "" && arg[0] == '-' {
+ c.brokenArgs = append(c.brokenArgs, arg)
+ }
+ }
+ if len(c.brokenArgs) != 0 {
+ return c
+ }
+ c.args = append(c.args, args...)
+ return c
+}
+
// RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored.
type RunOpts struct {
Env []string
@@ -138,8 +158,14 @@ func CommonCmdServEnvs() []string {
return commonBaseEnvs()
}
+var ErrBrokenCommand = errors.New("git command is broken")
+
// Run runs the command with the RunOpts
func (c *Command) Run(opts *RunOpts) error {
+ if len(c.brokenArgs) != 0 {
+ log.Error("git command is broken: %s, broken args: %s", c.String(), strings.Join(c.brokenArgs, " "))
+ return ErrBrokenCommand
+ }
if opts == nil {
opts = &RunOpts{}
}
diff --git a/modules/git/command_test.go b/modules/git/command_test.go
index 67d4ca388e..52d25c9c74 100644
--- a/modules/git/command_test.go
+++ b/modules/git/command_test.go
@@ -26,4 +26,19 @@ func TestRunWithContextStd(t *testing.T) {
assert.Contains(t, err.Error(), "exit status 129 - unknown option:")
assert.Empty(t, stdout)
}
+
+ cmd = NewCommand(context.Background())
+ cmd.AddDynamicArguments("-test")
+ assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand)
+
+ cmd = NewCommand(context.Background())
+ cmd.AddDynamicArguments("--test")
+ assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand)
+
+ subCmd := "version"
+ cmd = NewCommand(context.Background()).AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production
+ stdout, stderr, err = cmd.RunStdString(&RunOpts{})
+ assert.NoError(t, err)
+ assert.Empty(t, stderr)
+ assert.Contains(t, stdout, "git version")
}
diff --git a/modules/git/commit.go b/modules/git/commit.go
index 82b5e0b25d..6a99d7b458 100644
--- a/modules/git/commit.go
+++ b/modules/git/commit.go
@@ -163,7 +163,7 @@ func AllCommitsCount(ctx context.Context, repoPath string, hidePRRefs bool, file
// CommitsCountFiles returns number of total commits of until given revision.
func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath []string) (int64, error) {
cmd := NewCommand(ctx, "rev-list", "--count")
- cmd.AddArguments(revision...)
+ cmd.AddDynamicArguments(revision...)
if len(relpath) > 0 {
cmd.AddArguments("--")
cmd.AddArguments(relpath...)
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index e6fec4d1a3..8d848076bc 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -158,7 +158,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co
// add previous arguments except for --grep and --all
hashCmd.AddArguments(args...)
// add keyword as <commit>
- hashCmd.AddArguments(v)
+ hashCmd.AddDynamicArguments(v)
// search with given constraints for commit matching sha hash of v
hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path})
@@ -208,14 +208,15 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) (
}()
go func() {
stderr := strings.Builder{}
- err := NewCommand(repo.Ctx, "log", revision, "--follow",
- "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page),
- prettyLogFormat, "--", file).
- Run(&RunOpts{
- Dir: repo.Path,
- Stdout: stdoutWriter,
- Stderr: &stderr,
- })
+ gitCmd := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow",
+ "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page))
+ gitCmd.AddDynamicArguments(revision)
+ gitCmd.AddArguments("--", file)
+ err := gitCmd.Run(&RunOpts{
+ Dir: repo.Path,
+ Stdout: stdoutWriter,
+ Stderr: &stderr,
+ })
if err != nil {
_ = stdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
} else {
diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go
index c0c91c6fc6..164ae07322 100644
--- a/modules/git/repo_stats.go
+++ b/modules/git/repo_stats.go
@@ -59,15 +59,15 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string)
_ = stdoutWriter.Close()
}()
- args := []string{"log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", fmt.Sprintf("--since='%s'", since)}
+ gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", fmt.Sprintf("--since='%s'", since))
if len(branch) == 0 {
- args = append(args, "--branches=*")
+ gitCmd.AddArguments("--branches=*")
} else {
- args = append(args, "--first-parent", branch)
+ gitCmd.AddArguments("--first-parent").AddDynamicArguments(branch)
}
stderr := new(strings.Builder)
- err = NewCommand(repo.Ctx, args...).Run(&RunOpts{
+ err = gitCmd.Run(&RunOpts{
Env: []string{},
Dir: repo.Path,
Stdout: stdoutWriter,
diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go
index 271382525a..0f3c021344 100644
--- a/modules/gitgraph/graph.go
+++ b/modules/gitgraph/graph.go
@@ -24,19 +24,17 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo
page = 1
}
- args := make([]string, 0, 12+len(branches)+len(files))
-
- args = append(args, "--graph", "--date-order", "--decorate=full")
+ graphCmd := git.NewCommand(r.Ctx, "log", "--graph", "--date-order", "--decorate=full")
if hidePRRefs {
- args = append(args, "--exclude="+git.PullPrefix+"*")
+ graphCmd.AddArguments("--exclude=" + git.PullPrefix + "*")
}
if len(branches) == 0 {
- args = append(args, "--all")
+ graphCmd.AddArguments("--all")
}
- args = append(args,
+ graphCmd.AddArguments(
"-C",
"-M",
fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page),
@@ -44,15 +42,12 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo
fmt.Sprintf("--pretty=format:%s", format))
if len(branches) > 0 {
- args = append(args, branches...)
+ graphCmd.AddDynamicArguments(branches...)
}
- args = append(args, "--")
if len(files) > 0 {
- args = append(args, files...)
+ graphCmd.AddArguments("--")
+ graphCmd.AddArguments(files...)
}
-
- graphCmd := git.NewCommand(r.Ctx, "log")
- graphCmd.AddArguments(args...)
graph := NewGraph()
stderr := new(strings.Builder)