!510 · krz/gitbay

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

 admins:
 #+begin_src sh
-gitbay webhook add <url> --secret s3cret [--events push,issue.created]  # default *
+printf %s "$SECRET" | gitbay webhook add <url> --secret - [--events push,issue.created]  # default *
 gitbay webhook list
 gitbay webhook deliveries [--limit 50]     # status, attempts, last error
 gitbay webhook redeliver <delivery-id>     # requeue, including dead letters
 gitbay webhook remove <id>
 #+end_src
+The signing secret is read from stdin with =--secret -=; a value on the
+command line is refused, since argv shows in process listings and shell
+history. Over the JSON API it goes in the request's =stdin= field.
 ** Events
 Every event this forge records, and so every name =--events= may take.

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

 containers and failed took CI down with it. Instead: create a throwaway
 repository the runner account can read (public, or granted read — a
 private one is "not found" to the runner and the build stays pending),
-give it one job that names the CI image, and deploy the runner with
+give it one job that names the CI image, attach the runner's key to it,
-=-repos= naming only that repository. The production unit, with its real
+and deploy the runner with =-repos= naming only that repository. The
-hardening, then claims nothing else; other repositories' builds queue
+production unit, with its real hardening, then claims nothing else;
-until =-repos= is switched back, which is a pause, not an outage.
+other repositories' builds queue until =-repos= is removed again, which
+is a pause, not an outage.
 #+begin_src sh
-gitbay repo create cmc/ci-smoke          # then push a .gitbay/ci.yml naming the image
+gitbay repo create cmc/runner-scratch   # then push a .gitbay/ci.yml naming the image
-sed -i 's#-repos krz/gitbay #-repos cmc/ci-smoke #' /etc/systemd/system/gitbay-runner.service.d/override.conf
+# 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
-gitbay build log cmc/ci-smoke 1         # green: switch -repos back, redeploy
+gitbay build log cmc/runner-scratch 1   # green: remove -repos, redeploy, delete the scratch repository
 #+end_src
 *Do not deploy an isolating runner to a host that has not been
 make read-only. Those paths are prefixed =-= so they are ignored when
 absent: the drop-in installs on unprepared hosts too, and a unit that
 refused to start would stop every build.
-The nightly canary on =cmc/ci-smoke= only runs if the runner's =-repos=
-names that repository too; a scoped runner claims nothing else.
 =gitbay-runner-prune.timer= prunes unused images weekly, as the runner's
 user: rootless storage belongs to that user, and root's prune would not
 see it. An unpruned image store on a 40GB host is a slow outage.

CHANGELOG.org +5

 * Unreleased
