Skip to content

Add extra HTTP headers on webhook POSTs. - #775

Merged
davidnewhall merged 3 commits into
mainfrom
feat/hook-headers
Sep 21, 2026
Merged

davidnewhall merged 3 commits into
mainfrom
feat/hook-headers

Conversation

@davidnewhall

Copy link
Copy Markdown
Collaborator

Webhook POSTs can send extra headers (Cloudflare Access, ntfy-compatible gateways, and similar). Custom headers are applied first; Content-Type and ntfy Bearer from token still win.

Summary

  • Named [webhook.<slug>.headers] table (env UN_WEBHOOK_<slug>_HEADERS_<Name>). Names are letters, digits, underscore, or hyphen.
  • Settings form shows Headers in Advanced when you pick a named template, set a custom template file, or headers already exist. Auto URL sniff still hides the editor.
  • Live GET blanks Authorization and other secret-looking header values; file GET keeps them for round-trip.

Test plan

  • Pick Discord (named template): Advanced shows Headers; add X-Api-Key and Test/Save; remote sees that header plus JSON Content-Type
  • ntfy named template with token and a custom Authorization header: Bearer from token wins
  • Auto URL sniff (paste Discord URL, template empty): Headers editor stays hidden
  • Custom template file: Headers editor shows even without a named template
  • Live config GET redacts Authorization; file GET still has the value
  • Command hooks form is unchanged

Made with Cursor

Named templates and custom files get an Advanced editor; Auto URL sniff still hides it. Content-Type and ntfy Bearer still win after those headers.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Four unresolved moderate issues remain involving header preservation, command-hook handling, control-character validation, and config-test validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds configurable extra HTTP headers to webhook POSTs, including precedence rules, validation, redaction, API support, and settings UI.

Changes:

  • Adds per-webhook header configuration and environment mapping.
  • Extends API testing, serialization, schema, and secret redaction.
  • Adds frontend editing, localization, tests, and documentation.
File Summary
pkg/​unpackerr/​openapi.json Documents header fields in the API schema.
pkg/​unpackerr/​configtest.go Supports headers in webhook test requests.
pkg/​unpackerr/​configput_test.go Tests live header redaction.
pkg/​unpackerr/​configapi.go Redacts secret header values.
pkg/​hooks/​worker_test.go Tests header precedence.
pkg/​hooks/​validate.go Validates webhook headers.
pkg/​hooks/​http.go Sends custom headers before protected built-ins.
pkg/​hooks/​headers.go Implements header validation and redaction.
pkg/​hooks/​headers_test.go Tests header helper behavior.
pkg/​hooks/​config.go Adds header configuration and cloning support.
pkg/​configdef/​live.go Serializes nested instance headers.
pkg/​configdef/​live_test.go Tests TOML header rendering.
pkg/​configdef/​definitions.yml Defines header configuration and environment variables.
frontend/​src/​pages/​settings/​HooksForm.svelte Adds the advanced headers editor and payload support.
frontend/​src/​lib/​types.ts Adds frontend header types.
frontend/​src/​lib/​slug.ts Adds header environment mapping.
frontend/​src/​lib/​i18n/​locales/​nl.json Adds Dutch header translations.
frontend/​src/​lib/​i18n/​locales/​es.json Adds Spanish header translations.
frontend/​src/​lib/​i18n/​locales/​en.json Adds English header translations.
frontend/​src/​lib/​i18n/​locales/​el.json Adds Greek header translations.
frontend/​src/​lib/​hooktmpl.ts Controls header-editor visibility.
examples/​unpackerr.conf.example Documents header configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/src/pages/settings/HooksForm.svelte Outdated
Comment thread pkg/unpackerr/configtest.go
}

