!514 · krz/gitbay

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

 is a pause, not an outage.
 #+begin_src sh
+# on a machine with an admin gitbay identity (root on the host has none),
+# with the runner's public key copied from the host:
 gitbay repo create cmc/runner-scratch   # then push a .gitbay/ci.yml naming the image
+scp -P 2222 root@<host>:/var/lib/gitbay-runner/.ssh/id_ed25519.pub runner.pub
+gitbay repo runner add cmc/runner-scratch < runner.pub
 # on the host:
-gitbay repo runner add cmc/runner-scratch < /var/lib/gitbay-runner/.ssh/id_ed25519.pub
 sed -i 's#^ExecStart=/usr/local/bin/gitbay-runner #&-repos cmc/runner-scratch #' /etc/systemd/system/gitbay-runner.service.d/override.conf
 systemctl daemon-reload && systemctl restart gitbay-runner
+# back on the admin machine:
 gitbay build log cmc/runner-scratch 1   # green: remove -repos, redeploy, delete the scratch repository
 #+end_src

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

 - Commands that create a credential (SSH, deploy and runner keys, API
   tokens, email verification, login links, PGP keys, device tokens) or
   grant access (repository and organization roles, teams, transfers,
-  admin promote and enable, webhooks, secrets, mirrors) are refused
+  repository visibility, admin promote and enable, webhooks, secrets,
+  mirrors) are refused
   from a browser session that signed in more than 15 minutes ago
   (=control.ReauthWindow=, =Command.NeedsRecentSignIn=). The sign-in
   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	\| Control \| Status \| Evidence \|	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	\| Webhook payload integrity \| in place \| HMAC-SHA256 header \|	79	\| Webhook payload integrity \| in place \| HMAC-SHA256 header \|
80	\| SMTP credentials protected in transit \| in place \| STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) \|	80	\| SMTP credentials protected in transit \| in place \| STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) \|
81	\| Upload size limits \| in place \| per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB \|	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	\| #259 \| Recovery \| No restore has been exercised; the drill is written (Admin wiki) and not yet run \| high \|	13	\| #259 \| Recovery \| No restore has been exercised; the drill is written (Admin wiki) and not yet run \| high \|
14	\| #260 \| CI network \| Builds share the runner's source address; no egress policy \| medium \|	14	\| #260 \| CI network \| Builds share the runner's source address; no egress policy \| medium \|
15	\| #261 \| Various \| Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift \| medium \|	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	* Not filed	18	* Not filed
19		19

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

 immediately before running git and pin it to the checked addresses
 (=internal/gitpin=), so a DNS answer that changes after validation
 still cannot reach private space. Redirects are never followed.
