fix: resolve all critical code scanning alerts (SSRF + type confusion)#82
Merged
Conversation
Address 18 critical-severity CodeQL alerts: **Server-side request forgery (SSRF) - js/request-forgery:** - Add URL validation to block private/internal networks in test endpoints (test-backend, test-frontend, test/route.ts, pluginTester.ts) - Sanitize path parameters with encodeURIComponent to prevent path traversal in plugin-server and plugin-publisher backend - Validate stream IDs in Daydream API client to prevent URL manipulation - Strengthen host allowlist validation in externalProxy middleware to use exact match or proper subdomain check (prevents evil-example.com matching) - Validate baseUrl protocol in LivepeerAIClient and LivepeerCliClient constructors to prevent protocol injection - Add path sanitization for dynamic URL segments in AI client methods **Type confusion through parameter tampering - js/type-confusion-through-parameter-tampering:** - Add runtime type guard in getControlnetsForModel() to handle cases where modelId may not be a string due to parameter tampering Alerts fixed: #51, #62-75, #275-276 Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
scope/shell
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds SSRF and path-traversal protections across multiple services and clients: URL validation (ipaddr.js), hostname allowlist tightening, path-segment sanitization, stream ID validation, and parameter encoding, integrated into existing request/control flows without changing public API signatures. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web-next/src/app/api/v1/plugin-publisher/test-backend/route.ts`:
- Around line 11-44: The validateExternalUrl function currently only blocks a
few specific loopback names/literals and misses entire ranges (127.0.0.0/8,
169.254.0.0/16) and IPv6 link-local/ULA ranges (fe80::/10, fc00::/7), and it
doesn't normalize trailing dots or bracketed IPv6 literals; update
validateExternalUrl to: normalize the hostname by trimming trailing dots and
removing surrounding brackets for IPv6, detect if the hostname is an IP literal
(use a parsing/is-IP check) and reject any address in 127.0.0.0/8,
169.254.0.0/16, 0.0.0.0/8, IPv6 loopback (::1), IPv6 link-local fe80::/10 and
ULA fc00::/7, and continue to reject private ranges (10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16) and local names (localhost, *.local, *.internal, *.localhost) so
all edge-case IP literals and trailing-dot hostnames are blocked; apply the same
hardened logic wherever validateExternalUrl is duplicated.
In `@plugins/daydream-video/backend/src/services/daydream.ts`:
- Around line 229-237: The validateStreamId function currently enforces an
overly restrictive regex (/^[a-zA-Z0-9_-]+$/) which can reject valid opaque IDs
from Daydream; update validateStreamId to only check that streamId is a
non-empty string (e.g., typeof streamId === 'string' && streamId.length > 0) or
alternatively replace the validator with a comment and rely on Daydream-returned
IDs as-is, ensuring any callers that use validateStreamId (function name:
validateStreamId) still handle absence/null cases and throw a clear error when
streamId is missing.
🧹 Nitpick comments (1)
packages/plugin-server-sdk/src/middleware/externalProxy.ts (1)
164-168: Normalize hostnames to handle case-insensitive DNS and trailing dots in allowedHosts config.
allowedHostsentries come from user configuration and are not automatically normalized. If a config entry contains uppercase letters or a trailing dot (e.g.,'API.EXAMPLE.COM.'), the comparison will fail even though it's semantically equivalent. While Node.js's WHATWG URL parser already lowercasesparsed.hostname, theallowedHostsarray should be normalized at initialization to prevent false negatives.♻️ Suggested hardening
const { allowedHosts, @@ } = config; + const normalizedAllowedHosts = allowedHosts.map((h) => + h.trim().replace(/\.$/, '').toLowerCase() + ); + @@ - const isAllowedHost = allowedHosts.some( - (h) => parsed.hostname === h || parsed.hostname.endsWith(`.${h}`) - ); + const hostname = parsed.hostname.replace(/\.$/, ''); + const isAllowedHost = normalizedAllowedHosts.some( + (h) => hostname === h || hostname.endsWith(`.${h}`) + );
eliteprox
added a commit
that referenced
this pull request
Feb 11, 2026
Address 103 high-severity CodeQL alerts across 34 files: **Insecure temporary files (js/insecure-temporary-file) — 20 alerts:** - Replace predictable temp paths with crypto.randomUUID() and fs.mkdtempSync() **User-controlled bypass (js/user-controlled-bypass) — 14 alerts:** - Add typeof checks for auth tokens, codes, emails, passwords - Remove user-controlled skipVerification flag from publish endpoints - Validate GitHub webhook headers with typeof guards - Add OAuth error code allowlist in callback handler **Remote property injection (js/remote-property-injection) — 6 alerts:** - Block __proto__, constructor, prototype keys in config merge/assignment **File system race (js/file-system-race) — 3 alerts:** - Replace TOCTOU check-then-read with try/catch around direct operations **Tainted format string (js/tainted-format-string) — 5 alerts:** - Sanitize user input (strip control chars/newlines) before logging **Incomplete sanitization + bad HTML filter (js/bad-tag-filter) — 2 alerts:** - Replace single-pass regex HTML stripping with iterative loop **Path injection (js/path-injection) — 20 alerts:** - Add safeResolvePath() for storage paths - Add sanitizePathComponent() for plugin/version path segments - Add validateFilePath() for uploaded file paths **SQL injection (js/sql-injection) — 5 alerts:** - Add validateIdentifier() strict allowlist for DDL identifiers - Sanitize database names, usernames, and passwords **XSS through DOM (js/xss-through-dom) — 2 alerts:** - Validate avatar URLs with URL constructor, block javascript: URIs **Reflected XSS (js/reflected-xss) — 1 alert:** - Validate plugin name against allowlist before HTML interpolation **Insecure download (js/insecure-download) — 2 alerts:** - Enforce HTTPS protocol validation before fetch calls **Insecure randomness (js/insecure-randomness) — 1 alert:** - Replace Math.random() with crypto.randomInt() **Insufficient password hash (js/insufficient-password-hash) — 2 alerts:** - Replace SHA-256 with crypto.scryptSync for API key hashing **Missing rate limiting (js/missing-rate-limiting) — 13 alerts:** - Add in-memory rate limiting to auth, registry, embed, and config routes **Insecure Helmet configuration (js/insecure-helmet-configuration) — 2 alerts:** - Replace disabled CSP with proper CSP directives **Incomplete URL sanitization (js/incomplete-url-substring-sanitization) — 5 alerts:** - Replace string includes/startsWith with URL constructor parsing Alerts fixed: #1-50, #52-53, #57-61, #76-77, #82-97, #102-128, #279-280 Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Feb 11, 2026
Fixes raised by Vercel, CodeRabbit, and GitHub Advanced Security: - Harden validateExternalUrl() across all 4 copies: - Block full 127.0.0.0/8 loopback range (not just 127.0.0.1) - Block 169.254.0.0/16 link-local (AWS EC2 metadata at 169.254.169.254) - Block 0.0.0.0/8 range - Block IPv6 link-local (fe80::/10) and ULA (fc00::/7) ranges - Normalize trailing dots in hostnames (e.g., localhost.) - Relax validateStreamId() for Daydream API: - Daydream stream IDs are opaque strings, not restricted to [a-zA-Z0-9_-] - Now validates non-empty string + blocks path traversal (../ \) - Normalize allowedHosts in externalProxy: - Trim, lowercase, and strip trailing dots at initialization time - Normalize incoming hostname before comparison - Strengthen sanitizePath() in AIClient: - Add explicit path traversal checks before encodeURIComponent Co-authored-by: Cursor <cursoragent@cursor.com>
Changes gateway deployment, but best to patch this up for now Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web-next/src/app/api/v1/plugin-publisher/test-frontend/route.ts (1)
70-74:⚠️ Potential issue | 🔴 CriticalSSRF bypass via redirect following.
fetch()follows redirects by default. An attacker can host a URL that passes validation, then have it return a 3xx redirect tohttp://127.0.0.1/or another internal address. The redirect target is never validated.🐛 Proposed fix to disable automatic redirects
const response = await fetch(frontendUrl, { method: 'GET', signal: controller.signal, headers: { Accept: '*/*' }, + redirect: 'manual', }); clearTimeout(timeoutId); - if (!response.ok) { + // Handle redirects - reject them or validate the target + if (response.status >= 300 && response.status < 400) { + testErrors.push('Redirects are not allowed for security reasons'); + return { success: false, errors: testErrors }; + } + + if (!response.ok && response.status < 300) { testErrors.push(`Frontend URL returned ${response.status}: ${response.statusText}`);Alternatively, if redirects are needed, extract the
Locationheader and validate it withvalidateExternalUrl()before following manually.apps/web-next/src/app/api/v1/plugin-publisher/test/route.ts (2)
88-94:⚠️ Potential issue | 🔴 CriticalFetch follows redirects by default, bypassing SSRF validation.
An attacker can host a page that redirects to a private/internal address. The initial URL passes validation, but
fetchfollows the redirect to the internal target without re-validation, enabling SSRF.🔒 Proposed fix to block redirect following
const response = await fetch(frontendUrl, { method: 'GET', signal: controller.signal, + redirect: 'error', headers: { 'Accept': '*/*', }, });Alternatively, use
redirect: 'manual'and validate theLocationheader if you need to support redirects to external URLs.
155-158:⚠️ Potential issue | 🔴 CriticalSame redirect bypass vulnerability as in
testFrontendLoading.Add
redirect: 'error'to prevent SSRF via redirect.🔒 Proposed fix
const response = await fetch(healthUrl, { method: 'GET', signal: controller.signal, + redirect: 'error', });
🤖 Fix all issues with AI agents
In `@apps/web-next/src/app/api/v1/plugin-publisher/test-frontend/route.ts`:
- Around line 32-49: The hostname checks miss IPv4-mapped IPv6 addresses (e.g.
::ffff:127.0.0.1) which can bypass SSRF filtering; update the validation in the
route handler (the block referencing hostname and isIpv6Literal) to detect
IPv4-mapped IPv6 forms (e.g. leading ::ffff: or ::FFFF:) and extract the mapped
IPv4 portion, then run that extracted IPv4 through the same private/local checks
you already use for plain IPv4 (startsWith '127.', '10.', '192.168.',
/^172\.(1[6-9]|2\d|3[01])\./, etc.); alternatively call a robust IP parsing
utility (Node's net or an IP library) to canonicalize hostname to IPv4/IPv6 and
validate the canonical address against your existing private/internal rules so
mapped addresses cannot bypass the checks.
In `@apps/web-next/src/app/api/v1/plugin-publisher/test/route.ts`:
- Around line 29-49: The SSRF check misses IPv4-mapped IPv6 like
"::ffff:127.0.0.1"; modify the validation to normalize such addresses before
applying the existing checks: when hostname starts with the IPv4-mapped prefix
(e.g. "::ffff:") extract the trailing IPv4 portion (the part after the last ':')
into a new variable (e.g. hostToCheck) and set isIpv6Literal accordingly, then
run the current private-network checks against hostToCheck (keeping existing
tests for startsWith('127.'), /^172\./, etc.) so mapped IPv4s are caught by the
IPv4 rules.
In `@plugins/daydream-video/backend/src/services/daydream.ts`:
- Around line 229-242: validateStreamId currently only rejects literal '..', '/'
and '\' but allows percent-encoded traversal like '%2e%2e' or '%2f'; update
validateStreamId to first safely attempt decodeURIComponent(streamId) inside a
try/catch (throw on decode failure), then check both the decoded value for '..',
'/' and '\' and also the original raw string for percent-encoded byte sequences
corresponding to '.' '/' and '\' (case‑insensitive '%2e', '%2f', '%5c' and
combinations like '%2e%2e') and throw the same error if any are found; keep the
existing error messages and use the function name validateStreamId to locate
where to apply these checks.
🧹 Nitpick comments (1)
apps/web-next/src/app/api/v1/plugin-publisher/test-frontend/route.ts (1)
136-140: Consider adding type validation forfrontendUrl.The check
!frontendUrlcatches missing/falsy values but not type mismatches. If a client sends an object or array,new URL()will coerce it viatoString(), potentially causing unexpected behavior.🛡️ Optional type guard
const body = await request.json(); const { frontendUrl } = body; - if (!frontendUrl) { + if (!frontendUrl || typeof frontendUrl !== 'string') { return errors.badRequest('frontendUrl is required'); }
Replace hand-rolled IP string checks with ipaddr.js (v2.3.0) to properly handle all IP edge cases raised in PR review: - IPv4-mapped IPv6 bypass (::ffff:127.0.0.1, ::ffff:169.254.169.254) → ipaddr.process() normalizes these to IPv4 before range check - Full range classification via addr.range() covers: loopback, private, linkLocal, uniqueLocal, carrierGradeNat, unspecified, reserved — no manual CIDR matching needed - Trailing dots, bracket stripping, and case normalization Also fixes percent-encoded path traversal bypass (%2e%2e, %2f, %5c) in validateStreamId() and sanitizePath() by decoding before checking. Closes review comments from Vercel, CodeRabbit, and GitHub Advanced Security. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/livepeer-svc/src/server.ts (1)
482-494:⚠️ Potential issue | 🔴 CriticalSSRF fix is incomplete —
cliUrlis not validated.While
aiUrlis now validated, the requiredcliUrlparameter is not subject to the same validation. SincecliUrlis used to createLivepeerCliClientinstances (line 190) that make HTTP requests, an attacker can register a node with a maliciouscliUrlpointing to internal services (e.g.,http://169.254.169.254/for cloud metadata).Apply the same loopback restriction to
cliUrl(and considermediaUrlif it's used for outbound requests elsewhere):Proposed fix
+function isAllowedCliUrl(url: string): boolean { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return false; + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return false; + } + const allowedHosts = new Set(['localhost', '127.0.0.1', '::1']); + return allowedHosts.has(parsed.hostname); +} router.post('/livepeer/nodes', async (req, res) => { const { id, name, cliUrl, aiUrl, mediaUrl, role } = req.body || {}; if (!id || !cliUrl) { return res.status(400).json({ success: false, error: { code: 'BAD_REQUEST', message: 'id and cliUrl required' } }); } + if (!isAllowedCliUrl(cliUrl)) { + return res + .status(400) + .json({ success: false, error: { code: 'BAD_REQUEST', message: 'invalid cliUrl' } }); + } if (!isAllowedAiUrl(aiUrl)) {
🤖 Fix all issues with AI agents
In `@packages/livepeer-node-client/src/clients/AIClient.ts`:
- Around line 65-75: The LivepeerAIClient constructor currently enforces
loopback-only hosts which conflicts with livepeer-svc's isAllowedAiUrl() that
allows any http/https remote URL; update the LivepeerAIClient constructor (class
LivepeerAIClient) to align with isAllowedAiUrl() by removing the loopback-only
hostname check and only validate protocol (http/https) or else call/shared-use
the same isAllowedAiUrl() helper used by livepeer-svc; update the thrown error
text to reference LIVEPEER_AI_URL/node.aiUrl and add/adjust tests and docs to
reflect that remote AI gateway URLs are allowed (or, if you prefer the stricter
security model, instead update isAllowedAiUrl() to enforce loopback-only
hostnames and update its callers and docs accordingly)—pick one approach and
make the validation consistent across both places.
- Around line 186-188: In getLiveVideoStatus, CodeQL flags SSRF because hostname
validation happens elsewhere; refactor the method to build the request using the
new URL() constructor (e.g., const url = new
URL(`/live/video-to-video/${safeId}/status`, this.baseUrl)) and perform an
explicit origin/hostname validation on url.origin or url.hostname inline
(allowlist against the expected origin/host) before calling fetch; keep the
existing sanitizePath(streamId) usage for safeId and rely on the same baseUrl
validated in the constructor but duplicate the origin/hostname check here so
static analysis recognizes the safe pattern.
🧹 Nitpick comments (1)
services/livepeer-svc/src/server.ts (1)
159-173: SSRF validation logic is sound.The function correctly parses the URL, validates the protocol, and restricts to loopback hosts. The use of
URLconstructor handles malformed inputs gracefully.Minor edge case:
localhost.(FQDN with trailing dot) would not matchlocalhostin the Set, potentially allowing a bypass on some systems. This is low risk but could be addressed by normalizing the hostname:- return allowedHosts.has(parsed.hostname); + const hostname = parsed.hostname.replace(/\.$/, ''); + return allowedHosts.has(hostname);
Comment on lines
65
to
+75
| constructor(baseUrl: string = 'http://localhost:9935') { | ||
| // Validate baseUrl to prevent SSRF via constructor injection | ||
| const parsed = new URL(baseUrl); | ||
| if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { | ||
| throw new Error(`LivepeerAIClient: unsupported protocol "${parsed.protocol}"`); | ||
| } | ||
| // Only allow loopback hosts for the AI gateway to avoid SSRF against arbitrary hosts. | ||
| const allowedHosts = new Set(['localhost', '127.0.0.1', '::1']); | ||
| if (!allowedHosts.has(parsed.hostname)) { | ||
| throw new Error(`LivepeerAIClient: disallowed hostname "${parsed.hostname}"`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for LivepeerAIClient instantiations across the codebase
rg -n 'new\s+LivepeerAIClient|LivepeerAIClient\s*\(' --type ts --type js -A 3 -B 1Repository: livepeer/naap
Length of output: 778
🏁 Script executed:
# Also check for any baseUrl references in configuration or setup files
rg -n 'baseUrl' packages/livepeer-node-client --type ts --type js -B 2 -A 2Repository: livepeer/naap
Length of output: 10025
🏁 Script executed:
# Check for test files that might show expected usage patterns
fd -e test.ts -e test.js -e spec.ts -e spec.js packages/livepeer-node-client | head -20Repository: livepeer/naap
Length of output: 478
🏁 Script executed:
# Look at the entire AIClient.ts file to understand the context better
wc -l packages/livepeer-node-client/src/clients/AIClient.tsRepository: livepeer/naap
Length of output: 114
🏁 Script executed:
# Search for LIVEPEER_AI_URL configuration and initialization
rg -n 'LIVEPEER_AI_URL' --type ts --type js --type env --type md -B 2 -A 2Repository: livepeer/naap
Length of output: 84
🏁 Script executed:
# Check for aiUrl configuration in node setup
rg -n 'aiUrl|node\.aiUrl' --type ts --type js -B 3 -A 2Repository: livepeer/naap
Length of output: 3394
🏁 Script executed:
# Look for environment files or configuration documentation
fd -e '.env*' -e 'config.*' --full-path 'packages/livepeer-node-client|services' | head -20Repository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Check test files to see how the client is typically instantiated
fd -e 'test.ts' -e 'test.js' -e 'spec.ts' -e '*.spec.js' | xargs rg -l 'LivepeerAIClient' 2>/dev/null | head -5Repository: livepeer/naap
Length of output: 39
🏁 Script executed:
# Find the isAllowedAiUrl function definition
rg -n 'isAllowedAiUrl' --type ts --type js -B 5 -A 10Repository: livepeer/naap
Length of output: 2372
🏁 Script executed:
# Also check for any environment variable definitions or defaults
rg -n 'LIVEPEER_AI_URL\s*=' --type ts --type js -A 2Repository: livepeer/naap
Length of output: 615
🏁 Script executed:
# Check the complete constructor validation in both AIClient and CliClient for comparison
rg -n 'constructor.*baseUrl' packages/livepeer-node-client/src/clients --type ts -A 15Repository: livepeer/naap
Length of output: 4694
Address validation mismatch: server accepts remote aiUrl but AIClient constructor rejects it.
Lines 65-75 implement loopback-only hostname validation. This conflicts with isAllowedAiUrl() in livepeer-svc (line 159–169), which accepts any http/https URL. If LIVEPEER_AI_URL or node.aiUrl is set to a remote gateway—which the server-side validation permits—LivepeerAIClient will throw at instantiation, breaking deployments.
All current call sites (pipeline-gateway, livepeer-svc) default to localhost, but this restriction contradicts the broader allowlist design. Clarify whether loopback-only is the intended security boundary. If remote gateways must be supported, either:
- Align AIClient validation with
isAllowedAiUrl(), or - Update
isAllowedAiUrl()to enforce loopback-only, or - Document that only loopback AI gateways are supported and update environment defaults accordingly.
🤖 Prompt for AI Agents
In `@packages/livepeer-node-client/src/clients/AIClient.ts` around lines 65 - 75,
The LivepeerAIClient constructor currently enforces loopback-only hosts which
conflicts with livepeer-svc's isAllowedAiUrl() that allows any http/https remote
URL; update the LivepeerAIClient constructor (class LivepeerAIClient) to align
with isAllowedAiUrl() by removing the loopback-only hostname check and only
validate protocol (http/https) or else call/shared-use the same isAllowedAiUrl()
helper used by livepeer-svc; update the thrown error text to reference
LIVEPEER_AI_URL/node.aiUrl and add/adjust tests and docs to reflect that remote
AI gateway URLs are allowed (or, if you prefer the stricter security model,
instead update isAllowedAiUrl() to enforce loopback-only hostnames and update
its callers and docs accordingly)—pick one approach and make the validation
consistent across both places.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-next/src/app/api/v1/plugin-publisher/test-frontend/route.ts (1)
10-80:⚠️ Potential issue | 🔴 CriticalSSRF gaps: Domains resolving to private IPs and redirect chains not validated.
The current implementation only blocks literal IPs and hardcoded internal suffixes (
.localhost,.internal,.local). A domain that resolves to127.0.0.1or private ranges will pass validation. Additionally,fetch()follows redirects by default (up to 20), without validating each target, allowing an attacker to redirect through internal IPs.To fully mitigate:
- Add DNS resolution (async): Resolve the hostname and validate all returned IPs against blocked ranges before fetching.
- Validate redirects: Either disable with
redirect: 'manual'and validate eachLocationheader, or re-validate each redirect target's IP.Note: Even after adding DNS lookup, there remains a TOCTOU window where the HTTP client performs a fresh DNS resolution; for strict protection, consider pinning the connection to the validated IP address directly instead of relying on the domain name during fetch.
🤖 Fix all issues with AI agents
In `@plugins/plugin-publisher/backend/src/services/pluginTester.ts`:
- Around line 23-67: The validateExternalUrl function must be made async and
perform DNS resolution of the hostname (using dns.promises.resolve4/resolve6 or
lookup) before returning true: resolve all A/AAAA records for parsed.hostname,
run each resolved IP through ipaddr.process(...).range() and compare against
BLOCKED_RANGES (reject if any match), and keep existing literal-IP checks;
update validateExternalUrl signature/usages accordingly. Additionally, when
making outbound requests (the code path that calls validateExternalUrl and
performs fetch), set fetch(..., { redirect: 'manual' }) and reject non-2xx
responses or, for 3xx responses, validate the Location header by passing the
resolved/normalized Location URL back through validateExternalUrl for each
redirect step before following it (or simply abort on any redirect to internal
addresses). Ensure all places referencing validateExternalUrl are updated to
await the new async behavior.
…testing routes - Updated fetch requests in testFrontendLoading and testBackendHealth functions to include redirect: 'error' option, ensuring that any redirection issues are properly handled. - This change enhances the robustness of the testing framework by preventing silent failures during API calls.
…F bypass - Updated hostname validation in LivepeerAIClient to normalize trailing dots (e.g., "localhost." to "localhost") before checking against the allowed hosts. This change enhances security by preventing potential SSRF attacks through hostname manipulation.
- Introduced ipaddr.js (v2.3.0) to enhance IP address handling in the project. - This addition supports robust validation and normalization of IP addresses, addressing edge cases and improving security measures against potential SSRF vulnerabilities.
- Introduced a validatedOrigin property to the LivepeerAIClient class to ensure that all constructed URLs match the expected origin, preventing potential SSRF vulnerabilities. - Refactored fetch calls to utilize a new buildUrl method, which enforces origin checks and improves URL construction safety across various API endpoints. - Updated methods including textToSpeech, llmStream, getLiveVideoStatus, processRequest, getNetworkCapabilities, and others to use the new buildUrl method for consistent validation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web-next/src/app/api/v1/plugin-publisher/test/route.ts`:
- Around line 6-67: The validator currently only checks literal IPs and
suffixes; to prevent DNS rebinding change validateExternalUrl to be async and
resolve the hostname's A and AAAA records (use Node's
dns.promises.resolve4/resolve6 or dns.promises.lookup with all:true) and
validate every resolved address with ipaddr.process(...).range() against
BLOCKED_RANGES (same logic as the existing ip check), returning invalid if any
resolved address is in a blocked range; update all callers (testFrontendLoading,
testBackendHealth, etc.) to await validateExternalUrl before performing fetches.
- Refactored validateExternalUrl to be asynchronous and added DNS resolution for domain names, improving security against DNS-rebinding attacks. - Introduced isBlockedIp function to check if an IP address falls within blocked ranges, enhancing internal network protection. - Implemented safeFetch function to handle redirects safely, ensuring each redirect target is validated against allowed URLs before following. - Updated testFrontendLoading and testBackendHealth functions to utilize safeFetch for improved error handling and SSRF protection.
github-actions
Bot
added
size/XL
- Refactored validateExternalUrl to be asynchronous, enabling DNS resolution for domain names and enhancing security against DNS-rebinding attacks. - Introduced isBlockedIp function to check if an IP address falls within blocked ranges, improving internal network protection. - Updated testFrontendLoading and testBackendHealth functions to await validateExternalUrl, ensuring proper validation of URLs before proceeding with requests.
- Updated validateExternalUrl to only allow HTTPS protocol, enhancing security by preventing HTTP requests. - Modified safeFetch to validate each request and redirect target for HTTPS compliance, throwing errors for non-compliant URLs. - Updated test cases in testFrontendLoading and testBackendHealth to use HTTPS URLs, ensuring consistent testing against the new validation rules.
seanhanca
approved these changes
Feb 12, 2026
seanhanca
pushed a commit
that referenced
this pull request
Feb 12, 2026
Address 103 high-severity CodeQL alerts across 34 files: **Insecure temporary files (js/insecure-temporary-file) — 20 alerts:** - Replace predictable temp paths with crypto.randomUUID() and fs.mkdtempSync() **User-controlled bypass (js/user-controlled-bypass) — 14 alerts:** - Add typeof checks for auth tokens, codes, emails, passwords - Remove user-controlled skipVerification flag from publish endpoints - Validate GitHub webhook headers with typeof guards - Add OAuth error code allowlist in callback handler **Remote property injection (js/remote-property-injection) — 6 alerts:** - Block __proto__, constructor, prototype keys in config merge/assignment **File system race (js/file-system-race) — 3 alerts:** - Replace TOCTOU check-then-read with try/catch around direct operations **Tainted format string (js/tainted-format-string) — 5 alerts:** - Sanitize user input (strip control chars/newlines) before logging **Incomplete sanitization + bad HTML filter (js/bad-tag-filter) — 2 alerts:** - Replace single-pass regex HTML stripping with iterative loop **Path injection (js/path-injection) — 20 alerts:** - Add safeResolvePath() for storage paths - Add sanitizePathComponent() for plugin/version path segments - Add validateFilePath() for uploaded file paths **SQL injection (js/sql-injection) — 5 alerts:** - Add validateIdentifier() strict allowlist for DDL identifiers - Sanitize database names, usernames, and passwords **XSS through DOM (js/xss-through-dom) — 2 alerts:** - Validate avatar URLs with URL constructor, block javascript: URIs **Reflected XSS (js/reflected-xss) — 1 alert:** - Validate plugin name against allowlist before HTML interpolation **Insecure download (js/insecure-download) — 2 alerts:** - Enforce HTTPS protocol validation before fetch calls **Insecure randomness (js/insecure-randomness) — 1 alert:** - Replace Math.random() with crypto.randomInt() **Insufficient password hash (js/insufficient-password-hash) — 2 alerts:** - Replace SHA-256 with crypto.scryptSync for API key hashing **Missing rate limiting (js/missing-rate-limiting) — 13 alerts:** - Add in-memory rate limiting to auth, registry, embed, and config routes **Insecure Helmet configuration (js/insecure-helmet-configuration) — 2 alerts:** - Replace disabled CSP with proper CSP directives **Incomplete URL sanitization (js/incomplete-url-substring-sanitization) — 5 alerts:** - Replace string includes/startsWith with URL constructor parsing Alerts fixed: #1-50, #52-53, #57-61, #76-77, #82-97, #102-128, #279-280 Co-authored-by: Cursor <cursoragent@cursor.com>
seanhanca
pushed a commit
that referenced
this pull request
Feb 12, 2026
Address 103 high-severity CodeQL alerts across 34 files: **Insecure temporary files (js/insecure-temporary-file) — 20 alerts:** - Replace predictable temp paths with crypto.randomUUID() and fs.mkdtempSync() **User-controlled bypass (js/user-controlled-bypass) — 14 alerts:** - Add typeof checks for auth tokens, codes, emails, passwords - Remove user-controlled skipVerification flag from publish endpoints - Validate GitHub webhook headers with typeof guards - Add OAuth error code allowlist in callback handler **Remote property injection (js/remote-property-injection) — 6 alerts:** - Block __proto__, constructor, prototype keys in config merge/assignment **File system race (js/file-system-race) — 3 alerts:** - Replace TOCTOU check-then-read with try/catch around direct operations **Tainted format string (js/tainted-format-string) — 5 alerts:** - Sanitize user input (strip control chars/newlines) before logging **Incomplete sanitization + bad HTML filter (js/bad-tag-filter) — 2 alerts:** - Replace single-pass regex HTML stripping with iterative loop **Path injection (js/path-injection) — 20 alerts:** - Add safeResolvePath() for storage paths - Add sanitizePathComponent() for plugin/version path segments - Add validateFilePath() for uploaded file paths **SQL injection (js/sql-injection) — 5 alerts:** - Add validateIdentifier() strict allowlist for DDL identifiers - Sanitize database names, usernames, and passwords **XSS through DOM (js/xss-through-dom) — 2 alerts:** - Validate avatar URLs with URL constructor, block javascript: URIs **Reflected XSS (js/reflected-xss) — 1 alert:** - Validate plugin name against allowlist before HTML interpolation **Insecure download (js/insecure-download) — 2 alerts:** - Enforce HTTPS protocol validation before fetch calls **Insecure randomness (js/insecure-randomness) — 1 alert:** - Replace Math.random() with crypto.randomInt() **Insufficient password hash (js/insufficient-password-hash) — 2 alerts:** - Replace SHA-256 with crypto.scryptSync for API key hashing **Missing rate limiting (js/missing-rate-limiting) — 13 alerts:** - Add in-memory rate limiting to auth, registry, embed, and config routes **Insecure Helmet configuration (js/insecure-helmet-configuration) — 2 alerts:** - Replace disabled CSP with proper CSP directives **Incomplete URL sanitization (js/incomplete-url-substring-sanitization) — 5 alerts:** - Replace string includes/startsWith with URL constructor parsing Alerts fixed: #1-50, #52-53, #57-61, #76-77, #82-97, #102-128, #279-280 Co-authored-by: Cursor <cursoragent@cursor.com>
5 tasks
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
3 participants
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
Changes by category
Server-side request forgery (js/request-forgery) — 17 alerts:
test-backend/route.ts,test-frontend/route.ts,test/route.tsvalidateExternalUrl()to block private/internal network requestspluginTester.tsplugin-server/server.ts,plugin-publisher/server.tsencodeURIComponent()on user-supplied path params before URL interpolationdaydream.tsvalidateStreamId()regex guard on stream IDs used in API URLsexternalProxy.tsendsWith(h)→ exact match or.${h}subdomain checkAIClient.tssanitizePath()for dynamic URL segmentsCliClient.tsrequest()Type confusion (js/type-confusion-through-parameter-tampering) — 1 alert:
daydream.tstypeof modelId !== 'string'guard ingetControlnetsForModel()Test plan
Made with Cursor
Summary by CodeRabbit
Security Improvements
Stability