chadtmiller.com/knotless

Switch repository

Loading…

What changes

knotless-serve answers Cargo's registry protocol, one registry for each repository, at /cargo/<did>/<repo key>/: the sparse index, downloads, cargo publish, cargo yank and its undo, and cargo search. A token is <handle>:<app password>, which logs in to the owner's PDS for that request and is kept nowhere.

A release gains an optional cargo object: its index, the line of Cargo's index as cargo publish described it, and its description.

docs/design/packages.md has a new Crates section, and docs/reference/api.md lists the routes.

Why

So a repository's crates can be used with Cargo, as its images are with Docker. A throwaway server first showed real cargo working against release records.

Each choice settles one thing that spike found, as Cargo registry: a stock PDS types a .crate's blob as application/gzip lists:

• The address holds a DID and a key, not names, since Cargo keeps it in Cargo.lock and in other repositories' index lines. A rename was not tried.
• The index line is in the record, as Cargo's own JSON rather than as fields of ours, since the format is Cargo's to change and readers pass it on. No blob is read to answer the index, and a publish uploads one fewer.
• It reads the index, so a takedown holds, and a publish and a yank write their records to it themselves, since cargo publish waits for its crate and warns after a minute.
• A version differing only in build metadata is found once its record exists, as it has another key: the record is deleted and the publish refused. The index keeps the oldest should both remain.

How it was checked

a_cargo_registry runs real cargo against a served index and the PDS: publish, renamed and optional dependencies and namespaced features, a build from nothing kept, refusals, a yank against a lockfile and a new resolve, and search. Unit tests cover the framing, the tarball, the index line and the layout. The rest is left to CI.

Shortcomings

An app password can do far more than publish, and sits in ~/.cargo/credentials.toml in plain text. Scoped tokens are the follow-up.

Public repositories only. No commit or check on a release made this way, as Cargo sends neither. cargo owner is not answered. A crate name with capitals is refused, as a release's name must be lowercase.

package_of reads the name and version out of Cargo.toml by hand, enough for what cargo package writes, since no TOML parser is in the workspace. Not tried: a remote PDS, or the releases page showing how to use a crate, which comes next in this stack.

Review

The lexicon first, then crates/knotless-serve/src/web/cargo.rs's publish.

🤖 Generated with Claude Code

This adds a per-repository Cargo registry to knotless-serve (sparse index, download, publish, yank and its undo, search). It stores a crate as a release with an optional cargo object holding Cargo's index line and a description. I read the lexicon, cargo.rs, records.rs and release.rs in full, plus AGENTS.md. I did not read web.rs, releases.rs, steps.rs, the tests or the docs, and I did not look at the checks.

The design holds up. Auth is checked before the body is read, and the session DID must equal the repository owner's. The index line is validated against the record's package and version before it is served. The lexicon and Rust limits agree: 1000 chars is at most 4000 bytes. Build-metadata duplicates are handled by the oldest-wins rule plus a delete-and-refuse on publish. A change file is present.

Two small things, neither blocking:

• Failed publish response: in publish, if get_record or index.put fails after target.create succeeded, the client gets a 502 although the release exists. A retry then says "published already". It is rare, but a warn or a note in the error would help.
• Large manifest allocation: manifest_of trusts the tar header size for Cargo.toml and reads up to MOST_UNPACKED (1 GiB) into a String. Only the owner can reach this, but capping the manifest read at something like 1 MiB is cheap.

This round adds three changes on top of the commit I reviewed: a merge of the base branch, a wider description limit, and a yank record that no longer requires repo.

• Description limit: maxLength rises from 4000 to 10000 bytes, and maxGraphemes stays at 1000. publish truncates to 1000 characters, which is at most 4000 bytes, so the Rust side still fits.
• Yank record: repo is now optional in both the lexicon and the Rust struct (#[serde(default)]). index.rs takes the repo key from the record key's prefix instead. This is consistent, provided every yank record's key begins with <repo key>.. That holds for the ones cargo.rs writes, but I did not trace other writers.
• Earlier suggestions: my two non-blocking suggestions (cap the Cargo.toml read in manifest_of, and tolerate a failed get_record or index.put after the release is created) are not addressed in this diff. I left them non-blocking, so they don't hold up approval. I would still take them as a follow-up.
• Not checked: I did not look at the checks this round.

This round changes only tests. In releases.rs, a stranger's auth is bound once instead of being repeated. In web.rs, a_cargo_registry now uploads a real blob whose bytes don't match the recorded sha256, instead of citing a blob the PDS never held. That is a sturdier way to test the 502 on download, and the intent is the same. I read only this diff, not the rest of the pull again. No checks showed for this commit, so I can't say whether they pass. My two earlier non-blocking suggestions are still open: cap the Cargo.toml read in manifest_of, and handle a failed get_record or index.put after the release is created. Neither blocks approval, and both would suit a follow-up.

Carried forward: the change this pull makes at 36d8387b25b325ce2f8b8444719bccb35aec6983 is the one reviewed at 313f5195a66d459181b7cc39243fee94f53fe3be, so that review stands. What is new comes from its base.

Since the commit I last reviewed, this head adds only merges from main. They bring in the release/published-already change, the merge-queue opt-in, crawler robots.txt rules and the code-search skip for empty repositories. I did not re-read those changes or the cargo code, which is unchanged apart from the merges.

The published already change is relevant to publish. Target::create now reports that message only when the record can be read back, and otherwise gives the real failure. That makes a failed publish easier to diagnose, which partly covers my earlier note about a failed write after the release exists.

Two earlier non-blocking suggestions are still open:

• Cap the Cargo.toml read in manifest_of.
• Handle a failed get_record or index.put after the release is created. Today the client gets a 502 although the release exists.

Checks on this commit were still queued or running when I looked. Only docs had passed, so I can't say the rest pass. Neither suggestion blocks approval, and both suit a follow-up.

Since de4eca8, the only change is one commit that adds release's large-string goat warning to the lint exceptions in harness/lexicons.mjs, and updates the "two warnings" wording in lexicons/AGENTS.md to match. The stated reason is sound: Cargo's index line can be tens of KB (the comment gives web-sys at 54 KB), and a published lexicon limit can't be raised later. I read only this diff, not the cargo code again. I didn't check CI results.

My two earlier non-blocking suggestions are still open and don't hold up approval: cap the Cargo.toml read in manifest_of, and tolerate a failed get_record or index.put after the release is created.

Out of review rounds, approved at e754b266. Since then only a 7-line change in cargo.rs removing an unreachable branch for the coverage gate, no change in behaviour. All seven checks pass at c04f8377.