From 387171c29becaaf59ff227a15e749e36b77c6ec5 Mon Sep 17 00:00:00 2001 From: Julio Jimenez Date: Tue, 29 Sep 2026 12:20:24 -0400 Subject: [PATCH] feat(slack): slack-notify-on always|failure switch Signed-off-by: Julio Jimenez --- CLAUDE.md | 2 +- Dockerfile | 4 +- README.md | 11 ++++- action.yml | 5 +++ cmd/clickbom/main.go | 28 +++++++++++-- cmd/clickbom/main_test.go | 65 ++++++++++++++++++++++++++++++ internal/config/config.go | 19 +++++++++ internal/config/config_test.go | 73 +++++++++++++++++++++++++++++++++- 8 files changed, 198 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0617f2e..5d3344a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ docker run --rm --platform linux/amd64 --entrypoint /usr/local/bin/cyclonedx cli ### Execution flow -`run()` in [main.go](cmd/clickbom/main.go) snapshots `config.SecretsFromEnv()`, loads the config, then calls `execute()` and reports its outcome: when `cfg.SlackWebhookURL` is set, `notifyOutcome` posts one success-or-failure message (bounded by a 2-minute context) built from `notify.RunContextFromEnv()`, `buildSummary(cfg)` (non-sensitive facts only) and `secretsForRedaction(cfg, startupSecrets)`. The run context is the runner's `GITHUB_*` / `RUNNER_OS` default env vars plus `CLICKBOM_JOB_CHECK_RUN_ID`, which action.yml fills from the `job-check-run-id` input whose default is `${{ job.check_run_id }}`: a Docker action's `runs.env` may only reference `inputs`, but input defaults may reference the `job` context, so the id reaches the container without any consumer change. The header links to `{server}/{repo}/actions/runs/{id}/job/{check_run_id}` (verified against the REST API: a job's `html_url` uses its check run id, with no attempt segment) and falls back to `.../runs/{id}[/attempts/{n}]` when the id is empty (older GHES). A notification failure is logged as a warning and never changes the exit status (`notifyOutcome` also recovers a panic without logging its value). `newSlackNotifier` and `notificationTimeout` are package variables so tests can point the notifier at an `httptest` server and shorten the deadline. A `LoadConfig` failure is reported too via `notifyConfigFailure`, which uses `config.SlackWebhookURLFromEnv()` because there is no `Config` to read it from and attaches no summary. `execute()` then branches on `cfg.Merge`: +`run()` in [main.go](cmd/clickbom/main.go) snapshots `config.SecretsFromEnv()`, loads the config, then calls `execute()` and reports its outcome: when `cfg.SlackWebhookURL` is set, `reportOutcome` posts one success-or-failure message (bounded by a 2-minute context) built from `notify.RunContextFromEnv()`, `buildSummary(cfg)` (non-sensitive facts only) and `secretsForRedaction(cfg, startupSecrets)`. The run context is the runner's `GITHUB_*` / `RUNNER_OS` default env vars plus `CLICKBOM_JOB_CHECK_RUN_ID`, which action.yml fills from the `job-check-run-id` input whose default is `${{ job.check_run_id }}`: a Docker action's `runs.env` may only reference `inputs`, but input defaults may reference the `job` context, so the id reaches the container without any consumer change. The header links to `{server}/{repo}/actions/runs/{id}/job/{check_run_id}` (verified against the REST API: a job's `html_url` uses its check run id, with no attempt segment) and falls back to `.../runs/{id}[/attempts/{n}]` when the id is empty (older GHES). `reportOutcome` first applies the `slack-notify-on` switch (`cfg.SlackNotifyOn`, env `SLACK_NOTIFY_ON`, default `always`; `Sanitize` trims, lower-cases and rejects anything but `always`/`failure`, and treats empty as `always` so a `Config` built without `LoadConfig` still posts): with `failure`, a successful run logs "Slack notification skipped" at Info instead of posting (`shouldNotify`). A notification failure is logged as a warning and never changes the exit status (`notifyOutcome` also recovers a panic without logging its value). `newSlackNotifier` and `notificationTimeout` are package variables so tests can point the notifier at an `httptest` server and shorten the deadline. A `LoadConfig` failure is reported too via `notifyConfigFailure`, which uses `config.SlackWebhookURLFromEnv()` because there is no `Config` to read it from and attaches no summary; it bypasses the `slack-notify-on` switch on purpose (a failure is always posted, and an invalid `SLACK_NOTIFY_ON` value is itself such a failure). `execute()` then branches on `cfg.Merge`: - **Normal mode** (`handleNormalMode`): dispatches on `cfg.SBOMSource` (`github` / `mend` / `wiz` / `trivy`) → download/generate → `ExtractSBOMFromWrapper` (unwraps the `{"sbom": {...}}` envelope GitHub's deprecated synchronous endpoint used; the asynchronous export returns a bare SPDX document, so for GitHub this is now a pass-through kept for compatibility) → `DetectSBOMFormat` (by inspecting `bomFormat` or `spdxVersion`) → `ConvertSBOM` to the requested format → upload to S3 → optionally upload to ClickHouse. diff --git a/Dockerfile b/Dockerfile index 69733f3..1a8bf92 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,7 @@ RUN apk update && apk upgrade --available --no-cache LABEL maintainer="ClickHouse Security Team" \ description="ClickBOM - SBOM Management Tool" \ - version="2.1.0" + version="2.2.0" # Install build dependencies RUN apk add --no-cache \ @@ -97,7 +97,7 @@ FROM gcr.io/distroless/cc-debian13:nonroot LABEL maintainer="ClickHouse Security Team" \ description="ClickBOM - SBOM Management Tool" \ - version="2.1.0" \ + version="2.2.0" \ security.scan="enabled" # Copy from tools stage diff --git a/README.md b/README.md index ed09dc0..3369574 100644 --- a/README.md +++ b/README.md @@ -134,15 +134,17 @@ Downloads SBOMs from GitHub, Mend, and Wiz, or generates them from container ima | Name | Description | Default | Required | Sensitive | | ----------------- | ------------------------------------------------------------------------------------ | ------------------------- | -------- | --------- | | slack-webhook-url | Slack incoming webhook that receives one success or failure message per run | | false | true | +| slack-notify-on | Which outcomes to post: `always` (every run) or `failure` (failed runs only) | `always` | false | false | | job-check-run-id | Id of the running job, used only to link the message to the job. Leave the default. | `${{ job.check_run_id }}` | false | false | - When set, ClickBOM posts one message per run to whichever Slack workspace owns the webhook: whether the run succeeded or failed, the repository, workflow, job and step that ran it, what triggered it (event, branch, short commit, actor), the SBOM source, the S3 object written, the ClickHouse database and table when configured, the duration, and a link. On failure the first line of the error is included. +- `slack-notify-on: failure` posts failed runs only, for channels where one message per successful run is too noisy. A run that fails configuration validation is still posted, as long as the webhook itself is valid. The value is case-insensitive; anything other than `always` or `failure` is rejected at start-up. - The link opens the job itself. `job.check_run_id` is evaluated as the default of `job-check-run-id` and handed to the container, so no workflow change is needed; it also tells matrix legs apart, which share a job key. On a GitHub Enterprise Server release without `job.check_run_id` the value is empty and the link opens the workflow run instead (the specific attempt when re-run). - **Job** is the job's key in the workflow file (`GITHUB_JOB`), not its `name:`. **Step** is the step's `id:` (`GITHUB_ACTION`); give the ClickBOM step an `id` for a readable label, otherwise GitHub generates one such as `__ClickHouse_ClickBOM`. - Only Slack *incoming webhook* URLs are accepted: `https://hooks.slack.com/services/...` (or `hooks.slack-gov.com` for GovSlack). Workflow Builder webhook triggers (`/triggers/...`, `/workflows/...`) are rejected at start-up because they only take flat key/value payloads. The URL is a credential: pass it from a secret. ClickBOM never logs it, and a rejected value is not echoed in the error. - Nothing marked Sensitive in this document reaches Slack. For Mend and Wiz the message names the scope (`project scope`, `product scope`, `report`) rather than the identifier and omits the ClickHouse table name, which embeds that identifier. The error text is redacted before posting: URL query strings and credentials are removed, every Sensitive input value (including the table-name spelling of Mend and Wiz identifiers), bearer and basic-auth headers, socket addresses, AWS access key ids and GitHub tokens are replaced with `***`, and only the first line is sent. - Notification failures are logged as warnings and never change the outcome of the job; delivery is bounded to about two minutes (three attempts). A run that fails configuration validation is reported too, as long as the webhook itself is valid. A retry after a timed-out delivery can produce a duplicate message. -- The inputs first ship in `v2.1.0`; consumers pinned to `v2.0.0` or `v2.0.1` need a ref bump to use them. +- `slack-webhook-url` and `job-check-run-id` first ship in `v2.1.0` and `slack-notify-on` in `v2.2.0`; consumers pinned to an older tag need a ref bump to use them. ## Usage @@ -674,7 +676,12 @@ Each run posts one message, for example: > > **Workflow** Upload SBOM · **Job** clickbom · **Step** clickbom · **Trigger** push on main @ 0123456 by octocat · **Source** github · my-org/my-repo · **Output** s3://my-sbom-bucket/clickbom.json (cyclonedx) · **Duration** 1m23s -A failed run is posted the same way, in red, with the first line of the error, so a matrix of many ClickBOM jobs can share one channel. +A failed run is posted the same way, in red, with the first line of the error, so a matrix of many ClickBOM jobs can share one channel. If one message per successful run is too noisy, add `slack-notify-on: failure` next to the webhook and only failed runs are posted: + +```yaml + slack-webhook-url: ${{ secrets.SLACK_WEBHOOK_URL }} + slack-notify-on: failure +``` ## Runtime Image diff --git a/action.yml b/action.yml index b889fd1..6ec7b64 100644 --- a/action.yml +++ b/action.yml @@ -151,6 +151,10 @@ inputs: slack-webhook-url: description: 'Slack incoming webhook URL (https://hooks.slack.com/services/...). When set, ClickBOM posts a success or failure message with the workflow, job, step, trigger, SBOM source, S3 target and a link to the job. Pass it from a secret; it is never logged.' required: false + slack-notify-on: + description: 'Which outcomes to post to Slack: "always" (every run) or "failure" (failed runs only, including configuration failures). Use failure when one message per successful run is too noisy. Ignored when slack-webhook-url is unset.' + required: false + default: 'always' job-check-run-id: description: 'Id of the running job, used only to link the Slack message to the job. Leave the default: job.check_run_id is evaluated here because a Docker action can only see the inputs context. Empty on GitHub Enterprise Server releases without job.check_run_id, in which case the message links to the run.' required: false @@ -208,6 +212,7 @@ runs: DEBUG: ${{ inputs.debug }} # Notifications SLACK_WEBHOOK_URL: ${{ inputs.slack-webhook-url }} + SLACK_NOTIFY_ON: ${{ inputs.slack-notify-on }} CLICKBOM_JOB_CHECK_RUN_ID: ${{ inputs.job-check-run-id }} branding: icon: 'list' diff --git a/cmd/clickbom/main.go b/cmd/clickbom/main.go index ab264f2..b46766a 100644 --- a/cmd/clickbom/main.go +++ b/cmd/clickbom/main.go @@ -46,11 +46,12 @@ func run() error { ctx := context.Background() - // Every outcome of the run, success or failure, is reported once. A failed - // notification is only logged: it must never change the exit status. + // Every outcome of the run is reported once, unless slack-notify-on is + // "failure" and the run succeeded. A failed notification is only logged: + // it must never change the exit status. notifier := newSlackNotifier(cfg.SlackWebhookURL) err = execute(ctx, cfg) - notifyOutcome(ctx, notifier, notify.Event{ + reportOutcome(ctx, cfg, notifier, notify.Event{ Run: notify.RunContextFromEnv(), Summary: buildSummary(cfg), Err: err, @@ -488,6 +489,27 @@ var newSlackNotifier = notify.NewSlackNotifier // variable so tests can shorten it. var notificationTimeout = 2 * time.Minute +// shouldNotify reports whether a finished run is posted: every failure, and a +// success unless SLACK_NOTIFY_ON is "failure". An empty value (a Config built +// without LoadConfig) counts as "always". Configuration failures never pass +// through here: notifyConfigFailure posts them regardless of the switch. +func shouldNotify(cfg *config.Config, err error) bool { + return err != nil || cfg.SlackNotifyOn != config.SlackNotifyOnFailure +} + +// reportOutcome applies the slack-notify-on switch in front of notifyOutcome. +// A suppressed success is logged only when a webhook is configured, so runs +// without Slack stay quiet. +func reportOutcome(ctx context.Context, cfg *config.Config, notifier *notify.SlackNotifier, ev notify.Event) { + if shouldNotify(cfg, ev.Err) { + notifyOutcome(ctx, notifier, ev) + return + } + if notifier != nil { + logger.Info("Slack notification skipped: the run succeeded and slack-notify-on is %q", cfg.SlackNotifyOn) + } +} + // notifyOutcome posts ev and logs, but never returns, a delivery failure. func notifyOutcome(ctx context.Context, notifier *notify.SlackNotifier, ev notify.Event) { if notifier == nil { diff --git a/cmd/clickbom/main_test.go b/cmd/clickbom/main_test.go index 1001323..77f087c 100644 --- a/cmd/clickbom/main_test.go +++ b/cmd/clickbom/main_test.go @@ -495,6 +495,71 @@ func TestNotifyConfigFailure_PostsRedactedFailure(t *testing.T) { } } +func TestShouldNotify(t *testing.T) { + failed := errors.New("boom") + tests := []struct { + name string + mode string + err error + want bool + }{ + {"always posts a success", config.SlackNotifyAlways, nil, true}, + {"always posts a failure", config.SlackNotifyAlways, failed, true}, + {"failure skips a success", config.SlackNotifyOnFailure, nil, false}, + {"failure posts a failure", config.SlackNotifyOnFailure, failed, true}, + {"an unset switch counts as always", "", nil, true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := shouldNotify(&config.Config{SlackNotifyOn: tc.mode}, tc.err); got != tc.want { + t.Errorf("shouldNotify(%q, %v) = %v, want %v", tc.mode, tc.err, got, tc.want) + } + }) + } +} + +func TestReportOutcome_HonoursNotifyOn(t *testing.T) { + calls := 0 + var posted string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + body, _ := io.ReadAll(r.Body) + posted = string(body) + _, _ = w.Write([]byte("ok")) + })) + defer srv.Close() + notifier := notify.NewSlackNotifier(srv.URL + "/services/T/B/X") + logs := captureLogs(t) + ctx := context.Background() + cfg := &config.Config{SlackWebhookURL: "https://hooks.slack.com/services/T/B/X", SlackNotifyOn: config.SlackNotifyOnFailure} + + reportOutcome(ctx, cfg, notifier, notify.Event{}) + if calls != 0 { + t.Fatalf("a success with slack-notify-on=failure was posted: %s", posted) + } + if !strings.Contains(logs.String(), "[INFO]") || !strings.Contains(logs.String(), "Slack notification skipped") { + t.Errorf("the suppressed success must be logged at Info: %q", logs.String()) + } + + reportOutcome(ctx, cfg, notifier, notify.Event{Err: errors.New("boom")}) + if calls != 1 || !strings.Contains(posted, "ClickBOM failed") || !strings.Contains(posted, "boom") { + t.Errorf("a failure with slack-notify-on=failure must be posted: calls=%d body=%s", calls, posted) + } + + cfg.SlackNotifyOn = config.SlackNotifyAlways + reportOutcome(ctx, cfg, notifier, notify.Event{}) + if calls != 2 || !strings.Contains(posted, "ClickBOM succeeded") { + t.Errorf("a success with slack-notify-on=always must be posted: calls=%d body=%s", calls, posted) + } + + // Without a webhook there is nothing to suppress, so nothing is logged. + logs.Reset() + reportOutcome(ctx, &config.Config{SlackNotifyOn: config.SlackNotifyOnFailure}, nil, notify.Event{}) + if logs.Len() != 0 { + t.Errorf("no webhook: expected no log output, got %q", logs.String()) + } +} + func TestNotifyOutcome_NilNotifierIsNoop(t *testing.T) { logs := captureLogs(t) notifyOutcome(context.Background(), nil, notify.Event{}) diff --git a/internal/config/config.go b/internal/config/config.go index 67edd10..b6db3b6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,6 +18,12 @@ const ( SourceTrivy = "trivy" ) +// SLACK_NOTIFY_ON values: post every outcome, or failures only. +const ( + SlackNotifyAlways = "always" + SlackNotifyOnFailure = "failure" +) + // Config holds the application configuration. type Config struct { // GitHub @@ -78,6 +84,7 @@ type Config struct { // Notifications SlackWebhookURL string // Slack incoming webhook; a credential, never logged + SlackNotifyOn string // SlackNotifyAlways or SlackNotifyOnFailure } // LoadConfig loads configuration from environment variables. @@ -149,6 +156,7 @@ func LoadConfig() (*Config, error) { // Notifications SlackWebhookURL: os.Getenv("SLACK_WEBHOOK_URL"), + SlackNotifyOn: getEnvOrDefault("SLACK_NOTIFY_ON", SlackNotifyAlways), } // Sanitize inputs @@ -359,6 +367,17 @@ func (c *Config) Sanitize() error { return err } } + // SLACK_NOTIFY_ON is a closed set. It is not sensitive, so the rejected + // value may be quoted. Empty (a Config built without LoadConfig) means + // always, which is also the action.yml default. + switch strings.ToLower(strings.TrimSpace(c.SlackNotifyOn)) { + case "", SlackNotifyAlways: + c.SlackNotifyOn = SlackNotifyAlways + case SlackNotifyOnFailure: + c.SlackNotifyOn = SlackNotifyOnFailure + default: + return fmt.Errorf("invalid SLACK_NOTIFY_ON: %q (must be always or failure)", c.SlackNotifyOn) + } if err := c.sanitizeUUIDs(); err != nil { return err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 96d5d67..795a127 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -374,6 +374,76 @@ func TestLoadConfig_SlackWebhookURL(t *testing.T) { }) } +func TestLoadConfig_SlackNotifyOn(t *testing.T) { + load := func(t *testing.T, value string) (*Config, error) { + t.Helper() + env := map[string]string{"S3_BUCKET": "test-bucket", "REPOSITORY": "owner/repo"} + if value != "" { + env["SLACK_NOTIFY_ON"] = value + } + setEnv(t, env) + return LoadConfig() + } + + t.Run("defaults to always", func(t *testing.T) { + cfg, err := load(t, "") + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if cfg.SlackNotifyOn != SlackNotifyAlways { + t.Errorf("SlackNotifyOn = %q, want %q", cfg.SlackNotifyOn, SlackNotifyAlways) + } + }) + + t.Run("accepts both values, trimmed and case-insensitively, without a webhook", func(t *testing.T) { + for value, want := range map[string]string{ + "always": SlackNotifyAlways, + "failure": SlackNotifyOnFailure, + " Failure\n": SlackNotifyOnFailure, + "ALWAYS": SlackNotifyAlways, + "\tfailure\t": SlackNotifyOnFailure, + } { + cfg, err := load(t, value) + if err != nil { + t.Errorf("LoadConfig(SLACK_NOTIFY_ON=%q): %v", value, err) + continue + } + if cfg.SlackNotifyOn != want { + t.Errorf("SLACK_NOTIFY_ON=%q: SlackNotifyOn = %q, want %q", value, cfg.SlackNotifyOn, want) + } + } + }) + + t.Run("rejects other values naming the variable and the value", func(t *testing.T) { + for _, value := range []string{"success", "never", "true", "fail", "always,failure"} { + _, err := load(t, value) + if err == nil { + t.Errorf("LoadConfig accepted SLACK_NOTIFY_ON=%q", value) + continue + } + if !strings.Contains(err.Error(), "SLACK_NOTIFY_ON") || !strings.Contains(err.Error(), value) { + t.Errorf("SLACK_NOTIFY_ON=%q: error %q should name the variable and the value", value, err) + } + } + }) + + t.Run("Sanitize treats an empty field as always", func(t *testing.T) { + // Sanitize also range-checks the Mend timings and the SBOM closed + // sets, so the fixture carries their LoadConfig defaults. + cfg := &Config{ + S3Bucket: "test-bucket", Repository: "owner/repo", + SBOMSource: SourceGitHub, SBOMFormat: "cyclonedx", + MendMaxWaitTime: 1800, MendPollInterval: 30, + } + if err := cfg.Sanitize(); err != nil { + t.Fatalf("Sanitize: %v", err) + } + if cfg.SlackNotifyOn != SlackNotifyAlways { + t.Errorf("SlackNotifyOn = %q, want %q", cfg.SlackNotifyOn, SlackNotifyAlways) + } + }) +} + func TestSecretsFromEnv(t *testing.T) { setEnv(t, map[string]string{"S3_BUCKET": "public-bucket"}) if got := SecretsFromEnv(); len(got) != 0 { @@ -414,6 +484,7 @@ func TestConfigSecrets(t *testing.T) { SlackWebhookURL: "https://hooks.slack.com/services/T/B/X", // Non-sensitive inputs must never be redacted, or the message becomes useless. S3Bucket: "bucket", S3Key: "key.json", Repository: "o/r", ClickHouseDatabase: "db", ClickHouseUsername: "user", TrivyImage: "img:1", + SlackNotifyOn: SlackNotifyOnFailure, // would blank the word "failure" in every posted error } got := cfg.Secrets() set := map[string]bool{} @@ -436,7 +507,7 @@ func TestConfigSecrets(t *testing.T) { t.Errorf("Secrets() is missing %q", want) } } - for _, public := range []string{"bucket", "key.json", "o/r", "db", "user", "img:1"} { + for _, public := range []string{"bucket", "key.json", "o/r", "db", "user", "img:1", "failure"} { if set[public] { t.Errorf("Secrets() wrongly contains non-sensitive value %q", public) }