Files
sencho/docs
Anso 21ec5e7e0a fix(webhooks): harden trigger response surface (#1177)
* fix(webhooks): harden trigger response surface

Bundles six audit findings on the incoming-webhooks trigger path. All
changes preserve the documented happy path: a CI caller signing the exact
request body with the webhook secret still receives 202 Accepted.

* Uniform 404 on every unauthenticated rejection (missing webhook,
  disabled webhook, non-paid tier, missing signature header, missing
  raw body, signature mismatch). The four-way response surface previously
  let an unauthenticated probe enumerate webhook ids and fingerprint the
  instance's licence tier; callers now see one shape for any failed auth.
* Fail closed when express.json()'s verify callback did not capture the
  raw request body. Previously the handler fell back to
  JSON.stringify(req.body), which compares the HMAC against a
  re-serialised payload that is not byte-equal to what the client signed.
* Pass the already-loaded webhook through to WebhookService.execute()
  instead of re-fetching by id. Closes the delete-during-execution race
  where an admin deletion between the trigger handler's load and the async
  dispatch silently dropped the execution row. The webhook_executions
  table has ON DELETE CASCADE, so recordExecution now wraps the insert in
  try/catch and logs a warning when the FK constraint trips because the
  parent webhook was deleted mid-flight.
* Redact bearer tokens, JWTs, URL credentials, and homedir paths from
  error strings before persisting to webhook_executions.error. The
  execution history is readable by any paid user via GET /webhooks/:id/
  history; redactSensitiveText gains three home-directory patterns
  (/home/<user>, /Users/<user>, <drive>:\Users\<user>) and now runs on
  every error stored from this path.
* Cap webhook name at 100 characters on both POST and PUT, rejecting
  non-string and oversized values with 400 before they reach the DB.
* Validate the body's action override against a typed allowlist
  (isWebhookAction type guard) on the trigger endpoint, returning 400
  before queueing execution. An unknown override no longer reaches
  recordExecution as a stored failure row.

Tests updated to pass db.getWebhook(id)! instead of the raw id to the new
execute() signature. Docs at docs/features/webhooks.mdx updated to reflect
the new uniform 404 response, the new 400-on-invalid-action behaviour, and
a rewritten troubleshooting accordion that walks operators through every
cause of the uniform 404.

* test(webhooks): cover trigger handler auth, race, and redaction paths

Adds 21 vitest cases for the public webhook trigger handler and the
WebhookService.execute / recordExecution pipeline, plus 3 cases for the
new homedir patterns in redactSensitiveText.

webhooks-trigger.test.ts covers, per audit finding:

* M1 + H3 uniform 404: id unknown, webhook disabled, non-paid tier,
  missing signature header, missing rawBody, sha1= prefix, malformed
  hex signature, sig mismatch. Each asserts identical 404 body so a
  future regression that re-introduces 401 / 403 / PAID_REQUIRED breaks
  one of the 8 tests.
* Happy path: 202 with configured action, valid action override,
  unknown action override returns 400 after auth succeeds (L2),
  non-string action override returns 400.
* L1 name cap: POST and PUT both reject names over 100 chars and
  non-string names; 100-char boundary still accepted; PUT allows
  partial updates that omit name.
* M5 race: deleting the parent webhook before recordExecution runs no
  longer crashes the async dispatch; the FK cascade is swallowed with
  a console.warn, and a happy-path test pins the recordExecution row.
* M6 redaction: stubs ComposeService.runCommand to throw errors
  containing a bearer token and a homedir path, then asserts the
  persisted webhook_executions.error has both scrubbed.

safe-log.test.ts gains three unit tests pinning the new homedir
patterns in redactSensitiveText (Linux, macOS, Windows). The existing
credentials test is untouched.

Tests use prototype spies on FileSystemService and ComposeService (both
hand out a fresh instance per nodeId), so per-test mocks do not leak.
beforeEach restores all mocks and reapplies the LicenseService 'paid'
spy. Closes audit finding H2 (zero trigger-path test coverage).

* docs(webhooks): sync openapi spec with new trigger response surface

Brings docs/openapi.yaml in line with the behaviour changes from the
trigger hardening commit. Mintlify auto-generates the per-endpoint
reference pages from this spec, so the spec drift would surface as
wrong response codes in the public API reference.

POST /api/webhooks and PUT /api/webhooks/🆔
  * name: maxLength 100 (matches MAX_WEBHOOK_NAME_LENGTH on the route).
  * action enum: add git-pull (pre-existing omission; the route has
    always accepted it).
  * node_id: documented as an integer property (pre-existing omission).

POST /api/webhooks/:id/trigger:
  * requestBody required: true (body is now mandatory; the H3
    fail-closed branch rejects a missing rawBody).
  * action override: enum restricted to the allowlist.
  * 401 and 403 responses removed.
  * 404 response: description rewritten to reflect uniform-404
    behaviour; the body is { error: "Webhook not found or signature
    invalid" } for every unauthenticated reject.
  * 400 response added for an authenticated request whose action
    override is not in the allowlist.
2026-05-23 15:42:49 -04:00
..