diff --git a/README.md b/README.md index 59d004bf..5d628a1b 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,16 @@ command = "notify-send 'Review done for {repo_name} ({sha})'" Template variables: `{job_id}`, `{repo}`, `{repo_name}`, `{sha}`, `{verdict}`, `{error}`. +For generic JSON webhook endpoints, use the built-in `webhook` hook type to +POST the review event directly: + +```toml +[[hooks]] +event = "review.completed" +type = "webhook" +url = "https://example.com/roborev-webhook" +``` + ### Beads Integration The built-in `beads` hook type creates [beads](https://github.com/steveyegge/beads) issues diff --git a/cmd/roborev/stream.go b/cmd/roborev/stream.go index bb69617a..a6097c74 100644 --- a/cmd/roborev/stream.go +++ b/cmd/roborev/stream.go @@ -82,8 +82,11 @@ Examples: return fmt.Errorf("stream failed: %s", body) } - // Stream events - pass through lines directly to preserve all fields + // Stream events - pass through lines directly to preserve all fields. + // Use a 1MB buffer because review.completed events can include + // large findings payloads that exceed the default 64KB limit. scanner := bufio.NewScanner(resp.Body) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) for scanner.Scan() { if ctx.Err() != nil { return nil diff --git a/internal/config/config.go b/internal/config/config.go index 6e641d7c..257d466b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,9 +38,10 @@ func IsConfigParseError(err error) bool { // HookConfig defines a hook that runs on review events type HookConfig struct { - Event string `toml:"event"` // "review.failed", "review.completed", "review.*" - Command string `toml:"command"` // shell command with {var} templates - Type string `toml:"type"` // "beads" for built-in, empty for command + Event string `toml:"event"` // "review.failed", "review.completed", "review.*" + Command string `toml:"command"` // shell command with {var} templates + Type string `toml:"type"` // "beads" or "webhook"; empty or "command" runs Command + URL string `toml:"url" sensitive:"true"` // webhook destination URL when Type is "webhook" } type AdvancedConfig struct { diff --git a/internal/config/keyval.go b/internal/config/keyval.go index f3f30b4f..7c2bad90 100644 --- a/internal/config/keyval.go +++ b/internal/config/keyval.go @@ -44,6 +44,12 @@ func structType(t reflect.Type) (reflect.Type, bool) { if t.Kind() == reflect.Ptr { t = t.Elem() } + if t.Kind() == reflect.Slice { + t = t.Elem() + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + } return t, t.Kind() == reflect.Struct } diff --git a/internal/config/keyval_test.go b/internal/config/keyval_test.go index bca23932..b6ca6205 100644 --- a/internal/config/keyval_test.go +++ b/internal/config/keyval_test.go @@ -625,6 +625,9 @@ func TestIsSensitiveKey(t *testing.T) { if IsSensitiveKey("ci.github_app_id") { t.Error("expected ci.github_app_id to not be sensitive") } + if !IsSensitiveKey("hooks.url") { + t.Error("expected hooks.url to be sensitive") + } } func TestIsGlobalKey(t *testing.T) { diff --git a/internal/daemon/broadcaster.go b/internal/daemon/broadcaster.go index 724c5ea8..ff88162a 100644 --- a/internal/daemon/broadcaster.go +++ b/internal/daemon/broadcaster.go @@ -120,6 +120,7 @@ func (e Event) MarshalJSON() ([]byte, error) { SHA string `json:"sha"` Agent string `json:"agent,omitempty"` Verdict string `json:"verdict,omitempty"` + Findings string `json:"findings,omitempty"` Error string `json:"error,omitempty"` }{ Type: e.Type, @@ -130,6 +131,7 @@ func (e Event) MarshalJSON() ([]byte, error) { SHA: e.SHA, Agent: e.Agent, Verdict: e.Verdict, + Findings: e.Findings, Error: e.Error, }) } diff --git a/internal/daemon/event_test.go b/internal/daemon/event_test.go index c8ef2d00..2638fdef 100644 --- a/internal/daemon/event_test.go +++ b/internal/daemon/event_test.go @@ -16,6 +16,7 @@ func TestEvent_MarshalJSON(t *testing.T) { SHA: "abc123", Agent: "claude-code", Verdict: "F", + Findings: "Missing input validation", } data, err := event.MarshalJSON() @@ -40,6 +41,7 @@ func TestEvent_MarshalJSON(t *testing.T) { {"sha", "abc123"}, {"agent", "claude-code"}, {"verdict", "F"}, + {"findings", "Missing input validation"}, } if len(decoded) != len(tests) { diff --git a/internal/daemon/hooks.go b/internal/daemon/hooks.go index ad70877c..87964577 100644 --- a/internal/daemon/hooks.go +++ b/internal/daemon/hooks.go @@ -1,13 +1,20 @@ package daemon import ( + "bytes" + "encoding/json" + "errors" "fmt" + "io" "log" + "net/http" + neturl "net/url" "os/exec" "path/filepath" "runtime" "strings" "sync" + "time" "github.com/roborev-dev/roborev/internal/config" gitpkg "github.com/roborev-dev/roborev/internal/git" @@ -144,6 +151,17 @@ func (hr *HookRunner) handleEvent(event Event) { continue } + if hook.Type == "webhook" { + if hook.URL == "" { + continue + } + + fired++ + hr.wg.Add(1) + go hr.postWebhook(hook.URL, event) + continue + } + cmd := resolveCommand(hook, event) if cmd == "" { continue @@ -270,3 +288,67 @@ func (hr *HookRunner) runHook(command, workDir string) { hr.logger.Printf("Hook output (cmd=%q): %s", command, output) } } + +func (hr *HookRunner) postWebhook(webhookURL string, event Event) { + defer hr.wg.Done() + safeURL := redactWebhookURL(webhookURL) + + payload, err := json.Marshal(event) + if err != nil { + hr.logger.Printf("Webhook error (url=%q): marshal event: %v", safeURL, err) + return + } + + req, err := http.NewRequest(http.MethodPost, webhookURL, bytes.NewReader(payload)) + if err != nil { + hr.logger.Printf("Webhook error (url=%q): build request: %v", safeURL, redactURLError(err)) + return + } + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Do(req) + if err != nil { + hr.logger.Printf("Webhook error (url=%q): %v", safeURL, redactURLError(err)) + return + } + defer resp.Body.Close() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + if len(body) > 0 { + hr.logger.Printf("Webhook error (url=%q): status %s: %s", safeURL, resp.Status, strings.TrimSpace(string(body))) + return + } + hr.logger.Printf("Webhook error (url=%q): status %s", safeURL, resp.Status) + } +} + +func redactWebhookURL(raw string) string { + parsed, err := neturl.Parse(raw) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return "" + } + + redacted := &neturl.URL{ + Scheme: parsed.Scheme, + Host: parsed.Host, + } + + if p := parsed.EscapedPath(); p != "" && p != "/" { + redacted.Path = "/..." + } + + return redacted.String() +} + +// redactURLError unwraps *url.Error to return only its inner +// error, preventing Go's HTTP client from leaking the raw URL +// (including secret path segments) in log output. +func redactURLError(err error) error { + var ue *neturl.Error + if errors.As(err, &ue) { + return ue.Err + } + return err +} diff --git a/internal/daemon/hooks_test.go b/internal/daemon/hooks_test.go index 931dad6e..0e3f479d 100644 --- a/internal/daemon/hooks_test.go +++ b/internal/daemon/hooks_test.go @@ -2,8 +2,12 @@ package daemon import ( "bytes" + "encoding/json" "fmt" "log" + "net/http" + "net/http/httptest" + neturl "net/url" "os" "path/filepath" "runtime" @@ -512,6 +516,185 @@ func TestHookRunnerNoMatchDoesNotFire(t *testing.T) { } } +func TestHookRunnerWebhookPostsEventJSON(t *testing.T) { + type webhookRequest struct { + header http.Header + event map[string]any + } + + reqCh := make(chan webhookRequest, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + + var payload map[string]any + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode webhook payload: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + + reqCh <- webhookRequest{header: r.Header.Clone(), event: payload} + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + cfg := &config.Config{ + Hooks: []config.HookConfig{ + {Event: "review.completed", Type: "webhook", URL: server.URL}, + }, + } + + hr, broadcaster := setupRunner(t, cfg) + broadcaster.Broadcast(Event{ + Type: "review.completed", + TS: time.Date(2026, 2, 25, 14, 30, 0, 0, time.UTC), + JobID: 42, + Repo: "/Users/wesm/code/roborev", + RepoName: "roborev", + SHA: "abc123def456", + Agent: "claude-code", + Verdict: "F", + Findings: "Missing validation in handler", + }) + + hr.WaitUntilIdle() + + select { + case req := <-reqCh: + if got := req.header.Get("Content-Type"); got != "application/json" { + t.Fatalf("content-type = %q, want application/json", got) + } + if got := req.event["type"]; got != "review.completed" { + t.Fatalf("type = %v, want review.completed", got) + } + if got := req.event["ts"]; got != "2026-02-25T14:30:00Z" { + t.Fatalf("ts = %v, want 2026-02-25T14:30:00Z", got) + } + if got := req.event["job_id"]; got != float64(42) { + t.Fatalf("job_id = %v, want 42", got) + } + if got := req.event["findings"]; got != "Missing validation in handler" { + t.Fatalf("findings = %v, want webhook payload to include findings", got) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for webhook request") + } +} + +func TestHookRunnerWebhookLogsHTTPError(t *testing.T) { + var buf bytes.Buffer + logger := log.New(&buf, "", 0) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusBadGateway) + })) + defer server.Close() + + webhookURL, err := neturl.Parse(server.URL) + if err != nil { + t.Fatalf("parse server URL: %v", err) + } + webhookURL.User = neturl.UserPassword("token", "secret") + webhookURL.Path = "/services/team/webhook" + webhookURL.RawQuery = "api_key=12345" + webhookURL.Fragment = "frag" + + cfg := &config.Config{ + Hooks: []config.HookConfig{ + {Event: "review.failed", Type: "webhook", URL: webhookURL.String()}, + }, + } + + hr := &HookRunner{cfgGetter: NewStaticConfig(cfg), logger: logger} + hr.handleEvent(Event{ + Type: "review.failed", + JobID: 7, + Repo: t.TempDir(), + Error: "agent timeout", + }) + + hr.wg.Wait() + + logOutput := buf.String() + if !strings.Contains(logOutput, "Webhook error") { + t.Fatalf("expected webhook error log, got %q", logOutput) + } + if !strings.Contains(logOutput, "502 Bad Gateway") { + t.Fatalf("expected HTTP status in log, got %q", logOutput) + } + if strings.Contains(logOutput, "token") || strings.Contains(logOutput, "secret") { + t.Fatalf("expected credentials to be redacted from log, got %q", logOutput) + } + if strings.Contains(logOutput, "api_key") || strings.Contains(logOutput, "12345") || strings.Contains(logOutput, "frag") { + t.Fatalf("expected query string and fragment to be redacted from log, got %q", logOutput) + } + if !strings.Contains(logOutput, "/...") { + t.Fatalf("expected redacted path in log, got %q", logOutput) + } + if strings.Contains(logOutput, "/services") { + t.Fatalf("expected path segments to be fully redacted, got %q", logOutput) + } +} + +func TestRedactWebhookURL(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "userinfo query and fragment are removed", + in: "https://token:secret@example.com/services/team/webhook?api_key=123#frag", + want: "https://example.com/...", + }, + { + name: "single path segment is fully redacted", + in: "https://example.com/webhook", + want: "https://example.com/...", + }, + { + name: "no path is preserved as-is", + in: "https://example.com", + want: "https://example.com", + }, + { + name: "invalid URL is hidden", + in: "://token@example.com", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := redactWebhookURL(tt.in); got != tt.want { + t.Fatalf("redactWebhookURL(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestRedactURLError(t *testing.T) { + inner := fmt.Errorf("connection refused") + urlErr := &neturl.Error{ + Op: "Post", + URL: "https://hooks.example.com/secret-token", + Err: inner, + } + + got := redactURLError(urlErr) + if got != inner { + t.Fatalf("expected inner error, got %v", got) + } + if strings.Contains(got.Error(), "secret-token") { + t.Fatal("redacted error still contains secret") + } + + plain := fmt.Errorf("some other error") + if redactURLError(plain) != plain { + t.Fatal("non-url.Error should be returned as-is") + } +} + func TestHooksSliceNotAliased(t *testing.T) { // Verify that repo hooks don't leak into the global config's Hooks slice