fix(tasks): redact webhook query values and the reason phrase

[sroussey]

Based on #744 (retarget to main once that merges). Independent of #745 — both touch packages/tasks, different files.

What

For the notification tasks the webhook URL is the credential, and redactWebhookUrlIn is the control that keeps it out of a diagnostic quoting the endpoint's reply. It reaches the caller because SlackNotifyTask and DiscordNotifyTask both set includeBodyInError: true. Two shapes walked straight through it.

Query values were never candidates. Candidates came only from pathname+search, search, and the individual path segments. An endpoint authenticating with ?token=SUPERSECRET1 that answers bad token SUPERSECRET1 matched nothing: the whole ?token=SUPERSECRET1 pair was admitted, but the echo quotes the value on its own. ?token= auth is an ordinary deployment shape.

All-lowercase candidates were exempted outright, on the theory that an all-lowercase run is a word rather than a token. So /hooks/supersecrettoken echoed back as rejected: supersecrettoken went out verbatim, despite being 16 characters and clearly a secret.

Rider, same function: response.statusText was interpolated into the message unconditionally and stored as httpStatusText, with no redaction pass over it — and, unlike the failure body, it was not withheld for a private destination. A server puts whatever it likes in a reason phrase, so that left the SSRF read the body suppression exists to prevent open through a narrower channel.

Why this fix

Admission is now by length only. The services / webhooks exemption the lowercase rule was really protecting is expressed directly, as a named set of the supported providers' routing segments — which is what it always meant, stated in terms that do not also exempt every lowercase secret.

Query values are admitted in both raw and decoded form, and the raw query is split by hand rather than read from searchParams: that decoder turns + into a space, so a decoded value would not match the bytes an endpoint echoes back. A malformed escape falls back to the raw form, which is what appears in an echo anyway.

The 8-char floor (SECURITY_LIMITS.webhookMinRedactableSegmentChars) is unchanged and now applies to query values too, so a short ?t=abc still leaks. That is the same policy segments already had; the JSDoc now says so rather than leaving it to be rediscovered.

Not folded in: the REDIRECT_NOT_FOLLOWED diagnostic (same file, different function — it rebuilds the transport's refusal as INVALID_URL). It requires flipping three baked-in assertions and is a diagnostics-quality issue, not a leak. Deliberately deferred.

Tests

Six cases, all through the tasks — redactWebhookUrlIn is not on the public surface (WebhookPost.ts is not re-exported from common.ts), and WebhookNotifyTask sets includeBodyInError: false, so Slack/Discord are the drivers.

case	pre-fix
?token=SUPERSECRET1 + 403 body bad token SUPERSECRET1 → absent from .message and .stack	fails
/hooks/supersecrettoken + 403 body rejected: supersecrettoken → absent	fails
/notifications/deploy + unknown notifications route → notifications is redacted (the accepted cost, pinned)	fails
public 403 with reason phrase token SECRETTOKEN bad → absent from message and httpStatusText	fails
private 400 with reason phrase index=cluster-secrets shard=3 → message has 400, not cluster-secrets; httpStatusText undefined	fails
structural guard: Discord 400 body invalid webhooks payload → webhooks survives	passes both sides

The last is the pre-existing over-redaction guard; its comment previously justified the exemption as "all-lowercase runs are words, not tokens" and now names the real reason (a provider routing segment), pinning the replacement for the deleted rule.

Actually executed:

• Against pre-fix WebhookPost.ts (reverted, tests kept): 5 failed / 89 passed — exactly the five new cases.
• With the fix: NotifyTask + NotifyTaskTransport + FetchUrlSsrf → 173 passed.
• Whole packages/test/src/test/task/ directory: 67 files, 1055 passed, 24 skipped.
• prettier --check clean.

No pre-existing test asserted on statusText; the private-destination cases assert toContain("400") and httpStatus, and the public paths use "Not Found" / "Bad Request", which survive redaction unchanged. Confirmed by the full-directory run.

Risk / blast radius

Two behaviour changes a reviewer has to accept:

1. A generic webhook whose path carries a long lowercase word (/notifications/deploy) now has that word redacted out of echoed diagnostics. This is the stated cost of dropping the exemption; there is no signal separating such a word from a lowercase token. Pinned by its own test so it cannot surprise anyone later.
2. httpStatusText is now undefined for a private destination, where it previously carried the reason phrase. Consumers reading that field for private posts see a missing value rather than a string.

Confined to redactWebhookUrlIn and the failure-throw in postWebhookJson. Both are shared by all three notification tasks; WebhookNotifyTask never surfaces bodies, so only its httpStatusText behaviour changes.

Unverified

• No fuzzing over URL shapes; coverage is the six cases above plus the pre-existing redaction suite.
• Whether any downstream consumer depends on httpStatusText being present for a private destination was not audited outside this repo.
• tsc -p packages/tasks cannot run cleanly in this checkout (unbuilt .d.ts under use-source mode); no type check against real build output was possible.
• Run under Node v22.22.2, not the Node 24 the repo asks for.

---

Generated by Claude Code