From 10b5e8188a6900cebd6ba4073cc3f6f0654cb2a7 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 14:52:06 +0200 Subject: [PATCH] Refuse a yarn lockfile the scanner stopped in, and a forge answer the cap cut Either was read as whole and could leave a pinned version unnamed (review of #128). --- modules/gitea/cmd/npm-registry/forge.go | 10 ++++++++- modules/gitea/cmd/npm-registry/refs.go | 12 +++++++--- modules/gitea/cmd/npm-registry/refs_test.go | 21 ++++++++++++++++-- .../gitea/cmd/npm-registry/retention_test.go | 22 +++++++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/modules/gitea/cmd/npm-registry/forge.go b/modules/gitea/cmd/npm-registry/forge.go index 5ce649ec..948a3d21 100644 --- a/modules/gitea/cmd/npm-registry/forge.go +++ b/modules/gitea/cmd/npm-registry/forge.go @@ -78,6 +78,9 @@ func (f *Forge) password() (string, error) { return strings.TrimSpace(string(raw)), nil } +// maxBody is the largest answer read from the forge. +const maxBody = 32 << 20 + // do sends one request and answers the body; 404 is errNotFound. func (f *Forge) do(ctx context.Context, method, path string) ([]byte, error) { password, err := f.password() @@ -95,10 +98,15 @@ func (f *Forge) do(ctx context.Context, method, path string) ([]byte, error) { return nil, fmt.Errorf("the forge did not answer %s %s: %w", method, path, err) } defer res.Body.Close() - body, err := io.ReadAll(io.LimitReader(res.Body, 32<<20)) + // One byte past the cap is read, so a body the cap cut is told from one that fits it exactly: a + // cut body is refused, never parsed as if it were whole. + body, err := io.ReadAll(io.LimitReader(res.Body, maxBody+1)) if err != nil { return nil, err } + if len(body) > maxBody { + return nil, fmt.Errorf("the forge's answer to %s %s is larger than %d MiB and was not read whole", method, path, maxBody>>20) + } switch { case res.StatusCode == http.StatusNotFound: return nil, fmt.Errorf("%s %s: %w", method, path, errNotFound) diff --git a/modules/gitea/cmd/npm-registry/refs.go b/modules/gitea/cmd/npm-registry/refs.go index 371d50ea..55fcfa45 100644 --- a/modules/gitea/cmd/npm-registry/refs.go +++ b/modules/gitea/cmd/npm-registry/refs.go @@ -98,7 +98,7 @@ func ReadLock(base string, content []byte, where string, known map[string]bool) case "package-lock.json", "npm-shrinkwrap.json": return readNpmLock(content, where, known) case "yarn.lock": - return readYarnLock(content, where, known), nil + return readYarnLock(content, where, known) case "pnpm-lock.yaml": return readTextLock(content, where, known), nil } @@ -199,7 +199,10 @@ func cutLast(s, sep string) (string, string, bool) { // readYarnLock reads both yarn formats: a header of the specs it resolves (`"@a/b@^1.0.0", "@a/b@^1.1.0":`) // and an indented `version "1.2.3"` (classic) or `version: 1.2.3` (berry) under it. -func readYarnLock(content []byte, where string, known map[string]bool) []Pinned { +// +// A line longer than the scanner's buffer stops it; that is an error, never a lockfile read to its end, +// because what follows the line pins versions too. +func readYarnLock(content []byte, where string, known map[string]bool) ([]Pinned, error) { var out []Pinned var names []string scanner := bufio.NewScanner(bytes.NewReader(content)) @@ -229,7 +232,10 @@ func readYarnLock(content []byte, where string, known map[string]bool) []Pinned addYarn(&out, names, strings.Trim(v, `"`), where, known) } } - return out + if err := scanner.Err(); err != nil { + return out, fmt.Errorf("%s was not read to its end: %w", where, err) + } + return out, nil } func addYarn(out *[]Pinned, names []string, version, where string, known map[string]bool) { diff --git a/modules/gitea/cmd/npm-registry/refs_test.go b/modules/gitea/cmd/npm-registry/refs_test.go index 6b91f310..430e3094 100644 --- a/modules/gitea/cmd/npm-registry/refs_test.go +++ b/modules/gitea/cmd/npm-registry/refs_test.go @@ -66,11 +66,11 @@ func TestAnNpmLockfileOfEveryVersionNamesWhatItPins(t *testing.T) { func TestAYarnLockOfEitherFormatNamesWhatItPins(t *testing.T) { classic := "# yarn lockfile v1\n\n\"@novox/mesh-sdk@^0.1.0\", \"@novox/mesh-sdk@^0.1.2\":\n version \"0.1.4\"\n resolved \"x\"\n dependencies:\n left-pad \"^1\"\n\nleft-pad@^1:\n version \"1.3.0\"\n" - if got := pins(readYarnLock([]byte(classic), "r", knownHere)); got != "@novox/mesh-sdk@0.1.4" { + if got := pins(mustYarn(t, classic)); got != "@novox/mesh-sdk@0.1.4" { t.Fatalf("classic: %s", got) } berry := "__metadata:\n version: 6\n\n\"@novox/ui@npm:^2.0.0\":\n version: 2.0.7\n resolution: \"@novox/ui@npm:2.0.7\"\n" - if got := pins(readYarnLock([]byte(berry), "r", knownHere)); got != "@novox/ui@2.0.7" { + if got := pins(mustYarn(t, berry)); got != "@novox/ui@2.0.7" { t.Fatalf("berry: %s", got) } } @@ -196,3 +196,20 @@ func TestAnEntryOfAnUnknownShapeIsSaidAndTheRestIsRead(t *testing.T) { t.Fatalf("got %v", err) } } + +func mustYarn(t *testing.T, content string) []Pinned { + t.Helper() + out, err := readYarnLock([]byte(content), "r", knownHere) + if err != nil { + t.Fatal(err) + } + return out +} + +// A yarn lockfile with a line longer than the scanner holds is an error, not a file read to its end. +func TestAYarnLockfileCutShortIsAnError(t *testing.T) { + long := "\"@novox/mesh-sdk@^0.1.0\":\n version \"0.1.4\"\n# " + strings.Repeat("x", 2<<20) + "\n\"@novox/ui@^2\":\n version \"2.0.7\"\n" + if _, err := readYarnLock([]byte(long), "r", knownHere); err == nil { + t.Fatal("a lockfile the scanner stopped in was read as whole") + } +} diff --git a/modules/gitea/cmd/npm-registry/retention_test.go b/modules/gitea/cmd/npm-registry/retention_test.go index 1b0fdeaf..9c252803 100644 --- a/modules/gitea/cmd/npm-registry/retention_test.go +++ b/modules/gitea/cmd/npm-registry/retention_test.go @@ -488,3 +488,25 @@ func TestAnUnreadableLockfileStopsARealRunBeforeAnythingIsDeleted(t *testing.T) t.Errorf("said %v", dry.Said) } } + +// An answer larger than the cap is refused, never parsed as if the cut body were whole; one exactly at +// the cap is read. +func TestAnAnswerTheCapCutIsRefused(t *testing.T) { + size := maxBody + 1 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(make([]byte, size)) + })) + t.Cleanup(srv.Close) + file := filepath.Join(t.TempDir(), "admin.secret") + if err := os.WriteFile(file, []byte("x\n"), 0o600); err != nil { + t.Fatal(err) + } + f := &Forge{URL: srv.URL, User: "admin", PasswordFile: file, Owner: "acme"} + if _, err := f.do(context.Background(), http.MethodGet, "/big"); err == nil || !strings.Contains(err.Error(), "not read whole") { + t.Fatalf("a cut answer: %v", err) + } + size = maxBody + if body, err := f.do(context.Background(), http.MethodGet, "/big"); err != nil || len(body) != maxBody { + t.Fatalf("an answer at the cap: %d bytes, %v", len(body), err) + } +}