Commit e2caeed16c
e2caeed16caa25dd29de6005138def60eec8abfb
parent: c657efe681
Verified · cmc ci/build: success ci/test: failure
cmc <hello@cleberg.net> · 2026-09-07 18:58 UTC
control: a refused webhook URL is a failure, not a usage error
webhook add handed every ValidateURL error to failErr, whose fallback is
exit 2. The refusals the server decides after parsing — an unresolvable
host, a private address — now answer exit 1, so every client shows the
reason instead of a generic error. The command line's shape is still
exit 2.
Closes #187
internal/control/webhook.go
+3 −1
| @@ -53,7 +53,9 @@ func runWebhookAdd(c *Ctx, args []string) int { |
| 53 | 53 | return code |
| 54 | 54 | } |
| 55 | 55 | if err := webhook.ValidateURL(url, c.Cfg.Webhooks.AllowLocal); err != nil { |
| 56 | | return c.failErr(err) |
| 56 | // The command line parsed; the value is what the server refuses. |
| 57 | // Exit 1 carries the reason to every client verbatim (#187). |
| 58 | return c.fail(protocol.ExitFailure, "%v", err) |
| 57 | 59 | } |
| 58 | 60 | id, err := c.Store.AddWebhook(repo.ID, url, secret, events) |
| 59 | 61 | if err != nil { |
internal/control/webhook_test.go
added
+41
| @@ -0,0 +1,41 @@ |
| 1 | package control |
| 2 | |
| 3 | import ( |
| 4 | "bytes" |
| 5 | "strings" |
| 6 | "testing" |
| 7 | |
| 8 | "gitbay.org/gitbay/internal/protocol" |
| 9 | "gitbay.org/gitbay/internal/store" |
| 10 | ) |
| 11 | |
| 12 | // TestWebhookAddRefusedURLIsAFailure: a URL the server refuses after |
| 13 | // parsing it — here one that resolves to a loopback address — is the |
| 14 | // caller's value being rejected, not a malformed command line. Exit 1 |
| 15 | // carries the sentence verbatim to every client; exit 2 reads as an |
| 16 | // app bug and is shown as a generic error (#187). |
| 17 | func TestWebhookAddRefusedURLIsAFailure(t *testing.T) { |
| 18 | st, repo, uid := newQueueTestRepo(t) |
| 19 | var out, errOut bytes.Buffer |
| 20 | c := &Ctx{ |
| 21 | User: store.User{ID: uid, Username: "alice"}, |
| 22 | Store: st, |
| 23 | Scope: "full", |
| 24 | Stdin: strings.NewReader(""), |
| 25 | Stdout: &out, |
| 26 | Stderr: &errOut, |
| 27 | } |
| 28 | code := Dispatch(c, []string{"webhook", "add", repo.Path(), "http://127.0.0.1/hook"}) |
| 29 | if code != protocol.ExitFailure { |
| 30 | t.Fatalf("exit %d, want %d (%s)", code, protocol.ExitFailure, strings.TrimSpace(errOut.String())) |
| 31 | } |
| 32 | if !strings.Contains(errOut.String(), "private or local address") { |
| 33 | t.Errorf("message %q lacks the server's reason", strings.TrimSpace(errOut.String())) |
| 34 | } |
| 35 | // The shape of the command line is still a usage error. |
| 36 | out.Reset() |
| 37 | errOut.Reset() |
| 38 | if code := Dispatch(c, []string{"webhook", "add", repo.Path()}); code != protocol.ExitUsage { |
| 39 | t.Errorf("missing url: exit %d, want %d", code, protocol.ExitUsage) |
| 40 | } |
| 41 | } |