{"item":{"uri":"at://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mzgyznklioc5","kind":"pull","title":"knotless-serve answers repositories' sites on their own domain","state":"merged","author":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","avatar":"https://cdn.bsky.app/img/avatar/plain/did:plc:bcgltzqazw5tb6k2g3ttenbj/bafkreig5bcb7dl7aphw6eg4e7kjkvpdshfexdvk7jldg53ypre6ecdhesu@jpeg"},"createdAt":"2026-11-03T23:37:34.669Z","comments":16},"body":"## What changes\n\n**`knotless-serve --pages-domain knotless.page`** answers repositories'\nsites, wrapped around every other route. A request whose host is that\ndomain or a name under it is answered as a site and nothing else. Only\n`GET` and `HEAD` are allowed (anything else is 405), and nothing else on\nthe server is reachable from those hosts. The flag needs `--public-url`,\nsince an owner's root (`/`) redirects to their page there.\n\nA request is answered in this order:\n\n1. **The owner.** The host's label is read back to a handle, with `--`\n for `-` and `-` for `.`, and resolved to a DID. The DID's own claimed\n handle must match it. A 24-character label of `[a-z2-7]` is a plc id.\n2. **The repository.** It's found by name in the index, then its\n `dev.knotless.site` record by the repository's key. Taken-down records,\n repositories and accounts are left out.\n3. **The path.** `x/` serves `x/index.html`. `x` with only\n `x/index.html` redirects to `x/`, and `/` to `//`. A page\n no file has gets `fallback`, and anything else missing gets `notFound`\n with 404, or a plain 404.\n4. **The file.** It's read from the owner's PDS on first use, joined from\n its parts, checked against the record's size and SHA-256, and kept by\n its hash beside release files. A mismatch is a 502. Responses carry\n the record's `contentType`, `nosniff`, an ETag of the hash (304 on a\n match) and `max-age=60`.\n\n**The index** gains a `site` table. It keeps a public `dev.knotless.site`\nrecord by URI, passing by one in a space or one that isn't a site's\nrecord. It's backfilled, dropped with its repository and purged with its\naccount. `site` is in `GROWING`, and its one query is by primary key.\n\n## Why\n\nThis is the serving half of pages (docs/design/pages.md), on top of\n`knotless site`. That pull is\nat://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mzgyhfum74gm,\nand this branch holds its commits until it lands.\n\n## How it was checked\n\n- `tests/sites.rs`: an appview following the PDS answers a site deployed\n with `knotless site`. It covers both owner labels, the ETag and 304,\n directory redirects, the owner's root, the 404 page and plain 404s\n (no site, no repository, a bad path, unknown and unresolvable labels),\n 405, other hosts passing through, a redeploy as a single-page app, a\n tampered hash giving 502, and taking the site down.\n- Unit tests cover the path rules, `is_page`, percent-decoding, host\n parsing and the index's `site` table.\n- `tests/serve.rs` runs the binary with `--pages-domain`, and checks it\n refuses the flag without `--public-url`.\n\nNothing was built here: CI builds and runs it.\n\n## Shortcomings\n\n- A renamed repository's old path does not redirect yet, though the\n design page says it does.\n- Private sites, custom domains, and the `deploy/` changes (Caddy,\n wildcard certificate, DNS) are later pulls.\n- The map cache is cleared whole once it holds 1,024 sites.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)\n","html":"

What changes

knotless-serve --pages-domain knotless.page answers repositories'\nsites, wrapped around every other route. A request whose host is that\ndomain or a name under it is answered as a site and nothing else. Only\nGET and HEAD are allowed (anything else is 405), and nothing else on\nthe server is reachable from those hosts. The flag needs --public-url,\nsince an owner's root (/) redirects to their page there.

A request is answered in this order:

The owner. The host's label is read back to a handle, with --\nfor - and - for ., and resolved to a DID. The DID's own claimed\nhandle must match it. A 24-character label of [a-z2-7] is a plc id.
The repository. It's found by name in the index, then its\ndev.knotless.site record by the repository's key. Taken-down records,\nrepositories and accounts are left out.
The path. x/ serves x/index.html. x with only\nx/index.html redirects to x/, and /<repo> to /<repo>/. A page\nno file has gets fallback, and anything else missing gets notFound\nwith 404, or a plain 404.
The file. It's read from the owner's PDS on first use, joined from\nits parts, checked against the record's size and SHA-256, and kept by\nits hash beside release files. A mismatch is a 502. Responses carry\nthe record's contentType, nosniff, an ETag of the hash (304 on a\nmatch) and max-age=60.

The index gains a site table. It keeps a public dev.knotless.site\nrecord by URI, passing by one in a space or one that isn't a site's\nrecord. It's backfilled, dropped with its repository and purged with its\naccount. site is in GROWING, and its one query is by primary key.

Why

This is the serving half of pages (docs/design/pages.md), on top of\nknotless site. That pull is\nknotless site deploys a directory as a repository's website,\nand this branch holds its commits until it lands.

How it was checked

tests/sites.rs: an appview following the PDS answers a site deployed\nwith knotless site. It covers both owner labels, the ETag and 304,\ndirectory redirects, the owner's root, the 404 page and plain 404s\n(no site, no repository, a bad path, unknown and unresolvable labels),\n405, other hosts passing through, a redeploy as a single-page app, a\ntampered hash giving 502, and taking the site down.
Unit tests cover the path rules, is_page, percent-decoding, host\nparsing and the index's site table.
tests/serve.rs runs the binary with --pages-domain, and checks it\nrefuses the flag without --public-url.

Nothing was built here: CI builds and runs it.

Shortcomings

A renamed repository's old path does not redirect yet, though the\ndesign page says it does.
Private sites, custom domains, and the deploy/ changes (Caddy,\nwildcard certificate, DNS) are later pulls.
The map cache is cleared whole once it holds 1,024 sites.

🤖 Generated with Claude Code

\n","base":"feature/pages","head":{"repo":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","name":"knotless"},"branch":"pages/serve"},"merged":"20dbf7d1511d27e9d6b6463f2fad3b886cbe3dd2","comments":[{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogqs2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:38:51.342Z","body":"This adds `--pages-domain` to knotless-serve. Requests on that domain, or on a name under it, are answered from `dev.knotless.site` records, with files read from the owner's PDS and cached by hash. The index gains a `site` table. The path rules, 304s, redirects and the takedown filtering look right.\n\n**Must fix: arbitrary file read and write through `sha256`.** `SiteFile.sha256` comes from a record the repository owner controls. Nothing checks it before it goes into file paths:\n- `fetch` does `dir.join(&file.sha256)` and calls `try_exists`. A record with `sha256: \"../../../../../../../../../../../../../../../../../../etc/passwd\"` (padded to 64 characters) passes the lexicon's length rule. The PDS doesn't validate this unknown lexicon, and the index only runs `serde_json::from_value`. The file exists, so `keep` is skipped and `File::open` streams it back to the requester.\n- `keep` builds `.{sha256}.{n}` under `dir` and writes there. A `sha256` such as `./../x` gives `../../x.0`, which sits outside the cache directory. The hash check fails afterwards, but the file has already been created, and removing it is only best effort.\n\nThe fix is to require exactly 64 lowercase hex characters. Do it in the index, where a record that fails is passed by like a non-site record, and again in `fetch` (or `SiteFile`) as a second check. Add a test that a record with a traversal `sha256` is not served.\n\n**Smaller: open redirect.** `answer` redirects a bare `/` to `/{name}/` with `name` taken after percent-decoding. `GET /%5Cevil.com` gives `Location: /\\evil.com/`, and browsers read that as `//evil.com`. Reject a `name` with `\\` or control characters, or redirect to the percent-encoded form.","html":"

This adds --pages-domain to knotless-serve. Requests on that domain, or on a name under it, are answered from dev.knotless.site records, with files read from the owner's PDS and cached by hash. The index gains a site table. The path rules, 304s, redirects and the takedown filtering look right.

Must fix: arbitrary file read and write through sha256. SiteFile.sha256 comes from a record the repository owner controls. Nothing checks it before it goes into file paths:

fetch does dir.join(&file.sha256) and calls try_exists. A record with sha256: \"../../../../../../../../../../../../../../../../../../etc/passwd\" (padded to 64 characters) passes the lexicon's length rule. The PDS doesn't validate this unknown lexicon, and the index only runs serde_json::from_value. The file exists, so keep is skipped and File::open streams it back to the requester.
keep builds .{sha256}.{n} under dir and writes there. A sha256 such as ./../x gives ../../x.0, which sits outside the cache directory. The hash check fails afterwards, but the file has already been created, and removing it is only best effort.

The fix is to require exactly 64 lowercase hex characters. Do it in the index, where a record that fails is passed by like a non-site record, and again in fetch (or SiteFile) as a second check. Add a test that a record with a traversal sha256 is not served.

Smaller: open redirect. answer redirects a bare /<name> to /{name}/ with name taken after percent-decoding. GET /%5Cevil.com gives Location: /\\evil.com/, and browsers read that as //evil.com. Reject a name with \\ or control characters, or redirect to the percent-encoded form.

\n","verdict":"changes","commit":"310e0e3af9a31816dff684a831020f0a2da9745f"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogue2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:38:51.342Z","body":"`file.sha256` is unvalidated record data, so this join can leave `dir`. With `../…` as the hash, `try_exists` is true for any existing file and it is then opened and served. Require 64 lowercase hex characters before using it as a filename.","html":"

file.sha256 is unvalidated record data, so this join can leave dir. With ../… as the hash, try_exists is true for any existing file and it is then opened and served. Require 64 lowercase hex characters before using it as a filename.

\n","commit":"310e0e3af9a31816dff684a831020f0a2da9745f","line":{"path":"crates/knotless-serve/src/sites.rs","line":321,"outdated":true,"lines":[{"number":318,"text":" ) -> anyhow::Result {"},{"number":319,"text":" let ctx = &self.app.ctx;"},{"number":320,"text":" let dir = crate::releases::kept_dir(ctx);"},{"number":321,"text":" let at = dir.join(&file.sha256);"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogqs2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogvc2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:38:51.342Z","body":"The same unvalidated hash goes into this temp-file name, so `./../x` creates a file outside `dir` before the hash check fails.","html":"

The same unvalidated hash goes into this temp-file name, so ./../x creates a file outside dir before the hash check fails.

\n","commit":"310e0e3af9a31816dff684a831020f0a2da9745f","line":{"path":"crates/knotless-serve/src/sites.rs","line":345,"outdated":true,"lines":[{"number":342,"text":" let remote = Remote::open(ctx, &url).await?;"},{"number":343,"text":" tokio::fs::create_dir_all(dir).await?;"},{"number":344,"text":" let n = KEEPING.fetch_add(1, Ordering::Relaxed);"},{"number":345,"text":" let keeping = dir.join(format!(\".{}.{n}\", file.sha256));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogqs2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogvw2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:38:51.342Z","body":"`name` is percent-decoded, so `/%5Cevil.com` redirects to `/\\evil.com/`, which browsers treat as `//evil.com`. Reject or encode `\\` and control characters.","html":"

name is percent-decoded, so /%5Cevil.com redirects to /\\evil.com/, which browsers treat as //evil.com. Reject or encode \\ and control characters.

\n","commit":"310e0e3af9a31816dff684a831020f0a2da9745f","line":{"path":"crates/knotless-serve/src/sites.rs","line":260,"outdated":true,"lines":[{"number":257,"text":" return moved(&format!(\"{}/{named}\", self.home.trim_end_matches('/')));"},{"number":258,"text":" }"},{"number":259,"text":" let Some(rest) = rest else {"},{"number":260,"text":" return moved(&format!(\"/{name}/\"));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz3wogqs2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6futz72b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:40:14.390Z","body":"The head moved from 310e0e3 to 8928687, but only through merge commits. The serve code is unchanged, so nothing I asked for last round has been done.\n\n- **`sha256` path traversal (must fix).** `fetch` still does `dir.join(&file.sha256)` and `keep` still builds `.{sha256}.{n}` from the same unvalidated record value. A record owner can make the server serve any readable file, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, where a record that fails is passed by like a non-site record. Check again in `fetch` as a second line. Add a test that a record with a traversal `sha256` is not served.\n- **Open redirect.** `/%5Cevil.com` still redirects to `/\\evil.com/`, which browsers read as `//evil.com`. Reject or percent-encode `\\` and control characters in `name`.\n\nBecause this is a security fix, please don't merge until both are done. The pull is already queued, so you may want to remove it from the queue while you fix them.","html":"

The head moved from 310e0e3 to 8928687, but only through merge commits. The serve code is unchanged, so nothing I asked for last round has been done.

sha256 path traversal (must fix). fetch still does dir.join(&file.sha256) and keep still builds .{sha256}.{n} from the same unvalidated record value. A record owner can make the server serve any readable file, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, where a record that fails is passed by like a non-site record. Check again in fetch as a second line. Add a test that a record with a traversal sha256 is not served.
Open redirect. /%5Cevil.com still redirects to /\\evil.com/, which browsers read as //evil.com. Reject or percent-encode \\ and control characters in name.

Because this is a security fix, please don't merge until both are done. The pull is already queued, so you may want to remove it from the queue while you fix them.

\n","verdict":"changes","commit":"8928687c5caf21a9a778e1ef98661b77c118772d"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6fuu5h2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:40:14.390Z","body":"Still unvalidated: `file.sha256` can be `../…`, so this join leaves `dir` and an existing file is opened and served. Check for 64 lowercase hex before use.","html":"

Still unvalidated: file.sha256 can be ../…, so this join leaves dir and an existing file is opened and served. Check for 64 lowercase hex before use.

\n","commit":"8928687c5caf21a9a778e1ef98661b77c118772d","line":{"path":"crates/knotless-serve/src/sites.rs","line":321,"outdated":true,"lines":[{"number":318,"text":" ) -> anyhow::Result {"},{"number":319,"text":" let ctx = &self.app.ctx;"},{"number":320,"text":" let dir = crate::releases::kept_dir(ctx);"},{"number":321,"text":" let at = dir.join(&file.sha256);"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6futz72b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6fuu6i2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:40:14.390Z","body":"Still unvalidated: this temp name is built from the same `sha256`, so a value like `./../x` creates a file outside `dir`.","html":"

Still unvalidated: this temp name is built from the same sha256, so a value like ./../x creates a file outside dir.

\n","commit":"8928687c5caf21a9a778e1ef98661b77c118772d","line":{"path":"crates/knotless-serve/src/sites.rs","line":345,"outdated":true,"lines":[{"number":342,"text":" let remote = Remote::open(ctx, &url).await?;"},{"number":343,"text":" tokio::fs::create_dir_all(dir).await?;"},{"number":344,"text":" let n = KEEPING.fetch_add(1, Ordering::Relaxed);"},{"number":345,"text":" let keeping = dir.join(format!(\".{}.{n}\", file.sha256));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6futz72b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6fuu7q2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-03T23:40:14.390Z","body":"`name` is percent-decoded, so `/%5Cevil.com` redirects to `/\\evil.com/`, an open redirect. Reject or encode `\\` and control characters.","html":"

name is percent-decoded, so /%5Cevil.com redirects to /\\evil.com/, an open redirect. Reject or encode \\ and control characters.

\n","commit":"8928687c5caf21a9a778e1ef98661b77c118772d","line":{"path":"crates/knotless-serve/src/sites.rs","line":260,"outdated":true,"lines":[{"number":257,"text":" return moved(&format!(\"{}/{named}\", self.home.trim_end_matches('/')));"},{"number":258,"text":" }"},{"number":259,"text":" let Some(rest) = rest else {"},{"number":260,"text":" return moved(&format!(\"/{name}/\"));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzgz6futz72b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w4d2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:00:04.886Z","body":"The head moved from 8928687 to a885225 through a merge and two small commits. Neither fix I asked for is in, and the one new commit (\"The firehose keeps a site's record for the index\") touches neither.\n\n- **`sha256` path traversal (must fix, still open).** `fetch` still does `dir.join(&file.sha256)` (`sites.rs:321`). `keep` still builds `.{sha256}.{n}` from the same unchecked record value (`sites.rs:345`). A repository owner can make the server serve any readable file, or create files outside the cache directory. The index test still stores a site with `sha256: \"a\"` (`index.rs:7002`), so the index checks nothing here. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again in `fetch` as a second line. Add a test that a traversal `sha256` is not served. In `tests/sites.rs` the hash only appears in the tamper test (`tests/sites.rs:158`), and that one uses valid hex.\n- **Open redirect (still open).** `moved(&format!(\"/{name}/\"))` at `sites.rs:260` uses the percent-decoded `name`, so `/%5Cevil.com` redirects to `/\\evil.com/`. Browsers read that as `//evil.com`. Reject `\\` and control characters in `name`, or percent-encode it.\n\nI'm asking for changes again because the first one is a file-read hole reachable by any account. The queue's pre-flight check on 8928687 also failed, so check the checks on the new head.","html":"

The head moved from 8928687 to a885225 through a merge and two small commits. Neither fix I asked for is in, and the one new commit (\"The firehose keeps a site's record for the index\") touches neither.

sha256 path traversal (must fix, still open). fetch still does dir.join(&file.sha256) (sites.rs:321). keep still builds .{sha256}.{n} from the same unchecked record value (sites.rs:345). A repository owner can make the server serve any readable file, or create files outside the cache directory. The index test still stores a site with sha256: \"a\" (index.rs:7002), so the index checks nothing here. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again in fetch as a second line. Add a test that a traversal sha256 is not served. In tests/sites.rs the hash only appears in the tamper test (tests/sites.rs:158), and that one uses valid hex.
Open redirect (still open). moved(&format!(\"/{name}/\")) at sites.rs:260 uses the percent-decoded name, so /%5Cevil.com redirects to /\\evil.com/. Browsers read that as //evil.com. Reject \\ and control characters in name, or percent-encode it.

I'm asking for changes again because the first one is a file-read hole reachable by any account. The queue's pre-flight check on 8928687 also failed, so check the checks on the new head.

\n","verdict":"changes","commit":"a885225ff8eba48bdc5587c322729078cd5a350d"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w6f2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:00:04.886Z","body":"Still unvalidated: `file.sha256` can be `../…`, so this join leaves `dir` and an existing file is opened and served. Require 64 lowercase hex before using it as a filename, in the index and here.","html":"

Still unvalidated: file.sha256 can be ../…, so this join leaves dir and an existing file is opened and served. Require 64 lowercase hex before using it as a filename, in the index and here.

\n","commit":"a885225ff8eba48bdc5587c322729078cd5a350d","line":{"path":"crates/knotless-serve/src/sites.rs","side":"new","line":321,"outdated":true,"lines":[{"number":318,"text":" ) -> anyhow::Result {"},{"number":319,"text":" let ctx = &self.app.ctx;"},{"number":320,"text":" let dir = crate::releases::kept_dir(ctx);"},{"number":321,"text":" let at = dir.join(&file.sha256);"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w4d2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w6v2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:00:04.886Z","body":"Same unvalidated `sha256` in the temp name: `./../x` creates a file outside `dir` before the hash check fails.","html":"

Same unvalidated sha256 in the temp name: ./../x creates a file outside dir before the hash check fails.

\n","commit":"a885225ff8eba48bdc5587c322729078cd5a350d","line":{"path":"crates/knotless-serve/src/sites.rs","side":"new","line":345,"outdated":true,"lines":[{"number":342,"text":" let remote = Remote::open(ctx, &url).await?;"},{"number":343,"text":" tokio::fs::create_dir_all(dir).await?;"},{"number":344,"text":" let n = KEEPING.fetch_add(1, Ordering::Relaxed);"},{"number":345,"text":" let keeping = dir.join(format!(\".{}.{n}\", file.sha256));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w4d2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w772b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:00:04.886Z","body":"`name` is percent-decoded, so `/%5Cevil.com` gives `Location: /\\evil.com/`, an open redirect. Reject or encode `\\` and control characters.","html":"

name is percent-decoded, so /%5Cevil.com gives Location: /\\evil.com/, an open redirect. Reject or encode \\ and control characters.

\n","commit":"a885225ff8eba48bdc5587c322729078cd5a350d","line":{"path":"crates/knotless-serve/src/sites.rs","side":"new","line":260,"outdated":true,"lines":[{"number":257,"text":" return moved(&format!(\"{}/{named}\", self.home.trim_end_matches('/')));"},{"number":258,"text":" }"},{"number":259,"text":" let Some(rest) = rest else {"},{"number":260,"text":" return moved(&format!(\"/{name}/\"));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2bv7w4d2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2j47zvm2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:04:07.111Z","body":"The head moved from a885225 to 133bf27 with one new commit (\"Every collection a backfill reads, the firehose keeps\"). Neither fix I asked for is in.\n\n- **`sha256` path traversal (must fix, still open).** `fetch` still does `dir.join(&file.sha256)` (`sites.rs:321`). `keep` still builds `.{sha256}.{n}` from the same unchecked record value (`sites.rs:345`). A repository owner can make the server serve any file it can read, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again in `fetch` as a second line. Add a test that a traversal `sha256` is not served.\n- **Open redirect (still open).** `moved(&format!(\"/{name}/\"))` at `sites.rs:260` uses the percent-decoded `name`. `/%5Cevil.com` redirects to `/\\evil.com/`, which browsers read as `//evil.com`. Reject `\\` and control characters in `name`, or percent-encode it.\n\nPlease don't merge until both are done.","html":"

The head moved from a885225 to 133bf27 with one new commit (\"Every collection a backfill reads, the firehose keeps\"). Neither fix I asked for is in.

sha256 path traversal (must fix, still open). fetch still does dir.join(&file.sha256) (sites.rs:321). keep still builds .{sha256}.{n} from the same unchecked record value (sites.rs:345). A repository owner can make the server serve any file it can read, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again in fetch as a second line. Add a test that a traversal sha256 is not served.
Open redirect (still open). moved(&format!(\"/{name}/\")) at sites.rs:260 uses the percent-decoded name. /%5Cevil.com redirects to /\\evil.com/, which browsers read as //evil.com. Reject \\ and control characters in name, or percent-encode it.

Please don't merge until both are done.

\n","verdict":"changes","commit":"133bf27ed1184f316a31ff3199336a02abe4a310"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2j47zy52b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:04:07.111Z","body":"`file.sha256` comes straight from the owner's record and is joined into a path unchecked. Require 64 lowercase hex here and in the index.","html":"

file.sha256 comes straight from the owner's record and is joined into a path unchecked. Require 64 lowercase hex here and in the index.

\n","commit":"133bf27ed1184f316a31ff3199336a02abe4a310","line":{"path":"crates/knotless-serve/src/sites.rs","line":321,"outdated":true,"lines":[{"number":318,"text":" ) -> anyhow::Result {"},{"number":319,"text":" let ctx = &self.app.ctx;"},{"number":320,"text":" let dir = crate::releases::kept_dir(ctx);"},{"number":321,"text":" let at = dir.join(&file.sha256);"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2j47zvm2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2j47zyz2b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:04:07.111Z","body":"`name` is percent-decoded, so `/%5Cevil.com` redirects to `/\\evil.com/`. Reject `\\` and control characters, or percent-encode the name.","html":"

name is percent-decoded, so /%5Cevil.com redirects to /\\evil.com/. Reject \\ and control characters, or percent-encode the name.

\n","commit":"133bf27ed1184f316a31ff3199336a02abe4a310","line":{"path":"crates/knotless-serve/src/sites.rs","line":260,"outdated":true,"lines":[{"number":257,"text":" return moved(&format!(\"{}/{named}\", self.home.trim_end_matches('/')));"},{"number":258,"text":" }"},{"number":259,"text":" let Some(rest) = rest else {"},{"number":260,"text":" return moved(&format!(\"/{name}/\"));"}]},"review":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh2j47zvm2b"},{"uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh3lfssy52b","author":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"createdAt":"2026-11-04T00:23:18.014Z","body":"Both problems from my earlier rounds are now fixed at 8c5a70c.\n\n- **`sha256` traversal:** `fetch` now checks the hash with `valid_sha256` (`sites.rs:327`) before any path is built from it. `valid_sha256` requires exactly 64 lowercase hex characters, so `keep`'s `.{sha256}.{n}` name (`sites.rs:353`) is safe too.\n- **Open redirect:** `answer` returns a 404 for a `name` that contains `\\` or a control character, before it builds the `/{name}/` redirect (`sites.rs:259`).\n\nI did not check whether the index also rejects a bad hash, or whether tests for these two cases exist. The check in `fetch` is enough to close the hole.","html":"

Both problems from my earlier rounds are now fixed at 8c5a70c.

sha256 traversal: fetch now checks the hash with valid_sha256 (sites.rs:327) before any path is built from it. valid_sha256 requires exactly 64 lowercase hex characters, so keep's .{sha256}.{n} name (sites.rs:353) is safe too.
Open redirect: answer returns a 404 for a name that contains \\ or a control character, before it builds the /{name}/ redirect (sites.rs:259).

I did not check whether the index also rejects a bad hash, or whether tests for these two cases exist. The check in fetch is enough to close the hole.

\n","verdict":"approve","commit":"8c5a70cce2b357e8894ed8a6be1e8385c8c9040d"}],"reviews":[{"reviewer":{"did":"did:plc:kqi2glvzerg4dt66k64co2sl","handle":"review.knotless.dev","bot":true},"verdict":"approve","uri":"at://did:plc:kqi2glvzerg4dt66k64co2sl/dev.knotless.comment/3mzh3lfssy52b","commit":"8c5a70cce2b357e8894ed8a6be1e8385c8c9040d","outdated":false,"maintainer":false}],"closes":[],"checks":[],"minutes":[{"runner":{"did":"did:plc:4bocpkn5zkwe4wuu4plrj5kt","handle":"runner.knotless.dev","bot":true},"minutes":12647,"freeing":[{"at":"2026-11-26T00:00:00Z","minutes":1938},{"at":"2026-11-27T00:00:00Z","minutes":751},{"at":"2026-11-28T00:00:00Z","minutes":864},{"at":"2026-11-29T00:00:00Z","minutes":776},{"at":"2026-11-30T00:00:00Z","minutes":289},{"at":"2026-12-01T00:00:00Z","minutes":544},{"at":"2026-12-02T00:00:00Z","minutes":1086},{"at":"2026-12-03T00:00:00Z","minutes":979},{"at":"2026-12-04T00:00:00Z","minutes":718},{"at":"2026-12-05T00:00:00Z","minutes":282},{"at":"2026-12-06T00:00:00Z","minutes":174},{"at":"2026-12-07T00:00:00Z","minutes":1812},{"at":"2026-12-08T00:00:00Z","minutes":1688},{"at":"2026-12-09T00:00:00Z","minutes":353},{"at":"2026-12-10T00:00:00Z","minutes":370},{"at":"2026-12-11T00:00:00Z","minutes":16}],"updatedAt":"2026-11-11T06:06:20.035Z"},{"runner":{"did":"did:plc:ejh2v3z5ieqhqgz3ybk6r35x","handle":"ci.knotless.dev","bot":true},"minutes":14931,"freeing":[{"at":"2026-11-24T00:00:00Z","minutes":1},{"at":"2026-11-25T00:00:00Z","minutes":2513},{"at":"2026-11-26T00:00:00Z","minutes":1801},{"at":"2026-11-27T00:00:00Z","minutes":1597},{"at":"2026-11-28T00:00:00Z","minutes":922},{"at":"2026-11-29T00:00:00Z","minutes":1059},{"at":"2026-11-30T00:00:00Z","minutes":370},{"at":"2026-12-01T00:00:00Z","minutes":1046},{"at":"2026-12-02T00:00:00Z","minutes":1475},{"at":"2026-12-03T00:00:00Z","minutes":1198},{"at":"2026-12-04T00:00:00Z","minutes":959},{"at":"2026-12-05T00:00:00Z","minutes":353},{"at":"2026-12-06T00:00:00Z","minutes":322},{"at":"2026-12-07T00:00:00Z","minutes":535},{"at":"2026-12-08T00:00:00Z","minutes":351},{"at":"2026-12-09T00:00:00Z","minutes":178},{"at":"2026-12-10T00:00:00Z","minutes":242}],"updatedAt":"2026-11-11T06:00:20.652Z"}],"changes":{"repo":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","name":"knotless"},"commits":[{"sha":"20dbf7d1511d27e9d6b6463f2fad3b886cbe3dd2","parents":[{"sha":"d067ffc98471b363ed7563cd91cd8fd2c25e70f0"}],"author":{"name":"Chad Miller","actor":{"did":"did:plc:bcgltzqazw5tb6k2g3ttenbj","handle":"chadtmiller.com","avatar":"https://cdn.bsky.app/img/avatar/plain/did:plc:bcgltzqazw5tb6k2g3ttenbj/bafkreig5bcb7dl7aphw6eg4e7kjkvpdshfexdvk7jldg53ypre6ecdhesu@jpeg"}},"coauthors":[],"committer":{"name":"ci.knotless.dev"},"date":"2026-11-03T16:21:44-08:00","committedAt":"2026-11-04T00:57:52+00:00","message":"knotless-serve answers repositories' sites on their own domain\n\n## What changes\n\n**`knotless-serve --pages-domain knotless.page`** answers repositories'\nsites, wrapped around every other route. A request whose host is that\ndomain or a name under it is answered as a site and nothing else. Only\n`GET` and `HEAD` are allowed (anything else is 405), and nothing else on\nthe server is reachable from those hosts. The flag needs `--public-url`,\nsince an owner's root (`/`) redirects to their page there.\n\nA request is answered in this order:\n\n1. **The owner.** The host's label is read back to a handle, with `--`\n for `-` and `-` for `.`, and resolved to a DID. The DID's own claimed\n handle must match it. A 24-character label of `[a-z2-7]` is a plc id.\n2. **The repository.** It's found by name in the index, then its\n `dev.knotless.site` record by the repository's key. Taken-down records,\n repositories and accounts are left out.\n3. **The path.** `x/` serves `x/index.html`. `x` with only\n `x/index.html` redirects to `x/`, and `/` to `//`. A page\n no file has gets `fallback`, and anything else missing gets `notFound`\n with 404, or a plain 404.\n4. **The file.** It's read from the owner's PDS on first use, joined from\n its parts, checked against the record's size and SHA-256, and kept by\n its hash beside release files. A mismatch is a 502. Responses carry\n the record's `contentType`, `nosniff`, an ETag of the hash (304 on a\n match) and `max-age=60`.\n\n**The index** gains a `site` table. It keeps a public `dev.knotless.site`\nrecord by URI, passing by one in a space or one that isn't a site's\nrecord. It's backfilled, dropped with its repository and purged with its\naccount. `site` is in `GROWING`, and its one query is by primary key.\n\n## Why\n\nThis is the serving half of pages (docs/design/pages.md), on top of\n`knotless site`. That pull is\nat://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mzgyhfum74gm,\nand this branch holds its commits until it lands.\n\n## How it was checked\n\n- `tests/sites.rs`: an appview following the PDS answers a site deployed\n with `knotless site`. It covers both owner labels, the ETag and 304,\n directory redirects, the owner's root, the 404 page and plain 404s\n (no site, no repository, a bad path, unknown and unresolvable labels),\n 405, other hosts passing through, a redeploy as a single-page app, a\n tampered hash giving 502, and taking the site down.\n- Unit tests cover the path rules, `is_page`, percent-decoding, host\n parsing and the index's `site` table.\n- `tests/serve.rs` runs the binary with `--pages-domain`, and checks it\n refuses the flag without `--public-url`.\n\nNothing was built here: CI builds and runs it.\n\n## Shortcomings\n\n- A renamed repository's old path does not redirect yet, though the\n design page says it does.\n- Private sites, custom domains, and the `deploy/` changes (Caddy,\n wildcard certificate, DNS) are later pulls.\n- The map cache is cleared whole once it holds 1,024 sites.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)","pull":{"uri":"at://did:plc:bcgltzqazw5tb6k2g3ttenbj/dev.knotless.pull/3mzgyznklioc5","title":"knotless-serve answers repositories' sites on their own domain"}}],"files":[]},"card":"/chadtmiller.com/knotless/pull/did:plc:bcgltzqazw5tb6k2g3ttenbj/3mzgyznklioc5/card.png?v=c21fe900138493b4"}

chadtmiller.com/knotless Switch repository Loading…

knotless-serve answers repositories' sites on their own domain

chadtmiller.com/knotless

Switch repository

Loading…