diff --git a/backend/src/__tests__/safe-log.test.ts b/backend/src/__tests__/safe-log.test.ts index 53eec18f..19ae6854 100644 --- a/backend/src/__tests__/safe-log.test.ts +++ b/backend/src/__tests__/safe-log.test.ts @@ -39,4 +39,27 @@ describe('redactSensitiveText', () => { expect(text).not.toContain('user.windows'); expect(text).toContain('D:\\Users\\\\Sencho\\compose.yaml'); }); + + it('strips Windows-style homedir usernames when the drive letter is lowercase', () => { + const text = redactSensitiveText('failed: c:\\Users\\user.lowercase\\app\\compose.yaml'); + + expect(text).not.toContain('user.lowercase'); + expect(text).toContain('c:\\Users\\\\app\\compose.yaml'); + }); + + it('redacts Basic auth credentials embedded after Authorization header', () => { + const text = redactSensitiveText( + 'upstream 401: Authorization: Basic dXNlcjpwYXNzd29yZA== rejected by registry', + ); + + expect(text).not.toContain('dXNlcjpwYXNzd29yZA'); + expect(text).toContain('[redacted]'); + }); + + it('redacts a bare Basic auth scheme without an Authorization header', () => { + const text = redactSensitiveText('curl error: header Basic c2VjcmV0OnZhbHVl was rejected'); + + expect(text).not.toContain('c2VjcmV0OnZhbHVl'); + expect(text).toContain('Basic [redacted]'); + }); }); diff --git a/backend/src/__tests__/webhooks-trigger.test.ts b/backend/src/__tests__/webhooks-trigger.test.ts index e2b31b16..72a538a8 100644 --- a/backend/src/__tests__/webhooks-trigger.test.ts +++ b/backend/src/__tests__/webhooks-trigger.test.ts @@ -172,6 +172,70 @@ describe('POST /api/webhooks/:id/trigger: uniform unauthenticated 404 (M1, H3)', expect(res.status).toBe(404); expect(res.body).toEqual(expected); }); + + it('runs validateSignature even when the webhook id is unknown (timing oracle)', async () => { + const sigSpy = vi.spyOn(WebhookService.getInstance(), 'validateSignature'); + const body = '{"probe":true}'; + + const res = await request(app) + .post('/api/webhooks/9999999/trigger') + .set('Content-Type', 'application/json') + .set('X-Webhook-Signature', `sha256=${'a'.repeat(64)}`) + .send(body); + + expect(res.status).toBe(404); + expect(sigSpy).toHaveBeenCalledTimes(1); + const [usedPayload, usedSecret, usedSignature] = sigSpy.mock.calls[0]; + // HMAC ran against the attacker-supplied body, not a short-circuited + // empty string; the unknown-id path must do the same work as the + // wrong-signature path. + expect(usedPayload).toBe(body); + // Decoy secret got plumbed through so the HMAC compute was real. + expect(typeof usedSecret).toBe('string'); + expect((usedSecret as string).length).toBeGreaterThan(0); + expect(usedSignature).toContain('sha256='); + }); + + it('runs validateSignature even when the signature header is missing', async () => { + const sigSpy = vi.spyOn(WebhookService.getInstance(), 'validateSignature'); + const { id } = createWebhook(); + const body = '{}'; + + const res = await request(app) + .post(`/api/webhooks/${id}/trigger`) + .set('Content-Type', 'application/json') + .send(body); + + expect(res.status).toBe(404); + expect(sigSpy).toHaveBeenCalledTimes(1); + const [usedPayload, , usedSignature] = sigSpy.mock.calls[0]; + expect(usedPayload).toBe(body); + // Empty-string signature flowed into validateSignature instead of + // short-circuiting before the HMAC compute. + expect(usedSignature).toBe(''); + }); +}); + +describe('WebhookService.validateSignature: constant-time over input shape', () => { + it('returns false but does not throw on an empty signature string', () => { + const result = WebhookService.getInstance().validateSignature('payload', 'secret', ''); + expect(result).toBe(false); + }); + + it('returns false but does not throw on a wrong-prefix signature', () => { + const result = WebhookService.getInstance().validateSignature('payload', 'secret', 'sha1=abc'); + expect(result).toBe(false); + }); + + it('returns false but does not throw on a malformed hex signature', () => { + const result = WebhookService.getInstance().validateSignature('payload', 'secret', 'sha256=not-hex-data-shorter-than-64-chars'); + expect(result).toBe(false); + }); + + it('returns false but does not throw on a hex signature of the wrong length', () => { + const result = WebhookService.getInstance().validateSignature('payload', 'secret', `sha256=${'a'.repeat(32)}`); + expect(result).toBe(false); + }); }); describe('POST /api/webhooks/:id/trigger: authenticated happy path', () => { diff --git a/backend/src/routes/webhooks.ts b/backend/src/routes/webhooks.ts index 4acf2873..ee559fc1 100644 --- a/backend/src/routes/webhooks.ts +++ b/backend/src/routes/webhooks.ts @@ -147,6 +147,15 @@ webhooksRouter.get('/:id/history', authMiddleware, async (req: Request, res: Res // Every unauthenticated rejection returns the same 404 with the same body so // callers cannot enumerate webhook ids or fingerprint the instance's licence // tier from the response surface. Successful authentication still returns 202. +// +// The handler also runs the HMAC computation on every path (using a decoy +// secret and an empty buffer when the real ones are missing) so the wall- +// clock cost of a reject path matches the wall-clock cost of a real-shape +// wrong-secret path. Without this, repeated near-rate-limit probes with a +// large attacker-controlled body could distinguish a valid-and-enabled +// webhook id on a paid tier from the other reject cases via response +// latency. Timing now depends only on the size of the request body, which +// the attacker already controls and which reveals nothing webhook-specific. webhooksRouter.post('/:id/trigger', webhookTriggerLimiter, async (req: Request, res: Response): Promise => { const unauthenticated = (): void => { res.status(404).json({ error: 'Webhook not found or signature invalid' }); @@ -156,22 +165,27 @@ webhooksRouter.post('/:id/trigger', webhookTriggerLimiter, async (req: Request, const db = DatabaseService.getInstance(); const webhook = db.getWebhook(id); - if (!webhook || !webhook.enabled) return unauthenticated(); - if (LicenseService.getInstance().getTier() !== 'paid') return unauthenticated(); - - const signature = req.headers['x-webhook-signature'] as string; - if (!signature) return unauthenticated(); - - // Fail closed when the raw body was not captured. express.json()'s verify - // callback populates req.rawBody for every parsed body; an absent rawBody - // means the request had no body or an unparseable content-type. Falling - // back to JSON.stringify(req.body) would compare the HMAC against a - // re-serialised payload that is not byte-equal to what the client signed. - if (!req.rawBody) return unauthenticated(); - const payload = req.rawBody.toString('utf-8'); + const tier = LicenseService.getInstance().getTier(); + const signature = req.headers['x-webhook-signature'] as string | undefined; + // Unconditional HMAC. The decoy secret keeps the work non-skippable when + // the webhook does not exist; an empty buffer keeps it non-skippable + // when express.json()'s verify callback did not capture a body. The + // empty-string signature still flows through validateSignature, which + // is constant-time over every shape and will return false against the + // all-zero sentinel. Reject conditions are checked after the HMAC has + // already run so the timing of every reject path matches the timing of + // a real-shape wrong-secret request. const svc = WebhookService.getInstance(); - if (!svc.validateSignature(payload, webhook.secret, signature)) return unauthenticated(); + const payload = (req.rawBody ?? Buffer.alloc(0)).toString('utf-8'); + const secretForHmac = webhook?.secret ?? WebhookService.getDecoySecret(); + const sigOk = svc.validateSignature(payload, secretForHmac, signature ?? ''); + + if (!webhook || !webhook.enabled) return unauthenticated(); + if (tier !== 'paid') return unauthenticated(); + if (!signature) return unauthenticated(); + if (!req.rawBody) return unauthenticated(); + if (!sigOk) return unauthenticated(); // Use action from body if provided, otherwise use webhook default. // Validate against the action allowlist before queueing execution so an diff --git a/backend/src/services/WebhookService.ts b/backend/src/services/WebhookService.ts index 2c752967..b1e50934 100644 --- a/backend/src/services/WebhookService.ts +++ b/backend/src/services/WebhookService.ts @@ -18,6 +18,12 @@ const REMOTE_WEBHOOK_REQUEST_TIMEOUT_MS = 30_000; export class WebhookService { private static instance: WebhookService; + // Stable per-process decoy secret used to keep HMAC work non-skippable on + // reject paths (unknown webhook id, disabled, non-paid tier, etc.). Never + // accepts a signature: the trigger handler decides the final 202 / 404 + // outcome from independent conditions and only consults the HMAC result + // when every other check has already passed. + private static decoySecret: string | null = null; public static getInstance(): WebhookService { if (!WebhookService.instance) { @@ -26,27 +32,39 @@ export class WebhookService { return WebhookService.instance; } + public static getDecoySecret(): string { + if (!WebhookService.decoySecret) { + WebhookService.decoySecret = crypto.randomBytes(32).toString('hex'); + } + return WebhookService.decoySecret; + } + public generateSecret(): string { return crypto.randomBytes(32).toString('hex'); } public validateSignature(payload: string, secret: string, signature: string): boolean { + // Always perform HMAC and timingSafeEqual against a fixed-length + // buffer so the wall-clock cost of this call is independent of the + // shape of the input signature. Without this, the early-return paths + // (missing header, wrong prefix, malformed hex) would skip the HMAC + // and let an attacker distinguish those cases from a real-shape + // wrong-secret case through repeated near-rate-limit probes with a + // large attacker-controlled body. Timing now depends only on the + // size of `payload`, which the attacker already controls and which + // does not reveal anything about the webhook id or licence tier. + const expected = crypto.createHmac('sha256', secret).update(payload).digest(); + const provided = Buffer.alloc(32); + let formatOk = false; + const parts = signature.split('='); - if (parts.length !== 2 || parts[0] !== 'sha256') return false; - - const expected = crypto - .createHmac('sha256', secret) - .update(payload) - .digest('hex'); - - try { - return crypto.timingSafeEqual( - Buffer.from(expected, 'hex'), - Buffer.from(parts[1], 'hex'), - ); - } catch { - return false; + if (parts.length === 2 && parts[0] === 'sha256' && /^[0-9a-fA-F]{64}$/.test(parts[1])) { + provided.write(parts[1], 'hex'); + formatOk = true; } + + const sigEq = crypto.timingSafeEqual(expected, provided); + return formatOk && sigEq; } public async gitSourceExists(stackName: string, nodeId: number): Promise { diff --git a/backend/src/utils/safeLog.ts b/backend/src/utils/safeLog.ts index 7812e0f5..ef170f13 100644 --- a/backend/src/utils/safeLog.ts +++ b/backend/src/utils/safeLog.ts @@ -18,10 +18,11 @@ export function redactSensitiveText(value: unknown): string { const s = typeof value === 'string' ? value : String(value); return s .replace(/Bearer\s+[A-Za-z0-9\-._~+/=]+/gi, 'Bearer [redacted]') + .replace(/Basic\s+[A-Za-z0-9+/=]+/gi, 'Basic [redacted]') .replace(/[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}/g, '[redacted-jwt]') .replace(/https?:\/\/[^/\s:@]+:[^/\s@]+@/gi, 'https://[redacted]@') .replace(/((?:authorization|token|password|secret|api[_-]?key)\s*[:=]\s*)[^\s,;]+/gi, '$1[redacted]') .replace(/\/home\/[^/\s'"]+/g, '/home/') .replace(/\/Users\/[^/\s'"]+/g, '/Users/') - .replace(/([A-Z]):\\Users\\[^\\/\s'"]+/g, '$1:\\Users\\'); + .replace(/([A-Za-z]):\\Users\\[^\\/\s'"]+/g, '$1:\\Users\\'); } diff --git a/docs/features/webhooks.mdx b/docs/features/webhooks.mdx index 4728ca96..c28a73d3 100644 --- a/docs/features/webhooks.mdx +++ b/docs/features/webhooks.mdx @@ -4,7 +4,7 @@ description: Trigger stack actions from CI/CD pipelines via HTTP webhooks with H --- - 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. 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. + + 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. + + 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. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 7c8067c1..b01bd4f6 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -214,13 +214,16 @@ components: properties: id: type: integer + node_id: + type: integer + description: Node the webhook executes against. name: type: string stack_name: type: string action: type: string - enum: [deploy, restart, stop, start, pull] + enum: [deploy, restart, stop, start, pull, git-pull] secret: type: string description: Masked secret (e.g., `****...****`). Full secret is only returned on creation. @@ -1465,7 +1468,7 @@ paths: description: Node the webhook executes against. Defaults to the request's resolved node or the default node. responses: "201": - description: Webhook created. The `secret` field is the HMAC signing key — save it now. + description: Webhook created. The `secret` field is the HMAC signing key; save it now. content: application/json: schema: @@ -1476,7 +1479,7 @@ paths: type: integer secret: type: string - description: HMAC-SHA256 signing secret. Store securely — this is the only time it's returned in full. + description: HMAC-SHA256 signing secret. Store securely; this is the only time it's returned in full. "400": description: Validation error. content: @@ -1635,6 +1638,12 @@ paths: application/json: schema: $ref: "#/components/schemas/Error" + "429": + description: Rate limit exceeded. The trigger endpoint allows 500 requests per minute per source IP; CI/CD callers behind shared NAT may share that budget. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" "500": $ref: "#/components/responses/InternalError"