!514 · krz/gitbay

.gitbay/wiki/Admin.org +5 −1

		@@ -934,11 +934,15 @@ other repositories' builds queue until =-repos= is removed again, which
934	934	is a pause, not an outage.
935	935
936	936	#+begin_src sh
	937	# on a machine with an admin gitbay identity (root on the host has none),
	938	# with the runner's public key copied from the host:
937	939	gitbay repo create cmc/runner-scratch # then push a .gitbay/ci.yml naming the image
	940	scp -P 2222 root@<host>:/var/lib/gitbay-runner/.ssh/id_ed25519.pub runner.pub
	941	gitbay repo runner add cmc/runner-scratch < runner.pub
938	942	# on the host:
939		gitbay repo runner add cmc/runner-scratch < /var/lib/gitbay-runner/.ssh/id_ed25519.pub
940	943	sed -i 's#^ExecStart=/usr/local/bin/gitbay-runner #&-repos cmc/runner-scratch #' /etc/systemd/system/gitbay-runner.service.d/override.conf
941	944	systemctl daemon-reload && systemctl restart gitbay-runner
	945	# back on the admin machine:
942	946	gitbay build log cmc/runner-scratch 1 # green: remove -repos, redeploy, delete the scratch repository
943	947	#+end_src
944	948

.gitbay/wiki/Architecture/05-Identity-and-Access.org +2 −1

		@@ -121,7 +121,8 @@ button and the displayed status (=internal/control/mr.go=):
121	121	- Commands that create a credential (SSH, deploy and runner keys, API
122	122	tokens, email verification, login links, PGP keys, device tokens) or
123	123	grant access (repository and organization roles, teams, transfers,
124		admin promote and enable, webhooks, secrets, mirrors) are refused
	124	repository visibility, admin promote and enable, webhooks, secrets,
	125	mirrors) are refused
125	126	from a browser session that signed in more than 15 minutes ago
126	127	(=control.ReauthWindow=, =Command.NeedsRecentSignIn=). The sign-in
127	128	time is =web_sessions.created_at=, which idle renewal does not move;

.gitbay/wiki/Architecture/09-Controls.org +1 −1

		@@ -75,7 +75,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
75	75
76	76	\| Control \| Status \| Evidence \|
77	77	\|---------------------------------------------+----------+------------------------------------------------------------------\|
78		\| 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=) \|
	78	\| SSRF protection on user-supplied URLs \| partial \| webhooks at save and connect; mirrors at save and sync, =repo import= before its fetch, git pinned to the checked address (=internal/gitpin=); =repo import-issues --api-base= checked once and fetched unpinned (#301) \|
79	79	\| Webhook payload integrity \| in place \| HMAC-SHA256 header \|
80	80	\| SMTP credentials protected in transit \| in place \| STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) \|
81	81	\| Upload size limits \| in place \| per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB \|

.gitbay/wiki/Architecture/10-Known-Gaps.org +1 −1

		@@ -13,7 +13,7 @@ what the 2026-09-27 review found; remove a row when its issue closes.
13	13	\| #259 \| Recovery \| No restore has been exercised; the drill is written (Admin wiki) and not yet run \| high \|
14	14	\| #260 \| CI network \| Builds share the runner's source address; no egress policy \| medium \|
15	15	\| #261 \| Various \| Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift \| medium \|
16		\| #301 \| SSRF \| =repo import-issues --api-base= fetches without an address check or pin \| medium \|
	16	\| #301 \| SSRF \| =repo import-issues --api-base= checks the address once; fetches unpinned \| medium \|
17	17
18	18	* Not filed
19	19

.gitbay/wiki/Threat-Model.org +5 −4

		@@ -102,10 +102,11 @@ at connect time, and mirror sync and =repo import= resolve and check
