Skip to content

fix(controlplane): redact secrets and close SSRF gaps across config, delivery, and notifications#2706

Merged
mekilis merged 5 commits into
mainfrom
smart/security-hardening-vuln-batch
Jul 7, 2026
Merged

fix(controlplane): redact secrets and close SSRF gaps across config, delivery, and notifications#2706
mekilis merged 5 commits into
mainfrom
smart/security-hardening-vuln-batch

Conversation

@mekilis

@mekilis mekilis commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Batch of security hardening fixes across the control plane and delivery paths.

  • Config secret leak: strip license/checkout secrets from GetConfiguration before the payload leaves the UI/portal route.
  • Token refresh gate: gate token refresh on single-user-mode license, matching login. Fails closed on licenser error.
  • Response header redaction: mask auth credentials in event-delivery and delivery-attempt responses. Partial last-4 reveal, short/low-entropy values fully masked, webhook signatures kept visible for debugging. Redaction is response-only via a shallow clone, so stored and dispatch values are untouched. Endpoint response headers (e.g. Set-Cookie) are redacted too.
  • SSRF: slack webhook: validate slack_webhook_url on endpoint create/update and route slack notifications through the netjail dispatcher client.
  • SSRF: meta-event empty type: validate the meta-event URL after normalizing an empty type to http, closing the empty-type bypass. Guard nil SSL on the project-update path (create defaults it, update did not).

Test plan

  • go test ./api/... ./services/... ./internal/pkg/middleware/... ./util/...
  • Verify GetConfiguration response contains no license/checkout secrets
  • Verify delivery-attempt / event-delivery responses mask auth headers but show X-Convoy-Signature
  • Verify stored attempt/DB values remain unmasked
  • Verify slack_webhook_url and meta-event URLs reject loopback/private/link-local targets
  • Token refresh rejected in single-user mode without license

Note

High Risk
Touches authentication, license gating, secret exposure, and SSRF egress for user-controlled URLs—areas where mistakes have direct security impact, though behavior is largely fail-closed with tests.

Overview
Security hardening across configuration exposure, auth, delivery APIs, and outbound HTTP.

Configuration: GetConfiguration now strips license keys, checkout nonces, and related billing secrets before responses reach UI/portal routes (billing still uses dedicated endpoints).

Auth: Login and token refresh share IsPrimaryInstanceAdmin so refresh cannot bypass single-user license limits. Transient license/DB failures return 5xx (ErrCodeInternal) instead of “invalid credentials”; lapsed-license denials stay 403 on both paths.

Delivery APIs: Event delivery and delivery-attempt JSON redacts sensitive request/response headers for portal-link callers; JWT/API-key callers see raw values. Redaction is response-only (shallow clone); stored data and webhook signatures stay visible where intended.

SSRF: Slack webhook URLs are validated on endpoint create/update; meta-event URLs are validated after defaulting empty type to HTTP (with nil-safe SSL). Notifications use a dedicated HTTP client with connect-time private-IP blocking and enforced TLS (not webhook insecure_skip_verify).

Reviewed by Cursor Bugbot for commit 146ea8e. Bugbot is set up for automated code reviews on this repo. Configure here.

…delivery, and notifications

- strip license/checkout secrets from GetConfiguration before it leaves the UI/portal route
- gate token refresh on single-user-mode license, matching login (fail-closed on licenser error)
- mask auth credentials in event-delivery and delivery-attempt responses (partial last-4, short values fully masked, webhook signatures kept visible for debugging), response-only so stored/dispatch values survive
- redact endpoint response headers (e.g. Set-Cookie) on delivery attempts too
- validate slack_webhook_url on endpoint create/update and route slack notifications through the netjail dispatcher client
- validate meta-event URL after normalizing an empty type to http, closing the empty-type SSRF bypass; guard nil SSL on the project-update path
Comment thread api/models/delivery_attempt.go
…lers

redact request/response headers only for portal-link (external) callers on
event-delivery and delivery-attempt responses. api-key and authenticated
dashboard (jwt) callers now see raw headers, matching meta-event visibility;
portal holders remain masked. decision keyed on the auth credential type via
canViewRawHeaders (fail-closed on nil), and the response models default to
redaction so a bare literal cannot leak.
Comment thread worker/task/process_notifications.go Outdated
…hout license

portal header redaction now fully masks sensitive/non-allowlisted values (no
trailing bytes revealed), so customer-injected header names cannot leak secrets
to portal viewers. api-key and dashboard callers still receive raw headers and
webhook signatures stay visible.

slack notifications now post through a dedicated client with a connect-time
ssrf guard that blocks private/reserved destinations regardless of the IpRules
license, closing the dns->private path that netjail only covered when licensed.
this matches the existing unconditional write-time block on slack_webhook_url.
Comment thread net/dispatcher.go Outdated
Comment thread worker/task/process_notifications.go
… tls verify

the slack notification client cloned the webhook transport, which caused two
regressions:

- the dial-time ssrf guard also ran on the hop to a configured forward proxy,
  which is typically private, so notifications failed while webhook delivery
  used the same proxy fine. the guard now applies only when egress is direct;
  when a proxy is configured, egress control is delegated to the proxy as with
  webhook delivery.
- the clone inherited dispatcher.insecure_skip_verify, disabling tls
  verification for notifications. the notification transport now always verifies
  tls (custom-ca RootCAs preserved), regardless of the webhook compat flag.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4987130. Configure here.

Comment thread api/handlers/auth.go
…ication ssrf guard per request

login and token refresh mapped transient server-side failures (db lookups,
license evaluation, token generation) to auth denials: refresh returned 401
"invalid token" and login returned 403 "invalid credentials". a transient
outage was indistinguishable from a bad credential, and a license-lookup error
could surface as a definitive "license expired".

- add ErrCodeInternal and tag every server-side failure in the login and
  refresh services with it; handlers now return 503 for internal, 403 for
  license expired, and 401/403 only for real auth failures.
- IsPrimaryInstanceAdmin now propagates IsMultiUserMode errors instead of
  swallowing them and falling through to the single-user admin checks.

the notification ssrf guard also disabled itself globally whenever a forward
proxy was configured, leaving NO_PROXY and hostname-only targets to dial
directly with no guard. make the decision per request: the guard is always
installed and only the actual proxy hop is bypassed via a request-scoped marker,
so every direct dial stays guarded. tls verification is also forced on the
notification transport regardless of the webhook insecure_skip_verify flag.
@mekilis mekilis merged commit bc0b5ec into main Jul 7, 2026
33 checks passed
@mekilis mekilis deleted the smart/security-hardening-vuln-batch branch July 7, 2026 18:29
mekilis added a commit that referenced this pull request Jul 8, 2026
bump VERSION to v26.6.2 and backfill CHANGELOG.md through v26.6.2 with
git-cliff. the changelog had frozen at 26.3.6 while tags advanced to
v26.6.1, and VERSION had frozen at v26.2.2 (nothing in the release flow
bumps it). regenerate the missing 26.3.7, 26.6.0, 26.6.1 sections and add
26.6.2 for the three commits since v26.6.1 (#2705, #2706, #2707).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants