Skip to content

fix(http): preserve caller's URL encoding in path params #1600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hugo-ccabral wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(http): preserve caller's URL encoding in path params #1600

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 19 additions & 47 deletions utils/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,78 +97,57 @@ export interface HttpClientOptions {
}

/**
* Encode path segment for URLs while preserving forward slashes
* Encodes special characters but leaves / intact for API compatibility
*/
function encodePathSegment(value: string): string {
// Split by /, encode each segment, then rejoin
return value
.split("/")
.map((segment) => encodeURIComponent(segment))
.join("/");
}

/**
* Normalize and validate path parameters to prevent path traversal attacks
* Validate path parameters to prevent path traversal attacks.
* Validates against a fully-decoded copy of the value but returns the
* original input verbatim so the caller's encoding level is preserved
* (e.g. a caller-encoded "%2520" stays "%2520" rather than collapsing to "%20").
*/
function normalizePathParam(
value: string | number,
paramName: string,
): string {
const str = String(value);

// Step 1: Decode any URL encoding to catch encoded attacks
let decoded = str;
try {
// Decode multiple times to catch double-encoding
// Decode repeatedly so we can detect attacks hidden behind double-encoding.
let prev = "";
while (prev !== decoded) {
prev = decoded;
decoded = decodeURIComponent(decoded);
}
} catch {
// If decode fails, keep the last successful decoded value
// Do not reset to original string as that would bypass security checks
// Partial decode is acceptable for validation purposes.

@cubic-dev-ai cubic-dev-ai Bot May 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Security: When decodeURIComponent throws on malformed sequences (e.g., %2e%2e%2f%GGfoo), the entire decode fails and decoded remains equal to the original encoded string. The subsequent includes("../") check won't detect the traversal because it's still percent-encoded. If a downstream proxy partially decodes valid sequences while ignoring invalid ones, this could allow a traversal bypass. Consider rejecting inputs that fail to fully decode rather than silently proceeding with the un-decoded value.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At utils/http.ts, line 120:

<comment>Security: When `decodeURIComponent` throws on malformed sequences (e.g., `%2e%2e%2f%GGfoo`), the entire decode fails and `decoded` remains equal to the original encoded string. The subsequent `includes("../")` check won't detect the traversal because it's still percent-encoded. If a downstream proxy partially decodes valid sequences while ignoring invalid ones, this could allow a traversal bypass. Consider rejecting inputs that fail to fully decode rather than silently proceeding with the un-decoded value.</comment>

<file context>
@@ -97,78 +97,57 @@ export interface HttpClientOptions {
   } catch {
-    // If decode fails, keep the last successful decoded value
-    // Do not reset to original string as that would bypass security checks
+    // Partial decode is acceptable for validation purposes.
   }
 
</file context>
Suggested change
// Partial decode is acceptable for validation purposes.
throw new Error(
`Invalid URL encoding in parameter '${paramName}'`,
);

}
Comment on lines 119 to 121

@coderabbitai coderabbitai Bot May 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Consider rejecting inputs that fail to fully decode.

If decodeURIComponent throws on malformed sequences (e.g., %GG), validation proceeds against the original encoded string. An input like %2e%2e%2f%GGfoo would bypass the ../ check since decoded remains unchanged at %2e%2e%2f%GGfoo.

While standard URL decoders should also fail on such input, some non-conforming servers or proxies may decode valid sequences while ignoring invalid ones. Rejecting undecodable input would close this edge case.

🛡️ Suggested hardening 
   } catch {
-    // Partial decode is acceptable for validation purposes.
+    throw new Error(
+      `Invalid URL encoding in parameter '${paramName}'`,
+    );
   }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/http.ts` around lines 119 - 121, The catch that swallows
decodeURIComponent errors should instead treat undecodable input as invalid: in
the decode block (where decodeURIComponent is called and the local variable
decoded is set) do not leave the original encoded string in place on
error—return/throw a validation failure so the caller (e.g., the path validation
logic around decoded) rejects the input; update that catch to mark the input as
undecodable (for example by returning false or throwing a specific error) so
inputs like `%2e%2e%2f%GGfoo` cannot bypass the `../` check.


// Step 2: Check for path traversal in decoded value
if (decoded.includes("../") || decoded.includes("..\\")) {
throw new Error(
`Path traversal detected in parameter '${paramName}'`,
);
}

// Step 3: Block absolute paths
if (decoded.startsWith("/") || decoded.startsWith("\\")) {
throw new Error(
`Absolute paths not allowed in parameter '${paramName}'`,
);
}

// Step 4: Normalize path separators and clean up
const normalized = decoded
.replace(/\\/g, "/") // Convert backslashes to forward slashes
.replace(/\/+/g, "/") // Collapse multiple slashes
.replace(/^\/|\/$/g, ""); // Remove leading/trailing slashes

// Step 5: Final validation - ensure no ".." or "." segments remain
const segments = normalized.split("/");
const segments = decoded.replace(/\\/g, "/").split("/");
for (const segment of segments) {
if (segment === ".." || segment === ".") {
if (segment === "..") {

@cubic-dev-ai cubic-dev-ai Bot May 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Allowing . path segments lets URL() collapse the path (e.g. /users/. -> /users/), so a literal path param can resolve to a different endpoint.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At utils/http.ts, line 137:

<comment>Allowing `.` path segments lets `URL()` collapse the path (e.g. `/users/.` -> `/users/`), so a literal path param can resolve to a different endpoint.</comment>

<file context>
@@ -97,78 +97,57 @@ export interface HttpClientOptions {
+  const segments = decoded.replace(/\\/g, "/").split("/");
   for (const segment of segments) {
-    if (segment === ".." || segment === ".") {
+    if (segment === "..") {
       throw new Error(
         `Invalid path segment in parameter '${paramName}'`,
</file context>
Suggested change
if (segment === "..") {
if (segment === ".." || segment === ".") {

throw new Error(
`Invalid path segment in parameter '${paramName}'`,
);
}
}

// Step 6: Block null bytes
if (normalized.includes("\0")) {
if (str.includes("\0") || decoded.includes("\0")) {
throw new Error(
`Null byte detected in parameter '${paramName}'`,
);
}

return normalized;
return str;
}

/**
Expand Down Expand Up @@ -267,27 +246,20 @@ export const createHttpClient = <T>(
throw new TypeError(`HttpClient: Missing ${name} at ${path}`);
}

// Normalize and validate path parameters to prevent path traversal
// Validate path parameters to prevent path traversal.
// We intentionally pass the value through verbatim — re-encoding
// here would break callers that already pre-encoded special
// characters (e.g. "%2520" must reach the server as "%2520",
// not collapse to "%20" or "%2525").
if (param !== undefined) {
try {
// Handle array params (for wildcard routes like /*)
if (Array.isArray(param)) {
return param.map((item) => {
const itemStr = String(item);
const normalized = normalizePathParam(itemStr, name);
// Encode special chars but preserve forward slashes for API compatibility
return encodePathSegment(normalized);
});
return param.map((item) =>
normalizePathParam(String(item), name)
);
}

// Handle single value params
const paramStr = String(param);
const normalized = normalizePathParam(paramStr, name);
// Encode special chars but preserve forward slashes for API compatibility
return encodePathSegment(normalized);
return normalizePathParam(String(param), name);
} catch (_error) {
// Translate validation errors into generic HTTP 400 errors
// without exposing the original input value or error details
throw new HttpError(
400,
`Invalid parameter '${name}'`,
Expand Down
Loading