docs/plans/2026-09-28-followups.md
2628 lines · 91107 bytes
1# Review follow-ups implementation plan
2
3> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4
5**Goal:** Close #287, #284, #298, #285 and #297: the runner drop-in's
6comment names the repositories the runner is really attached to;
7`webhook add` takes its signing secret on stdin only; `repo import
8--from` fetches only over http(s) from a resolved, checked and pinned
9address; an LFS transfer token dies with the SSH key that obtained it;
10and a browser session mints credentials only within 15 minutes of
11signing in.
12
13**Architecture:** No migrations. Four MRs on independent branches off
14`main`. The resolve-check-pin logic mirror sync gained in #279 moves
15out of `internal/mirror` into a new `internal/gitpin` package, which
16mirror sync and `repo import` both call (`internal/control` cannot
17import `internal/mirror`: mirror imports control). LFS tokens gain a
18key id in their signed payload, and `httpd.lfsAuth` checks it with
19`store.LiveSSHKeys`, the liveness query #256 added. For #297,
20`web_sessions.created_at` is the sign-in time (only `/login?token=`
21creates a session, and the idle renewal from #276 never writes
22`created_at`), so `store.WebSessionUser` returns it on the user,
23every web dispatch copies it into a new `Ctx.SignedInAt`, and
24`control.runChecked` refuses a `MintsCredential` command when it is
25older than `control.ReauthWindow`, next to the #257 `Expires` check.
26SSH and the API never set `SignedInAt`, so they are untouched.
27
28**Tech stack:** Go 1.27, SQLite (modernc), `golang.org/x/crypto/ssh`,
29cobra, git ≥ 2.37 (`http.curloptResolve`), git-lfs (e2e only).
30
31**Spec:** none — the issue texts of #284, #285, #287, #297 and #298 on
32krz/gitbay, and the decisions below, are the spec.
33
34## Global constraints
35
36- Four MRs, each on its own branch off `main`, in a fresh worktree
37 (`git worktree add ../gitbay-<branch> -b <branch> main`); the
38 checkout may hold another session's edits.
39- Commits are signed (the repository refuses unsigned ones); messages
40 reference the issue they touch (`Ref #N`), and the commit that
41 finishes an issue says `Closes #N`. No attribution to any assistant,
42 model or AI anywhere: commits, MR bodies, comments.
43- MR: `gitbay mr create --source <branch> --target main --title "..."`;
44 merge with `gitbay mr merge <n> --strategy ff` once CI is green
45 (this repository requires signed commits, so `squash`/`merge` are
46 refused), then delete the branch locally and on the remote. If the
47 merge reports the branch is behind, rebase onto `main`, force-push,
48 merge again.
49- Locally: `go build ./...`, `go vet ./...`, the unit tests of every
50 touched package, and at most the one e2e test named in the task
51 (`go test ./e2e -run TestName -count=1`). CI on bay1 runs the full
52 suite.
53- No new migrations; the next free number stays 0067.
54- Registries that fail CI when a new thing lacks its row: a top-level
55 route word in `internal/policy/names.go`; a new page template in the
56 width map of `TestMainWidthClass` (`internal/web/web_test.go`); a new
57 `ReadOnly` command in `readArgs` in `e2e/readonly_test.go`; a new
58 control command needs a `pass()` entry in `cmd/gitbay/main.go`
59 (`cmd/gitbay/coverage_test.go`); a command that reads stdin needs
60 `ReadsStdin: true`; a changed `Summary` needs
61 `go test ./cmd/gitbay -run TestSummariesAreCurrent -update`
62 (`cmd/gitbay/summaries_gen.go`). This plan adds no route, no
63 template, no command and changes no summary; it sets `ReadsStdin` on
64 `webhook add` (Task 1.2).
65- Secrets travel on stdin, never argv; never logged or echoed.
66- Wiki pages live in `.gitbay/wiki/`. Update the page in the MR that
67 changes the behaviour it describes; close the matching row in
68 `Architecture/10-Known-Gaps.org` and update the row in
69 `Architecture/09-Controls.org`. Parity is not affected by any of the
70 four MRs (webhooks have no web form; no capability changes surface).
71- `CHANGELOG.org`: each MR's last task appends its entries at the end
72 of the `* Unreleased` bullet list, directly above
73 `* v1.36.0 — 2026-09-23`.
74- Writing style: plain, direct, no hype; code comments match the
75 surrounding density; no before/after narration in comments or docs.
76
77## Decisions (binding, from the brief)
78
79- #287: correct the comment in `deploy/gitbay-runner.override.conf`:
80 `cmc/ci-smoke` no longer exists; the runner is attached to krz/gitbay,
81 krz/hutch, krz/keycask, krz/orgo, krz/skunky-art and cmc/cleberg.net;
82 a scratch repository is created and attached for runner validation.
83- #284: `webhook add` takes the secret from stdin only (`--secret -`,
84 `ReadsStdin: true`); a literal `--secret <value>` is refused with a
85 message saying to pipe it, as `repo secret set` does.
86- #298: `repo import --from` accepts http and https only; `git://` and
87 anything else is refused with a message to use the https URL. Import
88 gets the same resolve, check and pin as mirror sync, through one
89 shared helper.
90- #285: LFS transfer tokens carry the SSH key id (deploy keys
91 included); each LFS request that presents a token checks the key
92 still exists, is unexpired, and its account is not disabled. Tokens
93 minted before the upgrade (no key id) are refused.
94- #297: a `MintsCredential` command dispatched from a web session is
95 refused unless the session signed in within the last 15 minutes; the
96 web shows the form again with the message and a "Sign in again" link
97 that returns to the form. The check lives in `control.runChecked`.
98
99## Findings from the code that shape this plan
100
101- `repo secret set` takes no value argument at all
102 (`internal/control/build.go:426-451`): a third argument is a usage
103 error, and an empty stdin is refused with
104 `no value on stdin (pipe it: printf %s TOKEN | ...)`. `webhook add`
105 mirrors both refusals, as exit 2.
106- No web handler builds `webhook add` (Parity: webhooks, web `no`), so
107 #284 changes only the command, the CLI passthrough, one e2e call
108 (`e2e/webhook_test.go:115-116`) and `API.org:153`.
109- The CLI forwards stdin for a `stdinOK` command only when `usesStdin`
110 (`cmd/gitbay/main.go:358-368`) sees `--file -`, `--key -` or
111 `--token-stdin`; it must learn `--secret -`.
112- Deploy keys are `ssh_keys` rows with scope `deploy:<repo>:ro|rw`
113 (`internal/policy/access.go:60-79`). `git-lfs-authenticate` is
114 reached from `sshd.Exec` (`internal/sshd/sshd.go:492-499`), which
115 holds the authenticating `store.SSHKey`, so its `ID` identifies user
116 keys and deploy keys alike, and `store.LiveSSHKeys`
117 (`internal/store/revoke.go:36-62`) already answers "registered,
118 unexpired, account not disabled" for both.
119- `lfsBatch` (`internal/httpd/lfs.go:130`) mints transfer tokens for
120 anonymous batch requests on public repositories too. Those carry key
121 id 0, and `lfsAuth` honours a key-0 token only as an anonymous
122 download of a public repository.
123- A pre-upgrade LFS token's payload has three fields
124 (`repo:op:exp`, `internal/lfs/lfs.go:130`); the new payload has four,
125 so `Verify` refuses the old shape by field count. The server returns
126 `expires_in` with every token; nothing in `internal/lfs` makes a
127 client retry after a refusal. git-lfs obtains a token per process
128 from `git-lfs-authenticate`, so only a transfer in flight across the
129 deploy fails; the upgrade note says so.
130- `web_sessions.created_at` defaults to insert time
131 (`0001_init.up.sql:192-197`), `CreateWebSession` is called only from
132 `/login?token=` (`internal/httpd/accounts.go:153`), and
133 `WebSessionUser`'s renewal updates `last_used_at` and `expires_at`
134 only (`internal/store/sessions.go:98-100`). `created_at` is the
135 sign-in time; no column is needed.
136- Web dispatch builds a `control.Ctx` in five places
137 (`internal/httpd/control.go:35, 59, 105, 147, 188`), each from a
138 `store.User` and nothing else. Carrying the sign-in time on the user
139 that `viewer` returns reaches all five without changing a handler
140 signature; one `webCtx` helper replaces the five literals so none can
141 miss the field.
142- Web forms that dispatch a `MintsCredential` command:
143 `keys add`, `email verify`, `token create` (`/settings`,
144 `internal/httpd/account.go:224, 277, 309`) and `repo runner add`
145 (repository settings, `internal/httpd/settings.go:193`). The account
146 page reports a refusal through the flash cookie and a redirect; the
147 repository settings page re-renders the form with the notice.
148- `setNext` (`internal/httpd/flash.go:52`) sets the `gitbay_next`
149 cookie that `/login?token=` follows after creating a session
150 (`accounts.go:158`); the login page names the destination.
151- `repo import`'s unit-level harness: `newQueueTestRepo` and
152 `pruneCtx` (`internal/control/build_test.go:49`,
153 `mrprune_test.go:68`). `pruneCtx` leaves `Limits.CloneTimeoutSec`
154 at 0, which is an already-expired context; the import tests set it.
155- `TestRepoImport` (`e2e/import_test.go`) imports from its own
156 instance at `127.0.0.1` on a default instance and imports once over
157 `git://`. Both stop working: it moves to `allow_local = true`, and
158 the `git://` import becomes a refusal case.
159- `deploy/gitbay-runner.override.conf:35-39` and `Admin.org:923-937,
160 953-954` both describe `cmc/ci-smoke` and a `-repos krz/gitbay` on
161 `ExecStart` that the unit no longer has (`override.conf:87`); the
162 Admin procedure's `sed` matches nothing.
163
164## Order and dependencies
165
166| # | Branch | Closes | Migration | Depends on |
167|---|---|---|---|---|
168| 1 | `webhook-secret-stdin` | #287, #284 | — | — |
169| 2 | `import-pin-address` | #298 | — | — |
170| 3 | `lfs-token-key` | #285 | — | — |
171| 4 | `web-mint-reauth` | #297 | — | — |
172
173#287 and #284 share MR 1: #287 is a comment and a wiki procedure, too
174small for its own review, and neither touches a file the other does.
175No MR changes code another MR changes; none is stacked. Overlaps are
176textual only and resolve on rebase by keeping both sides:
177
178- Every MR appends to `* Unreleased` in `CHANGELOG.org`.
179- MRs 2, 3 and 4 edit `Threat-Model.org` and
180 `Architecture/09-Controls.org` (different paragraphs and rows); MRs
181 2 and 4 each delete their own row from `Architecture/10-Known-Gaps.org`;
182 MRs 3 and 4 edit different rows of `Architecture/05-Identity-and-Access.org`.
183
184Land in the table's order; any order works.
185
186## File map
187
188| File | MR | Responsibility |
189|---|---|---|
190| `deploy/gitbay-runner.override.conf` | 1 | comment names the real attachments |
191| `.gitbay/wiki/Admin.org` | 1, 2 | scratch-repository procedure; import in `[webhooks]`/`[limits]` |
192| `internal/control/webhook.go`, `webhook_test.go` | 1 | `--secret -` from stdin, literal refused |
193| `cmd/gitbay/main.go`, `stdinpayload_test.go` | 1 | forward stdin for `--secret -` |
194| `e2e/webhook_test.go` | 1 | pipe the secret |
195| `.gitbay/wiki/API.org` | 1 | webhook add usage |
196| `internal/gitpin/gitpin.go`, `gitpin_test.go` | 2 | resolve, check, pin args, clean env, git version |
197| `internal/mirror/mirror.go`, `mirror_test.go` | 2 | sync through `gitpin` |
198| `internal/gitutil/gitutil.go` | 2 | `FetchMirror`, `RemoteDefaultBranch` take pin args and a whole env |
199| `internal/control/import.go`, `import_test.go` | 2 | http(s) only, resolve, check, pin |
200| `e2e/import_test.go` | 2 | `allow_local`, `git://` refused |
201| `.gitbay/wiki/Users.org`, `Architecture/02-Components.org` | 2 | import rules; `internal/gitpin` row |
202| `internal/lfs/lfs.go`, `lfs_test.go` | 3 | key id in the token, `Grant` |
203| `internal/sshd/lfs.go`, `internal/sshd/sshd.go` | 3 | sign with the authenticating key |
204| `internal/httpd/lfs.go`, `lfsauth_test.go` | 3 | liveness check, key-0 rule |
205| `e2e/lfs_test.go` | 3 | `TestLFSTokenEndsWithItsKey` |
206| `.gitbay/wiki/Architecture/04-Trust-Boundaries.org` | 3 | LFS flow |
207| `internal/store/users.go`, `sessions.go`, `sessions_test.go` | 4 | `User.SignedInAt` |
208| `internal/control/control.go`, `reauth_test.go` | 4 | `SignedInAt`, `ReauthWindow`, `ReauthRefusal`, the check |
209| `internal/httpd/control.go`, `flash.go`, `account.go`, `settings.go`, `account_test.go`, `reauth_test.go` | 4 | `webCtx`, the sign-in link |
210| `internal/web/templates/account.html`, `settings.html` | 4 | the link |
211| `.gitbay/wiki/Threat-Model.org`, `Architecture/05-Identity-and-Access.org`, `09-Controls.org`, `10-Known-Gaps.org` | 2, 3, 4 | docs per MR |
212| `CHANGELOG.org` | 1–4 | entries under `* Unreleased` |
213
214---
215
216# MR 1: runner comment and webhook secret on stdin (branch `webhook-secret-stdin`, closes #287 and #284)
217
218### Task 1.1: the runner drop-in and the Admin procedure name what exists
219
220**Files:**
221- Modify: `deploy/gitbay-runner.override.conf:35-39`
222- Modify: `.gitbay/wiki/Admin.org:923-937`, `:953-954`
223
224**Interfaces:** none.
225
226- [ ] **Step 1: Correct the drop-in's comment**
227
228Replace lines 35-39:
229
230```
231# The runner polls as a non-admin account with a runner-scoped key, and
232# claims only the repositories that key is attached to (`repo runner
233# add`): krz/gitbay and cmc/ci-smoke. The attachments are the boundary,
234# so ExecStart names no -repos. cmc/ci-smoke is the nightly isolation
235# canary; keep it attached or its scheduled build waits forever.
236```
237
238with:
239
240```
241# The runner polls as a non-admin account with a runner-scoped key, and
242# claims only the repositories that key is attached to (`repo runner
243# add`): krz/gitbay, krz/hutch, krz/keycask, krz/orgo, krz/skunky-art
244# and cmc/cleberg.net. The attachments are the boundary, so ExecStart
245# names no -repos. To validate a runner change, create a scratch
246# repository, attach this key to it, and run with -repos naming only
247# that repository until the change is proven (Admin wiki, CI runner).
248```
249
250- [ ] **Step 2: Correct the Admin procedure**
251
252In `.gitbay/wiki/Admin.org`, replace lines 923-937 (from
253`*Validate podman mode on a scratch repository` through `#+end_src`)
254with:
255
256```org
257*Validate podman mode on a scratch repository before pointing the runner
258at real ones.* Every deploy that switched the whole instance to
259containers and failed took CI down with it. Instead: create a throwaway
260repository the runner account can read (public, or granted read — a
261private one is "not found" to the runner and the build stays pending),
262give it one job that names the CI image, attach the runner's key to it,
263and deploy the runner with =-repos= naming only that repository. The
264production unit, with its real hardening, then claims nothing else;
265other repositories' builds queue until =-repos= is removed again, which
266is a pause, not an outage.
267
268#+begin_src sh
269gitbay repo create cmc/runner-scratch # then push a .gitbay/ci.yml naming the image
270# on the host:
271gitbay repo runner add cmc/runner-scratch < /var/lib/gitbay-runner/.ssh/id_ed25519.pub
272sed -i 's#^ExecStart=/usr/local/bin/gitbay-runner #&-repos cmc/runner-scratch #' /etc/systemd/system/gitbay-runner.service.d/override.conf
273systemctl daemon-reload && systemctl restart gitbay-runner
274gitbay build log cmc/runner-scratch 1 # green: remove -repos, redeploy, delete the scratch repository
275#+end_src
276```
277
278Delete lines 953-954:
279
280```org
281The nightly canary on =cmc/ci-smoke= only runs if the runner's =-repos=
282names that repository too; a scoped runner claims nothing else.
283```
284
285- [ ] **Step 3: Check nothing else names the old repository**
286
287Run: `grep -rn "ci-smoke" --exclude-dir=.git . | grep -v "docs/plans\|docs/specs\|CHANGELOG"`
288Expected: no output.
289
290- [ ] **Step 4: Commit**
291
292```bash
293git add deploy/gitbay-runner.override.conf .gitbay/wiki/Admin.org
294git commit -S -m "deploy: runner comment and Admin procedure name the real attachments
295
296cmc/ci-smoke no longer exists and ExecStart carries no -repos; the
297scratch-repository check attaches the key and adds -repos for the run.
298
299Closes #287"
300```
301
302### Task 1.2: `webhook add` reads the secret from stdin
303
304**Files:**
305- Modify: `internal/control/webhook.go:1-13` (imports), `:16-24` (registration), `:47-81` (`runWebhookAdd`)
306- Test: `internal/control/webhook_test.go`
307
308**Interfaces:**
309- Produces: `webhook add <owner/name> <url> [--secret -] [--events ...]`, `ReadsStdin: true`. Refusal texts, exit 2:
310 `the secret is read from stdin, never argv: pipe it and pass --secret - (printf %s SECRET | ... --secret -)` and
311 `no secret on stdin (pipe it: printf %s SECRET | ... --secret -)`.
312
313- [ ] **Step 1: Write the failing test**
314
315Append to `internal/control/webhook_test.go`:
316
317```go
318// The signing secret arrives on stdin with --secret -, like a build
319// secret: a value on the command line is refused before anything is
320// stored, since argv shows in /proc and in shell history (#284).
321func TestWebhookAddSecretFromStdin(t *testing.T) {
322 st, repo, uid := newQueueTestRepo(t)
323 run := func(stdin string, argv ...string) (string, int) {
324 c, errOut := pruneCtx(st, t.TempDir(), store.User{ID: uid, Username: "alice"})
325 c.Cfg.Limits.WriteRate = -1
326 c.Cfg.Webhooks.AllowLocal = true
327 c.Stdin = strings.NewReader(stdin)
328 code := Dispatch(c, argv)
329 return errOut.String(), code
330 }
331 msg, code := run("", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "s3cret")
332 if code != protocol.ExitUsage || !strings.Contains(msg, "--secret -") {
333 t.Fatalf("literal secret: exit %d, %q", code, msg)
334 }
335 if msg, code := run("", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "-"); code != protocol.ExitUsage || !strings.Contains(msg, "no secret on stdin") {
336 t.Fatalf("empty stdin: exit %d, %q", code, msg)
337 }
338 if hooks, err := st.ListWebhooks(repo.ID); err != nil || len(hooks) != 0 {
339 t.Fatalf("a refused add stored %+v (%v)", hooks, err)
340 }
341 if msg, code := run("s3cret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "-"); code != protocol.ExitOK {
342 t.Fatalf("piped secret: exit %d, %q", code, msg)
343 }
344 // Without --secret nothing reads stdin and the hook is unsigned.
345 if msg, code := run("not a secret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/other"); code != protocol.ExitOK {
346 t.Fatalf("no secret: exit %d, %q", code, msg)
347 }
348 hooks, err := st.ListWebhooks(repo.ID)
349 if err != nil || len(hooks) != 2 {
350 t.Fatalf("hooks: %+v %v", hooks, err)
351 }
352 if hooks[0].Secret != "s3cret" || hooks[1].Secret != "" {
353 t.Fatalf("secrets: %q, %q", hooks[0].Secret, hooks[1].Secret)
354 }
355}
356```
357
358- [ ] **Step 2: Run it and see it fail**
359
360Run: `go test ./internal/control -run TestWebhookAddSecretFromStdin -count=1`
361Expected: FAIL at `literal secret: exit 0` (the literal is accepted today).
362
363- [ ] **Step 3: Implement**
364
365Imports in `internal/control/webhook.go` gain `"strings"`:
366
367```go
368import (
369 "errors"
370 "fmt"
371 "io"
372 "strconv"
373 "strings"
374
375 "gitbay.org/gitbay/internal/policy"
376 "gitbay.org/gitbay/internal/protocol"
377 "gitbay.org/gitbay/internal/store"
378 "gitbay.org/gitbay/internal/webhook"
379)
380```
381
382The registration:
383
384```go
385 register(Command{Path: []string{"webhook", "add"},
386 Summary: "add a webhook",
387 Usage: "webhook add <owner/name> <url> [--secret -] [--events push,issue.created|*]",
388 Flags: []Flag{
389 {"--secret", "-", "read the secret that signs deliveries from stdin", ""},
390 {"--events", "push,issue.created|*", "which events to send", "*"},
391 },
392 Examples: []string{
393 "webhook add krz/gitbay https://ci.example.org/hook --events push",
394 "webhook add krz/gitbay https://ci.example.org/hook --secret - < secret.txt",
395 },
396 ReadsStdin: true,
397 Run: runWebhookAdd})
398```
399
400`runWebhookAdd`, whole function:
401
402```go
403func runWebhookAdd(c *Ctx, args []string) int {
404 f, err := c.parseArgs(args, flagSpec{Values: []string{"--secret", "--events"}, MaxPos: 2, Usage: "webhook add <owner/name> <url> [--secret -] [--events push,issue.created|*]"})
405 if err != nil {
406 return c.fail(protocol.ExitUsage, "%v", err)
407 }
408 path, url, events := f.pos(0), f.pos(1), "*"
409 if f.Has("--events") {
410 events = f.Value("--events")
411 }
412 if path == "" || url == "" {
413 return c.usage()
414 }
415 // Secrets travel on stdin: argv shows in /proc and in shell history.
416 if f.Has("--secret") && f.Value("--secret") != "-" {
417 return c.fail(protocol.ExitUsage, "the secret is read from stdin, never argv: pipe it and pass --secret - (printf %%s SECRET | ... --secret -)")
418 }
419 repo, code := resolveRepo(c, path, policy.CanAdmin)
420 if code >= 0 {
421 return code
422 }
423 // A name that is not an event is a subscription that never fires, and
424 // nothing would ever say so. Checked before the URL, which resolves
425 // DNS: a typo here should not need a reachable host to report.
426 if code := checkEventNames(c, events); code >= 0 {
427 return code
428 }
429 if err := webhook.ValidateURL(url, c.Cfg.Webhooks.AllowLocal); err != nil {
430 // The command line parsed; the value is what the server refuses.
431 // Exit 1 carries the reason to every client verbatim (#187).
432 return c.fail(protocol.ExitFailure, "%v", err)
433 }
434 secret := ""
435 if f.Has("--secret") {
436 raw, err := io.ReadAll(io.LimitReader(c.Stdin, 64<<10))
437 if err != nil {
438 return c.fail(protocol.ExitFailure, "reading secret: %v", err)
439 }
440 secret = strings.TrimRight(string(raw), "\n")
441 if secret == "" {
442 return c.fail(protocol.ExitUsage, "no secret on stdin (pipe it: printf %%s SECRET | ... --secret -)")
443 }
444 }
445 id, err := c.Store.AddWebhook(repo.ID, url, secret, events)
446 if err != nil {
447 return c.fail(protocol.ExitFailure, "%v", err)
448 }
449 return c.emit(map[string]any{"id": id, "url": url, "events": events}, func(w io.Writer) {
450 fmt.Fprintf(w, "webhook %d added for %s (%s)\n", id, repo.Path(), events)
451 })
452}
453```
454
455- [ ] **Step 4: Run the package**
456
457Run: `go test ./internal/control -count=1`
458Expected: PASS, including `TestWebhookAddRefusedURLIsAFailure`,
459`TestStdinCommandsReadStdin` and the help tests (the usage names
460`--secret`, the flag is described, both examples resolve to
461`webhook add`).
462
463- [ ] **Step 5: Commit**
464
465```bash
466git add internal/control/webhook.go internal/control/webhook_test.go
467git commit -S -m "webhook: add reads the signing secret from stdin
468
469--secret - reads it; a value on the command line is refused.
470
471Ref #284"
472```
473
474### Task 1.3: the CLI forwards stdin for `--secret -`
475
476**Files:**
477- Modify: `cmd/gitbay/main.go:358-368` (`usesStdin`), `:753` (`webhook add` pass)
478- Test: `cmd/gitbay/stdinpayload_test.go`
479
480**Interfaces:**
481- Consumes: `webhook add ... --secret -` (Task 1.2).
482
483- [ ] **Step 1: Write the failing test**
484
485Append to `cmd/gitbay/stdinpayload_test.go`:
486
487```go
488// webhook add --secret - reads the secret on the server, so the CLI must
489// forward stdin for it the way it does for --file - (#284).
490func TestUsesStdinForSecretDash(t *testing.T) {
491 if !usesStdin([]string{"alice/app", "https://ci.example/hook", "--secret", "-"}) {
492 t.Error("--secret - does not forward stdin")
493 }
494 if usesStdin([]string{"alice/app", "https://ci.example/hook", "--events", "push"}) {
495 t.Error("forwarded stdin with no flag asking for it")
496 }
497}
498```
499
500- [ ] **Step 2: Run it and see it fail**
501
502Run: `go test ./cmd/gitbay -run TestUsesStdinForSecretDash -count=1`
503Expected: FAIL, `--secret - does not forward stdin`.
504
505- [ ] **Step 3: Implement**
506
507`usesStdin`:
508
509```go
510// usesStdin reports whether the arguments request stdin content.
511func usesStdin(args []string) bool {
512 for i, a := range args {
513 if (a == "--file" || a == "--key" || a == "--secret") && i+1 < len(args) && args[i+1] == "-" {
514 return true
515 }
516 if a == "--token-stdin" {
517 return true
518 }
519 }
520 return false
521}
522```
523
524`cmd/gitbay/main.go:753`:
525
526```go
527 pass("add", passOpts{server: []string{"webhook", "add"}, needsRepo: true, stdinOK: true, stdinWhat: "the webhook secret", stdinSecret: true}),
528```
529
530- [ ] **Step 4: Run the package**
531
532Run: `go test ./cmd/gitbay -count=1 && go vet ./cmd/gitbay`
533Expected: PASS (coverage, summaries and stdin-payload tests included).
534
535- [ ] **Step 5: Commit**
536
537```bash
538git add cmd/gitbay/main.go cmd/gitbay/stdinpayload_test.go
539git commit -S -m "cli: webhook add forwards stdin for --secret -
540
541Ref #284"
542```
543
544### Task 1.4: e2e, API wiki and changelog
545
546**Files:**
547- Modify: `e2e/webhook_test.go:115-116`
548- Modify: `.gitbay/wiki/API.org:153` and the paragraph after its `#+end_src`
549- Modify: `CHANGELOG.org` (`* Unreleased`)
550
551**Interfaces:** none.
552
553- [ ] **Step 1: Pipe the secret in the e2e test**
554
555`e2e/webhook_test.go:115-116`:
556
557```go
558 if _, errOut, code := inst.ssh(t, aliceKey, "s3cret\n",
559 "webhook", "add", "alice/proj", hookURL, "--secret", "-"); code != 0 {
560```
561
562The HMAC check below it (`hmac.New(sha256.New, []byte("s3cret"))`)
563stays: the server trims the trailing newline.
564
565- [ ] **Step 2: Run the e2e test**
566
567Run: `go build ./... && go test ./e2e -run 'TestWebhooks$' -count=1`
568Expected: PASS.
569
570- [ ] **Step 3: API wiki**
571
572`.gitbay/wiki/API.org:153` becomes:
573
574```org
575printf %s "$SECRET" | gitbay webhook add <url> --secret - [--events push,issue.created] # default *
576```
577
578After that block's `#+end_src` and before `** Events`, add:
579
580```org
581
582The signing secret is read from stdin with =--secret -=; a value on the
583command line is refused, since argv shows in process listings and shell
584history. Over the JSON API it goes in the request's =stdin= field.
585```
586
587- [ ] **Step 4: Changelog**
588
589Append to the `* Unreleased` list, directly above `* v1.36.0 — 2026-09-23`:
590
591```org
592- =webhook add= reads the signing secret from stdin with =--secret -=;
593 a value on the command line is refused, since argv shows in process
594 listings and shell history. A script that passed the value must pipe
595 it: =printf %s "$SECRET" | gitbay webhook add <repo> <url> --secret -=
596 (#284).
597```
598
599- [ ] **Step 5: Commit, MR, merge**
600
601```bash
602git add e2e/webhook_test.go .gitbay/wiki/API.org CHANGELOG.org
603git commit -S -m "webhook: document --secret -, pipe it in the e2e test
604
605Closes #284"
606git push -u origin webhook-secret-stdin
607gitbay mr create --source webhook-secret-stdin --target main --title "webhook add reads its secret from stdin; runner comment names the real attachments"
608```
609
610Merge `--strategy ff` after CI, delete the branch both places.
611
612---
613
614# MR 2: import checks and pins its address (branch `import-pin-address`, closes #298)
615
616### Task 2.1: `internal/gitpin`
617
618**Files:**
619- Create: `internal/gitpin/gitpin.go`
620- Create: `internal/gitpin/gitpin_test.go`
621
622**Interfaces:**
623- Produces:
624 - `type Lookup func(ctx context.Context, host string) ([]net.IP, error)`
625 - `func LookupIP(ctx context.Context, host string) ([]net.IP, error)`
626 - `type Remote struct { URL *url.URL; IPs []net.IP }`
627 - `func Resolve(ctx context.Context, lookup Lookup, raw string, allowLocal bool) (Remote, error)`
628 - `func (r Remote) Args() []string`
629 - `func Env(home string) []string`
630 - `func VersionOK(out string) error`
631 - `func CheckGit(ctx context.Context) error`
632
633- [ ] **Step 1: Write the failing tests**
634
635`internal/gitpin/gitpin_test.go`:
636
637```go
638package gitpin
639
640import (
641 "context"
642 "net"
643 "net/url"
644 "slices"
645 "strings"
646 "testing"
647)
648
649func answer(ips ...string) Lookup {
650 return func(context.Context, string) ([]net.IP, error) {
651 var out []net.IP
652 for _, s := range ips {
653 out = append(out, net.ParseIP(s))
654 }
655 return out, nil
656 }
657}
658
659func TestResolve(t *testing.T) {
660 ctx := context.Background()
661 r, err := Resolve(ctx, answer("203.0.113.5"), "https://git.example/x.git", false)
662 if err != nil || r.URL.Hostname() != "git.example" || len(r.IPs) != 1 {
663 t.Fatalf("public: %+v %v", r, err)
664 }
665 if _, err := Resolve(ctx, answer("203.0.113.5", "10.0.0.7"), "https://git.example/x.git", false); err == nil || !strings.Contains(err.Error(), "10.0.0.7") {
666 t.Fatalf("private: %v", err)
667 }
668 if _, err := Resolve(ctx, answer("10.0.0.7"), "https://git.example/x.git", true); err != nil {
669 t.Fatalf("allow_local: %v", err)
670 }
671 // An empty resolve list would leave curl to resolve the host itself.
672 if _, err := Resolve(ctx, answer(), "https://git.example/x.git", true); err == nil || !strings.Contains(err.Error(), "no address") {
673 t.Fatalf("empty answer: %v", err)
674 }
675 for _, raw := range []string{"git://git.example/x.git", "ssh://git.example/x.git", "file:///etc"} {
676 _, err := Resolve(ctx, func(context.Context, string) ([]net.IP, error) {
677 t.Fatalf("looked up a host for %s", raw)
678 return nil, nil
679 }, raw, true)
680 if err == nil || !strings.Contains(err.Error(), "not http or https") {
681 t.Errorf("%s: %v", raw, err)
682 }
683 }
684}
685
686func TestArgs(t *testing.T) {
687 u, _ := url.Parse("https://git.example/x.git")
688 got := Remote{u, []net.IP{net.ParseIP("203.0.113.5"), net.ParseIP("2001:db8::1")}}.Args()
689 want := []string{"-c", "http.followRedirects=false",
690 "-c", "http.curloptResolve=git.example:443:203.0.113.5,[2001:db8::1]"}
691 if !slices.Equal(got, want) {
692 t.Fatalf("https: %q", got)
693 }
694 u, _ = url.Parse("http://git.example:8080/x.git")
695 if got := (Remote{u, []net.IP{net.ParseIP("203.0.113.5")}}).Args(); got[3] != "http.curloptResolve=git.example:8080:203.0.113.5" {
696 t.Fatalf("http with port: %q", got)
697 }
698 // An address literal is its own resolution; there is nothing to pin.
699 u, _ = url.Parse("https://203.0.113.5/x.git")
700 if got := (Remote{u, []net.IP{net.ParseIP("203.0.113.5")}}).Args(); !slices.Equal(got, []string{"-c", "http.followRedirects=false"}) {
701 t.Fatalf("literal: %q", got)
702 }
703}
704
705func TestEnv(t *testing.T) {
706 want := []string{"GIT_TERMINAL_PROMPT=0", "HOME=/srv/gitbay",
707 "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"}
708 if got := Env("/srv/gitbay"); !slices.Equal(got, want) {
709 t.Fatalf("Env = %q", got)
710 }
711}
712
713func TestVersionOK(t *testing.T) {
714 for _, s := range []string{"git version 2.37.0", "git version 2.47.3", "git version 2.39.5 (Apple Git-154)",
715 "git version 2.45.2.windows.1", "git version 3.0.0\n"} {
716 if err := VersionOK(s); err != nil {
717 t.Errorf("%q: %v", s, err)
718 }
719 }
720 for _, s := range []string{"git version 2.36.9", "git version 1.99.0", "git version 2", "nonsense", ""} {
721 if err := VersionOK(s); err == nil {
722 t.Errorf("%q accepted", s)
723 }
724 }
725}
726```
727
728- [ ] **Step 2: Run them and see them fail**
729
730Run: `go test ./internal/gitpin -count=1`
731Expected: build failure, `undefined: Lookup` / `undefined: Resolve`.
732
733- [ ] **Step 3: Implement**
734
735`internal/gitpin/gitpin.go`:
736
737```go
738// Package gitpin runs git against a user-supplied http or https remote
739// only at addresses resolved and checked immediately before: mirror
740// sync (#279) and repo import (#298).
741package gitpin
742
743import (
744 "context"
745 "fmt"
746 "net"
747 "net/url"
748 "os/exec"
749 "strconv"
750 "strings"
751
752 "gitbay.org/gitbay/internal/toolpath"
753 "gitbay.org/gitbay/internal/webhook"
754)
755
756// Lookup resolves a host to its addresses.
757type Lookup func(ctx context.Context, host string) ([]net.IP, error)
758
759// LookupIP is the system resolver.
760func LookupIP(ctx context.Context, host string) ([]net.IP, error) {
761 return net.DefaultResolver.LookupIP(ctx, "ip", host)
762}
763
764// Remote is a URL whose host resolved to IPs, every one of which passed
765// the address check.
766type Remote struct {
767 URL *url.URL
768 IPs []net.IP
769}
770
771// Resolve parses raw, requires http or https, resolves the host with
772// lookup, and refuses it when it resolves to nothing or, unless
773// allowLocal, to any private or local address.
774func Resolve(ctx context.Context, lookup Lookup, raw string, allowLocal bool) (Remote, error) {
775 u, err := url.Parse(raw)
776 if err != nil {
777 return Remote{}, err
778 }
779 if u.Scheme != "https" && u.Scheme != "http" {
780 return Remote{}, fmt.Errorf("URL scheme %q is not http or https", u.Scheme)
781 }
782 host := u.Hostname()
783 if host == "" {
784 return Remote{}, fmt.Errorf("URL has no host")
785 }
786 ips, err := lookup(ctx, host)
787 if err != nil {
788 return Remote{}, fmt.Errorf("resolving %s: %w", host, err)
789 }
790 if len(ips) == 0 {
791 // An empty resolve list would leave curl to resolve the host itself.
792 return Remote{}, fmt.Errorf("%s resolves to no address", host)
793 }
794 if err := webhook.CheckAddrs(host, ips, allowLocal); err != nil {
795 return Remote{}, err
796 }
797 return Remote{URL: u, IPs: ips}, nil
798}
799
800// Args are git's leading -c options for r: curl's resolve list pins
801// the host to the checked addresses, and with redirects off a server
802// cannot send git on to a host nobody checked. An address literal
803// needs no pin.
804func (r Remote) Args() []string {
805 args := []string{"-c", "http.followRedirects=false"}
806 host := r.URL.Hostname()
807 if net.ParseIP(host) != nil {
808 return args
809 }
810 port := r.URL.Port()
811 if port == "" {
812 port = "443"
813 if r.URL.Scheme == "http" {
814 port = "80"
815 }
816 }
817 addrs := make([]string, len(r.IPs))
818 for i, ip := range r.IPs {
819 if ip.To4() == nil {
820 addrs[i] = "[" + ip.String() + "]"
821 } else {
822 addrs[i] = ip.String()
823 }
824 }
825 return append(args, "-c", "http.curloptResolve="+host+":"+port+":"+strings.Join(addrs, ","))
826}
827
828// Env is git's whole environment for a pinned remote. No system or
829// global gitconfig: a proxy, URL rewrite or redirect setting there
830// would take git around the pin.
831func Env(home string) []string {
832 return []string{"GIT_TERMINAL_PROMPT=0", "HOME=" + home,
833 "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"}
834}
835
836// VersionOK accepts the output of `git version` for git 2.37 or later,
837// the first release with http.curloptResolve. An older git ignores the
838// setting and would resolve the host itself.
839func VersionOK(out string) error {
840 fields := strings.Fields(out)
841 if len(fields) >= 3 && fields[0] == "git" && fields[1] == "version" {
842 parts := strings.Split(fields[2], ".")
843 if len(parts) >= 2 {
844 major, err1 := strconv.Atoi(parts[0])
845 minor, err2 := strconv.Atoi(parts[1])
846 if err1 == nil && err2 == nil {
847 if major > 2 || major == 2 && minor >= 37 {
848 return nil
849 }
850 return fmt.Errorf("git %s is older than 2.37 and cannot pin remote addresses", fields[2])
851 }
852 }
853 }
854 return fmt.Errorf("cannot read git version from %q", strings.TrimSpace(out))
855}
856
857// CheckGit runs the server's git and refuses one that cannot pin.
858func CheckGit(ctx context.Context) error {
859 out, err := exec.CommandContext(ctx, toolpath.Look("git"), "version").Output()
860 if err != nil {
861 return fmt.Errorf("running git version: %v", err)
862 }
863 return VersionOK(string(out))
864}
865```
866
867`webhook` imports only `store`, and `store` imports neither `gitpin`
868nor `webhook`: no cycle.
869
870- [ ] **Step 4: Run the package**
871
872Run: `go test ./internal/gitpin -count=1 && go vet ./internal/gitpin`
873Expected: PASS.
874
875- [ ] **Step 5: Commit**
876
877```bash
878git add internal/gitpin
879git commit -S -m "gitpin: resolve, check and pin a git remote
880
881The mirror worker's resolve-check-pin, as a package import can share.
882
883Ref #298"
884```
885
886### Task 2.2: mirror sync goes through `gitpin`
887
888**Files:**
889- Modify: `internal/mirror/mirror.go:8-26` (imports), `:46-79` (`New`, `Run`), `:102-167` (`sync`); delete `:169-215` (`pinArgs`, `gitVersionOK`)
890- Modify: `internal/mirror/mirror_test.go:3-19` (imports), `:141-160` (`TestSweepRefusesWithAnOldGit`); delete `:162-176` (`TestGitVersionOK`) and `:242-260` (`TestPinArgs`)
891
892**Interfaces:**
893- Consumes: `gitpin.LookupIP`, `gitpin.Resolve`, `gitpin.Remote.Args`, `gitpin.Env`, `gitpin.VersionOK`, `gitpin.CheckGit` (Task 2.1).
894- Produces: no new names; `Worker.Lookup` keeps its type.
895
896- [ ] **Step 1: Point the old-git test at the moved function**
897
898In `TestSweepRefusesWithAnOldGit`, replace
899
900```go
901 w.gitErr = gitVersionOK("git version 2.36.1")
902```
903
904with
905
906```go
907 w.gitErr = gitpin.VersionOK("git version 2.36.1")
908```
909
910Delete `TestGitVersionOK` and `TestPinArgs` (now `gitpin`'s
911`TestVersionOK` and `TestArgs`). Add
912`"gitbay.org/gitbay/internal/gitpin"` to the test imports.
913
914- [ ] **Step 2: Baseline**
915
916This task is a refactor: the mirror tests are the check, green before
917and after.
918
919Run: `go test ./internal/mirror -count=1`
920Expected: PASS (`gitpin.VersionOK` exists from Task 2.1; the package's
921own `pinArgs` and `gitVersionOK` still exist until Step 3).
922
923- [ ] **Step 3: Implement**
924
925Imports:
926
927```go
928import (
929 "context"
930 "fmt"
931 "log/slog"
932 "net"
933 "os"
934 "os/exec"
935 "path/filepath"
936 "time"
937
938 "gitbay.org/gitbay/internal/config"
939 "gitbay.org/gitbay/internal/control"
940 "gitbay.org/gitbay/internal/gitpin"
941 "gitbay.org/gitbay/internal/store"
942 "gitbay.org/gitbay/internal/toolpath"
943)
944```
945
946`New` sets `Lookup: gitpin.LookupIP` in place of the inline closure:
947
948```go
949 return &Worker{St: st, Cfg: cfg, Tick: tick, Lookup: gitpin.LookupIP}
950```
951
952`Run`, the first lines through the `slog.Error`:
953
954```go
955func (w *Worker) Run(ctx context.Context) {
956 if err := gitpin.CheckGit(ctx); err != nil {
957 w.gitErr = fmt.Errorf("mirrors disabled: %w", err)
958 slog.Error("mirror: not syncing", "err", w.gitErr)
959 }
960 t := time.NewTicker(w.Tick)
961```
962
963`sync`, whole function:
964
965```go
966func (w *Worker) sync(m store.Mirror) error {
967 repo, err := w.St.RepoByID(m.RepoID)
968 if err != nil {
969 return err
970 }
971 dir := control.RepoDir(w.Cfg.Server.Root, repo.OwnerName, repo.Name)
972
973 ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
974 defer cancel()
975 // The URL was checked when saved, but DNS can answer differently
976 // now. Check what it resolves to at sync time, then let git connect
977 // to exactly those addresses.
978 remote, err := gitpin.Resolve(ctx, w.Lookup, m.URL, w.Cfg.Webhooks.AllowLocal)
979 if err != nil {
980 return err
981 }
982
983 env := gitpin.Env(w.Cfg.Server.Root)
984 if m.Token != "" {
985 askpass := filepath.Join(w.Cfg.Server.Root, "mirror-askpass.sh")
986 if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil {
987 return err
988 }
989 user := m.Username
990 if user == "" {
991 user = "x-access-token"
992 }
993 env = append(env,
994 "GIT_ASKPASS="+askpass,
995 "GITBAY_MIRROR_USER="+user,
996 "GITBAY_MIRROR_TOKEN="+m.Token)
997 }
998
999 args := append(remote.Args(), "-C", dir)
1000 if m.Direction == "push" {
1001 // Branches and tags only: internal refs (merge-requests) stay home.
1002 args = append(args, "push", "--prune", m.URL,
1003 "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*")
1004 } else {
1005 args = append(args, "fetch", "--prune", m.URL,
1006 "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*")
1007 }
1008 cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...)
1009 cmd.Env = env
1010 if out, err := cmd.CombinedOutput(); err != nil {
1011 return fmt.Errorf("git %s: %v: %.300s", m.Direction, err, out)
1012 }
1013 return nil
1014}
1015```
1016
1017Delete `pinArgs` and `gitVersionOK`.
1018
1019- [ ] **Step 4: Run the mirror tests and the mirror e2e**
1020
1021Run: `go test ./internal/mirror ./internal/gitpin -count=1 && go vet ./internal/mirror`
1022Expected: PASS. `TestSyncRefusesANonHTTPScheme` matches
1023`not http or https`, `TestSyncRefusesAnEmptyAnswer` matches
1024`no address`, `TestSweepRefusesWithAnOldGit` matches `2.37`.
1025
1026Run: `go build ./... && go test ./e2e -run TestMirrors -count=1`
1027Expected: PASS.
1028
1029- [ ] **Step 5: Commit**
1030
1031```bash
1032git add internal/mirror
1033git commit -S -m "mirror: sync through gitpin
1034
1035Ref #298"
1036```
1037
1038### Task 2.3: `repo import` accepts http(s) only and pins the address
1039
1040**Files:**
1041- Modify: `internal/gitutil/gitutil.go:218-233` (`FetchMirror`), `:256-273` (`RemoteDefaultBranch`)
1042- Modify: `internal/control/import.go:3-16` (imports), `:80-116`, `:146-158`
1043- Create: `internal/control/import_test.go`
1044
1045**Interfaces:**
1046- Consumes: `gitpin.CheckGit`, `gitpin.Resolve`, `gitpin.Remote.Args`, `gitpin.Env`, `gitpin.Lookup`, `gitpin.LookupIP` (Task 2.1).
1047- Produces:
1048 - `func FetchMirror(ctx context.Context, dir, url string, errW io.Writer, pin, env []string) error`
1049 - `func RemoteDefaultBranch(ctx context.Context, url string, pin, env []string) (string, error)`
1050 - `var importLookup gitpin.Lookup = gitpin.LookupIP` (package `control`; tests replace it)
1051 - Refusal: `import fetches over http:// and https:// only; use the repository's https:// URL` (exit 2).
1052
1053- [ ] **Step 1: Write the failing tests**
1054
1055`internal/control/import_test.go`:
1056
1057```go
1058package control
1059
1060import (
1061 "bytes"
1062 "context"
1063 "net"
1064 "net/http/cgi"
1065 "net/http/httptest"
1066 "net/url"
1067 "os"
1068 "path/filepath"
1069 "slices"
1070 "strings"
1071 "testing"
1072
1073 "gitbay.org/gitbay/internal/protocol"
1074 "gitbay.org/gitbay/internal/store"
1075)
1076
1077func importCtx(t *testing.T, allowLocal bool) (*Ctx, *bytes.Buffer, *store.Store, string) {
1078 t.Helper()
1079 st, _, uid := newQueueTestRepo(t)
1080 root := t.TempDir()
1081 c, errOut := pruneCtx(st, root, store.User{ID: uid, Username: "alice"})
1082 c.Cfg.Limits.WriteRate = -1
1083 c.Cfg.Limits.CloneTimeoutSec = 60
1084 c.Cfg.Webhooks.AllowLocal = allowLocal
1085 c.Stdin = strings.NewReader("")
1086 return c, errOut, st, root
1087}
1088
1089// importUpstream serves a bare repository with one commit on main over
1090// smart HTTP and returns its URL and that commit.
1091func importUpstream(t *testing.T) (string, string) {
1092 t.Helper()
1093 git := gitRunner(t)
1094 parent := t.TempDir()
1095 bare := filepath.Join(parent, "remote.git")
1096 work := filepath.Join(parent, "work")
1097 git(parent, "init", "-q", "--bare", "--initial-branch=main", bare)
1098 git(parent, "init", "-q", "--initial-branch=main", work)
1099 git(work, "commit", "-q", "--allow-empty", "-m", "one")
1100 git(work, "push", "-q", bare, "main")
1101 sha := strings.TrimSpace(git(work, "rev-parse", "HEAD"))
1102 execPath := strings.TrimSpace(git(parent, "--exec-path"))
1103 srv := httptest.NewServer(&cgi.Handler{
1104 Path: filepath.Join(execPath, "git-http-backend"),
1105 Env: []string{"GIT_PROJECT_ROOT=" + parent, "GIT_HTTP_EXPORT_ALL=1"},
1106 })
1107 t.Cleanup(srv.Close)
1108 return srv.URL + "/remote.git", sha
1109}
1110
1111// git:// cannot be held to a checked address, so import refuses it
1112// before creating anything (#298).
1113func TestRepoImportRefusesGitScheme(t *testing.T) {
1114 c, errOut, st, _ := importCtx(t, true)
1115 code := Dispatch(c, []string{"repo", "import", "alice/x", "--from", "git://example.org/x.git"})
1116 if code != protocol.ExitUsage || !strings.Contains(errOut.String(), "use the repository's https:// URL") {
1117 t.Fatalf("exit %d, %q", code, errOut.String())
1118 }
1119 if _, err := st.RepoByPath("alice/x"); err == nil {
1120 t.Fatal("a refused import created a repository")
1121 }
1122}
1123
1124// A source on loopback is refused on a default instance, and nothing is
1125// left behind.
1126func TestRepoImportRefusesALocalAddress(t *testing.T) {
1127 c, errOut, st, root := importCtx(t, false)
1128 code := Dispatch(c, []string{"repo", "import", "alice/x", "--from", "http://127.0.0.1:9/x.git"})
1129 if code != protocol.ExitFailure || !strings.Contains(errOut.String(), "127.0.0.1") {
1130 t.Fatalf("exit %d, %q", code, errOut.String())
1131 }
1132 if _, err := st.RepoByPath("alice/x"); err == nil {
1133 t.Fatal("a refused import created a repository")
1134 }
1135 if _, err := os.Stat(RepoDir(root, "alice", "x")); !os.IsNotExist(err) {
1136 t.Fatalf("a refused import left a directory: %v", err)
1137 }
1138}
1139
1140// import.test does not resolve; the import works only because git was
1141// pinned to the address import looked up and checked.
1142func TestRepoImportConnectsToTheCheckedAddress(t *testing.T) {
1143 remote, sha := importUpstream(t)
1144 u, _ := url.Parse(remote)
1145 c, errOut, _, root := importCtx(t, true)
1146 var asked []string
1147 prev := importLookup
1148 importLookup = func(ctx context.Context, host string) ([]net.IP, error) {
1149 asked = append(asked, host)
1150 return []net.IP{net.ParseIP("127.0.0.1")}, nil
1151 }
1152 t.Cleanup(func() { importLookup = prev })
1153
1154 code := Dispatch(c, []string{"repo", "import", "alice/copy", "--from", "http://import.test:" + u.Port() + "/remote.git"})
1155 if code != protocol.ExitOK {
1156 t.Fatalf("exit %d: %s", code, errOut.String())
1157 }
1158 if out := c.Stdout.(*bytes.Buffer).String(); !strings.Contains(out, "default main") {
1159 t.Fatalf("output %q: the default branch was not read through the pin", out)
1160 }
1161 got := strings.TrimSpace(gitRunner(t)(RepoDir(root, "alice", "copy"), "rev-parse", "refs/heads/main"))
1162 if got != sha {
1163 t.Fatalf("main = %s, want %s", got, sha)
1164 }
1165 if !slices.Equal(asked, []string{"import.test"}) {
1166 t.Fatalf("looked up %v", asked)
1167 }
1168}
1169```
1170
1171- [ ] **Step 2: Run them and see them fail**
1172
1173Run: `go test ./internal/control -run 'TestRepoImport' -count=1`
1174Expected: build failure, `undefined: importLookup`.
1175
1176- [ ] **Step 3: Implement the gitutil half**
1177
1178`internal/gitutil/gitutil.go`, `FetchMirror` with its comment:
1179
1180```go
1181// FetchMirror pulls all branches, tags, and notes from a foreign URL into
1182// the bare repository at dir, forcing updates. Progress streams to errW so
1183// an interactive caller can watch. pin is git's leading -c options
1184// (gitpin.Remote.Args); env is git's whole environment and carries
1185// credentials via GIT_ASKPASS: the URL itself must never contain them.
1186func FetchMirror(ctx context.Context, dir, url string, errW io.Writer, pin, env []string) error {
1187 args := append(append([]string{}, pin...), "-C", dir, "fetch", "--progress", "--no-write-fetch-head", url,
1188 "+refs/heads/*:refs/heads/*",
1189 "+refs/tags/*:refs/tags/*",
1190 "+refs/notes/*:refs/notes/*")
1191 cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...)
1192 cmd.Env = env
1193 cmd.Stderr = errW
1194 if err := cmd.Run(); err != nil {
1195 return fmt.Errorf("fetch from %s: %w", url, err)
1196 }
1197 return nil
1198}
1199```
1200
1201`RemoteDefaultBranch`, the comment and the first three lines:
1202
1203```go
1204// RemoteDefaultBranch asks the remote which branch HEAD points at. pin
1205// and env are as for FetchMirror.
1206func RemoteDefaultBranch(ctx context.Context, url string, pin, env []string) (string, error) {
1207 args := append(append([]string{}, pin...), "ls-remote", "--symref", url, "HEAD")
1208 cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...)
1209 cmd.Env = env
1210 out, err := cmd.Output()
1211```
1212
1213The rest of `RemoteDefaultBranch` is unchanged.
1214
1215- [ ] **Step 4: Implement the import half**
1216
1217Imports in `internal/control/import.go`:
1218
1219```go
1220import (
1221 "bufio"
1222 "context"
1223 "fmt"
1224 "io"
1225 "os"
1226 "path/filepath"
1227 "strings"
1228 "time"
1229
1230 "gitbay.org/gitbay/internal/gitpin"
1231 "gitbay.org/gitbay/internal/gitutil"
1232 "gitbay.org/gitbay/internal/policy"
1233 "gitbay.org/gitbay/internal/protocol"
1234)
1235```
1236
1237After `askpassScript` (line 39), add:
1238
1239```go
1240// importLookup resolves an import's host; tests replace it.
1241var importLookup gitpin.Lookup = gitpin.LookupIP
1242```
1243
1244Replace lines 80-116 (from `// Scheme allowlist.` through the closing
1245brace of the token `if`/`else`) with:
1246
1247```go
1248 // http and https only. git:// has no equivalent of curl's resolve
1249 // list, so its connection cannot be held to a checked address;
1250 // file:// would read the server's filesystem, and ssh:// would use
1251 // the server's own keys.
1252 if !strings.HasPrefix(from, "https://") && !strings.HasPrefix(from, "http://") {
1253 return c.fail(protocol.ExitUsage, "import fetches over http:// and https:// only; use the repository's https:// URL")
1254 }
1255 if strings.ContainsAny(from, "@") {
1256 // Credentials belong on stdin, not in the URL where they would
1257 // land in process listings and logs.
1258 return c.fail(protocol.ExitUsage, "do not embed credentials in the URL; use --token-stdin")
1259 }
1260
1261 // Resolve and check the host now and hold git to those addresses,
1262 // as mirror sync does (#298).
1263 timeout := time.Duration(c.Cfg.Limits.CloneTimeoutSec) * time.Second
1264 ctx, cancel := context.WithTimeout(context.Background(), timeout)
1265 defer cancel()
1266 if err := gitpin.CheckGit(ctx); err != nil {
1267 return c.fail(protocol.ExitFailure, "import unavailable: %v", err)
1268 }
1269 remote, err := gitpin.Resolve(ctx, importLookup, from, c.Cfg.Webhooks.AllowLocal)
1270 if err != nil {
1271 return c.fail(protocol.ExitFailure, "%v", err)
1272 }
1273
1274 // The token is read from stdin and handed to git via GIT_ASKPASS and
1275 // the environment — never argv, never the database, never a log line.
1276 env := gitpin.Env(c.Cfg.Server.Root)
1277 if tokenStdin {
1278 token, err := bufio.NewReader(io.LimitReader(c.Stdin, 4096)).ReadString('\n')
1279 if err != nil && err != io.EOF {
1280 return c.fail(protocol.ExitFailure, "reading token: %v", err)
1281 }
1282 token = strings.TrimSpace(token)
1283 if token == "" {
1284 return c.fail(protocol.ExitUsage, "--token-stdin given but stdin held no token")
1285 }
1286 askpass := filepath.Join(c.Cfg.Server.Root, "askpass.sh")
1287 if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil {
1288 return c.fail(protocol.ExitFailure, "%v", err)
1289 }
1290 env = append(env, "GIT_ASKPASS="+askpass, "GITBAY_IMPORT_TOKEN="+token)
1291 }
1292```
1293
1294Replace lines 146-158 (the old `timeout`/`ctx`/`cancel`, the fetch
1295and the default-branch lookup) with:
1296
1297```go
1298 fmt.Fprintf(c.Stderr, "importing %s into %s ...\n", from, path)
1299 if err := gitutil.FetchMirror(ctx, dir, from, c.Stderr, remote.Args(), env); err != nil {
1300 cleanup()
1301 return c.fail(protocol.ExitFailure, "import failed: %v", err)
1302 }
1303
1304 branch, err := gitutil.RemoteDefaultBranch(ctx, from, remote.Args(), env)
1305 if err != nil {
1306 branch = "main" // remote gone quiet after the fetch; keep the default
1307 }
1308```
1309
1310- [ ] **Step 5: Run the packages**
1311
1312Run: `go build ./... && go vet ./internal/control ./internal/gitutil && go test ./internal/control ./internal/gitutil -count=1`
1313Expected: PASS, including the three `TestRepoImport*` tests.
1314
1315- [ ] **Step 6: Commit**
1316
1317```bash
1318git add internal/gitutil/gitutil.go internal/control/import.go internal/control/import_test.go
1319git commit -S -m "import: http(s) only, address checked and pinned like a mirror sync
1320
1321git:// is refused: it cannot be held to a checked address.
1322
1323Ref #298"
1324```
1325
1326### Task 2.4: e2e, wiki and changelog
1327
1328**Files:**
1329- Modify: `e2e/import_test.go:14`, `:85-92`, `:105-114`
1330- Modify: `.gitbay/wiki/Threat-Model.org:91-103`, `Architecture/09-Controls.org:78`, `Architecture/10-Known-Gaps.org:17`, `Architecture/02-Components.org` (packages table), `Admin.org:196-198`, `:201`, `Users.org:257-259`
1331- Modify: `CHANGELOG.org`
1332
1333**Interfaces:** none.
1334
1335- [ ] **Step 1: Update the e2e test**
1336
1337`e2e/import_test.go:14`: the test imports from its own instance on
1338`127.0.0.1`, which a default instance now refuses:
1339
1340```go
1341 inst := startInstanceWith(t, "[webhooks]\nallow_local = true\n")
1342```
1343
1344Delete lines 85-92 (`// Import over git:// too.` through the closing
1345brace of the `git://` import).
1346
1347The refusal table, lines 105-114:
1348
1349```go
1350 // Refusals: bad scheme, git://, credentials in URL, existing name, foreign owner.
1351 cases := []struct {
1352 args []string
1353 want string
1354 }{
1355 {[]string{"repo", "import", "alice/x", "--from", "file:///etc"}, "http:// and https:// only"},
1356 {[]string{"repo", "import", "alice/x", "--from", "git://127.0.0.1:1/alice/src.git"}, "use the repository's https:// URL"},
1357 {[]string{"repo", "import", "alice/x", "--from", "https://token@github.com/a/b"}, "--token-stdin"},
1358 {[]string{"repo", "import", "alice/mirror", "--from", httpURL}, "already exists"},
1359 {[]string{"repo", "import", "bob/x", "--from", httpURL}, "not you and not an organization"},
1360 }
1361```
1362
1363- [ ] **Step 2: Run it**
1364
1365Run: `go build ./... && go test ./e2e -run 'TestRepoImport$' -count=1`
1366Expected: PASS.
1367
1368- [ ] **Step 3: Wiki**
1369
1370`Threat-Model.org`, the paragraph under `* Network-facing request
1371forgery` (lines 91-103) becomes:
1372
1373```org
1374Webhook delivery, GitHub-history import =--api-base=, mirror remotes
1375and =repo import --from=, which make the *server* open an outbound
1376connection to a user-supplied address, pass the same SSRF guard: the
1377scheme must be http/https and, unless =webhooks.allow_local= is set,
1378the resolved address must not be loopback, private, shared
1379(100.64.0.0/10), link-local, or multicast. The webhook dialer re-checks
1380at connect time, and mirror sync and =repo import= resolve and check
1381immediately before running git and pin it to the checked addresses
1382(=internal/gitpin=), so a DNS answer that changes after validation
1383still cannot reach private space. Redirects are never followed.
1384=repo import= refuses =git://=, which cannot be pinned.
1385```
1386
1387`Architecture/09-Controls.org:78`:
1388
1389```org
1390| SSRF protection on user-supplied URLs | in place | webhooks at save and connect; mirrors at save and sync, =repo import= before its fetch, git pinned to the checked address (=internal/gitpin=) |
1391```
1392
1393`Architecture/10-Known-Gaps.org`: delete the `#298` row (line 17).
1394
1395`Architecture/02-Components.org`, after the `=internal/mirror=` row:
1396
1397```org
1398| =internal/gitpin= | Resolves and checks a user-supplied http(s) remote and pins git to the checked addresses; mirror sync and =repo import=. |
1399```
1400
1401`Admin.org:196-198` (`[webhooks]`):
1402
1403```org
1404- =allow_local= (false) — permit webhook, mirror and import targets on
1405 loopback, private, shared (100.64.0.0/10), link-local or multicast
1406 addresses. Leave off unless you know why you need it (SSRF).
1407```
1408
1409`Admin.org:201` (`[limits]`):
1410
1411```org
1412- =clone_timeout= (3600s) — cap on =repo import= fetches. An import
1413 takes http and https URLs only, passes the same address check as a
1414 mirror sync and is pinned the same way, so it needs git 2.37 too.
1415```
1416
1417`Users.org`, after the `#+end_src` at line 257 and before
1418`Sourcehut has no API of that shape`:
1419
1420```org
1421=repo import= fetches over http and https only, from an address that
1422passes the same check as a webhook target; a =git://= URL is refused,
1423so use the repository's https URL.
1424
1425```
1426
1427- [ ] **Step 4: Changelog**
1428
1429Append to the `* Unreleased` list, directly above `* v1.36.0 — 2026-09-23`:
1430
1431```org
1432- =repo import --from= takes http and https URLs only; =git://= is
1433 refused, since its connection cannot be held to a checked address.
1434 The host is resolved and checked like a mirror's, git connects only
1435 to the checked addresses with redirects off, and the system and
1436 global gitconfig are ignored. A source that redirects (a renamed
1437 repository) fails; import from the URL it redirects to. Needs git
1438 2.37 or later on the server (#298).
1439```
1440
1441- [ ] **Step 5: Commit, MR, merge**
1442
1443```bash
1444git add e2e/import_test.go .gitbay/wiki/Threat-Model.org .gitbay/wiki/Architecture/09-Controls.org \
1445 .gitbay/wiki/Architecture/10-Known-Gaps.org .gitbay/wiki/Architecture/02-Components.org \
1446 .gitbay/wiki/Admin.org .gitbay/wiki/Users.org CHANGELOG.org
1447git commit -S -m "import: document the address check; e2e imports with allow_local
1448
1449Closes #298"
1450git push -u origin import-pin-address
1451gitbay mr create --source import-pin-address --target main --title "repo import: http(s) only, address checked and pinned"
1452```
1453
1454Merge `--strategy ff` after CI, delete the branch both places.
1455
1456---
1457
1458# MR 3: LFS tokens die with their key (branch `lfs-token-key`, closes #285)
1459
1460### Task 3.1: the token names the key that obtained it
1461
1462**Files:**
1463- Modify: `internal/lfs/lfs.go:122-169` (`Sign`, `Verify`)
1464- Create: `internal/lfs/lfs_test.go`
1465- Modify: `internal/sshd/lfs.go:16-78` (`runLFSAuthenticate`)
1466- Modify: `internal/sshd/sshd.go:499` (call site)
1467- Modify: `internal/httpd/lfs.go:38-58` (`lfsAuth`), `:99`, `:130` (`lfsBatch`), `:180-185` (`lfsDownload`), `:199-204` (`lfsUpload`)
1468
1469**Interfaces:**
1470- Produces:
1471 - `func Sign(secret []byte, repoID, keyID int64, op string, now time.Time) string`
1472 - `type Grant struct { RepoID, KeyID int64; Op string }`
1473 - `func Verify(secret []byte, token string, now time.Time) (Grant, bool)`
1474 - `func runLFSAuthenticate(cfg config.Config, st *store.Store, user store.User, key store.SSHKey, argv []string, stdout, stderr io.Writer) int`
1475 - `func (s *Server) lfsAuth(r *http.Request, repo store.Repo) (op string, keyID int64)`
1476- Consumed by Task 3.2: `lfsAuth`'s signature, `Grant.KeyID`.
1477
1478- [ ] **Step 1: Write the failing tests**
1479
1480`internal/lfs/lfs_test.go`:
1481
1482```go
1483package lfs
1484
1485import (
1486 "crypto/hmac"
1487 "crypto/sha256"
1488 "encoding/base64"
1489 "fmt"
1490 "testing"
1491 "time"
1492)
1493
1494func TestTokenCarriesTheKey(t *testing.T) {
1495 secret := []byte("secret")
1496 now := time.Now()
1497 tok := Sign(secret, 7, 42, "upload", now)
1498 g, ok := Verify(secret, tok, now)
1499 if !ok || g != (Grant{RepoID: 7, KeyID: 42, Op: "upload"}) {
1500 t.Fatalf("Verify = %+v, %v", g, ok)
1501 }
1502 if _, ok := Verify(secret, tok, now.Add(TokenTTL+time.Second)); ok {
1503 t.Error("an expired token verified")
1504 }
1505 if _, ok := Verify([]byte("other"), tok, now); ok {
1506 t.Error("a token verified under another secret")
1507 }
1508 if g, ok := Verify(secret, Sign(secret, 7, 0, "download", now), now); !ok || g.KeyID != 0 {
1509 t.Errorf("anonymous grant = %+v, %v", g, ok)
1510 }
1511}
1512
1513// A token minted before tokens named their key has three fields. It is
1514// refused, not read as a grant bound to no key (#285).
1515func TestUnboundTokenRefused(t *testing.T) {
1516 secret := []byte("secret")
1517 payload := fmt.Sprintf("%d:%s:%d", 7, "upload", time.Now().Add(TokenTTL).Unix())
1518 mac := hmac.New(sha256.New, secret)
1519 mac.Write([]byte(payload))
1520 tok := base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
1521 base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
1522 if g, ok := Verify(secret, tok, time.Now()); ok {
1523 t.Fatalf("a pre-upgrade token verified: %+v", g)
1524 }
1525}
1526```
1527
1528- [ ] **Step 2: Run them and see them fail**
1529
1530Run: `go test ./internal/lfs -count=1`
1531Expected: build failure, `too many arguments in call to Sign` /
1532`undefined: Grant`.
1533
1534- [ ] **Step 3: Implement the lfs package**
1535
1536Replace lines 122-169 of `internal/lfs/lfs.go`:
1537
1538```go
1539// Tokens bridge SSH authentication to the HTTP endpoints: stateless,
1540// HMAC-signed, scoped to one repo and one operation, short-lived, and
1541// bound to the SSH key that obtained them, which must still be live
1542// when the token is used (#285). The secret persists in the settings
1543// table so tokens survive restarts.
1544
1545const TokenTTL = time.Hour
1546
1547// Sign mints a token for op ("download" or "upload") on repoID, bound
1548// to keyID: the SSH key, user or deploy, that asked for it, or 0 for an
1549// anonymous download of a public repository.
1550func Sign(secret []byte, repoID, keyID int64, op string, now time.Time) string {
1551 payload := fmt.Sprintf("%d:%d:%s:%d", repoID, keyID, op, now.Add(TokenTTL).Unix())
1552 mac := hmac.New(sha256.New, secret)
1553 mac.Write([]byte(payload))
1554 return base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
1555 base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
1556}
1557
1558// Grant is what a verified token authorizes.
1559type Grant struct {
1560 RepoID int64
1561 KeyID int64 // 0: an anonymous download of a public repository
1562 Op string
1563}
1564
1565// Verify checks a token's MAC, shape and expiry. A token from before
1566// tokens named their key does not verify.
1567func Verify(secret []byte, token string, now time.Time) (Grant, bool) {
1568 payloadB64, macB64, found := strings.Cut(token, ".")
1569 if !found {
1570 return Grant{}, false
1571 }
1572 payload, err := base64.RawURLEncoding.DecodeString(payloadB64)
1573 if err != nil {
1574 return Grant{}, false
1575 }
1576 gotMAC, err := base64.RawURLEncoding.DecodeString(macB64)
1577 if err != nil {
1578 return Grant{}, false
1579 }
1580 mac := hmac.New(sha256.New, secret)
1581 mac.Write(payload)
1582 if !hmac.Equal(mac.Sum(nil), gotMAC) {
1583 return Grant{}, false
1584 }
1585 parts := strings.Split(string(payload), ":")
1586 if len(parts) != 4 {
1587 return Grant{}, false
1588 }
1589 repoID, err1 := strconv.ParseInt(parts[0], 10, 64)
1590 keyID, err2 := strconv.ParseInt(parts[1], 10, 64)
1591 exp, err3 := strconv.ParseInt(parts[3], 10, 64)
1592 if err1 != nil || err2 != nil || err3 != nil || keyID < 0 || now.Unix() > exp {
1593 return Grant{}, false
1594 }
1595 if parts[2] != "download" && parts[2] != "upload" {
1596 return Grant{}, false
1597 }
1598 return Grant{RepoID: repoID, KeyID: keyID, Op: parts[2]}, true
1599}
1600```
1601
1602- [ ] **Step 4: Update the callers**
1603
1604`internal/sshd/lfs.go`, lines 16-29 (doc comment, signature and the
1605usage check) become:
1606
1607```go
1608// runLFSAuthenticate answers the git-lfs client's SSH probe:
1609//
1610// git-lfs-authenticate <path> download|upload
1611//
1612// with the HTTP endpoint and a short-lived repo- and operation-scoped
1613// token. Access rules mirror the git transports: download needs read,
1614// upload needs write; deploy keys authorize by their binding alone, and
1615// every denial on an invisible repo reads as nonexistence. The token
1616// names key, so it stops working when the key does (#285).
1617func runLFSAuthenticate(cfg config.Config, st *store.Store, user store.User, key store.SSHKey,
1618 argv []string, stdout, stderr io.Writer) int {
1619 scope := key.Scope
1620 if len(argv) != 3 || (argv[2] != "download" && argv[2] != "upload") {
1621 fmt.Fprintln(stderr, "usage: git-lfs-authenticate <path> download|upload")
1622 return protocol.ExitUsage
1623 }
1624```
1625
1626Lines 30-69 stay as they are (they read `scope`). Line 70 becomes:
1627
1628```go
1629 token := lfs.Sign([]byte(secret), repo.ID, key.ID, op, time.Now())
1630```
1631
1632`internal/sshd/sshd.go:499`:
1633
1634```go
1635 return runLFSAuthenticate(cfg, st, user, key, argv, stdout, stderr)
1636```
1637
1638`internal/httpd/lfs.go`, `lfsAuth` for now carries the key id through
1639without checking it (Task 3.2 adds the check):
1640
1641```go
1642// lfsAuth resolves what the request may do to the repo: "upload",
1643// "download", or "" for no access, and the key the grant rests on (0
1644// for none). Tokens are repo-scoped; without one, public repos allow
1645// anonymous download only.
1646func (s *Server) lfsAuth(r *http.Request, repo store.Repo) (string, int64) {
1647 auth := r.Header.Get("Authorization")
1648 if tok, ok := strings.CutPrefix(auth, "Bearer "); ok {
1649 secret, err := s.lfsSecret()
1650 if err != nil {
1651 return "", 0
1652 }
1653 g, ok := lfs.Verify(secret, tok, time.Now())
1654 if !ok || g.RepoID != repo.ID {
1655 return "", 0
1656 }
1657 return g.Op, g.KeyID
1658 }
1659 if repo.Visibility == "public" {
1660 return "download", 0
1661 }
1662 return "", 0
1663}
1664```
1665
1666`lfsBatch` line 99 and line 130:
1667
1668```go
1669 granted, keyID := s.lfsAuth(r, repo)
1670```
1671
1672```go
1673 transferToken := lfs.Sign(secret, repo.ID, keyID, req.Operation, time.Now())
1674```
1675
1676`lfsDownload`, lines 181-185:
1677
1678```go
1679 repo, err := s.st.RepoByPath(r.PathValue("owner") + "/" + r.PathValue("repo"))
1680 if err != nil {
1681 lfsError(w, http.StatusNotFound, "not found")
1682 return
1683 }
1684 if op, _ := s.lfsAuth(r, repo); op == "" {
1685 lfsError(w, http.StatusNotFound, "not found")
1686 return
1687 }
1688```
1689
1690`lfsUpload`, lines 200-204:
1691
1692```go
1693 repo, err := s.st.RepoByPath(r.PathValue("owner") + "/" + r.PathValue("repo"))
1694 if err != nil {
1695 lfsError(w, http.StatusNotFound, "not found")
1696 return
1697 }
1698 if op, _ := s.lfsAuth(r, repo); op != "upload" {
1699 lfsError(w, http.StatusNotFound, "not found")
1700 return
1701 }
1702```
1703
1704- [ ] **Step 5: Run the packages**
1705
1706Run: `go build ./... && go vet ./internal/lfs ./internal/sshd ./internal/httpd && go test ./internal/lfs ./internal/sshd ./internal/httpd -count=1`
1707Expected: PASS.
1708
1709- [ ] **Step 6: Commit**
1710
1711```bash
1712git add internal/lfs internal/sshd/lfs.go internal/sshd/sshd.go internal/httpd/lfs.go
1713git commit -S -m "lfs: a transfer token names the key that obtained it
1714
1715A pre-upgrade token, which names none, no longer verifies.
1716
1717Ref #285"
1718```
1719
1720### Task 3.2: an LFS request checks the token's key is live
1721
1722**Files:**
1723- Modify: `internal/httpd/lfs.go` (`lfsAuth`)
1724- Create: `internal/httpd/lfsauth_test.go`
1725
1726**Interfaces:**
1727- Consumes: `lfs.Sign`, `lfs.Verify`, `Grant` (Task 3.1); `store.LiveSSHKeys(ids []int64) (map[int64]bool, error)`.
1728
1729- [ ] **Step 1: Write the failing tests**
1730
1731`internal/httpd/lfsauth_test.go`:
1732
1733```go
1734package httpd
1735
1736import (
1737 "net/http"
1738 "net/http/httptest"
1739 "testing"
1740 "time"
1741
1742 "gitbay.org/gitbay/internal/lfs"
1743 "gitbay.org/gitbay/internal/store"
1744)
1745
1746func lfsRequest(tok string) *http.Request {
1747 r := httptest.NewRequest("GET", "/alice/app.git/info/lfs/objects/x", nil)
1748 if tok != "" {
1749 r.Header.Set("Authorization", "Bearer "+tok)
1750 }
1751 return r
1752}
1753
1754func lfsTestRepo(t *testing.T, st *store.Store, uid int64, name, visibility string) store.Repo {
1755 t.Helper()
1756 id, err := st.CreateRepo("user", uid, name, visibility)
1757 if err != nil {
1758 t.Fatal(err)
1759 }
1760 repo, err := st.RepoByID(id)
1761 if err != nil {
1762 t.Fatal(err)
1763 }
1764 return repo
1765}
1766
1767// A token works only while its key does: removed, expired or on a
1768// disabled account, the key takes its tokens with it (#285).
1769func TestLFSTokenNeedsALiveKey(t *testing.T) {
1770 s, st, u := newTokenTestServer(t)
1771 repo := lfsTestRepo(t, st, u.ID, "app", "private")
1772 secret, err := s.lfsSecret()
1773 if err != nil {
1774 t.Fatal(err)
1775 }
1776 addKey := func(fp string, exp *time.Time) int64 {
1777 t.Helper()
1778 if err := st.AddSSHKeyFrom(u.ID, fp, "ssh-ed25519", []byte(fp), "full", "", store.KeyOrigin{ExpiresAt: exp}); err != nil {
1779 t.Fatal(err)
1780 }
1781 k, err := st.SSHKeyByFingerprint(fp)
1782 if err != nil {
1783 t.Fatal(err)
1784 }
1785 return k.ID
1786 }
1787
1788 live := addKey("SHA256:live", nil)
1789 tok := lfs.Sign(secret, repo.ID, live, "upload", time.Now())
1790 if op, key := s.lfsAuth(lfsRequest(tok), repo); op != "upload" || key != live {
1791 t.Fatalf("live key: %q, %d", op, key)
1792 }
1793
1794 past := time.Now().Add(-time.Minute)
1795 expired := addKey("SHA256:expired", &past)
1796 if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, expired, "upload", time.Now())), repo); op != "" {
1797 t.Errorf("expired key: %q", op)
1798 }
1799
1800 if err := st.RemoveSSHKey(u.ID, "SHA256:live"); err != nil {
1801 t.Fatal(err)
1802 }
1803 if op, _ := s.lfsAuth(lfsRequest(tok), repo); op != "" {
1804 t.Errorf("removed key: %q", op)
1805 }
1806
1807 other := addKey("SHA256:other", nil)
1808 otherTok := lfs.Sign(secret, repo.ID, other, "download", time.Now())
1809 if op, _ := s.lfsAuth(lfsRequest(otherTok), repo); op != "download" {
1810 t.Fatalf("second key before disable: %q", op)
1811 }
1812 if err := st.SetUserDisabled(u.ID, true); err != nil {
1813 t.Fatal(err)
1814 }
1815 if op, _ := s.lfsAuth(lfsRequest(otherTok), repo); op != "" {
1816 t.Errorf("disabled account: %q", op)
1817 }
1818}
1819
1820// A token with no key comes from an anonymous batch on a public
1821// repository and is worth exactly what anonymous is: a download, while
1822// the repository is public.
1823func TestLFSAnonymousTokenOnlyDownloadsPublic(t *testing.T) {
1824 s, st, u := newTokenTestServer(t)
1825 pub := lfsTestRepo(t, st, u.ID, "big", "public")
1826 priv := lfsTestRepo(t, st, u.ID, "vault", "private")
1827 secret, err := s.lfsSecret()
1828 if err != nil {
1829 t.Fatal(err)
1830 }
1831 now := time.Now()
1832 if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "download", now)), pub); op != "download" {
1833 t.Errorf("public download: %q", op)
1834 }
1835 if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "upload", now)), pub); op != "" {
1836 t.Errorf("anonymous upload: %q", op)
1837 }
1838 if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, priv.ID, 0, "download", now)), priv); op != "" {
1839 t.Errorf("private download: %q", op)
1840 }
1841}
1842```
1843
1844- [ ] **Step 2: Run them and see them fail**
1845
1846Run: `go test ./internal/httpd -run 'TestLFS' -count=1`
1847Expected: FAIL: `expired key: "upload"`, `removed key: "upload"`,
1848`disabled account: "download"`, `anonymous upload: "upload"`,
1849`private download: "download"`.
1850
1851- [ ] **Step 3: Implement**
1852
1853`lfsAuth`:
1854
1855```go
1856// lfsAuth resolves what the request may do to the repo: "upload",
1857// "download", or "" for no access, and the key the grant rests on (0
1858// for none). A token is bound to the SSH key that obtained it and
1859// works only while that key is registered, unexpired and on an enabled
1860// account (#285). Without one, public repos allow anonymous download
1861// only.
1862func (s *Server) lfsAuth(r *http.Request, repo store.Repo) (string, int64) {
1863 auth := r.Header.Get("Authorization")
1864 if tok, ok := strings.CutPrefix(auth, "Bearer "); ok {
1865 secret, err := s.lfsSecret()
1866 if err != nil {
1867 return "", 0
1868 }
1869 g, ok := lfs.Verify(secret, tok, time.Now())
1870 if !ok || g.RepoID != repo.ID {
1871 return "", 0
1872 }
1873 if g.KeyID == 0 {
1874 // Minted by an anonymous batch: worth what anonymous is.
1875 if g.Op == "download" && repo.Visibility == "public" {
1876 return "download", 0
1877 }
1878 return "", 0
1879 }
1880 live, err := s.st.LiveSSHKeys([]int64{g.KeyID})
1881 if err != nil || !live[g.KeyID] {
1882 return "", 0
1883 }
1884 return g.Op, g.KeyID
1885 }
1886 if repo.Visibility == "public" {
1887 return "download", 0
1888 }
1889 return "", 0
1890}
1891```
1892
1893- [ ] **Step 4: Run the package**
1894
1895Run: `go test ./internal/httpd -count=1 && go vet ./internal/httpd`
1896Expected: PASS.
1897
1898- [ ] **Step 5: Commit**
1899
1900```bash
1901git add internal/httpd/lfs.go internal/httpd/lfsauth_test.go
1902git commit -S -m "lfs: refuse a token whose key was removed, expired or disabled
1903
1904Ref #285"
1905```
1906
1907### Task 3.3: e2e, wiki and changelog
1908
1909**Files:**
1910- Modify: `e2e/lfs_test.go` (new test after `TestLFS`)
1911- Modify: `.gitbay/wiki/Architecture/05-Identity-and-Access.org:25`, `Architecture/04-Trust-Boundaries.org:109-116`, `Architecture/09-Controls.org:28`, `Threat-Model.org:44-53`
1912- Modify: `CHANGELOG.org`
1913
1914**Interfaces:**
1915- Consumes: e2e helpers `startInstance`, `inst.newKey`, `inst.admin`, `inst.ssh`, `fingerprint` (`e2e/revoke_test.go:38`).
1916
1917- [ ] **Step 1: Write the e2e test**
1918
1919Append to `e2e/lfs_test.go`:
1920
1921```go
1922// A transfer token works only while the key that obtained it does:
1923// removing the key ends it before its hour is up (#285). No git-lfs
1924// client needed: the token comes from git-lfs-authenticate over SSH.
1925func TestLFSTokenEndsWithItsKey(t *testing.T) {
1926 t.Parallel()
1927 inst := startInstance(t)
1928 aliceKey := inst.newKey(t, "alice")
1929 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
1930 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/vault", "--private"); code != 0 {
1931 t.Fatalf("repo create: %s", errOut)
1932 }
1933 spare := inst.newKey(t, "spare")
1934 pub, err := os.ReadFile(spare + ".pub")
1935 if err != nil {
1936 t.Fatal(err)
1937 }
1938 if _, errOut, code := inst.ssh(t, aliceKey, string(pub), "keys", "add"); code != 0 {
1939 t.Fatalf("keys add: %s", errOut)
1940 }
1941 out, errOut, code := inst.ssh(t, spare, "", "git-lfs-authenticate", "alice/vault", "download")
1942 if code != 0 {
1943 t.Fatalf("authenticate: %s", errOut)
1944 }
1945 var grant struct {
1946 Header map[string]string `json:"header"`
1947 }
1948 if err := json.Unmarshal([]byte(out), &grant); err != nil {
1949 t.Fatalf("authenticate JSON: %v\n%s", err, out)
1950 }
1951 batch := func() int {
1952 body := fmt.Sprintf(`{"operation":"download","transfers":["basic"],"objects":[{"oid":%q,"size":4}]}`, strings.Repeat("ab", 32))
1953 req, _ := http.NewRequest("POST",
1954 fmt.Sprintf("http://127.0.0.1:%d/alice/vault.git/info/lfs/objects/batch", inst.httpPort),
1955 strings.NewReader(body))
1956 req.Header.Set("Content-Type", "application/vnd.git-lfs+json")
1957 req.Header.Set("Authorization", grant.Header["Authorization"])
1958 resp, err := http.DefaultClient.Do(req)
1959 if err != nil {
1960 t.Fatal(err)
1961 }
1962 resp.Body.Close()
1963 return resp.StatusCode
1964 }
1965 if code := batch(); code != 200 {
1966 t.Fatalf("batch with a live key: %d", code)
1967 }
1968 if _, errOut, code := inst.ssh(t, aliceKey, "", "keys", "remove", fingerprint(t, spare+".pub")); code != 0 {
1969 t.Fatalf("keys remove: %s", errOut)
1970 }
1971 if code := batch(); code != 404 {
1972 t.Fatalf("batch after the key was removed: %d, want 404", code)
1973 }
1974}
1975```
1976
1977The file already imports `encoding/json`, `fmt`, `net/http`, `os` and
1978`strings`.
1979
1980- [ ] **Step 2: Run it**
1981
1982Run: `go build ./... && go test ./e2e -run 'TestLFSTokenEndsWithItsKey$' -count=1`
1983Expected: PASS.
1984
1985- [ ] **Step 3: Wiki**
1986
1987`Architecture/05-Identity-and-Access.org:25`, the LFS row:
1988
1989```org
1990| LFS transfer token | HMAC-SHA256 over repo, SSH key, operation, expiry | not stored (stateless) | one repository, upload or download | 1 h | with its key: refused once the key is removed or expires, or its account is disabled |
1991```
1992
1993`Architecture/04-Trust-Boundaries.org`, section `** F. LFS`, the
1994paragraph becomes:
1995
1996```org
1997=git-lfs-authenticate= over SSH applies the same repository checks as
1998git transport and returns a one-hour HMAC token scoped to repository,
1999operation and the SSH key that asked for it, deploy keys included
2000(=internal/sshd/lfs.go=, =internal/lfs/lfs.go=). The HTTP batch, upload
2001and download endpoints verify that token and that its key is still
2002registered, unexpired and on an enabled account (=store.LiveSSHKeys=);
2003public repositories allow anonymous download. Objects are verified
2004against their SHA-256 id on upload.
2005```
2006
2007`Architecture/09-Controls.org:28`:
2008
2009```org
2010| Revocation takes effect immediately | in place | removing a key or disabling an account closes its connections; every exec re-reads its key (=internal/sshd/sshd.go=); LFS transfer tokens are refused with their key (=internal/httpd/lfs.go=) |
2011```
2012
2013`Threat-Model.org`, the `*Revocation is immediate.*` bullet: after
2014"a push killed before its pre-receive hook answers moves no ref."
2015insert:
2016
2017```org
2018 An LFS transfer token names the key that obtained it and is refused
2019 from the moment that key is.
2020```
2021
2022- [ ] **Step 4: Changelog**
2023
2024Append to the `* Unreleased` list, directly above `* v1.36.0 — 2026-09-23`:
2025
2026```org
2027- LFS transfer tokens name the SSH key that obtained them, deploy keys
2028 included, and every LFS request checks that the key is still
2029 registered, unexpired and on an enabled account: removing a key or
2030 disabling an account ends its tokens at once instead of within the
2031 hour (#285). *Operators:* tokens minted before the upgrade are
2032 refused. git-lfs asks =git-lfs-authenticate= for a token each time
2033 it runs, so only a transfer running across the restart fails, with
2034 "repository not found"; running the command again fixes it.
2035```
2036
2037- [ ] **Step 5: Commit, MR, merge**
2038
2039```bash
2040git add e2e/lfs_test.go .gitbay/wiki/Architecture/05-Identity-and-Access.org \
2041 .gitbay/wiki/Architecture/04-Trust-Boundaries.org .gitbay/wiki/Architecture/09-Controls.org \
2042 .gitbay/wiki/Threat-Model.org CHANGELOG.org
2043git commit -S -m "lfs: e2e for a token outliving its key; document the binding
2044
2045Closes #285"
2046git push -u origin lfs-token-key
2047gitbay mr create --source lfs-token-key --target main --title "lfs: transfer tokens die with the key that obtained them"
2048```
2049
2050Merge `--strategy ff` after CI, delete the branch both places.
2051
2052---
2053
2054# MR 4: minting from the web needs a recent sign-in (branch `web-mint-reauth`, closes #297)
2055
2056### Task 4.1: the session's sign-in time reaches the user
2057
2058**Files:**
2059- Modify: `internal/store/users.go:11-17` (`User`)
2060- Modify: `internal/store/sessions.go:83-102` (`WebSessionUser`)
2061- Test: `internal/store/sessions_test.go`
2062
2063**Interfaces:**
2064- Produces: `store.User.SignedInAt time.Time` — set by `WebSessionUser` from `web_sessions.created_at`; zero on every other path.
2065
2066- [ ] **Step 1: Write the failing test**
2067
2068Append to `internal/store/sessions_test.go`:
2069
2070```go
2071// A session's sign-in time is its creation; using the session renews
2072// its idle expiry and leaves the sign-in time alone (#297).
2073func TestWebSessionUserSignedInAt(t *testing.T) {
2074 s, uid := sessionFixture(t)
2075 _, hash, err := NewToken()
2076 if err != nil {
2077 t.Fatal(err)
2078 }
2079 if err := s.CreateWebSession(hash, uid, 7*24*time.Hour); err != nil {
2080 t.Fatal(err)
2081 }
2082 u, err := s.WebSessionUser(hash)
2083 if err != nil {
2084 t.Fatal(err)
2085 }
2086 if age := time.Since(u.SignedInAt); age < 0 || age > time.Minute {
2087 t.Fatalf("fresh session signed in %v ago", age)
2088 }
2089
2090 signedIn := time.Now().Add(-2 * time.Hour)
2091 if _, err := s.DB.Exec("UPDATE web_sessions SET created_at = ?, last_used_at = ? WHERE token_hash = ?",
2092 fmtTime(signedIn), fmtTime(signedIn), hash); err != nil {
2093 t.Fatal(err)
2094 }
2095 u, err = s.WebSessionUser(hash)
2096 if err != nil {
2097 t.Fatal(err)
2098 }
2099 var last string
2100 if err := s.DB.QueryRow("SELECT last_used_at FROM web_sessions WHERE token_hash = ?", hash).Scan(&last); err != nil {
2101 t.Fatal(err)
2102 }
2103 if last == fmtTime(signedIn) {
2104 t.Fatal("using the session did not renew it")
2105 }
2106 if want := signedIn.UTC().Truncate(time.Millisecond); !u.SignedInAt.Equal(want) {
2107 t.Fatalf("SignedInAt = %v, want %v", u.SignedInAt, want)
2108 }
2109}
2110```
2111
2112- [ ] **Step 2: Run it and see it fail**
2113
2114Run: `go test ./internal/store -run TestWebSessionUserSignedInAt -count=1`
2115Expected: build failure, `u.SignedInAt undefined`.
2116
2117- [ ] **Step 3: Implement**
2118
2119`internal/store/users.go`, `User`:
2120
2121```go
2122type User struct {
2123 ID int64
2124 Username string
2125 IsAdmin bool
2126 Pending bool // self-registered, email not yet verified
2127 Disabled bool // administratively suspended
2128 // SignedInAt is when the browser session this user came from was
2129 // created by a login. Set by WebSessionUser only; zero elsewhere.
2130 SignedInAt time.Time
2131}
2132```
2133
2134`internal/store/sessions.go`, `WebSessionUser`:
2135
2136```go
2137// WebSessionUser resolves a session cookie hash to its user, with the
2138// session's sign-in time, and renews the session's idle expiry. A
2139// session is written at most once a minute, so a burst of requests
2140// costs one UPDATE. Renewal never moves created_at: only a login
2141// creates a session, so created_at is when it signed in.
2142func (s *Store) WebSessionUser(hash string) (User, error) {
2143 now := time.Now()
2144 var userID int64
2145 var created string
2146 err := s.DB.QueryRow(
2147 "SELECT user_id, created_at FROM web_sessions WHERE token_hash = ? AND expires_at > ?",
2148 hash, fmtTime(now)).Scan(&userID, &created)
2149 if errors.Is(err, sql.ErrNoRows) {
2150 return User{}, ErrNotFound
2151 }
2152 if err != nil {
2153 return User{}, err
2154 }
2155 s.DB.Exec(`UPDATE web_sessions SET last_used_at = ?, expires_at = min(absolute_expires_at, ?)
2156 WHERE token_hash = ? AND last_used_at < ?`,
2157 fmtTime(now), fmtTime(now.Add(WebSessionIdle)), hash, fmtTime(now.Add(-time.Minute)))
2158 u, err := s.UserByID(userID)
2159 if err != nil {
2160 return User{}, err
2161 }
2162 if t := parseTime(sql.NullString{String: created, Valid: true}); t != nil {
2163 u.SignedInAt = *t
2164 }
2165 return u, nil
2166}
2167```
2168
2169- [ ] **Step 4: Run the package**
2170
2171Run: `go test ./internal/store -count=1 && go vet ./internal/store`
2172Expected: PASS.
2173
2174- [ ] **Step 5: Commit**
2175
2176```bash
2177git add internal/store/users.go internal/store/sessions.go internal/store/sessions_test.go
2178git commit -S -m "store: a web session's user carries its sign-in time
2179
2180Ref #297"
2181```
2182
2183### Task 4.2: Dispatch refuses a stale session's credential mint
2184
2185**Files:**
2186- Modify: `internal/control/control.go:21-70` (`Ctx`), `:205-209` (`runChecked`, the `Expires` block)
2187- Create: `internal/control/reauth_test.go`
2188
2189**Interfaces:**
2190- Consumes: none from Task 4.1 (the field is filled by httpd in Task 4.3).
2191- Produces:
2192 - `const ReauthWindow = 15 * time.Minute`
2193 - `const ReauthRefusal = "creating a credential from the web needs a sign-in from the last 15 minutes; sign in again, then submit the form again"`
2194 - `Ctx.SignedInAt *time.Time`
2195
2196- [ ] **Step 1: Write the failing test**
2197
2198`internal/control/reauth_test.go`:
2199
2200```go
2201package control
2202
2203import (
2204 "strings"
2205 "testing"
2206 "time"
2207
2208 "gitbay.org/gitbay/internal/protocol"
2209 "gitbay.org/gitbay/internal/store"
2210)
2211
2212// A browser session mints credentials only within ReauthWindow of
2213// signing in; SSH and the API carry no session and are not affected
2214// (#297).
2215func TestMintNeedsRecentWebSignIn(t *testing.T) {
2216 st, _, uid := newQueueTestRepo(t)
2217 user := store.User{ID: uid, Username: "alice"}
2218 run := func(signedIn *time.Time, stdin string, argv ...string) (string, int) {
2219 c, errOut := pruneCtx(st, t.TempDir(), user)
2220 c.Cfg.Limits.WriteRate = -1
2221 c.SignedInAt = signedIn
2222 c.Stdin = strings.NewReader(stdin)
2223 code := Dispatch(c, argv)
2224 return strings.TrimSpace(errOut.String()), code
2225 }
2226 fresh := time.Now().Add(-time.Minute)
2227 stale := time.Now().Add(-ReauthWindow - time.Minute)
2228
2229 if msg, code := run(&stale, authorizedKey(t, "stale"), "keys", "add"); code != protocol.ExitDenied || msg != ReauthRefusal {
2230 t.Fatalf("stale session: exit %d, %q", code, msg)
2231 }
2232 if msg, code := run(&fresh, authorizedKey(t, "fresh"), "keys", "add"); code != protocol.ExitOK {
2233 t.Fatalf("fresh session: exit %d, %q", code, msg)
2234 }
2235 // SSH and the API set no SignedInAt.
2236 if msg, code := run(nil, authorizedKey(t, "ssh"), "keys", "add"); code != protocol.ExitOK {
2237 t.Fatalf("no session: exit %d, %q", code, msg)
2238 }
2239 // A command that mints nothing is not held back.
2240 if msg, code := run(&stale, "", "keys", "list"); code != protocol.ExitOK {
2241 t.Fatalf("keys list on a stale session: exit %d, %q", code, msg)
2242 }
2243 keys, err := st.ListSSHKeys(uid)
2244 if err != nil || len(keys) != 2 {
2245 t.Fatalf("keys: %d %v, want the fresh and the ssh one", len(keys), err)
2246 }
2247}
2248```
2249
2250- [ ] **Step 2: Run it and see it fail**
2251
2252Run: `go test ./internal/control -run TestMintNeedsRecentWebSignIn -count=1`
2253Expected: build failure, `c.SignedInAt undefined`, `undefined: ReauthWindow`.
2254
2255- [ ] **Step 3: Implement**
2256
2257`Ctx`, after `Expires`:
2258
2259```go
2260 // SignedInAt is when the browser session behind this request signed
2261 // in; nil off the web. Dispatch refuses MintsCredential commands when
2262 // it is older than ReauthWindow.
2263 SignedInAt *time.Time
2264```
2265
2266After the `Ctx` type:
2267
2268```go
2269// ReauthWindow is how long after signing in a browser session may run a
2270// MintsCredential command. A session lasts days and its cookie is a
2271// bearer credential; what it creates must come from a recent sign-in
2272// (#297).
2273const ReauthWindow = 15 * time.Minute
2274
2275// ReauthRefusal is what a web session signed in longer ago than
2276// ReauthWindow gets; the web shows a sign-in link beside it.
2277const ReauthRefusal = "creating a credential from the web needs a sign-in from the last 15 minutes; sign in again, then submit the form again"
2278```
2279
2280`runChecked`, directly after the `Expires` block:
2281
2282```go
2283 if cmd.MintsCredential && c.SignedInAt != nil && time.Since(*c.SignedInAt) > ReauthWindow {
2284 return c.fail(protocol.ExitDenied, "%s", ReauthRefusal)
2285 }
2286```
2287
2288- [ ] **Step 4: Run the package**
2289
2290Run: `go test ./internal/control -count=1 && go vet ./internal/control`
2291Expected: PASS.
2292
2293- [ ] **Step 5: Commit**
2294
2295```bash
2296git add internal/control/control.go internal/control/reauth_test.go
2297git commit -S -m "control: a web session mints credentials only soon after signing in
2298
2299Ref #297"
2300```
2301
2302### Task 4.3: every web dispatch carries the sign-in time; the refusal links to sign-in
2303
2304**Files:**
2305- Modify: `internal/httpd/control.go:25-200` (the five `control.Ctx` literals → `webCtx`)
2306- Modify: `internal/httpd/flash.go` (add `reauthNotice`; import `control`)
2307- Modify: `internal/httpd/account.go:131-154` (`renderAccount`)
2308- Modify: `internal/httpd/settings.go:20-30` (`settingsPage`), `:67-74` (`settingsFormWith`)
2309- Modify: `internal/web/templates/account.html:5`, `settings.html:6`
2310- Modify: `internal/httpd/account_test.go:289-303` (`newTokenTestServer`)
2311- Create: `internal/httpd/reauth_test.go`
2312
2313**Interfaces:**
2314- Consumes: `store.User.SignedInAt` (Task 4.1); `control.Ctx.SignedInAt`, `control.ReauthWindow`, `control.ReauthRefusal` (Task 4.2).
2315- Produces:
2316 - `func (s *Server) webCtx(u store.User, stdin string, stdout, stderr io.Writer) *control.Ctx`
2317 - `func (s *Server) reauthNotice(w http.ResponseWriter, notice, path string) bool`
2318 - page fields `Reauth bool` on the account page struct and `settingsPage`.
2319
2320- [ ] **Step 1: Write the failing tests**
2321
2322`internal/httpd/reauth_test.go`:
2323
2324```go
2325package httpd
2326
2327import (
2328 "net/http"
2329 "net/http/httptest"
2330 "net/url"
2331 "strings"
2332 "testing"
2333 "time"
2334
2335 "gitbay.org/gitbay/internal/control"
2336 "gitbay.org/gitbay/internal/store"
2337)
2338
2339// A session signed in longer ago than ReauthWindow cannot mint from the
2340// settings page: the form comes back with the refusal and a sign-in
2341// link, and the sign-in returns to /settings (#297).
2342func TestWebMintNeedsRecentSignIn(t *testing.T) {
2343 s, st, u := newTokenTestServer(t)
2344 stale := u
2345 stale.SignedInAt = time.Now().Add(-control.ReauthWindow - time.Minute)
2346 rr := submitAccountForm(t, s, stale, url.Values{"field": {"token-create"}, "name": {"laptop"}, "scope": {"full"}})
2347 if rr.Code != http.StatusSeeOther {
2348 t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
2349 }
2350 if list, err := st.ListAPITokens(u.ID); err != nil || len(list) != 0 {
2351 t.Fatalf("a stale session minted %+v (%v)", list, err)
2352 }
2353
2354 req := httptest.NewRequest("GET", "/settings", nil)
2355 for _, c := range rr.Result().Cookies() {
2356 req.AddCookie(c)
2357 }
2358 page := httptest.NewRecorder()
2359 s.accountPage(page, req, stale)
2360 body := page.Body.String()
2361 if !strings.Contains(body, control.ReauthRefusal) {
2362 t.Fatalf("refusal not shown: %s", body)
2363 }
2364 if !strings.Contains(body, `<a href="/login">Sign in again</a>`) {
2365 t.Fatalf("no sign-in link: %s", body)
2366 }
2367 var next string
2368 for _, c := range page.Result().Cookies() {
2369 if c.Name == nextCookie {
2370 next = c.Value
2371 }
2372 }
2373 if next != url.QueryEscape("/settings") {
2374 t.Fatalf("gitbay_next = %q, want /settings", next)
2375 }
2376}
2377
2378// An API token has no browser session: minting through the API is not
2379// held to the sign-in window.
2380func TestAPIMintIgnoresTheSignInWindow(t *testing.T) {
2381 s, st, u := newTokenTestServer(t)
2382 if err := st.CreateAPIToken(u.ID, "ci", store.HashToken("gb_reauthtest"), "full", nil, 0); err != nil {
2383 t.Fatal(err)
2384 }
2385 req := httptest.NewRequest("POST", "/api/v1/cmd",
2386 strings.NewReader(`{"argv":["token","create","--name","second","--scope","read"]}`))
2387 req.Header.Set("Authorization", "Bearer gb_reauthtest")
2388 rr := httptest.NewRecorder()
2389 s.apiCmd(rr, req)
2390 if rr.Code != http.StatusOK {
2391 t.Fatalf("status %d: %s", rr.Code, rr.Body.String())
2392 }
2393}
2394```
2395
2396`internal/httpd/account_test.go`, `newTokenTestServer`'s return: the
2397session behind the tests' forms signed in just now, so the minting
2398tests exercise a fresh session:
2399
2400```go
2401 return New(config.Default(), st, nil), st, store.User{ID: uid, Username: "alice", SignedInAt: time.Now()}
2402```
2403
2404(`account_test.go` gains the `"time"` import if it lacks it.)
2405
2406- [ ] **Step 2: Run them and see them fail**
2407
2408Run: `go test ./internal/httpd -run 'TestWebMintNeedsRecentSignIn|TestAPIMintIgnoresTheSignInWindow' -count=1`
2409Expected: `TestWebMintNeedsRecentSignIn` FAILs with `status 200`
2410(the stale session minted); `TestAPIMintIgnoresTheSignInWindow` PASSes.
2411
2412- [ ] **Step 3: One Ctx for every web dispatch**
2413
2414`internal/httpd/control.go`, after `runControl`:
2415
2416```go
2417// webCtx is the Ctx every web dispatch runs under: the session's user
2418// with full scope, and when that session signed in, which Dispatch
2419// checks before a command that mints a credential (#297).
2420func (s *Server) webCtx(u store.User, stdin string, stdout, stderr io.Writer) *control.Ctx {
2421 signedIn := u.SignedInAt
2422 return &control.Ctx{
2423 User: u,
2424 Source: "web",
2425 Scope: "full",
2426 Store: s.st,
2427 Cfg: s.cfg,
2428 Stdin: strings.NewReader(stdin),
2429 Stdout: stdout,
2430 Stderr: stderr,
2431 ViaAPI: true,
2432 SignedInAt: &signedIn,
2433 }
2434}
2435```
2436
2437Replace the five literals:
2438
2439- `runControlCode`: `ctx := s.webCtx(u, "", &stdout, &stderr)`
2440- `runControlStream`:
2441 ```go
2442 ctx := s.webCtx(u, "", out, &stderr)
2443 ctx.Done = done
2444 ctx.Stopping = s.stopping
2445 ```
2446- `runControlStdinCode`: `ctx := s.webCtx(u, stdin, &stdout, &stderr)`
2447- `dispatchIntoStdin`:
2448 ```go
2449 ctx := s.webCtx(u, stdin, &stdout, &stderr)
2450 ctx.JSON = true
2451 ```
2452- `dispatchJSON`:
2453 ```go
2454 ctx := s.webCtx(u, stdin, &stdout, &stderr)
2455 ctx.JSON = true
2456 ```
2457
2458`internal/httpd/flash.go`, import `"gitbay.org/gitbay/internal/control"`
2459and add after `peekNext`:
2460
2461```go
2462// reauthNotice reports whether notice is Dispatch's refusal for a
2463// session that signed in too long ago to mint a credential and, when it
2464// is, remembers path so the sign-in the page links to returns there
2465// (#297).
2466func (s *Server) reauthNotice(w http.ResponseWriter, notice, path string) bool {
2467 if notice != control.ReauthRefusal {
2468 return false
2469 }
2470 s.setNext(w, path)
2471 return true
2472}
2473```
2474
2475- [ ] **Step 4: The pages show the link**
2476
2477`internal/httpd/account.go`, `renderAccount`: before `s.render(...)`:
2478
2479```go
2480 notice := s.takeFlash(w, r)
2481 reauth := s.reauthNotice(w, notice, "/settings")
2482```
2483
2484In the page struct, after `TokenShown`:
2485
2486```go
2487 Reauth bool // Notice is the stale-session refusal: link to sign in
2488```
2489
2490and the value list:
2491
2492```go
2493 }{s.baseFor(u), "account", keys, pgp, emails, profile, profileLinksText(profile.Links),
2494 aboutRepo, aboutEdit, s.cfg.SiteHost(),
2495 notice, r.URL.Query().Get("m"), mailOn, watchOn, pushOn, devices, theme,
2496 tokens, tokenShown, reauth})
2497```
2498
2499`internal/web/templates/account.html:5`:
2500
2501```html
2502{{if .Notice}}<p class="error" role="alert">{{.Notice}}{{if .Reauth}} <a href="/login">Sign in again</a>{{end}}</p>{{end}}
2503```
2504
2505`internal/httpd/settings.go`, `settingsPage` gains after `Saved`:
2506
2507```go
2508 Reauth bool // Notice is the stale-session refusal: link to sign in
2509```
2510
2511and `settingsFormWith`'s render:
2512
2513```go
2514 s.render(w, "settings.html", settingsPage{
2515 repoPage: p, Topics: topics, Branches: branches,
2516 DepsEnabled: deps.Enabled, Deps: deps,
2517 Runners: runners,
2518 Notice: notice,
2519 Saved: strings.HasPrefix(notice, "Saved "),
2520 Reauth: s.reauthNotice(w, notice, r.URL.Path),
2521 Submitted: subm,
2522 })
2523```
2524
2525`internal/web/templates/settings.html:6`:
2526
2527```html
2528{{if .Notice}}{{if .Saved}}<p class="notice" role="status">{{.Notice}}</p>{{else}}<p class="error" role="alert">{{.Notice}}{{if .Reauth}} <a href="/login">Sign in again</a>{{end}}</p>{{end}}{{end}}
2529```
2530
2531- [ ] **Step 5: Run the packages**
2532
2533Run: `go build ./... && go vet ./internal/httpd ./internal/web && go test ./internal/httpd ./internal/web -count=1`
2534Expected: PASS, including the existing token tests on the fresh
2535session from `newTokenTestServer`.
2536
2537Run: `go test ./e2e -run 'TestAccountSettingsWeb$' -count=1`
2538Expected: PASS (its session is created by a login moments earlier).
2539
2540- [ ] **Step 6: Commit**
2541
2542```bash
2543git add internal/httpd internal/web/templates/account.html internal/web/templates/settings.html
2544git commit -S -m "web: dispatch carries the session's sign-in time; a stale one gets a sign-in link
2545
2546Ref #297"
2547```
2548
2549### Task 4.4: wiki and changelog
2550
2551**Files:**
2552- Modify: `.gitbay/wiki/Threat-Model.org:57-63`, `Architecture/09-Controls.org:29`, `Architecture/10-Known-Gaps.org:16`, `Architecture/05-Identity-and-Access.org:21`, `:106-120`
2553- Modify: `CHANGELOG.org`
2554
2555**Interfaces:** none.
2556
2557- [ ] **Step 1: Wiki**
2558
2559`Threat-Model.org`, in the `*The control plane is one command
2560registry*` bullet, replace `Browser sessions are not covered yet
2561(#297).` with:
2562
2563```org
2564 A browser session can create one only within 15 minutes of signing
2565 in (=control.ReauthWindow=, #297).
2566```
2567
2568`Architecture/09-Controls.org:29`:
2569
2570```org
2571| Delegation bounded by the delegating credential | in place | expiring tokens refused on =MintsCredential= commands; credentials record their creating token; a browser session mints only within 15 minutes of signing in (=internal/control/control.go=) |
2572```
2573
2574`Architecture/10-Known-Gaps.org`: delete the `#297` row (line 16).
2575
2576`Architecture/05-Identity-and-Access.org:21`, the web session row:
2577
2578```org
2579| Web session | 32 random bytes hex, cookie =gitbay_session= | SHA-256 hash | full account; credential-minting forms only within 15 minutes of sign-in | 12 h idle, 7 days absolute | logout, =web sessions revoke= |
2580```
2581
2582and in `* Session security on the web`, after the destructive-actions
2583bullet:
2584
2585```org
2586- Forms that create a credential (SSH and runner keys, API tokens,
2587 email verification) need a sign-in from the last 15 minutes; an
2588 older session gets the form back with a sign-in link that returns to
2589 it (=control.ReauthWindow=, =internal/httpd/flash.go=).
2590```
2591
2592- [ ] **Step 2: Changelog**
2593
2594Append to the `* Unreleased` list, directly above `* v1.36.0 — 2026-09-23`:
2595
2596```org
2597- A browser session creates credentials (SSH and runner keys, API
2598 tokens, verified addresses) only within 15 minutes of signing in. An
2599 older session gets the form back with a "Sign in again" link, and the
2600 login link returns to the form. SSH and API tokens are unaffected
2601 (#297).
2602```
2603
2604- [ ] **Step 3: Commit, MR, merge**
2605
2606```bash
2607git add .gitbay/wiki/Threat-Model.org .gitbay/wiki/Architecture/09-Controls.org \
2608 .gitbay/wiki/Architecture/10-Known-Gaps.org .gitbay/wiki/Architecture/05-Identity-and-Access.org \
2609 CHANGELOG.org
2610git commit -S -m "wiki: web credential minting needs a recent sign-in
2611
2612Closes #297"
2613git push -u origin web-mint-reauth
2614gitbay mr create --source web-mint-reauth --target main --title "web: minting a credential needs a sign-in from the last 15 minutes"
2615```
2616
2617Merge `--strategy ff` after CI, delete the branch both places.
2618
2619---
2620
2621## Out of scope, noted
2622
2623- `repo import-issues` validates `--api-base` once
2624 (`internal/control/ghimport.go:162`) and then fetches pull heads with
2625 `gitutil.FetchPullHeads` (`ghimport.go:364-379`) and calls the API
2626 with a plain `http.Client`, neither pinned. It is the same class of
2627 gap as #298 and is not in any of the five issues; filed as #301
2628 rather than widening MR 2.