fix(proxy): log when SOCKS5 credential auto-encoding rewrites URL (#157) (#209)

* fix(proxy): log when SOCKS5 credential auto-encoding rewrites URL (#157)

Auto URL-encoding of SOCKS5 credentials (added in v0.3.26 to fix Chromium's
'=' truncation bug) currently happens silently. Users debugging connectivity
have no way to know the wrapper rewrote their proxy URL — the original #157
thread took 8 round-trips to surface this exact ambiguity.

Emit a log when re-encoding actually changes the URL: INFO on Python's
'cloakbrowser' logger, console.debug in JavaScript. Stays silent on
already-encoded inputs and credential-less URLs to avoid false-positive
noise. Credentials are not included in the log message.

Tests: 3 new cases per language (Python caplog, JS vi.spyOn console.debug)
covering trigger / silent-when-encoded / silent-when-no-creds.

* fix(proxy): gate log on credential change, not full URL diff

Per Copilot review on #209: urlparse cosmetically lowercases scheme and
hostname, so comparing the full reconstructed URL to the input would emit
"Auto URL-encoded SOCKS5..." even for inputs like
`socks5://USER:pass@HOST.com:1080` where no credential encoding happened.

Compare raw vs encoded user/password substrings instead. Mirror the same
condition in JS for parity (JS's manual parser preserves case today, but the
credential-level compare is more robust against future changes).

Adds one regression test per language.

cloakbrowser/browser.py

@@ -813,6 +813,10 @@ def _normalize_socks_string_url(url: str) -> str:
     truncate them at special chars like '='. Idempotent: pre-encoded input stays
     the same (decoded then re-encoded).
 
+    Emits an INFO log when re-encoding actually changes the URL, so users who
+    previously hit silent SOCKS5 fallback (#157) can see what the wrapper did.
+    Silent on already-encoded inputs (no false-positive noise).
+
     On unparseable input (invalid port, broken IPv6 literal, etc.) logs a
     warning and returns the original string — preserves pre-fix pass-through
     behavior so Chromium's own error handling kicks in.
@@ -828,18 +832,30 @@ def _normalize_socks_string_url(url: str) -> str:
     # urlparse returns None for absent components, "" for present-but-empty.
     if parsed.username is None and parsed.password is None:
         return url
-    enc_user = quote(unquote(parsed.username), safe="") if parsed.username else ""
+    raw_user = parsed.username or ""
+    enc_user = quote(unquote(raw_user), safe="") if raw_user else ""
     # Preserve the colon separator when password component is present, even if
     # empty, so `user:@host` stays `user:@host`.
     if parsed.password is not None:
-        enc_pass = quote(unquote(parsed.password), safe="") if parsed.password else ""
+        raw_pass = parsed.password
+        enc_pass = quote(unquote(raw_pass), safe="") if raw_pass else ""
     else:
+        raw_pass = None
         enc_pass = None
-    return _assemble_socks_url(
+    normalized = _assemble_socks_url(
         parsed.scheme, parsed.hostname or "", parsed.port,
         enc_user, enc_pass,
         parsed.path, parsed.params, parsed.query, parsed.fragment,
     )
+    # Compare credentials, not the full URL: urlparse cosmetically lowercases
+    # scheme and hostname, so a full-string compare would falsely fire on
+    # `socks5://USER:pass@HOST.com:1080` even when no encoding work happened.
+    if enc_user != raw_user or enc_pass != raw_pass:
+        logger.info(
+            "Auto URL-encoded SOCKS5 proxy credentials (special characters "
+            "detected). Pre-encode the URL to suppress this notice."
+        )
+    return normalized
 
 
 def _extract_proxy_url(proxy: str | ProxySettings | None) -> str | None:

js/src/proxy.ts

@@ -136,7 +136,19 @@ export function normalizeSocksStringUrl(urlStr: string): string {
     const encPass = hasPassword
       ? (rawPassEnc ? encodeURIComponent(lenientDecodeURIComponent(rawPassEnc)) : "")
       : null;
-    return assembleSocksUrl(scheme, encUser, encPass, hostAndRest);
+    const normalized = assembleSocksUrl(scheme, encUser, encPass, hostAndRest);
+    // Compare credentials, not the full URL: keeps the log condition focused
+    // on real encoding work, not cosmetic differences (parity with the Python
+    // implementation, which has to skip urlparse's hostname lowercasing).
+    const credsChanged = encUser !== rawUserEnc
+      || (hasPassword ? encPass !== rawPassEnc : false);
+    if (credsChanged) {
+      console.debug(
+        "[cloakbrowser] Auto URL-encoded SOCKS5 proxy credentials (special " +
+        "characters detected). Pre-encode the URL to suppress this notice.",
+      );
+    }
+    return normalized;
   } catch (e) {
     console.warn(`[cloakbrowser] Could not normalize SOCKS5 proxy URL, passing through unchanged: ${(e as Error).message}`);
     return urlStr;

js/tests/proxy.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, vi } from "vitest";
 import { parseProxyUrl, isSocksProxy, resolveProxyConfig } from "../src/proxy.js";
 import type { LaunchOptions } from "../src/types.js";
 
@@ -262,4 +262,63 @@ describe("resolveProxyConfig", () => {
     const { proxyArgs } = resolveProxyConfig("socks5://user:a@b@c@host:1080");
     expect(proxyArgs).toEqual(["--proxy-server=socks5://user:a%40b%40c@host:1080"]);
   });
+
+  // Visibility for #157: when wrapper actually rewrites the URL, surface a
+  // debug log so users debugging silent SOCKS5 fallback can see what happened.
+  it("logs debug message when SOCKS5 credentials get re-encoded", () => {
+    const debugSpy = vi.spyOn(console, "debug").mockImplementation(() => {});
+    try {
+      resolveProxyConfig("socks5://user:pass=123@host:1080");
+      expect(debugSpy).toHaveBeenCalledWith(
+        expect.stringContaining("Auto URL-encoded SOCKS5"),
+      );
+      // Credentials must not leak into the log.
+      const calls = debugSpy.mock.calls.flat().join(" ");
+      expect(calls).not.toContain("pass=123");
+      expect(calls).not.toContain("pass%3D123");
+    } finally {
+      debugSpy.mockRestore();
+    }
+  });
+
+  it("stays silent when SOCKS5 URL is already encoded (no log spam)", () => {
+    const debugSpy = vi.spyOn(console, "debug").mockImplementation(() => {});
+    try {
+      resolveProxyConfig("socks5://user:pass%3D123@host:1080");
+      const reencodedCalls = debugSpy.mock.calls
+        .flat()
+        .filter((arg) => typeof arg === "string" && arg.includes("Auto URL-encoded SOCKS5"));
+      expect(reencodedCalls).toHaveLength(0);
+    } finally {
+      debugSpy.mockRestore();
+    }
+  });
+
+  it("stays silent when SOCKS5 URL has no credentials", () => {
+    const debugSpy = vi.spyOn(console, "debug").mockImplementation(() => {});
+    try {
+      resolveProxyConfig("socks5://host:1080");
+      const reencodedCalls = debugSpy.mock.calls
+        .flat()
+        .filter((arg) => typeof arg === "string" && arg.includes("Auto URL-encoded SOCKS5"));
+      expect(reencodedCalls).toHaveLength(0);
+    } finally {
+      debugSpy.mockRestore();
+    }
+  });
+
+  it("stays silent when only host case differs (no credential rewrite)", () => {
+    // Parity with Python: log condition must track credential changes, not
+    // cosmetic URL-string differences (regression for Copilot's PR #209 review).
+    const debugSpy = vi.spyOn(console, "debug").mockImplementation(() => {});
+    try {
+      resolveProxyConfig("socks5://USER:pass@HOST.com:1080");
+      const reencodedCalls = debugSpy.mock.calls
+        .flat()
+        .filter((arg) => typeof arg === "string" && arg.includes("Auto URL-encoded SOCKS5"));
+      expect(reencodedCalls).toHaveLength(0);
+    } finally {
+      debugSpy.mockRestore();
+    }
+  });
 });

tests/test_proxy.py

@@ -284,6 +284,42 @@ class TestResolveProxyConfig:
         _, args = _resolve_proxy_config("socks5://user:pass%3D123@host:1080")
         assert args == ["--proxy-server=socks5://user:pass%3D123@host:1080"]
 
+    def test_socks5_string_logs_info_when_reencoding(self, caplog):
+        # When wrapper actually rewrites the URL (e.g. unencoded '=' in pwd),
+        # surface an INFO log so users debugging SOCKS5 connectivity (#157)
+        # can see what the wrapper did instead of being silently surprised.
+        import logging
+        with caplog.at_level(logging.INFO, logger="cloakbrowser"):
+            _resolve_proxy_config("socks5://user:pass=123@host:1080")
+        assert any("Auto URL-encoded SOCKS5" in r.message for r in caplog.records)
+        # Credentials must not leak into the log.
+        for r in caplog.records:
+            assert "pass=123" not in r.message
+            assert "pass%3D123" not in r.message
+
+    def test_socks5_string_silent_when_already_encoded(self, caplog):
+        # Idempotent path: pre-encoded URL produces no log noise.
+        import logging
+        with caplog.at_level(logging.INFO, logger="cloakbrowser"):
+            _resolve_proxy_config("socks5://user:pass%3D123@host:1080")
+        assert not any("Auto URL-encoded SOCKS5" in r.message for r in caplog.records)
+
+    def test_socks5_string_silent_when_no_credentials(self, caplog):
+        # No userinfo at all → no encoding work → no log.
+        import logging
+        with caplog.at_level(logging.INFO, logger="cloakbrowser"):
+            _resolve_proxy_config("socks5://host:1080")
+        assert not any("Auto URL-encoded SOCKS5" in r.message for r in caplog.records)
+
+    def test_socks5_string_silent_when_only_cosmetic_change(self, caplog):
+        # urlparse lowercases scheme and hostname, but credentials are
+        # untouched. The log must NOT fire for these cosmetic-only rewrites
+        # (regression for Copilot's review on PR #209).
+        import logging
+        with caplog.at_level(logging.INFO, logger="cloakbrowser"):
+            _resolve_proxy_config("socks5://USER:pass@HOST.com:1080")
+        assert not any("Auto URL-encoded SOCKS5" in r.message for r in caplog.records)
+
     def test_socks5_string_no_creds_unchanged(self):
         _, args = _resolve_proxy_config("socks5://host:1080")
         assert args == ["--proxy-server=socks5://host:1080"]