steid
Sign in

@jamesgill /

steid

steid/plans/progress.md
22.8 KBCode·Blame·Raw
1# Progress
2
3## This attempt (#3, Topcoat)
4
5190 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---
288
289## Reference: what attempt #2 proved
290
291Not this repo's progress. This is a catalogue of what was built and **verified working**
292in `steid-backup-2026-07-31`, so the rebuild can crib rather than rediscover.
293
294Final state: single crate, ~4,200 LOC, 60 passing tests, five milestones.
295
296### Identity
297
298Domain model (User, Organization, Membership, Actor, Role), `Email` and
299`PasswordHash` value objects, typed IDs, `DomainError`, four repository ports with
300in-memory and SQLite implementations each. `RegistrationPolicy` (Personal / Invite /
301Open) driving which routes exist. Argon2 hashing behind a `PasswordHasher` port with a
302stub for tests. Use cases: `bootstrap_owner`, `register_user`, `login`, `create_invite`.
303Signed-cookie sessions via an `AuthUser` extractor.
304
305**Gotcha:** organizations must be saved before users — the FK runs that direction.
306Both `bootstrap_owner` and `register_user` had to be fixed for this.
307
308### Repo model
309
310`Repository` entity, `RepoId`, `Visibility` (Public/Private), `RepoRepository` port,
311migration `006_create_repositories.sql`. `create_repo` use case validates the name,
312rejects duplicates, and initialises the bare repo on disk in the same call. Bare repos
313live at `{data_dir}/{org}/{repo}.git`, `data_dir` defaulting to `./data`. Repos are
314created empty, no initial commit, like GitHub.
315
316### Git over SSH
317
318`GitStorage` port (`init_bare`, `repo_path`) with `DiskGitStorage` shelling out to
319`git init --bare`. `GitProtocolServer` port (`upload_pack`, `receive_pack`) with
320`GitBinary` spawning `git upload-pack` / `git receive-pack` via
321`tokio::process::Command` and pumping stdio with `tokio::io::copy`.
322
323`serve_clone` and `serve_push` use cases enforce visibility and actor checks **before
324any protocol byte flows** — that ordering is the whole point of putting them in the
325application layer.
326
327#### SSH channel bridging
328
329The fiddly part, and worth re-reading if SSH ever returns as a transport
330(milestone 8+ — [0001](decisions/0001-git-over-http-not-ssh.md) chose HTTP):
331
332- Store `Channel<Msg>` per `ChannelId` in the handler's map on `channel_open_session`
333- On `exec_request`, take the channel, split it with `into_stream()` +
334 `tokio::io::split`
335- Take stderr via `make_writer_ext(Some(1))` **before** `into_stream()` — that call
336 consumes the channel, so the order is not optional
337- No `data()` or `channel_eof()` handlers needed once the streams are split
338
339An earlier iteration used mpsc channels, custom `ChannelReader`/`ChannelWriter`, and a
340`spawn_blocking` thread. All of it was deleted and the result was simpler.
341
342### SSH key auth and authorization
343
344`SshKey { id, user_id, name, fingerprint, openssh }` + port, migration
345`007_create_ssh_keys.sql` (`fingerprint` UNIQUE). Fingerprints are SHA256 via
346`russh::keys::ssh_key::PublicKey::fingerprint(HashAlg::Sha256)`, stored as `SHA256:…`.
347`add_ssh_key` parses the openssh blob, dedupes on fingerprint, and re-encodes to a
348canonical form before storing.
349
350`auth_none` rejects. `auth_publickey` fingerprints the offered key, looks it up, and
351on a match stores `user_id` on the handler; `exec_request` builds the real `Actor`
352from it.
353
354Authorization rules as shipped:
355
356| Operation | Requirement |
357|---|---|
358| Clone, public repo | open |
359| Clone, private repo | any membership in the repo's org |
360| Push | `Role::Owner` membership in the repo's org |
361
362Web UI at `/{owner}/keys` — owner-only, lists fingerprints, accepts openssh via
363textarea, revokes per-row.
364
365**Verified end-to-end:** clone with an unregistered key → `Permission denied` (exit
366128); register via web UI → clone and push both succeed; second unregistered key →
367rejected at auth; revoke via web UI → subsequent clone rejected at auth.
368
369### Security notes
370
371Attempt #2 ran with a **named, deliberate backdoor** between milestones: SSH accepted
372any connection and passed a placeholder `Actor` (`UserId("ssh-anonymous")`), leaving
373push open to anyone who could reach the port. It was recorded with an explicit
374tightening point (`ssh.rs::exec_request`) and a closing milestone, and it did close.
375
376That practice is worth keeping. When this attempt opens a hole to make progress, name
377it, name the line that closes it, and name the milestone.
378
379### Never built
380
381Repo browsing (tree/blob/log), HTTP smart protocol, personal access tokens, flash
382messages, issues, PRs, blogs, pages, project showcases. Milestones 5–8 in
383[ROADMAP.md](ROADMAP.md) are all greenfield.