if body.Headers != nil {
hook.Headers = maps.Clone(body.Headers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Separate issue from the validation one above: this replaces the live clone's merged headers wholesale. The omitted case keeps live, but the UI posts the map whenever the editor is visible (named template, custom file, or any header present) rather than omitting it, so env-overlay children never ride along in that case. The flagship flow is a named template plus UN_WEBHOOK_..._HEADERS_* (Cloudflare Access pairs set in env): Test then runs without the headers the live post carries. Post-save live is the posted map plus env children, so re-overlay env children from u.envUsed after applying the posted map, or have the UI merge env children into what it posts.

for _, hook := range items {
if hook != nil {
hook.Token = ""
hooks.RedactHeaderSecrets(hook.Headers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Live GET redacts here, but GET /api/n doesn't: envValueSecret only blanks PASSWORD/_PASS/_API_KEY/_TOKEN names, so any authenticated non-* caller reads full UN_WEBHOOK_..._HEADERS_Authorization, ..._CF-Access-Client-Secret, and ..._X-Api-Key values — the exact env vars this PR's docs recommend. Blank for non-* when the suffix contains _HEADERS_ and the child name matches the same secret heuristic as RedactHeaderSecrets; share one predicate between the two call sites.

@qwen-pr-bot qwen-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request changes — reviewed at head 98f4380, base 6892915 (single commit, 22 files).

Read the whole diff: the [webhook.<slug>.headers] table with per-key env overrides (UN_WEBHOOK_<slug>_HEADERS_<Name>), application order in setRequestHeaders, save-time validation, live-GET redaction with file-GET round-trip, the instance-nested TOML rendering in live.go, and the editor gating in HooksForm.svelte. Copilot reviewed first; its two findings are addressed below in current form.

Executed validation (advanced mode granted): at head, go test ./... all pass, go vet ./... clean, golangci-lint v2.13 zero issues, frontend npm run check zero errors, and go generate ./... reproduces the committed unpackerr.conf.example exactly. Public CI is green at head (golangci-lint ×4, gotest ×3 OSes, Snyk). I also ran six throwaway tests against the head commit, deleted afterwards: they confirm Copilot's data-loss finding end-to-end and produced findings 2 and 3 below.

1. Saving wipes file-defined headers whenever env owns any header child. Copilot's flag on HooksForm.svelte holds, reproduced end-to-end: with X-Static in [webhook.discord.headers] and UN_WEBHOOK_discord_HEADERS_X-Api-Key in env, savePayload strips the whole map and the PUT writes the body as the file document, so X-Static disappears from file and live. Env children are per-key overlays here, unlike scalar fields whose file value is inert once env owns them. I verified the suggested direction works: strip only for isCmd; keeping the file's children in the PUT body preserves them while env still overlays.

2. Test drops env-overlay children whenever the editor is visible (inline, configtest.go): the form posts the file-shaped map instead of omitting, so the flagship named-template-plus-env-headers case is tested without the env headers. Copilot's adjacent validation point also holds but is narrower than stated: only direct API callers can produce names the form blocks, and CRLF values are skipped silently at send while save rejects them.

3. GET /api/n exposes header-child secrets to non- callers* (inline, configapi.go): the same values this PR's own live-GET redaction treats as secret, and the same env vars its docs recommend.

Nit: headers: 'HEADERS' in HOOK_ENV_FIELDS implies a whole-map env var that cnfg never consumes (a bare UN_WEBHOOK_x_HEADERS is ignored without children), so that omitEnvFields entry never fires.

The design underneath is right: the new cnfg map overlay seeds from the existing entry instead of a zero value, which is what lets file and env headers compose, and the [webhook.discord.headers] render-with-parse-back test plus the live-redaction/file-round-trip split are exactly the right guards. Fix the three items above and this is in good shape.

omitEnvFields(envPrefix, row.slug, row.value, HOOK_ENV_FIELDS),
)
if (isCmd || envHas(envField(envPrefix, row.slug, 'HEADERS_*'))) {
hook = { ...hook, headers: undefined }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still here: env children are per-key overlays, not a whole-map replacement, so this strip wipes file-defined headers on save whenever any child is env-owned. Strip only for isCmd. (Still open from my last review.)

}

if body.Headers != nil {
hook.Headers = maps.Clone(body.Headers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still a wholesale replace: Test runs drop env-overlay children whenever the editor is visible. Re-overlay from u.envUsed after applying the posted map. (r4060343798)

for _, hook := range items {
if hook != nil {
hook.Token = ""
hooks.RedactHeaderSecrets(hook.Headers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still open: envValueSecret doesn't match _HEADERS_ children, so GET /api/n returns UN_WEBHOOK_..._HEADERS_Authorization and friends to non-* callers while live GET redacts them. (r4060344018)

@qwen-pr-bot qwen-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This push is a merge of main (xtractr bump, the recursion-exclude fix) and doesn't touch any of the flagged header code, so the three items from my last review are all still open at this head. Same verdict until they're fixed.

…v GET.

Do not wipe file headers when env only sets children, re-apply those children after a posted test map, and share SecretHeaderName with GET /api/config/env. Tighten labels that sit on help text instead of an input.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Four moderate issues remain in frontend environment-header handling and webhook environment-header parsing.

Review effort: Lite
Findings: None

Resolved since last review (2)

@davidnewhall
davidnewhall merged commit 7b0a621 into main Sep 21, 2026
30 checks passed
@davidnewhall
davidnewhall deleted the feat/hook-headers branch September 21, 2026 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants