From 3ed8bc2cf1e818ab7e49ce4fb5bae37c9d720073 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Wed, 11 Nov 2026 16:31:26 +0100 Subject: [PATCH 1/3] Delete a cached Git repository from the UI and API With cache_delete set, DELETE /api/git/{upstream}/{repo} removes one repository from the git cache and /ui/git gets a Delete button per repository. The repository is moved aside under the cache's lock, as eviction does, so the next clone fetches it again. One that a clone, a fetch or a client is using answers 409. The response names the repository and the bytes freed. --- README.md | 1 + docs/configuration.md | 10 ++- docs/swagger/docs.go | 59 ++++++++++++++ docs/swagger/swagger.json | 59 ++++++++++++++ internal/gitcache/remove.go | 54 +++++++++++++ internal/gitcache/remove_test.go | 73 ++++++++++++++++++ internal/server/git_delete.go | 53 +++++++++++++ internal/server/git_delete_test.go | 77 +++++++++++++++++++ internal/server/git_ui.go | 9 ++- internal/server/server.go | 6 ++ .../templates/pages/git_repositories.html | 7 +- 11 files changed, 403 insertions(+), 5 deletions(-) create mode 100644 internal/gitcache/remove.go create mode 100644 internal/gitcache/remove_test.go create mode 100644 internal/server/git_delete.go create mode 100644 internal/server/git_delete_test.go diff --git a/README.md b/README.md index 7ba3cb21..c169e7ff 100644 --- a/README.md +++ b/README.md @@ -1381,6 +1381,7 @@ Registered when `cache_delete` is set. It has no authentication of its own. | Endpoint | Description | |----------|-------------| | `DELETE /api/cache/{ecosystem}/{name}/{version}` | Delete a version's cached files, or only `?filename=` | +| `DELETE /api/git/{upstream}/{repo}` | Delete a repository from the git cache | ### Enrichment API diff --git a/docs/configuration.md b/docs/configuration.md index 40e94343..f30f242f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1888,7 +1888,7 @@ cache_delete: true Or via environment variable: `PROXY_CACHE_DELETE=true`. -The endpoint has no authentication. Before enabling it, restrict `/api/cache` and `/ui` to trusted users at your ingress or reverse proxy. The UI sends its deletes to `/ui/api/cache`, so whatever protects `/ui` protects those too. +The endpoints have no authentication. Before enabling them, restrict `/api/cache`, `/api/git` and `/ui` to trusted users at your ingress or reverse proxy. The UI sends its deletes to `/ui/api/cache` and `/ui/api/git`, so whatever protects `/ui` protects those too. Delete every cached file of a version, or only the one named by `filename`: @@ -1901,6 +1901,14 @@ The name is the package name the UI shows, for example `group:artifact` for Mave Pushed archives are not touched: a whole-version delete skips them, and naming one answers 409. Delete those through the [hosted API](#hosted-packages). +With `upstream.git` set, `DELETE /api/git/{upstream}/{repo}` removes one repository from the [git cache](#git-repositories), and the `/ui/git` page gets a Delete button per repository: + +```bash +curl -X DELETE http://localhost:8080/api/git/github/acme/widgets.git +``` + +`{upstream}` is the name under `upstream.git`, and `{repo}` the repository path with or without `.git`. The response names the repository and the bytes freed (`freed_bytes`). The next clone fetches it from the upstream again. A repository that a clone, a fetch or a client is using at that moment answers 409; try again once it is done. Git LFS objects are deleted through `/api/cache` like other artifacts. + ## Mirror Command The `proxy mirror` command pre-populates the cache from various sources. It accepts the same storage and database flags as `serve`. diff --git a/docs/swagger/docs.go b/docs/swagger/docs.go index fe8723d8..22a922dd 100644 --- a/docs/swagger/docs.go +++ b/docs/swagger/docs.go @@ -182,6 +182,54 @@ const docTemplate = `{ } } }, + "/api/git/{upstream}/{repo}": { + "delete": { + "description": "Removes one repository from the git cache, so the next clone fetches it from the upstream again. A repository that a clone, a fetch or a client is using answers 409. Needs cache_delete.", + "produces": [ + "application/json" + ], + "tags": [ + "cache" + ], + "summary": "Delete a cached Git repository", + "parameters": [ + { + "type": "string", + "description": "Name of the upstream.git entry", + "name": "upstream", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Repository path under the upstream, with or without .git", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/server.GitDeleteResponse" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/server.ErrorResponse" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/server.ErrorResponse" + } + } + } + } + }, "/api/hosted/{ecosystem}/{name}": { "get": { "produces": [ @@ -1074,6 +1122,17 @@ const docTemplate = `{ } } }, + "server.GitDeleteResponse": { + "type": "object", + "properties": { + "freed_bytes": { + "type": "integer" + }, + "repository": { + "type": "string" + } + } + }, "server.HealthCheck": { "type": "object", "properties": { diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index 496aecde..1832c05a 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -175,6 +175,54 @@ } } }, + "/api/git/{upstream}/{repo}": { + "delete": { + "description": "Removes one repository from the git cache, so the next clone fetches it from the upstream again. A repository that a clone, a fetch or a client is using answers 409. Needs cache_delete.", + "produces": [ + "application/json" + ], + "tags": [ + "cache" + ], + "summary": "Delete a cached Git repository", + "parameters": [ + { + "type": "string", + "description": "Name of the upstream.git entry", + "name": "upstream", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Repository path under the upstream, with or without .git", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/server.GitDeleteResponse" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/server.ErrorResponse" + } + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/server.ErrorResponse" + } + } + } + } + }, "/api/hosted/{ecosystem}/{name}": { "get": { "produces": [ @@ -1067,6 +1115,17 @@ } } }, + "server.GitDeleteResponse": { + "type": "object", + "properties": { + "freed_bytes": { + "type": "integer" + }, + "repository": { + "type": "string" + } + } + }, "server.HealthCheck": { "type": "object", "properties": { diff --git a/internal/gitcache/remove.go b/internal/gitcache/remove.go new file mode 100644 index 00000000..87705672 --- /dev/null +++ b/internal/gitcache/remove.go @@ -0,0 +1,54 @@ +package gitcache + +import ( + "errors" + "os" + "path/filepath" +) + +var ( + // ErrNotCached is returned by Remove for a repository the cache does + // not hold. + ErrNotCached = errors.New("repository is not cached") + // ErrInUse is returned by Remove for a repository a clone, a fetch or + // a client is using. + ErrInUse = errors.New("repository is in use") +) + +// Remove deletes the cached repository under key, as eviction does, and +// returns its size. The next request for it clones it again. A repository +// in use is not removed and returns ErrInUse, so nothing reads it while it +// is deleted. +func (c *Cache) Remove(key string) (int64, error) { + c.mu.Lock() + rp := c.repos[c.repoKey(key)] + if rp == nil || !rp.exists() { + c.mu.Unlock() + return 0, ErrNotCached + } + if rp.users > 0 { + c.mu.Unlock() + return 0, ErrInUse + } + // Moved aside under the lock, so a request that comes after it clones + // the repository again instead of reading it while it is deleted. + trash, err := os.MkdirTemp(c.tmpDir(), "removed-") + if err == nil { + err = os.Rename(rp.dir, filepath.Join(trash, "repo")) + } + if err != nil { + c.mu.Unlock() + if trash != "" { + _ = os.Remove(trash) + } + return 0, err + } + delete(c.repos, rp.key) + delete(c.tooLarge, rp.key) + size := rp.size.Load() + c.mu.Unlock() + + c.logger.Info("git cache: removed repository", "repository", rp.key, "size", size) + c.removeEvicted([]string{trash}) + return size, nil +} diff --git a/internal/gitcache/remove_test.go b/internal/gitcache/remove_test.go new file mode 100644 index 00000000..0587a108 --- /dev/null +++ b/internal/gitcache/remove_test.go @@ -0,0 +1,73 @@ +package gitcache + +import ( + "errors" + "path/filepath" + "testing" +) + +func TestRemoveDeletesARepository(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + dir := filepath.Join(tc.dir, "up", "repo.git") + want := tc.repo("up/repo.git").size.Load() + + freed, err := tc.Remove("up/repo.git") + if err != nil { + t.Fatalf("Remove: %v", err) + } + if freed != want || freed == 0 { + t.Errorf("freed %d bytes, want %d", freed, want) + } + if fileExists(dir) || tc.repo("up/repo.git") != nil { + t.Error("the repository is still cached") + } + if len(tc.Repositories()) != 0 { + t.Errorf("Repositories() = %+v, want none", tc.Repositories()) + } + if _, err := tc.Remove("up/repo.git"); !errors.Is(err, ErrNotCached) { + t.Errorf("second Remove: %v, want ErrNotCached", err) + } + + // The next clone fetches it again. + requests := upstream.gitRequests.Load() + tc.clone(t, "repo") + if upstream.gitRequests.Load() == requests || !fileExists(dir) { + t.Error("the clone after Remove did not fetch the repository again") + } +} + +// A repository that a clone, a fetch or a client is using is not removed, +// so nothing reads it while it is deleted. +func TestRemoveRefusesARepositoryInUse(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + + rp := tc.acquire("up/repo.git") + if _, err := tc.Remove("up/repo.git"); !errors.Is(err, ErrInUse) { + t.Errorf("Remove while in use: %v, want ErrInUse", err) + } + tc.release(rp) + if !fileExists(filepath.Join(tc.dir, "up", "repo.git")) { + t.Fatal("the repository was removed while in use") + } + if _, err := tc.Remove("up/repo.git"); err != nil { + t.Errorf("Remove once idle: %v", err) + } +} + +func TestRemoveUnknownRepository(t *testing.T) { + upstream := newFakeUpstream(t) + tc := newTestCache(t, upstream.server.URL, nil) + for _, key := range []string{"up/missing.git", "up/../up.git", ""} { + if _, err := tc.Remove(key); !errors.Is(err, ErrNotCached) { + t.Errorf("Remove(%q): %v, want ErrNotCached", key, err) + } + } +} diff --git a/internal/server/git_delete.go b/internal/server/git_delete.go new file mode 100644 index 00000000..a1f68d78 --- /dev/null +++ b/internal/server/git_delete.go @@ -0,0 +1,53 @@ +package server + +import ( + "errors" + "net/http" + "strings" + + "github.com/git-pkgs/proxy/internal/gitcache" + "github.com/go-chi/chi/v5" +) + +// GitDeleteResponse names the cached Git repository removed and the space +// it took. +type GitDeleteResponse struct { + Repository string `json:"repository"` + FreedBytes int64 `json:"freed_bytes"` +} + +// handleGitDelete handles DELETE /api/git/{upstream}/{repo} +// @Summary Delete a cached Git repository +// @Description Removes one repository from the git cache, so the next clone fetches it from the upstream again. A repository that a clone, a fetch or a client is using answers 409. Needs cache_delete. +// @Tags cache +// @Produce json +// @Param upstream path string true "Name of the upstream.git entry" +// @Param repo path string true "Repository path under the upstream, with or without .git" +// @Success 200 {object} GitDeleteResponse +// @Failure 404 {object} ErrorResponse +// @Failure 409 {object} ErrorResponse +// @Router /api/git/{upstream}/{repo} [delete] +func (s *Server) handleGitDelete(w http.ResponseWriter, r *http.Request) { + name, repo, _ := strings.Cut(chi.URLParam(r, "*"), "/") + repo = strings.TrimSuffix(repo, ".git") + upstream, ok := s.cfg.Upstream.Git[name] + if !ok || repo == "" { + notFound(w, "repository not cached") + return + } + freed, err := s.git.Remove(gitcache.UpstreamDir(name, upstream.URL) + "/" + repo + ".git") + switch { + case errors.Is(err, gitcache.ErrNotCached): + notFound(w, "repository not cached") + return + case errors.Is(err, gitcache.ErrInUse): + writeError(w, http.StatusConflict, ErrCodeConflict, "repository is being cloned, fetched or read; try again") + return + case err != nil: + s.logger.Error("failed to delete cached git repository", "upstream", name, "repository", repo, "error", err) + internalError(w, "failed to delete cached repository") + return + } + s.logger.Info("deleted cached git repository", "upstream", name, "repository", repo, "freed", freed) + writeJSON(w, GitDeleteResponse{Repository: name + "/" + repo, FreedBytes: freed}) +} diff --git a/internal/server/git_delete_test.go b/internal/server/git_delete_test.go new file mode 100644 index 00000000..35cfd24f --- /dev/null +++ b/internal/server/git_delete_test.go @@ -0,0 +1,77 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/git-pkgs/proxy/internal/config" +) + +func TestGitRepositoryDelete(t *testing.T) { + for _, prefix := range []string{"/api/git/", "/ui/api/git/"} { + t.Run(prefix, func(t *testing.T) { + root := t.TempDir() + newGitTestRepo(t, root, "team/repo") + upstream := httptest.NewServer(gitBackend(t, root)) + defer upstream.Close() + cfg, _ := startGitTestServer(t, func(cfg *config.Config) { + cfg.Upstream.Git = map[string]config.GitUpstream{"up": {URL: upstream.URL}} + cfg.CacheDelete = true + }) + gitCommand(t, "", "clone", "--quiet", cfg.BaseURL+"/git/up/team/repo.git", filepath.Join(t.TempDir(), "clone")) + + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); !strings.Contains(body, `data-cache-url="/ui/api/git/up/team/repo.git"`) { + t.Errorf("the git page has no delete control:\n%s", body) + } + + status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+"up/team/repo.git", "", nil) + if status != http.StatusOK { + t.Fatalf("DELETE: status %d\n%s", status, body) + } + var resp GitDeleteResponse + if err := json.Unmarshal([]byte(body), &resp); err != nil { + t.Fatalf("decoding %s: %v", body, err) + } + if resp.Repository != "up/team/repo" || resp.FreedBytes <= 0 { + t.Errorf("response = %+v", resp) + } + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); !strings.Contains(body, "No repository is cached yet") { + t.Errorf("the repository is still listed:\n%s", body) + } + + for _, path := range []string{"up/team/repo.git", "up/team/repo", "other/team/repo.git", "up/"} { + if status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+path, "", nil); status != http.StatusNotFound { + t.Errorf("DELETE %s: status %d, want 404\n%s", path, status, body) + } + } + }) + } +} + +func TestGitRepositoryDeleteNeedsCacheDelete(t *testing.T) { + root := t.TempDir() + newGitTestRepo(t, root, "team/widgets") + upstream := httptest.NewServer(gitBackend(t, root)) + defer upstream.Close() + cfg, _ := startGitTestServer(t, func(cfg *config.Config) { + cfg.Upstream.Git = map[string]config.GitUpstream{"up": {URL: upstream.URL}} + }) + gitCommand(t, "", "clone", "--quiet", cfg.BaseURL+"/git/up/team/widgets.git", filepath.Join(t.TempDir(), "clone")) + + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); strings.Contains(body, "data-cache-url") { + t.Errorf("the git page shows a delete control without cache_delete:\n%s", body) + } + for _, prefix := range []string{"/api/git/", "/ui/api/git/"} { + status, _ := request(t, http.MethodDelete, cfg.BaseURL+prefix+"up/team/widgets.git", "", nil) + if status == http.StatusOK { + t.Errorf("DELETE %s: status 200 without cache_delete", prefix) + } + } + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); !strings.Contains(body, "team/widgets") { + t.Error("the repository was deleted without cache_delete") + } +} diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index 661d48c1..ca81ce88 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -28,9 +28,11 @@ type GitRepositoriesData struct { // GitRepositoryRow is one cached repository on the git page. type GitRepositoryRow struct { Upstream, Repo, CloneURL string - Size string - Requests string - LastChecked, LastUsed string + // DeleteURL is where the UI sends a delete of the repository. + DeleteURL string + Size string + Requests string + LastChecked, LastUsed string // LastError names the kind of error the last refresh failed with. LastError, LastErrorAt string // Branch, Commit, Subject and CommitAge describe the newest commit of @@ -98,6 +100,7 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, Upstream: name, Repo: strings.TrimSuffix(path, ".git"), CloneURL: strings.TrimSuffix(baseURL, "/") + "/git/" + escapePath(name+"/"+path), + DeleteURL: "/ui/api/git/" + escapePath(name+"/"+path), Size: formatSize(repo.Size), Requests: plural(int(repo.Requests), "request", "requests"), LastChecked: formatTimeAgo(repo.LastChecked), diff --git a/internal/server/server.go b/internal/server/server.go index 19ba5bee..be194da3 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -389,6 +389,9 @@ func (s *Server) serve(listener net.Listener) error { s.mountProjectUI(ui) if s.git != nil { ui.Get("/git", s.handleGitRepositories) + if s.cfg.CacheDelete { + ui.Delete("/api/git/*", s.handleGitDelete) + } } }) r.Get("/", func(w http.ResponseWriter, r *http.Request) { @@ -396,6 +399,9 @@ func (s *Server) serve(listener net.Listener) error { }) if s.cfg.CacheDelete { r.Delete("/api/cache/{ecosystem}/*", s.handleCacheDelete) + if s.git != nil { + r.Delete("/api/git/*", s.handleGitDelete) + } } s.mountEnrichmentAPI(r) diff --git a/internal/server/templates/pages/git_repositories.html b/internal/server/templates/pages/git_repositories.html index fb9bee2c..92713196 100644 --- a/internal/server/templates/pages/git_repositories.html +++ b/internal/server/templates/pages/git_repositories.html @@ -9,9 +9,10 @@

Git repositories

{{if .Rows}}
+ {{if .CacheDelete}}{{end}}
{{range .Rows}} -
+
{{.Upstream}} / {{.Repo}}
{{.CloneURL}}
@@ -28,6 +29,7 @@

Git repositories

{{.Requests}} {{if .LastChecked}}checked {{.LastChecked}}{{end}} {{if .LastUsed}}used {{.LastUsed}}{{end}} + {{if $.CacheDelete}}{{end}}
{{end}} @@ -38,4 +40,7 @@

Git repositories

No repository is cached yet

{{end}} +{{if and .CacheDelete .Rows}} + +{{end}} {{end}} From 0a48042dd0676a0bb8789877c4978ffbfb9f89c2 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Wed, 11 Nov 2026 16:52:52 +0100 Subject: [PATCH 2/3] Answer 409 for a first clone and read the decoded path A repository whose first clone was still running answered 404, since its directory is moved into place only at the end; it is in use, so it now answers 409. Removing a repository clears its failed wants: one the old copy could not fit may fit in the next clone. The delete reads the repository from the decoded request path, as the clone handler does. chi's wildcard stays encoded when the request escapes a character, so team%2Frepo named another repository. --- internal/gitcache/remove.go | 15 +++++++++- internal/gitcache/remove_test.go | 44 ++++++++++++++++++++++++++++++ internal/server/git_delete.go | 6 ++-- internal/server/git_delete_test.go | 6 ++-- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/internal/gitcache/remove.go b/internal/gitcache/remove.go index 87705672..1a2f1267 100644 --- a/internal/gitcache/remove.go +++ b/internal/gitcache/remove.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "strings" ) var ( @@ -22,14 +23,20 @@ var ( func (c *Cache) Remove(key string) (int64, error) { c.mu.Lock() rp := c.repos[c.repoKey(key)] - if rp == nil || !rp.exists() { + if rp == nil { c.mu.Unlock() return 0, ErrNotCached } + // Before the directory check: a first clone is built elsewhere and + // moved into place once done. if rp.users > 0 { c.mu.Unlock() return 0, ErrInUse } + if !rp.exists() { + c.mu.Unlock() + return 0, ErrNotCached + } // Moved aside under the lock, so a request that comes after it clones // the repository again instead of reading it while it is deleted. trash, err := os.MkdirTemp(c.tmpDir(), "removed-") @@ -45,6 +52,12 @@ func (c *Cache) Remove(key string) (int64, error) { } delete(c.repos, rp.key) delete(c.tooLarge, rp.key) + // A want the old copy could not fit may fit in the next clone. + for k := range c.failedWants { + if strings.HasPrefix(k, rp.key+" ") { + delete(c.failedWants, k) + } + } size := rp.size.Load() c.mu.Unlock() diff --git a/internal/gitcache/remove_test.go b/internal/gitcache/remove_test.go index 0587a108..4c3b91a0 100644 --- a/internal/gitcache/remove_test.go +++ b/internal/gitcache/remove_test.go @@ -71,3 +71,47 @@ func TestRemoveUnknownRepository(t *testing.T) { } } } + +// A repository whose first clone is still running is in use, not missing. +func TestRemoveRefusesARepositoryBeingCloned(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + arrived, release := upstream.holdFetches() + cloned := make(chan error, 1) + go func() { + _, err := tryGit(t, "", "clone", "--quiet", tc.url("repo"), filepath.Join(t.TempDir(), "clone")) + cloned <- err + }() + <-arrived + if _, err := tc.Remove("up/repo.git"); !errors.Is(err, ErrInUse) { + t.Errorf("Remove during the first clone: %v, want ErrInUse", err) + } + release() + if err := <-cloned; err != nil { + t.Fatalf("clone: %v", err) + } +} + +// Wants a removed repository could not get may fit once it is cloned again, +// so removing it clears them, and only them. +func TestRemoveForgetsTheRepositorysFailedWants(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + oid := "0123456789abcdef0123456789abcdef01234567" + tc.recordFailedWants("up/repo.git", []string{oid}, ErrTooLarge) + tc.recordFailedWants("up/other.git", []string{oid}, ErrTooLarge) + + if _, err := tc.Remove("up/repo.git"); err != nil { + t.Fatalf("Remove: %v", err) + } + if got := tc.withoutFailedWants("up/repo.git", []string{oid}); len(got) != 1 { + t.Error("the removed repository's failed want is still held back") + } + if got := tc.withoutFailedWants("up/other.git", []string{oid}); len(got) != 0 { + t.Error("another repository's failed want was cleared") + } +} diff --git a/internal/server/git_delete.go b/internal/server/git_delete.go index a1f68d78..6c9aa84e 100644 --- a/internal/server/git_delete.go +++ b/internal/server/git_delete.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/git-pkgs/proxy/internal/gitcache" - "github.com/go-chi/chi/v5" ) // GitDeleteResponse names the cached Git repository removed and the space @@ -28,7 +27,10 @@ type GitDeleteResponse struct { // @Failure 409 {object} ErrorResponse // @Router /api/git/{upstream}/{repo} [delete] func (s *Server) handleGitDelete(w http.ResponseWriter, r *http.Request) { - name, repo, _ := strings.Cut(chi.URLParam(r, "*"), "/") + // From the decoded path, as the clone handler reads it: chi's wildcard + // is still encoded when the request escapes a character such as "/". + _, path, _ := strings.Cut(r.URL.Path, "/api/git/") + name, repo, _ := strings.Cut(path, "/") repo = strings.TrimSuffix(repo, ".git") upstream, ok := s.cfg.Upstream.Git[name] if !ok || repo == "" { diff --git a/internal/server/git_delete_test.go b/internal/server/git_delete_test.go index 35cfd24f..44d8f770 100644 --- a/internal/server/git_delete_test.go +++ b/internal/server/git_delete_test.go @@ -28,7 +28,9 @@ func TestGitRepositoryDelete(t *testing.T) { t.Errorf("the git page has no delete control:\n%s", body) } - status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+"up/team/repo.git", "", nil) + // An encoded separator names the repository the clone handler + // does, team/repo. + status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+"up/team%2Frepo.git", "", nil) if status != http.StatusOK { t.Fatalf("DELETE: status %d\n%s", status, body) } @@ -43,7 +45,7 @@ func TestGitRepositoryDelete(t *testing.T) { t.Errorf("the repository is still listed:\n%s", body) } - for _, path := range []string{"up/team/repo.git", "up/team/repo", "other/team/repo.git", "up/"} { + for _, path := range []string{"up/team/repo.git", "up/team/repo", "other/team/repo.git", "up/", "up/team%252Frepo.git"} { if status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+path, "", nil); status != http.StatusNotFound { t.Errorf("DELETE %s: status %d, want 404\n%s", path, status, body) } From 4cd5e4f75d6817e86d397716c511004b105d70aa Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Thu, 12 Nov 2026 11:01:26 +0000 Subject: [PATCH 3/3] Name /api/git in the cache_delete notes and test a literal %2F before the delete --- config.example.yaml | 7 ++++--- internal/config/config.go | 7 ++++--- internal/server/git_delete_test.go | 6 +++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/config.example.yaml b/config.example.yaml index a99dbebf..6148d20b 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -22,9 +22,10 @@ base_url: "http://localhost:8080" # scanning. Set to "0" to disable. Default: "5m". Does not limit proxy mirror. # artifact_timeout: "5m" -# Register DELETE /api/cache and show Delete buttons on the UI's version page, -# for removing cached artifacts so they are fetched again. Has no -# authentication: restrict /api/cache and /ui at the ingress first. +# Register DELETE /api/cache and, with upstream.git set, DELETE /api/git, and +# show Delete buttons on the UI's version and git pages, for removing cached +# artifacts and repositories so they are fetched again. Has no authentication: +# restrict /api/cache, /api/git and /ui at the ingress first. # cache_delete: false # Largest upstream artifact to fetch (e.g. "2GB"). Larger ones are refused diff --git a/internal/config/config.go b/internal/config/config.go index f93c996d..0cfaa3b8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -212,9 +212,10 @@ type Config struct { // Mirror configures scheduled mirroring. Mirror MirrorConfig `json:"mirror" yaml:"mirror"` - // CacheDelete enables DELETE /api/cache for removing cached artifacts, and - // the delete buttons on the UI's version page. It has no authentication of - // its own, so restrict /api/cache and /ui at the ingress when enabling it. + // CacheDelete enables DELETE /api/cache and DELETE /api/git for removing + // cached artifacts and git repositories, and the delete buttons on the + // UI's version and git pages. It has no authentication of its own, so + // restrict /api/cache, /api/git and /ui at the ingress when enabling it. CacheDelete bool `json:"cache_delete" yaml:"cache_delete"` // Hosted configures pushing packages to the proxy (/api/hosted). diff --git a/internal/server/git_delete_test.go b/internal/server/git_delete_test.go index 44d8f770..11df476c 100644 --- a/internal/server/git_delete_test.go +++ b/internal/server/git_delete_test.go @@ -24,6 +24,10 @@ func TestGitRepositoryDelete(t *testing.T) { }) gitCommand(t, "", "clone", "--quiet", cfg.BaseURL+"/git/up/team/repo.git", filepath.Join(t.TempDir(), "clone")) + // A literal %2F names no cached repository, and leaves team/repo. + if status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+"up/team%252Frepo.git", "", nil); status != http.StatusNotFound { + t.Errorf("DELETE up/team%%252Frepo.git: status %d, want 404\n%s", status, body) + } if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); !strings.Contains(body, `data-cache-url="/ui/api/git/up/team/repo.git"`) { t.Errorf("the git page has no delete control:\n%s", body) } @@ -45,7 +49,7 @@ func TestGitRepositoryDelete(t *testing.T) { t.Errorf("the repository is still listed:\n%s", body) } - for _, path := range []string{"up/team/repo.git", "up/team/repo", "other/team/repo.git", "up/", "up/team%252Frepo.git"} { + for _, path := range []string{"up/team/repo.git", "up/team/repo", "other/team/repo.git", "up/"} { if status, body := request(t, http.MethodDelete, cfg.BaseURL+prefix+path, "", nil); status != http.StatusNotFound { t.Errorf("DELETE %s: status %d, want 404\n%s", path, status, body) }