102	102	immediately before running git and pin it to the checked addresses
103	103	(=internal/gitpin=), so a DNS answer that changes after validation
104	104	still cannot reach private space. Redirects are never followed.
105		=repo import= refuses =git://=, which cannot be pinned, and a host
106		written as a bare number or in hex/octal (=0x7f.1=, =2130706433=,
107		=127.1=) rather than dotted decimal, since that form resolves
108		differently across parsers. GitHub-history import (=repo
	105	=repo import= refuses =git://=, which cannot be pinned. Mirror sync
	106	and =repo import= refuse a host written as a bare number or in
	107	hex/octal (=0x7f.1=, =2130706433=, =127.1=) rather than dotted
	108	decimal, since that form resolves differently across parsers; =repo
	109	mirror add= refuses it when the mirror is saved. GitHub-history import (=repo
109	110	import-issues --api-base=) is not yet pinned (#301).
110	111
111	112	* Rendering pushed markup

.gitbay/wiki/Users.org +2 −1

		@@ -702,7 +702,8 @@ or =--all= ends them from the terminal, which is where a lost laptop is
702	702	handled.
703	703
704	704	Actions that create a credential or grant access (adding a key, token
705		or email, org and repository roles, transfers) ask you to sign in again
	705	or email, org and repository roles, transfers, a repository's
	706	visibility) ask you to sign in again
706	707	when your web sign-in is older than 15 minutes. The form shows a "Sign
707	708	in again" link and the login returns to the page. Idle renewal does not
708	709	extend this window.

CHANGELOG.org +27 −16

		@@ -9,8 +9,9 @@ anything beyond "replace the binary and restart" is needed.
9	9	- =webhook add= reads the signing secret from stdin with =--secret -=;
10	10	a value on the command line is refused, since argv shows in process
11	11	listings and shell history. A script that passed the value must pipe
12		it: =printf %s "$SECRET" \| gitbay webhook add <repo> <url> --secret -=
13		(#284).
	12	it: =printf %s "$SECRET" \| gitbay webhook add <repo> <url> --secret -=.
	13	The iOS app's webhook add breaks until it sends the secret on stdin
	14	(krz/gitbay-ios#21) (#284).
14	15	- =repo import --from= takes http and https URLs only; =git://= is
15	16	refused, since its connection cannot be held to a checked address.
16	17	The host is resolved and checked like a mirror's, git connects only
		@@ -18,21 +19,16 @@ anything beyond "replace the binary and restart" is needed.
18	19	global gitconfig are ignored. A source that redirects (a renamed
19	20	repository) fails; import from the URL it redirects to. Needs git
20	21	2.37 or later on the server (#298).
21		Upgrade note. =repo import= and mirror sync now refuse a =git://=
22		source and a =--from=/remote URL carrying a query or fragment. A
23		mirror or import whose host is written numerically (=127.1=,
24		=2130706433=, =0x7f.1=) rather than as a dotted address is refused
25		too; rewrite it before upgrading.
26		- A web session older than 15 minutes cannot mint a credential or grant
27		access — keys, PGP keys, tokens, org membership, and the admin
28		promote/enable actions — and the form it tried shows a sign-in link
29		that returns there (#297).
	22	- =repo mirror add= refuses a host written numerically (=127.1=,
	23	=2130706433=, =0x7f.1=) when the mirror is added, rather than at its
	24	first sync (#298).
30	25	- A browser session creates credentials and grants access — keys, PGP
31	26	keys, tokens, verified addresses, org and repository roles, transfers,
32		webhooks, secrets, mirrors, and the admin promote/enable actions —
33		only within 15 minutes of signing in. An older session gets the form
34		back with a "Sign in again" link, and the login returns to it. SSH and
35		API tokens are unaffected (#297).
	27	repository visibility, webhooks, secrets, mirrors, and the admin
	28	promote/enable and repository-visibility actions — only within 15
	29	minutes of signing in. An
	30	older session gets the form back with a "Sign in again" link, and the
	31	login returns to it. SSH and API tokens are unaffected (#297).
36	32	- The builds page's status badge section gives an org-mode snippet
37	33	beside the Markdown one, for a README.org (#299).
38	34	- API tokens on the settings page: create with a scope and optional
		@@ -95,6 +91,12 @@ entries; if extracting one leaves a repository's =refs/= directory
95	91	missing, =gitbayd admin backup --verify <archive>= names it, and
96	92	=mkdir -p <root>/repos/<owner>/<name>.git/refs= fixes it.
97	93
	94	Upgrade note. =repo import= now refuses a =git://= source and a
	95	=--from= URL carrying a query or fragment. Mirrors and imports whose
	96	host is written numerically (=127.1=, =2130706433=, =0x7f.1=) rather
	97	than as a dotted address are refused too; rewrite the URL before
	98	upgrading (#298).
	99
98	100	- A token with a =--ttl= is refused on every command that creates a
99	101	credential: tokens, keys, deploy keys, runner keys, login links,
100	102	invites, accounts and verified addresses (#257).
		@@ -294,12 +296,21 @@ missing, =gitbayd admin backup --verify <archive>= names it, and
294	296	hour (#285). Operators: tokens minted before the upgrade are
295	297	refused. git-lfs asks =git-lfs-authenticate= for a token each time
296	298	it runs, so only a transfer running across the restart fails, with
297		"repository not found"; running the command again fixes it.
	299	"repository not found"; running the command again fixes it. A
	300	download from a public repository that presents a token issued
	301	before the upgrade is refused the same way, not served anonymously,
	302	and needs the same rerun.
298	303	- Every LFS request also repeats the repository check for the token's
299	304	key: a collaborator whose access is revoked or reduced, or a reader
300	305	of a public repository made private, loses the token's use with the
301	306	access, and an upload token stops working once its repository is
302	307	archived (#285).
	308	- LFS transfer tokens carry a hash of their key's fingerprint beside
	309	its id, and a token whose key id now belongs to another key is
	310	refused, since SQLite gives a new key the id of the last deleted one.
	311	The token format changed: tokens minted before the upgrade are
	312	refused, and a transfer running across the restart needs running
	313	again (#303).
303	314
304	315	* v1.36.0 — 2026-09-23
305	316

internal/control/admin.go +5 −4

		@@ -80,10 +80,11 @@ func init() {
80	80	Examples: []string{"admin repo unarchive alice/old-project"},
81	81	Run: runAdminRepoUnarchive})
82	82	register(Command{Path: []string{"admin", "repo", "visibility"},
83		Summary: "set any repository's visibility (instance admins; audited)",
84		Usage: "admin repo visibility <owner/name> public\|private",
85		Examples: []string{"admin repo visibility alice/secret private"},
86		Run: runAdminRepoVisibility})
	83	NeedsRecentSignIn: true,
	84	Summary: "set any repository's visibility (instance admins; audited)",
	85	Usage: "admin repo visibility <owner/name> public\|private",
	86	Examples: []string{"admin repo visibility alice/secret private"},
	87	Run: runAdminRepoVisibility})
87	88	register(Command{Path: []string{"admin", "repo", "delete"},
88	89	Summary: "delete any repository (instance admins; audited)",
89	90	Usage: "admin repo delete <owner/name> --yes",

internal/control/control_test.go +2 −1

		@@ -180,7 +180,8 @@ func TestStdinCommandsReadStdin(t *testing.T) {
180	180	for _, cmd := range Commands() {
181	181	u := cmd.Usage
182	182	wants := strings.Contains(u, "--file -") \|\| strings.Contains(u, "< ") \|\|
183		strings.Contains(u, "stdin") \|\| strings.Contains(u, "--key -")
	183	strings.Contains(u, "stdin") \|\| strings.Contains(u, "--key -") \|\|
	184	strings.Contains(u, "--secret -")
184	185	if wants && !cmd.ReadsStdin {
185	186	t.Errorf("%s: usage %q reads stdin but ReadsStdin is not set", strings.Join(cmd.Path, " "), u)
186	187	}

internal/control/mirrorcmd.go +9

		@@ -5,9 +5,11 @@ import (
5	5	"errors"
6	6	"fmt"
7	7	"io"
	8	"net/url"
8	9	"strconv"
9	10	"strings"
10	11
	12	"gitbay.org/gitbay/internal/gitpin"
11	13	"gitbay.org/gitbay/internal/policy"
12	14	"gitbay.org/gitbay/internal/protocol"
13	15	"gitbay.org/gitbay/internal/store"
		@@ -54,6 +56,13 @@ func runMirrorAdd(c *Ctx, args []string) int {
54	56	if path == "" \|\| urlArg == "" \|\| (direction != "push" && direction != "pull") {
55	57	return c.usage()
56	58	}
	59	// Sync refuses a numerically written host; say so now, before a
	60	// resolver gets to read it.
	61	if u, err := url.Parse(urlArg); err == nil {
	62	if err := gitpin.CheckHost(u.Hostname()); err != nil {
	63	return c.failInput(err)
	64	}
	65	}
57	66	// The worker's git process dials this URL from the server: same SSRF
58	67	// surface as a webhook target, same rules.
59	68	if err := webhook.ValidateURL(urlArg, c.Cfg.Webhooks.AllowLocal); err != nil {

internal/control/mirrorcmd_test.go added +29

		@@ -0,0 +1,29 @@
	1	package control
	2
	3	import (
	4	"strings"
	5	"testing"
	6
	7	"gitbay.org/gitbay/internal/protocol"
	8	)
	9
	10	// mirror add refuses a host sync would refuse as numeric, on an
	11	// instance that allows local targets or not, and stores nothing (#298).
	12	func TestMirrorAddRefusesNumericHost(t *testing.T) {
	13	for _, allowLocal := range []bool{true, false} {
	14	for _, host := range []string{"127.1", "2130706433", "0x7f.1"} {
	15	c, errOut, st, _ := importCtx(t, allowLocal)
	16	code := Dispatch(c, []string{"repo", "mirror", "add", "alice/app", "https://" + host + "/x.git", "--direction", "pull"})
	17	if code != protocol.ExitUsage \|\| !strings.Contains(errOut.String(), "numeric address") {
	18	t.Fatalf("%s (allow_local %v): exit %d, %q", host, allowLocal, code, errOut.String())
	19	}
	20	repo, err := st.RepoByPath("alice/app")
	21	if err != nil {
	22	t.Fatal(err)
	23	}
	24	if ms, err := st.ListMirrors(repo.ID); err != nil \|\| len(ms) != 0 {
	25	t.Fatalf("%s: mirrors %v, %v", host, ms, err)
	26	}
	27	}
	28	}
	29	}

internal/control/reauth_test.go +2

		@@ -109,6 +109,7 @@ func TestNeedsRecentSignInSet(t *testing.T) {
109	109	want := []string{
110	110	"admin email verify",
111	111	"admin invite",
	112	"admin repo visibility",
112	113	"admin user create",
113	114	"admin user enable",
114	115	"admin user promote",
		@@ -125,6 +126,7 @@ func TestNeedsRecentSignInSet(t *testing.T) {
125	126	"repo mirror add",
126	127	"repo runner add",
127	128	"repo secret set",
	129	"repo settings visibility",
128	130	"repo transfer",
129	131	"token create",
130	132	"web login",

internal/control/repo.go +3 −1

		@@ -118,7 +118,9 @@ func init() {
118	118	Summary: "set repository visibility",
119	119	Usage: "repo settings visibility <owner/name> public\|private",
120	120	Examples: []string{"repo settings visibility krz/gitbay public"},
121		Run: runSetVisibility})
	121	// Making a repository public shows it to everyone.
	122	NeedsRecentSignIn: true,
	123	Run: runSetVisibility})
122	124	register(Command{Path: []string{"repo", "settings", "website"},
123	125	Summary: "set the repository website",
124	126	Usage: "repo settings website <owner/name> <url> ('' clears)",

internal/control/webhook.go +1 −1

		@@ -87,7 +87,7 @@ func runWebhookAdd(c *Ctx, args []string) int {
87	87	if err != nil {
88	88	return c.fail(protocol.ExitFailure, "reading secret: %v", err)
89	89	}
90		secret = strings.TrimRight(string(raw), "\n")
	90	secret = strings.TrimRight(string(raw), "\r\n")
91	91	if secret == "" {
92	92	return c.fail(protocol.ExitUsage, "no secret on stdin (pipe it: printf %%s SECRET \| ... --secret -)")
93	93	}

internal/control/webhook_test.go +7 −3

		@@ -70,11 +70,15 @@ func TestWebhookAddSecretFromStdin(t *testing.T) {
70	70	if msg, code := run("not a secret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/other"); code != protocol.ExitOK {
71	71	t.Fatalf("no secret: exit %d, %q", code, msg)
72	72	}
	73	// A secret from a file with CRLF line endings loses the \r too.
	74	if msg, code := run("crlf\r\n", "webhook", "add", repo.Path(), "http://127.0.0.1/crlf", "--secret", "-"); code != protocol.ExitOK {
	75	t.Fatalf("CRLF secret: exit %d, %q", code, msg)
	76	}
73	77	hooks, err := st.ListWebhooks(repo.ID)
74		if err != nil \|\| len(hooks) != 2 {
	78	if err != nil \|\| len(hooks) != 3 {
75	79	t.Fatalf("hooks: %+v %v", hooks, err)
76	80	}
77		if hooks[0].Secret != "s3cret" \|\| hooks[1].Secret != "" {
78		t.Fatalf("secrets: %q, %q", hooks[0].Secret, hooks[1].Secret)
	81	if hooks[0].Secret != "s3cret" \|\| hooks[1].Secret != "" \|\| hooks[2].Secret != "crlf" {
	82	t.Fatalf("secrets: %q, %q, %q", hooks[0].Secret, hooks[1].Secret, hooks[2].Secret)
79	83	}
80	84	}

internal/gitpin/gitpin.go +13 −4

		@@ -46,10 +46,8 @@ func Resolve(ctx context.Context, lookup Lookup, raw string, allowLocal bool) (R
46	46	if host == "" {
47	47	return Remote{}, fmt.Errorf("URL has no host")
48	48	}
49		if net.ParseIP(host) == nil && numericHost(host) {
50		// 127.1, 2130706433 and 0x7f.1 are loopback to curl's parser
51		// but not to Go's; refuse rather than leave them to a resolver.
52		return Remote{}, fmt.Errorf("host %q is a numeric address in a form other than dotted decimal; write it as a.b.c.d", host)
	49	if err := CheckHost(host); err != nil {
	50	return Remote{}, err
53	51	}
54	52	ips, err := lookup(ctx, host)
55	53	if err != nil {
		@@ -65,6 +63,17 @@ func Resolve(ctx context.Context, lookup Lookup, raw string, allowLocal bool) (R
65	63	return Remote{URL: u, IPs: ips}, nil
66	64	}
67	65
	66	// CheckHost refuses a host written as a number in a form other than
	67	// an IP literal: 127.1, 2130706433 and 0x7f.1 are loopback to curl's
	68	// parser but not to Go's, so they are refused rather than left to a
	69	// resolver.
	70	func CheckHost(host string) error {
	71	if net.ParseIP(host) == nil && numericHost(host) {
	72	return fmt.Errorf("host %q is a numeric address in a form other than dotted decimal; write it as a.b.c.d", host)
	73	}
	74	return nil
	75	}
	76
68	77	// numericHost reports whether every label of host is a decimal, octal
69	78	// or hex number, the shapes inet_aton reads as an IPv4 address.
70	79	func numericHost(host string) bool {

internal/httpd/lfs.go +17 −6

		@@ -41,7 +41,8 @@ func (s *Server) lfsSecret() ([]byte, error) {
41	41	// for none). A token is bound to the SSH key that obtained it and
42	42	// works only while that key is registered, unexpired and on an enabled
43	43	// account, and while the key still has the access its operation needs
44		// on the repo (#285). Without one, public repos allow anonymous
	44	// on the repo (#285). A key that took over a deleted key's id does not
	45	// match the token's fingerprint pin (#303). Without one, public repos allow anonymous
45	46	// download only.
46	47	func (s Server) lfsAuth(r http.Request, repo store.Repo) (string, int64) {
47	48	auth := r.Header.Get("Authorization")
		@@ -62,7 +63,7 @@ func (s Server) lfsAuth(r http.Request, repo store.Repo) (string, int64) {
62	63	return "", 0
63	64	}
64	65	live, err := s.st.LiveSSHKeys([]int64{g.KeyID})
65		if err != nil \|\| !live[g.KeyID] \|\| !s.lfsKeyAllows(g.KeyID, repo, g.Op == "upload") {
	66	if err != nil \|\| !live[g.KeyID] \|\| !s.lfsKeyAllows(g.KeyID, g.KeyPin, repo, g.Op == "upload") {
66	67	return "", 0
67	68	}
68	69	return g.Op, g.KeyID
		@@ -75,13 +76,14 @@ func (s Server) lfsAuth(r http.Request, repo store.Repo) (string, int64) {
75	76
76	77	// lfsKeyAllows repeats git-lfs-authenticate's access check for the key
77	78	// now: a deploy key by its binding, any other key by its account's
78		// access narrowed by the key's scope. An archived repo takes no uploads.
79		func (s *Server) lfsKeyAllows(keyID int64, repo store.Repo, write bool) bool {
	79	// access narrowed by the key's scope. The key must be the one the token
	80	// was minted for, by fingerprint pin. An archived repo takes no uploads.
	81	func (s *Server) lfsKeyAllows(keyID int64, pin string, repo store.Repo, write bool) bool {
80	82	if write && repo.Settings.Archived {
81	83	return false
82	84	}
83	85	key, err := s.st.SSHKeyByID(keyID)
84		if err != nil {
	86	if err != nil \|\| lfs.KeyPin(key.Fingerprint) != pin {
85	87	return false
86	88	}
87	89	if policy.IsDeployScope(key.Scope) {
		@@ -171,7 +173,16 @@ func (s Server) lfsBatch(w http.ResponseWriter, r http.Request) {
171	173	lfsError(w, http.StatusInternalServerError, "lfs secret unavailable")
172	174	return
173	175	}
174		transferToken := lfs.Sign(secret, repo.ID, keyID, req.Operation, time.Now())
	176	fingerprint := ""
	177	if keyID != 0 {
	178	key, err := s.st.SSHKeyByID(keyID)
	179	if err != nil {
	180	lfsError(w, http.StatusNotFound, "repository not found")
	181	return
	182	}
	183	fingerprint = key.Fingerprint
	184	}
	185	transferToken := lfs.Sign(secret, repo.ID, keyID, fingerprint, req.Operation, time.Now())
175	186	base := fmt.Sprintf("%s/%s/%s.git/info/lfs/objects",
176	187	strings.TrimSuffix(s.cfg.Server.SiteURL, "/"), repo.OwnerName, repo.Name)
177	188	authHeader := map[string]string{"Authorization": "Bearer " + transferToken}

internal/httpd/lfsauth_test.go +44 −16

		@@ -54,14 +54,14 @@ func TestLFSTokenNeedsALiveKey(t *testing.T) {
54	54	}
55	55
56	56	live := addKey("SHA256:live", nil)
57		tok := lfs.Sign(secret, repo.ID, live, "upload", time.Now())
	57	tok := lfs.Sign(secret, repo.ID, live, "SHA256:live", "upload", time.Now())
58	58	if op, key := s.lfsAuth(lfsRequest(tok), repo); op != "upload" \|\| key != live {
59	59	t.Fatalf("live key: %q, %d", op, key)
60	60	}
61	61
62	62	past := time.Now().Add(-time.Minute)
63	63	expired := addKey("SHA256:expired", &past)
64		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, expired, "upload", time.Now())), repo); op != "" {
	64	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, expired, "SHA256:expired", "upload", time.Now())), repo); op != "" {
65	65	t.Errorf("expired key: %q", op)
66	66	}
67	67
		@@ -73,7 +73,7 @@ func TestLFSTokenNeedsALiveKey(t *testing.T) {
73	73	}
74	74
75	75	other := addKey("SHA256:other", nil)
76		otherTok := lfs.Sign(secret, repo.ID, other, "download", time.Now())
	76	otherTok := lfs.Sign(secret, repo.ID, other, "SHA256:other", "download", time.Now())
77	77	if op, _ := s.lfsAuth(lfsRequest(otherTok), repo); op != "download" {
78	78	t.Fatalf("second key before disable: %q", op)
79	79	}
		@@ -97,13 +97,13 @@ func TestLFSAnonymousTokenOnlyDownloadsPublic(t *testing.T) {
97	97	t.Fatal(err)
98	98	}
99	99	now := time.Now()
100		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "download", now)), pub); op != "download" {
	100	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "", "download", now)), pub); op != "download" {
101	101	t.Errorf("public download: %q", op)
102	102	}
103		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "upload", now)), pub); op != "" {
	103	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "", "upload", now)), pub); op != "" {
104	104	t.Errorf("anonymous upload: %q", op)
105	105	}
106		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, priv.ID, 0, "download", now)), priv); op != "" {
	106	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, priv.ID, 0, "", "download", now)), priv); op != "" {
107	107	t.Errorf("private download: %q", op)
108	108	}
109	109	}
		@@ -140,7 +140,7 @@ func TestLFSTokenNeedsCurrentAccess(t *testing.T) {
140	140	t.Fatal(err)
141	141	}
142	142	bobKey := lfsTestKey(t, st, bob, "SHA256:bob", "full")
143		up := lfs.Sign(secret, repo.ID, bobKey, "upload", now)
	143	up := lfs.Sign(secret, repo.ID, bobKey, "SHA256:bob", "upload", now)
144	144	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "upload" {
145	145	t.Fatalf("collaborator upload: %q", op)
146	146	}
		@@ -153,7 +153,7 @@ func TestLFSTokenNeedsCurrentAccess(t *testing.T) {
153	153	if err := st.RevokeAccess(repo.ID, bob); err != nil {
154	154	t.Fatal(err)
155	155	}
156		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, bobKey, "download", now)), repo); op != "" {
	156	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, bobKey, "SHA256:bob", "download", now)), repo); op != "" {
157	157	t.Errorf("download after access was revoked: %q", op)
158	158	}
159	159
		@@ -163,7 +163,7 @@ func TestLFSTokenNeedsCurrentAccess(t *testing.T) {
163	163	t.Fatal(err)
164	164	}
165	165	carolKey := lfsTestKey(t, st, carol, "SHA256:carol", "full")
166		down := lfs.Sign(secret, pub.ID, carolKey, "download", now)
	166	down := lfs.Sign(secret, pub.ID, carolKey, "SHA256:carol", "download", now)
167	167	if op, _ := s.lfsAuth(lfsRequest(down), pub); op != "download" {
168	168	t.Fatalf("public download: %q", op)
169	169	}
		@@ -194,12 +194,12 @@ func TestLFSDeployKeyToken(t *testing.T) {
194	194	ro := fmt.Sprintf("deploy:%d:ro", repo.ID)
195	195
196	196	live := lfsTestKey(t, st, u.ID, "SHA256:deploy-live", rw)
197		if op, key := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "upload", now)), repo); op != "upload" \|\| key != live {
	197	if op, key := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "SHA256:deploy-live", "upload", now)), repo); op != "upload" \|\| key != live {
198	198	t.Fatalf("live deploy key: %q, %d", op, key)
199	199	}
200	200
201	201	removed := lfsTestKey(t, st, u.ID, "SHA256:deploy-removed", rw)
202		tok := lfs.Sign(secret, repo.ID, removed, "download", now)
	202	tok := lfs.Sign(secret, repo.ID, removed, "SHA256:deploy-removed", "download", now)
203	203	if err := st.RemoveDeployKey(repo.ID, "SHA256:deploy-removed"); err != nil {
204	204	t.Fatal(err)
205	205	}
		@@ -208,17 +208,17 @@ func TestLFSDeployKeyToken(t *testing.T) {
208	208	}
209	209
210	210	readOnly := lfsTestKey(t, st, u.ID, "SHA256:deploy-ro", ro)
211		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "download", now)), repo); op != "download" {
	211	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "SHA256:deploy-ro", "download", now)), repo); op != "download" {
212	212	t.Errorf("read-only deploy key download: %q", op)
213	213	}
214		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "upload", now)), repo); op != "" {
	214	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "SHA256:deploy-ro", "upload", now)), repo); op != "" {
215	215	t.Errorf("read-only deploy key upload: %q", op)
216	216	}
217	217
218	218	if err := st.SetUserDisabled(u.ID, true); err != nil {
219	219	t.Fatal(err)
220	220	}
221		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "download", now)), repo); op != "" {
	221	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "SHA256:deploy-live", "download", now)), repo); op != "" {
222	222	t.Errorf("deploy key of a disabled account: %q", op)
223	223	}
224	224	}
		@@ -234,7 +234,7 @@ func TestLFSUploadTokenRefusedOnceArchived(t *testing.T) {
234	234	}
235	235	key := lfsTestKey(t, st, u.ID, "SHA256:owner", "full")
236	236	now := time.Now()
237		up := lfs.Sign(secret, repo.ID, key, "upload", now)
	237	up := lfs.Sign(secret, repo.ID, key, "SHA256:owner", "upload", now)
