Skip to content

Commit a22f28f

Browse files
authored
fix: stop routing all traffic through EnvHttpProxyAgent by default (#359)
* chore(deps): bump undici to 7.x Our engines field requires Node >=20.20.0, which already satisfies undici 7.x (>=20.18.1). The 6.x pin was over-conservative — the original pin message referenced Node 18 support, but we don't actually support 18. 8.x would require Node >=22.19.0, which would exclude Node 20 users. * fix(proxy): only install EnvHttpProxyAgent when proxy env vars are set PR #338 set EnvHttpProxyAgent as the default dispatcher unconditionally so that Node's fetch (which ignores HTTP_PROXY/HTTPS_PROXY/NO_PROXY by default) would pick up standard proxy env vars for managed-network users. The tradeoff: we also took over the HTTP stack for every user, including those whose shell has a stale or incidental proxy var (from a VPN client, old dev setup, or inherited env) that wasn't meant to apply to Figma API traffic. Those users silently routed through an intermediary that returned 403, and our error message still blamed Figma (issue #358). Gate the swap on at least one of HTTP_PROXY/HTTPS_PROXY/NO_PROXY being set. Corporate users get the zero-config behavior they had before; users with no proxy vars keep Node's native dispatcher untouched. * fix(fetch): attach truncated response body to HTTP errors Throwing \`Fetch failed with status 403\` and discarding the body makes it impossible to tell a real Figma 403 from a proxy/intermediary 403. Read up to 500 chars of the body, collapse whitespace, and attach it to both the error message and the \`responseBody\` property on HttpError. Callers can pass \`redactFromErrorBody\` to scrub secrets from the body before it's exposed — defense in depth against chatty intermediaries that sometimes mirror request metadata into error pages. FigmaService passes its API key and OAuth token through this channel so they're scrubbed even in the unlikely case a proxy echoes them back. * fix(figma): include response body in 403 error message The canned \"Figma API returned 403 Forbidden\" message lied when the 403 actually came from an HTTP intermediary (proxy, firewall, VPN) rather than Figma itself. Surface the response body so the LLM and user can see what actually rejected the request, and add the intermediary case to the list of common causes. * feat(telemetry): add proxy_env_set common property PR #338 made proxy configuration load-bearing for every user's HTTP stack. Without a telemetry dimension for proxy-env presence we can't answer "do auth-category 403s correlate with proxy env vars" empirically across the user base — we can only respond to individual tickets. Add \`proxy_env_set\` to the common properties attached to every PostHog event. Boolean, so no proxy URL is logged. Shared helper in \`src/utils/proxy-env.ts\` so server.ts (dispatcher gating) and telemetry/client.ts (reporting) agree on the definition. * feat(telemetry): replace proxy_env_set with proxy_mode, surface bypass hint on 403 \`proxy_mode: none | explicit | env\` distinguishes --proxy/FIGMA_PROXY from env-derived proxies, which the boolean couldn't. Lets us correlate #358-style failures with the specific configuration that produced them. When a 403 surfaces and a proxy is configured, the error now hints at the right bypass: unset --proxy for explicit mode, or set NO_PROXY=api.figma.com for env mode. Users already have NO_PROXY as a standard escape hatch (EnvHttpProxyAgent respects it) — no new flag. * feat(proxy): support --proxy=none to bypass env-var proxies Users with a system-level HTTPS_PROXY that misbehaves for api.figma.com specifically couldn't easily opt out via the CLI — NO_PROXY worked but required knowing the convention and the right hostname. Accept 'none' as a special value for --proxy (also via FIGMA_PROXY) to explicitly skip both ProxyAgent and EnvHttpProxyAgent, leaving Node's default dispatcher in place regardless of what's in the environment. No new flag surface; reuses the existing knob. * chore: code cleanup pass + follow-ups Incorporates an automated cleanup pass across the proxy/dispatcher code, plus two manual follow-ups on top: - server.ts: consolidate proxy branches, unify commentary, drop the emitWarning suppression (undici 7.x no longer emits UNDICI-EHPA). - fetch-json.ts: push optional-handling for redactFromErrorBody to the boundary (destructure default + filter(Boolean)), tighten signatures. - proxy-env.ts: reorder (type first), extract PROXY_ENV_VARS constant. Restored a short note on hasProxyEnv so future readers know it's shared across three files and shouldn't be re-inlined. - figma.ts: split ternary into explicit per-mode branches, add rationale JSDoc on buildForbiddenMessage. Aligned HttpError cast style between buildForbiddenMessage and buildRateLimitMessage — both now use (error as HttpError).field ?? fallback with no optional chain, safe because both only run from the requestWithSize catch block.
1 parent 759d0e4 commit a22f28f

8 files changed

Lines changed: 151 additions & 35 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
"js-yaml": "^4.1.1",
6565
"posthog-node": "^5.29.2",
6666
"remeda": "^2.20.1",
67-
"undici": "^6.24.1",
67+
"undici": "^7.25.0",
6868
"zod": "^3.25.76"
6969
},
7070
"devDependencies": {

pnpm-lock.yaml

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/bin.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ const argv = cli({
4444
},
4545
proxy: {
4646
type: String,
47-
description: "HTTP proxy URL for networks that require a proxy (e.g. http://proxy:8080)",
47+
description:
48+
"HTTP proxy URL for networks that require a proxy (e.g. http://proxy:8080). Pass 'none' to ignore HTTP_PROXY/HTTPS_PROXY from the environment and connect directly.",
4849
},
4950
stdio: {
5051
type: Boolean,

src/server.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { Server } from "http";
55
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
66
import { ProxyAgent, EnvHttpProxyAgent, setGlobalDispatcher } from "undici";
77
import { Logger } from "./utils/logger.js";
8+
import { hasProxyEnv, setProxyMode } from "./utils/proxy-env.js";
89
import { createServer } from "./mcp/index.js";
910
import type { ServerConfig } from "./config.js";
1011
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
@@ -23,17 +24,21 @@ const activeConnections = new Set<ActiveConnection>();
2324
* Start the MCP server in either stdio or HTTP mode.
2425
*/
2526
export async function startServer(config: ServerConfig): Promise<void> {
26-
if (config.proxy) {
27+
// Three outcomes: explicit proxy URL → ProxyAgent; no proxy but env vars set
28+
// → EnvHttpProxyAgent; otherwise Node's default (includes `--proxy=none`,
29+
// which lets users opt out of system-level proxy vars misbehaving for
30+
// api.figma.com — see issue #358).
31+
//
32+
// We deliberately do NOT install EnvHttpProxyAgent when no proxy vars are
33+
// present, so a stale or incidental var in the user's shell (VPN client,
34+
// old dev setup) can't silently route Figma traffic through an intermediary
35+
// that may return 403.
36+
if (config.proxy && config.proxy !== "none") {
2737
setGlobalDispatcher(new ProxyAgent(config.proxy));
28-
} else {
29-
// EnvHttpProxyAgent automatically respects HTTP_PROXY/HTTPS_PROXY/NO_PROXY
30-
// env vars when present, and falls through to direct connections when absent.
31-
// Suppress the UNDICI-EHPA experimental warning — the API is stable
32-
// enough for our use case and the warning is noise for end users.
33-
const { emitWarning } = process;
34-
process.emitWarning = () => {};
38+
setProxyMode("explicit");
39+
} else if (!config.proxy && hasProxyEnv()) {
3540
setGlobalDispatcher(new EnvHttpProxyAgent());
36-
process.emitWarning = emitWarning;
41+
setProxyMode("env");
3742
}
3843

3944
const telemetryEnabled = telemetry.initTelemetry({

src/services/figma.ts

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { downloadAndProcessImage, type ImageProcessingResult } from "~/utils/ima
99
import { Logger, writeLogs } from "~/utils/logger.js";
1010
import { fetchJSON } from "~/utils/fetch-json.js";
1111
import { getErrorMeta } from "~/utils/error-meta.js";
12+
import { proxyMode } from "~/utils/proxy-env.js";
1213
import type { HttpError } from "~/utils/fetch-json.js";
1314

1415
export type FigmaAuthOptions = {
@@ -76,14 +77,15 @@ export class FigmaService {
7677

7778
return await fetchJSON<T & { status?: number }>(`${this.baseUrl}${endpoint}`, {
7879
headers,
80+
redactFromErrorBody: [this.apiKey, this.oauthToken],
7981
});
8082
} catch (error) {
8183
const meta = getErrorMeta(error);
8284
if (meta.http_status === 429) {
8385
throw new Error(buildRateLimitMessage(error), { cause: error });
8486
}
8587
if (meta.http_status === 403) {
86-
throw new Error(buildForbiddenMessage(endpoint), { cause: error });
88+
throw new Error(buildForbiddenMessage(endpoint, error), { cause: error });
8789
}
8890
const errorMessage = error instanceof Error ? error.message : String(error);
8991
throw new Error(
@@ -336,26 +338,48 @@ export class FigmaService {
336338
}
337339

338340
/**
339-
* Build a user-facing 429 message from the Figma rate-limit response headers.
340-
* Figma includes plan tier, seat-level limit type, retry-after, and an upgrade
341-
* link — all of which let us give targeted guidance instead of a generic
342-
* "try again later."
343-
*
344-
* See https://developers.figma.com/docs/rest-api/rate-limits/
341+
* Build a user-facing 403 message. Includes the raw response body (redacted +
342+
* truncated by fetchJSON) because corporate proxies/firewalls frequently
343+
* reject requests with their own 403 HTML before they ever reach Figma, and
344+
* that body is the fastest way for a user to recognize "oh, this is Zscaler."
345345
*/
346-
function buildForbiddenMessage(endpoint: string): string {
347-
return [
348-
`Figma API returned 403 Forbidden for '${endpoint}'.`,
349-
"This usually means one of:",
346+
function buildForbiddenMessage(endpoint: string, error: unknown): string {
347+
const body = (error as HttpError).responseBody;
348+
const parts = [`Request to Figma API endpoint '${endpoint}' returned 403 Forbidden.`];
349+
if (body) parts.push(`Response body: ${body}`);
350+
parts.push(
351+
"This is typically one of:",
350352
"- The access token doesn't have permission to this file (it must be owned by or explicitly shared with the token's account)",
351353
"- The file's share settings don't allow viewers to copy/share/export",
352354
"- For team/org files, the API token may not have access to that team",
355+
"- An HTTP intermediary (corporate proxy, firewall, VPN) rejected the request before it reached Figma — check the response body above for clues",
353356
"Troubleshooting guide: https://www.framelink.ai/docs/troubleshooting#cannot-access-file",
354-
].join("\n");
357+
);
358+
const mode = proxyMode();
359+
if (mode === "explicit") {
360+
parts.push(
361+
"",
362+
"Note: this server is configured to route requests through an explicit proxy (--proxy/FIGMA_PROXY). If the proxy may be the source of the 403, unset it, change it to --proxy=none, or bypass it for this host.",
363+
);
364+
} else if (mode === "env") {
365+
parts.push(
366+
"",
367+
"Note: this server picked up a proxy from HTTP_PROXY/HTTPS_PROXY in your environment. If the proxy may be the source of the 403, set NO_PROXY=api.figma.com, pass --proxy=none, or unset HTTP_PROXY/HTTPS_PROXY.",
368+
);
369+
}
370+
return parts.join("\n");
355371
}
356372

373+
/**
374+
* Build a user-facing 429 message from the Figma rate-limit response headers.
375+
* Figma includes plan tier, seat-level limit type, retry-after, and an upgrade
376+
* link — all of which let us give targeted guidance instead of a generic
377+
* "try again later."
378+
*
379+
* See https://developers.figma.com/docs/rest-api/rate-limits/
380+
*/
357381
function buildRateLimitMessage(error: unknown): string {
358-
const headers = (error as HttpError)?.responseHeaders ?? {};
382+
const headers = (error as HttpError).responseHeaders ?? {};
359383
const retryAfter = headers["retry-after"];
360384
const planTier = headers["x-figma-plan-tier"];
361385
const limitType = headers["x-figma-rate-limit-type"];

src/telemetry/client.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { randomUUID } from "node:crypto";
22
import { PostHog } from "posthog-node";
3+
import { proxyMode, type ProxyMode } from "~/utils/proxy-env.js";
34
import type { InitTelemetryOptions } from "./types.js";
45

56
// Write-only project key for the Framelink MCP analytics project.
@@ -13,6 +14,16 @@ type CommonProperties = {
1314
os_platform: NodeJS.Platform;
1415
nodejs_major: number;
1516
is_ci: boolean;
17+
/**
18+
* Which dispatcher the server installed for outbound fetches:
19+
* `none` (Node default), `explicit` (--proxy/FIGMA_PROXY), or `env`
20+
* (EnvHttpProxyAgent driven by HTTP_PROXY/HTTPS_PROXY/NO_PROXY).
21+
*
22+
* Lets us correlate failure rates (especially auth-category 403s — see
23+
* issue #358) with the proxy configuration a user was actually running
24+
* under, without logging the proxy URL itself.
25+
*/
26+
proxy_mode: ProxyMode;
1627
};
1728

1829
let client: PostHog | undefined;
@@ -72,6 +83,7 @@ export function initTelemetry(opts?: InitTelemetryOptions): boolean {
7283
os_platform: process.platform,
7384
nodejs_major: parseNodeMajor(process.versions.node),
7485
is_ci: Boolean(process.env.CI),
86+
proxy_mode: proxyMode(),
7587
};
7688

7789
// disableGeoip: false is load-bearing — the Node SDK defaults GeoIP to off,

src/utils/fetch-json.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,25 @@ type RequestOptions = RequestInit & {
77
* Avoids complexity of needing to deal with `instanceof Headers`, which is not supported in some environments.
88
*/
99
headers?: Record<string, string>;
10+
/**
11+
* Secrets to scrub from the response body attached to thrown HTTP errors.
12+
* Defense in depth: api.figma.com never echoes credentials, but HTTP
13+
* intermediaries (corporate proxies, MITM filters) sometimes mirror
14+
* request metadata into error pages.
15+
*/
16+
redactFromErrorBody?: string[];
1017
};
1118

1219
/**
1320
* Error thrown on HTTP failures. Carries response headers so callers (e.g.
1421
* figma.ts) can read rate-limit metadata without needing access to the
15-
* original Response object.
22+
* original Response object, plus the (possibly-truncated, redacted) response
23+
* body so callers can distinguish Figma errors from proxy/intermediary errors.
1624
*/
17-
export type HttpError = Error & { responseHeaders?: Record<string, string> };
25+
export type HttpError = Error & {
26+
responseHeaders?: Record<string, string>;
27+
responseBody?: string;
28+
};
1829

1930
const CONNECTION_ERROR_CODES = new Set([
2031
"ECONNRESET",
@@ -28,21 +39,30 @@ const CONNECTION_ERROR_CODES = new Set([
2839
// server-side failures. 4xx other than 429 are caller errors and not retryable.
2940
const RETRYABLE_STATUSES = new Set([408, 425, 429, 500, 502, 503, 504]);
3041

42+
// Cap the attached error body. Corp-firewall HTML blocks can be 50KB+;
43+
// we only need enough to identify the origin ("Blocked by Zscaler", etc.).
44+
const MAX_ERROR_BODY_CHARS = 500;
45+
3146
export async function fetchJSON<T extends { status?: number }>(
3247
url: string,
3348
options: RequestOptions = {},
3449
): Promise<{ data: T; rawSize: number }> {
50+
const { redactFromErrorBody = [], ...fetchOptions } = options;
3551
try {
36-
const response = await fetch(url, options);
52+
const response = await fetch(url, fetchOptions);
3753

3854
if (!response.ok) {
3955
const responseHeaders: Record<string, string> = {};
4056
response.headers.forEach((value, key) => {
4157
responseHeaders[key] = value;
4258
});
59+
const responseBody = await readErrorBody(response, redactFromErrorBody.filter(Boolean));
60+
const bodySuffix = responseBody ? `\nResponse body: ${responseBody}` : "";
4361
const httpError: HttpError = Object.assign(
44-
new Error(`Fetch failed with status ${response.status}: ${response.statusText}`),
45-
{ responseHeaders },
62+
new Error(
63+
`Fetch failed with status ${response.status}: ${response.statusText}${bodySuffix}`,
64+
),
65+
{ responseHeaders, responseBody },
4666
);
4767
tagError(httpError, {
4868
http_status: response.status,
@@ -85,3 +105,29 @@ function httpStatusCategory(status: number): ErrorCategory {
85105
if (status === 401 || status === 403) return "auth";
86106
return "figma_api";
87107
}
108+
109+
async function readErrorBody(
110+
response: Response,
111+
redactSecrets: string[],
112+
): Promise<string | undefined> {
113+
// Body read can fail if the connection is killed mid-response; we'd rather
114+
// surface the status/headers we already have than mask it with a body-read
115+
// error.
116+
let text: string;
117+
try {
118+
text = await response.text();
119+
} catch {
120+
return undefined;
121+
}
122+
if (!text) return undefined;
123+
124+
// Collapse whitespace so HTML error pages read as one line in error messages.
125+
let result = text.replace(/\s+/g, " ").trim();
126+
for (const secret of redactSecrets) {
127+
result = result.replaceAll(secret, "[REDACTED]");
128+
}
129+
if (result.length > MAX_ERROR_BODY_CHARS) {
130+
result = result.slice(0, MAX_ERROR_BODY_CHARS) + "… [truncated]";
131+
}
132+
return result;
133+
}

src/utils/proxy-env.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Which dispatcher is installed on the global fetch. `env` means
3+
* EnvHttpProxyAgent is routing, but a specific request may still go direct
4+
* when NO_PROXY matches — treat this as configuration state, not as
5+
* "was this request proxied."
6+
*/
7+
export type ProxyMode = "none" | "explicit" | "env";
8+
9+
const PROXY_ENV_VARS = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"];
10+
11+
/**
12+
* Whether the host environment has any HTTP proxy var set (case-insensitive).
13+
* Shared so server.ts, telemetry/client.ts, and figma.ts agree on what counts
14+
* as "proxy env" — don't inline this check.
15+
*/
16+
export function hasProxyEnv(): boolean {
17+
return PROXY_ENV_VARS.some((n) => process.env[n] || process.env[n.toLowerCase()]);
18+
}
19+
20+
let currentMode: ProxyMode = "none";
21+
22+
export function setProxyMode(mode: ProxyMode): void {
23+
currentMode = mode;
24+
}
25+
26+
export function proxyMode(): ProxyMode {
27+
return currentMode;
28+
}

0 commit comments

Comments
 (0)