diff --git a/app/api/middleware/encode/encode.go b/app/api/middleware/encode/encode.go index 67f9aaeec..a89a82b01 100644 --- a/app/api/middleware/encode/encode.go +++ b/app/api/middleware/encode/encode.go @@ -184,7 +184,7 @@ func pathTerminatedWithMarkerAndURL( } // if marker was found - convert to escaped version (skip first character in case path starts with '/'). - escapedPath := path[0:1] + strings.ReplaceAll(path[1:], types.PathSeparator, EncodedPathSeparator) + escapedPath := path[0:1] + strings.ReplaceAll(path[1:], types.PathSeparatorAsString, EncodedPathSeparator) prefixWithPath := prefix + path + marker prefixWithEscapedPath := prefix + escapedPath + markerReplacement diff --git a/app/api/middleware/goget/goget.go b/app/api/middleware/goget/goget.go index 697921f76..4a24d373e 100644 --- a/app/api/middleware/goget/goget.go +++ b/app/api/middleware/goget/goget.go @@ -15,21 +15,28 @@ package goget import ( + "errors" "fmt" "html/template" "net/http" "regexp" "strings" + "github.com/harness/gitness/app/api/auth" "github.com/harness/gitness/app/api/controller/repo" "github.com/harness/gitness/app/api/render" "github.com/harness/gitness/app/api/request" + "github.com/harness/gitness/app/paths" "github.com/harness/gitness/app/url" - "github.com/harness/gitness/types" + "github.com/harness/gitness/store" + + "github.com/rs/zerolog/log" ) -var goGetTmpl *template.Template -var httpRegex = regexp.MustCompile("https?://") +var ( + goGetTmpl *template.Template + httpRegex = regexp.MustCompile("https?://") +) func init() { var err error @@ -49,8 +56,10 @@ func init() { } // Middleware writes to response with html meta tags go-import and go-source. +// +//nolint:gocognit func Middleware( - config *types.Config, + maxRepoPathDepth int, repoCtrl *repo.Controller, urlProvider url.Provider, ) func(http.Handler) http.Handler { @@ -64,27 +73,64 @@ func Middleware( ctx := r.Context() session, _ := request.AuthSessionFrom(ctx) - repoRef, err := request.GetRepoRefFromPath(r) + importPath, err := request.GetRepoRefFromPath(r) if err != nil { render.TranslatedUserError(ctx, w, err) return } - repository, err := repoCtrl.Find(ctx, session, repoRef) - if err != nil { - render.TranslatedUserError(ctx, w, err) + // go get can also be used with (sub)module suffixes (e.g 127.0.0.1/my-project/my-repo/v2) + // for which go expects us to return the import path corresponding to the repository root + // (e.g. 127.0.0.1/my-project/my-repo). + // + // WARNING: This can lead to ambiguities as we allow matching paths between spaces and repos: + // 1. (space)foo + (repo)bar + (sufix)v2 = 127.0.0.1/my-project/my-repo/v2 + // 2. (space)foo/bar + (repo)v2 = 127.0.0.1/my-project/my-repo/v2 + // + // We handle ambiguities by always choosing the repo with the longest path (e.g. 2. case above). + // To solve go get related ambiguities users would have to move their repositories. + var repository *repo.RepositoryOutput + var repoRef string + + pathSegments := paths.Segments(importPath) + if len(pathSegments) > maxRepoPathDepth { + pathSegments = pathSegments[:maxRepoPathDepth] + } + + for l := len(pathSegments); l >= 2; l-- { + repoRef = paths.Concatenate(pathSegments[:l]...) + + repository, err = repoCtrl.Find(ctx, session, repoRef) + if err == nil { + break + } + if errors.Is(err, store.ErrResourceNotFound) { + log.Ctx(ctx).Debug().Err(err). + Msgf("repository %q doesn't exist, assume submodule and try again", repoRef) + continue + } + if errors.Is(err, auth.ErrNotAuthorized) { + // To avoid leaking information about repos' existence we continue as if it wasn't found. + // WARNING: This can lead to different import results depending on access (very unlikely) + log.Ctx(ctx).Debug().Err(err). + Msgf("user has no access on repository %q, assume submodule and try again", repoRef) + continue + } + + render.TranslatedUserError(ctx, w, fmt.Errorf("failed to find repo for path %q: %w", repoRef, err)) return } - defaultBranch := config.Git.DefaultBranch - if repository.DefaultBranch != "" { - defaultBranch = repository.DefaultBranch + if repository == nil { + render.NotFound(ctx, w) + return } uiRepoURL := urlProvider.GenerateUIRepoURL(ctx, repoRef) - goImportURL := httpRegex.ReplaceAllString(uiRepoURL, "") cloneURL := urlProvider.GenerateGITCloneURL(ctx, repoRef) - prefix := fmt.Sprintf("%s/files/%s/~", uiRepoURL, defaultBranch) + goImportURL := httpRegex.ReplaceAllString(cloneURL, "") + goImportURL = strings.TrimSuffix(goImportURL, ".git") + prefix := fmt.Sprintf("%s/files/%s/~", uiRepoURL, repository.DefaultBranch) insecure := "" if strings.HasPrefix(uiRepoURL, "http:") { diff --git a/app/paths/paths.go b/app/paths/paths.go index 591d0e276..d6c58ebe2 100644 --- a/app/paths/paths.go +++ b/app/paths/paths.go @@ -19,6 +19,8 @@ import ( "strings" "github.com/harness/gitness/types" + + "github.com/gotidy/ptr" ) var ( @@ -28,13 +30,13 @@ var ( // DisectLeaf splits a path into its parent path and the leaf name // e.g. space1/space2/space3 -> (space1/space2, space3, nil). func DisectLeaf(path string) (string, string, error) { - path = strings.Trim(path, types.PathSeparator) + path = strings.Trim(path, types.PathSeparatorAsString) if path == "" { return "", "", ErrPathEmpty } - i := strings.LastIndex(path, types.PathSeparator) + i := strings.LastIndex(path, types.PathSeparatorAsString) if i == -1 { return "", path, nil } @@ -45,13 +47,13 @@ func DisectLeaf(path string) (string, string, error) { // DisectRoot splits a path into its root space and sub-path // e.g. space1/space2/space3 -> (space1, space2/space3, nil). func DisectRoot(path string) (string, string, error) { - path = strings.Trim(path, types.PathSeparator) + path = strings.Trim(path, types.PathSeparatorAsString) if path == "" { return "", "", ErrPathEmpty } - i := strings.Index(path, types.PathSeparator) + i := strings.Index(path, types.PathSeparatorAsString) if i == -1 { return path, "", nil } @@ -63,38 +65,62 @@ func DisectRoot(path string) (string, string, error) { * Concatenate two paths together (takes care of leading / trailing '/') * e.g. (space1/, /space2/) -> space1/space2 * - * NOTE: "//" is not a valid path, so all '/' will be trimmed. + * NOTE: All leading, trailing, and consecutive '/' will be trimmed to ensure correct paths. */ -func Concatenate(path1 string, path2 string) string { - path1 = strings.Trim(path1, types.PathSeparator) - path2 = strings.Trim(path2, types.PathSeparator) - - if path1 == "" { - return path2 - } else if path2 == "" { - return path1 +func Concatenate(paths ...string) string { + if len(paths) == 0 { + return "" } - return path1 + types.PathSeparator + path2 + sb := strings.Builder{} + for i := 0; i < len(paths); i++ { + // remove all leading, trailing, and consecutive '/' + var nextRune *rune + for _, r := range paths[i] { + // skip '/' if we already have '/' as next rune or we don't have anything other than '/' yet + if (nextRune == nil || *nextRune == types.PathSeparator) && r == types.PathSeparator { + continue + } + + // first time we take a rune we have to make sure we add a separator if needed (guaranteed no '/'). + if nextRune == nil && sb.Len() > 0 { + sb.WriteString(types.PathSeparatorAsString) + } + + // flush the previous rune before storing the next rune + if nextRune != nil { + sb.WriteRune(*nextRune) + } + + nextRune = ptr.Of(r) + } + + // flush the final rune + if nextRune != nil && *nextRune != types.PathSeparator { + sb.WriteRune(*nextRune) + } + } + + return sb.String() } // Segments returns all segments of the path // e.g. /space1/space2/space3 -> [space1, space2, space3]. func Segments(path string) []string { - path = strings.Trim(path, types.PathSeparator) - return strings.Split(path, types.PathSeparator) + path = strings.Trim(path, types.PathSeparatorAsString) + return strings.Split(path, types.PathSeparatorAsString) } // IsAncesterOf returns true iff 'path' is an ancestor of 'other' or they are the same. // e.g. other = path(/.*). func IsAncesterOf(path string, other string) bool { - path = strings.Trim(path, types.PathSeparator) - other = strings.Trim(other, types.PathSeparator) + path = strings.Trim(path, types.PathSeparatorAsString) + other = strings.Trim(other, types.PathSeparatorAsString) // add "/" to both to handle space1/inner and space1/in return strings.Contains( - other+types.PathSeparator, - path+types.PathSeparator, + other+types.PathSeparatorAsString, + path+types.PathSeparatorAsString, ) } diff --git a/app/paths/paths_test.go b/app/paths/paths_test.go new file mode 100644 index 000000000..ba9a46db1 --- /dev/null +++ b/app/paths/paths_test.go @@ -0,0 +1,141 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package paths + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_Concatenate(t *testing.T) { + type testCase struct { + in []string + want string + } + tests := []testCase{ + { + in: nil, + want: "", + }, + { + in: []string{}, + want: "", + }, + { + in: []string{ + "", + }, + want: "", + }, + { + in: []string{ + "", + "", + }, + want: "", + }, + { + in: []string{ + "a", + }, + want: "a", + }, + { + in: []string{ + "", + "a", + }, + want: "a", + }, + { + in: []string{ + "a", + "", + }, + want: "a", + }, + { + in: []string{ + "", + "a", + "", + }, + want: "a", + }, + { + in: []string{ + "a", + "b", + }, + want: "a/b", + }, + { + in: []string{ + "a", + "b", + "c", + }, + want: "a/b/c", + }, + { + in: []string{ + "seg1", + "seg2/seg3", + "seg4", + }, + want: "seg1/seg2/seg3/seg4", + }, + { + in: []string{ + "/", + "/", + "/", + }, + want: "", + }, + { + in: []string{ + "//", + }, + want: "", + }, + { + in: []string{ + "/a/", + }, + want: "a", + }, + { + in: []string{ + "/a/", + "/b/", + }, + want: "a/b", + }, + { + in: []string{ + "/a/b/", + "//c//d//", + "///e///f///", + }, + want: "a/b/c/d/e/f", + }, + } + for _, tt := range tests { + got := Concatenate(tt.in...) + assert.Equal(t, tt.want, got, "path isn't matching for %v", tt.in) + } +} diff --git a/app/router/git.go b/app/router/git.go index 2078fbf2d..d1d3a8b4a 100644 --- a/app/router/git.go +++ b/app/router/git.go @@ -29,6 +29,7 @@ import ( "github.com/harness/gitness/app/auth/authn" "github.com/harness/gitness/app/url" "github.com/harness/gitness/types" + "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" "github.com/go-chi/chi" @@ -43,6 +44,12 @@ func NewGitHandler( authenticator authn.Authenticator, repoCtrl *repo.Controller, ) http.Handler { + // maxRepoDepth depends on config + maxRepoDepth := check.MaxRepoPathDepth + if !config.NestedSpacesEnabled { + maxRepoDepth = 2 + } + // Use go-chi router for inner routing. r := chi.NewRouter() @@ -60,7 +67,7 @@ func NewGitHandler( r.Use(middlewareauthn.Attempt(authenticator)) r.Route(fmt.Sprintf("/{%s}", request.PathParamRepoRef), func(r chi.Router) { - r.Use(goget.Middleware(config, repoCtrl, urlProvider)) + r.Use(goget.Middleware(maxRepoDepth, repoCtrl, urlProvider)) // routes that aren't coming from git r.Group(func(r chi.Router) { // redirect to repo (meant for UI, in case user navigates to clone url in browser) diff --git a/types/check/path.go b/types/check/path.go index 93c983423..6a4a8ea70 100644 --- a/types/check/path.go +++ b/types/check/path.go @@ -22,8 +22,8 @@ import ( ) const ( - maxPathSegmentsForSpace = 9 - maxPathSegments = 10 + MaxSpacePathDepth = 9 + MaxRepoPathDepth = 10 ) var ( @@ -32,13 +32,13 @@ var ( } ErrPathInvalidDepth = &ValidationError{ fmt.Sprintf("A path can have at most %d segments (%d for spaces).", - maxPathSegments, maxPathSegmentsForSpace), + MaxRepoPathDepth, MaxSpacePathDepth), } ErrEmptyPathSegment = &ValidationError{ "Empty segments are not allowed.", } ErrPathCantBeginOrEndWithSeparator = &ValidationError{ - fmt.Sprintf("Path can't start or end with the separator ('%s').", types.PathSeparator), + fmt.Sprintf("Path can't start or end with the separator ('%s').", types.PathSeparatorAsString), } ) @@ -49,7 +49,7 @@ func Path(path string, isSpace bool, identifierCheck SpaceIdentifier) error { } // ensure path doesn't begin or end with / - if path[:1] == types.PathSeparator || path[len(path)-1:] == types.PathSeparator { + if path[:1] == types.PathSeparatorAsString || path[len(path)-1:] == types.PathSeparatorAsString { return ErrPathCantBeginOrEndWithSeparator } @@ -59,7 +59,7 @@ func Path(path string, isSpace bool, identifierCheck SpaceIdentifier) error { } // ensure all segments of the path are valid identifiers - segments := strings.Split(path, types.PathSeparator) + segments := strings.Split(path, types.PathSeparatorAsString) for i, s := range segments { if s == "" { return ErrEmptyPathSegment @@ -83,6 +83,6 @@ func PathDepth(path string, isSpace bool) error { // IsPathTooDeep Checks if the provided path is too long. // NOTE: A repository path can be one deeper than a space path (as otherwise the space would be useless). func IsPathTooDeep(path string, isSpace bool) bool { - l := strings.Count(path, types.PathSeparator) + 1 - return (!isSpace && l > maxPathSegments) || (isSpace && l > maxPathSegmentsForSpace) + l := strings.Count(path, types.PathSeparatorAsString) + 1 + return (!isSpace && l > MaxRepoPathDepth) || (isSpace && l > MaxSpacePathDepth) } diff --git a/types/path.go b/types/path.go index e133a7224..53642ecee 100644 --- a/types/path.go +++ b/types/path.go @@ -17,7 +17,8 @@ package types import "encoding/json" const ( - PathSeparator = "/" + PathSeparatorAsString = string(PathSeparator) + PathSeparator = '/' ) // SpacePath represents a full path to a space.