fix(tasks): give the webhook header credential a lifecycle — fail closed on a store miss, redact it out of output

[sroussey]

Stacked fix onto PR #744 — this branch targets claude/notify-merge-main, not main, so it reviews and merges as a follow-up to that PR rather than duplicating its diff.

Two commits, in this order on purpose: the first guarantees a resolved secret exists, the second guarantees it cannot come back.

1. [MEDIUM] credential_key failed OPEN on a store miss

url_credential_key already refuses to post when the store cannot answer a configured key. credential_key failed open on the identical case: applyCredentialToHeaders returns the caller's headers unchanged when credential is undefined, and the input resolver yields exactly that on a miss — so a locked store or a mistyped key silently sent the notification unauthenticated (or unsigned) and reported success.

resolveWebhookCredential(credential, credentialConfigured, portName, label) now sits beside resolveWebhookUrl and throws FetchUrlErrorCode.CONFIGURATION when a key is configured but the resolved value is missing or empty. The message names the port and tells the operator to unlock the store or fix the key; it never echoes a value.

WebhookNotifyTask calls it before applyCredentialToHeaders, discriminating on Object.hasOwn(input, "credential_key") — resolveSchemaInputs writes resolved[key] = undefined on a miss, so the port is PRESENT either way and only its presence separates "no credential wanted" from "credential wanted but the store could not answer".

applyCredentialToHeaders is left fail-open and untouched: it is pure, has no configured signal, and is shared with FetchUrlTask, which must not inherit an unasked-for behavior change.

It fails closed also under credential_scheme: "none". The operator configured a key; "resolve but don't send" is a debug affordance, not a reason to swallow a locked store.

Out of scope, noted as a follow-up: FetchUrlTask has the same fail-open shape. It is not touched here because its credential seam is shared and its callers are wider than these three notify tasks.

2. [HIGH] The header credential could be echoed straight back into response

WebhookNotifyTask sets readSuccessBody: true, and the only redaction on a success body was redactWebhookUrlIn(text, url) — which knows the URL and nothing else. The secret newly placed on Authorization/credential_header was never a redaction candidate, so an echoing endpoint (webhook.site, RequestBin, a chatty 200) returned up to 1 KB of it verbatim into response: task output, pipeable and persisted with the run. The module header claimed the URL secret never reaches task output; the second secret shape had no equivalent guard.

createWebhookRedactor(url, secrets) returns a (text: string) => string and is now the single function every echoed string goes through:

• success body (postWebhookJson), failure body suffix, statusText, and the JSON.stringify failure detail;
• inside toRedactedWebhookError (both the rebuilt-message and generic-detail paths) and redactedStackFrom, whose signatures now take the redactor instead of the raw url.

Order inside the redactor matters: exact secrets FIRST (longest-first), THEN redactWebhookUrlIn. Reversed, the URL pass can chop a substring out of a secret and leave the remainder unmatched. Each secret is admitted raw plus its encodeURIComponent variant, mirroring admitVariants.

No length floor, unlike URL path segments — those are guesses about which part of a URL is a token, whereas a header secret is a known exact value handed in by the caller. Residual, stated in the JSDoc: a very short secret can mangle a diagnostic. That beats leaking a short API key.

readonly secrets: readonly string[] | undefined is declared, not optional, on WebhookPostRequest — house rule, and it forces all three call sites to answer the question. WebhookNotifyTask passes input.credential_key === undefined ? [] : [input.credential_key] (the port holds the RESOLVED secret by execute time); SlackNotifyTask and DiscordNotifyTask pass undefined.

Redaction runs unconditionally, including under credential_scheme: "none": a value never sent cannot be echoed, so redacting costs nothing and removes a scheme-dependent branch from a security path.

The module header's invariant is restated over values: no value this module was handed as a secret reaches a message, an output port, or a stack.

Signature change, called out

redactedStackFrom(original, rebuilt, redact) and toRedactedWebhookError(error, url, label, callerSignal, redact) now take the redactor. redactedStackFrom is exported from the package root (common.ts re-exports ./util/WebhookPost) and toRedactedWebhookError is module-private. Verified with git grep across packages/, providers/ and examples/: zero consumers outside WebhookPost.ts.

Tests

Extended credential misconfiguration: a configured credential_key the store cannot answer fails closed (PermanentJobError, code CONFIGURATION, message names credential_key, mockFetch called zero times — this posted unauthenticated and returned success before); a store miss under scheme "none" also fails closed; an unconfigured credential_key leaves plain headers working (regression twin of the existing URL case).

New header credential redaction describe: an echoing endpoint cannot return the header credential in response; a reason phrase quoting the credential is redacted (new Response("", {status: 400, statusText: "bad token <secret>"}) — the error path actually open for this task, since includeBodyInError: false already closes the body path; both error.message and formatErrorChainForDiagnostics asserted clean); a transport error quoting the credential is redacted (a TypeError whose cause quotes it; message and .stack asserted); URL redaction still applies alongside a header secret (regression guard against reordering); a secret spanning the truncation boundary leaves no usable prefix (a 4 KB body with the secret at ~1010 chars, straddling the 1024-char cut — pins redact-before-truncate).

Verification

• bun scripts/test.ts task vitest — 67 files passed, 1116 passed / 24 skipped. All 7 new assertions were confirmed failing on the base branch first, each for the stated reason; the two commits were re-split so the first is green on its own.
• bun run format — no changes in packages/tasks or packages/test (it also reformats pre-existing unrelated files under examples/ and providers/; those were reverted and are not in this diff).

---

Generated by Claude Code

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	60.54%	39089 / 64557
🔵	Statements	60.03%	41021 / 68330
🔵	Functions	61.11%	7569 / 12384
🔵	Branches	48.92%	20041 / 40964

File Coverage

No changed files found.

Generated in workflow #3181 for commit 54cac74 by the Vitest Coverage Report Action