Skip to content

feat: add HTTP/HTTPS proxy and TLS support (#40) - #43

Open
alerizzo wants to merge 2 commits into
mainfrom
feat/proxy-tls-support
Open

feat: add HTTP/HTTPS proxy and TLS support (#40)#43
alerizzo wants to merge 2 commits into
mainfrom
feat/proxy-tls-support

Conversation

@alerizzo

@alerizzo alerizzo commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #40. Supersedes #39.

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 is unusable behind a corporate proxy.

Approach

Delegate to configureProxy() from @codacy/tooling (0.1.00.22.0) — the same function the Codacy Analysis CLI calls. We write no proxy logic. Upstream installs a global undici dispatcher with per-request protocol + NO_PROXY routing, bare host:port normalization, and SSL_CERT_FILE/NODE_EXTRA_CA_CERTS CA 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.md now records that proxy behavior changes belong in analysis-cli's packages/tooling/src/proxy.ts, followed by a dependency bump here.

src/utils/proxy.ts is a thin seam — configureProxyFromEnv() calls upstream and routes its throw into handleError(), giving red Error: <message> and exit 1 like every other failure here. It's a module rather than an inline try/catch because nothing in this repo has ever executed src/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:

Variable Purpose
HTTPS_PROXY / HTTP_PROXY (or lowercase) Proxy URL per scheme; bare host:port accepted
NO_PROXY / no_proxy Hosts that bypass the proxy (*, .suffix), matched per request
SSL_CERT_FILE / NODE_EXTRA_CA_CERTS PEM CA bundle for a TLS-intercepting proxy
CODACY_CLI_INSECURE Disable TLS verification, last resort (warns on stderr)

Behaviorally a no-op when none are set — though not free; see the cost note below.

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.1 as a direct dependency, and review found:

  • undici@8.10.1 requires Node >= 22.19.0, against this package's engines: ">=20". Verified on Node v20.20.2 (latest 20.x): require("undici") throws TypeError: webidl.util.markAsUncloneable is not a function at module load, so codacy --version crashed for every Node 20 user with no proxy configured. Tooling's undici@^6.21.0 supports Node >= 18.17.
  • NO_PROXY was evaluated once against the Codacy host, then a process-global agent was installed — so NO_PROXY=cveawg.mitre.org was ignored, and NO_PROXY=app.codacy.com disabled the proxy for everything.
  • A bare host:port crashed the CLI with a raw undici stack trace. That form is what curl accepts, i.e. exactly this feature's audience.
  • No CA support in codeSSL_CERT_FILE was silently ignored.

That Node 20 regression passed CI, because the proxy tests mocked undici wholesale and nothing executed the entry point. So this PR also adds a smoke step running the built CLI three ways (plain, with HTTPS_PROXY, and with a bad SSL_CERT_FILE expected to fail) on both matrix legs.

Verification

Beyond tsc --noEmit and 614 passing tests, verified against a local CONNECT-logging proxy:

Startup cost — disclosed, with a follow-up

@codacy/tooling@0.22.0 imports undici at module scope rather than behind configureProxy's "nothing configured" early-out, so every invocation pays for loading it — --help and --version included, proxy or not.

Measured, 20 interleaved runs of --version on Node 20 against a main build:

main (no proxy code): median 119 ms
this branch:          median 146 ms
delta:                        27 ms   (~23%)

@codacy/tooling also stops being a phantom dependency: src/types/codacy-config.ts uses export type, which tsc erases, so dist/ previously contained no require("@codacy/tooling") at all.

Upstream 0.23.0, published today, fixes this — it moves the import behind a memoized lazy factory and adds proxy-URL validation with credential redaction (both filed from this work in analysis-cli's docs/tech-debt.md). It is deliberately not taken here: it cannot be installed or verified in the environment this branch was built in. I did check the one thing that could break on the bump — its CA-bundle error text is unchanged, so the CI smoke step's substring assertion survives it. Worth a small follow-up PR that bumps and re-measures.

Dependency is pinned exactly, after a correction

This branch first used ^0.22.0, on the reasoning that upstream proxy fixes would then arrive without a bump PR. That reasoning was wrong, and an adversarial review caught it: for a pre-1.0 package the caret spans patches only (^0.22.0>=0.22.0 <0.23.0-0), so it would never have picked up 0.23.0. It bought silent patch-level drift — against a dependency this repo has no proxy coverage for, since the 4 unit tests mock it away — and none of the intended upside. Now pinned exactly, bumps deliberate.

For the record, the related claim that "every other dependency here is pinned exactly" was also wrong: cli-table3 was already ^0.6.3 before this branch.

Known gap

The 4 unit tests mock @codacy/tooling entirely, so they pin only this repo's seam — that we delegate with no overrides, and that a throw becomes exit 1 with the message intact. NO_PROXY matching, scheme routing and bare-host handling are verified upstream and by the manual runs above, but nothing in this repo's CI would catch a regression in them. The smoke step covers construction plus the CA-failure message only. Called out rather than papered over; closing it properly means either integration tests with a local proxy here, or trusting upstream's suite, and that's a judgement call worth making explicitly.

🤖 Generated with Claude Code

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>
@codacy-production

codacy-production Bot commented Sep 7, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 medium · 3 minor

Alerts:
⚠ 4 issues (≤ 0 issues of at least minor severity)

Results:
4 new issues

Category Results
BestPractice 1 medium
Comprehensibility 3 minor

View in Codacy

🟢 Metrics 17 complexity · 0 duplication

Metric Results
Complexity 17
Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread AGENTS.md
| `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 |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ LOW RISK

Define the 'PEM' (Privacy-Enhanced Mail) acronym to improve documentation clarity.

This might be a simple fix:

Suggested change
| `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 |

See Issue in Codacy

Comment thread AGENTS.md
- 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ 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:

Suggested change
- **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.

See Issue in Codacy
See Issue in Codacy

Follow-up to an adversarial review of this branch. Three claims were wrong and
one dependency range did not do what it was chosen to do.

- Pin @codacy/tooling to exact 0.22.0, was ^0.22.0. The caret was chosen so
  upstream proxy fixes would arrive without a bump PR, but for a pre-1.0
  package the caret spans patches only (^0.22.0 resolves to >=0.22.0 <0.23.0-0),
  so it never would have picked up a 0.23.0. It bought silent patch drift with
  no proxy coverage in this repo to catch a regression, and none of the upside.
  Bumps are now deliberate. Also drops the claim that every other dependency
  here is pinned exactly -- cli-table3 was already ^0.6.3 before this branch.

- configureProxyFromEnv's doc claimed a bad CA bundle is "the one thing
  configureProxy throws on". It is not: a malformed proxy URL throws too, as
  whatever new URL() or undici's ProxyAgent raises. Verified against the
  installed 0.22.0 -- HTTPS_PROXY="not a url" gives `Error: Invalid URL`,
  ftp:// gives `Error: invalid url`. The catch is intentionally broad, so the
  comment now states that contract instead of enumerating a list that rots.

- "No-op when nothing is set" was true of behavior but not of cost. 0.22.0
  imports undici at module scope rather than behind configureProxy's early-out,
  so every invocation pays it, --help and --version included. Measured ~27 ms
  median against a ~119 ms baseline (20 interleaved runs, Node 20). Disclosed
  in the module header and the changeset rather than left implied.

Upstream 0.23.0 (published today) moves that import behind a lazy factory and
adds proxy-URL validation with credential redaction. Not taken here: it cannot
be installed or verified in this environment. Its CA-bundle error text is
unchanged, so the CI smoke step's substring assertion survives the bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codacy CLI is unable to connect to API when ran behing corporate proxy

1 participant