From 5a6817d6706b08f2d855406d26805daefc3cb0dc Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 10:06:34 +0300 Subject: [PATCH 01/16] docs(forge): F2 spec decisions (default-branch authority, capacity reservation, new_branch) and FINDINGS summary From the #297 follow-ups that belong before F2/F3: the row is the default-branch authority, admission reserves capacity before receive-pack and answers 507, contents writes with new_branch apply the policy to the target branch, and the FINDINGS summary line now says 24/24 and 2/24. --- crates/forge/conformance/FINDINGS.md | 4 ++-- .../specs/2026-10-22-p38-forge-design.md | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/forge/conformance/FINDINGS.md b/crates/forge/conformance/FINDINGS.md index 73d47505..3b208df7 100644 --- a/crates/forge/conformance/FINDINGS.md +++ b/crates/forge/conformance/FINDINGS.md @@ -5,8 +5,8 @@ Recorded 2026-10-22 against `gitea/gitea:1.22.6-rootless` and `1.26.2-rootless` letters match `fixtures//-*.json`. Every statement below was observed, not read from source. "Both" means both versions answered the same. -Each version passes its own fixtures (22/22). Checking the 1.22.6 fixtures against 1.26.2 passes 3/22 -(I, Q, V); every failure is a difference listed in the last table. +Each version passes its own fixtures (24/24). Checking the 1.22.6 fixtures against 1.26.2 passes only 2/24; +every failure is a difference listed in the last table. ## Headline answers diff --git a/docs/superpowers/specs/2026-10-22-p38-forge-design.md b/docs/superpowers/specs/2026-10-22-p38-forge-design.md index 0185cacb..26358585 100644 --- a/docs/superpowers/specs/2026-10-22-p38-forge-design.md +++ b/docs/superpowers/specs/2026-10-22-p38-forge-design.md @@ -326,8 +326,13 @@ line (`Status:` and headers), then the body is streamed. lock. - **Admission control** before any git process: global and per-repo limits on concurrent git children, per-process wall-clock timeout, per-process - `RLIMIT_CPU`/`RLIMIT_FSIZE`, a free-space reserve checked before accepting a - push, and a per-owner byte quota (default off). `receive.maxInputSize` bounds + `RLIMIT_CPU`/`RLIMIT_FSIZE`, a free-space **reservation** (not a bare check) and a + per-owner byte quota (default off). Before spawning `receive-pack`, admission + reserves `min(Content-Length, --max-pack-size)` (the pack cap when the body is + chunked) against free space minus the configured reserve, and against the owner + quota; it releases the reservation when the push ends and answers 507 `Insufficient + Storage` when it cannot reserve, so concurrent pushes cannot each pass a check that + together overcommit the disk. `receive.maxInputSize` bounds the incoming pack only, so expansion is bounded by the quota and the disk reserve; the container applies cgroup memory and PID limits. - **Process ownership:** each child runs in its own process group; cancellation, @@ -350,7 +355,10 @@ Minimum `git` 2.40 in tests; the container pins a current release treated as literals with NUL-delimited output (`-z`). Output is size-capped. - **Reads:** `for-each-ref`, `rev-parse`, `log`/`rev-list` with a fixed `-z` format, `cat-file`, `ls-tree -r -z`, `diff-tree`, `archive --format=zip|tar.gz`. -- **Contents write** (POST/PUT): validate the path (reject empty, `.`, `..`, +- **Contents write** (POST/PUT): with `new_branch`, the protection policy applies to the + **target** branch (a new branch has no rule), creating it from a protected base is + allowed (recorded: 201 with `new_branch` where a write to the protected branch is + 403), and the base ref is read once and used as the CAS old value. Validate the path (reject empty, `.`, `..`, `.git` and its NTFS/HFS aliases, absolute paths; mode 100644 only), `hash-object -w`, a temp `GIT_INDEX_FILE` (`read-tree`, `update-index --cacheinfo`, `write-tree`), `commit-tree`, then `update-ref ` (zero old id @@ -465,8 +473,9 @@ unknown sha is 404 (1.26.2). Each has an entry in `departures.json`. removal) is removed; and a `deleting` row with neither directory is deleted. A repository name is reusable as soon as its row is gone, because directories are addressed by immutable id. -- **Default branch:** PATCH updates the row and bare `HEAD` together; the - reconciler repairs a mismatch. A repo is `empty` when `HEAD` does not resolve +- **Default branch:** PATCH writes bare `HEAD` first, then the row; on a mismatch at + startup **the row is the authority** and the reconciler resets `HEAD` from it. A first + push to a branch other than the default leaves `empty: true` (recorded). A repo is `empty` when `HEAD` does not resolve (computed live). **Repository names** follow the recorded Gitea rules: pattern `[A-Za-z0-9._-]`, at most 100 characters, case-insensitive uniqueness per owner (a database index on the lower-cased name), `.` and `..` reserved, and the From 801772742f5d9314f8aae16ce83eaeff5700461f Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:10:12 +0300 Subject: [PATCH 02/16] docs(forge): F2a plan (repos API, lifecycle, collaborators, branches) --- .../plans/2026-10-23-p38-f2a-repos-api.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md diff --git a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md new file mode 100644 index 00000000..0a81cfdd --- /dev/null +++ b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md @@ -0,0 +1,164 @@ +# P38 F2a: repositories API, lifecycle, collaborators, branches (plan) + +Slice F2 of the sc-forge spec is split in two PRs by agreement: **F2a** (this plan: repo +metadata and API, bare-repo lifecycle with reconcilers, collaborators, branch reads, the +`git` runner they need) and **F2b** (smart-HTTP transport through `git http-backend`, hooks, +ref-namespace policy, admission control, process groups, threat-model pass #1; planned +after F2a merges). Spec: `docs/superpowers/specs/2026-10-22-p38-forge-design.md` +("Repository lifecycle", "Server-side Git operations", "Authentication", Appendix A). + +## Scope + +Routes (all under `/api/v1`, one row each in `routes::ROUTES` with class, scope, actor): + +| Route | Scope | Actor / access | +|---|---|---| +| `POST /orgs/{o}/repos` | write:organization | org owner or site admin (row exists, F1) | +| `POST /user/repos`, `GET /user/repos` | write:user / read:user | caller owns | +| `GET /orgs/{o}/repos` | read:organization | org visible to caller; lists repos the caller can read | +| `GET /repos/{o}/{r}` | read:repository | read | +| `PATCH /repos/{o}/{r}` | write:repository | admin on the repo | +| `DELETE /repos/{o}/{r}` | write:repository | admin on the repo | +| `GET /repos/search` | read:repository | filtered by caller access | +| `GET /repos/{o}/{r}/collaborators`, `.../{u}/permission` | read:repository | admin | +| `PUT\|DELETE /repos/{o}/{r}/collaborators/{u}` | write:repository | admin | +| `GET /repos/{o}/{r}/branches`, `.../branches/{b}` | read:repository | read | +| `DELETE /repos/{o}/{r}/branches/{b}` | write:repository | write; default branch refused (protection arrives with F4's policy function) | + +Out of F2a (later slices): git transport (F2b), tags/refs/commits/trees/contents/archive (F3), +pulls/statuses/merge/protection (F4). Teams and web UI stay out of scope. + +## Data model (migration 2) + +```sql +CREATE TABLE repos ( + id INTEGER PRIMARY KEY AUTOINCREMENT, -- immutable; the directory name + owner_id INTEGER NOT NULL REFERENCES users(id), + name TEXT NOT NULL, lower_name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', website TEXT NOT NULL DEFAULT '', + default_branch TEXT NOT NULL, + state TEXT NOT NULL CHECK (state IN ('creating','live','deleting')), + created_unix INTEGER NOT NULL, updated_unix INTEGER NOT NULL, + UNIQUE (owner_id, lower_name) -- also covers creating/deleting rows +); +CREATE TABLE collaborators ( + repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + mode TEXT NOT NULL CHECK (mode IN ('read','write','admin')), + PRIMARY KEY (repo_id, user_id)); +``` + +`private` is not stored: every repo is private (spec: no anonymous read; `private:false` +accepted and answered `true`). A `creating`/`deleting` repo is invisible to every API route +(404) and to git (F2b). + +## Access model (one function) + +`access(conn, user, repo) -> Perm {None, Read, Write, Admin}`: site admin = Admin; the +owning user = Admin; an org's owners = Admin; else the collaborator mode; else None. Org +membership alone grants nothing. Every repo route resolves the repo **and** the permission in +`dispatch` (new `Actor::Repo{owner, repo, min}`), before any handler runs: + +- token with `public-only` -> 403 `token scope is limited to public repos` (recorded R#1), + before any existence information; +- missing, not `live`, or `Perm::None` -> the same 404 `The target couldn't be found.` + (recorded: foreign token gets what a missing repo gets); +- visible but below `min` -> 403 (text pinned from the fixtures when a scenario records one; + placeholder `message_contains` until then, tracked in #297). + +Search and list endpoints filter by the same function, in SQL where possible +(`owner_id = ?`, `EXISTS collaborator`, org-owner join, admin = all) so counts and +`X-Total-Count` are exact. + +## Git runner (`gitproc.rs`) + +The one place a `git` child is spawned (F2b adds the CGI path on the same primitives): +`env_clear`, fixed `PATH`, `HOME=/tmp/home`, `GIT_CONFIG_NOSYSTEM`, `GIT_CONFIG_GLOBAL=/dev/null`, +`GIT_NO_REPLACE_OBJECTS=1`, `GIT_TERMINAL_PROMPT=0`, per-spawn config through +`GIT_CONFIG_COUNT` (`core.hooksPath=/hooks`, `core.fsync=all`, `core.fsyncMethod=fsync`, +`gc.auto=0`, `core.useReplaceRefs=false`, `receive.fsckObjects=true`), explicit author and +committer identity from the acting user for commit-producing spawns, own process group +(`CommandExt::process_group(0)`), wall-clock timeout that kills the group (`libc::killpg`) +and reaps it, stdout and stderr capped and drained concurrently, a global semaphore of +concurrent children. Inputs: `repo_dir` is always built from the integer id; refs and +revisions go through `rev-parse --verify --end-of-options`; everything else is a literal. + +Used in F2a for: `init --bare --initial-branch= --template=`, `symbolic-ref HEAD`, +`for-each-ref -z`, `rev-parse --verify`, `hash-object -w`, `mktree`/`commit-tree` (auto-init), +`update-ref -d` (branch delete), `log -1 -z` (branch object). + +## Lifecycle (journaled, crash-safe) + +**Create**: insert row `creating` (unique violation -> 409 `The repository with the same name +already exists.`) -> `git init --bare` into `repos/.git` -> optional auto-init commit +(README.md `# \n`, message `Initial commit`, author = creating user) via plumbing, ref +`refs/heads/` -> fsync the repo dir and `repos/` -> set row `live` -> 201. +**Delete**: row `deleting` -> (F2b: terminate registered children) -> rename the directory into +`trash/.git` -> fsync both parents -> delete the row (collaborators cascade) -> remove the +trash entry. **PATCH default_branch**: branch must exist (422 otherwise: departure), write bare +`HEAD` first, then the row; the row is the authority on mismatch. + +**Reconciler** (`lifecycle::reconcile`, run at `serve` start and callable from tests), all by +the spec's recovery table: a `creating` row is completed if its directory is a valid bare repo +with the expected `HEAD`, else its directory is removed and the row deleted; a directory in +`repos/` with no live/creating row is removed; `deleting` + directory present -> resume at the +rename; `deleting` + trash present -> finish by deleting the row; trash with no row -> removed; +`deleting` with neither -> row deleted; a `live` row whose `HEAD` differs from `default_branch` +-> `symbolic-ref` repair; a `live` row whose directory is missing -> logged and left +(never silently dropped). + +## Names (extend `names.rs`) + +Repo names: `[A-Za-z0-9._-]`, at most 100 chars, `.` and `..` reserved, suffixes `.git`, +`.wiki`, `.rss`, `.atom` rejected with Gitea's texts (`name is reserved [name: .]`, +`name pattern is not allowed [pattern: *.git]`, `[Name]: AlphaDashDot`, `[Name]: MaxSize`); +leading `.`/`-`, `a..b`, and `api`/`admin`/... accepted (repos live under their id). +Case-insensitive uniqueness per owner via `lower_name`. Lookups are case-insensitive for owner +and name (recorded M#3-5). + +## Repo JSON + +One `repo_json(app, repo, owner, perm, empty)` builds the ~70-key object recorded in C#0 +(constants for the Gitea feature flags; `private`, `permissions` {admin,push,pull} from +`Perm`, `empty` from `rev-parse --verify HEAD` live, `default_branch`, URLs from the +normalized base, `ssh_url` as `ssh:////.git`, `size` 0, `watchers_count` 1). +Search wraps it as `{"ok":true,"data":[...]}` with `X-Total-Count`; user/org repo lists are +bare arrays. Branch object per E/D fixtures. + +## Tests (written first) + +1. Unit: names, access matrix (every role x repo kind), repo JSON key set against the C#0 + fixture keys, reconciler cases with injected crash points (one test per row of the recovery + table, on a temp data dir with real `git`). +2. Rust e2e over a raw socket (existing harness): scenario R complete, A/B/X up to their next + later-slice call, C#0-26 and C#34-38 (everything without a git transport step), D#0-6/12, + K's API half, L, M#0-22 and #41-44/49-51 minus rename departures, N#0-2/9-11; each asserts the + recorded status, headers, key set and texts, plus negative tests: foreign token == missing + repo, `public-only` token, read collaborator cannot PATCH/DELETE/PUT collaborator, org + non-owner cannot create, duplicate case-insensitive name, delete then recreate gets a new + id and directory, a deleting repo is invisible. +3. Conformance: `slice-pending.json` updated (R drops out; A/B/X stop at their F3/F4 setup + call); `check` gains git-command comparison (#297) because git steps now run for real in C. +4. Real `git` is required in CI (already on the runners); the minimum is the spec's 2.40. + +## Order of work + +1. migration 2 + store functions + `access` + names (tests first) +2. `gitproc.rs` (tests: env scrub, timeout kills group, output cap, semaphore) +3. lifecycle create/delete/reconcile (crash-point tests) +4. handlers + route rows + `Actor::Repo` in dispatch +5. conformance updates, `--pending` shrink, docs (spec "as built", README, ROADMAP) +6. independent review (Opus + Sol) of the final diff; fix; PR + +## Risks and open points + +- `Actor::Repo` puts repo resolution in `dispatch`; a handler that takes `{r}` without it + must not compile into the table (test enumerates rows with `{r}` in the pattern). +- Repo-name case: store and compare lower-cased; the displayed `name` keeps the stored case. +- `git init --bare` inherits the host's templates unless `--template=` is empty; the runner + always passes it, and a test asserts the new repo has no `hooks/*.sample` and no config + beyond the minimum. +- fsync durability is asserted by code review plus the crash-point tests, not by power-loss + tests (F5). +- Blocking work: handlers already run on blocking threads; the git runner is synchronous in + F2a (F2b adds async streaming for the CGI). From 5dee242ae10b93473adfeab68edc1168837fd85e Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:19:28 +0300 Subject: [PATCH 03/16] docs(forge): revise F2a plan after Opus and Sol review Public-only listings are empty 200s, listing counts are the visible count, rename is an explicit 422, interrupted creates are always rolled back, unknown directories move to orphans/ instead of being deleted, a data-dir lock and per-repo mutex serialize lifecycle operations, the runner always passes --git-dir, branch names are validated before git, the gate is restated to match how --pending works, and collaborator authority is decided. --- .../plans/2026-10-23-p38-f2a-repos-api.md | 196 ++++++++++++------ 1 file changed, 132 insertions(+), 64 deletions(-) diff --git a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md index 0a81cfdd..4fa1a10b 100644 --- a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md +++ b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md @@ -14,16 +14,16 @@ Routes (all under `/api/v1`, one row each in `routes::ROUTES` with class, scope, | Route | Scope | Actor / access | |---|---|---| | `POST /orgs/{o}/repos` | write:organization | org owner or site admin (row exists, F1) | -| `POST /user/repos`, `GET /user/repos` | write:user / read:user | caller owns | -| `GET /orgs/{o}/repos` | read:organization | org visible to caller; lists repos the caller can read | +| `POST /user/repos`, `GET /user/repos` | write:user / read:user | create: caller owns; list: every repo the caller can read (owned, collaborator, org-owned) | +| `GET /orgs/{o}/repos` | read:organization | 200 for any existing org, listing only repos the caller can read (see "Listings") | | `GET /repos/{o}/{r}` | read:repository | read | | `PATCH /repos/{o}/{r}` | write:repository | admin on the repo | -| `DELETE /repos/{o}/{r}` | write:repository | admin on the repo | +| `DELETE /repos/{o}/{r}` | write:repository | owner-level: owning user, org owner, site admin (not an admin collaborator, as Gitea's `reqOwner`) | | `GET /repos/search` | read:repository | filtered by caller access | | `GET /repos/{o}/{r}/collaborators`, `.../{u}/permission` | read:repository | admin | | `PUT\|DELETE /repos/{o}/{r}/collaborators/{u}` | write:repository | admin | -| `GET /repos/{o}/{r}/branches`, `.../branches/{b}` | read:repository | read | -| `DELETE /repos/{o}/{r}/branches/{b}` | write:repository | write; default branch refused (protection arrives with F4's policy function) | +| `GET /repos/{o}/{r}/branches`, `.../branches/{b..}` | read:repository | read (`{b..}` is a tail capture: branch names contain `/`, recorded E#5) | +| `DELETE /repos/{o}/{r}/branches/{b..}` | write:repository | write; the default branch is refused with 403 `can not delete default branch` (recorded Q#15); protection arrives with F4's policy function | Out of F2a (later slices): git transport (F2b), tags/refs/commits/trees/contents/archive (F3), pulls/statuses/merge/protection (F4). Teams and web UI stay out of scope. @@ -52,60 +52,110 @@ CREATE TABLE collaborators ( accepted and answered `true`). A `creating`/`deleting` repo is invisible to every API route (404) and to git (F2b). -## Access model (one function) - -`access(conn, user, repo) -> Perm {None, Read, Write, Admin}`: site admin = Admin; the -owning user = Admin; an org's owners = Admin; else the collaborator mode; else None. Org -membership alone grants nothing. Every repo route resolves the repo **and** the permission in -`dispatch` (new `Actor::Repo{owner, repo, min}`), before any handler runs: - -- token with `public-only` -> 403 `token scope is limited to public repos` (recorded R#1), - before any existence information; -- missing, not `live`, or `Perm::None` -> the same 404 `The target couldn't be found.` - (recorded: foreign token gets what a missing repo gets); -- visible but below `min` -> 403 (text pinned from the fixtures when a scenario records one; - placeholder `message_contains` until then, tracked in #297). - -Search and list endpoints filter by the same function, in SQL where possible -(`owner_id = ?`, `EXISTS collaborator`, org-owner join, admin = all) so counts and -`X-Total-Count` are exact. +## Access model (one function, shared with F2b) + +`access::resolve(conn, user, owner, name) -> Option` is the only +place that turns (caller, repo) into a permission; the F2b git path calls the same function, +so API and git cannot drift. `perm` is `Read < Write < Admin`: site admin = Admin; the owning +user, or an owner of the owning org = Admin and `is_owner`; else the collaborator mode; else +none. Org membership alone grants nothing. Repo routes resolve in `dispatch` through a new +`Actor::Repo{owner, repo, min, owner_only}` row field, in this order, before any handler: + +1. authentication, then scope (F1 order, unchanged); +2. token with `public-only` -> 403 `token scope is limited to public repos` (recorded R#1), + before any existence information (individual repo routes only; listings below); +3. repo missing, not `live`, or no access -> the identical 404 (`The target couldn't be + found.`; foreign token gets what a missing repo gets); +4. visible but below `min`, or `owner_only` without `is_owner` -> 403. The text is pinned from + a recording where one exists and otherwise a plain `You do not have permission to perform + this action`; no placeholder reaches a departure. + +Collaborator rules (decided here, the recordings only exercise read/write grants): +only Admin may list, add, change or remove collaborators; an admin collaborator may grant any +mode including `admin` and may remove other collaborators, but owner authority never depends +on a collaborator row and cannot be revoked through them; adding the owner (or an owner of the +owning org) as a collaborator is 422; the target must be a user, not an organization (422 +`user does not exist [uid: 0, name: x]`, recorded C#17); `permission` must be `read`, `write` +or `admin`, default `write`, anything else 422; a user may not remove their own admin-by-row +access through the last-admin path only if they are the sole admin collaborator and not the +owner (not enforced: the owner can always repair it). Collaborators are returned in the order +they were added (recorded C#18). + +## Listings (search, `/user/repos`, `/orgs/{o}/repos`) + +All three filter in SQL by the same predicate as `resolve` (owner, org-owner join, collaborator +row, admin = all) intersected with the token: a `public-only` token sees **nothing** (200, empty +data, `X-Total-Count: 0`; recorded R#3), never a 403. `X-Total-Count` is the visible count. +K#54-56 recorded counts that include repos the caller cannot see (11 for an outsider whose body +is empty); sc-forge answers the visible count, a reviewed departure on those exchanges (the +200-for-an-invisible-org itself is kept: F1 already accepts that org existence leaks, recorded +`HasOrgOrUserVisible` vs `GetOrgByName`). ## Git runner (`gitproc.rs`) The one place a `git` child is spawned (F2b adds the CGI path on the same primitives): `env_clear`, fixed `PATH`, `HOME=/tmp/home`, `GIT_CONFIG_NOSYSTEM`, `GIT_CONFIG_GLOBAL=/dev/null`, -`GIT_NO_REPLACE_OBJECTS=1`, `GIT_TERMINAL_PROMPT=0`, per-spawn config through +`GIT_ATTR_NOSYSTEM=1`, `GIT_NO_REPLACE_OBJECTS=1`, `GIT_TERMINAL_PROMPT=0`, `GIT_CEILING_DIRECTORIES=`, +and **always** `--git-dir=` (never discovery: a repo with a missing or corrupt `HEAD`, +the case the reconciler repairs, must not make git walk up to an enclosing repository), per-spawn config through `GIT_CONFIG_COUNT` (`core.hooksPath=/hooks`, `core.fsync=all`, `core.fsyncMethod=fsync`, `gc.auto=0`, `core.useReplaceRefs=false`, `receive.fsckObjects=true`), explicit author and committer identity from the acting user for commit-producing spawns, own process group (`CommandExt::process_group(0)`), wall-clock timeout that kills the group (`libc::killpg`) and reaps it, stdout and stderr capped and drained concurrently, a global semaphore of -concurrent children. Inputs: `repo_dir` is always built from the integer id; refs and -revisions go through `rev-parse --verify --end-of-options`; everything else is a literal. +concurrent children. The semaphore is acquired with a deadline (503 on expiry) before any +stdin is written, stdout/stderr are drained by their own threads (not the blocking pool), the +timeout covers waits and stdin, and every exit path kills the group and reaps it. Inputs: +`repo_dir` is always built from the integer id; **branch names are validated first** by a +`check-ref-format`-equivalent (`refname::branch(name)`: no `..`, no control chars or space, +no `~^:?*[\`, no leading `-`, no trailing `.lock`/`/`/`.`, not `@`, no `@{`), then +addressed as the full ref `refs/heads/` with `show-ref --verify` (never as a revision, so +`main~1` cannot pass); blob and commit-message bytes go through stdin, never argv; commit +identity values (login, email) must contain no NUL, newline, `<` or `>` (and the F1 admin +user create/patch gains the same email check). `for-each-ref` uses a fixed +`--format=%(refname)%00%(objectname)` with explicit `--sort`, not `-z` (not a git option). Used in F2a for: `init --bare --initial-branch= --template=`, `symbolic-ref HEAD`, -`for-each-ref -z`, `rev-parse --verify`, `hash-object -w`, `mktree`/`commit-tree` (auto-init), +`for-each-ref --format`, `show-ref --verify`, `rev-parse --verify`, `hash-object -w --no-filters --stdin`, `mktree`/`commit-tree` (auto-init), `update-ref -d` (branch delete), `log -1 -z` (branch object). -## Lifecycle (journaled, crash-safe) +## Lifecycle (journaled, crash-safe, serialized) + +**Single writer.** `serve` takes an exclusive `flock` on `/.lock` before anything else and +fails if it cannot (a second `serve` or a concurrent reconcile cannot mutate the same +directory). Reconcile runs once, at startup, before the listener is bound; it is also callable +from tests. **Per-repo mutex** (in-process map keyed by repo id): create-finalize, delete, +PATCH default_branch and branch delete take it, re-check `state = 'live'` under it, and use +conditional SQL (`... WHERE id = ? AND state = 'live'`). The git runner refuses to spawn for a +repo whose row is not `live` (F2b also registers its children for the delete to terminate). **Create**: insert row `creating` (unique violation -> 409 `The repository with the same name -already exists.`) -> `git init --bare` into `repos/.git` -> optional auto-init commit -(README.md `# \n`, message `Initial commit`, author = creating user) via plumbing, ref -`refs/heads/` -> fsync the repo dir and `repos/` -> set row `live` -> 201. -**Delete**: row `deleting` -> (F2b: terminate registered children) -> rename the directory into -`trash/.git` -> fsync both parents -> delete the row (collaborators cascade) -> remove the -trash entry. **PATCH default_branch**: branch must exist (422 otherwise: departure), write bare -`HEAD` first, then the row; the row is the authority on mismatch. - -**Reconciler** (`lifecycle::reconcile`, run at `serve` start and callable from tests), all by -the spec's recovery table: a `creating` row is completed if its directory is a valid bare repo -with the expected `HEAD`, else its directory is removed and the row deleted; a directory in -`repos/` with no live/creating row is removed; `deleting` + directory present -> resume at the -rename; `deleting` + trash present -> finish by deleting the row; trash with no row -> removed; -`deleting` with neither -> row deleted; a `live` row whose `HEAD` differs from `default_branch` --> `symbolic-ref` repair; a `live` row whose directory is missing -> logged and left -(never silently dropped). +already exists.`) -> `git init --bare` into `repos/.git` -> optional auto-init commit via +plumbing -> fsync **every file and directory of the new tree** (about ten entries with +`--template=`) and `repos/`, propagating any fsync error -> set row `live` -> 201. Any failure +before `live` rolls back in-process (remove directory, delete row) and answers 500, so a failed +create never blocks the name; if the rollback itself fails the row stays `creating` and the +next startup reconcile finishes it. +**Delete** (owner-level): row `deleting` -> rename the directory into `trash/.git` -> fsync +both parents -> transactional row delete (collaborators cascade) -> remove the trash entry +(failure is logged; the startup reconcile removes it). A failed rename is rolled back to `live`. +**PATCH default_branch**: under the repo mutex, `show-ref --verify refs/heads/` must succeed +(422 otherwise: departure), write bare `HEAD` (`symbolic-ref`), then the conditional row update; +the row is the authority on mismatch. **PATCH `name`** (rename) is rejected with 422 +`repository rename is not supported` (departure; M#42-45/49-51), never silently accepted. Other +Gitea fields (`has_*`, `allow_*`, `private`, ...) are accepted and ignored, answered with the +fixed values (recorded C#6-8). + +**Reconciler** (`lifecycle::reconcile`, startup only, all outcomes logged), by the spec's table +with these amendments: **every `creating` row is rolled back** (directory removed, row deleted; +the row does not record `auto_init`, so a half-initialized repo can never be promoted, and the +client never saw a 201 for it); a directory in `repos/` with no row at all is **moved to +`/orphans/.git` and logged, never deleted** (a restored or recreated `forge.db` must +not erase repositories); `deleting` + directory present -> resume at the rename; `deleting` + +trash present -> finish by deleting the row; trash with no row -> removed; `deleting` with +neither -> row deleted; a `live` row whose `HEAD` differs from `default_branch` -> +`symbolic-ref` repair; a `live` row whose directory is missing -> logged and left (never +silently dropped). ## Names (extend `names.rs`) @@ -127,33 +177,51 @@ bare arrays. Branch object per E/D fixtures. ## Tests (written first) -1. Unit: names, access matrix (every role x repo kind), repo JSON key set against the C#0 - fixture keys, reconciler cases with injected crash points (one test per row of the recovery - table, on a temp data dir with real `git`). -2. Rust e2e over a raw socket (existing harness): scenario R complete, A/B/X up to their next - later-slice call, C#0-26 and C#34-38 (everything without a git transport step), D#0-6/12, - K's API half, L, M#0-22 and #41-44/49-51 minus rename departures, N#0-2/9-11; each asserts the - recorded status, headers, key set and texts, plus negative tests: foreign token == missing - repo, `public-only` token, read collaborator cannot PATCH/DELETE/PUT collaborator, org - non-owner cannot create, duplicate case-insensitive name, delete then recreate gets a new - id and directory, a deleting repo is invisible. -3. Conformance: `slice-pending.json` updated (R drops out; A/B/X stop at their F3/F4 setup - call); `check` gains git-command comparison (#297) because git steps now run for real in C. -4. Real `git` is required in CI (already on the runners); the minimum is the spec's 2.40. +1. Unit: names and branch-name validation (incl. `main~1`, `-x`, `a..b`, `x.lock`, `@{`), the + access matrix (every role x repo kind x token kind, including `public-only` and + collaborator-admin vs owner-only DELETE), repo JSON key set against the C#0 fixture keys, + identity validation, the reconciler (one test per row of the recovery table plus: restored + empty `forge.db` moves directories to `orphans/`, never deletes; half-initialized create is + rolled back; second `serve` fails on the lock), and the git runner (env scrub, explicit + `--git-dir`, timeout kills the group, output cap, semaphore deadline). +2. Rust e2e over a raw socket (existing harness), mirroring the recorded exchanges that need + no later slice: R complete (incl. R#3 empty search for `public-only`), L complete, A/B/X up + to their F3/F4 setup call, C#0-26 and C#34-38 (everything without a git transport step; the + `dev` branch and extra commits of N are made with direct `git` against `repos/.git`), + N#0-2 and 9-11, K's API half and D#0-6 where the route is F2a (branches, branch get), M's + name rules (#0-22, #41-44, #49-51 with the rename departures), plus negatives: foreign token + == missing repo, read collaborator cannot PATCH, admin collaborator can grant but cannot + DELETE the repo, owner cannot be added as a collaborator, org non-owner cannot create, + case-insensitive duplicate, delete then recreate gets a new id and directory, a `deleting` + repo is invisible, `main~1` as default_branch is 422, branch names with `/` route. +3. Conformance gate (`forge-check.sh`, `--only` widened): `A`, `B` and `X` stop at their F3/F4 + setup call, `C` at its first transport step (`c.wc` clone), `N` at its first `push_branch`, + `L` and `R` complete; `D`, `K`, `M` stay out of the gate until F2b (they call `commits`, + `info/refs`, contents and redirects as recorded exchanges, which `--pending` cannot hide). + Pinned stop texts go in `slice-pending.json`. Git-command comparison in `check` (#297) moves + to F2b, where git steps first run for real. New departures: K#54-56 counts, M rename (422) + and the 404 text overrides reconciled to sc-forge's real text. +4. Real `git` is required (already on the runners); the minimum is the spec's 2.40. ## Order of work -1. migration 2 + store functions + `access` + names (tests first) -2. `gitproc.rs` (tests: env scrub, timeout kills group, output cap, semaphore) -3. lifecycle create/delete/reconcile (crash-point tests) -4. handlers + route rows + `Actor::Repo` in dispatch -5. conformance updates, `--pending` shrink, docs (spec "as built", README, ROADMAP) -6. independent review (Opus + Sol) of the final diff; fix; PR +1. decide the F2a conformance gate (done above) and widen `forge-check.sh --only` with its pending stops +2. migration 2 + store functions + `access::resolve` + names and branch-name validation (tests first) +3. `gitproc.rs` (tests: env scrub, `--git-dir`, timeout kills group, output cap, semaphore deadline) +4. data-dir lock, per-repo mutex, lifecycle create/delete/PATCH, reconciler (crash-point tests) +5. router tail capture (`{b..}`), `Actor::Repo`, handlers, listings +6. conformance updates, departures, docs (spec "as built", README, ROADMAP) +7. independent review (Opus + Sol) of the final diff; fix; PR ## Risks and open points -- `Actor::Repo` puts repo resolution in `dispatch`; a handler that takes `{r}` without it - must not compile into the table (test enumerates rows with `{r}` in the pattern). +- `Actor::Repo` puts repo resolution in `dispatch`; a test enumerates every row whose pattern + has `{r}` and fails unless it uses `Actor::Repo`. +- Known limit: a push that deletes the branch a concurrent PATCH just chose as default can + leave `HEAD` pointing at a missing branch (repo reads as `empty`); the row stays the + authority and the F2b pre-receive hook refuses deleting the row's default branch. +- Per-source failure of the git runner under load is a 503, not a queue: admission is bounded + and has a deadline. - Repo-name case: store and compare lower-cased; the displayed `name` keeps the stored case. - `git init --bare` inherits the host's templates unless `--template=` is empty; the runner always passes it, and a test asserts the new repo has no `hooks/*.sample` and no config From 66059d3be7516037759d2fbc3632ac8b0d6751f2 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:19:34 +0300 Subject: [PATCH 04/16] docs(forge): tidy collaborator rules in the F2a plan --- docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md index 4fa1a10b..e266c800 100644 --- a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md +++ b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md @@ -76,9 +76,7 @@ mode including `admin` and may remove other collaborators, but owner authority n on a collaborator row and cannot be revoked through them; adding the owner (or an owner of the owning org) as a collaborator is 422; the target must be a user, not an organization (422 `user does not exist [uid: 0, name: x]`, recorded C#17); `permission` must be `read`, `write` -or `admin`, default `write`, anything else 422; a user may not remove their own admin-by-row -access through the last-admin path only if they are the sole admin collaborator and not the -owner (not enforced: the owner can always repair it). Collaborators are returned in the order +or `admin`, default `write`, anything else 422; there is no last-admin rule (the owner can always repair access). Collaborators are returned in the order they were added (recorded C#18). ## Listings (search, `/user/repos`, `/orgs/{o}/repos`) From 62f36d4d4dc19bc6cc4e86838a4e6419b086e867 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:22:15 +0300 Subject: [PATCH 05/16] feat(forge): F2a repo store, access resolution, name and branch-name validation (wip) --- crates/forge/src/db.rs | 21 ++ crates/forge/src/lib.rs | 2 + crates/forge/src/names.rs | 93 +++++ crates/forge/src/refname.rs | 96 ++++++ crates/forge/src/repos.rs | 659 ++++++++++++++++++++++++++++++++++++ 5 files changed, 871 insertions(+) create mode 100644 crates/forge/src/refname.rs create mode 100644 crates/forge/src/repos.rs diff --git a/crates/forge/src/db.rs b/crates/forge/src/db.rs index e1d1532b..9a8e13ec 100644 --- a/crates/forge/src/db.rs +++ b/crates/forge/src/db.rs @@ -49,6 +49,27 @@ const MIGRATIONS: &[&str] = &[ expires_unix INTEGER, UNIQUE (user_id, name) );", + // 2: repositories (named by immutable id on disk) and collaborators. `private` is not + // stored: every repository is private. + "CREATE TABLE repos ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + owner_id INTEGER NOT NULL REFERENCES users(id), + name TEXT NOT NULL, + lower_name TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + website TEXT NOT NULL DEFAULT '', + default_branch TEXT NOT NULL, + state TEXT NOT NULL CHECK (state IN ('creating', 'live', 'deleting')), + created_unix INTEGER NOT NULL, + updated_unix INTEGER NOT NULL, + UNIQUE (owner_id, lower_name) + ); + CREATE TABLE collaborators ( + repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + mode TEXT NOT NULL CHECK (mode IN ('read', 'write', 'admin')), + PRIMARY KEY (repo_id, user_id) + );", ]; /// Database failures a caller reports to the operator. diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 12f413d7..758a6130 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -12,6 +12,8 @@ pub mod db; pub mod http; pub mod limits; pub mod names; +pub mod refname; +pub mod repos; pub mod routes; pub mod scope; pub mod server; diff --git a/crates/forge/src/names.rs b/crates/forge/src/names.rs index 3eaf36e9..cd634662 100644 --- a/crates/forge/src/names.rs +++ b/crates/forge/src/names.rs @@ -96,3 +96,96 @@ mod tests { assert!(check(&"x".repeat(40)).is_ok()); } } + +/// Why a repository name was refused (Gitea's texts are built by the caller). +#[derive(Debug, PartialEq, Eq)] +pub enum RepoNameError { + Empty, + /// `.` or `..`. + Reserved, + /// A forbidden suffix such as `*.git`. + Pattern(&'static str), + /// A character outside `[A-Za-z0-9._-]`. + BadChars, + TooLong, +} + +const REPO_MAX_LEN: usize = 100; +const REPO_SUFFIXES: [&str; 4] = [".git", ".wiki", ".rss", ".atom"]; + +/// Validate a repository name by the recorded Gitea rules: `[A-Za-z0-9._-]`, at most 100 +/// characters, `.` and `..` reserved, and the suffixes `.git`, `.wiki`, `.rss`, `.atom` refused. +/// A leading `.` or `-`, `a..b` and words like `api` or `admin` are accepted (recorded 201): +/// repositories live on disk under their immutable id, never under their name. +pub fn check_repo(name: &str) -> Result<(), RepoNameError> { + if name.is_empty() { + return Err(RepoNameError::Empty); + } + if name.chars().count() > REPO_MAX_LEN { + return Err(RepoNameError::TooLong); + } + if name == "." || name == ".." { + return Err(RepoNameError::Reserved); + } + if !name + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b'_')) + { + return Err(RepoNameError::BadChars); + } + let lower = name.to_ascii_lowercase(); + for suffix in REPO_SUFFIXES { + if lower.ends_with(suffix) { + return Err(RepoNameError::Pattern(match suffix { + ".git" => "*.git", + ".wiki" => "*.wiki", + ".rss" => "*.rss", + _ => "*.atom", + })); + } + } + Ok(()) +} + +#[cfg(test)] +mod repo_tests { + use super::*; + + #[test] + fn follows_the_recorded_repo_rules() { + // Recorded 201 (scenario M): these names are accepted. + for ok in [ + "Mixed", + ".hidden", + "-dash", + "a..b", + "api", + "admin", + "user", + "assets", + "ok_name-1.x", + ] { + assert_eq!(check_repo(ok), Ok(()), "{ok}"); + } + assert_eq!(check_repo("."), Err(RepoNameError::Reserved)); + assert_eq!(check_repo(".."), Err(RepoNameError::Reserved)); + assert_eq!( + check_repo("trail.git"), + Err(RepoNameError::Pattern("*.git")) + ); + assert_eq!( + check_repo("wiki.wiki"), + Err(RepoNameError::Pattern("*.wiki")) + ); + assert_eq!(check_repo("feed.RSS"), Err(RepoNameError::Pattern("*.rss"))); + assert_eq!( + check_repo("feed.atom"), + Err(RepoNameError::Pattern("*.atom")) + ); + assert_eq!(check_repo("a b"), Err(RepoNameError::BadChars)); + assert_eq!(check_repo("\u{fc}n\u{ef}"), Err(RepoNameError::BadChars)); + assert_eq!(check_repo(&"x".repeat(101)), Err(RepoNameError::TooLong)); + assert_eq!(check_repo(&"x".repeat(100)), Ok(())); + assert_eq!(check_repo(""), Err(RepoNameError::Empty)); + } +} diff --git a/crates/forge/src/refname.rs b/crates/forge/src/refname.rs new file mode 100644 index 00000000..1a2ad37e --- /dev/null +++ b/crates/forge/src/refname.rs @@ -0,0 +1,96 @@ +//! Branch-name validation, applied before a name ever reaches `git`. It follows +//! `git check-ref-format` for `refs/heads/` and is stricter where git is +//! lenient in a way that matters here (`HEAD`, leading `-`). Names are always +//! addressed as the full ref `refs/heads/`, never as a revision, so +//! `main~1` or `HEAD:path` can never be accepted as a branch. + +const MAX_LEN: usize = 255; + +/// Is `name` an acceptable branch name? +pub fn valid_branch(name: &str) -> bool { + if name.is_empty() || name.len() > MAX_LEN || name == "@" || name == "HEAD" { + return false; + } + if name.starts_with('-') || name.starts_with('/') || name.ends_with('/') || name.ends_with('.') + { + return false; + } + if name.contains("..") || name.contains("@{") || name.contains("//") { + return false; + } + if name + .chars() + .any(|c| c.is_control() || matches!(c, ' ' | '~' | '^' | ':' | '?' | '*' | '[' | '\\')) + { + return false; + } + name.split('/') + .all(|c| !c.starts_with('.') && !c.ends_with(".lock")) +} + +/// The full ref for a (validated) branch name. +pub fn branch_ref(name: &str) -> String { + format!("refs/heads/{name}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_ordinary_and_slashed_names() { + for ok in [ + "main", + "dev", + "feat/x", + "docs/plan-01", + "release-1.2", + "a.b", + "ünï", + "x_y", + ] { + assert!(valid_branch(ok), "{ok}"); + } + } + + #[test] + fn rejects_revision_syntax_and_git_forbidden_forms() { + for bad in [ + "", + "-x", + "--help", + "/a", + "a/", + "a//b", + "a..b", + "main~1", + "main^", + "HEAD:path", + "a b", + "a\tb", + "a\nb", + "a\0b", + "a?b", + "a*b", + "a[b", + "a\\b", + "a@{u}", + "@", + "HEAD", + ".hidden", + "a/.hidden", + "x.lock", + "a/x.lock", + "a.", + ] { + assert!(!valid_branch(bad), "{bad:?}"); + } + assert!(!valid_branch(&"x".repeat(256))); + assert!(valid_branch(&"x".repeat(255))); + } + + #[test] + fn the_full_ref_is_what_reaches_git() { + assert_eq!(branch_ref("feat/x"), "refs/heads/feat/x"); + } +} diff --git a/crates/forge/src/repos.rs b/crates/forge/src/repos.rs new file mode 100644 index 00000000..80d7838b --- /dev/null +++ b/crates/forge/src/repos.rs @@ -0,0 +1,659 @@ +//! Repositories and collaborators: the queries, and the one function +//! ([`resolve`]) that turns a caller and a repository name into a permission. +//! The F2b git path calls the same function, so API and git access cannot drift. + +use crate::store::{self, StoreError, User}; +use rusqlite::{params, Connection, OptionalExtension, Row}; + +/// What a caller may do in a repository. `Read < Write < Admin`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum Perm { + Read, + Write, + Admin, +} + +impl Perm { + /// The stored collaborator mode. + pub fn as_mode(self) -> &'static str { + match self { + Perm::Read => "read", + Perm::Write => "write", + Perm::Admin => "admin", + } + } + + /// Parse a collaborator mode. + pub fn from_mode(s: &str) -> Option { + match s { + "read" => Some(Perm::Read), + "write" => Some(Perm::Write), + "admin" => Some(Perm::Admin), + _ => None, + } + } +} + +/// A repository row. +#[derive(Clone, Debug)] +pub struct Repo { + pub id: i64, + pub owner_id: i64, + pub owner: String, + pub name: String, + pub description: String, + pub website: String, + pub default_branch: String, + pub state: String, + pub created: i64, + pub updated: i64, +} + +const REPO_COLS: &str = + "r.id, r.owner_id, u.login, r.name, r.description, r.website, r.default_branch, \ + r.state, r.created_unix, r.updated_unix"; +const REPO_FROM: &str = "FROM repos r JOIN users u ON u.id = r.owner_id"; + +fn repo_row(r: &Row) -> rusqlite::Result { + Ok(Repo { + id: r.get(0)?, + owner_id: r.get(1)?, + owner: r.get(2)?, + name: r.get(3)?, + description: r.get(4)?, + website: r.get(5)?, + default_branch: r.get(6)?, + state: r.get(7)?, + created: r.get(8)?, + updated: r.get(9)?, + }) +} + +/// Insert a repository in state `creating`. `Exists` when the owner already has one of that +/// name, in any state and in any letter case. +pub fn insert_creating( + conn: &Connection, + owner_id: i64, + name: &str, + default_branch: &str, + description: &str, + website: &str, + now: i64, +) -> Result { + let r = conn.execute( + "INSERT INTO repos (owner_id, name, lower_name, description, website, default_branch, state, \ + created_unix, updated_unix) VALUES (?1, ?2, ?3, ?4, ?5, ?6, 'creating', ?7, ?7)", + params![owner_id, name, name.to_lowercase(), description, website, default_branch, now], + ); + match r { + Ok(_) => find_by_id(conn, conn.last_insert_rowid())?.ok_or(StoreError::NotFound), + Err(rusqlite::Error::SqliteFailure(f, _)) + if f.code == rusqlite::ErrorCode::ConstraintViolation => + { + Err(StoreError::Exists) + } + Err(e) => Err(e.into()), + } +} + +/// A repository by id, in any state. +pub fn find_by_id(conn: &Connection, id: i64) -> rusqlite::Result> { + conn.query_row( + &format!("SELECT {REPO_COLS} {REPO_FROM} WHERE r.id = ?1"), + [id], + repo_row, + ) + .optional() +} + +/// A repository by owner login and name (case-insensitive), in any state. +pub fn find_row(conn: &Connection, owner: &str, name: &str) -> rusqlite::Result> { + conn.query_row( + &format!("SELECT {REPO_COLS} {REPO_FROM} WHERE u.lower_login = ?1 AND r.lower_name = ?2"), + [owner.to_lowercase(), name.to_lowercase()], + repo_row, + ) + .optional() +} + +/// Every repository in a state (for the reconciler). +pub fn list_in_state(conn: &Connection, state: &str) -> rusqlite::Result> { + let mut st = conn.prepare(&format!( + "SELECT {REPO_COLS} {REPO_FROM} WHERE r.state = ?1 ORDER BY r.id" + ))?; + let rows = st.query_map([state], repo_row)?; + rows.collect() +} + +/// Move a repository between states; `false` when it was not in `from` (someone else moved it). +pub fn set_state( + conn: &Connection, + id: i64, + from: &str, + to: &str, + now: i64, +) -> rusqlite::Result { + Ok(conn.execute( + "UPDATE repos SET state = ?3, updated_unix = ?4 WHERE id = ?1 AND state = ?2", + params![id, from, to, now], + )? > 0) +} + +/// Delete the row (collaborators cascade). +pub fn delete_row(conn: &Connection, id: i64) -> rusqlite::Result<()> { + conn.execute("DELETE FROM repos WHERE id = ?1", [id])?; + Ok(()) +} + +/// Fields `PATCH` may change; `None` leaves a field alone. +#[derive(Default)] +pub struct RepoPatch { + pub description: Option, + pub website: Option, + pub default_branch: Option, +} + +/// Apply a patch to a `live` repository only (`false` when it is not live any more). +pub fn update_live(conn: &Connection, id: i64, p: &RepoPatch, now: i64) -> rusqlite::Result { + use rusqlite::types::Value; + let mut sets = vec!["updated_unix = ?".to_string()]; + let mut args: Vec = vec![Value::Integer(now)]; + for (col, v) in [ + ("description", &p.description), + ("website", &p.website), + ("default_branch", &p.default_branch), + ] { + if let Some(v) = v { + sets.push(format!("{col} = ?")); + args.push(Value::Text(v.clone())); + } + } + args.push(Value::Integer(id)); + Ok(conn.execute( + &format!( + "UPDATE repos SET {} WHERE id = ? AND state = 'live'", + sets.join(", ") + ), + rusqlite::params_from_iter(args), + )? > 0) +} + +/// A caller's standing in a repository. +#[derive(Clone, Debug)] +pub struct Access { + pub repo: Repo, + pub perm: Perm, + /// The owning user, an owner of the owning organization, or a site admin: authority that + /// does not depend on a collaborator row. + pub is_owner: bool, +} + +/// The standing of `user` in a repository, or `None` when it does not exist, is not `live`, +/// or the caller has no access at all (a missing and an invisible repository are one answer). +/// Site admin = Admin; the owning user and the owners of the owning organization = Admin and +/// owner; otherwise the collaborator mode. Organization membership alone grants nothing. +pub fn resolve( + conn: &Connection, + user: &User, + owner: &str, + name: &str, +) -> rusqlite::Result> { + let Some(repo) = find_row(conn, owner, name)?.filter(|r| r.state == "live") else { + return Ok(None); + }; + let owns = repo.owner_id == user.id || store::is_org_owner(conn, repo.owner_id, user.id)?; + if owns || user.is_admin { + return Ok(Some(Access { + repo, + perm: Perm::Admin, + is_owner: owns || user.is_admin, + })); + } + let mode: Option = conn + .query_row( + "SELECT mode FROM collaborators WHERE repo_id = ?1 AND user_id = ?2", + [repo.id, user.id], + |r| r.get(0), + ) + .optional()?; + Ok(mode.and_then(|m| Perm::from_mode(&m)).map(|perm| Access { + repo, + perm, + is_owner: false, + })) +} + +/// Search and list filters. +#[derive(Default)] +pub struct Filter { + /// Substring of the name or description (case-insensitive). + pub q: Option, + /// Only repositories of this owner. + pub owner_id: Option, +} + +/// How a listing is ordered. +#[derive(Clone, Copy)] +pub enum Sort { + /// By id (creation order). + Id, + /// By name. + Alpha, +} + +/// One page of the `live` repositories `user` can read, and the total, filtered by the same +/// rule as [`resolve`]. `public_only` (a token restricted to public repositories) sees nothing: +/// every repository here is private. +pub fn list_visible( + conn: &Connection, + user: &User, + public_only: bool, + filter: &Filter, + sort: Sort, + page: i64, + limit: i64, +) -> rusqlite::Result<(Vec, i64)> { + if public_only { + return Ok((vec![], 0)); + } + use rusqlite::types::Value; + let mut wheres = vec!["r.state = 'live'".to_string()]; + let mut args: Vec = vec![]; + if !user.is_admin { + wheres.push( + "(r.owner_id = ? OR r.owner_id IN (SELECT org_id FROM org_members WHERE user_id = ?) \ + OR r.id IN (SELECT repo_id FROM collaborators WHERE user_id = ?))" + .into(), + ); + args.extend(std::iter::repeat_n(Value::Integer(user.id), 3)); + } + if let Some(o) = filter.owner_id { + wheres.push("r.owner_id = ?".into()); + args.push(Value::Integer(o)); + } + if let Some(q) = filter.q.as_deref().filter(|q| !q.is_empty()) { + wheres.push( + "(r.lower_name LIKE ? ESCAPE '\\' OR lower(r.description) LIKE ? ESCAPE '\\')".into(), + ); + let pat = format!( + "%{}%", + q.to_lowercase() + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_") + ); + args.extend([Value::Text(pat.clone()), Value::Text(pat)]); + } + let w = wheres.join(" AND "); + let total: i64 = conn.query_row( + &format!("SELECT COUNT(*) {REPO_FROM} WHERE {w}"), + rusqlite::params_from_iter(args.clone()), + |r| r.get(0), + )?; + let order = match sort { + Sort::Id => "r.id", + Sort::Alpha => "r.lower_name, r.id", + }; + args.push(Value::Integer(limit)); + args.push(Value::Integer((page - 1) * limit)); + let mut st = conn.prepare(&format!( + "SELECT {REPO_COLS} {REPO_FROM} WHERE {w} ORDER BY {order} LIMIT ? OFFSET ?" + ))?; + let rows = st.query_map(rusqlite::params_from_iter(args), repo_row)?; + Ok((rows.collect::>()?, total)) +} + +/// Add or change a collaborator (`write` is the default mode, chosen by the caller). +pub fn set_collaborator( + conn: &Connection, + repo_id: i64, + user_id: i64, + mode: Perm, +) -> rusqlite::Result<()> { + conn.execute( + "INSERT INTO collaborators (repo_id, user_id, mode) VALUES (?1, ?2, ?3) \ + ON CONFLICT (repo_id, user_id) DO UPDATE SET mode = excluded.mode", + params![repo_id, user_id, mode.as_mode()], + )?; + Ok(()) +} + +/// Remove a collaborator; `false` when there was none. +pub fn remove_collaborator( + conn: &Connection, + repo_id: i64, + user_id: i64, +) -> rusqlite::Result { + Ok(conn.execute( + "DELETE FROM collaborators WHERE repo_id = ?1 AND user_id = ?2", + [repo_id, user_id], + )? > 0) +} + +/// The collaborators of a repository in the order they were added, with their modes. +pub fn collaborators( + conn: &Connection, + repo_id: i64, + page: i64, + limit: i64, +) -> rusqlite::Result<(Vec<(User, Perm)>, i64)> { + let total: i64 = conn.query_row( + "SELECT COUNT(*) FROM collaborators WHERE repo_id = ?1", + [repo_id], + |r| r.get(0), + )?; + let mut st = conn.prepare( + "SELECT user_id, mode FROM collaborators WHERE repo_id = ?1 ORDER BY rowid LIMIT ?2 OFFSET ?3", + )?; + let ids: Vec<(i64, String)> = st + .query_map(params![repo_id, limit, (page - 1) * limit], |r| { + Ok((r.get(0)?, r.get(1)?)) + })? + .collect::>()?; + let mut out = Vec::new(); + for (uid, mode) in ids { + if let (Some(u), Some(p)) = (store::find_by_id(conn, uid)?, Perm::from_mode(&mode)) { + out.push((u, p)); + } + } + Ok((out, total)) +} + +/// A collaborator's mode, if any. +pub fn collaborator_mode( + conn: &Connection, + repo_id: i64, + user_id: i64, +) -> rusqlite::Result> { + let m: Option = conn + .query_row( + "SELECT mode FROM collaborators WHERE repo_id = ?1 AND user_id = ?2", + [repo_id, user_id], + |r| r.get(0), + ) + .optional()?; + Ok(m.and_then(|m| Perm::from_mode(&m))) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db; + use crate::store::{create_account, create_org, NewAccount}; + use std::path::PathBuf; + + struct Env { + dir: PathBuf, + conn: Connection, + } + impl Drop for Env { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } + } + + fn env(tag: &str) -> Env { + let dir = + std::env::temp_dir().join(format!("scl-forge-repos-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let conn = db::open_for_admin(&dir).unwrap(); + Env { dir, conn } + } + + fn user(c: &Connection, login: &str, admin: bool) -> User { + let a = NewAccount { + login: login.into(), + is_admin: admin, + visibility: "public".into(), + ..Default::default() + }; + create_account(c, &a, 1).unwrap() + } + + fn live(c: &Connection, owner: &User, name: &str) -> Repo { + let r = insert_creating(c, owner.id, name, "main", "", "", 1).unwrap(); + assert!(set_state(c, r.id, "creating", "live", 2).unwrap()); + find_by_id(c, r.id).unwrap().unwrap() + } + + #[test] + fn names_are_unique_per_owner_in_any_case_and_any_state() { + let e = env("uniq"); + let (alice, bob) = (user(&e.conn, "alice", false), user(&e.conn, "bob", false)); + insert_creating(&e.conn, alice.id, "Mixed", "main", "", "", 1).unwrap(); + for dup in ["mixed", "MIXED", "Mixed"] { + assert!( + matches!( + insert_creating(&e.conn, alice.id, dup, "main", "", "", 1), + Err(StoreError::Exists) + ), + "{dup} (still creating)" + ); + } + assert!( + insert_creating(&e.conn, bob.id, "mixed", "main", "", "", 1).is_ok(), + "another owner may reuse the name" + ); + } + + #[test] + fn the_access_matrix() { + let mut e = env("matrix"); + let (admin, owner, reader, writer, adm_collab, outsider) = ( + user(&e.conn, "root", true), + user(&e.conn, "owner", false), + user(&e.conn, "reader", false), + user(&e.conn, "writer", false), + user(&e.conn, "admcol", false), + user(&e.conn, "outsider", false), + ); + let org = create_org( + &mut e.conn, + &NewAccount { + login: "acme".into(), + is_org: true, + visibility: "private".into(), + ..Default::default() + }, + owner.id, + 1, + ) + .unwrap(); + let personal = live(&e.conn, &owner, "personal"); + let orgrepo = live(&e.conn, &org, "orgrepo"); + for r in [&personal, &orgrepo] { + set_collaborator(&e.conn, r.id, reader.id, Perm::Read).unwrap(); + set_collaborator(&e.conn, r.id, writer.id, Perm::Write).unwrap(); + set_collaborator(&e.conn, r.id, adm_collab.id, Perm::Admin).unwrap(); + } + let got = |u: &User, o: &str, n: &str| { + resolve(&e.conn, u, o, n) + .unwrap() + .map(|a| (a.perm, a.is_owner)) + }; + for (o, n) in [("owner", "personal"), ("acme", "orgrepo")] { + assert_eq!(got(&admin, o, n), Some((Perm::Admin, true)), "site admin"); + assert_eq!( + got(&owner, o, n), + Some((Perm::Admin, true)), + "owner / org owner" + ); + assert_eq!( + got(&adm_collab, o, n), + Some((Perm::Admin, false)), + "admin collaborator is not owner" + ); + assert_eq!(got(&writer, o, n), Some((Perm::Write, false))); + assert_eq!(got(&reader, o, n), Some((Perm::Read, false))); + assert_eq!(got(&outsider, o, n), None, "no row, no access"); + } + // case-insensitive lookup, same answer + assert_eq!(got(&owner, "OWNER", "PERSONAL"), Some((Perm::Admin, true))); + // not live -> invisible to everyone, owner and site admin included + let creating = insert_creating(&e.conn, owner.id, "half", "main", "", "", 1).unwrap(); + assert_eq!(got(&owner, "owner", "half"), None); + assert_eq!(got(&admin, "owner", "half"), None); + assert!(set_state(&e.conn, creating.id, "creating", "live", 2).unwrap()); + assert!( + !set_state(&e.conn, creating.id, "creating", "live", 3).unwrap(), + "the transition is conditional" + ); + assert_eq!(got(&owner, "owner", "half"), Some((Perm::Admin, true))); + // org membership alone grants nothing: an org owner row exists only for owners in F1, + // so an outsider who is merely a collaborator elsewhere gains nothing here. + assert_eq!(got(&reader, "acme", "nope"), None); + } + + #[test] + fn listings_follow_the_same_rule_and_public_only_sees_nothing() { + let mut e = env("list"); + let (admin, owner, reader, outsider) = ( + user(&e.conn, "root", true), + user(&e.conn, "owner", false), + user(&e.conn, "reader", false), + user(&e.conn, "outsider", false), + ); + let org = create_org( + &mut e.conn, + &NewAccount { + login: "acme".into(), + is_org: true, + visibility: "private".into(), + ..Default::default() + }, + owner.id, + 1, + ) + .unwrap(); + let a = live(&e.conn, &owner, "alpha"); + live(&e.conn, &owner, "beta"); + live(&e.conn, &org, "gamma"); + insert_creating(&e.conn, owner.id, "hidden-creating", "main", "", "", 1).unwrap(); + set_collaborator(&e.conn, a.id, reader.id, Perm::Read).unwrap(); + let names = |u: &User, po: bool, f: &Filter| { + let (rs, total) = list_visible(&e.conn, u, po, f, Sort::Alpha, 1, 50).unwrap(); + ( + rs.into_iter() + .map(|r| format!("{}/{}", r.owner, r.name)) + .collect::>(), + total, + ) + }; + let all = Filter::default(); + assert_eq!( + names(&admin, false, &all), + ( + // `sort=alpha` orders by repository name, not by owner + vec![ + "owner/alpha".into(), + "owner/beta".into(), + "acme/gamma".into() + ], + 3 + ) + ); + assert_eq!( + names(&owner, false, &all).1, + 3, + "owner sees own and org-owned repos" + ); + assert_eq!(names(&reader, false, &all), (vec!["owner/alpha".into()], 1)); + assert_eq!(names(&outsider, false, &all), (vec![], 0)); + assert_eq!( + names(&owner, true, &all), + (vec![], 0), + "public-only sees nothing, not an error" + ); + assert_eq!( + names( + &admin, + false, + &Filter { + q: Some("ALP".into()), + ..Default::default() + } + ) + .0, + vec!["owner/alpha".to_string()] + ); + assert_eq!( + names( + &admin, + false, + &Filter { + q: Some("100%".into()), + ..Default::default() + } + ) + .1, + 0, + "LIKE wildcards are escaped" + ); + // paging keeps the visible total + let (rs, total) = list_visible(&e.conn, &admin, false, &all, Sort::Id, 2, 2).unwrap(); + assert_eq!((rs.len(), total), (1, 3)); + } + + #[test] + fn collaborators_keep_insertion_order_and_cascade_on_delete() { + let e = env("collab"); + let (owner, a, b) = ( + user(&e.conn, "owner", false), + user(&e.conn, "a", false), + user(&e.conn, "b", false), + ); + let r = live(&e.conn, &owner, "r"); + set_collaborator(&e.conn, r.id, b.id, Perm::Write).unwrap(); + set_collaborator(&e.conn, r.id, a.id, Perm::Read).unwrap(); + set_collaborator(&e.conn, r.id, b.id, Perm::Admin).unwrap(); // a change keeps b's place + let (cs, total) = collaborators(&e.conn, r.id, 1, 50).unwrap(); + assert_eq!( + ( + cs.iter() + .map(|(u, p)| (u.login.as_str(), *p)) + .collect::>(), + total + ), + (vec![("b", Perm::Admin), ("a", Perm::Read)], 2) + ); + assert!(remove_collaborator(&e.conn, r.id, a.id).unwrap()); + assert!(!remove_collaborator(&e.conn, r.id, a.id).unwrap()); + delete_row(&e.conn, r.id).unwrap(); + let left: i64 = e + .conn + .query_row("SELECT COUNT(*) FROM collaborators", [], |r| r.get(0)) + .unwrap(); + assert_eq!(left, 0); + } + + #[test] + fn update_only_touches_a_live_repo() { + let e = env("update"); + let owner = user(&e.conn, "owner", false); + let r = insert_creating(&e.conn, owner.id, "r", "main", "", "", 1).unwrap(); + let p = RepoPatch { + description: Some("d".into()), + ..Default::default() + }; + assert!( + !update_live(&e.conn, r.id, &p, 5).unwrap(), + "a creating repo is not patched" + ); + set_state(&e.conn, r.id, "creating", "live", 2).unwrap(); + assert!(update_live(&e.conn, r.id, &p, 5).unwrap()); + let got = find_by_id(&e.conn, r.id).unwrap().unwrap(); + assert_eq!( + ( + got.description.as_str(), + got.default_branch.as_str(), + got.updated + ), + ("d", "main", 5) + ); + set_state(&e.conn, r.id, "live", "deleting", 6).unwrap(); + assert!( + !update_live(&e.conn, r.id, &p, 7).unwrap(), + "a deleting repo is not patched" + ); + } +} From dc24db757bc2055c909f4f8aff54de029d830acb Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:24:35 +0300 Subject: [PATCH 06/16] feat(forge): F2a git runner (wip) --- Cargo.lock | 1 + crates/forge/Cargo.toml | 1 + crates/forge/src/gitproc.rs | 548 ++++++++++++++++++++++++++++++++++++ crates/forge/src/lib.rs | 1 + crates/forge/src/limits.rs | 31 ++ 5 files changed, 582 insertions(+) create mode 100644 crates/forge/src/gitproc.rs diff --git a/Cargo.lock b/Cargo.lock index d41f61db..e68f98f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5550,6 +5550,7 @@ dependencies = [ "http-body-util", "hyper", "hyper-util", + "libc", "rusqlite", "scl-crypto", "serde_json", diff --git a/crates/forge/Cargo.toml b/crates/forge/Cargo.toml index f0f2e3e5..6864ef57 100644 --- a/crates/forge/Cargo.toml +++ b/crates/forge/Cargo.toml @@ -26,6 +26,7 @@ scl-crypto = { path = "../crypto" } http-body-util = "0.1.5" hyper = { version = "1", features = ["http1", "server"] } hyper-util = { version = "0.1.21", features = ["tokio", "server"] } +libc = "0.2.191" rusqlite = { version = "0.40.2", features = ["bundled"] } serde_json = "1.0.154" thiserror = "2.0.18" diff --git a/crates/forge/src/gitproc.rs b/crates/forge/src/gitproc.rs new file mode 100644 index 00000000..e7fd0ea3 --- /dev/null +++ b/crates/forge/src/gitproc.rs @@ -0,0 +1,548 @@ +//! The one place a `git` child process is spawned. Every spawn gets an environment built from +//! scratch (nothing inherited), an explicit `--git-dir` (never repository discovery), per-spawn +//! configuration through `GIT_CONFIG_COUNT`, its own process group, a wall-clock deadline that +//! kills the whole group, capped output drained by dedicated threads, and an admission slot +//! acquired with a deadline so a busy server answers 503 instead of queueing. +//! +//! Inputs are the caller's responsibility to validate (branch names through `refname`, identity +//! through [`Identity::new`]); this module never builds a revision from client text. + +use crate::limits::Gate; +use std::ffi::OsStr; +use std::io::{Read, Write}; +use std::os::unix::process::CommandExt; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitStatus, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +/// Oldest `git` the server supports (`merge-tree --write-tree` needs 2.38; the spec pins 2.40). +pub const MIN_GIT: (u32, u32) = (2, 40); + +/// Why a git spawn failed. +#[derive(Debug, thiserror::Error)] +pub enum GitError { + /// No admission slot within the deadline. + #[error("too many git operations in flight")] + Busy, + /// The deadline passed; the process group was killed. Carries what was captured. + #[error("git timed out")] + Timeout { stdout: Vec }, + /// More output than the cap; the process group was killed. + #[error("git produced too much output")] + OutputTooLarge, + #[error("could not run git: {0}")] + Spawn(#[from] std::io::Error), +} + +/// What a finished git run produced. +#[derive(Debug)] +pub struct Out { + pub status: ExitStatus, + pub stdout: Vec, + pub stderr: Vec, +} + +impl Out { + pub fn ok(&self) -> bool { + self.status.success() + } + + /// stdout as text, trimmed. + pub fn text(&self) -> String { + String::from_utf8_lossy(&self.stdout).trim().to_string() + } +} + +/// An author/committer identity for commit-producing spawns. The values reach git through +/// environment variables, so NUL, newline and angle brackets (which would forge another +/// header field) are refused. +#[derive(Clone, Debug)] +pub struct Identity { + name: String, + email: String, +} + +impl Identity { + /// `None` when either value could not be a safe identity. + pub fn new(name: &str, email: &str) -> Option { + let safe = + |s: &str| !s.is_empty() && !s.chars().any(|c| c.is_control() || matches!(c, '<' | '>')); + (safe(name) && safe(email)).then(|| Identity { + name: name.into(), + email: email.into(), + }) + } +} + +/// One git invocation. +#[derive(Default)] +pub struct Spec<'a> { + /// The bare repository (absolute path built from the integer id), or `None` for commands + /// that need none (`--version`, `init` takes its path as an argument). + pub repo: Option<&'a Path>, + pub args: &'a [&'a str], + pub stdin: Option<&'a [u8]>, + pub identity: Option<&'a Identity>, +} + +/// The runner. +pub struct Git { + data_dir: PathBuf, + bin: PathBuf, + gate: Gate, + timeout: Duration, + admission: Duration, + max_output: usize, +} + +impl Git { + /// Build a runner for `data_dir` using the `git` at `bin`. Creates the empty hook directory + /// and scratch `HOME` it points git at. + pub fn new(data_dir: &Path, bin: PathBuf, concurrency: usize) -> std::io::Result { + std::fs::create_dir_all(data_dir.join("hooks"))?; + std::fs::create_dir_all(data_dir.join("tmp").join("home"))?; + Ok(Git { + data_dir: data_dir.to_path_buf(), + bin, + gate: Gate::new(concurrency), + timeout: Duration::from_secs(30), + admission: Duration::from_secs(5), + max_output: 16 << 20, + }) + } + + pub fn with_limits(mut self, timeout: Duration, admission: Duration, max_output: usize) -> Git { + self.timeout = timeout; + self.admission = admission; + self.max_output = max_output; + self + } + + /// The environment of every spawn: built from scratch, nothing inherited. + pub fn env(&self, identity: Option<&Identity>) -> Vec<(String, String)> { + let d = self.data_dir.display().to_string(); + let mut env: Vec<(String, String)> = vec![ + ("PATH".into(), "/usr/bin:/bin".into()), + ("HOME".into(), format!("{d}/tmp/home")), + ("LC_ALL".into(), "C".into()), + ("GIT_CONFIG_NOSYSTEM".into(), "1".into()), + ("GIT_CONFIG_GLOBAL".into(), "/dev/null".into()), + ("GIT_ATTR_NOSYSTEM".into(), "1".into()), + ("GIT_NO_REPLACE_OBJECTS".into(), "1".into()), + ("GIT_TERMINAL_PROMPT".into(), "0".into()), + ("GIT_CEILING_DIRECTORIES".into(), d.clone()), + ]; + let config = [ + ("core.hooksPath", format!("{d}/hooks")), + ("core.fsync", "all".to_string()), + ("core.fsyncMethod", "fsync".to_string()), + ("core.useReplaceRefs", "false".to_string()), + ("gc.auto", "0".to_string()), + ("receive.fsckObjects", "true".to_string()), + ]; + env.push(("GIT_CONFIG_COUNT".into(), config.len().to_string())); + for (i, (k, v)) in config.into_iter().enumerate() { + env.push((format!("GIT_CONFIG_KEY_{i}"), k.into())); + env.push((format!("GIT_CONFIG_VALUE_{i}"), v)); + } + if let Some(id) = identity { + for who in ["AUTHOR", "COMMITTER"] { + env.push((format!("GIT_{who}_NAME"), id.name.clone())); + env.push((format!("GIT_{who}_EMAIL"), id.email.clone())); + } + } + env + } + + /// Run git as described by `spec`. + pub fn run(&self, spec: &Spec) -> Result { + self.run_program(self.bin.as_os_str(), spec) + } + + fn run_program(&self, program: &OsStr, spec: &Spec) -> Result { + let _slot = self + .gate + .acquire_timeout(self.admission) + .ok_or(GitError::Busy)?; + let mut cmd = Command::new(program); + cmd.env_clear().envs(self.env(spec.identity)); + if let Some(repo) = spec.repo { + cmd.arg(format!("--git-dir={}", repo.display())); + } + cmd.args(spec.args) + .current_dir(&self.data_dir) + .stdin(if spec.stdin.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .process_group(0); + let mut child = cmd.spawn()?; + let pgid = child.id() as i32; + let kill_group = move || { + // SAFETY: killpg only sends a signal; the child leads its own group (process_group(0)). + unsafe { libc::killpg(pgid, libc::SIGKILL) }; + }; + + let over = Arc::new(AtomicBool::new(false)); + let drain = |mut r: Box, cap: usize, over: Arc| { + std::thread::spawn(move || { + let mut out = Vec::new(); + let mut buf = [0u8; 16 * 1024]; + while let Ok(n) = r.read(&mut buf) { + if n == 0 { + break; + } + if out.len() + n > cap { + over.store(true, Ordering::SeqCst); + // Keep draining so a still-running child never blocks on a full pipe. + continue; + } + out.extend_from_slice(&buf[..n]); + } + out + }) + }; + let t_out = drain( + Box::new(child.stdout.take().expect("piped")), + self.max_output, + over.clone(), + ); + let t_err = drain( + Box::new(child.stderr.take().expect("piped")), + self.max_output, + over.clone(), + ); + let t_in = spec.stdin.map(|bytes| { + let mut w = child.stdin.take().expect("piped"); + let bytes = bytes.to_vec(); + std::thread::spawn(move || { + let _ = w.write_all(&bytes); // a child that exits early closes the pipe: not an error here + }) + }); + + let deadline = Instant::now() + self.timeout; + let status = loop { + if let Some(st) = child.try_wait()? { + break Ok(st); + } + if over.load(Ordering::SeqCst) { + kill_group(); + let _ = child.wait(); + break Err(GitError::OutputTooLarge); + } + if Instant::now() >= deadline { + kill_group(); + let _ = child.wait(); + break Err(GitError::Timeout { stdout: Vec::new() }); + } + std::thread::sleep(Duration::from_millis(2)); + }; + // The group may outlive the leader (a hook's background child): end it on every path. + kill_group(); + if let Some(t) = t_in { + let _ = t.join(); + } + let (stdout, stderr) = ( + t_out.join().unwrap_or_default(), + t_err.join().unwrap_or_default(), + ); + match status { + Ok(status) if !over.load(Ordering::SeqCst) => Ok(Out { + status, + stdout, + stderr, + }), + Ok(_) => Err(GitError::OutputTooLarge), + Err(GitError::Timeout { .. }) => Err(GitError::Timeout { stdout }), + Err(e) => Err(e), + } + } +} + +/// Find `git`: `SC_FORGE_GIT` if set, else the first `git` on the server's own `PATH`. +pub fn find_git() -> Option { + if let Some(p) = std::env::var_os("SC_FORGE_GIT") { + return Some(PathBuf::from(p)); + } + std::env::split_paths(&std::env::var_os("PATH")?) + .map(|d| d.join("git")) + .find(|p| p.is_file()) +} + +/// `(major, minor)` from `git version 2.54.0 (Apple Git-157)`. +pub fn parse_version(text: &str) -> Option<(u32, u32)> { + let v = text.split_whitespace().nth(2)?; + let mut it = v.split('.'); + Some((it.next()?.parse().ok()?, it.next()?.parse().ok()?)) +} + +/// Check the installed git is new enough; returns its version text. +pub fn check_version(git: &Git) -> Result { + let out = git + .run(&Spec { + args: &["--version"], + ..Default::default() + }) + .map_err(|e| e.to_string())?; + let text = out.text(); + match parse_version(&text) { + Some(v) if v >= MIN_GIT => Ok(text), + Some(_) => Err(format!( + "{text}: sc-forge needs git {}.{} or newer", + MIN_GIT.0, MIN_GIT.1 + )), + None => Err(format!("cannot read the git version from {text:?}")), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn tmp(tag: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!("scl-forge-git-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d + } + + fn git(tag: &str) -> (Git, PathBuf) { + let d = tmp(tag); + ( + Git::new(&d, find_git().expect("git on PATH"), 4).unwrap(), + d, + ) + } + + fn init(g: &Git, d: &Path) -> PathBuf { + let repo = d.join("repos").join("1.git"); + std::fs::create_dir_all(d.join("repos")).unwrap(); + let out = g + .run(&Spec { + args: &[ + "init", + "--bare", + "--quiet", + "--template=", + "--initial-branch=main", + repo.to_str().unwrap(), + ], + ..Default::default() + }) + .unwrap(); + assert!(out.ok(), "{}", String::from_utf8_lossy(&out.stderr)); + repo + } + + #[test] + fn the_installed_git_is_new_enough_and_versions_parse() { + let (g, d) = git("version"); + assert!(check_version(&g).is_ok()); + assert_eq!( + parse_version("git version 2.54.0 (Apple Git-157)"), + Some((2, 54)) + ); + assert_eq!(parse_version("git version 2.39.5"), Some((2, 39))); + assert!(parse_version("nonsense").is_none()); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn the_environment_is_built_from_scratch() { + let (g, d) = git("env"); + let id = Identity::new("alice", "alice@example.invalid").unwrap(); + let env = g.env(Some(&id)); + let keys: Vec<&str> = env.iter().map(|(k, _)| k.as_str()).collect(); + for must in [ + "GIT_NO_REPLACE_OBJECTS", + "GIT_CONFIG_NOSYSTEM", + "GIT_ATTR_NOSYSTEM", + "GIT_CEILING_DIRECTORIES", + "GIT_AUTHOR_EMAIL", + "GIT_COMMITTER_NAME", + ] { + assert!(keys.contains(&must), "{must}"); + } + let hooks = env + .iter() + .find(|(k, _)| k.starts_with("GIT_CONFIG_VALUE")) + .map(|(_, v)| v.clone()) + .unwrap(); + assert_eq!( + hooks, + format!("{}/hooks", d.display()), + "hooksPath is pinned to the server's directory" + ); + assert!( + g.env(None) + .iter() + .all(|(k, _)| !k.starts_with("GIT_AUTHOR")), + "no identity unless asked" + ); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn identity_refuses_forged_header_characters() { + assert!(Identity::new("alice", "a@example.invalid").is_some()); + for bad in [ + ("a\nb", "e@x"), + ("a", "e@x\n"), + ("a<", "e@x"), + ("a", "e@x>"), + ("a\0", "e@x"), + ("", "e@x"), + ("a", ""), + ] { + assert!(Identity::new(bad.0, bad.1).is_none(), "{bad:?}"); + } + } + + #[test] + fn git_runs_against_an_explicit_dir_with_server_config_and_no_user_config() { + let (g, d) = git("config"); + let repo = init(&g, &d); + let hooks = g + .run(&Spec { + repo: Some(&repo), + args: &["config", "--get", "core.hooksPath"], + ..Default::default() + }) + .unwrap(); + assert_eq!(hooks.text(), format!("{}/hooks", d.display())); + let user = g + .run(&Spec { + repo: Some(&repo), + args: &["config", "--get", "user.name"], + ..Default::default() + }) + .unwrap(); + assert!(!user.ok(), "no user, global or system config is read"); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn a_corrupt_repo_dir_never_resolves_an_enclosing_repository() { + let (g, d) = git("discovery"); + let outer = d.join("outer"); + std::fs::create_dir_all(&outer).unwrap(); + assert!(g + .run(&Spec { + args: &["init", "--quiet", outer.to_str().unwrap()], + ..Default::default() + }) + .unwrap() + .ok()); + let inner = outer.join("repos").join("2.git"); // a damaged repo: just an empty directory inside another repo + std::fs::create_dir_all(&inner).unwrap(); + let out = g + .run(&Spec { + repo: Some(&inner), + args: &["rev-parse", "--git-dir"], + ..Default::default() + }) + .unwrap(); + assert!( + !out.ok(), + "git must not walk up to the enclosing repository: {}", + out.text() + ); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn stdin_reaches_git_and_objects_are_content_addressed() { + let (g, d) = git("stdin"); + let repo = init(&g, &d); + let out = g + .run(&Spec { + repo: Some(&repo), + args: &["hash-object", "-w", "--no-filters", "--stdin"], + stdin: Some(b"hello\n"), + ..Default::default() + }) + .unwrap(); + assert_eq!(out.text(), "ce013625030ba8dba906f756967f9e9ca394464a"); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn a_timeout_kills_the_whole_process_group() { + let (g, d) = git("timeout"); + let g = g.with_limits(Duration::from_millis(400), Duration::from_secs(5), 1 << 20); + // The shell starts a grandchild that would outlive it, prints its pid, then waits. + let err = g + .run_program( + OsStr::new("/bin/sh"), + &Spec { + args: &["-c", "sleep 60 & echo $!; wait"], + ..Default::default() + }, + ) + .unwrap_err(); + let GitError::Timeout { stdout } = err else { + panic!("expected a timeout, got {err:?}") + }; + let pid: i32 = String::from_utf8_lossy(&stdout) + .trim() + .parse() + .expect("grandchild pid"); + std::thread::sleep(Duration::from_millis(100)); + // SAFETY: signal 0 only probes for existence. + let alive = unsafe { libc::kill(pid, 0) } == 0; + assert!(!alive, "the grandchild {pid} must be killed with its group"); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn output_over_the_cap_kills_the_child() { + let (g, d) = git("cap"); + let g = g.with_limits(Duration::from_secs(10), Duration::from_secs(5), 4096); + let t = Instant::now(); + let err = g + .run_program(OsStr::new("/usr/bin/yes"), &Spec::default()) + .unwrap_err(); + assert!(matches!(err, GitError::OutputTooLarge), "{err:?}"); + assert!( + t.elapsed() < Duration::from_secs(5), + "killed promptly, not at the timeout" + ); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn a_busy_runner_answers_busy_after_the_admission_deadline() { + let (g, d) = git("busy"); + let g = Git::new(&d, g.bin.clone(), 1).unwrap().with_limits( + Duration::from_secs(10), + Duration::from_millis(80), + 1 << 20, + ); + let g = Arc::new(g); + let g2 = g.clone(); + let hold = std::thread::spawn(move || { + g2.run_program( + OsStr::new("/bin/sleep"), + &Spec { + args: &["0.6"], + ..Default::default() + }, + ) + }); + std::thread::sleep(Duration::from_millis(100)); + let err = g + .run(&Spec { + args: &["--version"], + ..Default::default() + }) + .unwrap_err(); + assert!(matches!(err, GitError::Busy), "{err:?}"); + assert!(hold.join().unwrap().is_ok()); + let _ = std::fs::remove_dir_all(d); + } +} diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 758a6130..3da93e88 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -9,6 +9,7 @@ pub mod admin; pub mod api; pub mod auth; pub mod db; +pub mod gitproc; pub mod http; pub mod limits; pub mod names; diff --git a/crates/forge/src/limits.rs b/crates/forge/src/limits.rs index 8368cb6d..bdc201e9 100644 --- a/crates/forge/src/limits.rs +++ b/crates/forge/src/limits.rs @@ -24,6 +24,26 @@ impl Gate { } } + /// Wait up to `deadline` for a slot; `None` when none became free (the caller answers + /// 503 instead of queueing without bound). + pub fn acquire_timeout(&self, deadline: Duration) -> Option> { + let end = Instant::now() + deadline; + let mut n = self.free.lock().unwrap_or_else(|e| e.into_inner()); + while *n == 0 { + let left = end.checked_duration_since(Instant::now())?; + let (g, res) = self + .cv + .wait_timeout(n, left) + .unwrap_or_else(|e| e.into_inner()); + n = g; + if res.timed_out() && *n == 0 { + return None; + } + } + *n -= 1; + Some(Permit(self)) + } + /// Block until a slot is free. pub fn acquire(&self) -> Permit<'_> { let mut n = self.free.lock().unwrap_or_else(|e| e.into_inner()); @@ -376,6 +396,17 @@ mod tests { ); } + #[test] + fn gate_acquire_times_out_instead_of_queueing() { + let g = Gate::new(1); + let held = g.acquire(); + let t = Instant::now(); + assert!(g.acquire_timeout(Duration::from_millis(60)).is_none()); + assert!(t.elapsed() >= Duration::from_millis(55)); + drop(held); + assert!(g.acquire_timeout(Duration::from_millis(60)).is_some()); + } + #[test] fn gate_bounds_concurrency() { let g = Gate::new(1); From c330f73b656de91b781c9e7f60d7f464d7deabf4 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:26:51 +0300 Subject: [PATCH 07/16] feat(forge): F2a lifecycle, reconciler, process-group-safe runner (wip) --- crates/forge/src/gitproc.rs | 87 +++- crates/forge/src/lib.rs | 1 + crates/forge/src/lifecycle.rs | 903 ++++++++++++++++++++++++++++++++++ 3 files changed, 976 insertions(+), 15 deletions(-) create mode 100644 crates/forge/src/lifecycle.rs diff --git a/crates/forge/src/gitproc.rs b/crates/forge/src/gitproc.rs index e7fd0ea3..95d14849 100644 --- a/crates/forge/src/gitproc.rs +++ b/crates/forge/src/gitproc.rs @@ -225,25 +225,32 @@ impl Git { }) }); + // Detect the leader's exit without reaping it (`waitid` with WNOWAIT): while the leader is + // an unreaped zombie its pid, and so the process-group id, cannot be reused, so the group + // signal below can never reach an unrelated group. Reap only after signalling. let deadline = Instant::now() + self.timeout; - let status = loop { - if let Some(st) = child.try_wait()? { - break Ok(st); + let outcome = loop { + match exited_unreaped(pgid) { + Ok(true) => break None, + Ok(false) => {} + Err(e) => { + kill_group(); + let _ = child.wait(); + return Err(e.into()); + } } if over.load(Ordering::SeqCst) { - kill_group(); - let _ = child.wait(); - break Err(GitError::OutputTooLarge); + break Some(GitError::OutputTooLarge); } if Instant::now() >= deadline { - kill_group(); - let _ = child.wait(); - break Err(GitError::Timeout { stdout: Vec::new() }); + break Some(GitError::Timeout { stdout: Vec::new() }); } std::thread::sleep(Duration::from_millis(2)); }; - // The group may outlive the leader (a hook's background child): end it on every path. + // Whatever the reason we stopped, end the whole group (a hook's background child may + // outlive the leader) while the leader still holds the group id, then reap it. kill_group(); + let status = child.wait()?; if let Some(t) = t_in { let _ = t.join(); } @@ -251,19 +258,45 @@ impl Git { t_out.join().unwrap_or_default(), t_err.join().unwrap_or_default(), ); - match status { - Ok(status) if !over.load(Ordering::SeqCst) => Ok(Out { + match outcome { + None if !over.load(Ordering::SeqCst) => Ok(Out { status, stdout, stderr, }), - Ok(_) => Err(GitError::OutputTooLarge), - Err(GitError::Timeout { .. }) => Err(GitError::Timeout { stdout }), - Err(e) => Err(e), + None | Some(GitError::OutputTooLarge) => Err(GitError::OutputTooLarge), + Some(GitError::Timeout { .. }) => Err(GitError::Timeout { stdout }), + Some(e) => Err(e), } } } +/// Has child `pid` exited? Uses `waitid(WEXITED | WNOHANG | WNOWAIT)`, which reports exit +/// without reaping, so the pid stays reserved until the caller waits on the child. +fn exited_unreaped(pid: i32) -> std::io::Result { + // SAFETY: an all-zero siginfo_t is a valid value for waitid to fill in. + let mut info: libc::siginfo_t = unsafe { std::mem::zeroed() }; + // SAFETY: `info` is a valid out-pointer; the options only observe, never reap. + let r = unsafe { + libc::waitid( + libc::P_PID, + pid as libc::id_t, + &mut info, + libc::WEXITED | libc::WNOHANG | libc::WNOWAIT, + ) + }; + if r != 0 { + return Err(std::io::Error::last_os_error()); + } + // With WNOHANG, si_pid stays 0 when no child has changed state. + #[cfg(target_os = "linux")] + // SAFETY: waitid filled the siginfo for a SIGCHLD-class report. + let pid_set = unsafe { info.si_pid() } != 0; + #[cfg(not(target_os = "linux"))] + let pid_set = info.si_pid != 0; + Ok(pid_set) +} + /// Find `git`: `SC_FORGE_GIT` if set, else the first `git` on the server's own `PATH`. pub fn find_git() -> Option { if let Some(p) = std::env::var_os("SC_FORGE_GIT") { @@ -499,6 +532,30 @@ mod tests { let _ = std::fs::remove_dir_all(d); } + #[test] + fn a_normal_exit_still_ends_a_lingering_background_child() { + let (g, d) = git("linger"); + // The leader exits at once, leaving a grandchild that would run for a minute. + let out = g + .run_program( + OsStr::new("/bin/sh"), + &Spec { + args: &["-c", "sleep 60 & echo $!"], + ..Default::default() + }, + ) + .unwrap(); + assert!(out.ok()); + let pid: i32 = out.text().parse().expect("grandchild pid"); + std::thread::sleep(Duration::from_millis(100)); + // SAFETY: signal 0 only probes for existence. + assert!( + unsafe { libc::kill(pid, 0) } != 0, + "the grandchild {pid} must not outlive a run" + ); + let _ = std::fs::remove_dir_all(d); + } + #[test] fn output_over_the_cap_kills_the_child() { let (g, d) = git("cap"); diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 3da93e88..fbecf3e5 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -11,6 +11,7 @@ pub mod auth; pub mod db; pub mod gitproc; pub mod http; +pub mod lifecycle; pub mod limits; pub mod names; pub mod refname; diff --git a/crates/forge/src/lifecycle.rs b/crates/forge/src/lifecycle.rs new file mode 100644 index 00000000..2471022e --- /dev/null +++ b/crates/forge/src/lifecycle.rs @@ -0,0 +1,903 @@ +//! Repository lifecycle: create, delete, change the default branch, and the startup reconciler. +//! +//! Directories are named by the immutable repository id (`repos/.git`), so deleting and +//! recreating a name yields a new directory a straggler cannot see. Every multi-step operation +//! is ordered so a crash at any point leaves a state the reconciler resolves: +//! +//! * create: row `creating` -> `git init` -> optional auto-init commit -> fsync the tree -> row `live` +//! * delete: row `deleting` -> rename into `trash/` -> fsync parents -> delete row -> remove trash +//! +//! `serve` holds the [`DirLock`] for its lifetime, so no second process can reconcile or mutate +//! the same directory, and a per-repository mutex serializes the server's own operations on one id. + +use crate::gitproc::{Git, GitError, Identity, Spec}; +use crate::refname; +use crate::repos::{self, Repo, RepoPatch}; +use crate::store::{StoreError, User}; +use rusqlite::Connection; +use std::collections::HashMap; +use std::fs::File; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +/// Exclusive hold on the data directory for the life of the process. +pub struct DirLock(#[allow(dead_code)] File); + +impl DirLock { + /// Take the lock, or fail if another process holds it. + pub fn acquire(data_dir: &Path) -> Result { + std::fs::create_dir_all(data_dir).map_err(|e| e.to_string())?; + let f = std::fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(data_dir.join(".lock")) + .map_err(|e| e.to_string())?; + f.try_lock().map_err(|_| { + format!( + "another sc-forge process holds {}; refusing to share the data directory", + data_dir.display() + ) + })?; + Ok(DirLock(f)) + } +} + +/// Why a lifecycle operation failed. +#[derive(Debug, thiserror::Error)] +pub enum LifecycleError { + #[error("database error: {0}")] + Db(#[from] rusqlite::Error), + #[error("io error: {0}")] + Io(#[from] std::io::Error), + #[error("{0}")] + Git(#[from] GitError), + /// The name is taken (any letter case, any state). + #[error("exists")] + Exists, + /// The repository is not `live` any more (deleted or being deleted). + #[error("gone")] + Gone, + /// The branch does not exist. + #[error("no such branch")] + NoSuchBranch, + /// The default branch name is not a valid branch name. + #[error("invalid branch name")] + BadBranch, + /// A git command that must succeed failed. + #[error("git failed: {0}")] + GitFailed(String), + /// A test-injected crash: the operation stopped here and cleaned nothing up. + #[error("injected crash at {0}")] + Crash(&'static str), +} + +/// What the reconciler did. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Report { + pub creating_rolled_back: usize, + pub deleting_finished: usize, + pub trash_removed: usize, + pub orphans_moved: usize, + pub head_repaired: usize, + pub missing_dirs: usize, +} + +/// Owns the on-disk layout and the per-repository serialization. +pub struct Lifecycle { + data_dir: PathBuf, + pub git: Arc, + locks: Mutex>>>, + #[cfg(test)] + crash_at: Mutex>, +} + +impl Lifecycle { + pub fn new(data_dir: &Path, git: Arc) -> std::io::Result { + for d in ["repos", "trash", "orphans"] { + std::fs::create_dir_all(data_dir.join(d))?; + } + Ok(Lifecycle { + data_dir: data_dir.to_path_buf(), + git, + locks: Mutex::default(), + #[cfg(test)] + crash_at: Mutex::default(), + }) + } + + /// `repos/.git`: always built from the integer id. + pub fn repo_dir(&self, id: i64) -> PathBuf { + self.data_dir.join("repos").join(format!("{id}.git")) + } + + fn trash_dir(&self, id: i64) -> PathBuf { + self.data_dir.join("trash").join(format!("{id}.git")) + } + + /// The mutex that serializes the server's operations on repository `id`. + pub fn lock_for(&self, id: i64) -> Arc> { + self.locks + .lock() + .unwrap_or_else(|e| e.into_inner()) + .entry(id) + .or_default() + .clone() + } + + #[cfg(test)] + fn arm(&self, step: &'static str) { + *self.crash_at.lock().unwrap() = Some(step); + } + + /// A crash point: in tests, an armed step stops the operation without cleanup. + #[allow(unused_variables)] + fn step(&self, name: &'static str) -> Result<(), LifecycleError> { + #[cfg(test)] + { + let mut armed = self.crash_at.lock().unwrap(); + if *armed == Some(name) { + *armed = None; // one-shot: a retry after the "restart" runs normally + return Err(LifecycleError::Crash(name)); + } + } + Ok(()) + } + + fn run( + &self, + repo: &Path, + args: &[&str], + stdin: Option<&[u8]>, + id: Option<&Identity>, + ) -> Result { + let out = self.git.run(&Spec { + repo: Some(repo), + args, + stdin, + identity: id, + })?; + if out.ok() { + Ok(out.text()) + } else { + Err(LifecycleError::GitFailed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )) + } + } + + /// Create a repository owned by `owner` (the caller has already authorized it) and, with + /// `auto_init`, its first commit authored by `actor`. A failure before the row is `live` + /// rolls everything back in-process, so a failed create never blocks the name. + #[allow(clippy::too_many_arguments)] + pub fn create( + &self, + conn: &Connection, + owner: &User, + actor: &User, + name: &str, + description: &str, + default_branch: &str, + auto_init: bool, + now: i64, + ) -> Result { + if !refname::valid_branch(default_branch) { + return Err(LifecycleError::BadBranch); + } + let row = + repos::insert_creating(conn, owner.id, name, default_branch, description, "", now) + .map_err(|e| match e { + StoreError::Exists => LifecycleError::Exists, + StoreError::Db(d) => LifecycleError::Db(d), + StoreError::NotFound => LifecycleError::Gone, + })?; + match self.build(conn, &row, actor, auto_init, now) { + Ok(()) => repos::find_by_id(conn, row.id)?.ok_or(LifecycleError::Gone), + Err(e @ LifecycleError::Crash(_)) => Err(e), // a crash cleans nothing up: the reconciler does + Err(e) => { + // An ordinary failure rolls back now; if even that fails the row stays `creating` + // and the next startup finishes the job. + let _ = remove_any(&self.repo_dir(row.id)); + let _ = repos::delete_row(conn, row.id); + Err(e) + } + } + } + + fn build( + &self, + conn: &Connection, + row: &Repo, + actor: &User, + auto_init: bool, + now: i64, + ) -> Result<(), LifecycleError> { + self.step("inserted")?; + let dir = self.repo_dir(row.id); + let out = self.git.run(&Spec { + args: &[ + "init", + "--bare", + "--quiet", + "--template=", + &format!("--initial-branch={}", row.default_branch), + &dir.display().to_string(), + ], + ..Default::default() + })?; + if !out.ok() { + return Err(LifecycleError::GitFailed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )); + } + self.step("initialized")?; + if auto_init { + let email = if actor.email.is_empty() { + format!("{}@noreply.localhost", actor.login) + } else { + actor.email.clone() + }; + let id = Identity::new(&actor.login, &email) + .ok_or_else(|| LifecycleError::GitFailed("unusable commit identity".into()))?; + let readme = format!("# {}\n\n", row.name); + let blob = self.run( + &dir, + &["hash-object", "-w", "--no-filters", "--stdin"], + Some(readme.as_bytes()), + None, + )?; + let tree = self.run( + &dir, + &["mktree"], + Some(format!("100644 blob {blob}\tREADME.md\n").as_bytes()), + None, + )?; + let commit = self.run( + &dir, + &["commit-tree", &tree], + Some(b"Initial commit\n"), + Some(&id), + )?; + let branch_ref = refname::branch_ref(&row.default_branch); + self.run( + &dir, + &[ + "update-ref", + &branch_ref, + &commit, + "0000000000000000000000000000000000000000", + ], + None, + None, + )?; + } + self.step("committed")?; + fsync_tree(&dir)?; + fsync_dir(&self.data_dir.join("repos"))?; + self.step("synced")?; + if !repos::set_state(conn, row.id, "creating", "live", now)? { + return Err(LifecycleError::Gone); + } + Ok(()) + } + + /// Delete a repository: `deleting` -> rename into `trash/` -> fsync -> row delete -> remove trash. + /// `Gone` when it was not `live` (someone else is already deleting it). + pub fn delete(&self, conn: &Connection, repo: &Repo, now: i64) -> Result<(), LifecycleError> { + let lock = self.lock_for(repo.id); + let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + if !repos::set_state(conn, repo.id, "live", "deleting", now)? { + return Err(LifecycleError::Gone); + } + self.step("marked")?; + let (from, to) = (self.repo_dir(repo.id), self.trash_dir(repo.id)); + if let Err(e) = std::fs::rename(&from, &to) { + // Nothing moved: put the row back so the repository is usable again. + let _ = repos::set_state(conn, repo.id, "deleting", "live", now); + return Err(e.into()); + } + self.step("renamed")?; + fsync_dir(&self.data_dir.join("repos"))?; + fsync_dir(&self.data_dir.join("trash"))?; + repos::delete_row(conn, repo.id)?; + self.step("row-deleted")?; + if let Err(e) = std::fs::remove_dir_all(&to) { + eprintln!( + "sc-forge: could not remove trash/{}.git ({e}); startup reconcile will", + repo.id + ); + } + self.locks + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(&repo.id); + Ok(()) + } + + /// Make `branch` the default: it must exist; bare `HEAD` is written first, then the row. + pub fn set_default_branch( + &self, + conn: &Connection, + repo: &Repo, + branch: &str, + now: i64, + ) -> Result<(), LifecycleError> { + let lock = self.lock_for(repo.id); + let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + let current = repos::find_by_id(conn, repo.id)? + .filter(|r| r.state == "live") + .ok_or(LifecycleError::Gone)?; + let dir = self.repo_dir(current.id); + let full = refname::branch_ref(branch); + let exists = self.git.run(&Spec { + repo: Some(&dir), + args: &["show-ref", "--verify", "--quiet", &full], + ..Default::default() + })?; + if !exists.ok() { + return Err(LifecycleError::NoSuchBranch); + } + self.run(&dir, &["symbolic-ref", "HEAD", &full], None, None)?; + self.step("head-written")?; + let patch = RepoPatch { + default_branch: Some(branch.to_string()), + ..Default::default() + }; + if !repos::update_live(conn, current.id, &patch, now)? { + return Err(LifecycleError::Gone); + } + Ok(()) + } + + /// Delete a branch (the caller has refused the default branch). `NoSuchBranch` when absent. + pub fn delete_branch( + &self, + conn: &Connection, + repo: &Repo, + branch: &str, + ) -> Result<(), LifecycleError> { + let lock = self.lock_for(repo.id); + let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + let current = repos::find_by_id(conn, repo.id)? + .filter(|r| r.state == "live") + .ok_or(LifecycleError::Gone)?; + if current.default_branch == branch { + return Err(LifecycleError::GitFailed( + "refusing to delete the default branch".into(), + )); + } + let dir = self.repo_dir(current.id); + let full = refname::branch_ref(branch); + let exists = self.git.run(&Spec { + repo: Some(&dir), + args: &["show-ref", "--verify", "--quiet", &full], + ..Default::default() + })?; + if !exists.ok() { + return Err(LifecycleError::NoSuchBranch); + } + self.run(&dir, &["update-ref", "-d", &full], None, None)?; + Ok(()) + } + + /// Resolve every state a crash can leave. Run once at startup, before the listener is bound + /// and while holding the [`DirLock`]. Nothing is ever deleted that a live repository might + /// own: a directory with no row at all moves to `orphans/`. + pub fn reconcile(&self, conn: &Connection, now: i64) -> Result { + let mut rep = Report::default(); + // 1. creating: roll back (the row does not record auto_init, so a half-initialized + // repository is never promoted; the client never saw a 201 for it). + for r in repos::list_in_state(conn, "creating")? { + let _ = remove_any(&self.repo_dir(r.id)); + repos::delete_row(conn, r.id)?; + rep.creating_rolled_back += 1; + } + // 2. deleting: resume at the rename, then finish. + for r in repos::list_in_state(conn, "deleting")? { + let (from, to) = (self.repo_dir(r.id), self.trash_dir(r.id)); + if from.exists() { + if to.exists() { + std::fs::remove_dir_all(&to)?; + } + std::fs::rename(&from, &to)?; + fsync_dir(&self.data_dir.join("repos"))?; + fsync_dir(&self.data_dir.join("trash"))?; + } + repos::delete_row(conn, r.id)?; + let _ = std::fs::remove_dir_all(&to); + rep.deleting_finished += 1; + } + // 3. trash with no row. + for e in std::fs::read_dir(self.data_dir.join("trash"))? { + let p = e?.path(); + let known = file_id(&p) + .and_then(|id| repos::find_by_id(conn, id).ok().flatten()) + .is_some(); + if !known { + remove_any(&p)?; + rep.trash_removed += 1; + } + } + // 4. a directory in repos/ with no row at all: keep it, out of the way, never delete it. + for e in std::fs::read_dir(self.data_dir.join("repos"))? { + let p = e?.path(); + let known = file_id(&p) + .and_then(|id| repos::find_by_id(conn, id).ok().flatten()) + .is_some(); + if !known { + let name = p + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_default(); + let mut to = self.data_dir.join("orphans").join(&name); + let mut n = 0; + while to.exists() { + n += 1; + to = self.data_dir.join("orphans").join(format!("{name}.{n}")); + } + std::fs::rename(&p, &to)?; + eprintln!( + "sc-forge: {} has no repository row; moved to {}", + p.display(), + to.display() + ); + rep.orphans_moved += 1; + } + } + // 5. live: HEAD follows the row (the row is the authority); a missing directory is + // reported, never silently dropped. + for r in repos::list_in_state(conn, "live")? { + let dir = self.repo_dir(r.id); + if !dir.is_dir() { + eprintln!( + "sc-forge: repository {}/{} (id {}) has no directory", + r.owner, r.name, r.id + ); + rep.missing_dirs += 1; + continue; + } + let want = refname::branch_ref(&r.default_branch); + let head = self.git.run(&Spec { + repo: Some(&dir), + args: &["symbolic-ref", "-q", "HEAD"], + ..Default::default() + })?; + if head.text() != want { + self.run(&dir, &["symbolic-ref", "HEAD", &want], None, None)?; + rep.head_repaired += 1; + } + } + let _ = now; + Ok(rep) + } +} + +/// Remove a path whether it is a directory or something that is not (a stray file). +fn remove_any(p: &Path) -> std::io::Result<()> { + match std::fs::symlink_metadata(p) { + Ok(m) if m.is_dir() => std::fs::remove_dir_all(p), + Ok(_) => std::fs::remove_file(p), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e), + } +} + +/// The id in a `.git` directory name. +fn file_id(p: &Path) -> Option { + p.file_name()?.to_str()?.strip_suffix(".git")?.parse().ok() +} + +/// fsync a directory (so its entries are durable). +pub fn fsync_dir(p: &Path) -> std::io::Result<()> { + File::open(p)?.sync_all() +} + +/// fsync every file and directory under `p`, depth first, propagating any failure. +pub fn fsync_tree(p: &Path) -> std::io::Result<()> { + for e in std::fs::read_dir(p)? { + let e = e?; + if e.file_type()?.is_dir() { + fsync_tree(&e.path())?; + } else { + File::open(e.path())?.sync_all()?; + } + } + fsync_dir(p) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db; + use crate::gitproc::find_git; + use crate::store::{create_account, NewAccount}; + + struct Env { + dir: PathBuf, + conn: Connection, + lc: Lifecycle, + owner: User, + } + impl Drop for Env { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } + } + + fn env(tag: &str) -> Env { + let dir = std::env::temp_dir().join(format!("scl-forge-life-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let conn = db::open_for_admin(&dir).unwrap(); + let git = Arc::new(Git::new(&dir, find_git().expect("git"), 4).unwrap()); + let lc = Lifecycle::new(&dir, git).unwrap(); + let owner = create_account( + &conn, + &NewAccount { + login: "owner".into(), + email: "owner@example.invalid".into(), + visibility: "public".into(), + ..Default::default() + }, + 1, + ) + .unwrap(); + Env { + dir, + conn, + lc, + owner, + } + } + + fn make(e: &Env, name: &str, init: bool) -> Result { + e.lc.create(&e.conn, &e.owner, &e.owner, name, "", "main", init, 10) + } + + fn head(e: &Env, r: &Repo) -> String { + e.lc.git + .run(&Spec { + repo: Some(&e.lc.repo_dir(r.id)), + args: &["symbolic-ref", "HEAD"], + ..Default::default() + }) + .unwrap() + .text() + } + + fn rev(e: &Env, r: &Repo, what: &str) -> Option { + let o = + e.lc.git + .run(&Spec { + repo: Some(&e.lc.repo_dir(r.id)), + args: &["rev-parse", "--verify", "-q", what], + ..Default::default() + }) + .unwrap(); + o.ok().then(|| o.text()) + } + + #[test] + fn create_with_auto_init_makes_a_bare_repo_with_one_commit_by_the_actor() { + let e = env("autoinit"); + let r = make(&e, "alpha", true).unwrap(); + assert_eq!(r.state, "live"); + let dir = e.lc.repo_dir(r.id); + assert!( + dir.join("HEAD").is_file() && !dir.join(".git").exists(), + "a bare repo named by id" + ); + assert!( + !dir.join("hooks").exists() + || std::fs::read_dir(dir.join("hooks")).unwrap().count() == 0, + "no template hooks" + ); + assert_eq!(head(&e, &r), "refs/heads/main"); + let log = + e.lc.git + .run(&Spec { + repo: Some(&dir), + args: &["log", "-1", "--format=%an|%ae|%cn|%s", "main"], + ..Default::default() + }) + .unwrap() + .text(); + assert_eq!(log, "owner|owner@example.invalid|owner|Initial commit"); + let blob = + e.lc.git + .run(&Spec { + repo: Some(&dir), + args: &["show", "main:README.md"], + ..Default::default() + }) + .unwrap() + .text(); + assert_eq!(blob, "# alpha", "recorded 8-byte README `# \\n\\n`"); + } + + #[test] + fn create_without_auto_init_is_empty_and_head_names_the_default_branch() { + let e = env("empty"); + let r = + e.lc.create(&e.conn, &e.owner, &e.owner, "blank", "", "trunk", false, 10) + .unwrap(); + assert_eq!(head(&e, &r), "refs/heads/trunk"); + assert!(rev(&e, &r, "HEAD").is_none(), "unborn: empty"); + } + + #[test] + fn a_duplicate_name_in_any_case_is_refused_and_a_failed_create_does_not_block_it() { + let e = env("dup"); + make(&e, "Mixed", false).unwrap(); + assert!(matches!( + make(&e, "mixed", false), + Err(LifecycleError::Exists) + )); + assert!(matches!( + e.lc.create( + &e.conn, + &e.owner, + &e.owner, + "zed", + "", + "bad..branch", + false, + 10 + ), + Err(LifecycleError::BadBranch) + )); + assert!( + repos::find_row(&e.conn, "owner", "zed").unwrap().is_none(), + "an invalid branch never inserts a row" + ); + // A create whose git step fails (the directory is occupied by a file) rolls back + // in-process and frees the name. + std::fs::write(e.lc.repo_dir(2), b"in the way").unwrap(); + assert!(matches!( + make(&e, "zed", false), + Err(LifecycleError::GitFailed(_)) + )); + assert!( + repos::find_row(&e.conn, "owner", "zed").unwrap().is_none(), + "row rolled back" + ); + assert!(!e.lc.repo_dir(2).exists(), "the leftover is removed"); + assert!( + make(&e, "zed", false).is_ok(), + "the name is reusable at once" + ); + } + + #[test] + fn a_crash_at_every_create_step_is_rolled_back_by_the_reconciler() { + for step in ["inserted", "initialized", "committed", "synced"] { + let e = env(&format!("crash-create-{step}")); + e.lc.arm(step); + let err = make(&e, "doomed", true).unwrap_err(); + assert!( + matches!(err, LifecycleError::Crash(s) if s == step), + "{err:?}" + ); + let row = repos::find_row(&e.conn, "owner", "doomed") + .unwrap() + .unwrap(); + assert_eq!(row.state, "creating"); + // "restart": a fresh runner over the same directory + let rep = e.lc.reconcile(&e.conn, 20).unwrap(); + assert_eq!(rep.creating_rolled_back, 1, "{step}"); + assert!( + repos::find_row(&e.conn, "owner", "doomed") + .unwrap() + .is_none(), + "{step}: row gone" + ); + assert!(!e.lc.repo_dir(row.id).exists(), "{step}: directory gone"); + assert!( + make(&e, "doomed", true).is_ok(), + "{step}: the name is free and a retry gets a full repo" + ); + } + } + + #[test] + fn delete_moves_to_trash_then_removes_and_a_recreated_name_gets_a_new_id() { + let e = env("delete"); + let r = make(&e, "doomed", true).unwrap(); + e.lc.delete(&e.conn, &r, 20).unwrap(); + assert!(repos::find_by_id(&e.conn, r.id).unwrap().is_none()); + assert!(!e.lc.repo_dir(r.id).exists() && !e.lc.trash_dir(r.id).exists()); + assert!( + matches!(e.lc.delete(&e.conn, &r, 21), Err(LifecycleError::Gone)), + "second delete" + ); + let again = make(&e, "doomed", true).unwrap(); + assert_ne!( + again.id, r.id, + "a recreated name is a new repository in a new directory" + ); + } + + #[test] + fn a_crash_at_every_delete_step_is_finished_by_the_reconciler() { + for step in ["marked", "renamed", "row-deleted"] { + let e = env(&format!("crash-delete-{step}")); + let r = make(&e, "doomed", true).unwrap(); + e.lc.arm(step); + assert!( + matches!(e.lc.delete(&e.conn, &r, 20), Err(LifecycleError::Crash(_))), + "{step}" + ); + e.lc.reconcile(&e.conn, 30).unwrap(); + assert!( + repos::find_by_id(&e.conn, r.id).unwrap().is_none(), + "{step}: row gone" + ); + assert!( + !e.lc.repo_dir(r.id).exists() && !e.lc.trash_dir(r.id).exists(), + "{step}: no directory left" + ); + assert!(make(&e, "doomed", false).is_ok(), "{step}: name reusable"); + } + } + + #[test] + fn reconcile_handles_the_remaining_table_rows() { + let e = env("table"); + // a deleting row with neither directory + let r = make(&e, "ghost", false).unwrap(); + std::fs::remove_dir_all(e.lc.repo_dir(r.id)).unwrap(); + repos::set_state(&e.conn, r.id, "live", "deleting", 5).unwrap(); + // trash with no row, and a directory with no row at all + std::fs::create_dir_all(e.lc.trash_dir(99).join("objects")).unwrap(); + std::fs::create_dir_all(e.lc.repo_dir(98).join("objects")).unwrap(); + std::fs::write(e.lc.repo_dir(98).join("precious"), b"data").unwrap(); + // a live row whose directory is missing is reported and left + let lost = make(&e, "lost", false).unwrap(); + std::fs::remove_dir_all(e.lc.repo_dir(lost.id)).unwrap(); + // a live row whose HEAD disagrees with the row is repaired from the row + let drift = + e.lc.create(&e.conn, &e.owner, &e.owner, "drift", "", "main", true, 10) + .unwrap(); + e.lc.git + .run(&Spec { + repo: Some(&e.lc.repo_dir(drift.id)), + args: &["symbolic-ref", "HEAD", "refs/heads/other"], + ..Default::default() + }) + .unwrap(); + + let rep = e.lc.reconcile(&e.conn, 40).unwrap(); + assert_eq!( + rep, + Report { + creating_rolled_back: 0, + deleting_finished: 1, + trash_removed: 1, + orphans_moved: 1, + head_repaired: 1, + missing_dirs: 1 + } + ); + assert!(repos::find_by_id(&e.conn, r.id).unwrap().is_none()); + assert!(!e.lc.trash_dir(99).exists()); + assert!( + !e.lc.repo_dir(98).exists() + && e.dir + .join("orphans") + .join("98.git") + .join("precious") + .is_file(), + "an unknown directory is moved, never deleted" + ); + assert!( + repos::find_by_id(&e.conn, lost.id).unwrap().is_some(), + "a live row is never dropped" + ); + assert_eq!(head(&e, &drift), "refs/heads/main"); + } + + #[test] + fn a_restored_empty_database_never_erases_repositories() { + let e = env("restore"); + let r = make(&e, "precious", true).unwrap(); + let dir = e.lc.repo_dir(r.id); + // lose the database: a fresh, empty one over the same directories + std::fs::remove_file(db::db_path(&e.dir)).unwrap(); + let conn = db::open_for_admin(&e.dir).unwrap(); + let lc = Lifecycle::new(&e.dir, e.lc.git.clone()).unwrap(); + let rep = lc.reconcile(&conn, 50).unwrap(); + assert_eq!(rep.orphans_moved, 1); + assert!( + e.dir + .join("orphans") + .join(format!("{}.git", r.id)) + .join("HEAD") + .is_file(), + "the repository survives in orphans/" + ); + assert!(!dir.exists()); + } + + #[test] + fn set_default_branch_requires_an_existing_branch_and_writes_head_then_row() { + let e = env("default"); + let r = make(&e, "alpha", true).unwrap(); + let dir = e.lc.repo_dir(r.id); + assert!(matches!( + e.lc.set_default_branch(&e.conn, &r, "dev", 20), + Err(LifecycleError::NoSuchBranch) + )); + assert!( + matches!( + e.lc.set_default_branch(&e.conn, &r, "main~1", 20), + Err(LifecycleError::NoSuchBranch) + ), + "a revision is not a branch" + ); + let main = rev(&e, &r, "main").unwrap(); + e.lc.run(&dir, &["update-ref", "refs/heads/dev", &main], None, None) + .unwrap(); + e.lc.set_default_branch(&e.conn, &r, "dev", 20).unwrap(); + assert_eq!(head(&e, &r), "refs/heads/dev"); + assert_eq!( + repos::find_by_id(&e.conn, r.id) + .unwrap() + .unwrap() + .default_branch, + "dev" + ); + // after a crash between HEAD and the row, the row wins at the next startup + e.lc.arm("head-written"); + let before = repos::find_by_id(&e.conn, r.id).unwrap().unwrap(); + e.lc.run( + &dir, + &["update-ref", "refs/heads/feat/x", &main], + None, + None, + ) + .unwrap(); + assert!(matches!( + e.lc.set_default_branch(&e.conn, &before, "feat/x", 21), + Err(LifecycleError::Crash(_)) + )); + assert_eq!(head(&e, &r), "refs/heads/feat/x"); + let rep = e.lc.reconcile(&e.conn, 30).unwrap(); + assert_eq!(rep.head_repaired, 1); + assert_eq!(head(&e, &r), "refs/heads/dev"); + } + + #[test] + fn branch_delete_refuses_the_default_and_needs_the_branch_to_exist() { + let e = env("branchdel"); + let r = make(&e, "alpha", true).unwrap(); + let main = rev(&e, &r, "main").unwrap(); + e.lc.run( + &e.lc.repo_dir(r.id), + &["update-ref", "refs/heads/dev", &main], + None, + None, + ) + .unwrap(); + assert!( + e.lc.delete_branch(&e.conn, &r, "main").is_err(), + "the default branch stays" + ); + e.lc.delete_branch(&e.conn, &r, "dev").unwrap(); + assert!(rev(&e, &r, "refs/heads/dev").is_none()); + assert!(matches!( + e.lc.delete_branch(&e.conn, &r, "dev"), + Err(LifecycleError::NoSuchBranch) + )); + } + + #[test] + fn a_second_process_cannot_share_the_data_directory() { + let dir = std::env::temp_dir().join(format!("scl-forge-dirlock-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let first = DirLock::acquire(&dir).unwrap(); + // flock is per open file description, so a second open in this process conflicts too + assert!(DirLock::acquire(&dir).is_err()); + drop(first); + assert!(DirLock::acquire(&dir).is_ok()); + let _ = std::fs::remove_dir_all(&dir); + } +} From e80dff8aa8f1c91c97894a1ec131cb9a14a8d49e Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:32:00 +0300 Subject: [PATCH 08/16] feat(forge): F2a repository API, collaborators, branches, router actor and tail capture (wip) --- crates/forge/src/api.rs | 27 +- crates/forge/src/cli.rs | 23 +- crates/forge/src/gitread.rs | 274 ++++++++++++++ crates/forge/src/lib.rs | 4 + crates/forge/src/repo_api.rs | 590 +++++++++++++++++++++++++++++++ crates/forge/src/routes.rs | 203 ++++++++++- crates/forge/src/server.rs | 3 +- crates/forge/tests/common/mod.rs | 10 +- crates/forge/tests/e2e_a.rs | 11 +- 9 files changed, 1118 insertions(+), 27 deletions(-) create mode 100644 crates/forge/src/gitread.rs create mode 100644 crates/forge/src/repo_api.rs diff --git a/crates/forge/src/api.rs b/crates/forge/src/api.rs index f35ccc5c..bc2ff69a 100644 --- a/crates/forge/src/api.rs +++ b/crates/forge/src/api.rs @@ -16,7 +16,7 @@ pub const GITEA_VERSION: &str = "1.22.6"; const DEFAULT_PAGE: i64 = 30; const MAX_PAGE: i64 = 50; -fn user_json(base: &str, u: &User) -> Value { +pub(crate) fn user_json(base: &str, u: &User) -> Value { json!({ "id": u.id, "login": u.login, @@ -44,7 +44,7 @@ fn user_json(base: &str, u: &User) -> Value { }) } -fn org_json(base: &str, o: &User) -> Value { +pub(crate) fn org_json(base: &str, o: &User) -> Value { json!({ "id": o.id, "name": o.login, @@ -60,7 +60,7 @@ fn org_json(base: &str, o: &User) -> Value { }) } -fn no_such_user(ctx: &Ctx, name: &str) -> Resp { +pub(crate) fn no_such_user(ctx: &Ctx, name: &str) -> Resp { error( &ctx.app.base, 404, @@ -77,12 +77,12 @@ pub fn org_missing(app: &crate::routes::App, name: &str) -> Resp { ) } -fn no_such_org(ctx: &Ctx, name: &str) -> Resp { +pub(crate) fn no_such_org(ctx: &Ctx, name: &str) -> Resp { org_missing(ctx.app, name) } /// A JSON object body; an empty body is `{}`. -fn body(ctx: &Ctx) -> Result { +pub(crate) fn body(ctx: &Ctx) -> Result { if ctx.req.body.is_empty() { return Ok(json!({})); } @@ -92,11 +92,11 @@ fn body(ctx: &Ctx) -> Result { } } -fn str_field(v: &Value, key: &str) -> Option { +pub(crate) fn str_field(v: &Value, key: &str) -> Option { v.get(key).and_then(Value::as_str).map(String::from) } -fn page_params(ctx: &Ctx) -> (i64, i64) { +pub(crate) fn page_params(ctx: &Ctx) -> (i64, i64) { let num = |k: &str| ctx.req.param(k).and_then(|v| v.parse::().ok()); // Page is clamped so offset arithmetic cannot overflow. let page = num("page") @@ -111,7 +111,14 @@ fn page_params(ctx: &Ctx) -> (i64, i64) { } /// 200 with `X-Total-Count` and Gitea's `Link` header. -fn paged(ctx: &Ctx, path: &str, total: i64, page: i64, limit: i64, items: Vec) -> Resp { +pub(crate) fn paged( + ctx: &Ctx, + path: &str, + total: i64, + page: i64, + limit: i64, + items: Vec, +) -> Resp { let last = ((total + limit - 1) / limit).max(1); let url = |p: i64, rel: &str| { format!( @@ -168,7 +175,7 @@ pub fn get_user(ctx: &mut Ctx) -> Resp { /// Visible to the caller: public and limited organizations to any signed-in /// caller, private ones to owners and site admins. -fn org_visible(ctx: &Ctx, org: &User) -> rusqlite::Result { +pub(crate) fn org_visible(ctx: &Ctx, org: &User) -> rusqlite::Result { let me = &ctx.who.user; Ok(org.visibility != "private" || me.is_admin || store::is_org_owner(ctx.conn, org.id, me.id)?) } @@ -236,7 +243,7 @@ fn name_error(label: &str, name: &str, e: NameError) -> String { } } -fn valid_visibility(v: &str) -> bool { +pub(crate) fn valid_visibility(v: &str) -> bool { matches!(v, "public" | "limited" | "private") } diff --git a/crates/forge/src/cli.rs b/crates/forge/src/cli.rs index 143a7614..93661273 100644 --- a/crates/forge/src/cli.rs +++ b/crates/forge/src/cli.rs @@ -53,6 +53,9 @@ struct ServeArgs { /// Largest JSON request body, in bytes. #[arg(long, default_value_t = 1 << 20)] max_body_bytes: usize, + /// Concurrent `git` child processes (a busy server answers 503 rather than queueing). + #[arg(long, default_value_t = 8)] + git_concurrency: usize, /// Concurrent password verifications (Argon2 is memory-hard). #[arg(long, default_value_t = 4)] argon2_concurrency: usize, @@ -226,7 +229,25 @@ fn run_serve(a: ServeArgs) -> Result<()> { cidrs, }; db::open_for_serve(&a.data_dir)?; - let app = make_app(a.data_dir, &public_url, proxies, a.argon2_concurrency); + // One server per data directory: the lock is held until this function returns. + let _data_lock = + scl_forge::lifecycle::DirLock::acquire(&a.data_dir).map_err(anyhow::Error::msg)?; + let git_bin = + scl_forge::gitproc::find_git().context("git not found: install git or set SC_FORGE_GIT")?; + let git = std::sync::Arc::new(scl_forge::gitproc::Git::new( + &a.data_dir, + git_bin, + a.git_concurrency, + )?); + let git_version = scl_forge::gitproc::check_version(&git).map_err(anyhow::Error::msg)?; + eprintln!("sc-forge: {git_version}"); + let life = scl_forge::lifecycle::Lifecycle::new(&a.data_dir, git)?; + // Resolve whatever a crash left, before the listener is bound. + let report = life.reconcile(&db::connect(&a.data_dir)?, scl_forge::timefmt::now())?; + if report != scl_forge::lifecycle::Report::default() { + eprintln!("sc-forge: reconciled {report:?}"); + } + let app = make_app(a.data_dir, &public_url, proxies, a.argon2_concurrency, life); let limits = Limits { max_body: a.max_body_bytes, max_connections: a.max_connections, diff --git a/crates/forge/src/gitread.rs b/crates/forge/src/gitread.rs new file mode 100644 index 00000000..9d2b33b3 --- /dev/null +++ b/crates/forge/src/gitread.rs @@ -0,0 +1,274 @@ +//! Read-only views of a bare repository, built on the [`Git`] runner. Branch names reach git +//! only as the full ref `refs/heads/` after [`refname::valid_branch`]; commit data is +//! parsed from the raw commit object, so no format string or message content can confuse the +//! parser. + +use crate::gitproc::{Git, GitError, Spec}; +use crate::refname; +use std::path::Path; + +/// A branch and the commit it points at. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Branch { + pub name: String, + pub oid: String, +} + +/// One person line of a commit (`author`/`committer`). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Person { + pub name: String, + pub email: String, + /// Seconds since the epoch. + pub when: i64, +} + +/// The parts of a commit the API shows. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CommitInfo { + pub oid: String, + /// The full message, as stored (it keeps its trailing newline). + pub message: String, + pub author: Person, + pub committer: Person, +} + +/// All branches, sorted by name. +pub fn list_branches(git: &Git, dir: &Path) -> Result, GitError> { + let out = git.run(&Spec { + repo: Some(dir), + args: &[ + "for-each-ref", + "--sort=refname", + "--format=%(refname)%00%(objectname)", + "refs/heads", + ], + ..Default::default() + })?; + Ok(String::from_utf8_lossy(&out.stdout) + .lines() + .filter_map(|l| { + let (r, oid) = l.split_once('\0')?; + Some(Branch { + name: r.strip_prefix("refs/heads/")?.to_string(), + oid: oid.to_string(), + }) + }) + .collect()) +} + +/// The commit a branch points at, or `None` when the branch does not exist (or the name is not +/// a valid branch name). +pub fn branch_oid(git: &Git, dir: &Path, name: &str) -> Result, GitError> { + if !refname::valid_branch(name) { + return Ok(None); + } + let full = refname::branch_ref(name); + let out = git.run(&Spec { + repo: Some(dir), + args: &["rev-parse", "--verify", "-q", "--end-of-options", &full], + ..Default::default() + })?; + Ok(out.ok().then(|| out.text())) +} + +/// A repository is empty when `HEAD` does not resolve to a commit (computed live). +pub fn is_empty(git: &Git, dir: &Path) -> Result { + let out = git.run(&Spec { + repo: Some(dir), + args: &["rev-parse", "--verify", "-q", "HEAD"], + ..Default::default() + })?; + Ok(!out.ok()) +} + +fn person(line: &str) -> Option { + // `Name 1700000000 +0200` + let (rest, _tz) = line.rsplit_once(' ')?; + let (rest, when) = rest.rsplit_once(' ')?; + let (name, email) = rest.rsplit_once(" <")?; + Some(Person { + name: name.to_string(), + email: email.strip_suffix('>')?.to_string(), + when: when.parse().ok()?, + }) +} + +/// Parse the commit `oid` (a full object id from a prior git answer, never client text). +pub fn commit(git: &Git, dir: &Path, oid: &str) -> Result, GitError> { + let out = git.run(&Spec { + repo: Some(dir), + args: &["cat-file", "commit", oid], + ..Default::default() + })?; + if !out.ok() { + return Ok(None); + } + let text = String::from_utf8_lossy(&out.stdout).to_string(); + let (head, message) = text.split_once("\n\n").unwrap_or((text.as_str(), "")); + let (mut author, mut committer) = (None, None); + for line in head.lines() { + if let Some(v) = line.strip_prefix("author ") { + author = person(v); + } else if let Some(v) = line.strip_prefix("committer ") { + committer = person(v); + } + } + Ok(match (author, committer) { + (Some(author), Some(committer)) => Some(CommitInfo { + oid: oid.to_string(), + message: message.to_string(), + author, + committer, + }), + _ => None, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::gitproc::{find_git, Identity}; + use std::path::PathBuf; + + fn repo(tag: &str) -> (Git, PathBuf, PathBuf) { + let d = + std::env::temp_dir().join(format!("scl-forge-gitread-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + let g = Git::new(&d, find_git().expect("git"), 4).unwrap(); + let dir = d.join("1.git"); + assert!(g + .run(&Spec { + args: &[ + "init", + "--bare", + "--quiet", + "--template=", + "--initial-branch=main", + dir.to_str().unwrap() + ], + ..Default::default() + }) + .unwrap() + .ok()); + (g, d, dir) + } + + fn commit_on(g: &Git, dir: &Path, branch: &str, msg: &str) -> String { + let id = Identity::new("Zoë Q", "zoe@example.invalid").unwrap(); + let tree = g + .run(&Spec { + repo: Some(dir), + args: &["mktree"], + stdin: Some(b""), + ..Default::default() + }) + .unwrap() + .text(); + let oid = g + .run(&Spec { + repo: Some(dir), + args: &["commit-tree", &tree], + stdin: Some(msg.as_bytes()), + identity: Some(&id), + }) + .unwrap() + .text(); + assert!(g + .run(&Spec { + repo: Some(dir), + args: &["update-ref", &refname::branch_ref(branch), &oid], + ..Default::default() + }) + .unwrap() + .ok()); + oid + } + + #[test] + fn an_unborn_repo_is_empty_and_has_no_branches() { + let (g, d, dir) = repo("unborn"); + assert!(is_empty(&g, &dir).unwrap()); + assert_eq!(list_branches(&g, &dir).unwrap(), vec![]); + assert_eq!(branch_oid(&g, &dir, "main").unwrap(), None); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn branches_sort_by_name_and_resolve_only_as_full_refs() { + let (g, d, dir) = repo("branches"); + let a = commit_on(&g, &dir, "main", "first\n"); + let b = commit_on(&g, &dir, "feat/x", "feat two\n"); + assert!(!is_empty(&g, &dir).unwrap()); + assert_eq!( + list_branches(&g, &dir).unwrap(), + vec![ + Branch { + name: "feat/x".into(), + oid: b.clone() + }, + Branch { + name: "main".into(), + oid: a.clone() + } + ] + ); + assert_eq!(branch_oid(&g, &dir, "feat/x").unwrap(), Some(b)); + for not_a_branch in [ + "main~1", + "main^", + "HEAD", + "refs/heads/main", + "nope", + "--all", + ] { + assert_eq!( + branch_oid(&g, &dir, not_a_branch).unwrap(), + None, + "{not_a_branch}" + ); + } + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn commit_fields_come_from_the_raw_object() { + let (g, d, dir) = repo("commit"); + let oid = commit_on( + &g, + &dir, + "main", + "subject line\n\nbody with and\nnewlines\n", + ); + let c = commit(&g, &dir, &oid).unwrap().unwrap(); + assert_eq!( + c.message, + "subject line\n\nbody with and\nnewlines\n" + ); + assert_eq!( + (c.author.name.as_str(), c.author.email.as_str()), + ("Zoë Q", "zoe@example.invalid") + ); + assert_eq!(c.committer, c.author); + assert!(c.author.when > 1_700_000_000); + assert_eq!( + commit(&g, &dir, "0000000000000000000000000000000000000001").unwrap(), + None + ); + let _ = std::fs::remove_dir_all(d); + } + + #[test] + fn person_lines_parse_names_with_spaces_and_angle_brackets_in_the_email_only() { + assert_eq!( + person("A B C 1700000000 +0200").unwrap(), + Person { + name: "A B C".into(), + email: "a@x".into(), + when: 1_700_000_000 + } + ); + assert!(person("garbage").is_none()); + } +} diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index fbecf3e5..a74186a6 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -4,17 +4,21 @@ //! from F2 on); on other targets the crate is empty and the binary prints a //! notice. Design: `docs/superpowers/specs/2026-10-22-p38-forge-design.md`. #![cfg(unix)] +// The recorded repository object has ~70 keys; `json!` expands recursively over them. +#![recursion_limit = "256"] pub mod admin; pub mod api; pub mod auth; pub mod db; pub mod gitproc; +pub mod gitread; pub mod http; pub mod lifecycle; pub mod limits; pub mod names; pub mod refname; +pub mod repo_api; pub mod repos; pub mod routes; pub mod scope; diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs new file mode 100644 index 00000000..dd77eedf --- /dev/null +++ b/crates/forge/src/repo_api.rs @@ -0,0 +1,590 @@ +//! F2a handlers: repositories, search and lists, collaborators, branches. Authentication, scope +//! and the repository permission check already passed in `routes::dispatch` (`ctx.access`). + +use crate::api::{body, org_visible, page_params, paged, str_field, user_json}; +use crate::gitread; +use crate::http::{error, not_found, Resp}; +use crate::lifecycle::LifecycleError; +use crate::names::{self, RepoNameError}; +use crate::refname; +use crate::repos::{self, Access, Filter, Perm, Repo, RepoPatch, Sort}; +use crate::routes::{App, Ctx}; +use crate::scope; +use crate::store::{self, User}; +use crate::timefmt::{now, rfc3339}; +use serde_json::{json, Value}; + +fn acc<'a>(ctx: &'a Ctx) -> Result<&'a Access, Resp> { + ctx.access.as_ref().ok_or_else(|| { + ctx.app + .internal(&"repository route without a resolved repository") + }) +} + +fn life_err(app: &App, e: LifecycleError) -> Resp { + match e { + LifecycleError::Exists => error( + &app.base, + 409, + "The repository with the same name already exists.", + ), + LifecycleError::BadBranch => error(&app.base, 422, "[DefaultBranch]: invalid branch name"), + LifecycleError::Gone => not_found(&app.base, "The target couldn't be found.", None), + LifecycleError::Git(crate::gitproc::GitError::Busy) => { + error(&app.base, 503, "too many git operations in flight") + } + other => app.internal(&other), + } +} + +fn ssh_authority(base: &str) -> String { + base.split("://") + .nth(1) + .unwrap_or(base) + .split('/') + .next() + .unwrap_or("") + .to_string() +} + +/// The ~70-key repository object recorded from Gitea. `perm` is the *caller's* standing. +fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Result { + let owner = store::find_by_id(conn, r.owner_id) + .map_err(|e| app.internal(&e))? + .ok_or_else(|| app.internal(&"repo without owner"))?; + let empty = + gitread::is_empty(&app.life.git, &app.life.repo_dir(r.id)).map_err(|e| app.internal(&e))?; + let mut owner_json = user_json(&app.base, &owner); + owner_json["active"] = json!(false); // recorded: the owner object inside a repository reads inactive + let (b, full) = (&app.base, format!("{}/{}", r.owner, r.name)); + Ok(json!({ + "id": r.id, + "owner": owner_json, + "name": r.name, + "full_name": full, + "description": r.description, + "empty": empty, + "private": true, + "fork": false, + "template": false, + "parent": null, + "mirror": false, + "size": 0, + "language": "", + "languages_url": format!("{b}/api/v1/repos/{full}/languages"), + "html_url": format!("{b}/{full}"), + "url": format!("{b}/api/v1/repos/{full}"), + "link": "", + "ssh_url": format!("ssh://{}/{full}.git", ssh_authority(b)), + "clone_url": format!("{b}/{full}.git"), + "original_url": "", + "website": r.website, + "stars_count": 0, + "forks_count": 0, + "watchers_count": 1, + "open_issues_count": 0, + "open_pr_counter": 0, + "release_counter": 0, + "default_branch": r.default_branch, + "archived": false, + "created_at": rfc3339(r.created), + "updated_at": rfc3339(r.updated), + "archived_at": "1970-01-01T00:00:00Z", + "permissions": {"admin": perm >= Perm::Admin, "push": perm >= Perm::Write, "pull": true}, + "has_issues": true, + "internal_tracker": {"enable_time_tracker": true, "allow_only_contributors_to_track_time": true, "enable_issue_dependencies": true}, + "has_wiki": true, + "has_pull_requests": true, + "has_projects": true, + "projects_mode": "all", + "has_releases": true, + "has_packages": true, + "has_actions": false, + "ignore_whitespace_conflicts": false, + "allow_merge_commits": true, + "allow_rebase": true, + "allow_rebase_explicit": true, + "allow_squash_merge": true, + "allow_fast_forward_only_merge": true, + "allow_rebase_update": true, + "default_delete_branch_after_merge": false, + "default_merge_style": "merge", + "default_allow_maintainer_edit": false, + "avatar_url": "", + "internal": false, + "mirror_interval": "", + "object_format_name": "sha1", + "mirror_updated": "1970-01-01T00:00:00Z", + "repo_transfer": null, + })) +} + +/// A listed repository with the caller's standing computed per row. +fn listed(ctx: &Ctx, rows: &[Repo]) -> Result, Resp> { + rows.iter() + .map(|r| { + let perm = repos::resolve(ctx.conn, &ctx.who.user, &r.owner, &r.name) + .map_err(|e| ctx.app.internal(&e))? + .map_or(Perm::Read, |a| a.perm); + repo_json(ctx.app, ctx.conn, r, perm) + }) + .collect() +} + +fn token_public_only(ctx: &Ctx) -> bool { + ctx.who.scopes.as_deref().is_some_and(scope::public_only) +} + +fn name_error(e: RepoNameError, name: &str) -> String { + match e { + RepoNameError::Empty => "[Name]: Required".into(), + RepoNameError::Reserved => format!("name is reserved [name: {name}]"), + RepoNameError::Pattern(p) => format!("name pattern is not allowed [pattern: {p}]"), + RepoNameError::BadChars => "[Name]: AlphaDashDot".into(), + RepoNameError::TooLong => "[Name]: MaxSize".into(), + } +} + +/// Create a repository owned by `owner` for the caller (shared by the org and user routes). +fn create_for(ctx: &mut Ctx, owner: &User) -> Resp { + let b = match body(ctx) { + Ok(b) => b, + Err(r) => return r, + }; + let base = ctx.app.base.clone(); + let name = str_field(&b, "name").unwrap_or_default(); + if let Err(e) = names::check_repo(&name) { + return error(&base, 422, &name_error(e, &name)); + } + let branch = str_field(&b, "default_branch") + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| "main".into()); + if !refname::valid_branch(&branch) { + return error(&base, 422, "[DefaultBranch]: invalid branch name"); + } + // `private` is accepted and ignored: every repository is private (recorded Gitea forces it + // on create; an anonymous-read hole on PATCH is not copied). + let auto_init = b.get("auto_init").and_then(Value::as_bool).unwrap_or(false); + let description = str_field(&b, "description").unwrap_or_default(); + let created = ctx.app.life.create( + ctx.conn, + owner, + &ctx.who.user, + &name, + &description, + &branch, + auto_init, + now(), + ); + match created { + Ok(r) => match repo_json(ctx.app, ctx.conn, &r, Perm::Admin) { + Ok(v) => Resp::json(201, &v), + Err(resp) => resp, + }, + Err(e) => life_err(ctx.app, e), + } +} + +/// `POST /orgs/{o}/repos`: the actor check already required an owner of `{o}` or a site admin. +pub fn create_org_repo(ctx: &mut Ctx) -> Resp { + let name = ctx.param("o").to_string(); + match store::find_by_login(ctx.conn, &name) { + Ok(Some(org)) if org.is_org => create_for(ctx, &org), + Ok(_) => crate::api::org_missing(ctx.app, &name), + Err(e) => ctx.app.internal(&e), + } +} + +/// `POST /user/repos`: the caller owns the new repository. +pub fn create_user_repo(ctx: &mut Ctx) -> Resp { + let me = ctx.who.user.clone(); + create_for(ctx, &me) +} + +/// `GET /repos/{o}/{r}`. +pub fn get_repo(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a, + Err(r) => return r, + }; + match repo_json(ctx.app, ctx.conn, &a.repo, a.perm) { + Ok(v) => Resp::json(200, &v), + Err(r) => r, + } +} + +/// `PATCH /repos/{o}/{r}`: description, website and default branch; rename is refused; the +/// other Gitea fields (`private`, `has_*`, `allow_*`, ...) are accepted and ignored. +pub fn patch_repo(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let b = match body(ctx) { + Ok(b) => b, + Err(r) => return r, + }; + let base = ctx.app.base.clone(); + if str_field(&b, "name").is_some_and(|n| n != a.repo.name) { + return error(&base, 422, "repository rename is not supported"); + } + if let Some(branch) = str_field(&b, "default_branch").filter(|n| *n != a.repo.default_branch) { + if !refname::valid_branch(&branch) { + return error(&base, 422, "[DefaultBranch]: invalid branch name"); + } + match ctx + .app + .life + .set_default_branch(ctx.conn, &a.repo, &branch, now()) + { + Ok(()) => {} + Err(LifecycleError::NoSuchBranch) => { + return error( + &base, + 422, + &format!("branch does not exist [name: {branch}]"), + ) + } + Err(e) => return life_err(ctx.app, e), + } + } + let patch = RepoPatch { + description: str_field(&b, "description"), + website: str_field(&b, "website"), + default_branch: None, + }; + match repos::update_live(ctx.conn, a.repo.id, &patch, now()) { + Ok(true) => {} + Ok(false) => return not_found(&base, "The target couldn't be found.", None), + Err(e) => return ctx.app.internal(&e), + } + match repos::find_by_id(ctx.conn, a.repo.id) { + Ok(Some(r)) => match repo_json(ctx.app, ctx.conn, &r, a.perm) { + Ok(v) => Resp::json(200, &v), + Err(resp) => resp, + }, + Ok(None) => not_found(&base, "The target couldn't be found.", None), + Err(e) => ctx.app.internal(&e), + } +} + +/// `DELETE /repos/{o}/{r}` (owner-level, checked by the router). +pub fn delete_repo(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + match ctx.app.life.delete(ctx.conn, &a.repo, now()) { + Ok(()) => Resp::no_content(), + Err(e) => life_err(ctx.app, e), + } +} + +fn list_response( + ctx: &Ctx, + path: &str, + rows: &[Repo], + total: i64, + page: i64, + limit: i64, + wrap: bool, +) -> Resp { + let items = match listed(ctx, rows) { + Ok(i) => i, + Err(r) => return r, + }; + if !wrap { + return paged(ctx, path, total, page, limit, items); + } + let mut r = Resp::json(200, &json!({"ok": true, "data": items})) + .with("x-total-count", total.to_string()); + if let Some(link) = paged(ctx, path, total, page, limit, vec![]) + .headers + .iter() + .find(|(k, _)| *k == "link") + { + r = r.with("link", link.1.clone()); + } + r +} + +fn query_list(ctx: &Ctx, filter: &Filter, sort: Sort, path: &str, wrap: bool) -> Resp { + let (page, limit) = page_params(ctx); + match repos::list_visible( + ctx.conn, + &ctx.who.user, + token_public_only(ctx), + filter, + sort, + page, + limit, + ) { + Ok((rows, total)) => list_response(ctx, path, &rows, total, page, limit, wrap), + Err(e) => ctx.app.internal(&e), + } +} + +/// `GET /repos/search`: `{"ok": true, "data": [...]}`; a `public-only` token sees nothing. +pub fn search(ctx: &mut Ctx) -> Resp { + let q = ctx.req.param("q").map(|v| v.replace('+', " ")); + let sort = if ctx.req.param("sort") == Some("alpha") { + Sort::Alpha + } else { + Sort::Id + }; + query_list( + ctx, + &Filter { q, owner_id: None }, + sort, + "/repos/search", + true, + ) +} + +/// `GET /user/repos`: every repository the caller can read. +pub fn list_user_repos(ctx: &mut Ctx) -> Resp { + query_list(ctx, &Filter::default(), Sort::Id, "/user/repos", false) +} + +/// `GET /orgs/{o}/repos`: 200 for any existing organization, listing only what the caller can read. +pub fn list_org_repos(ctx: &mut Ctx) -> Resp { + let name = ctx.param("o").to_string(); + match store::find_by_login(ctx.conn, &name) { + Ok(Some(org)) if org.is_org => { + // Whether the caller may see the organization itself does not change the answer: the + // list is filtered by repository access either way (the counts are the visible count). + let _ = org_visible(ctx, &org); + let path = format!("/orgs/{}/repos", org.login); + query_list( + ctx, + &Filter { + owner_id: Some(org.id), + ..Default::default() + }, + Sort::Id, + &path, + false, + ) + } + Ok(_) => crate::api::org_missing(ctx.app, &name), + Err(e) => ctx.app.internal(&e), + } +} + +fn collab_target(ctx: &Ctx) -> Result { + let name = ctx.param("u"); + match store::find_by_login(ctx.conn, name) { + Ok(Some(u)) if !u.is_org => Ok(u), + Ok(_) => Err(error( + &ctx.app.base, + 422, + &format!("user does not exist [uid: 0, name: {name}]"), + )), + Err(e) => Err(ctx.app.internal(&e)), + } +} + +/// `GET /repos/{o}/{r}/collaborators`: in the order they were added. +pub fn list_collaborators(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let (page, limit) = page_params(ctx); + match repos::collaborators(ctx.conn, a.repo.id, page, limit) { + Ok((rows, total)) => { + let items = rows + .iter() + .map(|(u, _)| user_json(&ctx.app.base, u)) + .collect(); + let path = format!("/repos/{}/{}/collaborators", a.repo.owner, a.repo.name); + paged(ctx, &path, total, page, limit, items) + } + Err(e) => ctx.app.internal(&e), + } +} + +/// `PUT /repos/{o}/{r}/collaborators/{u}` with `{"permission": read|write|admin}` (default write). +pub fn put_collaborator(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let b = match body(ctx) { + Ok(b) => b, + Err(r) => return r, + }; + let target = match collab_target(ctx) { + Ok(u) => u, + Err(r) => return r, + }; + let base = ctx.app.base.clone(); + let mode = match str_field(&b, "permission") { + None => Perm::Write, + Some(m) => match Perm::from_mode(&m) { + Some(p) => p, + None => return error(&base, 422, "[Permission]: invalid permission"), + }, + }; + let is_owner = target.id == a.repo.owner_id + || store::is_org_owner(ctx.conn, a.repo.owner_id, target.id).unwrap_or(false); + if is_owner { + return error(&base, 422, "the repository owner cannot be a collaborator"); + } + match repos::set_collaborator(ctx.conn, a.repo.id, target.id, mode) { + Ok(()) => Resp::no_content(), + Err(e) => ctx.app.internal(&e), + } +} + +/// `DELETE /repos/{o}/{r}/collaborators/{u}`. +pub fn delete_collaborator(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let target = match collab_target(ctx) { + Ok(u) => u, + Err(r) => return r, + }; + match repos::remove_collaborator(ctx.conn, a.repo.id, target.id) { + Ok(_) => Resp::no_content(), + Err(e) => ctx.app.internal(&e), + } +} + +/// `GET /repos/{o}/{r}/collaborators/{u}/permission`. +pub fn collaborator_permission(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let target = match collab_target(ctx) { + Ok(u) => u, + Err(r) => return r, + }; + let owner = target.id == a.repo.owner_id + || store::is_org_owner(ctx.conn, a.repo.owner_id, target.id).unwrap_or(false); + let perm = if owner { + "owner" + } else { + match repos::collaborator_mode(ctx.conn, a.repo.id, target.id) { + Ok(Some(p)) => p.as_mode(), + Ok(None) => "none", + Err(e) => return ctx.app.internal(&e), + } + }; + let mut u = user_json(&ctx.app.base, &target); + u["active"] = json!(false); // recorded + Resp::json( + 200, + &json!({"permission": perm, "role_name": perm, "user": u}), + ) +} + +fn branch_json(ctx: &Ctx, a: &Access, br: &gitread::Branch) -> Result { + let dir = ctx.app.life.repo_dir(a.repo.id); + let c = gitread::commit(&ctx.app.life.git, &dir, &br.oid) + .map_err(|e| ctx.app.internal(&e))? + .ok_or_else(|| ctx.app.internal(&"branch points at no commit"))?; + let who = |p: &gitread::Person| json!({"name": p.name, "email": p.email, "username": ""}); + let full = format!("{}/{}", a.repo.owner, a.repo.name); + Ok(json!({ + "name": br.name, + "commit": { + "id": c.oid, + "message": c.message, + "url": format!("{}/{full}/commit/{}", ctx.app.base, c.oid), + "author": who(&c.author), + "committer": who(&c.committer), + "verification": {"verified": false, "reason": "gpg.error.not_signed_commit", "signature": "", "signer": null, "payload": ""}, + "timestamp": rfc3339(c.committer.when), + "added": null, + "removed": null, + "modified": null, + }, + "protected": false, + "required_approvals": 0, + "enable_status_check": false, + "status_check_contexts": [], + "user_can_push": a.perm >= Perm::Write, + "user_can_merge": a.perm >= Perm::Write, + "effective_branch_protection_name": "", + })) +} + +fn no_branch(ctx: &Ctx, name: &str) -> Resp { + not_found( + &ctx.app.base, + "The target couldn't be found.", + Some(&format!("branch does not exist [name: {name}]")), + ) +} + +/// `GET /repos/{o}/{r}/branches`: sorted by name; an empty repository answers `null`. +pub fn list_branches(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let all = match gitread::list_branches(&ctx.app.life.git, &ctx.app.life.repo_dir(a.repo.id)) { + Ok(b) => b, + Err(e) => return ctx.app.internal(&e), + }; + if all.is_empty() { + return Resp::json(200, &Value::Null).with("x-total-count", "0".into()); + } + let (page, limit) = page_params(ctx); + let total = all.len() as i64; + let start = (((page - 1) * limit) as usize).min(all.len()); + let rows = &all[start..(start + limit as usize).min(all.len())]; + let items: Result, Resp> = rows.iter().map(|b| branch_json(ctx, &a, b)).collect(); + match items { + Ok(items) => paged( + ctx, + &format!("/repos/{}/{}/branches", a.repo.owner, a.repo.name), + total, + page, + limit, + items, + ), + Err(r) => r, + } +} + +/// `GET /repos/{o}/{r}/branches/{b..}`. +pub fn get_branch(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let name = ctx.param("b").to_string(); + match gitread::branch_oid(&ctx.app.life.git, &ctx.app.life.repo_dir(a.repo.id), &name) { + Ok(Some(oid)) => match branch_json(ctx, &a, &gitread::Branch { name, oid }) { + Ok(v) => Resp::json(200, &v), + Err(r) => r, + }, + Ok(None) => no_branch(ctx, &name), + Err(e) => ctx.app.internal(&e), + } +} + +/// `DELETE /repos/{o}/{r}/branches/{b..}`: the default branch stays (403, recorded). +pub fn delete_branch(ctx: &mut Ctx) -> Resp { + let a = match acc(ctx) { + Ok(a) => a.clone(), + Err(r) => return r, + }; + let name = ctx.param("b").to_string(); + if !refname::valid_branch(&name) { + return no_branch(ctx, &name); + } + if name == a.repo.default_branch { + return error(&ctx.app.base, 403, "can not delete default branch"); + } + match ctx.app.life.delete_branch(ctx.conn, &a.repo, &name) { + Ok(()) => Resp::no_content(), + Err(LifecycleError::NoSuchBranch) => no_branch(ctx, &name), + Err(e) => life_err(ctx.app, e), + } +} diff --git a/crates/forge/src/routes.rs b/crates/forge/src/routes.rs index 0db4079f..3dd31b01 100644 --- a/crates/forge/src/routes.rs +++ b/crates/forge/src/routes.rs @@ -5,9 +5,11 @@ use crate::auth::{self, Authed}; use crate::http::{error, not_found, Req, Resp}; +use crate::lifecycle::Lifecycle; use crate::limits::{Backoff, Gate, Proxies}; +use crate::repos::{self, Access, Perm}; use crate::scope::{self, Cat, Level, Need}; -use crate::{api, db, store}; +use crate::{api, db, repo_api, store}; use rusqlite::Connection; use std::path::PathBuf; @@ -19,11 +21,19 @@ pub struct App { pub backoff: Backoff, pub gate: Gate, pub proxies: Proxies, + /// Repository directories, the git runner and per-repository serialization. + pub life: Lifecycle, } impl App { /// Build the shared state; also computes the dummy password hash so the first unknown login is not slower. - pub fn new(data_dir: PathBuf, public_url: &str, proxies: Proxies, argon_slots: usize) -> App { + pub fn new( + data_dir: PathBuf, + public_url: &str, + proxies: Proxies, + argon_slots: usize, + life: Lifecycle, + ) -> App { scl_crypto::warm_up(); App { data_dir, @@ -31,6 +41,7 @@ impl App { backoff: Backoff::default(), gate: Gate::new(argon_slots), proxies, + life, } } @@ -60,6 +71,13 @@ pub enum Actor { /// The caller must own the organization named by this path parameter, or be a /// site admin; a missing organization is a 404. OrgOwnerOrSiteAdmin(&'static str), + /// A repository route (`{o}`/`{r}`): the caller needs at least this permission. Resolved by + /// `repos::resolve`, so a missing, not-live or invisible repository is one 404, and a token + /// restricted to public repositories is refused before any existence is revealed. + Repo(Perm), + /// Like `Repo(Admin)`, but only the owning user, an owner of the owning organization or a + /// site admin (not an admin collaborator): deleting the repository. + RepoOwner, } /// What a handler receives; authentication and policy already passed. @@ -68,6 +86,8 @@ pub struct Ctx<'a> { pub req: &'a Req, pub conn: &'a mut Connection, pub who: &'a Authed, + /// The caller's standing, set for `Actor::Repo` / `Actor::RepoOwner` routes. + pub access: Option, params: Params, } @@ -117,7 +137,7 @@ const fn n(cat: Cat, level: Level) -> Option { Some(Need(cat, level)) } -use Actor::{Any, OrgOwnerOrSiteAdmin, SelfOrSiteAdmin, SiteAdmin}; +use Actor::{Any, OrgOwnerOrSiteAdmin, Repo, RepoOwner, SelfOrSiteAdmin, SiteAdmin}; use Class::{Password, Token}; /// The route table. F2 onward append rows; a row cannot omit its class, @@ -212,14 +232,21 @@ pub static ROUTES: &[Route] = &[ Any, api::get_org, ), - // Declared now so scope and actor checks apply; the handlers arrive with F2. r( - "POST", - "/orgs/{o}/repos", + "GET", + "/repos/search", Token, - n(Cat::Organization, Level::Write), - OrgOwnerOrSiteAdmin("o"), - api::not_implemented, + n(Cat::Repository, Level::Read), + Any, + repo_api::search, + ), + r( + "GET", + "/user/repos", + Token, + n(Cat::User, Level::Read), + Any, + repo_api::list_user_repos, ), r( "POST", @@ -227,7 +254,103 @@ pub static ROUTES: &[Route] = &[ Token, n(Cat::User, Level::Write), Any, - api::not_implemented, + repo_api::create_user_repo, + ), + r( + "GET", + "/orgs/{o}/repos", + Token, + n(Cat::Organization, Level::Read), + Any, + repo_api::list_org_repos, + ), + r( + "POST", + "/orgs/{o}/repos", + Token, + n(Cat::Organization, Level::Write), + OrgOwnerOrSiteAdmin("o"), + repo_api::create_org_repo, + ), + r( + "GET", + "/repos/{o}/{r}", + Token, + n(Cat::Repository, Level::Read), + Repo(Perm::Read), + repo_api::get_repo, + ), + r( + "PATCH", + "/repos/{o}/{r}", + Token, + n(Cat::Repository, Level::Write), + Repo(Perm::Admin), + repo_api::patch_repo, + ), + r( + "DELETE", + "/repos/{o}/{r}", + Token, + n(Cat::Repository, Level::Write), + RepoOwner, + repo_api::delete_repo, + ), + r( + "GET", + "/repos/{o}/{r}/collaborators", + Token, + n(Cat::Repository, Level::Read), + Repo(Perm::Admin), + repo_api::list_collaborators, + ), + r( + "GET", + "/repos/{o}/{r}/collaborators/{u}/permission", + Token, + n(Cat::Repository, Level::Read), + Repo(Perm::Admin), + repo_api::collaborator_permission, + ), + r( + "PUT", + "/repos/{o}/{r}/collaborators/{u}", + Token, + n(Cat::Repository, Level::Write), + Repo(Perm::Admin), + repo_api::put_collaborator, + ), + r( + "DELETE", + "/repos/{o}/{r}/collaborators/{u}", + Token, + n(Cat::Repository, Level::Write), + Repo(Perm::Admin), + repo_api::delete_collaborator, + ), + r( + "GET", + "/repos/{o}/{r}/branches", + Token, + n(Cat::Repository, Level::Read), + Repo(Perm::Read), + repo_api::list_branches, + ), + r( + "GET", + "/repos/{o}/{r}/branches/{b..}", + Token, + n(Cat::Repository, Level::Read), + Repo(Perm::Read), + repo_api::get_branch, + ), + r( + "DELETE", + "/repos/{o}/{r}/branches/{b..}", + Token, + n(Cat::Repository, Level::Write), + Repo(Perm::Write), + repo_api::delete_branch, ), ]; @@ -259,7 +382,18 @@ fn match_pattern(pattern: &'static str, path: &str) -> Option { match (ps.next(), xs.next()) { (None, None) => return Some(caps), (Some(p), Some(x)) => { - if let Some(name) = p.strip_prefix('{').and_then(|s| s.strip_suffix('}')) { + if let Some(name) = p.strip_prefix('{').and_then(|s| s.strip_suffix("..}")) { + // Tail capture: the rest of the path, which may contain `/` (branch names). + let mut parts = vec![x]; + parts.extend(xs.by_ref()); + if ps.next().is_some() || parts.iter().any(|p| p.is_empty()) { + return None; + } + let decoded: Option> = + parts.into_iter().map(percent_decode).collect(); + caps.push((name, decoded?.join("/"))); + return Some(caps); + } else if let Some(name) = p.strip_prefix('{').and_then(|s| s.strip_suffix('}')) { if x.is_empty() { return None; } @@ -327,6 +461,38 @@ fn healthz(app: &App) -> Resp { r } +/// Resolve the repository a route names and check the caller's standing, in the one order every +/// repository route follows: a token restricted to public repositories is refused (nothing is +/// revealed), a missing or invisible repository is the same 404, an insufficient permission is +/// a 403. +fn resolve_repo( + app: &App, + conn: &Connection, + who: &Authed, + owner: &str, + name: &str, + min: Perm, + owner_only: bool, +) -> Result { + if who.scopes.as_deref().is_some_and(scope::public_only) { + return Err(error( + &app.base, + 403, + "token scope is limited to public repos", + )); + } + match repos::resolve(conn, &who.user, owner, name) { + Err(e) => Err(app.internal(&e)), + Ok(None) => Err(not_found(&app.base, "The target couldn't be found.", None)), + Ok(Some(a)) if a.perm < min || (owner_only && !a.is_owner) => Err(error( + &app.base, + 403, + "You do not have permission to perform this action", + )), + Ok(Some(a)) => Ok(a), + } +} + /// Handle one request end to end. Blocking (SQLite, Argon2): the server runs it /// on a blocking thread. pub fn dispatch(app: &App, req: &Req) -> Resp { @@ -388,6 +554,7 @@ pub fn dispatch(app: &App, req: &Req) -> Resp { .map(|(_, v)| v.as_str()) .unwrap_or("") }; + let mut access: Option = None; let denied = match route.actor { Actor::Any => None, Actor::SiteAdmin => { @@ -396,6 +563,19 @@ pub fn dispatch(app: &App, req: &Req) -> Resp { Actor::SelfOrSiteAdmin(p) => (!who.user.is_admin && !who.user.login.eq_ignore_ascii_case(param(p))) .then_some("doer should be the site admin or be same as the contextUser"), + Actor::Repo(_) | Actor::RepoOwner => { + let (min, owner_only) = match route.actor { + Actor::Repo(m) => (m, false), + _ => (Perm::Admin, true), + }; + match resolve_repo(app, &conn, &who, param("o"), param("r"), min, owner_only) { + Ok(a) => { + access = Some(a); + None + } + Err(resp) => return resp, + } + } Actor::OrgOwnerOrSiteAdmin(p) => match store::find_by_login(&conn, param(p)) { Ok(Some(org)) if org.is_org => match store::is_org_owner(&conn, org.id, who.user.id) { Ok(true) => None, @@ -419,6 +599,7 @@ pub fn dispatch(app: &App, req: &Req) -> Resp { req, conn: &mut conn, who: &who, + access, params, }) } diff --git a/crates/forge/src/server.rs b/crates/forge/src/server.rs index 2a99c888..6dd1cf72 100644 --- a/crates/forge/src/server.rs +++ b/crates/forge/src/server.rs @@ -231,8 +231,9 @@ pub fn make_app( public_url: &str, proxies: Proxies, argon_slots: usize, + life: crate::lifecycle::Lifecycle, ) -> Arc { - Arc::new(App::new(data_dir, public_url, proxies, argon_slots)) + Arc::new(App::new(data_dir, public_url, proxies, argon_slots, life)) } #[cfg(test)] diff --git a/crates/forge/tests/common/mod.rs b/crates/forge/tests/common/mod.rs index d9b401ae..86c25852 100644 --- a/crates/forge/tests/common/mod.rs +++ b/crates/forge/tests/common/mod.rs @@ -22,6 +22,7 @@ pub struct Srv { pub base: String, /// All-scope token of the site admin `siteadmin`. pub admin: String, + _data_lock: scl_forge::lifecycle::DirLock, stop: Option>, thread: Option>, } @@ -106,7 +107,13 @@ impl Srv { std_l.set_nonblocking(true).unwrap(); let addr = std_l.local_addr().unwrap(); let base = format!("http://{addr}"); - let mut app = scl_forge::routes::App::new(dir.clone(), &base, Proxies::default(), 4); + let data_lock = scl_forge::lifecycle::DirLock::acquire(&dir).unwrap(); + let git = std::sync::Arc::new( + scl_forge::gitproc::Git::new(&dir, scl_forge::gitproc::find_git().expect("git"), 8) + .unwrap(), + ); + let life = scl_forge::lifecycle::Lifecycle::new(&dir, git).unwrap(); + let mut app = scl_forge::routes::App::new(dir.clone(), &base, Proxies::default(), 4, life); if let Some(b) = backoff { app.backoff = b; } @@ -133,6 +140,7 @@ impl Srv { }); }); Srv { + _data_lock: data_lock, addr, dir, base, diff --git a/crates/forge/tests/e2e_a.rs b/crates/forge/tests/e2e_a.rs index 0d67e544..dbaee29b 100644 --- a/crates/forge/tests/e2e_a.rs +++ b/crates/forge/tests/e2e_a.rs @@ -900,7 +900,7 @@ fn user_and_org_repo_creation_routes_apply_scope_and_actor_rules() { ) ); } - // The owner and a site admin pass the policy (the handler itself arrives with F2). + // The owner and a site admin pass the policy and really create (F2a); a second create is 409. assert_eq!( s.post( "/api/v1/orgs/botorg/repos", @@ -908,12 +908,17 @@ fn user_and_org_repo_creation_routes_apply_scope_and_actor_rules() { json!({"name": "legit"}) ) .status, - 501 + 201 ); assert_eq!( s.post("/api/v1/orgs/botorg/repos", &adm, json!({"name": "legit"})) .status, - 501 + 409 + ); + assert_eq!( + s.post("/api/v1/orgs/botorg/repos", &adm, json!({"name": "legit2"})) + .status, + 201 ); let r = s.post("/api/v1/orgs/ghost/repos", &adm, json!({"name": "x"})); assert_eq!( From 0fd155bbc23a8048f73b5f917148688315673547 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:37:10 +0300 Subject: [PATCH 09/16] feat(forge): F2a e2e tests, conformance gate (A B C L N R), owner and sort fidelity (wip) --- crates/forge/conformance/departures.json | 24 +- crates/forge/conformance/forge-check.sh | 2 +- crates/forge/conformance/slice-pending.json | 5 +- crates/forge/src/repo_api.rs | 15 +- crates/forge/src/repos.rs | 11 +- crates/forge/tests/common/mod.rs | 43 ++ crates/forge/tests/e2e_repos.rs | 809 ++++++++++++++++++++ crates/forge/tests/route_table.rs | 19 + 8 files changed, 907 insertions(+), 21 deletions(-) create mode 100644 crates/forge/tests/e2e_repos.rs diff --git a/crates/forge/conformance/departures.json b/crates/forge/conformance/departures.json index b3746a36..f8b1a15a 100644 --- a/crates/forge/conformance/departures.json +++ b/crates/forge/conformance/departures.json @@ -690,13 +690,15 @@ "match": "PATCH /api/v1/repos/acme/pub1", "expect": { "ignore_fields": [ - "private" + "private", + "internal" ], "fields": { - "private": true + "private": true, + "internal": false } }, - "reason": "sc-forge keeps private:true; only `private` may differ" + "reason": "sc-forge keeps private:true and internal:false (every repository is private, none is `internal`); Gitea flips both when private:false is PATCHed. Only `private` and `internal` may differ" }, { "scenario": "L", @@ -705,13 +707,15 @@ "match": "PATCH /api/v1/repos/acme/pub1", "expect": { "ignore_fields": [ - "private" + "private", + "internal" ], "fields": { - "private": true + "private": true, + "internal": false } }, - "reason": "sc-forge keeps private:true; only `private` may differ" + "reason": "sc-forge keeps private:true and internal:false (every repository is private, none is `internal`); Gitea flips both when private:false is PATCHed. Only `private` and `internal` may differ" }, { "scenario": "L", @@ -720,13 +724,15 @@ "match": "GET /api/v1/repos/acme/pub1", "expect": { "ignore_fields": [ - "private" + "private", + "internal" ], "fields": { - "private": true + "private": true, + "internal": false } }, - "reason": "GET after PATCH private:false still reports private:true" + "reason": "sc-forge keeps private:true and internal:false (every repository is private, none is `internal`); Gitea flips both when private:false is PATCHed. Only `private` and `internal` may differ" }, { "scenario": "H", diff --git a/crates/forge/conformance/forge-check.sh b/crates/forge/conformance/forge-check.sh index e3798bde..4186d8c9 100755 --- a/crates/forge/conformance/forge-check.sh +++ b/crates/forge/conformance/forge-check.sh @@ -25,4 +25,4 @@ for _ in $(seq 50); do done python3 -I "$here/run.py" check --base-url "http://127.0.0.1:$port" --admin-token "$token" \ --fixtures "$here/fixtures/1.22.6" --departures "$here/departures.json" --pending "$here/slice-pending.json" \ - --only A,B "$@" || { echo "--- server log"; cat "$data/serve.log"; exit 1; } + --only A,B,C,L,N,R "$@" || { echo "--- server log"; cat "$data/serve.log"; exit 1; } diff --git a/crates/forge/conformance/slice-pending.json b/crates/forge/conformance/slice-pending.json index 27778705..faa170a3 100644 --- a/crates/forge/conformance/slice-pending.json +++ b/crates/forge/conformance/slice-pending.json @@ -1,4 +1,5 @@ { - "A": [6, "POST /api/v1/user/repos"], - "B": [25, "POST /api/v1/user/repos"] + "B": [25, "GET /api/v1/repos/alice/scoped/contents/README.md"], + "C": [27, "git clone"], + "N": [0, "git clone"] } diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs index dd77eedf..1cd82eed 100644 --- a/crates/forge/src/repo_api.rs +++ b/crates/forge/src/repo_api.rs @@ -1,7 +1,7 @@ //! F2a handlers: repositories, search and lists, collaborators, branches. Authentication, scope //! and the repository permission check already passed in `routes::dispatch` (`ctx.access`). -use crate::api::{body, org_visible, page_params, paged, str_field, user_json}; +use crate::api::{body, page_params, paged, str_field, user_json}; use crate::gitread; use crate::http::{error, not_found, Resp}; use crate::lifecycle::LifecycleError; @@ -56,6 +56,8 @@ fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Re gitread::is_empty(&app.life.git, &app.life.repo_dir(r.id)).map_err(|e| app.internal(&e))?; let mut owner_json = user_json(&app.base, &owner); owner_json["active"] = json!(false); // recorded: the owner object inside a repository reads inactive + owner_json["login_name"] = json!(""); // recorded: the nested owner never carries the login_name + owner_json["is_admin"] = json!(false); // recorded: nor does it ever read as a site admin let (b, full) = (&app.base, format!("{}/{}", r.owner, r.name)); Ok(json!({ "id": r.id, @@ -147,6 +149,11 @@ fn name_error(e: RepoNameError, name: &str) -> String { /// Create a repository owned by `owner` for the caller (shared by the org and user routes). fn create_for(ctx: &mut Ctx, owner: &User) -> Resp { + // Every repository is private, so a token restricted to public repositories may not create + // one; refused before the name or any existence is looked at (as `resolve_repo` does). + if token_public_only(ctx) { + return error(&ctx.app.base, 403, "token scope is limited to public repos"); + } let b = match body(ctx) { Ok(b) => b, Err(r) => return r, @@ -351,9 +358,9 @@ pub fn list_org_repos(ctx: &mut Ctx) -> Resp { let name = ctx.param("o").to_string(); match store::find_by_login(ctx.conn, &name) { Ok(Some(org)) if org.is_org => { - // Whether the caller may see the organization itself does not change the answer: the - // list is filtered by repository access either way (the counts are the visible count). - let _ = org_visible(ctx, &org); + // The list is filtered by repository access whether or not the organization itself is + // visible: a collaborator of a repository inside a private organization still sees it + // (recorded K#55), and the counts are the visible count. let path = format!("/orgs/{}/repos", org.login); query_list( ctx, diff --git a/crates/forge/src/repos.rs b/crates/forge/src/repos.rs index 80d7838b..37c190e5 100644 --- a/crates/forge/src/repos.rs +++ b/crates/forge/src/repos.rs @@ -237,7 +237,7 @@ pub struct Filter { pub enum Sort { /// By id (creation order). Id, - /// By name. + /// By full name, `owner/name`. Alpha, } @@ -292,7 +292,8 @@ pub fn list_visible( )?; let order = match sort { Sort::Id => "r.id", - Sort::Alpha => "r.lower_name, r.id", + // recorded: `sort=alpha` orders by the full name `owner/name` + Sort::Alpha => "u.lower_login, r.lower_name, r.id", }; args.push(Value::Integer(limit)); args.push(Value::Integer((page - 1) * limit)); @@ -543,11 +544,11 @@ mod tests { assert_eq!( names(&admin, false, &all), ( - // `sort=alpha` orders by repository name, not by owner + // `sort=alpha` orders by the full name `owner/name` vec![ + "acme/gamma".into(), "owner/alpha".into(), - "owner/beta".into(), - "acme/gamma".into() + "owner/beta".into() ], 3 ) diff --git a/crates/forge/tests/common/mod.rs b/crates/forge/tests/common/mod.rs index 86c25852..a840869d 100644 --- a/crates/forge/tests/common/mod.rs +++ b/crates/forge/tests/common/mod.rs @@ -208,6 +208,49 @@ impl Srv { self.send("POST", path, Some(auth), Some(&body)) } + /// Run git directly against repository `id`'s bare directory (the test's stand-in for a push, + /// which F2b serves over HTTP). + pub fn git(&self, id: i64, args: &[&str], stdin: Option<&[u8]>) -> String { + let g = scl_forge::gitproc::Git::new( + &self.dir, + scl_forge::gitproc::find_git().expect("git"), + 4, + ) + .unwrap(); + let dir = self.dir.join("repos").join(format!("{id}.git")); + let id_ = scl_forge::gitproc::Identity::new("T", "t@example.invalid").unwrap(); + let out = g + .run(&scl_forge::gitproc::Spec { + repo: Some(&dir), + args, + stdin, + identity: Some(&id_), + }) + .unwrap(); + assert!( + out.ok(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + out.text() + } + + /// Make branch `name` in repository `id` point at a new empty commit on top of `parent` (or a root). + pub fn commit_on(&self, id: i64, name: &str, parent: Option<&str>) -> String { + let tree = self.git(id, &["mktree"], Some(b"")); + let mut args = vec!["commit-tree", tree.as_str()]; + if let Some(p) = parent { + args.extend(["-p", p]); + } + let oid = self.git(id, &args, Some(b"msg\n")); + self.git( + id, + &["update-ref", &format!("refs/heads/{name}"), &oid], + None, + ); + oid + } + /// Create a user through the admin API, returning (login, password). pub fn mk_user(&self, login: &str) -> String { let pw = format!("pw-{login}-0123456789"); diff --git a/crates/forge/tests/e2e_repos.rs b/crates/forge/tests/e2e_repos.rs new file mode 100644 index 00000000..379647b9 --- /dev/null +++ b/crates/forge/tests/e2e_repos.rs @@ -0,0 +1,809 @@ +//! F2a: repositories, listings, collaborators, branches, over a real socket. The recorded +//! exchanges that need no later slice are also replayed by `forge-check.sh`; these tests add the +//! negative and cross-cutting cases the recordings do not cover. +#![cfg(unix)] + +mod common; +use common::*; +use serde_json::{json, Value}; + +fn fixture_keys(file: &str, exchange: usize) -> Vec { + let p = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("conformance/fixtures/1.22.6") + .join(file); + let v: Value = serde_json::from_slice(&std::fs::read(p).unwrap()).unwrap(); + v["exchanges"][exchange]["body"] + .as_object() + .unwrap() + .keys() + .cloned() + .collect() +} + +struct World { + s: Srv, + adm: String, +} + +/// acme (private org owned by siteadmin) plus users alice (owner of nothing), bob, carol, dave +/// each with an all-scope token. +fn world() -> (World, std::collections::HashMap<&'static str, String>) { + let s = Srv::start(); + let adm = token(&s.admin); + assert_eq!( + s.post( + "/api/v1/orgs", + &adm, + json!({"username": "acme", "visibility": "private"}) + ) + .status, + 201 + ); + let mut t = std::collections::HashMap::new(); + for u in ["alice", "bob", "carol", "dave"] { + let pw = s.mk_user(u); + t.insert(u, s.mk_token(u, &pw, "all", &["all"])); + } + (World { s, adm }, t) +} + +impl World { + fn mk(&self, name: &str, init: bool) -> Value { + let r = self.s.post( + "/api/v1/orgs/acme/repos", + &self.adm, + json!({"name": name, "auto_init": init, "private": true}), + ); + assert_eq!(r.status, 201, "{}", r.msg()); + r.json() + } + fn collab(&self, repo: &str, user: &str, mode: &str) { + let r = self.s.send( + "PUT", + &format!("/api/v1/repos/acme/{repo}/collaborators/{user}"), + Some(&self.adm), + Some(&json!({"permission": mode})), + ); + assert_eq!(r.status, 204, "{}", r.msg()); + } +} + +#[test] +fn create_get_patch_delete_round_trip_and_the_recorded_object_shape() { + let (w, _) = world(); + let r = w.mk("alpha", true); + let want = fixture_keys("C-repos.json", 0); + for k in &want { + assert!(r.get(k).is_some(), "repo object lacks recorded key {k}"); + } + assert_eq!( + ( + r["full_name"].as_str(), + r["private"].as_bool(), + r["empty"].as_bool(), + r["default_branch"].as_str() + ), + (Some("acme/alpha"), Some(true), Some(false), Some("main")) + ); + assert_eq!( + r["permissions"], + json!({"admin": true, "push": true, "pull": true}) + ); + // an empty repo reads empty, with the requested default branch + let e = + w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": "blank", "auto_init": false, "default_branch": "trunk"}), + ) + .json(); + assert_eq!( + (e["empty"].as_bool(), e["default_branch"].as_str()), + (Some(true), Some("trunk")) + ); + + // PATCH: description and website land; private:false is accepted and stays true; has_* ignored + let p = w.s.send("PATCH", "/api/v1/repos/acme/alpha", Some(&w.adm), Some(&json!({"description": "A", "website": "https://x.invalid", "private": false, "has_actions": true}))); + assert_eq!(p.status, 200); + assert_eq!( + ( + p.json()["description"].as_str(), + p.json()["website"].as_str(), + p.json()["private"].as_bool(), + p.json()["has_actions"].as_bool() + ), + ( + Some("A"), + Some("https://x.invalid"), + Some(true), + Some(false) + ) + ); + // rename is refused, naming the same name is a no-op, a missing default branch is 422 + let rn = w.s.send( + "PATCH", + "/api/v1/repos/acme/alpha", + Some(&w.adm), + Some(&json!({"name": "renamed"})), + ); + assert_eq!( + (rn.status, rn.msg().as_str()), + (422, "repository rename is not supported") + ); + assert_eq!( + w.s.send( + "PATCH", + "/api/v1/repos/acme/alpha", + Some(&w.adm), + Some(&json!({"name": "alpha"})) + ) + .status, + 200 + ); + let nb = w.s.send( + "PATCH", + "/api/v1/repos/acme/alpha", + Some(&w.adm), + Some(&json!({"default_branch": "nope"})), + ); + assert_eq!( + (nb.status, nb.msg().as_str()), + (422, "branch does not exist [name: nope]") + ); + let rev = w.s.send( + "PATCH", + "/api/v1/repos/acme/alpha", + Some(&w.adm), + Some(&json!({"default_branch": "main~1"})), + ); + assert_eq!(rev.status, 422, "a revision is not a branch name"); + + // delete, then the name is free and a recreate is a different repository + let id = r["id"].as_i64().unwrap(); + assert_eq!( + w.s.send("DELETE", "/api/v1/repos/acme/alpha", Some(&w.adm), None) + .status, + 204 + ); + assert_eq!(w.s.get("/api/v1/repos/acme/alpha", &w.adm).status, 404); + assert_eq!( + w.s.send("DELETE", "/api/v1/repos/acme/alpha", Some(&w.adm), None) + .status, + 404 + ); + assert!(!w.s.dir.join("repos").join(format!("{id}.git")).exists()); + let again = w.mk("alpha", true); + assert_ne!(again["id"].as_i64().unwrap(), id); +} + +#[test] +fn names_follow_the_recorded_rules_and_are_unique_in_any_case() { + let (w, _) = world(); + w.mk("Mixed", false); + for dup in ["mixed", "MIXED"] { + let d = + w.s.post("/api/v1/orgs/acme/repos", &w.adm, json!({"name": dup})); + assert_eq!( + (d.status, d.msg().as_str()), + (409, "The repository with the same name already exists."), + "{dup}" + ); + } + for case in [ + "/repos/acme/mixed", + "/repos/acme/Mixed", + "/repos/ACME/mixed", + ] { + assert_eq!( + w.s.get(&format!("/api/v1{case}"), &w.adm).json()["name"], + "Mixed", + "{case}" + ); + } + for (name, status, msg) in [ + (".", 422, "name is reserved [name: .]"), + ("..", 422, "name is reserved [name: ..]"), + ( + "trail.git", + 422, + "name pattern is not allowed [pattern: *.git]", + ), + ("a b", 422, "[Name]: AlphaDashDot"), + ("", 422, "[Name]: Required"), + (".hidden", 201, ""), + ("-dash", 201, ""), + ("a..b", 201, ""), + ("api", 201, ""), + ] { + let r = + w.s.post("/api/v1/orgs/acme/repos", &w.adm, json!({"name": name})); + assert_eq!(r.status, status, "{name}"); + if status != 201 { + assert_eq!(r.msg(), msg, "{name}"); + } + } + let long = w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": "x".repeat(101)}), + ); + assert_eq!(long.msg(), "[Name]: MaxSize"); + let bad = w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": "ok", "default_branch": "a..b"}), + ); + assert_eq!(bad.status, 422); +} + +#[test] +fn a_foreign_token_gets_what_a_missing_repo_gets() { + let (w, t) = world(); + w.mk("secret", false); + let foreign = w.s.get("/api/v1/repos/acme/secret", &token(&t["dave"])); + let missing = w.s.get("/api/v1/repos/acme/ghost", &token(&t["dave"])); + assert_eq!( + (foreign.status, &foreign.raw), + (404, &missing.raw), + "no existence oracle" + ); + for path in ["/branches", "/branches/main", "/collaborators"] { + let a = w.s.get( + &format!("/api/v1/repos/acme/secret{path}"), + &token(&t["dave"]), + ); + let b = w.s.get( + &format!("/api/v1/repos/acme/ghost{path}"), + &token(&t["dave"]), + ); + assert_eq!((a.status, &a.raw), (404, &b.raw), "{path}"); + } + assert_eq!( + w.s.send( + "DELETE", + "/api/v1/repos/acme/secret", + Some(&token(&t["dave"])), + None + ) + .status, + 404 + ); + assert_eq!( + w.s.send( + "PATCH", + "/api/v1/repos/acme/secret", + Some(&token(&t["dave"])), + Some(&json!({})) + ) + .status, + 404 + ); +} + +#[test] +fn collaborator_modes_limit_what_each_can_do() { + let (w, t) = world(); + w.mk("r", true); + w.collab("r", "bob", "read"); + w.collab("r", "carol", "write"); + w.collab("r", "alice", "admin"); + let call = |who: &str, method: &str, path: &str, body: Option| { + w.s.send( + method, + &format!("/api/v1{path}"), + Some(&token(&t[who])), + body.as_ref(), + ) + .status + }; + // read: can read, cannot write anything + assert_eq!(call("bob", "GET", "/repos/acme/r", None), 200); + assert_eq!( + w.s.get("/api/v1/repos/acme/r", &token(&t["bob"])).json()["permissions"], + json!({"admin": false, "push": false, "pull": true}) + ); + for (m, p) in [ + ("PATCH", "/repos/acme/r"), + ("DELETE", "/repos/acme/r"), + ("PUT", "/repos/acme/r/collaborators/dave"), + ("GET", "/repos/acme/r/collaborators"), + ("DELETE", "/repos/acme/r/branches/x"), + ] { + assert_eq!(call("bob", m, p, Some(json!({}))), 403, "read {m} {p}"); + } + // write: can delete branches, still cannot administer + assert_eq!( + w.s.get("/api/v1/repos/acme/r", &token(&t["carol"])).json()["permissions"], + json!({"admin": false, "push": true, "pull": true}) + ); + for (m, p) in [ + ("PATCH", "/repos/acme/r"), + ("DELETE", "/repos/acme/r"), + ("PUT", "/repos/acme/r/collaborators/dave"), + ] { + assert_eq!(call("carol", m, p, Some(json!({}))), 403, "write {m} {p}"); + } + // admin collaborator: administers, may grant admin, but cannot delete the repository + assert_eq!( + call( + "alice", + "PATCH", + "/repos/acme/r", + Some(json!({"description": "x"})) + ), + 200 + ); + assert_eq!( + call( + "alice", + "PUT", + "/repos/acme/r/collaborators/dave", + Some(json!({"permission": "admin"})) + ), + 204 + ); + assert_eq!( + call("alice", "DELETE", "/repos/acme/r", None), + 403, + "only owner-level may delete" + ); + // the new admin (dave) is in the listing in insertion order, and removal works + let names: Vec = + w.s.get("/api/v1/repos/acme/r/collaborators", &w.adm) + .json() + .as_array() + .unwrap() + .iter() + .map(|u| u["login"].as_str().unwrap().into()) + .collect(); + assert_eq!(names, ["bob", "carol", "alice", "dave"]); + assert_eq!( + call("alice", "DELETE", "/repos/acme/r/collaborators/dave", None), + 204 + ); + assert_eq!( + call("dave", "GET", "/repos/acme/r", None), + 404, + "removed: invisible again" + ); +} + +#[test] +fn collaborator_edge_cases() { + let (w, _) = world(); + w.mk("r", false); + let put = |u: &str, body: Value| { + w.s.send( + "PUT", + &format!("/api/v1/repos/acme/r/collaborators/{u}"), + Some(&w.adm), + Some(&body), + ) + }; + let r = put("nobody-here", json!({"permission": "write"})); + assert_eq!( + (r.status, r.msg().as_str()), + (422, "user does not exist [uid: 0, name: nobody-here]") + ); + let r = put("acme", json!({})); + assert_eq!( + (r.status, r.msg().as_str()), + (422, "user does not exist [uid: 0, name: acme]"), + "an organization is not a collaborator" + ); + assert_eq!( + put("siteadmin", json!({})).status, + 422, + "the owner (org owner) cannot be added" + ); + assert_eq!(put("bob", json!({"permission": "superuser"})).status, 422); + assert_eq!( + put("bob", json!({})).status, + 204, + "write is the default mode" + ); + let perm = + w.s.get("/api/v1/repos/acme/r/collaborators/bob/permission", &w.adm) + .json(); + assert_eq!( + ( + perm["permission"].as_str(), + perm["role_name"].as_str(), + perm["user"]["login"].as_str() + ), + (Some("write"), Some("write"), Some("bob")) + ); + assert_eq!( + w.s.get( + "/api/v1/repos/acme/r/collaborators/siteadmin/permission", + &w.adm + ) + .json()["permission"], + "owner" + ); + assert_eq!( + w.s.get( + "/api/v1/repos/acme/r/collaborators/carol/permission", + &w.adm + ) + .json()["permission"], + "none" + ); +} + +#[test] +fn a_public_only_token_sees_and_creates_nothing() { + let (w, _) = world(); + w.mk("r", false); + let pw = w.s.mk_user("pat"); + let po = token(&w.s.mk_token( + "pat", + &pw, + "po", + &[ + "public-only", + "read:repository", + "write:user", + "read:user", + "write:organization", + ], + )); + let r = w.s.get("/api/v1/repos/acme/r", &po); + assert_eq!( + (r.status, r.msg().as_str()), + (403, "token scope is limited to public repos") + ); + let r = w.s.get("/api/v1/repos/acme/ghost", &po); + assert_eq!(r.status, 403, "refused before existence is looked at"); + for path in [ + "/api/v1/repos/search?limit=50", + "/api/v1/user/repos", + "/api/v1/orgs/acme/repos", + ] { + let l = w.s.get(path, &po); + assert_eq!(l.status, 200, "{path}"); + assert_eq!(l.headers["x-total-count"], "0", "{path}"); + } + assert_eq!( + w.s.get("/api/v1/repos/search", &po).json(), + json!({"ok": true, "data": []}) + ); + let c = w.s.post("/api/v1/user/repos", &po, json!({"name": "mine"})); + assert_eq!( + (c.status, c.msg().as_str()), + (403, "token scope is limited to public repos") + ); +} + +#[test] +fn listings_are_filtered_by_access_with_visible_counts() { + let (w, t) = world(); + for n in ["a1", "a2", "a3"] { + w.mk(n, false); + } + w.collab("a2", "bob", "read"); + let list = |tok: &str, path: &str| { + let r = w.s.get(path, &token(tok)); + let v = r.json(); + let items = v.get("data").cloned().unwrap_or(v); + ( + items + .as_array() + .unwrap() + .iter() + .map(|x| x["full_name"].as_str().unwrap().to_string()) + .collect::>(), + r.headers.get("x-total-count").cloned().unwrap(), + ) + }; + assert_eq!( + list(&t["bob"], "/api/v1/repos/search?limit=50&sort=alpha"), + (vec!["acme/a2".to_string()], "1".to_string()) + ); + assert_eq!( + list(&t["dave"], "/api/v1/repos/search"), + (vec![], "0".to_string()) + ); + assert_eq!( + list(&t["bob"], "/api/v1/user/repos"), + (vec!["acme/a2".to_string()], "1".to_string()) + ); + // an invisible org still answers 200 with only what the caller can read (recorded K#55) + assert_eq!( + list(&t["bob"], "/api/v1/orgs/acme/repos"), + (vec!["acme/a2".to_string()], "1".to_string()) + ); + assert_eq!( + list(&t["dave"], "/api/v1/orgs/acme/repos"), + (vec![], "0".to_string()) + ); + assert_eq!( + w.s.get("/api/v1/orgs/ghost/repos", &token(&t["bob"])) + .status, + 404 + ); + // the admin sees all three, paging keeps the total, q filters + let all = + w.s.get("/api/v1/repos/search?limit=2&page=2&sort=alpha", &w.adm); + assert_eq!( + ( + all.json()["data"].as_array().unwrap().len(), + all.headers["x-total-count"].as_str() + ), + (1, "3") + ); + assert_eq!( + list(&w.s.admin, "/api/v1/repos/search?q=A2").0, + vec!["acme/a2".to_string()] + ); +} + +#[test] +fn branches_are_listed_fetched_and_deleted_with_slashes_in_their_names() { + let (w, t) = world(); + let r = w.mk("br", false); + let id = r["id"].as_i64().unwrap(); + let empty = w.s.get("/api/v1/repos/acme/br/branches", &w.adm); + assert_eq!( + (empty.status, empty.json()), + (200, Value::Null), + "recorded: an empty repo lists null" + ); + let main = w.s.commit_on(id, "main", None); + w.s.commit_on(id, "feat/x", Some(&main)); + let list = w.s.get("/api/v1/repos/acme/br/branches", &w.adm); + let names: Vec<&str> = list + .json() + .as_array() + .unwrap() + .iter() + .map(|b| b["name"].as_str().unwrap()) + .collect::>() + .into_iter() + .map(|s| Box::leak(s.to_string().into_boxed_str()) as &str) + .collect(); + assert_eq!(names, ["feat/x", "main"]); + assert_eq!(list.headers["x-total-count"], "2"); + let b = + w.s.get("/api/v1/repos/acme/br/branches/feat/x", &w.adm) + .json(); + assert_eq!(b["name"], "feat/x"); + assert_eq!(b["commit"]["message"], "msg\n"); + assert_eq!(b["commit"]["author"]["email"], "t@example.invalid"); + for k in [ + "protected", + "required_approvals", + "enable_status_check", + "status_check_contexts", + "user_can_push", + "user_can_merge", + "effective_branch_protection_name", + ] { + assert!(b.get(k).is_some(), "{k}"); + } + let nope = w.s.get("/api/v1/repos/acme/br/branches/nope", &w.adm); + assert_eq!( + (nope.status, nope.json()["errors"][0].as_str()), + (404, Some("branch does not exist [name: nope]")) + ); + assert_eq!( + w.s.get("/api/v1/repos/acme/br/branches/main~1", &w.adm) + .status, + 404, + "never resolved as a revision" + ); + // delete: the default branch stays (403), others go (204), then 404 + let d = w.s.send( + "DELETE", + "/api/v1/repos/acme/br/branches/main", + Some(&w.adm), + None, + ); + assert_eq!( + (d.status, d.msg().as_str()), + (403, "can not delete default branch") + ); + assert_eq!( + w.s.send( + "DELETE", + "/api/v1/repos/acme/br/branches/feat/x", + Some(&w.adm), + None + ) + .status, + 204 + ); + assert_eq!( + w.s.send( + "DELETE", + "/api/v1/repos/acme/br/branches/feat/x", + Some(&w.adm), + None + ) + .status, + 404 + ); + // a read collaborator cannot delete branches + w.collab("br", "bob", "read"); + assert_eq!( + w.s.send( + "DELETE", + "/api/v1/repos/acme/br/branches/main", + Some(&token(&t["bob"])), + None + ) + .status, + 403 + ); + // changing the default branch works once the branch exists + w.s.commit_on(id, "dev", Some(&main)); + let p = w.s.send( + "PATCH", + "/api/v1/repos/acme/br", + Some(&w.adm), + Some(&json!({"default_branch": "dev"})), + ); + assert_eq!( + (p.status, p.json()["default_branch"].as_str()), + (200, Some("dev")) + ); + assert_eq!( + w.s.git(id, &["symbolic-ref", "HEAD"], None), + "refs/heads/dev" + ); +} + +#[test] +fn every_token_form_authenticates_on_a_repo_route_and_a_password_does_not() { + let (w, t) = world(); + w.mk("authrepo", true); + w.collab("authrepo", "bob", "write"); + let tok = &t["bob"]; + for (label, auth) in [ + ("token", token(tok)), + ("Bearer", format!("Bearer {tok}")), + ("basic token password", basic("bob", tok)), + ("basic token user", basic(tok, "")), + ("basic oauth2", basic("oauth2", tok)), + ("basic other name", basic("someone", tok)), + ] { + let r = w.s.get("/api/v1/repos/acme/authrepo", &auth); + assert_eq!( + (r.status, r.json()["full_name"].as_str()), + (200, Some("acme/authrepo")), + "{label}" + ); + } + let bad = w.s.get( + "/api/v1/repos/acme/authrepo", + &format!("Bearer {}", "e".repeat(40)), + ); + assert_eq!( + (bad.status, bad.msg().as_str()), + (401, "user does not exist [uid: 0, name: ]") + ); + assert_eq!( + w.s.send("GET", "/api/v1/repos/acme/authrepo", None, None) + .status, + 403 + ); + assert_eq!( + w.s.get( + "/api/v1/repos/acme/authrepo", + &basic("bob", "pw-bob-0123456789") + ) + .status, + 401, + "a password never authenticates an ordinary route" + ); +} + +#[test] +fn org_and_user_repo_creation_respect_ownership() { + let (w, t) = world(); + // alice owns user repos; a non-owner cannot create in acme; an org owner can + let r = w.s.post( + "/api/v1/user/repos", + &token(&t["alice"]), + json!({"name": "mine", "auto_init": true}), + ); + assert_eq!( + ( + r.status, + r.json()["full_name"].as_str(), + r.json()["owner"]["login"].as_str() + ), + (201, Some("alice/mine"), Some("alice")) + ); + let own = w.s.get("/api/v1/user/repos", &token(&t["alice"])); + assert_eq!(own.headers["x-total-count"], "1"); + let no = w.s.post( + "/api/v1/orgs/acme/repos", + &token(&t["alice"]), + json!({"name": "intruder"}), + ); + assert_eq!( + no.status, 404, + "acme is private: a non-owner cannot even tell it exists" + ); + // another user cannot read alice's repo, the site admin can + assert_eq!( + w.s.get("/api/v1/repos/alice/mine", &token(&t["bob"])) + .status, + 404 + ); + assert_eq!(w.s.get("/api/v1/repos/alice/mine", &w.adm).status, 200); + // the initial commit is authored by the creating user + let id = r.json()["id"].as_i64().unwrap(); + assert_eq!( + w.s.git(id, &["log", "-1", "--format=%an|%ae", "main"], None), + "alice|alice@example.invalid" + ); +} + +#[test] +fn concurrent_creates_of_one_name_yield_exactly_one_repository() { + let (w, _) = world(); + let (s, adm) = (&w.s, &w.adm); + let results: Vec = std::thread::scope(|sc| { + let hs: Vec<_> = (0..6) + .map(|_| { + sc.spawn(|| { + s.post( + "/api/v1/orgs/acme/repos", + adm, + json!({"name": "race", "auto_init": true}), + ) + .status + }) + }) + .collect(); + hs.into_iter().map(|h| h.join().unwrap()).collect() + }); + assert_eq!( + results.iter().filter(|c| **c == 201).count(), + 1, + "{results:?}" + ); + assert!( + results.iter().all(|c| *c == 201 || *c == 409), + "{results:?}" + ); + let dirs = std::fs::read_dir(w.s.dir.join("repos")).unwrap().count(); + assert_eq!(dirs, 1, "no leftover directory from the losers"); +} + +#[test] +fn deleting_a_repo_while_requests_are_in_flight_leaves_nothing_behind() { + let (w, _) = world(); + let r = w.mk("gone", true); + let id = r["id"].as_i64().unwrap(); + let (s, adm) = (&w.s, &w.adm); + std::thread::scope(|sc| { + let readers: Vec<_> = (0..4) + .map(|_| { + sc.spawn(|| { + (0..10) + .map(|_| s.get("/api/v1/repos/acme/gone/branches", adm).status) + .collect::>() + }) + }) + .collect(); + std::thread::sleep(std::time::Duration::from_millis(30)); + assert_eq!( + s.send("DELETE", "/api/v1/repos/acme/gone", Some(adm), None) + .status, + 204 + ); + for h in readers { + for st in h.join().unwrap() { + assert!( + st == 200 || st == 404, + "reads see the repo or its absence, never an error: {st}" + ); + } + } + }); + assert!(!w.s.dir.join("repos").join(format!("{id}.git")).exists()); + assert!(!w.s.dir.join("trash").join(format!("{id}.git")).exists()); +} diff --git a/crates/forge/tests/route_table.rs b/crates/forge/tests/route_table.rs index a5330180..ba3950e0 100644 --- a/crates/forge/tests/route_table.rs +++ b/crates/forge/tests/route_table.rs @@ -100,3 +100,22 @@ fn token_routes_are_exactly_the_password_routes() { ); } } + +#[test] +fn every_route_with_a_repository_parameter_resolves_it_through_the_actor() { + use scl_forge::routes::Actor; + for r in ROUTES.iter().filter(|r| r.pattern.contains("{r}")) { + assert!( + matches!(r.actor, Actor::Repo(_) | Actor::RepoOwner), + "{} {} names a repository but does not use Actor::Repo/RepoOwner", + r.method, + r.pattern + ); + assert!( + r.need.is_some(), + "{} {} must declare a scope", + r.method, + r.pattern + ); + } +} From 782fc6b85197478e8598084dc58c9b0cd2f42c84 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:38:32 +0300 Subject: [PATCH 10/16] docs(forge): F2a decisions as built, roadmap, conformance gate --- ROADMAP.md | 5 +- crates/forge/conformance/README.md | 2 +- .../specs/2026-10-22-p38-forge-design.md | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index df8a3f6b..9245da97 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -701,8 +701,9 @@ a Gitea-compatible Git forge (bare repos over smart HTTP, a recorded subset of `/api/v1`, SQLite metadata) so a platform can swap its project Git host by config. Design: `docs/superpowers/specs/2026-10-22-p38-forge-design.md`. Slices F0 (contract -capture) through F6 (acceptance on the first deployment); F0 and F1 (server -skeleton, users, organizations, tokens, `admin` CLI) are built. Hosted repos are +capture) through F6 (acceptance on the first deployment); F0, F1 (server +skeleton, users, organizations, tokens, `admin` CLI) and F2a (repositories API, lifecycle +and reconciler, collaborators, branches) are built. Hosted repos are bare Git and get none of the `.sc/` features. Other follow-up work is tracked as GitHub issues and listed under Deferred diff --git a/crates/forge/conformance/README.md b/crates/forge/conformance/README.md index ada1b9ac..cee78105 100644 --- a/crates/forge/conformance/README.md +++ b/crates/forge/conformance/README.md @@ -46,7 +46,7 @@ python3 -I run.py diff fixtures/1.22.6 fixtures/1.26.2 # offline version diff python3 -I run.py show fixtures/1.26.2 --only G --body # what was recorded ``` -Against the in-tree server use `./forge-check.sh` (it bootstraps an empty data directory with the `admin` CLI, +Against the in-tree server use `./forge-check.sh` (it replays scenarios `A,B,C,L,N,R`; the others wait for their routes, see `slice-pending.json` and the spec's "F2a decisions") (it bootstraps an empty data directory with the `admin` CLI, starts `sc-forge serve` on a free loopback port, runs `check` with the departures and `slice-pending.json`, and stops the server; CI runs it). `check --pending FILE` takes a JSON map `{scenario: [N, "METHOD /path"]}` (a bare `N` is also accepted): the server is expected to answer the first N exchanges of that scenario, the scenario must then stop at the named setup diff --git a/docs/superpowers/specs/2026-10-22-p38-forge-design.md b/docs/superpowers/specs/2026-10-22-p38-forge-design.md index 26358585..1f9cb171 100644 --- a/docs/superpowers/specs/2026-10-22-p38-forge-design.md +++ b/docs/superpowers/specs/2026-10-22-p38-forge-design.md @@ -684,6 +684,60 @@ header. Decisions drawn from it, beyond those stated in the sections above: until F2. `GET /api/healthz` and `GET /healthz` both answer. Only owners exist in `org_members` (no member roles, no teams). +### F2a decisions (as built) + +F2a is the repository half of F2 (plan: `docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md`); +the smart-HTTP transport, hooks, ref policy, admission control and threat-model pass #1 are F2b. + +- **Access.** One function, `repos::resolve`, turns (caller, repository) into a permission + (site admin or owner or org owner = Admin and owner; collaborator = its mode; else none) and + is the only place that does; F2b's git path calls it too. A missing, not-`live` or invisible + repository is one 404; a token restricted to `public-only` is refused 403 `token scope is + limited to public repos` before any existence is revealed, on every repository route and on + creation; below the route's minimum is 403. `DELETE` needs owner-level authority (an admin + collaborator cannot delete), `PATCH` and the collaborator routes need Admin, branch delete + needs Write. +- **Collaborators.** Only an Admin manages them; an admin collaborator may grant any mode + including `admin`; the owner (or an owner of the owning organization) cannot be added (422); + the target must be a user (422 `user does not exist [uid: 0, name: x]` also for an + organization); `permission` is `read|write|admin`, default `write`. Listed in the order added. +- **Listings** (`/repos/search`, `/user/repos`, `/orgs/{o}/repos`) filter in SQL by the same + rule; a `public-only` token sees an empty 200 with `X-Total-Count: 0`; `X-Total-Count` is the + visible count (recorded K#55-56 counted repositories the caller cannot see; a reviewed + departure once K runs). `sort=alpha` orders by the full name `owner/name`. An existing + organization the caller cannot see still answers 200 with what the caller can read (a + collaborator of a repository inside it sees it). +- **PATCH.** `description`, `website` and `default_branch` apply; `name` other than the current + one is 422 `repository rename is not supported`; a default branch that does not exist (or is + not a valid branch name, e.g. `main~1`) is 422; `private`, `has_*`, `allow_*` and the other + Gitea fields are accepted, ignored and answered with fixed values. The repository object keeps + `private: true` and `internal: false` (recorded Gitea flips both on `private:false`). +- **Object quirks kept.** The owner object nested in a repository has `active: false`, + `login_name: ""` and `is_admin: false` (recorded), `size` is 0, `empty` is computed live from + `HEAD`, a repository's initial commit has the message `Initial commit` and the author the + creating user, and `README.md` is `# ` plus two newlines. +- **Branches.** `{b..}` is a tail capture (names contain `/`). Branch names are validated as git + would (`refname::valid_branch`) and addressed only as `refs/heads/`; deleting the default + branch is 403 `can not delete default branch`; an empty repository lists branches as `null`. +- **Lifecycle.** `serve` holds an exclusive `flock` on `/.lock`, requires git 2.40 or + newer, then reconciles once before binding the listener. Per-repository mutexes serialize + create-finalize, delete, default-branch change and branch delete, each re-checking `live` + under the lock with conditional SQL. Creation fsyncs the whole new tree and `repos/`; any + failure before `live` rolls back in-process; **every `creating` row is rolled back at + startup** (the row does not record `auto_init`); a directory in `repos/` with no row at all is + **moved to `orphans/`, never deleted**, so a restored or recreated database cannot erase + repositories; a `live` row whose directory is missing is reported, never dropped. +- **Git runner.** Every spawn is `env_clear` with an explicit `--git-dir` (never discovery), + `GIT_CEILING_DIRECTORIES`, pinned per-spawn config, its own process group (signalled while the + leader is still an unreaped zombie, so the group id cannot be reused), a deadline, capped + output drained by dedicated threads, and an admission slot with a deadline (503 when busy, + `--git-concurrency`, default 8). Commit identity values containing control characters or + angle brackets are refused. +- **Conformance gate.** `forge-check.sh` replays `A`, `B`, `C`, `L`, `N` and `R`; `B` stops at + its first contents call (F3), `C` and `N` at their first `git clone` (F2b), and `X`, `D`, + `K`, `M` stay out until their routes exist (they make recorded calls to unbuilt routes, which + `--pending` cannot hide). 97 recorded exchanges are verified for real. + ## Open questions 1. The merge of a PR whose head is already in base (recorded: 405 `try again later`, From d247ed2437c019abcf5eb2eec54ff6655a106d8a Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:38:37 +0300 Subject: [PATCH 11/16] docs(forge): tidy README sentence --- crates/forge/conformance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge/conformance/README.md b/crates/forge/conformance/README.md index cee78105..aed01489 100644 --- a/crates/forge/conformance/README.md +++ b/crates/forge/conformance/README.md @@ -46,7 +46,7 @@ python3 -I run.py diff fixtures/1.22.6 fixtures/1.26.2 # offline version diff python3 -I run.py show fixtures/1.26.2 --only G --body # what was recorded ``` -Against the in-tree server use `./forge-check.sh` (it replays scenarios `A,B,C,L,N,R`; the others wait for their routes, see `slice-pending.json` and the spec's "F2a decisions") (it bootstraps an empty data directory with the `admin` CLI, +Against the in-tree server use `./forge-check.sh` (it replays scenarios `A,B,C,L,N,R`; the others wait for their routes, see `slice-pending.json` and the spec's "F2a decisions"; it bootstraps an empty data directory with the `admin` CLI, starts `sc-forge serve` on a free loopback port, runs `check` with the departures and `slice-pending.json`, and stops the server; CI runs it). `check --pending FILE` takes a JSON map `{scenario: [N, "METHOD /path"]}` (a bare `N` is also accepted): the server is expected to answer the first N exchanges of that scenario, the scenario must then stop at the named setup From c3c39614e08955fcf434f791dd189b5024c45c50 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:52:56 +0300 Subject: [PATCH 12/16] fix(forge): F2a review fixes (Opus, Sol) Busy runner is 503 on every route, a git failure is an error and never an empty repo, reads take a shared per-repository lock so they cannot straddle a delete, the reconciler propagates database errors and keeps data in orphans/, a failed cleanup keeps its journal row, a relative data dir is made absolute, the layout is fsynced, PATCH default-branch is decided under the lock, search decodes q and its Link URLs repeat the query, collaborator and email input is validated, and the tests that would have passed on broken code are strengthened. --- crates/forge/src/api.rs | 28 +- crates/forge/src/gitproc.rs | 71 ++++- crates/forge/src/gitread.rs | 39 ++- crates/forge/src/lifecycle.rs | 290 +++++++++++++++--- crates/forge/src/repo_api.rs | 108 ++++++- crates/forge/src/routes.rs | 2 +- crates/forge/tests/common/mod.rs | 24 +- crates/forge/tests/e2e_repos.rs | 227 +++++++++++++- .../plans/2026-10-23-p38-f2a-repos-api.md | 5 +- .../specs/2026-10-22-p38-forge-design.md | 38 ++- 10 files changed, 744 insertions(+), 88 deletions(-) diff --git a/crates/forge/src/api.rs b/crates/forge/src/api.rs index bc2ff69a..232455a9 100644 --- a/crates/forge/src/api.rs +++ b/crates/forge/src/api.rs @@ -119,10 +119,26 @@ pub(crate) fn paged( limit: i64, items: Vec, ) -> Resp { + paged_with(ctx, path, total, page, limit, &[], items) +} + +/// Like [`paged`], carrying `extra` query parameters (already percent-encoded, in the order +/// Gitea emits them: alphabetical after `limit` and `page`) into every `Link` URL, so following +/// a page link repeats the same query. +pub(crate) fn paged_with( + ctx: &Ctx, + path: &str, + total: i64, + page: i64, + limit: i64, + extra: &[(&str, String)], + items: Vec, +) -> Resp { + let tail: String = extra.iter().map(|(k, v)| format!("&{k}={v}")).collect(); let last = ((total + limit - 1) / limit).max(1); let url = |p: i64, rel: &str| { format!( - "<{}/api/v1{path}?limit={limit}&page={p}>; rel=\"{rel}\"", + "<{}/api/v1{path}?limit={limit}&page={p}{tail}>; rel=\"{rel}\"", ctx.app.base ) }; @@ -243,6 +259,12 @@ fn name_error(label: &str, name: &str, e: NameError) -> String { } } +/// An email must be usable as a commit identity too (it reaches git through the environment): +/// no control characters and no angle brackets. +fn email_ok(e: &str) -> bool { + e.contains('@') && !e.chars().any(|c| c.is_control() || matches!(c, '<' | '>')) +} + pub(crate) fn valid_visibility(v: &str) -> bool { matches!(v, "public" | "limited" | "private") } @@ -315,7 +337,7 @@ pub fn admin_create_user(ctx: &mut Ctx) -> Resp { if email.is_empty() { return error(&base, 422, "[Email]: Required"); } - if !email.contains('@') { + if !email_ok(&email) { return error(&base, 422, "[Email]: Email"); } if password.is_empty() { @@ -372,7 +394,7 @@ pub fn admin_patch_user(ctx: &mut Ctx) -> Resp { return error(&base, 422, "[Visibility]: invalid visibility"); } let email = str_field(&b, "email"); - if email.as_deref().is_some_and(|e| !e.contains('@')) { + if email.as_deref().is_some_and(|e| !email_ok(e)) { return error(&base, 422, "[Email]: Email"); } let password_hash = match str_field(&b, "password") { diff --git a/crates/forge/src/gitproc.rs b/crates/forge/src/gitproc.rs index 95d14849..10014233 100644 --- a/crates/forge/src/gitproc.rs +++ b/crates/forge/src/gitproc.rs @@ -34,6 +34,9 @@ pub enum GitError { OutputTooLarge, #[error("could not run git: {0}")] Spawn(#[from] std::io::Error), + /// git ran and failed in a way that means "something is wrong", not "not found". + #[error("git failed: {0}")] + Failed(String), } /// What a finished git run produced. @@ -103,8 +106,11 @@ impl Git { pub fn new(data_dir: &Path, bin: PathBuf, concurrency: usize) -> std::io::Result { std::fs::create_dir_all(data_dir.join("hooks"))?; std::fs::create_dir_all(data_dir.join("tmp").join("home"))?; + // Absolute, once: the child runs with the data directory as its working directory, so a + // relative path would resolve against itself and target `data/data/...`. + let data_dir = std::fs::canonicalize(data_dir)?; Ok(Git { - data_dir: data_dir.to_path_buf(), + data_dir, bin, gate: Gate::new(concurrency), timeout: Duration::from_secs(30), @@ -113,6 +119,17 @@ impl Git { }) } + /// The absolute data directory this runner works in. + pub fn data_dir(&self) -> &Path { + &self.data_dir + } + + /// Hold one admission slot until the returned permit is dropped. For tests that need a + /// saturated runner (every other spawn waits out its admission deadline and gets `Busy`). + pub fn occupy_for_tests(&self) -> crate::limits::Permit<'_> { + self.gate.acquire() + } + pub fn with_limits(mut self, timeout: Duration, admission: Duration, max_output: usize) -> Git { self.timeout = timeout; self.admission = admission; @@ -408,7 +425,7 @@ mod tests { .unwrap(); assert_eq!( hooks, - format!("{}/hooks", d.display()), + format!("{}/hooks", g.data_dir().display()), "hooksPath is pinned to the server's directory" ); assert!( @@ -420,6 +437,54 @@ mod tests { let _ = std::fs::remove_dir_all(d); } + #[test] + fn a_relative_data_dir_becomes_absolute_and_repos_land_where_expected() { + let rel = PathBuf::from(format!("scl-forge-rel-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&rel); + let g = Git::new(&rel, find_git().expect("git"), 2).unwrap(); + assert!(g.data_dir().is_absolute()); + let repo = g.data_dir().join("repos").join("1.git"); + std::fs::create_dir_all(g.data_dir().join("repos")).unwrap(); + let out = g + .run(&Spec { + args: &[ + "init", + "--bare", + "--quiet", + "--template=", + repo.to_str().unwrap(), + ], + ..Default::default() + }) + .unwrap(); + assert!(out.ok()); + assert!( + rel.join("repos").join("1.git").join("HEAD").is_file(), + "created under the path the caller gave, not `rel/rel/...`" + ); + assert!(!rel.join(&rel).exists()); + let _ = std::fs::remove_dir_all(&rel); + } + + #[test] + fn a_child_inherits_nothing_from_the_server_environment() { + let (g, d) = git("inherit"); + std::env::set_var("SC_FORGE_TEST_SENTINEL", "leaked"); + let out = g + .run_program(OsStr::new("/usr/bin/env"), &Spec::default()) + .unwrap(); + std::env::remove_var("SC_FORGE_TEST_SENTINEL"); + let text = String::from_utf8_lossy(&out.stdout).to_string(); + assert!( + !text.contains("SC_FORGE_TEST_SENTINEL"), + "env_clear must hide the parent environment:\n{text}" + ); + assert!( + text.contains("GIT_NO_REPLACE_OBJECTS=1") && text.contains("GIT_CEILING_DIRECTORIES=") + ); + let _ = std::fs::remove_dir_all(d); + } + #[test] fn identity_refuses_forged_header_characters() { assert!(Identity::new("alice", "a@example.invalid").is_some()); @@ -447,7 +512,7 @@ mod tests { ..Default::default() }) .unwrap(); - assert_eq!(hooks.text(), format!("{}/hooks", d.display())); + assert_eq!(hooks.text(), format!("{}/hooks", g.data_dir().display())); let user = g .run(&Spec { repo: Some(&repo), diff --git a/crates/forge/src/gitread.rs b/crates/forge/src/gitread.rs index 9d2b33b3..093e463c 100644 --- a/crates/forge/src/gitread.rs +++ b/crates/forge/src/gitread.rs @@ -45,6 +45,12 @@ pub fn list_branches(git: &Git, dir: &Path) -> Result, GitError> { ], ..Default::default() })?; + if !out.ok() { + // A damaged or vanished repository is not an empty one. + return Err(GitError::Failed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )); + } Ok(String::from_utf8_lossy(&out.stdout) .lines() .filter_map(|l| { @@ -57,6 +63,18 @@ pub fn list_branches(git: &Git, dir: &Path) -> Result, GitError> { .collect()) } +/// Exit status 1 from `rev-parse --verify -q` means "no such ref"; anything else (128: not a +/// repository, corrupt) is a failure the caller must not read as absence. +fn found(out: &crate::gitproc::Out) -> Result { + match out.status.code() { + Some(0) => Ok(true), + Some(1) => Ok(false), + _ => Err(GitError::Failed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )), + } +} + /// The commit a branch points at, or `None` when the branch does not exist (or the name is not /// a valid branch name). pub fn branch_oid(git: &Git, dir: &Path, name: &str) -> Result, GitError> { @@ -69,7 +87,7 @@ pub fn branch_oid(git: &Git, dir: &Path, name: &str) -> Result, G args: &["rev-parse", "--verify", "-q", "--end-of-options", &full], ..Default::default() })?; - Ok(out.ok().then(|| out.text())) + Ok(found(&out)?.then(|| out.text())) } /// A repository is empty when `HEAD` does not resolve to a commit (computed live). @@ -79,7 +97,7 @@ pub fn is_empty(git: &Git, dir: &Path) -> Result { args: &["rev-parse", "--verify", "-q", "HEAD"], ..Default::default() })?; - Ok(!out.ok()) + Ok(!found(&out)?) } fn person(line: &str) -> Option { @@ -195,6 +213,23 @@ mod tests { let _ = std::fs::remove_dir_all(d); } + #[test] + fn a_damaged_repository_is_an_error_not_an_empty_one() { + let (g, d, _dir) = repo("damaged"); + let broken = d.join("2.git"); // an empty directory: not a repository + std::fs::create_dir_all(&broken).unwrap(); + assert!(matches!(is_empty(&g, &broken), Err(GitError::Failed(_)))); + assert!(matches!( + list_branches(&g, &broken), + Err(GitError::Failed(_)) + )); + assert!(matches!( + branch_oid(&g, &broken, "main"), + Err(GitError::Failed(_)) + )); + let _ = std::fs::remove_dir_all(d); + } + #[test] fn branches_sort_by_name_and_resolve_only_as_full_refs() { let (g, d, dir) = repo("branches"); diff --git a/crates/forge/src/lifecycle.rs b/crates/forge/src/lifecycle.rs index 2471022e..a43c0d5c 100644 --- a/crates/forge/src/lifecycle.rs +++ b/crates/forge/src/lifecycle.rs @@ -18,7 +18,7 @@ use rusqlite::Connection; use std::collections::HashMap; use std::fs::File; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, RwLock}; /// Exclusive hold on the data directory for the life of the process. pub struct DirLock(#[allow(dead_code)] File); @@ -80,6 +80,7 @@ pub struct Report { pub trash_removed: usize, pub orphans_moved: usize, pub head_repaired: usize, + pub head_repair_failed: usize, pub missing_dirs: usize, } @@ -87,7 +88,7 @@ pub struct Report { pub struct Lifecycle { data_dir: PathBuf, pub git: Arc, - locks: Mutex>>>, + locks: Mutex>>>, #[cfg(test)] crash_at: Mutex>, } @@ -97,8 +98,15 @@ impl Lifecycle { for d in ["repos", "trash", "orphans"] { std::fs::create_dir_all(data_dir.join(d))?; } + // Absolute (the git children run with this as their working directory) and durable: the + // layout directories' own entries must survive a power loss before any row points at them. + let data_dir = std::fs::canonicalize(data_dir)?; + for d in ["repos", "trash", "orphans"] { + fsync_dir(&data_dir.join(d))?; + } + fsync_dir(&data_dir)?; Ok(Lifecycle { - data_dir: data_dir.to_path_buf(), + data_dir, git, locks: Mutex::default(), #[cfg(test)] @@ -106,6 +114,24 @@ impl Lifecycle { }) } + /// Run `f` (a read of repository `id` on disk) under the shared lock, and only if the + /// repository is still `live` once the lock is held. `None` means it is gone (deleted or being + /// deleted): the caller answers as for a missing repository. Without this, a read that + /// overlapped the delete's rename could see a half-vanished repository as an empty one. + pub fn with_read( + &self, + conn: &Connection, + id: i64, + f: impl FnOnce() -> T, + ) -> Result, rusqlite::Error> { + let lock = self.lock_for(id); + let _shared = lock.read().unwrap_or_else(|e| e.into_inner()); + Ok(match repos::find_by_id(conn, id)? { + Some(r) if r.state == "live" => Some(f()), + _ => None, + }) + } + /// `repos/.git`: always built from the integer id. pub fn repo_dir(&self, id: i64) -> PathBuf { self.data_dir.join("repos").join(format!("{id}.git")) @@ -115,8 +141,10 @@ impl Lifecycle { self.data_dir.join("trash").join(format!("{id}.git")) } - /// The mutex that serializes the server's operations on repository `id`. - pub fn lock_for(&self, id: i64) -> Arc> { + /// The lock that serializes the server's operations on repository `id`: delete, default-branch + /// change and branch delete take it exclusively; reads of the repository take it shared (see + /// [`Lifecycle::with_read`]), so a read can never straddle the delete's rename. + pub fn lock_for(&self, id: i64) -> Arc> { self.locks .lock() .unwrap_or_else(|e| e.into_inner()) @@ -197,8 +225,15 @@ impl Lifecycle { Err(e) => { // An ordinary failure rolls back now; if even that fails the row stays `creating` // and the next startup finishes the job. - let _ = remove_any(&self.repo_dir(row.id)); - let _ = repos::delete_row(conn, row.id); + match remove_any(&self.repo_dir(row.id)) { + Ok(()) => { + let _ = repos::delete_row(conn, row.id); + } + Err(c) => eprintln!( + "sc-forge: could not remove {} ({c}); the row stays `creating` for the next startup", + self.repo_dir(row.id).display() + ), + } Err(e) } } @@ -274,6 +309,7 @@ impl Lifecycle { self.step("committed")?; fsync_tree(&dir)?; fsync_dir(&self.data_dir.join("repos"))?; + fsync_dir(&self.data_dir)?; self.step("synced")?; if !repos::set_state(conn, row.id, "creating", "live", now)? { return Err(LifecycleError::Gone); @@ -285,7 +321,7 @@ impl Lifecycle { /// `Gone` when it was not `live` (someone else is already deleting it). pub fn delete(&self, conn: &Connection, repo: &Repo, now: i64) -> Result<(), LifecycleError> { let lock = self.lock_for(repo.id); - let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + let _held = lock.write().unwrap_or_else(|e| e.into_inner()); if !repos::set_state(conn, repo.id, "live", "deleting", now)? { return Err(LifecycleError::Gone); } @@ -323,10 +359,13 @@ impl Lifecycle { now: i64, ) -> Result<(), LifecycleError> { let lock = self.lock_for(repo.id); - let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + let _held = lock.write().unwrap_or_else(|e| e.into_inner()); let current = repos::find_by_id(conn, repo.id)? .filter(|r| r.state == "live") .ok_or(LifecycleError::Gone)?; + if current.default_branch == branch { + return Ok(()); // decided under the lock: a concurrent PATCH may already have set it + } let dir = self.repo_dir(current.id); let full = refname::branch_ref(branch); let exists = self.git.run(&Spec { @@ -357,11 +396,12 @@ impl Lifecycle { branch: &str, ) -> Result<(), LifecycleError> { let lock = self.lock_for(repo.id); - let _held = lock.lock().unwrap_or_else(|e| e.into_inner()); + let _held = lock.write().unwrap_or_else(|e| e.into_inner()); let current = repos::find_by_id(conn, repo.id)? .filter(|r| r.state == "live") .ok_or(LifecycleError::Gone)?; - if current.default_branch == branch { + // Case-insensitive: on a case-insensitive filesystem `MAIN` aliases the loose ref `main`. + if current.default_branch.eq_ignore_ascii_case(branch) { return Err(LifecycleError::GitFailed( "refusing to delete the default branch".into(), )); @@ -380,15 +420,38 @@ impl Lifecycle { Ok(()) } + /// Move `p` out of the way into `orphans/` (never deleting it). + fn to_orphans(&self, p: &Path) -> std::io::Result { + let name = p + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_default(); + let mut to = self.data_dir.join("orphans").join(&name); + let mut n = 0; + while to.exists() { + n += 1; + to = self.data_dir.join("orphans").join(format!("{name}.{n}")); + } + std::fs::rename(p, &to)?; + fsync_dir(&self.data_dir.join("orphans"))?; + Ok(to) + } + /// Resolve every state a crash can leave. Run once at startup, before the listener is bound /// and while holding the [`DirLock`]. Nothing is ever deleted that a live repository might - /// own: a directory with no row at all moves to `orphans/`. + /// own: a directory with no row, and the directory of a `creating` row, move to `orphans/` + /// (a restored database can resurrect either). A database error aborts the pass before any + /// directory is touched on its account. pub fn reconcile(&self, conn: &Connection, now: i64) -> Result { let mut rep = Report::default(); // 1. creating: roll back (the row does not record auto_init, so a half-initialized - // repository is never promoted; the client never saw a 201 for it). + // repository is never promoted; the client never saw a 201 for it). The directory is + // kept in orphans/, and the row is deleted only after that succeeded. for r in repos::list_in_state(conn, "creating")? { - let _ = remove_any(&self.repo_dir(r.id)); + let dir = self.repo_dir(r.id); + if std::fs::symlink_metadata(&dir).is_ok() { + self.to_orphans(&dir)?; + } repos::delete_row(conn, r.id)?; rep.creating_rolled_back += 1; } @@ -407,12 +470,13 @@ impl Lifecycle { let _ = std::fs::remove_dir_all(&to); rep.deleting_finished += 1; } - // 3. trash with no row. + // 3. trash with no row. A lookup error propagates: it must never read as "no row". for e in std::fs::read_dir(self.data_dir.join("trash"))? { let p = e?.path(); - let known = file_id(&p) - .and_then(|id| repos::find_by_id(conn, id).ok().flatten()) - .is_some(); + let known = match file_id(&p) { + Some(id) => repos::find_by_id(conn, id)?.is_some(), + None => false, + }; if !known { remove_any(&p)?; rep.trash_removed += 1; @@ -421,21 +485,12 @@ impl Lifecycle { // 4. a directory in repos/ with no row at all: keep it, out of the way, never delete it. for e in std::fs::read_dir(self.data_dir.join("repos"))? { let p = e?.path(); - let known = file_id(&p) - .and_then(|id| repos::find_by_id(conn, id).ok().flatten()) - .is_some(); + let known = match file_id(&p) { + Some(id) => repos::find_by_id(conn, id)?.is_some(), + None => false, + }; if !known { - let name = p - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_default(); - let mut to = self.data_dir.join("orphans").join(&name); - let mut n = 0; - while to.exists() { - n += 1; - to = self.data_dir.join("orphans").join(format!("{name}.{n}")); - } - std::fs::rename(&p, &to)?; + let to = self.to_orphans(&p)?; eprintln!( "sc-forge: {} has no repository row; moved to {}", p.display(), @@ -445,7 +500,8 @@ impl Lifecycle { } } // 5. live: HEAD follows the row (the row is the authority); a missing directory is - // reported, never silently dropped. + // reported, never silently dropped; one repository that cannot be repaired is reported + // and does not stop the others (or the server) from starting. for r in repos::list_in_state(conn, "live")? { let dir = self.repo_dir(r.id); if !dir.is_dir() { @@ -457,14 +513,28 @@ impl Lifecycle { continue; } let want = refname::branch_ref(&r.default_branch); - let head = self.git.run(&Spec { - repo: Some(&dir), - args: &["symbolic-ref", "-q", "HEAD"], - ..Default::default() - })?; - if head.text() != want { + let fix = || -> Result { + let head = self.git.run(&Spec { + repo: Some(&dir), + args: &["symbolic-ref", "-q", "HEAD"], + ..Default::default() + })?; + if head.text() == want { + return Ok(false); + } self.run(&dir, &["symbolic-ref", "HEAD", &want], None, None)?; - rep.head_repaired += 1; + Ok(true) + }; + match fix() { + Ok(true) => rep.head_repaired += 1, + Ok(false) => {} + Err(e) => { + eprintln!( + "sc-forge: cannot repair HEAD of {}/{} (id {}): {e}", + r.owner, r.name, r.id + ); + rep.head_repair_failed += 1; + } } } let _ = now; @@ -774,6 +844,7 @@ mod tests { trash_removed: 1, orphans_moved: 1, head_repaired: 1, + head_repair_failed: 0, missing_dirs: 1 } ); @@ -900,4 +971,143 @@ mod tests { assert!(DirLock::acquire(&dir).is_ok()); let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn an_interrupted_creates_directory_is_kept_in_orphans_not_deleted() { + let e = env("keep-creating"); + e.lc.arm("committed"); + assert!(matches!( + make(&e, "half", true), + Err(LifecycleError::Crash(_)) + )); + let id = repos::find_row(&e.conn, "owner", "half") + .unwrap() + .unwrap() + .id; + let rep = e.lc.reconcile(&e.conn, 20).unwrap(); + assert_eq!(rep.creating_rolled_back, 1); + assert!(!e.lc.repo_dir(id).exists()); + assert!( + e.dir + .join("orphans") + .join(format!("{id}.git")) + .join("HEAD") + .is_file(), + "the data survives, out of service" + ); + assert!(repos::find_row(&e.conn, "owner", "half").unwrap().is_none()); + } + + #[test] + fn a_failed_cleanup_keeps_the_creating_row_so_the_name_stays_blocked_until_it_is_done() { + use std::os::unix::fs::PermissionsExt; + let e = env("cleanup-fails"); + e.lc.arm("committed"); + assert!(matches!( + make(&e, "stuck", true), + Err(LifecycleError::Crash(_)) + )); + let id = repos::find_row(&e.conn, "owner", "stuck") + .unwrap() + .unwrap() + .id; + // repos/ becomes read-only: neither removal nor a move out of it can succeed + let repos_dir = e.dir.join("repos"); + std::fs::set_permissions(&repos_dir, std::fs::Permissions::from_mode(0o500)).unwrap(); + let blocked = e.lc.reconcile(&e.conn, 20); + std::fs::set_permissions(&repos_dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + if std::fs::metadata(&repos_dir) + .map(|_| nix_is_root()) + .unwrap_or(false) + { + return; // root ignores directory permissions; the scenario cannot be staged + } + assert!(blocked.is_err(), "the failed move is reported"); + let row = repos::find_row(&e.conn, "owner", "stuck") + .unwrap() + .expect("the journal row survives a failed cleanup"); + assert_eq!(row.state, "creating"); + assert!(e.lc.repo_dir(id).exists(), "nothing was lost"); + assert!( + matches!(make(&e, "stuck", false), Err(LifecycleError::Exists)), + "the name stays blocked until the cleanup is done" + ); + assert_eq!( + e.lc.reconcile(&e.conn, 30).unwrap().creating_rolled_back, + 1, + "the next pass finishes it" + ); + assert!(make(&e, "stuck", false).is_ok()); + } + + fn nix_is_root() -> bool { + // SAFETY: geteuid has no preconditions. + unsafe { libc::geteuid() == 0 } + } + + #[test] + fn a_database_error_aborts_the_pass_before_any_directory_moves() { + let e = env("db-error"); + let live = make(&e, "alive", true).unwrap(); + std::fs::create_dir_all(e.lc.repo_dir(98).join("objects")).unwrap(); // an orphan to be moved + e.conn + .execute_batch("ALTER TABLE repos RENAME TO repos_gone") + .unwrap(); + assert!(e.lc.reconcile(&e.conn, 20).is_err()); + assert!( + e.lc.repo_dir(live.id).join("HEAD").is_file(), + "a live repository is never moved on a lookup error" + ); + assert!(e.lc.repo_dir(98).exists(), "nothing was touched"); + } + + #[test] + fn one_unrepairable_repository_does_not_stop_the_pass() { + let e = env("one-bad"); + let bad = make(&e, "bad", true).unwrap(); + let good = + e.lc.create(&e.conn, &e.owner, &e.owner, "good", "", "main", true, 10) + .unwrap(); + e.lc.git + .run(&Spec { + repo: Some(&e.lc.repo_dir(good.id)), + args: &["symbolic-ref", "HEAD", "refs/heads/other"], + ..Default::default() + }) + .unwrap(); + std::fs::remove_file(e.lc.repo_dir(bad.id).join("HEAD")).unwrap(); // no longer a repository + let rep = e.lc.reconcile(&e.conn, 20).unwrap(); + assert_eq!((rep.head_repair_failed, rep.head_repaired), (1, 1)); + assert_eq!(head(&e, &good), "refs/heads/main"); + } + + #[test] + fn naming_the_current_default_branch_is_a_no_op_decided_under_the_lock() { + let e = env("default-noop"); + let r = make(&e, "alpha", true).unwrap(); + e.lc.set_default_branch(&e.conn, &r, "main", 20).unwrap(); // already main + assert_eq!(head(&e, &r), "refs/heads/main"); + // a stale snapshot naming another branch as current still resolves against the row + let stale = Repo { + default_branch: "dev".into(), + ..r.clone() + }; + e.lc.set_default_branch(&e.conn, &stale, "main", 21) + .unwrap(); + assert_eq!( + repos::find_by_id(&e.conn, r.id) + .unwrap() + .unwrap() + .default_branch, + "main" + ); + } + + #[test] + fn the_default_branch_cannot_be_deleted_by_a_case_alias() { + let e = env("case-alias"); + let r = make(&e, "alpha", true).unwrap(); + assert!(e.lc.delete_branch(&e.conn, &r, "MAIN").is_err()); + assert!(rev(&e, &r, "refs/heads/main").is_some()); + } } diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs index 1cd82eed..42fdd190 100644 --- a/crates/forge/src/repo_api.rs +++ b/crates/forge/src/repo_api.rs @@ -1,7 +1,8 @@ //! F2a handlers: repositories, search and lists, collaborators, branches. Authentication, scope //! and the repository permission check already passed in `routes::dispatch` (`ctx.access`). -use crate::api::{body, page_params, paged, str_field, user_json}; +use crate::api::{body, page_params, paged, paged_with, str_field, user_json}; +use crate::gitproc::GitError; use crate::gitread; use crate::http::{error, not_found, Resp}; use crate::lifecycle::LifecycleError; @@ -37,6 +38,19 @@ fn life_err(app: &App, e: LifecycleError) -> Resp { } } +/// The one mapping of a failed read of a repository: busy is 503; a repository that is no +/// longer `live` (a read that raced its delete) is the same 404 as a missing one; anything else +/// is a 500. +fn git_read_err(app: &App, conn: &rusqlite::Connection, repo_id: i64, e: GitError) -> Resp { + if matches!(e, GitError::Busy) { + return error(&app.base, 503, "too many git operations in flight"); + } + match repos::find_by_id(conn, repo_id) { + Ok(Some(r)) if r.state == "live" => app.internal(&e), + _ => not_found(&app.base, "The target couldn't be found.", None), + } +} + fn ssh_authority(base: &str) -> String { base.split("://") .nth(1) @@ -52,8 +66,15 @@ fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Re let owner = store::find_by_id(conn, r.owner_id) .map_err(|e| app.internal(&e))? .ok_or_else(|| app.internal(&"repo without owner"))?; - let empty = - gitread::is_empty(&app.life.git, &app.life.repo_dir(r.id)).map_err(|e| app.internal(&e))?; + // Under the shared repository lock, and only while the repository is still live: a read must + // never straddle a delete's rename and see a half-vanished repository as an empty one. + let empty = match app.life.with_read(conn, r.id, || { + gitread::is_empty(&app.life.git, &app.life.repo_dir(r.id)) + }) { + Err(e) => return Err(app.internal(&e)), + Ok(None) => return Err(not_found(&app.base, "The target couldn't be found.", None)), + Ok(Some(res)) => res.map_err(|e| git_read_err(app, conn, r.id, e))?, + }; let mut owner_json = user_json(&app.base, &owner); owner_json["active"] = json!(false); // recorded: the owner object inside a repository reads inactive owner_json["login_name"] = json!(""); // recorded: the nested owner never carries the login_name @@ -235,7 +256,9 @@ pub fn patch_repo(ctx: &mut Ctx) -> Resp { if str_field(&b, "name").is_some_and(|n| n != a.repo.name) { return error(&base, 422, "repository rename is not supported"); } - if let Some(branch) = str_field(&b, "default_branch").filter(|n| *n != a.repo.default_branch) { + // Always handed to the locked operation, which compares against the row under the lock (a + // snapshot taken before the lock could be stale). + if let Some(branch) = str_field(&b, "default_branch") { if !refname::valid_branch(&branch) { return error(&base, 422, "[DefaultBranch]: invalid branch name"); } @@ -333,19 +356,49 @@ fn query_list(ctx: &Ctx, filter: &Filter, sort: Sort, path: &str, wrap: bool) -> /// `GET /repos/search`: `{"ok": true, "data": [...]}`; a `public-only` token sees nothing. pub fn search(ctx: &mut Ctx) -> Resp { - let q = ctx.req.param("q").map(|v| v.replace('+', " ")); + // `q` arrives percent-encoded with `+` for a space (form encoding). + let q = ctx + .req + .param("q") + .and_then(|v| crate::routes::percent_decode(&v.replace('+', " "))); let sort = if ctx.req.param("sort") == Some("alpha") { Sort::Alpha } else { Sort::Id }; - query_list( - ctx, + // Page links repeat the query, in the recorded order (alphabetical after limit and page). + let mut extra: Vec<(&str, String)> = Vec::new(); + if let Some(raw) = ctx.req.param("q") { + extra.push(("q", raw.to_string())); + } + if ctx.req.param("sort") == Some("alpha") { + extra.push(("sort", "alpha".to_string())); + } + let (page, limit) = page_params(ctx); + match repos::list_visible( + ctx.conn, + &ctx.who.user, + token_public_only(ctx), &Filter { q, owner_id: None }, sort, - "/repos/search", - true, - ) + page, + limit, + ) { + Ok((rows, total)) => { + let items = match listed(ctx, &rows) { + Ok(i) => i, + Err(r) => return r, + }; + let links = paged_with(ctx, "/repos/search", total, page, limit, &extra, vec![]); + let mut r = Resp::json(200, &json!({"ok": true, "data": items})) + .with("x-total-count", total.to_string()); + if let Some((_, link)) = links.headers.iter().find(|(k, _)| *k == "link") { + r = r.with("link", link.clone()); + } + r + } + Err(e) => ctx.app.internal(&e), + } } /// `GET /user/repos`: every repository the caller can read. @@ -426,6 +479,11 @@ pub fn put_collaborator(ctx: &mut Ctx) -> Resp { Err(r) => return r, }; let base = ctx.app.base.clone(); + if b.get("permission") + .is_some_and(|v| !v.is_string() && !v.is_null()) + { + return error(&base, 422, "[Permission]: invalid permission"); + } let mode = match str_field(&b, "permission") { None => Perm::Write, Some(m) => match Perm::from_mode(&m) { @@ -492,7 +550,7 @@ pub fn collaborator_permission(ctx: &mut Ctx) -> Resp { fn branch_json(ctx: &Ctx, a: &Access, br: &gitread::Branch) -> Result { let dir = ctx.app.life.repo_dir(a.repo.id); let c = gitread::commit(&ctx.app.life.git, &dir, &br.oid) - .map_err(|e| ctx.app.internal(&e))? + .map_err(|e| git_read_err(ctx.app, ctx.conn, a.repo.id, e))? .ok_or_else(|| ctx.app.internal(&"branch points at no commit"))?; let who = |p: &gitread::Person| json!({"name": p.name, "email": p.email, "username": ""}); let full = format!("{}/{}", a.repo.owner, a.repo.name); @@ -520,6 +578,16 @@ fn branch_json(ctx: &Ctx, a: &Access, br: &gitread::Branch) -> Result Resp) -> Resp { + match ctx.app.life.with_read(ctx.conn, a.repo.id, || f(ctx)) { + Ok(Some(r)) => r, + Ok(None) => not_found(&ctx.app.base, "The target couldn't be found.", None), + Err(e) => ctx.app.internal(&e), + } +} + fn no_branch(ctx: &Ctx, name: &str) -> Resp { not_found( &ctx.app.base, @@ -534,9 +602,13 @@ pub fn list_branches(ctx: &mut Ctx) -> Resp { Ok(a) => a.clone(), Err(r) => return r, }; + read_locked(ctx, &a, |ctx| list_branches_inner(ctx, &a)) +} + +fn list_branches_inner(ctx: &Ctx, a: &Access) -> Resp { let all = match gitread::list_branches(&ctx.app.life.git, &ctx.app.life.repo_dir(a.repo.id)) { Ok(b) => b, - Err(e) => return ctx.app.internal(&e), + Err(e) => return git_read_err(ctx.app, ctx.conn, a.repo.id, e), }; if all.is_empty() { return Resp::json(200, &Value::Null).with("x-total-count", "0".into()); @@ -545,7 +617,7 @@ pub fn list_branches(ctx: &mut Ctx) -> Resp { let total = all.len() as i64; let start = (((page - 1) * limit) as usize).min(all.len()); let rows = &all[start..(start + limit as usize).min(all.len())]; - let items: Result, Resp> = rows.iter().map(|b| branch_json(ctx, &a, b)).collect(); + let items: Result, Resp> = rows.iter().map(|b| branch_json(ctx, a, b)).collect(); match items { Ok(items) => paged( ctx, @@ -565,14 +637,18 @@ pub fn get_branch(ctx: &mut Ctx) -> Resp { Ok(a) => a.clone(), Err(r) => return r, }; + read_locked(ctx, &a, |ctx| get_branch_inner(ctx, &a)) +} + +fn get_branch_inner(ctx: &Ctx, a: &Access) -> Resp { let name = ctx.param("b").to_string(); match gitread::branch_oid(&ctx.app.life.git, &ctx.app.life.repo_dir(a.repo.id), &name) { - Ok(Some(oid)) => match branch_json(ctx, &a, &gitread::Branch { name, oid }) { + Ok(Some(oid)) => match branch_json(ctx, a, &gitread::Branch { name, oid }) { Ok(v) => Resp::json(200, &v), Err(r) => r, }, Ok(None) => no_branch(ctx, &name), - Err(e) => ctx.app.internal(&e), + Err(e) => git_read_err(ctx.app, ctx.conn, a.repo.id, e), } } @@ -586,7 +662,7 @@ pub fn delete_branch(ctx: &mut Ctx) -> Resp { if !refname::valid_branch(&name) { return no_branch(ctx, &name); } - if name == a.repo.default_branch { + if name.eq_ignore_ascii_case(&a.repo.default_branch) { return error(&ctx.app.base, 403, "can not delete default branch"); } match ctx.app.life.delete_branch(ctx.conn, &a.repo, &name) { diff --git a/crates/forge/src/routes.rs b/crates/forge/src/routes.rs index 3dd31b01..34d32776 100644 --- a/crates/forge/src/routes.rs +++ b/crates/forge/src/routes.rs @@ -358,7 +358,7 @@ pub static ROUTES: &[Route] = &[ type Params = Vec<(&'static str, String)>; /// Decode `%XX` escapes in a path segment; `None` when malformed or not UTF-8. -fn percent_decode(s: &str) -> Option { +pub(crate) fn percent_decode(s: &str) -> Option { let b = s.as_bytes(); let mut out = Vec::with_capacity(b.len()); let mut i = 0; diff --git a/crates/forge/tests/common/mod.rs b/crates/forge/tests/common/mod.rs index a840869d..4cd53194 100644 --- a/crates/forge/tests/common/mod.rs +++ b/crates/forge/tests/common/mod.rs @@ -22,6 +22,8 @@ pub struct Srv { pub base: String, /// All-scope token of the site admin `siteadmin`. pub admin: String, + /// The runner the server uses (tests occupy its slots to stage a busy server). + pub runner: std::sync::Arc, _data_lock: scl_forge::lifecycle::DirLock, stop: Option>, thread: Option>, @@ -83,7 +85,20 @@ impl Srv { Srv::start_cfg(Limits::default(), Some(b)) } + /// Start with a runner of `slots` concurrent git children and the given admission deadline. + pub fn start_git(slots: usize, admission: std::time::Duration) -> Srv { + Srv::start_full(Limits::default(), None, Some((slots, admission))) + } + fn start_cfg(limits: Limits, backoff: Option) -> Srv { + Srv::start_full(limits, backoff, None) + } + + fn start_full( + limits: Limits, + backoff: Option, + git_cfg: Option<(usize, std::time::Duration)>, + ) -> Srv { let dir = std::env::temp_dir().join(format!( "scl-forge-e2e-{}-{}", std::process::id(), @@ -108,11 +123,13 @@ impl Srv { let addr = std_l.local_addr().unwrap(); let base = format!("http://{addr}"); let data_lock = scl_forge::lifecycle::DirLock::acquire(&dir).unwrap(); + let (slots, admission) = git_cfg.unwrap_or((8, std::time::Duration::from_secs(5))); let git = std::sync::Arc::new( - scl_forge::gitproc::Git::new(&dir, scl_forge::gitproc::find_git().expect("git"), 8) - .unwrap(), + scl_forge::gitproc::Git::new(&dir, scl_forge::gitproc::find_git().expect("git"), slots) + .unwrap() + .with_limits(std::time::Duration::from_secs(30), admission, 16 << 20), ); - let life = scl_forge::lifecycle::Lifecycle::new(&dir, git).unwrap(); + let life = scl_forge::lifecycle::Lifecycle::new(&dir, git.clone()).unwrap(); let mut app = scl_forge::routes::App::new(dir.clone(), &base, Proxies::default(), 4, life); if let Some(b) = backoff { app.backoff = b; @@ -140,6 +157,7 @@ impl Srv { }); }); Srv { + runner: git, _data_lock: data_lock, addr, dir, diff --git a/crates/forge/tests/e2e_repos.rs b/crates/forge/tests/e2e_repos.rs index 379647b9..35171e02 100644 --- a/crates/forge/tests/e2e_repos.rs +++ b/crates/forge/tests/e2e_repos.rs @@ -775,35 +775,244 @@ fn concurrent_creates_of_one_name_yield_exactly_one_repository() { #[test] fn deleting_a_repo_while_requests_are_in_flight_leaves_nothing_behind() { + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; let (w, _) = world(); let r = w.mk("gone", true); let id = r["id"].as_i64().unwrap(); + let before = w.s.get("/api/v1/repos/acme/gone/branches", &w.adm).json(); + assert!(before.is_array()); let (s, adm) = (&w.s, &w.adm); + let (reads, stop) = (AtomicUsize::new(0), AtomicBool::new(false)); std::thread::scope(|sc| { let readers: Vec<_> = (0..4) .map(|_| { sc.spawn(|| { - (0..10) - .map(|_| s.get("/api/v1/repos/acme/gone/branches", adm).status) - .collect::>() + let mut seen = Vec::new(); + while !stop.load(Ordering::SeqCst) { + let r = s.get("/api/v1/repos/acme/gone/branches", adm); + reads.fetch_add(1, Ordering::SeqCst); + seen.push((r.status, r.json())); + } + seen }) }) .collect(); - std::thread::sleep(std::time::Duration::from_millis(30)); + while reads.load(Ordering::SeqCst) < 8 { + std::thread::sleep(std::time::Duration::from_millis(2)); // overlap is proven, not slept + } assert_eq!( s.send("DELETE", "/api/v1/repos/acme/gone", Some(adm), None) .status, 204 ); + std::thread::sleep(std::time::Duration::from_millis(50)); + stop.store(true, Ordering::SeqCst); + let mut ok_before_delete = 0; for h in readers { - for st in h.join().unwrap() { - assert!( - st == 200 || st == 404, - "reads see the repo or its absence, never an error: {st}" - ); + for (status, body) in h.join().unwrap() { + match status { + 200 => { + assert_eq!(body, before, "a read that wins the race sees the full branch list, never an empty or partial one"); + ok_before_delete += 1; + } + 404 => {} + other => panic!("reads see the repo or its absence, never {other}"), + } } } + assert!(ok_before_delete > 0, "some reads overlapped the delete"); }); assert!(!w.s.dir.join("repos").join(format!("{id}.git")).exists()); assert!(!w.s.dir.join("trash").join(format!("{id}.git")).exists()); } + +#[test] +fn a_write_collaborator_can_delete_branches_and_scope_is_checked_before_existence() { + let (w, t) = world(); + let id = w.mk("r", false)["id"].as_i64().unwrap(); + let main = w.s.commit_on(id, "main", None); + w.s.commit_on(id, "topic", Some(&main)); + w.collab("r", "carol", "write"); + assert_eq!( + w.s.send( + "DELETE", + "/api/v1/repos/acme/r/branches/topic", + Some(&token(&t["carol"])), + None + ) + .status, + 204 + ); + // a token without the repository scope: 403 on a repo that exists and on one that does not + let pw = w.s.mk_user("scoped"); + let user_only = token(&w.s.mk_token("scoped", &pw, "u", &["read:user"])); + for repo in ["r", "ghost"] { + let r = w.s.get(&format!("/api/v1/repos/acme/{repo}"), &user_only); + assert_eq!( + (r.status, r.msg().as_str()), + ( + 403, + "token does not have at least one of required scope(s): [read:repository]" + ), + "{repo}" + ); + } +} + +#[test] +fn a_repository_that_is_being_deleted_is_invisible_to_everyone() { + let (w, _) = world(); + w.mk("going", false); + let conn = scl_forge::db::connect(&w.s.dir).unwrap(); + conn.execute( + "UPDATE repos SET state = 'deleting' WHERE name = 'going'", + [], + ) + .unwrap(); + for path in [ + "/api/v1/repos/acme/going", + "/api/v1/repos/acme/going/branches", + ] { + assert_eq!( + w.s.get(path, &w.adm).status, + 404, + "{path}: even for a site admin" + ); + } + assert_eq!( + w.s.get("/api/v1/repos/search", &w.adm).headers["x-total-count"], + "0" + ); + let dup = + w.s.post("/api/v1/orgs/acme/repos", &w.adm, json!({"name": "going"})); + assert_eq!( + dup.status, 409, + "the name stays blocked until the delete finishes" + ); +} + +#[test] +fn search_decodes_q_and_page_links_repeat_the_query() { + let (w, _) = world(); + for (n, d) in [ + ("s1", "hello world"), + ("s2", "hello world"), + ("s3", "hello world"), + ("other", "nothing"), + ] { + let r = w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": n, "description": d}), + ); + assert_eq!(r.status, 201); + } + for q in ["hello%20world", "hello+world"] { + let r = w.s.get( + &format!("/api/v1/repos/search?q={q}&limit=2&sort=alpha"), + &w.adm, + ); + assert_eq!(r.headers["x-total-count"], "3", "{q}"); + assert_eq!(r.json()["data"].as_array().unwrap().len(), 2, "{q}"); + } + let r = w.s.get( + "/api/v1/repos/search?q=hello%20world&limit=2&sort=alpha", + &w.adm, + ); + let link = r.headers["link"].clone(); + assert!( + link.contains("limit=2&page=2&q=hello%20world&sort=alpha>; rel=\"next\""), + "{link}" + ); +} + +#[test] +fn malformed_collaborator_and_email_input_is_refused() { + let (w, _) = world(); + w.mk("r", false); + for bad in [ + json!({"permission": 123}), + json!({"permission": {"x": 1}}), + json!({"permission": true}), + ] { + let r = w.s.send( + "PUT", + "/api/v1/repos/acme/r/collaborators/bob", + Some(&w.adm), + Some(&bad), + ); + assert_eq!(r.status, 422, "{bad}"); + } + assert_eq!( + w.s.get("/api/v1/repos/acme/r/collaborators", &w.adm) + .headers["x-total-count"], + "0", + "nothing was granted" + ); + // an email that could not be a commit identity is refused where users are made + let r = w.s.post( + "/api/v1/admin/users", + &w.adm, + json!({"username": "evil", "email": "x@y>", "password": "pw"}), + ); + assert_eq!((r.status, r.msg().as_str()), (422, "[Email]: Email")); + let r = w.s.post( + "/api/v1/admin/users", + &w.adm, + json!({"username": "evil2", "email": "x@y\nz", "password": "pw"}), + ); + assert_eq!(r.status, 422); +} + +#[test] +fn a_saturated_runner_answers_503_on_reads_and_writes_not_500() { + let s = Srv::start_git(1, std::time::Duration::from_millis(150)); + let adm = token(&s.admin); + s.post("/api/v1/orgs", &adm, json!({"username": "acme"})); + assert_eq!( + s.post( + "/api/v1/orgs/acme/repos", + &adm, + json!({"name": "r", "auto_init": true}) + ) + .status, + 201 + ); + let slot = s.runner.occupy_for_tests(); + for (method, path, body) in [ + ("GET", "/api/v1/repos/acme/r", None), + ("GET", "/api/v1/repos/acme/r/branches", None), + ("GET", "/api/v1/repos/acme/r/branches/main", None), + ("GET", "/api/v1/repos/search", None), + ( + "POST", + "/api/v1/orgs/acme/repos", + Some(json!({"name": "other"})), + ), + ( + "PATCH", + "/api/v1/repos/acme/r", + Some(json!({"default_branch": "dev"})), + ), + ] { + let r = s.send(method, path, Some(&adm), body.as_ref()); + assert_eq!( + r.status, + 503, + "{method} {path}: {}", + String::from_utf8_lossy(&r.raw) + ); + } + drop(slot); + assert_eq!( + s.get("/api/v1/repos/acme/r", &adm).status, + 200, + "and it recovers" + ); + // a failed create under load did not leave its name blocked + assert_eq!( + s.post("/api/v1/orgs/acme/repos", &adm, json!({"name": "other"})) + .status, + 201 + ); +} diff --git a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md index e266c800..16cb5e90 100644 --- a/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md +++ b/docs/superpowers/plans/2026-10-23-p38-f2a-repos-api.md @@ -124,8 +124,9 @@ fails if it cannot (a second `serve` or a concurrent reconcile cannot mutate the directory). Reconcile runs once, at startup, before the listener is bound; it is also callable from tests. **Per-repo mutex** (in-process map keyed by repo id): create-finalize, delete, PATCH default_branch and branch delete take it, re-check `state = 'live'` under it, and use -conditional SQL (`... WHERE id = ? AND state = 'live'`). The git runner refuses to spawn for a -repo whose row is not `live` (F2b also registers its children for the delete to terminate). +conditional SQL (`... WHERE id = ? AND state = 'live'`). *As built:* the lock is a read/write lock; reads of +repository data take it shared and re-check `live` (`Lifecycle::with_read`); the runner itself has no +knowledge of rows (F2b registers its children for the delete to terminate). **Create**: insert row `creating` (unique violation -> 409 `The repository with the same name already exists.`) -> `git init --bare` into `repos/.git` -> optional auto-init commit via diff --git a/docs/superpowers/specs/2026-10-22-p38-forge-design.md b/docs/superpowers/specs/2026-10-22-p38-forge-design.md index 1f9cb171..3b07e780 100644 --- a/docs/superpowers/specs/2026-10-22-p38-forge-design.md +++ b/docs/superpowers/specs/2026-10-22-p38-forge-design.md @@ -720,19 +720,39 @@ the smart-HTTP transport, hooks, ref policy, admission control and threat-model would (`refname::valid_branch`) and addressed only as `refs/heads/`; deleting the default branch is 403 `can not delete default branch`; an empty repository lists branches as `null`. - **Lifecycle.** `serve` holds an exclusive `flock` on `/.lock`, requires git 2.40 or - newer, then reconciles once before binding the listener. Per-repository mutexes serialize - create-finalize, delete, default-branch change and branch delete, each re-checking `live` - under the lock with conditional SQL. Creation fsyncs the whole new tree and `repos/`; any - failure before `live` rolls back in-process; **every `creating` row is rolled back at - startup** (the row does not record `auto_init`); a directory in `repos/` with no row at all is - **moved to `orphans/`, never deleted**, so a restored or recreated database cannot erase - repositories; a `live` row whose directory is missing is reported, never dropped. + newer, then reconciles once before binding the listener. The data directory is made absolute + once (the git children run with it as their working directory) and its layout directories are + fsynced. A per-repository read/write lock serializes the server's operations on one id: + delete, default-branch change and branch delete take it exclusively and re-check `live` under + it with conditional SQL; **reads of a repository's data take it shared and re-check `live`** + (`Lifecycle::with_read`), so a read can never straddle a delete's rename and see a + half-vanished repository as an empty one. Creation fsyncs the whole new tree, `repos/` and the + data directory; any failure before `live` rolls back in-process, and the `creating` row is + deleted only after the directory was removed (a failed cleanup keeps the row, so the name + stays blocked until the next startup finishes it). At startup **every `creating` row is rolled + back** (the row does not record `auto_init`) and its directory is **moved to `orphans/`, not + deleted**, as is a directory with no row at all: a restored or recreated database can + resurrect either, and nothing the server did not just create is ever deleted. A database error + during reconcile aborts the pass before any directory is touched on its account; a `live` row + whose directory is missing is reported, never dropped; one repository whose `HEAD` cannot be + repaired is reported and does not stop the server from starting. - **Git runner.** Every spawn is `env_clear` with an explicit `--git-dir` (never discovery), `GIT_CEILING_DIRECTORIES`, pinned per-spawn config, its own process group (signalled while the leader is still an unreaped zombie, so the group id cannot be reused), a deadline, capped output drained by dedicated threads, and an admission slot with a deadline (503 when busy, - `--git-concurrency`, default 8). Commit identity values containing control characters or - angle brackets are refused. + `--git-concurrency`, default 8); every handler answers a busy runner 503, never 500. A read + that fails because git failed (exit 128, a damaged repository) is an error, not an empty + repository. Commit identity values containing control characters or angle brackets are + refused, including in the email given to `admin/users` create and patch. +- **Search and paging.** `q` is percent-decoded (`+` is a space); every `Link` URL repeats the + query in the recorded order (`limit`, `page`, then `q`, `sort` alphabetically). A collaborator + `permission` of the wrong JSON type is 422, never a default grant. The default branch cannot + be deleted through a case alias (`MAIN`), whatever the filesystem. +- **Rollout.** Migration 2 has no down path and an older `serve` refuses a newer schema, so + rolling the binary back means restoring a pre-F2a `forge.db`; the next startup then moves + every repository directory to `orphans/` (safe, and recoverable by restoring the database). + Not verified against Gitea's source: `/user/repos` is scoped `write:user`/`read:user` as + recorded (B#26, B#34); whether Gitea also requires a repository scope there is open. - **Conformance gate.** `forge-check.sh` replays `A`, `B`, `C`, `L`, `N` and `R`; `B` stops at its first contents call (F3), `C` and `N` at their first `git clone` (F2b), and `X`, `D`, `K`, `M` stay out until their routes exist (they make recorded calls to unbuilt routes, which From 442028656e1a81b00a623db430ed5272be2a92a0 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 11:55:24 +0300 Subject: [PATCH 13/16] fix(forge): re-encode q strictly in search Link URLs --- crates/forge/src/api.rs | 16 +++++++++++++++- crates/forge/src/repo_api.rs | 8 +++++--- crates/forge/tests/e2e_repos.rs | 23 +++++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/forge/src/api.rs b/crates/forge/src/api.rs index 232455a9..a3bbdd34 100644 --- a/crates/forge/src/api.rs +++ b/crates/forge/src/api.rs @@ -122,7 +122,21 @@ pub(crate) fn paged( paged_with(ctx, path, total, page, limit, &[], items) } -/// Like [`paged`], carrying `extra` query parameters (already percent-encoded, in the order +/// Percent-encode everything except the RFC 3986 unreserved characters, so a value can sit inside +/// a `<...>` link target without being able to close it or start another entry. +pub(crate) fn pct_encode(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'.' | b'_' | b'~') { + out.push(b as char); + } else { + out.push_str(&format!("%{b:02X}")); + } + } + out +} + +/// Like [`paged`], carrying `extra` query parameters (already strictly percent-encoded with [`pct_encode`], in the order /// Gitea emits them: alphabetical after `limit` and `page`) into every `Link` URL, so following /// a page link repeats the same query. pub(crate) fn paged_with( diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs index 42fdd190..755d5452 100644 --- a/crates/forge/src/repo_api.rs +++ b/crates/forge/src/repo_api.rs @@ -1,7 +1,7 @@ //! F2a handlers: repositories, search and lists, collaborators, branches. Authentication, scope //! and the repository permission check already passed in `routes::dispatch` (`ctx.access`). -use crate::api::{body, page_params, paged, paged_with, str_field, user_json}; +use crate::api::{body, page_params, paged, paged_with, pct_encode, str_field, user_json}; use crate::gitproc::GitError; use crate::gitread; use crate::http::{error, not_found, Resp}; @@ -368,8 +368,10 @@ pub fn search(ctx: &mut Ctx) -> Resp { }; // Page links repeat the query, in the recorded order (alphabetical after limit and page). let mut extra: Vec<(&str, String)> = Vec::new(); - if let Some(raw) = ctx.req.param("q") { - extra.push(("q", raw.to_string())); + if let Some(decoded) = &q { + // Re-encoded strictly from the decoded value, never echoed from the raw query: a raw + // `>` or `"` must not be able to close the link target or start another entry. + extra.push(("q", pct_encode(decoded))); } if ctx.req.param("sort") == Some("alpha") { extra.push(("sort", "alpha".to_string())); diff --git a/crates/forge/tests/e2e_repos.rs b/crates/forge/tests/e2e_repos.rs index 35171e02..b061ca34 100644 --- a/crates/forge/tests/e2e_repos.rs +++ b/crates/forge/tests/e2e_repos.rs @@ -924,6 +924,29 @@ fn search_decodes_q_and_page_links_repeat_the_query() { link.contains("limit=2&page=2&q=hello%20world&sort=alpha>; rel=\"next\""), "{link}" ); + + // a hostile q cannot break out of the link target or forge another Link entry + let evil = "x%3E%3B%20rel%3D%22next%22%2C%20%3Chttp%3A%2F%2Fevil.invalid%2F"; + let r = + w.s.get(&format!("/api/v1/repos/search?q={evil}&limit=1"), &w.adm); + assert_eq!(r.status, 200); + let link = r.headers.get("link").cloned().unwrap_or_default(); + assert!( + !link.contains("evil.invalid/") && !link.contains(", ').count(), + "{link}" + ); + // the same text sent raw (unencoded) is never echoed + let r = w.s.get( + "/api/v1/repos/search?q=x>;rel=\"next\", Date: Fri, 23 Oct 2026 12:01:33 +0300 Subject: [PATCH 14/16] chore(deps): bump ctutils 0.4.2 -> 0.4.5 (RUSTSEC-2026-0385) Same lock-only change as the standalone PR; it clears the cargo audit job on this branch. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e68f98f5..f9660b53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1434,9 +1434,9 @@ checksum = "914a755b7c2d4af2bdcff7ce1739e2db9a1b81a9b07123d8015786ae03c0980d" [[package]] name = "ctutils" -version = "0.4.2" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +checksum = "38d28fb6aa7dd680b3ffd1a3e64e115c177bdba63c0ae7f9cd48129ff05c4fa0" dependencies = [ "cmov", ] From b433e5a0c8963dd0a1551756e01fd0061ea33248 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 12:07:49 +0300 Subject: [PATCH 15/16] fix(forge): bot review fixes on #301 Take the data-dir lock before migrations, one gone or damaged repository no longer fails a listing, a committed create or PATCH is reported from what it committed instead of a later git read, and only a unique violation means the name exists. --- crates/forge/src/cli.rs | 5 +- crates/forge/src/repo_api.rs | 103 +++++++++++++++----- crates/forge/src/repos.rs | 24 ++++- crates/forge/tests/e2e_repos.rs | 160 ++++++++++++++++++++++++++++++++ 4 files changed, 267 insertions(+), 25 deletions(-) diff --git a/crates/forge/src/cli.rs b/crates/forge/src/cli.rs index 93661273..d8372a92 100644 --- a/crates/forge/src/cli.rs +++ b/crates/forge/src/cli.rs @@ -228,10 +228,11 @@ fn run_serve(a: ServeArgs) -> Result<()> { header: a.trusted_proxy_header.map(|h| h.to_ascii_lowercase()), cidrs, }; - db::open_for_serve(&a.data_dir)?; - // One server per data directory: the lock is held until this function returns. + // One server per data directory, taken before anything touches it (migrations included): the + // lock is held until this function returns. let _data_lock = scl_forge::lifecycle::DirLock::acquire(&a.data_dir).map_err(anyhow::Error::msg)?; + db::open_for_serve(&a.data_dir)?; let git_bin = scl_forge::gitproc::find_git().context("git not found: install git or set SC_FORGE_GIT")?; let git = std::sync::Arc::new(scl_forge::gitproc::Git::new( diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs index 755d5452..99baba34 100644 --- a/crates/forge/src/repo_api.rs +++ b/crates/forge/src/repo_api.rs @@ -61,20 +61,39 @@ fn ssh_authority(base: &str) -> String { .to_string() } -/// The ~70-key repository object recorded from Gitea. `perm` is the *caller's* standing. -fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Result { - let owner = store::find_by_id(conn, r.owner_id) - .map_err(|e| app.internal(&e))? - .ok_or_else(|| app.internal(&"repo without owner"))?; +/// Is the repository empty (`HEAD` does not resolve)? A live read of the repository on disk, under +/// the shared lock; `Err` is the response to send (404 if it is gone, 503 if the runner is busy). +fn compute_empty(app: &App, conn: &rusqlite::Connection, r: &Repo) -> Result { // Under the shared repository lock, and only while the repository is still live: a read must // never straddle a delete's rename and see a half-vanished repository as an empty one. - let empty = match app.life.with_read(conn, r.id, || { + match app.life.with_read(conn, r.id, || { gitread::is_empty(&app.life.git, &app.life.repo_dir(r.id)) }) { - Err(e) => return Err(app.internal(&e)), - Ok(None) => return Err(not_found(&app.base, "The target couldn't be found.", None)), - Ok(Some(res)) => res.map_err(|e| git_read_err(app, conn, r.id, e))?, - }; + Err(e) => Err(app.internal(&e)), + Ok(None) => Err(not_found(&app.base, "The target couldn't be found.", None)), + Ok(Some(res)) => res.map_err(|e| git_read_err(app, conn, r.id, e)), + } +} + +/// The repository object with a live read of `empty`. +fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Result { + let empty = compute_empty(app, conn, r)?; + repo_json_known(app, conn, r, perm, empty) +} + +/// The ~70-key repository object recorded from Gitea, with `empty` already known (a create or +/// PATCH that has committed must not be reported as failed by a later git read). `perm` is the +/// *caller's* standing. +fn repo_json_known( + app: &App, + conn: &rusqlite::Connection, + r: &Repo, + perm: Perm, + empty: bool, +) -> Result { + let owner = store::find_by_id(conn, r.owner_id) + .map_err(|e| app.internal(&e))? + .ok_or_else(|| app.internal(&"repo without owner"))?; let mut owner_json = user_json(&app.base, &owner); owner_json["active"] = json!(false); // recorded: the owner object inside a repository reads inactive owner_json["login_name"] = json!(""); // recorded: the nested owner never carries the login_name @@ -142,16 +161,41 @@ fn repo_json(app: &App, conn: &rusqlite::Connection, r: &Repo, perm: Perm) -> Re })) } -/// A listed repository with the caller's standing computed per row. +/// The listed repositories with the caller's standing computed per row. One row never fails the +/// page: a repository deleted after the query ran is skipped, and one whose directory cannot be +/// read (a `live` row whose directory is missing, which the reconciler reports) is listed as +/// empty, since `HEAD` does not resolve there. A busy runner is still a 503 for the whole page. fn listed(ctx: &Ctx, rows: &[Repo]) -> Result, Resp> { - rows.iter() - .map(|r| { - let perm = repos::resolve(ctx.conn, &ctx.who.user, &r.owner, &r.name) - .map_err(|e| ctx.app.internal(&e))? - .map_or(Perm::Read, |a| a.perm); - repo_json(ctx.app, ctx.conn, r, perm) - }) - .collect() + let mut out = Vec::with_capacity(rows.len()); + for r in rows { + let perm = repos::resolve(ctx.conn, &ctx.who.user, &r.owner, &r.name) + .map_err(|e| ctx.app.internal(&e))? + .map_or(Perm::Read, |a| a.perm); + let read = ctx.app.life.with_read(ctx.conn, r.id, || { + gitread::is_empty(&ctx.app.life.git, &ctx.app.life.repo_dir(r.id)) + }); + let empty = match read { + Err(e) => return Err(ctx.app.internal(&e)), + Ok(None) => continue, // deleted since the query + Ok(Some(Ok(empty))) => empty, + Ok(Some(Err(GitError::Busy))) => { + return Err(error( + &ctx.app.base, + 503, + "too many git operations in flight", + )) + } + Ok(Some(Err(e))) => { + eprintln!( + "sc-forge: cannot read {}/{} (id {}): {e}; listed as empty", + r.owner, r.name, r.id + ); + true + } + }; + out.push(repo_json_known(ctx.app, ctx.conn, r, perm, empty)?); + } + Ok(out) } fn token_public_only(ctx: &Ctx) -> bool { @@ -205,7 +249,8 @@ fn create_for(ctx: &mut Ctx, owner: &User) -> Resp { now(), ); match created { - Ok(r) => match repo_json(ctx.app, ctx.conn, &r, Perm::Admin) { + // `empty` is known: only an auto_init create has a commit. No git read after committing. + Ok(r) => match repo_json_known(ctx.app, ctx.conn, &r, Perm::Admin, !auto_init) { Ok(v) => Resp::json(201, &v), Err(resp) => resp, }, @@ -256,6 +301,14 @@ pub fn patch_repo(ctx: &mut Ctx) -> Resp { if str_field(&b, "name").is_some_and(|n| n != a.repo.name) { return error(&base, 422, "repository rename is not supported"); } + // Read `empty` before anything is changed: once the patch has committed, a later failing git + // read must not report it as failed. Making an existing branch the default means HEAD + // resolves, so a changed default is never empty. + let empty_before = match compute_empty(ctx.app, ctx.conn, &a.repo) { + Ok(e) => e, + Err(r) => return r, + }; + let mut default_changed = false; // Always handed to the locked operation, which compares against the row under the lock (a // snapshot taken before the lock could be stale). if let Some(branch) = str_field(&b, "default_branch") { @@ -267,7 +320,7 @@ pub fn patch_repo(ctx: &mut Ctx) -> Resp { .life .set_default_branch(ctx.conn, &a.repo, &branch, now()) { - Ok(()) => {} + Ok(()) => default_changed = branch != a.repo.default_branch, Err(LifecycleError::NoSuchBranch) => { return error( &base, @@ -289,7 +342,13 @@ pub fn patch_repo(ctx: &mut Ctx) -> Resp { Err(e) => return ctx.app.internal(&e), } match repos::find_by_id(ctx.conn, a.repo.id) { - Ok(Some(r)) => match repo_json(ctx.app, ctx.conn, &r, a.perm) { + Ok(Some(r)) => match repo_json_known( + ctx.app, + ctx.conn, + &r, + a.perm, + !default_changed && empty_before, + ) { Ok(v) => Resp::json(200, &v), Err(resp) => resp, }, diff --git a/crates/forge/src/repos.rs b/crates/forge/src/repos.rs index 37c190e5..7de5d3ff 100644 --- a/crates/forge/src/repos.rs +++ b/crates/forge/src/repos.rs @@ -88,7 +88,7 @@ pub fn insert_creating( match r { Ok(_) => find_by_id(conn, conn.last_insert_rowid())?.ok_or(StoreError::NotFound), Err(rusqlite::Error::SqliteFailure(f, _)) - if f.code == rusqlite::ErrorCode::ConstraintViolation => + if f.extended_code == rusqlite::ffi::SQLITE_CONSTRAINT_UNIQUE => { Err(StoreError::Exists) } @@ -437,6 +437,28 @@ mod tests { ); } + #[test] + fn only_a_unique_violation_means_the_name_exists() { + let e = env("unique-only"); + let owner = user(&e.conn, "alice", false); + insert_creating(&e.conn, owner.id, "a", "main", "", "", 1).unwrap(); + assert!(matches!( + insert_creating(&e.conn, owner.id, "A", "main", "", "", 1), + Err(StoreError::Exists) + )); + // a foreign-key failure (no such owner) is a database error, never "already exists" + assert!(matches!( + insert_creating(&e.conn, 9_999, "b", "main", "", "", 1), + Err(StoreError::Db(_)) + )); + // so is a CHECK failure + let bad = e.conn.execute( + "INSERT INTO repos (owner_id, name, lower_name, default_branch, state, created_unix, updated_unix) VALUES (?1, 'c', 'c', 'main', 'bogus', 1, 1)", + [owner.id], + ); + assert!(bad.is_err()); + } + #[test] fn the_access_matrix() { let mut e = env("matrix"); diff --git a/crates/forge/tests/e2e_repos.rs b/crates/forge/tests/e2e_repos.rs index b061ca34..87a00e7e 100644 --- a/crates/forge/tests/e2e_repos.rs +++ b/crates/forge/tests/e2e_repos.rs @@ -1039,3 +1039,163 @@ fn a_saturated_runner_answers_503_on_reads_and_writes_not_500() { 201 ); } + +#[test] +fn one_repository_with_a_missing_directory_does_not_fail_the_listing() { + let (w, _) = world(); + let healthy = w.mk("healthy", true); + let lost = w.mk("lost", true); + std::fs::remove_dir_all(w.s.dir.join("repos").join(format!("{}.git", lost["id"]))).unwrap(); + for path in [ + "/api/v1/repos/search?sort=alpha", + "/api/v1/user/repos", + "/api/v1/orgs/acme/repos", + ] { + let r = w.s.get(path, &w.adm); + assert_eq!(r.status, 200, "{path}: {}", String::from_utf8_lossy(&r.raw)); + let v = r.json(); + let items = v.get("data").cloned().unwrap_or(v); + let by_name: std::collections::HashMap = items + .as_array() + .unwrap() + .iter() + .map(|x| { + ( + x["name"].as_str().unwrap().to_string(), + x["empty"].as_bool().unwrap(), + ) + }) + .collect(); + assert_eq!( + by_name.get("healthy"), + Some(&false), + "{path}: the healthy repo is listed normally" + ); + assert_eq!( + by_name.get("lost"), + Some(&true), + "{path}: the damaged one is listed as empty, not a 500 for the page" + ); + } + assert_eq!(w.s.get("/api/v1/repos/acme/healthy", &w.adm).status, 200); + let _ = healthy; +} + +#[test] +fn a_create_and_a_patch_report_success_from_what_they_committed() { + let (w, _) = world(); + let r = w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": "fresh", "auto_init": false}), + ); + assert_eq!((r.status, r.json()["empty"].as_bool()), (201, Some(true))); + let r = w.s.post( + "/api/v1/orgs/acme/repos", + &w.adm, + json!({"name": "seeded", "auto_init": true}), + ); + assert_eq!((r.status, r.json()["empty"].as_bool()), (201, Some(false))); + // PATCH keeps `empty` and, when the default becomes an existing branch, reports not empty + let id = r.json()["id"].as_i64().unwrap(); + let main = w.s.git(id, &["rev-parse", "main"], None); + w.s.git(id, &["update-ref", "refs/heads/dev", &main], None); + let p = w.s.send( + "PATCH", + "/api/v1/repos/acme/seeded", + Some(&w.adm), + Some(&json!({"default_branch": "dev", "description": "d"})), + ); + assert_eq!( + ( + p.status, + p.json()["empty"].as_bool(), + p.json()["default_branch"].as_str() + ), + (200, Some(false), Some("dev")) + ); + let id2 = w.s.get("/api/v1/repos/acme/fresh", &w.adm).json()["id"] + .as_i64() + .unwrap(); + w.s.commit_on(id2, "other", None); // branches exist but HEAD (main) is still unborn + let p = w.s.send( + "PATCH", + "/api/v1/repos/acme/fresh", + Some(&w.adm), + Some(&json!({"description": "x"})), + ); + assert_eq!( + p.json()["empty"].as_bool(), + Some(true), + "HEAD still does not resolve" + ); + let p = w.s.send( + "PATCH", + "/api/v1/repos/acme/fresh", + Some(&w.adm), + Some(&json!({"default_branch": "other"})), + ); + assert_eq!( + p.json()["empty"].as_bool(), + Some(false), + "an existing branch as default resolves" + ); +} + +#[test] +fn a_second_server_on_the_same_data_directory_exits_on_the_lock_before_touching_the_schema() { + use std::io::{BufRead, BufReader}; + use std::process::{Command, Stdio}; + let dir = std::env::temp_dir().join(format!("scl-forge-two-servers-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let bin = env!("CARGO_BIN_EXE_sc-forge"); + let serve = || { + let mut c = Command::new(bin); + c.args([ + "serve", + "--listen", + "127.0.0.1:0", + "--public-url", + "http://127.0.0.1:1", + "--data-dir", + ]) + .arg(&dir) + .stderr(Stdio::piped()); + c + }; + let mut first = serve().spawn().unwrap(); + let mut lines = BufReader::new(first.stderr.take().unwrap()).lines(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + let mut listening = false; + while std::time::Instant::now() < deadline { + match lines.next() { + Some(Ok(l)) if l.contains("listening on") => { + listening = true; + break; + } + Some(Ok(_)) => {} + _ => break, + } + } + assert!(listening, "the first server must start"); + // Make the schema newer than the second binary could migrate: if it migrated before the lock, + // it would fail on the schema; failing on the lock proves the lock comes first. + let c = scl_forge::db::connect(&dir).unwrap(); + c.pragma_update(None, "user_version", scl_forge::db::SCHEMA_VERSION + 1) + .unwrap(); + drop(c); + let out = serve().output().unwrap(); + let err = String::from_utf8_lossy(&out.stderr); + let _ = first.kill(); + let _ = first.wait(); + assert!(!out.status.success()); + assert!( + err.contains("another sc-forge process"), + "the lock is refused first: {err}" + ); + assert!( + !err.contains("newer than"), + "the second server never reached the schema check: {err}" + ); + let _ = std::fs::remove_dir_all(&dir); +} From 862408bf846fac2e6035102ab814ce8ec9ca75ad Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Fri, 23 Oct 2026 12:19:11 +0300 Subject: [PATCH 16/16] fix(forge): Codex review fixes on #301 A row whose access vanished mid-listing is skipped, show-ref failures on a damaged repository are git failures not missing branches, the git path is always absolute, /healthz probes every table of the schema, and searches fold Unicode case in descriptions. --- crates/forge/Cargo.toml | 2 +- crates/forge/src/db.rs | 12 +++++++ crates/forge/src/gitproc.rs | 55 +++++++++++++++++++++++++++++---- crates/forge/src/lifecycle.rs | 43 +++++++++++++++++++------- crates/forge/src/repo_api.rs | 9 ++++-- crates/forge/src/repos.rs | 43 +++++++++++++++++++++++++- crates/forge/src/routes.rs | 2 +- crates/forge/tests/e2e_repos.rs | 18 +++++++++++ 8 files changed, 161 insertions(+), 23 deletions(-) diff --git a/crates/forge/Cargo.toml b/crates/forge/Cargo.toml index 6864ef57..10b00ce8 100644 --- a/crates/forge/Cargo.toml +++ b/crates/forge/Cargo.toml @@ -27,7 +27,7 @@ http-body-util = "0.1.5" hyper = { version = "1", features = ["http1", "server"] } hyper-util = { version = "0.1.21", features = ["tokio", "server"] } libc = "0.2.191" -rusqlite = { version = "0.40.2", features = ["bundled"] } +rusqlite = { version = "0.40.2", features = ["bundled", "functions"] } serde_json = "1.0.154" thiserror = "2.0.18" tokio = { version = "1.54.1", features = ["rt-multi-thread", "net", "io-util", "signal", "time", "macros", "sync"] } diff --git a/crates/forge/src/db.rs b/crates/forge/src/db.rs index 9a8e13ec..4fe97336 100644 --- a/crates/forge/src/db.rs +++ b/crates/forge/src/db.rs @@ -72,6 +72,10 @@ const MIGRATIONS: &[&str] = &[ );", ]; +/// Every table the current schema creates; `/healthz` probes each, so a missing or unreadable one +/// (whatever the recorded version says) reads as not ready. +pub const TABLES: &[&str] = &["users", "org_members", "tokens", "repos", "collaborators"]; + /// Database failures a caller reports to the operator. #[derive(Debug, thiserror::Error)] pub enum DbError { @@ -102,6 +106,14 @@ pub fn connect(data_dir: &Path) -> Result { conn.pragma_update(None, "journal_mode", "WAL")?; conn.pragma_update(None, "synchronous", "FULL")?; conn.pragma_update(None, "foreign_keys", "ON")?; + // SQLite's built-in lower() folds ASCII only; searches need Unicode-aware case folding. + conn.create_scalar_function( + "ulower", + 1, + rusqlite::functions::FunctionFlags::SQLITE_UTF8 + | rusqlite::functions::FunctionFlags::SQLITE_DETERMINISTIC, + |ctx| Ok(ctx.get::(0)?.to_lowercase()), + )?; Ok(conn) } diff --git a/crates/forge/src/gitproc.rs b/crates/forge/src/gitproc.rs index 10014233..4cc8cc61 100644 --- a/crates/forge/src/gitproc.rs +++ b/crates/forge/src/gitproc.rs @@ -314,16 +314,28 @@ fn exited_unreaped(pid: i32) -> std::io::Result { Ok(pid_set) } -/// Find `git`: `SC_FORGE_GIT` if set, else the first `git` on the server's own `PATH`. -pub fn find_git() -> Option { - if let Some(p) = std::env::var_os("SC_FORGE_GIT") { - return Some(PathBuf::from(p)); +/// Resolve where `git` is: `configured` (the `SC_FORGE_GIT` value) if given, else `git` on `path`. +/// The result is always absolute: every spawn runs with the data directory as its working +/// directory, so a relative path (or a relative `PATH` entry) would be looked up there instead of +/// where the operator started the server. +pub fn resolve_git(configured: Option<&OsStr>, path: Option<&OsStr>) -> Option { + let name: &OsStr = configured.unwrap_or_else(|| OsStr::new("git")); + if Path::new(name).components().count() > 1 { + return std::path::absolute(name).ok().filter(|p| p.is_file()); } - std::env::split_paths(&std::env::var_os("PATH")?) - .map(|d| d.join("git")) + std::env::split_paths(path?) + .filter_map(|d| std::path::absolute(d.join(name)).ok()) .find(|p| p.is_file()) } +/// Find `git` from the server's own environment (`SC_FORGE_GIT`, then `PATH`), as an absolute path. +pub fn find_git() -> Option { + resolve_git( + std::env::var_os("SC_FORGE_GIT").as_deref(), + std::env::var_os("PATH").as_deref(), + ) +} + /// `(major, minor)` from `git version 2.54.0 (Apple Git-157)`. pub fn parse_version(text: &str) -> Option<(u32, u32)> { let v = text.split_whitespace().nth(2)?; @@ -485,6 +497,37 @@ mod tests { let _ = std::fs::remove_dir_all(d); } + #[test] + fn the_git_path_is_always_absolute() { + let d = tmp("resolve-git"); + std::fs::create_dir_all(d.join("bin")).unwrap(); + std::fs::write(d.join("bin").join("mygit"), b"#!/bin/sh\n").unwrap(); + let cwd = std::env::current_dir().unwrap(); + // a slash-containing relative value is made absolute against the current directory + let rel = d + .strip_prefix(&cwd) + .map(|p| p.join("bin/mygit")) + .unwrap_or_else(|_| d.join("bin/mygit")); + let got = resolve_git(Some(rel.as_os_str()), None).expect("resolved"); + assert!(got.is_absolute() && got.ends_with("bin/mygit"), "{got:?}"); + // a bare name is searched on PATH, and a relative PATH entry is made absolute too + let path = std::env::join_paths([d.join("bin")]).unwrap(); + assert!(resolve_git(Some(OsStr::new("mygit")), Some(&path)) + .unwrap() + .is_absolute()); + let rel_dir = d + .join("bin") + .strip_prefix(&cwd) + .map(|p| p.to_path_buf()) + .unwrap_or_else(|_| d.join("bin")); + let rel_path = std::env::join_paths([rel_dir]).unwrap(); + assert!(resolve_git(Some(OsStr::new("mygit")), Some(&rel_path)) + .unwrap() + .is_absolute()); + assert!(resolve_git(Some(OsStr::new("nope-not-here")), Some(&path)).is_none()); + let _ = std::fs::remove_dir_all(d); + } + #[test] fn identity_refuses_forged_header_characters() { assert!(Identity::new("alice", "a@example.invalid").is_some()); diff --git a/crates/forge/src/lifecycle.rs b/crates/forge/src/lifecycle.rs index a43c0d5c..55cff1e8 100644 --- a/crates/forge/src/lifecycle.rs +++ b/crates/forge/src/lifecycle.rs @@ -350,6 +350,24 @@ impl Lifecycle { Ok(()) } + /// Does branch ref `full` exist? `show-ref --verify` exits 1 for "no such ref"; any other + /// failure (128: the directory is missing or corrupt) is the repository's damage and must + /// not read as bad user input. + fn branch_exists(&self, dir: &Path, full: &str) -> Result { + let out = self.git.run(&Spec { + repo: Some(dir), + args: &["show-ref", "--verify", "--quiet", full], + ..Default::default() + })?; + match out.status.code() { + Some(0) => Ok(true), + Some(1) => Ok(false), + _ => Err(LifecycleError::GitFailed( + String::from_utf8_lossy(&out.stderr).trim().to_string(), + )), + } + } + /// Make `branch` the default: it must exist; bare `HEAD` is written first, then the row. pub fn set_default_branch( &self, @@ -368,12 +386,7 @@ impl Lifecycle { } let dir = self.repo_dir(current.id); let full = refname::branch_ref(branch); - let exists = self.git.run(&Spec { - repo: Some(&dir), - args: &["show-ref", "--verify", "--quiet", &full], - ..Default::default() - })?; - if !exists.ok() { + if !self.branch_exists(&dir, &full)? { return Err(LifecycleError::NoSuchBranch); } self.run(&dir, &["symbolic-ref", "HEAD", &full], None, None)?; @@ -408,12 +421,7 @@ impl Lifecycle { } let dir = self.repo_dir(current.id); let full = refname::branch_ref(branch); - let exists = self.git.run(&Spec { - repo: Some(&dir), - args: &["show-ref", "--verify", "--quiet", &full], - ..Default::default() - })?; - if !exists.ok() { + if !self.branch_exists(&dir, &full)? { return Err(LifecycleError::NoSuchBranch); } self.run(&dir, &["update-ref", "-d", &full], None, None)?; @@ -1103,6 +1111,17 @@ mod tests { ); } + #[test] + fn a_damaged_repository_is_a_failure_not_a_missing_branch() { + let e = env("damaged-branch"); + let r = make(&e, "alpha", true).unwrap(); + std::fs::remove_file(e.lc.repo_dir(r.id).join("HEAD")).unwrap(); // no longer a repository + let a = e.lc.set_default_branch(&e.conn, &r, "dev", 20); + assert!(matches!(a, Err(LifecycleError::GitFailed(_))), "{a:?}"); + let b = e.lc.delete_branch(&e.conn, &r, "dev"); + assert!(matches!(b, Err(LifecycleError::GitFailed(_))), "{b:?}"); + } + #[test] fn the_default_branch_cannot_be_deleted_by_a_case_alias() { let e = env("case-alias"); diff --git a/crates/forge/src/repo_api.rs b/crates/forge/src/repo_api.rs index 99baba34..d1d67fb3 100644 --- a/crates/forge/src/repo_api.rs +++ b/crates/forge/src/repo_api.rs @@ -168,9 +168,14 @@ fn repo_json_known( fn listed(ctx: &Ctx, rows: &[Repo]) -> Result, Resp> { let mut out = Vec::with_capacity(rows.len()); for r in rows { - let perm = repos::resolve(ctx.conn, &ctx.who.user, &r.owner, &r.name) + // Access is re-resolved per row; a row the caller can no longer read (revoked, deleted or + // being deleted since the query ran) is skipped, never shown with a default permission. + let Some(perm) = repos::resolve(ctx.conn, &ctx.who.user, &r.owner, &r.name) .map_err(|e| ctx.app.internal(&e))? - .map_or(Perm::Read, |a| a.perm); + .map(|a| a.perm) + else { + continue; + }; let read = ctx.app.life.with_read(ctx.conn, r.id, || { gitread::is_empty(&ctx.app.life.git, &ctx.app.life.repo_dir(r.id)) }); diff --git a/crates/forge/src/repos.rs b/crates/forge/src/repos.rs index 7de5d3ff..be5dcd50 100644 --- a/crates/forge/src/repos.rs +++ b/crates/forge/src/repos.rs @@ -273,7 +273,7 @@ pub fn list_visible( } if let Some(q) = filter.q.as_deref().filter(|q| !q.is_empty()) { wheres.push( - "(r.lower_name LIKE ? ESCAPE '\\' OR lower(r.description) LIKE ? ESCAPE '\\')".into(), + "(r.lower_name LIKE ? ESCAPE '\\' OR ulower(r.description) LIKE ? ESCAPE '\\')".into(), ); let pat = format!( "%{}%", @@ -617,6 +617,47 @@ mod tests { assert_eq!((rs.len(), total), (1, 3)); } + #[test] + fn search_folds_case_for_non_ascii_descriptions_too() { + let e = env("unicode-q"); + let owner = user(&e.conn, "owner", false); + let r = live(&e.conn, &owner, "pastry"); + update_live( + &e.conn, + r.id, + &RepoPatch { + description: Some("Éclair and STRÜDEL".into()), + ..Default::default() + }, + 3, + ) + .unwrap(); + for q in [ + "éclair", + "ÉCLAIR", + "strüdel", + "Strüdel", + "ECLAIR".to_lowercase().as_str(), + ] { + let hit = list_visible( + &e.conn, + &owner, + false, + &Filter { + q: Some(q.into()), + ..Default::default() + }, + Sort::Id, + 1, + 50, + ) + .unwrap() + .1; + // "eclair" (no accent) is a different word, so it correctly finds nothing + assert_eq!(hit, i64::from(q != "eclair"), "{q}"); + } + } + #[test] fn collaborators_keep_insertion_order_and_cascade_on_delete() { let e = env("collab"); diff --git a/crates/forge/src/routes.rs b/crates/forge/src/routes.rs index 34d32776..75a4eba2 100644 --- a/crates/forge/src/routes.rs +++ b/crates/forge/src/routes.rs @@ -437,7 +437,7 @@ fn database_ready(app: &App) -> bool { let version = conn.query_row("PRAGMA user_version", [], |r| r.get::<_, i64>(0)); // The version alone can be forged by a replacement file; the tables must answer too. version.is_ok_and(|v| v == db::SCHEMA_VERSION) - && ["users", "org_members", "tokens"].iter().all(|t| { + && db::TABLES.iter().all(|t| { conn.query_row(&format!("SELECT 1 FROM {t} LIMIT 1"), [], |_| Ok(())) .map_or_else( |e| matches!(e, rusqlite::Error::QueryReturnedNoRows), diff --git a/crates/forge/tests/e2e_repos.rs b/crates/forge/tests/e2e_repos.rs index 87a00e7e..6379f725 100644 --- a/crates/forge/tests/e2e_repos.rs +++ b/crates/forge/tests/e2e_repos.rs @@ -1199,3 +1199,21 @@ fn a_second_server_on_the_same_data_directory_exits_on_the_lock_before_touching_ ); let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn readiness_covers_the_repository_tables_too() { + for table in ["repos", "collaborators"] { + let (w, _) = world(); + assert_eq!(w.s.send("GET", "/api/healthz", None, None).status, 200); + let conn = scl_forge::db::connect(&w.s.dir).unwrap(); + conn.execute_batch(&format!( + "PRAGMA foreign_keys = OFF; ALTER TABLE {table} RENAME TO {table}_gone;" + )) + .unwrap(); + assert_eq!( + w.s.send("GET", "/api/healthz", None, None).status, + 503, + "without {table} the server is not ready" + ); + } +}