feat: add HTTP/HTTPS proxy and TLS support (#40) - #43
Conversation
Node's global fetch — used by the generated API client and the MITRE CVE lookup in commands/finding.ts — ignores HTTP_PROXY/HTTPS_PROXY/NO_PROXY, so the CLI was unusable behind a corporate proxy. Rather than reimplement it, delegate to configureProxy() from @codacy/tooling (bumped 0.1.0 -> ^0.22.0), the same function the Codacy Analysis CLI calls. It installs a global undici dispatcher with per-request protocol and NO_PROXY routing, bare host:port normalization, and SSL_CERT_FILE/NODE_EXTRA_CA_CERTS CA loading. Keeping the implementation upstream is what keeps the environment contract identical across the Codacy tools; a local copy would drift. src/utils/proxy.ts is a thin seam: configureProxyFromEnv() calls it and routes its deliberate fail-loud throw (unreadable or non-PEM CA bundle) into handleError(), giving red `Error: <message>` and exit 1 like every other failure here. analysis-cli exits 2 because it has a documented exit-code scheme; this CLI does not, and exits 1 everywhere. Called at the top of src/index.ts. Ordering is only constrained to precede program.parse, since the dispatcher is resolved per request — it goes first so the network stack is set up before we point it at the API. Kept top-level rather than in the preAction hook so a typo'd SSL_CERT_FILE fails even on --version. Also add a CI smoke step that runs the built entry point three ways (plain, with HTTPS_PROXY set, and with a bad SSL_CERT_FILE expected to fail). Nothing previously executed src/index.ts — every command test builds a bare new Command() — which is how a proxy dependency that cannot even load on Node 20 could pass CI. Upstream owns the proxy semantics and their 24 tests, so only the seam is tested here (4 new tests, 614 total). Supersedes #39. Co-Authored-By: rattalur <145406381+rattalur@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 medium (1 false positive) |
| Comprehensibility | 3 minor |
🟢 Metrics 17 complexity · 0 duplication
Metric Results Complexity 17 Duplication 0
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
This PR implements HTTP/HTTPS proxy and TLS support by delegating configuration to the shared @codacy/tooling library. This approach ensures the CLI honors standard environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY, SSL_CERT_FILE) and maintains compatibility with Node.js 20.
While Codacy analysis is flagged as 'not up to standards' due to coverage requirements, the code quality itself is high. All primary acceptance criteria, including per-request NO_PROXY routing and custom CA bundle support, are addressed and verified by the test scenarios. Documentation nits regarding acronym definitions and paragraph length are the only remaining items.
Test suggestions
- Found recommended test scenario: Verify configureProxyFromEnv delegates to tooling without passing overrides to maintain environment parity
- Found recommended test scenario: Verify configureProxyFromEnv catches CA bundle errors and routes them through the shared handleError for fatal exit
- Found recommended test scenario: Smoke test: ensure built CLI entry point loads and runs on Node 20 and 22 without proxy configuration
- Found recommended test scenario: Smoke test: ensure built CLI handles dispatcher construction on Node 20 when proxy variables are present
- Found recommended test scenario: Smoke test: ensure built CLI exits with non-zero status and clear error when SSL_CERT_FILE points to a missing file
TIP How was this review? Give us feedback
| | `CODACY_PROJECT_TOKEN` | One of the two | Repository (project) token, scoped to one repository. Get it from Codacy > Repository > Settings > Integrations > Project API token. **Outranks `CODACY_API_TOKEN`** — see `SPECS/repository-tokens.md` | | ||
| | `HTTPS_PROXY` / `HTTP_PROXY` | No | Proxy URL per scheme (lowercase also honored). Resolved by `@codacy/tooling`'s `configureProxy()`, called once from `src/index.ts` via `configureProxyFromEnv()` | | ||
| | `NO_PROXY` / `no_proxy` | No | Comma-separated hosts that bypass the proxy (`*`, `.suffix`), matched **per request** — not once at startup | | ||
| | `SSL_CERT_FILE` / `NODE_EXTRA_CA_CERTS` | No | PEM CA bundle for a TLS-intercepting proxy. **Replaces** the default trust store; unreadable or non-PEM is fatal by design | |
There was a problem hiding this comment.
⚪ LOW RISK
Define the 'PEM' (Privacy-Enhanced Mail) acronym to improve documentation clarity.
This might be a simple fix:
| | `SSL_CERT_FILE` / `NODE_EXTRA_CA_CERTS` | No | PEM CA bundle for a TLS-intercepting proxy. **Replaces** the default trust store; unreadable or non-PEM is fatal by design | | |
| `SSL_CERT_FILE` / `NODE_EXTRA_CA_CERTS` | No | Privacy-Enhanced Mail (PEM) CA bundle for a TLS-intercepting proxy. **Replaces** the default trust store; unreadable or non-PEM is fatal by design | |
| - Default cadence is `POLL_INTERVAL_MS` (10s), capped at `MAX_WAIT_MS` (20min). | ||
| - **Error handling:** Use `try/catch` with the shared `handleError()` from `src/utils/error.ts` | ||
| - **API base URL:** `https://app.codacy.com/api/v3` (configured in `src/index.ts` via `OpenAPI.BASE`) | ||
| - **Proxy / TLS:** never hand-roll this. Outbound HTTP configuration is delegated to `configureProxy()` from `@codacy/tooling`, wrapped by `configureProxyFromEnv()` in `src/utils/proxy.ts` and called once at the top of `src/index.ts`. It installs a global `undici` dispatcher, so every `fetch` — the generated client and the CVE lookup alike — is covered without touching generated code. Keeping the implementation upstream is what keeps the environment contract identical to the Codacy Analysis CLI; a local reimplementation would drift. If proxy behavior needs to change, change it in `analysis-cli`'s `packages/tooling/src/proxy.ts` and bump the dependency here. |
There was a problem hiding this comment.
⚪ LOW RISK
Break this long paragraph into shorter sentences to improve readability. This helps clarify the separate architectural steps of delegating configuration to tooling, entry point initialization, and the dispatcher's scope. Also, define 'CVE' (Common Vulnerabilities and Exposures) on its first use.
This might be a simple fix:
| - **Proxy / TLS:** never hand-roll this. Outbound HTTP configuration is delegated to `configureProxy()` from `@codacy/tooling`, wrapped by `configureProxyFromEnv()` in `src/utils/proxy.ts` and called once at the top of `src/index.ts`. It installs a global `undici` dispatcher, so every `fetch` — the generated client and the CVE lookup alike — is covered without touching generated code. Keeping the implementation upstream is what keeps the environment contract identical to the Codacy Analysis CLI; a local reimplementation would drift. If proxy behavior needs to change, change it in `analysis-cli`'s `packages/tooling/src/proxy.ts` and bump the dependency here. | |
| - **Proxy / TLS:** never hand-roll this. Outbound HTTP configuration is delegated to `configureProxy()` from `@codacy/tooling`, wrapped by `configureProxyFromEnv()` in `src/utils/proxy.ts`. This is called once at the top of `src/index.ts` to install a global `undici` dispatcher. This ensures every `fetch` — including the generated client and Common Vulnerabilities and Exposures (CVE) lookups — is covered. Keeping the implementation upstream maintains the environment contract with the Codacy Analysis CLI. If proxy behavior needs to change, update it in `analysis-cli`'s `packages/tooling/src/proxy.ts` and bump the dependency here. |
Closes #40. Supersedes #39.
Node's global
fetch— used by the generated API client and the MITRE CVE lookup incommands/finding.ts— ignoresHTTP_PROXY/HTTPS_PROXY/NO_PROXY, so the CLI is unusable behind a corporate proxy.Approach
Delegate to
configureProxy()from@codacy/tooling(0.1.0→^0.22.0) — the same function the Codacy Analysis CLI calls. We write no proxy logic. Upstream installs a globalundicidispatcher with per-request protocol +NO_PROXYrouting, barehost:portnormalization, andSSL_CERT_FILE/NODE_EXTRA_CA_CERTSCA loading.Keeping the implementation upstream is the point: it's what makes the environment contract identical across the Codacy tools. A local copy would drift.
AGENTS.mdnow records that proxy behavior changes belong inanalysis-cli'spackages/tooling/src/proxy.ts, followed by a dependency bump here.src/utils/proxy.tsis a thin seam —configureProxyFromEnv()calls upstream and routes its deliberate fail-loud throw (unreadable or non-PEM CA bundle) intohandleError(), giving redError: <message>and exit 1 like every other failure here. It's a module rather than an inlinetry/catchbecause nothing in this repo has ever executedsrc/index.ts, so inline logic would be the only untestable branch in the feature.Supported variables — same names as the Analysis CLI and the VS Code extension:
HTTPS_PROXY/HTTP_PROXY(or lowercase)host:portacceptedNO_PROXY/no_proxy*,.suffix), matched per requestSSL_CERT_FILE/NODE_EXTRA_CA_CERTSCODACY_CLI_INSECURENo-op when none are set.
Why not #39
@rattalur found a real gap and their PR's docs — particularly the curl-works-but-the-CLI-doesn't diagnostic — were better than most. But that implementation hand-rolled the proxy logic with
undici@8.10.1as a direct dependency, and review found:undici@8.10.1requires Node >= 22.19.0, against this package'sengines: ">=20". Verified on Node v20.20.2 (latest 20.x):require("undici")throwsTypeError: webidl.util.markAsUncloneable is not a functionat module load, socodacy --versioncrashed for every Node 20 user with no proxy configured. Tooling'sundici@^6.21.0supports Node >= 18.17.NO_PROXYwas evaluated once against the Codacy host, then a process-global agent was installed — soNO_PROXY=cveawg.mitre.orgwas ignored, andNO_PROXY=app.codacy.comdisabled the proxy for everything.host:portcrashed the CLI with a raw undici stack trace. That form is what curl accepts, i.e. exactly this feature's audience.SSL_CERT_FILEwas silently ignored.That Node 20 regression passed CI, because the proxy tests mocked
undiciwholesale and nothing executed the entry point. So this PR also adds a smoke step running the built CLI three ways (plain, withHTTPS_PROXY, and with a badSSL_CERT_FILEexpected to fail) on both matrix legs.Verification
Beyond
tsc --noEmitand 614 passing tests, verified against a local CONNECT-logging proxy:dist/index.js --versionstarts fine (the Add undici-based HTTP/HTTPS proxy support with tests and documentation #39 failure mode)NO_PROXY→ bothapp.codacy.comandcveawg.mitre.orgloggedNO_PROXY=app.codacy.com→ onlycveawg.mitre.orglogged; Codacy went direct. This is the per-request routing Add undici-based HTTP/HTTPS proxy support with tests and documentation #39 could not doNO_PROXY=*→ nothing proxiedHTTPS_PROXY=proxy.corp:8080(bare host) → normalized, no crashSSL_CERT_FILE→Error: Failed to read CA certificate .../No PEM certificate found ..., exit 1codacy infoend-to-end through the proxy with real credentials → succeeded, CONNECT loggedTwo things to flag
The caret is deliberate. Every other dependency here is pinned exactly;
@codacy/toolingis^0.22.0so upstream proxy fixes flow in without a bump PR. Say the word if you'd rather pin it.@codacy/toolingstops being a phantom dependency.src/types/codacy-config.tsusesexport type, which tsc erases —dist/currently contains norequire("@codacy/tooling"). Now it does, and since tooling's entry is a barrel with a top-levelundiciimport, requiring it loads all of undici. Measured warm: existing deps ~25-27 ms, undici adds ~33-35 ms, so module-load time roughly doubles (~35 ms absolute) on every invocation including--help.The right fix is one line upstream — move
undicito a lazyrequireinsideconfigureProxy, after its existing no-op early return — which helps analysis-cli too. I deliberately did not work around it downstream with an env sniff, since that would re-implement the contract we're reusing and would fail silently when upstream adds a variable we don't sniff for.🤖 Generated with Claude Code