~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

Cargo publish reads at most 1 MiB of Cargo.toml and succeeds once made

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:serve/cargo-publish

What changes

A crate's Cargo.toml is read only up to 1 MiB. A publish whose manifest is larger is refused, as one with none is. The size came from the tar header, so a publish could make serve read up to 1 GiB into memory.

A publish succeeds once its release is made. Reading the record back, putting it in the index, and reading the index for a version differing only in build metadata no longer answer 502 when they fail. The feed indexes the record soon after, and should the build metadata check go unread, the index answers the oldest of the two.

Why

Both are the review's notes on knotless-serve answers Cargo's registry for a repository's crates. A 502 after the record exists told Cargo the publish failed, and a retry was then refused as published already.

How it was checked

a_crates_manifest_is_found_in_it finds no manifest in a crate whose Cargo.toml is a byte over 1 MiB. The tolerated failures need a PDS or index that fails between two calls, which no test makes; a_cargo_registry still covers the path where they succeed.

Shortcomings

A failure after the record is made is now dropped without a log: serve has no log for a request's failures it carries on past.

Approved

This pull caps the Cargo.toml read at 1 MiB and makes a publish succeed once its release record exists.

  • Manifest cap: the size check runs before the take(size).read_to_string, so the tar header can no longer force a read of up to 1 GiB. The new test covers a manifest one byte over the cap.
  • Tolerated failures: the code reads the record back and puts it in the index with errors dropped. A failed index.releases becomes an empty list. I traced the build-metadata check through these cases and found no wrong result:
    • If the put fails but releases succeeds, an older sibling version is still found and the new record is rolled back.
    • If releases fails, both versions stay, as the body and docs say.
  • Rollback path: it still returns 502 on failure, which is right because the publish is being refused there.
  • Docs: packages.md and api.md are updated to match.

Dropping these failures without a log is a known tradeoff and the body states it.

Merged as c3ba398