Commit `3398f2216b`

3398f2216b15a2f2c365ddcdf8ecd3e80af93a76

parent: 57707aebf9

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-09 03:48 UTC

control: the runner bypass is the key, not the account

A scope-runner key is confined to its attachments whoever owns it, so a
runner key on an instance admin's account no longer claims every
repository's builds and secrets. runnerAdmin replaces the c.User.IsAdmin
checks in runnerMayBuild and runRunnerNext.

admin runners renders a runner key's attachments in the scope column
whatever -repos it polled with, and none when it has no attachment; any
other key keeps what it asked for, or any.

repo runner add refuses a key whose account cannot read the repository:
the runner clones what it builds, and the claim carries the repository's
secrets.

Ref #184

internal/control/admin.go +9 −5

		@@ -459,19 +459,23 @@ func runAdminRunners(c *Ctx, args []string) int {
459	459	if runners == nil {
460	460	runners = []store.Runner{}
461	461	}
	462	// The scope column is what the key may claim, not what it asked for. A
	463	// runner key is confined to its attachments, so they replace whatever
	464	// -repos it polled with, and none of them means none. Any other key
	465	// keeps the repositories it asked for, or the whole instance.
462	466	for i := range runners {
463		if runners[i].Scope != "" {
464		continue
465		}
466	467	key, err := c.Store.SSHKeyByID(runners[i].KeyID)
467	468	if err != nil \|\| key.Scope != "runner" {
468		continue // an admin key with no -repos: any
	469	continue
469	470	}
470	471	paths, err := c.Store.RunnerRepoPaths(runners[i].KeyID)
471	472	if err != nil {
472	473	return c.fail(protocol.ExitFailure, "%v", err)
473	474	}
474		runners[i].Scope = strings.Join(paths, ",")
	475	runners[i].Scope = "none"
	476	if len(paths) > 0 {
	477	runners[i].Scope = strings.Join(paths, ",")
	478	}
475	479	}
476	480	d := map[string]any{"queue": queue, "runners": runners}
477	481	return c.emit(d, func(w io.Writer) {

internal/control/build.go +10 −3

		@@ -324,11 +324,18 @@ func runnerSession(c *Ctx) (store.SSHKey, int) {
324	324	return key, -1
325	325	}
326	326
	327	// runnerAdmin reports whether a session claims builds instance-wide. The
	328	// bypass is the key, not the account: a scope-runner key is confined to
	329	// its attachments whoever owns it, including an instance admin.
	330	func runnerAdmin(c *Ctx) bool {
	331	return c.User.IsAdmin && c.Scope != "runner"
	332	}
	333
327	334	// runnerMayBuild reports whether a runner session may act on a
328		// repository's builds: an admin user may on any, a runner key on the
	335	// repository's builds: an admin key may on any, a runner key on the
329	336	// repositories it is attached to (#184).
330	337	func runnerMayBuild(c *Ctx, key store.SSHKey, repoID int64) (bool, error) {
331		if c.User.IsAdmin {
	338	if runnerAdmin(c) {
332	339	return true, nil
333	340	}
334	341	return c.Store.RunnerAttached(key.ID, repoID)
		@@ -373,7 +380,7 @@ func runRunnerNext(c *Ctx, args []string) int {
373	380	}
374	381	repoIDs = append(repoIDs, repo.ID)
375	382	}
376		if !c.User.IsAdmin && len(repoIDs) == 0 {
	383	if !runnerAdmin(c) && len(repoIDs) == 0 {
377	384	repoIDs, err = c.Store.RunnerRepoIDs(key.ID)
378	385	if err != nil {
379	386	return c.fail(protocol.ExitFailure, "%v", err)

internal/control/runnerattach_test.go +42 −4

		@@ -118,22 +118,60 @@ func TestRunnerNextAttachedClaimsOwnRepoOnly(t *testing.T) {
118	118	}
119	119
120	120	// The heartbeat is recorded against the key, and admin runners shows it
121		// with its fingerprint and attachments.
	121	// with its fingerprint and attachments. The column is the attachments even
	122	// when the key polled with a narrower -repos, and none when it has no
	123	// attachment at all.
122	124	func TestAdminRunnersShowsKeyAndAttachments(t *testing.T) {
123	125	f := newAttachFixture(t)
124		if err := f.st.AttachRunner(f.aliceKey.ID, f.app.ID); err != nil {
125		t.Fatal(err)
	126	for _, id := range []int64{f.app.ID, f.evil.ID} {
	127	if err := f.st.AttachRunner(f.aliceKey.ID, id); err != nil {
	128	t.Fatal(err)
	129	}
126	130	}
127	131	c, _ := f.ctx(f.alice, f.aliceKey, false)
	132	runRunnerNext(c, []string{"alice/app"})
	133	c, _ = f.ctx(f.mallory, f.malloryKey, false)
128	134	runRunnerNext(c, nil)
129	135	admin, out := f.ctx(f.alice, f.aliceKey, true)
130	136	admin.Scope = "full"
131	137	if code := runAdminRunners(admin, nil); code != protocol.ExitOK {
132	138	t.Fatalf("admin runners: exit %d: %s", code, out.String())
133	139	}
134		if !strings.Contains(out.String(), "alice\tSHA256:alice\t") \|\| !strings.Contains(out.String(), "\talice/app\t") {
	140	if !strings.Contains(out.String(), "alice\tSHA256:alice\t") \|\|
	141	!strings.Contains(out.String(), "\talice/app,mallory/evil\t") {
135	142	t.Fatalf("row lacks fingerprint or attachments:\n%s", out.String())
136	143	}
	144	if !strings.Contains(out.String(), "\tnone\t") {
	145	t.Fatalf("mallory's unattached runner key is not none:\n%s", out.String())
	146	}
	147	}
	148
	149	// The instance-admin bypass is the key, not the account: a runner-scoped
	150	// key on an admin account claims only what it is attached to.
	151	func TestRunnerNextAdminAccountRunnerKeyIsConfined(t *testing.T) {
	152	f := newAttachFixture(t)
	153	c, out := f.ctx(f.alice, f.aliceKey, true)
	154	if code := runRunnerNext(c, nil); code != protocol.ExitOK \|\| !strings.Contains(out.String(), "no pending builds") {
	155	t.Fatalf("exit %d: %s", code, out.String())
	156	}
	157	for _, b := range []struct {
	158	repo store.Repo
	159	number int64
	160	}{{f.app, f.appBuild}, {f.evil, f.evilBuild}} {
	161	if got, _ := f.st.BuildByNumber(b.repo.ID, b.number); got.Status != "pending" {
	162	t.Fatalf("%s claimed by an unattached runner key: %s", b.repo.Path(), got.Status)
	163	}
	164	}
	165	if err := f.st.AttachRunner(f.aliceKey.ID, f.app.ID); err != nil {
	166	t.Fatal(err)
	167	}
	168	c, out = f.ctx(f.alice, f.aliceKey, true)
	169	if code := runRunnerNext(c, nil); code != protocol.ExitOK \|\| !strings.Contains(out.String(), "alice/app") {
	170	t.Fatalf("attached claim: exit %d: %s", code, out.String())
	171	}
	172	if got, _ := f.st.BuildByNumber(f.evil.ID, f.evilBuild); got.Status != "pending" {
	173	t.Fatalf("mallory's build was claimed: %s", got.Status)
	174	}
137	175	}
138	176
139	177	// Untrusted builds are skipped unless the runner asks.

internal/control/runnerrepo.go +16

		@@ -66,6 +66,22 @@ func runRepoRunnerAdd(c *Ctx, args []string) int {
66	66	case key.UserID != c.User.ID && !c.User.IsAdmin:
67	67	return c.fail(protocol.ExitDenied, "%s belongs to another account", fp)
68	68	}
	69	// The runner clones what it builds, so the key's account must be able
	70	// to read the repository. The caller's own key needs no check: they
	71	// hold admin on the repository to get here.
	72	if key.UserID != c.User.ID {
	73	owner, err := c.Store.UserByID(key.UserID)
	74	if err != nil {
	75	return c.fail(protocol.ExitFailure, "%v", err)
	76	}
	77	grant, err := c.Store.AccessRole(repo.ID, owner.ID)
	78	if err != nil {
	79	return c.fail(protocol.ExitFailure, "%v", err)
	80	}
	81	if !policy.CanRead(owner, repo, grant) {
	82	return c.fail(protocol.ExitDenied, "%s belongs to %s, who cannot read %s", fp, owner.Username, repo.Path())
	83	}
	84	}
69	85	if err := c.Store.AttachRunner(key.ID, repo.ID); err != nil {
70	86	return c.fail(protocol.ExitFailure, "%v", err)
71	87	}

internal/control/runnerrepo_test.go +17

		@@ -104,4 +104,21 @@ func TestRepoRunnerAddRefusesWrongKeys(t *testing.T) {
104	104	if code := runRepoRunnerAdd(c, []string{repo.Path()}); code != protocol.ExitOK {
105	105	t.Fatalf("admin could not attach another account's runner key: exit %d %s", code, out.String())
106	106	}
	107	// The runner clones what it builds: bob cannot read alice's private
	108	// repository, so not even an admin may attach his key to it.
	109	secretID, err := st.CreateRepo("user", uid, "secret", "private")
	110	if err != nil {
	111	t.Fatal(err)
	112	}
	113	secret, err := st.RepoByID(secretID)
	114	if err != nil {
	115	t.Fatal(err)
	116	}
	117	c, out = repoRunnerCtx(t, st, uid, true, testRunnerPub)
	118	if code := runRepoRunnerAdd(c, []string{secret.Path()}); code != protocol.ExitDenied {
	119	t.Fatalf("key attached to a repo its account cannot read: exit %d %s", code, out.String())
	120	}
	121	if runners, _ := st.ListRepoRunners(secret.ID); len(runners) != 0 {
	122	t.Fatalf("attached anyway: %+v", runners)
	123	}
107	124	}

Commit 3398f2216b

Commit `3398f2216b`