respond to review: encode the env password, report which credential was used

1. mergeUpstreamPassword() assigned the env value to url.password raw. That
   setter escapes "@" and "/" but not "%", while mockttp reads the credential
   back through url.parse().auth, which decodeURIComponent()s it — so a
   password of "100%pass" made that decode throw URIError, and "p%20ss"
   silently authenticated as "p ss". encodeURIComponent() first is lossless:
   the setter escapes nothing encodeURIComponent leaves alone. Pinned by a
   round-trip test over "100%pass", "p%20ss" and "%".

2. Omitting the password when the server has no PROXY_MCP_UPSTREAM_PASSWORD
   was a silent no-op reported as success, surfacing later as unexplained 407s.
   The set-upstream responses now carry passwordSource: env | url | none.

3. README: the env var has to be in the spawned server's environment, so the
   'export' example could not work for a stdio server. Replaced with
   'claude mcp add -e' and an .mcp.json "env" block.

Author: MatousMarik

README.md

@@ -189,15 +189,38 @@ Supported upstream URL schemes: `socks4://`, `socks5://`, `http://`, `https://`,
 
 Tool calls and tool results are both persisted by the MCP client. To avoid
 writing an upstream password there on every call, set it in the server's
-environment and pass a URL with a username but no password:
+environment and pass a URL with a username but no password.
+
+The variable has to be in the environment of the **server process**, which the
+MCP client spawns — exporting it in your own shell does not reach it:
 
 ```bash
-export PROXY_MCP_UPSTREAM_PASSWORD="s3cret"
+claude mcp add proxy-mcp -e PROXY_MCP_UPSTREAM_PASSWORD=s3cret -- npx -y proxy-mcp@latest
+```
 
+```json
+{
+  "mcpServers": {
+    "proxy-mcp": {
+      "command": "npx",
+      "args": ["-y", "proxy-mcp@latest"],
+      "env": { "PROXY_MCP_UPSTREAM_PASSWORD": "s3cret" }
+    }
+  }
+}
+```
+
+Then omit the password from the call:
+
+```bash
 proxy_set_upstream --proxy_url "http://user@upstream.example:1080"
 # routes as http://user:s3cret@upstream.example:1080
 ```
 
+The response reports which credential was used — `passwordSource` is `env`,
+`url` or `none`. `none` on a URL you expected the variable to complete means the
+server does not have it in its environment.
+
 Applies to `proxy_set_upstream`, `proxy_set_host_upstream` and
 `proxy_mobile_setup`. A URL that already carries a password is used as-is, so
 existing calls are unaffected. One credential covers all upstreams; a URL
@@ -410,7 +433,8 @@ proxy_mobile_setup \
 Applies to BOTH listeners. Use `proxy_set_upstream` after the fact to change it without restarting.
 
 As with `proxy_set_upstream`, omit the password and set
-`PROXY_MCP_UPSTREAM_PASSWORD` in the environment to keep it out of the call.
+`PROXY_MCP_UPSTREAM_PASSWORD` in the server's environment to keep it out of the
+call.
 
 ### Verifying each step
 

src/tools/mobile.ts

@@ -24,7 +24,7 @@ import { join } from "node:path";
 import { randomBytes } from "node:crypto";
 import { proxyManager } from "../state.js";
 import { interceptorManager } from "../interceptors/manager.js";