+- =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 -=
+  (#284).
 - 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

cmd/gitbay/main.go +2 −2

	@@ -357,7 +357,7 @@ func isEmptyReader(r io.Reader) bool {
357	// usesStdin reports whether the arguments request stdin content.	357	// usesStdin reports whether the arguments request stdin content.
358	func usesStdin(args []string) bool {	358	func usesStdin(args []string) bool {
359	for i, a := range args {	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	return true	361	return true
362	}	362	}
363	if a == "--token-stdin" {	363	if a == "--token-stdin" {
	@@ -750,7 +750,7 @@ func webCmd() *cobra.Command {
750		750
751	func webhookCmd() *cobra.Command {	751	func webhookCmd() *cobra.Command {
752	return group("webhook", "outbound event delivery",	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	pass("list", passOpts{server: []string{"webhook", "list"}, needsRepo: true}),	754	pass("list", passOpts{server: []string{"webhook", "list"}, needsRepo: true}),
755	pass("remove", passOpts{server: []string{"webhook", "remove"}, needsRepo: true}),	755	pass("remove", passOpts{server: []string{"webhook", "remove"}, needsRepo: true}),
756	pass("deliveries", passOpts{server: []string{"webhook", "deliveries"}, needsRepo: true}),	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	isTerminal = f	114	isTerminal = f
115	return func() { isTerminal = prev }	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

 [Service]
 # The runner polls as a non-admin account with a runner-scoped key, and
 # claims only the repositories that key is attached to (`repo runner
-# add`): krz/gitbay and cmc/ci-smoke. The attachments are the boundary,
+# add`): krz/gitbay, krz/hutch, krz/keycask, krz/orgo, krz/skunky-art
-# so ExecStart names no -repos. cmc/ci-smoke is the nightly isolation
+# and cmc/cleberg.net. The attachments are the boundary, so ExecStart
-# canary; keep it attached or its scheduled build waits forever.
+# names no -repos. To validate a runner change, create a scratch
+# repository, attach this key to it, and run with -repos naming only
+# that repository until the change is proven (Admin wiki, CI runner).
 #
 # Two layers of resource caps. MemoryMax and CPUQuota bound the unit —
 # 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	recv := startHookReceiver(t)	113	recv := startHookReceiver(t)
114	hookURL := "http://" + recv.addr + "/hook"	114	hookURL := "http://" + recv.addr + "/hook"
115	if _, errOut, code := inst.ssh(t, aliceKey, "",	115	if _, errOut, code := inst.ssh(t, aliceKey, "s3cret\n",
116	"webhook", "add", "alice/proj", hookURL, "--secret", "s3cret"); code != 0 {	116	"webhook", "add", "alice/proj", hookURL, "--secret", "-"); code != 0 {
117	t.Fatalf("webhook add: %s", errOut)	117	t.Fatalf("webhook add: %s", errOut)
118	}	118	}
119		119

internal/control/webhook.go +26 −6

 	"fmt"
 	"io"
 	"strconv"
+	"strings"
 	"gitbay.org/gitbay/internal/policy"
 	"gitbay.org/gitbay/internal/protocol"
 func init() {
 	register(Command{Path: []string{"webhook", "add"},
 		Summary: "add a webhook",
-		Usage:   "webhook add <owner/name> <url> [--secret <s>] [--events push,issue.created|*]",
+		Usage:   "webhook add <owner/name> <url> [--secret -] [--events push,issue.created|*]",
 		Flags: []Flag{
-			{"--secret", "<s>", "signs deliveries so the receiver can verify them", ""},
+			{"--secret", "-", "read the secret that signs deliveries from stdin", ""},
 			{"--events", "push,issue.created|*", "which events to send", "*"},
 		},
-		Examples: []string{"webhook add krz/gitbay https://ci.example.org/hook --events push"},
+		Examples: []string{
-		Run:      runWebhookAdd})
+			"webhook add krz/gitbay https://ci.example.org/hook --events push",
+			"webhook add krz/gitbay https://ci.example.org/hook --secret - < secret.txt",
+		},
+		ReadsStdin: true,
+		Run:        runWebhookAdd})
 	register(Command{Path: []string{"webhook", "list"},
 		Summary:  "list webhooks",
 		Usage:    "webhook list <owner/name>",
 }
 func runWebhookAdd(c *Ctx, args []string) int {
-	f, err := c.parseArgs(args, flagSpec{Values: []string{"--secret", "--events"}, MaxPos: 2, Usage: "webhook add <owner/name> <url> [--secret <s>] [--events push,issue.created|*]"})
+	f, err := c.parseArgs(args, flagSpec{Values: []string{"--secret", "--events"}, MaxPos: 2, Usage: "webhook add <owner/name> <url> [--secret -] [--events push,issue.created|*]"})
 	if err != nil {
 		return c.fail(protocol.ExitUsage, "%v", err)
 	}
-	path, url, secret, events := f.pos(0), f.pos(1), f.Value("--secret"), "*"
+	path, url, events := f.pos(0), f.pos(1), "*"
 	if f.Has("--events") {
 		events = f.Value("--events")
 	}
 	if path == "" || url == "" {
 		return c.usage()
 	}
+	// Secrets travel on stdin: argv shows in /proc and in shell history.
+	if f.Has("--secret") && f.Value("--secret") != "-" {
+		return c.fail(protocol.ExitUsage, "the secret is read from stdin, never argv: pipe it and pass --secret - (printf %%s SECRET | ... --secret -)")
+	}
 	repo, code := resolveRepo(c, path, policy.CanAdmin)
 	if code >= 0 {
 		return code
 		// Exit 1 carries the reason to every client verbatim (#187).
 		return c.fail(protocol.ExitFailure, "%v", err)
 	}
+	secret := ""
+	if f.Has("--secret") {
+		raw, err := io.ReadAll(io.LimitReader(c.Stdin, 64<<10))
+		if err != nil {
+			return c.fail(protocol.ExitFailure, "reading secret: %v", err)
+		}
+		secret = strings.TrimRight(string(raw), "\n")
+		if secret == "" {
+			return c.fail(protocol.ExitUsage, "no secret on stdin (pipe it: printf %%s SECRET | ... --secret -)")
+		}
+	}
 	id, err := c.Store.AddWebhook(repo.ID, url, secret, events)
 	if err != nil {
 		return c.fail(protocol.ExitFailure, "%v", err)

internal/control/webhook_test.go +39

	@@ -39,3 +39,42 @@ func TestWebhookAddRefusedURLIsAFailure(t *testing.T) {
39	t.Errorf("missing url: exit %d, want %d", code, protocol.ExitUsage)	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	}

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