fix(webhooks): address Codex review of PR #1177 (#1181)

* fix(webhooks): close HMAC timing oracle on trigger reject paths

The trigger handler in PR #1177 returned a uniform 404 for every
unauthenticated rejection, but only the wrong-signature path computed
an HMAC over the request body. The other reject paths (unknown id,
disabled webhook, non-paid tier, missing X-Webhook-Signature header,
missing rawBody) short-circuited before any HMAC work. Repeated
near-rate-limit probes with a large attacker-controlled body could
distinguish a valid-and-enabled paid webhook id from the other reject
cases through response latency.

WebhookService.validateSignature is now constant-time over every input
shape: it always runs crypto.createHmac and crypto.timingSafeEqual
against fixed-length 32-byte buffers regardless of whether the
signature is missing, has the wrong prefix, is malformed hex, or is
the wrong length. The trigger handler calls it unconditionally before
any reject branch fires, using a stable per-process decoy secret
(WebhookService.getDecoySecret) when the webhook does not exist and an
empty buffer when the request has no body. Response timing now depends
only on the size of the request body, which the attacker already
controls and which reveals nothing webhook-specific.

Six new tests pin the behaviour: validateSignature is observed firing
on the unknown-id and missing-signature paths through a spy assertion,
and four direct-call tests confirm validateSignature returns false
without throwing for empty, wrong-prefix, malformed-hex, and
wrong-length signatures.

* fix(safe-log): redact Basic auth and lowercase Windows drive letters

The redactSensitiveText helper now covers two cases the prior chain
missed:

* Authorization: Basic <base64> previously left the base64 payload
  intact. The existing key/value regex caught only the literal word
  Basic before stopping at the space. A new Basic\s+[A-Za-z0-9+/=]+
  replacement runs before the key/value regex so the credential is
  scrubbed first.
* Windows homedir paths like c:\Users\<user>\... with a lowercase
  drive letter previously slipped through because the regex required
  [A-Z]. Changed to [A-Za-z] so both letter cases are covered.

Two new tests pin both fixes.

* docs(webhooks): document 429, fix shared schema, comply with D27/D31

* Trigger endpoint declares the 429 response that webhookTriggerLimiter
  can return (500 requests per minute per source IP); both
  docs/openapi.yaml and the response table in docs/features/webhooks.mdx
  carry the new row, and a new troubleshooting accordion explains the
  shared-NAT scenario.
* Shared Webhook schema in docs/openapi.yaml extends the action enum to
  include git-pull and documents the node_id property. The GET list
  endpoint returns these fields; the prior schema would have failed
  validation for any git-pull row.
* docs/features/webhooks.mdx:7 rewritten from a customer-side role
  enumeration ("non-admins on a paid tier can view the list but cannot
  manage it") to a single requirement statement ("Webhooks require a
  Skipper or Admiral license. Managing webhooks is admin-only.") per
  CLAUDE.md D27/D31; the prior phrasing was customer-side fence-spec.
* Two em dashes in webhook description strings I had touched in the
  prior OpenAPI sync commit replaced with semicolons per D18.
This commit is contained in:
Anso
2026-05-23 16:03:48 -04:00
committed by GitHub
parent bb44db0cb1
commit f03c9dc7b6
7 changed files with 167 additions and 33 deletions
+6 -1
View File
@@ -4,7 +4,7 @@ description: Trigger stack actions from CI/CD pipelines via HTTP webhooks with H
---
<Note>
Webhooks require a **Skipper** or **Admiral** license. Creating, editing, and deleting webhooks is restricted to admin users; non-admins on a paid tier can view the list but cannot manage it.
Webhooks require a **Skipper** or **Admiral** license. Managing webhooks is admin-only.
</Note>
Sencho webhooks let external systems trigger stack actions over HTTP. The typical use case: your CI pipeline builds a new image, then calls a Sencho webhook to deploy the updated stack, with no manual intervention required.
@@ -109,6 +109,7 @@ Valid override values are: `deploy`, `restart`, `stop`, `start`, `pull`, `git-pu
| `202 Accepted` | `{ "message": "Webhook accepted", "action": "deploy" }` | Signature is valid; the action is now running asynchronously. A 202 means accepted, not finished. |
| `400 Bad Request` | `{ "error": "action must be one of: deploy, restart, stop, start, pull, git-pull" }` | The request authenticated successfully but the body's `action` override was not in the allowlist. |
| `404 Not Found` | `{ "error": "Webhook not found or signature invalid" }` | The webhook id is unknown, the webhook is disabled, the signature header is missing, the request body was empty, or the signature did not match. Sencho returns the same response for every unauthenticated case so callers cannot probe ids or licence state. |
| `429 Too Many Requests` | `{ "error": "Too many webhook triggers. Please try again shortly." }` | The trigger endpoint allows 500 requests per minute per source IP. CI/CD callers behind shared NAT or datacenter egress may share that budget. |
## CI/CD integration examples
@@ -180,6 +181,10 @@ Sencho retains the last 100 executions per webhook and surfaces the 20 most rece
A 202 means the action was accepted, not that it produced a visible change. **Pull & Update** is a no-op if the registry has no newer image for any service in the stack; **Start** is a no-op on an already-running stack. Check the stack's **Activity** sheet to confirm what actually ran, and the webhook's **Recent executions** for the action's status and duration.
</Accordion>
<Accordion title="My trigger returns 429 'Too many webhook triggers'.">
The trigger endpoint is rate limited to 500 requests per minute per source IP. CI/CD runners behind shared NAT or a datacenter egress pool all share that budget. Either space the calls out, send them from a less crowded egress, or split the work across multiple webhooks if a single workflow fires hundreds of triggers in close succession.
</Accordion>
<Accordion title="I cannot create a Git source sync webhook for this stack.">
The **Git source sync** action can only be selected for a stack that already has a [Git source](/features/git-sources) attached. If you try to create one without it, the form returns a validation error. Attach a Git source to the stack in its Compose editor, then create the webhook.
</Accordion>