webhook add reads the secret from stdin; runner docs name real attachments !510

merged merged by cmc on 2026-09-29 00:19 UTC · krz/gitbay:webhook-secret-stdin into main

9 files changed, +105 −23

Layout: unified · split

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

		@@ -150,13 +150,17 @@ Per-repository outbound POSTs for repository events. Managed by repo
150	150	admins:
151	151
152	152	#+begin_src sh
153		gitbay webhook add <url> --secret s3cret [--events push,issue.created] # default *
	153	printf %s "$SECRET" \| gitbay webhook add <url> --secret - [--events push,issue.created] # default *
154	154	gitbay webhook list
155	155	gitbay webhook deliveries [--limit 50] # status, attempts, last error
156	156	gitbay webhook redeliver <delivery-id> # requeue, including dead letters
157	157	gitbay webhook remove <id>
158	158	#+end_src
159	159
	160	The signing secret is read from stdin with =--secret -=; a value on the
	161	command line is refused, since argv shows in process listings and shell
	162	history. Over the JSON API it goes in the request's =stdin= field.
	163
160	164	** Events
161	165
162	166	Every event this forge records, and so every name =--events= may take.

.gitbay/wiki/Admin.org +10 −9

		@@ -925,16 +925,19 @@ at real ones.* Every deploy that switched the whole instance to
925	925	containers and failed took CI down with it. Instead: create a throwaway
926	926	repository the runner account can read (public, or granted read — a
927	927	private one is "not found" to the runner and the build stays pending),
928		give it one job that names the CI image, and deploy the runner with
929		=-repos= naming only that repository. The production unit, with its real
930		hardening, then claims nothing else; other repositories' builds queue
931		until =-repos= is switched back, which is a pause, not an outage.
	928	give it one job that names the CI image, attach the runner's key to it,
	929	and deploy the runner with =-repos= naming only that repository. The
	930	production unit, with its real hardening, then claims nothing else;
	931	other repositories' builds queue until =-repos= is removed again, which
	932	is a pause, not an outage.
932	933
933	934	#+begin_src sh
934		gitbay repo create cmc/ci-smoke # then push a .gitbay/ci.yml naming the image
935		sed -i 's#-repos krz/gitbay #-repos cmc/ci-smoke #' /etc/systemd/system/gitbay-runner.service.d/override.conf
	935	gitbay repo create cmc/runner-scratch # then push a .gitbay/ci.yml naming the image
	936	# on the host:
	937	gitbay repo runner add cmc/runner-scratch < /var/lib/gitbay-runner/.ssh/id_ed25519.pub
	938	sed -i 's#^ExecStart=/usr/local/bin/gitbay-runner #&-repos cmc/runner-scratch #' /etc/systemd/system/gitbay-runner.service.d/override.conf
936	939	systemctl daemon-reload && systemctl restart gitbay-runner
937		gitbay build log cmc/ci-smoke 1 # green: switch -repos back, redeploy
	940	gitbay build log cmc/runner-scratch 1 # green: remove -repos, redeploy, delete the scratch repository
938	941	#+end_src
939	942
940	943	*Do not deploy an isolating runner to a host that has not been
		@@ -950,8 +953,6 @@ management and =ReadWritePaths= for podman's store under
950	953	make read-only. Those paths are prefixed =-= so they are ignored when
951	954	absent: the drop-in installs on unprepared hosts too, and a unit that
952	955	refused to start would stop every build.
953		The nightly canary on =cmc/ci-smoke= only runs if the runner's =-repos=
954		names that repository too; a scoped runner claims nothing else.
955	956	=gitbay-runner-prune.timer= prunes unused images weekly, as the runner's
956	957	user: rootless storage belongs to that user, and root's prune would not
957	958	see it. An unpruned image store on a 40GB host is a slow outage.

CHANGELOG.org +5

		@@ -6,6 +6,11 @@ anything beyond "replace the binary and restart" is needed.
6	6
7	7	* Unreleased
8	8
	9	- =webhook add= reads the signing secret from stdin with =--secret -=;
	10	a value on the command line is refused, since argv shows in process
	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).
