@jpgilldev / steid

steid/plans/progress.md
43.4 KBRaw
1# Progress
2
3## This attempt (#3, Topcoat)
4
5445 tests. Active milestone in [current.md](current.md).
6
7### Milestone 0 — Skeleton · done
8
9Topcoat 0.5 app serving pages, `AppConfig` from `STEID_*` env, SQLite pool in app
10context. Split into a library plus a thin binary — the domain layer had no consumers
11yet and read as ~30 dead-code warnings in a bare binary, and it unlocks `tests/`.
12Topcoat's link-time page discovery works from a library; that was checked, not assumed.
13
14**Requires rustc ≥ 1.95.** On older toolchains `cargo add topcoat` silently resolves to
15an empty `topcoat v0.0.0` placeholder instead of failing.
16
17### Milestone 1 — Identity, thin · done
18
19**Domain.** Typed IDs, `Email`, `PasswordHash`, `OrgName`, `Organization`, `User`,
20`Membership`, `Role`, `Actor`, `Session`, `SetupToken`, `DomainError`. Value objects
21pair `new()` (validates) with `from_trusted()` (skips, for rows already validated).
22
23**Application.** `claim_instance`, `login`, `resolve_actor`, `record_session`,
24`end_session`, `sweep_expired`. `PasswordHasher` port with Argon2 and a stub.
25
26**Infrastructure.** Migrations for orgs, users, memberships, sessions. In-memory and
27SQLite implementations of every port. Root layout, `/setup`, `/login`, `/logout`, home,
28and `/api/me`.
29
30`describe_identity` is the first use case with two consumers — the home page and
31`/api/me` both read through it. Until that existed, "the application layer is
32transport-neutral" was an assertion with one caller behind it.
33
34**Verified in a browser and by curl:** wrong token refused with nothing written;
35correct token creates org + user + owner membership and signs the owner in; session
36authenticates; logout clears cookie and row; wrong password bounces; right one signs
37in; re-claiming a claimed instance is refused.
38
39#### Decisions worth remembering
40
41- **`Actor` is an enum with an explicit `Anonymous`**, not `Option<UserId>`. Attempt #2
42 used a placeholder `UserId("ssh-anonymous")` and it became a security hole. A variant
43 can't be forgotten the way a sentinel can.
44- **`login` verifies a dummy hash when no user matched.** Returning early on the
45 unknown-email path makes it measurably faster and leaks which addresses have
46 accounts. A test pins that the dummy stays parseable — if it stops being, `verify`
47 bails early and the defence dies silently.
48- **`SetupToken` compares in constant time.** An early-return comparison leaks how much
49 of the token is right, which recovers it a character at a time.
50- **The setup token is only in app context while unclaimed**, so a claimed instance has
51 nothing for a claim attempt to match.
52- **SQLite ignores foreign keys unless asked**, per connection. `foreign_keys(true)`
53 plus a test that a user pointing at a missing org is refused.
54- **`sqlx migrate add` stamps versions to the second** — three calls in one second
55 collide, leaving apply order ambiguous between tables that reference each other.
56- **An unparseable role surfaces as an error**, never as "no membership". The latter
57 silently downgrades an owner to no access.
58- **`Secure` session cookies over plain-HTTP localhost fail silently.** See
59 [runbook.md](runbook.md#steid_insecure_cookies--development-only). This one actually
60 bit, and it looked exactly like broken auth logic.
61
62### Milestone 2 — Profile page · done
63
64`/{handle}` is the real profile page: label, handle, bio, and the section frame for
65repositories, writing, and projects. Public, renders signed out, 404s on an unknown
66handle, and resolves regardless of casing. `/` forwards a signed-in owner to their own
67profile. `/api/users/{handle}` serves the same read model as JSON.
68
69URLs settled as root handles with grouped application routes
70([0004](decisions/0004-root-handles-grouped-routes.md)), superseding
71[0003](decisions/0003-scoped-urls.md) the same day. A twenty-word reserved list in
72`OrgName::new` keeps handles from shadowing routes.
73
74#### Decisions worth remembering
75
76- **`PublicProfile` has no email field, deliberately.** `Identity` does, and `/api/me`
77 returns it, because that endpoint describes the caller to themselves. Giving the type
78 that reaches the page nowhere to put an email makes the leak impossible rather than
79 merely avoided.
80- **`viewer_is_owner` is decided in the use case**, so the web form and `/api` cannot
81 disagree about who may edit. Tested for a signed-in stranger and a non-owner member —
82 "signed in" quietly becoming "allowed" is the usual failure.
83- **`Organization::update_profile` clears on blank input** rather than storing
84 whitespace, so cleared and never-set are one state and the page renders one case. A
85 rejected edit applies nothing.
86- **Bio length counts characters, not bytes.** A byte check would reject a bio of
87 accented text well under the limit.
88- **`path_param` is an attribute macro in 0.5**, not function-like. The vendored crate
89 is the authority for the pinned version, not the docs on `main`.
90- **Components are invoked bare inside `view!`**, and `if`/`match`/`for`/`let` are
91 native to the macro.
92- **`#[query_params]` needs `error = …`** to work with `?`; otherwise the error borrows
93 from `cx` and escapes the handler.
94- **Forms re-render on failure and redirect on success.** Redirecting after a
95 validation error throws away what was typed and hides the reason.
96- **Styling is Tailwind via Topcoat's build script**, with registry components copied
97 in rather than depended on ([0005](decisions/0005-tailwind-and-copied-components.md)).
98 Components reference theme tokens, never raw colours.
99
100### Milestone 3 — Repo model · done
101
102Repositories exist as records and as bare repos on disk, and they appear on the
103profile. Domain, both persistence adapters, `GitStorage` with `DiskGitStorage` behind
104it, `create_repo` / `view_repo` / `list_repos`, the `/{handle}/repos/new` form and
105`/{handle}/repos/{name}` page, the profile's Repositories section, and
106`/api/users/{handle}/repos`. How git is invoked is recorded in
107[0006](decisions/0006-git-binary-behind-narrow-ports.md).
108
109**Verified in a browser and by curl:** the owner creates a repo through the form, a
110bare repo appears at `{data_dir}/{handle}/{name}.git`, and it lists on the profile; a
111private repo is absent for a signed-out visitor on both the page and `/api`; an unknown
112handle 404s rather than answering `[]`. `git clone` does not work yet — Milestone 4.
113
114#### Decisions worth remembering
115
116- **`git init` on an existing repository exits 0 and re-initialises in silence.**
117 Measured, not assumed. So `AlreadyExists` has to be our own `path.exists()` check —
118 there is no exit code to key off. Refusing rather than adopting matters because a
119 directory with no matching row is an orphan from a crashed create, and re-initialising
120 it would resurface a private repository's objects under a fresh record.
121- **`git init` creates missing parent directories itself**, so there is no
122 `create_dir_all` before it. This was in the plan and the probe removed it.
123- **`--template=` takes a new bare repo from 18 files to 2.** The default seeds sixteen
124 `.sample` hooks. Timed at 15.2ms against 13.1ms across 20 runs — so the ~2ms is not
125 the reason; Steid installs its own hooks later and the samples would be noise to work
126 around.
127- **`--initial-branch=main` is explicit** so the host's `init.defaultBranch` cannot
128 decide it. This machine's git already says `main`, which is precisely why a drift
129 would go unnoticed — hence the test.
130- **`GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM` are pointed at `/dev/null`**, and the
131 five `GIT_*` variables that redirect object storage are removed from the child
132 environment. `GIT_DIR` was checked and does *not* override an explicit path argument,
133 but `GIT_OBJECT_DIRECTORY` does redirect where objects land, and the failure is
134 silent — the repository just looks empty.
135- **One `run_git` owns the invocation.** With a single caller this looks premature; it
136 is a private function rather than a public abstraction for that reason. The point is
137 that Milestone 4's `http-backend` spawn cannot quietly disagree about isolation.
138- **`tokio::process` and `tokio::fs`, never the `std` equivalents.** `init_bare` is not
139 hot — ~13ms, once per repository — but `remove_dir_all` on a repo with real history
140 walks every loose object and would stall a runtime worker. The performance that
141 matters is Milestone 4's per-request spawn, not this.
142- **`tempfile` for test fixtures, not `target/`.** Parallel-safe by construction and
143 self-cleaning on panic. Debris under `target/` would be actively harmful here, since
144 `init_bare` refuses a path that already exists.
145- **Membership is resolved once per listing, not once per row.** Obvious in hindsight;
146 the shape that invites the mistake is filtering inside a loop that can `await`.
147- **One empty state serves "no repositories" and "none you may see".** A distinct
148 message for the second — or any count — leaks that private repositories exist and how
149 many. Tested, because it is the kind of thing a later "helpful" tweak would undo.
150- **`redirect()` is a 307, and 307 preserves the method.** Post/redirect/get needs a
151 303, or the browser re-POSTs the form to its redirect target. Milestone 2's settings
152 form shipped with this and nothing caught it — every test passed, because the tests
153 are on the use case and the bug is in the reply. Found by following the redirect with
154 curl. The fix is a `StatusCode::SEE_OTHER` plus a `Location` pair inside `view!`,
155 wrapped as `web::context::location`, because `see_other()` is a response type and
156 `#[page]` must return a view for the layout to wrap the failure re-render.
157 `RedirectError::new` is private, so a 303 cannot be built as an error, and the
158 error-to-response path only downcasts topcoat's own error types — a custom one
159 becomes a 500.
160- **`#[page]` returns a view; `#[route]` returns a response.** That is the whole reason
161 the redirect is spelled awkwardly: a form handler needs both a redirect and a
162 full-page re-render, and only the view path gets the layout.
163- **Boolean HTML attributes take an explicit value in `view!`** — `required=(true)`, not
164 bare `required`, which fails to parse. `false` omits the attribute entirely, so
165 `selected=(bool)` on an `<option>` is correct rather than rendering `selected="false"`.
166- **`topcoat ui add select` needs the `icon-iconify` feature and a staged icon set.**
167 The chevron comes from `feather`, staged in `build.rs`. No new crates, but the build
168 fails with a clear message until the set is staged.
169- **`is_org_owner` moved to `application/authz.rs`** on its second caller. Owner-ness
170 gates the profile edit, repo creation, and later PATs and push; two copies of an
171 authorization predicate drift, and the direction they drift is open.
172- **The compensating transaction is safe to do by path.** `remove` after a failed save
173 can only ever delete what `init_bare` just created, because the loser of a concurrent
174 create never gets past `init_bare`. Non-obvious enough that it is commented in the
175 code as well as here.
176- **Compensation is best-effort.** If the removal also fails, the caller still gets the
177 error that started it — an orphaned directory is the documented failure mode, and
178 replacing the real error with the cleanup's error would hide the cause.
179- **`InMemoryGitStorage` enforces `AlreadyExists` too.** A permissive fake would let
180 `create_repo` pass while the real adapter refused. The fake mirroring the rule is the
181 point of having two implementations.
182- **`/api` resolves the handle without loading a profile.** `handle_param` split out
183 of `profile_for` so the repo listing route 404s on `list_repos`' own `None`. Routing
184 it through the profile would have made that `None` unreachable and left the page and
185 `/api` disagreeing about what an empty portfolio means — the exact duplication
186 `architecture.md` says to watch for.
187- **The `/api` listing is a bare array, not an envelope.** Matches `/api/users/{handle}`
188 returning a bare object. Pagination later means a wrapper and a breaking change; taken
189 knowingly, since personal-first means a handful of repositories and there are no
190 consumers yet.
191- **`Repository::description` stays.** Added unrequested and flagged; kept on review
192 because this milestone's own "Done when" puts repositories on the profile, which
193 makes it a consumer inside the milestone rather than speculation. Worth noting the
194 window that closed: the repositories migration had not yet been applied to the dev
195 database, so removing the column would have been a free in-place edit rather than a
196 second migration.
197
198### Milestone 4a — Clone over HTTP · done
199
200`git clone` works against a public repository, for anyone, with no credentials.
201`GitProtocolServer` (CGI-shaped) with `GitHttpBackend` behind it, the `serve_git` use
202case, and three routes under `/{handle}/repos/{name}.git/`. Bodies stream both
203directions.
204
205**Verified against a real client**, not only by unit test: an anonymous `git clone` of a
206201-ref repository returns 201 commits and 203 refs with `HEAD` matching the origin, on
207protocol v2 and on v0; a private repository answers 404 to an anonymous clone but 200 to
208its owner's browser session; `git push` is refused with 403; and unknown repo, unknown
209handle, missing `service`, an unknown service, a missing `.git` suffix, and a
210dumb-protocol object path all answer 404 while the repository page still answers 200.
211
212`git http-backend`'s contract, probed against git 2.50.1 by driving the CGI from a
213throwaway server and cloning through it. Everything below is measured, not read.
214
215#### The contract
216
217- **`HTTP_CONTENT_ENCODING`, not `CONTENT_ENCODING`.** CGI gives only `Content-Type` and
218 `Content-Length` unprefixed names; every other request header is `HTTP_`-prefixed, and
219 `http-backend` looks for the prefixed one. **This is the finding that would have cost a
220 day.** With the wrong name, `http-backend` hands the still-compressed body to
221 `upload-pack`, which dies with `bad line length character` and the client reports
222 `fatal: expected 'packfile'` — nothing names gzip, or the environment, anywhere in the
223 failure. And it only happens once a repository has enough refs for the client to bother
224 compressing: a one-ref test repo passes.
225- **`HTTP_GIT_PROTOCOL`** carries `version=2` through to `upload-pack`. Clones verified
226 on v2 and on `protocol.version=0`, both 201 commits, both gzipped.
227- **`GIT_PROJECT_ROOT` plus `PATH_INFO`**, where `PATH_INFO` is the on-disk path relative
228 to the root. Steid's URL and its storage layout differ — `/{handle}/repos/{name}.git/…`
229 against `{data_dir}/{handle}/{name}.git` — so the adapter rewrites the middle segment
230 out. `GIT_PROJECT_ROOT` is the data directory.
231- **`GIT_HTTP_EXPORT_ALL=1` is required.** Without it every repository answers `Status:
232 404 Not Found` and `Repository not exported`, unless a `git-daemon-export-ok` marker
233 file sits in the bare repo (confirmed: dropping that file in re-enables it).
234- **Headers are CRLF-terminated and end at `\r\n\r\n`.** No bare-LF variant was
235 observed, but the adapter should accept one rather than hang.
236- **`Status:` appears only on failure.** Its absence means 200, and it must be
237 translated, not forwarded as a header.
238- **`http-backend` sets its own `Content-Type` and cache headers** — `Expires: Fri, 01
239 Jan 1980`, `Pragma: no-cache`, `Cache-Control: no-cache, max-age=0, must-revalidate`.
240 Steid forwards them rather than inventing its own.
241
242#### Decisions worth remembering
243
244- **`GIT_HTTP_EXPORT_ALL`, never `git-daemon-export-ok`.** The marker file is git's own
245 visibility mechanism and it looks tempting, but visibility lives in the `repositories`
246 table and the use case is what enforces it. A marker file would be a second source of
247 truth for the same question, free to drift from the first, and the drift direction is
248 "private repository still clonable". Steid decides; git is told to stop asking.
249- **`receive-pack` is refused by default** — `Status: 403 Forbidden`, `Service not
250 enabled: 'receive-pack'`, without any configuration. Convenient for 4a, but the routes
251 still refuse writes explicitly rather than leaning on it: a default that helpfully
252 changes is not an authorization decision.
253- **A non-zero exit can arrive after the headers are already out.** The gzip failure
254 exited 1 having emitted a complete, successful-looking header block. So the exit code
255 cannot gate the response — by the time it is known, the status is sent. It belongs in
256 the log.
257- **A missing repository is `Status: 404` with exit 0.** Failure is reported in the
258 CGI stream, not the exit code, and the two disagree in both directions.
259- **The router is the allowlist.** Only `info/refs`, `git-upload-pack` and
260 `git-receive-pack` are routed. Handed any other path, `http-backend` serves
261 dumb-protocol object files straight off disk — a read of a repository nothing
262 authorized. Verified: `/…​.git/objects/info/packs` answers 404.
263- **The endpoint is named by the route, not parsed from the path.** Three routes, three
264 literal `GitEndpoint` values, and `serve_git` rebuilds `path_info` from the validated
265 handle and name. The string that decides authorization and the string handed to git
266 are therefore the same string.
267- **Existence is settled before permission.** A push to a repository the actor cannot
268 see answers 404, not 403 — a 403 would confirm a private repository by that name
269 exists. Costs nothing to get right at the start and is invisible to test later.
270- **`BufReader` is what makes the header/body split safe.** The reader keeps whatever it
271 read past the blank line, so handing the reader itself back as the response body
272 carries the already-buffered first bytes of the pack with it. Parsing headers into a
273 separate buffer and then streaming the rest would silently drop them.
274- **The child's exit code cannot gate the response.** A protocol failure exits non-zero
275 *after* a complete, successful-looking header block has been written. By the time the
276 status is known it has been sent, so the exit code goes to the log and nowhere else.
277- **Stderr must be drained, not merely piped.** An unread pipe fills and blocks the
278 backend mid-transfer. It is read in the same task that reaps the child.
279- **`body_limit` does not exist** in `topcoat-router` 0.5.0 — the warning carried from
280 [0001](decisions/0001-git-over-http-not-ssh.md) is stale. Bodies are read by the
281 handler with a caller-chosen limit via `to_bytes`, and the git routes take `Body`
282 unbuffered so no limit applies at all.
283- **`impl<B> IntoResponse for http::Response<B>`** means a handler can return its own
284 `http_body::Body` and Topcoat re-bodies it. That is what lets the pack stream without
285 a framework-specific body type.
286
287### Milestone 4b — Push and tokens · done
288
289`git push` works over HTTP for the owner, and a private repository is clonable by
290someone holding a token for it. `PersonalAccessToken` with both storage adapters,
291`issue_token` / `list_tokens` / `revoke_token` / `authenticate_token`, HTTP Basic on the
292git routes, and token management at `/{handle}/settings/tokens`. Decisions recorded in
293[0007](decisions/0007-tokens-over-http-basic.md).
294
295**Verified end to end**, issuing the token through the UI rather than seeding one:
296push of 201 refs to a public repo and to a private one; clone of the private repo
297returning 201 commits; anonymous clone of the public repo still open with no prompt;
298401 with `WWW-Authenticate: Basic` for a private repo, a **nonexistent** repo, an
299unknown handle and a push advertisement alike; a wrong token challenged rather than
300accepted; and after revoking, both clone and push answer 401 while the public repo stays
301open. The token is shown once and never again on reload, the revoke button disappears
302from the list, and the page is 403 for anyone else.
303
304#### Decisions worth remembering
305
306- **`http-backend` refuses `receive-pack` by default, and Steid authorizing the push is
307 not enough.** The symptom is a 403 that looks like Steid's own refusal but is git's:
308 `Service not enabled: 'receive-pack'`. It needs `-c http.receivepack=true` **before**
309 the subcommand. That flag is set from a `GitRequest` field the use case turns on only
310 after the authorization check passes, so git remains a second refusal behind Steid's
311 rather than being switched on wholesale — if the rules are ever wrong, git still says
312 no.
313- **The uniform 401 is what makes authenticated cloning possible at all.** A git client
314 offers a credential only after a 401, so the 4a behaviour of answering 404 for a
315 private repository made an authenticated private clone unreachable. Extending the 401
316 to repositories that do not exist is what keeps it from leaking which private names
317 are real.
318- **A bad credential falls through to anonymous rather than failing.** The caller then
319 gets the same challenge as someone who presented nothing and can try again, which is
320 also how a stale session cookie behaves.
321- **Basic accepts the token in the password field, or in the username with no password.**
322 Git puts it in the password; people paste it into the username. The alternative is an
323 authentication failure with nothing to explain it.
324- **Issuing a token deliberately does not redirect**, unlike every other form here. The
325 secret exists only in that response, and surviving a redirect would mean putting a live
326 credential in a URL — browser history, logs, referrers. Reloading issues a second
327 token, which is harmless and visible in the list.
328- **Revoking someone else's token is `NotFound`, not `Forbidden`.** That a token id
329 exists but belongs to another user is not a fact worth confirming.
330- **Tokens do not expire.** A credential pasted into a machine and forgotten is worth
331 less if it stops working silently; revocation is the control that matters.
332- **`in_memory.rs` keeps `mod tests` in the middle of the file**, like `sqlite.rs`.
333 Appending an implementation to the end lands it inside a later impl block.
334
335#### Measured, for Milestone 5
336
337Taken at the end of 4b, on a 201-commit repository, 50 runs averaged per command:
338
339| Command | Per call |
340|---|---|
341| `git rev-parse HEAD` | 11.2 ms |
342| `git ls-tree -l HEAD` | 11.7 ms |
343| `git cat-file -p HEAD:` | 11.6 ms |
344| `git log -20 --format=…` | 11.8 ms |
345| `git for-each-ref` | 14.4 ms |
346
347**The cost is starting git, not the query** — every command lands in the same band
348regardless of the work it does, matching the ~13ms `git init --bare` from Milestone 3.
349A three-call page is therefore ~35ms of pure overhead, and a per-file last-commit column
350at one call per entry would be ~230ms for twenty files. This is the input
351[0006](decisions/0006-git-binary-behind-narrow-ports.md) asked for before reconsidering
352`gix` on the read path.
353
354### Milestone 5 — Repo browsing · done
355
356A repository is readable on the web: the file tree at a revision, a file's contents with
357line numbers, and the commit log. `ObjectId` / `RefName` / `RepoPath` / `EntryKind` /
358`TreeEntry` / `CommitSummary` in the domain, a `GitQuery` port with `DiskGitQuery` and an
359in-memory fake behind it, `browse_repo` and `repo_log` read models, and pages at
360`/{handle}/repos/{name}`, `/tree/{rev}`, `/tree/{rev}/-/{path}`, `/log` and `/log/{rev}`.
361The read-path decision and its upgrade ladder are in
362[0006](decisions/0006-git-binary-behind-narrow-ports.md#amendment--20260829-the-milestone-5-read-path).
363
364**Verified against Steid's own repository, hosted on Steid.** A freshly created repo
365shows push instructions; after pushing 57 commits the page lists the tree; directories
366descend and breadcrumb back; `src/domain/session.rs` renders with line numbers and
367correct HTML escaping; the log shows 50 commits with author and relative time. Unknown
368revision, unknown path, `../etc/passwd`, unknown repo and unknown handle all 404, clone
369still works, and a private repository answers 404 to an anonymous visitor on **every**
370browse route while its owner sees it.
371
372#### Decisions worth remembering
373
374- **`cat-file --batch-check` with the spec on stdin is the lookup primitive.** It exits
375 **0** and prints `<spec> missing` for anything unresolvable, which is what makes "not
376 found" a *value read off stdout* rather than an exit code to interpret. The rule the
377 adapter follows: **a non-zero exit is always an error; absence is a marker in the
378 output** (`missing`, `ambiguous`, `dangling`, `notdir`). The obvious alternatives are
379 worse — `rev-parse --verify --quiet` returns 1 for an unknown ref but 128 for a missing
380 repository, and `ls-tree` pointed at a blob is a `fatal:` for what is, to a visitor, a
381 404. Passing the spec on **stdin** also means no revision or path can ever be read as a
382 flag, whatever validation upstream does or stops doing.
383- **`git log` in a repository with no commits is a fatal error, not empty output**, hence
384 resolving the revision first. An extra fork, in exchange for not reading meaning out of
385 a localised stderr string.
386- **`%s` is git's *subject*, not the first line** — with no blank line in the message it
387 joins the whole first paragraph with spaces. The adapter takes `.lines().next()` rather
388 than trusting that.
389- **Negative `%ct` exists** in imported histories and would panic `UNIX_EPOCH + Duration`.
390- **A symlink is indistinguishable from a file in the object store** — both are blobs — so
391 `read_blob` returns a symlink's target path as its content. Deliberate; making it
392 `Ok(None)` would cost an extra `ls-tree` of the parent.
393- **The empty repository is a first-class state, not an error.** Steid creates
394 repositories empty and Milestone 4 made it easy to have one never pushed to, so
395 `default_branch` returning `None` means "no commits" and the page offers the three
396 commands to push rather than a broken listing.
397- **`%2F` in a revision segment survives routing.** Topcoat matches on the raw path and
398 percent-decodes after, so a slashed branch works as a single segment — which is what
399 makes the `/-/` separator sufficient without a ref lookup.
400- **Three agents worked in parallel on disjoint files**, with the port, the fake and a
401 non-panicking stub adapter written first so neither branch could break the other's
402 build. The stub answering "nothing there" rather than `todo!()` is what let the pages
403 be developed and run before the adapter existed.
404
405### Milestone 5b — Deployable by anyone · in progress
406
407Distributed as a binary plus its `assets/` directory, installed by one command, behind
408Caddy for automatic HTTPS. `release.sh`, `install.sh`, `deploy/`, a `README.md`, and the
409operability the service needs: `/healthz`, `STEID_SETUP_TOKEN`, and rate limiting.
410
411#### Topcoat findings, both load-bearing
412
413- **`topcoat::start()` already handles SIGTERM.** `serve()` calls `serve_until(…,
414 shutdown_signal())`, and that selects on Ctrl+C *and* SIGTERM on Unix: it stops
415 accepting, drops the listener so a replacement process can bind the port, and drains
416 in-flight requests up to `shutdown_timeout`. So systemd stop/restart does not cut a
417 clone mid-pack, and **no wiring was needed** — the work was finding this out rather
418 than building it.
419- **The peer socket address is not reachable from a handler.** `internal_serve` discards
420 it at accept time (`let (stream, _remote) = accepted?;`) and never puts it on the
421 request extensions; the context exposes only parts, method, uri, headers and
422 extensions. **Any IP-based decision in Steid is therefore header-based by necessity**,
423 not by choice, and closing that would take an upstream change.
424
425#### Decisions worth remembering
426
427- **The rate limiter keys on the *rightmost* `X-Forwarded-For` entry.** A proxy
428 *appends* the address it accepted from, so the last entry is the one the nearest proxy
429 wrote and the only one a client cannot forge. The leftmost — what "the real client IP"
430 usually means — is exactly the attacker-controlled one. Two proxy hops collapse clients
431 onto the inner proxy's address, which is stricter, so being wrong that way is safe.
432- **A global cap backs the per-key one**, because without a peer address a forged key
433 cannot be disproved. It turns key forgery from a total bypass into a modest speed-up.
434 The cost is that a flood can lock the login form for a minute — a recoverable denial
435 against an unrecoverable guessed password.
436- **The limiter's map is bounded.** An unbounded map keyed by attacker-controlled values
437 is itself the denial of service; at the cap it sweeps expired entries and otherwise
438 falls through to the global window, which is stricter rather than permissive.
439- **glibc on bullseye, not static musl.** musl failed on `ring` (Debian's `musl-gcc`
440 rejects `-m64`), but the decisive argument is that **musl buys a dependency-free binary
441 and Steid hard-requires `git` on PATH** — the portability is unusable. Bullseye pins the
442 glibc floor at 2.31, covering Debian 11+ and Ubuntu 20.04+; bookworm would need 2.36
443 and silently exclude Ubuntu 22.04.
444- **The runtime binary links a TLS stack it never uses.** `ring` ← `rustls` ← `ureq` ←
445 Topcoat's `icon-iconify`/`tailwind`, whose `ureq` downloads the Tailwind CLI *at build
446 time*. Those features are on the normal dependency as well as the build one, so the
447 crypto ships too. Moving them would shrink the binary and drop an unused dependency
448 from the attack surface. Not attempted; it is also what made the musl build fail where
449 it did.
450- **`STEID_SETUP_TOKEN` does not undermine [0002](decisions/0002-first-run-claim-not-config-bootstrap.md).**
451 What that ADR refused to put in configuration was the owner's *password* — long-lived,
452 goes stale, ends up in a repository. This is the one-time claim secret the operator was
453 already copying out of a log line. It is validated for strength, never printed, and
454 ignored entirely once claimed.
455
456### Gap-filling while 5b was blocked on DNS · done
457
458Built while the domain transfer was in flight, so none of it belongs to a milestone.
459Three things: repository settings, browse usability, and README rendering — the last of
460which is Milestone 6's markdown pipeline arriving early because a repository page needed
461it.
462
463#### Repository settings — a hole, not a feature
464
465Repositories were **create-only**. No edit, no delete, and therefore **no way to
466un-publish something published by accident**. Now: description and visibility are
467editable, and a repository can be deleted behind a type-the-name confirmation.
468
469- **Delete writes the row first, then the directory best-effort.** An orphaned directory
470 only blocks reusing that name and is already `create_repo`'s documented failure mode;
471 an orphaned row is a repository that lists on the profile and 404s when clicked. The
472 visible failure is the worse one, so the ordering avoids it.
473- **Two different refusals, deliberately.** A repository the actor may not *see* is
474 `NotFound` (a 403 would confirm a private repo by that name exists); one they can see
475 but do not own is `Forbidden`, matching `create_repo`, because the resource is public
476 anyway. The page collapses both to 404.
477- **Renaming is still impossible, now with a comment saying why.** The bare repo lives at
478 `{data_dir}/{handle}/{name}.git`, so a rename is a directory move that breaks every
479 existing clone — and doing the row half only breaks them silently.
480
481#### Markdown — raw HTML is structurally impossible, not merely disabled
482
483- **`pulldown-cmark` does not sanitise URLs.** Its `escape_href` only percent-escapes, so
484 `javascript:` would have survived into a rendered README. The renderer uses a scheme
485 **allowlist** (`http`, `https`, `mailto`, `ftp`, `ftps`, `tel`) and strips ASCII control
486 characters *before* the check as well as on output, because browsers strip them too and
487 `java&#9;script:` is otherwise a live bypass.
488- **Adding the crate with `default-features = false` turned off its `html` feature**, so
489 `push_html` does not exist in this build. The renderer therefore walks the event stream
490 and writes every tag itself. Forced rather than chosen, and better: the emittable tag
491 set is exactly what the writer spells out, so raw HTML cannot pass through by
492 construction. `Event::Html` is written as *text*, so `<script>` is visible and inert
493 rather than silently vanishing.
494- **Escaping goes through `topcoat::view::HtmlContext`**, the same escaper `view!` uses —
495 not a hand-rolled one.
496- **Smart punctuation is off**: it rewrites `--flag` to an en dash, quietly corrupting CLI
497 flags in README prose.
498- Relative *links* are rewritten into tree URLs; relative *images* are deliberately left
499 alone, because a tree URL serves a page and rewriting would swap a 404 for a broken
500 image. Pointing them at `/raw/` is the obvious follow-up now that route exists.
501
502#### Browse — the switcher and raw files
503
504- **`for-each-ref` asks only for `%(refname)`.** `%(objecttype)` is `commit` for both a
505 branch and a lightweight tag, so the *namespace* is the only thing that answers
506 branch-versus-tag.
507- **Raw files are served as `application/octet-stream`, always** — never the file's own
508 type and never guessed from an extension — with `nosniff`, `Content-Disposition:
509 attachment` and `default-src 'none'; sandbox`. A repository-supplied `.html` or `.svg`
510 served as its real type on this origin is stored XSS against the viewer's session.
511 `text/plain` was rejected because browsers render it and have been talked into sniffing
512 it as HTML. The filename is repository content arriving in a header, so it is reduced to
513 `[A-Za-z0-9._-]` against header injection.
514- **The switcher costs one more fork (~13ms) on tree and log pages**, measured
515 interleaved against `ls-tree` to cancel out load — same band, confirming again that the
516 fork is the cost. It is not called on the repository page or an empty repository.
517
518#### Verified
519
520Against a running instance with a deliberately hostile README: **zero real `<script>` or
521`<img>` tags in the output**, the markup present once as escaped inert text, the
522`javascript:` link stripped of its `href` while `https://example.com` survived, the table
523rendered, raw bytes SHA-256 identical with the headers above, the switcher listing a
524branch and a tag, and the Settings link visible to the owner and absent for anonymous.
525
526### The profile page, rebuilt · done
527
528Flat navigation, per [ui.md](ui.md#the-profile-page). Tabs instead of stacked sections,
529one lead item distinguished by weight and space rather than size, hairlines instead of
530boxes, and a `/{handle}/repos` index for the tab to point at.
531
532#### Decisions worth remembering
533
534- **Existing rows were stamped with the migration time, not left at 0.** A repository
535 pushed to for months would otherwise read "updated 56 years ago" on the very page the
536 column exists to order. Wrong by a bounded amount and self-correcting on the first
537 push, against wrong forever and looking broken.
538- **The single-pin invariant lives in the use case, not in a constraint.** The rule is
539 "pinning this unpins that", and a unique index can only *refuse*, never unpin. The two
540 writes are not one transaction; a crash between them leaves nothing pinned, which is
541 the harmless direction.
542- **`updated_at` is touched on the authorized write path**, before the protocol is
543 reached — so a clone never moves it and neither does a refused push. It records that a
544 push was *authorized*, not that it succeeded; waiting for the subprocess would be a
545 much larger change for a small gain. A failed touch is logged and does not fail the
546 push.
547- **The lead is picked out of the listing**, not fetched separately, so the lead and the
548 list cannot disagree about which repository is pinned.
549- **Ordering changed from name to recency**, and two existing tests had been passing by
550 accident: everything created inside one second tied and fell back to the alphabet.
551
552#### Two Topcoat traps, both silent
553
554- **`let profile = …` in a module with `#[page] async fn profile` is a unit-struct
555 pattern, not a binding.** The page's name is a unit struct in module scope. The error
556 points at neither the page nor the shadowing.
557- **Forgetting `topcoat asset bundle` after `cargo build` produces a broken-looking
558 page, not an error.** New utility classes are simply absent, so gaps collapse and type
559 falls back to browser defaults — it reads as a design mistake. Cost one confused
560 screenshot.
561
562### Security review before going public · done
563
564Attempted with three blind reviewer agents; all three died to the machine sleeping, so
565the review was done directly instead. **Findings were verified by attacking a running
566instance, not by reading comments** — the codebase argues confidently for its own
567safety and those arguments are exactly what needed testing.
568
569#### What held, demonstrated rather than assumed
570
571| Probe | Result |
572|---|---|
573| Private repo: page, tree, log, settings, raw, `/api` | 404 on every surface; `/api` does not leak the name |
574| `rev=--upload-pack=touch /tmp/pwned` and five other injections | 404, no execution |
575| Four path-traversal spellings incl. `%2e%2e` and `....//` | 404 |
576| Login brute force | 10 attempts then 429 |
577| Error bodies | bare "not found", nothing internal |
578| Raw `.html` and `.svg` from a repo | `octet-stream` + `nosniff` + `attachment` + sandbox CSP |
579| Filename `ev"il; drop.txt` in a header | sanitised to `ev_il__drop.txt`, no header injection |
580| `<script>` in a viewed file | escaped, zero live tags |
581
582**The production cookie is `__Host-session; HttpOnly; Secure; SameSite=Lax; Path=/`** —
583the strictest cookie form available, and it forbids a `Domain` attribute so a
584compromised subdomain cannot inject one.
585
586**A near-miss worth recording:** the first check appeared to show production serving
587`steid-dev-session` with no `Secure`. It was a testing error — **`dotenvy` reads `.env`
588from the working directory**, and running the binary from the repo root picked up the
589gitignored dev `.env`. Verify from a neutral directory, and note the same trap applies
590to the deployment: the systemd unit's `WorkingDirectory=/opt/steid` means a stray `.env`
591there would silently override the environment file.
592
593#### The one real finding: no security headers on HTML
594
595Fixed with a layer, deliberately a layer rather than a per-handler concern — a page
596added without them would simply not have them and nothing would fail.
597
598- **Steid renders no JavaScript at all**, not a script tag on any page, so
599 `default-src 'none'` is a policy the product can genuinely keep. That is far stricter
600 than a typical CSP and worth defending: if a feature ever needs script, weakening it
601 should be a deliberate decision.
602- `'unsafe-inline'` is granted for **styles only**, and only because Topcoat's icon
603 macro emits `style="vertical-align: -0.125em"`. A framework constraint, not a choice.
604- **`Referrer-Policy: no-referrer`**, not the usual `strict-origin-when-cross-origin`: a
605 private repository's URL contains its name, and a README may link anywhere, so a
606 referrer would hand that name to whatever the visitor clicked.
607- Headers are set with `entry().or_insert()`, never `insert()` — the raw endpoint's own
608 stricter `default-src 'none'; sandbox` must not be relaxed by a blanket overwrite.
609 Verified that it survives.
610
611#### Not covered, and why
612
613- **The member-but-not-owner path could not be demonstrated.** There is no registration,
614 so a second account cannot exist until Milestone 7. It is covered by use-case tests and
615 nothing else — re-verify for real the moment a second account is possible.
616- **No automated test asserts the headers.** There is no HTTP-level test harness in this
617 project, so a regression here would be silent. That is a gap, not a decision.
618- No dependency audit (`cargo audit` was not run), and no review of the deployment
619 scripts beyond the earlier container dry-run.
620
621---
622
623## Reference: what attempt #2 proved
624
625Not this repo's progress. This is a catalogue of what was built and **verified working**
626in `steid-backup-2026-07-31`, so the rebuild can crib rather than rediscover.
627
628Final state: single crate, ~4,200 LOC, 60 passing tests, five milestones.
629
630### Identity
631
632Domain model (User, Organization, Membership, Actor, Role), `Email` and
633`PasswordHash` value objects, typed IDs, `DomainError`, four repository ports with
634in-memory and SQLite implementations each. `RegistrationPolicy` (Personal / Invite /
635Open) driving which routes exist. Argon2 hashing behind a `PasswordHasher` port with a
636stub for tests. Use cases: `bootstrap_owner`, `register_user`, `login`, `create_invite`.
637Signed-cookie sessions via an `AuthUser` extractor.
638
639**Gotcha:** organizations must be saved before users — the FK runs that direction.
640Both `bootstrap_owner` and `register_user` had to be fixed for this.
641
642### Repo model
643
644`Repository` entity, `RepoId`, `Visibility` (Public/Private), `RepoRepository` port,
645migration `006_create_repositories.sql`. `create_repo` use case validates the name,
646rejects duplicates, and initialises the bare repo on disk in the same call. Bare repos
647live at `{data_dir}/{org}/{repo}.git`, `data_dir` defaulting to `./data`. Repos are
648created empty, no initial commit, like GitHub.
649
650### Git over SSH
651
652`GitStorage` port (`init_bare`, `repo_path`) with `DiskGitStorage` shelling out to
653`git init --bare`. `GitProtocolServer` port (`upload_pack`, `receive_pack`) with
654`GitBinary` spawning `git upload-pack` / `git receive-pack` via
655`tokio::process::Command` and pumping stdio with `tokio::io::copy`.
656
657`serve_clone` and `serve_push` use cases enforce visibility and actor checks **before
658any protocol byte flows** — that ordering is the whole point of putting them in the
659application layer.
660
661#### SSH channel bridging
662
663The fiddly part, and worth re-reading if SSH ever returns as a transport
664(milestone 8+ — [0001](decisions/0001-git-over-http-not-ssh.md) chose HTTP):
665
666- Store `Channel<Msg>` per `ChannelId` in the handler's map on `channel_open_session`
667- On `exec_request`, take the channel, split it with `into_stream()` +
668 `tokio::io::split`
669- Take stderr via `make_writer_ext(Some(1))` **before** `into_stream()` — that call
670 consumes the channel, so the order is not optional
671- No `data()` or `channel_eof()` handlers needed once the streams are split
672
673An earlier iteration used mpsc channels, custom `ChannelReader`/`ChannelWriter`, and a
674`spawn_blocking` thread. All of it was deleted and the result was simpler.
675
676### SSH key auth and authorization
677
678`SshKey { id, user_id, name, fingerprint, openssh }` + port, migration
679`007_create_ssh_keys.sql` (`fingerprint` UNIQUE). Fingerprints are SHA256 via
680`russh::keys::ssh_key::PublicKey::fingerprint(HashAlg::Sha256)`, stored as `SHA256:…`.
681`add_ssh_key` parses the openssh blob, dedupes on fingerprint, and re-encodes to a
682canonical form before storing.
683
684`auth_none` rejects. `auth_publickey` fingerprints the offered key, looks it up, and
685on a match stores `user_id` on the handler; `exec_request` builds the real `Actor`
686from it.
687
688Authorization rules as shipped:
689
690| Operation | Requirement |
691|---|---|
692| Clone, public repo | open |
693| Clone, private repo | any membership in the repo's org |
694| Push | `Role::Owner` membership in the repo's org |
695
696Web UI at `/{owner}/keys` — owner-only, lists fingerprints, accepts openssh via
697textarea, revokes per-row.
698
699**Verified end-to-end:** clone with an unregistered key → `Permission denied` (exit
700128); register via web UI → clone and push both succeed; second unregistered key →
701rejected at auth; revoke via web UI → subsequent clone rejected at auth.
702
703### Security notes
704
705Attempt #2 ran with a **named, deliberate backdoor** between milestones: SSH accepted
706any connection and passed a placeholder `Actor` (`UserId("ssh-anonymous")`), leaving
707push open to anyone who could reach the port. It was recorded with an explicit
708tightening point (`ssh.rs::exec_request`) and a closing milestone, and it did close.
709
710That practice is worth keeping. When this attempt opens a hole to make progress, name
711it, name the line that closes it, and name the milestone.
712
713### Never built
714
715Repo browsing (tree/blob/log), HTTP smart protocol, personal access tokens, flash
716messages, issues, PRs, blogs, pages, project showcases. Milestones 5–8 in
717[ROADMAP.md](ROADMAP.md) are all greenfield.