Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
846 changes: 846 additions & 0 deletions docs/plans/2026-05-27-multi-github-destination-web.md

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions docs/specs/2026-04-22-github-webhook-handler.md
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ export function getSkipReason(data, action, env) {
| `GITHUB_WEBHOOK_SECRET` | HMAC-SHA256 verification | Vault (`dx_mysticat/{env}/mysticat-github-service` key `github_app_webhook_secret`), delivered via Secrets Manager |
| `MYSTICAT_GITHUB_JOBS_QUEUE_URL` | SQS queue URL for job enqueue | Infrastructure module output (`module.mysticat_github_service.work_queue_url`) |
| `GITHUB_APP_SLUG` | App login for reviewer match (default: `mysticat`) | Environment variable or hardcoded |
| `GITHUB_TARGETS` | JSON registry of webhook destinations (`[{id, match, appSlug, webhookSecretEnvVar}]`), evaluated top-to-bottom with the `default` entry last. UNSET = legacy single-secret path (no `target_id` emitted). See the multi-destination ADR (`mysticat-architecture` `platform/decisions/support-multiple-github-destinations.md`). | api-service env (per env) |
| `GITHUB_WEBHOOK_SECRET_GHEC` | Per-target HMAC secret for the `ghec` destination (only when GHEC is enabled; named by that target's `webhookSecretEnvVar`) | Vault -> Secrets Manager, same channel as `GITHUB_WEBHOOK_SECRET` |
| `MYSTICAT_OBSERVABILITY_SLACK_TOKEN` | Dedicated chat:write-only bot token for the Slack observability feed (best-effort; absent disables Slack posting) | Vault (`dx_mysticat/{env}/...`) -> helix-deploy package secret; on the rotation list |
| `MYSTICAT_OBSERVABILITY_SLACK_CHANNEL` | Channel id the web tier posts to and propagates to the worker; OMIT to disable observability entirely | api-service env (per env) |

Expand Down
24 changes: 15 additions & 9 deletions src/controllers/webhooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,19 @@ function WebhooksController(context) {
const deliveryId = ctx.pathInfo?.headers?.['x-github-delivery'];
const { data } = ctx;

// Validate required config up front. GITHUB_APP_SLUG is a security-relevant
// decision (which bot can trigger automated runs). Missing config must be
// a 5xx so GitHub retries once it is fixed — returning 204 here would
// mean GitHub treats the delivery as succeeded and never redelivers,
// permanently losing every webhook during the misconfiguration window.
if (!env.GITHUB_APP_SLUG) {
log.error('GITHUB_APP_SLUG not configured', { deliveryId });
return internalServerError('GITHUB_APP_SLUG not configured');
// Destination resolved by the HMAC handler (registry mode). Legacy mode has
// no profile target_id/app_slug -> fall back to env.GITHUB_APP_SLUG and emit
// no target_id (worker then resolves its flat keys).
const profile = ctx.attributes?.authInfo?.getProfile?.() || {};
const targetId = profile.target_id;
const appSlug = profile.app_slug || env.GITHUB_APP_SLUG;

// Security-relevant: which bot can trigger automated runs. In registry mode
// this comes from the target's appSlug; in legacy mode from env. A missing
// value must be a 5xx (GitHub retries) rather than a 204 (delivery lost).
if (!appSlug) {
log.error('No app slug resolved (GITHUB_APP_SLUG unset and no target app_slug)', { deliveryId });
return internalServerError('app slug not configured');
}

// Filter on event type BEFORE validating payload fields. Unmapped events
Expand Down Expand Up @@ -140,7 +145,7 @@ function WebhooksController(context) {
}

// Apply trigger rules (returns skip reason string or null)
const skipReason = getSkipReason(data, action, env);
const skipReason = getSkipReason(data, action, env, appSlug);
if (skipReason) {
log.info('Skipping webhook', {
skipReason,
Expand Down Expand Up @@ -212,6 +217,7 @@ function WebhooksController(context) {
job_type: jobType,
workspace_repos: workspaceRepos,
retry_count: 0,
...(targetId ? { target_id: targetId } : {}),
...(observability ? { observability } : {}),
};

Expand Down
132 changes: 132 additions & 0 deletions src/support/github-targets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

// GitHub destination registry + classifier. The web tier classifies each
// inbound webhook to a destination ("target") from the SIGNED body, so the
// worker can select per-destination credentials by a non-secret target_id.
// Secrets are NOT in this registry: webhookSecretEnvVar names the env var that
// carries the secret (injected from the deploy secret store).

/**
* Parse + validate the GITHUB_TARGETS env var.
* @param {object} env - context.env
* @returns {Array|null} ordered target array, or null when GITHUB_TARGETS is
* unset (legacy single-secret mode).
* @throws {Error} when GITHUB_TARGETS is set but structurally invalid.
*/
export function parseTargets(env) {
const raw = env?.GITHUB_TARGETS;
if (!raw) {
return null;
}
let parsed;
try {
parsed = JSON.parse(raw);
} catch (e) {
throw new Error(`GITHUB_TARGETS is not valid JSON: ${e.message}`);
}
if (!Array.isArray(parsed) || parsed.length === 0) {
throw new Error('GITHUB_TARGETS must be a non-empty JSON array');
}
const ids = new Set();
parsed.forEach((t, i) => {
if (!t || typeof t.id !== 'string' || !t.id) {
throw new Error(`GITHUB_TARGETS[${i}] is missing a string "id"`);
}
if (ids.has(t.id)) {
throw new Error(`GITHUB_TARGETS has duplicate id "${t.id}"`);
}
ids.add(t.id);
if (typeof t.appSlug !== 'string' || !t.appSlug) {
throw new Error(`GITHUB_TARGETS["${t.id}"] is missing a string "appSlug"`);
}
if (typeof t.webhookSecretEnvVar !== 'string' || !t.webhookSecretEnvVar) {
throw new Error(`GITHUB_TARGETS["${t.id}"] is missing a string "webhookSecretEnvVar"`);
}
// Defense-in-depth for operator-authored config: webhookSecretEnvVar is used
// as `context.env[name]`, so a typo like "__proto__" would resolve to a
// truthy prototype object (not a secret) and break HMAC. Restrict to the
// conventional env-var charset so a bad name fails loudly at parse.
if (!/^[A-Z][A-Z0-9_]*$/.test(t.webhookSecretEnvVar)) {
throw new Error(`GITHUB_TARGETS["${t.id}"].webhookSecretEnvVar must be a valid env var name (^[A-Z][A-Z0-9_]*$)`);
}
const isDefault = t.match?.default === true;
const hasSlugs = Array.isArray(t.match?.enterpriseSlug)
&& t.match.enterpriseSlug.length > 0
&& t.match.enterpriseSlug.every((s) => typeof s === 'string' && s.length > 0);
if (!isDefault && !hasSlugs) {
throw new Error(`GITHUB_TARGETS["${t.id}"] needs match.default:true or a non-empty match.enterpriseSlug[] of strings`);
}
if (isDefault && i !== parsed.length - 1) {
throw new Error(`GITHUB_TARGETS default entry "${t.id}" must be last`);
}
});
const defaults = parsed.filter((t) => t.match?.default === true);
if (defaults.length !== 1) {
throw new Error(`GITHUB_TARGETS must have exactly one match.default:true entry (found ${defaults.length})`);
}
return parsed;
}

function hostOf(htmlUrl) {
if (typeof htmlUrl !== 'string') {
return null;
}
try {
return new URL(htmlUrl).host;
} catch {
return null;
}
}

/**
* Extract the classification signals from the raw (signed) webhook body.
* @param {string} rawBody
* @returns {{host: (string|null), enterpriseSlug: (string|null)}|null} null when
* the body is not valid JSON.
*/
export function extractClassificationMetadata(rawBody) {
let payload;
try {
payload = JSON.parse(rawBody);
} catch {
return null;
}
if (!payload || typeof payload !== 'object') {
return null;
}
const enterpriseSlug = typeof payload.enterprise?.slug === 'string' ? payload.enterprise.slug : null;
return { host: hostOf(payload.repository?.html_url), enterpriseSlug };
}

/**
* Classify webhook metadata to a target, or signal skip.
* @param {{host: (string|null), enterpriseSlug: (string|null)}} meta
* @param {Array} targets - validated registry (default entry last)
* @returns {object|{skip: true}} the matched target, or { skip: true }
*/
export function classify(meta, targets) {
const { host, enterpriseSlug } = meta;
// A positively-identified non-github.com host (e.g. a GHES host) has no
// in-scope target. A null/unknown host (ping / no repository) falls through
// to the github.com catch-all.
if (host !== null && host !== 'github.com') {
return { skip: true };
}
// Registry is validated default-last (parseTargets), so a slug-specific entry
// is always evaluated before the catch-all default in this find loop.
const match = targets.find((t) => t.match?.default === true
|| (enterpriseSlug
&& Array.isArray(t.match?.enterpriseSlug)
&& t.match.enterpriseSlug.includes(enterpriseSlug)));
return match || { skip: true };
}
130 changes: 86 additions & 44 deletions src/support/github-webhook-hmac-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,50 @@
import crypto from 'crypto';
import AbstractHandler from '@adobe/spacecat-shared-http-utils/src/auth/handlers/abstract.js';
import AuthInfo from '@adobe/spacecat-shared-http-utils/src/auth/auth-info.js';
import { parseTargets, classify, extractClassificationMetadata } from './github-targets.js';

const SIGNATURE_PATTERN = /^sha256=[a-f0-9]{64}$/;
const WEBHOOK_PATH_PATTERN = /^\/?webhooks\//;
// Real GitHub webhook payloads are typically under 100 KB; GitHub caps at 25 MB.
// Reject larger bodies before HMAC computation to prevent pre-auth resource exhaustion.
const MAX_BODY_BYTES = 1024 * 1024; // 1 MiB

// Timing-safe HMAC compare. Both buffers are 71 chars ("sha256=" + 64 hex):
// SIGNATURE_PATTERN guaranteed the input length and the HMAC hex is fixed-length,
// so timingSafeEqual will not throw on a length mismatch.
function verifySignature(signature, rawBody, secret) {
const expected = `sha256=${crypto.createHmac('sha256', secret).update(rawBody).digest('hex')}`;
return crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected));
}

class GitHubWebhookHmacHandler extends AbstractHandler {
constructor(log) {
super('github-webhook-hmac', log);
}

// Read the raw body and enforce the 1 MiB cap. Returns the raw string, or null
// (already logged) on empty / oversized. Two-tier: a Content-Length precheck
// (honest-client-only; attacker can omit the header) then the post-read byte
// length (the real enforcement). request.text() returns the cached body.
async readBodyWithLimits(request) {
const contentLength = Number(request.headers.get('content-length'));
if (Number.isFinite(contentLength) && contentLength > MAX_BODY_BYTES) {
this.log(`Payload too large: ${contentLength} bytes`, 'warn');
return null;
}
const rawBody = await request.text();
if (!rawBody) {
this.log('Empty request body for webhook', 'warn');
return null;
}
const byteLength = Buffer.byteLength(rawBody, 'utf8');
if (byteLength > MAX_BODY_BYTES) {
this.log(`Payload too large after read: ${byteLength} bytes`, 'warn');
return null;
}
return rawBody;
}

async checkAuth(request, context) {
// Path-scoped: only handle /webhooks/* routes. Tolerate suffix with or
// without leading slash (production sets it with leading slash).
Expand All @@ -33,73 +65,83 @@ class GitHubWebhookHmacHandler extends AbstractHandler {
}

const signature = request.headers.get('x-hub-signature-256');

// Not a GitHub webhook request -- let other handlers try
if (!signature) {
return null;
}

// Validate signature format FIRST: structural check, no I/O, no config.
// Must run before the secret presence check to prevent error-log amplification
// on pre-auth malformed requests when GITHUB_WEBHOOK_SECRET is unset.
// Also prevents timingSafeEqual from throwing on length mismatch later.
// Runs before any secret/registry work to prevent error-log amplification on
// pre-auth malformed requests, and before timingSafeEqual to avoid a throw.
if (!SIGNATURE_PATTERN.test(signature)) {
this.log('Malformed X-Hub-Signature-256 header', 'warn');
return null;
}

const secret = context.env?.GITHUB_WEBHOOK_SECRET;
if (!secret) {
this.log('GITHUB_WEBHOOK_SECRET not configured (misconfigured=true)', 'error');
return null;
const targetsRaw = context.env?.GITHUB_TARGETS;

// ---- Legacy path: no registry configured -> today's exact behaviour ----
// Single GITHUB_WEBHOOK_SECRET, no target_id. The secret presence is checked
// BEFORE reading the body, preserving the early-bail.
if (!targetsRaw) {
const secret = context.env?.GITHUB_WEBHOOK_SECRET;
if (!secret) {
this.log('GITHUB_WEBHOOK_SECRET not configured (misconfigured=true)', 'error');
return null;
}
const rawBody = await this.readBodyWithLimits(request);
if (rawBody === null) {
return null;
}
if (!verifySignature(signature, rawBody, secret)) {
this.log('HMAC signature mismatch', 'warn');
return null;
}
return new AuthInfo()
.withAuthenticated(true)
.withProfile({ user_id: 'github-webhook' })
.withType('github_webhook');
}

// Two-tier size check:
// - Pre-read via Content-Length: honest-client-only (attacker can omit the
// header to skip this branch). Saves the body read when the header is set.
// - Post-read via Buffer.byteLength (below): the actual enforcement, catches
// missing or falsified Content-Length.
const contentLength = Number(request.headers.get('content-length'));
if (Number.isFinite(contentLength) && contentLength > MAX_BODY_BYTES) {
this.log(`Payload too large: ${contentLength} bytes`, 'warn');
// ---- Registry path: classify from the SIGNED body, select the candidate ----
// target's secret, verify HMAC once. Parsing before verifying is safe: a
// forged body just selects a candidate whose secret it cannot forge.
let targets;
try {
targets = parseTargets(context.env);
} catch (e) {
// Malformed registry is a misconfiguration; null -> 401 (visible failed
// delivery), matching the missing-secret handling above.
this.log(`Invalid GITHUB_TARGETS config (misconfigured=true): ${e.message}`, 'error');
return null;
}

// Read raw body from request. bodyData middleware runs BEFORE authWrapper
// in the .with() chain (last .with() = outermost = runs first), so bodyData
// has already consumed the stream and set context.data. request.text()
// returns the cached body via @adobe/helix-universal's Request implementation.
const rawBody = await request.text();
if (!rawBody) {
this.log('Empty request body for webhook', 'warn');
const rawBody = await this.readBodyWithLimits(request);
if (rawBody === null) {
return null;
}

// Second size check after reading (content-length may be absent or wrong).
// Use byte length (not JS string length) for a byte-accurate cap.
const byteLength = Buffer.byteLength(rawBody, 'utf8');
if (byteLength > MAX_BODY_BYTES) {
this.log(`Payload too large after read: ${byteLength} bytes`, 'warn');
const meta = extractClassificationMetadata(rawBody);
if (meta === null) {
this.log('Webhook body is not valid JSON', 'warn');
return null;
}

// Compute expected HMAC
const expected = `sha256=${crypto
.createHmac('sha256', secret)
.update(rawBody)
.digest('hex')}`;

// Timing-safe comparison (both are guaranteed 71 chars: "sha256=" + 64 hex)
const sigBuffer = Buffer.from(signature);
const expectedBuffer = Buffer.from(expected);
if (!crypto.timingSafeEqual(sigBuffer, expectedBuffer)) {
const result = classify(meta, targets);
// host not an in-scope GitHub destination (e.g. a GHES host): skip + log, NO
// HMAC. The body is untrusted, so do not interpolate meta.host into the log.
if (result.skip) {
this.log('Skipping webhook: host is not an in-scope GitHub destination', 'warn');
return null;
}
const secret = context.env?.[result.webhookSecretEnvVar];
if (!secret) {
this.log(`Webhook secret for target ${result.id} not configured (misconfigured=true)`, 'error');
return null;
}
if (!verifySignature(signature, rawBody, secret)) {
this.log('HMAC signature mismatch', 'warn');
return null;
}

return new AuthInfo()
.withAuthenticated(true)
.withProfile({ user_id: 'github-webhook' })
.withProfile({ user_id: 'github-webhook', target_id: result.id, app_slug: result.appSlug })
.withType('github_webhook');
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/utils/github-trigger-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ export const EVENT_JOB_MAP = {
* @param {object} data - Parsed webhook payload
* @param {string} action - The event action (e.g. 'review_requested', 'labeled')
* @param {object} env - Environment variables
* @param {string} [appSlug] - Allowed-bot slug; defaults to env.GITHUB_APP_SLUG
* @returns {string|null} Skip reason or null
*/
export function getSkipReason(data, action, env) {
export function getSkipReason(data, action, env, appSlug = env.GITHUB_APP_SLUG) {
const pr = data.pull_request;
// Caller must validate env.GITHUB_APP_SLUG before invoking (the controller
// returns 500 on missing config so GitHub retries, rather than 204 from a skip).
const appSlug = env.GITHUB_APP_SLUG;
// appSlug is resolved by the caller: the per-target appSlug in registry mode,
// else env.GITHUB_APP_SLUG (the default). Used to form the expected bot
// reviewer login. Defaulting keeps existing 3-arg callers unchanged.

// Unsupported actions (auto-triggers deferred to Phase 3)
if (action === 'opened' || action === 'ready_for_review') {
Expand Down
Loading
Loading