From 3f1fcd0d2d08df52fde15c3d095066df35f4ae75 Mon Sep 17 00:00:00 2001
From: Ajay Ghanwat <ghanwat.ajay@workroot.in>
Date: Tue, 4 Aug 2026 19:10:09 +0530
Subject: [PATCH] fix(authz): stop encodeURI from double-encoding a
 percent-escaped redirectTo

Fix round 3 for Task 7 (N5, minor-to-important): fix round 2's
encodeURI(options.redirectTo) fixed the non-ASCII crash but broke the
most common real use of redirectTo -- a return-path query param that's
already percent-encoded (e.g. /login?next=%2Fdash) -- because encodeURI
also escapes "%", double-encoding it to %252Fdash. Replaced with
headerSafePath(), a codepoint loop that encodes only codepoints above
0x7f (matching isLocalPath's style: no regex, no source escapes) and
leaves "%" alone.

Added tests: an already-percent-encoded target round-trips unchanged;
a non-ASCII target still 303s without throwing and the location is
ASCII-only; a plain ASCII target passes through byte-identical.
---
 packages/authz/src/middleware.ts       | 22 ++++++++++++---
 packages/authz/test/middleware.test.ts | 37 +++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/packages/authz/src/middleware.ts b/packages/authz/src/middleware.ts
index bbc5b8e5..e0e46c61 100644
--- a/packages/authz/src/middleware.ts
+++ b/packages/authz/src/middleware.ts
@@ -166,6 +166,20 @@ function isLocalPath(target: string): boolean {
   return true;
 }
 
+/**
+ * Header values must be Latin-1, so a localized path would otherwise throw
+ * inside `new Response` and 500 on a denial path. Encode ONLY the codepoints
+ * that cannot be sent: encodeURI would also escape "%", corrupting a target
+ * that already carries a percent-encoded return path.
+ */
+function headerSafePath(value: string): string {
+  let out = "";
+  for (const character of value) {
+    out += character.codePointAt(0)! <= 0x7f ? character : encodeURIComponent(character);
+  }
+  return out;
+}
+
 const NO_STORE_HEADERS = { "cache-control": "private, no-store" } as const;
 
 export interface GuardOptions {
@@ -203,9 +217,11 @@ export function guardPermission(permission: string, options: GuardOptions = {}):
       if (isLocalPath(options.redirectTo)) {
         return new Response(null, {
           status: 303,
-          // encodeURI: a non-ASCII local path (e.g. a localized login route)
-          // is valid config but not a valid raw header value.
-          headers: { location: encodeURI(options.redirectTo), ...NO_STORE_HEADERS },
+          // headerSafePath, not encodeURI: a non-ASCII local path (e.g. a
+          // localized login route) is valid config but not a valid raw
+          // header value, while encodeURI would also mangle a target that
+          // already carries a percent-encoded return path.
+          headers: { location: headerSafePath(options.redirectTo), ...NO_STORE_HEADERS },
         });
       }
       // JSON.stringify, not string interpolation: this branch exists precisely
diff --git a/packages/authz/test/middleware.test.ts b/packages/authz/test/middleware.test.ts
index cbfa7e2d..b3d29d90 100644
--- a/packages/authz/test/middleware.test.ts
+++ b/packages/authz/test/middleware.test.ts
@@ -319,17 +319,46 @@ describe("guardPermission hardening", () => {
     expect(res.headers.get("cache-control")).toBe("private, no-store");
   });
 
-  test("a non-ASCII redirectTo returns 303 with an encoded location rather than throwing", async () => {
+  test("a non-ASCII redirectTo returns 303 without throwing, and the location is ASCII-only", async () => {
     const ctx = makeCtx({ id: "u1" });
     await withMiddleware(ctx);
-    // Built at runtime (no \u escapes in source) per the repo-wide constraint.
-    const target = "/" + String.fromCharCode(0x65e5) + String.fromCharCode(0x672c);
+    // Built at runtime via String.fromCodePoint (no non-ASCII characters
+    // typed into the source) per the repo-wide constraint.
+    const target = "/" + String.fromCodePoint(0x65e5) + String.fromCodePoint(0x672c);
     const res = await guardPermission("post:write", { redirectTo: target })(
       ctx,
       async () => new Response("passed"),
     );
     expect(res.status).toBe(303);
-    expect(res.headers.get("location")).toBe(encodeURI(target));
+    const location = res.headers.get("location");
+    expect(location).not.toBeNull();
+    for (const ch of location ?? "") {
+      expect(ch.codePointAt(0)! <= 0x7f).toBe(true);
+    }
+  });
+
+  test("an already-percent-encoded redirectTo round-trips unchanged", async () => {
+    const ctx = makeCtx({ id: "u1" });
+    await withMiddleware(ctx);
+    const res = await guardPermission("post:write", { redirectTo: "/login?next=%2Fdash" })(
+      ctx,
+      async () => new Response("passed"),
+    );
+    expect(res.status).toBe(303);
+    const location = res.headers.get("location");
+    expect(location).toBe("/login?next=%2Fdash");
+    expect(location).not.toContain("%25");
+  });
+
+  test("a plain ASCII redirectTo is passed through byte-identical", async () => {
+    const ctx = makeCtx({ id: "u1" });
+    await withMiddleware(ctx);
+    const res = await guardPermission("post:write", { redirectTo: "/login?next=/dashboard" })(
+      ctx,
+      async () => new Response("passed"),
+    );
+    expect(res.status).toBe(303);
+    expect(res.headers.get("location")).toBe("/login?next=/dashboard");
   });
 
   test("redirectTo is ignored for an /api/ request, which gets 403 instead", async () => {