Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Required: GitHub App credentials
APP_ID=
PRIVATE_KEY=

# Required. Temper refuses to start if this is unset or equals the literal
# string "development" (Probot's fail-open default — without a real secret,
# forged webhooks signed with "development" would be silently accepted).
# Generate with: openssl rand -hex 32
WEBHOOK_SECRET=

# Optional: Target organization (also configurable in config.yml)
Expand Down
53 changes: 53 additions & 0 deletions __tests__/unit/webhook-secret.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
jest.mock('../../src/logger.js', () => {
const log = { info: jest.fn(), warn: jest.fn(), error: jest.fn() };
return { getLogger: () => log, setLogger: jest.fn() };
});

import { assertWebhookSecret } from '../../src/app.js';

describe('assertWebhookSecret (Bug #2 — trust-boundary fail-fast)', () => {
it('throws when WEBHOOK_SECRET is missing', () => {
expect(() => assertWebhookSecret({})).toThrow(/required/i);
});

it('throws when WEBHOOK_SECRET is the empty string', () => {
expect(() => assertWebhookSecret({ WEBHOOK_SECRET: '' })).toThrow(/required/i);
});

it('throws when WEBHOOK_SECRET is whitespace-only', () => {
expect(() => assertWebhookSecret({ WEBHOOK_SECRET: ' ' })).toThrow(/required/i);
});

it('throws when WEBHOOK_SECRET equals the literal "development"', () => {
expect(() => assertWebhookSecret({ WEBHOOK_SECRET: 'development' })).toThrow(
/"development"/
);
});

it('does NOT throw when WEBHOOK_SECRET merely contains "development" as substring', () => {
// Exact-match only — a real secret may legitimately have the word.
expect(() =>
assertWebhookSecret({ WEBHOOK_SECRET: 'development-secret-9f2c' })
).not.toThrow();
expect(() =>
assertWebhookSecret({ WEBHOOK_SECRET: 'predev-elopment' })
).not.toThrow();
});

it('passes for a non-trivial secret', () => {
expect(() =>
assertWebhookSecret({ WEBHOOK_SECRET: 'a1b2c3d4e5f6g7h8i9j0' })
).not.toThrow();
});

it('reads from process.env by default', () => {
const original = process.env.WEBHOOK_SECRET;
process.env.WEBHOOK_SECRET = 'real-secret-here';
try {
expect(() => assertWebhookSecret()).not.toThrow();
} finally {
if (original === undefined) delete process.env.WEBHOOK_SECRET;
else process.env.WEBHOOK_SECRET = original;
}
});
});
6 changes: 5 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { fileURLToPath } from 'url';
import { run } from 'probot';
import { registerApp, mapLegacyEnvVars } from './src/app.js';
import { registerApp, mapLegacyEnvVars, assertWebhookSecret } from './src/app.js';
import { configureRepository } from './src/repository.js';
import { applyBranchProtection } from './src/branch-protection.js';
import { applyTemplates, applyCodeowners } from './src/templates.js';
Expand Down Expand Up @@ -29,6 +29,10 @@ if (process.argv[1] === fileURLToPath(import.meta.url)) {
(async () => {
try {
mapLegacyEnvVars();
// Trust-boundary check: refuse to boot with a missing or
// "development" webhook secret. See Bug #2 in
// docs/agent-fleet/bugs.md (wave-1 Security auditor).
assertWebhookSecret();
console.log('Starting Temper...');
console.log(`Environment: ${process.env.NODE_ENV || 'development'}`);
console.log(`Port: ${process.env.PORT || 3000}`);
Expand Down
37 changes: 37 additions & 0 deletions src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,42 @@ function initScheduler(app) {
return { store: _taskStore, scheduler: _scheduler };
}

/**
* Fail fast at startup when WEBHOOK_SECRET is unset, empty, or equals the
* literal string "development".
*
* Without this, Probot defaults the webhook secret to "development" when the
* env var is missing — silently accepting forged webhooks signed with that
* trivially-known string. The wave-1 security auditor flagged this as the
* single highest-impact trust-boundary fail-open in the bot. (Bug #2 in
* docs/agent-fleet/bugs.md.)
*
* Exported so it can be unit-tested without booting Probot. Throws Error so
* the caller's existing try/catch in index.js produces the standard
* "Error starting Probot:" message.
*
* Call this AFTER mapLegacyEnvVars so legacy GITHUB_WEBHOOK_SECRET → WEBHOOK_SECRET
* mapping is honoured before validation.
*/
function assertWebhookSecret(env = process.env) {
const secret = env.WEBHOOK_SECRET;
if (!secret || secret.trim() === '') {
throw new Error(
'WEBHOOK_SECRET environment variable is required. ' +
'Without it Probot defaults to the literal string "development", ' +
'silently accepting forged webhooks. Set it in .env or via your ' +
'process supervisor.'
);
}
if (secret === 'development') {
throw new Error(
'WEBHOOK_SECRET cannot equal the literal string "development" — ' +
'that is Probot\'s fail-open default. Set a real secret (e.g. ' +
'`openssl rand -hex 32`).'
);
}
}

function mapLegacyEnvVars() {
if (process.env.GITHUB_APP_ID && !process.env.APP_ID) {
process.env.APP_ID = process.env.GITHUB_APP_ID;
Expand All @@ -1151,6 +1187,7 @@ function mapLegacyEnvVars() {
export {
registerApp,
mapLegacyEnvVars,
assertWebhookSecret,
createCustomRoutesHandler,
applySecurityHeaders,
initScheduler,
Expand Down
Loading