From 0abad31a62f9b33a29dfa101c922d70a4de87e35 Mon Sep 17 00:00:00 2001 From: Joel Robotham Date: Tue, 24 Mar 2026 09:24:19 +1100 Subject: [PATCH] fix: reject org-level URLs in RepoPathFromURL to prevent mirror deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RepoPathFromURL now validates that the URL contains at least an owner and repository component (e.g. host/owner/repo). Previously, an org-only URL like https://github.com/squareup/ would resolve to the mirror path state/git-mirrors/github.com/squareup, and a failed clone attempt would call os.RemoveAll on that path — deleting every squareup mirror on the pod. This was triggered in production when a curl request to /git/github.com/squareup/ caused startClone to wipe the parent directory containing all squareup repo mirrors. Amp-Thread-ID: https://ampcode.com/threads/T-019d1ca3-be59-7241-9166-3342769c959a Co-authored-by: Amp --- internal/gitclone/manager.go | 12 ++++++++++-- internal/gitclone/manager_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 576904bb..f350ecaa 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -265,13 +265,21 @@ func (m *Manager) DiscoverExisting(ctx context.Context) ([]*Repository, error) { } // RepoPathFromURL extracts a normalised "host/path" from an upstream Git URL, -// stripping any ".git" suffix. +// stripping any ".git" suffix. The URL must contain at least an owner and +// repository component (e.g. "github.com/org/repo"); org-only URLs like +// "github.com/org" are rejected to prevent operations on parent directories +// that could affect other repositories. func RepoPathFromURL(upstreamURL string) (string, error) { parsed, err := url.Parse(upstreamURL) if err != nil { return "", errors.Wrap(err, "parse upstream URL") } - return filepath.Join(parsed.Host, strings.TrimSuffix(parsed.Path, ".git")), nil + trimmed := strings.Trim(strings.TrimSuffix(parsed.Path, ".git"), "/") + parts := strings.Split(trimmed, "/") + if len(parts) < 2 || parts[0] == "" || parts[1] == "" { + return "", errors.Errorf("invalid repository URL %q: must contain at least owner and repository (e.g. host/owner/repo)", upstreamURL) + } + return filepath.Join(parsed.Host, trimmed), nil } func (m *Manager) clonePathForURL(upstreamURL string) (string, error) { diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index aad30a8e..956e3f19 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -253,6 +253,36 @@ def456 refs/heads/develop assert.Equal(t, "789012", refs["refs/tags/v1.0.0"]) } +func TestRepoPathFromURL(t *testing.T) { + tests := []struct { + name string + url string + want string + wantErr bool + }{ + {name: "valid org/repo", url: "https://github.com/squareup/blox", want: "github.com/squareup/blox"}, + {name: "valid with .git suffix", url: "https://github.com/squareup/blox.git", want: "github.com/squareup/blox"}, + {name: "valid nested path", url: "https://gitlab.com/group/subgroup/repo", want: "gitlab.com/group/subgroup/repo"}, + {name: "org only", url: "https://github.com/squareup", wantErr: true}, + {name: "org only trailing slash", url: "https://github.com/squareup/", wantErr: true}, + {name: "org only with .git", url: "https://github.com/squareup/.git", wantErr: true}, + {name: "host only", url: "https://github.com", wantErr: true}, + {name: "host only trailing slash", url: "https://github.com/", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := RepoPathFromURL(tt.url) + if tt.wantErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid repository URL") + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + func TestState_String(t *testing.T) { assert.Equal(t, "empty", StateEmpty.String()) assert.Equal(t, "cloning", StateCloning.String())