-=repo import= refuses =git://=, which cannot be pinned, and a host
+=repo import= refuses =git://=, which cannot be pinned. Mirror sync
-written as a bare number or in hex/octal (=0x7f.1=, =2130706433=,
+and =repo import= refuse a host written as a bare number or in
-=127.1=) rather than dotted decimal, since that form resolves
+hex/octal (=0x7f.1=, =2130706433=, =127.1=) rather than dotted
-differently across parsers. GitHub-history import (=repo
+decimal, since that form resolves differently across parsers; =repo
+mirror add= refuses it when the mirror is saved. GitHub-history import (=repo
 import-issues --api-base=) is not yet pinned (#301).
 * 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	handled.	702	handled.
703		703
704	Actions that create a credential or grant access (adding a key, token	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	when your web sign-in is older than 15 minutes. The form shows a "Sign	707	when your web sign-in is older than 15 minutes. The form shows a "Sign
707	in again" link and the login returns to the page. Idle renewal does not	708	in again" link and the login returns to the page. Idle renewal does not
708	extend this window.	709	extend this window.

CHANGELOG.org +27 −16

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

internal/control/admin.go +5 −4

 		Examples: []string{"admin repo unarchive alice/old-project"},
 		Run:      runAdminRepoUnarchive})
 	register(Command{Path: []string{"admin", "repo", "visibility"},
-		Summary:  "set any repository's visibility (instance admins; audited)",
+		NeedsRecentSignIn: true,
-		Usage:    "admin repo visibility <owner/name> public|private",
+		Summary:           "set any repository's visibility (instance admins; audited)",
-		Examples: []string{"admin repo visibility alice/secret private"},
+		Usage:             "admin repo visibility <owner/name> public|private",
-		Run:      runAdminRepoVisibility})
+		Examples:          []string{"admin repo visibility alice/secret private"},
+		Run:               runAdminRepoVisibility})
 	register(Command{Path: []string{"admin", "repo", "delete"},
 		Summary: "delete any repository (instance admins; audited)",
 		Usage:   "admin repo delete <owner/name> --yes",

internal/control/control_test.go +2 −1

 	for _, cmd := range Commands() {
 		u := cmd.Usage
 		wants := strings.Contains(u, "--file -") || strings.Contains(u, "< ") ||
-			strings.Contains(u, "stdin") || strings.Contains(u, "--key -")
+			strings.Contains(u, "stdin") || strings.Contains(u, "--key -") ||
+			strings.Contains(u, "--secret -")
 		if wants && !cmd.ReadsStdin {
 			t.Errorf("%s: usage %q reads stdin but ReadsStdin is not set", strings.Join(cmd.Path, " "), u)
 		}

internal/control/mirrorcmd.go +9

 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"strconv"
 	"strings"
+	"gitbay.org/gitbay/internal/gitpin"
 	"gitbay.org/gitbay/internal/policy"
 	"gitbay.org/gitbay/internal/protocol"
 	"gitbay.org/gitbay/internal/store"
 	if path == "" || urlArg == "" || (direction != "push" && direction != "pull") {
 		return c.usage()
 	}
+	// Sync refuses a numerically written host; say so now, before a
+	// resolver gets to read it.
+	if u, err := url.Parse(urlArg); err == nil {
+		if err := gitpin.CheckHost(u.Hostname()); err != nil {
+			return c.failInput(err)
+		}
+	}
 	// The worker's git process dials this URL from the server: same SSRF
 	// surface as a webhook target, same rules.
 	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

 	want := []string{
 		"admin email verify",
 		"admin invite",
+		"admin repo visibility",
 		"admin user create",
 		"admin user enable",
 		"admin user promote",
 		"repo mirror add",
 		"repo runner add",
 		"repo secret set",
+		"repo settings visibility",
 		"repo transfer",
 		"token create",
 		"web login",

internal/control/repo.go +3 −1

 		Summary:  "set repository visibility",
 		Usage:    "repo settings visibility <owner/name> public|private",
 		Examples: []string{"repo settings visibility krz/gitbay public"},
-		Run:      runSetVisibility})
+		// Making a repository public shows it to everyone.
+		NeedsRecentSignIn: true,
+		Run:               runSetVisibility})
 	register(Command{Path: []string{"repo", "settings", "website"},
 		Summary:  "set the repository website",
 		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	if err != nil {	87	if err != nil {
88	return c.fail(protocol.ExitFailure, "reading secret: %v", err)	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	if secret == "" {	91	if secret == "" {
92	return c.fail(protocol.ExitUsage, "no secret on stdin (pipe it: printf %%s SECRET \| ... --secret -)")	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

 	if msg, code := run("not a secret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/other"); code != protocol.ExitOK {
 		t.Fatalf("no secret: exit %d, %q", code, msg)
 	}
+	// A secret from a file with CRLF line endings loses the \r too.
+	if msg, code := run("crlf\r\n", "webhook", "add", repo.Path(), "http://127.0.0.1/crlf", "--secret", "-"); code != protocol.ExitOK {
+		t.Fatalf("CRLF secret: exit %d, %q", code, msg)
+	}
 	hooks, err := st.ListWebhooks(repo.ID)
-	if err != nil || len(hooks) != 2 {
+	if err != nil || len(hooks) != 3 {
 		t.Fatalf("hooks: %+v %v", hooks, err)
 	}
-	if hooks[0].Secret != "s3cret" || hooks[1].Secret != "" {
+	if hooks[0].Secret != "s3cret" || hooks[1].Secret != "" || hooks[2].Secret != "crlf" {
-		t.Fatalf("secrets: %q, %q", hooks[0].Secret, hooks[1].Secret)
+		t.Fatalf("secrets: %q, %q, %q", hooks[0].Secret, hooks[1].Secret, hooks[2].Secret)
 	}
 }

internal/gitpin/gitpin.go +13 −4

 	if host == "" {
 		return Remote{}, fmt.Errorf("URL has no host")
 	}
-	if net.ParseIP(host) == nil && numericHost(host) {
+	if err := CheckHost(host); err != nil {
-		// 127.1, 2130706433 and 0x7f.1 are loopback to curl's parser
+		return Remote{}, err
-		// but not to Go's; refuse rather than leave them to a resolver.
-		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)
 	}
 	ips, err := lookup(ctx, host)
 	if err != nil {
 	return Remote{URL: u, IPs: ips}, nil
 }
+// CheckHost refuses a host written as a number in a form other than
+// an IP literal: 127.1, 2130706433 and 0x7f.1 are loopback to curl's
+// parser but not to Go's, so they are refused rather than left to a
+// resolver.
+func CheckHost(host string) error {
+	if net.ParseIP(host) == nil && numericHost(host) {
+		return fmt.Errorf("host %q is a numeric address in a form other than dotted decimal; write it as a.b.c.d", host)
+	}
+	return nil
+}
 // numericHost reports whether every label of host is a decimal, octal
 // or hex number, the shapes inet_aton reads as an IPv4 address.
 func numericHost(host string) bool {

internal/httpd/lfs.go +17 −6

 // for none). A token is bound to the SSH key that obtained it and
 // works only while that key is registered, unexpired and on an enabled
 // account, and while the key still has the access its operation needs
-// on the repo (#285). Without one, public repos allow anonymous
+// on the repo (#285). A key that took over a deleted key's id does not
+// match the token's fingerprint pin (#303). Without one, public repos allow anonymous
 // download only.
 func (s *Server) lfsAuth(r *http.Request, repo store.Repo) (string, int64) {
 	auth := r.Header.Get("Authorization")
 			return "", 0
 		}
 		live, err := s.st.LiveSSHKeys([]int64{g.KeyID})
-		if err != nil || !live[g.KeyID] || !s.lfsKeyAllows(g.KeyID, repo, g.Op == "upload") {
+		if err != nil || !live[g.KeyID] || !s.lfsKeyAllows(g.KeyID, g.KeyPin, repo, g.Op == "upload") {
 			return "", 0
 		}
 		return g.Op, g.KeyID
 // lfsKeyAllows repeats git-lfs-authenticate's access check for the key
 // now: a deploy key by its binding, any other key by its account's
-// access narrowed by the key's scope. An archived repo takes no uploads.
+// access narrowed by the key's scope. The key must be the one the token
-func (s *Server) lfsKeyAllows(keyID int64, repo store.Repo, write bool) bool {
+// was minted for, by fingerprint pin. An archived repo takes no uploads.
+func (s *Server) lfsKeyAllows(keyID int64, pin string, repo store.Repo, write bool) bool {
 	if write && repo.Settings.Archived {
 		return false
 	}
 	key, err := s.st.SSHKeyByID(keyID)
-	if err != nil {
+	if err != nil || lfs.KeyPin(key.Fingerprint) != pin {
 		return false
 	}
 	if policy.IsDeployScope(key.Scope) {
 		lfsError(w, http.StatusInternalServerError, "lfs secret unavailable")
 		return
 	}
-	transferToken := lfs.Sign(secret, repo.ID, keyID, req.Operation, time.Now())
+	fingerprint := ""
+	if keyID != 0 {
+		key, err := s.st.SSHKeyByID(keyID)
+		if err != nil {
+			lfsError(w, http.StatusNotFound, "repository not found")
+			return
+		}
+		fingerprint = key.Fingerprint
+	}
+	transferToken := lfs.Sign(secret, repo.ID, keyID, fingerprint, req.Operation, time.Now())
 	base := fmt.Sprintf("%s/%s/%s.git/info/lfs/objects",
 		strings.TrimSuffix(s.cfg.Server.SiteURL, "/"), repo.OwnerName, repo.Name)
 	authHeader := map[string]string{"Authorization": "Bearer " + transferToken}

internal/httpd/lfsauth_test.go +44 −16

 	}
 	live := addKey("SHA256:live", nil)
-	tok := lfs.Sign(secret, repo.ID, live, "upload", time.Now())
+	tok := lfs.Sign(secret, repo.ID, live, "SHA256:live", "upload", time.Now())
 	if op, key := s.lfsAuth(lfsRequest(tok), repo); op != "upload" || key != live {
 		t.Fatalf("live key: %q, %d", op, key)
 	}
 	past := time.Now().Add(-time.Minute)
 	expired := addKey("SHA256:expired", &past)
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, expired, "upload", time.Now())), repo); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, expired, "SHA256:expired", "upload", time.Now())), repo); op != "" {
 		t.Errorf("expired key: %q", op)
 	}
 	}
 	other := addKey("SHA256:other", nil)
-	otherTok := lfs.Sign(secret, repo.ID, other, "download", time.Now())
+	otherTok := lfs.Sign(secret, repo.ID, other, "SHA256:other", "download", time.Now())
 	if op, _ := s.lfsAuth(lfsRequest(otherTok), repo); op != "download" {
 		t.Fatalf("second key before disable: %q", op)
 	}
 		t.Fatal(err)
 	}
 	now := time.Now()
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "download", now)), pub); op != "download" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "", "download", now)), pub); op != "download" {
 		t.Errorf("public download: %q", op)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "upload", now)), pub); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, pub.ID, 0, "", "upload", now)), pub); op != "" {
 		t.Errorf("anonymous upload: %q", op)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, priv.ID, 0, "download", now)), priv); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, priv.ID, 0, "", "download", now)), priv); op != "" {
 		t.Errorf("private download: %q", op)
 	}
 }
 		t.Fatal(err)
 	}
 	bobKey := lfsTestKey(t, st, bob, "SHA256:bob", "full")