-import { mergeUpstreamPassword } from "../utils.js";
+import { mergeUpstreamPassword, upstreamPasswordSource } from "../utils.js";
 
 function errorToString(e: unknown): string {
   if (e instanceof Error) return e.message;
@@ -313,6 +313,9 @@ export function registerMobileTools(server: McpServer): void {
               transparent_port: transparentPortUsed,
               block_quic,
               upstream_set: upstreamSet,
+              ...(upstream_proxy_url && resolvedUpstream
+                ? { password_source: upstreamPasswordSource(upstream_proxy_url, resolvedUpstream) }
+                : {}),
               cert_injected: certInjected,
               android_target_id: androidTargetId,
               sudo_script: scriptPath,

src/tools/upstream.ts

@@ -5,7 +5,7 @@
 import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
 import { z } from "zod";
 import { proxyManager } from "../state.js";
-import { mergeUpstreamPassword, redactProxyUrl } from "../utils.js";
+import { mergeUpstreamPassword, redactProxyUrl, upstreamPasswordSource } from "../utils.js";
 
 export function registerUpstreamTools(server: McpServer): void {
   server.tool(
@@ -25,6 +25,7 @@ export function registerUpstreamTools(server: McpServer): void {
             text: JSON.stringify({
               status: "success",
               message: `Global upstream set to ${redactProxyUrl(resolved)}`,
+              passwordSource: upstreamPasswordSource(proxy_url, resolved),
               noProxy: no_proxy || [],
             }),
           }],
@@ -72,6 +73,7 @@ export function registerUpstreamTools(server: McpServer): void {
             text: JSON.stringify({
               status: "success",
               message: `Upstream for '${hostname}' set to ${redactProxyUrl(resolved)}`,
+              passwordSource: upstreamPasswordSource(proxy_url, resolved),
             }),
           }],
         };

src/utils.ts

@@ -124,7 +124,7 @@ export function redactProxyUrl(proxyUrl: string): string {
  *
  * The value can only ever land in the password slot, which is the one field
  * redactProxyUrl() masks — a substituted secret cannot be echoed back through
- * the username, host, path or query. URL serialization percent-encodes it, so a
+ * the username, host, path or query. The value is percent-encoded, so a
  * password containing "@" or "/" cannot re-point the upstream at another host.
  *
  * A URL with no username, or one that already carries a password, is returned
@@ -145,6 +145,32 @@ export function mergeUpstreamPassword(
   }
   if (!url.username || url.password) return proxyUrl;
 
-  url.password = password;
+  // encodeURIComponent, not the raw value: the password setter escapes "@" and
+  // "/" but leaves "%" alone, and mockttp reads the credential back through
+  // url.parse().auth, which decodeURIComponent()s it. A raw "%" therefore makes
+  // that decode throw, and a raw "%20" silently decodes to a space. Encoding
+  // first is lossless — the setter escapes nothing encodeURIComponent leaves.
+  url.password = encodeURIComponent(password);
   return url.toString();
 }
+
+/**
+ * Which credential a resolved upstream URL ended up using, for the tool
+ * response.
+ *
+ * A caller that omits the password when the server has no
+ * PROXY_MCP_UPSTREAM_PASSWORD otherwise gets a plain "success" and finds out
+ * only later, as unexplained 407s on unrelated requests. "none" says so at the
+ * call that can still be corrected.
+ */
+export function upstreamPasswordSource(
+  original: string,
+  resolved: string,
+): "env" | "url" | "none" {
+  if (resolved !== original) return "env";
+  try {
+    return new URL(original).password ? "url" : "none";
+  } catch {
+    return "none";
+  }
+}

test/unit/utils.test.ts

@@ -1,7 +1,7 @@
 import { describe, it } from "node:test";
 import assert from "node:assert/strict";
 import { parse } from "node:url";
-import { truncateResult, getLocalIP, serializeHeaders, capString, redactProxyUrl, mergeUpstreamPassword } from "../../src/utils.js";
+import { truncateResult, getLocalIP, serializeHeaders, capString, redactProxyUrl, mergeUpstreamPassword, upstreamPasswordSource } from "../../src/utils.js";
 
 describe("truncateResult", () => {
   it("returns short data unchanged", () => {
@@ -158,6 +158,18 @@ describe("mergeUpstreamPassword", () => {
     assert.equal(new URL(out).host, "host:8000");
   });
 
+  it("survives the round-trip with a '%' in the password", () => {
+    // The password setter escapes "@" and "/" but not "%", while url.parse()
+    // decodeURIComponent()s the auth: a raw "%" made that decode throw, and a
+    // raw "%20" decoded to a space. Both must come back byte-identical.
+    for (const secret of ["100%pass", "p%20ss", "%"]) {
+      const out = mergeUpstreamPassword("http://user@host:8000", {
+        PROXY_MCP_UPSTREAM_PASSWORD: secret,
+      });
+      assert.equal(parse(out).auth, `user:${secret}`);
+    }
+  });
+
   it("reaches https-proxy-agent intact, including a ':' in the password", () => {
     // https-proxy-agent takes the whole auth string, so ":" is safe here.
     const secret = "pa:ss/word";
@@ -206,3 +218,18 @@ describe("mergeUpstreamPassword", () => {
     assert.equal(mergeUpstreamPassword("not a url", env), "not a url");
   });
 });
+
+describe("upstreamPasswordSource", () => {
+  const env = { PROXY_MCP_UPSTREAM_PASSWORD: "s3cret" };
+  const source = (url: string, e: NodeJS.ProcessEnv) =>
+    upstreamPasswordSource(url, mergeUpstreamPassword(url, e));
+
+  it("reports where the password came from", () => {
+    assert.equal(source("http://user@host:8000", env), "env");
+    assert.equal(source("http://user:mine@host:8000", env), "url");
+    // The case worth reporting: caller omitted it and the server has no
+    // variable, so the upstream is about to be used unauthenticated.
+    assert.equal(source("http://user@host:8000", {}), "none");
+    assert.equal(source("http://host:8000", env), "none");
+  });
+});