~ 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

knotless-serve answers repositories' sites on their own domain

Merged
Merged chadtmiller.com asked to merge into feature/pages from chadtmiller.com/knotless:pages/serve

What changes

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

A request is answered in this order:

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

The index gains a site table. It keeps a public dev.knotless.site record by URI, passing by one in a space or one that isn't a site's record. It's backfilled, dropped with its repository and purged with its account. 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 knotless site. That pull is knotless site deploys a directory as a repository's website, and this branch holds its commits until it lands.

How it was checked

  • tests/sites.rs: an appview following the PDS answers a site deployed with knotless site. It covers both owner labels, the ETag and 304, directory redirects, the owner's root, the 404 page and plain 404s (no site, no repository, a bad path, unknown and unresolvable labels), 405, other hosts passing through, a redeploy as a single-page app, a tampered hash giving 502, and taking the site down.
  • Unit tests cover the path rules, is_page, percent-decoding, host parsing and the index's site table.
  • tests/serve.rs runs the binary with --pages-domain, and checks it refuses 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 design page says it does.
  • Private sites, custom domains, and the deploy/ changes (Caddy, wildcard certificate, DNS) are later pulls.
  • The map cache is cleared whole once it holds 1,024 sites.

🤖 Generated with Claude Code

Asked for changes

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.

crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> {
let ctx = &self.app.ctx;
let dir = crate::releases::kept_dir(ctx);
let at = dir.join(&file.sha256);
review.knotless.dev

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.

crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?;
tokio::fs::create_dir_all(dir).await?;
let n = KEEPING.fetch_add(1, Ordering::Relaxed);
let keeping = dir.join(format!(".{}.{n}", file.sha256));
review.knotless.dev

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

crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/')));
}
let Some(rest) = rest else {
return moved(&format!("/{name}/"));
review.knotless.dev

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

Asked for changes

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.

crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> {
let ctx = &self.app.ctx;
let dir = crate::releases::kept_dir(ctx);
let at = dir.join(&file.sha256);
review.knotless.dev

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.

crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?;
tokio::fs::create_dir_all(dir).await?;
let n = KEEPING.fetch_add(1, Ordering::Relaxed);
let keeping = dir.join(format!(".{}.{n}", file.sha256));
review.knotless.dev

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

crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/')));
}
let Some(rest) = rest else {
return moved(&format!("/{name}/"));
review.knotless.dev

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

Asked for changes

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.

crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> {
let ctx = &self.app.ctx;
let dir = crate::releases::kept_dir(ctx);
let at = dir.join(&file.sha256);
review.knotless.dev

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.

crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?;
tokio::fs::create_dir_all(dir).await?;
let n = KEEPING.fetch_add(1, Ordering::Relaxed);
let keeping = dir.join(format!(".{}.{n}", file.sha256));
review.knotless.dev

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

crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/')));
}
let Some(rest) = rest else {
return moved(&format!("/{name}/"));
review.knotless.dev

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

Asked for changes

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.

crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> {
let ctx = &self.app.ctx;
let dir = crate::releases::kept_dir(ctx);
let at = dir.join(&file.sha256);
review.knotless.dev

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.

crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/')));
}
let Some(rest) = rest else {
return moved(&format!("/{name}/"));
review.knotless.dev

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

Approved

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.

Merged as 20dbf7d