-	up := lfs.Sign(secret, repo.ID, bobKey, "upload", now)
+	up := lfs.Sign(secret, repo.ID, bobKey, "SHA256:bob", "upload", now)
 	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "upload" {
 		t.Fatalf("collaborator upload: %q", op)
 	}
 	if err := st.RevokeAccess(repo.ID, bob); err != nil {
 		t.Fatal(err)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, bobKey, "download", now)), repo); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, bobKey, "SHA256:bob", "download", now)), repo); op != "" {
 		t.Errorf("download after access was revoked: %q", op)
 	}
 		t.Fatal(err)
 	}
 	carolKey := lfsTestKey(t, st, carol, "SHA256:carol", "full")
-	down := lfs.Sign(secret, pub.ID, carolKey, "download", now)
+	down := lfs.Sign(secret, pub.ID, carolKey, "SHA256:carol", "download", now)
 	if op, _ := s.lfsAuth(lfsRequest(down), pub); op != "download" {
 		t.Fatalf("public download: %q", op)
 	}
 	ro := fmt.Sprintf("deploy:%d:ro", repo.ID)
 	live := lfsTestKey(t, st, u.ID, "SHA256:deploy-live", rw)
-	if op, key := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "upload", now)), repo); op != "upload" || key != live {
+	if op, key := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "SHA256:deploy-live", "upload", now)), repo); op != "upload" || key != live {
 		t.Fatalf("live deploy key: %q, %d", op, key)
 	}
 	removed := lfsTestKey(t, st, u.ID, "SHA256:deploy-removed", rw)
