From cfb1cb1168726a3a4c13aeafaa9728d82e7e84fe Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Fri, 3 Feb 2023 11:23:52 -0500 Subject: [PATCH 01/20] update to build with go1.20 (#22732) as title --------- Co-authored-by: Lauris BH --- .drone.yml | 32 +++++++++++++-------------- .golangci.yml | 4 ++-- Dockerfile | 2 +- Dockerfile.rootless | 2 +- Makefile | 4 ++-- docs/config.yaml | 4 ++-- go.mod | 2 +- models/db/sql_postgres_with_schema.go | 7 ++---- 8 files changed, 27 insertions(+), 30 deletions(-) diff --git a/.drone.yml b/.drone.yml index a8fa7eba368..f9da8f97438 100644 --- a/.drone.yml +++ b/.drone.yml @@ -25,7 +25,7 @@ steps: - make deps-frontend - name: deps-backend - image: golang:1.19 + image: golang:1.20 pull: always commands: - make deps-backend @@ -88,7 +88,7 @@ steps: depends_on: [deps-frontend] - name: checks-backend - image: golang:1.19 + image: golang:1.20 commands: - make --always-make checks-backend # ensure the 'go-licenses' make target runs depends_on: [deps-backend] @@ -109,7 +109,7 @@ steps: depends_on: [deps-frontend] - name: build-backend-no-gcc - image: golang:1.18 # this step is kept as the lowest version of golang that we support + image: golang:1.19 # this step is kept as the lowest version of golang that we support pull: always environment: GO111MODULE: on @@ -122,7 +122,7 @@ steps: path: /go - name: build-backend-arm64 - image: golang:1.19 + image: golang:1.20 environment: GO111MODULE: on GOPROXY: https://goproxy.io @@ -138,7 +138,7 @@ steps: path: /go - name: build-backend-windows - image: golang:1.19 + image: golang:1.20 environment: GO111MODULE: on GOPROXY: https://goproxy.io @@ -153,7 +153,7 @@ steps: path: /go - name: build-backend-386 - image: golang:1.19 + image: golang:1.20 environment: GO111MODULE: on GOPROXY: https://goproxy.io @@ -247,7 +247,7 @@ steps: - pull_request - name: deps-backend - image: golang:1.19 + image: golang:1.20 pull: always commands: - make deps-backend @@ -364,7 +364,7 @@ steps: path: /go - name: generate-coverage - image: golang:1.19 + image: golang:1.20 commands: - make coverage environment: @@ -440,7 +440,7 @@ steps: - pull_request - name: deps-backend - image: golang:1.19 + image: golang:1.20 pull: always commands: - make deps-backend @@ -557,7 +557,7 @@ steps: - name: test-e2e image: mcr.microsoft.com/playwright:v1.29.2-focal commands: - - curl -sLO https://go.dev/dl/go1.19.linux-amd64.tar.gz && tar -C /usr/local -xzf go1.19.linux-amd64.tar.gz + - curl -sLO https://go.dev/dl/go1.20.linux-amd64.tar.gz && tar -C /usr/local -xzf go1.20.linux-amd64.tar.gz - groupadd --gid 1001 gitea && useradd -m --gid 1001 --uid 1001 gitea - apt-get -qq update && apt-get -qqy install build-essential - export TEST_PGSQL_SCHEMA='' @@ -656,7 +656,7 @@ trigger: steps: - name: download - image: golang:1.19 + image: golang:1.20 pull: always commands: - timeout -s ABRT 40m make generate-license generate-gitignore @@ -720,7 +720,7 @@ steps: - make deps-frontend - name: deps-backend - image: golang:1.19 + image: golang:1.20 pull: always commands: - make deps-backend @@ -729,7 +729,7 @@ steps: path: /go - name: static - image: techknowlogick/xgo:go-1.19.x + image: techknowlogick/xgo:go-1.20.x pull: always commands: # Upgrade to node 18 once https://github.com/techknowlogick/xgo/issues/163 is resolved @@ -841,7 +841,7 @@ steps: - make deps-frontend - name: deps-backend - image: golang:1.19 + image: golang:1.20 pull: always commands: - make deps-backend @@ -850,7 +850,7 @@ steps: path: /go - name: static - image: techknowlogick/xgo:go-1.19.x + image: techknowlogick/xgo:go-1.20.x pull: always commands: # Upgrade to node 18 once https://github.com/techknowlogick/xgo/issues/163 is resolved @@ -932,7 +932,7 @@ trigger: steps: - name: build-docs - image: golang:1.19 + image: golang:1.20 commands: - cd docs - make trans-copy clean build diff --git a/.golangci.yml b/.golangci.yml index 7635e83a372..caa237370e3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,7 +28,7 @@ linters: fast: false run: - go: 1.19 + go: 1.20 timeout: 10m skip-dirs: - node_modules @@ -74,7 +74,7 @@ linters-settings: - name: modifies-value-receiver gofumpt: extra-rules: true - lang-version: "1.19" + lang-version: "1.20" depguard: list-type: denylist # Check the list against standard lib. diff --git a/Dockerfile b/Dockerfile index 3ee474bb342..89f000882c5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ #Build stage -FROM golang:1.19-alpine3.17 AS build-env +FROM golang:1.20-alpine3.17 AS build-env ARG GOPROXY ENV GOPROXY ${GOPROXY:-direct} diff --git a/Dockerfile.rootless b/Dockerfile.rootless index a43a63fa10c..e92ce857d3c 100644 --- a/Dockerfile.rootless +++ b/Dockerfile.rootless @@ -1,5 +1,5 @@ #Build stage -FROM golang:1.19-alpine3.17 AS build-env +FROM golang:1.20-alpine3.17 AS build-env ARG GOPROXY ENV GOPROXY ${GOPROXY:-direct} diff --git a/Makefile b/Makefile index 4d7c5078755..717d7cafc66 100644 --- a/Makefile +++ b/Makefile @@ -23,13 +23,13 @@ SHASUM ?= shasum -a 256 HAS_GO = $(shell hash $(GO) > /dev/null 2>&1 && echo "GO" || echo "NOGO" ) COMMA := , -XGO_VERSION := go-1.19.x +XGO_VERSION := go-1.20.x AIR_PACKAGE ?= github.com/cosmtrek/air@v1.40.4 EDITORCONFIG_CHECKER_PACKAGE ?= github.com/editorconfig-checker/editorconfig-checker/cmd/editorconfig-checker@2.6.0 ERRCHECK_PACKAGE ?= github.com/kisielk/errcheck@v1.6.2 GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.4.0 -GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1 +GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.0 GXZ_PAGAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.10 MISSPELL_PACKAGE ?= github.com/client9/misspell/cmd/misspell@v0.3.4 SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.30.3 diff --git a/docs/config.yaml b/docs/config.yaml index 0b47c0ffd83..0a6d5d13f23 100644 --- a/docs/config.yaml +++ b/docs/config.yaml @@ -19,8 +19,8 @@ params: author: The Gitea Authors website: https://docs.gitea.io version: 1.18.1 - minGoVersion: 1.18 - goVersion: 1.19 + minGoVersion: 1.19 + goVersion: 1.20 minNodeVersion: 16 search: nav repo: "https://github.com/go-gitea/gitea" diff --git a/go.mod b/go.mod index a929508e0de..eb23bd9e32a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module code.gitea.io/gitea -go 1.18 +go 1.19 require ( code.gitea.io/actions-proto-go v0.2.0 diff --git a/models/db/sql_postgres_with_schema.go b/models/db/sql_postgres_with_schema.go index ec63447f6f6..c2694b37bb9 100644 --- a/models/db/sql_postgres_with_schema.go +++ b/models/db/sql_postgres_with_schema.go @@ -37,9 +37,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) { } schemaValue, _ := driver.String.ConvertValue(setting.Database.Schema) - // golangci lint is incorrect here - there is no benefit to using driver.ExecerContext here - // and in any case pq does not implement it - if execer, ok := conn.(driver.Execer); ok { //nolint + if execer, ok := conn.(driver.Execer); ok { _, err := execer.Exec(`SELECT set_config( 'search_path', $1 || ',' || current_setting('search_path'), @@ -63,8 +61,7 @@ func (d *postgresSchemaDriver) Open(name string) (driver.Conn, error) { // driver.String.ConvertValue will never return err for string - // golangci lint is incorrect here - there is no benefit to using stmt.ExecWithContext here - _, err = stmt.Exec([]driver.Value{schemaValue}) //nolint + _, err = stmt.Exec([]driver.Value{schemaValue}) if err != nil { _ = conn.Close() return nil, err From ce4fd95233b0012fa5510bd3f2d049a0edac7903 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 3 Feb 2023 19:22:11 +0200 Subject: [PATCH 02/20] Use native error checking with `exec.ErrDot` (#22735) This was meant to land in #22073 but was blocked until #22732 was merged Signed-off-by: Yarden Shoham --- modules/setting/setting.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index f591727ca34..afd7a40150d 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -6,6 +6,7 @@ package setting import ( "encoding/base64" + "errors" "fmt" "math" "net" @@ -466,8 +467,7 @@ func getAppPath() (string, error) { } if err != nil { - // FIXME: Once we switch to go 1.19 use !errors.Is(err, exec.ErrDot) - if !strings.Contains(err.Error(), "cannot run executable found relative to current directory") { + if !errors.Is(err, exec.ErrDot) { return "", err } appPath, err = filepath.Abs(os.Args[0]) From 01f082287d7957ed63a0865b26e04ad23382c715 Mon Sep 17 00:00:00 2001 From: Francesco Siddi Date: Fri, 3 Feb 2023 23:25:55 +0100 Subject: [PATCH 03/20] Remove 'primary' class from tab counter labels (#22687) Using the primary color for each label counter makes the use of color redundant, as well as suggesting this is a call to action. Use the base grey color instead. ![grey_lables](https://user-images.githubusercontent.com/451841/215778889-0d5dddad-353f-4703-a48f-1540080dee26.jpg) --- templates/repo/header.tmpl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 0e50169486b..b860c1d26ce 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -163,7 +163,7 @@ {{svg "octicon-issue-opened"}} {{.locale.Tr "repo.issues"}} {{if .Repository.NumOpenIssues}} - {{CountFmt .Repository.NumOpenIssues}} + {{CountFmt .Repository.NumOpenIssues}} {{end}} {{end}} @@ -178,7 +178,7 @@ {{svg "octicon-git-pull-request"}} {{.locale.Tr "repo.pulls"}} {{if .Repository.NumOpenPulls}} - {{CountFmt .Repository.NumOpenPulls}} + {{CountFmt .Repository.NumOpenPulls}} {{end}} {{end}} @@ -187,7 +187,7 @@ {{svg "octicon-play"}} {{.locale.Tr "actions.actions"}} {{if .Repository.NumOpenActionRuns}} - {{CountFmt .Repository.NumOpenActionRuns}} + {{CountFmt .Repository.NumOpenActionRuns}} {{end}} {{end}} @@ -202,7 +202,7 @@ {{svg "octicon-project"}} {{.locale.Tr "repo.project_board"}} {{if .Repository.NumOpenProjects}} - {{CountFmt .Repository.NumOpenProjects}} + {{CountFmt .Repository.NumOpenProjects}} {{end}} {{end}} @@ -211,7 +211,7 @@ {{svg "octicon-tag"}} {{.locale.Tr "repo.releases"}} {{if .NumReleases}} - {{CountFmt .NumReleases}} + {{CountFmt .NumReleases}} {{end}} {{end}} From 3c5655ce18056277917092d370bbdfbcdaaa8ae6 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 3 Feb 2023 23:11:48 +0000 Subject: [PATCH 04/20] Improve trace logging for pulls and processes (#22633) Our trace logging is far from perfect and is difficult to follow. This PR: * Add trace logging for process manager add and remove. * Fixes an errant read file for git refs in getMergeCommit * Brings in the pullrequest `String` and `ColorFormat` methods introduced in #22568 * Adds a lot more logging in to testPR etc. Ref #22578 --------- Signed-off-by: Andrew Thornton Co-authored-by: Lunny Xiao Co-authored-by: delvh Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: techknowlogick --- models/issues/pull.go | 67 ++++++++++- modules/log/colors.go | 7 ++ modules/log/log.go | 11 ++ modules/process/manager.go | 22 ++-- modules/process/manager_test.go | 2 +- routers/web/repo/pull.go | 51 ++++----- services/automerge/automerge.go | 26 ++--- services/pull/check.go | 194 ++++++++++++++++---------------- services/pull/patch.go | 2 +- 9 files changed, 233 insertions(+), 149 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index 6ff6502e4eb..044fb5fa04d 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "io" + "strconv" "strings" "code.gitea.io/gitea/models/db" @@ -132,6 +133,27 @@ const ( PullRequestStatusAncestor ) +func (status PullRequestStatus) String() string { + switch status { + case PullRequestStatusConflict: + return "CONFLICT" + case PullRequestStatusChecking: + return "CHECKING" + case PullRequestStatusMergeable: + return "MERGEABLE" + case PullRequestStatusManuallyMerged: + return "MANUALLY_MERGED" + case PullRequestStatusError: + return "ERROR" + case PullRequestStatusEmpty: + return "EMPTY" + case PullRequestStatusAncestor: + return "ANCESTOR" + default: + return strconv.Itoa(int(status)) + } +} + // PullRequestFlow the flow of pull request type PullRequestFlow int @@ -202,6 +224,42 @@ func DeletePullsByBaseRepoID(ctx context.Context, repoID int64) error { return err } +// ColorFormat writes a colored string to identify this struct +func (pr *PullRequest) ColorFormat(s fmt.State) { + if pr == nil { + log.ColorFprintf(s, "PR[%d]%s#%d[%s...%s:%s]", + log.NewColoredIDValue(0), + log.NewColoredValue("/"), + log.NewColoredIDValue(0), + log.NewColoredValue(""), + log.NewColoredValue("/"), + log.NewColoredValue(""), + ) + return + } + + log.ColorFprintf(s, "PR[%d]", log.NewColoredIDValue(pr.ID)) + if pr.BaseRepo != nil { + log.ColorFprintf(s, "%s#%d[%s...", log.NewColoredValue(pr.BaseRepo.FullName()), + log.NewColoredIDValue(pr.Index), log.NewColoredValue(pr.BaseBranch)) + } else { + log.ColorFprintf(s, "Repo[%d]#%d[%s...", log.NewColoredIDValue(pr.BaseRepoID), + log.NewColoredIDValue(pr.Index), log.NewColoredValue(pr.BaseBranch)) + } + if pr.HeadRepoID == pr.BaseRepoID { + log.ColorFprintf(s, "%s]", log.NewColoredValue(pr.HeadBranch)) + } else if pr.HeadRepo != nil { + log.ColorFprintf(s, "%s:%s]", log.NewColoredValue(pr.HeadRepo.FullName()), log.NewColoredValue(pr.HeadBranch)) + } else { + log.ColorFprintf(s, "Repo[%d]:%s]", log.NewColoredIDValue(pr.HeadRepoID), log.NewColoredValue(pr.HeadBranch)) + } +} + +// String represents the pr as a simple string +func (pr *PullRequest) String() string { + return log.ColorFormatAsString(pr) +} + // MustHeadUserName returns the HeadRepo's username if failed return blank func (pr *PullRequest) MustHeadUserName(ctx context.Context) string { if err := pr.LoadHeadRepo(ctx); err != nil { @@ -234,7 +292,8 @@ func (pr *PullRequest) LoadAttributes(ctx context.Context) (err error) { return nil } -// LoadHeadRepo loads the head repository +// LoadHeadRepo loads the head repository, pr.HeadRepo will remain nil if it does not exist +// and thus ErrRepoNotExist will never be returned func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { if !pr.isHeadRepoLoaded && pr.HeadRepo == nil && pr.HeadRepoID > 0 { if pr.HeadRepoID == pr.BaseRepoID { @@ -249,14 +308,14 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { pr.HeadRepo, err = repo_model.GetRepositoryByID(ctx, pr.HeadRepoID) if err != nil && !repo_model.IsErrRepoNotExist(err) { // Head repo maybe deleted, but it should still work - return fmt.Errorf("GetRepositoryByID(head): %w", err) + return fmt.Errorf("pr[%d].LoadHeadRepo[%d]: %w", pr.ID, pr.HeadRepoID, err) } pr.isHeadRepoLoaded = true } return nil } -// LoadBaseRepo loads the target repository +// LoadBaseRepo loads the target repository. ErrRepoNotExist may be returned. func (pr *PullRequest) LoadBaseRepo(ctx context.Context) (err error) { if pr.BaseRepo != nil { return nil @@ -274,7 +333,7 @@ func (pr *PullRequest) LoadBaseRepo(ctx context.Context) (err error) { pr.BaseRepo, err = repo_model.GetRepositoryByID(ctx, pr.BaseRepoID) if err != nil { - return fmt.Errorf("repo_model.GetRepositoryByID(base): %w", err) + return fmt.Errorf("pr[%d].LoadBaseRepo[%d]: %w", pr.ID, pr.BaseRepoID, err) } return nil } diff --git a/modules/log/colors.go b/modules/log/colors.go index 02781afe84e..85e205cb676 100644 --- a/modules/log/colors.go +++ b/modules/log/colors.go @@ -383,6 +383,13 @@ func (cv *ColoredValue) Format(s fmt.State, c rune) { s.Write(*cv.resetBytes) } +// ColorFormatAsString returns the result of the ColorFormat without the color +func ColorFormatAsString(colorVal ColorFormatted) string { + s := new(strings.Builder) + _, _ = ColorFprintf(&protectedANSIWriter{w: s, mode: removeColor}, "%-v", colorVal) + return s.String() +} + // SetColorBytes will allow a user to set the colorBytes of a colored value func (cv *ColoredValue) SetColorBytes(colorBytes []byte) { cv.colorBytes = &colorBytes diff --git a/modules/log/log.go b/modules/log/log.go index 73f908dfab5..9057cda37e6 100644 --- a/modules/log/log.go +++ b/modules/log/log.go @@ -9,6 +9,8 @@ import ( "runtime" "strings" "sync" + + "code.gitea.io/gitea/modules/process" ) type loggerMap struct { @@ -285,6 +287,15 @@ func (l *LoggerAsWriter) Log(msg string) { } func init() { + process.Trace = func(start bool, pid process.IDType, description string, parentPID process.IDType, typ string) { + if start && parentPID != "" { + Log(1, TRACE, "Start %s: %s (from %s) (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(parentPID, FgYellow), NewColoredValue(typ, Reset)) + } else if start { + Log(1, TRACE, "Start %s: %s (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(typ, Reset)) + } else { + Log(1, TRACE, "Done %s: %s", NewColoredValue(pid, FgHiYellow), NewColoredValue(description, Reset)) + } + } _, filename, _, _ := runtime.Caller(0) prefix = strings.TrimSuffix(filename, "modules/log/log.go") if prefix == filename { diff --git a/modules/process/manager.go b/modules/process/manager.go index 1272510067f..fdfca3db7a1 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -6,6 +6,7 @@ package process import ( "context" + "log" "runtime/pprof" "strconv" "sync" @@ -43,6 +44,18 @@ type IDType string // - it is simply an alias for context.CancelFunc and is only for documentary purposes type FinishedFunc = context.CancelFunc +var Trace = defaultTrace // this global can be overridden by particular logging packages - thus avoiding import cycles + +func defaultTrace(start bool, pid IDType, description string, parentPID IDType, typ string) { + if start && parentPID != "" { + log.Printf("start process %s: %s (from %s) (%s)", pid, description, parentPID, typ) + } else if start { + log.Printf("start process %s: %s (%s)", pid, description, typ) + } else { + log.Printf("end process %s: %s", pid, description) + } +} + // Manager manages all processes and counts PIDs. type Manager struct { mutex sync.Mutex @@ -154,6 +167,7 @@ func (pm *Manager) Add(ctx context.Context, description string, cancel context.C pm.processMap[pid] = process pm.mutex.Unlock() + Trace(true, pid, description, parentPID, processType) pprofCtx := pprof.WithLabels(ctx, pprof.Labels(DescriptionPProfLabel, description, PPIDPProfLabel, string(parentPID), PIDPProfLabel, string(pid), ProcessTypePProfLabel, processType)) if currentlyRunning { @@ -185,18 +199,12 @@ func (pm *Manager) nextPID() (start time.Time, pid IDType) { return start, pid } -// Remove a process from the ProcessManager. -func (pm *Manager) Remove(pid IDType) { - pm.mutex.Lock() - delete(pm.processMap, pid) - pm.mutex.Unlock() -} - func (pm *Manager) remove(process *process) { pm.mutex.Lock() defer pm.mutex.Unlock() if p := pm.processMap[process.PID]; p == process { delete(pm.processMap, process.PID) + Trace(false, process.PID, process.Description, process.ParentPID, process.Type) } } diff --git a/modules/process/manager_test.go b/modules/process/manager_test.go index 527072713ff..2e2e35d24a0 100644 --- a/modules/process/manager_test.go +++ b/modules/process/manager_test.go @@ -82,7 +82,7 @@ func TestManager_Remove(t *testing.T) { assert.NotEqual(t, GetContext(p1Ctx).GetPID(), GetContext(p2Ctx).GetPID(), "expected to get different pids got %s == %s", GetContext(p2Ctx).GetPID(), GetContext(p1Ctx).GetPID()) - pm.Remove(GetPID(p2Ctx)) + finished() _, exists := pm.processMap[GetPID(p2Ctx)] assert.False(t, exists, "PID %d is in the list but shouldn't", GetPID(p2Ctx)) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 71bc98d13d0..11d336d4eca 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -926,59 +926,54 @@ func MergePullRequest(ctx *context.Context) { pr := issue.PullRequest pr.Issue = issue pr.Issue.Repo = ctx.Repo.Repository - manuallMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged + manualMerge := repo_model.MergeStyle(form.Do) == repo_model.MergeStyleManuallyMerged forceMerge := form.ForceMerge != nil && *form.ForceMerge // start with merging by checking - if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manuallMerge, forceMerge); err != nil { - if errors.Is(err, pull_service.ErrIsClosed) { + if err := pull_service.CheckPullMergable(ctx, ctx.Doer, &ctx.Repo.Permission, pr, manualMerge, forceMerge); err != nil { + switch { + case errors.Is(err, pull_service.ErrIsClosed): if issue.IsPull { ctx.Flash.Error(ctx.Tr("repo.pulls.is_closed")) - ctx.Redirect(issue.Link()) } else { ctx.Flash.Error(ctx.Tr("repo.issues.closed_title")) - ctx.Redirect(issue.Link()) } - } else if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) { + case errors.Is(err, pull_service.ErrUserNotAllowedToMerge): ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed")) - ctx.Redirect(issue.Link()) - } else if errors.Is(err, pull_service.ErrHasMerged) { + case errors.Is(err, pull_service.ErrHasMerged): ctx.Flash.Error(ctx.Tr("repo.pulls.has_merged")) - ctx.Redirect(issue.Link()) - } else if errors.Is(err, pull_service.ErrIsWorkInProgress) { + case errors.Is(err, pull_service.ErrIsWorkInProgress): ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_wip")) - ctx.Redirect(issue.Link()) - } else if errors.Is(err, pull_service.ErrNotMergableState) { + case errors.Is(err, pull_service.ErrNotMergableState): ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) - ctx.Redirect(issue.Link()) - } else if models.IsErrDisallowedToMerge(err) { + case models.IsErrDisallowedToMerge(err): ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_not_ready")) - ctx.Redirect(issue.Link()) - } else if asymkey_service.IsErrWontSign(err) { - ctx.Flash.Error(err.Error()) // has not translation ... - ctx.Redirect(issue.Link()) - } else if errors.Is(err, pull_service.ErrDependenciesLeft) { + case asymkey_service.IsErrWontSign(err): + ctx.Flash.Error(err.Error()) // has no translation ... + case errors.Is(err, pull_service.ErrDependenciesLeft): ctx.Flash.Error(ctx.Tr("repo.issues.dependency.pr_close_blocked")) - ctx.Redirect(issue.Link()) - } else { + default: ctx.ServerError("WebCheck", err) + return } + + ctx.Redirect(issue.Link()) return } // handle manually-merged mark - if manuallMerge { + if manualMerge { if err := pull_service.MergedManually(pr, ctx.Doer, ctx.Repo.GitRepo, form.MergeCommitID); err != nil { - if models.IsErrInvalidMergeStyle(err) { + switch { + + case models.IsErrInvalidMergeStyle(err): ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) - ctx.Redirect(issue.Link()) - } else if strings.Contains(err.Error(), "Wrong commit ID") { + case strings.Contains(err.Error(), "Wrong commit ID"): ctx.Flash.Error(ctx.Tr("repo.pulls.wrong_commit_id")) - ctx.Redirect(issue.Link()) - } else { + default: ctx.ServerError("MergedManually", err) + return } - return } ctx.Redirect(issue.Link()) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index 15d94e79209..74cfb8da8fd 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -164,7 +164,7 @@ func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model. func handlePull(pullID int64, sha string) { ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), - fmt.Sprintf("Handle AutoMerge of pull[%d] with sha[%s]", pullID, sha)) + fmt.Sprintf("Handle AutoMerge of PR[%d] with sha[%s]", pullID, sha)) defer finished() pr, err := issues_model.GetPullRequestByID(ctx, pullID) @@ -176,7 +176,7 @@ func handlePull(pullID int64, sha string) { // Check if there is a scheduled pr in the db exists, scheduledPRM, err := pull_model.GetScheduledMergeByPullID(ctx, pr.ID) if err != nil { - log.Error("pull[%d] GetScheduledMergeByPullID: %v", pr.ID, err) + log.Error("%-v GetScheduledMergeByPullID: %v", pr, err) return } if !exists { @@ -188,13 +188,13 @@ func handlePull(pullID int64, sha string) { // did not succeed or was not finished yet. if err = pr.LoadHeadRepo(ctx); err != nil { - log.Error("pull[%d] LoadHeadRepo: %v", pr.ID, err) + log.Error("%-v LoadHeadRepo: %v", pr, err) return } headGitRepo, err := git.OpenRepository(ctx, pr.HeadRepo.RepoPath()) if err != nil { - log.Error("OpenRepository: %v", err) + log.Error("OpenRepository %-v: %v", pr.HeadRepo, err) return } defer headGitRepo.Close() @@ -202,40 +202,40 @@ func handlePull(pullID int64, sha string) { headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) if pr.HeadRepo == nil || !headBranchExist { - log.Warn("Head branch of auto merge pr does not exist [HeadRepoID: %d, Branch: %s, PR ID: %d]", pr.HeadRepoID, pr.HeadBranch, pr.ID) + log.Warn("Head branch of auto merge %-v does not exist [HeadRepoID: %d, Branch: %s]", pr, pr.HeadRepoID, pr.HeadBranch) return } // Check if all checks succeeded pass, err := pull_service.IsPullCommitStatusPass(ctx, pr) if err != nil { - log.Error("IsPullCommitStatusPass: %v", err) + log.Error("%-v IsPullCommitStatusPass: %v", pr, err) return } if !pass { - log.Info("Scheduled auto merge pr has unsuccessful status checks [PullID: %d]", pr.ID) + log.Info("Scheduled auto merge %-v has unsuccessful status checks", pr) return } // Merge if all checks succeeded doer, err := user_model.GetUserByID(ctx, scheduledPRM.DoerID) if err != nil { - log.Error("GetUserByIDCtx: %v", err) + log.Error("Unable to get scheduled User[%d]: %v", scheduledPRM.DoerID, err) return } perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, doer) if err != nil { - log.Error("GetUserRepoPermission: %v", err) + log.Error("GetUserRepoPermission %-v: %v", pr.HeadRepo, err) return } if err := pull_service.CheckPullMergable(ctx, doer, &perm, pr, false, false); err != nil { if errors.Is(pull_service.ErrUserNotAllowedToMerge, err) { - log.Info("PR %d was scheduled to automerge by an unauthorized user", pr.ID) + log.Info("%-v was scheduled to automerge by an unauthorized user", pr) return } - log.Error("pull[%d] CheckPullMergable: %v", pr.ID, err) + log.Error("%-v CheckPullMergable: %v", pr, err) return } @@ -244,13 +244,13 @@ func handlePull(pullID int64, sha string) { baseGitRepo = headGitRepo } else { if err = pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) + log.Error("%-v LoadBaseRepo: %v", pr, err) return } baseGitRepo, err = git.OpenRepository(ctx, pr.BaseRepo.RepoPath()) if err != nil { - log.Error("OpenRepository: %v", err) + log.Error("OpenRepository %-v: %v", pr.BaseRepo, err) return } defer baseGitRepo.Close() diff --git a/services/pull/check.go b/services/pull/check.go index db86378909c..481491c73bb 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "os" "strconv" "strings" @@ -27,7 +26,6 @@ import ( "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" asymkey_service "code.gitea.io/gitea/services/asymkey" ) @@ -50,14 +48,14 @@ func AddToTaskQueue(pr *issues_model.PullRequest) { pr.Status = issues_model.PullRequestStatusChecking err := pr.UpdateColsIfNotMerged(db.DefaultContext, "status") if err != nil { - log.Error("AddToTaskQueue.UpdateCols[%d].(add to queue): %v", pr.ID, err) + log.Error("AddToTaskQueue(%-v).UpdateCols.(add to queue): %v", pr, err) } else { - log.Trace("Adding PR ID: %d to the test pull requests queue", pr.ID) + log.Trace("Adding %-v to the test pull requests queue", pr) } return err }) if err != nil && err != queue.ErrAlreadyInQueue { - log.Error("Error adding prID %d to the test pull requests queue: %v", pr.ID, err) + log.Error("Error adding %-v to the test pull requests queue: %v", pr, err) } } @@ -69,12 +67,14 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce } if err := pr.LoadIssue(ctx); err != nil { + log.Error("Unable to load issue[%d] for %-v: %v", pr.IssueID, pr, err) return err } else if pr.Issue.IsClosed { return ErrIsClosed } if allowedMerge, err := IsUserAllowedToMerge(ctx, pr, *perm, doer); err != nil { + log.Error("Error whilst checking if %-v is allowed to merge %-v: %v", doer, pr, err) return err } else if !allowedMerge { return ErrUserNotAllowedToMerge @@ -98,15 +98,19 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce } if err := CheckPullBranchProtections(ctx, pr, false); err != nil { - if models.IsErrDisallowedToMerge(err) { - if force { - if isRepoAdmin, err2 := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); err2 != nil { - return err2 - } else if !isRepoAdmin { - return err - } - } - } else { + if !models.IsErrDisallowedToMerge(err) { + log.Error("Error whilst checking pull branch protection for %-v: %v", pr, err) + return err + } + + if !force { + return err + } + + if isRepoAdmin, err2 := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); err2 != nil { + log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, err2) + return err2 + } else if !isRepoAdmin { return err } } @@ -144,7 +148,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer // checkAndUpdateStatus checks if pull request is possible to leaving checking status, // and set to be either conflict or mergeable. func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) { - // Status is not changed to conflict means mergeable. + // If status has not been changed to conflict by testPatch then we are mergeable if pr.Status == issues_model.PullRequestStatusChecking { pr.Status = issues_model.PullRequestStatusMergeable } @@ -152,79 +156,69 @@ func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) { // Make sure there is no waiting test to process before leaving the checking status. has, err := prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10)) if err != nil { - log.Error("Unable to check if the queue is waiting to reprocess pr.ID %d. Error: %v", pr.ID, err) + log.Error("Unable to check if the queue is waiting to reprocess %-v. Error: %v", pr, err) } - if !has { - if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files"); err != nil { - log.Error("Update[%d]: %v", pr.ID, err) - } + if has { + log.Trace("Not updating status for %-v as it is due to be rechecked", pr) + return + } + + if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files"); err != nil { + log.Error("Update[%-v]: %v", pr, err) } } -// getMergeCommit checks if a pull request got merged +// getMergeCommit checks if a pull request has been merged // Returns the git.Commit of the pull request if merged func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Commit, error) { - if pr.BaseRepo == nil { - var err error - pr.BaseRepo, err = repo_model.GetRepositoryByID(ctx, pr.BaseRepoID) - if err != nil { - return nil, fmt.Errorf("GetRepositoryByID: %w", err) - } + if err := pr.LoadBaseRepo(ctx); err != nil { + return nil, fmt.Errorf("unable to load base repo for %s: %w", pr, err) } - indexTmpPath, err := os.MkdirTemp(os.TempDir(), "gitea-"+pr.BaseRepo.Name) - if err != nil { - return nil, fmt.Errorf("Failed to create temp dir for repository %s: %w", pr.BaseRepo.RepoPath(), err) - } - defer func() { - if err := util.RemoveAll(indexTmpPath); err != nil { - log.Warn("Unable to remove temporary index path: %s: Error: %v", indexTmpPath, err) - } - }() + prHeadRef := pr.GetGitRefName() - headFile := pr.GetGitRefName() - - // Check if a pull request is merged into BaseBranch - _, _, err = git.NewCommand(ctx, "merge-base", "--is-ancestor").AddDynamicArguments(headFile, pr.BaseBranch). - RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath(), Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}}) - if err != nil { - // Errors are signaled by a non-zero status that is not 1 + // Check if the pull request is merged into BaseBranch + if _, _, err := git.NewCommand(ctx, "merge-base", "--is-ancestor"). + AddDynamicArguments(prHeadRef, pr.BaseBranch). + RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath()}); err != nil { if strings.Contains(err.Error(), "exit status 1") { + // prHeadRef is not an ancestor of the base branch return nil, nil } - return nil, fmt.Errorf("git merge-base --is-ancestor: %w", err) + // Errors are signaled by a non-zero status that is not 1 + return nil, fmt.Errorf("%-v git merge-base --is-ancestor: %w", pr, err) } - commitIDBytes, err := os.ReadFile(pr.BaseRepo.RepoPath() + "/" + headFile) + // If merge-base successfully exits then prHeadRef is an ancestor of pr.BaseBranch + + // Find the head commit id + prHeadCommitID, err := git.GetFullCommitID(ctx, pr.BaseRepo.RepoPath(), prHeadRef) if err != nil { - return nil, fmt.Errorf("ReadFile(%s): %w", headFile, err) + return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err) } - commitID := string(commitIDBytes) - if len(commitID) < git.SHAFullLength { - return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID) - } - cmd := commitID[:git.SHAFullLength] + ".." + pr.BaseBranch // Get the commit from BaseBranch where the pull request got merged - mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse").AddDynamicArguments(cmd). - RunStdString(&git.RunOpts{Dir: "", Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}}) + mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse"). + AddDynamicArguments(prHeadCommitID + ".." + pr.BaseBranch). + RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath()}) if err != nil { return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %w", err) } else if len(mergeCommit) < git.SHAFullLength { // PR was maybe fast-forwarded, so just use last commit of PR - mergeCommit = commitID[:git.SHAFullLength] + mergeCommit = prHeadCommitID } + mergeCommit = strings.TrimSpace(mergeCommit) gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath()) if err != nil { - return nil, fmt.Errorf("OpenRepository: %w", err) + return nil, fmt.Errorf("%-v OpenRepository: %w", pr.BaseRepo, err) } defer gitRepo.Close() - commit, err := gitRepo.GetCommit(mergeCommit[:git.SHAFullLength]) + commit, err := gitRepo.GetCommit(mergeCommit) if err != nil { - return nil, fmt.Errorf("GetMergeCommit[%v]: %w", mergeCommit[:git.SHAFullLength], err) + return nil, fmt.Errorf("GetMergeCommit[%s]: %w", mergeCommit, err) } return commit, nil @@ -234,7 +228,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com // When a pull request got manually merged mark the pull request as merged func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("PullRequest[%d].LoadBaseRepo: %v", pr.ID, err) + log.Error("%-v LoadBaseRepo: %v", pr, err) return false } @@ -244,47 +238,50 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { return false } } else { - log.Error("PullRequest[%d].BaseRepo.GetUnit(unit.TypePullRequests): %v", pr.ID, err) + log.Error("%-v BaseRepo.GetUnit(unit.TypePullRequests): %v", pr, err) return false } commit, err := getMergeCommit(ctx, pr) if err != nil { - log.Error("PullRequest[%d].getMergeCommit: %v", pr.ID, err) + log.Error("%-v getMergeCommit: %v", pr, err) return false } - if commit != nil { - pr.MergedCommitID = commit.ID.String() - pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix()) - pr.Status = issues_model.PullRequestStatusManuallyMerged - merger, _ := user_model.GetUserByEmail(commit.Author.Email) - // When the commit author is unknown set the BaseRepo owner as merger - if merger == nil { - if pr.BaseRepo.Owner == nil { - if err = pr.BaseRepo.GetOwner(ctx); err != nil { - log.Error("BaseRepo.GetOwner[%d]: %v", pr.ID, err) - return false - } - } - merger = pr.BaseRepo.Owner - } - pr.Merger = merger - pr.MergerID = merger.ID - - if merged, err := pr.SetMerged(ctx); err != nil { - log.Error("PullRequest[%d].setMerged : %v", pr.ID, err) - return false - } else if !merged { - return false - } - - notification.NotifyMergePullRequest(ctx, merger, pr) - - log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) - return true + if commit == nil { + // no merge commit found + return false } - return false + + pr.MergedCommitID = commit.ID.String() + pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix()) + pr.Status = issues_model.PullRequestStatusManuallyMerged + merger, _ := user_model.GetUserByEmail(commit.Author.Email) + + // When the commit author is unknown set the BaseRepo owner as merger + if merger == nil { + if pr.BaseRepo.Owner == nil { + if err = pr.BaseRepo.GetOwner(ctx); err != nil { + log.Error("%-v BaseRepo.GetOwner: %v", pr, err) + return false + } + } + merger = pr.BaseRepo.Owner + } + pr.Merger = merger + pr.MergerID = merger.ID + + if merged, err := pr.SetMerged(ctx); err != nil { + log.Error("%-v setMerged : %v", pr, err) + return false + } else if !merged { + return false + } + + notification.NotifyMergePullRequest(ctx, merger, pr) + + log.Info("manuallyMerged[%-v]: Marked as manually merged into %s/%s by commit id: %s", pr, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) + return true } // InitializePullRequests checks and tests untested patches of pull requests. @@ -300,10 +297,10 @@ func InitializePullRequests(ctx context.Context) { return default: if err := prPatchCheckerQueue.PushFunc(strconv.FormatInt(prID, 10), func() error { - log.Trace("Adding PR ID: %d to the pull requests patch checking queue", prID) + log.Trace("Adding PR[%d] to the pull requests patch checking queue", prID) return nil }); err != nil { - log.Error("Error adding prID: %s to the pull requests patch checking queue %v", prID, err) + log.Error("Error adding PR[%d] to the pull requests patch checking queue %v", prID, err) } } } @@ -327,23 +324,30 @@ func testPR(id int64) { pr, err := issues_model.GetPullRequestByID(ctx, id) if err != nil { - log.Error("GetPullRequestByID[%d]: %v", id, err) + log.Error("Unable to GetPullRequestByID[%d] for testPR: %v", id, err) return } + log.Trace("Testing %-v", pr) + defer func() { + log.Trace("Done testing %-v (status: %s)", pr, pr.Status) + }() + if pr.HasMerged { + log.Trace("%-v is already merged (status: %s, merge commit: %s)", pr, pr.Status, pr.MergedCommitID) return } if manuallyMerged(ctx, pr) { + log.Trace("%-v is manually merged (status: %s, merge commit: %s)", pr, pr.Status, pr.MergedCommitID) return } if err := TestPatch(pr); err != nil { - log.Error("testPatch[%d]: %v", pr.ID, err) + log.Error("testPatch[%-v]: %v", pr, err) pr.Status = issues_model.PullRequestStatusError if err := pr.UpdateCols("status"); err != nil { - log.Error("update pr [%d] status to PullRequestStatusError failed: %v", pr.ID, err) + log.Error("update pr [%-v] status to PullRequestStatusError failed: %v", pr, err) } return } diff --git a/services/pull/patch.go b/services/pull/patch.go index 26a72a7371b..a3084196bbc 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -60,7 +60,7 @@ var patchErrorSuffices = []string{ // TestPatch will test whether a simple patch will apply func TestPatch(pr *issues_model.PullRequest) error { - ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("TestPatch: Repo[%d]#%d", pr.BaseRepoID, pr.Index)) + ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("TestPatch: %s", pr)) defer finished() // Clone base repo. From 6bc3079c0036a54d1b8ab04c5a6e14111c719c9a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 4 Feb 2023 10:30:43 +0800 Subject: [PATCH 05/20] Refactor git command package to improve security and maintainability (#22678) This PR follows #21535 (and replace #22592) ## Review without space diff https://github.com/go-gitea/gitea/pull/22678/files?diff=split&w=1 ## Purpose of this PR 1. Make git module command completely safe (risky user inputs won't be passed as argument option anymore) 2. Avoid low-level mistakes like https://github.com/go-gitea/gitea/pull/22098#discussion_r1045234918 3. Remove deprecated and dirty `CmdArgCheck` function, hide the `CmdArg` type 4. Simplify code when using git command ## The main idea of this PR * Move the `git.CmdArg` to the `internal` package, then no other package except `git` could use it. Then developers could never do `AddArguments(git.CmdArg(userInput))` any more. * Introduce `git.ToTrustedCmdArgs`, it's for user-provided and already trusted arguments. It's only used in a few cases, for example: use git arguments from config file, help unit test with some arguments. * Introduce `AddOptionValues` and `AddOptionFormat`, they make code more clear and simple: * Before: `AddArguments("-m").AddDynamicArguments(message)` * After: `AddOptionValues("-m", message)` * - * Before: `AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)))` * After: `AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email)` ## FAQ ### Why these changes were not done in #21535 ? #21535 is mainly a search&replace, it did its best to not change too much logic. Making the framework better needs a lot of changes, so this separate PR is needed as the second step. ### The naming of `AddOptionXxx` According to git's manual, the `--xxx` part is called `option`. ### How can it guarantee that `internal.CmdArg` won't be not misused? Go's specification guarantees that. Trying to access other package's internal package causes compilation error. And, `golangci-lint` also denies the git/internal package. Only the `git/command.go` can use it carefully. ### There is still a `ToTrustedCmdArgs`, will it still allow developers to make mistakes and pass untrusted arguments? Generally speaking, no. Because when using `ToTrustedCmdArgs`, the code will be very complex (see the changes for examples). Then developers and reviewers can know that something might be unreasonable. ### Why there was a `CmdArgCheck` and why it's removed? At the moment of #21535, to reduce unnecessary changes, `CmdArgCheck` was introduced as a hacky patch. Now, almost all code could be written as `cmd := NewCommand(); cmd.AddXxx(...)`, then there is no need for `CmdArgCheck` anymore. ### Why many codes for `signArg == ""` is deleted? Because in the old code, `signArg` could never be empty string, it's either `-S[key-id]` or `--no-gpg-sign`. So the `signArg == ""` is just dead code. --------- Co-authored-by: Lunny Xiao --- .golangci.yml | 1 + modules/git/command.go | 97 ++++++++++----- modules/git/command_test.go | 11 ++ modules/git/commit.go | 18 +-- modules/git/git.go | 2 +- modules/git/internal/cmdarg.go | 9 ++ modules/git/repo.go | 2 +- modules/git/repo_archive.go | 4 +- modules/git/repo_attribute.go | 40 +++--- modules/git/repo_blame.go | 8 +- modules/git/repo_branch_gogit.go | 4 +- modules/git/repo_branch_nogogit.go | 18 +-- modules/git/repo_commit.go | 100 +++++++-------- modules/git/repo_compare.go | 14 +-- modules/git/repo_stats.go | 4 +- modules/git/repo_tag.go | 4 +- modules/git/repo_tag_nogogit.go | 2 +- modules/git/repo_tree.go | 3 +- modules/git/tree_nogogit.go | 13 +- modules/gitgraph/graph.go | 10 +- modules/repository/init.go | 9 +- routers/web/repo/blame.go | 2 +- routers/web/repo/compare.go | 2 +- routers/web/repo/http.go | 6 +- routers/web/repo/lfs.go | 2 +- routers/web/repo/view.go | 2 +- services/cron/tasks_basic.go | 6 +- services/cron/tasks_extended.go | 6 +- services/gitdiff/gitdiff.go | 80 +++++------- services/gitdiff/gitdiff_test.go | 2 +- services/mirror/mirror_pull.go | 10 +- services/mirror/mirror_push.go | 4 +- services/pull/merge.go | 92 +++++--------- services/pull/patch.go | 117 +++++++++--------- services/repository/check.go | 13 +- services/repository/files/patch.go | 8 +- services/repository/files/temp_repo.go | 14 +-- services/repository/files/update.go | 2 +- services/repository/files/upload.go | 2 +- .../git_helper_for_declarative_test.go | 20 +-- tests/integration/git_test.go | 2 +- tests/integration/pull_merge_test.go | 2 +- 42 files changed, 382 insertions(+), 385 deletions(-) create mode 100644 modules/git/internal/cmdarg.go diff --git a/.golangci.yml b/.golangci.yml index caa237370e3..aa04e0a8efd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -84,6 +84,7 @@ linters-settings: - github.com/unknwon/com: "use gitea's util and replacements" - io/ioutil: "use os or io instead" - golang.org/x/exp: "it's experimental and unreliable." + - code.gitea.io/gitea/modules/git/internal: "do not use the internal package, use AddXxx function instead" issues: max-issues-per-linter: 0 diff --git a/modules/git/command.go b/modules/git/command.go index d88fcd1a8c8..0bc8103116a 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -16,14 +16,20 @@ import ( "time" "unsafe" + "code.gitea.io/gitea/modules/git/internal" //nolint:depguard // only this file can use the internal type CmdArg, other files and packages should use AddXxx functions "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/util" ) +// TrustedCmdArgs returns the trusted arguments for git command. +// It's mainly for passing user-provided and trusted arguments to git command +// In most cases, it shouldn't be used. Use AddXxx function instead +type TrustedCmdArgs []internal.CmdArg + var ( // globalCommandArgs global command args for external package setting - globalCommandArgs []CmdArg + globalCommandArgs TrustedCmdArgs // defaultCommandExecutionTimeout default command execution timeout duration defaultCommandExecutionTimeout = 360 * time.Second @@ -42,8 +48,6 @@ type Command struct { brokenArgs []string } -type CmdArg string - func (c *Command) String() string { if len(c.args) == 0 { return c.name @@ -53,7 +57,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 ...CmdArg) *Command { +func NewCommand(ctx context.Context, args ...internal.CmdArg) *Command { // Make an explicit copy of globalCommandArgs, otherwise append might overwrite it cargs := make([]string, 0, len(globalCommandArgs)+len(args)) for _, arg := range globalCommandArgs { @@ -70,15 +74,9 @@ func NewCommand(ctx context.Context, args ...CmdArg) *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 ...CmdArg) *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 ...CmdArg) *Command { +func NewCommandContextNoGlobals(ctx context.Context, args ...internal.CmdArg) *Command { cargs := make([]string, 0, len(args)) for _, arg := range args { cargs = append(cargs, string(arg)) @@ -96,27 +94,70 @@ func (c *Command) SetParentContext(ctx context.Context) *Command { return c } -// SetDescription sets the description for this command which be returned on -// c.String() +// SetDescription sets the description for this command which be returned on c.String() func (c *Command) SetDescription(desc string) *Command { c.desc = desc return c } -// AddArguments adds new git 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 ...CmdArg) *Command { +// isSafeArgumentValue checks if the argument is safe to be used as a value (not an option) +func isSafeArgumentValue(s string) bool { + return s == "" || s[0] != '-' +} + +// isValidArgumentOption checks if the argument is a valid option (starting with '-'). +// It doesn't check whether the option is supported or not +func isValidArgumentOption(s string) bool { + return s != "" && s[0] == '-' +} + +// AddArguments adds new git arguments (option/value) to the command. It only accepts string literals, or trusted CmdArg. +// Type CmdArg is in the internal package, so it can not be used outside of this package directly, +// it makes sure that user-provided arguments won't cause RCE risks. +// User-provided arguments should be passed by other AddXxx functions +func (c *Command) AddArguments(args ...internal.CmdArg) *Command { for _, arg := range args { c.args = append(c.args, string(arg)) } 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 +// AddOptionValues adds a new option with a list of non-option values +// For example: AddOptionValues("--opt", val) means 2 arguments: {"--opt", val}. +// The values are treated as dynamic argument values. It equals to: AddArguments("--opt") then AddDynamicArguments(val). +func (c *Command) AddOptionValues(opt internal.CmdArg, args ...string) *Command { + if !isValidArgumentOption(string(opt)) { + c.brokenArgs = append(c.brokenArgs, string(opt)) + return c + } + c.args = append(c.args, string(opt)) + c.AddDynamicArguments(args...) + return c +} + +// AddOptionFormat adds a new option with a format string and arguments +// For example: AddOptionFormat("--opt=%s %s", val1, val2) means 1 argument: {"--opt=val1 val2"}. +func (c *Command) AddOptionFormat(opt string, args ...any) *Command { + if !isValidArgumentOption(opt) { + c.brokenArgs = append(c.brokenArgs, opt) + return c + } + // a quick check to make sure the format string matches the number of arguments, to find low-level mistakes ASAP + if strings.Count(strings.ReplaceAll(opt, "%%", ""), "%") != len(args) { + c.brokenArgs = append(c.brokenArgs, opt) + return c + } + s := fmt.Sprintf(opt, args...) + c.args = append(c.args, s) + return c +} + +// AddDynamicArguments adds new dynamic argument values to the command. +// The arguments may come from user input and can not be trusted, so no leading '-' is allowed to avoid passing options. +// TODO: in the future, this function can be renamed to AddArgumentValues func (c *Command) AddDynamicArguments(args ...string) *Command { for _, arg := range args { - if arg != "" && arg[0] == '-' { + if !isSafeArgumentValue(arg) { c.brokenArgs = append(c.brokenArgs, arg) } } @@ -137,14 +178,14 @@ func (c *Command) AddDashesAndList(list ...string) *Command { return c } -// CmdArgCheck checks whether the string is safe to be used as a dynamic argument. -// It panics if the check fails. Usually it should not be used, it's just for refactoring purpose -// deprecated -func CmdArgCheck(s string) CmdArg { - if s != "" && s[0] == '-' { - panic("invalid git cmd argument: " + s) +// ToTrustedCmdArgs converts a list of strings (trusted as argument) to TrustedCmdArgs +// In most cases, it shouldn't be used. Use AddXxx function instead +func ToTrustedCmdArgs(args []string) TrustedCmdArgs { + ret := make(TrustedCmdArgs, len(args)) + for i, arg := range args { + ret[i] = internal.CmdArg(arg) } - return CmdArg(s) + return ret } // RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored. @@ -364,9 +405,9 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS } // AllowLFSFiltersArgs return globalCommandArgs with lfs filter, it should only be used for tests -func AllowLFSFiltersArgs() []CmdArg { +func AllowLFSFiltersArgs() TrustedCmdArgs { // Now here we should explicitly allow lfs filters to run - filteredLFSGlobalArgs := make([]CmdArg, len(globalCommandArgs)) + filteredLFSGlobalArgs := make(TrustedCmdArgs, len(globalCommandArgs)) j := 0 for _, arg := range globalCommandArgs { if strings.Contains(string(arg), "lfs") { diff --git a/modules/git/command_test.go b/modules/git/command_test.go index 2dca2d0d344..4e5f991d311 100644 --- a/modules/git/command_test.go +++ b/modules/git/command_test.go @@ -41,3 +41,14 @@ func TestRunWithContextStd(t *testing.T) { assert.Empty(t, stderr) assert.Contains(t, stdout, "git version") } + +func TestGitArgument(t *testing.T) { + assert.True(t, isValidArgumentOption("-x")) + assert.True(t, isValidArgumentOption("--xx")) + assert.False(t, isValidArgumentOption("")) + assert.False(t, isValidArgumentOption("x")) + + assert.True(t, isSafeArgumentValue("")) + assert.True(t, isSafeArgumentValue("x")) + assert.False(t, isSafeArgumentValue("-x")) +} diff --git a/modules/git/commit.go b/modules/git/commit.go index 14710de6121..1f6289ed022 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "errors" - "fmt" "io" "os/exec" "strconv" @@ -91,8 +90,8 @@ func AddChanges(repoPath string, all bool, files ...string) error { } // AddChangesWithArgs marks local changes to be ready for commit. -func AddChangesWithArgs(repoPath string, globalArgs []CmdArg, all bool, files ...string) error { - cmd := NewCommandNoGlobals(append(globalArgs, "add")...) +func AddChangesWithArgs(repoPath string, globalArgs TrustedCmdArgs, all bool, files ...string) error { + cmd := NewCommandContextNoGlobals(DefaultContext, globalArgs...).AddArguments("add") if all { cmd.AddArguments("--all") } @@ -111,17 +110,18 @@ type CommitChangesOptions struct { // CommitChanges commits local changes with given committer, author and message. // If author is nil, it will be the same as committer. func CommitChanges(repoPath string, opts CommitChangesOptions) error { - cargs := make([]CmdArg, len(globalCommandArgs)) + cargs := make(TrustedCmdArgs, len(globalCommandArgs)) copy(cargs, globalCommandArgs) return CommitChangesWithArgs(repoPath, cargs, opts) } // CommitChangesWithArgs commits local changes with given committer, author and message. // If author is nil, it will be the same as committer. -func CommitChangesWithArgs(repoPath string, args []CmdArg, opts CommitChangesOptions) error { - cmd := NewCommandNoGlobals(args...) +func CommitChangesWithArgs(repoPath string, args TrustedCmdArgs, opts CommitChangesOptions) error { + cmd := NewCommandContextNoGlobals(DefaultContext, args...) if opts.Committer != nil { - cmd.AddArguments("-c", CmdArg("user.name="+opts.Committer.Name), "-c", CmdArg("user.email="+opts.Committer.Email)) + cmd.AddOptionValues("-c", "user.name="+opts.Committer.Name) + cmd.AddOptionValues("-c", "user.email="+opts.Committer.Email) } cmd.AddArguments("commit") @@ -129,9 +129,9 @@ func CommitChangesWithArgs(repoPath string, args []CmdArg, opts CommitChangesOpt opts.Author = opts.Committer } if opts.Author != nil { - cmd.AddArguments(CmdArg(fmt.Sprintf("--author='%s <%s>'", opts.Author.Name, opts.Author.Email))) + cmd.AddOptionFormat("--author='%s <%s>'", opts.Author.Name, opts.Author.Email) } - cmd.AddArguments("-m").AddDynamicArguments(opts.Message) + cmd.AddOptionValues("-m", opts.Message) _, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) // No stderr but exit status 1 means nothing to commit. diff --git a/modules/git/git.go b/modules/git/git.go index f5919d82dca..2feb242ac5d 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -383,6 +383,6 @@ func configUnsetAll(key, value string) error { } // Fsck verifies the connectivity and validity of the objects in the database -func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args ...CmdArg) error { +func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args TrustedCmdArgs) error { return NewCommand(ctx, "fsck").AddArguments(args...).Run(&RunOpts{Timeout: timeout, Dir: repoPath}) } diff --git a/modules/git/internal/cmdarg.go b/modules/git/internal/cmdarg.go new file mode 100644 index 00000000000..f8f3c2011ea --- /dev/null +++ b/modules/git/internal/cmdarg.go @@ -0,0 +1,9 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package internal + +// CmdArg represents a command argument for git command, and it will be used for the git command directly without any further processing. +// In most cases, you should use the "AddXxx" functions to add arguments, but not use this type directly. +// Casting a risky (user-provided) string to CmdArg would cause security issues if it's injected with a "--xxx" argument. +type CmdArg string diff --git a/modules/git/repo.go b/modules/git/repo.go index 4ba40d20af0..e77a3a6ad8e 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -115,7 +115,7 @@ func Clone(ctx context.Context, from, to string, opts CloneRepoOptions) error { } // CloneWithArgs original repository to target path. -func CloneWithArgs(ctx context.Context, args []CmdArg, from, to string, opts CloneRepoOptions) (err error) { +func CloneWithArgs(ctx context.Context, args TrustedCmdArgs, from, to string, opts CloneRepoOptions) (err error) { toDir := path.Dir(to) if err = os.MkdirAll(toDir, os.ModePerm); err != nil { return err diff --git a/modules/git/repo_archive.go b/modules/git/repo_archive.go index cff9724f005..2b45a50f191 100644 --- a/modules/git/repo_archive.go +++ b/modules/git/repo_archive.go @@ -57,9 +57,9 @@ func (repo *Repository) CreateArchive(ctx context.Context, format ArchiveType, t cmd := NewCommand(ctx, "archive") if usePrefix { - cmd.AddArguments(CmdArg("--prefix=" + filepath.Base(strings.TrimSuffix(repo.Path, ".git")) + "/")) + cmd.AddOptionFormat("--prefix=%s", filepath.Base(strings.TrimSuffix(repo.Path, ".git"))+"/") } - cmd.AddArguments(CmdArg("--format=" + format.String())) + cmd.AddOptionFormat("--format=%s", format.String()) cmd.AddDynamicArguments(commitID) var stderr strings.Builder diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 404d9e502c0..e7d5fb68068 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -17,7 +17,7 @@ import ( type CheckAttributeOpts struct { CachedOnly bool AllAttributes bool - Attributes []CmdArg + Attributes []string Filenames []string IndexFile string WorkTree string @@ -48,7 +48,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[ } else { for _, attribute := range opts.Attributes { if attribute != "" { - cmd.AddArguments(attribute) + cmd.AddDynamicArguments(attribute) } } } @@ -95,7 +95,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[ // CheckAttributeReader provides a reader for check-attribute content that can be long running type CheckAttributeReader struct { // params - Attributes []CmdArg + Attributes []string Repo *Repository IndexFile string WorkTree string @@ -111,19 +111,6 @@ type CheckAttributeReader struct { // Init initializes the CheckAttributeReader func (c *CheckAttributeReader) Init(ctx context.Context) error { - cmdArgs := []CmdArg{"check-attr", "--stdin", "-z"} - - if len(c.IndexFile) > 0 { - cmdArgs = append(cmdArgs, "--cached") - c.env = append(c.env, "GIT_INDEX_FILE="+c.IndexFile) - } - - if len(c.WorkTree) > 0 { - c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree) - } - - c.env = append(c.env, "GIT_FLUSH=1") - if len(c.Attributes) == 0 { lw := new(nulSeparatedAttributeWriter) lw.attributes = make(chan attributeTriple) @@ -134,11 +121,22 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error { return fmt.Errorf("no provided Attributes to check") } - cmdArgs = append(cmdArgs, c.Attributes...) - cmdArgs = append(cmdArgs, "--") - c.ctx, c.cancel = context.WithCancel(ctx) - c.cmd = NewCommand(c.ctx, cmdArgs...) + c.cmd = NewCommand(c.ctx, "check-attr", "--stdin", "-z") + + if len(c.IndexFile) > 0 { + c.cmd.AddArguments("--cached") + c.env = append(c.env, "GIT_INDEX_FILE="+c.IndexFile) + } + + if len(c.WorkTree) > 0 { + c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree) + } + + c.env = append(c.env, "GIT_FLUSH=1") + + // The empty "--" comes from #16773 , and it seems unnecessary because nothing else would be added later. + c.cmd.AddDynamicArguments(c.Attributes...).AddArguments("--") var err error @@ -294,7 +292,7 @@ func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeRe } checker := &CheckAttributeReader{ - Attributes: []CmdArg{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, Repo: repo, IndexFile: indexFilename, WorkTree: worktree, diff --git a/modules/git/repo_blame.go b/modules/git/repo_blame.go index 7f44735f9f9..e25efa2d3bf 100644 --- a/modules/git/repo_blame.go +++ b/modules/git/repo_blame.go @@ -3,7 +3,9 @@ package git -import "fmt" +import ( + "fmt" +) // FileBlame return the Blame object of file func (repo *Repository) FileBlame(revision, path, file string) ([]byte, error) { @@ -14,8 +16,8 @@ func (repo *Repository) FileBlame(revision, path, file string) ([]byte, error) { // LineBlame returns the latest commit at the given line func (repo *Repository) LineBlame(revision, path, file string, line uint) (*Commit, error) { res, _, err := NewCommand(repo.Ctx, "blame"). - AddArguments(CmdArg(fmt.Sprintf("-L %d,%d", line, line))). - AddArguments("-p").AddDynamicArguments(revision). + AddOptionFormat("-L %d,%d", line, line). + AddOptionValues("-p", revision). AddDashesAndList(file).RunStdString(&RunOpts{Dir: path}) if err != nil { return nil, err diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index ca19d3827e5..f9896a7a090 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -50,8 +50,8 @@ func (repo *Repository) IsBranchExist(name string) bool { return reference.Type() != plumbing.InvalidReference } -// GetBranches returns branches from the repository, skipping skip initial branches and -// returning at most limit branches, or all branches if limit is 0. +// GetBranches returns branches from the repository, skipping "skip" initial branches and +// returning at most "limit" branches, or all branches if "limit" is 0. func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { var branchNames []string diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 7559513c9b4..b1e7c8b73e6 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -59,10 +59,10 @@ func (repo *Repository) IsBranchExist(name string) bool { return repo.IsReferenceExist(BranchPrefix + name) } -// GetBranchNames returns branches from the repository, skipping skip initial branches and -// returning at most limit branches, or all branches if limit is 0. +// GetBranchNames returns branches from the repository, skipping "skip" initial branches and +// returning at most "limit" branches, or all branches if "limit" is 0. func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { - return callShowRef(repo.Ctx, repo.Path, BranchPrefix, []CmdArg{BranchPrefix, "--sort=-committerdate"}, skip, limit) + return callShowRef(repo.Ctx, repo.Path, BranchPrefix, TrustedCmdArgs{BranchPrefix, "--sort=-committerdate"}, skip, limit) } // WalkReferences walks all the references from the repository @@ -73,19 +73,19 @@ func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refn // WalkReferences walks all the references from the repository // refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty. func (repo *Repository) WalkReferences(refType ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) { - var args []CmdArg + var args TrustedCmdArgs switch refType { case ObjectTag: - args = []CmdArg{TagPrefix, "--sort=-taggerdate"} + args = TrustedCmdArgs{TagPrefix, "--sort=-taggerdate"} case ObjectBranch: - args = []CmdArg{BranchPrefix, "--sort=-committerdate"} + args = TrustedCmdArgs{BranchPrefix, "--sort=-committerdate"} } return walkShowRef(repo.Ctx, repo.Path, args, skip, limit, walkfn) } // callShowRef return refs, if limit = 0 it will not limit -func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []CmdArg, skip, limit int) (branchNames []string, countAll int, err error) { +func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs TrustedCmdArgs, skip, limit int) (branchNames []string, countAll int, err error) { countAll, err = walkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) error { branchName = strings.TrimPrefix(branchName, trimPrefix) branchNames = append(branchNames, branchName) @@ -95,7 +95,7 @@ func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []C return branchNames, countAll, err } -func walkShowRef(ctx context.Context, repoPath string, extraArgs []CmdArg, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { +func walkShowRef(ctx context.Context, repoPath string, extraArgs TrustedCmdArgs, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { stdoutReader, stdoutWriter := io.Pipe() defer func() { _ = stdoutReader.Close() @@ -104,7 +104,7 @@ func walkShowRef(ctx context.Context, repoPath string, extraArgs []CmdArg, skip, go func() { stderrBuilder := &strings.Builder{} - args := []CmdArg{"for-each-ref", "--format=%(objectname) %(refname)"} + args := TrustedCmdArgs{"for-each-ref", "--format=%(objectname) %(refname)"} args = append(args, extraArgs...) err := NewCommand(ctx, args...).Run(&RunOpts{ Dir: repoPath, diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 8343e348435..a62e0670fed 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -89,7 +89,7 @@ func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) ([]*Commit, error) { stdout, _, err := NewCommand(repo.Ctx, "log"). - AddArguments(CmdArg("--skip="+strconv.Itoa((page-1)*pageSize)), CmdArg("--max-count="+strconv.Itoa(pageSize)), prettyLogFormat). + AddOptionFormat("--skip=%d", (page-1)*pageSize).AddOptionFormat("--max-count=%d", pageSize).AddArguments(prettyLogFormat). AddDynamicArguments(id.String()). RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { @@ -99,32 +99,36 @@ func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) ([]*Commit, } func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Commit, error) { - // create new git log command with limit of 100 commis + // add common arguments to git command + addCommonSearchArgs := func(c *Command) { + // ignore case + c.AddArguments("-i") + + // add authors if present in search query + if len(opts.Authors) > 0 { + for _, v := range opts.Authors { + c.AddOptionFormat("--author=%s", v) + } + } + + // add committers if present in search query + if len(opts.Committers) > 0 { + for _, v := range opts.Committers { + c.AddOptionFormat("--committer=%s", v) + } + } + + // add time constraints if present in search query + if len(opts.After) > 0 { + c.AddOptionFormat("--after=%s", opts.After) + } + if len(opts.Before) > 0 { + c.AddOptionFormat("--before=%s", opts.Before) + } + } + + // create new git log command with limit of 100 commits cmd := NewCommand(repo.Ctx, "log", "-100", prettyLogFormat).AddDynamicArguments(id.String()) - // ignore case - args := []CmdArg{"-i"} - - // add authors if present in search query - if len(opts.Authors) > 0 { - for _, v := range opts.Authors { - args = append(args, CmdArg("--author="+v)) - } - } - - // add committers if present in search query - if len(opts.Committers) > 0 { - for _, v := range opts.Committers { - args = append(args, CmdArg("--committer="+v)) - } - } - - // add time constraints if present in search query - if len(opts.After) > 0 { - args = append(args, CmdArg("--after="+opts.After)) - } - if len(opts.Before) > 0 { - args = append(args, CmdArg("--before="+opts.Before)) - } // pretend that all refs along with HEAD were listed on command line as // https://git-scm.com/docs/git-log#Documentation/git-log.txt---all @@ -137,12 +141,12 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // note this is done only for command created above if len(opts.Keywords) > 0 { for _, v := range opts.Keywords { - cmd.AddArguments(CmdArg("--grep=" + v)) + cmd.AddOptionFormat("--grep=%s", v) } } // search for commits matching given constraints and keywords in commit msg - cmd.AddArguments(args...) + addCommonSearchArgs(cmd) stdout, _, err := cmd.RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err @@ -160,7 +164,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // create new git log command with 1 commit limit hashCmd := NewCommand(repo.Ctx, "log", "-1", prettyLogFormat) // add previous arguments except for --grep and --all - hashCmd.AddArguments(args...) + addCommonSearchArgs(hashCmd) // add keyword as hashCmd.AddDynamicArguments(v) @@ -213,8 +217,8 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( go func() { stderr := strings.Builder{} gitCmd := NewCommand(repo.Ctx, "rev-list"). - AddArguments(CmdArg("--max-count=" + strconv.Itoa(setting.Git.CommitsRangeSize*page))). - AddArguments(CmdArg("--skip=" + strconv.Itoa(skip))) + AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize*page). + AddOptionFormat("--skip=%d", skip) gitCmd.AddDynamicArguments(revision) gitCmd.AddDashesAndList(file) err := gitCmd.Run(&RunOpts{ @@ -295,21 +299,21 @@ func (repo *Repository) CommitsBetweenLimit(last, before *Commit, limit, skip in var stdout []byte var err error if before == nil { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", - "--max-count", CmdArg(strconv.Itoa(limit)), - "--skip", CmdArg(strconv.Itoa(skip))). + stdout, _, err = NewCommand(repo.Ctx, "rev-list"). + AddOptionValues("--max-count", strconv.Itoa(limit)). + AddOptionValues("--skip", strconv.Itoa(skip)). AddDynamicArguments(last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } else { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", - "--max-count", CmdArg(strconv.Itoa(limit)), - "--skip", CmdArg(strconv.Itoa(skip))). + stdout, _, err = NewCommand(repo.Ctx, "rev-list"). + AddOptionValues("--max-count", strconv.Itoa(limit)). + AddOptionValues("--skip", strconv.Itoa(skip)). AddDynamicArguments(before.ID.String() + ".." + last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil && strings.Contains(err.Error(), "no merge base") { // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. // previously it would return the results of git rev-list --max-count n before last so let's try that... - stdout, _, err = NewCommand(repo.Ctx, "rev-list", - "--max-count", CmdArg(strconv.Itoa(limit)), - "--skip", CmdArg(strconv.Itoa(skip))). + stdout, _, err = NewCommand(repo.Ctx, "rev-list"). + AddOptionValues("--max-count", strconv.Itoa(limit)). + AddOptionValues("--skip", strconv.Itoa(skip)). AddDynamicArguments(before.ID.String(), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } } @@ -349,12 +353,11 @@ func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { // commitsBefore the limit is depth, not total number of returned commits. func (repo *Repository) commitsBefore(id SHA1, limit int) ([]*Commit, error) { - cmd := NewCommand(repo.Ctx, "log") + cmd := NewCommand(repo.Ctx, "log", prettyLogFormat) if limit > 0 { - cmd.AddArguments(CmdArg("-"+strconv.Itoa(limit)), prettyLogFormat).AddDynamicArguments(id.String()) - } else { - cmd.AddArguments(prettyLogFormat).AddDynamicArguments(id.String()) + cmd.AddOptionFormat("-%d", limit) } + cmd.AddDynamicArguments(id.String()) stdout, _, runErr := cmd.RunStdBytes(&RunOpts{Dir: repo.Path}) if runErr != nil { @@ -393,10 +396,9 @@ func (repo *Repository) getCommitsBeforeLimit(id SHA1, num int) ([]*Commit, erro func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) { if CheckGitVersionAtLeast("2.7.0") == nil { - stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", - CmdArg("--count="+strconv.Itoa(limit)), - "--format=%(refname:strip=2)", "--contains"). - AddDynamicArguments(commit.ID.String(), BranchPrefix). + stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", "--format=%(refname:strip=2)"). + AddOptionFormat("--count=%d", limit). + AddOptionValues("--contains", commit.ID.String(), BranchPrefix). RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err @@ -406,7 +408,7 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) return branches, nil } - stdout, _, err := NewCommand(repo.Ctx, "branch", "--contains").AddDynamicArguments(commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "branch").AddOptionValues("--contains", commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index b1b55c88a44..9a4d66f2f52 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -172,25 +172,21 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis // GetDiffShortStat counts number of changed files, number of additions and deletions func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) { - numFiles, totalAdditions, totalDeletions, err = GetDiffShortStat(repo.Ctx, repo.Path, CmdArgCheck(base+"..."+head)) + numFiles, totalAdditions, totalDeletions, err = GetDiffShortStat(repo.Ctx, repo.Path, nil, base+"..."+head) if err != nil && strings.Contains(err.Error(), "no merge base") { - return GetDiffShortStat(repo.Ctx, repo.Path, CmdArgCheck(base), CmdArgCheck(head)) + return GetDiffShortStat(repo.Ctx, repo.Path, nil, base, head) } return numFiles, totalAdditions, totalDeletions, err } // GetDiffShortStat counts number of changed files, number of additions and deletions -func GetDiffShortStat(ctx context.Context, repoPath string, args ...CmdArg) (numFiles, totalAdditions, totalDeletions int, err error) { +func GetDiffShortStat(ctx context.Context, repoPath string, trustedArgs TrustedCmdArgs, dynamicArgs ...string) (numFiles, totalAdditions, totalDeletions int, err error) { // Now if we call: // $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875 // we get: // " 9902 files changed, 2034198 insertions(+), 298800 deletions(-)\n" - args = append([]CmdArg{ - "diff", - "--shortstat", - }, args...) - - stdout, _, err := NewCommand(ctx, args...).RunStdString(&RunOpts{Dir: repoPath}) + cmd := NewCommand(ctx, "diff", "--shortstat").AddArguments(trustedArgs...).AddDynamicArguments(dynamicArgs...) + stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) if err != nil { return 0, 0, 0, err } diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index d6e91f25a9a..41f94e24f99 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -40,7 +40,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) since := fromTime.Format(time.RFC3339) - stdout, _, runErr := NewCommand(repo.Ctx, "rev-list", "--count", "--no-merges", "--branches=*", "--date=iso", CmdArg(fmt.Sprintf("--since='%s'", since))).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand(repo.Ctx, "rev-list", "--count", "--no-merges", "--branches=*", "--date=iso").AddOptionFormat("--since='%s'", since).RunStdString(&RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -60,7 +60,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) _ = stdoutWriter.Close() }() - gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", CmdArg(fmt.Sprintf("--since='%s'", since))) + gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso").AddOptionFormat("--since='%s'", since) if len(branch) == 0 { gitCmd.AddArguments("--branches=*") } else { diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 8aa06545d4c..ae877f02110 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -121,7 +121,9 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { rc := &RunOpts{Dir: repo.Path, Stdout: stdoutWriter, Stderr: &stderr} go func() { - err := NewCommand(repo.Ctx, "for-each-ref", CmdArg("--format="+forEachRefFmt.Flag()), "--sort", "-*creatordate", "refs/tags").Run(rc) + err := NewCommand(repo.Ctx, "for-each-ref"). + AddOptionFormat("--format=%s", forEachRefFmt.Flag()). + AddArguments("--sort", "-*creatordate", "refs/tags").Run(rc) if err != nil { _ = stdoutWriter.CloseWithError(ConcatenateError(err, stderr.String())) } else { diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index d3331cf9b73..9080ffcfd78 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/modules/git/repo_tag_nogogit.go @@ -25,7 +25,7 @@ func (repo *Repository) IsTagExist(name string) bool { // GetTags returns all tags of the repository. // returning at most limit tags, or all if limit is 0. func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) { - tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, []CmdArg{TagPrefix, "--sort=-taggerdate"}, skip, limit) + tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, TrustedCmdArgs{TagPrefix, "--sort=-taggerdate"}, skip, limit) return tags, err } diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 5fea5c0aea4..63c33379bf5 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -6,7 +6,6 @@ package git import ( "bytes" - "fmt" "os" "strings" "time" @@ -45,7 +44,7 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString("\n") if opts.KeyID != "" || opts.AlwaysSign { - cmd.AddArguments(CmdArg(fmt.Sprintf("-S%s", opts.KeyID))) + cmd.AddOptionFormat("-S%s", opts.KeyID) } if opts.NoGPGSign { diff --git a/modules/git/tree_nogogit.go b/modules/git/tree_nogogit.go index 185317e7a70..ef598d7e913 100644 --- a/modules/git/tree_nogogit.go +++ b/modules/git/tree_nogogit.go @@ -100,14 +100,15 @@ func (t *Tree) ListEntries() (Entries, error) { // listEntriesRecursive returns all entries of current tree recursively including all subtrees // extraArgs could be "-l" to get the size, which is slower -func (t *Tree) listEntriesRecursive(extraArgs ...CmdArg) (Entries, error) { +func (t *Tree) listEntriesRecursive(extraArgs TrustedCmdArgs) (Entries, error) { if t.entriesRecursiveParsed { return t.entriesRecursive, nil } - args := append([]CmdArg{"ls-tree", "-t", "-r"}, extraArgs...) - args = append(args, CmdArg(t.ID.String())) - stdout, _, runErr := NewCommand(t.repo.Ctx, args...).RunStdBytes(&RunOpts{Dir: t.repo.Path}) + stdout, _, runErr := NewCommand(t.repo.Ctx, "ls-tree", "-t", "-r"). + AddArguments(extraArgs...). + AddDynamicArguments(t.ID.String()). + RunStdBytes(&RunOpts{Dir: t.repo.Path}) if runErr != nil { return nil, runErr } @@ -123,10 +124,10 @@ func (t *Tree) listEntriesRecursive(extraArgs ...CmdArg) (Entries, error) { // ListEntriesRecursiveFast returns all entries of current tree recursively including all subtrees, no size func (t *Tree) ListEntriesRecursiveFast() (Entries, error) { - return t.listEntriesRecursive() + return t.listEntriesRecursive(nil) } // ListEntriesRecursiveWithSize returns all entries of current tree recursively including all subtrees, with size func (t *Tree) ListEntriesRecursiveWithSize() (Entries, error) { - return t.listEntriesRecursive("--long") + return t.listEntriesRecursive(TrustedCmdArgs{"--long"}) } diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go index baedfe59806..331ad6b218d 100644 --- a/modules/gitgraph/graph.go +++ b/modules/gitgraph/graph.go @@ -7,7 +7,6 @@ import ( "bufio" "bytes" "context" - "fmt" "os" "strings" @@ -33,12 +32,9 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo graphCmd.AddArguments("--all") } - graphCmd.AddArguments( - "-C", - "-M", - git.CmdArg(fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page)), - "--date=iso", - git.CmdArg(fmt.Sprintf("--pretty=format:%s", format))) + graphCmd.AddArguments("-C", "-M", "--date=iso"). + AddOptionFormat("-n %d", setting.UI.GraphMaxCommitNum*page). + AddOptionFormat("--pretty=format:%s", format) if len(branches) > 0 { graphCmd.AddDynamicArguments(branches...) diff --git a/modules/repository/init.go b/modules/repository/init.go index 2b0d0be7bc9..5705fe5b998 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -316,14 +316,13 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi return fmt.Errorf("git add --all: %w", err) } - cmd := git.NewCommand(ctx, - "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), - "-m", "Initial commit", - ) + cmd := git.NewCommand(ctx, "commit"). + AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). + AddOptionValues("-m", "Initial commit") sign, keyID, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) if sign { - cmd.AddArguments(git.CmdArg("-S" + keyID)) + cmd.AddOptionFormat("-S%s", keyID) if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { // need to set the committer to the KeyID owner diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 50bfa9d3bd2..def7cfcadbe 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -217,7 +217,7 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m filename2attribute2info, err := ctx.Repo.GitRepo.CheckAttribute(git.CheckAttributeOpts{ CachedOnly: true, - Attributes: []git.CmdArg{"linguist-language", "gitlab-language"}, + Attributes: []string{"linguist-language", "gitlab-language"}, Filenames: []string{ctx.Repo.TreePath}, IndexFile: indexFilename, WorkTree: worktree, diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 35f923d561e..c4b8c814e67 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -560,7 +560,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { func PrepareCompareDiff( ctx *context.Context, ci *CompareInfo, - whitespaceBehavior git.CmdArg, + whitespaceBehavior git.TrustedCmdArgs, ) bool { var ( repo = ctx.Repo.Repository diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 2c91ed6178e..89c86e764e3 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -498,7 +498,8 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { } var stderr bytes.Buffer - cmd := git.NewCommand(h.r.Context(), git.CmdArgCheck(service), "--stateless-rpc").AddDynamicArguments(h.dir) + // the service is generated by ourselves, so it's safe to trust it + cmd := git.NewCommand(h.r.Context(), git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc").AddDynamicArguments(h.dir) cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir)) if err := cmd.Run(&git.RunOpts{ Dir: h.dir, @@ -570,7 +571,8 @@ func GetInfoRefs(ctx *context.Context) { } h.environ = append(os.Environ(), h.environ...) - refs, _, err := git.NewCommand(ctx, git.CmdArgCheck(service), "--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) + // the service is generated by ourselves, so we can trust it + refs, _, err := git.NewCommand(ctx, git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) if err != nil { log.Error(fmt.Sprintf("%v - %s", err, string(refs))) } diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go index 41d5edcdd8c..869a69c3772 100644 --- a/routers/web/repo/lfs.go +++ b/routers/web/repo/lfs.go @@ -146,7 +146,7 @@ func LFSLocks(ctx *context.Context) { } name2attribute2info, err := gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []git.CmdArg{"lockable"}, + Attributes: []string{"lockable"}, Filenames: filenames, CachedOnly: true, }) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 769e4e895cd..f314902374c 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -509,7 +509,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st filename2attribute2info, err := ctx.Repo.GitRepo.CheckAttribute(git.CheckAttributeOpts{ CachedOnly: true, - Attributes: []git.CmdArg{"linguist-language", "gitlab-language"}, + Attributes: []string{"linguist-language", "gitlab-language"}, Filenames: []string{ctx.Repo.TreePath}, IndexFile: indexFilename, WorkTree: worktree, diff --git a/services/cron/tasks_basic.go b/services/cron/tasks_basic.go index 05aef6623d4..aad0e395912 100644 --- a/services/cron/tasks_basic.go +++ b/services/cron/tasks_basic.go @@ -59,11 +59,7 @@ func registerRepoHealthCheck() { }, func(ctx context.Context, _ *user_model.User, config Config) error { rhcConfig := config.(*RepoHealthCheckConfig) // the git args are set by config, they can be safe to be trusted - args := make([]git.CmdArg, 0, len(rhcConfig.Args)) - for _, arg := range rhcConfig.Args { - args = append(args, git.CmdArg(arg)) - } - return repo_service.GitFsckRepos(ctx, rhcConfig.Timeout, args) + return repo_service.GitFsckRepos(ctx, rhcConfig.Timeout, git.ToTrustedCmdArgs(rhcConfig.Args)) }) } diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 520d940edf3..3e0dbd132eb 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -61,11 +61,7 @@ func registerGarbageCollectRepositories() { }, func(ctx context.Context, _ *user_model.User, config Config) error { rhcConfig := config.(*RepoHealthCheckConfig) // the git args are set by config, they can be safe to be trusted - args := make([]git.CmdArg, 0, len(rhcConfig.Args)) - for _, arg := range rhcConfig.Args { - args = append(args, git.CmdArg(arg)) - } - return repo_service.GitGcRepos(ctx, rhcConfig.Timeout, args...) + return repo_service.GitGcRepos(ctx, rhcConfig.Timeout, git.ToTrustedCmdArgs(rhcConfig.Args)) }) } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index d3ee93ec945..4a74c1a8944 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1056,7 +1056,7 @@ type DiffOptions struct { MaxLines int MaxLineCharacters int MaxFiles int - WhitespaceBehavior git.CmdArg + WhitespaceBehavior git.TrustedCmdArgs DirectComparison bool } @@ -1071,38 +1071,22 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff return nil, err } - argsLength := 6 - if len(opts.WhitespaceBehavior) > 0 { - argsLength++ - } - if len(opts.SkipTo) > 0 { - argsLength++ - } - if len(files) > 0 { - argsLength += len(files) + 1 - } - - diffArgs := make([]git.CmdArg, 0, argsLength) + cmdDiff := git.NewCommand(gitRepo.Ctx) if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { - diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M") - if len(opts.WhitespaceBehavior) != 0 { - diffArgs = append(diffArgs, opts.WhitespaceBehavior) - } - // append empty tree ref - diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904") - diffArgs = append(diffArgs, git.CmdArgCheck(opts.AfterCommitID)) + cmdDiff.AddArguments("diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"). + AddArguments(opts.WhitespaceBehavior...). + AddArguments("4b825dc642cb6eb9a060e54bf8d69288fbee4904"). // append empty tree ref + AddDynamicArguments(opts.AfterCommitID) } else { actualBeforeCommitID := opts.BeforeCommitID if len(actualBeforeCommitID) == 0 { parentCommit, _ := commit.Parent(0) actualBeforeCommitID = parentCommit.ID.String() } - diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M") - if len(opts.WhitespaceBehavior) != 0 { - diffArgs = append(diffArgs, opts.WhitespaceBehavior) - } - diffArgs = append(diffArgs, git.CmdArgCheck(actualBeforeCommitID)) - diffArgs = append(diffArgs, git.CmdArgCheck(opts.AfterCommitID)) + + cmdDiff.AddArguments("diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"). + AddArguments(opts.WhitespaceBehavior...). + AddDynamicArguments(actualBeforeCommitID, opts.AfterCommitID) opts.BeforeCommitID = actualBeforeCommitID } @@ -1111,16 +1095,11 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff // the skipping for us parsePatchSkipToFile := opts.SkipTo if opts.SkipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil { - diffArgs = append(diffArgs, git.CmdArg("--skip-to="+opts.SkipTo)) + cmdDiff.AddOptionFormat("--skip-to=%s", opts.SkipTo) parsePatchSkipToFile = "" } - if len(files) > 0 { - diffArgs = append(diffArgs, "--") - for _, file := range files { - diffArgs = append(diffArgs, git.CmdArg(file)) // it's safe to cast it to CmdArg because there is a "--" before - } - } + cmdDiff.AddDashesAndList(files...) reader, writer := io.Pipe() defer func() { @@ -1128,10 +1107,9 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff _ = writer.Close() }() - go func(ctx context.Context, diffArgs []git.CmdArg, repoPath string, writer *io.PipeWriter) { - cmd := git.NewCommand(ctx, diffArgs...) - cmd.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath)) - if err := cmd.Run(&git.RunOpts{ + go func() { + cmdDiff.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath)) + if err := cmdDiff.Run(&git.RunOpts{ Timeout: time.Duration(setting.Git.Timeout.Default) * time.Second, Dir: repoPath, Stderr: os.Stderr, @@ -1141,7 +1119,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff } _ = writer.Close() - }(gitRepo.Ctx, diffArgs, repoPath, writer) + }() diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile) if err != nil { @@ -1201,16 +1179,16 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff separator = ".." } - shortstatArgs := []git.CmdArg{git.CmdArgCheck(opts.BeforeCommitID + separator + opts.AfterCommitID)} + diffPaths := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA { - shortstatArgs = []git.CmdArg{git.EmptyTreeSHA, git.CmdArgCheck(opts.AfterCommitID)} + diffPaths = []string{git.EmptyTreeSHA, opts.AfterCommitID} } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...) + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) if err != nil && strings.Contains(err.Error(), "no merge base") { // git >= 2.28 now returns an error if base and head have become unrelated. // previously it would return the results of git diff --shortstat base head so let's try that... - shortstatArgs = []git.CmdArg{git.CmdArgCheck(opts.BeforeCommitID), git.CmdArgCheck(opts.AfterCommitID)} - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...) + diffPaths = []string{opts.BeforeCommitID, opts.AfterCommitID} + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) } if err != nil { return nil, err @@ -1324,17 +1302,17 @@ func CommentMustAsDiff(c *issues_model.Comment) *Diff { } // GetWhitespaceFlag returns git diff flag for treating whitespaces -func GetWhitespaceFlag(whitespaceBehavior string) git.CmdArg { - whitespaceFlags := map[string]string{ - "ignore-all": "-w", - "ignore-change": "-b", - "ignore-eol": "--ignore-space-at-eol", - "show-all": "", +func GetWhitespaceFlag(whitespaceBehavior string) git.TrustedCmdArgs { + whitespaceFlags := map[string]git.TrustedCmdArgs{ + "ignore-all": {"-w"}, + "ignore-change": {"-b"}, + "ignore-eol": {"--ignore-space-at-eol"}, + "show-all": nil, } if flag, ok := whitespaceFlags[whitespaceBehavior]; ok { - return git.CmdArg(flag) + return flag } log.Warn("unknown whitespace behavior: %q, default to 'show-all'", whitespaceBehavior) - return "" + return nil } diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 267f0e4cff6..eb9ed862e82 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -626,7 +626,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { return } defer gitRepo.Close() - for _, behavior := range []git.CmdArg{"-w", "--ignore-space-at-eol", "-b", ""} { + for _, behavior := range []git.TrustedCmdArgs{{"-w"}, {"--ignore-space-at-eol"}, {"-b"}, nil} { diffs, err := GetDiff(gitRepo, &DiffOptions{ AfterCommitID: "bd7063cc7c04689c4d082183d32a604ed27a24f9", diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 98e8d122a5d..7dee90352ea 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -203,11 +203,11 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v]: running git remote update...", m.Repo) - gitArgs := []git.CmdArg{"remote", "update"} + cmd := git.NewCommand(ctx, "remote", "update") if m.EnablePrune { - gitArgs = append(gitArgs, "--prune") + cmd.AddArguments("--prune") } - gitArgs = append(gitArgs, git.CmdArgCheck(m.GetRemoteName())) + cmd.AddDynamicArguments(m.GetRemoteName()) remoteURL, remoteErr := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) if remoteErr != nil { @@ -217,7 +217,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stdoutBuilder := strings.Builder{} stderrBuilder := strings.Builder{} - if err := git.NewCommand(ctx, gitArgs...). + if err := cmd. SetDescription(fmt.Sprintf("Mirror.runSync: %s", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, @@ -243,7 +243,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // Successful prune - reattempt mirror stderrBuilder.Reset() stdoutBuilder.Reset() - if err = git.NewCommand(ctx, gitArgs...). + if err = cmd. SetDescription(fmt.Sprintf("Mirror.runSync: %s", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index c0c68a3f541..2c1b00b60c2 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -37,10 +37,10 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str if _, _, err := cmd.RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } - if _, _, err := git.NewCommand(ctx, "config", "--add", git.CmdArg("remote."+m.RemoteName+".push"), "+refs/heads/*:refs/heads/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { + if _, _, err := git.NewCommand(ctx, "config", "--add").AddDynamicArguments("remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } - if _, _, err := git.NewCommand(ctx, "config", "--add", git.CmdArg("remote."+m.RemoteName+".push"), "+refs/tags/*:refs/tags/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { + if _, _, err := git.NewCommand(ctx, "config", "--add").AddDynamicArguments("remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } return nil diff --git a/services/pull/merge.go b/services/pull/merge.go index 7ffbdb78b02..edd5b601da9 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -370,16 +370,16 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode sig := doer.NewGitSig() committer := sig - // Determine if we should sign - var signArg git.CmdArg - sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) + // Determine if we should sign. If no signKeyID, use --no-gpg-sign to countermand the sign config (from gitconfig) + var signArgs git.TrustedCmdArgs + sign, signKeyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) if sign { - signArg = git.CmdArg("-S" + keyID) if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } + signArgs = git.ToTrustedCmdArgs([]string{"-S" + signKeyID}) } else { - signArg = git.CmdArg("--no-gpg-sign") + signArgs = append(signArgs, "--no-gpg-sign") } commitTimeStr := time.Now().Format(time.RFC3339) @@ -403,7 +403,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode return "", err } - if err := commitAndSignNoAuthor(ctx, pr, message, signArg, tmpBasePath, env); err != nil { + if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { log.Error("Unable to make final commit: %v", err) return "", err } @@ -505,7 +505,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode return "", err } if mergeStyle == repo_model.MergeStyleRebaseMerge { - if err := commitAndSignNoAuthor(ctx, pr, message, signArg, tmpBasePath, env); err != nil { + if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { log.Error("Unable to make final commit: %v", err) return "", err } @@ -523,35 +523,22 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode return "", fmt.Errorf("LoadPoster: %w", err) } sig := pr.Issue.Poster.NewGitSig() - if signArg == "" { - if err := git.NewCommand(ctx, "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), "-m").AddDynamicArguments(message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } else { - if setting.Repository.PullRequest.AddCoCommitterTrailers && committer.String() != sig.String() { - // add trailer - message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) - } - if err := git.NewCommand(ctx, "commit"). - AddArguments(signArg). - AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email))). - AddArguments("-m").AddDynamicArguments(message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } + if setting.Repository.PullRequest.AddCoCommitterTrailers && committer.String() != sig.String() { + // add trailer + message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) + } + if err := git.NewCommand(ctx, "commit"). + AddArguments(signArgs...). + AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). + AddOptionValues("-m", message). + Run(&git.RunOpts{ + Env: env, + Dir: tmpBasePath, + Stdout: &outbuf, + Stderr: &errbuf, + }); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return "", fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() @@ -649,30 +636,17 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode return mergeCommitID, nil } -func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message string, signArg git.CmdArg, tmpBasePath string, env []string) error { +func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message string, signArgs git.TrustedCmdArgs, tmpBasePath string, env []string) error { var outbuf, errbuf strings.Builder - if signArg == "" { - if err := git.NewCommand(ctx, "commit", "-m").AddDynamicArguments(message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } else { - if err := git.NewCommand(ctx, "commit").AddArguments(signArg).AddArguments("-m").AddDynamicArguments(message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } + if err := git.NewCommand(ctx, "commit").AddArguments(signArgs...).AddOptionValues("-m", message). + Run(&git.RunOpts{ + Env: env, + Dir: tmpBasePath, + Stdout: &outbuf, + Stderr: &errbuf, + }); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } return nil } diff --git a/services/pull/patch.go b/services/pull/patch.go index a3084196bbc..c2ccc75bdcc 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -376,16 +376,16 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * prConfig := prUnit.PullRequestsConfig() // 6. Prepare the arguments to apply the patch against the index - args := []git.CmdArg{"apply", "--check", "--cached"} + cmdApply := git.NewCommand(gitRepo.Ctx, "apply", "--check", "--cached") if prConfig.IgnoreWhitespaceConflicts { - args = append(args, "--ignore-whitespace") + cmdApply.AddArguments("--ignore-whitespace") } is3way := false if git.CheckGitVersionAtLeast("2.32.0") == nil { - args = append(args, "--3way") + cmdApply.AddArguments("--3way") is3way = true } - args = append(args, git.CmdArgCheck(patchPath)) + cmdApply.AddDynamicArguments(patchPath) // 7. Prep the pipe: // - Here we could do the equivalent of: @@ -407,71 +407,70 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * // 8. Run the check command conflict = false - err = git.NewCommand(gitRepo.Ctx, args...). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stderr: stderrWriter, - PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { - // Close the writer end of the pipe to begin processing - _ = stderrWriter.Close() - defer func() { - // Close the reader on return to terminate the git command if necessary - _ = stderrReader.Close() - }() + err = cmdApply.Run(&git.RunOpts{ + Dir: tmpBasePath, + Stderr: stderrWriter, + PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { + // Close the writer end of the pipe to begin processing + _ = stderrWriter.Close() + defer func() { + // Close the reader on return to terminate the git command if necessary + _ = stderrReader.Close() + }() - const prefix = "error: patch failed:" - const errorPrefix = "error: " - const threewayFailed = "Failed to perform three-way merge..." - const appliedPatchPrefix = "Applied patch to '" - const withConflicts = "' with conflicts." + const prefix = "error: patch failed:" + const errorPrefix = "error: " + const threewayFailed = "Failed to perform three-way merge..." + const appliedPatchPrefix = "Applied patch to '" + const withConflicts = "' with conflicts." - conflicts := make(container.Set[string]) + conflicts := make(container.Set[string]) - // Now scan the output from the command - scanner := bufio.NewScanner(stderrReader) - for scanner.Scan() { - line := scanner.Text() - log.Trace("PullRequest[%d].testPatch: stderr: %s", pr.ID, line) - if strings.HasPrefix(line, prefix) { - conflict = true - filepath := strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0]) - conflicts.Add(filepath) - } else if is3way && line == threewayFailed { - conflict = true - } else if strings.HasPrefix(line, errorPrefix) { - conflict = true - for _, suffix := range patchErrorSuffices { - if strings.HasSuffix(line, suffix) { - filepath := strings.TrimSpace(strings.TrimSuffix(line[len(errorPrefix):], suffix)) - if filepath != "" { - conflicts.Add(filepath) - } - break + // Now scan the output from the command + scanner := bufio.NewScanner(stderrReader) + for scanner.Scan() { + line := scanner.Text() + log.Trace("PullRequest[%d].testPatch: stderr: %s", pr.ID, line) + if strings.HasPrefix(line, prefix) { + conflict = true + filepath := strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0]) + conflicts.Add(filepath) + } else if is3way && line == threewayFailed { + conflict = true + } else if strings.HasPrefix(line, errorPrefix) { + conflict = true + for _, suffix := range patchErrorSuffices { + if strings.HasSuffix(line, suffix) { + filepath := strings.TrimSpace(strings.TrimSuffix(line[len(errorPrefix):], suffix)) + if filepath != "" { + conflicts.Add(filepath) } - } - } else if is3way && strings.HasPrefix(line, appliedPatchPrefix) && strings.HasSuffix(line, withConflicts) { - conflict = true - filepath := strings.TrimPrefix(strings.TrimSuffix(line, withConflicts), appliedPatchPrefix) - if filepath != "" { - conflicts.Add(filepath) + break } } - // only list 10 conflicted files - if len(conflicts) >= 10 { - break + } else if is3way && strings.HasPrefix(line, appliedPatchPrefix) && strings.HasSuffix(line, withConflicts) { + conflict = true + filepath := strings.TrimPrefix(strings.TrimSuffix(line, withConflicts), appliedPatchPrefix) + if filepath != "" { + conflicts.Add(filepath) } } - - if len(conflicts) > 0 { - pr.ConflictedFiles = make([]string, 0, len(conflicts)) - for key := range conflicts { - pr.ConflictedFiles = append(pr.ConflictedFiles, key) - } + // only list 10 conflicted files + if len(conflicts) >= 10 { + break } + } - return nil - }, - }) + if len(conflicts) > 0 { + pr.ConflictedFiles = make([]string, 0, len(conflicts)) + for key := range conflicts { + pr.ConflictedFiles = append(pr.ConflictedFiles, key) + } + } + + return nil + }, + }) // 9. Check if the found conflictedfiles is non-zero, "err" could be non-nil, so we should ignore it if we found conflicts. // Note: `"err" could be non-nil` is due that if enable 3-way merge, it doesn't return any error on found conflicts. diff --git a/services/repository/check.go b/services/repository/check.go index e9d65aea4a8..3a1f0b7f306 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -23,7 +23,7 @@ import ( ) // GitFsckRepos calls 'git fsck' to check repository health. -func GitFsckRepos(ctx context.Context, timeout time.Duration, args []git.CmdArg) error { +func GitFsckRepos(ctx context.Context, timeout time.Duration, args git.TrustedCmdArgs) error { log.Trace("Doing: GitFsck") if err := db.Iterate( @@ -47,10 +47,10 @@ func GitFsckRepos(ctx context.Context, timeout time.Duration, args []git.CmdArg) } // GitFsckRepo calls 'git fsck' to check an individual repository's health. -func GitFsckRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args []git.CmdArg) error { +func GitFsckRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args git.TrustedCmdArgs) error { log.Trace("Running health check on repository %-v", repo) repoPath := repo.RepoPath() - if err := git.Fsck(ctx, repoPath, timeout, args...); err != nil { + if err := git.Fsck(ctx, repoPath, timeout, args); err != nil { log.Warn("Failed to health check repository (%-v): %v", repo, err) if err = system_model.CreateRepositoryNotice("Failed to health check repository (%s): %v", repo.FullName(), err); err != nil { log.Error("CreateRepositoryNotice: %v", err) @@ -60,9 +60,8 @@ func GitFsckRepo(ctx context.Context, repo *repo_model.Repository, timeout time. } // GitGcRepos calls 'git gc' to remove unnecessary files and optimize the local repository -func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg) error { +func GitGcRepos(ctx context.Context, timeout time.Duration, args git.TrustedCmdArgs) error { log.Trace("Doing: GitGcRepos") - args = append([]git.CmdArg{"gc"}, args...) if err := db.Iterate( ctx, @@ -86,9 +85,9 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg) } // GitGcRepo calls 'git gc' to remove unnecessary files and optimize the local repository -func GitGcRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args []git.CmdArg) error { +func GitGcRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args git.TrustedCmdArgs) error { log.Trace("Running git gc on %-v", repo) - command := git.NewCommand(ctx, args...). + command := git.NewCommand(ctx, "gc").AddArguments(args...). SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())) var stdout string var err error diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 73ee0fa815e..f65199cfcb9 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -141,14 +141,12 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user stdout := &strings.Builder{} stderr := &strings.Builder{} - args := []git.CmdArg{"apply", "--index", "--recount", "--cached", "--ignore-whitespace", "--whitespace=fix", "--binary"} - + cmdApply := git.NewCommand(ctx, "apply", "--index", "--recount", "--cached", "--ignore-whitespace", "--whitespace=fix", "--binary") if git.CheckGitVersionAtLeast("2.32") == nil { - args = append(args, "-3") + cmdApply.AddArguments("-3") } - cmd := git.NewCommand(ctx, args...) - if err := cmd.Run(&git.RunOpts{ + if err := cmdApply.Run(&git.RunOpts{ Dir: t.basePath, Stdout: stdout, Stderr: stderr, diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 1f3375cdcc8..a086d15a4f9 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -233,11 +233,9 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co _, _ = messageBytes.WriteString(message) _, _ = messageBytes.WriteString("\n") - var args []git.CmdArg + cmdCommitTree := git.NewCommand(t.ctx, "commit-tree").AddDynamicArguments(treeHash) if parent != "" { - args = []git.CmdArg{"commit-tree", git.CmdArgCheck(treeHash), "-p", git.CmdArgCheck(parent)} - } else { - args = []git.CmdArg{"commit-tree", git.CmdArgCheck(treeHash)} + cmdCommitTree.AddOptionValues("-p", parent) } var sign bool @@ -249,7 +247,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co sign, keyID, signer, _ = asymkey_service.SignInitialCommit(t.ctx, t.repo.RepoPath(), author) } if sign { - args = append(args, git.CmdArg("-S"+keyID)) + cmdCommitTree.AddOptionFormat("-S%s", keyID) if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email { // Add trailers @@ -264,7 +262,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co committerSig = signer } } else { - args = append(args, "--no-gpg-sign") + cmdCommitTree.AddArguments("--no-gpg-sign") } if signoff { @@ -281,7 +279,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) - if err := git.NewCommand(t.ctx, args...). + if err := cmdCommitTree. Run(&git.RunOpts{ Env: env, Dir: t.basePath, @@ -364,7 +362,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { t.repo.FullName(), err, stderr) } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.ctx, t.basePath, "--cached", "HEAD") + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.ctx, t.basePath, git.TrustedCmdArgs{"--cached"}, "HEAD") if err != nil { return nil, err } diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 58b7a5e082b..45a46923966 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -370,7 +370,7 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do if setting.LFS.StartServer && hasOldBranch { // Check there is no way this can return multiple infos filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []git.CmdArg{"filter"}, + Attributes: []string{"filter"}, Filenames: []string{treePath}, CachedOnly: true, }) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index e7289dd60d2..cf2f7019b68 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -96,7 +96,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use var filename2attribute2info map[string]map[string]string if setting.LFS.StartServer { filename2attribute2info, err = t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []git.CmdArg{"filter"}, + Attributes: []string{"filter"}, Filenames: names, CachedOnly: true, }) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 9e3ff9c4484..1e99783096e 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -154,16 +154,16 @@ func doGitAddRemote(dstPath, remoteName string, u *url.URL) func(*testing.T) { } } -func doGitPushTestRepository(dstPath string, args ...git.CmdArg) func(*testing.T) { +func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"push", "-u"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, "push", "-u").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitPushTestRepositoryFail(dstPath string, args ...git.CmdArg) func(*testing.T) { +func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"push"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, "push").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.Error(t, err) } } @@ -175,23 +175,23 @@ func doGitCreateBranch(dstPath, branch string) func(*testing.T) { } } -func doGitCheckoutBranch(dstPath string, args ...git.CmdArg) func(*testing.T) { +func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "checkout"), args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommandContextNoGlobals(git.DefaultContext, git.AllowLFSFiltersArgs()...).AddArguments("checkout").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitMerge(dstPath string, args ...git.CmdArg) func(*testing.T) { +func doGitMerge(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"merge"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, "merge").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitPull(dstPath string, args ...git.CmdArg) func(*testing.T) { +func doGitPull(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "pull"), args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommandContextNoGlobals(git.DefaultContext, git.AllowLFSFiltersArgs()...).AddArguments("pull").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index a11bad21b7b..420a8676b9e 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -509,7 +509,7 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr })) t.Run("CreateHeadBranch", doGitCreateBranch(dstPath, headBranch)) - t.Run("PushToHeadBranch", doGitPushTestRepository(dstPath, "origin", git.CmdArgCheck(headBranch))) + t.Run("PushToHeadBranch", doGitPushTestRepository(dstPath, "origin", headBranch)) t.Run("CreateEmptyPullRequest", func(t *testing.T) { pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) assert.NoError(t, err) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index e80822a7520..090a27c39ef 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -287,7 +287,7 @@ func TestCantMergeUnrelated(t *testing.T) { assert.NoError(t, err) sha := strings.TrimSpace(stdout.String()) - _, _, err = git.NewCommand(git.DefaultContext, "update-index", "--add", "--replace", "--cacheinfo", "100644", git.CmdArgCheck(sha), "somewher-over-the-rainbow").RunStdString(&git.RunOpts{Dir: path}) + _, _, err = git.NewCommand(git.DefaultContext, "update-index", "--add", "--replace", "--cacheinfo").AddDynamicArguments("100644", sha, "somewher-over-the-rainbow").RunStdString(&git.RunOpts{Dir: path}) assert.NoError(t, err) treeSha, _, err := git.NewCommand(git.DefaultContext, "write-tree").RunStdString(&git.RunOpts{Dir: path}) From c2774d9e80d9a436d9c2044960369c4db227e3a0 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 4 Feb 2023 04:17:43 +0100 Subject: [PATCH 06/20] Fix color of tertiary button on dark theme (#22739) Before: Screenshot 2023-02-03 at 14 07 34 After: Screenshot 2023-02-03 at 14 07 52 This is the only instance of such a button in all templates. --------- Co-authored-by: Lauris BH Co-authored-by: Lunny Xiao --- web_src/less/_base.less | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/web_src/less/_base.less b/web_src/less/_base.less index 26fc83785b7..e58bf53f5df 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -2200,9 +2200,7 @@ a.ui.label:hover { border: 1px solid var(--color-light-border); color: var(--color-text); } -.ui.tertiary.button { - border: none; -} + .page-content .ui.button { box-shadow: none !important; } @@ -2319,6 +2317,15 @@ a.ui.label:hover { color: var(--color-secondary-dark-8) !important; } +.ui.tertiary.button { + color: var(--color-text-light); + border: none; +} + +.ui.tertiary.button:hover { + color: var(--color-text); +} + .ui.primary.label, .ui.primary.labels .label { background-color: var(--color-primary) !important; From 2741546bed546fbe9792b48119743cfaebfec6bf Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sat, 4 Feb 2023 01:48:38 -0500 Subject: [PATCH 07/20] Repositories: by default disable all units except code and pulls on forks (#22541) Most of the time forks are used for contributing code only, so not having issues, projects, release and packages is a better default for such cases. They can still be enabled in the settings. A new option `DEFAULT_FORK_REPO_UNITS` is added to configure the default units on forks. Also add missing `repo.packages` unit to documentation. code by: @brechtvl ## :warning: BREAKING :warning: When forking a repository, the fork will now have issues, projects, releases, packages and wiki disabled. These can be enabled in the repository settings afterwards. To change back to the previous default behavior, configure `DEFAULT_FORK_REPO_UNITS` to be the same value as `DEFAULT_REPO_UNITS`. Co-authored-by: Brecht Van Lommel --- custom/conf/app.example.ini | 10 +++- .../doc/advanced/config-cheat-sheet.en-us.md | 3 +- models/unit/unit.go | 58 ++++++++++++------- modules/repository/create.go | 12 ++-- modules/repository/generate.go | 2 +- modules/setting/repository.go | 2 + services/repository/adopt.go | 2 +- services/repository/fork.go | 2 +- 8 files changed, 60 insertions(+), 31 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 77efe1417ba..b0260f21d13 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -927,14 +927,18 @@ ROUTER = console ;USE_COMPAT_SSH_URI = false ;; ;; Close issues as long as a commit on any branch marks it as fixed -;; Comma separated list of globally disabled repo units. Allowed values: repo.issues, repo.ext_issues, repo.pulls, repo.wiki, repo.ext_wiki, repo.projects +;; Comma separated list of globally disabled repo units. Allowed values: repo.issues, repo.ext_issues, repo.pulls, repo.wiki, repo.ext_wiki, repo.projects, repo.packages ;DISABLED_REPO_UNITS = ;; -;; Comma separated list of default repo units. Allowed values: repo.code, repo.releases, repo.issues, repo.pulls, repo.wiki, repo.projects. +;; Comma separated list of default new repo units. Allowed values: repo.code, repo.releases, repo.issues, repo.pulls, repo.wiki, repo.projects, repo.packages. ;; Note: Code and Releases can currently not be deactivated. If you specify default repo units you should still list them for future compatibility. ;; External wiki and issue tracker can't be enabled by default as it requires additional settings. ;; Disabled repo units will not be added to new repositories regardless if it is in the default list. -;DEFAULT_REPO_UNITS = repo.code,repo.releases,repo.issues,repo.pulls,repo.wiki,repo.projects +;DEFAULT_REPO_UNITS = repo.code,repo.releases,repo.issues,repo.pulls,repo.wiki,repo.projects,repo.packages +;; +;; Comma separated list of default forked repo units. +;; The set of allowed values and rules are the same as DEFAULT_REPO_UNITS. +;DEFAULT_FORK_REPO_UNITS = repo.code,repo.pulls ;; ;; Prefix archive files by placing them in a directory named after the repository ;PREFIX_ARCHIVE_FILES = true diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 441bb824ad2..e12d1feeb9b 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -104,7 +104,8 @@ In addition there is _`StaticRootPath`_ which can be set as a built-in at build - `ENABLE_PUSH_CREATE_USER`: **false**: Allow users to push local repositories to Gitea and have them automatically created for a user. - `ENABLE_PUSH_CREATE_ORG`: **false**: Allow users to push local repositories to Gitea and have them automatically created for an org. - `DISABLED_REPO_UNITS`: **_empty_**: Comma separated list of globally disabled repo units. Allowed values: \[repo.issues, repo.ext_issues, repo.pulls, repo.wiki, repo.ext_wiki, repo.projects\] -- `DEFAULT_REPO_UNITS`: **repo.code,repo.releases,repo.issues,repo.pulls,repo.wiki,repo.projects**: Comma separated list of default repo units. Allowed values: \[repo.code, repo.releases, repo.issues, repo.pulls, repo.wiki, repo.projects\]. Note: Code and Releases can currently not be deactivated. If you specify default repo units you should still list them for future compatibility. External wiki and issue tracker can't be enabled by default as it requires additional settings. Disabled repo units will not be added to new repositories regardless if it is in the default list. +- `DEFAULT_REPO_UNITS`: **repo.code,repo.releases,repo.issues,repo.pulls,repo.wiki,repo.projects,repo.packages**: Comma separated list of default new repo units. Allowed values: \[repo.code, repo.releases, repo.issues, repo.pulls, repo.wiki, repo.projects\]. Note: Code and Releases can currently not be deactivated. If you specify default repo units you should still list them for future compatibility. External wiki and issue tracker can't be enabled by default as it requires additional settings. Disabled repo units will not be added to new repositories regardless if it is in the default list. +- `DEFAULT_FORK_REPO_UNITS`: **repo.code,repo.pulls**: Comma separated list of default forked repo units. The set of allowed values and rules is the same as `DEFAULT_REPO_UNITS`. - `PREFIX_ARCHIVE_FILES`: **true**: Prefix archive files by placing them in a directory named after the repository. - `DISABLE_MIGRATIONS`: **false**: Disable migrating feature. - `DISABLE_STARS`: **false**: Disable stars feature. diff --git a/models/unit/unit.go b/models/unit/unit.go index bcd0572ab9e..a2a9079dc25 100644 --- a/models/unit/unit.go +++ b/models/unit/unit.go @@ -94,6 +94,12 @@ var ( TypePackages, } + // ForkRepoUnits contains the default unit types for forks + DefaultForkRepoUnits = []Type{ + TypeCode, + TypePullRequests, + } + // NotAllowedDefaultRepoUnits contains units that can't be default NotAllowedDefaultRepoUnits = []Type{ TypeExternalWiki, @@ -110,26 +116,41 @@ var ( DisabledRepoUnits = []Type{} ) -// LoadUnitConfig load units from settings -func LoadUnitConfig() { - setDefaultRepoUnits := FindUnitTypes(setting.Repository.DefaultRepoUnits...) - // Default repo units set if setting is not empty - if len(setDefaultRepoUnits) > 0 { +// Get valid set of default repository units from settings +func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type { + units := defaultUnits + + // Use setting if not empty + if len(settingDefaultUnits) > 0 { // MustRepoUnits required as default - DefaultRepoUnits = make([]Type, len(MustRepoUnits)) - copy(DefaultRepoUnits, MustRepoUnits) - for _, defaultU := range setDefaultRepoUnits { - if !defaultU.CanBeDefault() { - log.Warn("Not allowed as default unit: %s", defaultU.String()) + units = make([]Type, len(MustRepoUnits)) + copy(units, MustRepoUnits) + for _, settingUnit := range settingDefaultUnits { + if !settingUnit.CanBeDefault() { + log.Warn("Not allowed as default unit: %s", settingUnit.String()) continue } // MustRepoUnits already added - if defaultU.CanDisable() { - DefaultRepoUnits = append(DefaultRepoUnits, defaultU) + if settingUnit.CanDisable() { + units = append(units, settingUnit) } } } + // Remove disabled units + for _, disabledUnit := range DisabledRepoUnits { + for i, unit := range units { + if unit == disabledUnit { + units = append(units[:i], units[i+1:]...) + } + } + } + + return units +} + +// LoadUnitConfig load units from settings +func LoadUnitConfig() { DisabledRepoUnits = FindUnitTypes(setting.Repository.DisabledRepoUnits...) // Check that must units are not disabled for i, disabledU := range DisabledRepoUnits { @@ -138,14 +159,11 @@ func LoadUnitConfig() { DisabledRepoUnits = append(DisabledRepoUnits[:i], DisabledRepoUnits[i+1:]...) } } - // Remove disabled units from default units - for _, disabledU := range DisabledRepoUnits { - for i, defaultU := range DefaultRepoUnits { - if defaultU == disabledU { - DefaultRepoUnits = append(DefaultRepoUnits[:i], DefaultRepoUnits[i+1:]...) - } - } - } + + setDefaultRepoUnits := FindUnitTypes(setting.Repository.DefaultRepoUnits...) + DefaultRepoUnits = validateDefaultRepoUnits(DefaultRepoUnits, setDefaultRepoUnits) + setDefaultForkRepoUnits := FindUnitTypes(setting.Repository.DefaultForkRepoUnits...) + DefaultForkRepoUnits = validateDefaultRepoUnits(DefaultForkRepoUnits, setDefaultForkRepoUnits) } // UnitGlobalDisabled checks if unit type is global disabled diff --git a/modules/repository/create.go b/modules/repository/create.go index 454a192ddbb..7bcda0fe45f 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -30,7 +30,7 @@ import ( ) // CreateRepositoryByExample creates a repository for the user/organization. -func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt bool) (err error) { +func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) { if err = repo_model.IsUsableRepoName(repo.Name); err != nil { return err } @@ -67,8 +67,12 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re } // insert units for repo - units := make([]repo_model.RepoUnit, 0, len(unit.DefaultRepoUnits)) - for _, tp := range unit.DefaultRepoUnits { + defaultUnits := unit.DefaultRepoUnits + if isFork { + defaultUnits = unit.DefaultForkRepoUnits + } + units := make([]repo_model.RepoUnit, 0, len(defaultUnits)) + for _, tp := range defaultUnits { if tp == unit.TypeIssues { units = append(units, repo_model.RepoUnit{ RepoID: repo.ID, @@ -212,7 +216,7 @@ func CreateRepository(doer, u *user_model.User, opts CreateRepoOptions) (*repo_m var rollbackRepo *repo_model.Repository if err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { - if err := CreateRepositoryByExample(ctx, doer, u, repo, false); err != nil { + if err := CreateRepositoryByExample(ctx, doer, u, repo, false, false); err != nil { return err } diff --git a/modules/repository/generate.go b/modules/repository/generate.go index b6a1d7b43ef..31d5ebbb11f 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -319,7 +319,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ TrustModel: templateRepo.TrustModel, } - if err = CreateRepositoryByExample(ctx, doer, owner, generateRepo, false); err != nil { + if err = CreateRepositoryByExample(ctx, doer, owner, generateRepo, false, false); err != nil { return nil, err } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index d78b63a1f3f..f53de17a4dd 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -41,6 +41,7 @@ var ( EnablePushCreateOrg bool DisabledRepoUnits []string DefaultRepoUnits []string + DefaultForkRepoUnits []string PrefixArchiveFiles bool DisableMigrations bool DisableStars bool `ini:"DISABLE_STARS"` @@ -157,6 +158,7 @@ var ( EnablePushCreateOrg: false, DisabledRepoUnits: []string{}, DefaultRepoUnits: []string{}, + DefaultForkRepoUnits: []string{}, PrefixArchiveFiles: true, DisableMigrations: false, DisableStars: false, diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 8ebf2b6a3e6..280c4cc035a 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -67,7 +67,7 @@ func AdoptRepository(doer, u *user_model.User, opts repo_module.CreateRepoOption } } - if err := repo_module.CreateRepositoryByExample(ctx, doer, u, repo, true); err != nil { + if err := repo_module.CreateRepositoryByExample(ctx, doer, u, repo, true, false); err != nil { return err } if err := adoptRepository(ctx, repoPath, doer, repo, opts); err != nil { diff --git a/services/repository/fork.go b/services/repository/fork.go index ad534be887f..c3ca89e02e7 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -119,7 +119,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork }() err = db.WithTx(ctx, func(txCtx context.Context) error { - if err = repo_module.CreateRepositoryByExample(txCtx, doer, owner, repo, false); err != nil { + if err = repo_module.CreateRepositoryByExample(txCtx, doer, owner, repo, false, true); err != nil { return err } From ea13b23349ef98249deeb9469f6b1444de42abf5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 4 Feb 2023 18:30:55 +0800 Subject: [PATCH 08/20] Escape path for the file list (#22741) Fix #22740 --- web_src/js/features/repo-findfile.js | 6 +++++- web_src/js/features/repo-findfile.test.js | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/repo-findfile.js b/web_src/js/features/repo-findfile.js index 7b8833e793c..f279dfeeb73 100644 --- a/web_src/js/features/repo-findfile.js +++ b/web_src/js/features/repo-findfile.js @@ -72,6 +72,10 @@ export function filterRepoFilesWeighted(files, filter) { return filterResult; } +export function escapePath(s) { + return s.split('/').map(encodeURIComponent).join('/'); +} + function filterRepoFiles(filter) { const treeLink = $repoFindFileInput.attr('data-url-tree-link'); $repoFindFileTableBody.empty(); @@ -83,7 +87,7 @@ function filterRepoFiles(filter) { for (const r of filterResult) { const $row = $(tmplRow); const $a = $row.find('a'); - $a.attr('href', `${treeLink}/${r.matchResult.join('')}`); + $a.attr('href', `${treeLink}/${escapePath(r.matchResult.join(''))}`); const $octiconFile = $(svg('octicon-file')).addClass('mr-3'); $a.append($octiconFile); // if the target file path is "abc/xyz", to search "bx", then the matchResult is ['a', 'b', 'c/', 'x', 'yz'] diff --git a/web_src/js/features/repo-findfile.test.js b/web_src/js/features/repo-findfile.test.js index a90b0bf0a2c..50321853960 100644 --- a/web_src/js/features/repo-findfile.test.js +++ b/web_src/js/features/repo-findfile.test.js @@ -1,5 +1,5 @@ import {describe, expect, test} from 'vitest'; -import {strSubMatch, calcMatchedWeight, filterRepoFilesWeighted} from './repo-findfile.js'; +import {strSubMatch, calcMatchedWeight, filterRepoFilesWeighted, escapePath} from './repo-findfile.js'; describe('Repo Find Files', () => { test('strSubMatch', () => { @@ -32,4 +32,9 @@ describe('Repo Find Files', () => { expect(res).toHaveLength(2); expect(res[0].matchResult).toEqual(['', 'word', '.txt']); }); + + test('escapePath', () => { + expect(escapePath('a/b/c')).toEqual('a/b/c'); + expect(escapePath('a/b/ c')).toEqual('a/b/%20c'); + }); }); From 4d20a4a1baeb9ed1bb5b3ed7c44b6046f6387303 Mon Sep 17 00:00:00 2001 From: delvh Date: Sat, 4 Feb 2023 14:26:38 +0100 Subject: [PATCH 09/20] Remove ONLY_SHOW_RELEVANT_REPOS setting (#21962) Every user can already disable the filter manually, so the explicit setting is absolutely useless and only complicates the logic. Previously, there was also unexpected behavior when multiple query parameters were present. --------- Co-authored-by: zeripath Co-authored-by: Lunny Xiao --- custom/conf/app.example.ini | 4 ---- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 2 -- models/repo/repo_list.go | 8 ++++---- modules/setting/setting.go | 2 -- routers/web/explore/repo.go | 11 +++++------ 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index b0260f21d13..5a1edf9fbb7 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1222,10 +1222,6 @@ ROUTER = console ;; ;; Whether to enable a Service Worker to cache frontend assets ;USE_SERVICE_WORKER = false -;; -;; Whether to only show relevant repos on the explore page when no keyword is specified and default sorting is used. -;; A repo is considered irrelevant if it's a fork or if it has no metadata (no description, no icon, no topic). -;ONLY_SHOW_RELEVANT_REPOS = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index e12d1feeb9b..67ca7a51662 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -231,8 +231,6 @@ The following configuration set `Content-Type: application/vnd.android.package-a - `DEFAULT_SHOW_FULL_NAME`: **false**: Whether the full name of the users should be shown where possible. If the full name isn't set, the username will be used. - `SEARCH_REPO_DESCRIPTION`: **true**: Whether to search within description at repository search on explore page. - `USE_SERVICE_WORKER`: **false**: Whether to enable a Service Worker to cache frontend assets. -- `ONLY_SHOW_RELEVANT_REPOS`: **false** Whether to only show relevant repos on the explore page when no keyword is specified and default sorting is used. - A repo is considered irrelevant if it's a fork or if it has no metadata (no description, no icon, no topic). ### UI - Admin (`ui.admin`) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index c6e9a204d13..d64368daa6b 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -494,7 +494,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { } if opts.OnlyShowRelevant { - // Only show a repo that either has a topic or description. + // Only show a repo that has at least a topic, an icon, or a description subQueryCond := builder.NewCond() // Topic checking. Topics are present. @@ -504,13 +504,13 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { subQueryCond = subQueryCond.Or(builder.And(builder.Neq{"topics": "null"}, builder.Neq{"topics": "[]"})) } - // Description checking. Description not empty. + // Description checking. Description not empty subQueryCond = subQueryCond.Or(builder.Neq{"description": ""}) - // Repo has a avatar. + // Repo has a avatar subQueryCond = subQueryCond.Or(builder.Neq{"avatar": ""}) - // Always hide repo's that are empty. + // Always hide repo's that are empty subQueryCond = subQueryCond.And(builder.Eq{"is_empty": false}) cond = cond.And(subQueryCond) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index afd7a40150d..23cd90553eb 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -241,7 +241,6 @@ var ( CustomEmojisMap map[string]string `ini:"-"` SearchRepoDescription bool UseServiceWorker bool - OnlyShowRelevantRepos bool Notification struct { MinTimeout time.Duration @@ -1123,7 +1122,6 @@ func loadFromConf(allowEmpty bool, extraConfig string) { UI.DefaultShowFullName = Cfg.Section("ui").Key("DEFAULT_SHOW_FULL_NAME").MustBool(false) UI.SearchRepoDescription = Cfg.Section("ui").Key("SEARCH_REPO_DESCRIPTION").MustBool(true) UI.UseServiceWorker = Cfg.Section("ui").Key("USE_SERVICE_WORKER").MustBool(false) - UI.OnlyShowRelevantRepos = Cfg.Section("ui").Key("ONLY_SHOW_RELEVANT_REPOS").MustBool(false) HasRobotsTxt, err = util.IsFile(path.Join(CustomPath, "robots.txt")) if err != nil { diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go index 5271e39bbc9..e9684dd2865 100644 --- a/routers/web/explore/repo.go +++ b/routers/web/explore/repo.go @@ -17,7 +17,8 @@ import ( const ( // tplExploreRepos explore repositories page template - tplExploreRepos base.TplName = "explore/repos" + tplExploreRepos base.TplName = "explore/repos" + relevantReposOnlyParam string = "no_filter" ) // RepoSearchOptions when calling search repositories @@ -81,13 +82,11 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { default: ctx.Data["SortType"] = "recentupdate" orderBy = db.SearchOrderByRecentUpdated - onlyShowRelevant = setting.UI.OnlyShowRelevantRepos && !ctx.FormBool("no_filter") } + onlyShowRelevant = !ctx.FormBool(relevantReposOnlyParam) + keyword := ctx.FormTrim("q") - if keyword != "" { - onlyShowRelevant = false - } ctx.Data["OnlyShowRelevant"] = onlyShowRelevant @@ -139,7 +138,7 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { pager.SetDefaultParams(ctx) pager.AddParam(ctx, "topic", "TopicOnly") pager.AddParam(ctx, "language", "Language") - pager.AddParamString("no_filter", ctx.FormString("no_filter")) + pager.AddParamString(relevantReposOnlyParam, ctx.FormString(relevantReposOnlyParam)) ctx.Data["Page"] = pager ctx.HTML(http.StatusOK, opts.TplName) From 8574a6433fab47b6f20997f024c176490dfad1c0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 Feb 2023 22:35:08 +0800 Subject: [PATCH 10/20] Show all projects, not just repo projects and open/closed projects (#22640) This PR fixes two problems. One is when filter repository issues, only repository level projects are listed. Another is if you list open issues, only open projects will be displayed in filter options and if you list closed issues, only closed projects will be displayed in filter options. In this PR, both repository level and org/user level projects will be displayed in filter, and both open and closed projects will be listed as filter items. --------- Co-authored-by: John Olheiser Co-authored-by: zeripath Co-authored-by: delvh --- models/db/search.go | 6 ++++ models/issues/issue.go | 2 ++ options/locale/locale_en-US.ini | 3 +- routers/web/repo/issue.go | 10 ++---- templates/repo/issue/list.tmpl | 64 +++++++++++++++++++++++++++------ 5 files changed, 65 insertions(+), 20 deletions(-) diff --git a/models/db/search.go b/models/db/search.go index f5273cb6f6b..26e082756a5 100644 --- a/models/db/search.go +++ b/models/db/search.go @@ -27,3 +27,9 @@ const ( SearchOrderByForks SearchOrderBy = "num_forks ASC" SearchOrderByForksReverse SearchOrderBy = "num_forks DESC" ) + +const ( + // Which means a condition to filter the records which don't match any id. + // It's different from zero which means the condition could be ignored. + NoneID = -1 +) diff --git a/models/issues/issue.go b/models/issues/issue.go index 78cac900523..3ddc7992709 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -1251,6 +1251,8 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) { if opts.ProjectID > 0 { sess.Join("INNER", "project_issue", "issue.id = project_issue.issue_id"). And("project_issue.project_id=?", opts.ProjectID) + } else if opts.ProjectID == db.NoneID { // show those that are in no project + sess.And(builder.NotIn("issue.id", builder.Select("issue_id").From("project_issue"))) } if opts.ProjectBoardID != 0 { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 26217293a5e..f384056613c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1306,7 +1306,8 @@ issues.filter_label_no_select = All labels issues.filter_milestone = Milestone issues.filter_milestone_no_select = All milestones issues.filter_project = Project -issues.filter_project_no_select = All projects +issues.filter_project_all = All projects +issues.filter_project_none = No project issues.filter_assignee = Assignee issues.filter_assginee_no_select = All assignees issues.filter_poster = Author diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 5bff9e67f34..2193da5110e 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -363,16 +363,10 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti return 0 } - projects, _, err := project_model.FindProjects(ctx, project_model.SearchOptions{ - RepoID: repo.ID, - Type: project_model.TypeRepository, - IsClosed: util.OptionalBoolOf(isShowClosed), - }) - if err != nil { - ctx.ServerError("FindProjects", err) + retrieveProjects(ctx, repo) + if ctx.Written() { return } - ctx.Data["Projects"] = projects ctx.Data["IssueStats"] = issueStats ctx.Data["SelLabelIDs"] = labelIDs diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 23bcc60f94f..cf2a2a6bbab 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -75,19 +75,41 @@ - - diff --git a/templates/package/content/cargo.tmpl b/templates/package/content/cargo.tmpl new file mode 100644 index 00000000000..54c40a5b0dc --- /dev/null +++ b/templates/package/content/cargo.tmpl @@ -0,0 +1,62 @@ +{{if eq .PackageDescriptor.Package.Type "cargo"}} +

{{.locale.Tr "packages.installation"}}

+
+
+
+ +
[registry]
+default = "gitea"
+
+[registries.gitea]
+index = "{{AppUrl}}{{.PackageDescriptor.Owner.Name}}/_cargo-index.git"
+
+[net]
+git-fetch-with-cli = true
+
+
+ +
cargo add {{.PackageDescriptor.Package.Name}}@{{.PackageDescriptor.Version.Version}}
+
+
+ +
+
+
+ + {{if or .PackageDescriptor.Metadata.Description .PackageDescriptor.Metadata.Readme}} +

{{.locale.Tr "packages.about"}}

+ {{if .PackageDescriptor.Metadata.Description}}
{{.PackageDescriptor.Metadata.Description}}
{{end}} + {{if .PackageDescriptor.Metadata.Readme}}
{{RenderMarkdownToHtml .PackageDescriptor.Metadata.Readme}}
{{end}} + {{end}} + + {{if .PackageDescriptor.Metadata.Dependencies}} +

{{.locale.Tr "packages.dependencies"}}

+
+ + + + + + + + + {{range .PackageDescriptor.Metadata.Dependencies}} + + + + + {{end}} + +
{{.locale.Tr "packages.dependency.id"}}{{.locale.Tr "packages.dependency.version"}}
{{.Name}}{{.Req}}
+
+ {{end}} + + {{if .PackageDescriptor.Metadata.Keywords}} +

{{.locale.Tr "packages.keywords"}}

+
+ {{range .PackageDescriptor.Metadata.Keywords}} + {{.}} + {{end}} +
+ {{end}} +{{end}} diff --git a/templates/package/metadata/cargo.tmpl b/templates/package/metadata/cargo.tmpl new file mode 100644 index 00000000000..68232d48321 --- /dev/null +++ b/templates/package/metadata/cargo.tmpl @@ -0,0 +1,7 @@ +{{if eq .PackageDescriptor.Package.Type "cargo"}} + {{range .PackageDescriptor.Metadata.Authors}}
{{svg "octicon-person" 16 "mr-3"}} {{.}}
{{end}} + {{if .PackageDescriptor.Metadata.ProjectURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.project_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.cargo.details.repository_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.DocumentationURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.cargo.details.documentation_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.License}}
{{svg "octicon-law" 16 "mr-3"}} {{.PackageDescriptor.Metadata.License}}
{{end}} +{{end}} diff --git a/templates/package/shared/cargo.tmpl b/templates/package/shared/cargo.tmpl new file mode 100644 index 00000000000..b9c2ef4d2ba --- /dev/null +++ b/templates/package/shared/cargo.tmpl @@ -0,0 +1,24 @@ +

+ {{.locale.Tr "packages.owner.settings.cargo.title"}} +

+
+
+
+ +
+
+ {{.CsrfTokenHtml}} + +
+
+ +
+
+ {{.CsrfTokenHtml}} + +
+
+ +
+
+
diff --git a/templates/package/view.tmpl b/templates/package/view.tmpl index 4611cdb8d84..6209e6cb02e 100644 --- a/templates/package/view.tmpl +++ b/templates/package/view.tmpl @@ -19,6 +19,7 @@
+ {{template "package/content/cargo" .}} {{template "package/content/composer" .}} {{template "package/content/conan" .}} {{template "package/content/conda" .}} @@ -43,6 +44,7 @@ {{end}}
{{svg "octicon-calendar" 16 "mr-3"}} {{TimeSinceUnix .PackageDescriptor.Version.CreatedUnix $.locale}}
{{svg "octicon-download" 16 "mr-3"}} {{.PackageDescriptor.Version.DownloadCount}}
+ {{template "package/metadata/cargo" .}} {{template "package/metadata/composer" .}} {{template "package/metadata/conan" .}} {{template "package/metadata/conda" .}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5a3f2c2bb9e..d6e3009b35b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2100,6 +2100,7 @@ }, { "enum": [ + "cargo", "composer", "conan", "conda", diff --git a/templates/user/settings/packages.tmpl b/templates/user/settings/packages.tmpl index 9aae14f9343..472d84327a7 100644 --- a/templates/user/settings/packages.tmpl +++ b/templates/user/settings/packages.tmpl @@ -4,6 +4,7 @@
{{template "base/alert" .}} {{template "package/shared/cleanup_rules/list" .}} + {{template "package/shared/cargo" .}}
{{template "base/footer" .}} diff --git a/tests/integration/api_packages_cargo_test.go b/tests/integration/api_packages_cargo_test.go new file mode 100644 index 00000000000..0c542eaf1e8 --- /dev/null +++ b/tests/integration/api_packages_cargo_test.go @@ -0,0 +1,341 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integration + +import ( + "bytes" + "encoding/binary" + "fmt" + "io" + "net/http" + neturl "net/url" + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/packages" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/json" + cargo_module "code.gitea.io/gitea/modules/packages/cargo" + "code.gitea.io/gitea/modules/setting" + cargo_router "code.gitea.io/gitea/routers/api/packages/cargo" + cargo_service "code.gitea.io/gitea/services/packages/cargo" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestPackageCargo(t *testing.T) { + onGiteaRun(t, testPackageCargo) +} + +func testPackageCargo(t *testing.T, _ *neturl.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + packageName := "cargo-package" + packageVersion := "1.0.3" + packageDescription := "Package Description" + packageAuthor := "KN4CK3R" + packageHomepage := "https://gitea.io/" + packageLicense := "MIT" + + createPackage := func(name, version string) io.Reader { + metadata := `{ + "name":"` + name + `", + "vers":"` + version + `", + "description":"` + packageDescription + `", + "authors": ["` + packageAuthor + `"], + "deps":[ + { + "name":"dep", + "version_req":"1.0", + "registry": "https://gitea.io/user/_cargo-index", + "kind": "normal", + "default_features": true + } + ], + "homepage":"` + packageHomepage + `", + "license":"` + packageLicense + `" +}` + + var buf bytes.Buffer + binary.Write(&buf, binary.LittleEndian, uint32(len(metadata))) + buf.WriteString(metadata) + binary.Write(&buf, binary.LittleEndian, uint32(4)) + buf.WriteString("test") + return &buf + } + + err := cargo_service.InitializeIndexRepository(db.DefaultContext, user, user) + assert.NoError(t, err) + + repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, user.Name, cargo_service.IndexRepositoryName) + assert.NotNil(t, repo) + assert.NoError(t, err) + + readGitContent := func(t *testing.T, path string) string { + gitRepo, err := git.OpenRepository(db.DefaultContext, repo.RepoPath()) + assert.NoError(t, err) + defer gitRepo.Close() + + commit, err := gitRepo.GetBranchCommit(repo.DefaultBranch) + assert.NoError(t, err) + + blob, err := commit.GetBlobByPath(path) + assert.NoError(t, err) + + content, err := blob.GetBlobContent() + assert.NoError(t, err) + + return content + } + + root := fmt.Sprintf("%sapi/packages/%s/cargo", setting.AppURL, user.Name) + url := fmt.Sprintf("%s/api/v1/crates", root) + + t.Run("Index", func(t *testing.T) { + t.Run("Config", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + content := readGitContent(t, cargo_service.ConfigFileName) + + var config cargo_service.Config + err := json.Unmarshal([]byte(content), &config) + assert.NoError(t, err) + + assert.Equal(t, url, config.DownloadURL) + assert.Equal(t, root, config.APIURL) + }) + }) + + t.Run("Upload", func(t *testing.T) { + t.Run("InvalidNameOrVersion", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + content := createPackage("0test", "1.0.0") + + req := NewRequestWithBody(t, "PUT", url+"/new", content) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusBadRequest) + + var status cargo_router.StatusResponse + DecodeJSON(t, resp, &status) + assert.False(t, status.OK) + + content = createPackage("test", "-1.0.0") + + req = NewRequestWithBody(t, "PUT", url+"/new", content) + req = AddBasicAuthHeader(req, user.Name) + resp = MakeRequest(t, req, http.StatusBadRequest) + + DecodeJSON(t, resp, &status) + assert.False(t, status.OK) + }) + + t.Run("InvalidContent", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + metadata := `{"name":"test","vers":"1.0.0"}` + + var buf bytes.Buffer + binary.Write(&buf, binary.LittleEndian, uint32(len(metadata))) + buf.WriteString(metadata) + binary.Write(&buf, binary.LittleEndian, uint32(4)) + buf.WriteString("te") + + req := NewRequestWithBody(t, "PUT", url+"/new", &buf) + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusBadRequest) + }) + + t.Run("Valid", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithBody(t, "PUT", url+"/new", createPackage(packageName, packageVersion)) + MakeRequest(t, req, http.StatusUnauthorized) + + req = NewRequestWithBody(t, "PUT", url+"/new", createPackage(packageName, packageVersion)) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var status cargo_router.StatusResponse + DecodeJSON(t, resp, &status) + assert.True(t, status.OK) + + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeCargo) + assert.NoError(t, err) + assert.Len(t, pvs, 1) + + pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) + assert.NoError(t, err) + assert.NotNil(t, pd.SemVer) + assert.IsType(t, &cargo_module.Metadata{}, pd.Metadata) + assert.Equal(t, packageName, pd.Package.Name) + assert.Equal(t, packageVersion, pd.Version.Version) + + pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) + assert.NoError(t, err) + assert.Len(t, pfs, 1) + assert.Equal(t, fmt.Sprintf("%s-%s.crate", packageName, packageVersion), pfs[0].Name) + assert.True(t, pfs[0].IsLead) + + pb, err := packages.GetBlobByID(db.DefaultContext, pfs[0].BlobID) + assert.NoError(t, err) + assert.EqualValues(t, 4, pb.Size) + + req = NewRequestWithBody(t, "PUT", url+"/new", createPackage(packageName, packageVersion)) + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusConflict) + + t.Run("Index", func(t *testing.T) { + t.Run("Entry", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + content := readGitContent(t, cargo_service.BuildPackagePath(packageName)) + + var entry cargo_service.IndexVersionEntry + err := json.Unmarshal([]byte(content), &entry) + assert.NoError(t, err) + + assert.Equal(t, packageName, entry.Name) + assert.Equal(t, packageVersion, entry.Version) + assert.Equal(t, pb.HashSHA256, entry.FileChecksum) + assert.False(t, entry.Yanked) + assert.Len(t, entry.Dependencies, 1) + dep := entry.Dependencies[0] + assert.Equal(t, "dep", dep.Name) + assert.Equal(t, "1.0", dep.Req) + assert.Equal(t, "normal", dep.Kind) + assert.True(t, dep.DefaultFeatures) + assert.Empty(t, dep.Features) + assert.False(t, dep.Optional) + assert.Nil(t, dep.Target) + assert.NotNil(t, dep.Registry) + assert.Equal(t, "https://gitea.io/user/_cargo-index", *dep.Registry) + assert.Nil(t, dep.Package) + }) + + t.Run("Rebuild", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + err := cargo_service.RebuildIndex(db.DefaultContext, user, user) + assert.NoError(t, err) + + _ = readGitContent(t, cargo_service.BuildPackagePath(packageName)) + }) + }) + }) + }) + + t.Run("Download", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pv, err := packages.GetVersionByNameAndVersion(db.DefaultContext, user.ID, packages.TypeCargo, packageName, packageVersion) + assert.NoError(t, err) + assert.EqualValues(t, 0, pv.DownloadCount) + + pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pv.ID) + assert.NoError(t, err) + assert.Len(t, pfs, 1) + + req := NewRequest(t, "GET", fmt.Sprintf("%s/%s/%s/download", url, neturl.PathEscape(packageName), neturl.PathEscape(pv.Version))) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + assert.Equal(t, "test", resp.Body.String()) + + pv, err = packages.GetVersionByNameAndVersion(db.DefaultContext, user.ID, packages.TypeCargo, packageName, packageVersion) + assert.NoError(t, err) + assert.EqualValues(t, 1, pv.DownloadCount) + }) + + t.Run("Search", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + cases := []struct { + Query string + Page int + PerPage int + ExpectedTotal int64 + ExpectedResults int + }{ + {"", 0, 0, 1, 1}, + {"", 1, 10, 1, 1}, + {"cargo", 1, 0, 1, 1}, + {"cargo", 1, 10, 1, 1}, + {"cargo", 2, 10, 1, 0}, + {"test", 0, 10, 0, 0}, + } + + for i, c := range cases { + req := NewRequest(t, "GET", fmt.Sprintf("%s?q=%s&page=%d&per_page=%d", url, c.Query, c.Page, c.PerPage)) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result cargo_router.SearchResult + DecodeJSON(t, resp, &result) + + assert.Equal(t, c.ExpectedTotal, result.Meta.Total, "case %d: unexpected total hits", i) + assert.Len(t, result.Crates, c.ExpectedResults, "case %d: unexpected result count", i) + } + }) + + t.Run("Yank", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s/yank", url, neturl.PathEscape(packageName), neturl.PathEscape(packageVersion))) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var status cargo_router.StatusResponse + DecodeJSON(t, resp, &status) + assert.True(t, status.OK) + + content := readGitContent(t, cargo_service.BuildPackagePath(packageName)) + + var entry cargo_service.IndexVersionEntry + err := json.Unmarshal([]byte(content), &entry) + assert.NoError(t, err) + + assert.True(t, entry.Yanked) + }) + + t.Run("Unyank", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "PUT", fmt.Sprintf("%s/%s/%s/unyank", url, neturl.PathEscape(packageName), neturl.PathEscape(packageVersion))) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var status cargo_router.StatusResponse + DecodeJSON(t, resp, &status) + assert.True(t, status.OK) + + content := readGitContent(t, cargo_service.BuildPackagePath(packageName)) + + var entry cargo_service.IndexVersionEntry + err := json.Unmarshal([]byte(content), &entry) + assert.NoError(t, err) + + assert.False(t, entry.Yanked) + }) + + t.Run("ListOwners", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/%s/owners", url, neturl.PathEscape(packageName))) + resp := MakeRequest(t, req, http.StatusOK) + + var owners cargo_router.Owners + DecodeJSON(t, resp, &owners) + + assert.Len(t, owners.Users, 1) + assert.Equal(t, user.ID, owners.Users[0].ID) + assert.Equal(t, user.Name, owners.Users[0].Login) + assert.Equal(t, user.DisplayName(), owners.Users[0].Name) + }) +} diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index a6c335eb7e0..39852e212c3 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" packages_service "code.gitea.io/gitea/services/packages" + packages_cleanup_service "code.gitea.io/gitea/services/packages/cleanup" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -246,7 +247,7 @@ func TestPackageCleanup(t *testing.T) { _, err = packages_model.GetInternalVersionByNameAndVersion(db.DefaultContext, 2, packages_model.TypeContainer, "test", container_model.UploadVersion) assert.NoError(t, err) - err = packages_service.Cleanup(db.DefaultContext, duration) + err = packages_cleanup_service.Cleanup(db.DefaultContext, duration) assert.NoError(t, err) pbs, err = packages_model.FindExpiredUnreferencedBlobs(db.DefaultContext, duration) @@ -383,7 +384,7 @@ func TestPackageCleanup(t *testing.T) { pcr, err := packages_model.InsertCleanupRule(db.DefaultContext, c.Rule) assert.NoError(t, err) - err = packages_service.Cleanup(db.DefaultContext, duration) + err = packages_cleanup_service.Cleanup(db.DefaultContext, duration) assert.NoError(t, err) for _, v := range c.Versions { diff --git a/web_src/svg/gitea-cargo.svg b/web_src/svg/gitea-cargo.svg new file mode 100644 index 00000000000..dbec107ad0d --- /dev/null +++ b/web_src/svg/gitea-cargo.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file From c18a62279a54f1ba0dc0293dddbe197f527fbf00 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sun, 5 Feb 2023 19:57:38 +0800 Subject: [PATCH 14/20] Fix time to NotifyPullRequestSynchronized (#22650) Should call `PushToBaseRepo` before `notification.NotifyPullRequestSynchronized`. Or the notifier will get an old commit when reading branch `pull/xxx/head`. Found by ~#21937~ #22679. Co-authored-by: Lunny Xiao --- models/issues/pull_list.go | 13 ++++++++++++- services/pull/pull.go | 36 ++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index 007c2fd9033..68f62ffc433 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -13,6 +13,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" "xorm.io/xorm" ) @@ -175,7 +176,17 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error { } for _, pr := range prs { pr.Issue = set[pr.IssueID] - pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync + /* + Old code: + pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync + + It's worth panic because it's almost impossible to happen under normal use. + But in integration testing, an asynchronous task could read a database that has been reset. + So returning an error would make more sense, let the caller has a choice to ignore it. + */ + if pr.Issue == nil { + return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist) + } } return nil } diff --git a/services/pull/pull.go b/services/pull/pull.go index 317875d2112..0d260c93b1e 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -263,6 +263,24 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, return } + for _, pr := range prs { + log.Trace("Updating PR[%d]: composing new test task", pr.ID) + if pr.Flow == issues_model.PullRequestFlowGithub { + if err := PushToBaseRepo(ctx, pr); err != nil { + log.Error("PushToBaseRepo: %v", err) + continue + } + } else { + continue + } + + AddToTaskQueue(pr) + comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID) + if err == nil && comment != nil { + notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment) + } + } + if isSync { requests := issues_model.PullRequestList(prs) if err = requests.LoadAttributes(); err != nil { @@ -303,24 +321,6 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, } } - for _, pr := range prs { - log.Trace("Updating PR[%d]: composing new test task", pr.ID) - if pr.Flow == issues_model.PullRequestFlowGithub { - if err := PushToBaseRepo(ctx, pr); err != nil { - log.Error("PushToBaseRepo: %v", err) - continue - } - } else { - continue - } - - AddToTaskQueue(pr) - comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID) - if err == nil && comment != nil { - notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment) - } - } - log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch) prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(repoID, branch) if err != nil { From d74d16a4b1c79b5b1fc58998aa38bf299cee10ec Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sun, 5 Feb 2023 22:24:43 +0800 Subject: [PATCH 15/20] Set PR for issue when load attributes for PRs (#22766) A missing patch for #22650. --- models/issues/pull_list.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index 68f62ffc433..f4efd916c86 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -187,6 +187,7 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error { if pr.Issue == nil { return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist) } + pr.Issue.PullRequest = pr } return nil } From c07199f9ab4ac5a1ecbfd27616ad44bf35d00fc1 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 6 Feb 2023 01:06:26 +0900 Subject: [PATCH 16/20] remove update language in ProfilePost (#22748) Fixes https://github.com/go-gitea/gitea/issues/22703 Change language has been moved to `UpdateUserLang` Co-authored-by: Lunny Xiao --- routers/web/user/setting/profile.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index e01f3cdeea9..f0f053a514e 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -137,11 +137,8 @@ func ProfilePost(ctx *context.Context) { return } - // Update the language to the one we just set - middleware.SetLocaleCookie(ctx.Resp, ctx.Doer.Language, 0) - log.Trace("User settings updated: %s", ctx.Doer.Name) - ctx.Flash.Success(translation.NewLocale(ctx.Doer.Language).Tr("settings.update_profile_success")) + ctx.Flash.Success(ctx.Tr("settings.update_profile_success")) ctx.Redirect(setting.AppSubURL + "/user/settings") } From ff18d1744273d093d854f548662a0c204f220c16 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 5 Feb 2023 16:42:48 -0500 Subject: [PATCH 17/20] use drone secrets for s3 config (#22770) --- .drone.yml | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/.drone.yml b/.drone.yml index f9da8f97438..dc8423875a6 100644 --- a/.drone.yml +++ b/.drone.yml @@ -763,10 +763,16 @@ steps: image: woodpeckerci/plugin-s3:latest pull: always settings: - acl: public-read - bucket: gitea-artifacts - endpoint: https://ams3.digitaloceanspaces.com - path_style: true + acl: + from_secret: aws_s3_acl + region: + from_secret: aws_s3_region + bucket: + from_secret: aws_s3_bucket + endpoint: + from_secret: aws_s3_endpoint + path_style: + from_secret: aws_s3_path_style source: "dist/release/*" strip_prefix: dist/release/ target: "/gitea/${DRONE_BRANCH##release/v}" @@ -784,10 +790,16 @@ steps: - name: release-main image: woodpeckerci/plugin-s3:latest settings: - acl: public-read - bucket: gitea-artifacts - endpoint: https://ams3.digitaloceanspaces.com - path_style: true + acl: + from_secret: aws_s3_acl + region: + from_secret: aws_s3_region + bucket: + from_secret: aws_s3_bucket + endpoint: + from_secret: aws_s3_endpoint + path_style: + from_secret: aws_s3_path_style source: "dist/release/*" strip_prefix: dist/release/ target: /gitea/main @@ -886,10 +898,16 @@ steps: image: woodpeckerci/plugin-s3:latest pull: always settings: - acl: public-read - bucket: gitea-artifacts - endpoint: https://ams3.digitaloceanspaces.com - path_style: true + acl: + from_secret: aws_s3_acl + region: + from_secret: aws_s3_region + bucket: + from_secret: aws_s3_bucket + endpoint: + from_secret: aws_s3_endpoint + path_style: + from_secret: aws_s3_path_style source: "dist/release/*" strip_prefix: dist/release/ target: "/gitea/${DRONE_TAG##v}" From d987ac6bf1d78b3a9bbd213e73b871ebc687acb2 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Mon, 6 Feb 2023 02:49:21 +0100 Subject: [PATCH 18/20] Add Chef package registry (#22554) This PR implements a [Chef registry](https://chef.io/) to manage cookbooks. This package type was a bit complicated because Chef uses RSA signed requests as authentication with the registry. ![grafik](https://user-images.githubusercontent.com/1666336/213747995-46819fd8-c3d6-45a2-afd4-a4c3c8505a4a.png) ![grafik](https://user-images.githubusercontent.com/1666336/213748145-d01c9e81-d4dd-41e3-a3cc-8241862c3166.png) Co-authored-by: Lunny Xiao --- custom/conf/app.example.ini | 2 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + docs/content/doc/packages/chef.en-us.md | 96 +++ docs/content/doc/packages/overview.en-us.md | 1 + models/packages/descriptor.go | 3 + models/packages/package.go | 6 + modules/activitypub/user_settings.go | 5 +- modules/packages/chef/metadata.go | 134 +++++ modules/packages/chef/metadata_test.go | 92 +++ modules/setting/packages.go | 2 + modules/{activitypub => util}/keypair.go | 10 +- modules/{activitypub => util}/keypair_test.go | 6 +- options/locale/locale_en-US.ini | 12 +- public/img/svg/gitea-chef.svg | 1 + routers/api/packages/api.go | 21 + routers/api/packages/chef/auth.go | 270 +++++++++ routers/api/packages/chef/chef.go | 404 +++++++++++++ routers/api/v1/packages/package.go | 2 +- routers/web/user/setting/packages.go | 22 + routers/web/web.go | 1 + services/forms/package_form.go | 2 +- services/packages/packages.go | 2 + templates/package/content/chef.tmpl | 48 ++ templates/package/metadata/chef.tmpl | 5 + templates/package/metadata/container.tmpl | 4 +- templates/package/metadata/pub.tmpl | 4 +- templates/package/view.tmpl | 2 + templates/swagger/v1_json.tmpl | 1 + templates/user/settings/packages.tmpl | 18 + tests/integration/api_packages_chef_test.go | 560 ++++++++++++++++++ web_src/svg/gitea-chef.svg | 20 + 31 files changed, 1737 insertions(+), 20 deletions(-) create mode 100644 docs/content/doc/packages/chef.en-us.md create mode 100644 modules/packages/chef/metadata.go create mode 100644 modules/packages/chef/metadata_test.go rename modules/{activitypub => util}/keypair.go (76%) rename modules/{activitypub => util}/keypair_test.go (93%) create mode 100644 public/img/svg/gitea-chef.svg create mode 100644 routers/api/packages/chef/auth.go create mode 100644 routers/api/packages/chef/chef.go create mode 100644 templates/package/content/chef.tmpl create mode 100644 templates/package/metadata/chef.tmpl create mode 100644 tests/integration/api_packages_chef_test.go create mode 100644 web_src/svg/gitea-chef.svg diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 0f1f18646b6..b478785a077 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2460,6 +2460,8 @@ ROUTER = console ;LIMIT_TOTAL_OWNER_SIZE = -1 ;; Maximum size of a Cargo upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) ;LIMIT_SIZE_CARGO = -1 +;; Maximum size of a Chef upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) +;LIMIT_SIZE_CHEF = -1 ;; Maximum size of a Composer upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) ;LIMIT_SIZE_COMPOSER = -1 ;; Maximum size of a Conan upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index c9116edc665..04344b15dc7 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -1214,6 +1214,7 @@ Task queue configuration has been moved to `queue.task`. However, the below conf - `LIMIT_TOTAL_OWNER_COUNT`: **-1**: Maximum count of package versions a single owner can have (`-1` means no limits) - `LIMIT_TOTAL_OWNER_SIZE`: **-1**: Maximum size of packages a single owner can use (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) - `LIMIT_SIZE_CARGO`: **-1**: Maximum size of a Cargo upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) +- `LIMIT_SIZE_CHEF`: **-1**: Maximum size of a Chef upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) - `LIMIT_SIZE_COMPOSER`: **-1**: Maximum size of a Composer upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) - `LIMIT_SIZE_CONAN`: **-1**: Maximum size of a Conan upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) - `LIMIT_SIZE_CONDA`: **-1**: Maximum size of a Conda upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) diff --git a/docs/content/doc/packages/chef.en-us.md b/docs/content/doc/packages/chef.en-us.md new file mode 100644 index 00000000000..ecc774d7928 --- /dev/null +++ b/docs/content/doc/packages/chef.en-us.md @@ -0,0 +1,96 @@ +--- +date: "2023-01-20T00:00:00+00:00" +title: "Chef Packages Repository" +slug: "packages/chef" +draft: false +toc: false +menu: + sidebar: + parent: "packages" + name: "Chef" + weight: 5 + identifier: "chef" +--- + +# Chef Packages Repository + +Publish [Chef](https://chef.io/) cookbooks for your user or organization. + +**Table of Contents** + +{{< toc >}} + +## Requirements + +To work with the Chef package registry, you have to use [`knife`](https://docs.chef.io/workstation/knife/). + +## Authentication + +The Chef package registry does not use an username:password authentication but signed requests with a private:public key pair. +Visit the package owner settings page to create the necessary key pair. +Only the public key is stored inside Gitea. if you loose access to the private key you must re-generate the key pair. +[Configure `knife`](https://docs.chef.io/workstation/knife_setup/) to use the downloaded private key with your Gitea username as `client_name`. + +## Configure the package registry + +To [configure `knife`](https://docs.chef.io/workstation/knife_setup/) to use the Gitea package registry add the url to the `~/.chef/config.rb` file. + +``` +knife[:supermarket_site] = 'https://gitea.example.com/api/packages/{owner}/chef' +``` + +| Parameter | Description | +| --------- | ----------- | +| `owner` | The owner of the package. | + +## Publish a package + +To publish a Chef package execute the following command: + +```shell +knife supermarket share {package_name} +``` + +| Parameter | Description | +| -------------- | ----------- | +| `package_name` | The package name. | + +You cannot publish a package if a package of the same name and version already exists. You must delete the existing package first. + +## Install a package + +To install a package from the package registry, execute the following command: + +```shell +knife supermarket install {package_name} +``` + +Optional you can specify the package version: + +```shell +knife supermarket install {package_name} {package_version} +``` + +| Parameter | Description | +| ----------------- | ----------- | +| `package_name` | The package name. | +| `package_version` | The package version. | + +## Delete a package + +If you want to remove a package from the registry, execute the following command: + +```shell +knife supermarket unshare {package_name} +``` + +Optional you can specify the package version: + +```shell +knife supermarket unshare {package_name}/versions/{package_version} +``` + +| Parameter | Description | +| ----------------- | ----------- | +| `package_name` | The package name. | +| `package_version` | The package version. | diff --git a/docs/content/doc/packages/overview.en-us.md b/docs/content/doc/packages/overview.en-us.md index b3ccb73c196..1199d9ede0a 100644 --- a/docs/content/doc/packages/overview.en-us.md +++ b/docs/content/doc/packages/overview.en-us.md @@ -27,6 +27,7 @@ The following package managers are currently supported: | Name | Language | Package client | | ---- | -------- | -------------- | | [Cargo]({{< relref "doc/packages/cargo.en-us.md" >}}) | Rust | `cargo` | +| [Chef]({{< relref "doc/packages/chef.en-us.md" >}}) | - | `knife` | | [Composer]({{< relref "doc/packages/composer.en-us.md" >}}) | PHP | `composer` | | [Conan]({{< relref "doc/packages/conan.en-us.md" >}}) | C++ | `conan` | | [Conda]({{< relref "doc/packages/conda.en-us.md" >}}) | - | `conda` | diff --git a/models/packages/descriptor.go b/models/packages/descriptor.go index 40010eb720c..96f34a22ee9 100644 --- a/models/packages/descriptor.go +++ b/models/packages/descriptor.go @@ -12,6 +12,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/packages/cargo" + "code.gitea.io/gitea/modules/packages/chef" "code.gitea.io/gitea/modules/packages/composer" "code.gitea.io/gitea/modules/packages/conan" "code.gitea.io/gitea/modules/packages/conda" @@ -132,6 +133,8 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc switch p.Type { case TypeCargo: metadata = &cargo.Metadata{} + case TypeChef: + metadata = &chef.Metadata{} case TypeComposer: metadata = &composer.Metadata{} case TypeConan: diff --git a/models/packages/package.go b/models/packages/package.go index b6b033cc95a..32f30fab9b4 100644 --- a/models/packages/package.go +++ b/models/packages/package.go @@ -31,6 +31,7 @@ type Type string // List of supported packages const ( TypeCargo Type = "cargo" + TypeChef Type = "chef" TypeComposer Type = "composer" TypeConan Type = "conan" TypeConda Type = "conda" @@ -48,6 +49,7 @@ const ( var TypeList = []Type{ TypeCargo, + TypeChef, TypeComposer, TypeConan, TypeConda, @@ -68,6 +70,8 @@ func (pt Type) Name() string { switch pt { case TypeCargo: return "Cargo" + case TypeChef: + return "Chef" case TypeComposer: return "Composer" case TypeConan: @@ -103,6 +107,8 @@ func (pt Type) SVGName() string { switch pt { case TypeCargo: return "gitea-cargo" + case TypeChef: + return "gitea-chef" case TypeComposer: return "gitea-composer" case TypeConan: diff --git a/modules/activitypub/user_settings.go b/modules/activitypub/user_settings.go index ec5fa59842f..2d156c17e65 100644 --- a/modules/activitypub/user_settings.go +++ b/modules/activitypub/user_settings.go @@ -5,8 +5,11 @@ package activitypub import ( user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/util" ) +const rsaBits = 2048 + // GetKeyPair function returns a user's private and public keys func GetKeyPair(user *user_model.User) (pub, priv string, err error) { var settings map[string]*user_model.Setting @@ -14,7 +17,7 @@ func GetKeyPair(user *user_model.User) (pub, priv string, err error) { if err != nil { return } else if len(settings) == 0 { - if priv, pub, err = GenerateKeyPair(); err != nil { + if priv, pub, err = util.GenerateKeyPair(rsaBits); err != nil { return } if err = user_model.SetUserSetting(user.ID, user_model.UserActivityPubPrivPem, priv); err != nil { diff --git a/modules/packages/chef/metadata.go b/modules/packages/chef/metadata.go new file mode 100644 index 00000000000..a1c91870c29 --- /dev/null +++ b/modules/packages/chef/metadata.go @@ -0,0 +1,134 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package chef + +import ( + "archive/tar" + "compress/gzip" + "io" + "regexp" + "strings" + + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/validation" +) + +const ( + KeyBits = 4096 + SettingPublicPem = "chef.public_pem" +) + +var ( + ErrMissingMetadataFile = util.NewInvalidArgumentErrorf("metadata.json file is missing") + ErrInvalidName = util.NewInvalidArgumentErrorf("package name is invalid") + ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid") + + namePattern = regexp.MustCompile(`\A\S+\z`) + versionPattern = regexp.MustCompile(`\A\d+\.\d+(?:\.\d+)?\z`) +) + +// Package represents a Chef package +type Package struct { + Name string + Version string + Metadata *Metadata +} + +// Metadata represents the metadata of a Chef package +type Metadata struct { + Description string `json:"description,omitempty"` + LongDescription string `json:"long_description,omitempty"` + Author string `json:"author,omitempty"` + License string `json:"license,omitempty"` + RepositoryURL string `json:"repository_url,omitempty"` + Dependencies map[string]string `json:"dependencies,omitempty"` +} + +type chefMetadata struct { + Name string `json:"name"` + Description string `json:"description"` + LongDescription string `json:"long_description"` + Maintainer string `json:"maintainer"` + MaintainerEmail string `json:"maintainer_email"` + License string `json:"license"` + Platforms map[string]string `json:"platforms"` + Dependencies map[string]string `json:"dependencies"` + Providing map[string]string `json:"providing"` + Recipes map[string]string `json:"recipes"` + Version string `json:"version"` + SourceURL string `json:"source_url"` + IssuesURL string `json:"issues_url"` + Privacy bool `json:"privacy"` + ChefVersions [][]string `json:"chef_versions"` + Gems [][]string `json:"gems"` + EagerLoadLibraries bool `json:"eager_load_libraries"` +} + +// ParsePackage parses the Chef package file +func ParsePackage(r io.Reader) (*Package, error) { + gzr, err := gzip.NewReader(r) + if err != nil { + return nil, err + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + for { + hd, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + + if hd.Typeflag != tar.TypeReg { + continue + } + + if strings.Count(hd.Name, "/") != 1 { + continue + } + + if hd.FileInfo().Name() == "metadata.json" { + return ParseChefMetadata(tr) + } + } + + return nil, ErrMissingMetadataFile +} + +// ParseChefMetadata parses a metadata.json file to retrieve the metadata of a Chef package +func ParseChefMetadata(r io.Reader) (*Package, error) { + var cm chefMetadata + if err := json.NewDecoder(r).Decode(&cm); err != nil { + return nil, err + } + + if !namePattern.MatchString(cm.Name) { + return nil, ErrInvalidName + } + + if !versionPattern.MatchString(cm.Version) { + return nil, ErrInvalidVersion + } + + if !validation.IsValidURL(cm.SourceURL) { + cm.SourceURL = "" + } + + return &Package{ + Name: cm.Name, + Version: cm.Version, + Metadata: &Metadata{ + Description: cm.Description, + LongDescription: cm.LongDescription, + Author: cm.Maintainer, + License: cm.License, + RepositoryURL: cm.SourceURL, + Dependencies: cm.Dependencies, + }, + }, nil +} diff --git a/modules/packages/chef/metadata_test.go b/modules/packages/chef/metadata_test.go new file mode 100644 index 00000000000..6def4162a9d --- /dev/null +++ b/modules/packages/chef/metadata_test.go @@ -0,0 +1,92 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package chef + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +const ( + packageName = "gitea" + packageVersion = "1.0.1" + packageAuthor = "KN4CK3R" + packageDescription = "Package Description" + packageRepositoryURL = "https://gitea.io/gitea/gitea" +) + +func TestParsePackage(t *testing.T) { + t.Run("MissingMetadataFile", func(t *testing.T) { + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + tw := tar.NewWriter(zw) + tw.Close() + zw.Close() + + p, err := ParsePackage(&buf) + assert.Nil(t, p) + assert.ErrorIs(t, err, ErrMissingMetadataFile) + }) + + t.Run("Valid", func(t *testing.T) { + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + tw := tar.NewWriter(zw) + + content := `{"name":"` + packageName + `","version":"` + packageVersion + `"}` + + hdr := &tar.Header{ + Name: packageName + "/metadata.json", + Mode: 0o600, + Size: int64(len(content)), + } + tw.WriteHeader(hdr) + tw.Write([]byte(content)) + + tw.Close() + zw.Close() + + p, err := ParsePackage(&buf) + assert.NoError(t, err) + assert.NotNil(t, p) + assert.Equal(t, packageName, p.Name) + assert.Equal(t, packageVersion, p.Version) + assert.NotNil(t, p.Metadata) + }) +} + +func TestParseChefMetadata(t *testing.T) { + t.Run("InvalidName", func(t *testing.T) { + for _, name := range []string{" test", "test "} { + p, err := ParseChefMetadata(strings.NewReader(`{"name":"` + name + `","version":"1.0.0"}`)) + assert.Nil(t, p) + assert.ErrorIs(t, err, ErrInvalidName) + } + }) + + t.Run("InvalidVersion", func(t *testing.T) { + for _, version := range []string{"1", "1.2.3.4", "1.0.0 "} { + p, err := ParseChefMetadata(strings.NewReader(`{"name":"test","version":"` + version + `"}`)) + assert.Nil(t, p) + assert.ErrorIs(t, err, ErrInvalidVersion) + } + }) + + t.Run("Valid", func(t *testing.T) { + p, err := ParseChefMetadata(strings.NewReader(`{"name":"` + packageName + `","version":"` + packageVersion + `","description":"` + packageDescription + `","maintainer":"` + packageAuthor + `","source_url":"` + packageRepositoryURL + `"}`)) + assert.NotNil(t, p) + assert.NoError(t, err) + + assert.Equal(t, packageName, p.Name) + assert.Equal(t, packageVersion, p.Version) + assert.Equal(t, packageDescription, p.Metadata.Description) + assert.Equal(t, packageAuthor, p.Metadata.Author) + assert.Equal(t, packageRepositoryURL, p.Metadata.RepositoryURL) + }) +} diff --git a/modules/setting/packages.go b/modules/setting/packages.go index 190c17dd8fb..84da4eb53e8 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -26,6 +26,7 @@ var ( LimitTotalOwnerCount int64 LimitTotalOwnerSize int64 LimitSizeCargo int64 + LimitSizeChef int64 LimitSizeComposer int64 LimitSizeConan int64 LimitSizeConda int64 @@ -67,6 +68,7 @@ func newPackages() { Packages.LimitTotalOwnerSize = mustBytes(sec, "LIMIT_TOTAL_OWNER_SIZE") Packages.LimitSizeCargo = mustBytes(sec, "LIMIT_SIZE_CARGO") + Packages.LimitSizeChef = mustBytes(sec, "LIMIT_SIZE_CHEF") Packages.LimitSizeComposer = mustBytes(sec, "LIMIT_SIZE_COMPOSER") Packages.LimitSizeConan = mustBytes(sec, "LIMIT_SIZE_CONAN") Packages.LimitSizeConda = mustBytes(sec, "LIMIT_SIZE_CONDA") diff --git a/modules/activitypub/keypair.go b/modules/util/keypair.go similarity index 76% rename from modules/activitypub/keypair.go rename to modules/util/keypair.go index 299bdc43e33..5a3ce715a40 100644 --- a/modules/activitypub/keypair.go +++ b/modules/util/keypair.go @@ -1,7 +1,7 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package activitypub +package util import ( "crypto/rand" @@ -10,11 +10,9 @@ import ( "encoding/pem" ) -const rsaBits = 2048 - -// GenerateKeyPair generates a public and private keypair for signing actions by users for activitypub purposes -func GenerateKeyPair() (string, string, error) { - priv, _ := rsa.GenerateKey(rand.Reader, rsaBits) +// GenerateKeyPair generates a public and private keypair +func GenerateKeyPair(bits int) (string, string, error) { + priv, _ := rsa.GenerateKey(rand.Reader, bits) privPem, err := pemBlockForPriv(priv) if err != nil { return "", "", err diff --git a/modules/activitypub/keypair_test.go b/modules/util/keypair_test.go similarity index 93% rename from modules/activitypub/keypair_test.go rename to modules/util/keypair_test.go index 888254c9dae..c6f68c845a4 100644 --- a/modules/activitypub/keypair_test.go +++ b/modules/util/keypair_test.go @@ -1,7 +1,7 @@ // Copyright 2021 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package activitypub +package util import ( "crypto" @@ -17,7 +17,7 @@ import ( ) func TestKeygen(t *testing.T) { - priv, pub, err := GenerateKeyPair() + priv, pub, err := GenerateKeyPair(2048) assert.NoError(t, err) assert.NotEmpty(t, priv) @@ -28,7 +28,7 @@ func TestKeygen(t *testing.T) { } func TestSignUsingKeys(t *testing.T) { - priv, pub, err := GenerateKeyPair() + priv, pub, err := GenerateKeyPair(2048) assert.NoError(t, err) privPem, _ := pem.Decode([]byte(priv)) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index bc2e8cb91cf..a7506986f69 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3145,6 +3145,8 @@ keywords = Keywords details = Details details.author = Author details.project_site = Project Site +details.repository_site = Repository Site +details.documentation_site = Documentation Site details.license = License assets = Assets versions = Versions @@ -3157,6 +3159,9 @@ cargo.install = To install the package using Cargo, run the following command: cargo.documentation = For more information on the Cargo registry, see the documentation. cargo.details.repository_site = Repository Site cargo.details.documentation_site = Documentation Site +chef.registry = Setup this registry in your ~/.chef/config.rb file: +chef.install = To install the package, run the following command: +chef.documentation = For more information on the Chef registry, see the documentation. composer.registry = Setup this registry in your ~/.composer/config.json file: composer.install = To install the package using Composer, run the following command: composer.documentation = For more information on the Composer registry, see the documentation. @@ -3173,8 +3178,6 @@ conda.details.repository_site = Repository Site conda.details.documentation_site = Documentation Site container.details.type = Image Type container.details.platform = Platform -container.details.repository_site = Repository Site -container.details.documentation_site = Documentation Site container.pull = Pull the image from the command line: container.digest = Digest: container.documentation = For more information on the Container registry, see the documentation. @@ -3208,8 +3211,6 @@ npm.dependencies.optional = Optional Dependencies npm.details.tag = Tag pub.install = To install the package using Dart, run the following command: pub.documentation = For more information on the Pub registry, see the documentation. -pub.details.repository_site = Repository Site -pub.details.documentation_site = Documentation Site pypi.requires = Requires Python pypi.install = To install the package using pip, run the following command: pypi.documentation = For more information on the PyPI registry, see the documentation. @@ -3262,6 +3263,9 @@ owner.settings.cleanuprules.remove.days = Remove versions older than owner.settings.cleanuprules.remove.pattern = Remove versions matching owner.settings.cleanuprules.success.update = Cleanup rule has been updated. owner.settings.cleanuprules.success.delete = Cleanup rule has been deleted. +owner.settings.chef.title = Chef Registry +owner.settings.chef.keypair = Generate key pair +owner.settings.chef.keypair.description = Generate a key pair used to authenticate against the Chef registry. The previous key can not be used afterwards. [secrets] secrets = Secrets diff --git a/public/img/svg/gitea-chef.svg b/public/img/svg/gitea-chef.svg new file mode 100644 index 00000000000..8f1cd6165ca --- /dev/null +++ b/public/img/svg/gitea-chef.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 8ec9ae9bf70..9f77367d6f5 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/packages/cargo" + "code.gitea.io/gitea/routers/api/packages/chef" "code.gitea.io/gitea/routers/api/packages/composer" "code.gitea.io/gitea/routers/api/packages/conan" "code.gitea.io/gitea/routers/api/packages/conda" @@ -54,6 +55,7 @@ func CommonRoutes(ctx gocontext.Context) *web.Route { &auth.Basic{}, &nuget.Auth{}, &conan.Auth{}, + &chef.Auth{}, } if setting.Service.EnableReverseProxyAuth { authMethods = append(authMethods, &auth.ReverseProxy{}) @@ -86,6 +88,25 @@ func CommonRoutes(ctx gocontext.Context) *web.Route { }) }) }, reqPackageAccess(perm.AccessModeRead)) + r.Group("/chef", func() { + r.Group("/api/v1", func() { + r.Get("/universe", chef.PackagesUniverse) + r.Get("/search", chef.EnumeratePackages) + r.Group("/cookbooks", func() { + r.Get("", chef.EnumeratePackages) + r.Post("", reqPackageAccess(perm.AccessModeWrite), chef.UploadPackage) + r.Group("/{name}", func() { + r.Get("", chef.PackageMetadata) + r.Group("/versions/{version}", func() { + r.Get("", chef.PackageVersionMetadata) + r.Delete("", reqPackageAccess(perm.AccessModeWrite), chef.DeletePackageVersion) + r.Get("/download", chef.DownloadPackage) + }) + r.Delete("", reqPackageAccess(perm.AccessModeWrite), chef.DeletePackage) + }) + }) + }) + }, reqPackageAccess(perm.AccessModeRead)) r.Group("/composer", func() { r.Get("/packages.json", composer.ServiceIndex) r.Get("/search.json", composer.SearchPackages) diff --git a/routers/api/packages/chef/auth.go b/routers/api/packages/chef/auth.go new file mode 100644 index 00000000000..69f7b763ab6 --- /dev/null +++ b/routers/api/packages/chef/auth.go @@ -0,0 +1,270 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package chef + +import ( + "crypto" + "crypto/rsa" + "crypto/sha1" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "fmt" + "hash" + "math/big" + "net/http" + "path" + "regexp" + "strconv" + "strings" + "time" + + user_model "code.gitea.io/gitea/models/user" + chef_module "code.gitea.io/gitea/modules/packages/chef" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/auth" +) + +const ( + maxTimeDifference = 10 * time.Minute +) + +var ( + algorithmPattern = regexp.MustCompile(`algorithm=(\w+)`) + versionPattern = regexp.MustCompile(`version=(\d+\.\d+)`) + authorizationPattern = regexp.MustCompile(`\AX-Ops-Authorization-(\d+)`) +) + +// Documentation: +// https://docs.chef.io/server/api_chef_server/#required-headers +// https://github.com/chef-boneyard/chef-rfc/blob/master/rfc065-sign-v1.3.md +// https://github.com/chef/mixlib-authentication/blob/bc8adbef833d4be23dc78cb23e6fe44b51ebc34f/lib/mixlib/authentication/signedheaderauth.rb + +type Auth struct{} + +func (a *Auth) Name() string { + return "chef" +} + +// Verify extracts the user from the signed request +// If the request is signed with the user private key the user is verified. +func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) { + u, err := getUserFromRequest(req) + if err != nil { + return nil, err + } + if u == nil { + return nil, nil + } + + pub, err := getUserPublicKey(u) + if err != nil { + return nil, err + } + + if err := verifyTimestamp(req); err != nil { + return nil, err + } + + version, err := getSignVersion(req) + if err != nil { + return nil, err + } + + if err := verifySignedHeaders(req, version, pub.(*rsa.PublicKey)); err != nil { + return nil, err + } + + return u, nil +} + +func getUserFromRequest(req *http.Request) (*user_model.User, error) { + username := req.Header.Get("X-Ops-Userid") + if username == "" { + return nil, nil + } + + return user_model.GetUserByName(req.Context(), username) +} + +func getUserPublicKey(u *user_model.User) (crypto.PublicKey, error) { + pubKey, err := user_model.GetSetting(u.ID, chef_module.SettingPublicPem) + if err != nil { + return nil, err + } + + pubPem, _ := pem.Decode([]byte(pubKey)) + + return x509.ParsePKIXPublicKey(pubPem.Bytes) +} + +func verifyTimestamp(req *http.Request) error { + hdr := req.Header.Get("X-Ops-Timestamp") + if hdr == "" { + return util.NewInvalidArgumentErrorf("X-Ops-Timestamp header missing") + } + + ts, err := time.Parse(time.RFC3339, hdr) + if err != nil { + return err + } + + diff := time.Now().UTC().Sub(ts) + if diff < 0 { + diff = -diff + } + + if diff > maxTimeDifference { + return fmt.Errorf("time difference") + } + + return nil +} + +func getSignVersion(req *http.Request) (string, error) { + hdr := req.Header.Get("X-Ops-Sign") + if hdr == "" { + return "", util.NewInvalidArgumentErrorf("X-Ops-Sign header missing") + } + + m := versionPattern.FindStringSubmatch(hdr) + if len(m) != 2 { + return "", util.NewInvalidArgumentErrorf("invalid X-Ops-Sign header") + } + + switch m[1] { + case "1.0", "1.1", "1.2", "1.3": + default: + return "", util.NewInvalidArgumentErrorf("unsupported version") + } + + version := m[1] + + m = algorithmPattern.FindStringSubmatch(hdr) + if len(m) == 2 && m[1] != "sha1" && !(m[1] == "sha256" && version == "1.3") { + return "", util.NewInvalidArgumentErrorf("unsupported algorithm") + } + + return version, nil +} + +func verifySignedHeaders(req *http.Request, version string, pub *rsa.PublicKey) error { + authorizationData, err := getAuthorizationData(req) + if err != nil { + return err + } + + checkData := buildCheckData(req, version) + + switch version { + case "1.3": + return verifyDataNew(authorizationData, checkData, pub, crypto.SHA256) + case "1.2": + return verifyDataNew(authorizationData, checkData, pub, crypto.SHA1) + default: + return verifyDataOld(authorizationData, checkData, pub) + } +} + +func getAuthorizationData(req *http.Request) ([]byte, error) { + valueList := make(map[int]string) + for k, vs := range req.Header { + if m := authorizationPattern.FindStringSubmatch(k); m != nil { + index, _ := strconv.Atoi(m[1]) + var v string + if len(vs) == 0 { + v = "" + } else { + v = vs[0] + } + valueList[index] = v + } + } + + tmp := make([]string, len(valueList)) + for k, v := range valueList { + if k > len(tmp) { + return nil, fmt.Errorf("invalid X-Ops-Authorization headers") + } + tmp[k-1] = v + } + + return base64.StdEncoding.DecodeString(strings.Join(tmp, "")) +} + +func buildCheckData(req *http.Request, version string) []byte { + username := req.Header.Get("X-Ops-Userid") + if version != "1.0" && version != "1.3" { + sum := sha1.Sum([]byte(username)) + username = base64.StdEncoding.EncodeToString(sum[:]) + } + + var data string + if version == "1.3" { + data = fmt.Sprintf( + "Method:%s\nPath:%s\nX-Ops-Content-Hash:%s\nX-Ops-Sign:version=%s\nX-Ops-Timestamp:%s\nX-Ops-UserId:%s\nX-Ops-Server-API-Version:%s", + req.Method, + path.Clean(req.URL.Path), + req.Header.Get("X-Ops-Content-Hash"), + version, + req.Header.Get("X-Ops-Timestamp"), + username, + req.Header.Get("X-Ops-Server-Api-Version"), + ) + } else { + sum := sha1.Sum([]byte(path.Clean(req.URL.Path))) + data = fmt.Sprintf( + "Method:%s\nHashed Path:%s\nX-Ops-Content-Hash:%s\nX-Ops-Timestamp:%s\nX-Ops-UserId:%s", + req.Method, + base64.StdEncoding.EncodeToString(sum[:]), + req.Header.Get("X-Ops-Content-Hash"), + req.Header.Get("X-Ops-Timestamp"), + username, + ) + } + + return []byte(data) +} + +func verifyDataNew(signature, data []byte, pub *rsa.PublicKey, algo crypto.Hash) error { + var h hash.Hash + if algo == crypto.SHA256 { + h = sha256.New() + } else { + h = sha1.New() + } + if _, err := h.Write(data); err != nil { + return err + } + + return rsa.VerifyPKCS1v15(pub, algo, h.Sum(nil), signature) +} + +func verifyDataOld(signature, data []byte, pub *rsa.PublicKey) error { + c := new(big.Int) + m := new(big.Int) + m.SetBytes(signature) + e := big.NewInt(int64(pub.E)) + c.Exp(m, e, pub.N) + + out := c.Bytes() + + skip := 0 + for i := 2; i < len(out); i++ { + if i+1 >= len(out) { + break + } + if out[i] == 0xFF && out[i+1] == 0 { + skip = i + 2 + break + } + } + + if !util.SliceEqual(out[skip:], data) { + return fmt.Errorf("could not verify signature") + } + + return nil +} diff --git a/routers/api/packages/chef/chef.go b/routers/api/packages/chef/chef.go new file mode 100644 index 00000000000..28d07dea477 --- /dev/null +++ b/routers/api/packages/chef/chef.go @@ -0,0 +1,404 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package chef + +import ( + "errors" + "fmt" + "io" + "net/http" + "net/url" + "sort" + "strings" + "time" + + "code.gitea.io/gitea/models/db" + packages_model "code.gitea.io/gitea/models/packages" + "code.gitea.io/gitea/modules/context" + packages_module "code.gitea.io/gitea/modules/packages" + chef_module "code.gitea.io/gitea/modules/packages/chef" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/api/packages/helper" + packages_service "code.gitea.io/gitea/services/packages" +) + +func apiError(ctx *context.Context, status int, obj interface{}) { + type Error struct { + ErrorMessages []string `json:"error_messages"` + } + + helper.LogAndProcessError(ctx, status, obj, func(message string) { + ctx.JSON(status, Error{ + ErrorMessages: []string{message}, + }) + }) +} + +func PackagesUniverse(ctx *context.Context) { + pvs, _, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeChef, + IsInternal: util.OptionalBoolFalse, + }) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + pds, err := packages_model.GetPackageDescriptors(ctx, pvs) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + type VersionInfo struct { + LocationType string `json:"location_type"` + LocationPath string `json:"location_path"` + DownloadURL string `json:"download_url"` + Dependencies map[string]string `json:"dependencies"` + } + + baseURL := setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/chef/api/v1" + + universe := make(map[string]map[string]*VersionInfo) + for _, pd := range pds { + if _, ok := universe[pd.Package.Name]; !ok { + universe[pd.Package.Name] = make(map[string]*VersionInfo) + } + universe[pd.Package.Name][pd.Version.Version] = &VersionInfo{ + LocationType: "opscode", + LocationPath: baseURL, + DownloadURL: fmt.Sprintf("%s/cookbooks/%s/versions/%s/download", baseURL, url.PathEscape(pd.Package.Name), pd.Version.Version), + Dependencies: pd.Metadata.(*chef_module.Metadata).Dependencies, + } + } + + ctx.JSON(http.StatusOK, universe) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_list.rb +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_search.rb +func EnumeratePackages(ctx *context.Context) { + opts := &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeChef, + Name: packages_model.SearchValue{Value: ctx.FormTrim("q")}, + IsInternal: util.OptionalBoolFalse, + Paginator: db.NewAbsoluteListOptions( + ctx.FormInt("start"), + ctx.FormInt("items"), + ), + } + + switch strings.ToLower(ctx.FormTrim("order")) { + case "recently_updated", "recently_added": + opts.Sort = packages_model.SortCreatedDesc + default: + opts.Sort = packages_model.SortNameAsc + } + + pvs, total, err := packages_model.SearchLatestVersions(ctx, opts) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + pds, err := packages_model.GetPackageDescriptors(ctx, pvs) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + type Item struct { + CookbookName string `json:"cookbook_name"` + CookbookMaintainer string `json:"cookbook_maintainer"` + CookbookDescription string `json:"cookbook_description"` + Cookbook string `json:"cookbook"` + } + + type Result struct { + Start int `json:"start"` + Total int `json:"total"` + Items []*Item `json:"items"` + } + + baseURL := setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/chef/api/v1/cookbooks/" + + items := make([]*Item, 0, len(pds)) + for _, pd := range pds { + metadata := pd.Metadata.(*chef_module.Metadata) + + items = append(items, &Item{ + CookbookName: pd.Package.Name, + CookbookMaintainer: metadata.Author, + CookbookDescription: metadata.Description, + Cookbook: baseURL + url.PathEscape(pd.Package.Name), + }) + } + + skip, _ := opts.Paginator.GetSkipTake() + + ctx.JSON(http.StatusOK, &Result{ + Start: skip, + Total: int(total), + Items: items, + }) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_show.rb +func PackageMetadata(ctx *context.Context) { + packageName := ctx.Params("name") + + pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeChef, packageName) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + if len(pvs) == 0 { + apiError(ctx, http.StatusNotFound, nil) + return + } + + pds, err := packages_model.GetPackageDescriptors(ctx, pvs) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + sort.Slice(pds, func(i, j int) bool { + return pds[i].SemVer.LessThan(pds[j].SemVer) + }) + + type Result struct { + Name string `json:"name"` + Maintainer string `json:"maintainer"` + Description string `json:"description"` + Category string `json:"category"` + LatestVersion string `json:"latest_version"` + SourceURL string `json:"source_url"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + Deprecated bool `json:"deprecated"` + Versions []string `json:"versions"` + } + + baseURL := fmt.Sprintf("%sapi/packages/%s/chef/api/v1/cookbooks/%s/versions/", setting.AppURL, ctx.Package.Owner.Name, url.PathEscape(packageName)) + + versions := make([]string, 0, len(pds)) + for _, pd := range pds { + versions = append(versions, baseURL+pd.Version.Version) + } + + latest := pds[len(pds)-1] + + metadata := latest.Metadata.(*chef_module.Metadata) + + ctx.JSON(http.StatusOK, &Result{ + Name: latest.Package.Name, + Maintainer: metadata.Author, + Description: metadata.Description, + LatestVersion: baseURL + latest.Version.Version, + SourceURL: metadata.RepositoryURL, + CreatedAt: latest.Version.CreatedUnix.AsLocalTime(), + UpdatedAt: latest.Version.CreatedUnix.AsLocalTime(), + Deprecated: false, + Versions: versions, + }) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_show.rb +func PackageVersionMetadata(ctx *context.Context) { + packageName := ctx.Params("name") + packageVersion := strings.ReplaceAll(ctx.Params("version"), "_", ".") // Chef calls this endpoint with "_" instead of "."?! + + pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeChef, packageName, packageVersion) + if err != nil { + if err == packages_model.ErrPackageNotExist { + apiError(ctx, http.StatusNotFound, err) + return + } + apiError(ctx, http.StatusInternalServerError, err) + return + } + + pd, err := packages_model.GetPackageDescriptor(ctx, pv) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + type Result struct { + Version string `json:"version"` + TarballFileSize int64 `json:"tarball_file_size"` + PublishedAt time.Time `json:"published_at"` + Cookbook string `json:"cookbook"` + File string `json:"file"` + License string `json:"license"` + Dependencies map[string]string `json:"dependencies"` + } + + baseURL := fmt.Sprintf("%sapi/packages/%s/chef/api/v1/cookbooks/%s", setting.AppURL, ctx.Package.Owner.Name, url.PathEscape(pd.Package.Name)) + + metadata := pd.Metadata.(*chef_module.Metadata) + + ctx.JSON(http.StatusOK, &Result{ + Version: pd.Version.Version, + TarballFileSize: pd.Files[0].Blob.Size, + PublishedAt: pd.Version.CreatedUnix.AsLocalTime(), + Cookbook: baseURL, + File: fmt.Sprintf("%s/versions/%s/download", baseURL, pd.Version.Version), + License: metadata.License, + Dependencies: metadata.Dependencies, + }) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_share.rb +func UploadPackage(ctx *context.Context) { + file, _, err := ctx.Req.FormFile("tarball") + if err != nil { + apiError(ctx, http.StatusBadRequest, err) + return + } + defer file.Close() + + buf, err := packages_module.CreateHashedBufferFromReader(file, 32*1024*1024) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + defer buf.Close() + + pck, err := chef_module.ParsePackage(buf) + if err != nil { + if errors.Is(err, util.ErrInvalidArgument) { + apiError(ctx, http.StatusBadRequest, err) + } else { + apiError(ctx, http.StatusInternalServerError, err) + } + return + } + + if _, err := buf.Seek(0, io.SeekStart); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + _, _, err = packages_service.CreatePackageAndAddFile( + &packages_service.PackageCreationInfo{ + PackageInfo: packages_service.PackageInfo{ + Owner: ctx.Package.Owner, + PackageType: packages_model.TypeChef, + Name: pck.Name, + Version: pck.Version, + }, + Creator: ctx.Doer, + SemverCompatible: true, + Metadata: pck.Metadata, + }, + &packages_service.PackageFileCreationInfo{ + PackageFileInfo: packages_service.PackageFileInfo{ + Filename: strings.ToLower(pck.Version + ".tar.gz"), + }, + Creator: ctx.Doer, + Data: buf, + IsLead: true, + }, + ) + if err != nil { + switch err { + case packages_model.ErrDuplicatePackageVersion: + apiError(ctx, http.StatusBadRequest, err) + case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize: + apiError(ctx, http.StatusForbidden, err) + default: + apiError(ctx, http.StatusInternalServerError, err) + } + return + } + + ctx.JSON(http.StatusCreated, make(map[any]any)) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_download.rb +func DownloadPackage(ctx *context.Context) { + pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeChef, ctx.Params("name"), ctx.Params("version")) + if err != nil { + if err == packages_model.ErrPackageNotExist { + apiError(ctx, http.StatusNotFound, err) + return + } + apiError(ctx, http.StatusInternalServerError, err) + return + } + + pd, err := packages_model.GetPackageDescriptor(ctx, pv) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + pf := pd.Files[0].File + + s, _, err := packages_service.GetPackageFileStream(ctx, pf) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + defer s.Close() + + ctx.ServeContent(s, &context.ServeHeaderOptions{ + Filename: pf.Name, + LastModified: pf.CreatedUnix.AsLocalTime(), + }) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_unshare.rb +func DeletePackageVersion(ctx *context.Context) { + packageName := ctx.Params("name") + packageVersion := ctx.Params("version") + + err := packages_service.RemovePackageVersionByNameAndVersion( + ctx.Doer, + &packages_service.PackageInfo{ + Owner: ctx.Package.Owner, + PackageType: packages_model.TypeChef, + Name: packageName, + Version: packageVersion, + }, + ) + if err != nil { + if err == packages_model.ErrPackageNotExist { + apiError(ctx, http.StatusNotFound, err) + } else { + apiError(ctx, http.StatusInternalServerError, err) + } + return + } + + ctx.Status(http.StatusOK) +} + +// https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_unshare.rb +func DeletePackage(ctx *context.Context) { + pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeChef, ctx.Params("name")) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + if len(pvs) == 0 { + apiError(ctx, http.StatusNotFound, err) + return + } + + for _, pv := range pvs { + if err := packages_service.RemovePackageVersion(ctx.Doer, pv); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + } + + ctx.Status(http.StatusOK) +} diff --git a/routers/api/v1/packages/package.go b/routers/api/v1/packages/package.go index 2a20d66d1e0..ab077090d1c 100644 --- a/routers/api/v1/packages/package.go +++ b/routers/api/v1/packages/package.go @@ -40,7 +40,7 @@ func ListPackages(ctx *context.APIContext) { // in: query // description: package type filter // type: string - // enum: [cargo, composer, conan, conda, container, generic, helm, maven, npm, nuget, pub, pypi, rubygems, vagrant] + // enum: [cargo, chef, composer, conan, conda, container, generic, helm, maven, npm, nuget, pub, pypi, rubygems, vagrant] // - name: q // in: query // description: name filter diff --git a/routers/web/user/setting/packages.go b/routers/web/user/setting/packages.go index b3f8a3e41df..0d2eb14c202 100644 --- a/routers/web/user/setting/packages.go +++ b/routers/web/user/setting/packages.go @@ -5,10 +5,14 @@ package setting import ( "net/http" + "strings" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" + chef_module "code.gitea.io/gitea/modules/packages/chef" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" shared "code.gitea.io/gitea/routers/web/shared/packages" ) @@ -95,3 +99,21 @@ func RebuildCargoIndex(ctx *context.Context) { ctx.Redirect(setting.AppSubURL + "/user/settings/packages") } + +func RegenerateChefKeyPair(ctx *context.Context) { + priv, pub, err := util.GenerateKeyPair(chef_module.KeyBits) + if err != nil { + ctx.ServerError("GenerateKeyPair", err) + return + } + + if err := user_model.SetUserSetting(ctx.Doer.ID, chef_module.SettingPublicPem, pub); err != nil { + ctx.ServerError("SetUserSetting", err) + return + } + + ctx.ServeContent(strings.NewReader(priv), &context.ServeHeaderOptions{ + ContentType: "application/x-pem-file", + Filename: ctx.Doer.Name + ".priv", + }) +} diff --git a/routers/web/web.go b/routers/web/web.go index a024c0ac37e..68989560535 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -472,6 +472,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/initialize", user_setting.InitializeCargoIndex) m.Post("/rebuild", user_setting.RebuildCargoIndex) }) + m.Post("/chef/regenerate_keypair", user_setting.RegenerateChefKeyPair) }, packagesEnabled) m.Group("/secrets", func() { m.Get("", user_setting.Secrets) diff --git a/services/forms/package_form.go b/services/forms/package_form.go index 558ed54b655..b22ed47c775 100644 --- a/services/forms/package_form.go +++ b/services/forms/package_form.go @@ -15,7 +15,7 @@ import ( type PackageCleanupRuleForm struct { ID int64 Enabled bool - Type string `binding:"Required;In(cargo,composer,conan,conda,container,generic,helm,maven,npm,nuget,pub,pypi,rubygems,vagrant)"` + Type string `binding:"Required;In(cargo,chef,composer,conan,conda,container,generic,helm,maven,npm,nuget,pub,pypi,rubygems,vagrant)"` KeepCount int `binding:"In(0,1,5,10,25,50,100)"` KeepPattern string `binding:"RegexPattern"` RemoveDays int `binding:"In(0,7,14,30,60,90,180)"` diff --git a/services/packages/packages.go b/services/packages/packages.go index f5028407552..3abca7337c7 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -335,6 +335,8 @@ func CheckSizeQuotaExceeded(ctx context.Context, doer, owner *user_model.User, p switch packageType { case packages_model.TypeCargo: typeSpecificSize = setting.Packages.LimitSizeCargo + case packages_model.TypeChef: + typeSpecificSize = setting.Packages.LimitSizeChef case packages_model.TypeComposer: typeSpecificSize = setting.Packages.LimitSizeComposer case packages_model.TypeConan: diff --git a/templates/package/content/chef.tmpl b/templates/package/content/chef.tmpl new file mode 100644 index 00000000000..f0f8de7bd55 --- /dev/null +++ b/templates/package/content/chef.tmpl @@ -0,0 +1,48 @@ +{{if eq .PackageDescriptor.Package.Type "chef"}} +

{{.locale.Tr "packages.installation"}}

+
+
+
+ +
knife[:supermarket_site] = '{{AppUrl}}api/packages/{{.PackageDescriptor.Owner.Name}}/chef'
+
+
+ +
knife supermarket install {{.PackageDescriptor.Package.Name}} {{.PackageDescriptor.Version.Version}}
+
+
+ +
+
+
+ + {{if or .PackageDescriptor.Metadata.Description .PackageDescriptor.Metadata.LongDescription}} +

{{.locale.Tr "packages.about"}}

+
+ {{if .PackageDescriptor.Metadata.Description}}

{{.PackageDescriptor.Metadata.Description}}

{{end}} + {{if .PackageDescriptor.Metadata.LongDescription}}{{RenderMarkdownToHtml .PackageDescriptor.Metadata.LongDescription}}{{end}} +
+ {{end}} + + {{if .PackageDescriptor.Metadata.Dependencies}} +

{{.locale.Tr "packages.dependencies"}}

+
+ + + + + + + + + {{range $dependency, $version := .PackageDescriptor.Metadata.Dependencies}} + + + + + {{end}} + +
{{.locale.Tr "packages.dependency.id"}}{{.locale.Tr "packages.dependency.version"}}
{{$dependency}}{{$version}}
+
+ {{end}} +{{end}} diff --git a/templates/package/metadata/chef.tmpl b/templates/package/metadata/chef.tmpl new file mode 100644 index 00000000000..00fc0e6c341 --- /dev/null +++ b/templates/package/metadata/chef.tmpl @@ -0,0 +1,5 @@ +{{if eq .PackageDescriptor.Package.Type "chef"}} + {{if .PackageDescriptor.Metadata.Author}}
{{svg "octicon-person" 16 "mr-3"}} {{.PackageDescriptor.Metadata.Author}}
{{end}} + {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.repository_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.License}}
{{svg "octicon-law" 16 "mr-3"}} {{.PackageDescriptor.Metadata.License}}
{{end}} +{{end}} diff --git a/templates/package/metadata/container.tmpl b/templates/package/metadata/container.tmpl index 1b13072c38d..71df960f76f 100644 --- a/templates/package/metadata/container.tmpl +++ b/templates/package/metadata/container.tmpl @@ -4,6 +4,6 @@ {{range .PackageDescriptor.Metadata.Authors}}
{{svg "octicon-person" 16 "mr-3"}} {{.}}
{{end}} {{if .PackageDescriptor.Metadata.Licenses}}
{{svg "octicon-law" 16 "mr-3"}} {{.PackageDescriptor.Metadata.Licenses}}
{{end}} {{if .PackageDescriptor.Metadata.ProjectURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.project_site"}}
{{end}} - {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.container.details.repository_site"}}
{{end}} - {{if .PackageDescriptor.Metadata.DocumentationURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.container.details.documentation_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.repository_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.DocumentationURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.documentation_site"}}
{{end}} {{end}} diff --git a/templates/package/metadata/pub.tmpl b/templates/package/metadata/pub.tmpl index 8bd65b49d6f..632a506551c 100644 --- a/templates/package/metadata/pub.tmpl +++ b/templates/package/metadata/pub.tmpl @@ -1,5 +1,5 @@ {{if eq .PackageDescriptor.Package.Type "pub"}} {{if .PackageDescriptor.Metadata.ProjectURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.project_site"}}
{{end}} - {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.pub.details.repository_site"}}
{{end}} - {{if .PackageDescriptor.Metadata.DocumentationURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.pub.details.documentation_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.RepositoryURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.repository_site"}}
{{end}} + {{if .PackageDescriptor.Metadata.DocumentationURL}}
{{svg "octicon-link-external" 16 "mr-3"}} {{.locale.Tr "packages.details.documentation_site"}}
{{end}} {{end}} diff --git a/templates/package/view.tmpl b/templates/package/view.tmpl index 6209e6cb02e..a548d9e0b68 100644 --- a/templates/package/view.tmpl +++ b/templates/package/view.tmpl @@ -20,6 +20,7 @@
{{template "package/content/cargo" .}} + {{template "package/content/chef" .}} {{template "package/content/composer" .}} {{template "package/content/conan" .}} {{template "package/content/conda" .}} @@ -45,6 +46,7 @@
{{svg "octicon-calendar" 16 "mr-3"}} {{TimeSinceUnix .PackageDescriptor.Version.CreatedUnix $.locale}}
{{svg "octicon-download" 16 "mr-3"}} {{.PackageDescriptor.Version.DownloadCount}}
{{template "package/metadata/cargo" .}} + {{template "package/metadata/chef" .}} {{template "package/metadata/composer" .}} {{template "package/metadata/conan" .}} {{template "package/metadata/conda" .}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d6e3009b35b..812e626d2d9 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2101,6 +2101,7 @@ { "enum": [ "cargo", + "chef", "composer", "conan", "conda", diff --git a/templates/user/settings/packages.tmpl b/templates/user/settings/packages.tmpl index 472d84327a7..1faddab5ded 100644 --- a/templates/user/settings/packages.tmpl +++ b/templates/user/settings/packages.tmpl @@ -5,6 +5,24 @@ {{template "base/alert" .}} {{template "package/shared/cleanup_rules/list" .}} {{template "package/shared/cargo" .}} + +

+ {{.locale.Tr "packages.owner.settings.chef.title"}} +

+
+
+
+ +
+
+ {{.CsrfTokenHtml}} + +
+
+ +
+
+
{{template "base/footer" .}} diff --git a/tests/integration/api_packages_chef_test.go b/tests/integration/api_packages_chef_test.go new file mode 100644 index 00000000000..14baddca942 --- /dev/null +++ b/tests/integration/api_packages_chef_test.go @@ -0,0 +1,560 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha1" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "fmt" + "hash" + "math/big" + "mime/multipart" + "net/http" + "path" + "strings" + "testing" + "time" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/packages" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + chef_module "code.gitea.io/gitea/modules/packages/chef" + "code.gitea.io/gitea/modules/setting" + chef_router "code.gitea.io/gitea/routers/api/packages/chef" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestPackageChef(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + privPem := `-----BEGIN RSA PRIVATE KEY----- +MIIEpQIBAAKCAQEAtWp2PZz4TSU5A6ixw41HdbfBuGJwPuTtrsdoUf0DQ0/DJBNP +qOCBAgEu6ZdUqIbWJ5Da+nevjtncy5hENdi6XrXjyzlUxghMuXjE5SeLGpgfQvkq +bTkYaFpMe8PTzNeze3fei8+Eu6mzeb6g1GrqXznuPIc7bNss0w5iX9RiBM9dWPuX +onx9xSEy0LYqJm7yXmshNe1aRwkjG/y5C26BzBFnMKp9YRTua0DO1WqLNhcaRnda +lIFYouDNVTbwxSlYL16bZVoebqzZvLGrPvZJkPuCu6vH9brvOuYo0q8hLVNkBeXc +imRpsDjLhQYzEJjoMTbaiVGnjBky+PWNiofJnwIDAQABAoIBAQCotF1KxLt/ejr/ +9ROCh9JJXV3v6tL5GgkSPOv9Oq2bHgSZer/cixJNW+5VWd5nbiSe3K1WuJBw5pbW +Wj4sWORPiRRR+3mjQzqeS/nGJDTOwWJo9K8IrUzOVhLEEYLX/ksxaXJyT8PehFyb +vbNwdhCIB6ZNcXDItTWE+95twWJ5lxAIj2dNwZZni3UkwwjYnCnqFtvHCKOg0NH2 +RjQcFYmu3fncNeqLezUSdVyRyXxSCHsUdlYeX/e44StCnXdrmLUHlb2P27ZVdPGh +SW7qTUPpmJKekYiRPOpTLj+ZKXIsANkyWO+7dVtZLBm5bIyAsmp0W/DmK+wRsejj +alFbIsh5AoGBANJr7HSG695wkfn+kvu/V8qHbt+KDv4WjWHjGRsUqvxoHOUNkQmW +vZWdk4gjHYn1l+QHWmoOE3AgyqtCZ4bFILkZPLN/F8Mh3+r4B0Ac4biJJt7XGMNQ +Nv4wsk7TR7CCARsjO7GP1PT60hpjMvYmc1E36gNM7QIZE9jBE+L8eWYtAoGBANy2 +JOAWf+QeBlur6o9feH76cEmpQzUUq4Lj9mmnXgIirSsFoBnDb8VA6Ws+ltL9U9H2 +vaCoaTyi9twW9zWj+Ywg2mVR5nlSAPfdlTWS1GLUbDotlj5apc/lvnGuNlWzN+I4 +Tu64hhgBXqGvRZ0o7HzFodqRAkpVXp6CQCqBM7p7AoGAIgO0K3oL8t87ma/fTra1 +mFWgRJ5qogQ/Qo2VZ11F7ptd4GD7CxPE/cSFLsKOadi7fu75XJ994OhMGrcXSR/g +lEtSFqn6y15UdgU2FtUUX+I72FXo+Nmkqh5xFHDu68d4Kkzdv2xCvn81K3LRsByz +E3P4biQnQ+mN3cIIVu79KNkCgYEAm6uctrEn4y2KLn5DInyj8GuTZ2ELFhVOIzPG +SR7TH451tTJyiblezDHMcOfkWUx0IlN1zCr8jtgiZXmNQzg0erFxWKU7ebZtGGYh +J3g4dLx+2Unt/mzRJqFUgbnueOO/Nr+gbJ+ZdLUCmeeVohOLOTXrws0kYGl2Izab +K1+VrKECgYEAxQohoOegA0f4mofisXItbwwqTIX3bLpxBc4woa1sB4kjNrLo4slc +qtWZGVlRxwBvQUg0cYj+xtr5nyBdHLy0qwX/kMq4GqQnvW6NqsbrP3MjCZ8NX/Sj +A2W0jx50Hs/XNw6IZFLYgWVoOzCaD+jYFpHhzUZyQD6/rYhwhHrNQmU= +-----END RSA PRIVATE KEY-----` + + tmp, _ := pem.Decode([]byte(privPem)) + privKey, _ := x509.ParsePKCS1PrivateKey(tmp.Bytes) + + pubPem := `-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtWp2PZz4TSU5A6ixw41H +dbfBuGJwPuTtrsdoUf0DQ0/DJBNPqOCBAgEu6ZdUqIbWJ5Da+nevjtncy5hENdi6 +XrXjyzlUxghMuXjE5SeLGpgfQvkqbTkYaFpMe8PTzNeze3fei8+Eu6mzeb6g1Grq +XznuPIc7bNss0w5iX9RiBM9dWPuXonx9xSEy0LYqJm7yXmshNe1aRwkjG/y5C26B +zBFnMKp9YRTua0DO1WqLNhcaRndalIFYouDNVTbwxSlYL16bZVoebqzZvLGrPvZJ +kPuCu6vH9brvOuYo0q8hLVNkBeXcimRpsDjLhQYzEJjoMTbaiVGnjBky+PWNiofJ +nwIDAQAB +-----END PUBLIC KEY-----` + + err := user_model.SetUserSetting(user.ID, chef_module.SettingPublicPem, pubPem) + assert.NoError(t, err) + + t.Run("Authenticate", func(t *testing.T) { + auth := &chef_router.Auth{} + + t.Run("MissingUser", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "POST", "/dummy") + u, err := auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.NoError(t, err) + }) + + t.Run("NotExistingUser", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "POST", "/dummy") + req.Header.Set("X-Ops-Userid", "not-existing-user") + u, err := auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + }) + + t.Run("Timestamp", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "POST", "/dummy") + req.Header.Set("X-Ops-Userid", user.Name) + u, err := auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + req.Header.Set("X-Ops-Timestamp", "2023-01-01T00:00:00Z") + u, err = auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + }) + + t.Run("SigningVersion", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "POST", "/dummy") + req.Header.Set("X-Ops-Userid", user.Name) + req.Header.Set("X-Ops-Timestamp", time.Now().UTC().Format(time.RFC3339)) + u, err := auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + req.Header.Set("X-Ops-Sign", "version=none") + u, err = auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + req.Header.Set("X-Ops-Sign", "version=1.4") + u, err = auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + req.Header.Set("X-Ops-Sign", "version=1.0;algorithm=sha2") + u, err = auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + req.Header.Set("X-Ops-Sign", "version=1.0;algorithm=sha256") + u, err = auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + }) + + t.Run("SignedHeaders", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + ts := time.Now().UTC().Format(time.RFC3339) + + req := NewRequest(t, "POST", "/dummy") + req.Header.Set("X-Ops-Userid", user.Name) + req.Header.Set("X-Ops-Timestamp", ts) + req.Header.Set("X-Ops-Sign", "version=1.0;algorithm=sha1") + req.Header.Set("X-Ops-Content-Hash", "unused") + req.Header.Set("X-Ops-Authorization-4", "dummy") + u, err := auth.Verify(req, nil, nil, nil) + assert.Nil(t, u) + assert.Error(t, err) + + signRequest := func(t *testing.T, req *http.Request, version string) { + username := req.Header.Get("X-Ops-Userid") + if version != "1.0" && version != "1.3" { + sum := sha1.Sum([]byte(username)) + username = base64.StdEncoding.EncodeToString(sum[:]) + } + + req.Header.Set("X-Ops-Sign", "version="+version) + + var data []byte + if version == "1.3" { + data = []byte(fmt.Sprintf( + "Method:%s\nPath:%s\nX-Ops-Content-Hash:%s\nX-Ops-Sign:version=%s\nX-Ops-Timestamp:%s\nX-Ops-UserId:%s\nX-Ops-Server-API-Version:%s", + req.Method, + path.Clean(req.URL.Path), + req.Header.Get("X-Ops-Content-Hash"), + version, + req.Header.Get("X-Ops-Timestamp"), + username, + req.Header.Get("X-Ops-Server-Api-Version"), + )) + } else { + sum := sha1.Sum([]byte(path.Clean(req.URL.Path))) + data = []byte(fmt.Sprintf( + "Method:%s\nHashed Path:%s\nX-Ops-Content-Hash:%s\nX-Ops-Timestamp:%s\nX-Ops-UserId:%s", + req.Method, + base64.StdEncoding.EncodeToString(sum[:]), + req.Header.Get("X-Ops-Content-Hash"), + req.Header.Get("X-Ops-Timestamp"), + username, + )) + } + + for k := range req.Header { + if strings.HasPrefix(k, "X-Ops-Authorization-") { + req.Header.Del(k) + } + } + + var signature []byte + if version == "1.3" || version == "1.2" { + var h hash.Hash + var ch crypto.Hash + if version == "1.3" { + h = sha256.New() + ch = crypto.SHA256 + } else { + h = sha1.New() + ch = crypto.SHA1 + } + h.Write(data) + + signature, _ = rsa.SignPKCS1v15(rand.Reader, privKey, ch, h.Sum(nil)) + } else { + c := new(big.Int).SetBytes(data) + m := new(big.Int).Exp(c, privKey.D, privKey.N) + + signature = m.Bytes() + } + + enc := base64.StdEncoding.EncodeToString(signature) + + const chunkSize = 60 + chunks := make([]string, 0, (len(enc)-1)/chunkSize+1) + currentLen := 0 + currentStart := 0 + for i := range enc { + if currentLen == chunkSize { + chunks = append(chunks, enc[currentStart:i]) + currentLen = 0 + currentStart = i + } + currentLen++ + } + chunks = append(chunks, enc[currentStart:]) + + for i, chunk := range chunks { + req.Header.Set(fmt.Sprintf("X-Ops-Authorization-%d", i+1), chunk) + } + } + + for _, v := range []string{"1.0", "1.1", "1.2", "1.3"} { + t.Run(v, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + signRequest(t, req, v) + u, err = auth.Verify(req, nil, nil, nil) + assert.NotNil(t, u) + assert.NoError(t, err) + }) + } + }) + }) + + packageName := "test" + packageVersion := "1.0.1" + packageDescription := "Test Description" + packageAuthor := "KN4CK3R" + + root := fmt.Sprintf("/api/packages/%s/chef/api/v1", user.Name) + + uploadPackage := func(t *testing.T, version string, expectedStatus int) { + var body bytes.Buffer + mpw := multipart.NewWriter(&body) + part, _ := mpw.CreateFormFile("tarball", fmt.Sprintf("%s.tar.gz", version)) + zw := gzip.NewWriter(part) + tw := tar.NewWriter(zw) + + content := `{"name":"` + packageName + `","version":"` + version + `","description":"` + packageDescription + `","maintainer":"` + packageAuthor + `"}` + + hdr := &tar.Header{ + Name: packageName + "/metadata.json", + Mode: 0o600, + Size: int64(len(content)), + } + tw.WriteHeader(hdr) + tw.Write([]byte(content)) + + tw.Close() + zw.Close() + mpw.Close() + + req := NewRequestWithBody(t, "POST", root+"/cookbooks", &body) + req.Header.Add("Content-Type", mpw.FormDataContentType()) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, expectedStatus) + } + + t.Run("Upload", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithBody(t, "POST", root+"/cookbooks", bytes.NewReader([]byte{})) + MakeRequest(t, req, http.StatusUnauthorized) + + uploadPackage(t, packageVersion, http.StatusCreated) + + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeChef) + assert.NoError(t, err) + assert.Len(t, pvs, 1) + + pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) + assert.NoError(t, err) + assert.NotNil(t, pd.SemVer) + assert.IsType(t, &chef_module.Metadata{}, pd.Metadata) + assert.Equal(t, packageName, pd.Package.Name) + assert.Equal(t, packageVersion, pd.Version.Version) + + pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[0].ID) + assert.NoError(t, err) + assert.Len(t, pfs, 1) + assert.Equal(t, fmt.Sprintf("%s.tar.gz", packageVersion), pfs[0].Name) + assert.True(t, pfs[0].IsLead) + + uploadPackage(t, packageVersion, http.StatusBadRequest) + }) + + t.Run("Download", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/cookbooks/%s/versions/%s/download", root, packageName, packageVersion)) + MakeRequest(t, req, http.StatusOK) + }) + + t.Run("Universe", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", root+"/universe") + resp := MakeRequest(t, req, http.StatusOK) + + type VersionInfo struct { + LocationType string `json:"location_type"` + LocationPath string `json:"location_path"` + DownloadURL string `json:"download_url"` + Dependencies map[string]string `json:"dependencies"` + } + + var result map[string]map[string]*VersionInfo + DecodeJSON(t, resp, &result) + + assert.Len(t, result, 1) + assert.Contains(t, result, packageName) + + versions := result[packageName] + + assert.Len(t, versions, 1) + assert.Contains(t, versions, packageVersion) + + info := versions[packageVersion] + + assert.Equal(t, "opscode", info.LocationType) + assert.Equal(t, setting.AppURL+root[1:], info.LocationPath) + assert.Equal(t, fmt.Sprintf("%s%s/cookbooks/%s/versions/%s/download", setting.AppURL, root[1:], packageName, packageVersion), info.DownloadURL) + }) + + t.Run("Search", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + cases := []struct { + Query string + Start int + Items int + ExpectedTotal int + ExpectedResults int + }{ + {"", 0, 0, 1, 1}, + {"", 0, 10, 1, 1}, + {"gitea", 0, 10, 0, 0}, + {"test", 0, 10, 1, 1}, + {"test", 1, 10, 1, 0}, + } + + type Item struct { + CookbookName string `json:"cookbook_name"` + CookbookMaintainer string `json:"cookbook_maintainer"` + CookbookDescription string `json:"cookbook_description"` + Cookbook string `json:"cookbook"` + } + + type Result struct { + Start int `json:"start"` + Total int `json:"total"` + Items []*Item `json:"items"` + } + + for i, c := range cases { + req := NewRequest(t, "GET", fmt.Sprintf("%s/search?q=%s&start=%d&items=%d", root, c.Query, c.Start, c.Items)) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result Result + DecodeJSON(t, resp, &result) + + assert.Equal(t, c.ExpectedTotal, result.Total, "case %d: unexpected total hits", i) + assert.Len(t, result.Items, c.ExpectedResults, "case %d: unexpected result count", i) + + if len(result.Items) == 1 { + item := result.Items[0] + assert.Equal(t, packageName, item.CookbookName) + assert.Equal(t, packageAuthor, item.CookbookMaintainer) + assert.Equal(t, packageDescription, item.CookbookDescription) + assert.Equal(t, fmt.Sprintf("%s%s/cookbooks/%s", setting.AppURL, root[1:], packageName), item.Cookbook) + } + } + }) + + t.Run("EnumeratePackages", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + cases := []struct { + Sort string + Start int + Items int + ExpectedTotal int + ExpectedResults int + }{ + {"", 0, 0, 1, 1}, + {"", 0, 10, 1, 1}, + {"RECENTLY_ADDED", 0, 10, 1, 1}, + {"RECENTLY_UPDATED", 0, 10, 1, 1}, + {"", 1, 10, 1, 0}, + } + + type Item struct { + CookbookName string `json:"cookbook_name"` + CookbookMaintainer string `json:"cookbook_maintainer"` + CookbookDescription string `json:"cookbook_description"` + Cookbook string `json:"cookbook"` + } + + type Result struct { + Start int `json:"start"` + Total int `json:"total"` + Items []*Item `json:"items"` + } + + for i, c := range cases { + req := NewRequest(t, "GET", fmt.Sprintf("%s/cookbooks?start=%d&items=%d&sort=%s", root, c.Start, c.Items, c.Sort)) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result Result + DecodeJSON(t, resp, &result) + + assert.Equal(t, c.ExpectedTotal, result.Total, "case %d: unexpected total hits", i) + assert.Len(t, result.Items, c.ExpectedResults, "case %d: unexpected result count", i) + + if len(result.Items) == 1 { + item := result.Items[0] + assert.Equal(t, packageName, item.CookbookName) + assert.Equal(t, packageAuthor, item.CookbookMaintainer) + assert.Equal(t, packageDescription, item.CookbookDescription) + assert.Equal(t, fmt.Sprintf("%s%s/cookbooks/%s", setting.AppURL, root[1:], packageName), item.Cookbook) + } + } + }) + + t.Run("PackageMetadata", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/cookbooks/%s", root, packageName)) + resp := MakeRequest(t, req, http.StatusOK) + + type Result struct { + Name string `json:"name"` + Maintainer string `json:"maintainer"` + Description string `json:"description"` + Category string `json:"category"` + LatestVersion string `json:"latest_version"` + SourceURL string `json:"source_url"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + Deprecated bool `json:"deprecated"` + Versions []string `json:"versions"` + } + + var result Result + DecodeJSON(t, resp, &result) + + versionURL := fmt.Sprintf("%s%s/cookbooks/%s/versions/%s", setting.AppURL, root[1:], packageName, packageVersion) + + assert.Equal(t, packageName, result.Name) + assert.Equal(t, packageAuthor, result.Maintainer) + assert.Equal(t, packageDescription, result.Description) + assert.Equal(t, versionURL, result.LatestVersion) + assert.False(t, result.Deprecated) + assert.ElementsMatch(t, []string{versionURL}, result.Versions) + }) + + t.Run("PackageVersionMetadata", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/cookbooks/%s/versions/%s", root, packageName, packageVersion)) + resp := MakeRequest(t, req, http.StatusOK) + + type Result struct { + Version string `json:"version"` + TarballFileSize int64 `json:"tarball_file_size"` + PublishedAt time.Time `json:"published_at"` + Cookbook string `json:"cookbook"` + File string `json:"file"` + License string `json:"license"` + Dependencies map[string]string `json:"dependencies"` + } + + var result Result + DecodeJSON(t, resp, &result) + + packageURL := fmt.Sprintf("%s%s/cookbooks/%s", setting.AppURL, root[1:], packageName) + + assert.Equal(t, packageVersion, result.Version) + assert.Equal(t, packageURL, result.Cookbook) + assert.Equal(t, fmt.Sprintf("%s/versions/%s/download", packageURL, packageVersion), result.File) + }) + + t.Run("Delete", func(t *testing.T) { + uploadPackage(t, "1.0.2", http.StatusCreated) + uploadPackage(t, "1.0.3", http.StatusCreated) + + t.Run("Version", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "DELETE", fmt.Sprintf("%s/cookbooks/%s/versions/%s", root, packageName, "1.0.2")) + MakeRequest(t, req, http.StatusUnauthorized) + + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/cookbooks/%s/versions/%s", root, packageName, "1.0.2")) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusOK) + + pv, err := packages.GetVersionByNameAndVersion(db.DefaultContext, user.ID, packages.TypeChef, packageName, "1.0.2") + assert.Nil(t, pv) + assert.Error(t, err) + }) + + t.Run("Package", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "DELETE", fmt.Sprintf("%s/cookbooks/%s", root, packageName)) + MakeRequest(t, req, http.StatusUnauthorized) + + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/cookbooks/%s", root, packageName)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusOK) + + pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeChef) + assert.NoError(t, err) + assert.Empty(t, pvs) + }) + }) +} diff --git a/web_src/svg/gitea-chef.svg b/web_src/svg/gitea-chef.svg new file mode 100644 index 00000000000..ce318c62634 --- /dev/null +++ b/web_src/svg/gitea-chef.svg @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + From 50111c71c3d8feedb170a0122c97237f78d1751f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 Feb 2023 10:23:17 +0800 Subject: [PATCH 19/20] Refactor legacy strange git operations (#22756) During the refactoring of the git module, I found there were some strange operations. This PR tries to fix 2 of them 1. The empty argument `--` in repo_attribute.go, which was introduced by #16773. It seems unnecessary because nothing else would be added later. 2. The complex git service logic in repo/http.go. * Before: the `hasAccess` only allow `service == "upload-pack" || service == "receive-pack"` * After: unrelated code is removed. No need to call ToTrustedCmdArgs anymore. Co-authored-by: Lunny Xiao --- modules/git/repo_attribute.go | 3 +- routers/web/repo/http.go | 76 +++++++++++++---------------------- 2 files changed, 28 insertions(+), 51 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index e7d5fb68068..2b34f117f72 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -135,8 +135,7 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error { c.env = append(c.env, "GIT_FLUSH=1") - // The empty "--" comes from #16773 , and it seems unnecessary because nothing else would be added later. - c.cmd.AddDynamicArguments(c.Attributes...).AddArguments("--") + c.cmd.AddDynamicArguments(c.Attributes...) var err error diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 89c86e764e3..e82b94b9e84 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -424,60 +424,40 @@ func (h *serviceHandler) sendFile(contentType, file string) { // one or more key=value pairs separated by colons var safeGitProtocolHeader = regexp.MustCompile(`^[0-9a-zA-Z]+=[0-9a-zA-Z]+(:[0-9a-zA-Z]+=[0-9a-zA-Z]+)*$`) -func getGitConfig(ctx gocontext.Context, option, dir string) string { - out, _, err := git.NewCommand(ctx, "config").AddDynamicArguments(option).RunStdString(&git.RunOpts{Dir: dir}) - if err != nil { - log.Error("%v - %s", err, out) +func prepareGitCmdWithAllowedService(service string, h *serviceHandler) (*git.Command, error) { + if service == "receive-pack" && h.cfg.ReceivePack { + return git.NewCommand(h.r.Context(), "receive-pack"), nil } - return out[0 : len(out)-1] + if service == "upload-pack" && h.cfg.UploadPack { + return git.NewCommand(h.r.Context(), "upload-pack"), nil + } + + return nil, fmt.Errorf("service %q is not allowed", service) } -func getConfigSetting(ctx gocontext.Context, service, dir string) bool { - service = strings.ReplaceAll(service, "-", "") - setting := getGitConfig(ctx, "http."+service, dir) - - if service == "uploadpack" { - return setting != "false" - } - - return setting == "true" -} - -func hasAccess(ctx gocontext.Context, service string, h serviceHandler, checkContentType bool) bool { - if checkContentType { - if h.r.Header.Get("Content-Type") != fmt.Sprintf("application/x-git-%s-request", service) { - return false - } - } - - if !(service == "upload-pack" || service == "receive-pack") { - return false - } - if service == "receive-pack" { - return h.cfg.ReceivePack - } - if service == "upload-pack" { - return h.cfg.UploadPack - } - - return getConfigSetting(ctx, service, h.dir) -} - -func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { +func serviceRPC(h *serviceHandler, service string) { defer func() { if err := h.r.Body.Close(); err != nil { log.Error("serviceRPC: Close: %v", err) } }() - if !hasAccess(ctx, service, h, true) { + expectedContentType := fmt.Sprintf("application/x-git-%s-request", service) + if h.r.Header.Get("Content-Type") != expectedContentType { + log.Error("Content-Type (%q) doesn't match expected: %q", h.r.Header.Get("Content-Type"), expectedContentType) + h.w.WriteHeader(http.StatusUnauthorized) + return + } + + cmd, err := prepareGitCmdWithAllowedService(service, h) + if err != nil { + log.Error("Failed to prepareGitCmdWithService: %v", err) h.w.WriteHeader(http.StatusUnauthorized) return } h.w.Header().Set("Content-Type", fmt.Sprintf("application/x-git-%s-result", service)) - var err error reqBody := h.r.Body // Handle GZIP. @@ -498,8 +478,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { } var stderr bytes.Buffer - // the service is generated by ourselves, so it's safe to trust it - cmd := git.NewCommand(h.r.Context(), git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc").AddDynamicArguments(h.dir) + cmd.AddArguments("--stateless-rpc").AddDynamicArguments(h.dir) cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir)) if err := cmd.Run(&git.RunOpts{ Dir: h.dir, @@ -520,7 +499,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { func ServiceUploadPack(ctx *context.Context) { h := httpBase(ctx) if h != nil { - serviceRPC(ctx, *h, "upload-pack") + serviceRPC(h, "upload-pack") } } @@ -528,7 +507,7 @@ func ServiceUploadPack(ctx *context.Context) { func ServiceReceivePack(ctx *context.Context) { h := httpBase(ctx) if h != nil { - serviceRPC(ctx, *h, "receive-pack") + serviceRPC(h, "receive-pack") } } @@ -537,7 +516,7 @@ func getServiceType(r *http.Request) string { if !strings.HasPrefix(serviceType, "git-") { return "" } - return strings.Replace(serviceType, "git-", "", 1) + return strings.TrimPrefix(serviceType, "git-") } func updateServerInfo(ctx gocontext.Context, dir string) []byte { @@ -563,16 +542,15 @@ func GetInfoRefs(ctx *context.Context) { return } h.setHeaderNoCache() - if hasAccess(ctx, getServiceType(h.r), *h, false) { - service := getServiceType(h.r) - + service := getServiceType(h.r) + cmd, err := prepareGitCmdWithAllowedService(service, h) + if err == nil { if protocol := h.r.Header.Get("Git-Protocol"); protocol != "" && safeGitProtocolHeader.MatchString(protocol) { h.environ = append(h.environ, "GIT_PROTOCOL="+protocol) } h.environ = append(os.Environ(), h.environ...) - // the service is generated by ourselves, so we can trust it - refs, _, err := git.NewCommand(ctx, git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) + refs, _, err := cmd.AddArguments("--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) if err != nil { log.Error(fmt.Sprintf("%v - %s", err, string(refs))) } From c13eb8e6b326bf6008b29778f311d5e5f89d5541 Mon Sep 17 00:00:00 2001 From: Adi <39572144+Adito5393@users.noreply.github.com> Date: Mon, 6 Feb 2023 05:12:13 +0100 Subject: [PATCH 20/20] Add CLI option tenant ID for oauth2 source (#22769) Fixes #22713 --- cmd/admin.go | 11 +++++++++++ docs/content/doc/usage/command-line.en-us.md | 2 ++ 2 files changed, 13 insertions(+) diff --git a/cmd/admin.go b/cmd/admin.go index eafed24bdd6..4e2dc2bf06b 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -308,6 +308,11 @@ var ( Value: "false", Usage: "Use custom URLs for GitLab/GitHub OAuth endpoints", }, + cli.StringFlag{ + Name: "custom-tenant-id", + Value: "", + Usage: "Use custom Tenant ID for OAuth endpoints", + }, cli.StringFlag{ Name: "custom-auth-url", Value: "", @@ -829,6 +834,7 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { AuthURL: c.String("custom-auth-url"), ProfileURL: c.String("custom-profile-url"), EmailURL: c.String("custom-email-url"), + Tenant: c.String("custom-tenant-id"), } } else { customURLMapping = nil @@ -938,6 +944,7 @@ func runUpdateOauth(c *cli.Context) error { customURLMapping.AuthURL = oAuth2Config.CustomURLMapping.AuthURL customURLMapping.ProfileURL = oAuth2Config.CustomURLMapping.ProfileURL customURLMapping.EmailURL = oAuth2Config.CustomURLMapping.EmailURL + customURLMapping.Tenant = oAuth2Config.CustomURLMapping.Tenant } if c.IsSet("use-custom-urls") && c.IsSet("custom-token-url") { customURLMapping.TokenURL = c.String("custom-token-url") @@ -955,6 +962,10 @@ func runUpdateOauth(c *cli.Context) error { customURLMapping.EmailURL = c.String("custom-email-url") } + if c.IsSet("use-custom-urls") && c.IsSet("custom-tenant-id") { + customURLMapping.Tenant = c.String("custom-tenant-id") + } + oAuth2Config.CustomURLMapping = customURLMapping source.Cfg = oAuth2Config diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index d9d397df31b..f2e72d4fc03 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -124,6 +124,7 @@ Admin operations: - `--secret`: Client Secret. - `--auto-discover-url`: OpenID Connect Auto Discovery URL (only required when using OpenID Connect as provider). - `--use-custom-urls`: Use custom URLs for GitLab/GitHub OAuth endpoints. + - `--custom-tenant-id`: Use custom Tenant ID for OAuth endpoints. - `--custom-auth-url`: Use a custom Authorization URL (option for GitLab/GitHub). - `--custom-token-url`: Use a custom Token URL (option for GitLab/GitHub). - `--custom-profile-url`: Use a custom Profile URL (option for GitLab/GitHub). @@ -147,6 +148,7 @@ Admin operations: - `--secret`: Client Secret. - `--auto-discover-url`: OpenID Connect Auto Discovery URL (only required when using OpenID Connect as provider). - `--use-custom-urls`: Use custom URLs for GitLab/GitHub OAuth endpoints. + - `--custom-tenant-id`: Use custom Tenant ID for OAuth endpoints. - `--custom-auth-url`: Use a custom Authorization URL (option for GitLab/GitHub). - `--custom-token-url`: Use a custom Token URL (option for GitLab/GitHub). - `--custom-profile-url`: Use a custom Profile URL (option for GitLab/GitHub).