9	14	- The builds page's status badge section gives an org-mode snippet
10	15	beside the Markdown one, for a README.org (#299).
11	16	- API tokens on the settings page: create with a scope and optional

cmd/gitbay/main.go +2 −2

		@@ -357,7 +357,7 @@ func isEmptyReader(r io.Reader) bool {
357	357	// usesStdin reports whether the arguments request stdin content.
358	358	func usesStdin(args []string) bool {
359	359	for i, a := range args {
360		if (a == "--file" \|\| a == "--key") && i+1 < len(args) && args[i+1] == "-" {
	360	if (a == "--file" \|\| a == "--key" \|\| a == "--secret") && i+1 < len(args) && args[i+1] == "-" {
361	361	return true
362	362	}
363	363	if a == "--token-stdin" {
		@@ -750,7 +750,7 @@ func webCmd() *cobra.Command {
750	750
751	751	func webhookCmd() *cobra.Command {
752	752	return group("webhook", "outbound event delivery",
753		pass("add", passOpts{server: []string{"webhook", "add"}, needsRepo: true}),
	753	pass("add", passOpts{server: []string{"webhook", "add"}, needsRepo: true, stdinOK: true, stdinWhat: "the webhook secret", stdinSecret: true}),
754	754	pass("list", passOpts{server: []string{"webhook", "list"}, needsRepo: true}),
755	755	pass("remove", passOpts{server: []string{"webhook", "remove"}, needsRepo: true}),
756	756	pass("deliveries", passOpts{server: []string{"webhook", "deliveries"}, needsRepo: true}),

cmd/gitbay/stdinpayload_test.go +11

		@@ -114,3 +114,14 @@ func swapTerminal(f func(*os.File) bool) func() {
114	114	isTerminal = f
115	115	return func() { isTerminal = prev }
116	116	}
	117
	118	// webhook add --secret - reads the secret on the server, so the CLI must
	119	// forward stdin for it the way it does for --file - (#284).
	120	func TestUsesStdinForSecretDash(t *testing.T) {
	121	if !usesStdin([]string{"alice/app", "https://ci.example/hook", "--secret", "-"}) {
	122	t.Error("--secret - does not forward stdin")
	123	}
	124	if usesStdin([]string{"alice/app", "https://ci.example/hook", "--events", "push"}) {
	125	t.Error("forwarded stdin with no flag asking for it")
	126	}
	127	}

deploy/gitbay-runner.override.conf +5 −3

		@@ -34,9 +34,11 @@ After=gitbay-runner-egress.service
34	34	[Service]
35	35	# The runner polls as a non-admin account with a runner-scoped key, and
36	36	# claims only the repositories that key is attached to (`repo runner
37		# add`): krz/gitbay and cmc/ci-smoke. The attachments are the boundary,
38		# so ExecStart names no -repos. cmc/ci-smoke is the nightly isolation
39		# canary; keep it attached or its scheduled build waits forever.
	37	# add`): krz/gitbay, krz/hutch, krz/keycask, krz/orgo, krz/skunky-art
	38	# and cmc/cleberg.net. The attachments are the boundary, so ExecStart
	39	# names no -repos. To validate a runner change, create a scratch
	40	# repository, attach this key to it, and run with -repos naming only
	41	# that repository until the change is proven (Admin wiki, CI runner).
40	42	#
41	43	# Two layers of resource caps. MemoryMax and CPUQuota bound the unit —
42	44	# the runner and every build together — which is what keeps the forge

e2e/webhook_test.go +2 −2

		@@ -112,8 +112,8 @@ func TestWebhooks(t *testing.T) {
112	112
113	113	recv := startHookReceiver(t)
114	114	hookURL := "http://" + recv.addr + "/hook"
115		if _, errOut, code := inst.ssh(t, aliceKey, "",
116		"webhook", "add", "alice/proj", hookURL, "--secret", "s3cret"); code != 0 {
	115	if _, errOut, code := inst.ssh(t, aliceKey, "s3cret\n",
	116	"webhook", "add", "alice/proj", hookURL, "--secret", "-"); code != 0 {
117	117	t.Fatalf("webhook add: %s", errOut)
118	118	}
119	119

internal/control/webhook.go +26 −6

		@@ -5,6 +5,7 @@ import (
5	5	"fmt"
6	6	"io"
7	7	"strconv"
	8	"strings"
8	9
9	10	"gitbay.org/gitbay/internal/policy"
10	11	"gitbay.org/gitbay/internal/protocol"
		@@ -15,13 +16,17 @@ import (
15	16	func init() {
16	17	register(Command{Path: []string{"webhook", "add"},
17	18	Summary: "add a webhook",
18		Usage: "webhook add <owner/name> <url> [--secret <s>] [--events push,issue.created\|*]",
	19	Usage: "webhook add <owner/name> <url> [--secret -] [--events push,issue.created\|*]",
19	20	Flags: []Flag{
20		{"--secret", "<s>", "signs deliveries so the receiver can verify them", ""},
	21	{"--secret", "-", "read the secret that signs deliveries from stdin", ""},
21	22	{"--events", "push,issue.created\|", "which events to send", ""},
22	23	},
23		Examples: []string{"webhook add krz/gitbay https://ci.example.org/hook --events push"},
24		Run: runWebhookAdd})
	24	Examples: []string{
	25	"webhook add krz/gitbay https://ci.example.org/hook --events push",
	26	"webhook add krz/gitbay https://ci.example.org/hook --secret - < secret.txt",
	27	},
	28	ReadsStdin: true,
	29	Run: runWebhookAdd})
25	30	register(Command{Path: []string{"webhook", "list"},
26	31	Summary: "list webhooks",
27	32	Usage: "webhook list <owner/name>",
		@@ -45,17 +50,21 @@ func init() {
45	50	}
46	51
47	52	func runWebhookAdd(c *Ctx, args []string) int {
48		f, err := c.parseArgs(args, flagSpec{Values: []string{"--secret", "--events"}, MaxPos: 2, Usage: "webhook add <owner/name> <url> [--secret <s>] [--events push,issue.created\|*]"})
	53	f, err := c.parseArgs(args, flagSpec{Values: []string{"--secret", "--events"}, MaxPos: 2, Usage: "webhook add <owner/name> <url> [--secret -] [--events push,issue.created\|*]"})
49	54	if err != nil {
50	55	return c.fail(protocol.ExitUsage, "%v", err)
51	56	}
52		path, url, secret, events := f.pos(0), f.pos(1), f.Value("--secret"), "*"
	57	path, url, events := f.pos(0), f.pos(1), "*"
53	58	if f.Has("--events") {
54	59	events = f.Value("--events")
55	60	}
56	61	if path == "" \|\| url == "" {
57	62	return c.usage()
58	63	}
	64	// Secrets travel on stdin: argv shows in /proc and in shell history.
	65	if f.Has("--secret") && f.Value("--secret") != "-" {
	66	return c.fail(protocol.ExitUsage, "the secret is read from stdin, never argv: pipe it and pass --secret - (printf %%s SECRET \| ... --secret -)")
	67	}
59	68	repo, code := resolveRepo(c, path, policy.CanAdmin)
60	69	if code >= 0 {
61	70	return code
		@@ -71,6 +80,17 @@ func runWebhookAdd(c *Ctx, args []string) int {
71	80	// Exit 1 carries the reason to every client verbatim (#187).
72	81	return c.fail(protocol.ExitFailure, "%v", err)
73	82	}
	83	secret := ""
	84	if f.Has("--secret") {
	85	raw, err := io.ReadAll(io.LimitReader(c.Stdin, 64<<10))
	86	if err != nil {
	87	return c.fail(protocol.ExitFailure, "reading secret: %v", err)
	88	}
	89	secret = strings.TrimRight(string(raw), "\n")
	90	if secret == "" {
	91	return c.fail(protocol.ExitUsage, "no secret on stdin (pipe it: printf %%s SECRET \| ... --secret -)")
	92	}
	93	}
74	94	id, err := c.Store.AddWebhook(repo.ID, url, secret, events)
75	95	if err != nil {
76	96	return c.fail(protocol.ExitFailure, "%v", err)

internal/control/webhook_test.go +39

		@@ -39,3 +39,42 @@ func TestWebhookAddRefusedURLIsAFailure(t *testing.T) {
39	39	t.Errorf("missing url: exit %d, want %d", code, protocol.ExitUsage)
40	40	}
41	41	}
	42
	43	// The signing secret arrives on stdin with --secret -, like a build
	44	// secret: a value on the command line is refused before anything is
	45	// stored, since argv shows in /proc and in shell history (#284).
	46	func TestWebhookAddSecretFromStdin(t *testing.T) {
	47	st, repo, uid := newQueueTestRepo(t)
	48	run := func(stdin string, argv ...string) (string, int) {
	49	c, errOut := pruneCtx(st, t.TempDir(), store.User{ID: uid, Username: "alice"})
	50	c.Cfg.Limits.WriteRate = -1
	51	c.Cfg.Webhooks.AllowLocal = true
	52	c.Stdin = strings.NewReader(stdin)
	53	code := Dispatch(c, argv)
	54	return errOut.String(), code
	55	}
	56	msg, code := run("", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "s3cret")
	57	if code != protocol.ExitUsage \|\| !strings.Contains(msg, "--secret -") {
	58	t.Fatalf("literal secret: exit %d, %q", code, msg)
	59	}
	60	if msg, code := run("", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "-"); code != protocol.ExitUsage \|\| !strings.Contains(msg, "no secret on stdin") {
	61	t.Fatalf("empty stdin: exit %d, %q", code, msg)
	62	}
	63	if hooks, err := st.ListWebhooks(repo.ID); err != nil \|\| len(hooks) != 0 {
	64	t.Fatalf("a refused add stored %+v (%v)", hooks, err)
	65	}
	66	if msg, code := run("s3cret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/hook", "--secret", "-"); code != protocol.ExitOK {
	67	t.Fatalf("piped secret: exit %d, %q", code, msg)
	68	}
	69	// Without --secret nothing reads stdin and the hook is unsigned.
	70	if msg, code := run("not a secret\n", "webhook", "add", repo.Path(), "http://127.0.0.1/other"); code != protocol.ExitOK {
	71	t.Fatalf("no secret: exit %d, %q", code, msg)
	72	}
	73	hooks, err := st.ListWebhooks(repo.ID)
	74	if err != nil \|\| len(hooks) != 2 {
	75	t.Fatalf("hooks: %+v %v", hooks, err)
	76	}
	77	if hooks[0].Secret != "s3cret" \|\| hooks[1].Secret != "" {
	78	t.Fatalf("secrets: %q, %q", hooks[0].Secret, hooks[1].Secret)
	79	}
	80	}