-	tok := lfs.Sign(secret, repo.ID, removed, "download", now)
+	tok := lfs.Sign(secret, repo.ID, removed, "SHA256:deploy-removed", "download", now)
 	if err := st.RemoveDeployKey(repo.ID, "SHA256:deploy-removed"); err != nil {
 		t.Fatal(err)
 	}
 	}
 	readOnly := lfsTestKey(t, st, u.ID, "SHA256:deploy-ro", ro)
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "download", now)), repo); op != "download" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "SHA256:deploy-ro", "download", now)), repo); op != "download" {
 		t.Errorf("read-only deploy key download: %q", op)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "upload", now)), repo); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, readOnly, "SHA256:deploy-ro", "upload", now)), repo); op != "" {
 		t.Errorf("read-only deploy key upload: %q", op)
 	}
 	if err := st.SetUserDisabled(u.ID, true); err != nil {
 		t.Fatal(err)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "download", now)), repo); op != "" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, live, "SHA256:deploy-live", "download", now)), repo); op != "" {
 		t.Errorf("deploy key of a disabled account: %q", op)
 	}
 }
 	}
 	key := lfsTestKey(t, st, u.ID, "SHA256:owner", "full")
 	now := time.Now()
-	up := lfs.Sign(secret, repo.ID, key, "upload", now)
+	up := lfs.Sign(secret, repo.ID, key, "SHA256:owner", "upload", now)
 	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "upload" {
 		t.Fatalf("upload before archiving: %q", op)
 	}
 	if op, _ := s.lfsAuth(lfsRequest(up), repo); op != "" {
 		t.Errorf("upload after archiving: %q", op)
 	}