238	238	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "upload" {
239	239	t.Fatalf("upload before archiving: %q", op)
240	240	}
		@@ -248,7 +248,35 @@ func TestLFSUploadTokenRefusedOnceArchived(t *testing.T) {
248	248	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "" {
249	249	t.Errorf("upload after archiving: %q", op)
250	250	}
251		if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, key, "download", now)), repo); op != "download" {
	251	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, key, "SHA256:owner", "download", now)), repo); op != "download" {
252	252	t.Errorf("download after archiving: %q", op)
253	253	}
254	254	}
	255
	256	// SQLite gives a new key the id of the highest deleted one. A token
	257	// minted for the deleted key is refused when presented against the new
	258	// key; the new key's own token works (#303).
	259	func TestLFSTokenRefusedOnReusedKeyID(t *testing.T) {
	260	s, st, u := newTokenTestServer(t)
	261	repo := lfsTestRepo(t, st, u.ID, "app", "private")
	262	secret, err := s.lfsSecret()
	263	if err != nil {
	264	t.Fatal(err)
	265	}
	266	now := time.Now()
	267	oldID := lfsTestKey(t, st, u.ID, "SHA256:old", "full")
	268	old := lfs.Sign(secret, repo.ID, oldID, "SHA256:old", "upload", now)
	269	if err := st.RemoveSSHKey(u.ID, "SHA256:old"); err != nil {
	270	t.Fatal(err)
	271	}
	272	newID := lfsTestKey(t, st, u.ID, "SHA256:new", "full")
	273	if newID != oldID {
	274	t.Fatalf("new key got id %d, want the reused %d", newID, oldID)
	275	}
	276	if op, _ := s.lfsAuth(lfsRequest(old), repo); op != "" {
	277	t.Errorf("old key's token on the reused id: %q", op)
	278	}
	279	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, newID, "SHA256:new", "upload", now)), repo); op != "upload" {
	280	t.Errorf("new key's own token: %q", op)
	281	}
	282	}

