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.
This commit is contained in:
Anso
2026-05-23 15:42:49 -04:00
committed by GitHub
parent a9282671d5
commit 21ec5e7e0a
8 changed files with 547 additions and 64 deletions
+23
View File
@@ -16,4 +16,27 @@ describe('redactSensitiveText', () => {
expect(text).not.toContain('secret123');
expect(text).not.toContain('hunter2');
});
it('strips Linux-style homedir usernames while keeping the /home/ prefix', () => {
const text = redactSensitiveText('compose error reading /home/user-linux/docker/compose.yaml');
expect(text).not.toContain('user-linux');
expect(text).toContain('/home/<user>/docker/compose.yaml');
});
it('strips macOS-style homedir usernames while keeping the /Users/ prefix', () => {
const text = redactSensitiveText('failed to open /Users/user.macos/Projects/app/.env');
expect(text).not.toContain('user.macos');
expect(text).toContain('/Users/<user>/Projects/app/.env');
});
it('strips Windows-style homedir usernames while preserving the drive letter', () => {
const text = redactSensitiveText(
'ENOENT: no such file or directory, open \'D:\\Users\\user.windows\\Sencho\\compose.yaml\'',
);
expect(text).not.toContain('user.windows');
expect(text).toContain('D:\\Users\\<user>\\Sencho\\compose.yaml');
});
});
@@ -173,7 +173,7 @@ describe('node-aware Git source webhooks', () => {
enabled: true,
});
const result = await WebhookService.getInstance().execute(webhookId, 'git-pull', 'test');
const result = await WebhookService.getInstance().execute(db.getWebhook(webhookId)!, 'git-pull', 'test');
expect(result.success).toBe(false);
expect(result.error).toMatch(/unreachable|configured/i);
@@ -206,7 +206,7 @@ describe('node-aware Git source webhooks', () => {
signal?.addEventListener('abort', () => reject(new Error('aborted')));
}));
const pending = WebhookService.getInstance().execute(webhookId, 'git-pull', 'test');
const pending = WebhookService.getInstance().execute(db.getWebhook(webhookId)!, 'git-pull', 'test');
await vi.advanceTimersByTimeAsync(30_000);
const result = await pending;
@@ -0,0 +1,397 @@
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
import request from 'supertest';
import crypto from 'crypto';
import jwt from 'jsonwebtoken';
import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb';
let tmpDir: string;
let app: import('express').Express;
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
let LicenseService: typeof import('../services/LicenseService').LicenseService;
let WebhookService: typeof import('../services/WebhookService').WebhookService;
function adminToken(): string {
return jwt.sign({ username: TEST_USERNAME, role: 'admin' }, TEST_JWT_SECRET, { expiresIn: '1m' });
}
function sign(rawBody: string, secret: string): string {
return 'sha256=' + crypto.createHmac('sha256', secret).update(rawBody).digest('hex');
}
interface WebhookFixture {
id: number;
secret: string;
}
function createWebhook(opts: { action?: string; enabled?: boolean; name?: string; stack?: string } = {}): WebhookFixture {
const db = DatabaseService.getInstance();
const nodeId = db.getDefaultNode()!.id!;
const secret = WebhookService.getInstance().generateSecret();
const id = db.addWebhook({
node_id: nodeId,
name: opts.name ?? 'trigger-test',
stack_name: opts.stack ?? 'missing-stack',
action: (opts.action ?? 'restart') as never,
secret,
enabled: opts.enabled ?? true,
});
return { id, secret };
}
beforeAll(async () => {
tmpDir = await setupTestDb();
({ app } = await import('../index'));
({ DatabaseService } = await import('../services/DatabaseService'));
({ LicenseService } = await import('../services/LicenseService'));
({ WebhookService } = await import('../services/WebhookService'));
});
afterAll(() => {
cleanupTestDb(tmpDir);
});
beforeEach(() => {
vi.restoreAllMocks();
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
vi.spyOn(LicenseService.getInstance(), 'getVariant').mockReturnValue('skipper');
});
describe('POST /api/webhooks/:id/trigger: uniform unauthenticated 404 (M1, H3)', () => {
const expected = { error: 'Webhook not found or signature invalid' };
it('returns 404 when the webhook id is unknown', async () => {
const body = '{}';
const res = await request(app)
.post('/api/webhooks/9999999/trigger')
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, 'whatever'))
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
it('returns the same 404 when the webhook exists but is disabled', async () => {
const { id, secret } = createWebhook({ enabled: false });
const body = '{}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
it('returns the same 404 when the licence tier is not paid', async () => {
const { id, secret } = createWebhook();
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('community');
const body = '{}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
// The forbidden code from the prior surface must not leak.
expect(res.body.code).toBeUndefined();
});
it('returns the same 404 when the X-Webhook-Signature header is missing', async () => {
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(res.body).toEqual(expected);
});
it('returns the same 404 when the request has no body (H3 fail-closed)', async () => {
const { id, secret } = createWebhook();
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
// No Content-Type → express.json() does not run verify, so
// req.rawBody is never populated. The handler must fail closed
// instead of re-stringifying req.body to compute the HMAC.
.set('X-Webhook-Signature', sign('', secret));
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
it('returns the same 404 for a signature with the wrong prefix', async () => {
const { id, secret } = createWebhook();
const body = '{}';
const hex = crypto.createHmac('sha256', secret).update(body).digest('hex');
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', `sha1=${hex}`)
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
it('returns the same 404 for a malformed hex signature', async () => {
const { id } = createWebhook();
const body = '{}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', 'sha256=notavalidhex-zzzz')
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
it('returns the same 404 when the signature does not match the body', async () => {
const { id } = createWebhook();
const body = '{"foo":"bar"}';
const wrongSig = sign(body, 'wrong-secret-of-equal-length-as-the-real-one-1234');
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', wrongSig)
.send(body);
expect(res.status).toBe(404);
expect(res.body).toEqual(expected);
});
});
describe('POST /api/webhooks/:id/trigger: authenticated happy path', () => {
it('returns 202 and echoes the configured action', async () => {
const { id, secret } = createWebhook({ action: 'stop' });
const body = '{}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
expect(res.status).toBe(202);
expect(res.body).toMatchObject({ message: 'Webhook accepted', action: 'stop' });
});
it('accepts a valid action override and echoes it', async () => {
const { id, secret } = createWebhook({ action: 'restart' });
const body = '{"action":"start"}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
expect(res.status).toBe(202);
expect(res.body).toMatchObject({ action: 'start' });
});
it('rejects an unknown action override with 400 after the signature passes (L2)', async () => {
const { id, secret } = createWebhook();
const body = '{"action":"nuke-the-cluster"}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
// Auth succeeded, so the caller learns the action was rejected.
// Pre-auth callers would still get the uniform 404 instead.
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/action must be one of/);
});
it('rejects a non-string action override with 400', async () => {
const { id, secret } = createWebhook();
const body = '{"action":42}';
const res = await request(app)
.post(`/api/webhooks/${id}/trigger`)
.set('Content-Type', 'application/json')
.set('X-Webhook-Signature', sign(body, secret))
.send(body);
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/action must be one of/);
});
});
describe('POST /api/webhooks: name length cap (L1)', () => {
it('rejects a name longer than 100 characters', async () => {
const res = await request(app)
.post('/api/webhooks')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
name: 'a'.repeat(101),
stack_name: 'irrelevant-stack',
action: 'restart',
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/100 characters or fewer/);
});
it('rejects a non-string name', async () => {
const res = await request(app)
.post('/api/webhooks')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
name: { obj: 'not-a-string' },
stack_name: 'irrelevant-stack',
action: 'restart',
});
expect(res.status).toBe(400);
// 'name, stack_name, and action are required' catches this when name is
// truthy-but-not-a-string before the length check; either error message
// is acceptable for non-string input.
expect(res.body.error).toBeTruthy();
});
it('accepts a name at the 100-character boundary', async () => {
const res = await request(app)
.post('/api/webhooks')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
name: 'a'.repeat(100),
stack_name: 'boundary-stack',
action: 'restart',
});
expect(res.status).toBe(201);
expect(typeof res.body.secret).toBe('string');
});
});
describe('PUT /api/webhooks/:id: name length cap (L1)', () => {
it('rejects updating name to over 100 characters', async () => {
const { id } = createWebhook();
const res = await request(app)
.put(`/api/webhooks/${id}`)
.set('Authorization', `Bearer ${adminToken()}`)
.send({ name: 'b'.repeat(101) });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/100 characters or fewer/);
});
it('allows partial updates that omit name', async () => {
const { id } = createWebhook({ enabled: true });
const res = await request(app)
.put(`/api/webhooks/${id}`)
.set('Authorization', `Bearer ${adminToken()}`)
.send({ enabled: false });
expect(res.status).toBe(200);
expect(DatabaseService.getInstance().getWebhook(id)?.enabled).toBe(false);
});
});
describe('WebhookService.execute: delete-during-execution race (M5)', () => {
it('does not crash when the parent webhook is deleted before recordExecution runs', async () => {
// The webhook targets a stack that does not exist on disk, so
// executeLocal fails fast at the FileSystemService.getStacks() check
// and tries to write a failure row to webhook_executions. By the time
// that insert fires the parent row is gone, so the FK CASCADE makes
// the insert fail. The fix is for recordExecution to swallow that
// error with a console.warn instead of crashing the async dispatch.
const { id } = createWebhook({ action: 'restart', stack: 'definitely-not-on-disk' });
const webhook = DatabaseService.getInstance().getWebhook(id)!;
// Mid-flight delete.
DatabaseService.getInstance().deleteWebhook(id);
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
await expect(
WebhookService.getInstance().execute(webhook, 'restart', 'test', true),
).resolves.toMatchObject({ success: false });
// recordExecution caught the FK error and logged a single warning.
const calls = warnSpy.mock.calls.map(args => args.join(' '));
expect(calls.some(line => line.includes(`webhook ${id}`))).toBe(true);
});
it('records the execution row when the webhook persists through execution', async () => {
const { id } = createWebhook({ action: 'restart', stack: 'definitely-not-on-disk' });
const webhook = DatabaseService.getInstance().getWebhook(id)!;
const result = await WebhookService.getInstance().execute(webhook, 'restart', 'test', true);
expect(result.success).toBe(false);
// Filter by webhook_id rather than asserting toHaveLength on the
// entire history: getWebhookExecutions already scopes to this row's
// id, but tests in this file share a baseline DB and an earlier
// run could re-use an id range. Read it positionally instead.
const history = DatabaseService.getInstance().getWebhookExecutions(id);
expect(history.length).toBeGreaterThanOrEqual(1);
expect(history[0].status).toBe('failure');
expect(history[0].error).toMatch(/not found/i);
});
});
describe('webhook_executions.error redaction (M6)', () => {
// Both tests force executeLocal's switch-statement try/catch path so the
// raw upstream error flows through getErrorMessage -> recordExecution ->
// redactSensitiveText. FileSystemService.getStacks is stubbed to claim the
// stack exists, and ComposeService.runCommand is stubbed to throw the
// sensitive content. Spies attach to the prototypes because both
// singletons hand out fresh instances per nodeId.
it('strips bearer tokens before persisting the execution error', async () => {
const stack = 'redact-stack-bearer';
const { id } = createWebhook({ action: 'restart', stack });
const webhook = DatabaseService.getInstance().getWebhook(id)!;
const fs = await import('../services/FileSystemService');
const compose = await import('../services/ComposeService');
vi.spyOn(fs.FileSystemService.prototype, 'getStacks').mockResolvedValue([stack]);
vi.spyOn(compose.ComposeService.prototype, 'runCommand').mockRejectedValue(
new Error('upstream rejected: Authorization: Bearer abcdef1234567890tokenvalue'),
);
const result = await WebhookService.getInstance().execute(webhook, 'restart', 'test', true);
expect(result.success).toBe(false);
const history = DatabaseService.getInstance().getWebhookExecutions(id);
expect(history[0].error).toBeTruthy();
expect(history[0].error).not.toContain('abcdef1234567890tokenvalue');
expect(history[0].error).toContain('[redacted]');
});
it('strips homedir paths before persisting the execution error', async () => {
const stack = 'redact-stack-home';
const { id } = createWebhook({ action: 'restart', stack });
const webhook = DatabaseService.getInstance().getWebhook(id)!;
const fs = await import('../services/FileSystemService');
const compose = await import('../services/ComposeService');
vi.spyOn(fs.FileSystemService.prototype, 'getStacks').mockResolvedValue([stack]);
vi.spyOn(compose.ComposeService.prototype, 'runCommand').mockRejectedValue(
new Error('compose error reading /home/user-redact-target/docker/compose.yaml'),
);
const result = await WebhookService.getInstance().execute(webhook, 'restart', 'test', true);
expect(result.success).toBe(false);
const history = DatabaseService.getInstance().getWebhookExecutions(id);
expect(history[0].error).toBeTruthy();
expect(history[0].error).not.toContain('user-redact-target');
expect(history[0].error).toContain('/home/<user>');
});
});
+53 -24
View File
@@ -1,12 +1,17 @@
import { Router, type Request, type Response } from 'express';
import { DatabaseService } from '../services/DatabaseService';
import { DatabaseService, type WebhookAction } from '../services/DatabaseService';
import { WebhookService } from '../services/WebhookService';
import { LicenseService } from '../services/LicenseService';
import { authMiddleware } from '../middleware/auth';
import { requirePaid, requireAdmin } from '../middleware/tierGates';
import { webhookTriggerLimiter } from '../middleware/rateLimiters';
const VALID_WEBHOOK_ACTIONS = ['deploy', 'restart', 'stop', 'start', 'pull', 'git-pull'];
const VALID_WEBHOOK_ACTIONS: readonly WebhookAction[] = ['deploy', 'restart', 'stop', 'start', 'pull', 'git-pull'];
const MAX_WEBHOOK_NAME_LENGTH = 100;
function isWebhookAction(value: unknown): value is WebhookAction {
return typeof value === 'string' && (VALID_WEBHOOK_ACTIONS as readonly string[]).includes(value);
}
export const webhooksRouter = Router();
@@ -31,6 +36,10 @@ webhooksRouter.post('/', authMiddleware, async (req: Request, res: Response): Pr
res.status(400).json({ error: 'name, stack_name, and action are required' });
return;
}
if (typeof name !== 'string' || name.length > MAX_WEBHOOK_NAME_LENGTH) {
res.status(400).json({ error: `name must be a string of ${MAX_WEBHOOK_NAME_LENGTH} characters or fewer` });
return;
}
if (!VALID_WEBHOOK_ACTIONS.includes(action)) {
res.status(400).json({ error: `action must be one of: ${VALID_WEBHOOK_ACTIONS.join(', ')}` });
return;
@@ -73,6 +82,10 @@ webhooksRouter.put('/:id', authMiddleware, async (req: Request, res: Response):
if (!webhook) { res.status(404).json({ error: 'Webhook not found' }); return; }
const { name, stack_name, action, enabled, node_id } = req.body;
if (name !== undefined && (typeof name !== 'string' || name.length > MAX_WEBHOOK_NAME_LENGTH)) {
res.status(400).json({ error: `name must be a string of ${MAX_WEBHOOK_NAME_LENGTH} characters or fewer` });
return;
}
if (node_id !== undefined && !Number.isInteger(node_id)) {
res.status(400).json({ error: 'node_id must be an integer' });
return;
@@ -130,45 +143,61 @@ webhooksRouter.get('/:id/history', authMiddleware, async (req: Request, res: Res
});
// Public: authenticated via HMAC signature, not session cookie.
//
// 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.
webhooksRouter.post('/:id/trigger', webhookTriggerLimiter, async (req: Request, res: Response): Promise<void> => {
const unauthenticated = (): void => {
res.status(404).json({ error: 'Webhook not found or signature invalid' });
};
try {
const id = parseInt(req.params.id as string, 10);
const db = DatabaseService.getInstance();
const webhook = db.getWebhook(id);
if (!webhook || !webhook.enabled) {
res.status(404).json({ error: 'Webhook not found or disabled' });
return;
}
// Trigger only works with an active Skipper or Admiral license.
if (LicenseService.getInstance().getTier() !== 'paid') {
res.status(403).json({ error: 'This feature requires a Skipper or Admiral license.', code: 'PAID_REQUIRED' });
return;
}
if (!webhook || !webhook.enabled) return unauthenticated();
if (LicenseService.getInstance().getTier() !== 'paid') return unauthenticated();
const signature = req.headers['x-webhook-signature'] as string;
if (!signature) {
res.status(401).json({ error: 'Missing X-Webhook-Signature header' });
return;
}
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 rawBody = req.rawBody?.toString('utf-8') ?? JSON.stringify(req.body ?? {});
const svc = WebhookService.getInstance();
if (!svc.validateSignature(rawBody, webhook.secret, signature)) {
res.status(401).json({ error: 'Invalid signature' });
return;
}
if (!svc.validateSignature(payload, webhook.secret, signature)) return unauthenticated();
// Use action from body if provided, otherwise use webhook default.
const action = req.body?.action || webhook.action;
// Validate against the action allowlist before queueing execution so an
// attacker-supplied string never reaches recordExecution as a stored
// failure label.
const overrideAction = (req.body as { action?: unknown } | undefined)?.action;
let action: WebhookAction = webhook.action;
if (overrideAction !== undefined) {
if (!isWebhookAction(overrideAction)) {
res.status(400).json({ error: `action must be one of: ${VALID_WEBHOOK_ACTIONS.join(', ')}` });
return;
}
action = overrideAction;
}
const triggerSource = req.headers['user-agent'] || req.ip || null;
// Execute asynchronously; return 202 immediately.
res.status(202).json({ message: 'Webhook accepted', action });
const atomic = LicenseService.getInstance().getTier() === 'paid';
svc.execute(id, action, triggerSource, atomic).catch(err => {
// Pass the already-loaded webhook through so execute() never re-fetches
// by id. If an admin deletes the row between this line and the async
// dispatch the action still completes and recordExecution swallows the
// FK error from the CASCADE. atomic is unconditionally true: the tier
// gate above already rejected any caller without a Skipper/Admiral
// licence, so the deploy/pull paths always run in atomic mode here.
svc.execute(webhook, action, triggerSource, true).catch(err => {
console.error(`[Webhooks] Execution error for webhook ${id}:`, err);
});
} catch (error) {
+35 -13
View File
@@ -1,12 +1,13 @@
import crypto from 'crypto';
import { ComposeService } from './ComposeService';
import { DatabaseService } from './DatabaseService';
import { DatabaseService, type Webhook } from './DatabaseService';
import { FileSystemService } from './FileSystemService';
import { GitSourceService } from './GitSourceService';
import { LicenseService } from './LicenseService';
import { PROXY_TIER_HEADER, PROXY_VARIANT_HEADER } from './license-headers';
import { NodeRegistry } from './NodeRegistry';
import { getErrorMessage } from '../utils/errors';
import { redactSensitiveText } from '../utils/safeLog';
import { isValidStackName } from '../utils/validation';
import { assertPolicyGateAllows, buildSystemPolicyGateOptions } from '../helpers/policyGate';
@@ -58,13 +59,15 @@ export class WebhookService {
}
public async execute(
webhookId: number,
webhook: Webhook,
action: string,
triggerSource: string | null,
atomic?: boolean,
): Promise<ExecutionResult> {
const webhook = DatabaseService.getInstance().getWebhook(webhookId);
if (!webhook) throw new Error('Webhook not found');
if (webhook.id === undefined) {
throw new Error('Webhook must be loaded from the database before execution');
}
const webhookId = webhook.id;
const nodeId = webhook.node_id || NodeRegistry.getInstance().getDefaultNodeId();
const node = NodeRegistry.getInstance().getNode(nodeId);
@@ -283,14 +286,33 @@ export class WebhookService {
durationMs: number,
error: string | null,
): void {
DatabaseService.getInstance().addWebhookExecution({
webhook_id: webhookId,
action,
status,
trigger_source: triggerSource,
duration_ms: durationMs,
error,
executed_at: Date.now(),
});
// Execution history is readable by any paid user; scrub bearer tokens,
// JWTs, URL credentials, and homedir paths before persisting so a
// compose / remote-node error surfacing on the dashboard cannot leak
// operator secrets or infrastructure details.
const safeError = error === null ? null : redactSensitiveText(error);
try {
DatabaseService.getInstance().addWebhookExecution({
webhook_id: webhookId,
action,
status,
trigger_source: triggerSource,
duration_ms: durationMs,
error: safeError,
executed_at: Date.now(),
});
} catch (err) {
// The webhook_executions table has ON DELETE CASCADE on webhook_id,
// so a delete that races an in-flight execution removes the parent
// row and any insert here fails the FK constraint. Swallow that
// race: the trigger already returned 202 and the action either
// ran or failed before reaching this point. Other write errors
// are still worth logging as warnings so a structural problem
// does not go silent.
console.warn(
`[Webhooks] Could not record execution for webhook ${webhookId} ` +
`(parent webhook may have been deleted mid-flight): ${getErrorMessage(err, 'Unknown error')}`,
);
}
}
}
+4 -1
View File
@@ -20,5 +20,8 @@ export function redactSensitiveText(value: unknown): string {
.replace(/Bearer\s+[A-Za-z0-9\-._~+/=]+/gi, 'Bearer [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(/((?:authorization|token|password|secret|api[_-]?key)\s*[:=]\s*)[^\s,;]+/gi, '$1[redacted]')
.replace(/\/home\/[^/\s'"]+/g, '/home/<user>')
.replace(/\/Users\/[^/\s'"]+/g, '/Users/<user>')
.replace(/([A-Z]):\\Users\\[^\\/\s'"]+/g, '$1:\\Users\\<user>');
}
+10 -13
View File
@@ -100,16 +100,15 @@ By default, the webhook executes the action configured at creation time. You can
{ "action": "restart" }
```
Valid override values are: `deploy`, `restart`, `stop`, `start`, `pull`, `git-pull`. An unknown value causes the execution to fail and appear with an error in **Recent executions**.
Valid override values are: `deploy`, `restart`, `stop`, `start`, `pull`, `git-pull`. An unknown value is rejected with `400 Bad Request` before execution is queued.
### Responses
| Status | Body | Meaning |
|--------|------|---------|
| `202 Accepted` | `{ "message": "Webhook accepted", "action": "deploy" }` | Signature is valid; the action is now running asynchronously. A 202 means accepted, not finished. |
| `401 Unauthorized` | `{ "error": "Missing X-Webhook-Signature header" }` | The request omitted the signature header. |
| `401 Unauthorized` | `{ "error": "Invalid signature" }` | The provided signature did not match the expected HMAC. |
| `404 Not Found` | `{ "error": "Webhook not found or disabled" }` | The webhook id is unknown or its enable toggle is off. |
| `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. |
## CI/CD integration examples
@@ -166,19 +165,17 @@ Sencho retains the last 100 executions per webhook and surfaces the 20 most rece
The page requires a **Skipper** or **Admiral** license. If you are on a paid tier but the node switcher in the top-left shows a remote node, switch to **Local** to reveal the page.
</Accordion>
<Accordion title="My trigger returns '401 Invalid signature'.">
The signature must be computed over the **exact raw bytes** of the request body. Common causes:
<Accordion title="My trigger returns 404 'Webhook not found or signature invalid'.">
The trigger endpoint returns the same 404 for every unauthenticated case, so the response alone will not tell you which check failed. Work through this checklist:
- The CI step JSON-encodes the body before signing but sends a different body to curl (or vice versa). Sign the same string you send.
- The shell adds a trailing newline (use `echo -n` rather than `echo`).
- The header is missing the `sha256=` prefix; the full value is `sha256={hex}`.
- The id in the URL is wrong. Copy the trigger URL straight from the webhook card in **Settings → Alerts → Webhooks**.
- The webhook's enable toggle is off. Check the **On / Off** switch on the card.
- The request omitted the `X-Webhook-Signature` header. Confirm the full value, including the `sha256=` prefix; the header value is `sha256={hex}`.
- The request had no body. Send at least `{}` and sign that exact byte sequence.
- The signature was computed over different bytes from what the request carried. Sign the exact string you send. Common offenders: the CI step JSON-encodes the body before signing but pipes a different body to curl, or the shell adds a trailing newline (use `echo -n` rather than `echo`).
- The webhook secret used to sign does not match the secret stored in Sencho. The secret is shown once at creation; if you lost it, delete the webhook and create a new one.
</Accordion>
<Accordion title="My trigger returns '404 Webhook not found or disabled'.">
The id in the URL is wrong, or the webhook's enable toggle is off. Copy the trigger URL from the card and check the **On / Off** switch on the webhook in **Settings → Alerts → Webhooks**.
</Accordion>
<Accordion title="The action returns 202 but the stack does not change.">
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>
+23 -11
View File
@@ -1447,6 +1447,7 @@ paths:
properties:
name:
type: string
maxLength: 100
example: GitHub Deploy Hook
stack_name:
type: string
@@ -1454,11 +1455,14 @@ paths:
example: my-app
action:
type: string
enum: [deploy, restart, stop, start, pull]
description: Action to execute when the webhook is triggered.
enum: [deploy, restart, stop, start, pull, git-pull]
description: Action to execute when the webhook is triggered. `git-pull` requires a Git source attached to the stack.
enabled:
type: boolean
default: true
node_id:
type: integer
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.
@@ -1501,13 +1505,17 @@ paths:
properties:
name:
type: string
maxLength: 100
stack_name:
type: string
action:
type: string
enum: [deploy, restart, stop, start, pull]
enum: [deploy, restart, stop, start, pull, git-pull]
enabled:
type: boolean
node_id:
type: integer
description: Retarget the webhook to a different node.
responses:
"200":
description: Webhook updated.
@@ -1577,15 +1585,20 @@ paths:
summary: Trigger webhook
description: |
Externally triggers a webhook action. This endpoint is public but requires a valid
HMAC-SHA256 signature in the `X-Webhook-Signature` header.
HMAC-SHA256 signature in the `X-Webhook-Signature` header. Sign the exact raw bytes
of the request body; an empty body is rejected.
Compute the signature as: `sha256=` + HMAC-SHA256(raw_request_body, webhook_secret).
Every unauthenticated rejection (unknown id, disabled webhook, non-paid licence,
missing or invalid signature, empty body) returns the same `404` response so callers
cannot enumerate webhook ids or fingerprint the instance's licence tier.
security:
- webhookSignature: []
parameters:
- $ref: "#/components/parameters/idPath"
requestBody:
required: false
required: true
content:
application/json:
schema:
@@ -1593,7 +1606,8 @@ paths:
properties:
action:
type: string
description: Override the default webhook action.
enum: [deploy, restart, stop, start, pull, git-pull]
description: Override the default webhook action. Must be one of the allowed actions.
responses:
"202":
description: Webhook accepted and action queued.
@@ -1609,16 +1623,14 @@ paths:
action:
type: string
example: deploy
"401":
description: Missing or invalid signature.
"400":
description: Authentication succeeded but the body's `action` override was not in the allowlist.
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
"403":
$ref: "#/components/responses/Forbidden"
"404":
description: Webhook not found or disabled.
description: Authentication failed. The webhook is unknown, disabled, the licence is not paid, the signature header is missing, the body was empty, or the signature did not match. Sencho returns the same response for every unauthenticated case.
content:
application/json:
schema: