diff --git a/AGENTS.md b/AGENTS.md index 641f01b3..44defced 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,7 @@ Philosophy: "Create architecture that is performant and easy to test" - Follow single responsibility principle - Prefer public helper modules to lots of private methods - Use dependency injection for external services (REST client, etc.) +- Do not eagerly validate network/proxy/environment configuration while constructing command dependencies when the command has local-only or no-network paths. Defer validation until the first network operation and add regression tests for malformed env values on local paths. - For MCP/agent-facing tools, avoid coupled optional flags and default-true booleans. Design schemas for real agent calls, including empty strings, empty arrays, and explicit `false` values. - For GraphQL/API-backed tools, treat minimal data fetching as part of the tool contract. Before adding or changing selected fields, compare the query against every consumer (text, verbose, JSON, MCP, CLI, and internal callers), use conditional fields or separate queries for mode-specific data, and add tests that assert the wire variables/selections for compact and detailed modes. diff --git a/README.md b/README.md index 0bb3acad..5a9a6e0c 100644 --- a/README.md +++ b/README.md @@ -313,7 +313,7 @@ This repository contains the GitHits CLI and reusable MCP package: Requirements: -- Node.js `^20.12.0 || >=22.13.0` +- Node.js `^20.18.1 || >=22.13.0` - Bun Common commands: diff --git a/bun.lock b/bun.lock index a2eddbbb..fabda44b 100644 --- a/bun.lock +++ b/bun.lock @@ -16,6 +16,7 @@ "open": "^11.0.0", "semver": "7.8.5", "smol-toml": "^1.6.1", + "undici": "7.28.0", "yaml": "^2.9.0", "zod": "^4.4.3", }, @@ -42,7 +43,7 @@ }, "packages/mcp": { "name": "@githits/mcp", - "version": "0.6.0", + "version": "0.6.1", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", "zod": "^4.4.3", @@ -558,6 +559,8 @@ "typescript": ["typescript@6.0.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw=="], + "undici": ["undici@7.28.0", "", {}, "sha512-cRZYrTDwWznlnRiPjggAGxZXanty6M8RV1ff8Wm4LWXBp7/IG8v5DnOm74DtUBp9OONpK75YlPnIjQqX0dBDtA=="], + "undici-types": ["undici-types@7.16.0", "", {}, "sha512-Zz+aZWSj8LE6zoxD+xrjh4VfkIG8Ya6LvYkZqtUQGJPZjYl53ypCaUwWqo7eI0x66KBGeRo+mlBEkMSeSZ38Nw=="], "unpipe": ["unpipe@1.0.0", "", {}, "sha512-pjy2bYhSsufwWlKwPc+l3cN7+wuJlK6uz0YdJEOlQDbl6jo/YlPi4mb8agUkVC8BF7V8NuzeyPNqRksA3hztKQ=="], diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index 3202e6f9..dadb09fd 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -144,6 +144,16 @@ githits doctor --json Prints redacted diagnostics for comparing GitHits behavior across terminals or agents. The report includes CLI/runtime identity, selected environment variables, service URL sources, config file status, active and legacy auth storage locations, token/client/metadata presence and timestamps, and recommendations. Secret-bearing values such as tokens, client secrets, API tokens, and proxy credentials are never printed; presence is reported as `set` / `present` only. JSON output uses `schemaVersion: 1` for support tooling. +### Proxy Support + +CLI-originated HTTP traffic uses `src/services/proxy-fetch.ts`. This includes OAuth discovery, client registration, token exchange/refresh, REST API calls, code/package service calls, local MCP tool calls started through `githits mcp start`, and npm update checks. The fetch factory supports `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` plus lowercase aliases; lowercase values win when both cases are set, matching undici's env proxy precedence. + +Native Node env proxy support is used only when the user explicitly opted in and the running Node version is known to support that opt-in. `NODE_USE_ENV_PROXY=1` is treated as native on Node `22.21.0+` and `24.0.0+`; `--use-env-proxy` / `NODE_OPTIONS=--use-env-proxy` is treated as native on Node `22.21.0+` and `24.5.0+`. Older or unknown versions use the fallback so the Node `20.18.1` floor remains supported. + +Fallback proxy support uses the installed `undici` runtime dependency and per-request `ProxyAgent` dispatchers rather than mutating global fetch or the global dispatcher. Proxy URL validation and runtime proxy failures are sanitized before surfacing to CLI/MCP callers: credentials, path, query, and fragments are never printed. `doctor` remains presence/status-only for proxy environment variables. + +`bun run smoke:proxy-node` builds the proxy fetch module and runs a Node process against local target/proxy servers. It verifies that fallback mode really sends HTTP traffic through a local proxy, that `NO_PROXY` bypasses it, and that proxy URL redaction stays intact. + ### `githits pkg info` ``` diff --git a/docs/implementation/update-check.md b/docs/implementation/update-check.md index ee696db4..498e1e05 100644 --- a/docs/implementation/update-check.md +++ b/docs/implementation/update-check.md @@ -178,6 +178,10 @@ The service accepts injected dependencies for the current version, fetcher, clock, and file-system service. Tests should mock those dependencies rather than patch global state. +The CLI injects the proxy-aware fetcher from `src/services/proxy-fetch.ts`, so +the direct npm registry calls honor `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` +without requiring package-manager configuration. + ## Backend Compatibility Signals All backend requests already include `x-githits-client-version` and a diff --git a/package.json b/package.json index ddf4db7c..5cee1831 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "inspector": "npx @modelcontextprotocol/inspector bun run dev mcp", "smoke:cli": "bun run scripts/cli-smoke.ts", "smoke:mcp": "bun run scripts/mcp-smoke.ts", + "smoke:proxy-node": "bun run scripts/run-proxy-node-smoke.ts", "validate:packages": "bun run scripts/validate-public-packages.ts", "validate:packages:mcp-publish": "bun run scripts/validate-public-packages.ts --mcp-publish-dry-run", "sync:claude-skills": "bun run scripts/sync-claude-skill-assets.ts", @@ -79,7 +80,7 @@ "url": "https://github.com/githits-com/githits-cli/issues" }, "engines": { - "node": "^20.12.0 || >=22.13.0" + "node": "^20.18.1 || >=22.13.0" }, "publishConfig": { "access": "public" @@ -96,6 +97,7 @@ "open": "^11.0.0", "semver": "7.8.5", "smol-toml": "^1.6.1", + "undici": "7.28.0", "yaml": "^2.9.0", "zod": "^4.4.3" }, diff --git a/packages/mcp/package.json b/packages/mcp/package.json index e70cd22d..49f980d2 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -1,6 +1,6 @@ { "name": "@githits/mcp", - "version": "0.6.0", + "version": "0.6.1", "description": "Reusable MCP server APIs and tools for GitHits", "type": "module", "files": [ @@ -52,7 +52,7 @@ }, "homepage": "https://githits.com", "engines": { - "node": "^20.12.0 || >=22.13.0" + "node": "^20.18.1 || >=22.13.0" }, "publishConfig": { "access": "public" diff --git a/scripts/proxy-node-smoke.mjs b/scripts/proxy-node-smoke.mjs new file mode 100644 index 00000000..fc0996cc --- /dev/null +++ b/scripts/proxy-node-smoke.mjs @@ -0,0 +1,178 @@ +import { createServer } from "node:http"; +import { createServer as createHttpsServer } from "node:https"; +import { connect } from "node:net"; +import { pathToFileURL } from "node:url"; + +const modulePath = process.env.GITHITS_PROXY_FETCH_MODULE; +if (!modulePath) { + throw new Error("GITHITS_PROXY_FETCH_MODULE is required"); +} + +const { createCliFetch, redactProxyUrl } = await import( + pathToFileURL(modulePath).href +); + +async function main() { + const servers = []; + try { + const proxyRequests = []; + const connectRequests = []; + const secure = await listen( + createHttpsServer({ key: TEST_KEY, cert: TEST_CERT }, (_req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }); + res.end("secure"); + }), + ); + servers.push(secure.server); + + const proxy = await listen( + createServer((req, res) => { + proxyRequests.push(req.url ?? ""); + res.writeHead(200, { "Content-Type": "text/plain" }); + res.end("proxied"); + }), + ); + proxy.server.on("connect", (req, clientSocket, head) => { + connectRequests.push(req.url ?? ""); + const upstream = connect(secure.port, "127.0.0.1", () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n"); + if (head.length > 0) { + upstream.write(head); + } + upstream.pipe(clientSocket); + clientSocket.pipe(upstream); + }); + upstream.on("error", () => clientSocket.destroy()); + clientSocket.on("error", () => upstream.destroy()); + }); + servers.push(proxy.server); + + const direct = await listen( + createServer((_req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }); + res.end("direct"); + }), + ); + servers.push(direct.server); + + const fetchFn = createCliFetch({ + env: { + HTTP_PROXY: `http://user:pass@127.0.0.1:${proxy.port}`, + HTTPS_PROXY: `http://user:pass@127.0.0.1:${proxy.port}`, + NO_PROXY: `127.0.0.1:${direct.port}`, + NODE_USE_ENV_PROXY: "1", + }, + execArgv: [], + nodeOptions: "", + nodeVersion: "20.18.1", + }); + + const proxiedResponse = await fetchFn("http://example.test/proxy-check"); + assertEqual(await proxiedResponse.text(), "proxied", "proxy response body"); + assertEqual( + proxyRequests[0], + "http://example.test/proxy-check", + "proxy receives absolute-form HTTP request", + ); + + const secureResponse = await fetchFn("https://example.test/secure-check"); + assertEqual( + await secureResponse.text(), + "secure", + "HTTPS proxy response body", + ); + assertEqual( + connectRequests[0], + "example.test:443", + "HTTPS proxy receives CONNECT request", + ); + + const directResponse = await fetchFn( + `http://127.0.0.1:${direct.port}/direct`, + ); + assertEqual(await directResponse.text(), "direct", "NO_PROXY response body"); + assertEqual(proxyRequests.length, 1, "NO_PROXY bypasses proxy"); + + const redacted = redactProxyUrl( + "http://user:pass@proxy.example:8080/p?q=1#x", + ); + assertEqual(redacted, "http://proxy.example:8080/", "proxy URL redaction"); + + process.stdout.write("proxy-node-smoke passed\n"); + } finally { + await Promise.all(servers.map((server) => closeServer(server))); + } +} + +const TEST_KEY = `-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDT35OVlcqLp9Wl +OsdI3auhRvcZhRNeEMXZW4dCb++l6aieDIPpOEL88sQ6UzM2rQ9knrhJpwKDKeli +80S3h34zpt8ifOm4LqOjd5QaQgjLyl5lj7ewz6mjN50QSixN0/GOY113o4kn68/B +fyu3qnjMNtBoCIFSXXX+XsPVaxoaTxHtc/PIEjewisNetszvSWvVAHlv8I3TVbe4 +2lljvpDgavBN2/NX2uJi+SUKrC3Nju5w4JdpGrqI/0XeHHbxSffiVGUb0mblsx4B +XSZBZZGZllF1HZH5Omf8fjo4hHyFjl3A2jVdsbmeAS4sZyAfyNP8SfwQjInMylb7 +KcITOWxPAgMBAAECggEAPhc5Yw8Ayqim3cM5/8qmr57ib2ImaNy1fptqKjgvnQm6 +1oaIaeKJDyP+CbG0QoO5DR3OmBcPj2zK7qqoKrjUbUKsBalhvQ49+nvitUdA2Kg3 +vb++b1yMND7qEooKLcy876ODErqkSUS8H9Kq9ypIOGCf9rz3WTH2kFMpRPQcNDUL +UVRFfhAhzU5JSy5xeeTEMB0NI0gl+an95oD0bXyVzhB+8sU09bjU/yXoIuxH9enm +YXlW2jygDRK5DkPnoxZksCHLDFeGxU9MATg7NWC2StnzGPNWL/LcEzLldqJXAbzG +WWNFU7hcXPvBe+R7cUsSV+Yvl5CwZWge+5YiVMLZoQKBgQDw0+HzBVelQ95uGUw9 +sv6nyjXr0C85wFKzBb16Q4GYsnlo+HtwcEa0SiN7jz6nkEW4lSvyCcOr8Pm/HFqu ++mbMv9S6nuxZkWHnSU8/wk7hWZXd/bzfAe2170+eSjwYha63L4rWbMXsI//R4c9n +h7MkhKGULfaekza/xiVkKd/K6QKBgQDhOLZ162oXnfaYYJWoQ2FDq6hjIE+qy9xz +FdEz65MPBFo6pbJNft4e5usNMy2c0QBt4DAff/oRoww+GYexo13MwdsWnL9oyHCn +LwAqZoK9NjcFdkdpbNlRYzjj8POD6JPA0uvYhD0+JLlFKBjjuvJr2VOxQI1xJ7Vj +Yx7/5v0KdwKBgDkkLRJ6jAc8iURaYEqrc9zgD9c5+FqdlYHAtOqTpeZTQpdzjeZp +3XzdsnmYzWb4xnI7gsfVJUZg0QFVevbVlxqx0YnON4oxAqfcLx+TvR+fH/4iPHQ1 +gu+OLrgCKSwwW/o/H5QtDvEuwX5NM+b+vbTGe4grN778cxshqrGPdfgxAoGBAN7e +9UgphtoKGh1d7psM2nJRqxc0wUF97RABtf0QEH2azAMfNxuTARFJZ66vR2LYO/l/ +EYAKb5cGZzYIo4v44vidmUV+Jbf2Kex3CU3sFVJSFQ6VpkNAUKlGa+S86u1MuPHm +hzbCXaxiQOibrk2lEQIClNxhydYA+nF4hBOuLBcvAoGAHc7ADCJuaJ8R730iD4TJ +daXCXcZmMPZiq/Zu55/IvcTJCghiERse8dXOjgJRDEzc47V96icW8emtV85xQt+z +gN46s3p7T7ny/nfdQACk0GpYTZy0i8ZyPV8LQeJyADVty5EFBWEA/r/PFuN5mreZ +0v2DFEPG2UGRBMe3Cnh5o2g= +-----END PRIVATE KEY-----`; + +const TEST_CERT = `-----BEGIN CERTIFICATE----- +MIIDKDCCAhCgAwIBAgIUIgvE5JR9tgOP8zt+2Mc6zXIWRxYwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAwwMZXhhbXBsZS50ZXN0MB4XDTI2MDcwODA4MzAwNloXDTM2 +MDcwNTA4MzAwNlowFzEVMBMGA1UEAwwMZXhhbXBsZS50ZXN0MIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEA09+TlZXKi6fVpTrHSN2roUb3GYUTXhDF2VuH +Qm/vpemongyD6ThC/PLEOlMzNq0PZJ64SacCgynpYvNEt4d+M6bfInzpuC6jo3eU +GkIIy8peZY+3sM+pozedEEosTdPxjmNdd6OJJ+vPwX8rt6p4zDbQaAiBUl11/l7D +1WsaGk8R7XPzyBI3sIrDXrbM70lr1QB5b/CN01W3uNpZY76Q4GrwTdvzV9riYvkl +CqwtzY7ucOCXaRq6iP9F3hx28Un34lRlG9Jm5bMeAV0mQWWRmZZRdR2R+Tpn/H46 +OIR8hY5dwNo1XbG5ngEuLGcgH8jT/En8EIyJzMpW+ynCEzlsTwIDAQABo2wwajAd +BgNVHQ4EFgQUoKQ1P5wZFvKZ5p7kfdMODOgZ0BUwHwYDVR0jBBgwFoAUoKQ1P5wZ +FvKZ5p7kfdMODOgZ0BUwDwYDVR0TAQH/BAUwAwEB/zAXBgNVHREEEDAOggxleGFt +cGxlLnRlc3QwDQYJKoZIhvcNAQELBQADggEBADSU7zh+LwQS8Zr3/x7J8mk+lZ9S +s/X0kXjMGHhwyiHDg/mq2EkBvTmdRd6mu74RPrdAnkIQVxtOY8e4Te8aXniLXaAj +Kh3jViROU2H2PUsLIZPQdH6KE/M8EGRzkryPhdUS/KJ06s5Q3/5ebuQTkjAcyRGz +RQI6Bt1T1SdAULNCeSoqacU4CkODojkoMBEr2jAHrMS5JqVHlGsHrrrdxt10MLJX +wGtq+XtLvvZApLZ9OJNRX0dTCkNlFTCYrW+mqlm1SZ0HbzzYgcIYxLkKhNt0PN7I +sjHd8Ixk4fi6F8q+VezypajRKe+ZiXze7k+z9P8nBoHNI6YCXYRN3sbca0A= +-----END CERTIFICATE-----`; + +await main(); + +async function listen(server) { + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + return { server, port: server.address().port }; +} + +async function closeServer(server) { + if (!server.listening) { + return; + } + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); +} + +function assertEqual(actual, expected, label) { + if (actual !== expected) { + throw new Error( + `${label}: expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`, + ); + } +} diff --git a/scripts/run-proxy-node-smoke.ts b/scripts/run-proxy-node-smoke.ts new file mode 100644 index 00000000..c181a042 --- /dev/null +++ b/scripts/run-proxy-node-smoke.ts @@ -0,0 +1,79 @@ +import { spawnSync } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +async function main(): Promise { + const outdir = await mkdtemp(join(tmpdir(), "githits-proxy-smoke-")); + try { + await writeFile(join(outdir, "package.json"), '{"type":"module"}\n'); + const build = await Bun.build({ + entrypoints: ["src/services/proxy-fetch.ts"], + outdir, + target: "node", + format: "esm", + }); + + if (!build.success) { + for (const log of build.logs) { + console.error(log); + } + process.exit(1); + } + + const modulePath = join(outdir, "proxy-fetch.js"); + const caPath = join(outdir, "proxy-smoke-ca.pem"); + await writeFile(caPath, TEST_CERT); + const result = spawnSync("node", ["scripts/proxy-node-smoke.mjs"], { + stdio: "inherit", + env: { + ...childSmokeEnv(process.env), + GITHITS_PROXY_FETCH_MODULE: modulePath, + NODE_EXTRA_CA_CERTS: caPath, + }, + }); + + process.exit(result.status ?? 1); + } finally { + await rm(outdir, { recursive: true, force: true }); + } +} + +function childSmokeEnv( + env: NodeJS.ProcessEnv, +): Record { + const { + HTTP_PROXY, + HTTPS_PROXY, + NO_PROXY, + http_proxy, + https_proxy, + no_proxy, + NODE_OPTIONS, + NODE_USE_ENV_PROXY, + ...rest + } = env; + return rest; +} + +const TEST_CERT = `-----BEGIN CERTIFICATE----- +MIIDKDCCAhCgAwIBAgIUIgvE5JR9tgOP8zt+2Mc6zXIWRxYwDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAwwMZXhhbXBsZS50ZXN0MB4XDTI2MDcwODA4MzAwNloXDTM2 +MDcwNTA4MzAwNlowFzEVMBMGA1UEAwwMZXhhbXBsZS50ZXN0MIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEA09+TlZXKi6fVpTrHSN2roUb3GYUTXhDF2VuH +Qm/vpemongyD6ThC/PLEOlMzNq0PZJ64SacCgynpYvNEt4d+M6bfInzpuC6jo3eU +GkIIy8peZY+3sM+pozedEEosTdPxjmNdd6OJJ+vPwX8rt6p4zDbQaAiBUl11/l7D +1WsaGk8R7XPzyBI3sIrDXrbM70lr1QB5b/CN01W3uNpZY76Q4GrwTdvzV9riYvkl +CqwtzY7ucOCXaRq6iP9F3hx28Un34lRlG9Jm5bMeAV0mQWWRmZZRdR2R+Tpn/H46 +OIR8hY5dwNo1XbG5ngEuLGcgH8jT/En8EIyJzMpW+ynCEzlsTwIDAQABo2wwajAd +BgNVHQ4EFgQUoKQ1P5wZFvKZ5p7kfdMODOgZ0BUwHwYDVR0jBBgwFoAUoKQ1P5wZ +FvKZ5p7kfdMODOgZ0BUwDwYDVR0TAQH/BAUwAwEB/zAXBgNVHREEEDAOggxleGFt +cGxlLnRlc3QwDQYJKoZIhvcNAQELBQADggEBADSU7zh+LwQS8Zr3/x7J8mk+lZ9S +s/X0kXjMGHhwyiHDg/mq2EkBvTmdRd6mu74RPrdAnkIQVxtOY8e4Te8aXniLXaAj +Kh3jViROU2H2PUsLIZPQdH6KE/M8EGRzkryPhdUS/KJ06s5Q3/5ebuQTkjAcyRGz +RQI6Bt1T1SdAULNCeSoqacU4CkODojkoMBEr2jAHrMS5JqVHlGsHrrrdxt10MLJX +wGtq+XtLvvZApLZ9OJNRX0dTCkNlFTCYrW+mqlm1SZ0HbzzYgcIYxLkKhNt0PN7I +sjHd8Ixk4fi6F8q+VezypajRKe+ZiXze7k+z9P8nBoHNI6YCXYRN3sbca0A= +-----END CERTIFICATE-----`; + +await main(); diff --git a/src/cli.ts b/src/cli.ts index 3f2ac252..a4a37452 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -38,6 +38,7 @@ import { loadAutoLoginAuthSessionMetadata, } from "./container.js"; import { FileSystemServiceImpl } from "./services/filesystem-service.js"; +import { createLazyCliFetch } from "./services/proxy-fetch.js"; import { NpmRegistryUpdateCheckService } from "./services/update-check-service.js"; import { createRootCliPreAction } from "./shared/root-cli-pre-action.js"; @@ -57,6 +58,7 @@ const createUpdateCheckService = () => new NpmRegistryUpdateCheckService({ currentVersion: version, fileSystemService: new FileSystemServiceImpl(), + fetcher: createLazyCliFetch(), }); await enforceCachedRequiredUpdateForInvocation({ diff --git a/src/container.test.ts b/src/container.test.ts index 9345cf3c..ac91f049 100644 --- a/src/container.test.ts +++ b/src/container.test.ts @@ -41,6 +41,42 @@ async function withApiToken( } } +async function withEnvVars( + values: Record, + fn: () => Promise, +): Promise { + const originals = new Map(); + for (const [key, value] of Object.entries(values)) { + originals.set(key, process.env[key]); + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { + return await fn(); + } finally { + for (const [key, value] of originals) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } +} + +async function withoutProxyEnv(fn: () => Promise): Promise { + return withEnvVars( + { + HTTP_PROXY: undefined, + HTTPS_PROXY: undefined, + NO_PROXY: undefined, + http_proxy: undefined, + https_proxy: undefined, + no_proxy: undefined, + NODE_USE_ENV_PROXY: undefined, + NODE_OPTIONS: undefined, + }, + fn, + ); +} + describe("container auth dependencies", () => { it("login/logout auth dependencies still honor auth storage config with env token set", async () => { await withApiToken("ghi-test", async () => { @@ -63,6 +99,28 @@ describe("container auth dependencies", () => { }); }); + it("auth command dependencies defer proxy validation until network use", async () => { + await withApiToken(undefined, async () => { + await withAuthStorageEnv("file", async () => { + await withEnvVars({ HTTP_PROXY: "not a proxy secret" }, async () => { + const deps = await createAuthCommandDependencies(); + expect(deps.envApiToken).toBeUndefined(); + }); + }); + }); + }); + + it("auth status env-token path defers proxy validation for local status", async () => { + await withApiToken("ghi-test", async () => { + await withAuthStorageEnv("invalid", async () => { + await withEnvVars({ HTTP_PROXY: "not a proxy secret" }, async () => { + const deps = await createAuthStatusDependencies(); + expect(deps.envApiToken).toBe("ghi-test"); + }); + }); + }); + }); + it("auth command dependencies reject invalid auth storage config without env token", async () => { await withApiToken(undefined, async () => { await withAuthStorageEnv("invalid", async () => { @@ -76,42 +134,46 @@ describe("container auth dependencies", () => { describe("createContainer", () => { it("threads explicit client telemetry into constructed services", async () => { - await withApiToken("ghi-test", async () => { - const originalFetch = globalThis.fetch; - let capturedHeaders: Record | undefined; - const fetchFn = mock((_url: string, init?: RequestInit) => { - capturedHeaders = init?.headers as Record; - return Promise.resolve( - new Response(JSON.stringify([]), { - headers: { "Content-Type": "application/json" }, - }), - ); - }); - globalThis.fetch = fetchFn as unknown as typeof fetch; - - try { - const deps = await createContainer({ - resolveStoredToken: false, - clientName: "githits-cli/mcp", - agentProvider: () => ({ name: "cursor", version: "1.0.0" }), + await withoutProxyEnv(async () => + withApiToken("ghi-test", async () => { + const originalFetch = globalThis.fetch; + let capturedHeaders: Record | undefined; + const fetchFn = mock((_url: string, init?: RequestInit) => { + capturedHeaders = init?.headers as Record; + return Promise.resolve( + new Response(JSON.stringify([]), { + headers: { "Content-Type": "application/json" }, + }), + ); }); + globalThis.fetch = fetchFn as unknown as typeof fetch; - await deps.githitsService.getLanguages(); + try { + const deps = await createContainer({ + resolveStoredToken: false, + clientName: "githits-cli/mcp", + agentProvider: () => ({ name: "cursor", version: "1.0.0" }), + }); - expect(capturedHeaders?.Authorization).toBe("Bearer ghi-test"); - expect(capturedHeaders?.["User-Agent"]).toMatch(/^githits-cli\/\S+$/); - expect(capturedHeaders?.["x-githits-client-name"]).toBe( - "githits-cli/mcp", - ); - expect(capturedHeaders?.["x-githits-client-version"]).toMatch(/^\S+$/); - expect(capturedHeaders?.["x-githits-agent"]).toBe("cursor/1.0.0"); - expect(capturedHeaders?.["x-githits-session-id"]).toMatch( - /^[0-9a-f]{16}$/, - ); - } finally { - globalThis.fetch = originalFetch; - } - }); + await deps.githitsService.getLanguages(); + + expect(capturedHeaders?.Authorization).toBe("Bearer ghi-test"); + expect(capturedHeaders?.["User-Agent"]).toMatch(/^githits-cli\/\S+$/); + expect(capturedHeaders?.["x-githits-client-name"]).toBe( + "githits-cli/mcp", + ); + expect(capturedHeaders?.["x-githits-client-version"]).toMatch( + /^\S+$/, + ); + expect(capturedHeaders?.["x-githits-agent"]).toBe("cursor/1.0.0"); + expect(capturedHeaders?.["x-githits-session-id"]).toMatch( + /^[0-9a-f]{16}$/, + ); + } finally { + globalThis.fetch = originalFetch; + } + }), + ); }); describe("recordAuthFingerprint", () => { diff --git a/src/container.ts b/src/container.ts index 3d426fdf..44ac35b3 100644 --- a/src/container.ts +++ b/src/container.ts @@ -57,6 +57,7 @@ import { } from "./services/locked-auth-storage.js"; import { MigratingAuthStorage } from "./services/migrating-auth-storage.js"; import { ModeAwareFileAuthStorage } from "./services/mode-aware-file-auth-storage.js"; +import { createCliFetch, createLazyCliFetch } from "./services/proxy-fetch.js"; import { TokenManager } from "./services/token-manager.js"; const BASE_CLIENT_NAME = "githits-cli"; @@ -189,7 +190,7 @@ export async function createAuthCommandDependencies(): Promise + new GitHitsServiceImpl( + innerApiUrl, + token, + fetchFn, + undefined, + serviceRuntime, + ), serviceRuntime, ), }; diff --git a/src/services/proxy-fetch.test.ts b/src/services/proxy-fetch.test.ts new file mode 100644 index 00000000..f4a3687d --- /dev/null +++ b/src/services/proxy-fetch.test.ts @@ -0,0 +1,261 @@ +import { describe, expect, it, mock } from "bun:test"; +import { + type CliFetchOptions, + createCliFetch, + createLazyCliFetch, + getProxyConfig, + isNativeEnvProxyActive, + redactProxyUrl, + resolveProxyForUrl, +} from "./proxy-fetch.js"; + +describe("createCliFetch", () => { + it("returns the base fetch when proxy env vars are absent", async () => { + const baseFetch = mock(() => Promise.resolve(new Response("base"))); + + const fetchFn = createCliFetch({ + env: {}, + baseFetch: baseFetch as unknown as typeof fetch, + }); + const response = await fetchFn("https://api.githits.com/test"); + + expect(fetchFn).toBe(asBaseFetch(baseFetch)); + expect(await response.text()).toBe("base"); + }); + + it("uses native fetch only when opt-in is supported by the runtime", async () => { + const baseFetch = mock(() => Promise.resolve(new Response("native"))); + const undiciFetch = mock(() => Promise.resolve(new Response("fallback"))); + + const supported = createCliFetch({ + env: { + HTTP_PROXY: "http://proxy.local:8080", + NODE_USE_ENV_PROXY: "1", + }, + nodeVersion: "24.0.0", + baseFetch: asBaseFetch(baseFetch), + undiciFetch: asUndiciFetch(undiciFetch), + createProxyAgent: mock(() => fakeDispatcher()), + }); + + const unsupported = createCliFetch({ + env: { + HTTP_PROXY: "http://proxy.local:8080", + NODE_USE_ENV_PROXY: "1", + }, + nodeVersion: "20.18.1", + baseFetch: asBaseFetch(baseFetch), + undiciFetch: asUndiciFetch(undiciFetch), + createProxyAgent: mock(() => fakeDispatcher()), + }); + + expect(await (await supported("https://api.githits.com/test")).text()).toBe( + "native", + ); + expect( + await (await unsupported("https://api.githits.com/test")).text(), + ).toBe("fallback"); + }); + + it("does not treat unsupported --use-env-proxy opt-in as native support", async () => { + expect( + isNativeEnvProxyActive({ + env: {}, + execArgv: ["--use-env-proxy"], + nodeVersion: "20.18.1", + }), + ).toBe(false); + expect( + isNativeEnvProxyActive({ + env: {}, + execArgv: ["--use-env-proxy"], + nodeVersion: "24.4.0", + }), + ).toBe(false); + expect( + isNativeEnvProxyActive({ + env: {}, + execArgv: ["--use-env-proxy"], + nodeVersion: "24.5.0", + }), + ).toBe(true); + }); + + it("detects --use-env-proxy from NODE_OPTIONS on supported runtimes", () => { + expect( + isNativeEnvProxyActive({ + env: {}, + execArgv: [], + nodeOptions: "--trace-warnings --use-env-proxy", + nodeVersion: "22.21.0", + }), + ).toBe(true); + }); + + it("uses lowercase proxy env values before uppercase values", () => { + expect( + getProxyConfig({ + HTTP_PROXY: "http://upper.example:8080", + http_proxy: "http://lower.example:8080", + }).httpProxy, + ).toEqual({ name: "http_proxy", value: "http://lower.example:8080" }); + }); + + it("does not fall back to uppercase when lowercase proxy env is empty", () => { + expect( + getProxyConfig({ + HTTP_PROXY: "http://upper.example:8080", + http_proxy: "", + }).httpProxy, + ).toBeUndefined(); + }); + + it("uses HTTP_PROXY for HTTPS targets when HTTPS_PROXY is absent", () => { + const proxy = resolveProxyForUrl(new URL("https://api.githits.com"), { + httpProxy: { name: "HTTP_PROXY", value: "http://proxy.example:8080" }, + }); + + expect(proxy).toEqual({ + name: "HTTP_PROXY", + value: "http://proxy.example:8080", + }); + }); + + it("bypasses matching NO_PROXY hosts", async () => { + const baseFetch = mock(() => Promise.resolve(new Response("direct"))); + const undiciFetch = mock(() => Promise.resolve(new Response("proxied"))); + const fetchFn = createCliFetch({ + env: { + HTTP_PROXY: "http://proxy.local:8080", + NO_PROXY: "api.githits.com,.internal.example", + }, + baseFetch: asBaseFetch(baseFetch), + undiciFetch: asUndiciFetch(undiciFetch), + createProxyAgent: mock(() => fakeDispatcher()), + }); + + const directResponse = await fetchFn("https://api.githits.com/test"); + const proxiedResponse = await fetchFn("https://other.githits.com/test"); + + expect(await directResponse.text()).toBe("direct"); + expect(await proxiedResponse.text()).toBe("proxied"); + expect(baseFetch).toHaveBeenCalledTimes(1); + expect(undiciFetch).toHaveBeenCalledTimes(1); + }); + + it("bypasses IPv6 NO_PROXY entries", () => { + const proxyConfig = { + httpProxy: { name: "HTTP_PROXY", value: "http://proxy.example:8080" }, + noProxy: "::1,[2001:db8::1]:8080", + }; + + expect( + resolveProxyForUrl(new URL("http://[::1]:3000"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://[2001:db8::1]:8080"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://[2001:db8::1]:9090"), proxyConfig), + ).toEqual({ name: "HTTP_PROXY", value: "http://proxy.example:8080" }); + }); + + it("bypasses exact hosts and subdomains for NO_PROXY domain entries", () => { + const proxyConfig = { + httpProxy: { name: "HTTP_PROXY", value: "http://proxy.example:8080" }, + noProxy: "example.com,.internal.example,*.private.example", + }; + + expect( + resolveProxyForUrl(new URL("http://example.com"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://api.example.com"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://svc.internal.example"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://api.private.example"), proxyConfig), + ).toBeUndefined(); + expect( + resolveProxyForUrl(new URL("http://notexample.com"), proxyConfig), + ).toEqual({ name: "HTTP_PROXY", value: "http://proxy.example:8080" }); + }); + + it("bypasses all targets when NO_PROXY contains a wildcard entry", () => { + expect( + resolveProxyForUrl(new URL("http://api.githits.com"), { + httpProxy: { name: "HTTP_PROXY", value: "http://proxy.example:8080" }, + noProxy: "localhost,*", + }), + ).toBeUndefined(); + }); + + it("fails malformed proxy env values without exposing their value", () => { + expect(() => + createCliFetch({ env: { HTTP_PROXY: "not a proxy secret" } }), + ).toThrow("HTTP_PROXY must be an http:// or https:// proxy URL."); + expect(() => + createCliFetch({ env: { HTTP_PROXY: "not a proxy secret" } }), + ).not.toThrow("secret"); + }); + + it("defers malformed proxy env failures for lazy fetch callers", async () => { + const fetchFn = createLazyCliFetch({ + env: { HTTP_PROXY: "secret://user:pass@proxy.example/path" }, + }); + + await expect(fetchFn("http://target.example/test")).rejects.toThrow( + "HTTP_PROXY must be an http:// or https:// proxy URL.", + ); + await expect(fetchFn("http://target.example/test")).rejects.not.toThrow( + "user:pass", + ); + }); + + it("redacts credentials, path, query, and fragment from proxy URLs", () => { + expect( + redactProxyUrl("http://user:pass@proxy.example:8080/path?q=secret#frag"), + ).toBe("http://proxy.example:8080/"); + }); + + it("sanitizes fallback transport errors", async () => { + const undiciFetch = mock(() => + Promise.reject( + new Error( + "connect failed http://user:pass@proxy.example:8080/private?q=1", + ), + ), + ); + const fetchFn = createCliFetch({ + env: { HTTP_PROXY: "http://user:pass@proxy.example:8080/private?q=1" }, + undiciFetch: asUndiciFetch(undiciFetch), + createProxyAgent: mock(() => fakeDispatcher()), + }); + + await expect(fetchFn("http://target.example/test")).rejects.toThrow( + "Proxy request failed using HTTP_PROXY (http://proxy.example:8080/): connect failed http://proxy.example:8080/", + ); + await expect(fetchFn("http://target.example/test")).rejects.not.toThrow( + "user:pass", + ); + await expect(fetchFn("http://target.example/test")).rejects.not.toThrow( + "private", + ); + }); +}); + +function fakeDispatcher(): never { + return {} as never; +} + +function asBaseFetch(value: unknown): typeof fetch { + return value as typeof fetch; +} + +function asUndiciFetch( + value: unknown, +): NonNullable { + return value as NonNullable; +} diff --git a/src/services/proxy-fetch.ts b/src/services/proxy-fetch.ts new file mode 100644 index 00000000..c19a62d7 --- /dev/null +++ b/src/services/proxy-fetch.ts @@ -0,0 +1,361 @@ +import { type Dispatcher, ProxyAgent, fetch as undiciFetch } from "undici"; + +export interface CliFetchOptions { + env?: Record; + execArgv?: string[]; + nodeOptions?: string | undefined; + nodeVersion?: string; + baseFetch?: typeof fetch; + undiciFetch?: typeof undiciFetch; + createProxyAgent?: (proxyUrl: string) => ProxyDispatcher; +} + +export interface ProxyEnvSelection { + name: string; + value: string; +} + +export interface ProxyConfig { + httpProxy?: ProxyEnvSelection; + httpsProxy?: ProxyEnvSelection; + noProxy?: string; +} + +type FetchInput = Parameters[0]; +type FetchInit = Parameters[1]; +type UndiciInput = Parameters[0]; +type UndiciInit = NonNullable[1]>; +type ProxyDispatcher = Dispatcher & NonNullable; + +const NODE_USE_ENV_PROXY = "NODE_USE_ENV_PROXY"; +const USE_ENV_PROXY_FLAG = "--use-env-proxy"; + +export function createCliFetch(options: CliFetchOptions = {}): typeof fetch { + const env = options.env ?? process.env; + const baseFetch = options.baseFetch ?? globalThis.fetch; + const proxyConfig = getProxyConfig(env); + if (!proxyConfig.httpProxy && !proxyConfig.httpsProxy) { + return baseFetch; + } + if ( + isNativeEnvProxyActive({ + env, + execArgv: options.execArgv ?? process.execArgv, + nodeOptions: options.nodeOptions ?? env.NODE_OPTIONS, + nodeVersion: options.nodeVersion ?? process.versions.node, + }) + ) { + return baseFetch; + } + + validateProxySelection(proxyConfig.httpProxy); + validateProxySelection(proxyConfig.httpsProxy); + + const fetchWithDispatcher = options.undiciFetch ?? undiciFetch; + const createProxyAgent = + options.createProxyAgent ?? + ((proxyUrl: string) => + new ProxyAgent({ uri: proxyUrl, proxyTunnel: false })); + const proxyAgents = new Map(); + + return (async (input: FetchInput, init?: FetchInit) => { + const targetUrl = getRequestUrl(input); + if (!targetUrl) { + return baseFetch(input, init); + } + const proxy = resolveProxyForUrl(targetUrl, proxyConfig); + if (!proxy) { + return baseFetch(input, init); + } + + let dispatcher = proxyAgents.get(proxy.value); + if (!dispatcher) { + dispatcher = createProxyAgent(proxy.value); + proxyAgents.set(proxy.value, dispatcher); + } + + try { + const undiciInit = { + ...(init as Record | undefined), + dispatcher, + } as UndiciInit; + return await fetchWithDispatcher(input as UndiciInput, undiciInit); + } catch (error) { + throw createSanitizedProxyRequestError(proxy, error); + } + }) as typeof fetch; +} + +export function createLazyCliFetch( + options: CliFetchOptions = {}, +): typeof fetch { + let fetchFn: typeof fetch | undefined; + return (async (input: FetchInput, init?: FetchInit) => { + fetchFn ??= createCliFetch(options); + return await fetchFn(input, init); + }) as typeof fetch; +} + +export function getProxyConfig( + env: Record, +): ProxyConfig { + return { + httpProxy: getEnvSelection(env, "HTTP_PROXY"), + httpsProxy: getEnvSelection(env, "HTTPS_PROXY"), + noProxy: getEnvSelection(env, "NO_PROXY")?.value, + }; +} + +export function isNativeEnvProxyActive(options: { + env: Record; + execArgv: string[]; + nodeOptions?: string | undefined; + nodeVersion: string; +}): boolean { + const envOptIn = options.env[NODE_USE_ENV_PROXY] === "1"; + const flagOptIn = + hasUseEnvProxyFlag(options.execArgv) || + hasUseEnvProxyFlag(splitNodeOptions(options.nodeOptions)); + if (envOptIn && supportsNativeEnvProxyEnv(options.nodeVersion)) { + return true; + } + return flagOptIn && supportsNativeEnvProxyFlag(options.nodeVersion); +} + +export function resolveProxyForUrl( + targetUrl: URL, + proxyConfig: ProxyConfig, +): ProxyEnvSelection | undefined { + if (shouldBypassProxy(targetUrl, proxyConfig.noProxy)) { + return undefined; + } + if (targetUrl.protocol === "http:") { + return proxyConfig.httpProxy; + } + if (targetUrl.protocol === "https:") { + return proxyConfig.httpsProxy ?? proxyConfig.httpProxy; + } + return undefined; +} + +export function redactProxyUrl(value: string): string { + try { + const url = new URL(value); + url.username = ""; + url.password = ""; + url.pathname = ""; + url.search = ""; + url.hash = ""; + return url.toString(); + } catch { + return ""; + } +} + +function getEnvSelection( + env: Record, + upperName: string, +): ProxyEnvSelection | undefined { + const lowerName = upperName.toLowerCase(); + if (hasEnvKey(env, lowerName)) { + const lowerValue = env[lowerName]; + return lowerValue ? { name: lowerName, value: lowerValue } : undefined; + } + if (hasEnvKey(env, upperName)) { + const upperValue = env[upperName]; + return upperValue ? { name: upperName, value: upperValue } : undefined; + } + return undefined; +} + +function hasEnvKey( + env: Record, + key: string, +): boolean { + return Object.hasOwn(env, key); +} + +function validateProxySelection( + selection: ProxyEnvSelection | undefined, +): void { + if (!selection) { + return; + } + let parsed: URL; + try { + parsed = new URL(selection.value); + } catch { + throw new Error( + `${selection.name} must be an http:// or https:// proxy URL.`, + ); + } + if ( + (parsed.protocol !== "http:" && parsed.protocol !== "https:") || + !parsed.host + ) { + throw new Error( + `${selection.name} must be an http:// or https:// proxy URL.`, + ); + } +} + +function getRequestUrl(input: FetchInput): URL | undefined { + try { + if (typeof input === "string" || input instanceof URL) { + return new URL(input); + } + if (typeof Request !== "undefined" && input instanceof Request) { + return new URL(input.url); + } + } catch { + return undefined; + } + return undefined; +} + +function shouldBypassProxy( + targetUrl: URL, + noProxy: string | undefined, +): boolean { + if (!noProxy) { + return false; + } + if (noProxy.trim() === "*") { + return true; + } + const hostname = normalizeHostname(targetUrl.hostname); + const port = Number.parseInt(targetUrl.port, 10) || defaultPort(targetUrl); + for (const rawEntry of noProxy.split(/[,\s]/)) { + const entry = rawEntry.trim().toLowerCase(); + if (!entry) { + continue; + } + if (entry === "*") { + return true; + } + const { host: entryHost, port: entryPort } = parseNoProxyEntry(entry); + if (entryPort && entryPort !== port) { + continue; + } + if (matchesNoProxyHost(hostname, entryHost)) { + return true; + } + } + return false; +} + +function matchesNoProxyHost(hostname: string, entryHost: string): boolean { + const normalizedEntryHost = entryHost.replace(/^\*?\./, ""); + return ( + hostname === normalizedEntryHost || + hostname.endsWith(`.${normalizedEntryHost}`) + ); +} + +function parseNoProxyEntry(entry: string): { host: string; port: number } { + const bracketedIpv6 = entry.match(/^\[([^\]]+)\](?::(\d+))?$/); + if (bracketedIpv6?.[1]) { + return { + host: normalizeHostname(bracketedIpv6[1]), + port: bracketedIpv6[2] ? Number.parseInt(bracketedIpv6[2], 10) : 0, + }; + } + if (entry.includes(":")) { + const lastColon = entry.lastIndexOf(":"); + const maybePort = entry.slice(lastColon + 1); + const hostPart = entry.slice(0, lastColon); + if (!hostPart.includes(":") && /^\d+$/.test(maybePort)) { + return { + host: normalizeHostname(hostPart), + port: Number.parseInt(maybePort, 10), + }; + } + return { host: normalizeHostname(entry), port: 0 }; + } + return { host: normalizeHostname(entry), port: 0 }; +} + +function normalizeHostname(hostname: string): string { + return hostname.replace(/^\[|\]$/g, "").toLowerCase(); +} + +function defaultPort(url: URL): number { + if (url.protocol === "http:") { + return 80; + } + if (url.protocol === "https:") { + return 443; + } + return 0; +} + +function hasUseEnvProxyFlag(args: string[]): boolean { + return args.some((arg) => arg === USE_ENV_PROXY_FLAG); +} + +function splitNodeOptions(value: string | undefined): string[] { + if (!value) { + return []; + } + return value.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) ?? []; +} + +function supportsNativeEnvProxyEnv(nodeVersion: string): boolean { + const parsed = parseNodeVersion(nodeVersion); + if (!parsed) { + return false; + } + const [major, minor] = parsed; + if (major === 22) { + return minor >= 21; + } + if (major === 23) { + return false; + } + return major >= 24; +} + +function supportsNativeEnvProxyFlag(nodeVersion: string): boolean { + const parsed = parseNodeVersion(nodeVersion); + if (!parsed) { + return false; + } + const [major, minor] = parsed; + if (major === 22) { + return minor >= 21; + } + if (major === 24) { + return minor >= 5; + } + return major >= 25; +} + +function parseNodeVersion(value: string): [number, number] | undefined { + const match = value.match(/^(\d+)\.(\d+)\./); + if (!match?.[1] || !match[2]) { + return undefined; + } + return [Number.parseInt(match[1], 10), Number.parseInt(match[2], 10)]; +} + +function createSanitizedProxyRequestError( + proxy: ProxyEnvSelection, + error: unknown, +): Error { + const reason = sanitizeErrorMessage(error); + return new Error( + `Proxy request failed using ${proxy.name} (${redactProxyUrl(proxy.value)})${reason ? `: ${reason}` : "."}`, + ); +} + +function sanitizeErrorMessage(error: unknown): string { + if (!(error instanceof Error) || !error.message) { + return ""; + } + return error.message + .replace(/https?:\/\/\S+/gi, (match) => redactProxyUrl(match)) + .replace( + /\b(?!https?:)[A-Za-z][A-Za-z0-9+.-]*:\/\/\S+/gi, + "", + ); +}