Fix shared validation and webhook-delivery security issues - #637
Merged
therealjhay merged 2 commits intoSep 1, 2026
Conversation
) Added optional scrapeToken parameter to startMetricsServer(): - When scrapeToken is provided, requests must include 'Authorization: Bearer <token>' - Unauthorized requests receive 401 with WWW-Authenticate header - When scrapeToken is omitted, metrics served unauthenticated (network isolation required) - Logs auth status on startup Tests added: - Unauthorized scrape rejection (no token, wrong token, malformed header) - Valid token acceptance - Unauthenticated mode when token not configured - Auth status logging verification Operators can now: - Secure metrics endpoint with bearer token auth - Configure Prometheus scraper with bearer_token config - Or rely on network isolation (firewall/localhost binding) Closes Betta-Pay#528
…s-528-526-516-517
|
@Mutech939 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses four security and reliability issues in shared packages:
metrics-server.tsexposes metrics without auth #528: Added bearer token authentication to metrics serverencryption.tsuses a fixed key derivation without salt #526: Created encryption module with per-encryption saltChanges
Issue #528: Metrics Server Authentication
File:
shared/validation/metrics-server.tsAdded optional Bearer token authentication:
scrapeTokenparameter inStartMetricsServerOptionsAuthorization: Bearer <token>WWW-AuthenticateheaderSecurity benefit: Prevents internal metrics leakage on misconfigured networks.
Tests added (4 test cases):
Issue #526: Encryption with Per-Encryption Salt
Files:
shared/validation/encryption.ts(new)shared/validation/encryption.test.ts(new)Created secure encryption module:
base64(salt:iv:authTag:ciphertext)Security properties:
API:
Tests added (15 test cases):
Issue #516: Env-Configurable Webhook Concurrency
File:
shared/webhook-delivery/index.tsMade concurrency tunable without code changes:
WEBHOOK_CONCURRENCYenvironment variableresolveWebhookConcurrency()helper reads env with fallbackconcurrencyoption still overrides env varOperator benefit: Tune concurrency based on worker resources without redeploying code.
Usage:
Tests added (5 test cases):
resolveWebhookConcurrency()with valid/invalid env valuesIssue #517: Socket Cleanup in Finally Block
File:
shared/webhook-delivery/index.tsFixed potential timer/socket leaks:
clearTimeoutin both try and catch blocks (race condition on error path)clearTimeoutin finally block (guaranteed cleanup)AbortController.abort()called on all exit pathsReliability benefit: No resource leaks under failure scenarios.
Tests added (3 test cases):
Testing
Test Coverage Summary
CI Pipeline
Acceptance Criteria
Issue #528 ✅
scrapeTokenprovidedpnpm buildpassesIssue #526 ✅
pnpm buildpassesIssue #516 ✅
WEBHOOK_CONCURRENCYpnpm --filter @bettapay/webhook-delivery buildpassesIssue #517 ✅
pnpm --filter @bettapay/webhook-delivery buildpassesMigration Notes
Metrics Server Authentication
Backward compatible - existing deployments continue to work (unauthenticated mode).
To enable auth:
Configure Prometheus:
Encryption Module
New module - no migration needed. Use for encrypting sensitive data at rest:
Webhook Concurrency
Backward compatible - defaults to 10 when
WEBHOOK_CONCURRENCYunset.To tune:
Socket Cleanup
Transparent fix - no code changes required. Workers automatically benefit from leak prevention.
Checklist
Related Issues
Closes #528
Closes #526
Closes #516
Closes #517