fix(tasks): classify invalid webhook headers permanently, add a header credential seam

[sroussey]

Follow-up to #744, targeting the same base branch. Two review findings plus two low-severity ones. #744's redaction coverage, SSRF gating, retry clamping and real-transport tests are unchanged.

M1 — an invalid request header was classified RETRYABLE

WebhookPost.ts merged headers inside a try that only catches JSON.stringify. The merge itself cannot throw — but undici builds the Headers object inside fetch, and a malformed name or value rejects with a bare TypeError: name "TypeError", no code, not a FetchUrlJobError. toRedactedWebhookError matches none of its named branches, so it fell through to NETWORK_ERROR → RetryableJobError, and a queued consumer retried a typo forever.

Probed on Node v22.22.2 (fetch against a loopback server):

headers	result
{"X-Bad Name":"v"}, {"":"v"}, {"X:Y":"v"}	TypeError: Headers.append: "…" is an invalid header **name**.
{"X-Ok":"a\nb"}, {"X-Ok":"a\0b"}	TypeError: Headers.append: "…" is an invalid header **value**.
{"X-Ok":" "}, {"X-Ok":"\nabc"}, {"X-Ok":"a\tb"}	accepted — normalized, not rejected

Two things shape the fix:

1. undici's message quotes the header VALUE. That text lands in detailWithCause(error) and is spliced into a persisted RetryableJobError. With M4 below placing a resolved secret on a header, a token containing a \n would be written to durable storage by the very error reporting it. So the new guard names the header but never echoes the value, and M1 is a prerequisite for M4.
2. A leading-\n value is normalized, not rejected. So any pre-validation stricter than undici's own rule is a new rejection, not a mirror. The guard is deliberately stricter in both halves and the JSDoc says so: names are held to isBareHeaderName (letters/digits/hyphens — narrower than RFC 9110's token production), values reject NUL/CR/LF anywhere rather than after a trim step.

Also:

• isBareHeaderName is exported from FetchUrlCredentials.ts (CREDENTIAL_HEADER_NAME_PATTERN stays private; credentialHeaderName now calls the predicate — no behaviour change), so request headers and credential ports are held to one rule rather than two spellings of it.
• The header merge moves out of the JSON.stringify try and goes through a new withJsonContentType, which adds Content-Type only when no key case-insensitively matches. { "Content-Type": …, ...headers } left a caller's lowercase content-type as a second object property, and Headers folds the two into one comma-joined field — sending both content types.
• webhookPrivateEntitlements widens from four positional unknowns to an options object (base / url / urlCredentialKey / headerCredentialKey / allowPrivate). Either key now enforces the credential entitlement, but scoped keys off urlCredentialKey only — a header credential does not hide the destination, so unscoping on it would hand out a wildcard network:private grant for nothing. All three call sites updated.

M4 — no credential seam for auth / signature headers

WebhookNotifyTask had only url_credential_key (the whole URL is the secret, the Slack/Discord shape). A bearer or HMAC endpoint therefore had to inline its secret in the headers port — and Task.toJSON serializes defaults verbatim into the saved graph JSON.

Adds credential_key (format: "credential", hidden), credential_scheme (bearer/basic/header/none, default bearer, hidden) and credential_header (default Authorization, hidden), reusing the existing applyCredentialToHeaders / credentialHeaderName. Descriptions match FetchUrlTask's so the two read the same.

Order matters: credential placement happens first, then M1's validation inside postWebhookJson, so the credential-produced header is validated too. credentialHeaderName throws a TaskConfigurationError hardcoded "FetchUrlTask: …" carrying no FETCH_* code, so it is re-raised as a CONFIGURATION FetchUrlJobError under a WebhookNotifyTask: label.

No credential-port stripping is needed here (unlike FetchUrlTask, which forwards ...rest into a persisted job input) — the post is assembled field by field. There is a comment saying so. WebhookNotifyTask has no queued path, so FetchUrlTask's credential-vs-queue refusal has no analogue to port.

LOW

• The body-derived retry hint is gated behind !allowPrivate. DiscordNotifyTask passes retryAfterFromJsonBody: true and is reachable with allow_private_destination; retry_after is read from the body, which a declared private destination never surfaces — contradicting the invariant documented on allowPrivateDestination. The Retry-After header is transport-level and stays.
• packages/tasks/src/common.ts re-exports ./util/RetryAfter and ./util/WebhookPost. MAX_REQUEST_TIMEOUT_MS, redactWebhookUrlIn, postWebhookJson, compactPayload and retryDateFromRetryAfterHeader were all unreachable downstream.

One review claim refuted, deliberately not acted on

"a complete body ending in ... is reported as truncated"

WebhookPost.ts reads complete || surfaced.endsWith("...") ? surfaced : \${surfaced}...`— a complete body short-circuits oncomplete` and is returned unchanged. No change made.

Tests

New describe("request headers") in NotifyTask.test.ts (13 cases): invalid name → PermanentJobError/CONFIGURATION/message contains the name/mockFetch not called; a control char in a value → CONFIGURATION with expect(message).not.toContain("abc123") on a token-shaped value (this is the assertion guarding the secret echo); URL redacted in the message; a valid header still reaches the transport; a lowercase content-type override does not double up; credential_key → Authorization: Bearer <secret> and JSON.stringify(task.toJSON()) contains the key and not the secret; credential_scheme: "header" + credential_header; a caller authorization: "stale" replaced case-insensitively; credential_header: "X Bad" → CONFIGURATION labelled WebhookNotifyTask; and the composition test — a credential value carrying a control char is rejected with the secret absent from both .message and formatErrorChainForDiagnostics.

Entitlement coverage extended: credential_key alone enforces credential but does not unscope a declared private destination.

NotifyTaskTransport.test.ts gets two loopback cases against the real undici transport (the mocked safeFetch never constructs a Headers object, so it can prove the guard fires but not that the guard is still in front of the thing it guards): an invalid header never reaches the server (request count 0), and a header the guard admits round-trips intact — the half that would actually break if undici tightened its validation.

packages/tasks/README.md: the three credential ports, a bullet that a secret goes through credential_key so it never reaches graph JSON, and a bullet that an invalid header is a permanent CONFIGURATION failure rather than a retried network error.

Verification

$ bun scripts/test.ts task vitest
Running all tests in sections [task] — 67 file(s)
 Test Files  67 passed (67)
      Tests  1099 passed | 24 skipped (1123)

(baseline on this branch's merge-base: 1084 passed | 24 skipped — +15 new)

$ npx eslint packages/tasks/src packages/test/src/test/task
(no output)
$ npx prettier "packages/tasks/src/**/*.ts" "packages/test/src/test/task/*.ts" "packages/tasks/README.md" --check
All matched files use Prettier code style!

There is no root lint script; format is eslint --fix && prettier --check --write, run here in check-only form. npx tsgo -p packages/tasks/tsconfig.json reports the same errors before and after these changes (source-mode dist stubs carry no real .d.ts), so it is unaffected.

---

Generated by Claude Code