Skip to content

Commit 7ad6710

Browse files
zeripathzeripath
authored
Be more strict with git arguments (#7715)
* Be more strict with git arguments * fix-up commit test * use bindings for branch name
1 parent 1d8915a commit 7ad6710

12 files changed

+41
-20
lines changed

‎modules/git/commit.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ func AddChanges(repoPath string, all bool, files ...string) error {
169169
if all {
170170
cmd.AddArguments("--all")
171171
}
172+
cmd.AddArguments("--")
172173
_, err := cmd.AddArguments(files...).RunInDir(repoPath)
173174
return err
174175
}
@@ -304,6 +305,7 @@ func (c *Commit) GetFilesChangedSinceCommit(pastCommit string) ([]string, error)
304305
}
305306

306307
// FileChangedSinceCommit Returns true if the file given has changed since the the past commit
308+
// YOU MUST ENSURE THAT pastCommit is a valid commit ID.
307309
func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, error) {
308310
return c.repo.FileChangedBetweenCommits(filename, pastCommit, c.ID.String())
309311
}

‎modules/git/repo.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,7 @@ func Pull(repoPath string, opts PullRemoteOptions) error {
187187
if opts.All {
188188
cmd.AddArguments("--all")
189189
} else {
190-
cmd.AddArguments(opts.Remote)
191-
cmd.AddArguments(opts.Branch)
190+
cmd.AddArguments("--", opts.Remote, opts.Branch)
192191
}
193192

194193
if opts.Timeout <= 0 {
@@ -213,7 +212,7 @@ func Push(repoPath string, opts PushOptions) error {
213212
if opts.Force {
214213
cmd.AddArguments("-f")
215214
}
216-
cmd.AddArguments(opts.Remote, opts.Branch)
215+
cmd.AddArguments("--", opts.Remote, opts.Branch)
217216
_, err := cmd.RunInDirWithEnv(repoPath, opts.Env)
218217
return err
219218
}

‎modules/git/repo_branch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func (repo *Repository) DeleteBranch(name string, opts DeleteBranchOptions) erro
135135
cmd.AddArguments("-d")
136136
}
137137

138-
cmd.AddArguments(name)
138+
cmd.AddArguments("--", name)
139139
_, err := cmd.RunInDir(repo.Path)
140140

141141
return err

‎modules/git/repo_commit.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,20 +117,26 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
117117
return commit, nil
118118
}
119119

120-
// GetCommit returns commit object of by ID string.
121-
func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
120+
// ConvertToSHA1 returns a Hash object from a potential ID string
121+
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
122122
if len(commitID) != 40 {
123123
var err error
124-
actualCommitID, err := NewCommand("rev-parse", commitID).RunInDir(repo.Path)
124+
actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
125125
if err != nil {
126-
if strings.Contains(err.Error(), "unknown revision or path") {
127-
return nil, ErrNotExist{commitID, ""}
126+
if strings.Contains(err.Error(), "unknown revision or path") ||
127+
strings.Contains(err.Error(), "fatal: Needed a single revision") {
128+
return SHA1{}, ErrNotExist{commitID, ""}
128129
}
129-
return nil, err
130+
return SHA1{}, err
130131
}
131132
commitID = actualCommitID
132133
}
133-
id, err := NewIDFromString(commitID)
134+
return NewIDFromString(commitID)
135+
}
136+
137+
// GetCommit returns commit object of by ID string.
138+
func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
139+
id, err := repo.ConvertToSHA1(commitID)
134140
if err != nil {
135141
return nil, err
136142
}
@@ -243,6 +249,7 @@ func (repo *Repository) getFilesChanged(id1, id2 string) ([]string, error) {
243249
}
244250

245251
// FileChangedBetweenCommits Returns true if the file changed between commit IDs id1 and id2
252+
// You must ensure that id1 and id2 are valid commit ids.
246253
func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bool, error) {
247254
stdout, err := NewCommand("diff", "--name-only", "-z", id1, id2, "--", filename).RunInDirBytes(repo.Path)
248255
if err != nil {

‎modules/git/repo_commit_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,5 @@ func TestGetCommitWithBadCommitID(t *testing.T) {
5858
commit, err := bareRepo1.GetCommit("bad_branch")
5959
assert.Nil(t, commit)
6060
assert.Error(t, err)
61-
assert.EqualError(t, err, "object does not exist [id: bad_branch, rel_path: ]")
61+
assert.True(t, IsErrNotExist(err))
6262
}

‎modules/git/repo_compare.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (repo *Repository) GetMergeBase(tmpRemote string, base, head string) (strin
3939
}
4040
}
4141

42-
stdout, err := NewCommand("merge-base", base, head).RunInDir(repo.Path)
42+
stdout, err := NewCommand("merge-base", "--", base, head).RunInDir(repo.Path)
4343
return strings.TrimSpace(stdout), base, err
4444
}
4545

‎modules/git/repo_index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
// ReadTreeToIndex reads a treeish to the index
1313
func (repo *Repository) ReadTreeToIndex(treeish string) error {
1414
if len(treeish) != 40 {
15-
res, err := NewCommand("rev-parse", treeish).RunInDir(repo.Path)
15+
res, err := NewCommand("rev-parse", "--verify", treeish).RunInDir(repo.Path)
1616
if err != nil {
1717
return err
1818
}

‎modules/git/repo_tag.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ func (repo *Repository) IsTagExist(name string) bool {
2929

3030
// CreateTag create one tag in the repository
3131
func (repo *Repository) CreateTag(name, revision string) error {
32-
_, err := NewCommand("tag", name, revision).RunInDir(repo.Path)
32+
_, err := NewCommand("tag", "--", name, revision).RunInDir(repo.Path)
3333
return err
3434
}
3535

3636
// CreateAnnotatedTag create one annotated tag in the repository
3737
func (repo *Repository) CreateAnnotatedTag(name, message, revision string) error {
38-
_, err := NewCommand("tag", "-a", "-m", message, name, revision).RunInDir(repo.Path)
38+
_, err := NewCommand("tag", "-a", "-m", message, "--", name, revision).RunInDir(repo.Path)
3939
return err
4040
}
4141

@@ -153,7 +153,7 @@ func (repo *Repository) GetTagNameBySHA(sha string) (string, error) {
153153

154154
// GetTagID returns the object ID for a tag (annotated tags have both an object SHA AND a commit SHA)
155155
func (repo *Repository) GetTagID(name string) (string, error) {
156-
stdout, err := NewCommand("show-ref", name).RunInDir(repo.Path)
156+
stdout, err := NewCommand("show-ref", "--", name).RunInDir(repo.Path)
157157
if err != nil {
158158
return "", err
159159
}

‎modules/git/repo_tree.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
2626
// GetTree find the tree object in the repository.
2727
func (repo *Repository) GetTree(idStr string) (*Tree, error) {
2828
if len(idStr) != 40 {
29-
res, err := NewCommand("rev-parse", idStr).RunInDir(repo.Path)
29+
res, err := NewCommand("rev-parse", "--verify", idStr).RunInDir(repo.Path)
3030
if err != nil {
3131
return nil, err
3232
}

‎modules/repofiles/delete.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo
9292
// Assigned LastCommitID in opts if it hasn't been set
9393
if opts.LastCommitID == "" {
9494
opts.LastCommitID = commit.ID.String()
95+
} else {
96+
lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID)
97+
if err != nil {
98+
return nil, fmt.Errorf("DeleteRepoFile: Invalid last commit ID: %v", err)
99+
}
100+
opts.LastCommitID = lastCommitID.String()
95101
}
96102

97103
// Get the files in the index

‎modules/repofiles/update.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,13 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
190190
// Assigned LastCommitID in opts if it hasn't been set
191191
if opts.LastCommitID == "" {
192192
opts.LastCommitID = commit.ID.String()
193+
} else {
194+
lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID)
195+
if err != nil {
196+
return nil, fmt.Errorf("DeleteRepoFile: Invalid last commit ID: %v", err)
197+
}
198+
opts.LastCommitID = lastCommitID.String()
199+
193200
}
194201

195202
encoding := "UTF-8"

‎modules/structs/repo_file.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ type FileOptions struct {
1010
// message (optional) for the commit of this file. if not supplied, a default message will be used
1111
Message string `json:"message"`
1212
// branch (optional) to base this file from. if not given, the default branch is used
13-
BranchName string `json:"branch"`
13+
BranchName string `json:"branch" binding:"GitRefName;MaxSize(100)"`
1414
// new_branch (optional) will make a new branch from `branch` before creating the file
15-
NewBranchName string `json:"new_branch"`
15+
NewBranchName string `json:"new_branch" binding:"GitRefName;MaxSize(100)"`
1616
// `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)
1717
Author Identity `json:"author"`
1818
Committer Identity `json:"committer"`

0 commit comments

Comments
 (0)