chore: harden socket/proxy origin checks + log redaction + env var gate#1759
Open
paustint wants to merge 1 commit into
Open
chore: harden socket/proxy origin checks + log redaction + env var gate#1759paustint wants to merge 1 commit into
paustint wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens several security controls in the API by tightening origin checks for browser-facing proxies/sockets, hardening secret validation (including production-only guards), and reducing sensitive data exposure in logs.
Changes:
- Add/strengthen origin allowlists for the platform-event proxy and browser WebSocket upgrades; tighten scanner route gating to non-production.
- Harden env secret validation (min lengths) and add a production-only guard rejecting known placeholder secrets; update env generation/docs for the new secret.
- Improve security hygiene in webhooks/logging: atomic replay prevention for Mailgun webhooks + timing-safe signature compare; redact OAuth/Canvas error logs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/generate.env.mjs | Generates a new JETSTREAM_AUTH_WEB_EXT_JWT_SECRET when creating local .env files. |
| libs/api-config/src/lib/env-config.ts | Adds stronger secret validation and a production-only placeholder-secret guard. |
| apps/api/src/app/routes/scanner.routes.ts | Gates scanner session-minting routes to non-production environments. |
| apps/api/src/app/routes/platform-event.routes.ts | Adds same-origin allowlist checks and response-header allowlisting for the SF proxy. |
| apps/api/src/app/controllers/socket.controller.ts | Enforces an origin allowlist for browser WebSocket upgrades to mitigate CSWSH. |
| apps/api/src/app/controllers/oauth.controller.ts | Redacts sensitive OAuth callback query params from warning logs. |
| apps/api/src/app/controllers/mailgun-webhook.controller.ts | Uses atomic token consumption for replay protection and timing-safe signature comparison. |
| apps/api/src/app/controllers/canvas.controller.ts | Redacts sensitive Canvas OAuth callback query params from warning logs. |
| .env.example | Documents the new JETSTREAM_AUTH_WEB_EXT_JWT_SECRET env var. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+166
to
+173
| JETSTREAM_AUTH_WEB_EXT_JWT_SECRET: z.preprocess( | ||
| (value) => (typeof value === 'string' && value.trim() === '' ? undefined : value), | ||
| z | ||
| .string() | ||
| .min(32, { message: 'JETSTREAM_AUTH_WEB_EXT_JWT_SECRET must be at least 32 characters' }) | ||
| .optional() | ||
| .default('DEVELOPMENT_SECRET'), | ||
| ), |
Comment on lines
+336
to
+348
| * placeholders so a deployment can never silently run on a public constant (e.g. the | ||
| * JETSTREAM_AUTH_WEB_EXT_JWT_SECRET 'DEVELOPMENT_SECRET' fallback that signs all browser-extension/desktop JWTs). | ||
| * | ||
| * Production is detected the same way as the rest of this file: ENVIRONMENT === 'production'. CI is exempt | ||
| * because CI does not set ENVIRONMENT (so it defaults to 'production') yet legitimately relies on dev defaults. | ||
| */ | ||
| function assertProductionSecretsAreSafe(env: Env) { | ||
| const isProduction = env.ENVIRONMENT === 'production' && !env.CI; | ||
| if (!isProduction) { | ||
| return; | ||
| } | ||
|
|
||
| const KNOWN_PLACEHOLDER_SECRETS = new Set(['DEVELOPMENT_SECRET', 'changeme', 'secret', 'test', '']); |
Comment on lines
+137
to
+141
| const wasFirstUse = await webhookTokenCache.consumeOnceAsync(signature.token); | ||
| if (!wasFirstUse) { | ||
| logger.warn({ token: signature.token }, 'Mailgun webhook token already used (replay attack)'); | ||
| return res.status(403).send('Token already used'); | ||
| } |
Comment on lines
+13
to
+23
| function getAllowedProxyOrigins(): Set<string> { | ||
| const origins = new Set<string>(); | ||
| for (const url of [ENV.JETSTREAM_CLIENT_URL, ENV.JETSTREAM_SERVER_URL]) { | ||
| try { | ||
| origins.add(new URL(url).origin); | ||
| } catch { | ||
| // ignore malformed configuration | ||
| } | ||
| } | ||
| return origins; | ||
| } |
Comment on lines
+23
to
+33
| function getAllowedWebSocketOrigins(): Set<string> { | ||
| const origins = new Set<string>(); | ||
| for (const url of [ENV.JETSTREAM_CLIENT_URL, ENV.JETSTREAM_SERVER_URL]) { | ||
| try { | ||
| origins.add(new URL(url).origin); | ||
| } catch { | ||
| // ignore malformed configuration | ||
| } | ||
| } | ||
| return origins; | ||
| } |
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.
No description provided.