sshd: removing a key closes its connections !480

merged merged by cmc on 2026-09-28 21:48 UTC · krz/gitbay:revoke-closes-connections into main

20 files changed, +748 −61

Layout: unified · split

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

	@@ -15,8 +15,8 @@
15		15
16	\| Credential \| Format and generation \| Stored as \| Scope \| Expiry \| Revocation \|	16	\| Credential \| Format and generation \| Stored as \| Scope \| Expiry \| Revocation \|
17	\|--------------------+-------------------------------------------------+----------------------------------+--------------------------------------------+-------------------------------+-------------------------------------\|	17	\|--------------------+-------------------------------------------------+----------------------------------+--------------------------------------------+-------------------------------+-------------------------------------\|
18	\| SSH user key \| user's public key \| fingerprint and public blob \| =full=, =git=, or =runner= \| none \| =keys remove= (own keys) \|	18	\| SSH user key \| user's public key \| fingerprint and public blob \| =full=, =git=, or =runner= \| none \| =keys remove= (own keys); closes its connections \|
19	\| Deploy key \| public key \| same table, scope =deploy:<repo>:ro/rw= \| one repository, read or read-write \| none \| =repo deploy-key remove= (repo admin) \|	19	\| Deploy key \| public key \| same table, scope =deploy:<repo>:ro/rw= \| one repository, read or read-write \| none \| =repo deploy-key remove= (repo admin); closes its connections \|
20	\| API token \| =gb_= + 32 random bytes hex \| SHA-256 hash \| =full= or =read= \| optional =--ttl= \| =token revoke= \|	20	\| API token \| =gb_= + 32 random bytes hex \| SHA-256 hash \| =full= or =read= \| optional =--ttl= \| =token revoke= \|
21	\| Web session \| 32 random bytes hex, cookie =gitbay_session= \| SHA-256 hash \| full account \| 7 days, no sliding renewal \| logout, =web sessions revoke= \|	21	\| Web session \| 32 random bytes hex, cookie =gitbay_session= \| SHA-256 hash \| full account \| 7 days, no sliding renewal \| logout, =web sessions revoke= \|
22	\| Login link \| 32 random bytes hex in a URL \| SHA-256 hash, single use \| creates a web session \| 15 min (mail), 5 min (SSH) \| consumed on use \|	22	\| Login link \| 32 random bytes hex in a URL \| SHA-256 hash, single use \| creates a web session \| 15 min (mail), 5 min (SSH) \| consumed on use \|

.gitbay/wiki/Architecture/08-Operations.org −3

	@@ -84,6 +84,3 @@ is preserved.
84	\| Hide a repository \| =admin repo visibility <repo> private= \|	84	\| Hide a repository \| =admin repo visibility <repo> private= \|
85	\| Close registration \| =registration.mode = "closed"= and restart \|	85	\| Close registration \| =registration.mode = "closed"= and restart \|
86	\| See what happened \| =audit= (filter by actor, action, time) \|	86	\| See what happened \| =audit= (filter by actor, action, time) \|
87
88	Open connections of a removed key keep working until they close; see
89	#256.

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

	@@ -25,7 +25,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
25	\| Session cookie flags \| in place \| HttpOnly, SameSite=Lax, Secure with TLS (=internal/httpd/accounts.go=) \|	25	\| Session cookie flags \| in place \| HttpOnly, SameSite=Lax, Secure with TLS (=internal/httpd/accounts.go=) \|
26	\| Session lifetime \| partial \| 7 days absolute, no idle timeout (#276) \|	26	\| Session lifetime \| partial \| 7 days absolute, no idle timeout (#276) \|
27	\| Credential expiry \| partial \| API tokens optional; SSH and deploy keys none (#277) \|	27	\| Credential expiry \| partial \| API tokens optional; SSH and deploy keys none (#277) \|
28	\| Revocation takes effect immediately \| gap \| removed SSH key keeps open connections (#256) \|	28	\| 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=) \|
29	\| Delegation bounded by the delegating credential \| gap \| expiring tokens can mint lasting credentials (#257) \|	29	\| Delegation bounded by the delegating credential \| gap \| expiring tokens can mint lasting credentials (#257) \|
30		30
31	** Access control (V4)	31	** Access control (V4)

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

 | Issue | Area             | Gap                                                                   | Severity |
 |-------+------------------+-----------------------------------------------------------------------+----------|
 | #255  | CI isolation     | Untrusted and trusted builds of a repository share a writable build home | high  |
-| #256  | Authentication   | A removed SSH key keeps working on connections already open           | high     |
 | #257  | Credentials      | An expiring token can create credentials that outlive it; tokens default to full scope | high |
 | #258  | CI integrity     | Any writer can post a =ci/*= status; tree reuse ignores trust and image | high   |
 | #259  | Recovery         | No restore has been exercised; verification does not check git connectivity | high |
 | #281  | TLS              | No explicit minimum TLS version                                       | low      |
 | #282  | Hook socket      | Anything that can open =hook.sock= can act as any user                | medium   |
-Decisions already taken on these: #256 closes a removed key's
+Decisions already taken on these: #257 refuses credential
-connections, running commands included; #257 refuses credential
 creation from expiring tokens, records which token created each
 credential, and makes =read= the default scope.

.gitbay/wiki/Threat-Model.org +9

	@@ -36,6 +36,15 @@ matrix and the open gaps are in the [[file:Architecture/00-Overview.org][Archite
36		36
37	- SSH public key = identity. The SSH username is ignored; the presented	37	- SSH public key = identity. The SSH username is ignored; the presented
38	key's fingerprint resolves to an account. Key uniqueness is global.	38	key's fingerprint resolves to an account. Key uniqueness is global.
		39	- Revocation is immediate. Every exec and every git transport session
		40	re-reads its key. Removing a key, removing a deploy key, disabling or
		41	deleting an account closes the connections the affected keys opened:
		42	a git transport is killed with its children, and a push killed before
		43	its pre-receive hook answers moves no ref. A control command already
		44	inside its database write finishes it; its output is lost. A
		45	revocation made by =gitbayd admin= on the host, another process, is
		46	found within 15 seconds. In =ssh.mode = "system"= each exec is its
		47	own process: the next exec is refused, one already running is not cut.
39	- Per-instance trust. Email verification and key registration are local	48	- Per-instance trust. Email verification and key registration are local
40	to an instance and never transfer. Account migration re-registers keys	49	to an instance and never transfer. Account migration re-registers keys
41	and re-verifies emails on the target by design.	50	and re-verifies emails on the target by design.

.gitbay/wiki/Users.org +4

 =--label= gives one; =keys label= renames a key, and with no text
 clears the name. Labels are one line of up to 64 bytes.
+Removing a key closes every connection it opened, including the CLI's
+shared one; removing the key the current command runs on ends that
+command's connection too.
 Scopes: =full= (default; git plus every control command), =git= (git
 transport only — right for automation keys, which then cannot touch
 issues, settings, or your account), or =runner= (the CI runner's

CHANGELOG.org +6

 automatically on daemon start; upgrade notes appear per release when
 anything beyond "replace the binary and restart" is needed.
+* unreleased
+- Removing an SSH key, removing a deploy key, or disabling or deleting an
+  account closes every open connection using an affected key, git
+  transports included; every command re-reads its key (#256).
 * v1.36.0 — 2026-09-23
 Terminal output for the CLI (#254).

cmd/gitbayd/system.go +1 −1

	@@ -94,7 +94,7 @@ func shellCmd() *cobra.Command {
94	fmt.Fprintf(os.Stderr, "gitbay control plane: interactive shells are not available.\nTry: ssh <host> help\n")	94	fmt.Fprintf(os.Stderr, "gitbay control plane: interactive shells are not available.\nTry: ssh <host> help\n")
95	os.Exit(protocol.ExitUsage)	95	os.Exit(protocol.ExitUsage)
96	}	96	}
97	code := sshd.Exec(cfg, st, user, key.Scope, key.Fingerprint, control.ParseTerm(os.Getenv("GITBAY_TERM")), cmdline, os.Stdin, os.Stdout, os.Stderr, nil, nil)	97	code := sshd.Exec(cfg, st, user, key, control.ParseTerm(os.Getenv("GITBAY_TERM")), cmdline, os.Stdin, os.Stdout, os.Stderr, nil, nil, nil)
98	st.Close()	98	st.Close()
99	os.Exit(code)	99	os.Exit(code)
100	return nil	100	return nil

e2e/revoke_test.go added +155

@@ -0,0 +1,155 @@
	1	package e2e
	2
	3	import (
	4	"bufio"
	5	"fmt"
	6	"io"
	7	"os"
	8	"os/exec"
	9	"path/filepath"
	10	"strconv"
	11	"strings"
	12	"testing"
	13	"time"
	14	)
	15
	16	// pkt frames one pkt-line.
	17	func pkt(s string) string { return fmt.Sprintf("%04x%s", len(s)+4, s) }
	18
	19	// readPkt reads one pkt-line; a flush reads as "".
	20	func readPkt(r *bufio.Reader) (string, error) {
	21	var n [4]byte
	22	if _, err := io.ReadFull(r, n[:]); err != nil {
	23	return "", err
	24	}
	25	size, err := strconv.ParseUint(string(n[:]), 16, 16)
	26	if err != nil {
	27	return "", err
	28	}
	29	if size == 0 {
	30	return "", nil
	31	}
	32	buf := make([]byte, size-4)
	33	_, err = io.ReadFull(r, buf)
	34	return string(buf), err
	35	}
	36
	37	// fingerprint is the SHA256 fingerprint of a public key file.
	38	func fingerprint(t *testing.T, pubPath string) string {
	39	t.Helper()
	40	out, err := exec.Command("ssh-keygen", "-lf", pubPath).Output()
	41	if err != nil {
	42	t.Fatalf("ssh-keygen -lf: %v", err)
	43	}
	44	return strings.Fields(string(out))[1]
	45	}
	46
	47	// Removing a key cuts the connections it opened: every session
	48	// multiplexed on a ControlMaster, and a push in flight, which moves no
	49	// ref (#256).
	50	func TestRemovedKeyCutsMultiplexedConnection(t *testing.T) {
	51	t.Parallel()
	52	inst := startInstance(t)
	53	aliceKey := setupPublicRepo(t, inst, "alice/app")
	54	spare := inst.newKey(t, "spare")
	55	pub, err := os.ReadFile(spare + ".pub")
	56	if err != nil {
	57	t.Fatal(err)
	58	}
	59	if _, errOut, code := inst.ssh(t, aliceKey, string(pub), "keys", "add"); code != 0 {
	60	t.Fatalf("keys add: %s", errOut)
	61	}
	62
	63	// The control socket sits under the system temp dir with a short
	64	// name: t.TempDir() or %C on macOS passes the 104-byte socket path
	65	// limit.
	66	cmDir, err := os.MkdirTemp("", "cm")
	67	if err != nil {
	68	t.Fatal(err)
	69	}
	70	t.Cleanup(func() { os.RemoveAll(cmDir) })
	71	muxArgs := []string{
	72	"-p", fmt.Sprint(inst.port),
	73	"-i", aliceKey,
	74	"-o", "IdentitiesOnly=yes",
	75	"-o", "StrictHostKeyChecking=no",
	76	"-o", "UserKnownHostsFile=" + filepath.Join(inst.sshDir, "known_hosts"),
	77	"-o", "BatchMode=yes",
	78	"-o", "ControlMaster=auto",
	79	"-o", "ControlPath=" + filepath.Join(cmDir, "s"),
	80	"-o", "ControlPersist=60",
	81	}
	82	mux := func(args ...string) *exec.Cmd {
	83	return exec.Command("ssh", append(append([]string{}, muxArgs...), args...)...)
	84	}
	85	t.Cleanup(func() { mux("-O", "exit", "git@127.0.0.1").Run() })
	86
	87	if out, err := mux("git@127.0.0.1", "whoami").Output(); err != nil \|\| strings.TrimSpace(string(out)) != "alice" {
	88	var stderr []byte
	89	if ee, ok := err.(*exec.ExitError); ok {
	90	stderr = ee.Stderr
	91	}
	92	t.Fatalf("whoami over the master: %v %q %s", err, out, stderr)
	93	}
	94
	95	// A push held open mid-pack: the ref update is sent, the pack is not.
	96	push := mux("git@127.0.0.1", "git-receive-pack", "alice/app")
	97	stdin, err := push.StdinPipe()
	98	if err != nil {
	99	t.Fatal(err)
	100	}
	101	stdout, err := push.StdoutPipe()
	102	if err != nil {
	103	t.Fatal(err)
	104	}
	105	if err := push.Start(); err != nil {
	106	t.Fatal(err)
	107	}
	108	t.Cleanup(func() { push.Process.Kill() })
	109	adv := bufio.NewReader(stdout)
	110	first, err := readPkt(adv)
	111	if err != nil \|\| len(first) < 40 {
	112	t.Fatalf("advertisement: %q %v", first, err)
	113	}
	114	oldSHA := first[:40]
	115	for {
	116	line, err := readPkt(adv)
	117	if err != nil {
	118	t.Fatalf("advertisement: %v", err)
	119	}
	120	if line == "" {
	121	break
	122	}
	123	}
	124	newSHA := strings.Repeat("1", 40)
	125	io.WriteString(stdin, pkt(oldSHA+" "+newSHA+" refs/heads/main\x00report-status\n")+"0000")
	126	// A pack header announcing one object, and no object.
	127	stdin.Write([]byte("PACK\x00\x00\x00\x02\x00\x00\x00\x01"))
	128	exited := make(chan error, 1)
	129	go func() {
	130	io.Copy(io.Discard, adv)
	131	exited <- push.Wait()
	132	}()
	133
	134	if _, errOut, code := inst.ssh(t, spare, "", "keys", "remove", fingerprint(t, aliceKey+".pub")); code != 0 {
	135	t.Fatalf("keys remove: %s", errOut)
	136	}
	137	select {
	138	case err := <-exited:
	139	if err == nil {
	140	t.Fatal("the push exited cleanly after its key was removed")
	141	}
	142	case <-time.After(10 * time.Second):
	143	t.Fatal("the push outlived its key")
	144	}
	145
	146	// The master went with the connection; a new one authenticates
	147	// again, and the key is unknown.
	148	if out, err := mux("git@127.0.0.1", "whoami").CombinedOutput(); err == nil {
	149	t.Fatalf("whoami after removal succeeded: %s", out)
	150	}
	151	refs := mustGit(t, t.TempDir(), inst.gitEnv(spare), "ls-remote", inst.sshURL("alice/app"), "refs/heads/main")
	152	if !strings.HasPrefix(refs, oldSHA) {
	153	t.Fatalf("main moved: %s, want %s", refs, oldSHA)
	154	}
	155	}

internal/gitutil/gitutil.go +19 −3

	@@ -35,8 +35,10 @@ func InitBare(path, defaultBranch, hooksPath string) error {
35	// Transport streams one git transport service (upload-pack, receive-pack,	35	// Transport streams one git transport service (upload-pack, receive-pack,
36	// upload-archive). extraEnv entries are appended to the process environment;	36	// upload-archive). extraEnv entries are appended to the process environment;
37	// hooks read the GITBAY_* variables from it. maxPack caps incoming pack	37	// hooks read the GITBAY_* variables from it. maxPack caps incoming pack
38	// bytes on receive-pack (0 = unlimited).	38	// bytes on receive-pack (0 = unlimited). Closing cancel kills the service
39	func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer, extraEnv []string, maxPack int64) error {	39	// and everything it started; a push killed before its pre-receive hook
		40	// answers updates no refs. A nil cancel never fires.
		41	func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer, extraEnv []string, maxPack int64, cancel <-chan struct{}) error {
40	var args []string	42	var args []string
41	switch service {	43	switch service {
42	case "git-upload-pack", "git-receive-pack", "git-upload-archive":	44	case "git-upload-pack", "git-receive-pack", "git-upload-archive":
	@@ -52,7 +54,21 @@ func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer
52	cmd.Stdin = stdin	54	cmd.Stdin = stdin
53	cmd.Stdout = stdout	55	cmd.Stdout = stdout
54	cmd.Stderr = errW	56	cmd.Stderr = errW
55	return cmd.Run()	57	ownProcessGroup(cmd)
		58	if err := cmd.Start(); err != nil {
		59	return err
		60	}
		61	finished := make(chan struct{})
		62	go func() {
		63	select {
		64	case <-cancel:
		65	killTree(cmd)
		66	case <-finished:
		67	}
		68	}()
		69	err := cmd.Wait()
		70	close(finished)
		71	return err
56	}	72	}
57		73
58	// IsAncestor reports whether old is an ancestor of new in the repository at	74	// IsAncestor reports whether old is an ancestor of new in the repository at

internal/gitutil/proc_other.go added +13

@@ -0,0 +1,13 @@
	1	//go:build !unix
	2
	3	package gitutil
	4
	5	import "os/exec"
	6
	7	func ownProcessGroup(cmd *exec.Cmd) {}
	8
	9	func killTree(cmd *exec.Cmd) {
	10	if cmd.Process != nil {
	11	cmd.Process.Kill()
	12	}
	13	}

internal/gitutil/proc_unix.go added +26

@@ -0,0 +1,26 @@
	1	//go:build unix
	2
	3	package gitutil
	4
	5	import (
	6	"os/exec"
	7	"syscall"
	8	)
	9
	10	// ownProcessGroup puts cmd in a process group of its own, so killTree
	11	// ends what it started too: receive-pack runs index-pack and the hooks.
	12	func ownProcessGroup(cmd *exec.Cmd) {
	13	if cmd.SysProcAttr == nil {
	14	cmd.SysProcAttr = &syscall.SysProcAttr{}
	15	}
	16	cmd.SysProcAttr.Setpgid = true
	17	}
	18
	19	func killTree(cmd *exec.Cmd) {
	20	if cmd.Process == nil {
	21	return
	22	}
	23	if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil {
	24	cmd.Process.Kill()
	25	}
	26	}

internal/gitutil/transport_test.go added +33

@@ -0,0 +1,33 @@
	1	package gitutil
	2
	3	import (
	4	"io"
	5	"os/exec"
	6	"strings"
	7	"testing"
	8	"time"
	9	)
	10
	11	// Closing cancel kills the transport; it does not wait for the client
	12	// to hang up. Stdin ends only after the kill, as a cut connection's
	13	// does, so a clean exit here would mean the kill never happened.
	14	func TestTransportCancelKillsGit(t *testing.T) {
	15	dir := t.TempDir()
	16	if out, err := exec.Command("git", "init", "-q", "--bare", dir).CombinedOutput(); err != nil {
	17	t.Fatalf("git init: %v\n%s", err, out)
	18	}
	19	in, w := io.Pipe()
	20	cancel := make(chan struct{})
	21	errc := make(chan error, 1)
	22	go func() { errc <- Transport("git-upload-pack", dir, in, io.Discard, io.Discard, nil, 0, cancel) }()
	23	close(cancel)
	24	time.AfterFunc(500*time.Millisecond, func() { w.Close() })
	25	select {
	26	case err := <-errc:
	27	if err == nil \|\| !strings.Contains(err.Error(), "killed") {
	28	t.Fatalf("Transport returned %v, want the process killed", err)
	29	}
	30	case <-time.After(5 * time.Second):
	31	t.Fatal("Transport did not return after cancel")
	32	}
	33	}

internal/sshd/revoke_test.go added +112

@@ -0,0 +1,112 @@
	1	package sshd
	2
	3	import (
	4	"bytes"
	5	"errors"
	6	"strings"
	7	"testing"
	8	"time"
	9
	10	"golang.org/x/crypto/ssh"
	11	)
	12
	13	// execStatus runs cmd on a new session and returns its exit status and
	14	// stderr; -1 when the session could not run.
	15	func execStatus(client *ssh.Client, cmd string) (int, string) {
	16	sess, err := client.NewSession()
	17	if err != nil {
	18	return -1, err.Error()
	19	}
	20	defer sess.Close()
	21	var stderr bytes.Buffer
	22	sess.Stderr = &stderr
	23	err = sess.Run(cmd)
	24	var exit *ssh.ExitError
	25	switch {
	26	case err == nil:
	27	return 0, stderr.String()
	28	case errors.As(err, &exit):
	29	return exit.ExitStatus(), stderr.String()
	30	}
	31	return -1, err.Error()
	32	}
	33
	34	// waitClosed fails unless the server closes the client's connection
	35	// within five seconds.
	36	func waitClosed(t testing.T, client ssh.Client) {
	37	t.Helper()
	38	done := make(chan struct{})
	39	go func() { client.Wait(); close(done) }()
	40	select {
	41	case <-done:
	42	case <-time.After(5 * time.Second):
	43	t.Fatal("the connection stayed open")
	44	}
	45	}
	46
	47	// Each exec reads the key again. The rows change behind the store's
	48	// back here, so no revocation is announced and the connection stays up:
	49	// what refuses the command is the per-exec check alone.
	50	func TestExecRevalidatesKey(t *testing.T) {
	51	ts := newTestServer(t)
	52	if code, errOut := execStatus(ts.client, "whoami"); code != 0 {
	53	t.Fatalf("whoami: %d %s", code, errOut)
	54	}
	55	if _, err := ts.st.DB.Exec("UPDATE ssh_keys SET scope = 'git' WHERE id = ?", ts.keyID); err != nil {
	56	t.Fatal(err)
	57	}
	58	if code, errOut := execStatus(ts.client, "whoami"); code != 4 \|\| !strings.Contains(errOut, "does not allow control commands") {
	59	t.Fatalf("whoami after re-scope: %d %q", code, errOut)
	60	}
	61	if _, err := ts.st.DB.Exec("DELETE FROM ssh_keys WHERE id = ?", ts.keyID); err != nil {
	62	t.Fatal(err)
	63	}
	64	if code, errOut := execStatus(ts.client, "whoami"); code != 4 \|\| !strings.Contains(errOut, "no longer registered") {
	65	t.Fatalf("whoami after delete: %d %q", code, errOut)
	66	}
	67	}
	68
	69	// Removing the key cuts the connection, ending a command running on it.
	70	func TestRemoveKeyCutsConnection(t *testing.T) {
	71	ts := newTestServer(t)
	72	withBuild(t, ts)
	73	var stderr bytes.Buffer
	74	sess := startFollow(t, ts.client, &stderr)
	75	if err := ts.st.RemoveSSHKey(ts.uid, ts.fp); err != nil {
	76	t.Fatal(err)
	77	}
	78	waited := make(chan error, 1)
	79	go func() { waited <- sess.Wait() }()
	80	select {
	81	case err := <-waited:
	82	if err == nil {
	83	t.Fatal("the follow exited cleanly after its key was removed")
	84	}
	85	case <-time.After(5 * time.Second):
	86	t.Fatal("the follow outlived its key")
	87	}
	88	waitClosed(t, ts.client)
	89	}
	90
	91	func TestDisableCutsConnection(t *testing.T) {
	92	ts := newTestServer(t)
	93	if err := ts.st.SetUserDisabled(ts.uid, true); err != nil {
	94	t.Fatal(err)
	95	}
	96	waitClosed(t, ts.client)
	97	}
	98
	99	// A revocation made by another process is not announced here; the
	100	// sweep finds it. A live key survives the sweep.
	101	func TestSweepCutsOutOfProcessRevocation(t *testing.T) {
	102	ts := newTestServer(t)
	103	ts.srv.sweepOnce()
	104	if code, errOut := execStatus(ts.client, "whoami"); code != 0 {
	105	t.Fatalf("the sweep cut a live key: %d %s", code, errOut)
	106	}
	107	if _, err := ts.st.DB.Exec("UPDATE users SET disabled = 1 WHERE id = ?", ts.uid); err != nil {
	108	t.Fatal(err)
	109	}
	110	ts.srv.sweepOnce()
	111	waitClosed(t, ts.client)
	112	}

internal/sshd/sshd.go +121 −17

 	"fmt"
 	"io"
 	"log/slog"
+	"maps"
 	"net"
 	"os"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"sync"
 	"sync/atomic"
 type conn struct {
 	net    net.Conn
 	active atomic.Int32
+	// keyID and userID are the key that authenticated the connection and
+	// its account: 0 before the handshake and for an unregistered key.
+	// Guarded by Server.mu.
+	keyID, userID int64
+	revoked       chan struct{} // closed by cut
+	cutOnce       sync.Once
+}
+// cut ends the connection because its key was revoked: a git transport
+// on it is killed, and every other command loses its channel.
+func (c *conn) cut() {
+	c.cutOnce.Do(func() { close(c.revoked) })
+	c.net.Close()
 }
 func New(cfg config.Config, st *store.Store) (*Server, error) {
 		sc.AddHostKey(sg)
 	}
 	s.sshCfg = sc
+	st.OnRevoke(s.revoke)
 	return s, nil
 }
 	return &ssh.Permissions{Extensions: map[string]string{
 		"user-id": strconv.FormatInt(key.UserID, 10),
 		"key-id":  strconv.FormatInt(key.ID, 10),
-		"key-fp":  fp,
-		"scope":   key.Scope,
 	}}, nil
 }
 // Serve accepts connections on ln until it is closed.
 func (s *Server) Serve(ln net.Listener) error {
+	served := make(chan struct{})
+	defer close(served)
+	go s.sweep(served)
 	for {
 		nc, err := ln.Accept()
 		if err != nil {
 			return err
 		}
-		c := &conn{net: nc}
+		c := &conn{net: nc, revoked: make(chan struct{})}
 		s.mu.Lock()
 		s.conns[c] = struct{}{}
 		s.mu.Unlock()
 	}
 }
+// revoke closes the connections opened by the keys r names.
+func (s *Server) revoke(r store.Revoked) {
+	var cut []*conn
+	s.mu.Lock()
+	for c := range s.conns {
+		if c.keyID == 0 {
+			continue
+		}
+		if (r.UserID != 0 && c.userID == r.UserID) || slices.Contains(r.KeyIDs, c.keyID) {
+			cut = append(cut, c)
+		}
+	}
+	s.mu.Unlock()
+	for _, c := range cut {
+		c.cut()
+	}
+}
+// sweepInterval bounds how long a revocation this process was not told
+// about (gitbayd admin on the host) leaves a connection open.
+const sweepInterval = 15 * time.Second
+func (s *Server) sweep(served <-chan struct{}) {
+	t := time.NewTicker(sweepInterval)
+	defer t.Stop()
+	for {
+		select {
+		case <-t.C:
+			s.sweepOnce()
+		case <-served:
+			return
+		case <-s.stopping:
+			return
+		}
+	}
+}
+// sweepOnce cuts every connection whose key is no longer live. Only
+// connections whose key was asked about are judged: one that
+// authenticated while the query ran waits for the next sweep.
+func (s *Server) sweepOnce() {
+	asked := map[int64]bool{}
+	s.mu.Lock()
+	for c := range s.conns {
+		if c.keyID != 0 {
+			asked[c.keyID] = true
+		}
+	}
+	s.mu.Unlock()
+	if len(asked) == 0 {
+		return
+	}
+	live, err := s.st.LiveSSHKeys(slices.Collect(maps.Keys(asked)))
+	if err != nil {
+		slog.Error("ssh sweep: key lookup", "err", err)
+		return
+	}
+	var cut []*conn
+	s.mu.Lock()
+	for c := range s.conns {
+		if asked[c.keyID] && !live[c.keyID] {
+			cut = append(cut, c)
+		}
+	}
+	s.mu.Unlock()
+	for _, c := range cut {
+		c.cut()
+	}
+}
 // Stop ends the commands that run until something happens (build log
 // --follow), so a shutdown drain waits only for work that finishes. It
 // does not close connections; Shutdown does.
 		return
 	}
 	defer sconn.Close()
+	ext := sconn.Permissions.Extensions
+	s.mu.Lock()
+	c.keyID, _ = strconv.ParseInt(ext["key-id"], 10, 64)
+	c.userID, _ = strconv.ParseInt(ext["user-id"], 10, 64)
+	s.mu.Unlock()
 	go ssh.DiscardRequests(reqs)
 	for newCh := range chans {
 		c.active.Add(1)
 		go func() {
 			defer c.active.Add(-1)
-			s.handleSession(sconn, ch, chReqs)
+			s.handleSession(c, sconn, ch, chReqs)
 		}()
 	}
 }
-func (s *Server) handleSession(sconn *ssh.ServerConn, ch ssh.Channel, reqs <-chan *ssh.Request) {
+func (s *Server) handleSession(c *conn, sconn *ssh.ServerConn, ch ssh.Channel, reqs <-chan *ssh.Request) {
 	defer ch.Close()
 	var term control.Term
 	for req := range reqs {
 				}
 				close(done)
 			}()
-			code := s.runExec(sconn, ch, term, payload.Command, done)
+			code := s.runExec(c, sconn, ch, term, payload.Command, done)
 			sendExit(ch, code)
 			return
 		case "shell":
 	ch.SendRequest("exit-status", false, ssh.Marshal(&msg))
 }
-func (s *Server) runExec(sconn *ssh.ServerConn, ch ssh.Channel, term control.Term, cmdline string, done <-chan struct{}) int {
+func (s *Server) runExec(c *conn, sconn *ssh.ServerConn, ch ssh.Channel, term control.Term, cmdline string, done <-chan struct{}) int {
 	ext := sconn.Permissions.Extensions
 	if blob := ext["anon-key"]; blob != "" {
 		return s.runAnonymous(ch, blob, cmdline)
 	}
 	userID, _ := strconv.ParseInt(ext["user-id"], 10, 64)
 	keyID, _ := strconv.ParseInt(ext["key-id"], 10, 64)
+	// A connection outlives its commands, so the key is read again for
+	// each one: what it may do is what it may do now (#256).
+	key, err := s.st.SSHKeyByID(keyID)
+	if errors.Is(err, store.ErrNotFound) || (err == nil && key.UserID != userID) {
+		fmt.Fprintln(ch.Stderr(), "this key is no longer registered")
+		return protocol.ExitDenied
+	}
+	if err != nil {
+		slog.Error("ssh exec: key lookup", "err", err)
+		fmt.Fprintln(ch.Stderr(), "authentication temporarily unavailable")
+		return protocol.ExitFailure
+	}
 	user, err := s.st.UserByID(userID)
 	if err != nil {
 		fmt.Fprintln(ch.Stderr(), "account no longer exists")
 		return protocol.ExitDenied
 	}
 	_ = s.st.TouchSSHKey(keyID)
-	return Exec(s.cfg, s.st, user, ext["scope"], ext["key-fp"], term, cmdline, ch, ch, ch.Stderr(), done, s.stopping)
+	return Exec(s.cfg, s.st, user, key, term, cmdline, ch, ch, ch.Stderr(), done, s.stopping, c.revoked)
 }
 // runAnonymous handles a session from an unregistered key: the register
 // Exec runs one SSH exec command line for an authenticated key. It is the
 // single dispatch path shared by the embedded listener and the system-sshd
-// forced command (gitbayd shell).
+// forced command (gitbayd shell). Closing revoked kills a git transport.
-func Exec(cfg config.Config, st *store.Store, user store.User, scope, source string, term control.Term, cmdline string,
+func Exec(cfg config.Config, st *store.Store, user store.User, key store.SSHKey, term control.Term, cmdline string,
-	stdin io.Reader, stdout, stderr io.Writer, done, stopping <-chan struct{}) int {
+	stdin io.Reader, stdout, stderr io.Writer, done, stopping, revoked <-chan struct{}) int {
 	if user.Disabled {
 		fmt.Fprintln(stderr, "this account is disabled; contact the instance admin")
 		return protocol.ExitDenied
 				fmt.Fprintln(stderr, "your account is not active yet: verify your email first")
 				return protocol.ExitDenied
 			}
-			return runGit(cfg, st, user, scope, argv, stdin, stdout, stderr)
+			return runGit(cfg, st, user, key.Scope, argv, stdin, stdout, stderr, revoked)
 		case "git-lfs-authenticate":
 			// Part of the git transport, not the control plane: usable by
 			// git-scoped and deploy keys, with the transports' access rules.
 				fmt.Fprintln(stderr, "your account is not active yet: verify your email first")
 				return protocol.ExitDenied
 			}
-			return runLFSAuthenticate(cfg, st, user, scope, argv, stdout, stderr)
+			return runLFSAuthenticate(cfg, st, user, key.Scope, argv, stdout, stderr)
 		}
 	}
 	ctx := &control.Ctx{
 		User:     user,
-		Scope:    scope,
+		Scope:    key.Scope,
-		Source:   source,
+		Source:   key.Fingerprint,
 		Term:     term,
 		Store:    st,
 		Cfg:      cfg,
 // runGit streams a git transport service after access checks.
 func runGit(cfg config.Config, st *store.Store, user store.User, scope string, argv []string,
-	stdin io.Reader, stdout, stderr io.Writer) int {
+	stdin io.Reader, stdout, stderr io.Writer, revoked <-chan struct{}) int {
 	service := argv[0]
 	if len(argv) != 2 {
 		fmt.Fprintf(stderr, "usage: %s <path>\n", service)
 			}
 		}
 	}
-	if err := gitutil.Transport(service, dir, stdin, stdout, stderr, env, maxPack); err != nil {
+	if err := gitutil.Transport(service, dir, stdin, stdout, stderr, env, maxPack, revoked); err != nil {
 		return protocol.ExitFailure
 	}
 	return protocol.ExitOK

internal/sshd/sshd_test.go +46 −17

 	"gitbay.org/gitbay/internal/store"
 )
-// followServer starts an embedded server holding alice, her public repo
+// testServer is an embedded server over a fresh store holding alice
-// alice/app and a queued build 1 whose log has one line, and returns it
+// with one full-scope key, and a client connected with that key.
-// with a client connected as alice.
+type testServer struct {
-func followServer(t *testing.T) (*Server, *ssh.Client) {
+	srv    *Server
+	st     *store.Store
+	client *ssh.Client
+	uid    int64
+	keyID  int64
+	fp     string
+}
+func newTestServer(t *testing.T) testServer {
 	t.Helper()
 	root := t.TempDir()
 	st, err := store.Open(filepath.Join(root, "gitbay.db"))
 	if err != nil {
 		t.Fatal(err)
 	}
-	repoID, err := st.CreateRepo("user", uid, "app", "public")
-	if err != nil {
-		t.Fatal(err)
-	}
-	id, err := st.CreateBuild(repoID, "unit", "abc", "main", `["true"]`, "", "", true)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := st.AppendBuildLog(id, []byte("queued\n")); err != nil {
-		t.Fatal(err)
-	}
 	_, priv, err := ed25519.GenerateKey(rand.Reader)
 	if err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
 	}
 	pub := signer.PublicKey()
-	if err := st.AddSSHKey(uid, ssh.FingerprintSHA256(pub), pub.Type(), pub.Marshal(), "full", "test"); err != nil {
+	fp := ssh.FingerprintSHA256(pub)
+	if err := st.AddSSHKey(uid, fp, pub.Type(), pub.Marshal(), "full", "test"); err != nil {
+		t.Fatal(err)
+	}
+	key, err := st.SSHKeyByFingerprint(fp)
+	if err != nil {
 		t.Fatal(err)
 	}
 		t.Fatal(err)
 	}
 	t.Cleanup(func() { client.Close() })
-	return srv, client
+	return testServer{srv: srv, st: st, client: client, uid: uid, keyID: key.ID, fp: fp}
+}
+// withBuild gives alice the public repo alice/app and a queued build 1
+// whose log has one line.
+func withBuild(t *testing.T, ts testServer) {
+	t.Helper()
+	repoID, err := ts.st.CreateRepo("user", ts.uid, "app", "public")
+	if err != nil {
+		t.Fatal(err)
+	}
+	id, err := ts.st.CreateBuild(repoID, "unit", "abc", "main", `["true"]`, "", "", true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := ts.st.AppendBuildLog(id, []byte("queued\n")); err != nil {
+		t.Fatal(err)
+	}
+}
+// followServer starts an embedded server holding alice, her public repo
+// alice/app and a queued build 1 whose log has one line, and returns it
+// with a client connected as alice.
+func followServer(t *testing.T) (*Server, *ssh.Client) {
+	t.Helper()
+	ts := newTestServer(t)
+	withBuild(t, ts)
+	return ts.srv, ts.client
 }
 // startFollow runs build log --follow on a new session and returns once

internal/store/revoke.go added +60

@@ -0,0 +1,60 @@
	1	package store
	2
	3	import (
	4	"slices"
	5	"strings"
	6	)
	7
	8	// Revoked names SSH keys that stopped being valid: by id, or every key
	9	// of an account. The SSH listener closes the connections they opened.
	10	type Revoked struct {
	11	KeyIDs []int64
	12	UserID int64 // every key of this account; 0 for none
	13	}
	14
	15	// OnRevoke registers f to run after each revocation this process
	16	// commits. Revocations committed by another process (gitbayd admin on
	17	// the host) are not announced; the listener's sweep finds those.
	18	func (s *Store) OnRevoke(f func(Revoked)) {
	19	s.revokeMu.Lock()
	20	defer s.revokeMu.Unlock()
	21	s.onRevoke = append(s.onRevoke, f)
	22	}
	23
	24	// announce runs the subscribers. Call it after the commit, outside any
	25	// transaction.
	26	func (s *Store) announce(r Revoked) {
	27	s.revokeMu.Lock()
	28	fs := slices.Clone(s.onRevoke)
	29	s.revokeMu.Unlock()
	30	for _, f := range fs {
	31	f(r)
	32	}
	33	}
	34
	35	// LiveSSHKeys reports which of ids still name a registered key on an
	36	// account that is not disabled.
	37	func (s *Store) LiveSSHKeys(ids []int64) (map[int64]bool, error) {
	38	live := map[int64]bool{}
	39	if len(ids) == 0 {
	40	return live, nil
	41	}
	42	args := make([]any, len(ids))
	43	for i, id := range ids {
	44	args[i] = id
	45	}
	46	rows, err := s.DB.Query(`SELECT k.id FROM ssh_keys k JOIN users u ON u.id = k.user_id
	47	WHERE u.disabled = 0 AND k.id IN (?`+strings.Repeat(", ?", len(ids)-1)+`)`, args...)
	48	if err != nil {
	49	return nil, err
	50	}
	51	defer rows.Close()
	52	for rows.Next() {
	53	var id int64
	54	if err := rows.Scan(&id); err != nil {
	55	return nil, err
	56	}
	57	live[id] = true
	58	}
	59	return live, rows.Err()
	60	}

internal/store/revoke_test.go added +107

@@ -0,0 +1,107 @@
	1	package store
	2
	3	import (
	4	"slices"
	5	"testing"
	6	)
	7
	8	func revokeFixture(t testing.T) (Store, int64, *[]Revoked) {
	9	t.Helper()
	10	s := open(t)
	11	if err := s.MigrateUp(); err != nil {
	12	t.Fatal(err)
	13	}
	14	uid, err := s.CreateUser("alice", false)
	15	if err != nil {
	16	t.Fatal(err)
	17	}
	18	var got []Revoked
	19	s.OnRevoke(func(r Revoked) { got = append(got, r) })
	20	return s, uid, &got
	21	}
	22
	23	func keyID(t testing.T, s Store, fp string) int64 {
	24	t.Helper()
	25	k, err := s.SSHKeyByFingerprint(fp)
	26	if err != nil {
	27	t.Fatal(err)
	28	}
	29	return k.ID
	30	}
	31
	32	func TestRemovalsAnnounceTheirKeys(t *testing.T) {
	33	s, uid, got := revokeFixture(t)
	34	if err := s.AddSSHKey(uid, "SHA256:a", "ssh-ed25519", []byte("a"), "full", ""); err != nil {
	35	t.Fatal(err)
	36	}
	37	if err := s.AddSSHKey(uid, "SHA256:d", "ssh-ed25519", []byte("d"), "deploy:7:ro", ""); err != nil {
	38	t.Fatal(err)
	39	}
	40	a, d := keyID(t, s, "SHA256:a"), keyID(t, s, "SHA256:d")
	41
	42	if err := s.RemoveSSHKey(uid, "SHA256:a"); err != nil {
	43	t.Fatal(err)
	44	}
	45	if err := s.RemoveDeployKey(7, "SHA256:d"); err != nil {
	46	t.Fatal(err)
	47	}
	48	if err := s.SetUserDisabled(uid, true); err != nil {
	49	t.Fatal(err)
	50	}
	51	if err := s.SetUserDisabled(uid, false); err != nil {
	52	t.Fatal(err)
	53	}
	54	want := []Revoked{{KeyIDs: []int64{a}}, {KeyIDs: []int64{d}}, {UserID: uid}}
	55	if !slices.EqualFunc(*got, want, func(x, y Revoked) bool {
	56	return slices.Equal(x.KeyIDs, y.KeyIDs) && x.UserID == y.UserID
	57	}) {
	58	t.Fatalf("announced %+v, want %+v (enabling announces nothing)", *got, want)
	59	}
	60	// A removal that found nothing announces nothing.
	61	if err := s.RemoveSSHKey(uid, "SHA256:a"); err != ErrNotFound {
	62	t.Fatalf("second remove: %v", err)
	63	}
	64	if len(*got) != 3 {
	65	t.Fatalf("a miss was announced: %+v", *got)
	66	}
	67	}
	68
	69	func TestDeleteUserAnnounces(t *testing.T) {
	70	s, uid, got := revokeFixture(t)
	71	if err := s.DeleteUser(uid); err != nil {
	72	t.Fatal(err)
	73	}
	74	if len(got) != 1 \|\| (got)[0].UserID != uid {
	75	t.Fatalf("announced %+v", *got)
	76	}
	77	}
	78
	79	func TestLiveSSHKeys(t *testing.T) {
	80	s, uid, _ := revokeFixture(t)
	81	bob, err := s.CreateUser("bob", false)
	82	if err != nil {
	83	t.Fatal(err)
	84	}
	85	for _, k := range []struct {
	86	uid int64
	87	fp string
	88	}{{uid, "SHA256:a"}, {bob, "SHA256:b"}} {
	89	if err := s.AddSSHKey(k.uid, k.fp, "ssh-ed25519", []byte(k.fp), "full", ""); err != nil {
	90	t.Fatal(err)
	91	}
	92	}
	93	a, b := keyID(t, s, "SHA256:a"), keyID(t, s, "SHA256:b")
	94	if _, err := s.DB.Exec("UPDATE users SET disabled = 1 WHERE id = ?", bob); err != nil {
	95	t.Fatal(err)
	96	}
	97	live, err := s.LiveSSHKeys([]int64{a, b, 999})
	98	if err != nil {
	99	t.Fatal(err)
	100	}
	101	if !live[a] \|\| live[b] \|\| live[999] {
	102	t.Fatalf("live = %v; want only %d", live, a)
	103	}
	104	if live, err := s.LiveSSHKeys(nil); err != nil \|\| len(live) != 0 {
	105	t.Fatalf("no ids: %v %v", live, err)
	106	}
	107	}

internal/store/store.go +4

 	// by the next change to that build's row (BuildLogWait).
 	logMu   sync.Mutex
 	logWait map[int64]chan struct{}
+	// onRevoke runs after each key revocation this process commits.
+	revokeMu sync.Mutex
+	onRevoke []func(Revoked)
 }
 // Open opens (creating if needed) the database at path with WAL mode and

internal/store/users.go +28 −14

 	if n, _ := res.RowsAffected(); n == 0 {
 		return ErrNotFound
 	}
+	s.announce(Revoked{UserID: id})
 	return nil
 }
 // SetUserDisabled suspends or restores an account. Disabling drops every
 // credential that would grant a session on its own — web sessions, API
 // tokens, unclaimed login links — and leaves the SSH keys registered but
-// refused at every entry point until re-enabled.
+// refused at every entry point until re-enabled; connections they opened
+// are closed.
 func (s *Store) SetUserDisabled(userID int64, disabled bool) error {
 	v := 0
 	if disabled {
 				return err
 			}
 		}
+		s.announce(Revoked{UserID: userID})
 	}
 	return err
 }
 	return tx.Commit()
 }
-// RemoveSSHKey removes a key owned by userID and bumps the key epoch.
+// RemoveSSHKey removes a key owned by userID, bumps the key epoch, and
+// announces the revocation.
 func (s *Store) RemoveSSHKey(userID int64, fingerprint string) error {
 	tx, err := s.DB.Begin()
 	if err != nil {
 		return err
 	}
 	defer tx.Rollback()
-	res, err := tx.Exec("DELETE FROM ssh_keys WHERE user_id = ? AND fingerprint = ?", userID, fingerprint)
+	var id int64
+	err = tx.QueryRow("DELETE FROM ssh_keys WHERE user_id = ? AND fingerprint = ? RETURNING id", userID, fingerprint).Scan(&id)
+	if errors.Is(err, sql.ErrNoRows) {
+		return ErrNotFound
+	}
 	if err != nil {
 		return err
 	}
-	if n, _ := res.RowsAffected(); n == 0 {
-		return ErrNotFound
-	}
 	if err := bumpKeyEpoch(tx); err != nil {
 		return err
 	}
-	return tx.Commit()
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+	s.announce(Revoked{KeyIDs: []int64{id}})
+	return nil
 }
 // SetSSHKeyLabel renames a key owned by userID. Labels do not touch the
 		return err
 	}
 	defer tx.Rollback()
-	res, err := tx.Exec(
+	var id int64
-		"DELETE FROM ssh_keys WHERE fingerprint = ? AND scope LIKE 'deploy:' || ? || ':%'",
+	err = tx.QueryRow(
-		fingerprint, repoID)
+		"DELETE FROM ssh_keys WHERE fingerprint = ? AND scope LIKE 'deploy:' || ? || ':%' RETURNING id",
+		fingerprint, repoID).Scan(&id)
+	if errors.Is(err, sql.ErrNoRows) {
+		return ErrNotFound
+	}
 	if err != nil {
 		return err
 	}
-	if n, _ := res.RowsAffected(); n == 0 {
-		return ErrNotFound
-	}
 	if err := bumpKeyEpoch(tx); err != nil {
 		return err
 	}
-	return tx.Commit()
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+	s.announce(Revoked{KeyIDs: []int64{id}})
+	return nil
 }