Commit b711b70da5
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
docs/plans/2026-09-27-ci-trust-and-build-reporting.md +820 −200
| @@ -5,22 +5,27 @@ | ||
| 5 | 5 | **Goal:** Untrusted builds get a disposable home and never feed a |
| 6 | 6 | trusted build (#255, #258); `ci/*` statuses belong to the build |
| 7 | 7 | subsystem and merges can wait on named contexts (#258); a build can no |
| 8 | longer share the runner's source address (#260); a failed build names | |
| 9 | its step, exit and duration on the CLI and the web (#266). | |
| 8 | longer share the runner's source address, and reaches no port on the | |
| 9 | runner's host but the forge's public 22, 80 and 443 (#260); a failed | |
| 10 | build names its step, exit and duration on the CLI and the web (#266). | |
| 10 | 11 | |
| 11 | 12 | **Architecture:** The claim payload gains an explicit `trusted` flag and |
| 12 | the instance's public ssh destination. The runner picks the build home | |
| 13 | by trust (persistent per repository for trusted builds, fresh and | |
| 14 | removed for untrusted ones), keeps a loopback runner's builds off the | |
| 15 | host's loopback, and reports the failed step on `runner done`. The | |
| 16 | server refuses `ci/` contexts in `status set`, restricts tree and commit | |
| 17 | reuse to trusted builds on the same image, adds a `required_contexts` | |
| 18 | repository setting that `MergeGates` treats as pending until reported, | |
| 19 | stores the failed step and reason on the build, and cuts the log at its | |
| 20 | `$ <step>` lines for `build log --step` and the build page. | |
| 13 | the instance's public ssh destination (with the port when it is not | |
| 14 | 22). The runner picks the build home by trust (persistent per | |
| 15 | repository for trusted builds, fresh and removed for untrusted ones), | |
| 16 | keeps a loopback runner's builds off the host's loopback, and reports | |
| 17 | the failed step on `runner done`. An nftables table on the runner host, | |
| 18 | loaded by a oneshot unit the runner service requires, limits what the | |
| 19 | runner's uid may reach on the host itself. The server refuses `ci/` | |
| 20 | contexts in `status set`, restricts tree and commit reuse to trusted | |
| 21 | builds on the same declared image, adds a `required_contexts` | |
| 22 | repository setting that turns `require_checks` on and that `MergeGates` | |
| 23 | treats as pending until reported, stores the failed step and reason on | |
| 24 | the build, and cuts the log at its `$ <step>` lines for `build log | |
| 25 | --step` and the build page. | |
| 21 | 26 | |
| 22 | 27 | **Tech Stack:** Go, SQLite (hand-written SQL), `html/template`, rootless |
| 23 | podman with pasta, systemd. | |
| 28 | podman with pasta, nftables, systemd. | |
| 24 | 29 | |
| 25 | 30 | **Spec:** the issues themselves: krz/gitbay#255, #258, #260, #266 (texts |
| 26 | 31 | in the session's `issues.txt`), plus the decisions recorded under |
| @@ -102,11 +107,30 @@ in the session's `issues.txt`), plus the decisions recorded under | ||
| 102 | 107 | untrusted queue: a fork head that lands on a branch by fast-forward is |
| 103 | 108 | built again as trusted. `required_contexts` is a list in the |
| 104 | 109 | repository's settings JSON (no migration), set by |
| 105 | `repo settings require-contexts <owner/name> [<context>...]` (no | |
| 106 | contexts clears it); it applies only while `require_checks` is on, and | |
| 107 | the command says so on stderr when it is off. A missing required | |
| 108 | context makes the combined check `pending` and appears as | |
| 109 | `<context>=missing` in the unmet sentence and in `checks_missing`. | |
| 110 | `repo settings require-contexts <owner/name> [<context>...]`. Setting | |
| 111 | a non-empty list also turns `require_checks` on, in the same settings | |
| 112 | update; setting it empty clears the list and leaves `require_checks` | |
| 113 | as it was. `require-checks off` keeps the list, which then does | |
| 114 | nothing until the gate is on again. `repo settings show` prints | |
| 115 | `require checks` and `required contexts` side by side (today it prints | |
| 116 | neither), and on the web settings page the required-checks box is | |
| 117 | ticked after contexts are saved and its hint lists them. The gate | |
| 118 | itself only ever reads `require_checks`. A missing required context | |
| 119 | makes the combined check `pending` and appears as `<context>=missing` | |
| 120 | in the unmet sentence and in `checks_missing`. | |
| 121 | - **#258, default image.** Tree reuse keys on the job's declared | |
| 122 | `image:`. A job naming none is stored with `image = ''` and matches | |
| 123 | other such builds whatever the runner defaulted to; bumping a runner's | |
| 124 | `-image` does not invalidate them. This is documented on the CI page | |
| 125 | rather than fixed. Reuse is decided at queue time in `queueJobs`, | |
| 126 | before any runner is chosen, and the default image belongs to | |
| 127 | whichever runner claims the build: bay1 and the laptop runner already | |
| 128 | have different defaults. Reporting the resolved image on claim or | |
| 129 | `runner done` would record it after the fact, but a queue-time | |
| 130 | comparison would still have nothing to compare against, so jobs | |
| 131 | without an image would never be reused at all. The remedy is on the | |
| 132 | repository's side (name the image in `ci.yml`) or the operator's | |
| 133 | (`build trigger`, which never reuses, after bumping `-image`). | |
| 110 | 134 | - **#260.** Reading the code: the bay1 runner polls `git@127.0.0.1` |
| 111 | 135 | (`deploy/gitbay-runner.override.conf:78`); `buildSSH` |
| 112 | 136 | (`cmd/gitbay-runner/main.go:471-486`) sends podman builds to |
| @@ -119,25 +143,57 @@ in the session's `issues.txt`), plus the decisions recorded under | ||
| 119 | 143 | runner stays on loopback and its builds lose loopback: under podman, |
| 120 | 144 | when the runner's remote is loopback, containers run with |
| 121 | 145 | `--network pasta:--no-map-gw`, and `GITBAY_SSH` is the instance's |
| 122 | public destination (`git@<site host>`), which the server sends in the | |
| 123 | claim as `ssh`. A build then reaches the host only as an internet | |
| 124 | client does. **Egress policy:** builds, trusted or not, keep outbound | |
| 125 | internet access (a fork's merge request to a Go repository must fetch | |
| 126 | its modules); they get no host loopback; they reach the forge's | |
| 127 | public ports (22, 80, 443, and the admin sshd on 2222) exactly as | |
| 128 | anyone on the internet can. No nftables rules. `-isolation none` | |
| 129 | builds run on the host and share its loopback; that mode is for | |
| 130 | instances where every repository is trusted and says so already. | |
| 131 | - **Finding for #260, recorded for the runbook.** On gitbay.org the | |
| 132 | limiter's failure count is unreachable by an unknown key: | |
| 133 | `authenticate` admits an unknown key as an anonymous `register` | |
| 134 | session whenever `registration.mode` is not `closed` | |
| 135 | (`internal/sshd/sshd.go:136-143`), and `fail` is only called on the | |
| 136 | closed path (`sshd.go:145`). gitbay.org runs `registration = "open"`, | |
| 137 | so the throttling test on bay1 is expected to show no throttling at | |
| 138 | all; the separation matters for closed-registration instances. The | |
| 139 | runbook still measures source addresses on bay1, which is the part of | |
| 140 | #260 that holds on every instance. | |
| 146 | public destination, which the server sends in the claim as `ssh`. A | |
| 147 | build then reaches the host only as an internet client does. | |
| 148 | - **#260, `GITBAY_SSH` form.** `git@<site host>` when `[ssh] port` is 22 | |
| 149 | (or unset, which config validation treats as 22), and | |
| 150 | `git@<site host>:<port>` otherwise, built with `net.JoinHostPort` so | |
| 151 | an IPv6 literal is bracketed. hutch and orgo build | |
| 152 | `ssh://$GITBAY_SSH/<owner>/<name>.git`, which is a valid URL in both | |
| 153 | forms, so nothing changes for them on 22 and they work unchanged on | |
| 154 | another port. A script that runs a command uses `ssh | |
| 155 | ssh://$GITBAY_SSH …`, which OpenSSH accepts with or without the port; | |
| 156 | the Users page says so. The port is the daemon's `[ssh] port`, the | |
| 157 | one it listens on; an instance behind a port-mapping NAT is not | |
| 158 | modelled. | |
| 159 | - **#260, host egress.** Under rootless podman with pasta, a build's | |
| 160 | connections are made by pasta on the host from sockets owned by the | |
| 161 | runner's uid (`ci-runner`), the same uid the runner's own ssh runs as. | |
| 162 | An nftables table (`deploy/gitbay-runner-egress.nft`, loaded by | |
| 163 | `gitbay-runner-egress.service`, which `gitbay-runner.service` | |
| 164 | requires) matches output packets with `meta skuid "ci-runner"` that | |
| 165 | leave through `lo` — every packet to one of the host's own addresses, | |
| 166 | loopback or public, does — and allows only 127.0.0.1:22 (the runner's | |
| 167 | poll, clone and log stream), port 53 on loopback (the host resolver | |
| 168 | pasta forwards a build's DNS to), and 22, 80 and 443 on the public | |
| 169 | addresses. Everything else on the host is rejected: the admin sshd on | |
| 170 | 2222 on every address, and every service bound to loopback. Traffic | |
| 171 | to other hosts is not matched, so outbound internet stays open (a | |
| 172 | fork's merge request to a Go repository must fetch its modules). The | |
| 173 | rule applies to all builds, trusted and untrusted, and to | |
| 174 | `-isolation none` builds too, since they run as the same uid. uid | |
| 175 | alone cannot tell a build from its runner, so 127.0.0.1:22 stays open | |
| 176 | to the uid; `--no-map-gw` is what keeps builds off loopback. The two | |
| 177 | are separate layers and both ship. The runner does not start without | |
| 178 | the rule (`Requires=`), following `runner-podman-setup.sh`'s rule that | |
| 179 | a host that is not ready fails rather than runs builds unconfined; | |
| 180 | `make deploy-runner` loads the rule and checks, as `ci-runner`, that | |
| 181 | 127.0.0.1:22 answers and 2222 does not, before restarting the runner. | |
| 182 | The laptop runner (macOS, brew) is not covered. | |
| 183 | - **Finding for #260.** On gitbay.org the limiter's failure count is | |
| 184 | unreachable by an unknown key: `authenticate` admits an unknown key as | |
| 185 | an anonymous `register` session whenever `registration.mode` is not | |
| 186 | `closed` (`internal/sshd/sshd.go:139-144`), and `fail` is only called | |
| 187 | on the closed path (`sshd.go:145`). With registration closed, | |
| 188 | `authenticate` checks `allow` before it looks at the key | |
| 189 | (`sshd.go:123-129`), so once an address has `ssh_auth_rate` failures | |
| 190 | in the window every key from it is refused, the runner's included, and | |
| 191 | `success` (`sshd.go:149`) is never reached to clear the count; below | |
| 192 | the limit a success clears it. A unit test in `internal/sshd` records | |
| 193 | both modes (Task 3.3). No throttling test runs on production; the | |
| 194 | runbook measures, from inside a scratch build, the source address the | |
| 195 | forge sees and that 2222 and 127.0.0.1 are unreachable, and #260 | |
| 196 | closes when that is recorded on the CI wiki page. | |
| 141 | 197 | - **#266.** Duration is not stored: `Build.Elapsed()` |
| 142 | 198 | (`internal/store/builds.go:420`) already derives it from `started_at` |
| 143 | 199 | and `finished_at`. Migration 0065 adds `failed_step` (1-based, 0 for |
| @@ -161,7 +217,7 @@ in the session's `issues.txt`), plus the decisions recorded under | ||
| 161 | 217 | |---|---|---|---|---| |
| 162 | 218 | | 1 | `ci-untrusted-home` | #255 | — | — | |
| 163 | 219 | | 2 | `ci-status-trust` | #258 | — | — | |
| 164 | | 3 | `runner-source-address` | Ref #260 (closed by the runbook result commit) | — | Part 1 merged (both edit `runRunnerNext`'s payload and `stepEnv`) | | |
| 220 | | 3 | `runner-source-address` | Ref #260 (closed by the runbook result commit on the CI page) | — | Part 1 merged (both edit `runRunnerNext`'s payload and `stepEnv`) | | |
| 165 | 221 | | 4 | `build-failure-report` | #266 | 0065 | Part 1 merged (both change `run()`); Part 3 merged (both change `runStepsPodman`) | |
| 166 | 222 | |
| 167 | 223 | #255 goes first. Parts 1–4 land and deploy in order; each runner deploy |
| @@ -179,10 +235,15 @@ Other plans (all `docs/plans/2026-09-27-*.md`): | ||
| 179 | 235 | - Plan 5 (web-ux, #261) covers documentation drift. Two items seen here |
| 180 | 236 | and left alone: `deploy/gitbay-runner.override.conf:27-30` names |
| 181 | 237 | `cmc/ci-smoke`, which no longer exists; `cmd/gitbay-runner/main.go:513` |
| 182 | repeats its comment line. | |
| 238 | repeats its comment line. Part 3 adds a `[Unit]` section at the top of | |
| 239 | the same drop-in; if plan 5 edits its comment, whoever lands second | |
| 240 | rebases. | |
| 183 | 241 | - Plan 1 (credentials-and-sessions, #256) closes a removed key's |
| 184 | 242 | connections; the runner's `runner log` session is one such |
| 185 | connection, and no code here depends on it. | |
| 243 | connection, and no code here depends on it. Plan 1's key expiry | |
| 244 | (#277) may add a check to `authenticate`; Part 3's sshd test uses an | |
| 245 | unexpiring key and asserts only the limiter's behaviour, so it holds | |
| 246 | either way. | |
| 186 | 247 | |
| 187 | 248 | ## File map |
| 188 | 249 | |
| @@ -192,7 +253,7 @@ Other plans (all `docs/plans/2026-09-27-*.md`): | ||
| 192 | 253 | | `internal/control/buildlog.go` (create) | 4 | `LogSection`, `SplitBuildLog`, `FailedSection`, `tailLines` | |
| 193 | 254 | | `internal/control/status.go` | 2 | `ci/` refusal | |
| 194 | 255 | | `internal/control/mr.go`, `output.go` | 2 | `require-contexts`, `MergeGates`, `GatesOut.ChecksMissing` | |
| 195 | | `internal/control/repo.go` | 2 | `repo settings show` prints required contexts | | |
| 256 | | `internal/control/repo.go` | 2 | `repo settings show` prints require checks and required contexts | | |
| 196 | 257 | | `internal/store/builds.go` | 2, 4 | trust and image on reuse; failed step columns | |
| 197 | 258 | | `internal/store/repos.go` | 2 | `RepoSettings.RequiredContexts` | |
| 198 | 259 | | `internal/store/migrations/0065_build_failure.{up,down}.sql` | 4 | columns | |
| @@ -204,6 +265,9 @@ Other plans (all `docs/plans/2026-09-27-*.md`): | ||
| 204 | 265 | | `internal/web/templates/build.html`, `mr.html`, `settings.html` | 2, 4 | steps, gates row, form | |
| 205 | 266 | | `internal/web/static/style.css` | 4 | `pre.buildlog` wraps; step folds | |
| 206 | 267 | | `e2e/readonly_test.go`, `e2e/mrweb_test.go`, `e2e/status_test.go`, `e2e/settingsweb_test.go`, `e2e/ci_test.go` | 2, 4 | contexts off `ci/`; refusal; settings; failed step | |
| 268 | | `internal/sshd/sshd_test.go` | 3 | limiter behaviour by registration mode | | |
| 269 | | `deploy/gitbay-runner-egress.nft` (create), `deploy/gitbay-runner-egress.service` (create), `deploy/runner-egress-check.sh` (create) | 3 | host egress rule, its unit, the post-load check | | |
| 270 | | `deploy/gitbay-runner.override.conf`, `deploy/runner-podman-setup.sh`, `Makefile` | 3 | runner requires the rule; nftables installed; `deploy-runner` ships, loads and checks it | | |
| 207 | 271 | | `.gitbay/wiki/…` | all | as listed per task | |
| 208 | 272 | |
| 209 | 273 | --- |
| @@ -930,8 +994,9 @@ Replace `internal/store/builds.go:453-477` with: | ||
| 930 | 994 | // than a commit: a rebase that changes nothing in the tree has already |
| 931 | 995 | // been built (#177). Only a trusted build on the image the job names |
| 932 | 996 | // counts: a fork's result, or one from an image the job has left, does |
| 933 | // not stand for the repository's own (#258). An empty tree never | |
| 934 | // matches. | |
| 997 | // not stand for the repository's own (#258). A job naming no image | |
| 998 | // matches builds that named none, whichever default the runner used; | |
| 999 | // the CI wiki page says so. An empty tree never matches. | |
| 935 | 1000 | func (s *Store) SuccessBuildForTree(repoID int64, tree, job, image string) (Build, bool, error) { |
| 936 | 1001 | if tree == "" { |
| 937 | 1002 | return Build{}, false, nil |
| @@ -1000,14 +1065,20 @@ Ref #258" | ||
| 1000 | 1065 | - Modify: `internal/control/checksgate_test.go` (helper refactor, two tests) |
| 1001 | 1066 | - Test: `internal/control/mr_test.go` (append) |
| 1002 | 1067 | - Modify: `cmd/gitbay/main.go:580` (add a `pass`), `cmd/gitbay/summaries_gen.go` (regenerated) |
| 1003 | - Modify: `internal/httpd/settings.go:106-107`, `:228-229`; `internal/web/templates/settings.html:73-78`; `internal/web/templates/mr.html:124` | |
| 1004 | - Modify: `internal/httpd/mrpage_test.go` (`TestMRGatesRender`), `e2e/settingsweb_test.go` | |
| 1068 | - Modify: `internal/httpd/settings.go:106-107`, `:228-229`; `internal/web/templates/settings.html:73-78` (the require-checks form, its hint at line 75); `internal/web/templates/mr.html:124` | |
| 1069 | - Modify: `internal/httpd/mrpage_test.go` (`TestMRGatesRender`), `e2e/settingsweb_test.go:58` | |
| 1005 | 1070 | |
| 1006 | 1071 | **Interfaces:** |
| 1007 | 1072 | - Produces: |
| 1008 | 1073 | - `RepoSettings.RequiredContexts []string` (`json:"required_contexts,omitempty"`) |
| 1009 | 1074 | - `GatesOut.ChecksMissing []string` (`json:"checks_missing,omitempty"`) |
| 1010 | - command `repo settings require-contexts <owner/name> [<context>...]` | |
| 1075 | - command `repo settings require-contexts <owner/name> [<context>...]`: | |
| 1076 | a non-empty list is stored and sets `RequireChecks = true` in the | |
| 1077 | same update; an empty list clears the contexts and leaves | |
| 1078 | `RequireChecks` alone. | |
| 1079 | - `repo settings show` human output gains `require checks` and | |
| 1080 | `required contexts` rows (JSON already carries `require_checks` and | |
| 1081 | gains `required_contexts`). | |
| 1011 | 1082 | |
| 1012 | 1083 | - [ ] **Step 1: Write the failing tests** |
| 1013 | 1084 | |
| @@ -1122,42 +1193,88 @@ func TestRequiredContextsReportedPass(t *testing.T) { | ||
| 1122 | 1193 | } |
| 1123 | 1194 | ``` |
| 1124 | 1195 | |
| 1125 | Append to `internal/control/mr_test.go` (add imports `slices`, | |
| 1126 | `protocol`, `store` if absent): | |
| 1196 | Append to `internal/control/mr_test.go` (add `"slices"` to its imports; | |
| 1197 | `mrTestCtx` is that file's helper): | |
| 1127 | 1198 | |
| 1128 | 1199 | ```go |
| 1129 | // require-contexts stores a deduplicated list, refuses a context with | |
| 1130 | // whitespace, and clears with no contexts (#258). | |
| 1200 | // require-contexts stores a deduplicated list and turns require_checks | |
| 1201 | // on with it; an empty list clears the contexts and leaves | |
| 1202 | // require_checks as it was. A context with whitespace is refused (#258). | |
| 1131 | 1203 | func TestRequireContextsSetsAndClears(t *testing.T) { |
| 1132 | 1204 | st, repo, uid := newQueueTestRepo(t) |
| 1133 | 1205 | alice := store.User{ID: uid, Username: "alice"} |
| 1134 | run := func(args ...string) int { | |
| 1206 | dispatch := func(args ...string) int { | |
| 1135 | 1207 | t.Helper() |
| 1136 | c, _ := pruneCtx(st, t.TempDir(), alice) | |
| 1137 | return Dispatch(c, append([]string{"repo", "settings", "require-contexts", repo.Path()}, args...)) | |
| 1208 | c, _, _ := mrTestCtx(st, alice) | |
| 1209 | return Dispatch(c, args) | |
| 1138 | 1210 | } |
| 1139 | if code := run("lint", "ext/deploy", "lint"); code != protocol.ExitOK { | |
| 1211 | contexts := func(names ...string) int { | |
| 1212 | t.Helper() | |
| 1213 | return dispatch(append([]string{"repo", "settings", "require-contexts", repo.Path()}, names...)...) | |
| 1214 | } | |
| 1215 | settings := func() store.RepoSettings { | |
| 1216 | t.Helper() | |
| 1217 | got, err := st.RepoByID(repo.ID) | |
| 1218 | if err != nil { | |
| 1219 | t.Fatal(err) | |
| 1220 | } | |
| 1221 | return got.Settings | |
| 1222 | } | |
| 1223 | ||
| 1224 | if settings().RequireChecks { | |
| 1225 | t.Fatal("require_checks on in a new repository") | |
| 1226 | } | |
| 1227 | if code := contexts("lint", "ext/deploy", "lint"); code != protocol.ExitOK { | |
| 1140 | 1228 | t.Fatalf("set: exit %d", code) |
| 1141 | 1229 | } |
| 1142 | got, _ := st.RepoByID(repo.ID) | |
| 1143 | if !slices.Equal(got.Settings.RequiredContexts, []string{"lint", "ext/deploy"}) { | |
| 1144 | t.Fatalf("stored %v", got.Settings.RequiredContexts) | |
| 1230 | if s := settings(); !slices.Equal(s.RequiredContexts, []string{"lint", "ext/deploy"}) || !s.RequireChecks { | |
| 1231 | t.Fatalf("stored %v, require_checks %v; want [lint ext/deploy], on", s.RequiredContexts, s.RequireChecks) | |
| 1145 | 1232 | } |
| 1146 | if code := run("bad context"); code != protocol.ExitUsage { | |
| 1233 | if code := contexts("bad context"); code != protocol.ExitUsage { | |
| 1147 | 1234 | t.Fatalf("a context with a space: exit %d", code) |
| 1148 | 1235 | } |
| 1149 | if code := run(); code != protocol.ExitOK { | |
| 1236 | if code := contexts(); code != protocol.ExitOK { | |
| 1150 | 1237 | t.Fatalf("clear: exit %d", code) |
| 1151 | 1238 | } |
| 1152 | if got, _ := st.RepoByID(repo.ID); len(got.Settings.RequiredContexts) != 0 { | |
| 1153 | t.Fatalf("not cleared: %v", got.Settings.RequiredContexts) | |
| 1239 | if s := settings(); len(s.RequiredContexts) != 0 || !s.RequireChecks { | |
| 1240 | t.Fatalf("after clearing: contexts %v, require_checks %v; want none, still on", s.RequiredContexts, s.RequireChecks) | |
| 1241 | } | |
| 1242 | if code := dispatch("repo", "settings", "require-checks", repo.Path(), "off"); code != protocol.ExitOK { | |
| 1243 | t.Fatalf("require-checks off: exit %d", code) | |
| 1244 | } | |
| 1245 | if code := contexts(); code != protocol.ExitOK { | |
| 1246 | t.Fatalf("clear again: exit %d", code) | |
| 1247 | } | |
| 1248 | if settings().RequireChecks { | |
| 1249 | t.Fatal("clearing the list turned require_checks on") | |
| 1250 | } | |
| 1251 | } | |
| 1252 | ||
| 1253 | // settings show prints the checks gate beside the contexts it waits for, | |
| 1254 | // so a list that turned the gate on is visible where the gate is (#258). | |
| 1255 | func TestSettingsShowRequiredContexts(t *testing.T) { | |
| 1256 | st, repo, uid := newQueueTestRepo(t) | |
| 1257 | alice := store.User{ID: uid, Username: "alice"} | |
| 1258 | c, _, _ := mrTestCtx(st, alice) | |
| 1259 | if code := Dispatch(c, []string{"repo", "settings", "require-contexts", repo.Path(), "ext/deploy", "lint"}); code != protocol.ExitOK { | |
| 1260 | t.Fatalf("require-contexts: exit %d", code) | |
| 1261 | } | |
| 1262 | c, out, _ := mrTestCtx(st, alice) | |
| 1263 | if code := Dispatch(c, []string{"repo", "settings", "show", repo.Path()}); code != protocol.ExitOK { | |
| 1264 | t.Fatalf("settings show: exit %d", code) | |
| 1265 | } | |
| 1266 | got := strings.Join(strings.Fields(out.String()), " ") | |
| 1267 | for _, want := range []string{"require checks true", "required contexts ext/deploy, lint"} { | |
| 1268 | if !strings.Contains(got, want) { | |
| 1269 | t.Errorf("settings show lacks %q:\n%s", want, out.String()) | |
| 1270 | } | |
| 1154 | 1271 | } |
| 1155 | 1272 | } |
| 1156 | 1273 | ``` |
| 1157 | 1274 | |
| 1158 | 1275 | - [ ] **Step 2: Run them and see them fail** |
| 1159 | 1276 | |
| 1160 | Run: `go test ./internal/control -run 'TestRequire|TestRequiredContext' -count=1` | |
| 1277 | Run: `go test ./internal/control -run 'TestRequire|TestRequiredContext|TestSettingsShowRequiredContexts' -count=1` | |
| 1161 | 1278 | Expected: build failure (`s.RequiredContexts undefined`). |
| 1162 | 1279 | |
| 1163 | 1280 | - [ ] **Step 3: Store and output types** |
| @@ -1167,7 +1284,8 @@ Expected: build failure (`s.RequiredContexts undefined`). | ||
| 1167 | 1284 | ```go |
| 1168 | 1285 | RequireChecks bool `json:"require_checks,omitempty"` |
| 1169 | 1286 | // RequiredContexts are statuses require_checks waits for whether or |
| 1170 | // not they have reported; one that has not is pending (#258). | |
| 1287 | // not they have reported; one that has not is pending. Setting a | |
| 1288 | // non-empty list turns RequireChecks on (#258). | |
| 1171 | 1289 | RequiredContexts []string `json:"required_contexts,omitempty"` |
| 1172 | 1290 | ``` |
| 1173 | 1291 | |
| @@ -1184,8 +1302,8 @@ Registration, after `require-checks` at `mr.go:49`: | ||
| 1184 | 1302 | |
| 1185 | 1303 | ```go |
| 1186 | 1304 | register(Command{Path: []string{"repo", "settings", "require-contexts"}, |
| 1187 | Summary: "name the statuses require-checks waits for, reported or not", | |
| 1188 | Usage: "repo settings require-contexts <owner/name> [<context>...] (none clears)", | |
| 1305 | Summary: "name the statuses the checks gate waits for, and turn the gate on", | |
| 1306 | Usage: "repo settings require-contexts <owner/name> [<context>...] (none clears the list)", | |
| 1189 | 1307 | Examples: []string{"repo settings require-contexts krz/gitbay ci/build ci/test"}, |
| 1190 | 1308 | Run: runRequireContexts}) |
| 1191 | 1309 | ``` |
| @@ -1217,19 +1335,27 @@ func runRequireContexts(c *Ctx, args []string) int { | ||
| 1217 | 1335 | if code >= 0 { |
| 1218 | 1336 | return code |
| 1219 | 1337 | } |
| 1220 | s, err := c.Store.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) { s.RequiredContexts = contexts }) | |
| 1338 | // Naming contexts asks for the gate, so it turns require_checks on in | |
| 1339 | // the same update. Clearing the list leaves the gate as it was. | |
| 1340 | s, err := c.Store.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) { | |
| 1341 | s.RequiredContexts = contexts | |
| 1342 | if len(contexts) > 0 { | |
| 1343 | s.RequireChecks = true | |
| 1344 | } | |
| 1345 | }) | |
| 1221 | 1346 | if err != nil { |
| 1222 | 1347 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1223 | 1348 | } |
| 1224 | 1349 | return c.emit(s, func(w io.Writer) { |
| 1225 | if len(contexts) == 0 { | |
| 1226 | fmt.Fprintf(w, "required contexts cleared on %s\n", repo.Path()) | |
| 1350 | if len(contexts) > 0 { | |
| 1351 | fmt.Fprintf(w, "required contexts on %s: %s; require_checks on\n", repo.Path(), strings.Join(contexts, ", ")) | |
| 1227 | 1352 | return |
| 1228 | 1353 | } |
| 1229 | fmt.Fprintf(w, "required contexts on %s: %s\n", repo.Path(), strings.Join(contexts, ", ")) | |
| 1230 | if !s.RequireChecks { | |
| 1231 | fmt.Fprintf(c.Stderr, "they apply once require-checks is on: gitbay repo settings require-checks %s on\n", repo.Path()) | |
| 1354 | gate := "off" | |
| 1355 | if s.RequireChecks { | |
| 1356 | gate = "on" | |
| 1232 | 1357 | } |
| 1358 | fmt.Fprintf(w, "required contexts cleared on %s; require_checks %s\n", repo.Path(), gate) | |
| 1233 | 1359 | }) |
| 1234 | 1360 | } |
| 1235 | 1361 | ``` |
| @@ -1243,7 +1369,8 @@ the end of `if set.RequireChecks { … }`) with: | ||
| 1243 | 1369 | // Checks: with require_checks, every status the head carries must be |
| 1244 | 1370 | // green, a head something was going to report on must carry some, and |
| 1245 | 1371 | // every required context must have reported: one that has not is |
| 1246 | // pending whatever the others say (#258). | |
| 1372 | // pending whatever the others say (#258). Setting contexts turns | |
| 1373 | // require_checks on; turned off again, the list is kept and unread. | |
| 1247 | 1374 | statuses, err := st.ListCommitStatuses(repo.ID, headSHA) |
| 1248 | 1375 | if err != nil { |
| 1249 | 1376 | return g, err |
| @@ -1283,12 +1410,20 @@ the end of `if set.RequireChecks { … }`) with: | ||
| 1283 | 1410 | } |
| 1284 | 1411 | ``` |
| 1285 | 1412 | |
| 1286 | `repo.go:710-717`, add a field after `"protected tags"`: | |
| 1413 | `repo.go:710-717`, the `repo settings show` fields: today they print | |
| 1414 | neither the checks gate nor anything about it. Add two rows after | |
| 1415 | `"require mr"` (line 713), so the gate and the list that turned it on | |
| 1416 | read together: | |
| 1287 | 1417 | |
| 1288 | 1418 | ```go |
| 1419 | "require mr", strconv.FormatBool(repo.Settings.RequireMR), | |
| 1420 | "require checks", strconv.FormatBool(repo.Settings.RequireChecks), | |
| 1289 | 1421 | "required contexts", strings.Join(repo.Settings.RequiredContexts, ", "), |
| 1290 | 1422 | ``` |
| 1291 | 1423 | |
| 1424 | (`fields` skips a row whose value is empty, so a repository with no | |
| 1425 | contexts shows no `required contexts` row.) | |
| 1426 | ||
| 1292 | 1427 | - [ ] **Step 6: Run the control tests** |
| 1293 | 1428 | |
| 1294 | 1429 | Run: `go vet ./... && go test ./internal/control ./internal/store -count=1` |
| @@ -1320,13 +1455,20 @@ and in `fieldLabel` after `require-checks`: | ||
| 1320 | 1455 | return "required contexts" |
| 1321 | 1456 | ``` |
| 1322 | 1457 | |
| 1323 | `internal/web/templates/settings.html`, after the `require-checks` | |
| 1324 | form (line 78): | |
| 1458 | `internal/web/templates/settings.html`: the require-checks hint (line | |
| 1459 | 75) names the contexts the gate waits for, so a box ticked by saving | |
| 1460 | contexts says why: | |
| 1461 | ||
| 1462 | ```html | |
| 1463 | <div><label for="require-checks">Required checks</label><p class="hint">Requires CI to succeed.{{with .Repo.Settings.RequiredContexts}} Also waits for {{range $i, $c := .}}{{if $i}}, {{end}}<code>{{$c}}</code>{{end}} until they report{{if not $.Repo.Settings.RequireChecks}}, once this is on{{end}}.{{end}}</p></div> | |
| 1464 | ``` | |
| 1465 | ||
| 1466 | and after the `require-checks` form (line 78): | |
| 1325 | 1467 | |
| 1326 | 1468 | ```html |
| 1327 | 1469 | <form method="post" action="{{$base}}" class="setform"> |
| 1328 | 1470 | <input type="hidden" name="field" value="require-contexts"> |
| 1329 | <div><label for="contexts">Required contexts</label><p class="hint">Statuses the checks gate waits for until they report, separated by spaces. Applies with required checks on.</p></div> | |
| 1471 | <div><label for="contexts">Required contexts</label><p class="hint">Statuses the checks gate waits for until they report, separated by spaces. Saving any turns required checks on; saving none leaves it as it is.</p></div> | |
| 1330 | 1472 | <div><input type="text" id="contexts" name="contexts" value="{{range $i, $c := .Repo.Settings.RequiredContexts}}{{if $i}} {{end}}{{$c}}{{end}}" autocomplete="off"></div> |
| 1331 | 1473 | <div><button type="submit" class="btn">Save</button></div> |
| 1332 | 1474 | </form> |
| @@ -1348,15 +1490,23 @@ first `for` loop: | ||
| 1348 | 1490 | } |
| 1349 | 1491 | ``` |
| 1350 | 1492 | |
| 1351 | In `e2e/settingsweb_test.go`, after | |
| 1352 | `post(url.Values{"field": {"require-checks"}, …})`: | |
| 1493 | In `e2e/settingsweb_test.go`, replace line 58, | |
| 1494 | `post(url.Values{"field": {"require-checks"}, "require-checks": {"on"}})`, | |
| 1495 | with a contexts post, so the `"require_checks":true` the test already | |
| 1496 | expects from `settings show --json` (line 69) now comes from the | |
| 1497 | contexts turning the gate on: | |
| 1353 | 1498 | |
| 1354 | 1499 | ```go |
| 1355 | post(url.Values{"field": {"require-contexts"}, "contexts": {"ext/deploy lint"}}) | |
| 1500 | // Saving required contexts turns the checks gate on, and the page | |
| 1501 | // shows it ticked with the contexts in its hint (#258). | |
| 1502 | if body := post(url.Values{"field": {"require-contexts"}, "contexts": {"ext/deploy lint"}}); !strings.Contains(body, `id="require-checks" name="require-checks" value="on" checked`) || | |
| 1503 | !strings.Contains(body, `Also waits for <code>ext/deploy</code>, <code>lint</code> until they report.`) { | |
| 1504 | t.Fatalf("required contexts did not show as turning required checks on:\n%s", body) | |
| 1505 | } | |
| 1356 | 1506 | ``` |
| 1357 | 1507 | |
| 1358 | 1508 | and add `` `"required_contexts":["ext/deploy","lint"]` `` to the |
| 1359 | `settings show --json` want list. | |
| 1509 | `settings show --json` want list at line 69. | |
| 1360 | 1510 | |
| 1361 | 1511 | Run: `go test ./internal/httpd ./internal/web -count=1 && go test ./e2e -run TestRepoSettingsWeb -count=1` |
| 1362 | 1512 | Expected: PASS. |
| @@ -1365,7 +1515,7 @@ Expected: PASS. | ||
| 1365 | 1515 | |
| 1366 | 1516 | ```bash |
| 1367 | 1517 | git add internal/store/repos.go internal/control cmd/gitbay internal/httpd internal/web e2e/settingsweb_test.go |
| 1368 | git commit -S -m "repo settings: required contexts, pending until reported | |
| 1518 | git commit -S -m "repo settings: required contexts turn the checks gate on, pending until reported | |
| 1369 | 1519 | |
| 1370 | 1520 | Ref #258" |
| 1371 | 1521 | ``` |
| @@ -1391,8 +1541,11 @@ passed. Report under another prefix, such as =ext/=. | ||
| 1391 | 1541 | =repo settings require-contexts <repo> ext/deploy ci/test= names |
| 1392 | 1542 | statuses the checks gate waits for whether or not they have reported: |
| 1393 | 1543 | one that has not is =pending=, and =mr show= lists it as |
| 1394 | =ext/deploy=missing=. It applies while =require-checks= is on; with no | |
| 1395 | contexts it clears the list. | |
| 1544 | =ext/deploy=missing=. Naming any context turns =require-checks= on; | |
| 1545 | with no contexts the command clears the list and leaves | |
| 1546 | =require-checks= as it was. =require-checks off= keeps the list, which | |
| 1547 | waits for nothing until the gate is on again. =repo settings show= | |
| 1548 | prints both. | |
| 1396 | 1549 | ``` |
| 1397 | 1550 | |
| 1398 | 1551 | - [ ] **Step 2: CI.org** |
| @@ -1401,13 +1554,20 @@ In the *Dedupe* bullet, after "…naming the build it came from (#177).", | ||
| 1401 | 1554 | add: "Only a trusted build counts, and for tree reuse only one on the |
| 1402 | 1555 | image the job names: a fork's green build does not stand for the |
| 1403 | 1556 | repository's own, so its commit is built again when it lands on a |
| 1404 | branch (#258)." | |
| 1557 | branch (#258). A job that names no =image:= is compared as naming | |
| 1558 | none: its reuse does not notice the runner's default image changing, | |
| 1559 | because reuse is decided when the push is queued, before any runner | |
| 1560 | claims the build, and runners can differ in their default. Name the | |
| 1561 | image in =ci.yml= to tie reuse to it; after an operator changes a | |
| 1562 | runner's =-image=, =build trigger= builds a job afresh, since a | |
| 1563 | triggered build is never reused." | |
| 1405 | 1564 | |
| 1406 | 1565 | - [ ] **Step 3: Users.org** |
| 1407 | 1566 | |
| 1408 | 1567 | After "…a =ci/<job>= commit status, which =repo settings |
| 1409 | 1568 | require-checks= can gate merges on." add: "=repo settings |
| 1410 | require-contexts= names statuses the gate waits for until they report." | |
| 1569 | require-contexts= names statuses the gate waits for until they report, | |
| 1570 | and turns the gate on." | |
| 1411 | 1571 | |
| 1412 | 1572 | - [ ] **Step 4: Parity.org** |
| 1413 | 1573 | |
| @@ -1459,57 +1619,83 @@ with `--strategy ff` once CI is green; delete the branch both places. | ||
| 1459 | 1619 | |
| 1460 | 1620 | --- |
| 1461 | 1621 | |
| 1462 | # Part 3: separate the runner's source address from its builds (branch `runner-source-address`, #260) | |
| 1622 | # Part 3: separate the runner's source address from its builds, and limit what builds reach on its host (branch `runner-source-address`, #260) | |
| 1463 | 1623 | |
| 1464 | 1624 | ### Task 3.1: the claim carries the instance's public ssh destination |
| 1465 | 1625 | |
| 1466 | 1626 | **Files:** |
| 1467 | - Modify: `internal/control/build.go` (the payload from Task 1.1; a helper beside `runRunnerNext`) | |
| 1627 | - Modify: `internal/control/build.go` (imports; the payload from Task 1.1; a helper beside `runRunnerNext`) | |
| 1468 | 1628 | - Test: `internal/control/runnernext_test.go` (append) |
| 1469 | 1629 | |
| 1470 | 1630 | **Interfaces:** |
| 1471 | - Produces: `runner next --json` payload `"ssh": "git@<site host>"`, omitted when `site_url` is empty. | |
| 1631 | - Produces: `runner next --json` payload `"ssh": "git@<site host>"`, or | |
| 1632 | `"git@<site host>:<port>"` when `[ssh] port` is neither 0 nor 22; | |
| 1633 | omitted when `site_url` is empty. | |
| 1472 | 1634 | |
| 1473 | 1635 | - [ ] **Step 1: Write the failing test** |
| 1474 | 1636 | |
| 1475 | 1637 | ```go |
| 1476 | 1638 | // A build on the daemon's own host is given the public destination, not |
| 1477 | // the loopback address its runner polls (#260). | |
| 1639 | // the loopback address its runner polls. The port rides along only when | |
| 1640 | // it is not 22, so ssh://$GITBAY_SSH/<owner>/<name>.git is a valid URL | |
| 1641 | // either way (#260). | |
| 1478 | 1642 | func TestRunnerNextCarriesPublicSSH(t *testing.T) { |
| 1479 | 1643 | st, repo, uid, root, baseSHA, _ := setupOrphanRepo(t) |
| 1480 | if _, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", "", "", true); err != nil { | |
| 1481 | t.Fatal(err) | |
| 1482 | } | |
| 1483 | c, out := runnerCtx(st, uid, root) // site_url https://x.test | |
| 1484 | c.JSON = true | |
| 1485 | if code := runRunnerNext(c, nil); code != protocol.ExitOK { | |
| 1486 | t.Fatalf("runner next: exit %d, output:\n%s", code, out.String()) | |
| 1487 | } | |
| 1488 | if !strings.Contains(out.String(), `"ssh":"git@x.test"`) { | |
| 1489 | t.Fatalf("claim lacks the public destination:\n%s", out.String()) | |
| 1644 | for _, tc := range []struct { | |
| 1645 | port int | |
| 1646 | want string | |
| 1647 | }{ | |
| 1648 | {0, `"ssh":"git@x.test"`}, | |
| 1649 | {22, `"ssh":"git@x.test"`}, | |
| 1650 | {2022, `"ssh":"git@x.test:2022"`}, | |
| 1651 | } { | |
| 1652 | if _, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", "", "", true); err != nil { | |
| 1653 | t.Fatal(err) | |
| 1654 | } | |
| 1655 | c, out := runnerCtx(st, uid, root) // site_url https://x.test | |
| 1656 | c.Cfg.SSH.Port = tc.port | |
| 1657 | c.JSON = true | |
| 1658 | if code := runRunnerNext(c, nil); code != protocol.ExitOK { | |
| 1659 | t.Fatalf("port %d: runner next: exit %d, output:\n%s", tc.port, code, out.String()) | |
| 1660 | } | |
| 1661 | if !strings.Contains(out.String(), tc.want) { | |
| 1662 | t.Fatalf("port %d: claim lacks %s:\n%s", tc.port, tc.want, out.String()) | |
| 1663 | } | |
| 1490 | 1664 | } |
| 1491 | 1665 | } |
| 1492 | 1666 | ``` |
| 1493 | 1667 | |
| 1668 | Each pass queues one build and claims it, as in Task 1.1's test. | |
| 1669 | `runnerCtx` builds its `config.Config` by hand, so `SSH.Port` starts at | |
| 1670 | 0; config validation refuses 0 on a real instance (`config.go:367`), | |
| 1671 | and 0 is read as 22. | |
| 1672 | ||
| 1494 | 1673 | - [ ] **Step 2: Run it and see it fail** |
| 1495 | 1674 | |
| 1496 | 1675 | Run: `go test ./internal/control -run TestRunnerNextCarriesPublicSSH -count=1` |
| 1497 | Expected: FAIL, `claim lacks the public destination`. | |
| 1676 | Expected: FAIL, `port 0: claim lacks "ssh":"git@x.test"`. | |
| 1498 | 1677 | |
| 1499 | 1678 | - [ ] **Step 3: Implement** |
| 1500 | 1679 | |
| 1501 | Add after `maxOrphanSkip`: | |
| 1680 | Add `"net"` to `build.go`'s imports (`strconv` is already there), and | |
| 1681 | after `maxOrphanSkip`: | |
| 1502 | 1682 | |
| 1503 | 1683 | ```go |
| 1504 | 1684 | // publicSSH is the instance's ssh destination as anyone outside reaches |
| 1505 | 1685 | // it. A runner on the daemon's own host polls over loopback and hands |
| 1506 | 1686 | // its builds this instead, so no build connects from the runner's source |
| 1507 | // address (#260). Empty when site_url is not set. | |
| 1687 | // address (#260). The port is added only when it is not 22: hutch and | |
| 1688 | // orgo build ssh://$GITBAY_SSH/... URLs, valid in both forms. Empty when | |
| 1689 | // site_url is not set. | |
| 1508 | 1690 | func publicSSH(c *Ctx) string { |
| 1509 | if host := c.Cfg.SiteHost(); host != "" { | |
| 1510 | return "git@" + host | |
| 1691 | host := c.Cfg.SiteHost() | |
| 1692 | if host == "" { | |
| 1693 | return "" | |
| 1694 | } | |
| 1695 | if p := c.Cfg.SSH.Port; p != 0 && p != 22 { | |
| 1696 | return "git@" + net.JoinHostPort(host, strconv.Itoa(p)) | |
| 1511 | 1697 | } |
| 1512 | return "" | |
| 1698 | return "git@" + host | |
| 1513 | 1699 | } |
| 1514 | 1700 | ``` |
| 1515 | 1701 | |
| @@ -1560,7 +1746,8 @@ Replace `TestStepEnvCarriesInstanceAddress` (`env_test.go:190-212`) with: | ||
| 1560 | 1746 | // A build that talks back to the instance needs an address that works |
| 1561 | 1747 | // from where it runs. A runner polling over loopback keeps its podman |
| 1562 | 1748 | // builds off the host's loopback, so they get the instance's public |
| 1563 | // destination from the claim; any other remote is used as it is (#260). | |
| 1749 | // destination from the claim, port included when it is not 22; any other | |
| 1750 | // remote is used as it is (#260). | |
| 1564 | 1751 | func TestStepEnvCarriesInstanceAddress(t *testing.T) { |
| 1565 | 1752 | env := stepEnv(job{}, "/tmp/buildhome", "git@gitbay.org") |
| 1566 | 1753 | if !containsEnv(env, "GITBAY_SSH=git@gitbay.org") { |
| @@ -1569,6 +1756,7 @@ func TestStepEnvCarriesInstanceAddress(t *testing.T) { | ||
| 1569 | 1756 | for _, tc := range []struct{ remote, isolation, public, want string }{ |
| 1570 | 1757 | {"git@127.0.0.1", isolationNone, "git@gitbay.org", "git@127.0.0.1"}, |
| 1571 | 1758 | {"git@127.0.0.1", isolationPodman, "git@gitbay.org", "git@gitbay.org"}, |
| 1759 | {"git@127.0.0.1", isolationPodman, "git@gitbay.test:2022", "git@gitbay.test:2022"}, | |
| 1572 | 1760 | {"git@localhost", isolationPodman, "git@gitbay.org", "git@gitbay.org"}, |
| 1573 | 1761 | {"git@127.0.0.1", isolationPodman, "", "git@127.0.0.1"}, |
| 1574 | 1762 | {"git@gitbay.org", isolationPodman, "git@other.test", "git@gitbay.org"}, |
| @@ -1648,7 +1836,10 @@ func (r *runner) buildSSH(public string) string { | ||
| 1648 | 1836 | // source address, so a build sharing the runner's could throttle its |
| 1649 | 1837 | // polling (#260). --no-map-gw removes the mapping: the build reaches the |
| 1650 | 1838 | // host only at its public address, as any client on the internet does, |
| 1651 | // and keeps its outbound access. | |
| 1839 | // and keeps its outbound access. The host's nftables table | |
| 1840 | // (deploy/gitbay-runner-egress.nft) then limits it to 22, 80 and 443 | |
| 1841 | // there; it cannot tell a build from the runner by uid, so it leaves | |
| 1842 | // 127.0.0.1:22 open, and this flag is what keeps builds off it. | |
| 1652 | 1843 | func (r *runner) buildNetwork() []string { |
| 1653 | 1844 | if !r.loopbackRemote() { |
| 1654 | 1845 | return nil |
| @@ -1679,13 +1870,413 @@ git commit -S -m "runner: builds off the host's loopback when the runner polls o | ||
| 1679 | 1870 | Ref #260" |
| 1680 | 1871 | ``` |
| 1681 | 1872 | |
| 1682 | ### Task 3.3: egress policy in the wiki, and the MR | |
| 1873 | ### Task 3.3: the limiter's behaviour, by registration mode | |
| 1874 | ||
| 1875 | This test records what the code does today (see "Finding for #260" | |
| 1876 | under Decisions), so it passes on its first run. It is the evidence the | |
| 1877 | issue's on-production throttling test was meant to give, without | |
| 1878 | touching production. If it fails, the limiter differs from what | |
| 1879 | Decisions and the wiki say: stop and correct that text, not the test. | |
| 1683 | 1880 | |
| 1684 | 1881 | **Files:** |
| 1685 | - Modify: `.gitbay/wiki/Threat-Model.org` ("The CI runner", after the *Images* bullet) | |
| 1686 | - Modify: `.gitbay/wiki/CI.org` (new section before "* The table") | |
| 1687 | - Modify: `.gitbay/wiki/Users.org:550-555` (the `GITBAY_SSH` sentence) | |
| 1688 | - Modify: `.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org` (Network row), `04-Trust-Boundaries.org` (TB7), `09-Controls.org:91` | |
| 1882 | - Test: `internal/sshd/sshd_test.go` (append; every import it needs is already in the file) | |
| 1883 | ||
| 1884 | **Interfaces:** | |
| 1885 | - Consumes: `(*Server).authenticate`, `rateLimiter.seen` (package-internal). | |
| 1886 | ||
| 1887 | - [ ] **Step 1: Write the test** | |
| 1888 | ||
| 1889 | Append to `internal/sshd/sshd_test.go`: | |
| 1890 | ||
| 1891 | ```go | |
| 1892 | // authMeta is the connection metadata authenticate reads: only the | |
| 1893 | // remote address. | |
| 1894 | type authMeta struct { | |
| 1895 | ssh.ConnMetadata | |
| 1896 | addr net.Addr | |
| 1897 | } | |
| 1898 | ||
| 1899 | func (m authMeta) RemoteAddr() net.Addr { return m.addr } | |
| 1900 | ||
| 1901 | func authKey(t *testing.T) ssh.PublicKey { | |
| 1902 | t.Helper() | |
| 1903 | pub, _, err := ed25519.GenerateKey(rand.Reader) | |
| 1904 | if err != nil { | |
| 1905 | t.Fatal(err) | |
| 1906 | } | |
| 1907 | k, err := ssh.NewPublicKey(pub) | |
| 1908 | if err != nil { | |
| 1909 | t.Fatal(err) | |
| 1910 | } | |
| 1911 | return k | |
| 1912 | } | |
| 1913 | ||
| 1914 | // authServer is a Server holding what authenticate uses: a store with a | |
| 1915 | // runner account's key, the registration mode, and a limiter of three | |
| 1916 | // failures a minute. | |
| 1917 | func authServer(t *testing.T, mode string) (*Server, ssh.PublicKey) { | |
| 1918 | t.Helper() | |
| 1919 | st, err := store.Open(filepath.Join(t.TempDir(), "gitbay.db")) | |
| 1920 | if err != nil { | |
| 1921 | t.Fatal(err) | |
| 1922 | } | |
| 1923 | t.Cleanup(func() { st.Close() }) | |
| 1924 | if err := st.MigrateUp(); err != nil { | |
| 1925 | t.Fatal(err) | |
| 1926 | } | |
| 1927 | uid, err := st.CreateUser("ci", false) | |
| 1928 | if err != nil { | |
| 1929 | t.Fatal(err) | |
| 1930 | } | |
| 1931 | runner := authKey(t) | |
| 1932 | if err := st.AddSSHKey(uid, ssh.FingerprintSHA256(runner), runner.Type(), runner.Marshal(), "runner", ""); err != nil { | |
| 1933 | t.Fatal(err) | |
| 1934 | } | |
| 1935 | cfg := config.Default() | |
| 1936 | cfg.Registration.Mode = mode | |
| 1937 | return &Server{cfg: cfg, st: st, authLimiter: newRateLimiter(3, time.Minute)}, runner | |
| 1938 | } | |
| 1939 | ||
| 1940 | var ( | |
| 1941 | fromLoopback = authMeta{addr: &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 40000}} | |
| 1942 | fromPublic = authMeta{addr: &net.TCPAddr{IP: net.IPv4(203, 0, 113, 7), Port: 40000}} | |
| 1943 | ) | |
| 1944 | ||
| 1945 | // With registration closed an unknown key counts against its address. | |
| 1946 | // Below the limit a known key's success clears the count. At the limit | |
| 1947 | // authenticate refuses before it looks at the key, so the runner's own | |
| 1948 | // key from that address is refused too and its success never runs to | |
| 1949 | // clear anything, until the window passes. Another address is not | |
| 1950 | // affected. This is why a build must not share the runner's source | |
| 1951 | // address (#260). | |
| 1952 | func TestAuthLockoutHoldsAgainstTheRunnersKey(t *testing.T) { | |
| 1953 | s, runner := authServer(t, "closed") | |
| 1954 | stranger := authKey(t) | |
| 1955 | failTimes := func(n int) { | |
| 1956 | t.Helper() | |
| 1957 | for i := 0; i < n; i++ { | |
| 1958 | if _, err := s.authenticate(fromLoopback, stranger); err == nil { | |
| 1959 | t.Fatal("unknown key admitted with registration closed") | |
| 1960 | } | |
| 1961 | } | |
| 1962 | } | |
| 1963 | ||
| 1964 | failTimes(2) | |
| 1965 | if _, err := s.authenticate(fromLoopback, runner); err != nil { | |
| 1966 | t.Fatalf("runner below the limit: %v", err) | |
| 1967 | } | |
| 1968 | failTimes(2) | |
| 1969 | if _, err := s.authenticate(fromLoopback, runner); err != nil { | |
| 1970 | t.Fatalf("runner after its success cleared the count: %v", err) | |
| 1971 | } | |
| 1972 | ||
| 1973 | failTimes(3) | |
| 1974 | for i := 0; i < 2; i++ { | |
| 1975 | if _, err := s.authenticate(fromLoopback, runner); err == nil || !strings.Contains(err.Error(), "too many") { | |
| 1976 | t.Fatalf("attempt %d from a locked-out address: %v, want refused", i+1, err) | |
| 1977 | } | |
| 1978 | } | |
| 1979 | if _, err := s.authenticate(fromPublic, runner); err != nil { | |
| 1980 | t.Fatalf("another address was locked out too: %v", err) | |
| 1981 | } | |
| 1982 | ||
| 1983 | s.authLimiter.seen["127.0.0.1"].start = time.Now().Add(-2 * time.Minute) | |
| 1984 | if _, err := s.authenticate(fromLoopback, runner); err != nil { | |
| 1985 | t.Fatalf("runner after the window passed: %v", err) | |
| 1986 | } | |
| 1987 | } | |
| 1988 | ||
| 1989 | // With registration open or by invite, an unknown key is admitted to run | |
| 1990 | // register and never counts, so no number of unknown-key attempts locks | |
| 1991 | // the runner's address out. gitbay.org runs open registration (#260). | |
| 1992 | func TestAuthUnknownKeyCountsOnlyWhenClosed(t *testing.T) { | |
| 1993 | for _, mode := range []string{"open", "invite"} { | |
| 1994 | s, runner := authServer(t, mode) | |
| 1995 | for i := 0; i < 10; i++ { | |
| 1996 | p, err := s.authenticate(fromLoopback, authKey(t)) | |
| 1997 | if err != nil || p.Extensions["anon-key"] == "" { | |
| 1998 | t.Fatalf("%s: unknown key %d: %v %+v", mode, i+1, err, p) | |
| 1999 | } | |
| 2000 | } | |
| 2001 | if _, err := s.authenticate(fromLoopback, runner); err != nil { | |
| 2002 | t.Fatalf("%s: runner refused after unknown keys: %v", mode, err) | |
| 2003 | } | |
| 2004 | } | |
| 2005 | } | |
| 2006 | ``` | |
| 2007 | ||
| 2008 | - [ ] **Step 2: Run it** | |
| 2009 | ||
| 2010 | Run: `go vet ./internal/sshd && go test ./internal/sshd -run 'TestAuth' -count=1` | |
| 2011 | Expected: PASS. | |
| 2012 | ||
| 2013 | - [ ] **Step 3: Commit** | |
| 2014 | ||
| 2015 | ```bash | |
| 2016 | git add internal/sshd/sshd_test.go | |
| 2017 | git commit -S -m "sshd: test the auth limiter's lockout by registration mode | |
| 2018 | ||
| 2019 | Ref #260" | |
| 2020 | ``` | |
| 2021 | ||
| 2022 | ### Task 3.4: host egress rule for the runner's uid | |
| 2023 | ||
| 2024 | No Go code. The check script is the test: `make deploy-runner` runs it | |
| 2025 | after loading the rule and before restarting the runner, and runbook R3 | |
| 2026 | runs the whole path against the scratch repository before merge. | |
| 2027 | ||
| 2028 | **Files:** | |
| 2029 | - Create: `deploy/gitbay-runner-egress.nft`, `deploy/gitbay-runner-egress.service`, `deploy/runner-egress-check.sh` | |
| 2030 | - Modify: `deploy/gitbay-runner.override.conf` (a `[Unit]` section before `[Service]` at line 25) | |
| 2031 | - Modify: `deploy/runner-podman-setup.sh` (after `podman --version`, line 29) | |
| 2032 | - Modify: `Makefile:69-85` (`deploy-runner`) | |
| 2033 | ||
| 2034 | **Interfaces:** | |
| 2035 | - Produces: nftables table `inet gitbay_runner`; unit | |
| 2036 | `gitbay-runner-egress.service`, required by `gitbay-runner.service`; | |
| 2037 | rule file at `/etc/gitbay-runner/egress.nft` on the runner host. | |
| 2038 | ||
| 2039 | - [ ] **Step 1: The rule** | |
| 2040 | ||
| 2041 | Create `deploy/gitbay-runner-egress.nft`: | |
| 2042 | ||
| 2043 | ``` | |
| 2044 | #!/usr/sbin/nft -f | |
| 2045 | # Host egress for CI builds (#260). Loaded by gitbay-runner-egress.service, | |
| 2046 | # which gitbay-runner.service requires, so the runner does not start | |
| 2047 | # without it. `make deploy-runner` installs it as | |
| 2048 | # /etc/gitbay-runner/egress.nft. | |
| 2049 | # | |
| 2050 | # Under rootless podman with pasta, a build's connections are made by | |
| 2051 | # pasta on the host, from sockets owned by the runner's user, ci-runner. | |
| 2052 | # nftables sees them exactly as it sees the runner's own ssh, so this | |
| 2053 | # table cannot tell a build from its runner. It limits what that user | |
| 2054 | # reaches on this host, and the runner needs little: 127.0.0.1:22, to | |
| 2055 | # poll, clone and stream logs. | |
| 2056 | # | |
| 2057 | # Every packet to one of the host's own addresses, loopback or public, | |
| 2058 | # leaves through lo, so the output hook sees host-bound traffic as | |
| 2059 | # oifname "lo". Traffic to other hosts is not matched: builds keep | |
| 2060 | # outbound internet access, trusted or not (go mod download needs it). | |
| 2061 | # | |
| 2062 | # What ci-runner may reach on this host: | |
| 2063 | # 127.0.0.1:22 the forge over loopback, for the runner. Builds do | |
| 2064 | # not reach loopback at all: the runner starts them | |
| 2065 | # with pasta's gateway mapping off (--no-map-gw). | |
| 2066 | # loopback :53 the host's resolver, which pasta forwards a | |
| 2067 | # build's DNS to when the host's nameserver is a | |
| 2068 | # loopback address. | |
| 2069 | # public 22/80/443 the forge, as anyone on the internet reaches it. | |
| 2070 | # Everything else is rejected: the admin sshd on 2222 on every address, | |
| 2071 | # and any service bound to loopback. -isolation none builds run as the | |
| 2072 | # same user and get the same rule. | |
| 2073 | # | |
| 2074 | # The account name is resolved when the file is loaded. A restart of | |
| 2075 | # nftables.service (flush ruleset) removes this table; `systemctl | |
| 2076 | # reload gitbay-runner-egress` puts it back. | |
| 2077 | ||
| 2078 | table inet gitbay_runner | |
| 2079 | delete table inet gitbay_runner | |
| 2080 | ||
| 2081 | table inet gitbay_runner { | |
| 2082 | chain output { | |
| 2083 | type filter hook output priority filter; policy accept; | |
| 2084 | oifname "lo" meta skuid "ci-runner" jump host | |
| 2085 | } | |
| 2086 | ||
| 2087 | chain host { | |
| 2088 | ip daddr 127.0.0.1 tcp dport 22 accept | |
| 2089 | ip daddr 127.0.0.0/8 meta l4proto { tcp, udp } th dport 53 accept | |
| 2090 | ip6 daddr ::1 meta l4proto { tcp, udp } th dport 53 accept | |
| 2091 | ip daddr != 127.0.0.0/8 tcp dport { 22, 80, 443 } accept | |
| 2092 | ip6 daddr != ::1 tcp dport { 22, 80, 443 } accept | |
| 2093 | counter reject | |
| 2094 | } | |
| 2095 | } | |
| 2096 | ``` | |
| 2097 | ||
| 2098 | The first `table` line creates the table if it is missing so the | |
| 2099 | `delete` never fails; the file then replaces it in one transaction, so | |
| 2100 | a reload never leaves a moment without the rule. The uid match is in | |
| 2101 | the base chain's one rule rather than a `!=` accept, because a packet | |
| 2102 | with no socket (a kernel-sent reset) matches neither `==` nor `!=` on | |
| 2103 | `skuid` and would otherwise fall through to the reject. | |
| 2104 | ||
| 2105 | - [ ] **Step 2: The unit** | |
| 2106 | ||
| 2107 | Create `deploy/gitbay-runner-egress.service`: | |
| 2108 | ||
| 2109 | ``` | |
| 2110 | # Loads the CI runner's host egress rule (#260, | |
| 2111 | # deploy/gitbay-runner-egress.nft). gitbay-runner.service requires this | |
| 2112 | # unit, so the runner starts only with the rule in force; stopping this | |
| 2113 | # unit removes the table and stops the runner with it. | |
| 2114 | # | |
| 2115 | # Ordered after nftables.service and ufw.service: either may rewrite the | |
| 2116 | # ruleset at boot, and nftables.service's default config starts with | |
| 2117 | # flush ruleset. A missing unit in After= is ignored. | |
| 2118 | # | |
| 2119 | # Reload re-reads the file and replaces the table in one transaction; it | |
| 2120 | # does not restart the runner, which a restart of this unit would | |
| 2121 | # (Requires= propagates restarts). `make deploy-runner` reloads. | |
| 2122 | [Unit] | |
| 2123 | Description=Host egress rule for CI builds | |
| 2124 | After=nftables.service ufw.service | |
| 2125 | Before=gitbay-runner.service | |
| 2126 | ||
| 2127 | [Service] | |
| 2128 | Type=oneshot | |
| 2129 | RemainAfterExit=yes | |
| 2130 | ExecStart=/usr/sbin/nft -f /etc/gitbay-runner/egress.nft | |
| 2131 | ExecReload=/usr/sbin/nft -f /etc/gitbay-runner/egress.nft | |
| 2132 | ExecStop=/usr/sbin/nft delete table inet gitbay_runner | |
| 2133 | ||
| 2134 | [Install] | |
| 2135 | WantedBy=multi-user.target | |
| 2136 | ``` | |
| 2137 | ||
| 2138 | - [ ] **Step 3: The runner requires it** | |
| 2139 | ||
| 2140 | In `deploy/gitbay-runner.override.conf`, insert before `[Service]` | |
| 2141 | (line 25): | |
| 2142 | ||
| 2143 | ``` | |
| 2144 | [Unit] | |
| 2145 | # The host egress rule (#260, gitbay-runner-egress.nft) limits what this | |
| 2146 | # unit's user reaches on the host: 127.0.0.1:22 for the runner, the | |
| 2147 | # forge's public 22, 80 and 443 for builds, nothing else. Required, so | |
| 2148 | # the runner does not start without it: a table that failed to load must | |
| 2149 | # not mean builds reach the admin sshd. | |
| 2150 | Requires=gitbay-runner-egress.service | |
| 2151 | After=gitbay-runner-egress.service | |
| 2152 | ``` | |
| 2153 | ||
| 2154 | - [ ] **Step 4: The check** | |
| 2155 | ||
| 2156 | Create `deploy/runner-egress-check.sh`: | |
| 2157 | ||
| 2158 | ```sh | |
| 2159 | #!/bin/sh | |
| 2160 | # Check the CI runner's host egress rule (#260) as the runner's user: | |
| 2161 | # the forge over loopback on 22 must answer (the runner polls there), | |
| 2162 | # and the admin sshd on 2222 must not, on loopback or the public | |
| 2163 | # address. `make deploy-runner` runs this after loading the rule and | |
| 2164 | # before restarting the runner, and stops on a failure. | |
| 2165 | # | |
| 2166 | # ssh -p 2222 root@bay1 'sh -s' < deploy/runner-egress-check.sh | |
| 2167 | set -eu | |
| 2168 | ||
| 2169 | RUNNER_USER="${RUNNER_USER:-ci-runner}" | |
| 2170 | public=$(hostname -I | awk '{print $1}') | |
| 2171 | ||
| 2172 | probe() { | |
| 2173 | su -s /bin/bash "$RUNNER_USER" -c "timeout 5 bash -c 'exec 3<>/dev/tcp/$1/$2'" 2>/dev/null | |
| 2174 | } | |
| 2175 | ||
| 2176 | nft list table inet gitbay_runner >/dev/null | |
| 2177 | ||
| 2178 | for dest in 127.0.0.1:22 "$public:22"; do | |
| 2179 | if ! probe "${dest%:*}" "${dest##*:}"; then | |
| 2180 | echo "$RUNNER_USER cannot reach $dest: the egress rule would stop the runner" >&2 | |
| 2181 | exit 1 | |
| 2182 | fi | |
| 2183 | done | |
| 2184 | for dest in 127.0.0.1:2222 "$public:2222"; do | |
| 2185 | if probe "${dest%:*}" "${dest##*:}"; then | |
| 2186 | echo "$RUNNER_USER reaches $dest: the egress rule is not in force" >&2 | |
| 2187 | exit 1 | |
| 2188 | fi | |
| 2189 | done | |
| 2190 | echo "egress for $RUNNER_USER: 127.0.0.1:22 and $public:22 open, 2222 refused" | |
| 2191 | ``` | |
| 2192 | ||
| 2193 | `hostname -I` lists the host's addresses, IPv4 first on bay1; the | |
| 2194 | first is the public one there. | |
| 2195 | ||
| 2196 | - [ ] **Step 5: nftables on the host** | |
| 2197 | ||
| 2198 | In `deploy/runner-podman-setup.sh`, after the podman install block | |
| 2199 | (after `podman --version`, line 29): | |
| 2200 | ||
| 2201 | ```sh | |
| 2202 | # nft loads the runner's host egress rule (#260, | |
| 2203 | # deploy/gitbay-runner-egress.nft), which `make deploy-runner` ships and | |
| 2204 | # the runner's unit requires. Without nft the runner does not start. | |
| 2205 | echo "==> installing nftables" | |
| 2206 | if ! command -v nft >/dev/null 2>&1; then | |
| 2207 | apt-get update | |
| 2208 | DEBIAN_FRONTEND=noninteractive apt-get install -y nftables | |
| 2209 | fi | |
| 2210 | nft --version | |
| 2211 | ``` | |
| 2212 | ||
| 2213 | - [ ] **Step 6: `make deploy-runner` ships, loads and checks it** | |
| 2214 | ||
| 2215 | Replace `Makefile:69-85` with: | |
| 2216 | ||
| 2217 | ```make | |
| 2218 | deploy-runner: preflight | |
| 2219 | @echo "==> building $(RUNNER_BIN)" | |
| 2220 | $(CROSS) go build -trimpath -ldflags='$(LDFLAGS)' -o $(RUNNER_BIN) ./cmd/gitbay-runner | |
| 2221 | @echo "==> pushing runner to $(HOST)" | |
| 2222 | ./deploy/copy.sh $(HOST) $(PORT) $(RUNNER_BIN) /usr/local/bin/gitbay-runner.new | |
| 2223 | ssh -p $(PORT) root@$(HOST) 'mkdir -p /etc/systemd/system/gitbay-runner.service.d /etc/gitbay-runner' | |
| 2224 | ./deploy/copy.sh $(HOST) $(PORT) deploy/gitbay-runner.override.conf /etc/systemd/system/gitbay-runner.service.d/override.conf | |
| 2225 | ./deploy/copy.sh $(HOST) $(PORT) deploy/gitbay-runner-prune.service /etc/systemd/system/gitbay-runner-prune.service | |
| 2226 | ./deploy/copy.sh $(HOST) $(PORT) deploy/gitbay-runner-prune.timer /etc/systemd/system/gitbay-runner-prune.timer | |
| 2227 | ./deploy/copy.sh $(HOST) $(PORT) deploy/gitbay-runner-egress.nft /etc/gitbay-runner/egress.nft | |
| 2228 | ./deploy/copy.sh $(HOST) $(PORT) deploy/gitbay-runner-egress.service /etc/systemd/system/gitbay-runner-egress.service | |
| 2229 | @echo "==> loading the egress rule" | |
| 2230 | ssh -p $(PORT) root@$(HOST) 'set -eu; \ | |
| 2231 | nft -c -f /etc/gitbay-runner/egress.nft; \ | |
| 2232 | systemctl daemon-reload; \ | |
| 2233 | systemctl enable gitbay-runner-egress.service; \ | |
| 2234 | systemctl reload-or-restart gitbay-runner-egress.service' | |
| 2235 | ssh -p $(PORT) root@$(HOST) 'sh -s' < deploy/runner-egress-check.sh | |
| 2236 | ssh -p $(PORT) root@$(HOST) 'set -eu; \ | |
| 2237 | chmod 755 /usr/local/bin/gitbay-runner.new; \ | |
| 2238 | mv /usr/local/bin/gitbay-runner.new /usr/local/bin/gitbay-runner; \ | |
| 2239 | systemctl enable --now gitbay-runner-prune.timer; \ | |
| 2240 | systemctl restart gitbay-runner; \ | |
| 2241 | systemctl --no-pager --lines=3 status gitbay-runner; \ | |
| 2242 | systemctl --no-pager list-timers gitbay-runner-prune.timer' | |
| 2243 | ``` | |
| 2244 | ||
| 2245 | `nft -c` checks the file without applying it, so a syntax error stops | |
| 2246 | the deploy with the old table and the old runner in place. On the first | |
| 2247 | deploy `reload-or-restart` starts the unit, and starting a required unit | |
| 2248 | does not restart the runner that requires it; later deploys reload it. | |
| 2249 | The check runs before the runner restarts, so a failure stops the | |
| 2250 | deploy with the old binary in place, but the new table is already | |
| 2251 | loaded and applies to the running runner too. If the check says the | |
| 2252 | rule blocks 127.0.0.1:22, remove the table with `ssh -p 2222 | |
| 2253 | root@gitbay.org nft delete table inet gitbay_runner` (the unit stays | |
| 2254 | active, so the runner is not stopped with it), fix the rule, and deploy | |
| 2255 | again. Runbook R3 runs this path on the scratch runner before merge. | |
| 2256 | ||
| 2257 | - [ ] **Step 7: Verify locally** | |
| 2258 | ||
| 2259 | Run: `sh -n deploy/runner-egress-check.sh && sh -n deploy/runner-podman-setup.sh && make -n deploy-runner HOST=example.test` | |
| 2260 | Expected: no syntax errors; the dry run lists the two new copies, the | |
| 2261 | `nft -c` / `reload-or-restart` block, the check, then the runner block. | |
| 2262 | ||
| 2263 | - [ ] **Step 8: Commit** | |
| 2264 | ||
| 2265 | ```bash | |
| 2266 | git add deploy/gitbay-runner-egress.nft deploy/gitbay-runner-egress.service deploy/runner-egress-check.sh deploy/gitbay-runner.override.conf deploy/runner-podman-setup.sh Makefile | |
| 2267 | git commit -S -m "runner host: builds reach only the forge's public ports on it | |
| 2268 | ||
| 2269 | Ref #260" | |
| 2270 | ``` | |
| 2271 | ||
| 2272 | ### Task 3.5: egress policy in the wiki, and the MR | |
| 2273 | ||
| 2274 | **Files:** | |
| 2275 | - Modify: `.gitbay/wiki/Threat-Model.org` ("The CI runner", after the *Images* bullet at line 169) | |
| 2276 | - Modify: `.gitbay/wiki/CI.org` (new section before "* The table", line 55) | |
| 2277 | - Modify: `.gitbay/wiki/Users.org:551-555` (the `GITBAY_SSH` sentence) | |
| 2278 | - Modify: `.gitbay/wiki/Admin.org` (after "It is idempotent.", line 668) | |
| 2279 | - Modify: `.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org:70` (Network row), `04-Trust-Boundaries.org:27` (TB7), `09-Controls.org:91` | |
| 1689 | 2280 | |
| 1690 | 2281 | - [ ] **Step 1: Threat-Model** |
| 1691 | 2282 | |
| @@ -1693,18 +2284,27 @@ Add a bullet after *Images are provisioned…*: | ||
| 1693 | 2284 | |
| 1694 | 2285 | ```org |
| 1695 | 2286 | - *What a build can reach.* Outbound internet, trusted or not: a fork's |
| 1696 | merge request to a Go repository has to fetch its modules. Not the | |
| 1697 | host's loopback: a runner that polls the daemon over loopback starts | |
| 1698 | its containers with pasta's gateway mapping off, so a build reaches | |
| 1699 | the host only at its public address, as anyone on the internet does, | |
| 1700 | and =GITBAY_SSH= names that address. That keeps the runner's source | |
| 1701 | address, =127.0.0.1=, one no build can connect from; the SSH auth | |
| 1702 | limiter counts failures per address, and a build sharing the runner's | |
| 1703 | could throttle its polling (krz/gitbay#260). The forge's public ports | |
| 1704 | — 22, 80, 443 and the operator's sshd — are reachable from a build | |
| 1705 | exactly as from the internet. Under =-isolation none= a build runs on | |
| 1706 | the host and shares its loopback; that mode is for instances where | |
| 1707 | every repository is trusted. | |
| 2287 | merge request to a Go repository has to fetch its modules. On the | |
| 2288 | runner's host, only the forge's public ports 22, 80 and 443, exactly | |
| 2289 | as anyone on the internet reaches them. Two layers keep it there. A | |
| 2290 | runner that polls the daemon over loopback starts its containers with | |
| 2291 | pasta's gateway mapping off, so a build does not reach the host's | |
| 2292 | loopback, and =GITBAY_SSH= names the public address; that keeps the | |
| 2293 | runner's source address, =127.0.0.1=, one no build connects from. And | |
| 2294 | an nftables table (=deploy/gitbay-runner-egress.nft=) rejects every | |
| 2295 | connection the runner's user makes to the host's own addresses except | |
| 2296 | =127.0.0.1:22=, DNS on loopback, and 22, 80 and 443 on the public | |
| 2297 | address: the operator's sshd on 2222 and anything bound to loopback | |
| 2298 | are closed to builds. Under rootless podman a build's connections are | |
| 2299 | made by pasta as the runner's user, so the table cannot tell a build | |
| 2300 | from its runner and leaves =127.0.0.1:22= open; the gateway mapping | |
| 2301 | is what closes it to builds. The runner does not start without the | |
| 2302 | table. The SSH auth limiter counts failures per source address and, | |
| 2303 | once an address is over the limit, refuses every key from it until | |
| 2304 | the window passes, the runner's included; with registration open or | |
| 2305 | by invite an unknown key never counts (krz/gitbay#260). Under | |
| 2306 | =-isolation none= a build runs on the host and shares its loopback; | |
| 2307 | the table still applies, since it runs as the same user. | |
| 1708 | 2308 | ``` |
| 1709 | 2309 | |
| 1710 | 2310 | - [ ] **Step 2: CI.org** |
| @@ -1714,11 +2314,12 @@ Add before `* The table`: | ||
| 1714 | 2314 | ```org |
| 1715 | 2315 | * What a build can reach |
| 1716 | 2316 | |
| 1717 | Builds have outbound internet access, trusted and untrusted alike, and | |
| 1718 | no access to the runner host's loopback when the runner polls the | |
| 1719 | daemon over it: the forge is reached at its public address, the one in | |
| 1720 | =GITBAY_SSH=. See the Threat-Model page, "What a build can reach", for | |
| 1721 | why (krz/gitbay#260). | |
| 2317 | Builds have outbound internet access, trusted and untrusted alike. On | |
| 2318 | the runner's host they reach only the forge's public ports 22, 80 and | |
| 2319 | 443: not the host's loopback, not the operator's sshd. The forge is | |
| 2320 | reached at its public address, the one in =GITBAY_SSH=. See the | |
| 2321 | Threat-Model page, "What a build can reach", for how and why | |
| 2322 | (krz/gitbay#260). | |
| 1722 | 2323 | ``` |
| 1723 | 2324 | |
| 1724 | 2325 | - [ ] **Step 3: Users.org** |
| @@ -1727,30 +2328,50 @@ Replace "(=git@gitbay.org= from a runner elsewhere; inside a container | ||
| 1727 | 2328 | on the server's own runner the host is at a private address the runner |
| 1728 | 2329 | fills in)" with "(=git@gitbay.org=, the instance's public address, from |
| 1729 | 2330 | a runner elsewhere and from a container on the server's own runner |
| 1730 | alike)". | |
| 2331 | alike; =git@host:port= on an instance whose ssh is not on 22, so use it | |
| 2332 | as =ssh://$GITBAY_SSH/owner/name.git= or =ssh ssh://$GITBAY_SSH …=, | |
| 2333 | which work in both forms)". | |
| 1731 | 2334 | |
| 1732 | - [ ] **Step 4: Architecture** | |
| 2335 | - [ ] **Step 4: Admin.org** | |
| 1733 | 2336 | |
| 1734 | `07-CI-and-Supply-Chain.org` Network row: | |
| 2337 | After "…verifies rootless podman actually runs as that user. It is | |
| 2338 | idempotent." add: | |
| 1735 | 2339 | |
| 1736 | 2340 | ```org |
| 1737 | | Network | pasta; outbound open; a loopback runner's builds run with =--no-map-gw= and reach the host only at its public address (=main.go=, #260) | | |
| 2341 | It also installs nftables. =make deploy-runner= ships | |
| 2342 | =deploy/gitbay-runner-egress.nft= to =/etc/gitbay-runner/egress.nft= | |
| 2343 | with =gitbay-runner-egress.service=, which loads it and which the | |
| 2344 | runner's unit requires; it checks the file with =nft -c=, reloads the | |
| 2345 | unit, and runs =deploy/runner-egress-check.sh= as =ci-runner= before | |
| 2346 | restarting the runner: =127.0.0.1:22= and the public 22 must answer, | |
| 2347 | 2222 must not. The table limits the runner's user to =127.0.0.1:22=, | |
| 2348 | DNS on loopback, and 22, 80 and 443 on the host's public address; the | |
| 2349 | Threat-Model page says why. A restart of =nftables.service= flushes it; | |
| 2350 | =systemctl reload gitbay-runner-egress= restores it. | |
| 1738 | 2351 | ``` |
| 1739 | 2352 | |
| 1740 | `04-Trust-Boundaries.org` TB7: replace "the network is open (#260)" | |
| 1741 | with "outbound is open and the host's loopback is not reachable | |
| 2353 | - [ ] **Step 5: Architecture** | |
| 2354 | ||
| 2355 | `07-CI-and-Supply-Chain.org:70` Network row: | |
| 2356 | ||
| 2357 | ```org | |
| 2358 | | Network | pasta; outbound open; a loopback runner's builds run with =--no-map-gw= (=main.go=); on the host only public 22/80/443 (=gitbay-runner-egress.nft=, #260) | | |
| 2359 | ``` | |
| 2360 | ||
| 2361 | `04-Trust-Boundaries.org:27` TB7: replace "the network is open (#260)" | |
| 2362 | with "outbound is open; on the host only the forge's public ports | |
| 1742 | 2363 | (#260)". `09-Controls.org:91`: |
| 1743 | 2364 | |
| 1744 | 2365 | ```org |
| 1745 | | Build network egress restricted | partial | host loopback closed to builds; outbound open by decision (#260) | | |
| 2366 | | Build network egress restricted | partial | host: loopback closed, public 22/80/443 only (=gitbay-runner-egress.nft=); internet outbound open by decision (#260) | | |
| 1746 | 2367 | ``` |
| 1747 | 2368 | |
| 1748 | 2369 | The Known-Gaps row for #260 and its "What can a build reach…" question |
| 1749 | stay until the runbook's R3 results are recorded. | |
| 2370 | stay until the runbook's R3 results are recorded on the CI page. | |
| 1750 | 2371 | |
| 1751 | - [ ] **Step 5: Verify, commit, MR** | |
| 2372 | - [ ] **Step 6: Verify, commit, MR** | |
| 1752 | 2373 | |
| 1753 | Run: `go build ./... && go vet ./... && go test ./cmd/gitbay-runner ./internal/control -count=1` | |
| 2374 | Run: `go build ./... && go vet ./... && go test ./cmd/gitbay-runner ./internal/control ./internal/sshd -count=1` | |
| 1754 | 2375 | Expected: PASS. |
| 1755 | 2376 | |
| 1756 | 2377 | ```bash |
| @@ -1759,7 +2380,7 @@ git commit -S -m "wiki: what a build can reach | ||
| 1759 | 2380 | |
| 1760 | 2381 | Ref #260" |
| 1761 | 2382 | git push -u origin runner-source-address |
| 1762 | gitbay mr create --source runner-source-address --target main --title "runner: keep builds off the runner's source address" | |
| 2383 | gitbay mr create --source runner-source-address --target main --title "runner: keep builds off the runner's source address and the host's other ports" | |
| 1763 | 2384 | ``` |
| 1764 | 2385 | |
| 1765 | 2386 | Before merging, run the runbook's R3 on the scratch repository. Merge |
| @@ -3031,33 +3652,11 @@ with `--strategy ff` and delete the branch both places. | ||
| 3031 | 3652 | pasta exposes the host at `169.254.1.2` as its `--map-host-loopback` |
| 3032 | 3653 | default; newer podman instead passes `--map-guest-addr 169.254.1.2`, |
| 3033 | 3654 | which maps to the host's public address. Which one bay1's podman |
| 3034 | does is not in the repository. Runbook R0 measures it before Part 3 | |
| 3035 | deploys; if `--no-map-gw` is refused or leaves `127.0.0.1` reachable, | |
| 3036 | stop and revisit Part 3 before merging. | |
| 3037 | 2. **Default image and tree reuse.** Tree reuse now keys on the job's | |
| 3038 | declared `image:`. A job that names none runs on the runner's | |
| 3039 | `-image`, which the server does not know; bumping `-image` | |
| 3040 | (`gitbay-ci:2` → `:3`) does not invalidate reuse for such jobs. | |
| 3041 | Covering it would need the runner to report the image it resolved | |
| 3042 | (and its digest) on `runner done`, and reuse to compare that. Not | |
| 3043 | planned; confirm that the declared image is enough. | |
| 3044 | 3. **Non-22 ssh ports.** `GITBAY_SSH` is `user@host` — hutch and orgo | |
| 3045 | build URLs as `ssh://$GITBAY_SSH/...` — so the claim's `ssh` carries | |
| 3046 | no port. An instance with `[ssh] port` other than 22 and a loopback | |
| 3047 | runner gives its builds a destination without the port. gitbay.org | |
| 3048 | is on 22. Should the field carry the port (and the builds' scripts | |
| 3049 | change), or is this left to such an instance's own ssh config? | |
| 3050 | 4. **Required contexts without require-checks.** Decided here as "the | |
| 3051 | list applies only while require-checks is on, and the command says | |
| 3052 | so". The alternative is that setting contexts turns the gate on. | |
| 3053 | Confirm. | |
| 3054 | 5. **Throttling test on gitbay.org.** Under open registration the | |
| 3055 | limiter never counts an unknown key's attempt (see Decisions), so | |
| 3056 | the issue's throttling test on bay1 is expected to show nothing. R3 | |
| 3057 | runs it as the issue asks and records that; a closed-registration | |
| 3058 | scratch daemon on bay1 would be needed to show the limiter itself | |
| 3059 | separating the two addresses. Is the source-address measurement | |
| 3060 | enough to close #260? | |
| 3655 | does is not in the repository. Runbook R3 step 0 measures it before | |
| 3656 | Part 3 deploys; if `--no-map-gw` is refused or leaves `127.0.0.1` | |
| 3657 | reachable, stop and revisit Part 3 before merging. The nftables | |
| 3658 | table does not cover this path: a build's connection to 127.0.0.1:22 | |
| 3659 | through pasta is ci-runner's, like the runner's own poll. | |
| 3061 | 3660 | |
| 3062 | 3661 | --- |
| 3063 | 3662 | |
| @@ -3140,20 +3739,31 @@ not poll with several ssh calls a tick. Operator ssh is | ||
| 3140 | 3739 | before runners, and that `<workdir>/home` can be deleted after the |
| 3141 | 3740 | runner upgrade. Nothing further in the wiki; Part 1's MR updated it. |
| 3142 | 3741 | |
| 3143 | ### R3. Part 3 (#260): pasta check, source addresses, throttling | |
| 3742 | ### R3. Part 3 (#260): pasta check, egress rule, measurement from a build | |
| 3144 | 3743 | |
| 3145 | 0. Before merging Part 3, on bay1 as the runner user: | |
| 3744 | No throttling test runs on production; Task 3.3's unit test covers the | |
| 3745 | limiter. What is measured here is what a build sees. | |
| 3746 | ||
| 3747 | 0. Before merging Part 3, on bay1: re-run the host setup (idempotent; | |
| 3748 | it now installs nftables), then check pasta as the runner user: | |
| 3146 | 3749 | |
| 3147 | 3750 | ```sh |
| 3148 | ssh -p 2222 root@gitbay.org "podman --version; pasta --version | head -1" | |
| 3751 | ssh -p 2222 root@gitbay.org 'sh -s' < deploy/runner-podman-setup.sh | |
| 3752 | ssh -p 2222 root@gitbay.org "podman --version; pasta --version | head -1; nft --version; hostname -I" | |
| 3149 | 3753 | ssh -p 2222 root@gitbay.org "su - ci-runner -s /bin/sh -c 'podman --cgroup-manager=cgroupfs run --rm --pull=never --network pasta:--no-map-gw --entrypoint sh localhost/gitbay-ci:2 -c \"getent hosts proxy.golang.org; timeout 5 bash -c \\\"exec 3<>/dev/tcp/gitbay.org/22\\\" && echo public-ok; cat /proc/net/route\"'" |
| 3150 | 3754 | ``` |
| 3151 | 3755 | |
| 3152 | Expected: `proxy.golang.org` resolves, `public-ok` prints. If podman | |
| 3153 | rejects the option, stop (open question 1). | |
| 3756 | Expected: `proxy.golang.org` resolves, `public-ok` prints, and | |
| 3757 | `hostname -I` lists the public IPv4 address first (the check script | |
| 3758 | takes the first). If podman rejects the option, stop (open question | |
| 3759 | 1). | |
| 3154 | 3760 | 1. `make deploy` from the Part 3 branch, the R1 drop-in, then |
| 3155 | `make deploy-runner`. | |
| 3156 | 2. On `cmc/ci-scratch` `main`, a probe job (keep the step under 4096 | |
| 3761 | `make deploy-runner`. Its output shows `==> loading the egress rule` | |
| 3762 | and then `egress for ci-runner: 127.0.0.1:22 and <public>:22 open, | |
| 3763 | 2222 refused` before the runner restarts. If the check fails, follow | |
| 3764 | Task 3.4 Step 6 (remove the table, fix, deploy again) before | |
| 3765 | anything else: the running runner is under the new table. | |
| 3766 | 2. On `cmc/ci-scratch` `main`, a probe job (keep each step under 4096 | |
| 3157 | 3767 | bytes): |
| 3158 | 3768 | |
| 3159 | 3769 | ```yaml |
| @@ -3162,12 +3772,10 @@ not poll with several ssh calls a tick. Operator ssh is | ||
| 3162 | 3772 | steps: |
| 3163 | 3773 | - echo "GITBAY_SSH=$GITBAY_SSH" |
| 3164 | 3774 | - | |
| 3165 | gw=$(awk '$2=="00000000"{print $3}' /proc/net/route | head -1) | |
| 3166 | echo "gateway (hex, little-endian): $gw" | |
| 3167 | for a in 127.0.0.1 169.254.1.2; do timeout 5 bash -c "exec 3<>/dev/tcp/$a/22" && echo "reach $a:22" || echo "no $a:22"; done | |
| 3168 | - | | |
| 3169 | ssh-keygen -q -t ed25519 -N '' -f /tmp/k | |
| 3170 | for i in $(seq 1 30); do ssh -F /dev/null -i /tmp/k -o StrictHostKeyChecking=no -o BatchMode=yes -o ConnectTimeout=5 "$GITBAY_SSH" whoami; done | |
| 3775 | host=${GITBAY_SSH#*@}; host=${host%:*} | |
| 3776 | for t in 127.0.0.1:22 127.0.0.1:2222 169.254.1.2:22 $host:22 $host:80 $host:443 $host:2222 proxy.golang.org:443; do | |
| 3777 | timeout 5 bash -c "exec 3<>/dev/tcp/${t%:*}/${t##*:}" 2>/dev/null && echo "open $t" || echo "closed $t" | |
| 3778 | done | |
| 3171 | 3779 | - bash -c 'exec 3<>/dev/tcp/${GITBAY_SSH#*@}/22; sleep 90' |
| 3172 | 3780 | ``` |
| 3173 | 3781 | |
| @@ -3177,26 +3785,29 @@ not poll with several ssh calls a tick. Operator ssh is | ||
| 3177 | 3785 | ssh -p 2222 root@gitbay.org "ss -tn state established '( sport = :22 )'" |
| 3178 | 3786 | ``` |
| 3179 | 3787 | |
| 3180 | Record the peer address of the build's connection and of the | |
| 3181 | runner's `runner log` session (`127.0.0.1`). Expected: the build's | |
| 3182 | peer is the host's public address, never `127.0.0.1`. | |
| 3183 | 4. `gitbay audit --json` (admin): look for `auth.throttled` rows since | |
| 3184 | the build started. Expected: none, since registration is open (see | |
| 3185 | Decisions); the runner kept claiming (`gitbay admin runners` shows a | |
| 3186 | recent poll). | |
| 3187 | 5. Expected build log: `GITBAY_SSH=git@gitbay.org`, `no 127.0.0.1:22`, | |
| 3188 | the `169.254.1.2` result as measured, the `whoami` loop answering | |
| 3189 | from an anonymous session. | |
| 3788 | Note the peer address of the build's connection and of the runner's | |
| 3789 | `runner log` session. Expected: the runner's is `127.0.0.1`, the | |
| 3790 | build's is the host's public address, never `127.0.0.1`. | |
| 3791 | 4. Expected build log: `GITBAY_SSH=git@gitbay.org`; `closed | |
| 3792 | 127.0.0.1:22`, `closed 127.0.0.1:2222`; `169.254.1.2:22` as | |
| 3793 | measured (open only if podman maps that address to the public one); | |
| 3794 | `open gitbay.org:22`, `:80`, `:443`; `closed gitbay.org:2222`; | |
| 3795 | `open proxy.golang.org:443`. | |
| 3796 | 5. The runner kept polling: `gitbay admin runners` shows a recent poll | |
| 3797 | for the bay1 key, and the probe build finished with its log. | |
| 3190 | 3798 | 6. Remove the R1 drop-in. Trigger one real trusted job that talks back |
| 3191 | 3799 | (`gitbay build trigger krz/orgo <its release or pages job>` only if |
| 3192 | 3800 | one is due; otherwise wait for the next hutch/orgo scheduled job) and |
| 3193 | 3801 | check it reached `git@gitbay.org`. |
| 3194 | 7. Record in the wiki, one commit on a branch `wiki-260-results` | |
| 3195 | (`Closes #260`): the measured source addresses and the pasta/podman | |
| 3196 | versions under Threat-Model "What a build can reach"; the Known-Gaps | |
| 3197 | question "What can a build reach on the host's network?" answered | |
| 3198 | with the date and result, and the `#260` row removed; `09-Controls` | |
| 3199 | row status left `partial` (outbound is open by decision). | |
| 3802 | 7. Record the result, one commit on a branch `wiki-260-results` with | |
| 3803 | `Closes #260`: in CI.org under "What a build can reach", a dated | |
| 3804 | paragraph with the podman, pasta and nft versions, the source | |
| 3805 | address the forge saw for a build and for the runner (step 3), and | |
| 3806 | the reachability list from step 4. In | |
| 3807 | `Architecture/10-Known-Gaps.org`, remove the `#260` row and answer | |
| 3808 | "What can a build reach on the host's network?" with the date and a | |
| 3809 | pointer to the CI page. `09-Controls` stays `partial` (outbound is | |
| 3810 | open by decision). MR, `--strategy ff`, delete the branch. | |
| 3200 | 3811 | |
| 3201 | 3812 | ### R4. Part 4 (#266): validate the failure report |
| 3202 | 3813 | |
| @@ -3229,11 +3840,16 @@ not poll with several ssh calls a tick. Operator ssh is | ||
| 3229 | 3840 | |
| 3230 | 3841 | - **Coverage.** #255: explicit trust (1.1), disposable untrusted home |
| 3231 | 3842 | and trusted-only caches (1.2), the discard (R2.7), wiki (1.3). #258: |
| 3232 | `ci/` refused (2.1), reuse by trust and image (2.2), required contexts | |
| 3233 | with missing as pending in `MergeGates` (2.3), wiki (2.4). #260: code | |
| 3234 | (3.1, 3.2), egress policy in Threat-Model and CI (3.3), throttling | |
| 3235 | test and source-address measurement in the runbook (R3), with the | |
| 3236 | scratch-repository rule (R1). #266: runner names the step (4.3), stored | |
| 3843 | `ci/` refused (2.1), reuse by trust and declared image, the default | |
| 3844 | image's limit documented (2.2, 2.4), required contexts turning the | |
| 3845 | gate on, shown by `settings show` and the web page, missing as | |
| 3846 | pending in `MergeGates` (2.3), wiki (2.4). #260: public destination | |
| 3847 | with the port off 22 (3.1), builds off loopback (3.2), the limiter's | |
| 3848 | lockout as a unit test instead of a production test (3.3), the host | |
| 3849 | egress table with its unit, check and deploy wiring (3.4), egress | |
| 3850 | policy in Threat-Model, CI and Admin (3.5), the source address and | |
| 3851 | reachability measured from a scratch build and recorded on the CI | |
| 3852 | page (R3), with the scratch-repository rule (R1). #266: runner names the step (4.3), stored | |
| 3237 | 3853 | step and duration (4.1; duration derived), `build show` (4.4), |
| 3238 | 3854 | `build log --step`/`--tail` (4.4), web `<details>` per step with the |
| 3239 | 3855 | failed one open, `id="failed"`, "Jump to failure", duration beside |
| @@ -3248,3 +3864,7 @@ not poll with several ssh calls a tick. Operator ssh is | ||
| 3248 | 3864 | `LogSection` (4.4) are used by `logSteps` (4.5). `SuccessBuildForTree` |
| 3249 | 3865 | gains `image` in 2.2 and its one caller changes there. |
| 3250 | 3866 | `GatesOut.ChecksMissing` (2.3) is rendered in `mr.html` (2.3). |
| 3867 | `publicSSH` (3.1) fills the claim's `ssh`, read as `job.SSH` (3.2). | |
| 3868 | The table name `inet gitbay_runner` and the path | |
| 3869 | `/etc/gitbay-runner/egress.nft` match across the rule, the unit, the | |
| 3870 | check script and the Makefile (3.4). | |
docs/plans/2026-09-27-cli-ux.md +807 −112
| @@ -19,8 +19,19 @@ statuses" (`internal/httpd/builds.go`); both move into | ||
| 19 | 19 | a `Ctx.Term` set from the CLI's `--term=<cols>[,color]`; `c.program()` |
| 20 | 20 | already picks `"gitbay"` or `"ssh git@<host>"` from it for the `--help` |
| 21 | 21 | path, but `c.usage()`/`c.usageWith()` (the wrong-argument path) do not |
| 22 | yet call it, and the CLI's `auth` grouping is not a registry path at | |
| 23 | all, so its `--help` falls back to cobra's own listing. | |
| 22 | yet call it. Separately, eighteen CLI commands resolve to a server path | |
| 23 | that differs from what cobra's tree spells (`cmd/gitbay/main.go`'s | |
| 24 | `serverPath` annotations): everything under `auth` except a few whose | |
| 25 | noun already matches the registry (`auth email ...` -> `email ...`, | |
| 26 | `auth export` -> `account export`, `auth keys ...` -> `keys ...`, `auth | |
| 27 | pgp ...` -> `pgp ...`, `auth token ...` -> `token ...`, `auth whoami` -> | |
| 28 | `whoami`), and `repo topics list` -> `repo topics`. Printing the | |
| 29 | registered path verbatim for one of these gives a command that does not | |
| 30 | exist — `gitbay keys remove <fp>` is `unknown command "keys"`. #267 | |
| 31 | decided the CLI sends its own invoking path and the server prints that | |
| 32 | instead of the registered one wherever they differ; the CLI's `auth` | |
| 33 | grouping, still not a registry path at all, gets the same treatment for | |
| 34 | the several registry prefixes it gathers. | |
| 24 | 35 | |
| 25 | 36 | **Tech stack:** Go, `golang.org/x/crypto/ssh`, cobra. |
| 26 | 37 | |
| @@ -697,52 +708,177 @@ Wait for CI, merge with `--strategy ff`, delete the branch both places. | ||
| 697 | 708 | |
| 698 | 709 | # Part 2: help and usage print the form the caller typed (branch `cli-ux-help`, closes #267) |
| 699 | 710 | |
| 700 | ### Task 2.1: `c.usage()`/`c.usageWith()` print the program form | |
| 701 | ||
| 702 | `c.usage()` prints the bare registered usage (`usage: keys remove | |
| 703 | <fingerprint>`), with neither the `gitbay` nor the `ssh git@host` | |
| 704 | prefix `c.program()`/`helpVerb` already use for `--help`. Give both the | |
| 705 | same prefix, and mark a leading `<owner/name>` optional when the caller | |
| 706 | is the CLI at a terminal — the CLI fills it in from the clone's origin | |
| 707 | remote (`cmd/gitbay/ssh.go`'s `withRepo`); stock ssh never does. | |
| 711 | ### Task 2.1: the CLI sends the path it typed; usage and help print it | |
| 712 | ||
| 713 | `c.usage()` prints the bare *registered* usage (`usage: keys remove | |
| 714 | <fingerprint>`), with neither the `gitbay` nor the `ssh git@host` prefix | |
| 715 | `c.program()`/`helpVerb` already use for `--help` — and for eighteen | |
| 716 | commands (the ones listed in Architecture above) the registered path is | |
| 717 | not even something a caller can type: `gitbay keys remove <fp>` is | |
| 718 | `unknown command "keys"`, because the real command is `gitbay auth keys | |
| 719 | remove <fp>`. Fix both: give every usage/help line the program prefix, | |
| 720 | mark a leading `<owner/name>` optional at a terminal (the CLI fills it | |
| 721 | in from the clone's origin remote, `cmd/gitbay/ssh.go`'s `withRepo`; | |
| 722 | stock ssh never does), and have the CLI tell the server what it was | |
| 723 | actually typed as, so the server can print that instead of the | |
| 724 | registered path wherever the two differ. | |
| 725 | ||
| 726 | The CLI already tells the server one thing about the calling session | |
| 727 | this way: `--term=<cols>[,color]`, prepended to the command line by | |
| 728 | `runSSHPaged` and stripped off `argv[0]` by `Dispatch` before `Lookup` | |
| 729 | (`internal/control/control.go`). A second, sibling prefix, `--path=<cli | |
| 730 | path>`, carries what cobra resolved the call to | |
| 731 | (`cobra.Command.CommandPath()`, minus the leading `gitbay `). It travels | |
| 732 | as its own `--path=` argument rather than a new field packed into the | |
| 733 | `--term=` value: that value's `cols[,color]` grammar has no room for a | |
| 734 | string containing spaces (a CLI path always does), and a second prefix | |
| 735 | is one more `strings.CutPrefix` in the same loop, not a new mini-parser. | |
| 736 | Only the gitbay CLI ever sends it — stock ssh has no notion of a "path | |
| 737 | it resolved to" that differs from what was typed, because what was typed | |
| 738 | *is* the dispatch path — so `Dispatch` never invents one, and the field | |
| 739 | stays empty for the web and the API exactly like `Term` does. | |
| 708 | 740 | |
| 709 | 741 | **Files:** |
| 710 | - Modify: `internal/control/help.go` (`cliUsage`, new) | |
| 711 | - Modify: `internal/control/control.go` (`usage`, `usageWith`) | |
| 712 | - Modify: `internal/control/control_test.go` (`TestArgumentRefusalsNameTheUsage`) | |
| 742 | - Modify: `internal/control/control.go` (`Ctx.CLIPath`, `Dispatch`, `usage`, `usageWith`) | |
| 743 | - Modify: `internal/control/help.go` (`cliUsage`, `shownAs`, `cmdUsage`, `helpVerb`, `helpNoun`) | |
| 744 | - Modify: `internal/control/control_test.go` (`TestArgumentRefusalsNameTheUsage`, new `TestPathArgument`) | |
| 713 | 745 | - Test: `internal/control/help_test.go` |
| 746 | - Modify: `cmd/gitbay/ssh.go` (`cliPathOf`, `withCLIPath`, new) | |
| 747 | - Modify: `cmd/gitbay/main.go` (`pass`, `runServerHelp`, `runPass`, `group`, `serverHelp`) | |
| 748 | - Test: `cmd/gitbay/term_test.go` (the two new pure helpers) | |
| 749 | - Test: `cmd/gitbay/serverpath_test.go` (create) | |
| 750 | - Test: `e2e/cliusage_test.go` (create) | |
| 714 | 751 | |
| 715 | 752 | **Interfaces:** |
| 716 | - Produces: `func cliUsage(usage string) string` — marks the first | |
| 717 | `<owner/name>` optional; `func (c *Ctx) cmdUsage() string` — the | |
| 718 | program-prefixed, owner/name-optional-at-a-terminal usage line. | |
| 753 | - Produces: `Ctx.CLIPath string`; `func cliUsage(usage string) string`; | |
| 754 | `func (c *Ctx) shownAs(registered, full string) string`; `func (c | |
| 755 | *Ctx) cmdUsage() string`; `func cliPathOf(cmd *cobra.Command) string`; | |
| 756 | `func withCLIPath(cliPath string, argv []string) []string`. | |
| 757 | - Consumes (Task 2.2, 2.3): `Ctx.CLIPath`, `shownAs`, `withCLIPath`, `cliPathOf`. | |
| 719 | 758 | |
| 720 | - [ ] **Step 1: Write the failing test** | |
| 759 | - [ ] **Step 1: Write the failing test for the transport** | |
| 760 | ||
| 761 | ```go | |
| 762 | // TestPathArgument: --path= is read only as a leading argument (in | |
| 763 | // either order with --term=), and never over HTTP — mirrors | |
| 764 | // TestTermArgument, the mechanism it rides alongside. | |
| 765 | func TestPathArgument(t *testing.T) { | |
| 766 | cases := []struct { | |
| 767 | name string | |
| 768 | viaAPI bool | |
| 769 | argv []string | |
| 770 | want string | |
| 771 | wantArgv []string | |
| 772 | }{ | |
| 773 | {"leading", false, []string{"--path=auth keys remove", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, | |
| 774 | {"after term", false, []string{"--term=80", "--path=auth keys remove", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, | |
| 775 | {"before term", false, []string{"--path=auth keys remove", "--term=80", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, | |
| 776 | {"over HTTP", true, []string{"--path=auth keys remove", "keys", "remove", "abc"}, "", []string{"abc"}}, | |
| 777 | } | |
| 778 | for _, tc := range cases { | |
| 779 | c := &Ctx{Scope: "git", ViaAPI: tc.viaAPI, Stdout: io.Discard, Stderr: io.Discard} | |
| 780 | Dispatch(c, tc.argv) | |
| 781 | if c.CLIPath != tc.want { | |
| 782 | t.Errorf("%s: CLIPath %q, want %q", tc.name, c.CLIPath, tc.want) | |
| 783 | } | |
| 784 | if !slices.Equal(c.Argv, tc.wantArgv) { | |
| 785 | t.Errorf("%s: Argv %q, want %q", tc.name, c.Argv, tc.wantArgv) | |
| 786 | } | |
| 787 | } | |
| 788 | } | |
| 789 | ``` | |
| 790 | ||
| 791 | - [ ] **Step 2: Run and see it fail** | |
| 792 | ||
| 793 | Run: `go test ./internal/control -run TestPathArgument -count=1` | |
| 794 | Expected: FAIL to compile (`c.CLIPath undefined`). | |
| 795 | ||
| 796 | - [ ] **Step 3: Add the field and route it through `Dispatch`** | |
| 797 | ||
| 798 | In `internal/control/control.go`, `Ctx` gains a field next to `Term`: | |
| 799 | ||
| 800 | ```go | |
| 801 | // CLIPath is the path the gitbay CLI actually resolved this call to | |
| 802 | // (cobra.Command.CommandPath(), from a leading --path=), when it | |
| 803 | // differs from the registered path being dispatched (#267) — auth's | |
| 804 | // several groupings and repo topics list, today. Empty for stock | |
| 805 | // ssh, the web and the API: nothing but the gitbay CLI sends one. | |
| 806 | CLIPath string | |
| 807 | ``` | |
| 808 | ||
| 809 | `Dispatch` strips both leading pseudo-flags in a loop, in whichever | |
| 810 | order the caller sent them, replacing the single `--term=` check: | |
| 811 | ||
| 812 | ```go | |
| 813 | // A leading --term=<v> selects terminal output for this session, the | |
| 814 | // same as GITBAY_TERM; a leading --path=<v> carries the CLI's own | |
| 815 | // invoking path when it differs from the one being dispatched | |
| 816 | // (#267). Both come off before Lookup, in whichever order the | |
| 817 | // caller sent them: Lookup matches argv against a command's Path, | |
| 818 | // and either prefix in front would never match one. Over HTTP both | |
| 819 | // are dropped unread: the web and the API render no terminal and | |
| 820 | // have no CLI path of their own. | |
| 821 | for len(argv) > 0 { | |
| 822 | if v, ok := strings.CutPrefix(argv[0], "--term="); ok { | |
| 823 | if !c.ViaAPI { | |
| 824 | c.Term = ParseTerm(v) | |
| 825 | } | |
| 826 | argv = argv[1:] | |
| 827 | continue | |
| 828 | } | |
| 829 | if v, ok := strings.CutPrefix(argv[0], "--path="); ok { | |
| 830 | if !c.ViaAPI { | |
| 831 | c.CLIPath = v | |
| 832 | } | |
| 833 | argv = argv[1:] | |
| 834 | continue | |
| 835 | } | |
| 836 | break | |
| 837 | } | |
| 838 | if len(argv) == 0 { | |
| 839 | return c.fail(protocol.ExitUsage, "no command given; try: ssh <host> help") | |
| 840 | } | |
| 841 | ``` | |
| 842 | ||
| 843 | - [ ] **Step 4: Run** | |
| 844 | ||
| 845 | Run: `go test ./internal/control -run "TestPathArgument|TestTermArgument" -count=1` | |
| 846 | Expected: PASS. | |
| 847 | ||
| 848 | - [ ] **Step 5: Write the failing test for rendering** | |
| 721 | 849 | |
| 722 | 850 | ```go |
| 723 | 851 | func TestCmdUsagePrefixesTheProgram(t *testing.T) { |
| 724 | c := &Ctx{Cmd: Command{Usage: "keys remove <fingerprint>"}, Cfg: config.Config{Server: config.Server{SiteURL: "https://forge.test"}}} | |
| 852 | c := &Ctx{Cmd: Command{Path: []string{"keys", "remove"}, Usage: "keys remove <fingerprint>"}, Cfg: config.Config{Server: config.Server{SiteURL: "https://forge.test"}}} | |
| 725 | 853 | if got := c.cmdUsage(); got != "ssh git@forge.test keys remove <fingerprint>" { |
| 726 | 854 | t.Errorf("ssh form: %q", got) |
| 727 | 855 | } |
| 728 | 856 | c.Term = Term{Cols: 100} |
| 729 | 857 | if got := c.cmdUsage(); got != "gitbay keys remove <fingerprint>" { |
| 730 | t.Errorf("cli form: %q", got) | |
| 858 | t.Errorf("cli form, no CLIPath sent: %q", got) | |
| 859 | } | |
| 860 | c.CLIPath = "auth keys remove" | |
| 861 | if got := c.cmdUsage(); got != "gitbay auth keys remove <fingerprint>" { | |
| 862 | t.Errorf("cli form, mismatched registered path: %q", got) | |
| 731 | 863 | } |
| 732 | 864 | |
| 733 | c2 := &Ctx{Cmd: Command{Usage: "repo tree <owner/name> [<path>] [--ref <ref>]"}, Term: Term{Cols: 100}} | |
| 865 | c2 := &Ctx{Cmd: Command{Path: []string{"repo", "tree"}, Usage: "repo tree <owner/name> [<path>] [--ref <ref>]"}, Term: Term{Cols: 100}} | |
| 734 | 866 | if got := c2.cmdUsage(); got != "gitbay repo tree [<owner/name>] [<path>] [--ref <ref>]" { |
| 735 | 867 | t.Errorf("optional owner/name: %q", got) |
| 736 | 868 | } |
| 869 | c2.CLIPath = "repo tree" // matches the registered path: a no-op | |
| 870 | if got := c2.cmdUsage(); got != "gitbay repo tree [<owner/name>] [<path>] [--ref <ref>]" { | |
| 871 | t.Errorf("matching CLIPath changes nothing: %q", got) | |
| 872 | } | |
| 737 | 873 | } |
| 738 | 874 | ``` |
| 739 | 875 | |
| 740 | - [ ] **Step 2: Run and see it fail** | |
| 876 | - [ ] **Step 6: Run and see it fail** | |
| 741 | 877 | |
| 742 | 878 | Run: `go test ./internal/control -run TestCmdUsagePrefixesTheProgram -count=1` |
| 743 | 879 | Expected: FAIL to compile (`c.cmdUsage undefined`). |
| 744 | 880 | |
| 745 | - [ ] **Step 3: Implement `cliUsage` and `cmdUsage` in `internal/control/help.go`** | |
| 881 | - [ ] **Step 7: Implement `cliUsage`, `shownAs` and `cmdUsage` in `internal/control/help.go`** | |
| 746 | 882 | |
| 747 | 883 | ```go |
| 748 | 884 | // cliUsage marks a leading <owner/name> optional in a CLI-rendered usage |
| @@ -753,13 +889,30 @@ func cliUsage(usage string) string { | ||
| 753 | 889 | return strings.Replace(usage, "<owner/name>", "[<owner/name>]", 1) |
| 754 | 890 | } |
| 755 | 891 | |
| 756 | // cmdUsage is the registered usage as this call should see it: the | |
| 757 | // gitbay form with <owner/name> optional at a terminal, the ssh form | |
| 758 | // otherwise. Every usage message — the --help path and a wrong-argument | |
| 759 | // refusal alike — goes through this, so a caller never sees the bare | |
| 760 | // registered path with no program in front of it. | |
| 892 | // shownAs returns how a registered path should print to this caller: | |
| 893 | // the CLI path it sent (Ctx.CLIPath) standing in for the leading | |
| 894 | // portion that corresponds to registered, with full's remainder kept | |
| 895 | // as-is; or full unchanged for stock ssh, the API, or a caller whose | |
| 896 | // CLI path already agrees with the registered one. registered must be | |
| 897 | // a genuine leading substring of full (a command's own registered path | |
| 898 | // always is, against its own Usage or a sibling's full path). | |
| 899 | func (c *Ctx) shownAs(registered, full string) string { | |
| 900 | if c.CLIPath == "" || c.CLIPath == registered { | |
| 901 | return full | |
| 902 | } | |
| 903 | return c.CLIPath + strings.TrimPrefix(full, registered) | |
| 904 | } | |
| 905 | ||
| 906 | // cmdUsage is the registered usage as this call should see it: the CLI | |
| 907 | // path this session actually typed when it differs from the registered | |
| 908 | // one (#267), the gitbay form otherwise, the ssh form when there is no | |
| 909 | // terminal — with a leading <owner/name> marked optional at a terminal. | |
| 910 | // Every usage message — the --help path and a wrong-argument refusal | |
| 911 | // alike — goes through this, so a caller never sees a command it | |
| 912 | // cannot actually run. | |
| 761 | 913 | func (c *Ctx) cmdUsage() string { |
| 762 | shape := c.Cmd.Usage | |
| 914 | registered := joinPath(c.Cmd.Path) | |
| 915 | shape := c.shownAs(registered, c.Cmd.Usage) | |
| 763 | 916 | if c.Term.Cols > 0 { |
| 764 | 917 | shape = cliUsage(shape) |
| 765 | 918 | } |
| @@ -767,12 +920,12 @@ func (c *Ctx) cmdUsage() string { | ||
| 767 | 920 | } |
| 768 | 921 | ``` |
| 769 | 922 | |
| 770 | - [ ] **Step 4: Run** | |
| 923 | - [ ] **Step 8: Run** | |
| 771 | 924 | |
| 772 | 925 | Run: `go test ./internal/control -run TestCmdUsagePrefixesTheProgram -count=1` |
| 773 | 926 | Expected: PASS. |
| 774 | 927 | |
| 775 | - [ ] **Step 5: Route `usage`/`usageWith` through it** | |
| 928 | - [ ] **Step 9: Route `usage`/`usageWith` through it** | |
| 776 | 929 | |
| 777 | 930 | In `internal/control/control.go`: |
| 778 | 931 | |
| @@ -790,42 +943,137 @@ func (c *Ctx) usageWith(msg string) int { | ||
| 790 | 943 | } |
| 791 | 944 | ``` |
| 792 | 945 | |
| 793 | Also use it in `helpVerb` (`internal/control/help.go`), which today | |
| 794 | recomputes the same "cut at ` [--`" shape independently: | |
| 946 | - [ ] **Step 10: `helpVerb` gets the same treatment** | |
| 947 | ||
| 948 | `helpVerb` (`internal/control/help.go`) recomputes a "cut at ` [--`" | |
| 949 | shape independently, and lists sibling commands under SEE ALSO by their | |
| 950 | full registered path. Both go through `shownAs` now: | |
| 795 | 951 | |
| 796 | 952 | ```go |
| 797 | shape := cmd.Usage | |
| 953 | func (c *Ctx) helpVerb(w io.Writer, cmd Command, below []Command) { | |
| 954 | fmt.Fprintln(w, cmd.Summary) | |
| 955 | fmt.Fprintln(w) | |
| 956 | c.heading(w, "USAGE") | |
| 957 | // Cutting at the first optional flag drops the rest of the usage | |
| 958 | // syntax behind "[flags]" — safe only for what is actually optional. | |
| 959 | // A required flag (repo delete --yes) or an alternative | |
| 960 | // (notifications read <id>... | --all) has no " [--" to cut at, so | |
| 961 | // the usage prints whole. | |
| 962 | registered := joinPath(cmd.Path) | |
| 963 | shape := c.shownAs(registered, cmd.Usage) | |
| 964 | if c.Term.Cols > 0 { | |
| 965 | shape = cliUsage(shape) | |
| 966 | } | |
| 798 | 967 | if i := strings.Index(shape, " [--"); i >= 0 { |
| 799 | 968 | shape = shape[:i] + " [flags]" |
| 800 | 969 | } |
| 801 | 970 | fmt.Fprintf(w, " %s %s\n", c.program(), shape) |
| 971 | fmt.Fprintln(w) | |
| 972 | c.heading(w, "FLAGS") | |
| 973 | rows := make([][2]string, 0, len(cmd.Flags)+1) | |
| 974 | for _, f := range cmd.Flags { | |
| 975 | name := f.Name | |
| 976 | if f.Arg != "" { | |
| 977 | name += " " + f.Arg | |
| 978 | } | |
| 979 | desc := f.Desc | |
| 980 | if f.Default != "" { | |
| 981 | desc += " (default " + f.Default + ")" | |
| 982 | } | |
| 983 | rows = append(rows, [2]string{name, desc}) | |
| 984 | } | |
| 985 | rows = append(rows, [2]string{"--json", "machine-readable output"}) | |
| 986 | wide := 0 | |
| 987 | for _, r := range rows { | |
| 988 | wide = max(wide, cells(r[0])) | |
| 989 | } | |
| 990 | for _, r := range rows { | |
| 991 | c.wrapLine(w, " "+pad(r[0], wide)+" ", r[1]) | |
| 992 | } | |
| 993 | if len(cmd.Examples) > 0 { | |
| 994 | fmt.Fprintln(w) | |
| 995 | c.heading(w, "EXAMPLES") | |
| 996 | for _, ex := range cmd.Examples { | |
| 997 | c.wrapLine(w, " "+c.program()+" ", ex) | |
| 998 | } | |
| 999 | } | |
| 1000 | if len(below) > 0 { | |
| 1001 | fmt.Fprintln(w) | |
| 1002 | c.heading(w, "SEE ALSO") | |
| 1003 | for _, b := range below { | |
| 1004 | fmt.Fprintf(w, " %s %s\n", c.program(), c.shownAs(registered, joinPath(b.Path))) | |
| 1005 | } | |
| 1006 | } | |
| 1007 | } | |
| 802 | 1008 | ``` |
| 803 | 1009 | |
| 804 | stays as its own thing (it additionally collapses everything from the | |
| 805 | first optional flag into `[flags]`, which `cmdUsage` does not do), but | |
| 806 | its `c.program()` + owner/name handling should not fork from | |
| 807 | `cmdUsage`'s: replace the `c.program()` call with `cliUsage` applied the | |
| 808 | same way: | |
| 1010 | (Only the `USAGE` and `SEE ALSO` lines change; `FLAGS`/`EXAMPLES` are | |
| 1011 | reproduced above unchanged, for the diff to apply against the current | |
| 1012 | file — do not re-type them from scratch.) | |
| 1013 | ||
| 1014 | - [ ] **Step 11: `helpNoun` gets the same treatment** | |
| 1015 | ||
| 1016 | `helpNoun` prints `{program} {prefix} <verb> ...` and, per command, the | |
| 1017 | verb relative to `prefix`; the prefix itself needs the same swap (Task | |
| 1018 | 2.3 also gives it an `override` map, threaded through here empty for | |
| 1019 | every noun but the CLI-only `auth` alias): | |
| 809 | 1020 | |
| 810 | 1021 | ```go |
| 811 | shape := cmd.Usage | |
| 812 | if c.Term.Cols > 0 { | |
| 813 | shape = cliUsage(shape) | |
| 814 | } | |
| 815 | if i := strings.Index(shape, " [--"); i >= 0 { | |
| 816 | shape = shape[:i] + " [flags]" | |
| 1022 | func (c *Ctx) helpNoun(w io.Writer, prefix string, cmds []Command, override map[string]string) { | |
| 1023 | head := nounSummaries[strings.Fields(prefix)[0]] | |
| 1024 | fmt.Fprintln(w, head) | |
| 1025 | fmt.Fprintln(w) | |
| 1026 | c.heading(w, "USAGE") | |
| 1027 | display := c.shownAs(prefix, prefix) | |
| 1028 | fmt.Fprintf(w, " %s %s <verb> ...\n", c.program(), display) | |
| 1029 | rowText := func(cmd Command) string { | |
| 1030 | full := joinPath(cmd.Path) | |
| 1031 | if d, ok := override[full]; ok { | |
| 1032 | return d | |
| 1033 | } | |
| 1034 | return strings.TrimPrefix(full, prefix+" ") | |
| 1035 | } | |
| 1036 | wide := 0 | |
| 1037 | for _, cmd := range cmds { | |
| 1038 | wide = max(wide, cells(rowText(cmd))) | |
| 1039 | } | |
| 1040 | for _, section := range []struct { | |
| 1041 | title string | |
| 1042 | read bool | |
| 1043 | }{{"READ", true}, {"WRITE", false}} { | |
| 1044 | first := true | |
| 1045 | for _, cmd := range cmds { | |
| 1046 | if cmd.ReadOnly != section.read { | |
| 1047 | continue | |
| 1048 | } | |
| 1049 | if first { | |
| 1050 | fmt.Fprintln(w) | |
| 1051 | c.heading(w, section.title) | |
| 1052 | first = false | |
| 1053 | } | |
| 1054 | fmt.Fprintf(w, " %s %s\n", pad(rowText(cmd), wide), cmd.Summary) | |
| 1055 | } | |
| 817 | 1056 | } |
| 818 | fmt.Fprintf(w, " %s %s\n", c.program(), shape) | |
| 1057 | fmt.Fprintln(w) | |
| 1058 | fmt.Fprintf(w, "%s %s <verb> --help for flags.\n", c.program(), display) | |
| 1059 | } | |
| 819 | 1060 | ``` |
| 820 | 1061 | |
| 821 | - [ ] **Step 6: Update the test this changes** | |
| 1062 | `runHelp`'s one call site becomes `c.helpNoun(w, prefix, matched, nil)` | |
| 1063 | for now; Task 2.3 gives it a real map for the `auth` alias. A `nil` | |
| 1064 | map's zero value behaves like an empty one — `override[full]` on a | |
| 1065 | `nil` map is always `"", false` — so every other noun is unaffected. | |
| 1066 | ||
| 1067 | - [ ] **Step 12: Update the test `usage`/`usageWith` changes** | |
| 822 | 1068 | |
| 823 | 1069 | `TestArgumentRefusalsNameTheUsage` in `internal/control/control_test.go` |
| 824 | 1070 | asserts `errOut.String()` contains the bare `"usage: " + |
| 825 | 1071 | strings.Join(argv, " ")`; with no `Cfg.Server.SiteURL` and no `Term` set |
| 826 | 1072 | on its `Ctx`, the message now reads `usage: ssh git@ build show` (an |
| 827 | empty host — `hostOf("")` returns `""`). Set a `SiteURL` on the test's | |
| 828 | `Ctx` and assert the ssh-prefixed form: | |
| 1073 | empty host — `hostOf("")` returns `""`). None of these four commands is | |
| 1074 | one of the eighteen with a mismatched CLI path, and the test sends no | |
| 1075 | `--path=`, so `c.CLIPath` stays empty throughout — set a `SiteURL` and | |
| 1076 | assert the plain ssh-prefixed registered form: | |
| 829 | 1077 | |
| 830 | 1078 | ```go |
| 831 | 1079 | func TestArgumentRefusalsNameTheUsage(t *testing.T) { |
| @@ -848,18 +1096,353 @@ func TestArgumentRefusalsNameTheUsage(t *testing.T) { | ||
| 848 | 1096 | (Add `"gitbay.org/gitbay/internal/config"` to the file's imports if it |
| 849 | 1097 | is not already there.) |
| 850 | 1098 | |
| 851 | - [ ] **Step 7: Run the package** | |
| 1099 | - [ ] **Step 13: Run the package** | |
| 852 | 1100 | |
| 853 | 1101 | Run: `go test ./internal/control -count=1` |
| 854 | 1102 | Expected: PASS. Any other test asserting a bare `"usage: <path>..."` with |
| 855 | 1103 | no program prefix needs the same treatment — `grep -rn '"usage: ' |
| 856 | internal/control/*_test.go` finds them all. | |
| 1104 | internal/control/*_test.go` finds them all; a `help`-rendering test | |
| 1105 | asserting a bare `"gitbay keys ..."` line for one of the eighteen | |
| 1106 | commands needs the CLI-prefixed form instead. | |
| 1107 | ||
| 1108 | - [ ] **Step 14: The CLI side — write the failing tests for the two pure helpers** | |
| 1109 | ||
| 1110 | `cmd/gitbay/ssh.go` needs a way to read a cobra command's own path, and | |
| 1111 | a way to fold it onto a server command line, both pure and cheap to | |
| 1112 | unit test the way `termValue`/`pagerArgv`/`pages` already are (`cmd/gitbay/term_test.go`): | |
| 1113 | ||
| 1114 | ```go | |
| 1115 | func TestCLIPathOf(t *testing.T) { | |
| 1116 | root := &cobra.Command{Use: "gitbay"} | |
| 1117 | auth := &cobra.Command{Use: "auth"} | |
| 1118 | keys := &cobra.Command{Use: "keys"} | |
| 1119 | remove := &cobra.Command{Use: "remove"} | |
| 1120 | keys.AddCommand(remove) | |
| 1121 | auth.AddCommand(keys) | |
| 1122 | root.AddCommand(auth) | |
| 1123 | if got := cliPathOf(remove); got != "auth keys remove" { | |
| 1124 | t.Errorf("cliPathOf = %q", got) | |
| 1125 | } | |
| 1126 | } | |
| 1127 | ||
| 1128 | func TestWithCLIPath(t *testing.T) { | |
| 1129 | if got := withCLIPath("", []string{"keys", "remove", "abc"}); !slices.Equal(got, []string{"keys", "remove", "abc"}) { | |
| 1130 | t.Errorf("empty cliPath: %v", got) | |
| 1131 | } | |
| 1132 | got := withCLIPath("auth keys remove", []string{"keys", "remove", "abc"}) | |
| 1133 | want := []string{"--path=auth keys remove", "keys", "remove", "abc"} | |
| 1134 | if !slices.Equal(got, want) { | |
| 1135 | t.Errorf("got %v, want %v", got, want) | |
| 1136 | } | |
| 1137 | } | |
| 1138 | ``` | |
| 1139 | ||
| 1140 | Add these to `cmd/gitbay/term_test.go`, alongside `TestTermValue` and | |
| 1141 | `TestPagerArgv`; add `"slices"` and `"github.com/spf13/cobra"` to its | |
| 1142 | imports if not already there. | |
| 1143 | ||
| 1144 | - [ ] **Step 15: Run and see them fail** | |
| 1145 | ||
| 1146 | Run: `go test ./cmd/gitbay -run "TestCLIPathOf|TestWithCLIPath" -count=1` | |
| 1147 | Expected: FAIL to compile (`cliPathOf`/`withCLIPath` undefined). | |
| 1148 | ||
| 1149 | - [ ] **Step 16: Implement the two helpers in `cmd/gitbay/ssh.go`** | |
| 1150 | ||
| 1151 | Add `"github.com/spf13/cobra"` to the file's imports, then: | |
| 1152 | ||
| 1153 | ```go | |
| 1154 | // cliPathOf is the path this cobra command was actually reached by, | |
| 1155 | // stripped of the root's own name: "auth keys remove" for a command | |
| 1156 | // nested under auth > keys > remove. It is sent to the server as | |
| 1157 | // --path=, so usage and help can print what the caller can actually | |
| 1158 | // run even where that differs from the registered path being | |
| 1159 | // dispatched (cmd.Annotations[serverPath]) — #267. | |
| 1160 | func cliPathOf(cmd *cobra.Command) string { | |
| 1161 | return strings.TrimPrefix(cmd.CommandPath(), "gitbay ") | |
| 1162 | } | |
| 1163 | ||
| 1164 | // withCLIPath prepends --path=<cliPath> to a server command line, the | |
| 1165 | // same way runSSHPaged prepends --term=: a leading pseudo-flag Dispatch | |
| 1166 | // strips before Lookup, never confused for a real argument. Empty | |
| 1167 | // cliPath is a no-op — nothing to add for a caller with no cobra tree | |
| 1168 | // of its own to have resolved. | |
| 1169 | func withCLIPath(cliPath string, argv []string) []string { | |
| 1170 | if cliPath == "" { | |
| 1171 | return argv | |
| 1172 | } | |
| 1173 | return append([]string{"--path=" + cliPath}, argv...) | |
| 1174 | } | |
| 1175 | ``` | |
| 1176 | ||
| 1177 | - [ ] **Step 17: Run** | |
| 1178 | ||
| 1179 | Run: `go test ./cmd/gitbay -run "TestCLIPathOf|TestWithCLIPath" -count=1` | |
| 1180 | Expected: PASS. | |
| 1181 | ||
| 1182 | - [ ] **Step 18: Thread `cliPath` through `pass`, `runServerHelp`, `runPass`** | |
| 1183 | ||
| 1184 | In `cmd/gitbay/main.go`: | |
| 1185 | ||
| 1186 | ```go | |
| 1187 | func pass(use string, o passOpts) *cobra.Command { | |
| 1188 | return &cobra.Command{ | |
| 1189 | Use: use, | |
| 1190 | Short: summaries[strings.Join(o.server, " ")], | |
| 1191 | Annotations: map[string]string{ | |
| 1192 | serverPath: strings.Join(o.server, " "), | |
| 1193 | stdinMode: o.stdinModeName(), | |
| 1194 | stdinWhat: o.stdinWhat, | |
| 1195 | }, | |
| 1196 | DisableFlagParsing: true, | |
| 1197 | RunE: func(cmd *cobra.Command, args []string) error { | |
| 1198 | // The registry is the only place flags are written down, so | |
| 1199 | // --help asks the server rather than reprinting the one-line | |
| 1200 | // summary cobra holds. | |
| 1201 | cliPath := cliPathOf(cmd) | |
| 1202 | for _, a := range args { | |
| 1203 | if a == "--help" || a == "-h" { | |
| 1204 | os.Exit(runServerHelp(o, cliPath)) | |
| 1205 | } | |
| 1206 | } | |
| 1207 | os.Exit(runPass(o, cliPath, args)) | |
| 1208 | return nil | |
| 1209 | }, | |
| 1210 | } | |
| 1211 | } | |
| 1212 | ``` | |
| 1213 | ||
| 1214 | ```go | |
| 1215 | // runServerHelp prints the registry's usage for one command. | |
| 1216 | func runServerHelp(o passOpts, cliPath string) int { | |
| 1217 | t, err := resolveTarget() | |
| 1218 | if err != nil { | |
| 1219 | fmt.Fprintln(os.Stderr, "gitbay:", err) | |
| 1220 | return protocol.ExitFailure | |
| 1221 | } | |
| 1222 | return runSSH(t, withCLIPath(cliPath, append([]string{"help"}, o.server...)), strings.NewReader("")) | |
| 1223 | } | |
| 1224 | ||
| 1225 | func runPass(o passOpts, cliPath string, args []string) int { | |
| 1226 | ``` | |
| 1227 | ||
| 1228 | (`runPass`'s body is otherwise unchanged; only its signature gains | |
| 1229 | `cliPath string` as the second parameter, and its final line becomes:) | |
| 1230 | ||
| 1231 | ```go | |
| 1232 | return runSSHPaged(t, withCLIPath(cliPath, append(o.server, args...)), stdin, pages(o.server, args)) | |
| 1233 | ``` | |
| 1234 | ||
| 1235 | - [ ] **Step 19: Thread `cliPath` through `group`/`serverHelp`** | |
| 1236 | ||
| 1237 | ```go | |
| 1238 | func group(use, short string, subs ...*cobra.Command) *cobra.Command { | |
| 1239 | c := &cobra.Command{Use: use, Short: short} | |
| 1240 | c.AddCommand(subs...) | |
| 1241 | // A noun's help is the server's, like a command's: the registry is | |
| 1242 | // the only place flags are written down, and cobra's subcommand list | |
| 1243 | // carried none (#130). Offline, or for a noun the server does not | |
| 1244 | // know by that name, cobra's own tree still prints. | |
| 1245 | local := c.HelpFunc() | |
| 1246 | c.SetHelpFunc(func(cmd *cobra.Command, args []string) { | |
| 1247 | if !serverHelp(use, cliPathOf(cmd)) { | |
| 1248 | local(cmd, args) | |
| 1249 | } | |
| 1250 | }) | |
| 1251 | return c | |
| 1252 | } | |
| 1253 | ||
| 1254 | // serverHelp prints the registry's usage for a prefix and reports whether | |
| 1255 | // it did. cliPath is this invocation's own resolved cobra path (empty for | |
| 1256 | // a noun whose CLI path already matches its registered prefix). At a | |
| 1257 | // terminal it goes through the terminal-aware path, so it gets the same | |
| 1258 | // --term=<cols>[,color] treatment (and layout) as any other command; | |
| 1259 | // piped, it stays a quiet capture, so a network or lookup failure falls | |
| 1260 | // back to cobra's local help without noise. | |
| 1261 | func serverHelp(prefix, cliPath string) bool { | |
| 1262 | t, err := resolveTarget() | |
| 1263 | if err != nil { | |
| 1264 | return false | |
| 1265 | } | |
| 1266 | argv := withCLIPath(cliPath, []string{"help", prefix}) | |
| 1267 | if term.IsTerminal(int(os.Stdout.Fd())) { | |
| 1268 | return runSSH(t, argv, strings.NewReader("")) == 0 | |
| 1269 | } | |
| 1270 | out, code := sshCapture(t, argv) | |
| 1271 | if code != 0 || out == "" { | |
| 1272 | return false | |
| 1273 | } | |
| 1274 | fmt.Print(out) | |
| 1275 | return true | |
| 1276 | } | |
| 1277 | ``` | |
| 1278 | ||
| 1279 | - [ ] **Step 20: Build** | |
| 1280 | ||
| 1281 | Run: `go build ./... && go vet ./...` | |
| 1282 | Expected: builds clean. `keysAdd.RunE`/`pgpAdd.RunE` in `authCmd()` | |
| 1283 | (`cmd/gitbay/main.go`) are hand-built, not `pass()`-generated, so they | |
| 1284 | do not pick up `cliPath` from this step — Task 2.2 gives them the same | |
| 1285 | treatment where it already rewrites their bodies. | |
| 1286 | ||
| 1287 | - [ ] **Step 21: The cobra tree is the one place the eighteen mismatches | |
| 1288 | are allowed to be listed — a coverage test, not a hand check** | |
| 1289 | ||
| 1290 | A future command wired with a `serverPath` that does not match its own | |
| 1291 | `CommandPath()` is exactly this defect happening again; nothing should | |
| 1292 | have to remember to re-check it by hand. `TestEveryCommandIsReachable` | |
| 1293 | (`cmd/gitbay/coverage_test.go`) already walks `newRoot()` comparing | |
| 1294 | `Annotations[serverPath]` against the registry — this test walks the | |
| 1295 | same tree comparing it against `cliPathOf`, and pins today's known set | |
| 1296 | so any change to it (a new mismatch, or one of these being fixed to | |
| 1297 | match) shows up as a diff a reviewer has to look at. | |
| 1298 | ||
| 1299 | ```go | |
| 1300 | package main | |
| 1301 | ||
| 1302 | import ( | |
| 1303 | "slices" | |
| 1304 | "strings" | |
| 1305 | "testing" | |
| 1306 | ||
| 1307 | "github.com/spf13/cobra" | |
| 1308 | ) | |
| 1309 | ||
| 1310 | // TestServerPathMismatches pins the commands whose CLI path differs from | |
| 1311 | // the server path they dispatch (#267) — cmdUsage/help print the CLI | |
| 1312 | // path for exactly these, from cliPathOf, not the registered one. A new | |
| 1313 | // mismatch changes this list; update it deliberately, alongside the | |
| 1314 | // wiki's Parity page if it changes what a stock-ssh caller must type. | |
| 1315 | func TestServerPathMismatches(t *testing.T) { | |
| 1316 | want := []string{ | |
| 1317 | "auth email add", "auth email list", "auth email primary", | |
| 1318 | "auth email remove", "auth email verify", | |
| 1319 | "auth export", | |
| 1320 | "auth keys add", "auth keys label", "auth keys list", "auth keys remove", | |
| 1321 | "auth pgp add", "auth pgp list", "auth pgp remove", | |
| 1322 | "auth token create", "auth token list", "auth token revoke", | |
| 1323 | "auth whoami", | |
| 1324 | "repo topics list", | |
| 1325 | } | |
| 1326 | ||
| 1327 | var got []string | |
| 1328 | var walk func(*cobra.Command) | |
| 1329 | walk = func(c *cobra.Command) { | |
| 1330 | if p := c.Annotations[serverPath]; p != "" { | |
| 1331 | if cli := cliPathOf(c); cli != p { | |
| 1332 | got = append(got, cli) | |
| 1333 | } | |
| 1334 | } | |
| 1335 | for _, sub := range c.Commands() { | |
| 1336 | walk(sub) | |
| 1337 | } | |
| 1338 | } | |
| 1339 | root := newRoot() | |
| 1340 | root.InitDefaultHelpCmd() | |
| 1341 | walk(root) | |
| 1342 | slices.Sort(got) | |
| 1343 | ||
| 1344 | if !slices.Equal(got, want) { | |
| 1345 | t.Errorf("mismatched CLI paths = %v\nwant %v", got, want) | |
| 1346 | } | |
| 1347 | } | |
| 1348 | ``` | |
| 1349 | ||
| 1350 | - [ ] **Step 22: Run** | |
| 1351 | ||
| 1352 | Run: `go test ./cmd/gitbay -run TestServerPathMismatches -count=1` | |
| 1353 | Expected: PASS (the eighteen are already there today; this step only | |
| 1354 | adds the guard, it changes no behavior). | |
| 1355 | ||
| 1356 | - [ ] **Step 23: One end-to-end proof, real ssh and the real binary** | |
| 1357 | ||
| 1358 | Every other test here is a unit test against `Ctx`/cobra values built | |
| 1359 | by hand; this is the one e2e test for this task (Global Constraints | |
| 1360 | caps it at one), proving the wiring — `runSSHPaged`'s `--path=` prepend, | |
| 1361 | the server's `--path=` parse, `cmdUsage`'s substitution — actually | |
| 1362 | reaches an instance over real ssh, for the CLI and for stock ssh alike. | |
| 1363 | Model it on `e2e/term_test.go`'s `TestTermEnvSelectsTerminalOutput`. | |
| 1364 | ||
| 1365 | ```go | |
| 1366 | package e2e | |
| 857 | 1367 | |
| 858 | - [ ] **Step 8: Commit** | |
| 1368 | import ( | |
| 1369 | "strings" | |
| 1370 | "testing" | |
| 1371 | ) | |
| 1372 | ||
| 1373 | // The CLI sends its own invoking path so usage and help print a command | |
| 1374 | // that exists — gitbay auth keys remove, never the unregistered gitbay | |
| 1375 | // keys remove (#267). Stock ssh, which never sends one, keeps seeing | |
| 1376 | // the registered path: it is the only one it could ever type. | |
| 1377 | func TestCLIUsagePrintsTheInvokingPath(t *testing.T) { | |
| 1378 | t.Parallel() | |
| 1379 | inst := startInstance(t) | |
| 1380 | key := inst.newKey(t, "alice") | |
| 1381 | inst.admin(t, "admin", "user", "create", "alice", "--key", key+".pub", | |
| 1382 | "--email", "alice@example.test", "--verified") | |
| 1383 | ||
| 1384 | c := &cli{bin: buildGitbayCLI(t), configDir: t.TempDir(), inst: inst, key: key} | |
| 1385 | c.must(t, "", "", "remote", "add", "test", "127.0.0.1", | |
| 1386 | "--port", instPort(inst), | |
| 1387 | "--ssh-option", "-i", "--ssh-option", key, | |
| 1388 | "--ssh-option", "-oIdentitiesOnly=yes", | |
| 1389 | "--ssh-option", "-oStrictHostKeyChecking=no", | |
| 1390 | "--ssh-option", "-oUserKnownHostsFile="+inst.sshDir+"/kh", | |
| 1391 | "--ssh-option", "-oBatchMode=yes", | |
| 1392 | "--default") | |
| 1393 | ||
| 1394 | // A mismatched command: the CLI path (auth keys remove) differs from | |
| 1395 | // the registered one (keys remove). No fingerprint given, an actual | |
| 1396 | // wrong-argument refusal. | |
| 1397 | _, errOut, code := c.run(t, "", "", "auth", "keys", "remove") | |
| 1398 | if code == 0 || !strings.Contains(errOut, "usage: gitbay auth keys remove") { | |
| 1399 | t.Errorf("mismatched command: exit %d, stderr %q", code, errOut) | |
| 1400 | } | |
| 1401 | if strings.Contains(errOut, "usage: gitbay keys remove") { | |
| 1402 | t.Errorf("mismatched command leaked the registered path: %q", errOut) | |
| 1403 | } | |
| 1404 | ||
| 1405 | // A matching command: no CLI/registered difference, still the gitbay | |
| 1406 | // form (it is a terminal-adjacent test binary run, isTTY is false | |
| 1407 | // here, so this exercises the non-terminal ssh form instead — | |
| 1408 | // assert on the registered path itself, which is all cmdUsage can | |
| 1409 | // tell apart in that mode). | |
| 1410 | _, errOut2, code2 := c.run(t, "", "", "repo", "show") | |
| 1411 | if code2 == 0 || !strings.Contains(errOut2, "usage: ") || !strings.Contains(errOut2, "repo show") { | |
| 1412 | t.Errorf("matching command: exit %d, stderr %q", code2, errOut2) | |
| 1413 | } | |
| 1414 | ||
| 1415 | // Stock ssh, no CLI involved: the registered path, because it is the | |
| 1416 | // only one this caller could have typed. | |
| 1417 | _, errOut3, code3 := inst.ssh(t, key, "", "keys", "remove") | |
| 1418 | if code3 == 0 || !strings.Contains(errOut3, "usage: ssh git@") || !strings.Contains(errOut3, "keys remove") { | |
| 1419 | t.Errorf("stock ssh: exit %d, stderr %q", code3, errOut3) | |
| 1420 | } | |
| 1421 | if strings.Contains(errOut3, "auth keys remove") { | |
| 1422 | t.Errorf("stock ssh should never see the CLI-only auth prefix: %q", errOut3) | |
| 1423 | } | |
| 1424 | } | |
| 1425 | ``` | |
| 1426 | ||
| 1427 | (`instPort`/`inst.sshDir`/`c.run`/`inst.ssh` are whatever `e2e/cli_test.go` | |
| 1428 | and `e2e/term_test.go` already expose — read both before writing this | |
| 1429 | file and use their actual helper names and signatures rather than the | |
| 1430 | ones guessed here; `TestCLI` in `e2e/cli_test.go` is the fullest existing | |
| 1431 | example of standing up a `cli` value against a live `instance`.) | |
| 1432 | ||
| 1433 | - [ ] **Step 24: Run the one e2e test** | |
| 1434 | ||
| 1435 | Run: `go test ./e2e -run TestCLIUsagePrintsTheInvokingPath -count=1` | |
| 1436 | Expected: PASS. | |
| 1437 | ||
| 1438 | - [ ] **Step 25: Run everything this task touched, commit** | |
| 1439 | ||
| 1440 | Run: `go build ./... && go vet ./... && go test ./internal/control ./cmd/gitbay -count=1` | |
| 1441 | Expected: PASS. | |
| 859 | 1442 | |
| 860 | 1443 | ```bash |
| 861 | git add internal/control/help.go internal/control/control.go internal/control/control_test.go internal/control/help_test.go | |
| 862 | git commit -m "usage: print the program form (gitbay or ssh git@host), owner/name optional at a terminal" -m "Ref #267" | |
| 1444 | git add internal/control/help.go internal/control/control.go internal/control/control_test.go internal/control/help_test.go cmd/gitbay/ssh.go cmd/gitbay/main.go cmd/gitbay/term_test.go cmd/gitbay/serverpath_test.go e2e/cliusage_test.go | |
| 1445 | git commit -m "usage, help: print the CLI's own invoking path where it differs from the registered one, the ssh form otherwise" -m "Ref #267" | |
| 863 | 1446 | ``` |
| 864 | 1447 | |
| 865 | 1448 | ### Task 2.2: `--help` check in `keys add` and `pgp add` |
| @@ -907,30 +1490,56 @@ Expected: FAIL or timeout (stdin read attempted). | ||
| 907 | 1490 | - [ ] **Step 3: Implement** |
| 908 | 1491 | |
| 909 | 1492 | `keysAdd.RunE` and `pgpAdd.RunE` in `cmd/gitbay/main.go` each gain the |
| 910 | same loop `pass()` already has, before resolving the target: | |
| 1493 | same loop `pass()` already has, before resolving the target. They also | |
| 1494 | pick up `cliPath` here (Task 2.1 gave `runServerHelp` a second | |
| 1495 | parameter but could not touch these two hand-built `RunE`s, since this | |
| 1496 | task is what rewrites their bodies): `auth keys add` and `auth pgp add` | |
| 1497 | are two of the eighteen commands whose CLI path differs from the | |
| 1498 | registered one, so their own usage refusals need it exactly like every | |
| 1499 | `pass()`-generated command's do. | |
| 911 | 1500 | |
| 912 | 1501 | ```go |
| 913 | 1502 | keysAdd.RunE = func(cmd *cobra.Command, args []string) error { |
| 1503 | cliPath := cliPathOf(cmd) | |
| 914 | 1504 | for _, a := range args { |
| 915 | 1505 | if a == "--help" || a == "-h" { |
| 916 | os.Exit(runServerHelp(passOpts{server: []string{"keys", "add"}})) | |
| 1506 | os.Exit(runServerHelp(passOpts{server: []string{"keys", "add"}}, cliPath)) | |
| 917 | 1507 | } |
| 918 | 1508 | } |
| 919 | 1509 | t, err := resolveTarget() |
| 920 | ... | |
| 1510 | if err != nil { | |
| 1511 | return err | |
| 1512 | } | |
| 1513 | in, err := stdinPayload(os.Stdin, "an SSH public key", false) | |
| 1514 | if err != nil { | |
| 1515 | return err | |
| 1516 | } | |
| 1517 | os.Exit(runSSH(t, withCLIPath(cliPath, append([]string{"keys", "add"}, args...)), in)) | |
| 1518 | return nil | |
| 1519 | } | |
| 921 | 1520 | ``` |
| 922 | 1521 | |
| 923 | 1522 | and, for `pgpAdd`: |
| 924 | 1523 | |
| 925 | 1524 | ```go |
| 926 | 1525 | RunE: func(cmd *cobra.Command, args []string) error { |
| 1526 | cliPath := cliPathOf(cmd) | |
| 927 | 1527 | for _, a := range args { |
| 928 | 1528 | if a == "--help" || a == "-h" { |
| 929 | os.Exit(runServerHelp(passOpts{server: []string{"pgp", "add"}})) | |
| 1529 | os.Exit(runServerHelp(passOpts{server: []string{"pgp", "add"}}, cliPath)) | |
| 930 | 1530 | } |
| 931 | 1531 | } |
| 932 | 1532 | t, err := resolveTarget() |
| 933 | ... | |
| 1533 | if err != nil { | |
| 1534 | return err | |
| 1535 | } | |
| 1536 | in, err := stdinPayload(os.Stdin, "an armored OpenPGP public key", false) | |
| 1537 | if err != nil { | |
| 1538 | return err | |
| 1539 | } | |
| 1540 | os.Exit(runSSH(t, withCLIPath(cliPath, append([]string{"pgp", "add"}, args...)), in)) | |
| 1541 | return nil | |
| 1542 | }, | |
| 934 | 1543 | ``` |
| 935 | 1544 | |
| 936 | 1545 | - [ ] **Step 4: Run** |
| @@ -950,7 +1559,7 @@ git add cmd/gitbay/main.go cmd/gitbay/main_test.go | ||
| 950 | 1559 | git commit -m "auth keys add, pgp add: check --help before reading stdin" -m "Ref #267" |
| 951 | 1560 | ``` |
| 952 | 1561 | |
| 953 | ### Task 2.3: `auth --help` renders with the registry layout | |
| 1562 | ### Task 2.3: `auth --help` renders with the registry layout, in the CLI's own paths | |
| 954 | 1563 | |
| 955 | 1564 | `auth` is a CLI-only grouping — no registry command's path starts with |
| 956 | 1565 | `auth`, so `gitbay auth --help` asks the server for help on prefix |
| @@ -959,27 +1568,44 @@ falls back to cobra's own subcommand listing, which carries no flags or | ||
| 959 | 1568 | examples (the reason `group()` exists at all, per its own comment). |
| 960 | 1569 | Give the registry an alias table for CLI-only groupings so `auth` |
| 961 | 1570 | renders the same READ/WRITE, aligned-summary layout every real noun |
| 962 | gets. | |
| 1571 | gets — and, since none of the rows it gathers (`keys add`, `account | |
| 1572 | export`, ...) are commands a caller can actually type, each row prints | |
| 1573 | the CLI path it really takes (`auth keys add`, `auth export`), the same | |
| 1574 | substitution Task 2.1 gave a single command's own usage line. Unlike | |
| 1575 | Task 2.1's mismatches, which are one registered path to one CLI path, | |
| 1576 | `auth` gathers several unrelated registered prefixes into one grouping, | |
| 1577 | and one of them (`account export` -> `auth export`) does not even keep | |
| 1578 | the same word count — the alias table has to carry the CLI form | |
| 1579 | alongside each registered prefix explicitly; it cannot be derived by | |
| 1580 | pattern-matching the prefix the way Task 2.1's single-command swap is. | |
| 963 | 1581 | |
| 964 | 1582 | **Files:** |
| 965 | - Modify: `internal/control/help.go` (`runHelp`, `nounSummaries`) | |
| 1583 | - Modify: `internal/control/help.go` (`runHelp`, `nounAliases`, `nounSummaries`) | |
| 966 | 1584 | - Modify: `cmd/gitbay/main.go` (`authCmd`'s `group("auth", ...)` description) |
| 967 | 1585 | - Test: `internal/control/help_test.go` |
| 968 | 1586 | |
| 969 | 1587 | **Interfaces:** |
| 970 | - Produces: `var nounAliases map[string][]string` — a CLI-only noun name to the real registry prefixes it gathers. | |
| 1588 | - Produces: `type nounAlias struct { Registered, CLI string }`; `var | |
| 1589 | nounAliases map[string][]nounAlias` — a CLI-only noun name to the | |
| 1590 | registered prefixes it gathers, each paired with the CLI path that | |
| 1591 | reaches it. | |
| 971 | 1592 | |
| 972 | - [ ] **Step 1: Write the failing test** | |
| 1593 | - [ ] **Step 1: Write the failing tests** | |
| 1594 | ||
| 1595 | Two: the CLI form (a caller that sent `--path=auth`, as `gitbay auth | |
| 1596 | --help` now does per Task 2.1's `group`/`serverHelp` change), and the | |
| 1597 | ssh form (a caller that sent nothing, which cannot run an `auth | |
| 1598 | whatever` command and must not be told to). | |
| 973 | 1599 | |
| 974 | 1600 | ```go |
| 975 | 1601 | func TestHelpRendersAnAliasedNounWithTheRegistryLayout(t *testing.T) { |
| 976 | 1602 | var out bytes.Buffer |
| 977 | c := &Ctx{Stdout: &out, Term: Term{Cols: 100}} | |
| 1603 | c := &Ctx{Stdout: &out, Term: Term{Cols: 100}, CLIPath: "auth"} | |
| 978 | 1604 | if code := runHelp(c, []string{"auth"}); code != protocol.ExitOK { |
| 979 | 1605 | t.Fatalf("exit %d", code) |
| 980 | 1606 | } |
| 981 | 1607 | got := out.String() |
| 982 | for _, want := range []string{"whoami", "keys list", "pgp add", "token create", "account export"} { | |
| 1608 | for _, want := range []string{"auth whoami", "auth keys list", "auth pgp add", "auth token create", "auth export"} { | |
| 983 | 1609 | if !strings.Contains(got, want) { |
| 984 | 1610 | t.Errorf("missing %q in:\n%s", want, got) |
| 985 | 1611 | } |
| @@ -988,11 +1614,28 @@ func TestHelpRendersAnAliasedNounWithTheRegistryLayout(t *testing.T) { | ||
| 988 | 1614 | t.Errorf("auth did not resolve: %s", got) |
| 989 | 1615 | } |
| 990 | 1616 | } |
| 1617 | ||
| 1618 | func TestHelpRendersAnAliasedNounInRegisteredFormOverSSH(t *testing.T) { | |
| 1619 | var out bytes.Buffer | |
| 1620 | c := &Ctx{Stdout: &out} // no Term, no CLIPath: exactly stock ssh | |
| 1621 | if code := runHelp(c, []string{"auth"}); code != protocol.ExitOK { | |
| 1622 | t.Fatalf("exit %d", code) | |
| 1623 | } | |
| 1624 | got := out.String() | |
| 1625 | for _, want := range []string{"whoami", "keys list", "pgp add", "token create", "account export"} { | |
| 1626 | if !strings.Contains(got, want) { | |
| 1627 | t.Errorf("missing %q in:\n%s", want, got) | |
| 1628 | } | |
| 1629 | } | |
| 1630 | if strings.Contains(got, "auth keys list") { | |
| 1631 | t.Errorf("stock ssh should not see the CLI-only auth prefix: %s", got) | |
| 1632 | } | |
| 1633 | } | |
| 991 | 1634 | ``` |
| 992 | 1635 | |
| 993 | - [ ] **Step 2: Run and see it fail** | |
| 1636 | - [ ] **Step 2: Run and see them fail** | |
| 994 | 1637 | |
| 995 | Run: `go test ./internal/control -run TestHelpRendersAnAliasedNounWithTheRegistryLayout -count=1` | |
| 1638 | Run: `go test ./internal/control -run TestHelpRendersAnAliasedNoun -count=1` | |
| 996 | 1639 | Expected: FAIL (`no command matches "auth"`). |
| 997 | 1640 | |
| 998 | 1641 | - [ ] **Step 3: Implement the alias table and the lookup change** |
| @@ -1000,13 +1643,33 @@ Expected: FAIL (`no command matches "auth"`). | ||
| 1000 | 1643 | In `internal/control/help.go`, near `nounSummaries`: |
| 1001 | 1644 | |
| 1002 | 1645 | ```go |
| 1003 | // nounAliases groups a CLI-only noun (one with no registry path of its | |
| 1004 | // own, such as auth, which the cmd/gitbay CLI assembles from several | |
| 1005 | // unrelated registry prefixes) into the real prefixes it gathers, so | |
| 1006 | // `help auth` renders with the same layout a real noun gets instead of | |
| 1007 | // falling back to whatever a caller does when help fails. | |
| 1008 | var nounAliases = map[string][]string{ | |
| 1009 | "auth": {"account export", "whoami", "keys", "email", "pgp", "token"}, | |
| 1646 | // nounAlias is one bucket of registered commands, reachable under a | |
| 1647 | // CLI-only noun that is not itself a registry path (auth, gathering | |
| 1648 | // several unrelated registry prefixes): Registered is what runHelp | |
| 1649 | // matches against the registry, CLI is the path a gitbay caller | |
| 1650 | // actually types to reach it — not always Registered with the alias's | |
| 1651 | // own name stitched on (account export -> auth export drops a word), | |
| 1652 | // so the two are paired explicitly rather than derived. | |
| 1653 | type nounAlias struct { | |
| 1654 | Registered string | |
| 1655 | CLI string | |
| 1656 | } | |
| 1657 | ||
| 1658 | // nounAliases groups a CLI-only noun into the real prefixes it gathers, | |
| 1659 | // so `help auth` renders with the same layout a real noun gets instead | |
| 1660 | // of falling back to whatever a caller does when help fails. A stock | |
| 1661 | // ssh caller — the only one who could ever ask for a bare "auth" and | |
| 1662 | // get nothing back from the registry — sees the Registered forms | |
| 1663 | // unchanged; the CLI, having sent its own path, sees CLI. | |
| 1664 | var nounAliases = map[string][]nounAlias{ | |
| 1665 | "auth": { | |
| 1666 | {"account export", "auth export"}, | |
| 1667 | {"whoami", "auth whoami"}, | |
| 1668 | {"keys", "auth keys"}, | |
| 1669 | {"email", "auth email"}, | |
| 1670 | {"pgp", "auth pgp"}, | |
| 1671 | {"token", "auth token"}, | |
| 1672 | }, | |
| 1010 | 1673 | } |
| 1011 | 1674 | ``` |
| 1012 | 1675 | |
| @@ -1016,14 +1679,31 @@ and add, to `nounSummaries`: | ||
| 1016 | 1679 | "auth": "whoami, SSH and PGP keys, email, API tokens", |
| 1017 | 1680 | ``` |
| 1018 | 1681 | |
| 1019 | In `runHelp`, widen the match to every aliased prefix: | |
| 1682 | In `runHelp`, widen the match to every aliased prefix, and — only for a | |
| 1683 | caller that sent its own `CLIPath` — build the per-row override | |
| 1684 | `helpNoun` (Task 2.1) now accepts: | |
| 1020 | 1685 | |
| 1021 | 1686 | ```go |
| 1022 | 1687 | func runHelp(c *Ctx, args []string) int { |
| 1023 | 1688 | prefix := joinPath(args) |
| 1024 | 1689 | prefixes := []string{prefix} |
| 1690 | override := map[string]string{} | |
| 1025 | 1691 | if aliased, ok := nounAliases[prefix]; ok { |
| 1026 | prefixes = aliased | |
| 1692 | prefixes = nil | |
| 1693 | for _, a := range aliased { | |
| 1694 | prefixes = append(prefixes, a.Registered) | |
| 1695 | } | |
| 1696 | if c.CLIPath != "" { | |
| 1697 | for _, cmd := range registry { | |
| 1698 | p := joinPath(cmd.Path) | |
| 1699 | for _, a := range aliased { | |
| 1700 | if p == a.Registered || strings.HasPrefix(p, a.Registered+" ") { | |
| 1701 | override[p] = a.CLI + strings.TrimPrefix(p, a.Registered) | |
| 1702 | break | |
| 1703 | } | |
| 1704 | } | |
| 1705 | } | |
| 1706 | } | |
| 1027 | 1707 | } |
| 1028 | 1708 | var matched []Command |
| 1029 | 1709 | for _, cmd := range registry { |
| @@ -1035,15 +1715,41 @@ func runHelp(c *Ctx, args []string) int { | ||
| 1035 | 1715 | } |
| 1036 | 1716 | } |
| 1037 | 1717 | } |
| 1718 | if len(matched) == 0 { | |
| 1719 | return c.fail(protocol.ExitNotFound, "no command matches %q; try: help", prefix) | |
| 1720 | } | |
| 1721 | slices.SortFunc(matched, func(a, b Command) int { return strings.Compare(joinPath(a.Path), joinPath(b.Path)) }) | |
| 1722 | entries := make([]helpEntry, len(matched)) | |
| 1723 | for i, cmd := range matched { | |
| 1724 | entries[i] = helpEntry{Path: joinPath(cmd.Path), Summary: cmd.Summary, Usage: cmd.Usage, Flags: cmd.Flags, Examples: cmd.Examples} | |
| 1725 | } | |
| 1726 | return c.emit(entries, func(w io.Writer) { | |
| 1727 | switch { | |
| 1728 | case prefix == "": | |
| 1729 | for _, e := range entries { | |
| 1730 | summary := e.Summary | |
| 1731 | if c.Term.Cols > 0 { | |
| 1732 | if avail := c.Term.Cols - max(cells(e.Path), 24) - 1; avail > 0 { | |
| 1733 | summary = clip(summary, avail) | |
| 1734 | } | |
| 1735 | } | |
| 1736 | fmt.Fprintf(w, "%-24s %s\n", e.Path, summary) | |
| 1737 | } | |
| 1738 | case joinPath(matched[0].Path) == prefix: | |
| 1739 | c.helpVerb(w, matched[0], matched[1:]) | |
| 1740 | default: | |
| 1741 | c.helpNoun(w, prefix, matched, override) | |
| 1742 | } | |
| 1743 | }) | |
| 1744 | } | |
| 1038 | 1745 | ``` |
| 1039 | 1746 | |
| 1040 | The rest of `runHelp` is unchanged: `matched[0].Path` never equals | |
| 1041 | `"auth"` literally (nothing in the registry is named that), so the | |
| 1042 | alias always takes the `helpNoun` branch, which already handles a | |
| 1043 | command list whose paths do not share `prefix` as an actual prefix — | |
| 1044 | `strings.TrimPrefix` is a no-op on a path it does not match, so `keys | |
| 1045 | add`, `pgp add`, `token create` and `whoami` print by their real, full | |
| 1046 | paths under the `auth` heading. | |
| 1747 | `matched[0].Path` never equals `"auth"` literally (nothing in the | |
| 1748 | registry is named that), so an aliased noun always takes the | |
| 1749 | `helpNoun` branch. `override` stays an empty (non-nil) map for every | |
| 1750 | ordinary noun — `override[full]` misses for every row, and `helpNoun` | |
| 1751 | falls back to its plain `strings.TrimPrefix` — so this changes nothing | |
| 1752 | for `help repo` or any other real prefix. | |
| 1047 | 1753 | |
| 1048 | 1754 | - [ ] **Step 4: Sync the CLI's own description** |
| 1049 | 1755 | |
| @@ -1058,7 +1764,7 @@ paths under the `auth` heading. | ||
| 1058 | 1764 | |
| 1059 | 1765 | - [ ] **Step 5: Run** |
| 1060 | 1766 | |
| 1061 | Run: `go test ./internal/control -run TestHelpRendersAnAliasedNounWithTheRegistryLayout -count=1` | |
| 1767 | Run: `go test ./internal/control -run TestHelpRendersAnAliasedNoun -count=1` | |
| 1062 | 1768 | Expected: PASS. |
| 1063 | 1769 | |
| 1064 | 1770 | - [ ] **Step 6: Run both packages** |
| @@ -1070,21 +1776,9 @@ Expected: PASS. | ||
| 1070 | 1776 | |
| 1071 | 1777 | ```bash |
| 1072 | 1778 | git add internal/control/help.go internal/control/help_test.go cmd/gitbay/main.go |
| 1073 | git commit -m "help: auth (and any future CLI-only grouping) renders with the registry layout" -m "Ref #267" | |
| 1779 | git commit -m "help: auth (and any future CLI-only grouping) renders with the registry layout, in the CLI's own paths" -m "Ref #267" | |
| 1074 | 1780 | ``` |
| 1075 | 1781 | |
| 1076 | **Open question** (cannot be resolved from the code, flagging rather | |
| 1077 | than guessing): #267's own text quotes the desired usage as `gitbay auth | |
| 1078 | keys remove <fingerprint>` — with `auth` in the printed command — but | |
| 1079 | the server has no notion of the CLI's `auth` grouping; it only knows the | |
| 1080 | registered path `keys remove`. This task and Task 2.1 make the server | |
| 1081 | print `gitbay keys remove <fingerprint>` (correct, runnable, but missing | |
| 1082 | the `auth` cobra sits it under). Inserting `auth` would mean either | |
| 1083 | teaching the registry about a purely cobra-side grouping, or having the | |
| 1084 | CLI rewrite the server's usage string client-side by pattern-matching | |
| 1085 | its own command tree — decide which, if the exact wording matters, before | |
| 1086 | merging this MR. | |
| 1087 | ||
| 1088 | 1782 | ### Task 2.4: verb-phrase summaries |
| 1089 | 1783 | |
| 1090 | 1784 | Six commands' one-line summaries are bare nouns rather than a phrase |
| @@ -1813,10 +2507,14 @@ Wait for CI, merge with `--strategy ff`, delete the branch both places. | ||
| 1813 | 2507 | matches current behavior (`internal/control/dashboard.go`'s `section` |
| 1814 | 2508 | helper, `internal/control/control.go`'s `emit`) — no task needed. |
| 1815 | 2509 | - #267: CLI sends `--term`/`Ctx.Term` (already present; verified, not |
| 1816 | re-implemented) and the server now uses it in `usage()`/`usageWith()` | |
| 1817 | too (Task 2.1); `[<owner/name>]` optional from the CLI (Task 2.1); | |
| 1818 | `--help` check in `keys add`/`pgp add` (Task 2.2); `auth` rendered | |
| 1819 | with the registry layout (Task 2.3); verb-phrase summaries (Task 2.4). | |
| 2510 | re-implemented) and now also `--path`/`Ctx.CLIPath`, its own invoking | |
| 2511 | path — the same mechanism, a sibling prefix — used by | |
| 2512 | `usage()`/`usageWith()`/`helpVerb`/`helpNoun` so the eighteen commands | |
| 2513 | whose CLI path differs from the registered one print a command that | |
| 2514 | exists (Task 2.1); `[<owner/name>]` optional from the CLI (Task 2.1); | |
| 2515 | `--help` check in `keys add`/`pgp add`, now also sending their own | |
| 2516 | `cliPath` (Task 2.2); `auth` rendered with the registry layout, each | |
| 2517 | row in the CLI's own path (Task 2.3); verb-phrase summaries (Task 2.4). | |
| 1820 | 2518 | - #268: unregistered-key message (Task 3.1); `issue create` flags |
| 1821 | 2519 | (Task 3.2); `mr show` plurals (Task 3.3); `repo readme` (Task 3.4); |
| 1822 | 2520 | `repo show` mirror time (Task 3.5). |
| @@ -1836,14 +2534,11 @@ reads the named test file's other tests first, per each step's own | ||
| 1836 | 2534 | instruction, before writing the step. |
| 1837 | 2535 | |
| 1838 | 2536 | **Type consistency:** `FeedLine`/`FeedLines`/`WorstStatus` (Task 1.1) |
| 1839 | are used with the same names in Tasks 1.2 and 1.3. `cmdUsage`/`cliUsage` | |
| 1840 | (Task 2.1) are used with the same names in Task 2.3's commentary and | |
| 1841 | nowhere else redefines them. `PickReadme` (Task 3.4) is the only name | |
| 1842 | introduced for that logic and is used consistently in its own task. | |
| 1843 | ||
| 1844 | **Open question**, restated from Task 2.3: whether `gitbay auth keys | |
| 1845 | remove <fingerprint>`'s usage line should literally say `auth` (the | |
| 1846 | CLI's own grouping, invisible to the server) or `gitbay keys remove | |
| 1847 | <fingerprint>` (what the server can actually know and this plan | |
| 1848 | implements) needs a decision from whoever merges Part 2, since #267's | |
| 1849 | own text quotes the former. | |
| 2537 | are used with the same names in Tasks 1.2 and 1.3. `Ctx.CLIPath`, | |
| 2538 | `cmdUsage`/`cliUsage`/`shownAs` and the CLI-side `cliPathOf`/ | |
| 2539 | `withCLIPath` (Task 2.1) are used with the same names and signatures in | |
| 2540 | Task 2.2 (`keysAdd.RunE`/`pgpAdd.RunE` sending their own `cliPath`) and | |
| 2541 | Task 2.3 (`helpNoun`'s `override` map, the alias table's `CLI` field | |
| 2542 | built from the same substitution `shownAs` performs for a single | |
| 2543 | command). `PickReadme` (Task 3.4) is the only name introduced for that | |
| 2544 | logic and is used consistently in its own task. | |
docs/plans/2026-09-27-credentials-and-sessions.md +26 −41
| @@ -24,8 +24,8 @@ cap. | ||
| 24 | 24 | client for e2e. |
| 25 | 25 | |
| 26 | 26 | **Spec:** issues #256, #257, #276, #277, #278 on krz/gitbay (the |
| 27 | decisions on #256 and #257 are recorded there), and the brief | |
| 28 | `/private/tmp/claude-501/-Users-cmc-git-krz-gitbay/7b2f1ea4-aab6-44e2-b2ad-d4ec6852ce42/scratchpad/brief.md`. | |
| 27 | decisions on #256 and #257 are recorded there, and the later ones in | |
| 28 | "Decisions" at the end of this plan). | |
| 29 | 29 | |
| 30 | 30 | ## Global constraints |
| 31 | 31 | |
| @@ -3512,45 +3512,30 @@ gitbay mr create --source weblogin-limit --target main --title "web login over S | ||
| 3512 | 3512 | |
| 3513 | 3513 | --- |
| 3514 | 3514 | |
| 3515 | ## Open questions | |
| 3516 | ||
| 3517 | 1. **Expiring SSH keys and minting.** #277 says to consider it with | |
| 3518 | #257; this plan treats an expiring key like an expiring token and | |
| 3519 | refuses it the nine minting commands (MR 3, Task 3.2). One effect: | |
| 3520 | a person whose only key has a TTL cannot run `web login`, and must | |
| 3521 | use the mailed link. Confirm, or drop `Expires: key.ExpiresAt` from | |
| 3522 | `Exec` and `TestExpiringKeyCannotMint`. | |
| 3523 | 2. **Which commands mint.** The issue names token create, keys add, | |
| 3524 | repo deploy-key add, admin invite and web login. The plan adds | |
| 3525 | `repo runner add` (creates or attaches a key that claims builds), | |
| 3526 | `admin user create` (with `--key` or a verified address it is a way | |
| 3527 | in), `email verify` and `admin email verify` (a verified address | |
| 3528 | receives login links, so a token could plant a lasting way in). | |
| 3529 | `TestMintingCommandsMarked` pins the list. | |
| 3530 | 3. **LFS transfer tokens.** `git-lfs-authenticate` hands out an HMAC | |
| 3531 | token valid for an hour (`internal/lfs`), not stored, revocable only | |
| 3532 | by expiry. A key removed after minting one leaves LFS access for up | |
| 3533 | to an hour. Not in any of these issues; file separately? | |
| 3534 | 4. **System mode.** With `ssh.mode = "system"`, each exec is its own | |
| 3535 | forced-command process: the per-exec check applies, a running one is | |
| 3536 | not cut. bay1 runs embedded. Closing it would take a poll in | |
| 3537 | `gitbayd shell`; the plan documents the limit instead. | |
| 3538 | 5. **Commands already past dispatch.** "Running commands included" is | |
| 3539 | implemented as: git transports are killed, commands watching `Done` | |
| 3540 | stop, and a control command already inside its store write finishes | |
| 3541 | it with its output lost. Cancelling arbitrary control commands would | |
| 3542 | need a context threaded through every handler. | |
| 3543 | 6. **Removing the key you are on.** `keys remove <the key this session | |
| 3544 | uses>` cuts its own connection, so the CLI reports a connection | |
| 3545 | error instead of "removed". The removal has committed. Acceptable, | |
| 3546 | or should `revoke` skip the connection running the removal? | |
| 3547 | 7. **Idle window as a constant.** 12 hours is `store.WebSessionIdle`, | |
| 3548 | repeated in migration 0062. Should it be configurable? | |
| 3549 | ||
| 3550 | Noticed, not in scope: `token list` at a terminal shows a future | |
| 3551 | expiry through `relAge`, which clamps to zero and prints "just now" | |
| 3552 | (`internal/control/token.go:112-116`). `expiresText` (MR 3) would fix | |
| 3553 | it if applied there. | |
| 3515 | ## Decisions (2026-09-28) | |
| 3516 | ||
| 3517 | 1. **Expiring SSH keys and minting.** Confirmed: an expiring key is | |
| 3518 | refused the minting commands like an expiring token (MR 3, | |
| 3519 | Task 3.2). | |
| 3520 | 2. **Which commands mint.** Confirmed: the issue's five plus | |
| 3521 | `repo runner add`, `admin user create`, `email verify` and | |
| 3522 | `admin email verify`, pinned by `TestMintingCommandsMarked`. | |
| 3523 | 3. **LFS transfer tokens.** Filed as #285; not in this plan. | |
| 3524 | 4. **Removing the key you are on.** Confirmed: the session's own | |
| 3525 | connection is cut after the removal commits. | |
| 3526 | 5. **Idle window.** A constant (`store.WebSessionIdle`, 12 h); make it | |
| 3527 | configurable only when someone needs another value. | |
| 3528 | ||
| 3529 | Documented limits, stated on the Threat-Model page in MR 1: | |
| 3530 | ||
| 3531 | - With `ssh.mode = "system"` each exec is its own forced-command | |
| 3532 | process; the per-exec check applies, a running one is not cut. bay1 | |
| 3533 | runs embedded. | |
| 3534 | - A control command already inside its store write when the key is | |
| 3535 | revoked finishes the write and its output is lost; git transports are | |
| 3536 | killed and commands watching `Done` stop. | |
| 3537 | ||
| 3538 | The `token list` future-expiry display is #286. | |
| 3554 | 3539 | |
| 3555 | 3540 | ## Self-review |
| 3556 | 3541 | |
docs/plans/2026-09-27-data-at-rest-and-backup.md +434 −81
| @@ -5,8 +5,10 @@ | ||
| 5 | 5 | **Goal:** Seal the four secret columns under a key file outside the |
| 6 | 6 | database (#273), encrypt backup archives to an age recipient (#274), |
| 7 | 7 | and make `--verify` check git connectivity while repository moves and |
| 8 | deletions wait for a running backup (#259), ending with a restore drill | |
| 9 | the operator runs and records. | |
| 8 | deletions wait for a running backup (#259). The operator runbook moves | |
| 9 | the offsite copy from Scaleway to Cloudflare R2 under bucket locks, | |
| 10 | adds a separate keys repository for `apns.p8` and the secret key file, | |
| 11 | and holds the restore drill, which is deferred. | |
| 10 | 12 | |
| 11 | 13 | **Architecture:** A new `internal/seal` package holds AES-256-GCM keys |
| 12 | 14 | read from `server.secret_key_file` (default `/etc/gitbay/secret.key`, |
| @@ -30,7 +32,11 @@ decisions recorded in the brief: AES-GCM, key file under `/etc/gitbay` | ||
| 30 | 32 | mode 0600 excluded from backups, key id prefix on each value, rotation |
| 31 | 33 | command, re-encryption of existing rows; age recipients, `--verify` |
| 32 | 34 | takes an identity file; the clean-host drill is an operator runbook |
| 33 | recorded on the Admin wiki page. | |
| 35 | recorded on the Admin wiki page, deferred by the operator (#259 stays | |
| 36 | open), repeated quarterly and after any backup code change; the | |
| 37 | offsite restic repository moves to Cloudflare R2, and `apns.p8` and | |
| 38 | `secret.key` go to a separate small restic repository on R2 with its | |
| 39 | own bucket and password. | |
| 34 | 40 | |
| 35 | 41 | ## Global Constraints |
| 36 | 42 | |
| @@ -79,8 +85,9 @@ recorded on the Admin wiki page. | ||
| 79 | 85 | comments; the last key seals, all keys open. Mode must be 0600 or |
| 80 | 86 | stricter; anything group- or world-readable is refused. |
| 81 | 87 | - `server.secret_key_file` must not be inside `server.root` |
| 82 | (validation error), so neither the archive nor the restic snapshot | |
| 83 | of `/var/lib/gitbay` can carry it. | |
| 88 | (validation error), so neither the archive nor the main restic | |
| 89 | snapshot of `/var/lib/gitbay` can carry it. Its offsite copy is the | |
| 90 | separate keys repository (runbook D), never the main one. | |
| 84 | 91 | - A missing key file is fatal for every process that opens the |
| 85 | 92 | database through `openStore` (serve, shell, authorized-keys, host |
| 86 | 93 | admin commands), with a message naming the path and |
| @@ -95,7 +102,18 @@ recorded on the Admin wiki page. | ||
| 95 | 102 | | 1 | `secrets-at-rest` | Closes #273 | `internal/seal`, `server.secret_key_file`, store sealing, migration 0072, reseal at startup, `admin secrets init/rotate/check`, install.sh, e2e harness key, wiki | |
| 96 | 103 | | 2 | `backup-age` | Closes #274 | `[backup] age_recipients`, age-wrapped archives, `--verify --identity`, backup script globs, wiki | |
| 97 | 104 | | 3 | `backup-verify-lock` | Ref #259 | `gitutil.FsckConnectivity`, `--verify` extracts and checks each repository, `internal/backuplock`, delete/rename/transfer/org rename refused during a full backup, drill procedure and record table on the Admin page | |
| 98 | | — | `restore-drill-record` (operator) | Closes #259 | the first drill's numbers in the Admin page, Known-Gaps and Controls rows closed | | |
| 105 | | — | `offsite-r2-wiki` (operator) | Ref the R2 move issue (runbook D.1) | Admin, Threat-Model and Architecture pages describe R2 with bucket locks and the keys repository, after Scaleway is retired | | |
| 106 | | — | `restore-drill-record` (operator, deferred) | Closes #259 | the first drill's numbers in the Admin page, Known-Gaps and Controls rows closed | | |
| 107 | ||
| 108 | #259 stays open after MR 3: its commits say `Ref #259`, and only the | |
| 109 | drill record closes it. The operator has deferred the drill. | |
| 110 | ||
| 111 | The offsite job is not in the repository. `/usr/local/bin/gitbay-offsite` | |
| 112 | and its `gitbay-offsite.service`/`.timer` exist only on bay1; | |
| 113 | `deploy/cloud-init.yaml` does not template them (its | |
| 114 | `gitbay-backup.sh` carries only the comment "To ship offsite, add an | |
| 115 | rclone/s3 upload of $out here."). The move to R2 is therefore a | |
| 116 | runbook section (D) with the script edits written out, not a code MR. | |
| 99 | 117 | |
| 100 | 118 | MR 2 and MR 3 both edit `cmd/gitbayd/backup.go`; land them in order. |
| 101 | 119 | MR 2's `testConfig` helper comes from MR 1. |
| @@ -1834,9 +1852,11 @@ stored sealed: AES-256-GCM under a key in =server.secret_key_file=, | ||
| 1834 | 1852 | each value prefixed with the id of the key that sealed it |
| 1835 | 1853 | (=gbs1:<id>:=). The key file is not in the database, not under |
| 1836 | 1854 | =server.root=, and therefore in neither the local archives nor the |
| 1837 | restic snapshots. Keep a copy off the host; without it a restored | |
| 1838 | database's secrets cannot be opened, and gitbayd refuses to start | |
| 1839 | against them. | |
| 1855 | main restic repository. Its offsite copy is a separate restic | |
| 1856 | repository that holds only keys (see "Offsite copies"), with its own | |
| 1857 | bucket and password, so a leak of the main backup does not expose the | |
| 1858 | key that opens its secrets. Without the key a restored database's | |
| 1859 | secrets cannot be opened, and gitbayd refuses to start against them. | |
| 1840 | 1860 | |
| 1841 | 1861 | #+begin_src sh |
| 1842 | 1862 | gitbayd admin secrets init # once; deploy/install.sh does it on first install |
| @@ -1853,12 +1873,17 @@ gitbayd admin secrets rotate # new key, reseal, retire the old one (as root) | ||
| 1853 | 1873 | in clear and logs =sealed secret values=. |
| 1854 | 1874 | - Rotation: =rotate= adds a key, reseals every value under it in one |
| 1855 | 1875 | transaction, then removes the old keys. The daemon re-reads the file |
| 1856 | when it changes, so it needs no restart. Copy the new file off the | |
| 1857 | host afterwards. | |
| 1876 | when it changes, so it needs no restart. Back the new file up to the | |
| 1877 | keys repository afterwards. | |
| 1858 | 1878 | - Push devices are looked up by the SHA-256 of their token |
| 1859 | 1879 | (=push_devices.token_hash=), since two seals of one token differ. |
| 1860 | 1880 | ``` |
| 1861 | 1881 | |
| 1882 | If runbook D (the keys repository) has not run when this MR lands, | |
| 1883 | leave out the sentences naming the keys repository here and in | |
| 1884 | `06-Data-and-Cryptography.org` below; the `offsite-r2-wiki` MR adds | |
| 1885 | them. | |
| 1886 | ||
| 1862 | 1887 | - [ ] **Step 3: Architecture pages** |
| 1863 | 1888 | |
| 1864 | 1889 | `06-Data-and-Cryptography.org`: in the inventory, the CI secrets note becomes `sealed (AES-256-GCM)`, Integrations `webhook secret and mirror token sealed`, Notifications `device tokens sealed; looked up by SHA-256`. Add a row to "Outside the database": |
| @@ -1878,7 +1903,8 @@ and replace the paragraph "The code base contains no symmetric encryption. ..." | ||
| 1878 | 1903 | ```org |
| 1879 | 1904 | The database file or a backup read by anyone other than the =gitbay= |
| 1880 | 1905 | user discloses no CI secret, webhook secret, mirror token or device |
| 1881 | token without the key file, which neither carries. Rotation: | |
| 1906 | token without the key file, which neither carries; the key file's | |
| 1907 | only offsite copy is a separate keys repository. Rotation: | |
| 1882 | 1908 | =gitbayd admin secrets rotate= (Admin wiki). |
| 1883 | 1909 | ``` |
| 1884 | 1910 | |
| @@ -1894,7 +1920,7 @@ Update line 5's migration count to the number of files in | ||
| 1894 | 1920 | `09-Controls.org:59`: |
| 1895 | 1921 | |
| 1896 | 1922 | ```org |
| 1897 | | Secrets encrypted at rest | in place | AES-256-GCM, key file outside the database and backups (=internal/seal=) | | |
| 1923 | | Secrets encrypted at rest | in place | AES-256-GCM, key file outside the database and the main backups (=internal/seal=) | | |
| 1898 | 1924 | ``` |
| 1899 | 1925 | |
| 1900 | 1926 | `10-Known-Gaps.org`: delete the `#273` row. |
| @@ -1909,7 +1935,8 @@ If the file has no `* Unreleased` heading above the latest version, add one unde | ||
| 1909 | 1935 | replacing the binary, run =gitbayd admin secrets init= as root and |
| 1910 | 1936 | =chown gitbay:gitbay /etc/gitbay/secret.key= (=deploy/install.sh= does |
| 1911 | 1937 | both when the file is missing). The first start seals the stored |
| 1912 | secrets. Copy the key file off the host: backups do not carry it. | |
| 1938 | secrets. Back the key file up separately: =admin backup= archives do | |
| 1939 | not carry it (see the Admin wiki, "Secret key"). | |
| 1913 | 1940 | |
| 1914 | 1941 | - CI secrets, webhook secrets, mirror tokens and push device tokens are |
| 1915 | 1942 | stored sealed with AES-256-GCM (#273). =gitbayd admin secrets |
| @@ -2308,8 +2335,9 @@ New `** [backup]` section after `** [push]`: | ||
| 2308 | 2335 | =admin backup= encrypts every archive to them and appends =.age= to |
| 2309 | 2336 | its name. Generate the pair off the host with =age-keygen=; only the |
| 2310 | 2337 | public key goes here, so the host writes archives it cannot read. |
| 2311 | The restic copy is unaffected: it snapshots =/var/lib/gitbay=, not | |
| 2312 | the archives. | |
| 2338 | The restic copy is unaffected: the offsite job stages its own | |
| 2339 | =VACUUM INTO= of the live database and snapshots =/var/lib/gitbay=, | |
| 2340 | not the archives. | |
| 2313 | 2341 | ``` |
| 2314 | 2342 | |
| 2315 | 2343 | In `* Backup and restore`, after the `--verify` paragraph: |
| @@ -2333,7 +2361,7 @@ or on a restore host. | ||
| 2333 | 2361 | `06-Data-and-Cryptography.org`, At rest, Backups row: |
| 2334 | 2362 | |
| 2335 | 2363 | ```org |
| 2336 | | Backups | local archives age-encrypted when =[backup] age_recipients= is set; restic encrypts the offsite copy; neither carries the secret key file | | |
| 2364 | | Backups | local archives age-encrypted when =[backup] age_recipients= is set; restic encrypts the offsite copy; neither carries the secret key file, whose offsite copy is a separate keys repository | | |
| 2337 | 2365 | ``` |
| 2338 | 2366 | |
| 2339 | 2367 | `08-Operations.org`, Full archive and Database only rows: append `; age-encrypted when =[backup] age_recipients= is set` to Contents. |
| @@ -3085,12 +3113,15 @@ Add a new subsection after `** Secret key`: | ||
| 3085 | 3113 | ```org |
| 3086 | 3114 | ** Restore drill |
| 3087 | 3115 | |
| 3088 | A restore onto a clean host, run on a schedule and recorded below. | |
| 3089 | The disaster it rehearses is losing bay1, so the local archives are | |
| 3090 | gone with it and the sources are the offsite restic repository and | |
| 3091 | what the operator keeps off the host (=~/.config/gitbay/=: =offsite.env=, | |
| 3092 | =secret.key=, =config.toml=, =backup-identity.txt=). The steps are in | |
| 3093 | the data-at-rest plan's operator runbook | |
| 3116 | A restore onto a clean host, run quarterly and after any change to the | |
| 3117 | backup code (=cmd/gitbayd/backup.go=, the offsite job), and recorded | |
| 3118 | below. The disaster it rehearses is losing bay1, so the local archives | |
| 3119 | are gone with it and the sources are the main offsite restic | |
| 3120 | repository (repositories, LFS, the staged database, =config.toml=), | |
| 3121 | the keys repository (=secret.key=, =apns.p8=), and the operator's | |
| 3122 | password manager (=offsite.env=, the keys repository's password and | |
| 3123 | token, =backup-identity.txt=). The steps are in the data-at-rest | |
| 3124 | plan's operator runbook | |
| 3094 | 3125 | (=docs/plans/2026-09-27-data-at-rest-and-backup.md=). |
| 3095 | 3126 | |
| 3096 | 3127 | Time to service runs from the clean host's first root login to the |
| @@ -3101,7 +3132,11 @@ the time of the newest restic snapshot restored. | ||
| 3101 | 3132 | |------+------+-------------------------+-----------------+--------------+--------------+-----+----------------+----------+---------+-------| |
| 3102 | 3133 | ``` |
| 3103 | 3134 | |
| 3104 | - [ ] **Step 2: Architecture/08 and Threat-Model** | |
| 3135 | - [ ] **Step 2: Architecture/08, Known-Gaps and Threat-Model** | |
| 3136 | ||
| 3137 | `10-Known-Gaps.org:17`, the `#259` row's description becomes | |
| 3138 | `No restore has been exercised; the drill is written (Admin wiki) and not yet run`. | |
| 3139 | The row stays until the drill is recorded. | |
| 3105 | 3140 | |
| 3106 | 3141 | `08-Operations.org`: replace the `--verify` bullet (lines 60-62) with: |
| 3107 | 3142 | |
| @@ -3131,7 +3166,7 @@ Replace the "Recovery time" bullet with: | ||
| 3131 | 3166 | |
| 3132 | 3167 | ```bash |
| 3133 | 3168 | git add .gitbay/wiki |
| 3134 | git commit -S -m "wiki: backup verify, backup lock, restore drill record | |
| 3169 | git commit -S -m "wiki: backup verify, backup lock, restore drill procedure | |
| 3135 | 3170 | |
| 3136 | 3171 | Ref #259" |
| 3137 | 3172 | git push -u origin backup-verify-lock |
| @@ -3139,14 +3174,19 @@ gitbay mr create --source backup-verify-lock --target main --title "Backup verif | ||
| 3139 | 3174 | ``` |
| 3140 | 3175 | |
| 3141 | 3176 | Merge with `--strategy ff` after CI, delete the branch both places. |
| 3142 | #259 stays open until the drill below is recorded. | |
| 3177 | #259 stays open after this MR: every commit in it says `Ref #259`, and | |
| 3178 | only the drill record (runbook C, deferred) closes it. | |
| 3143 | 3179 | |
| 3144 | 3180 | --- |
| 3145 | 3181 | |
| 3146 | 3182 | # Operator runbook (cmc) |
| 3147 | 3183 | |
| 3148 | Run on bay1 and a clean host. Nothing here is automated by the MRs. | |
| 3149 | One forge write per shell call; bay1 root is `ssh -p 2222 root@gitbay.org`. | |
| 3184 | Run on bay1, the laptop and (for C) a clean host. Nothing here is | |
| 3185 | automated by the MRs. One forge write per shell call; bay1 root is | |
| 3186 | `ssh -p 2222 root@gitbay.org`. | |
| 3187 | ||
| 3188 | D does not depend on any MR and can run first; A.3 and C use the keys | |
| 3189 | repository it creates. | |
| 3150 | 3190 | |
| 3151 | 3191 | ## A. After MR 1 deploys |
| 3152 | 3192 | |
| @@ -3156,16 +3196,18 @@ One forge write per shell call; bay1 root is `ssh -p 2222 root@gitbay.org`. | ||
| 3156 | 3196 | → mode `-rw-------`, owner `gitbay gitbay`, one log line with a count. |
| 3157 | 3197 | 2. `ssh -p 2222 root@gitbay.org 'gitbayd --config /etc/gitbay/config.toml admin secrets check'` |
| 3158 | 3198 | → one `key <id>: N sealed` line, no `clear:` line. |
| 3159 | 3. Escrow: `scp -P 2222 root@gitbay.org:/etc/gitbay/secret.key ~/.config/gitbay/secret.key && chmod 600 ~/.config/gitbay/secret.key`. | |
| 3160 | Also copy `/etc/gitbay/config.toml` to `~/.config/gitbay/config.toml` | |
| 3161 | (mode 600) if no copy exists off the host; the drill needs it. | |
| 3199 | 3. Back the key up to the keys repository (D.9) with `secret.key` as | |
| 3200 | the file and `secret-key` as the tag. If D has not run yet, do D.2, | |
| 3201 | D.3 (keys bucket), D.4 (keys token) and D.9 first. After every | |
| 3202 | `admin secrets rotate`, repeat this step. | |
| 3162 | 3203 | 4. Check CI builds that use secrets (blotter, hutch, orgo) still run, |
| 3163 | 3204 | and a webhook delivery still verifies. |
| 3164 | 3205 | |
| 3165 | 3206 | ## B. After MR 2 deploys |
| 3166 | 3207 | |
| 3167 | 3208 | 1. On the laptop: `age-keygen -o ~/.config/gitbay/backup-identity.txt` |
| 3168 | (mode 600). Note the printed `age1...` public key. | |
| 3209 | (mode 600). Note the printed `age1...` public key. Put a copy of the | |
| 3210 | identity in the password manager. | |
| 3169 | 3211 | 2. On bay1, add to `/etc/gitbay/config.toml`: |
| 3170 | 3212 | ```toml |
| 3171 | 3213 | [backup] |
| @@ -3176,12 +3218,10 @@ One forge write per shell call; bay1 root is `ssh -p 2222 root@gitbay.org`. | ||
| 3176 | 3218 | `/usr/local/bin/gitbay-db-backup.sh` and `/usr/local/bin/gitbay-monitor.sh` |
| 3177 | 3219 | predate the cloud-init change (cloud-init runs once). Apply the same |
| 3178 | 3220 | glob edits as Task 2.3 Step 1 by hand. |
| 3179 | 4. Before relying on encryption, find how `/var/lib/gitbay-stage` (the | |
| 3180 | staged database restic copies) is produced. If it reads a local | |
| 3181 | archive, it must decrypt, which the host cannot; switch it to its own | |
| 3182 | `VACUUM INTO` or an unencrypted database-only snapshot kept under | |
| 3183 | `/var/lib/gitbay-stage` only. If it snapshots the live database | |
| 3184 | directly, nothing changes. | |
| 3221 | 4. The offsite job needs no change. `/usr/local/bin/gitbay-offsite` | |
| 3222 | builds `/var/lib/gitbay-stage` from live data (`sqlite3 gitbay.db | |
| 3223 | "VACUUM INTO ..."` and `cp -a /etc/gitbay/config.toml`), not from the | |
| 3224 | local archives, so encrypting the archives does not reach restic. | |
| 3185 | 3225 | 5. After the next hourly run: `ls -l /var/backups/gitbay/db | tail -2` |
| 3186 | 3226 | shows `.tar.gz.age`; the monitor's `db_snapshot_h` stays under 2. |
| 3187 | 3227 | Copy one archive to the laptop and run |
| @@ -3189,32 +3229,46 @@ One forge write per shell call; bay1 root is `ssh -p 2222 root@gitbay.org`. | ||
| 3189 | 3229 | (a local gitbayd build; `--verify` reads no config). |
| 3190 | 3230 | 6. Old unencrypted archives age out of the 7/48 rotation on their own. |
| 3191 | 3231 | |
| 3192 | ## C. Restore drill (after MR 3 deploys; closes #259) | |
| 3232 | ## C. Restore drill (deferred; closes #259) | |
| 3233 | ||
| 3234 | Deferred by the operator. Run it after MR 3 deploys and D is complete, | |
| 3235 | then quarterly, and after any change to `cmd/gitbayd/backup.go` or the | |
| 3236 | offsite job (`/usr/local/bin/gitbay-offsite`, its env file, the R2 lock | |
| 3237 | rules). Each run adds a row to the Admin page's table; the first one | |
| 3238 | closes #259. | |
| 3193 | 3239 | |
| 3194 | 3240 | Record every timestamp as you go. Start the clock at step 2. |
| 3195 | 3241 | |
| 3196 | 1. Pick the snapshot: `set -a; . ~/.config/gitbay/offsite.env; set +a; restic $RESTIC_OPTS snapshots --latest 1`. | |
| 3197 | Note its time (recovery point). `restic $RESTIC_OPTS ls latest /var/lib/gitbay-stage` | |
| 3198 | to find the staged database file name. | |
| 3242 | 1. On the laptop, export the `gitbay r2-prune` variables from the | |
| 3243 | password manager (D.5), then: | |
| 3244 | `restic snapshots --tag gitbay --latest 1`. Note its time (recovery | |
| 3245 | point). `restic ls latest /var/lib/gitbay-stage` shows the staged | |
| 3246 | database's file name and `config.toml`. | |
| 3199 | 3247 | 2. Provision a clean Ubuntu 24.04 host (throwaway VPS or local VM) with |
| 3200 | 3248 | `deploy/cloud-init.yaml`. First root login: **clock starts**. |
| 3201 | 3249 | 3. Before gitbayd ever starts, block outbound traffic so the restored |
| 3202 | 3250 | instance cannot send mail, deliver webhooks, push mirrors or call |
| 3203 | APNs: `ufw default deny outgoing; ufw allow out 53; ufw allow out to <restic endpoint> port 443; ufw reload`. | |
| 3204 | (Allow the restic endpoint only for the restore, then remove it.) | |
| 3205 | 4. Install restic, restore: `restic $RESTIC_OPTS restore latest --target / --include /var/lib/gitbay --include /var/lib/gitbay-stage`. | |
| 3206 | Replace `/var/lib/gitbay/gitbay.db` with the staged copy (the live | |
| 3207 | file in the snapshot may be mid-write); remove any `gitbay.db-wal` | |
| 3208 | and `gitbay.db-shm`. `chown -R gitbay:gitbay /var/lib/gitbay`. | |
| 3209 | 5. Config and key: copy `~/.config/gitbay/config.toml` to | |
| 3210 | `/etc/gitbay/config.toml` and `~/.config/gitbay/secret.key` to | |
| 3211 | `/etc/gitbay/secret.key` (`chown gitbay:gitbay`, mode 600). In the | |
| 3212 | config for the drill only: `site_url` to `http://<drill-ip>:8080`, | |
| 3251 | APNs: `ufw default deny outgoing; ufw allow out 53; ufw allow out to <account-id>.r2.cloudflarestorage.com port 443; ufw reload`. | |
| 3252 | (ufw resolves the name once; allow R2 only for the restore, then | |
| 3253 | remove the rule.) | |
| 3254 | 4. Install restic, and with the same variables exported on the drill | |
| 3255 | host: `restic restore latest --target / --include /var/lib/gitbay --include /var/lib/gitbay-stage`. | |
| 3256 | The snapshot excludes `gitbay.db`, `gitbay.db-wal`, `gitbay.db-shm` | |
| 3257 | and `hook.sock`: copy the staged database from | |
| 3258 | `/var/lib/gitbay-stage/` to `/var/lib/gitbay/gitbay.db`. | |
| 3259 | `chown -R gitbay:gitbay /var/lib/gitbay`. LFS objects are under | |
| 3260 | `/var/lib/gitbay/lfs` (inside `server.root`) and come back with it. | |
| 3261 | 5. Config and keys: copy `/var/lib/gitbay-stage/config.toml` to | |
| 3262 | `/etc/gitbay/config.toml`. From the laptop, with the `gitbay r2-keys` | |
| 3263 | variables exported (D.5): | |
| 3264 | ```sh | |
| 3265 | restic dump --tag secret-key latest /secret.key | ssh root@<drill-ip> -p 2222 'umask 077; cat > /etc/gitbay/secret.key; chown gitbay:gitbay /etc/gitbay/secret.key' | |
| 3266 | restic dump --tag apns latest /apns.p8 | ssh root@<drill-ip> -p 2222 'umask 077; cat > /etc/gitbay/apns.p8; chown gitbay:gitbay /etc/gitbay/apns.p8' | |
| 3267 | ``` | |
| 3268 | In the config for the drill only: `site_url` to `http://<drill-ip>:8080`, | |
| 3213 | 3269 | `[http] addr = ":8080"`, `tls = "off"`, remove `[mail]`, set |
| 3214 | 3270 | `registration.mode = "closed"`, `[push] enabled = false`, remove |
| 3215 | `[backup]` (step 7's archive is local and read back at once). If | |
| 3216 | `[lfs] root` is set outside `server.root`, note it: neither the | |
| 3217 | archive nor this restore carries those objects. | |
| 3271 | `[backup]` (step 7's archive is local and read back at once). | |
| 3218 | 3272 | 6. Install the gitbayd binary of the tag bay1 runs (`/healthz` names the |
| 3219 | 3273 | commit) with `deploy/install.sh <drill-ip> 2222`; it will not create |
| 3220 | 3274 | a key because one is present. |
| @@ -3226,7 +3280,8 @@ Record every timestamp as you go. Start the clock at step 2. | ||
| 3226 | 3280 | snapshot. |
| 3227 | 3281 | - Secrets: `gitbayd --config /etc/gitbay/config.toml admin secrets check` |
| 3228 | 3282 | → every value under one key, no error. The journal shows no |
| 3229 | `sealing secrets` failure. | |
| 3283 | `sealing secrets` failure. `sha256sum /etc/gitbay/apns.p8` equals | |
| 3284 | bay1's. | |
| 3230 | 3285 | - LFS: `cd /var/lib/gitbay/lfs && find . -type f | while read f; do [ "$(sha256sum < "$f" | cut -c1-64)" = "$(basename "$f")" ] || echo "BAD $f"; done` → no output; object count against bay1's. |
| 3231 | 3286 | - Release assets: every row's file exists with its digest: |
| 3232 | 3287 | ```sh |
| @@ -3251,33 +3306,322 @@ Record every timestamp as you go. Start the clock at step 2. | ||
| 3251 | 3306 | - `Architecture/08-Operations.org`: the "Recovery time" bullet names |
| 3252 | 3307 | the figure. |
| 3253 | 3308 | Branch `restore-drill-record`, one signed commit ending |
| 3254 | `Closes #259`, MR, ff merge, delete the branch. | |
| 3255 | 10. Destroy the drill host. Repeat the drill every quarter and after | |
| 3256 | any change to `cmd/gitbayd/backup.go` or the restic job, adding a | |
| 3257 | row each time. | |
| 3309 | `Closes #259`, MR, ff merge, delete the branch. Later drills add a | |
| 3310 | row with `Ref #259`. | |
| 3311 | 10. Destroy the drill host. | |
| 3312 | ||
| 3313 | ## D. Offsite backup to Cloudflare R2; keys repository | |
| 3314 | ||
| 3315 | Today: `gitbay-offsite.service` (timer nightly, about 00:19 UTC) runs | |
| 3316 | `/usr/local/bin/gitbay-offsite`, which sources `/etc/gitbay/offsite.env` | |
| 3317 | (`RESTIC_REPOSITORY` and the rest), builds `/var/lib/gitbay-stage` | |
| 3318 | with `sqlite3 gitbay.db "VACUUM INTO ..."` and `cp -a | |
| 3319 | /etc/gitbay/config.toml`, runs `restic backup --tag gitbay` of the stage | |
| 3320 | and `/var/lib/gitbay` excluding `gitbay.db`, `gitbay.db-wal`, | |
| 3321 | `gitbay.db-shm` and `hook.sock`, then `restic check` up to three | |
| 3322 | attempts. The target is Scaleway `fr-par` with append-only credentials; | |
| 3323 | forget and prune run from the laptop. `/etc/gitbay/apns.p8` and | |
| 3324 | `/etc/gitbay/offsite.env` are in no restic repository. | |
| 3325 | ||
| 3326 | After D: | |
| 3327 | ||
| 3328 | - The main repository is on R2, bucket `gitbay-offsite`. `config.toml` | |
| 3329 | stays in it through the stage, as today. | |
| 3330 | - `apns.p8` and `secret.key` are in a separate restic repository, | |
| 3331 | bucket `gitbay-keys`, with its own password and token. Neither the | |
| 3332 | password nor the token is ever on bay1: the laptop streams the files | |
| 3333 | out of bay1 into it, so a leak of the main backup or of bay1's | |
| 3334 | `offsite.env` does not reach the key that opens the sealed secrets. | |
| 3335 | - `offsite.env` stays on bay1 because the nightly job sources it. Its | |
| 3336 | only copy off bay1 is the password manager; neither repository | |
| 3337 | carries it. The keys repository's password and token exist only in | |
| 3338 | the password manager. | |
| 3339 | ||
| 3340 | What protects history changes. R2 has no append-only token (token | |
| 3341 | permissions are Admin R/W, Admin R, Object R/W, Object R, optionally | |
| 3342 | scoped to buckets), and restic needs to write and delete under | |
| 3343 | `locks/`, so the host holds Object R/W on its bucket. Bucket lock | |
| 3344 | rules refuse deletion and overwrite of matching objects for a period or | |
| 3345 | indefinitely, whatever the token, and changing them needs an Admin | |
| 3346 | token or the dashboard, neither of which is on bay1. So a compromised | |
| 3347 | host cannot delete or overwrite anything younger than the retention | |
| 3348 | period `RET`; unlike the Scaleway key, it can delete snapshots and data | |
| 3349 | older than `RET`. Prune from the laptop can likewise only remove what | |
| 3350 | is past `RET`. | |
| 3351 | ||
| 3352 | `RET` is 90 days in the commands below (open question 1). The lock | |
| 3353 | rules: | |
| 3354 | ||
| 3355 | | Prefix | Rule | Why | | |
| 3356 | |---|---|---| | |
| 3357 | | `data/` | `RET` days | pack files; prune deletes unused ones once past `RET` | | |
| 3358 | | `index/` | `RET` days | prune replaces index files (see D.6) | | |
| 3359 | | `snapshots/` | `RET` days | forget deletes snapshot files once past `RET` | | |
| 3360 | | `keys/` | indefinite | restic deletes a key file only on `restic key remove` | | |
| 3361 | | `config` | indefinite | written once at `restic init`; the repository cannot be opened without it | | |
| 3362 | | `locks/` | none | restic creates and deletes a lock on every command | | |
| 3363 | ||
| 3364 | `config` is not in the four prefixes named when this was decided; it | |
| 3365 | is one object that restic never deletes and without which nothing in | |
| 3366 | the repository opens, so it is locked too. With `keys/` indefinite, a | |
| 3367 | repository password change adds a key but cannot remove the old one. | |
| 3368 | ||
| 3369 | The keys bucket uses `--retention-indefinite` on every prefix except | |
| 3370 | `locks/` and is never forgotten or pruned: a retired secret key still | |
| 3371 | opens the database in every main snapshot sealed under it, and the | |
| 3372 | snapshots are a few hundred bytes. | |
| 3373 | ||
| 3374 | 1. File the issue the wiki MR (D.14) references: | |
| 3375 | `gitbay issue create krz/gitbay --title "Offsite backup: move from Scaleway to Cloudflare R2" --body "Runbook D of docs/plans/2026-09-27-data-at-rest-and-backup.md: R2 with bucket lock rules, a separate keys repository, Scaleway retired after 30 nights in parallel."` | |
| 3376 | Note the number. | |
| 3377 | 2. Buckets, from the laptop (`wrangler login` as the account owner): | |
| 3378 | ```sh | |
| 3379 | npx wrangler r2 bucket create gitbay-offsite-scratch --location weur | |
| 3380 | npx wrangler r2 bucket create gitbay-offsite --location weur | |
| 3381 | npx wrangler r2 bucket create gitbay-keys --location weur | |
| 3382 | ``` | |
| 3383 | `weur` is a location hint that keeps the data in western Europe as | |
| 3384 | Scaleway `fr-par` did. | |
| 3385 | 3. Lock rules. Scratch uses one day so D.6 can see a rule expire; | |
| 3386 | `gitbay-offsite` uses `RET`: | |
| 3387 | ```sh | |
| 3388 | lock() { # bucket retention-flag... | |
| 3389 | b=$1; shift | |
| 3390 | for p in data index snapshots; do | |
| 3391 | npx wrangler r2 bucket lock add "$b" --name "restic-$p" --prefix "$p/" "$@" | |
| 3392 | done | |
| 3393 | npx wrangler r2 bucket lock add "$b" --name restic-keys --prefix keys/ --retention-indefinite | |
| 3394 | npx wrangler r2 bucket lock add "$b" --name restic-config --prefix config --retention-indefinite | |
| 3395 | npx wrangler r2 bucket lock list "$b" | |
| 3396 | } | |
| 3397 | lock gitbay-offsite-scratch --retention-days 1 | |
| 3398 | lock gitbay-offsite --retention-days 90 | |
| 3399 | lock gitbay-keys --retention-indefinite | |
| 3400 | ``` | |
| 3401 | → five rules on each bucket, none covering `locks/`. No other object | |
| 3402 | in a restic repository starts with `config`, so that prefix matches | |
| 3403 | the one object. | |
| 3404 | 4. Tokens, in the dashboard (R2 → Manage API tokens), each "Object Read | |
| 3405 | & Write" and scoped to one bucket: | |
| 3406 | - `gitbay-host` → `gitbay-offsite`. Goes to bay1. | |
| 3407 | - `gitbay-prune` → `gitbay-offsite`. Laptop only: forget, prune, | |
| 3408 | check, restore. | |
| 3409 | - `gitbay-keys` → `gitbay-keys`. Laptop only. | |
| 3410 | - `gitbay-scratch` → `gitbay-offsite-scratch`. Laptop only; revoke | |
| 3411 | after D.6. | |
| 3412 | `gitbay-host` and `gitbay-prune` have the same rights; they are | |
| 3413 | separate so either can be revoked alone. The lock rules, not the | |
| 3414 | token, protect history. Store each access key id and secret in the | |
| 3415 | password manager. | |
| 3416 | 5. One password manager entry per repository, each holding the | |
| 3417 | variables restic reads: | |
| 3418 | ```sh | |
| 3419 | RESTIC_REPOSITORY=s3:https://<account-id>.r2.cloudflarestorage.com/<bucket> | |
| 3420 | RESTIC_PASSWORD=<openssl rand -base64 32, generated once per repository> | |
| 3421 | AWS_ACCESS_KEY_ID=<token access key id> | |
| 3422 | AWS_SECRET_ACCESS_KEY=<token secret access key> | |
| 3423 | AWS_DEFAULT_REGION=auto | |
| 3424 | ``` | |
| 3425 | Entries: `gitbay r2-host` (bucket `gitbay-offsite`, token | |
| 3426 | `gitbay-host`), `gitbay r2-prune` (same bucket and password, token | |
| 3427 | `gitbay-prune`), `gitbay r2-keys` (bucket `gitbay-keys`, its own | |
| 3428 | password, token `gitbay-keys`), `gitbay r2-scratch`. If the current | |
| 3429 | `offsite.env` sets anything else (`RESTIC_OPTS`, which the Admin | |
| 3430 | wiki's commands use), carry it into `r2-host` and `r2-prune`. The | |
| 3431 | commands below assume the named entry's variables are exported in | |
| 3432 | the shell. | |
| 3433 | 6. Validate on the scratch bucket (`gitbay r2-scratch`), from the | |
| 3434 | laptop, before anything real depends on it. Record each result in | |
| 3435 | the issue from D.1. | |
| 3436 | ```sh | |
| 3437 | restic init | |
| 3438 | mkdir -p /tmp/r2t && head -c 50M /dev/urandom > /tmp/r2t/a | |
| 3439 | restic backup --tag gitbay /tmp/r2t # snapshot 1 | |
| 3440 | head -c 50M /dev/urandom > /tmp/r2t/b | |
| 3441 | restic backup --tag gitbay /tmp/r2t # snapshot 2 | |
| 3442 | rm /tmp/r2t/a | |
| 3443 | restic backup --tag gitbay /tmp/r2t # snapshot 3 | |
| 3444 | restic check # passes; lock files come and go | |
| 3445 | restic forget --keep-last 1 # expect a refusal: snapshots 1-2 are younger than a day | |
| 3446 | restic check | |
| 3447 | restic prune --max-unused unlimited # record what it deletes or is refused | |
| 3448 | restic check | |
| 3449 | ``` | |
| 3450 | Expected on day 0: backup and check succeed (locks/ is unlocked); | |
| 3451 | forget fails to delete the two snapshot files and says so; check | |
| 3452 | still passes afterwards. Record whether prune tries to delete an | |
| 3453 | index file and fails. After 24 hours: | |
| 3454 | ```sh | |
| 3455 | restic forget --keep-last 1 | |
| 3456 | restic prune --max-unused unlimited | |
| 3457 | restic check --read-data | |
| 3458 | ``` | |
| 3459 | → forget removes snapshots 1 and 2, prune removes the pack holding | |
| 3460 | only `a`'s data and the superseded index files, and `check | |
| 3461 | --read-data` passes. Then `restic unlock` succeeds on a stale lock | |
| 3462 | left by interrupting a backup with Ctrl-C. | |
| 3463 | If prune on day 0 or day 1 fails on an index file younger than the | |
| 3464 | rule, restic's index rewrite is deleting files the lock keeps, and | |
| 3465 | prune would fail every run on the real bucket. Stop there and bring | |
| 3466 | the result back: the choice is between an index rule shorter than | |
| 3467 | the data rule, running prune only when no index file is younger | |
| 3468 | than `RET`, or leaving `index/` unlocked (it can be rebuilt from | |
| 3469 | `data/` with `restic repair index`). Do not continue to D.7 until | |
| 3470 | one is chosen. | |
| 3471 | `--max-unused unlimited` keeps prune from repacking partly used | |
| 3472 | packs, since a repack deletes the old pack and that pack may be | |
| 3473 | younger than `RET`. | |
| 3474 | 7. Initialise the real repositories from the laptop: | |
| 3475 | `restic init` with `gitbay r2-prune` exported, and again with | |
| 3476 | `gitbay r2-keys`. | |
| 3477 | 8. Install the host env file and run both targets. On bay1, paste the | |
| 3478 | `gitbay r2-host` entry into the file over stdin (Ctrl-D ends it): | |
| 3479 | ```sh | |
| 3480 | ssh -p 2222 root@gitbay.org 'umask 077; cat > /etc/gitbay/offsite-r2.env' | |
| 3481 | ``` | |
| 3482 | Keep the current script as `/usr/local/bin/gitbay-offsite.scaleway` | |
| 3483 | (`cp -a`). Edit `/usr/local/bin/gitbay-offsite`: leave the staging | |
| 3484 | lines (`VACUUM INTO`, `cp -a config.toml`) as they are and run once; | |
| 3485 | move the `. /etc/gitbay/offsite.env`, the `restic backup` and the | |
| 3486 | `restic check` retry loop into a function called once per env file, | |
| 3487 | each call in a subshell so one target's variables cannot reach the | |
| 3488 | other. With the flags the job uses today, the section after staging | |
| 3489 | reads: | |
| 3490 | ```sh | |
| 3491 | target() { | |
| 3492 | set -a; . "$1"; set +a | |
| 3493 | restic backup --tag gitbay \ | |
| 3494 | --exclude /var/lib/gitbay/gitbay.db \ | |
| 3495 | --exclude /var/lib/gitbay/gitbay.db-wal \ | |
| 3496 | --exclude /var/lib/gitbay/gitbay.db-shm \ | |
| 3497 | --exclude /var/lib/gitbay/hook.sock \ | |
| 3498 | /var/lib/gitbay-stage /var/lib/gitbay || return 1 | |
| 3499 | for i in 1 2 3; do | |
| 3500 | restic check && return 0 | |
| 3501 | done | |
| 3502 | return 1 | |
| 3503 | } | |
| 3504 | rc=0 | |
| 3505 | (target /etc/gitbay/offsite.env) || rc=1 | |
| 3506 | (target /etc/gitbay/offsite-r2.env) || rc=1 | |
| 3507 | exit $rc | |
| 3508 | ``` | |
| 3509 | Compare with `diff -u /usr/local/bin/gitbay-offsite.scaleway /usr/local/bin/gitbay-offsite` | |
| 3510 | and keep anything the current restic lines carry that this section | |
| 3511 | does not (`$RESTIC_OPTS`, a sleep between check attempts, the exact | |
| 3512 | exclude spelling). A failure on one target no longer stops the | |
| 3513 | other; the unit still fails if either did. The job now uploads | |
| 3514 | twice: check `TimeoutStartSec` in `systemctl cat gitbay-offsite.service` | |
| 3515 | against twice the last run's duration | |
| 3516 | (`journalctl -u gitbay-offsite -n 200`). | |
| 3517 | 9. Keys repository, from the laptop with `gitbay r2-keys` exported. | |
| 3518 | The file goes from bay1 to R2 through a pipe and is never written on | |
| 3519 | the laptop: | |
| 3520 | ```sh | |
| 3521 | ssh -p 2222 root@gitbay.org cat /etc/gitbay/apns.p8 | restic backup --stdin --stdin-filename apns.p8 --tag apns | |
| 3522 | restic dump --tag apns latest /apns.p8 | sha256sum | |
| 3523 | ssh -p 2222 root@gitbay.org sha256sum /etc/gitbay/apns.p8 | |
| 3524 | ``` | |
| 3525 | → the two digests match. After MR 1 deploys (A.3) and after every | |
| 3526 | rotation, the same with `secret.key` and `--tag secret-key`. After | |
| 3527 | an APNs key change, the same with `apns.p8`. | |
| 3528 | 10. First full backup: `ssh -p 2222 root@gitbay.org 'systemctl start gitbay-offsite.service; journalctl -u gitbay-offsite -n 50 --no-pager'` | |
| 3529 | → both targets back up and check. From the laptop with | |
| 3530 | `gitbay r2-prune`: `restic snapshots` shows one `gitbay` snapshot | |
| 3531 | with the stage and `/var/lib/gitbay`; `restic ls latest /var/lib/gitbay-stage` | |
| 3532 | lists the database copy and `config.toml`. | |
| 3533 | 11. Run both targets for 30 nights. Each week, from the laptop with | |
| 3534 | `gitbay r2-prune`: `restic snapshots --tag gitbay --latest 7` (one | |
| 3535 | per night) and `restic check --read-data-subset 1/4`, a different | |
| 3536 | quarter each week. At the end: `restic check --read-data` on R2 | |
| 3537 | (R2 does not charge egress). | |
| 3538 | 12. Laptop forget and prune for R2, from the laptop with | |
| 3539 | `gitbay r2-prune`: the forget policy used for Scaleway today plus | |
| 3540 | `--keep-within 90d`, so no snapshot younger than `RET` is ever | |
| 3541 | forgotten, then `restic prune --max-unused unlimited` (or what D.6 | |
| 3542 | settled on). Nothing is removable before day 90; the first prune | |
| 3543 | that deletes anything is after that. | |
| 3544 | 13. Retire Scaleway after the 30 nights and a clean `check --read-data`: | |
| 3545 | - On bay1: `mv /etc/gitbay/offsite-r2.env /etc/gitbay/offsite.env` | |
| 3546 | (the Scaleway file is replaced) and drop the second `target` call | |
| 3547 | from the script, so the job runs `(target /etc/gitbay/offsite.env)` | |
| 3548 | alone. Run the unit once and check the journal. | |
| 3549 | - Replace the `offsite.env` copy in the password manager with | |
| 3550 | `gitbay r2-host`; remove the Scaleway entries once the bucket is | |
| 3551 | gone. | |
| 3552 | - Revoke the Scaleway append-only key. Keep the Scaleway bucket | |
| 3553 | until R2 holds 90 days of snapshots, then delete it with the | |
| 3554 | Scaleway owner credentials from the laptop. | |
| 3555 | - Revoke `gitbay-scratch`. The scratch bucket's `keys/` and | |
| 3556 | `config` rules are indefinite: remove its five rules | |
| 3557 | (`npx wrangler r2 bucket lock remove gitbay-offsite-scratch --name <rule>`), | |
| 3558 | then empty and delete the bucket in the dashboard. | |
| 3559 | 14. Wiki MR, branch `offsite-r2-wiki`, commit ending `Ref #<D.1 issue>` | |
| 3560 | (and `Closes #<D.1 issue>` if the move is finished): | |
| 3561 | - `Admin.org`, `** Offsite copies`, the first paragraph becomes: | |
| 3562 | ```org | |
| 3563 | bay1 also takes a nightly restic snapshot of =/var/lib/gitbay= and | |
| 3564 | =/var/lib/gitbay-stage= (a =VACUUM INTO= copy of the database and | |
| 3565 | =config.toml=) to a Cloudflare R2 bucket. R2 has no append-only | |
| 3566 | token, so the host's token can write and delete objects; bucket | |
| 3567 | lock rules refuse deletion and overwrite of =data/=, =index/= and | |
| 3568 | =snapshots/= for 90 days, and of =keys/= and =config= for good, | |
| 3569 | whatever the token. Changing the rules needs an Admin token or | |
| 3570 | the dashboard, neither of which is on bay1. A compromised host | |
| 3571 | cannot remove anything younger than 90 days; it can remove older | |
| 3572 | snapshots. Forgetting, pruning and rewriting run from the | |
| 3573 | operator's machine, and can likewise only remove what is past 90 | |
| 3574 | days. | |
| 3575 | ||
| 3576 | =apns.p8= and =secret.key= are in a separate restic repository in | |
| 3577 | its own bucket, with its own password and token, neither of which | |
| 3578 | is on bay1; the operator streams the files into it from bay1. A | |
| 3579 | leak of the main backup therefore does not carry the key that | |
| 3580 | opens its secrets. =offsite.env= is in neither repository. | |
| 3581 | ``` | |
| 3582 | - `Admin.org`, `*** Removing a repository's history from every | |
| 3583 | snapshot`: after the sentence ending "rather than forgetting the | |
| 3584 | snapshots: everything else in them stays restorable.", add | |
| 3585 | "Snapshots and packs younger than 90 days are locked: | |
| 3586 | =rewrite --forget= and =prune= cannot remove them until they | |
| 3587 | age out." The `~/.config/gitbay/offsite.env` sourcing line | |
| 3588 | becomes "export the =gitbay r2-prune= entry from the password | |
| 3589 | manager". | |
| 3590 | - `Architecture/08-Operations.org:53`: "to object storage" → "to | |
| 3591 | Cloudflare R2"; lines 63-65 become "The host's R2 token can | |
| 3592 | delete, but bucket lock rules keep everything younger than 90 | |
| 3593 | days; the lock rules are changed only off the host | |
| 3594 | (documented: Admin wiki)." | |
| 3595 | - `Architecture/09-Controls.org:101`: | |
| 3596 | `| Backups offsite and delete-locked | in place | restic to R2; bucket lock rules, 90 days on data, index and snapshots (documented) |` | |
| 3597 | - `Architecture/04-Trust-Boundaries.org:30`: "append-only offsite | |
| 3598 | backup credentials" → "offsite backup under R2 bucket locks". | |
| 3599 | - `Threat-Model.org:195`: "restic append-only credentials" → | |
| 3600 | "restic under R2 bucket locks". | |
| 3601 | - `Architecture/diagrams/diagrams.py:165`: `"restic · append-only key"` | |
| 3602 | → `"restic · R2 bucket locks"`; regenerate with | |
| 3603 | `python3 .gitbay/wiki/Architecture/diagrams/diagrams.py .gitbay/wiki/Architecture/diagrams` | |
| 3604 | and commit the SVGs. | |
| 3605 | - If MR 1 landed without the keys-repository sentences (Task 1.6), | |
| 3606 | add them to `Admin.org` `** Secret key` and | |
| 3607 | `Architecture/06-Data-and-Cryptography.org` here. | |
| 3258 | 3608 | |
| 3259 | 3609 | --- |
| 3260 | 3610 | |
| 3261 | 3611 | ## Open questions |
| 3262 | 3612 | |
| 3263 | 1. How `/var/lib/gitbay-stage` is populated on bay1 is not in the | |
| 3264 | repository. If the staging step reads the local archives, enabling | |
| 3265 | `age_recipients` breaks the restic path (runbook B.4 checks this | |
| 3266 | before it matters). | |
| 3267 | 2. Is `/etc/gitbay/config.toml` kept anywhere off the host today? The | |
| 3268 | repository does not say; runbook A.3 creates a copy, and the drill | |
| 3269 | depends on it. | |
| 3270 | 3. Drill cadence: the plan proposes quarterly and after backup changes; | |
| 3271 | #259 says only "on a schedule". | |
| 3272 | 4. Where the clean host runs (throwaway VPS or local VM) is left to the | |
| 3273 | operator; the runbook works for either. | |
| 3274 | 5. `lfs.root` on bay1: if it points outside `server.root`, LFS objects | |
| 3275 | are in neither the archive nor the restic snapshot of | |
| 3276 | `/var/lib/gitbay`. The drill records it; fixing it is not in this | |
| 3277 | plan. | |
| 3278 | 6. Out of scope, noted while reading: `webhook add` takes `--secret` | |
| 3613 | 1. The lock retention `RET`: the runbook uses 90 days. It is the | |
| 3614 | window a compromised host cannot touch and the minimum age of | |
| 3615 | anything prune can remove, so it should be at least as long as the | |
| 3616 | shortest period the current forget policy keeps (that policy is on | |
| 3617 | the laptop, not in the repository). | |
| 3618 | 2. Out of scope, noted while reading: `webhook add` takes `--secret` | |
| 3279 | 3619 | on argv (`internal/control/webhook.go:16-23`), against the |
| 3280 | 3620 | stdin-only rule for secrets. |
| 3621 | 3. Out of scope: the offsite job (`/usr/local/bin/gitbay-offsite`, its | |
| 3622 | unit and timer) is not in the repository and `deploy/cloud-init.yaml` | |
| 3623 | does not template it, so a host built from cloud-init has no offsite | |
| 3624 | backup until the operator installs one by hand. | |
| 3281 | 3625 | |
| 3282 | 3626 | ## Self-review |
| 3283 | 3627 | |
| @@ -3286,14 +3630,23 @@ Record every timestamp as you go. Start the clock at step 2. | ||
| 3286 | 3630 | `rotate`), existing clear rows sealed at startup (1.4 `serve`, |
| 3287 | 3631 | `ResealSecrets`), missing key behaviour (1.4 `openStore`, documented |
| 3288 | 3632 | 1.6), backups do not carry it (validation 1.2, e2e 1.5), install |
| 3289 | provisioning (1.6). | |
| 3633 | provisioning (1.6), offsite copy only in the separate keys repository | |
| 3634 | (runbook A.3, D.9). | |
| 3290 | 3635 | - #274: age recipients config (2.1), encryption (2.2), `--verify |
| 3291 | --identity` (2.2), restic path unaffected by the archives and checked | |
| 3292 | for its staging step (runbook B.4), scripts (2.3). | |
| 3636 | --identity` (2.2), restic path unaffected by the archives because the | |
| 3637 | offsite job stages from live data (runbook B.4), scripts (2.3). | |
| 3293 | 3638 | - #259: `--verify` connectivity (3.1, 3.4), delete/rename/transfer held |
| 3294 | 3639 | (3.2, 3.3, 3.4), drill covering database integrity, connectivity, |
| 3295 | LFS, release assets, config, host keys, secrets and the key, with time | |
| 3296 | to service on the Admin page (3.5, runbook C). | |
| 3640 | LFS, release assets, config, host keys, secrets and the keys, with time | |
| 3641 | to service on the Admin page (3.5, runbook C). The drill is deferred; | |
| 3642 | MR 3 says `Ref #259` and #259 stays open until the first drill is | |
| 3643 | recorded. Cadence: quarterly and after any backup code change. | |
| 3644 | - Offsite move: R2 buckets, bucket-scoped tokens for host, prune and | |
| 3645 | keys, lock rules on `data/`, `index/`, `snapshots/`, `keys/` (and | |
| 3646 | `config`) with `locks/` open, validation on a scratch bucket with the | |
| 3647 | same rules, both targets in parallel for 30 nights, Scaleway retired, | |
| 3648 | wiki MR (runbook D). No code MR: the offsite job is not in the | |
| 3649 | repository or in `deploy/cloud-init.yaml`. | |
| 3297 | 3650 | - Names used across tasks: `seal.Keyring`/`Load`/`Seal`/`Open`/ |
| 3298 | 3651 | `CurrentID`/`KeyID`/`IsSealed`/`NewKey`/`ReadKeys`/`WriteKeys`; |
| 3299 | 3652 | `Store.SetKeyring`/`ResealSecrets`/`SecretKeyUse`; `testConfig`; |
docs/plans/2026-09-27-server-hardening.md +14 −13
| @@ -3636,28 +3636,29 @@ steps are run by hand. Operator ssh is `ssh -p 2222 root@gitbay.org`. | ||
| 3636 | 3636 | - limits: new `pack_*` settings with non-zero defaults; a burst of |
| 3637 | 3637 | clones now queues and, past the queue, is refused with 503 / exit 1. |
| 3638 | 3638 | |
| 3639 | # Open questions | |
| 3639 | # Decisions and remaining questions | |
| 3640 | ||
| 3641 | Decided 2026-09-28: | |
| 3642 | ||
| 3643 | - **receive-pack stays outside the pack limit.** | |
| 3644 | - **bay1's relay and git**, checked: git 2.47.3 (`http.curloptResolve` | |
| 3645 | needs 2.37); relay is AWS mail manager on port 587 and negotiates | |
| 3646 | STARTTLS (TLS 1.3, certificate verified). MRs 2 and 3 are not blocked; | |
| 3647 | runbook steps 1 and 2 stay as the check for other operators. | |
| 3648 | ||
| 3649 | Remaining: | |
| 3640 | 3650 | |
| 3641 | 3651 | 1. **System SSH mode.** With `ssh.mode = "system"` every session is a |
| 3642 | 3652 | separate `gitbayd shell` process, so (a) the pack limiter cannot |
| 3643 | 3653 | count SSH clones across sessions — this plan passes `nil` and says |
| 3644 | 3654 | so on the Admin page — and (b) audit rows written there are not |
| 3645 | 3655 | copied to the journal, because that process's stderr is the SSH |
| 3646 | client. The same applies to host `gitbayd admin …` commands (stderr | |
| 3647 | is the operator's terminal). Options: file-lock slots under | |
| 3648 | `<root>/packslots/` for (a), and `log/syslog` (journald collects it) | |
| 3649 | for (b). gitbay.org runs embedded mode, so neither is needed there. | |
| 3650 | Decide whether system mode needs them. | |
| 3656 | client. The same applies to host `gitbayd admin …` commands. gitbay.org | |
| 3657 | runs embedded mode; the plan documents the limit and adds nothing | |
| 3658 | for system mode. | |
| 3651 | 3659 | 2. **Default pack limits.** 3 / 2 / 32 / 60s are an estimate from the |
| 3652 | 3660 | one measured full clone (~1.5 cores). The runbook's benchmark |
| 3653 | 3661 | decides whether they stand. |
| 3654 | 3. **receive-pack and the limit.** index-pack on a large push is also | |
| 3655 | CPU-bound, but pushes need an account with write access and | |
| 3656 | killing or queueing receive-pack risks post-receive. This plan keeps | |
| 3657 | pushes outside the limit. Confirm. | |
| 3658 | 4. **bay1's mail relay and git version** are not visible from the | |
| 3659 | source tree; runbook steps 1 and 2 answer them before the matching | |
| 3660 | MRs merge. | |
| 3661 | 3662 | |
| 3662 | 3663 | # Self-review |
| 3663 | 3664 | |