Commit 8dcfa45a8a

8dcfa45a8ac03a5ff9c36828274d05acadcf846c

parent: 8ab6240513

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-29 00:10 UTC

webhook: add reads the signing secret from stdin

--secret - reads it; a value on the command line is refused.

Ref #284

Layout: unified · split

CHANGELOG.org +2
@@ -6,6 +6,8 @@ anything beyond "replace the binary and restart" is needed.
6 6
7* Unreleased 7* Unreleased
8 8
9- ~webhook add~'s ~--secret~ now reads the signing secret from stdin
10 (~--secret -~) instead of taking it as a command-line value (#284).
9- The builds page's status badge section gives an org-mode snippet 11- The builds page's status badge section gives an org-mode snippet
10 beside the Markdown one, for a README.org (#299). 12 beside the Markdown one, for a README.org (#299).
11- API tokens on the settings page: create with a scope and optional 13- API tokens on the settings page: create with a scope and optional
internal/control/webhook.go +26 −6
@@ -5,6 +5,7 @@ import (
5 "fmt" 5 "fmt"
6 "io" 6 "io"
7 "strconv" 7 "strconv"
8 "strings"
8 9
9 "gitbay.org/gitbay/internal/policy" 10 "gitbay.org/gitbay/internal/policy"
10 "gitbay.org/gitbay/internal/protocol" 11 "gitbay.org/gitbay/internal/protocol"
@@ -15,13 +16,17 @@ import (
15func init() { 16func init() {
16 register(Command{Path: []string{"webhook", "add"}, 17 register(Command{Path: []string{"webhook", "add"},
17 Summary: "add a webhook", 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 Flags: []Flag{ 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 {"--events", "push,issue.created|*", "which events to send", "*"}, 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 Examples: []string{
24 Run: runWebhookAdd}) 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 register(Command{Path: []string{"webhook", "list"}, 30 register(Command{Path: []string{"webhook", "list"},
26 Summary: "list webhooks", 31 Summary: "list webhooks",
27 Usage: "webhook list <owner/name>", 32 Usage: "webhook list <owner/name>",
@@ -45,17 +50,21 @@ func init() {
45} 50}
46 51
47func runWebhookAdd(c *Ctx, args []string) int { 52func 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 if err != nil { 54 if err != nil {
50 return c.fail(protocol.ExitUsage, "%v", err) 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 if f.Has("--events") { 58 if f.Has("--events") {
54 events = f.Value("--events") 59 events = f.Value("--events")
55 } 60 }
56 if path == "" || url == "" { 61 if path == "" || url == "" {
57 return c.usage() 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 repo, code := resolveRepo(c, path, policy.CanAdmin) 68 repo, code := resolveRepo(c, path, policy.CanAdmin)
60 if code >= 0 { 69 if code >= 0 {
61 return code 70 return code
@@ -71,6 +80,17 @@ func runWebhookAdd(c *Ctx, args []string) int {
71 // Exit 1 carries the reason to every client verbatim (#187). 80 // Exit 1 carries the reason to every client verbatim (#187).
72 return c.fail(protocol.ExitFailure, "%v", err) 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 id, err := c.Store.AddWebhook(repo.ID, url, secret, events) 94 id, err := c.Store.AddWebhook(repo.ID, url, secret, events)
75 if err != nil { 95 if err != nil {
76 return c.fail(protocol.ExitFailure, "%v", err) 96 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).
46func 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}