internal/lfs/lfs.go +27 −9

		@@ -129,24 +129,39 @@ const TokenTTL = time.Hour
129	129
130	130	// Sign mints a token for op ("download" or "upload") on repoID, bound
131	131	// to keyID: the SSH key, user or deploy, that asked for it, or 0 for an
132		// anonymous download of a public repository.
133		func Sign(secret []byte, repoID, keyID int64, op string, now time.Time) string {
134		payload := fmt.Sprintf("%d:%d:%s:%d", repoID, keyID, op, now.Add(TokenTTL).Unix())
	132	// anonymous download of a public repository. fingerprint is that key's
	133	// fingerprint, "" for key 0. SQLite reuses the id of a deleted key, so
	134	// the token carries a hash of the fingerprint as well and a new key
	135	// given the old id does not inherit the old key's tokens (#303).
	136	func Sign(secret []byte, repoID, keyID int64, fingerprint, op string, now time.Time) string {
	137	payload := fmt.Sprintf("%d:%d:%s:%s:%d", repoID, keyID, KeyPin(fingerprint), op, now.Add(TokenTTL).Unix())
135	138	mac := hmac.New(sha256.New, secret)
136	139	mac.Write([]byte(payload))
137	140	return base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
138	141	base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
139	142	}
140	143
	144	// KeyPin is the fingerprint's form in a token: the first 16 hex
	145	// characters of its SHA-256, or "" for no key.
	146	func KeyPin(fingerprint string) string {
	147	if fingerprint == "" {
	148	return ""
	149	}
	150	sum := sha256.Sum256([]byte(fingerprint))
	151	return hex.EncodeToString(sum[:8])
	152	}
	153
141	154	// Grant is what a verified token authorizes.
142	155	type Grant struct {
143	156	RepoID int64
144		KeyID int64 // 0: an anonymous download of a public repository
	157	KeyID int64 // 0: an anonymous download of a public repository
	158	KeyPin string // KeyPin of the key's fingerprint; "" when KeyID is 0
145	159	Op string
146	160	}
147	161
148	162	// Verify checks a token's MAC, shape and expiry. A token from before
149		// tokens named their key does not verify.
	163	// tokens named their key, or before they carried its fingerprint, does
	164	// not verify.
150	165	func Verify(secret []byte, token string, now time.Time) (Grant, bool) {
151	166	payloadB64, macB64, found := strings.Cut(token, ".")
152	167	if !found {
		@@ -166,19 +181,22 @@ func Verify(secret []byte, token string, now time.Time) (Grant, bool) {
166	181	return Grant{}, false
167	182	}
168	183	parts := strings.Split(string(payload), ":")
169		if len(parts) != 4 {
	184	if len(parts) != 5 {
170	185	return Grant{}, false
171	186	}
172	187	repoID, err1 := strconv.ParseInt(parts[0], 10, 64)
173	188	keyID, err2 := strconv.ParseInt(parts[1], 10, 64)
174		exp, err3 := strconv.ParseInt(parts[3], 10, 64)
	189	exp, err3 := strconv.ParseInt(parts[4], 10, 64)
175	190	if err1 != nil \|\| err2 != nil \|\| err3 != nil \|\| keyID < 0 \|\| now.Unix() > exp {
176	191	return Grant{}, false
177	192	}
178		if parts[2] != "download" && parts[2] != "upload" {
	193	if (keyID == 0) != (parts[2] == "") {
	194	return Grant{}, false
	195	}
	196	if parts[3] != "download" && parts[3] != "upload" {
179	197	return Grant{}, false
180	198	}
181		return Grant{RepoID: repoID, KeyID: keyID, Op: parts[2]}, true
	199	return Grant{RepoID: repoID, KeyID: keyID, KeyPin: parts[2], Op: parts[3]}, true
182	200	}
183	201
184	202	// NewSecret returns 32 random bytes, hex-encoded for the settings table.

internal/lfs/lfs_test.go +17 −3

		@@ -12,9 +12,9 @@ import (
12	12	func TestTokenCarriesTheKey(t *testing.T) {
13	13	secret := []byte("secret")
14	14	now := time.Now()
15		tok := Sign(secret, 7, 42, "upload", now)
	15	tok := Sign(secret, 7, 42, "SHA256:k", "upload", now)
16	16	g, ok := Verify(secret, tok, now)
17		if !ok \|\| g != (Grant{RepoID: 7, KeyID: 42, Op: "upload"}) {
	17	if !ok \|\| g != (Grant{RepoID: 7, KeyID: 42, KeyPin: KeyPin("SHA256:k"), Op: "upload"}) {
18	18	t.Fatalf("Verify = %+v, %v", g, ok)
19	19	}
20	20	if _, ok := Verify(secret, tok, now.Add(TokenTTL+time.Second)); ok {
		@@ -23,7 +23,7 @@ func TestTokenCarriesTheKey(t *testing.T) {
23	23	if _, ok := Verify([]byte("other"), tok, now); ok {
24	24	t.Error("a token verified under another secret")
25	25	}
26		if g, ok := Verify(secret, Sign(secret, 7, 0, "download", now), now); !ok \|\| g.KeyID != 0 {
	26	if g, ok := Verify(secret, Sign(secret, 7, 0, "", "download", now), now); !ok \|\| g.KeyID != 0 \|\| g.KeyPin != "" {
27	27	t.Errorf("anonymous grant = %+v, %v", g, ok)
28	28	}
29	29	}
		@@ -41,3 +41,17 @@ func TestUnboundTokenRefused(t *testing.T) {
41	41	t.Fatalf("a pre-upgrade token verified: %+v", g)
42	42	}
43	43	}
	44
	45	// A token minted before tokens carried the key's fingerprint has four
	46	// fields. It is refused (#303).
	47	func TestUnpinnedTokenRefused(t *testing.T) {
	48	secret := []byte("secret")
	49	payload := fmt.Sprintf("%d:%d:%s:%d", 7, 42, "upload", time.Now().Add(TokenTTL).Unix())
	50	mac := hmac.New(sha256.New, secret)
	51	mac.Write([]byte(payload))
	52	tok := base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
	53	base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
	54	if g, ok := Verify(secret, tok, time.Now()); ok {
	55	t.Fatalf("an unpinned token verified: %+v", g)
	56	}
	57	}

internal/sshd/lfs.go +1 −1

		@@ -69,7 +69,7 @@ func runLFSAuthenticate(cfg config.Config, st *store.Store, user store.User, key
69	69	fmt.Fprintln(stderr, "internal error")
70	70	return protocol.ExitFailure
71	71	}
72		token := lfs.Sign([]byte(secret), repo.ID, key.ID, op, time.Now())
	72	token := lfs.Sign([]byte(secret), repo.ID, key.ID, key.Fingerprint, op, time.Now())
73	73	json.NewEncoder(stdout).Encode(map[string]any{
74	74	"href": fmt.Sprintf("%s/%s/%s.git/info/lfs",
75	75	cfg.Server.SiteURL, repo.OwnerName, repo.Name),

Follow-ups review fixes: LFS key pin, mirror numeric hosts, visibility re-auth, docs !514