chore(notifications): bump tackle to v0.4.0

[loadez]

No description provided.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbdcbe0746

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject all non-global IPv6 ranges

Reject IPv6 addresses outside globally routable space rather than allowing every prefix not explicitly listed. On networks routing deprecated site-local space such as fec0::/10, or the local-use NAT64 prefix 64:ff9b:1::/48, these addresses reach this fallback as allowed; a URL resolving to one can therefore make the Slack or webhook worker dial an internal destination and bypass the SSRF guard.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve URL userinfo when pinning the target

Preserve uri.userinfo when rebuilding the pinned URL. For a previously accepted webhook such as https://user:password@example.com/hook, HTTPoison used the userinfo for HTTP Basic authentication, but the new target becomes https://<ip>:443/hook; both webhook and Slack requests consequently lose their credentials and receive authentication failures.

Useful? React with 👍 / 👎.

[skipi]

The egress guard is good work and the core design holds up — resolve-then-pin genuinely closes the DNS-rebinding window, rejecting when any resolved address is non-public is the right strictness, and the blocked/allowed test matrix is thorough. Four things before it should merge, one of them a silent production regression.

1. Please split this PR. The title is chore(notifications): bump tackle to v0.4.0; the tackle change is 2 lines out of 1135. The rest is a new 363-line SSRF egress guard, a new Target struct, both workers rewired onto it, hackney 1.18.1 → 1.25.0, ssl_verify_fun 1.1.6 → 1.1.7, an Alpine bump, and a new .mix-audit.txt. Reviewers triage by title, and this one says "skip me" for the single most security-load-bearing change in the batch.

2. Blocking — URL userinfo is dropped, which silently breaks basic-auth webhooks.

build_request_url/5 rebuilds scheme://ip:port/path?query and never reads %URI{}.userinfo; Target has no field that could carry it. But hackney lifts URL userinfo into HTTP Basic auth: hackney_url.erl:220-235 parses user/password off the authority, hackney.erl:326-337 inserts {basic_auth, {User, Password}} into the request options, and hackney_request.erl:41-73 encodes that into an Authorization: Basic header. The same chain exists in the currently locked 1.18.1 (hackney.erl:322-331, hackney_request.erl:41-53), so this is caused by the URL rewrite, not by the hackney bump. A webhook configured as https://user:pass@host/hook authenticates today and stops after this merges.

The failure is also invisible to us. webhook.ex:54-61 matches {:ok, response} → Logger.debug + Watchman.increment("notification.webhook.success"), and there is no status_code inspection anywhere in lib/. A 401 arrives as {:ok, %HTTPoison.Response{status_code: 401}} and is therefore counted as a success — no error metric, no retry, nothing above debug level. Logger.error and notification.webhook.failure fire only on transport-level {:error, _} (webhook.ex:70-76).

Nothing rejects such URLs today either: util/validator.ex:57-58 only checks that endpoint is non-nil and non-empty, with no URI.parse, and the form field is a plain text_input — so these configurations are accepted and working right now. Scope is webhook-only; Slack incoming-webhook URLs carry their secret in the path.

Suggested fix: add basic_auth to %Target{}, parse uri.userinfo in build_target/3 (URI.decode/1 each half, since hackney urldecodes them), and pass hackney: [basic_auth: target.basic_auth] from both workers — HTTPoison forwards :hackney options verbatim. Putting userinfo back into the rebuilt URL is a one-liner but makes target.url a credential-bearing string that webhook.ex already logs on failure. If the decision is that URL basic auth is unsupported, then rejecting userinfo in verify/1 with its own reason is also fine — it still breaks those webhooks, but loudly instead of as a 401 counted as success. Either way this needs a regression test; none of the five new test files touch userinfo or basic auth.

3. Pin the hackney floor, or set the TLS options explicitly.

ssl_options/2 sets verify: :verify_peer with no cacerts, depth or partial_chain, relying on hackney merging its own defaults underneath. That merge only exists from hackney 1.24 onward — I checked the source of 1.18.1, 1.20.1, 1.21.0 and 1.23.0, and hackney_connection:ssl_opts/2 returns the caller's ssl_options verbatim in all of them; 1.24.0 and 1.25.0 call merge_ssl_opts/2. So on any hackney below 1.24 this is verify_peer with no trust store, ssl_verify_hostname:verify_fun fails on {bad_cert, unknown_ca}, and every https delivery breaks.

Right now that's fine only because this PR happens to bump hackney to 1.25.0. Nothing enforces it: hackney is transitive via httpoison and undeclared in mix.exs, and the "https ssl_options bind SNI and hostname verification to the original host" test only asserts the returned keyword list — no real handshake, so CI cannot catch a regression here. Please add {:hackney, "~> 1.25", override: true}, or set cacerts: :certifi.cacerts() / depth / partial_chain explicitly so the module doesn't depend on undocumented merge semantics at all. The second option also protects whoever copies this module into another service.

4. Invert the IPv6 classifier to allow-only-global.

blocked?/1 blocks an enumerated list and allows everything else, so any prefix not listed is permitted by default. Two concrete gaps: 64:ff9b:1::/48 (RFC 8215 local-use NAT64) is not unwrapped because embedded_v4/8 requires c == 0, and deprecated site-local fec0::/10 falls straight through. For a security classifier, permitting only globally routable space and rejecting the rest fails in the safe direction. Two minor ones while you're in there: the {255,255,255,255} clause in blocked_v4?/4 is dead, since a >= 240 and a <= 255 already covers it, and 192.88.99.0/24 (deprecated 6to4 relay anycast) is not blocked.

Things I checked that are correct, so they don't need re-litigating: the Host header override cannot produce a duplicate header, because hackney uses store_new(<<"Host">>, Netloc, Headers0) at hackney_request.erl:630; coordinator.ex ignores worker return values, so the new {:error, :ssrf_blocked} cannot crash a caller; and follow_redirect: false is correctly set on both workers.