-	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, key, "download", now)), repo); op != "download" {
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, key, "SHA256:owner", "download", now)), repo); op != "download" {
 		t.Errorf("download after archiving: %q", op)
 	}
 }
+// SQLite gives a new key the id of the highest deleted one. A token
+// minted for the deleted key is refused when presented against the new
+// key; the new key's own token works (#303).
+func TestLFSTokenRefusedOnReusedKeyID(t *testing.T) {
+	s, st, u := newTokenTestServer(t)
+	repo := lfsTestRepo(t, st, u.ID, "app", "private")
+	secret, err := s.lfsSecret()
+	if err != nil {
+		t.Fatal(err)
+	}
+	now := time.Now()
+	oldID := lfsTestKey(t, st, u.ID, "SHA256:old", "full")
+	old := lfs.Sign(secret, repo.ID, oldID, "SHA256:old", "upload", now)
+	if err := st.RemoveSSHKey(u.ID, "SHA256:old"); err != nil {
+		t.Fatal(err)
+	}
+	newID := lfsTestKey(t, st, u.ID, "SHA256:new", "full")
+	if newID != oldID {
+		t.Fatalf("new key got id %d, want the reused %d", newID, oldID)
+	}
+	if op, _ := s.lfsAuth(lfsRequest(old), repo); op != "" {
+		t.Errorf("old key's token on the reused id: %q", op)
+	}
+	if op, _ := s.lfsAuth(lfsRequest(lfs.Sign(secret, repo.ID, newID, "SHA256:new", "upload", now)), repo); op != "upload" {
+		t.Errorf("new key's own token: %q", op)
+	}
+}

internal/lfs/lfs.go +27 −9

 // Sign mints a token for op ("download" or "upload") on repoID, bound
 // to keyID: the SSH key, user or deploy, that asked for it, or 0 for an
-// anonymous download of a public repository.
+// anonymous download of a public repository. fingerprint is that key's
-func Sign(secret []byte, repoID, keyID int64, op string, now time.Time) string {
+// fingerprint, "" for key 0. SQLite reuses the id of a deleted key, so
-	payload := fmt.Sprintf("%d:%d:%s:%d", repoID, keyID, op, now.Add(TokenTTL).Unix())
+// the token carries a hash of the fingerprint as well and a new key
+// given the old id does not inherit the old key's tokens (#303).
+func Sign(secret []byte, repoID, keyID int64, fingerprint, op string, now time.Time) string {
+	payload := fmt.Sprintf("%d:%d:%s:%s:%d", repoID, keyID, KeyPin(fingerprint), op, now.Add(TokenTTL).Unix())
 	mac := hmac.New(sha256.New, secret)
 	mac.Write([]byte(payload))
 	return base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
 		base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
 }
+// KeyPin is the fingerprint's form in a token: the first 16 hex
+// characters of its SHA-256, or "" for no key.
+func KeyPin(fingerprint string) string {
+	if fingerprint == "" {
+		return ""
+	}
+	sum := sha256.Sum256([]byte(fingerprint))
+	return hex.EncodeToString(sum[:8])
+}
 // Grant is what a verified token authorizes.
 type Grant struct {
 	RepoID int64
-	KeyID  int64 // 0: an anonymous download of a public repository
+	KeyID  int64  // 0: an anonymous download of a public repository
+	KeyPin string // KeyPin of the key's fingerprint; "" when KeyID is 0
 	Op     string
 }
 // Verify checks a token's MAC, shape and expiry. A token from before
-// tokens named their key does not verify.
+// tokens named their key, or before they carried its fingerprint, does
+// not verify.
 func Verify(secret []byte, token string, now time.Time) (Grant, bool) {
 	payloadB64, macB64, found := strings.Cut(token, ".")
 	if !found {
 		return Grant{}, false
 	}
 	parts := strings.Split(string(payload), ":")
-	if len(parts) != 4 {
+	if len(parts) != 5 {
 		return Grant{}, false
 	}
 	repoID, err1 := strconv.ParseInt(parts[0], 10, 64)
 	keyID, err2 := strconv.ParseInt(parts[1], 10, 64)
-	exp, err3 := strconv.ParseInt(parts[3], 10, 64)
+	exp, err3 := strconv.ParseInt(parts[4], 10, 64)
 	if err1 != nil || err2 != nil || err3 != nil || keyID < 0 || now.Unix() > exp {
 		return Grant{}, false
 	}
-	if parts[2] != "download" && parts[2] != "upload" {
+	if (keyID == 0) != (parts[2] == "") {
+		return Grant{}, false
+	}
+	if parts[3] != "download" && parts[3] != "upload" {
 		return Grant{}, false
 	}
-	return Grant{RepoID: repoID, KeyID: keyID, Op: parts[2]}, true
+	return Grant{RepoID: repoID, KeyID: keyID, KeyPin: parts[2], Op: parts[3]}, true
 }
 // NewSecret returns 32 random bytes, hex-encoded for the settings table.

internal/lfs/lfs_test.go +17 −3

 func TestTokenCarriesTheKey(t *testing.T) {
 	secret := []byte("secret")
 	now := time.Now()
-	tok := Sign(secret, 7, 42, "upload", now)
+	tok := Sign(secret, 7, 42, "SHA256:k", "upload", now)
 	g, ok := Verify(secret, tok, now)
-	if !ok || g != (Grant{RepoID: 7, KeyID: 42, Op: "upload"}) {
+	if !ok || g != (Grant{RepoID: 7, KeyID: 42, KeyPin: KeyPin("SHA256:k"), Op: "upload"}) {
 		t.Fatalf("Verify = %+v, %v", g, ok)
 	}
 	if _, ok := Verify(secret, tok, now.Add(TokenTTL+time.Second)); ok {
 	if _, ok := Verify([]byte("other"), tok, now); ok {
 		t.Error("a token verified under another secret")
 	}
-	if g, ok := Verify(secret, Sign(secret, 7, 0, "download", now), now); !ok || g.KeyID != 0 {
+	if g, ok := Verify(secret, Sign(secret, 7, 0, "", "download", now), now); !ok || g.KeyID != 0 || g.KeyPin != "" {
 		t.Errorf("anonymous grant = %+v, %v", g, ok)
 	}
 }
 		t.Fatalf("a pre-upgrade token verified: %+v", g)
 	}
 }
+// A token minted before tokens carried the key's fingerprint has four
+// fields. It is refused (#303).
+func TestUnpinnedTokenRefused(t *testing.T) {
+	secret := []byte("secret")
+	payload := fmt.Sprintf("%d:%d:%s:%d", 7, 42, "upload", time.Now().Add(TokenTTL).Unix())
+	mac := hmac.New(sha256.New, secret)
+	mac.Write([]byte(payload))
+	tok := base64.RawURLEncoding.EncodeToString([]byte(payload)) + "." +
+		base64.RawURLEncoding.EncodeToString(mac.Sum(nil))
+	if g, ok := Verify(secret, tok, time.Now()); ok {
+		t.Fatalf("an unpinned token verified: %+v", g)
+	}
+}

internal/sshd/lfs.go +1 −1

	@@ -69,7 +69,7 @@ func runLFSAuthenticate(cfg config.Config, st *store.Store, user store.User, key
69	fmt.Fprintln(stderr, "internal error")	69	fmt.Fprintln(stderr, "internal error")
70	return protocol.ExitFailure	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	json.NewEncoder(stdout).Encode(map[string]any{	73	json.NewEncoder(stdout).Encode(map[string]any{
74	"href": fmt.Sprintf("%s/%s/%s.git/info/lfs",	74	"href": fmt.Sprintf("%s/%s/%s.git/info/lfs",
75	cfg.Server.SiteURL, repo.OwnerName, repo.Name),	75	cfg.Server.SiteURL, repo.OwnerName, repo.Name),

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