Files
claude-hub/test/unit/core/webhook/WebhookRegistry.test.ts
Cheffromspace 348d4acaf8 feat: Implement modular webhook architecture for multi-provider support (#170)
* feat: Implement modular webhook architecture for multi-provider support

- Add generic webhook types and interfaces for provider-agnostic handling
- Create WebhookRegistry for managing providers and event handlers
- Implement WebhookProcessor for unified webhook request processing
- Add GitHubWebhookProvider implementing the new interfaces
- Create new /api/webhooks/:provider endpoint supporting multiple providers
- Update GitHub types to include missing id, email, and merged_at properties
- Add comprehensive unit tests for all webhook components
- Maintain backward compatibility with existing /api/webhooks/github endpoint

This architecture enables easy addition of new webhook providers (GitLab,
Bitbucket, etc.) while keeping the codebase modular and maintainable.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* security: Implement webhook security enhancements

- Add provider name validation against whitelist to prevent arbitrary provider injection
- Implement generic error messages to avoid information disclosure
- Make webhook signature verification mandatory in production environments
- Fix linter warnings in GitHubWebhookProvider.ts
- Add comprehensive security tests

Security improvements address:
- Input validation: Provider names validated against ALLOWED_WEBHOOK_PROVIDERS
- Error disclosure: Generic messages replace detailed error information
- Authentication: Signature verification cannot be skipped in production

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Fetch complete PR details for manual review commands

When processing @MCPClaude review commands on PR comments, the webhook
payload only contains minimal PR information. This fix ensures we fetch
the complete PR details from GitHub API to get the correct head/base
refs and SHA, preventing the "unknown" branch issue.

Also fixes test initialization issue in webhooks.test.ts.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Fix failing webhook route tests in CI

The webhook route tests were failing because the mock for the GitHub
provider module was incomplete. Updated the mock to include the
initializeGitHubProvider function to prevent import errors.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Move Jest mocks before imports to prevent auto-initialization

The webhook tests were failing in CI because the GitHub provider mock
was declared after the imports, allowing the auto-initialization to run.
Moving all mocks to the top of the file ensures they are in place before
any module loading occurs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Mock webhook registry to prevent auto-initialization in tests

The webhook route tests were failing because the webhook registry was
being imported and triggering auto-initialization. By fully mocking the
webhook registry module before any imports, we prevent side effects and
ensure tests run in isolation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Properly mock WebhookProcessor to avoid module initialization issues

The webhook route tests were failing in CI due to differences in module
loading between Node.js versions. By mocking the WebhookProcessor class
and moving imports after mocks are set up, we ensure consistent behavior
across environments. The mock now properly simulates the authorization
logic to maintain test coverage.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Remove side effects from webhook module initialization

The webhook tests were failing in CI because the GitHub provider was
being auto-initialized during module import, causing unpredictable
behavior across different Node.js versions and environments.

Changes:
- Moved provider initialization to dynamic import in non-test environments
- Simplified webhook route tests to avoid complex mocking
- Removed unnecessary mocks that were testing implementation details

This ensures deterministic test behavior across all environments.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Fix webhook tests mock configuration for secureCredentials

The webhook tests were failing with "secureCredentials.get is not a function"
because the mock wasn't properly configured for ES module default exports.

Changes:
- Added __esModule: true to the mock to properly handle default exports
- Removed debugging code from tests
- Tests now pass consistently in all environments

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
2025-06-02 22:01:24 -05:00

267 lines
7.9 KiB
TypeScript

import { WebhookRegistry } from '../../../../src/core/webhook/WebhookRegistry';
import type { WebhookProvider, WebhookEventHandler } from '../../../../src/types/webhook';
describe('WebhookRegistry', () => {
let registry: WebhookRegistry;
beforeEach(() => {
registry = new WebhookRegistry();
});
afterEach(() => {
registry.clear();
});
describe('registerProvider', () => {
it('should register a provider', () => {
const provider: WebhookProvider = {
name: 'test-provider',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
registry.registerProvider(provider);
expect(registry.hasProvider('test-provider')).toBe(true);
expect(registry.getProvider('test-provider')).toBe(provider);
});
it('should overwrite existing provider with warning', () => {
const provider1: WebhookProvider = {
name: 'test-provider',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
const provider2: WebhookProvider = {
name: 'test-provider',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
registry.registerProvider(provider1);
registry.registerProvider(provider2);
expect(registry.getProvider('test-provider')).toBe(provider2);
});
});
describe('registerHandler', () => {
it('should register a handler', () => {
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerHandler('test-provider', handler);
expect(registry.getHandlerCount('test-provider')).toBe(1);
});
it('should sort handlers by priority', () => {
const lowPriorityHandler: WebhookEventHandler = {
event: 'test.event',
priority: 10,
handle: jest.fn()
};
const highPriorityHandler: WebhookEventHandler = {
event: 'test.event',
priority: 100,
handle: jest.fn()
};
registry.registerHandler('test-provider', lowPriorityHandler);
registry.registerHandler('test-provider', highPriorityHandler);
const handlers = registry.getHandlers('test-provider', 'test.event');
expect(handlers[0]).toBe(highPriorityHandler);
expect(handlers[1]).toBe(lowPriorityHandler);
});
it('should handle handlers without priority', () => {
const handler1: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
const handler2: WebhookEventHandler = {
event: 'test.event',
priority: 50,
handle: jest.fn()
};
registry.registerHandler('test-provider', handler1);
registry.registerHandler('test-provider', handler2);
const handlers = registry.getHandlers('test-provider', 'test.event');
expect(handlers[0]).toBe(handler2);
expect(handlers[1]).toBe(handler1);
});
});
describe('getHandlers', () => {
it('should return handlers for exact event match', () => {
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerHandler('test-provider', handler);
const handlers = registry.getHandlers('test-provider', 'test.event');
expect(handlers).toHaveLength(1);
expect(handlers[0]).toBe(handler);
});
it('should return handlers for wildcard match', () => {
const handler: WebhookEventHandler = {
event: 'test.*',
handle: jest.fn()
};
registry.registerHandler('test-provider', handler);
expect(registry.getHandlers('test-provider', 'test.event')).toHaveLength(1);
expect(registry.getHandlers('test-provider', 'test.another')).toHaveLength(1);
expect(registry.getHandlers('test-provider', 'other.event')).toHaveLength(0);
});
it('should return handlers for regex match', () => {
const handler: WebhookEventHandler = {
event: /^issue\.(opened|closed)$/,
handle: jest.fn()
};
registry.registerHandler('test-provider', handler);
expect(registry.getHandlers('test-provider', 'issue.opened')).toHaveLength(1);
expect(registry.getHandlers('test-provider', 'issue.closed')).toHaveLength(1);
expect(registry.getHandlers('test-provider', 'issue.edited')).toHaveLength(0);
});
it('should return empty array for no matches', () => {
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerHandler('test-provider', handler);
const handlers = registry.getHandlers('test-provider', 'other.event');
expect(handlers).toHaveLength(0);
});
it('should handle case-insensitive provider names', () => {
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerHandler('TestProvider', handler);
expect(registry.getHandlers('testprovider', 'test.event')).toHaveLength(1);
expect(registry.getHandlers('TESTPROVIDER', 'test.event')).toHaveLength(1);
});
});
describe('getAllProviders', () => {
it('should return all registered providers', () => {
const provider1: WebhookProvider = {
name: 'provider1',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
const provider2: WebhookProvider = {
name: 'provider2',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
registry.registerProvider(provider1);
registry.registerProvider(provider2);
const providers = registry.getAllProviders();
expect(providers).toHaveLength(2);
expect(providers).toContain(provider1);
expect(providers).toContain(provider2);
});
it('should return empty array when no providers registered', () => {
expect(registry.getAllProviders()).toHaveLength(0);
});
});
describe('getHandlerCount', () => {
it('should return count for specific provider', () => {
const handler1: WebhookEventHandler = {
event: 'event1',
handle: jest.fn()
};
const handler2: WebhookEventHandler = {
event: 'event2',
handle: jest.fn()
};
registry.registerHandler('provider1', handler1);
registry.registerHandler('provider1', handler2);
registry.registerHandler('provider2', handler1);
expect(registry.getHandlerCount('provider1')).toBe(2);
expect(registry.getHandlerCount('provider2')).toBe(1);
});
it('should return total count when no provider specified', () => {
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerHandler('provider1', handler);
registry.registerHandler('provider2', handler);
registry.registerHandler('provider3', handler);
expect(registry.getHandlerCount()).toBe(3);
});
it('should return 0 for unknown provider', () => {
expect(registry.getHandlerCount('unknown')).toBe(0);
});
});
describe('clear', () => {
it('should clear all providers and handlers', () => {
const provider: WebhookProvider = {
name: 'test-provider',
verifySignature: jest.fn(),
parsePayload: jest.fn(),
getEventType: jest.fn(),
getEventDescription: jest.fn()
};
const handler: WebhookEventHandler = {
event: 'test.event',
handle: jest.fn()
};
registry.registerProvider(provider);
registry.registerHandler('test-provider', handler);
registry.clear();
expect(registry.hasProvider('test-provider')).toBe(false);
expect(registry.getHandlerCount()).toBe(0);
});
});
});