Conversation
|
Warning Rate limit exceeded@tcsenpai has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds new .gitignore patterns and implements Web2 request sanitization utilities; DAHR.toSerializable uses sanitized headers for storage, and handleWeb2 logs a redacted version of Web2 requests instead of the raw object. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HandleWeb2
participant Logger
participant DAHR
participant Storage
Client->>HandleWeb2: send IWeb2Request
HandleWeb2->>HandleWeb2: sanitizedForLog = sanitizeWeb2RequestForLogging(request)
HandleWeb2->>Logger: log(JSON.stringify(sanitizedForLog, null, 2))
HandleWeb2->>DAHR: forward request
DAHR->>DAHR: sanitizedForStorage = sanitizeWeb2RequestForStorage(request)
DAHR->>Storage: toSerializable(..., web2Request: sanitizedForStorage)
Storage-->>DAHR: ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨Latest suggestions up to 7760646
Previous suggestionsSuggestions up to commit 4399b2b
|
|||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/web2/dahr/DAHR.ts (1)
13-24: Consider documenting the sensitive headers list and verifying completeness.The list of sensitive headers is reasonable, but consider:
- Adding JSDoc comments to explain why these headers are sanitized
- Evaluating whether additional headers should be included (e.g.,
www-authenticate,x-csrf-token,authentication)/** * Set of sensitive HTTP header keys that should be excluded from serialization * to prevent exposure of authentication credentials, API keys, and session tokens. * All keys are lowercase to enable case-insensitive matching. */ const SENSITIVE_HEADER_KEYS = new Set([ "authorization", "proxy-authorization", "x-api-key", "api-key", "x-auth-token", "x-access-token", "x-authorization", "x-session-token", "cookie", "set-cookie", ])Should additional security-sensitive headers like
www-authenticate,authentication, orx-csrf-tokenbe included in this list?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.gitignore(1 hunks)src/features/web2/dahr/DAHR.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T22:59:52.023Z
Learnt from: tcsenpai
Repo: kynesyslabs/node PR: 477
File: src/libs/blockchain/gcr/handleGCR.ts:336-377
Timestamp: 2025-10-10T22:59:52.023Z
Learning: In the Storage Programs feature for the Demos Network, deleted storage programs are kept in the database with null metadata as an audit trail and historical record. Storage addresses are single-use and cannot be reused after deletion, preventing re-creation at the same address even after a DELETE operation. This is an intentional design decision by tcsenpai to preserve history.
Applied to files:
.gitignore
🔇 Additional comments (3)
.gitignore (1)
156-160: LGTM! Appropriate patterns for test artifacts and network captures.The added ignore patterns correctly exclude test fixtures, capture scripts, and network traffic files from version control.
src/features/web2/dahr/DAHR.ts (2)
26-26: LGTM! Appropriate type alias for non-nullable headers.
150-156: LGTM! Sanitization properly integrated into serialization.The header sanitization is correctly applied during serialization while preserving all other properties of the raw request. The conditional handling of undefined
rawis appropriate.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/web2/dahr/DAHR.ts (1)
13-24: LGTM! Comprehensive set of sensitive headers.The current list covers the most common sensitive authentication headers and credentials. The lowercase naming is correct for case-insensitive HTTP header matching.
Optionally, consider adding these headers for completeness:
"x-csrf-token""x-xsrf-token""www-authenticate""proxy-authenticate"These are less commonly used for outbound requests but can carry sensitive data in certain contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/features/web2/dahr/DAHR.ts(2 hunks)
🔇 Additional comments (2)
src/features/web2/dahr/DAHR.ts (2)
26-26: LGTM! Type alias improves readability.The type alias clarifies the intent without changing behavior.
34-52: LGTM! Past issues resolved.The function now includes JSDoc documentation and uses
Record<string, any>for type safety, addressing the previous review feedback. The implementation correctly handles all edge cases (undefined input, empty headers, all headers sensitive) and uses case-insensitive comparison appropriate for HTTP headers.
src/features/web2/dahr/DAHR.ts
Outdated
| const raw = this.web2Request.raw | ||
| const sanitizedRaw = raw | ||
| ? { | ||
| ...raw, | ||
| headers: sanitizeHeaders(raw.headers), | ||
| } | ||
| : raw |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Sanitization correctly applied to serialization.
The modification correctly sanitizes headers only during serialization, leaving the original _web2Request intact for internal use (e.g., in startProxy). This ensures credentials remain available for legitimate proxy operations while preventing exposure in serialized output.
Verify that all serialization paths go through toSerializable() and no code directly accesses or serializes DAHR._web2Request or DAHR.web2Request:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of DAHR serialization and direct web2Request access
# Find all calls to toSerializable on DAHR instances
echo "=== Calls to toSerializable() ==="
rg -nP --type=ts -C3 '\b\w+\.toSerializable\s*\(' -g '!**/test/**'
# Find direct access to DAHR.web2Request that might bypass sanitization
echo -e "\n=== Direct access to DAHR.web2Request ==="
rg -nP --type=ts -C3 '(dahr|DAHR)\.web2Request(?!:)' -g '!**/test/**'
# Find JSON.stringify or similar serialization of DAHR instances
echo -e "\n=== JSON.stringify of DAHR instances ==="
rg -nP --type=ts -C3 'JSON\.stringify.*\b(dahr|DAHR)\b' -g '!**/test/**'Length of output: 1646
🏁 Script executed:
#!/bin/bash
# Description: Verify DAHR instances aren't stored/passed/logged in ways that could bypass sanitization
# Check if DAHR instances are stored in variables/fields that might be accessed later
echo "=== DAHR instance storage/assignment ==="
rg -nP --type=ts -C5 '\b(dahr|DAHR)\s*=' -g '!**/test/**' | head -50
# Check if web2Request is accessed in logging, console, or debug contexts
echo -e "\n=== web2Request access in logging contexts ==="
rg -nP --type=ts -C3 '(console|log|debug|stringify).*web2Request|web2Request.*(console|log|debug)' -g '!**/test/**'
# Check if DAHR is passed to other functions beyond the immediate context
echo -e "\n=== DAHR passed as parameter ==="
rg -nP --type=ts -C2 'handleWeb2.*\(.*dahr|dahr.*\)' -g '!**/test/**'
# Check if web2Request.raw is accessed elsewhere
echo -e "\n=== web2Request.raw access ==="
rg -nP --type=ts -B2 -A2 'web2Request\.raw' -g '!**/test/**'Length of output: 14007
Logging exposes credentials before DAHR creation—update sanitization strategy.
The sanitization in toSerializable() is correct for serialized responses, but a critical security gap exists at an earlier stage:
At handleWeb2.ts:24, the entire web2Request (including headers with credentials) is logged via JSON.stringify before conversion to DAHR. This upstream logging bypasses the sanitization applied later in toSerializable().
Required actions:
- Remove or redact headers from the log statement at
handleWeb2.ts:24 - Verify that debug logging doesn't expose
web2Requestin other locations - Consider sanitizing headers at the request ingestion point (before logging), not just during serialization
The current approach sanitizes only the serialized output while leaving the original object logged with full credentials exposed.
🤖 Prompt for AI Agents
In src/features/web2/dahr/DAHR.ts around lines 156 to 162: the current
sanitization in toSerializable() is good but logging earlier in handleWeb2.ts
(line 24) serializes and logs the entire web2Request (including headers with
credentials), so remove or redact headers from that log call; specifically,
replace the log of the full web2Request with a sanitized copy that omits or
redacts web2Request.headers (or at minimum mask Authorization/Cookie/Set-Cookie
values) before JSON.stringify, and add sanitation at the request ingestion point
so any debug logs use the sanitized object; also scan other logging sites for
direct web2Request logging and update them to use the sanitized version.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the logging statement in handleWeb2.ts to verify the fix
echo "=== handleWeb2.ts logging implementation ==="
rg -nP --type=ts -C10 'JSON\.stringify.*web2Request|log.*web2Request' src/features/web2/handleWeb2.ts
# Check the sanitizeWeb2RequestForLogging implementation
echo -e "\n=== sanitizeWeb2Request.ts implementation ==="
cat src/features/web2/sanitizeWeb2Request.ts
# Verify no other locations log unsanitized web2Request
echo -e "\n=== Other potential logging of web2Request ==="
rg -nP --type=ts -C3 '(console|log|debug).*web2Request(?!ForLog)|JSON\.stringify.*web2Request(?!ForLog)' -g '!**/test/**' -g '!src/features/web2/sanitizeWeb2Request.ts'
Length of output: 2384
🏁 Script executed:
#!/bin/bash
# Read the entire handleWeb2.ts file to verify the logging implementation
cat src/features/web2/handleWeb2.ts
Length of output: 1608
@tcsenpai Excellent work! 🎉 The fix is properly implemented and addresses the security vulnerability completely.
Verified changes:
-
✅ handleWeb2.ts now sanitizes before logging (lines 25-30)
- Imports
sanitizeWeb2RequestForLogging - Creates sanitized copy before JSON.stringify
- Logs only the redacted version
- Imports
-
✅ sanitizeWeb2Request.ts provides comprehensive protection
- Covers all major credential headers (authorization, cookies, API keys, tokens)
- Uses redaction (
"[redacted]") for logging (shows headers exist without exposing values) - Uses stripping (removal) for storage in
toSerializable()
-
✅ No other unsafe logging detected in the codebase
The two-tier approach is ideal:
- Storage: strips sensitive headers entirely
- Logging: redacts values to
"[redacted]"(preserves context for debugging)
The original credentials remain available in the internal _web2Request for legitimate proxy operations. Well done! 🚀
🧬 Code Graph Analysis Results
File: src/utilities/generateUniqueId.ts
At lines 3-9:
export function generateUniqueId(): string {
const timestamp = Date.now().toString()
const random = Math.random().toString()
const data = timestamp + random
return Hashing.sha256(data)
}
File: src/features/web2/proxy/ProxyFactory.ts
At lines 6-15:
export class ProxyFactory {
/**
* Create a new Proxy instance.
* @param {string} dahrSessionId - The session ID.
* @returns {Proxy} The created Proxy instance.
*/
static createProxy(dahrSessionId: string): Proxy {
return new Proxy(dahrSessionId)
}
}
File: src/utilities/required.ts
At lines 16-26:
export default function required(
value: any,
msg = "Missing required element",
fatal = true,
): RequiredOutcome {
if (!value) {
if (fatal) throw new Error("[REQUIRED] " + msg)
return { success: false, message: msg }
}
return { success: true, message: "" }
}
File: src/features/web2/sanitizeWeb2Request.ts
At lines 64-82:
export function sanitizeWeb2RequestForStorage(
web2Request: IWeb2Request,
): IWeb2Request {
if (!web2Request) {
return web2Request
}
const raw = web2Request.raw
return {
...web2Request,
raw: raw
? {
...raw,
headers: stripSensitiveWeb2Headers(raw.headers),
}
: raw,
}
}
File: src/features/web2/validator.ts
At lines 17-146:
export function validateAndNormalizeHttpUrl(
input: string,
): UrlValidationResult {
const trimmed = (input ?? "").trim()
if (!trimmed) {
return { ok: false, status: 400, message: "Invalid URL: empty value" }
}
try {
const parsed = new URL(trimmed)
// 1. Ensure protocol is http(s)
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
return {
ok: false,
status: 400,
message: "Invalid URL scheme. Only http(s) are allowed",
}
}
// 2. Reject URLs with embedded credentials (username/password)
if (parsed.username || parsed.password) {
return {
ok: false,
status: 400,
message: "Invalid URL: embedded credentials are not allowed",
}
}
// 3. Reject URLs without a hostname
if (!parsed.hostname) {
return {
ok: false,
status: 400,
message: "Invalid URL: URL must have a hostname",
}
}
const hostLower = parsed.hostname.toLowerCase()
// 4. Reject localhost and loopback hostnames
if (hostLower === "localhost" || hostLower.endsWith(".localhost")) {
return {
ok: false,
status: 400,
message: "Localhost targets are not allowed",
}
}
// 5. Block loopback and private/link-local/reserved ranges (IPv4, IPv6, and IPv4-mapped IPv6)
const ipVersion = net.isIP(hostLower)
const isIPv6Unspecified = hostLower === "::"
const isIPv6Loopback = hostLower === "::1"
const isIPv6Multicast = hostLower.startsWith("ff")
const isIPv4MappedLoopback = hostLower.startsWith("::ffff:127.")
const isIPv4Loopback = /^127(?:\.\d{1,3}){3}$/.test(hostLower)
const isIPv4Private =
/^10\./.test(hostLower) ||
(/^172\.(\d{1,3})\./.test(hostLower) &&
(() => {
const m = hostLower.match(/^172\.(\d{1,3})\./)
if (!m) return false
const o = Number(m[1])
return o >= 16 && o <= 31
})()) ||
/^192\.168\./.test(hostLower) ||
/^169\.254\./.test(hostLower) ||
/^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(hostLower) || // 100.64.0.0/10
/^0\./.test(hostLower)
const isIPv6ULAorLL =
ipVersion === 6 &&
(hostLower.startsWith("fc") ||
hostLower.startsWith("fd") ||
/^fe[89ab][0-9a-f]*:/i.test(hostLower))
// Also block IPv4-mapped private ranges (::ffff:10.x, ::ffff:172.16-31.x, ::ffff:192.168.x, ::ffff:169.254.x, ::ffff:100.64-127.x)
const mappedMatch = hostLower.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/)
let isMappedPrivate = false
if (mappedMatch) {
const v4 = mappedMatch[1]
if (/^127(?:\.\d{1,3}){3}$/.test(v4)) isMappedPrivate = true
if (/^10\./.test(v4)) isMappedPrivate = true
const m172 = v4.match(/^172\.(\d{1,3})\./)
if (m172) {
const o = Number(m172[1])
if (o >= 16 && o <= 31) isMappedPrivate = true
}
if (/^192\.168\./.test(v4)) isMappedPrivate = true
if (/^169\.254\./.test(v4)) isMappedPrivate = true
if (/^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(v4))
isMappedPrivate = true
if (/^0\./.test(v4)) isMappedPrivate = true
}
if (
isIPv6Unspecified ||
isIPv6Loopback ||
isIPv6Multicast ||
isIPv4MappedLoopback ||
isIPv4Private ||
isMappedPrivate ||
isIPv6ULAorLL
) {
return {
ok: false,
status: 400,
message:
"Private, link-local, or loopback targets are not allowed",
}
}
// 6. Canonicalize the URL (lowercase host, strip default ports, remove fragment)
const canonicalUrlObject = new URL(parsed.toString())
canonicalUrlObject.hostname = canonicalUrlObject.hostname.toLowerCase()
// Strip default ports
if (
(canonicalUrlObject.protocol === "http:" &&
canonicalUrlObject.port === "80") ||
(canonicalUrlObject.protocol === "https:" &&
canonicalUrlObject.port === "443")
) {
canonicalUrlObject.port = ""
}
// Remove fragment
canonicalUrlObject.hash = ""
return { ok: true, normalizedUrl: canonicalUrlObject.toString() }
} catch {
return { ok: false, status: 400, message: "Invalid URL format" }
}
}
File: src/features/web2/proxy/Proxy.ts
At lines 23-528:
export class Proxy {
private _server: http.Server | https.Server | null = null
private _proxyPort = 0
private _isInitialized = false
private _currentTargetUrl = ""
constructor(
private readonly _dahrSessionId: string,
private readonly _proxyHost: string = "localhost",
private readonly _authConfig: IAuthorizationConfig = {
requireAuthForAll: SharedState.getInstance().PROD,
exceptions: [],
},
private readonly _sslConfig: {
verifyCertificates: boolean
} = {
verifyCertificates: SharedState.getInstance().PROD, // Enable in production, disable in dev
},
) {
required(this._dahrSessionId, "Missing dahr session Id")
}
/**
* Sends an HTTP/HTTPS request through the proxy.
* @returns Promise resolving to the response data
* @throws Error if the proxy server fails to start or if the request fails
*/
async sendHTTPRequest(
params: ISendHTTPRequestParams,
): Promise<IWeb2Result> {
const {
web2Request,
targetMethod,
targetHeaders,
payload,
targetAuthorization,
} = params
const targetUrl = web2Request.raw.url
required(web2Request.raw, "web2Request.raw")
// Only initialize the proxy server if it's not already running or the target URL has changed
if (!this._isInitialized || this._currentTargetUrl !== targetUrl) {
try {
await this.startProxyServer(targetUrl)
this._isInitialized = true
this._currentTargetUrl = targetUrl
} catch (error) {
console.error("[Web2API] Error starting proxy server:", error)
throw error
}
}
return new Promise((resolve, reject) => {
const headers = this.createHeaders(
targetMethod,
targetHeaders,
targetAuthorization,
targetUrl,
)
const req = http.request({
hostname: this._proxyHost,
port: this._proxyPort,
method: targetMethod,
path: "/",
headers,
timeout: 30000,
})
const chunks: Buffer[] = []
let responseHeaders: http.IncomingHttpHeaders = {}
let statusCode = 500
let statusMessage = "Unknown"
let requestHash: string | undefined
req.on("response", res => {
statusCode = res.statusCode || 500
statusMessage = res.statusMessage || "Unknown"
responseHeaders = res.headers
res.on("data", chunk => {
chunks.push(Buffer.from(chunk))
})
res.on("end", () => {
const dataBuffer = Buffer.concat(chunks)
const data = dataBuffer.toString()
// Create a hash over the exact UTF-8 bytes of the returned string data
const responseHash = Hashing.sha256Bytes(
Buffer.from(data, "utf8"),
)
const responseHeadersHash = Hashing.sha256(
this.canonicalizeHeaders(responseHeaders),
)
resolve({
status: statusCode,
statusText: statusMessage,
headers: responseHeaders,
data: data,
responseHash: responseHash,
responseHeadersHash: responseHeadersHash,
// Optional: include requestHash when a body was sent
...(requestHash ? { requestHash } : {}),
})
})
})
req.on("error", error => {
reject(error)
})
if (payload != null && !["GET", "DELETE"].includes(targetMethod)) {
const body =
typeof payload === "string"
? payload
: JSON.stringify(payload)
// Compute hash over the exact bytes we are about to transmit
requestHash = Hashing.sha256Bytes(Buffer.from(body, "utf8"))
;(req as any).setHeader(
"Content-Length",
Buffer.byteLength(body),
)
req.write(body)
}
req.end()
})
}
/**
* Stops the proxy server and cleans up resources.
* This method should be called when the proxy is no longer needed.
* It stops the proxy server and closes any open connections.
* @returns void
*/
async stopProxy(): Promise<void> {
const srv = this._server
if (!srv) {
this._isInitialized = false
return
}
await new Promise<void>(resolve => {
srv.close(() => {
// Only clear if we're still closing the same server
if (this._server === srv) {
this._server = null
this._isInitialized = false
}
resolve()
})
})
}
private async startProxyServer(targetUrl: string): Promise<void> {
if (this._isInitialized) {
await this.stopProxy()
}
await this.createNewServer(targetUrl)
}
private async createNewServer(targetUrl: string): Promise<void> {
return new Promise<void>((resolve, reject) => {
const { targetProtocol, targetHostname } = this.parseUrl(targetUrl)
// SSRF hardening: resolve DNS and block private/link-local/loopback destinations
const isDisallowedAddress = (addr: string): boolean => {
const lower = addr.toLowerCase()
const ipVersion = net.isIP(lower)
// Helper for IPv4 space
const isDisallowedV4 = (v4: string): boolean => {
if (/^127(?:\.\d{1,3}){3}$/.test(v4)) return true // loopback
if (/^10\./.test(v4)) return true // private
const m = v4.match(/^172\.(\d{1,3})\./)
if (m) {
const o = Number(m[1])
if (o >= 16 && o <= 31) return true
}
if (/^192\.168\./.test(v4)) return true // private
if (/^169\.254\./.test(v4)) return true // link-local
if (/^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(v4))
return true // CGNAT 100.64/10
if (/^0\./.test(v4)) return true // this network
if (/^(?:22[4-9]|23\d)\./.test(v4)) return true // multicast 224/4
if (/^(?:24\d|25[0-5])\./.test(v4)) return true // reserved 240/4 incl 255.255.255.255
return false
}
if (ipVersion === 6) {
if (lower === "::" || lower === "::1") return true // unspecified/loopback
if (lower.startsWith("ff")) return true // multicast ff00::/8
// ULA fc00::/7
if (lower.startsWith("fc") || lower.startsWith("fd"))
return true
// Link-local fe80::/10 → fe8x, fe9x, feax, febx
if (/^fe[89ab][0-9a-f]*:/i.test(lower)) return true
// IPv4-mapped IPv6 ::ffff:a.b.c.d → re-check mapped v4
const v4map = lower.match(
/^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/,
)
if (v4map && isDisallowedV4(v4map[1])) return true
return false
}
if (ipVersion === 4) {
return isDisallowedV4(lower)
}
return false
}
const preflight = async () => {
try {
// If hostname is already an IP, just check it; otherwise resolve all
const ipVersion = net.isIP(targetHostname)
if (ipVersion) {
if (isDisallowedAddress(targetHostname)) {
throw new Error(
"Target resolves to a private/link-local/loopback address",
)
}
} else {
const answers = await dns.lookup(targetHostname, {
all: true,
})
if (answers.some(a => isDisallowedAddress(a.address))) {
throw new Error(
"Target resolves to a private/link-local/loopback address",
)
}
}
} catch (e) {
reject(e)
return false
}
return true
}
// Create the proxy server (defaults; per-request options are supplied in proxyServer.web)
const proxyServer = httpProxy.createProxyServer({
target: targetUrl,
changeOrigin: true,
secure:
targetProtocol === "https:"
? this._sslConfig.verifyCertificates
: false,
ssl:
targetProtocol === "https:"
? {
rejectUnauthorized:
this._sslConfig.verifyCertificates,
}
: undefined,
})
// Handle proxy errors
proxyServer.on("error", (err: any, _req, res) => {
// Handle SSL certificate errors specifically
if (
err.code === "UNABLE_TO_VERIFY_LEAF_SIGNATURE" ||
err.code === "CERT_HAS_EXPIRED" ||
err.code === "DEPTH_ZERO_SELF_SIGNED_CERT"
) {
if (res instanceof http.ServerResponse) {
res.writeHead(502, {
"Content-Type": "application/json",
})
res.end(
JSON.stringify({
error: "SSL Certificate verification failed",
message: err.message,
code: err.code,
}),
)
}
return
}
// Handle other proxy errors
if (res instanceof http.ServerResponse) {
res.writeHead(500, {
"Content-Type": "application/json",
})
res.end(
JSON.stringify({
error: "Proxy error",
message: err.message,
}),
)
} else if (res instanceof net.Socket) {
console.error("[Web2API] Socket error:", err)
res.end(
"HTTP/1.1 500 Internal Server Error\r\n\r\n" +
JSON.stringify({
error: "Proxy error",
message: err.message,
}),
)
}
})
// Listen for proxy responses to set the correct status code
proxyServer.on("proxyRes", (proxyRes, _req, res) => {
// Set the status code and headers from the target API
res.writeHead(proxyRes.statusCode || 500, proxyRes.headers)
})
// Create the main HTTP server
this._server = http.createServer(async (req, res) => {
if (!this.isAuthorizedRequest(req)) {
res.writeHead(403)
res.end("Unauthorized")
return
}
// Ensure target is still safe at request time (DNS may have changed)
const ok = await preflight()
if (!ok) {
res.writeHead(400)
res.end("Invalid target host")
return
}
const { targetPathname, targetSearch, targetOrigin } =
this.parseUrl(targetUrl)
const outgoingPath = targetPathname + targetSearch
// Overwrite req.url with the correct path/query before proxying
req.url = outgoingPath
proxyServer.web(req, res, {
target: targetOrigin,
changeOrigin: true,
secure:
targetProtocol === "https:"
? this._sslConfig.verifyCertificates
: false,
})
})
// Start the server
this._server.listen(0, "0.0.0.0", () => {
const address = this._server?.address()
if (typeof address === "object" && address !== null) {
this._proxyPort = address.port
resolve()
} else {
reject(new Error("[Web2API] Failed to get server address"))
}
})
// Error handling for the main HTTP server
this._server.on("error", error => {
console.error("[Web2API] HTTP Server error:", error)
reject(error)
})
})
}
private isAuthorizedRequest(req: http.IncomingMessage): boolean {
const sessionIdHeader = req.headers["x-dahr-session-id"]
const url = req.url || ""
const method = req.method as Web2Method
// Check if this URL/method combination is in exceptions
const isExempt = this._authConfig.exceptions.some(
exception =>
exception.urlPattern.test(url) &&
exception.methods.includes(method),
)
if (isExempt) {
return true
}
if (this._authConfig.requireAuthForAll) {
return (
typeof sessionIdHeader === "string" &&
sessionIdHeader === this._dahrSessionId
)
}
return true
}
private parseUrl(url: string) {
const parsedUrl = new URL(url)
return {
targetProtocol: parsedUrl.protocol,
targetHostname: parsedUrl.hostname,
targetPathname: parsedUrl.pathname,
targetSearch: parsedUrl.search,
targetFullPath: parsedUrl.pathname + parsedUrl.search,
targetOrigin: parsedUrl.origin,
targetPort: parsedUrl.port
? Number(parsedUrl.port)
: parsedUrl.protocol === "https:"
? 443
: parsedUrl.protocol === "http:"
? 80
: undefined,
}
}
private createHeaders(
targetMethod: Web2Method,
targetHeaders: IWeb2Request["raw"]["headers"],
targetAuthorization: string,
targetUrl: string,
): IWeb2Request["raw"]["headers"] {
// Base headers - only essential ones
const headers: IWeb2Request["raw"]["headers"] = {
"x-dahr-session-id": this._dahrSessionId,
}
// Convert all targetHeaders values to strings
for (const [key, value] of Object.entries(targetHeaders)) {
if (Array.isArray(value)) {
headers[key] = value.join(", ")
} else if (value !== undefined) {
headers[key] = value.toString()
}
}
// Only set Content-Type if not provided by user
if (
["POST", "PUT", "PATCH"].includes(targetMethod) &&
!headers["Content-Type"]
) {
headers["Content-Type"] = "application/json"
}
// Default to identity encoding for deterministic response bytes if not set by caller
const hasAcceptEncoding = Object.keys(headers).some(
k => k.toLowerCase() === "accept-encoding",
)
if (!hasAcceptEncoding) {
headers["Accept-Encoding"] = "identity"
}
// Add Authorization if required
if (this.requiresAuthorization(targetUrl, targetMethod)) {
headers["Authorization"] = `Bearer ${targetAuthorization}`
}
return headers
}
/**
* Canonicalize headers for deterministic hashing:
* - Lowercase keys
* - Omit volatile headers (date, set-cookie)
* - Join array values with ", "
* - Trim whitespace
* - Sort by key
*/
private canonicalizeHeaders(headers: http.IncomingHttpHeaders): string {
const volatile = new Set([
"date",
"set-cookie",
"connection",
"keep-alive",
"transfer-encoding",
"upgrade",
"proxy-authenticate",
"proxy-authorization",
"te",
"trailer",
"via",
"warning",
"server",
// Optional: content-length can vary across intermediaries
"content-length",
]) // omit volatile/hop-by-hop headers
const entries: Array<{ key: string; value: string }> = []
for (const [rawKey, rawVal] of Object.entries(headers)) {
const key = rawKey.toLowerCase()
if (volatile.has(key)) continue
if (rawVal == null) continue
let value: string
if (Array.isArray(rawVal)) {
value = rawVal.join(", ")
} else {
value = String(rawVal)
}
entries.push({ key, value: value.trim() })
}
entries.sort((a, b) => (a.key < b.key ? -1 : a.key > b.key ? 1 : 0))
return entries.map(e => `${e.key}:${e.value}`).join("\n")
}
private requiresAuthorization(url: string, method: Web2Method): boolean {
if (this._authConfig.requireAuthForAll) {
for (const exception of this._authConfig.exceptions) {
if (
exception.urlPattern.test(url) &&
exception.methods.includes(method)
) {
return false
}
}
return true
}
return false
}
}
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
This PR modifies Files that will be reverted:
|
|
Tip For best results, initiate chat on the files or code changes. @tcsenpai, hello! How can I help you with this PR? Here are some things I can assist with:
What would you like me to focus on? |
|
@coderabbitai review the PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8270 @tcsenpai, I've completed a comprehensive review of PR #492. Here's my analysis: ✅ Overall AssessmentThe header sanitization implementation is well-designed and secure. The code properly prevents credential exposure in both storage and logging contexts. 🔒 Security AnalysisFalse Positive: Case-Sensitivity ConcernThe compliance bot flagged a case-sensitivity vulnerability, but this is NOT a valid concern. The code correctly handles case-insensitive matching: if (SENSITIVE_HEADER_KEYS.has(key.toLowerCase())) {This will properly catch all variations: Actual Security ConsiderationLine 83-85 in DAHR.ts: The public get web2Request(): IWeb2Request {
return this._web2Request
}While 💡 Strengths
🔧 Minor Suggestions
✨ Code Quality
Recommendation: ✅ Approve with optional consideration of the |
1 participant
PR Type
Enhancement, Bug fix
Description
Sanitize sensitive HTTP headers before serialization
Remove authorization, API keys, tokens, and cookies
Prevent accidental exposure of credentials in DAHR data
Diagram Walkthrough
File Walkthrough
DAHR.ts
Add header sanitization to prevent credential exposuresrc/features/web2/dahr/DAHR.ts
SENSITIVE_HEADER_KEYSset containing 10 sensitive header namessanitizeHeaders()function to filter out sensitive headerstoSerializable()method to sanitize headers before returningSummary by CodeRabbit
Chores
Security