fix(webhook): fail-closed on missing secret; refresh CLAUDE.md module map #102

Merged

charles merged 1 commit from fix/webhook-fail-closed into main

2026-04-19 20:04:54 +00:00

claude-desktop commented

2026-04-19 19:57:49 +00:00

Collaborator

Summary

Audit finding: src/webhook.ts was silently fail-open when the webhook secret was missing. loadWebhookConfig swallows the secret-file read error (logs a warning, continues with secret=""), and verifySignature returned true on empty secret — so every unsigned payload was accepted without verification. The warning scrolls past in startup logs and then disappears.

Also rolling in the CLAUDE.md module-map refresh since it's a one-file doc change and belongs in the same cleanup pass.

Changes

src/webhook.ts — verifySignature now returns false when the secret is empty, with a [webhook] rejecting payload — no secret configured warning on each rejection so the operator sees the misconfig as loud 403s instead of silent acceptance.
src/webhook.test.ts — point the test config at a real secret file and sign every makeRequest body. Added three tests:
- Invalid signature rejected (403)
- Missing x-forgejo-signature header rejected (403)
- Empty-secret config rejected (403) — direct regression guard for the fail-open bug
CLAUDE.md — drop the "Single-file service — everything in src/main.ts" claim (dead since the refactor). Added a Modules table documenting the 16 production files and their responsibilities.

Test plan

just qa — 276 pass, 0 fail
New regression test confirms fail-closed on empty secret
All 39 existing webhook.test.ts tests continue to pass with signing

Out of scope

Making loadWebhookConfig throw on missing secret file (would break local dev without a secret — current degrade-and-warn behaviour preserved)
Other audit findings (test coverage, route-handler extraction) — queued as follow-up PRs

🤖 Generated with Claude Code

## Summary Audit finding: `src/webhook.ts` was silently fail-open when the webhook secret was missing. `loadWebhookConfig` swallows the secret-file read error (logs a warning, continues with `secret=""`), and `verifySignature` returned `true` on empty secret — so every unsigned payload was accepted without verification. The warning scrolls past in startup logs and then disappears. Also rolling in the CLAUDE.md module-map refresh since it's a one-file doc change and belongs in the same cleanup pass. ## Changes - **`src/webhook.ts`** — `verifySignature` now returns `false` when the secret is empty, with a `[webhook] rejecting payload — no secret configured` warning on each rejection so the operator sees the misconfig as loud 403s instead of silent acceptance. - **`src/webhook.test.ts`** — point the test config at a real secret file and sign every `makeRequest` body. Added three tests: - Invalid signature rejected (403) - Missing `x-forgejo-signature` header rejected (403) - Empty-secret config rejected (403) — direct regression guard for the fail-open bug - **`CLAUDE.md`** — drop the "Single-file service — everything in `src/main.ts`" claim (dead since the refactor). Added a Modules table documenting the 16 production files and their responsibilities. ## Test plan - [x] `just qa` — 276 pass, 0 fail - [x] New regression test confirms fail-closed on empty secret - [x] All 39 existing `webhook.test.ts` tests continue to pass with signing ## Out of scope - Making `loadWebhookConfig` throw on missing secret file (would break local dev without a secret — current degrade-and-warn behaviour preserved) - Other audit findings (test coverage, route-handler extraction) — queued as follow-up PRs 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-19 19:57:50 +00:00

fix(webhook): fail-closed on missing secret; refresh CLAUDE.md module map

qa / qa (pull_request) Successful in 2m33s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

ea2f103ac1

`verifySignature` returned `true` when the webhook secret was empty
(e.g. `webhook_secret_file` missing — `loadWebhookConfig` swallows the
read error and leaves secret=""). Startup logged a warning, then every
payload was accepted without verification. Switch to fail-closed and
warn on each rejection so the misconfig surfaces as visible 403s
instead of silent acceptance.

- src/webhook.ts: `!secret` now returns false + warns, not true.
- src/webhook.test.ts: point test config at a real secret file, sign
  every `makeRequest` body. Added three regression tests: invalid
  signature rejected, missing signature rejected, empty-secret config
  rejected (this last one is the fail-open fix's regression guard).
- CLAUDE.md: drop the "single-file service" claim and add a Modules
  table covering all 16 production files (matches current layout).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

charles merged commit 16fa720934 into main

2026-04-19 20:04:54 +00:00

charles deleted branch fix/webhook-fail-closed

2026-04-19 20:04:54 +00:00

charles referenced this pull request from a commit

2026-04-19 20:04:55 +00:00

fix(webhook): fail-closed on missing secret; refresh CLAUDE.md module map (#102)

claude-desktop referenced this pull request

2026-04-19 20:23:07 +00:00

test(coverage): forgejo-api + cleanup tests; migrate cleanup consumers off mock.module #103

charles referenced this pull request from a commit

2026-04-19 20:35:23 +00:00