From 2132be03f2052391f7fd40762b7b59b1b84210e5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 04:29:52 +0000 Subject: [PATCH 01/13] fix(tls): support custom CA certificates for corporate proxies (CLI-1K6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add custom CA certificate loading so the CLI works behind TLS-intercepting corporate proxies (Zscaler, Netskope, etc.). Bun uses BoringSSL with a compiled-in Mozilla CA bundle and does not honor NODE_EXTRA_CA_CERTS or SSL_CERT_FILE natively — we now read these env vars and pass the PEM contents to Bun's fetch({ tls: { ca } }) option. CA loading priority: stored default > NODE_EXTRA_CA_CERTS > SSL_CERT_FILE. Users can register a CA cert via 'sentry cli defaults ca-cert /path/to/ca.pem' which also silences a security warning logged when env-sourced CAs target sentry.io (the warning creates a forensic trail for CI environments where an attacker could inject a rogue CA via env vars). Also improves TLS error handling: - Detect TLS cert errors by pattern and wrap in ApiError (not raw Error) - Show actionable guidance pointing to 'sentry cli defaults ca-cert' - Don't retry TLS errors (deterministic — retrying is pointless) --- AGENTS.md | 107 ++++++----- docs/src/content/docs/self-hosted.md | 2 + script/generate-docs-sections.ts | 5 + src/commands/cli/defaults.ts | 54 +++++- src/lib/custom-ca.ts | 191 +++++++++++++++++++ src/lib/db/defaults.ts | 28 +++ src/lib/env-registry.ts | 18 ++ src/lib/oauth.ts | 36 +++- src/lib/sentry-client.ts | 73 +++++++- test/commands/cli/defaults.test.ts | 2 + test/lib/custom-ca.test.ts | 270 +++++++++++++++++++++++++++ 11 files changed, 721 insertions(+), 65 deletions(-) create mode 100644 src/lib/custom-ca.ts create mode 100644 test/lib/custom-ca.test.ts diff --git a/AGENTS.md b/AGENTS.md index 7a1ab4b53..1bfd4b629 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1001,81 +1001,86 @@ mock.module("./some-module", () => ({ ### Architecture - -* **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` > \`SENTRY\_TOKEN\` > SQLite OAuth token. \`getEnvToken()\` trims env vars (empty/whitespace = unset). \`AuthSource\` tracks provenance. \`ENV\_SOURCE\_PREFIX = "env:"\` — use \`.length\` not hardcoded 4. Env tokens bypass refresh/expiry. \`isEnvTokenActive()\` guards auth commands. Logout must NOT clear stored auth when env token active. These functions stay in \`db/auth.ts\` despite not touching DB because they're tightly coupled with token retrieval. + +* **@sentry/api SDK integration: type wrapping pattern and pagination helpers**: @sentry/api SDK integration: wrap SDK types at \`src/lib/api/\*.ts\` boundaries with \`as unknown as SentryX\` casts; never leak SDK types to commands. Wrappers in \`src/types/sentry.ts\` use \`Partial\ & RequiredCore\`. \`src/lib/region.ts\` imports \`retrieveAnOrganization\` directly to avoid circular dep with api-client. \`unwrapResult\`/\`unwrapPaginatedResult\` MUST stay CLI-owned — SDK versions throw plain \`Error\`, breaking the 'all errors are CliError subclasses' invariant (see also 365e4299). Body-shape casts use \`Parameters\\[0]\["body"]\`. - -* **Consola chosen as CLI logger with Sentry createConsolaReporter integration**: Consola is the CLI logger with Sentry \`createConsolaReporter\` integration. Two reporters: FancyReporter (stderr) + Sentry structured logs. Level via \`SENTRY\_LOG\_LEVEL\`. \`buildCommand\` injects hidden \`--log-level\`/\`--verbose\` flags. \`withTag()\` creates independent instances; \`setLogLevel()\` propagates via registry. All user-facing output must use consola, not raw stderr. \`HandlerContext\` intentionally omits stderr. + +* **apiRequestToRegion/rawApiRequest options shape — no timeout/signal/headers on the typed API**: \`ApiRequestOptions\\` in \`src/lib/api/infrastructure.ts\` has only \`{ method, body, params, schema }\`. \`rawApiRequest\` adds \`headers?\`; neither exposes \`timeout\`/\`signal\`. Call sites pass \`(url, init: RequestInit)\` to authenticated fetch — never a \`Request\` (only @sentry/api SDK does). \`apiRequestToRegion\` auto-sets JSON Content-Type and \`JSON.stringify\`s body; \`rawApiRequest\` preserves string bodies, only sets JSON Content-Type when body is object and caller didn't provide one. 204/205 throw \`ApiError\` rather than crashing on \`response.json()\` — bulk-mutate callers must catch. - -* **DSN cache invalidation uses two-level mtime tracking (sourceMtimes + dirMtimes)**: DSN cache invalidation — two-level mtime tracking: \`sourceMtimes\` (DSN-bearing files, catches in-place edits) + \`dirMtimes\` (every walked dir, catches new files) + root mtime fast-path + 24h TTL. Dropping either map is a correctness regression. Walker emits mtimes via \`onDirectoryVisit\` hook + \`recordMtimes\` option; DSN scanner uses \`grepFiles({pattern: DSN\_PATTERN, recordMtimes: true, onDirectoryVisit})\` (~20% faster than walkFiles). \`scanCodeForFirstDsn\` stays on direct walker loop (worker init ~20ms dominates single-DSN). Invariants: \`processMatch\` must record mtime for EVERY file with host-validated DSN via \`fileHadValidDsn\` flag independent of \`seen.has(raw)\`. \`scanDirectory\` catch MUST return empty \`dirMtimes: {}\`, NOT partial map (would silently bless unvisited dirs); \`ConfigError\` re-throws. + +* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Shell completions (\`\_\_complete\`) set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before any imports, skipping \`createTracedDatabase\` and avoiding \`@sentry/node-core/light\` load (~85ms). Completion timing queued to \`completion\_telemetry\_queue\` SQLite table (~1ms); normal runs drain via \`DELETE FROM ... RETURNING\` and emit as \`Sentry.metrics.distribution\`. Achieves ~60ms dev / ~140ms CI within 200ms e2e budget. - -* **Grep worker pool: binary-transferable matches + streaming dispatch in src/lib/scan/**: Grep worker pool (\`src/lib/scan/worker-pool.ts\` + \`grep-worker.js\`): lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads \`\[pathIdx, lineNum, lineOffset, lineLength]\` + \`linePool\` string, transferred via \`postMessage(msg, \[ints.buffer])\` (~40% faster than structuredClone). Worker imported via \`with { type: 'text' }\` → \`Blob\` + \`URL.createObjectURL\`; \`new Worker(new URL(...))\` HANGS in \`bun build --compile\` binaries. FIFO \`pending\` queue per worker — per-dispatch \`addEventListener\` causes wrong-request resolution. \`ref()\`/\`unref()\` idempotent booleans, NOT refcounted — only unref when \`inflight\` drops to 0; spawn unref'd. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. Track dispatched/failed batches with \`Promise.allSettled\`; throw if all failed so DSN cache doesn't persist false-negatives. + +* **Fuzzy recovery auto-resolves dash/underscore slug mismatches without original-slug fallback**: Display-name project input (contains spaces) skips slug lookup, goes to name-based fuzzy search across four resolution sites: \`resolveProjectBySlug\`, \`resolveOrgProjectTarget\` (project-search), \`org-list.ts#handleProjectSearch\`, \`project/list.ts#handleProjectNotFound\`. \`parseOrgProjectArg\` detects spaces via \`looksLikeDisplayName()\` and sets \`originalSlug\` on \`project-search\`; sites check \`isDisplayName = originalSlug !== undefined\` and skip \`findProjectsBySlug\` (404s on URL-encoded spaces), going directly to \`triageProjectNotFound\` → \`findSimilarProjectsAcrossOrgs\`. \*\*Critical\*\*: recursive fuzzy recovery calls must NOT pass \`originalSlug\` — otherwise the recovered slug also skips lookup, causing infinite skip→empty→not-found loop. - -* **Host-scoped token model: auth.host column + three-layer enforcement**: Host-scoped token model (PR #844): every token bound to issuing host via \`auth.host\` column (schema v16), lazy-migrated from boot-env. Trust established ONLY via \`sentry auth login --url\` or shell-exported \`SENTRY\_HOST\`/\`SENTRY\_URL\` at boot — \`.sentryclirc\` URL never a trust source (mtime-based freshness doesn't work: git clone resets, \`touch -t\` backdates). Three enforcement layers: (1) \`applySentryUrlContext\` throws on URL-arg mismatch; (2) \`applySentryCliRcEnvShim\` throws on rc-url mismatch (auth login/logout bypass via \`skipUrlTrustCheck\`); (3) fetch-layer \`isRequestOriginTrusted\`. Region trust: in-process Set in \`db/regions.ts\`, auto-synced by \`setOrgRegion(s)\`. \`clearTrustedHostState\` must NOT clear login anchor (breaks IAP re-auth). Login refusal scoped to \`--token\`. \`HostScopeError\` (\`src/lib/errors.ts\`) is canonical formatter with overloads \`(message)\` and \`(source, destinationUrl, tokenHost)\`; used by rc-shim, URL-arg, fetch bearer, sntrys\_ claim, OAuth refresh. E2E: pass \`--url ${ctx.serverUrl}\` to \`auth login --token\`; child \`SENTRY\_URL\` alone doesn't anchor. + +* **Project cache is org-scoped with three key formats and three population paths**: \`project\_cache\` SQLite table uses three key shapes: \`{orgId}:{projectId}\` (DSN resolution), \`dsn:{publicKey}\` (DSN without orgId), \`list:{orgSlug}/{projectSlug}\` (batch from API). Helpers: \`getCachedProject\`, \`getCachedProjectByDsnKey\`, \`getCachedProjectsForOrg\` (completions), \`getCachedProjectBySlug\` (queries all three shapes for hot-path slug lookups; used by \`fetchProjectId\` to skip \`GET /projects/{org}/{project}/\`). Population paths: DSN resolution in resolve-target.ts, \`listProjects()\` batch via \`cacheProjectsForOrg\`, \`fetchProjectId\` seeds on API success. Resolution errors use live API via \`findSimilarProjectsAcrossOrgs\` — no cross-org cache search. - -* **isSentrySaasUrl vs isSaaSTrustOrigin: two intentional SaaS checks**: \`src/lib/sentry-urls.ts\` exports two SaaS-detection helpers with intentional split: (1) \`isSentrySaasUrl(url)\` — hostname-only check (\`sentry.io\` or \`\*.sentry.io\`), accepts any protocol/port. Used for routing/UX: custom-headers warning, \`getSentryBaseUrl\`/\`isSelfHosted\`, region resolution skip, telemetry \`is\_self\_hosted\` tag. (2) \`isSaaSTrustOrigin(url)\` — stricter: additionally requires \`https:\` and default port. Used for security decisions: token-host trust comparison, sentryclirc URL trust check, URL-arg trust, login refusal. Rule: hostname-only for routing/UX (don't break users behind TLS-terminating proxies with \`http://sentry.io\`); strict for credential scoping. JSDoc on \`isSentrySaasUrl\` points callers to \`isSaaSTrustOrigin\` for security contexts. Keep both implementations in sync re: hostname matching. + +* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping/auth quirks: (1) Events require org+project (\`/projects/{org}/{project}/events/{id}/\`); issues use legacy global \`/api/0/issues/{id}/\`; traces need only org. Cross-project search via Discover \`/organizations/{org}/events/?query=id:{eventId}\`. (2) \`/users/me/\` returns 403 for OAuth tokens — use \`/auth/\` instead (all token types, control silo). \`getControlSiloUrl()\` routes; \`SentryUserSchema\` uses \`.passthrough()\` since \`/auth/\` only requires \`id\`. (3) Chunk upload endpoint returns camelCase (\`chunkSize\`, \`chunksPerRequest\`, \`maxRequestSize\`, \`hashAlgorithm\`); \`AssembleResponse\` also camelCase — exception to snake\_case convention. - -* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic @ selectors resolve issues dynamically: \`@latest\`, \`@most\_frequent\` in \`parseIssueArg\` detected before \`validateResourceId\` (@ not in forbidden charset). \`SELECTOR\_MAP\` provides case-insensitive matching. \`resolveSelector\` maps to \`IssueSort\` values, calls \`listIssuesPaginated\` with \`perPage: 1\`, \`query: 'is:unresolved'\`. Supports org-prefixed: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through. \`ParsedIssueArg\` union includes \`{ type: 'selector' }\`. + +* **Sentry CLI authenticated fetch architecture with response caching**: Authenticated fetch (\`createAuthenticatedFetch\` in \`src/lib/sentry-client.ts\`): auth headers, 30s \`REQUEST\_TIMEOUT\_MS\`, retry max 2, 401 refresh, span tracing. Dual input: SDK \`Request\` vs \`(url, init)\`. \`buildAttemptFactory\` yields fresh \`(input, init)\` per attempt; \`Request\` clones; \`FormData\`/\`Blob\`/\`URLSearchParams\` pass through. Only bare \`ReadableStream\` needs materialization. Do NOT materialize FormData — strips multipart boundary. Internal aborts tagged \`INTERNAL\_TIMEOUT\_MARKER\` Symbol; last attempt throws \`TimeoutError\`. Per-endpoint \`ENDPOINT\_TIMEOUT\_OVERRIDES\` (e.g. \`/autofix/\` 120s). Response cache: \`http-cache-semantics\` RFC 7234 at \`~/.sentry/cache/responses/\`; GET 2xx only. On 4xx/5xx, \`apiRequestToRegion\` attaches allow-listed response headers to Sentry scope as \`api\_response\_headers\` context. Cache hit invisibility solved via module-level \`lastCacheHitAgeMs\` (set on hit, cleared per-call); \`src/lib/cache-hint.ts\` provides \`formatCacheHint()\`/\`appendCacheHint()\`, wired in \`buildCommand\` only when generator returns \`CommandReturn\` (bare \`return;\` paths skip). - -* **safe-read.ts wraps isRegularFile + Bun.file().text() for FIFO-safe user-path reads**: \`src/lib/safe-read.ts\` \`safeReadFile(path, operation): Promise\\` combines \`isRegularFile()\` + \`Bun.file().text()\` + broad error swallow (FIFO/ENOENT/EACCES/EPERM/EISDIR/ENOTDIR). Sole caller: \`apply-patchset.ts\`. \*\*Do NOT use for committed config loads\*\* — swallows EPERM/EISDIR, making \`chmod 000 .sentryclirc\` manifest as confusing 'no auth token'. For loud permission surfacing (\`tryReadSentryCliRc\`), call \`fs.promises.stat\` directly, gate on \`isFile()\`, catch only ENOENT/EACCES. \`read-files.ts\`/\`workflow-inputs.ts\` use direct stat to reuse one stat for size-gating. Test with real \`mkfifo\` + short timeout as hang detector. + +* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: resolve-target.ts cascade has 5 priority levels: (1) Explicit CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite config defaults, (4) DSN auto-detection, (5) Directory name inference. SENTRY\_PROJECT supports combo notation \`org/project\` — when used, SENTRY\_ORG is ignored. If combo parse fails (e.g. \`org/\`), the entire value is discarded. \`resolveFromEnvVars()\` helper is injected into all four resolution functions. - -* **Sentry SDK uses @sentry/node-core/light instead of @sentry/bun to avoid OTel overhead**: Sentry SDK uses \`@sentry/node-core/light\` instead of \`@sentry/bun\` to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\` to remove ~32 unused exports. Gotcha: \`LightNodeClient\` hardcodes \`runtime: { name: 'node' }\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init (mutable ref). Transport uses Node \`http\` instead of native \`fetch\`. Upstream: getsentry/sentry-javascript#19885, #19886. +### Decision - -* **Sentry token formats: only sntrys\_ embeds host claim, and it's unsigned**: Sentry token formats (verified in getsentry/sentry \`orgauthtoken\_token.py\`): \`sntryu\_\\` (user auth) — no claims; \`sntrys\_\\_\\` (org auth) — \*\*unsigned\*\*, plaintext base64, anyone can forge; \`sntrya\_\`/\`sntryi\_\` — random hex; OAuth — random, no prefix. \`sntrys\_\` payload is a UX hint, NOT verifiable; \`auth.host\` column \[\[019dc168-adb2-7bed-900e-cab5d3716099]] is strictly stronger. \`parseSntrysClaim\` in \`src/lib/token-claims.ts\` requires exactly 2 underscores, base64-decodes, requires \`iat\`, 2 KB cap, fail-open. Two consumers: (1) \`captureEnvTokenHost\` claim-first for \`sntrys\_\`: claim url > \`SENTRY\_HOST\`/\`SENTRY\_URL\` > \`DEFAULT\_SENTRY\_URL\` (defends against layered-CI \`$GITHUB\_ENV\` poisoning); for \`sntryu\_\`/OAuth, env wins (no \`SENTRY\_BOUND\_TOKEN\` protocol — narrow protection, broad UX cost). (2) \`prepareHeaders\` defense-in-depth — refuses bearer attach if request origin doesn't match claim url. + +* **Issue list global limit with fair per-project distribution and representation guarantees**: \`issue list --limit\` is a global total across all detected projects. \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus via cursor resume. \`trimWithProjectGuarantee\` ensures ≥1 issue per project before filling remaining slots. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination, keyed by sorted target fingerprint. - -* **Telemetry opt-out is env-var-only — no persistent preference or DO\_NOT\_TRACK**: Telemetry opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. DB read try/catch wrapped (runs before DB init). Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. \`sentry cli defaults\` uses variadic \`\[key, value?]\`: no args → show all; 1 arg → show key; 2 args → set; \`--clear\` without args → clear all (guarded); \`--clear key\` → clear specific. \`computeTelemetryEffective()\` returns resolved source for display. + +* **Prefer dedicated SQLite tables + migrations over metadata KV for non-trivial caches**: Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches. Schema migrations are cheap — don't shoehorn structured caches into \`metadata\` with dotted-prefix keys. Dedicated tables give clearer schema, proper indexes, simpler bulk-clear, no prefix collisions. \`metadata\` KV is fine for small scalars (defaults.\*, install.\*). Example: \`issue\_org\_cache\` (schema v15) replaced \`metadata\` keys \`issue\_org.{numericId}\`. Migration pattern: bump \`CURRENT\_SCHEMA\_VERSION\`, add \`EXPECTED\_TABLES.foo\`, add \`if (currentVersion < N) db.exec(EXPECTED\_TABLES.foo)\`. HTTP response cache (URL+headers, short TTLs) can't answer structural questions like 'which org owns issue 123?' — use dedicated tables for structural/mapping questions, HTTP cache for content. - -* **Zod schema on OutputConfig enables self-documenting JSON fields in help and SKILL.md**: Zod schema on OutputConfig enables self-documenting JSON fields: List commands register \`schema?: ZodType\` on \`OutputConfig\\`. \`extractSchemaFields()\` produces \`SchemaFieldInfo\[]\` from Zod shapes. \`buildFieldsFlag()\` enriches \`--fields\` brief; \`enrichDocsWithSchema()\` appends fields to \`fullDescription\`. Schema exposed as \`\_\_jsonSchema\` on built commands — \`introspect.ts\` reads it into \`CommandInfo.jsonFields\`, \`help.ts\` and \`generate-skill.ts\` render it. For \`buildOrgListCommand\`/\`dispatchOrgScopedList\`, pass \`schema\` via \`OrgListConfig\`. + +* **Top-level --help stays terse Stricli output, not branded help**: \`sentry --help\` and \`sentry -h\` MUST render Stricli's terse default template, NOT the branded help (Flags + Environment Variables sections). Agents parse \`--help\` output and branding wastes tokens. Branded help is reserved for human discovery paths: \`sentry\` (no-args, via \`defaultCommand: "help"\`) and \`sentry help\`. Do NOT add interception logic in \`src/cli.ts\` to rewrite \`--help\` → \`help\`. TTY/agent detection is not worth the complexity — agents have skills documentation; humans get the footer hint pointing to \`sentry help\`. Subcommand help (e.g. \`sentry issue --help\`) is also left to Stricli for command-specific flag rendering. -### Decision +### Gotcha - -* **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands use \`\ \\` positional pattern (Intent-First Correction UX): target is optional \`org/project\`. Use opportunistic arg swapping with \`log.warn()\` when args are wrong order — when intent is unambiguous, do what they meant. Normalize at command level, keep parsers pure. Model after \`gh\` CLI. Exception: \`auth\` uses \`defaultCommand: "status"\` (no viewable entity). Routes without defaults: \`cli\`, \`sourcemap\`, \`repo\`, \`team\`, \`trial\`, \`release\`, \`dashboard/widget\`. + +* **@sentry/api SDK can return non-array data for empty/edge responses**: \`@sentry/api\` SDK (in \`node\_modules/@sentry/api/dist/index.js\`) returns \`data = {}\` (not \`\[]\`) when response body is empty, has \`Content-Length: 0\`, or status 204; and returns a \`ReadableStream\` when \`Content-Type\` is missing. \`unwrapResult\` from \`src/lib/api/infrastructure.ts\` returns \`data\` as-is, and \`as unknown as SentryX\[]\` casts silently lie. Always guard array-typed SDK results with \`Array.isArray(data)\` before \`.map()\` — applied in \`listOrganizationsInRegion\` (CLI-1CQ). Self-hosted instances behind reverse proxies (nginx, Cloudflare, WAFs) commonly trigger this by stripping bodies or wrapping responses. Throw a descriptive \`ApiError\` on mismatch rather than letting \`TypeError: x.map is not a function\` bubble up minified. - -* **Sentry-derived terminal color palette tuned for dual-background contrast**: Terminal color palette tuned for dual-background contrast: 10-color chart palette derived from Sentry's categorical hues (\`static/app/utils/theme/scraps/tokens/color.tsx\`), adjusted to mid-luminance for ≥3:1 contrast on both dark and light backgrounds. Adjustments: orange #FF9838→#C06F20, green #67C800→#3D8F09, yellow #FFD00E→#9E8B18, purple #5D3EB2→#8B6AC8, indigo #50219C→#7B50D0; blurple/pink/magenta unchanged; teal #228A83 added. Hex preferred over ANSI 16-color for guaranteed contrast. + +* **Bun bytecode: true crashes esbuild→compile ESM bundles (Bun 1.3.11)**: Bun build flags for compiled CLI (\`script/build.ts\`): (1) Do NOT enable \`bytecode: true\` with esbuild→\`Bun.build({ compile })\` pipeline. Still broken on Bun 1.3.13 — crashes \`TypeError: Expected CommonJS module to have a function wrapper\` at entry.instantiate (esbuild emits ESM; bytecode loader mis-caches as CJS). Exit 0, no output. Upstream: oven-sh/bun#21097, #23490. (2) Pass \`autoloadDotenv: false\` and \`autoloadBunfig: false\` — otherwise user's \`.env\`/\`bunfig.toml\` silently injects into \`process.env\` (e.g. Next.js \`.env.local\` could override stored OAuth token). Shell env vars still work; suggest direnv for dir-scoped vars. -### Gotcha + +* **dist/bin.cjs runtime Node version check must match engines.node**: \`engines.node >=22.12\` matches Astro 6 floor. CI builds matrix \`\["22","24"]\`; docs jobs pin \`actions/setup-node@v6\` with \`node-version: "24"\` after \`setup-bun\`. The npm package's \`dist/bin.cjs\` (from \`script/bundle.ts\`) contains an inline Node guard that MUST match \`engines.node\`. Simple \`parseInt(process.versions.node) < 22\` misses 22.0.0–22.11.x — use explicit major+minor: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12)){...}\`. When bumping, update BIN\_WRAPPER string AND error message in lockstep. Without \`engine-strict=true\`, npm only warns — the runtime guard is real enforcement. - -* **AuthError constructor takes reason first, message second**: \`AuthError(reason: AuthErrorReason, message?: string)\` where \`AuthErrorReason\` is \`"not\_authenticated" | "expired" | "invalid"\`. Easy to accidentally swap args as \`new AuthError("Token expired", "expired")\` — the string \`"Token expired"\` gets assigned as \`reason\` (invalid enum value). Tests aren't type-checked (tsconfig excludes them), so TypeScript won't catch this. Correct: \`new AuthError("expired", "Token expired")\`. Default messages exist for each reason, so the second arg is often unnecessary. + +* **Making clearAuth() async breaks model-based tests — use non-async Promise\ return instead**: Making \`clearAuth()\` \`async\` breaks fast-check model-based tests — real async yields during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Keep non-async; return \`clearResponseCache().catch(...)\` directly. Model-based tests also need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. - -* **Biome noMisplacedAssertion fires on test-helper functions; use inline biome-ignore**: Biome's \`lint/suspicious/noMisplacedAssertion\` rule flags \`expect()\` calls outside \`test()\`/\`it()\` bodies, including in named helper functions used by multiple tests (e.g. \`expectTokenStored(spy, token)\`). File-level \`biome-ignore-all\` doesn't suppress this rule — must use individual \`// biome-ignore lint/suspicious/noMisplacedAssertion: \\` directly above each \`expect()\` line in the helper. Tests aren't type-checked but they ARE lint-checked, so this catches code that passes \`bun test\` but fails \`bun run lint\`. + +* **script/generate-api-schema.ts regex is brittle against SDK bundler output changes**: \`script/generate-api-schema.ts\` parses \`node\_modules/@sentry/api/dist/index.js\` with a regex (\`/var (\w+) = \\(options\S\*\\) => \\(options\S\*client \\?\\? client\\)\\.(\w+)\\(/g\`) to map SDK function names to URL+method pairs, producing \`src/generated/api-schema.json\`. If the SDK changes its generator's bundle format (e.g., switches to \`const\`, arrow vs function, different client-selection pattern), schema generation silently produces empty \`fn\` fields. When bumping \`@sentry/api\`, verify \`sentry schema\` output still populates function names. \`src/generated/api-schema.json\` is gitignored — regenerates on every dev/build/typecheck via \`bun run generate:schema\`. - -* **GET response cache bypasses fetch wrapper across tests**: \`sentry-client.ts::createAuthenticatedFetch\` checks the response cache BEFORE calling fetch for GET requests. Tests that mock \`globalThis.fetch\` and assert call counts will see 0 calls if a prior test cached the same URL — the cached response is served without invoking the wrapper. Fix in test \`beforeEach\`: \`import('./response-cache.js')\` then call \`resetCacheState()\` + \`disableResponseCache()\`. Pair with \`resetAuthenticatedFetch()\` if cached fetch instance is also stale. Symptom: \`expect(fetchCalls).toHaveLength(1)\` fails with \`Received length: 0\` only when run after another test hitting the same URL; passes in isolation. + +* **Source Map v3 spec allows null entries in sources array**: The Source Map v3 spec allows \`null\` entries in the \`sources\` array, and bundlers like esbuild actually produce them. Any code iterating over \`sources\` and calling string methods (e.g., \`.replaceAll()\`) must guard against null: \`map.sources.map((s) => typeof s === "string" ? s.replaceAll("\\\\", "/") : s)\`. Without the guard, \`null.replaceAll()\` throws \`TypeError\`. This applies to \`src/lib/sourcemap/debug-id.ts\` and any future sourcemap manipulation code. - -* **Node polyfill in script/node-polyfills.ts lacks Bun.file().stat() — use node:fs/promises stat instead**: \`script/node-polyfills.ts\` shims Bun APIs for npm (Node) distribution but is INCOMPLETE — \`Bun.file(path)\` only has \`size\`, \`lastModified\`, \`exists()\`, \`text()\`, \`json()\`, \`stat()\`; NOT \`.arrayBuffer()\`, \`.stream()\`, etc. Also no \`Bun.$\` shim. Tests run under Bun natively and never exercise the polyfill, so missing shims ship undetected (CLI-1EA/1EB: \`Bun.file().stat()\` regression, 400+ events). Prefer \`node:fs/promises\` directly for file ops; \`execSync\` from \`node:child\_process\` for shell. When extending polyfill, alias Node functions via \`bind\` not wrapper closures. Mirror polyfill tests to \`test/lib/\` — \`test:unit\` globs are narrow (\`test/lib test/commands test/types\`); tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` are NOT picked up by CI. + +* **Starlight 0.33+ route data moved from Astro.props to Astro.locals.starlightRoute**: Starlight 0.33+ / Astro 6 docs migration: (1) Route data moved from \`Astro.props\` to \`Astro.locals.starlightRoute\` — old \`Astro.props.sidebar\` is \`undefined\`. Field rename: \`slug\` → \`id\`. Import types via \`@astrojs/starlight/route-data\`. Built-in children (SiteTitle, Search, SocialIcons) take no props. \`starlight.social\` is array-form. (2) Content collections must migrate to Content Layer API: rename \`src/content/config.ts\` → \`src/content.config.ts\`, use \`docsLoader()\` + \`docsSchema()\`. Landing-page detection: \`id === ""\` (\`normalizeIndexSlug\` maps \`"index"\` → \`""\`). - -* **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable in Bun — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds inherited via redirects like \`exec … \ -* **runInteractiveLogin swallows errors and sets process.exitCode = 1**: \`runInteractiveLogin\` in \`src/lib/interactive-login.ts\` catches OAuth flow errors internally (device-code fetch failures, timeout, etc.) and returns falsy on failure. The login command then sets \`process.exitCode = 1\` and returns normally — the wrapped command function resolves, NOT rejects. Tests that mock fetch to throw and expect \`rejects.toThrow()\` will fail with \`resolved: Promise { \ }\`. Assert behavior via fetch-call inspection (\`fetchCalls.length > 0\`, header content) instead. \`requestDeviceCode\` requires \`SENTRY\_CLIENT\_ID\` env var — unset in tests means it throws \`ConfigError\` before any fetch fires. + +* **Bun global installs use .bun path segment for detection**: Bun global installs place scripts under \`~/.bun/install/global/node\_modules/\`. In \`detectPackageManagerFromPath()\`, check \`segments.includes('.bun')\` before npm fallback. Order: \`.pnpm\` → pnpm, \`.bun\` → bun, other \`node\_modules\` → npm. Yarn classic shares npm layout so falls through — acceptable because path detection is \*\*fallback\*\* after subprocess calls (which identify yarn correctly). Path detection must NOT override stored DB info, only serve as fallback when subprocess fails (e.g., Windows \`.cmd\` ENOENT). - -* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli flag parsing traps: (1) Unknown \`--flag\`s rejected — global flags parsed in \`bin.ts\` MUST be spliced from argv (check both \`--flag value\` and \`--flag=value\`). (2) \`FLAG\_NAME\_PATTERN\` requires 2+ chars after \`--\`; single-char flags like \`--x\` silently become positionals — use aliases (\`-x\` → longer name). Bit \`dashboard widget --x\`/\`--y\`. (3) \`FlagDef.hidden\` is propagated by \`extractFlags\` so \`generateCommandDoc\` filters hidden flags alongside \`help\`/\`helpAll\`; hidden \`--log-level\`/\`--verbose\` appear only in global options docs. + +* **Evict-then-read pattern: return cacheEvicted flag from helpers that clear cache on 404**: When a helper function transparently evicts a stale cache entry on 404 and falls back to an unscoped call, callers holding the now-stale cached value will let it win \`??\` chains. Fix: helper must return \`{ result, cacheEvicted }\` so callers compute \`effectiveCachedValue = cacheEvicted ? null : cachedValue\` before the \`??\` fallback, and re-cache the freshly-derived value. Applied in \`fetchIssueByNumericId\` in \`src/commands/issue/utils.ts\` — both \`resolveNumericIssue\` and \`resolveShareIssue\` consume the flag. A local cached variable outliving its DB entry is the common shape of this bug; always audit post-eviction read paths. - -* **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\`→skip); per-line verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). (3) Extractor \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (4) Wake-latch race: naive \`let notify=null; await new Promise(r=>notify=r)\` loses signals — use latched \`pendingWake\` flag. (5) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (6) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator; drain uncapped, set \`truncated=true\`. + +* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must be wrapped in try-catch so a broken/read-only DB doesn't crash a command whose primary operation succeeded. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. In login.ts, \`getCurrentUser()\` failure after token save must not block auth — log warning, continue. In upgrade.ts, \`setInstallInfo\` after legacy detection is guarded same way. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard (indicates invalid token). This is enforced by BugBot reviews — any \`setInstallInfo\`/\`setUserInfo\` call outside setup.ts's \`bestEffort()\` wrapper needs its own try-catch. -### Pattern + +* **Sentry CLI command docs are auto-generated from Stricli route tree with CI freshness check**: Sentry CLI command docs are auto-generated from Stricli route tree: Docs in \`docs/src/content/docs/commands/\*.md\` and skill files in \`plugins/sentry-cli/skills/sentry-cli/references/\*.md\` are generated via \`bun run generate:docs\`. Content between \`\\` markers is regenerated; hand-written examples go in \`docs/src/fragments/commands/\`. CI checks \`check:command-docs\` and \`check:skill\` fail if stale. Run generators after changing command parameters/flags/docs. + + +* **Stricli buildCommand output config injects json flag into func params**: Stricli command gotchas: (1) In \`func()\` handlers use \`this.stdout\`/\`this.stderr\` directly — NOT \`this.process.stdout\`. \`SentryContext\` has \`process\` and \`stdout\`/\`stderr\` as separate top-level properties; test mocks omit full \`process\` so \`this.process.stdout\` throws \`TypeError\` at runtime (TS doesn't catch). (2) \`output: { json: true, human: formatFn }\` auto-injects \`--json\`/\`--fields\` flags — type flags explicitly (\`flags: { json?: boolean }\`). Commands with interactive side effects (prompts, QR codes) should check \`flags.json\` and skip. (3) Route maps with \`defaultCommand\` blend the default command's flags into subcommand completions — completion tests must track \`hasDefaultCommand\` and skip strict subcommand-matching. + + +* **Token-type classification via literal prefix match (classifySentryToken)**: Token-type classification via literal prefix match: \`src/lib/token-type.ts\` \`classifySentryToken(token)\` returns \`'org-auth-token'\` (\`sntrys\_\` prefix), \`'user-auth-token'\` (\`sntryu\_\` prefix), or \`'oauth-or-legacy'\`. Case-sensitive \`startsWith\` matches Sentry backend's \`SENTRY\_ORG\_AUTH\_TOKEN\_PREFIX\`. Use to short-circuit commands where a token type is semantically invalid (e.g. \`whoami\` with org token — \`/auth/\` rejects \`sntrys\_\`) before a confusing API failure. \`getAuthToken()\` from \`db/auth\` returns the effective token (env + DB fallback). - -* **Test helpers for host-scoping security tests**: Test helpers for host-scoping security tests: \`test/helpers.ts\` provides shared utilities. \`useEnvSandbox(keys)\` registers beforeEach/afterEach to save+clear+restore env keys (do NOT use in tests that depend on preload's \`SENTRY\_AUTH\_TOKEN\`, e.g. \`sentryclirc-url-poison.test.ts\` calls \`getActiveTokenHost()\` which needs a token). \`resetHostScopingState()\` bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\` (always reset together). \`mintSntrysToken(payload)\` produces \`sntrys\_\\_\\` test tokens matching server format (rstrip \`=\`). \`extractFetchUrl(input)\` for fetch-mock assertions. \`useTestConfigDir\` \[\[019dc573-d853-735a-aeb5-68ff49afe037]] handles config-dir isolation separately. +### Preference - -* **Tests calling setAuthToken must pass {host} matching the mock URL**: Host-scoping test gotchas \[\[019dc168-adb2-7bed-900e-cab5d3716099]]: (1) Tests mocking \`fetch\` with non-SaaS URLs and calling \`setAuthToken(token, ttl)\` without \`{host}\` fail with \`HostScopeError\` — token defaults to SaaS via \`captureEnvTokenHost\`. Fix: \`setAuthToken("fake", 3600, { host: "https://sentry.example.com" })\`. SaaS URLs work via equivalence. (2) For \`assertRcUrlTrusted\` tests, env-token-host snapshot must lock BEFORE rc shim mutates env: sequence is \`resetEnvTokenHostForTesting()\` → delete \`SENTRY\_HOST\`/\`SENTRY\_URL\` → \`captureEnvTokenHost()\` → \`applySentryCliRcEnvShim(testDir)\` → \`assertRcUrlTrusted(testDir)\`. Without explicit capture, lazy auto-capture reads poisoned \`SENTRY\_URL\`. (3) E2E fixture \`createE2EContext\` parent must \`setAuthToken(token, ttl, {host: serverUrl})\` matching child's \`SENTRY\_URL\`; multi-region tests need \`registerTrustedRegionUrls\` during \`listOrganizationsUncached\` before fan-out (regional mocks on different localhost ports, no SaaS equivalence). Symptom: \`HostScopeError: Refusing to send credentials\`. + +* **PR workflow: address review comments, resolve threads, wait for CI**: PR workflow: (1) wait for CI; (2) check unresolved comments via \`gh api repos/.../pulls/N/comments\`; (3) fix in follow-up commits (NEVER amend a pushed commit without explicit user request + force push); (4) reply explaining fix; (5) resolve thread via \`gh api graphql resolveReviewThread\`; (6) push + re-check CI. BugBot/Seer/Warden/Cursor post new comments per-commit and often catch bugs in fix commits — re-check after each push. Dispatch a subagent review before declaring merge-ready. Branches: \`fix/\*\` or \`feat/\*\`. Style: \`Array.from(set)\` over spreads; 'allowlist' not 'whitelist'; \`arr.at(-1)\` over index math. Reviewer questions may be inquiries — confirm intent before changing. After reverts/changes affecting PR scope, update the PR description to match. diff --git a/docs/src/content/docs/self-hosted.md b/docs/src/content/docs/self-hosted.md index eb9c23d18..1a06d2287 100644 --- a/docs/src/content/docs/self-hosted.md +++ b/docs/src/content/docs/self-hosted.md @@ -77,6 +77,8 @@ If you pass a self-hosted Sentry URL as a command argument (e.g., an issue or ev | `SENTRY_FORCE_ENV_TOKEN` | Force env token over stored OAuth token | | `SENTRY_ORG` | Default organization slug | | `SENTRY_PROJECT` | Default project slug (supports `org/project` format) | +| `NODE_EXTRA_CA_CERTS` | Path to PEM file with additional CA certificates (for corporate proxies) | +| `SSL_CERT_FILE` | Fallback CA certificate bundle path | See [Configuration](./configuration/) for the full environment variable reference. diff --git a/script/generate-docs-sections.ts b/script/generate-docs-sections.ts index 6ae6902fa..0ea317c26 100644 --- a/script/generate-docs-sections.ts +++ b/script/generate-docs-sections.ts @@ -362,6 +362,11 @@ const SELF_HOSTED_TABLE_ENTRIES: readonly [string, string][] = [ ["SENTRY_FORCE_ENV_TOKEN", "Force env token over stored OAuth token"], ["SENTRY_ORG", "Default organization slug"], ["SENTRY_PROJECT", "Default project slug (supports `org/project` format)"], + [ + "NODE_EXTRA_CA_CERTS", + "Path to PEM file with additional CA certificates (for corporate proxies)", + ], + ["SSL_CERT_FILE", "Fallback CA certificate bundle path"], ]; /** diff --git a/src/commands/cli/defaults.ts b/src/commands/cli/defaults.ts index 9bca46424..4ce0a4be9 100644 --- a/src/commands/cli/defaults.ts +++ b/src/commands/cli/defaults.ts @@ -3,11 +3,12 @@ * * View and manage persistent CLI default settings. * - * Supports four defaults: + * Supports these defaults: * - `org` / `organization` — default organization slug * - `project` — default project slug * - `telemetry` — telemetry preference (on/off) * - `url` — Sentry instance URL (for self-hosted) + * - `ca-cert` — path to PEM file with custom CA certificates */ import type { SentryContext } from "../../context.js"; @@ -18,11 +19,13 @@ import { clearAllDefaults, type DefaultsState, getAllDefaults, + getDefaultCaCert, getDefaultHeaders, getDefaultOrganization, getDefaultProject, getDefaultUrl, getTelemetryPreference, + setDefaultCaCert, setDefaultHeaders, setDefaultOrganization, setDefaultProject, @@ -47,7 +50,13 @@ import { computeTelemetryEffective } from "../../lib/telemetry.js"; // --------------------------------------------------------------------------- /** Canonical key names matching DefaultsState fields */ -type DefaultKey = "organization" | "project" | "telemetry" | "url" | "headers"; +type DefaultKey = + | "organization" + | "project" + | "telemetry" + | "url" + | "headers" + | "ca-cert"; /** Handler for reading, writing, and clearing a single default */ type DefaultHandler = { @@ -131,6 +140,38 @@ const DEFAULTS_REGISTRY: Record = { }, clear: () => setDefaultHeaders(null), }, + "ca-cert": { + get: getDefaultCaCert, + set: (value) => { + const trimmed = value.trim(); + if (!trimmed) { + throw new ValidationError( + "CA certificate path cannot be empty.", + "ca-cert" + ); + } + const { resolve } = require("node:path") as typeof import("node:path"); + const resolved = resolve(trimmed); + let content: string; + try { + const { readFileSync } = require("node:fs") as typeof import("node:fs"); + content = readFileSync(resolved, "utf-8"); + } catch { + throw new ValidationError( + `CA certificate file not found or not readable: ${resolved}`, + "ca-cert" + ); + } + if (!content.includes("-----BEGIN CERTIFICATE-----")) { + throw new ValidationError( + "File does not contain PEM certificate data (expected -----BEGIN CERTIFICATE-----).", + "ca-cert" + ); + } + setDefaultCaCert(resolved); + }, + clear: () => setDefaultCaCert(null), + }, }; // --------------------------------------------------------------------------- @@ -140,6 +181,9 @@ const DEFAULTS_REGISTRY: Record = { /** Shorthand aliases for canonical keys (e.g., "org" → "organization") */ const KEY_ALIASES: Partial> = { org: "organization", + ca: "ca-cert", + cert: "ca-cert", + "ca-certs": "ca-cert", }; /** Resolve a user-provided key string to a canonical key, or null if unknown */ @@ -196,6 +240,7 @@ export const defaultsCommand = buildCommand({ "sentry cli defaults telemetry off # Disable telemetry\n" + "sentry cli defaults url https://... # Set Sentry URL (self-hosted)\n" + "sentry cli defaults headers 'X-IAP: t' # Set custom headers (self-hosted)\n" + + "sentry cli defaults ca-cert /path/to/ca.pem # Trust a custom CA certificate\n" + "sentry cli defaults org --clear # Clear a specific default\n" + "sentry cli defaults --clear --yes # Clear all defaults\n" + "```\n\n" + @@ -206,7 +251,8 @@ export const defaultsCommand = buildCommand({ "| `project` | Default project slug |\n" + "| `telemetry` | Telemetry preference (on/off, yes/no, true/false, 1/0) |\n" + "| `url` | Sentry instance URL (for self-hosted installations) |\n" + - "| `headers` | Custom HTTP headers for self-hosted proxies (semicolon-separated `Name: Value`) |", + "| `headers` | Custom HTTP headers for self-hosted proxies (semicolon-separated `Name: Value`) |\n" + + "| `ca-cert` | Path to PEM file with custom CA certificates (for corporate proxies) |", }, output: { human: formatDefaultsResult, @@ -260,7 +306,7 @@ export const defaultsCommand = buildCommand({ guardNonInteractive(flags); if (!isConfirmationBypassed(flags)) { const confirmed = await log.prompt( - "This will clear all defaults (organization, project, telemetry, URL, headers). Continue?", + "This will clear all defaults (organization, project, telemetry, URL, headers, ca-cert). Continue?", { type: "confirm" } ); if (confirmed !== true) { diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts new file mode 100644 index 000000000..428c63679 --- /dev/null +++ b/src/lib/custom-ca.ts @@ -0,0 +1,191 @@ +/** + * Custom CA certificate loading for corporate TLS proxies. + * + * Reads CA bundles from (in priority order): + * 1. `sentry cli defaults ca-cert` (stored path in SQLite) + * 2. `NODE_EXTRA_CA_CERTS` env var + * 3. `SSL_CERT_FILE` env var + * + * Returns a `tls` options object for Bun's `fetch()`. On the Node.js npm + * distribution, Node natively honors `NODE_EXTRA_CA_CERTS` so the extra + * `tls.ca` option is harmless (ignored by Node's fetch). + * + * Security model: When the CA source is an env var (not a stored default) + * AND the target is SaaS (`*.sentry.io`), a one-time warning is logged. + * `sentry cli defaults ca-cert` silences the warning — the user has + * explicitly acknowledged the custom CA. See CLI-1K6 plan for the full + * threat model discussion. + */ + +import { getDefaultCaCert } from "./db/defaults.js"; +import { getEnv } from "./env.js"; +import { logger } from "./logger.js"; +import { isSentrySaasUrl } from "./sentry-urls.js"; + +const log = logger.withTag("tls"); + +/** Where the loaded CA came from */ +export type CaSource = "default" | "env" | "none"; + +/** Cached resolved state — computed once per process */ +let resolved: { tls: { ca: string } } | undefined; +let resolvedSource: CaSource = "none"; +let hasResolved = false; +let warnedSaas = false; + +/** + * Attempt to read a PEM file. Returns the file contents on success, + * or undefined if the file doesn't exist or can't be read. + * Never throws — a missing CA file shouldn't crash the CLI. + */ +async function tryReadPem(path: string): Promise { + try { + const file = Bun.file(path); + if (!(await file.exists())) { + log.warn(`CA certificate file not found: ${path}`); + return; + } + const content = await file.text(); + if (!content.includes("-----BEGIN")) { + log.warn( + `CA certificate file does not appear to contain PEM data: ${path}` + ); + return; + } + return content; + } catch { + log.warn(`Failed to read CA certificate file: ${path}`); + return; + } +} + +/** + * Resolve custom CA certificates. Cached after first call. + * + * Priority: + * 1. Stored default (`sentry cli defaults ca-cert`) + * 2. `NODE_EXTRA_CA_CERTS` env var + * 3. `SSL_CERT_FILE` env var + */ +async function resolve(): Promise { + if (hasResolved) { + return; + } + hasResolved = true; + + // 1. Stored default — highest priority, silences SaaS warning + const storedPath = getDefaultCaCert(); + if (storedPath) { + const pem = await tryReadPem(storedPath); + if (pem) { + resolved = { tls: { ca: pem } }; + resolvedSource = "default"; + log.debug(`Loaded CA certificates from stored default: ${storedPath}`); + return; + } + // Stored path is stale/invalid — fall through to env vars + } + + // 2. NODE_EXTRA_CA_CERTS + const env = getEnv(); + const extraCerts = env.NODE_EXTRA_CA_CERTS?.trim(); + if (extraCerts) { + const pem = await tryReadPem(extraCerts); + if (pem) { + resolved = { tls: { ca: pem } }; + resolvedSource = "env"; + log.debug( + `Loaded CA certificates from NODE_EXTRA_CA_CERTS: ${extraCerts}` + ); + return; + } + } + + // 3. SSL_CERT_FILE + const sslCertFile = env.SSL_CERT_FILE?.trim(); + if (sslCertFile) { + const pem = await tryReadPem(sslCertFile); + if (pem) { + resolved = { tls: { ca: pem } }; + resolvedSource = "env"; + log.debug(`Loaded CA certificates from SSL_CERT_FILE: ${sslCertFile}`); + return; + } + } +} + +/** + * Get the `tls` options to spread into Bun's `fetch()` call. + * Returns undefined when no custom CAs are configured. + */ +export async function getCustomTlsOptions(): Promise< + { tls: { ca: string } } | undefined +> { + await resolve(); + return resolved; +} + +/** Get the source of the loaded CA certificates. */ +export function getCustomCaSource(): CaSource { + return resolvedSource; +} + +/** + * Log a one-time warning when env-sourced CAs are used for SaaS targets. + * + * Stored defaults (via `sentry cli defaults ca-cert`) are treated as + * explicit user acknowledgment and do NOT trigger this warning. + */ +export function warnIfSaasWithEnvCa(targetUrl: string): void { + if (warnedSaas || resolvedSource !== "env") { + return; + } + if (!isSentrySaasUrl(targetUrl)) { + return; + } + warnedSaas = true; + + const envVar = getEnv().NODE_EXTRA_CA_CERTS?.trim() + ? "NODE_EXTRA_CA_CERTS" + : "SSL_CERT_FILE"; + + log.warn( + `Using custom CA certificates from ${envVar} for sentry.io connections.\n` + + " If you intended this (e.g. corporate proxy), silence this warning:\n" + + " sentry cli defaults ca-cert /path/to/cert.pem" + ); +} + +/** + * TLS certificate error patterns from BoringSSL (Bun), OpenSSL (Node), and + * common TLS libraries. These are deterministic — retrying won't help. + */ +const TLS_ERROR_PATTERNS = [ + "unable to get local issuer certificate", + "unable to verify the first certificate", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "CERT_HAS_EXPIRED", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "ERR_TLS_CERT_ALTNAME_INVALID", +] as const; + +/** Check if an error is a TLS certificate verification failure. */ +export function isTlsCertError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + const msg = error.message; + return TLS_ERROR_PATTERNS.some((pattern) => msg.includes(pattern)); +} + +/** + * Reset all cached state. Test-only — not exported from the public API. + * @internal + */ +export function __resetForTests(): void { + resolved = undefined; + resolvedSource = "none"; + hasResolved = false; + warnedSaas = false; +} diff --git a/src/lib/db/defaults.ts b/src/lib/db/defaults.ts index 542f8b5c9..281e542d1 100644 --- a/src/lib/db/defaults.ts +++ b/src/lib/db/defaults.ts @@ -16,6 +16,7 @@ const DEFAULTS_PROJECT = "defaults.project"; const DEFAULTS_TELEMETRY = "defaults.telemetry"; const DEFAULTS_URL = "defaults.url"; const DEFAULTS_HEADERS = "defaults.headers"; +const DEFAULTS_CA_CERT = "defaults.ca-cert"; /** All metadata keys used for defaults (for bulk operations) */ const ALL_DEFAULTS_KEYS = [ @@ -24,6 +25,7 @@ const ALL_DEFAULTS_KEYS = [ DEFAULTS_TELEMETRY, DEFAULTS_URL, DEFAULTS_HEADERS, + DEFAULTS_CA_CERT, ]; /** State of all persistent defaults */ @@ -38,6 +40,8 @@ export type DefaultsState = { url: string | null; /** Custom HTTP headers for self-hosted proxy auth, or null if unset */ headers: string | null; + /** Path to a PEM file with custom CA certificates, or null if unset */ + "ca-cert": string | null; }; /** Parse a raw telemetry metadata value to a typed "on" | "off" | null. */ @@ -105,6 +109,16 @@ export function getDefaultHeaders(): string | null { return m.get(DEFAULTS_HEADERS) ?? null; } +/** + * Get the stored CA certificate file path, or null if not set. + * Used by `custom-ca.ts` as the highest-priority CA source. + */ +export function getDefaultCaCert(): string | null { + const db = getDatabase(); + const m = getMetadata(db, [DEFAULTS_CA_CERT]); + return m.get(DEFAULTS_CA_CERT) ?? null; +} + /** * Get all persistent defaults as a structured object. * Used by the `sentry cli defaults` show mode and JSON output. @@ -119,6 +133,7 @@ export function getAllDefaults(): DefaultsState { telemetry: parseTelemetryValue(telVal), url: m.get(DEFAULTS_URL) ?? null, headers: m.get(DEFAULTS_HEADERS) ?? null, + "ca-cert": m.get(DEFAULTS_CA_CERT) ?? null, }; } @@ -182,6 +197,19 @@ export function setDefaultHeaders(value: string | null): void { } } +/** + * Set or clear the stored CA certificate file path. Pass `null` to clear. + * The path should point to a PEM file containing CA certificates. + */ +export function setDefaultCaCert(path: string | null): void { + const db = getDatabase(); + if (path === null) { + clearMetadata(db, [DEFAULTS_CA_CERT]); + } else { + setMetadata(db, { [DEFAULTS_CA_CERT]: path }); + } +} + // --------------------------------------------------------------------------- // Bulk operations // --------------------------------------------------------------------------- diff --git a/src/lib/env-registry.ts b/src/lib/env-registry.ts index 7881faaaa..9854eb732 100644 --- a/src/lib/env-registry.ts +++ b/src/lib/env-registry.ts @@ -175,6 +175,24 @@ export const ENV_VAR_REGISTRY: readonly EnvVarEntry[] = [ example: "1", installOnly: true, }, + // -- TLS / Certificates -- + { + name: "NODE_EXTRA_CA_CERTS", + description: + "Path to a PEM file containing additional CA certificates to trust. " + + "Useful behind corporate TLS-intercepting proxies (Zscaler, Netskope, etc.).\n\n" + + "Can also be set persistently with `sentry cli defaults ca-cert`.", + example: "/path/to/corporate-ca.pem", + selfHosted: true, + }, + { + name: "SSL_CERT_FILE", + description: + "Fallback path to a PEM CA certificate bundle. " + + "Read when `NODE_EXTRA_CA_CERTS` is not set.", + example: "/etc/ssl/certs/ca-certificates.crt", + selfHosted: true, + }, // -- Display -- { name: "SENTRY_PLAIN_OUTPUT", diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index fbe89066e..6e91d47e1 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -12,6 +12,11 @@ import { TokenResponseSchema, } from "../types/index.js"; import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "./constants.js"; +import { + getCustomTlsOptions, + isTlsCertError, + warnIfSaasWithEnvCa, +} from "./custom-ca.js"; import { applyCustomHeaders } from "./custom-headers.js"; import { setAuthToken } from "./db/auth.js"; import { getEnv } from "./env.js"; @@ -101,13 +106,34 @@ async function fetchWithConnectionError( const effectiveInit: RequestInit = { ...init, headers: merged }; try { - return await fetch(url, effectiveInit); + const customTls = await getCustomTlsOptions(); + if (customTls) { + warnIfSaasWithEnvCa(url); + } + + return await fetch(url, { ...effectiveInit, ...customTls }); } catch (error) { + if (!(error instanceof Error)) { + throw error; + } + + // TLS certificate errors — give actionable guidance + if (isTlsCertError(error)) { + throw new ApiError( + `TLS certificate error connecting to ${getSentryUrl()}`, + 0, + `TLS certificate verification failed: ${error.message}\n\n` + + " To fix this, point the CLI to your CA certificate bundle:\n" + + " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + + " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + + " export NODE_EXTRA_CA_CERTS=/path/to/corporate-ca.pem" + ); + } + const isConnectionError = - error instanceof Error && - (error.message.includes("ECONNREFUSED") || - error.message.includes("fetch failed") || - error.message.includes("network")); + error.message.includes("ECONNREFUSED") || + error.message.includes("fetch failed") || + error.message.includes("network"); if (isConnectionError) { throw new ApiError( diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 84e240ea3..21052f31e 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -16,9 +16,15 @@ import { getConfiguredSentryUrl, getUserAgent, } from "./constants.js"; +import { + getCustomCaSource, + getCustomTlsOptions, + isTlsCertError, + warnIfSaasWithEnvCa, +} from "./custom-ca.js"; import { applyCustomHeaders } from "./custom-headers.js"; import { getAuthToken, refreshToken } from "./db/auth.js"; -import { HostScopeError, TimeoutError } from "./errors.js"; +import { ApiError, HostScopeError, TimeoutError } from "./errors.js"; import { logger } from "./logger.js"; import { clearLastCacheHitAge, @@ -250,10 +256,19 @@ async function fetchWithTimeout({ linkAbortSignal(externalSignal, controller); try { + // Spread custom TLS options (CA certs for corporate proxies). + // On Bun, this passes `tls: { ca }` to fetch(); on Node, the + // extra property is harmless (Node honors NODE_EXTRA_CA_CERTS natively). + const customTls = await getCustomTlsOptions(); + if (customTls) { + warnIfSaasWithEnvCa(extractFullUrl(input)); + } + return await fetch(input, { ...init, headers, signal: controller.signal, + ...customTls, }); } catch (error) { if (timedOut) { @@ -297,19 +312,62 @@ async function handleResponse( return { action: "done", response }; } +/** + * Build a user-friendly error message for TLS certificate failures. + * Content varies based on whether custom CAs are already loaded. + */ +function buildTlsErrorMessage(error: Error, hasCustomCa: boolean): string { + const cause = error.message; + + if (hasCustomCa) { + return ( + `TLS certificate verification failed: ${cause}\n\n` + + " Custom CA certificates are loaded but verification still failed.\n" + + " The certificate file may not contain the correct CA for this server.\n\n" + + " Check that your CA bundle includes the certificate authority used by\n" + + " your network proxy or Sentry instance." + ); + } + + return ( + `TLS certificate verification failed: ${cause}\n\n` + + " This usually means your network uses a TLS-intercepting proxy\n" + + " (corporate firewall, VPN) with a private certificate authority.\n\n" + + " To fix this, point the CLI to your CA certificate bundle:\n" + + " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + + " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + + " export NODE_EXTRA_CA_CERTS=/path/to/corporate-ca.pem" + ); +} + /** * Decide what to do with a fetch error. User aborts and an internal - * timeout on the last attempt throw immediately; other errors retry - * until the last attempt, then propagate. + * timeout on the last attempt throw immediately; TLS cert errors throw + * immediately with actionable guidance (retrying is pointless); other + * errors retry until the last attempt, then propagate. */ function handleFetchError( error: unknown, signal: AbortSignal | undefined | null, - isLastAttempt: boolean + isLastAttempt: boolean, + hasCustomCa = false ): AttemptResult { if (isUserAbort(error, signal)) { return { action: "throw", error }; } + + // TLS certificate errors are deterministic — don't retry + if (isTlsCertError(error)) { + return { + action: "throw", + error: new ApiError( + "TLS certificate error", + 0, + buildTlsErrorMessage(error as Error, hasCustomCa) + ), + }; + } + if (isInternalTimeout(error) && isLastAttempt) { const timeoutMs = (error as { timeoutMs?: number }).timeoutMs ?? REQUEST_TIMEOUT_MS; @@ -629,7 +687,12 @@ async function executeAttempt({ }); return handleResponse(response, headers, isLastAttempt); } catch (error) { - return handleFetchError(error, init?.signal, isLastAttempt); + return handleFetchError( + error, + init?.signal, + isLastAttempt, + getCustomCaSource() !== "none" + ); } } diff --git a/test/commands/cli/defaults.test.ts b/test/commands/cli/defaults.test.ts index 026dc0dea..e74002c62 100644 --- a/test/commands/cli/defaults.test.ts +++ b/test/commands/cli/defaults.test.ts @@ -116,6 +116,7 @@ describe("defaults storage", () => { telemetry: "off", url: "https://sentry.example.com", headers: null, + "ca-cert": null, }); }); @@ -127,6 +128,7 @@ describe("defaults storage", () => { telemetry: null, url: null, headers: null, + "ca-cert": null, }); }); diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts new file mode 100644 index 000000000..a3840bd64 --- /dev/null +++ b/test/lib/custom-ca.test.ts @@ -0,0 +1,270 @@ +/** + * Tests for custom CA certificate loading and TLS error detection. + * + * isTlsCertError is a pure function tested thoroughly here. + * CA loading tests use temp files and env sandboxing. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { + __resetForTests, + getCustomCaSource, + getCustomTlsOptions, + isTlsCertError, + warnIfSaasWithEnvCa, +} from "../../src/lib/custom-ca.js"; +import { setDefaultCaCert } from "../../src/lib/db/defaults.js"; +import { useTestConfigDir } from "../helpers.js"; + +// --------------------------------------------------------------------------- +// isTlsCertError — pure detection tests +// --------------------------------------------------------------------------- + +describe("isTlsCertError", () => { + test("detects 'unable to get local issuer certificate'", () => { + expect( + isTlsCertError(new Error("unable to get local issuer certificate")) + ).toBe(true); + }); + + test("detects 'unable to verify the first certificate'", () => { + expect( + isTlsCertError(new Error("unable to verify the first certificate")) + ).toBe(true); + }); + + test("detects UNABLE_TO_VERIFY_LEAF_SIGNATURE", () => { + expect(isTlsCertError(new Error("UNABLE_TO_VERIFY_LEAF_SIGNATURE"))).toBe( + true + ); + }); + + test("detects CERT_HAS_EXPIRED", () => { + expect(isTlsCertError(new Error("CERT_HAS_EXPIRED"))).toBe(true); + }); + + test("detects DEPTH_ZERO_SELF_SIGNED_CERT", () => { + expect(isTlsCertError(new Error("DEPTH_ZERO_SELF_SIGNED_CERT"))).toBe(true); + }); + + test("detects SELF_SIGNED_CERT_IN_CHAIN", () => { + expect(isTlsCertError(new Error("SELF_SIGNED_CERT_IN_CHAIN"))).toBe(true); + }); + + test("detects ERR_TLS_CERT_ALTNAME_INVALID", () => { + expect(isTlsCertError(new Error("ERR_TLS_CERT_ALTNAME_INVALID"))).toBe( + true + ); + }); + + test("detects pattern within larger message", () => { + expect( + isTlsCertError( + new Error( + "request to https://sentry.io failed, reason: unable to get local issuer certificate" + ) + ) + ).toBe(true); + }); + + test("returns false for non-TLS errors", () => { + expect(isTlsCertError(new Error("ECONNREFUSED"))).toBe(false); + expect(isTlsCertError(new Error("fetch failed"))).toBe(false); + expect(isTlsCertError(new Error("timeout"))).toBe(false); + expect(isTlsCertError(new Error("network error"))).toBe(false); + }); + + test("returns false for non-Error values", () => { + expect(isTlsCertError("unable to get local issuer certificate")).toBe( + false + ); + expect(isTlsCertError(null)).toBe(false); + expect(isTlsCertError(undefined)).toBe(false); + expect(isTlsCertError(42)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// CA loading — requires DB + env sandboxing +// --------------------------------------------------------------------------- + +describe("custom CA loading", () => { + const getConfigDir = useTestConfigDir("custom-ca-"); + const CERT_PEM = + "-----BEGIN CERTIFICATE-----\nMIIBxyz...\n-----END CERTIFICATE-----\n"; + + let savedNodeExtra: string | undefined; + let savedSslCert: string | undefined; + + beforeEach(() => { + __resetForTests(); + savedNodeExtra = process.env.NODE_EXTRA_CA_CERTS; + savedSslCert = process.env.SSL_CERT_FILE; + delete process.env.NODE_EXTRA_CA_CERTS; + delete process.env.SSL_CERT_FILE; + }); + + afterEach(() => { + if (savedNodeExtra !== undefined) { + process.env.NODE_EXTRA_CA_CERTS = savedNodeExtra; + } else { + delete process.env.NODE_EXTRA_CA_CERTS; + } + if (savedSslCert !== undefined) { + process.env.SSL_CERT_FILE = savedSslCert; + } else { + delete process.env.SSL_CERT_FILE; + } + }); + + test("returns undefined when no CAs configured", async () => { + const result = await getCustomTlsOptions(); + expect(result).toBeUndefined(); + expect(getCustomCaSource()).toBe("none"); + }); + + test("loads CA from stored default (highest priority)", async () => { + const certPath = join(getConfigDir(), "ca.pem"); + writeFileSync(certPath, CERT_PEM); + setDefaultCaCert(certPath); + + const result = await getCustomTlsOptions(); + expect(result).toBeDefined(); + expect(result?.tls.ca).toBe(CERT_PEM); + expect(getCustomCaSource()).toBe("default"); + }); + + test("loads CA from NODE_EXTRA_CA_CERTS", async () => { + const certPath = join(getConfigDir(), "extra-ca.pem"); + writeFileSync(certPath, CERT_PEM); + process.env.NODE_EXTRA_CA_CERTS = certPath; + + const result = await getCustomTlsOptions(); + expect(result).toBeDefined(); + expect(result?.tls.ca).toBe(CERT_PEM); + expect(getCustomCaSource()).toBe("env"); + }); + + test("loads CA from SSL_CERT_FILE as fallback", async () => { + const certPath = join(getConfigDir(), "ssl-cert.pem"); + writeFileSync(certPath, CERT_PEM); + process.env.SSL_CERT_FILE = certPath; + + const result = await getCustomTlsOptions(); + expect(result).toBeDefined(); + expect(result?.tls.ca).toBe(CERT_PEM); + expect(getCustomCaSource()).toBe("env"); + }); + + test("stored default takes priority over env vars", async () => { + const defaultPath = join(getConfigDir(), "default-ca.pem"); + const envPath = join(getConfigDir(), "env-ca.pem"); + writeFileSync(defaultPath, CERT_PEM); + writeFileSync( + envPath, + "-----BEGIN CERTIFICATE-----\nDIFFERENT\n-----END CERTIFICATE-----\n" + ); + setDefaultCaCert(defaultPath); + process.env.NODE_EXTRA_CA_CERTS = envPath; + + const result = await getCustomTlsOptions(); + expect(result?.tls.ca).toBe(CERT_PEM); + expect(getCustomCaSource()).toBe("default"); + }); + + test("NODE_EXTRA_CA_CERTS takes priority over SSL_CERT_FILE", async () => { + const extraPath = join(getConfigDir(), "extra.pem"); + const sslPath = join(getConfigDir(), "ssl.pem"); + writeFileSync(extraPath, CERT_PEM); + writeFileSync( + sslPath, + "-----BEGIN CERTIFICATE-----\nSSL\n-----END CERTIFICATE-----\n" + ); + process.env.NODE_EXTRA_CA_CERTS = extraPath; + process.env.SSL_CERT_FILE = sslPath; + + const result = await getCustomTlsOptions(); + expect(result?.tls.ca).toBe(CERT_PEM); + }); + + test("returns undefined when cert file does not exist", async () => { + process.env.NODE_EXTRA_CA_CERTS = "/nonexistent/path/ca.pem"; + + const result = await getCustomTlsOptions(); + expect(result).toBeUndefined(); + expect(getCustomCaSource()).toBe("none"); + }); + + test("returns undefined when cert file is not valid PEM", async () => { + const certPath = join(getConfigDir(), "not-pem.txt"); + writeFileSync(certPath, "this is not a PEM file"); + process.env.NODE_EXTRA_CA_CERTS = certPath; + + const result = await getCustomTlsOptions(); + expect(result).toBeUndefined(); + expect(getCustomCaSource()).toBe("none"); + }); + + test("caches result across calls", async () => { + const certPath = join(getConfigDir(), "cached.pem"); + writeFileSync(certPath, CERT_PEM); + process.env.NODE_EXTRA_CA_CERTS = certPath; + + const first = await getCustomTlsOptions(); + // Even if env var changes, cached result stays + process.env.NODE_EXTRA_CA_CERTS = "/different/path"; + const second = await getCustomTlsOptions(); + expect(first).toBe(second); + }); +}); + +// --------------------------------------------------------------------------- +// SaaS warning +// --------------------------------------------------------------------------- + +describe("warnIfSaasWithEnvCa", () => { + const getConfigDir = useTestConfigDir("saas-warn-"); + const CERT_PEM = + "-----BEGIN CERTIFICATE-----\nMIIBxyz...\n-----END CERTIFICATE-----\n"; + + let savedNodeExtra: string | undefined; + + beforeEach(() => { + __resetForTests(); + savedNodeExtra = process.env.NODE_EXTRA_CA_CERTS; + delete process.env.NODE_EXTRA_CA_CERTS; + delete process.env.SSL_CERT_FILE; + }); + + afterEach(() => { + if (savedNodeExtra !== undefined) { + process.env.NODE_EXTRA_CA_CERTS = savedNodeExtra; + } else { + delete process.env.NODE_EXTRA_CA_CERTS; + } + }); + + test("does not warn for stored default CAs targeting SaaS", async () => { + const certPath = join(getConfigDir(), "ca.pem"); + writeFileSync(certPath, CERT_PEM); + setDefaultCaCert(certPath); + await getCustomTlsOptions(); + + // Should not throw or warn — source is "default" not "env" + warnIfSaasWithEnvCa("https://us.sentry.io/api/0/organizations/"); + expect(getCustomCaSource()).toBe("default"); + }); + + test("does not warn for env CAs targeting self-hosted", async () => { + const certPath = join(getConfigDir(), "ca.pem"); + writeFileSync(certPath, CERT_PEM); + process.env.NODE_EXTRA_CA_CERTS = certPath; + await getCustomTlsOptions(); + + // Self-hosted URL — no warning + warnIfSaasWithEnvCa("https://sentry.example.com/api/0/"); + expect(getCustomCaSource()).toBe("env"); + }); +}); From bad0b379a6fddc74454e6302a14988a6a830ebdb Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 05:00:27 +0000 Subject: [PATCH 02/13] fix: address bot review findings - Fix async race in resolve(): cache the promise instead of a boolean flag so concurrent callers await the same I/O (Cursor Bugbot) - Fix isTlsCertError to walk error.cause chain for Node.js fetch which wraps TLS errors in TypeError('fetch failed') (Seer) - Add tests for error.cause detection --- src/lib/custom-ca.ts | 44 ++++++++++++++++++++++++++------------ test/lib/custom-ca.test.ts | 23 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 428c63679..103c88aca 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -30,7 +30,7 @@ export type CaSource = "default" | "env" | "none"; /** Cached resolved state — computed once per process */ let resolved: { tls: { ca: string } } | undefined; let resolvedSource: CaSource = "none"; -let hasResolved = false; +let resolvePromise: Promise | null = null; let warnedSaas = false; /** @@ -60,19 +60,14 @@ async function tryReadPem(path: string): Promise { } /** - * Resolve custom CA certificates. Cached after first call. + * Resolve custom CA certificates (inner implementation). * * Priority: * 1. Stored default (`sentry cli defaults ca-cert`) * 2. `NODE_EXTRA_CA_CERTS` env var * 3. `SSL_CERT_FILE` env var */ -async function resolve(): Promise { - if (hasResolved) { - return; - } - hasResolved = true; - +async function resolveInner(): Promise { // 1. Stored default — highest priority, silences SaaS warning const storedPath = getDefaultCaCert(); if (storedPath) { @@ -114,6 +109,18 @@ async function resolve(): Promise { } } +/** + * Resolve custom CA certificates. All concurrent callers await the same + * promise so the second caller never sees stale `undefined` while I/O + * is in flight. + */ +function resolve(): Promise { + if (!resolvePromise) { + resolvePromise = resolveInner(); + } + return resolvePromise; +} + /** * Get the `tls` options to spread into Bun's `fetch()` call. * Returns undefined when no custom CAs are configured. @@ -170,13 +177,22 @@ const TLS_ERROR_PATTERNS = [ "ERR_TLS_CERT_ALTNAME_INVALID", ] as const; -/** Check if an error is a TLS certificate verification failure. */ +/** + * Check if an error is a TLS certificate verification failure. + * Walks `error.cause` to handle Node.js `fetch` which wraps TLS errors + * in `TypeError: fetch failed` with the real error in `.cause`. + */ export function isTlsCertError(error: unknown): boolean { - if (!(error instanceof Error)) { - return false; + let current: unknown = error; + while (current instanceof Error) { + if ( + TLS_ERROR_PATTERNS.some((pattern) => current.message.includes(pattern)) + ) { + return true; + } + current = current.cause; } - const msg = error.message; - return TLS_ERROR_PATTERNS.some((pattern) => msg.includes(pattern)); + return false; } /** @@ -186,6 +202,6 @@ export function isTlsCertError(error: unknown): boolean { export function __resetForTests(): void { resolved = undefined; resolvedSource = "none"; - hasResolved = false; + resolvePromise = null; warnedSaas = false; } diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index a3840bd64..b9d3fe877 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -76,6 +76,29 @@ describe("isTlsCertError", () => { expect(isTlsCertError(new Error("network error"))).toBe(false); }); + test("detects TLS error wrapped in error.cause (Node.js fetch pattern)", () => { + const cause = new Error("unable to get local issuer certificate"); + const wrapper = new TypeError("fetch failed"); + wrapper.cause = cause; + expect(isTlsCertError(wrapper)).toBe(true); + }); + + test("detects deeply nested error.cause chain", () => { + const root = new Error("SELF_SIGNED_CERT_IN_CHAIN"); + const mid = new Error("request failed"); + mid.cause = root; + const outer = new TypeError("fetch failed"); + outer.cause = mid; + expect(isTlsCertError(outer)).toBe(true); + }); + + test("returns false for non-TLS error.cause", () => { + const cause = new Error("ECONNREFUSED"); + const wrapper = new TypeError("fetch failed"); + wrapper.cause = cause; + expect(isTlsCertError(wrapper)).toBe(false); + }); + test("returns false for non-Error values", () => { expect(isTlsCertError("unable to get local issuer certificate")).toBe( false From 583cec943952c38b208c189980aeeb19cb19d8e3 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 05:03:17 +0000 Subject: [PATCH 03/13] =?UTF-8?q?fix:=20TS=20error=20=E2=80=94=20capture?= =?UTF-8?q?=20narrowed=20type=20in=20local=20before=20closure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/custom-ca.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 103c88aca..9afe35dd6 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -185,9 +185,8 @@ const TLS_ERROR_PATTERNS = [ export function isTlsCertError(error: unknown): boolean { let current: unknown = error; while (current instanceof Error) { - if ( - TLS_ERROR_PATTERNS.some((pattern) => current.message.includes(pattern)) - ) { + const msg = current.message; + if (TLS_ERROR_PATTERNS.some((pattern) => msg.includes(pattern))) { return true; } current = current.cause; From 043b1ef3edbb843ea77c6050b500e738988deee5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 05:12:48 +0000 Subject: [PATCH 04/13] fix: address round 2 bot findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove CERT_HAS_EXPIRED and ERR_TLS_CERT_ALTNAME_INVALID from TLS patterns — not CA trust issues, would produce misleading guidance (Seer) - Extract root TLS error via getTlsCertErrorMessage() so Node.js 'fetch failed' wrappers don't hide the real error (Cursor Bugbot) - Wire getTlsCertErrorMessage into both sentry-client.ts and oauth.ts --- src/lib/custom-ca.ts | 27 +++++++++++++++++++++------ src/lib/oauth.ts | 7 ++++--- src/lib/sentry-client.ts | 6 ++++-- test/lib/custom-ca.test.ts | 30 ++++++++++++++++++++++-------- 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 9afe35dd6..907025784 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -164,17 +164,20 @@ export function warnIfSaasWithEnvCa(targetUrl: string): void { } /** - * TLS certificate error patterns from BoringSSL (Bun), OpenSSL (Node), and - * common TLS libraries. These are deterministic — retrying won't help. + * TLS certificate CA trust error patterns — errors that indicate the + * server's certificate chain cannot be verified against known CAs. + * These are fixable by providing a custom CA bundle. + * + * Excludes `CERT_HAS_EXPIRED` and `ERR_TLS_CERT_ALTNAME_INVALID` which + * are not CA trust issues (expired cert / hostname mismatch) and would + * produce misleading "add your CA cert" guidance. */ const TLS_ERROR_PATTERNS = [ "unable to get local issuer certificate", "unable to verify the first certificate", "UNABLE_TO_VERIFY_LEAF_SIGNATURE", - "CERT_HAS_EXPIRED", "DEPTH_ZERO_SELF_SIGNED_CERT", "SELF_SIGNED_CERT_IN_CHAIN", - "ERR_TLS_CERT_ALTNAME_INVALID", ] as const; /** @@ -183,15 +186,27 @@ const TLS_ERROR_PATTERNS = [ * in `TypeError: fetch failed` with the real error in `.cause`. */ export function isTlsCertError(error: unknown): boolean { + return getTlsCertErrorMessage(error) !== undefined; +} + +/** + * Walk the error cause chain and return the message of the first error + * that matches a TLS CA trust pattern, or undefined if none match. + * + * Node.js `fetch` wraps TLS errors in `TypeError: fetch failed` with + * the real error in `.cause` — this finds the root TLS message so + * callers display it instead of the generic wrapper. + */ +export function getTlsCertErrorMessage(error: unknown): string | undefined { let current: unknown = error; while (current instanceof Error) { const msg = current.message; if (TLS_ERROR_PATTERNS.some((pattern) => msg.includes(pattern))) { - return true; + return msg; } current = current.cause; } - return false; + return; } /** diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 6e91d47e1..8687e6c6e 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -14,7 +14,7 @@ import { import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "./constants.js"; import { getCustomTlsOptions, - isTlsCertError, + getTlsCertErrorMessage, warnIfSaasWithEnvCa, } from "./custom-ca.js"; import { applyCustomHeaders } from "./custom-headers.js"; @@ -118,11 +118,12 @@ async function fetchWithConnectionError( } // TLS certificate errors — give actionable guidance - if (isTlsCertError(error)) { + const tlsMsg = getTlsCertErrorMessage(error); + if (tlsMsg) { throw new ApiError( `TLS certificate error connecting to ${getSentryUrl()}`, 0, - `TLS certificate verification failed: ${error.message}\n\n` + + `TLS certificate verification failed: ${tlsMsg}\n\n` + " To fix this, point the CLI to your CA certificate bundle:\n" + " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 21052f31e..5acbd246e 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -19,6 +19,7 @@ import { import { getCustomCaSource, getCustomTlsOptions, + getTlsCertErrorMessage, isTlsCertError, warnIfSaasWithEnvCa, } from "./custom-ca.js"; @@ -314,10 +315,11 @@ async function handleResponse( /** * Build a user-friendly error message for TLS certificate failures. - * Content varies based on whether custom CAs are already loaded. + * Walks `error.cause` to extract the root TLS error (Node.js wraps + * TLS errors in `TypeError: fetch failed`). */ function buildTlsErrorMessage(error: Error, hasCustomCa: boolean): string { - const cause = error.message; + const cause = getTlsCertErrorMessage(error) ?? error.message; if (hasCustomCa) { return ( diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index b9d3fe877..3accd1779 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -12,6 +12,7 @@ import { __resetForTests, getCustomCaSource, getCustomTlsOptions, + getTlsCertErrorMessage, isTlsCertError, warnIfSaasWithEnvCa, } from "../../src/lib/custom-ca.js"; @@ -41,8 +42,14 @@ describe("isTlsCertError", () => { ); }); - test("detects CERT_HAS_EXPIRED", () => { - expect(isTlsCertError(new Error("CERT_HAS_EXPIRED"))).toBe(true); + test("does NOT detect CERT_HAS_EXPIRED (not a CA trust issue)", () => { + expect(isTlsCertError(new Error("CERT_HAS_EXPIRED"))).toBe(false); + }); + + test("does NOT detect ERR_TLS_CERT_ALTNAME_INVALID (not a CA trust issue)", () => { + expect(isTlsCertError(new Error("ERR_TLS_CERT_ALTNAME_INVALID"))).toBe( + false + ); }); test("detects DEPTH_ZERO_SELF_SIGNED_CERT", () => { @@ -53,12 +60,6 @@ describe("isTlsCertError", () => { expect(isTlsCertError(new Error("SELF_SIGNED_CERT_IN_CHAIN"))).toBe(true); }); - test("detects ERR_TLS_CERT_ALTNAME_INVALID", () => { - expect(isTlsCertError(new Error("ERR_TLS_CERT_ALTNAME_INVALID"))).toBe( - true - ); - }); - test("detects pattern within larger message", () => { expect( isTlsCertError( @@ -76,6 +77,19 @@ describe("isTlsCertError", () => { expect(isTlsCertError(new Error("network error"))).toBe(false); }); + test("getTlsCertErrorMessage extracts root cause from wrapped error", () => { + const cause = new Error("unable to get local issuer certificate"); + const wrapper = new TypeError("fetch failed"); + wrapper.cause = cause; + expect(getTlsCertErrorMessage(wrapper)).toBe( + "unable to get local issuer certificate" + ); + }); + + test("getTlsCertErrorMessage returns undefined for non-TLS errors", () => { + expect(getTlsCertErrorMessage(new Error("ECONNREFUSED"))).toBeUndefined(); + }); + test("detects TLS error wrapped in error.cause (Node.js fetch pattern)", () => { const cause = new Error("unable to get local issuer certificate"); const wrapper = new TypeError("fetch failed"); From acc094db9dd4e17a5ab0b2028df99cd8e3e3efeb Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 05:18:56 +0000 Subject: [PATCH 05/13] fix: add ca-cert to human output formatter (DEFAULT_LABELS + show rows) --- src/lib/formatters/human.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/formatters/human.ts b/src/lib/formatters/human.ts index e7c2a6ffa..65483486e 100644 --- a/src/lib/formatters/human.ts +++ b/src/lib/formatters/human.ts @@ -2448,6 +2448,7 @@ const DEFAULT_LABELS: Record = { telemetry: "Telemetry", url: "URL", headers: "Headers", + "ca-cert": "CA Certificate", }; /** @@ -2479,6 +2480,7 @@ function buildDefaultsShowRows(data: DefaultsResult): [string, string][] { ], ["URL", d.url ? safeCodeSpan(d.url) : notSet], ["Headers", d.headers ? safeCodeSpan(d.headers) : notSet], + ["CA Certificate", d["ca-cert"] ? safeCodeSpan(d["ca-cert"]) : notSet], ]; } From fb085687df9b148244d5d6d6e8184e13ed0e5217 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 06:16:17 +0000 Subject: [PATCH 06/13] fix: use static ESM imports in defaults ca-cert handler, restore SSL_CERT_FILE in tests --- src/commands/cli/defaults.ts | 4 ++-- test/lib/custom-ca.test.ts | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/commands/cli/defaults.ts b/src/commands/cli/defaults.ts index 4ce0a4be9..97c4cc828 100644 --- a/src/commands/cli/defaults.ts +++ b/src/commands/cli/defaults.ts @@ -11,6 +11,8 @@ * - `ca-cert` — path to PEM file with custom CA certificates */ +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; import { normalizeUrl } from "../../lib/constants.js"; @@ -150,11 +152,9 @@ const DEFAULTS_REGISTRY: Record = { "ca-cert" ); } - const { resolve } = require("node:path") as typeof import("node:path"); const resolved = resolve(trimmed); let content: string; try { - const { readFileSync } = require("node:fs") as typeof import("node:fs"); content = readFileSync(resolved, "utf-8"); } catch { throw new ValidationError( diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index 3accd1779..d8176c299 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -267,10 +267,12 @@ describe("warnIfSaasWithEnvCa", () => { "-----BEGIN CERTIFICATE-----\nMIIBxyz...\n-----END CERTIFICATE-----\n"; let savedNodeExtra: string | undefined; + let savedSslCert: string | undefined; beforeEach(() => { __resetForTests(); savedNodeExtra = process.env.NODE_EXTRA_CA_CERTS; + savedSslCert = process.env.SSL_CERT_FILE; delete process.env.NODE_EXTRA_CA_CERTS; delete process.env.SSL_CERT_FILE; }); @@ -281,6 +283,11 @@ describe("warnIfSaasWithEnvCa", () => { } else { delete process.env.NODE_EXTRA_CA_CERTS; } + if (savedSslCert !== undefined) { + process.env.SSL_CERT_FILE = savedSslCert; + } else { + delete process.env.SSL_CERT_FILE; + } }); test("does not warn for stored default CAs targeting SaaS", async () => { From cb1ab9fd263ae8f320165b0387d64db12fa5b6ba Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 07:01:35 +0000 Subject: [PATCH 07/13] refactor: deduplicate PEM validation via readCaCertFile, make resolve() sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract readCaCertFile() as single source of truth for PEM validation - defaults ca-cert handler now calls readCaCertFile instead of duplicating - resolve() is now sync (readFileSync), eliminating the async race entirely - getCustomTlsOptions() is no longer async — simpler call sites --- src/commands/cli/defaults.ts | 19 +---- src/lib/custom-ca.ts | 141 +++++++++++++++++------------------ src/lib/oauth.ts | 2 +- src/lib/sentry-client.ts | 2 +- test/lib/custom-ca.test.ts | 46 ++++++------ 5 files changed, 99 insertions(+), 111 deletions(-) diff --git a/src/commands/cli/defaults.ts b/src/commands/cli/defaults.ts index 97c4cc828..c9429a6b2 100644 --- a/src/commands/cli/defaults.ts +++ b/src/commands/cli/defaults.ts @@ -11,11 +11,11 @@ * - `ca-cert` — path to PEM file with custom CA certificates */ -import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; import { normalizeUrl } from "../../lib/constants.js"; +import { readCaCertFile } from "../../lib/custom-ca.js"; import { parseCustomHeaders } from "../../lib/custom-headers.js"; import { clearAllDefaults, @@ -153,20 +153,9 @@ const DEFAULTS_REGISTRY: Record = { ); } const resolved = resolve(trimmed); - let content: string; - try { - content = readFileSync(resolved, "utf-8"); - } catch { - throw new ValidationError( - `CA certificate file not found or not readable: ${resolved}`, - "ca-cert" - ); - } - if (!content.includes("-----BEGIN CERTIFICATE-----")) { - throw new ValidationError( - "File does not contain PEM certificate data (expected -----BEGIN CERTIFICATE-----).", - "ca-cert" - ); + const result = readCaCertFile(resolved); + if (!result.ok) { + throw new ValidationError(result.reason, "ca-cert"); } setDefaultCaCert(resolved); }, diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 907025784..85b01cbb4 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -30,105 +30,104 @@ export type CaSource = "default" | "env" | "none"; /** Cached resolved state — computed once per process */ let resolved: { tls: { ca: string } } | undefined; let resolvedSource: CaSource = "none"; -let resolvePromise: Promise | null = null; +let hasResolved = false; let warnedSaas = false; +/** + * Validate and read a CA certificate PEM file synchronously. + * Returns `{ ok: true, content }` on success or `{ ok: false, reason }` on failure. + * + * Used by both the eager validation in `sentry cli defaults ca-cert` and + * the lazy loading in `resolve()` — single source of truth for PEM validation. + */ +export function readCaCertFile( + path: string +): { ok: true; content: string } | { ok: false; reason: string } { + const { readFileSync } = require("node:fs") as typeof import("node:fs"); + let content: string; + try { + content = readFileSync(path, "utf-8"); + } catch { + return { + ok: false, + reason: `CA certificate file not found or not readable: ${path}`, + }; + } + if (!content.includes("-----BEGIN CERTIFICATE-----")) { + return { + ok: false, + reason: + "File does not contain PEM certificate data (expected -----BEGIN CERTIFICATE-----).", + }; + } + return { ok: true, content }; +} + /** * Attempt to read a PEM file. Returns the file contents on success, * or undefined if the file doesn't exist or can't be read. * Never throws — a missing CA file shouldn't crash the CLI. */ -async function tryReadPem(path: string): Promise { - try { - const file = Bun.file(path); - if (!(await file.exists())) { - log.warn(`CA certificate file not found: ${path}`); - return; - } - const content = await file.text(); - if (!content.includes("-----BEGIN")) { - log.warn( - `CA certificate file does not appear to contain PEM data: ${path}` - ); - return; - } - return content; - } catch { - log.warn(`Failed to read CA certificate file: ${path}`); +function tryReadPem(path: string): string | undefined { + const result = readCaCertFile(path); + if (!result.ok) { + log.warn(result.reason); return; } + return result.content; } /** - * Resolve custom CA certificates (inner implementation). + * Resolve custom CA certificates. Runs once per process. * - * Priority: - * 1. Stored default (`sentry cli defaults ca-cert`) - * 2. `NODE_EXTRA_CA_CERTS` env var - * 3. `SSL_CERT_FILE` env var + * Tries sources in priority order: stored default, NODE_EXTRA_CA_CERTS, + * SSL_CERT_FILE. First readable PEM wins. */ -async function resolveInner(): Promise { - // 1. Stored default — highest priority, silences SaaS warning - const storedPath = getDefaultCaCert(); - if (storedPath) { - const pem = await tryReadPem(storedPath); - if (pem) { - resolved = { tls: { ca: pem } }; - resolvedSource = "default"; - log.debug(`Loaded CA certificates from stored default: ${storedPath}`); - return; - } - // Stored path is stale/invalid — fall through to env vars +function resolve(): void { + if (hasResolved) { + return; } + hasResolved = true; - // 2. NODE_EXTRA_CA_CERTS const env = getEnv(); - const extraCerts = env.NODE_EXTRA_CA_CERTS?.trim(); - if (extraCerts) { - const pem = await tryReadPem(extraCerts); - if (pem) { - resolved = { tls: { ca: pem } }; - resolvedSource = "env"; - log.debug( - `Loaded CA certificates from NODE_EXTRA_CA_CERTS: ${extraCerts}` - ); - return; + const sources: { path: string; source: CaSource; label: string }[] = [ + { + path: getDefaultCaCert() ?? "", + source: "default", + label: "stored default", + }, + { + path: env.NODE_EXTRA_CA_CERTS?.trim() ?? "", + source: "env", + label: "NODE_EXTRA_CA_CERTS", + }, + { + path: env.SSL_CERT_FILE?.trim() ?? "", + source: "env", + label: "SSL_CERT_FILE", + }, + ]; + + for (const { path, source, label } of sources) { + if (!path) { + continue; } - } - - // 3. SSL_CERT_FILE - const sslCertFile = env.SSL_CERT_FILE?.trim(); - if (sslCertFile) { - const pem = await tryReadPem(sslCertFile); + const pem = tryReadPem(path); if (pem) { resolved = { tls: { ca: pem } }; - resolvedSource = "env"; - log.debug(`Loaded CA certificates from SSL_CERT_FILE: ${sslCertFile}`); + resolvedSource = source; + log.debug(`Loaded CA certificates from ${label}: ${path}`); return; } } } -/** - * Resolve custom CA certificates. All concurrent callers await the same - * promise so the second caller never sees stale `undefined` while I/O - * is in flight. - */ -function resolve(): Promise { - if (!resolvePromise) { - resolvePromise = resolveInner(); - } - return resolvePromise; -} - /** * Get the `tls` options to spread into Bun's `fetch()` call. * Returns undefined when no custom CAs are configured. */ -export async function getCustomTlsOptions(): Promise< - { tls: { ca: string } } | undefined -> { - await resolve(); +export function getCustomTlsOptions(): { tls: { ca: string } } | undefined { + resolve(); return resolved; } @@ -216,6 +215,6 @@ export function getTlsCertErrorMessage(error: unknown): string | undefined { export function __resetForTests(): void { resolved = undefined; resolvedSource = "none"; - resolvePromise = null; + hasResolved = false; warnedSaas = false; } diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 8687e6c6e..03e73f8a7 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -106,7 +106,7 @@ async function fetchWithConnectionError( const effectiveInit: RequestInit = { ...init, headers: merged }; try { - const customTls = await getCustomTlsOptions(); + const customTls = getCustomTlsOptions(); if (customTls) { warnIfSaasWithEnvCa(url); } diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 5acbd246e..a3946debc 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -260,7 +260,7 @@ async function fetchWithTimeout({ // Spread custom TLS options (CA certs for corporate proxies). // On Bun, this passes `tls: { ca }` to fetch(); on Node, the // extra property is harmless (Node honors NODE_EXTRA_CA_CERTS natively). - const customTls = await getCustomTlsOptions(); + const customTls = getCustomTlsOptions(); if (customTls) { warnIfSaasWithEnvCa(extractFullUrl(input)); } diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index d8176c299..804300ccd 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -156,46 +156,46 @@ describe("custom CA loading", () => { } }); - test("returns undefined when no CAs configured", async () => { - const result = await getCustomTlsOptions(); + test("returns undefined when no CAs configured", () => { + const result = getCustomTlsOptions(); expect(result).toBeUndefined(); expect(getCustomCaSource()).toBe("none"); }); - test("loads CA from stored default (highest priority)", async () => { + test("loads CA from stored default (highest priority)", () => { const certPath = join(getConfigDir(), "ca.pem"); writeFileSync(certPath, CERT_PEM); setDefaultCaCert(certPath); - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result).toBeDefined(); expect(result?.tls.ca).toBe(CERT_PEM); expect(getCustomCaSource()).toBe("default"); }); - test("loads CA from NODE_EXTRA_CA_CERTS", async () => { + test("loads CA from NODE_EXTRA_CA_CERTS", () => { const certPath = join(getConfigDir(), "extra-ca.pem"); writeFileSync(certPath, CERT_PEM); process.env.NODE_EXTRA_CA_CERTS = certPath; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result).toBeDefined(); expect(result?.tls.ca).toBe(CERT_PEM); expect(getCustomCaSource()).toBe("env"); }); - test("loads CA from SSL_CERT_FILE as fallback", async () => { + test("loads CA from SSL_CERT_FILE as fallback", () => { const certPath = join(getConfigDir(), "ssl-cert.pem"); writeFileSync(certPath, CERT_PEM); process.env.SSL_CERT_FILE = certPath; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result).toBeDefined(); expect(result?.tls.ca).toBe(CERT_PEM); expect(getCustomCaSource()).toBe("env"); }); - test("stored default takes priority over env vars", async () => { + test("stored default takes priority over env vars", () => { const defaultPath = join(getConfigDir(), "default-ca.pem"); const envPath = join(getConfigDir(), "env-ca.pem"); writeFileSync(defaultPath, CERT_PEM); @@ -206,12 +206,12 @@ describe("custom CA loading", () => { setDefaultCaCert(defaultPath); process.env.NODE_EXTRA_CA_CERTS = envPath; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result?.tls.ca).toBe(CERT_PEM); expect(getCustomCaSource()).toBe("default"); }); - test("NODE_EXTRA_CA_CERTS takes priority over SSL_CERT_FILE", async () => { + test("NODE_EXTRA_CA_CERTS takes priority over SSL_CERT_FILE", () => { const extraPath = join(getConfigDir(), "extra.pem"); const sslPath = join(getConfigDir(), "ssl.pem"); writeFileSync(extraPath, CERT_PEM); @@ -222,37 +222,37 @@ describe("custom CA loading", () => { process.env.NODE_EXTRA_CA_CERTS = extraPath; process.env.SSL_CERT_FILE = sslPath; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result?.tls.ca).toBe(CERT_PEM); }); - test("returns undefined when cert file does not exist", async () => { + test("returns undefined when cert file does not exist", () => { process.env.NODE_EXTRA_CA_CERTS = "/nonexistent/path/ca.pem"; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result).toBeUndefined(); expect(getCustomCaSource()).toBe("none"); }); - test("returns undefined when cert file is not valid PEM", async () => { + test("returns undefined when cert file is not valid PEM", () => { const certPath = join(getConfigDir(), "not-pem.txt"); writeFileSync(certPath, "this is not a PEM file"); process.env.NODE_EXTRA_CA_CERTS = certPath; - const result = await getCustomTlsOptions(); + const result = getCustomTlsOptions(); expect(result).toBeUndefined(); expect(getCustomCaSource()).toBe("none"); }); - test("caches result across calls", async () => { + test("caches result across calls", () => { const certPath = join(getConfigDir(), "cached.pem"); writeFileSync(certPath, CERT_PEM); process.env.NODE_EXTRA_CA_CERTS = certPath; - const first = await getCustomTlsOptions(); + const first = getCustomTlsOptions(); // Even if env var changes, cached result stays process.env.NODE_EXTRA_CA_CERTS = "/different/path"; - const second = await getCustomTlsOptions(); + const second = getCustomTlsOptions(); expect(first).toBe(second); }); }); @@ -290,22 +290,22 @@ describe("warnIfSaasWithEnvCa", () => { } }); - test("does not warn for stored default CAs targeting SaaS", async () => { + test("does not warn for stored default CAs targeting SaaS", () => { const certPath = join(getConfigDir(), "ca.pem"); writeFileSync(certPath, CERT_PEM); setDefaultCaCert(certPath); - await getCustomTlsOptions(); + getCustomTlsOptions(); // Should not throw or warn — source is "default" not "env" warnIfSaasWithEnvCa("https://us.sentry.io/api/0/organizations/"); expect(getCustomCaSource()).toBe("default"); }); - test("does not warn for env CAs targeting self-hosted", async () => { + test("does not warn for env CAs targeting self-hosted", () => { const certPath = join(getConfigDir(), "ca.pem"); writeFileSync(certPath, CERT_PEM); process.env.NODE_EXTRA_CA_CERTS = certPath; - await getCustomTlsOptions(); + getCustomTlsOptions(); // Self-hosted URL — no warning warnIfSaasWithEnvCa("https://sentry.example.com/api/0/"); From f2841c394adb3966f88908d38e20dd4e11e216fc Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 17:09:30 +0000 Subject: [PATCH 08/13] fix: address self-review findings - Add cycle detection (Set guard) in getTlsCertErrorMessage cause walker - Extract buildTlsErrorDetail to custom-ca.ts so both sentry-client.ts and oauth.ts share the hasCustomCa branching logic - Store resolvedLabel during resolve() so warnIfSaasWithEnvCa shows the actual source, not a re-read of env that may disagree - getCustomCaSource() now calls resolve() to prevent stale 'none' if called before getCustomTlsOptions() --- src/lib/custom-ca.ts | 49 ++++++++++++++++++++++++++++++++++------ src/lib/oauth.ts | 12 ++++------ src/lib/sentry-client.ts | 44 ++++-------------------------------- 3 files changed, 50 insertions(+), 55 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 85b01cbb4..3b6be641d 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -30,6 +30,7 @@ export type CaSource = "default" | "env" | "none"; /** Cached resolved state — computed once per process */ let resolved: { tls: { ca: string } } | undefined; let resolvedSource: CaSource = "none"; +let resolvedLabel = ""; let hasResolved = false; let warnedSaas = false; @@ -116,6 +117,7 @@ function resolve(): void { if (pem) { resolved = { tls: { ca: pem } }; resolvedSource = source; + resolvedLabel = label; log.debug(`Loaded CA certificates from ${label}: ${path}`); return; } @@ -133,6 +135,7 @@ export function getCustomTlsOptions(): { tls: { ca: string } } | undefined { /** Get the source of the loaded CA certificates. */ export function getCustomCaSource(): CaSource { + resolve(); return resolvedSource; } @@ -151,12 +154,8 @@ export function warnIfSaasWithEnvCa(targetUrl: string): void { } warnedSaas = true; - const envVar = getEnv().NODE_EXTRA_CA_CERTS?.trim() - ? "NODE_EXTRA_CA_CERTS" - : "SSL_CERT_FILE"; - log.warn( - `Using custom CA certificates from ${envVar} for sentry.io connections.\n` + + `Using custom CA certificates from ${resolvedLabel} for sentry.io connections.\n` + " If you intended this (e.g. corporate proxy), silence this warning:\n" + " sentry cli defaults ca-cert /path/to/cert.pem" ); @@ -197,8 +196,10 @@ export function isTlsCertError(error: unknown): boolean { * callers display it instead of the generic wrapper. */ export function getTlsCertErrorMessage(error: unknown): string | undefined { + const seen = new Set(); let current: unknown = error; - while (current instanceof Error) { + while (current instanceof Error && !seen.has(current)) { + seen.add(current); const msg = current.message; if (TLS_ERROR_PATTERNS.some((pattern) => msg.includes(pattern))) { return msg; @@ -209,12 +210,46 @@ export function getTlsCertErrorMessage(error: unknown): string | undefined { } /** - * Reset all cached state. Test-only — not exported from the public API. + * Build a user-friendly error detail for TLS certificate failures. + * Walks `error.cause` to extract the root TLS error (Node.js wraps + * TLS errors in `TypeError: fetch failed`). + * + * When custom CAs are already loaded, the message says "still failed" + * so the user knows to check their bundle — not re-run the same setup. + */ +export function buildTlsErrorDetail(error: Error): string { + const cause = getTlsCertErrorMessage(error) ?? error.message; + const hasCustomCa = getCustomCaSource() !== "none"; + + if (hasCustomCa) { + return ( + `TLS certificate verification failed: ${cause}\n\n` + + " Custom CA certificates are loaded but verification still failed.\n" + + " The certificate file may not contain the correct CA for this server.\n\n" + + " Check that your CA bundle includes the certificate authority used by\n" + + " your network proxy or Sentry instance." + ); + } + + return ( + `TLS certificate verification failed: ${cause}\n\n` + + " This usually means your network uses a TLS-intercepting proxy\n" + + " (corporate firewall, VPN) with a private certificate authority.\n\n" + + " To fix this, point the CLI to your CA certificate bundle:\n" + + " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + + " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + + " export NODE_EXTRA_CA_CERTS=/path/to/corporate-ca.pem" + ); +} + +/** + * Reset all cached state. Exported for test isolation only. * @internal */ export function __resetForTests(): void { resolved = undefined; resolvedSource = "none"; + resolvedLabel = ""; hasResolved = false; warnedSaas = false; } diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 03e73f8a7..017ba4516 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -13,8 +13,9 @@ import { } from "../types/index.js"; import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "./constants.js"; import { + buildTlsErrorDetail, getCustomTlsOptions, - getTlsCertErrorMessage, + isTlsCertError, warnIfSaasWithEnvCa, } from "./custom-ca.js"; import { applyCustomHeaders } from "./custom-headers.js"; @@ -118,16 +119,11 @@ async function fetchWithConnectionError( } // TLS certificate errors — give actionable guidance - const tlsMsg = getTlsCertErrorMessage(error); - if (tlsMsg) { + if (isTlsCertError(error)) { throw new ApiError( `TLS certificate error connecting to ${getSentryUrl()}`, 0, - `TLS certificate verification failed: ${tlsMsg}\n\n` + - " To fix this, point the CLI to your CA certificate bundle:\n" + - " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + - " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + - " export NODE_EXTRA_CA_CERTS=/path/to/corporate-ca.pem" + buildTlsErrorDetail(error) ); } diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index a3946debc..c09bf48e5 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -17,9 +17,8 @@ import { getUserAgent, } from "./constants.js"; import { - getCustomCaSource, + buildTlsErrorDetail, getCustomTlsOptions, - getTlsCertErrorMessage, isTlsCertError, warnIfSaasWithEnvCa, } from "./custom-ca.js"; @@ -313,35 +312,6 @@ async function handleResponse( return { action: "done", response }; } -/** - * Build a user-friendly error message for TLS certificate failures. - * Walks `error.cause` to extract the root TLS error (Node.js wraps - * TLS errors in `TypeError: fetch failed`). - */ -function buildTlsErrorMessage(error: Error, hasCustomCa: boolean): string { - const cause = getTlsCertErrorMessage(error) ?? error.message; - - if (hasCustomCa) { - return ( - `TLS certificate verification failed: ${cause}\n\n` + - " Custom CA certificates are loaded but verification still failed.\n" + - " The certificate file may not contain the correct CA for this server.\n\n" + - " Check that your CA bundle includes the certificate authority used by\n" + - " your network proxy or Sentry instance." - ); - } - - return ( - `TLS certificate verification failed: ${cause}\n\n` + - " This usually means your network uses a TLS-intercepting proxy\n" + - " (corporate firewall, VPN) with a private certificate authority.\n\n" + - " To fix this, point the CLI to your CA certificate bundle:\n" + - " sentry cli defaults ca-cert /path/to/corporate-ca.pem\n\n" + - " Or set the NODE_EXTRA_CA_CERTS environment variable:\n" + - " export NODE_EXTRA_CA_CERTS=/path/to/corporate-ca.pem" - ); -} - /** * Decide what to do with a fetch error. User aborts and an internal * timeout on the last attempt throw immediately; TLS cert errors throw @@ -351,8 +321,7 @@ function buildTlsErrorMessage(error: Error, hasCustomCa: boolean): string { function handleFetchError( error: unknown, signal: AbortSignal | undefined | null, - isLastAttempt: boolean, - hasCustomCa = false + isLastAttempt: boolean ): AttemptResult { if (isUserAbort(error, signal)) { return { action: "throw", error }; @@ -365,7 +334,7 @@ function handleFetchError( error: new ApiError( "TLS certificate error", 0, - buildTlsErrorMessage(error as Error, hasCustomCa) + buildTlsErrorDetail(error as Error) ), }; } @@ -689,12 +658,7 @@ async function executeAttempt({ }); return handleResponse(response, headers, isLastAttempt); } catch (error) { - return handleFetchError( - error, - init?.signal, - isLastAttempt, - getCustomCaSource() !== "none" - ); + return handleFetchError(error, init?.signal, isLastAttempt); } } From bcebade3c49b395c73f2c0ff2b783600b30d707f Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 17:20:30 +0000 Subject: [PATCH 09/13] fix: guard getDefaultCaCert() DB call so env vars are still tried on DB failure --- src/lib/custom-ca.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 3b6be641d..8acc03e33 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -90,13 +90,18 @@ function resolve(): void { } hasResolved = true; + // Build the source list. getDefaultCaCert() reads SQLite — if the DB + // is broken, fall through to env var sources instead of aborting. + let storedPath = ""; + try { + storedPath = getDefaultCaCert() ?? ""; + } catch { + log.debug("Failed to read stored ca-cert default from database"); + } + const env = getEnv(); const sources: { path: string; source: CaSource; label: string }[] = [ - { - path: getDefaultCaCert() ?? "", - source: "default", - label: "stored default", - }, + { path: storedPath, source: "default", label: "stored default" }, { path: env.NODE_EXTRA_CA_CERTS?.trim() ?? "", source: "env", From 1e6603ac270844b7c6871b7f60499bacbc4dfc1b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 17:25:22 +0000 Subject: [PATCH 10/13] fix: call resolve() in warnIfSaasWithEnvCa to ensure state is initialized --- src/lib/custom-ca.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index 8acc03e33..f5e7cac0b 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -151,6 +151,7 @@ export function getCustomCaSource(): CaSource { * explicit user acknowledgment and do NOT trigger this warning. */ export function warnIfSaasWithEnvCa(targetUrl: string): void { + resolve(); if (warnedSaas || resolvedSource !== "env") { return; } From 80d7da7f39974aba92704c2a46279d3b546e9eca Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 17:34:31 +0000 Subject: [PATCH 11/13] fix: concatenate custom CA with root certificates for additive semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bun's tls.ca replaces the default Mozilla CA bundle rather than supplementing it. Concatenate via [...rootCertificates, customPem] to match NODE_EXTRA_CA_CERTS additive behavior — without this, setting a corporate CA would break all public CA trust. --- src/lib/custom-ca.ts | 7 ++++++- test/lib/custom-ca.test.ts | 28 +++++++++++++++------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index f5e7cac0b..d34833503 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -17,6 +17,7 @@ * threat model discussion. */ +import { rootCertificates } from "node:tls"; import { getDefaultCaCert } from "./db/defaults.js"; import { getEnv } from "./env.js"; import { logger } from "./logger.js"; @@ -120,7 +121,11 @@ function resolve(): void { } const pem = tryReadPem(path); if (pem) { - resolved = { tls: { ca: pem } }; + // Bun's tls.ca replaces the default Mozilla CA bundle, so we must + // concatenate the custom CA with the built-in root certificates + // to preserve additive NODE_EXTRA_CA_CERTS semantics. + const combined = [...rootCertificates, pem].join("\n"); + resolved = { tls: { ca: combined } }; resolvedSource = source; resolvedLabel = label; log.debug(`Loaded CA certificates from ${label}: ${path}`); diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index 804300ccd..73665d993 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -169,7 +169,9 @@ describe("custom CA loading", () => { const result = getCustomTlsOptions(); expect(result).toBeDefined(); - expect(result?.tls.ca).toBe(CERT_PEM); + // Custom CA is concatenated with root certificates (additive semantics) + expect(result?.tls.ca).toContain(CERT_PEM); + expect(result?.tls.ca).toContain("-----BEGIN CERTIFICATE-----"); expect(getCustomCaSource()).toBe("default"); }); @@ -180,7 +182,7 @@ describe("custom CA loading", () => { const result = getCustomTlsOptions(); expect(result).toBeDefined(); - expect(result?.tls.ca).toBe(CERT_PEM); + expect(result?.tls.ca).toContain(CERT_PEM); expect(getCustomCaSource()).toBe("env"); }); @@ -191,39 +193,39 @@ describe("custom CA loading", () => { const result = getCustomTlsOptions(); expect(result).toBeDefined(); - expect(result?.tls.ca).toBe(CERT_PEM); + expect(result?.tls.ca).toContain(CERT_PEM); expect(getCustomCaSource()).toBe("env"); }); test("stored default takes priority over env vars", () => { const defaultPath = join(getConfigDir(), "default-ca.pem"); const envPath = join(getConfigDir(), "env-ca.pem"); + const ENV_PEM = + "-----BEGIN CERTIFICATE-----\nDIFFERENT\n-----END CERTIFICATE-----\n"; writeFileSync(defaultPath, CERT_PEM); - writeFileSync( - envPath, - "-----BEGIN CERTIFICATE-----\nDIFFERENT\n-----END CERTIFICATE-----\n" - ); + writeFileSync(envPath, ENV_PEM); setDefaultCaCert(defaultPath); process.env.NODE_EXTRA_CA_CERTS = envPath; const result = getCustomTlsOptions(); - expect(result?.tls.ca).toBe(CERT_PEM); + expect(result?.tls.ca).toContain(CERT_PEM); + expect(result?.tls.ca).not.toContain("DIFFERENT"); expect(getCustomCaSource()).toBe("default"); }); test("NODE_EXTRA_CA_CERTS takes priority over SSL_CERT_FILE", () => { const extraPath = join(getConfigDir(), "extra.pem"); const sslPath = join(getConfigDir(), "ssl.pem"); + const SSL_PEM = + "-----BEGIN CERTIFICATE-----\nSSLONLY\n-----END CERTIFICATE-----\n"; writeFileSync(extraPath, CERT_PEM); - writeFileSync( - sslPath, - "-----BEGIN CERTIFICATE-----\nSSL\n-----END CERTIFICATE-----\n" - ); + writeFileSync(sslPath, SSL_PEM); process.env.NODE_EXTRA_CA_CERTS = extraPath; process.env.SSL_CERT_FILE = sslPath; const result = getCustomTlsOptions(); - expect(result?.tls.ca).toBe(CERT_PEM); + expect(result?.tls.ca).toContain(CERT_PEM); + expect(result?.tls.ca).not.toContain("SSLONLY"); }); test("returns undefined when cert file does not exist", () => { From 9507d4946b48b7f11e851cf34da84b6ffc812438 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 1 May 2026 18:43:59 +0000 Subject: [PATCH 12/13] fix: drop SSL_CERT_FILE, add Node 24+ setDefaultCACertificates for unified experience - Drop SSL_CERT_FILE support (neither Bun nor Node honor it natively) - On Node 24+, call tls.setDefaultCACertificates() to inject custom CAs into the process-wide TLS trust store so Node's fetch() also works - On Node 22, NODE_EXTRA_CA_CERTS is handled natively by Node - Feature-detect setDefaultCACertificates (undefined on Node 22) --- script/generate-docs-sections.ts | 1 - src/lib/custom-ca.ts | 43 ++++++++++++++++++++++++++------ src/lib/env-registry.ts | 8 ------ test/lib/custom-ca.test.ts | 42 ------------------------------- 4 files changed, 35 insertions(+), 59 deletions(-) diff --git a/script/generate-docs-sections.ts b/script/generate-docs-sections.ts index 0ea317c26..ebeebed08 100644 --- a/script/generate-docs-sections.ts +++ b/script/generate-docs-sections.ts @@ -366,7 +366,6 @@ const SELF_HOSTED_TABLE_ENTRIES: readonly [string, string][] = [ "NODE_EXTRA_CA_CERTS", "Path to PEM file with additional CA certificates (for corporate proxies)", ], - ["SSL_CERT_FILE", "Fallback CA certificate bundle path"], ]; /** diff --git a/src/lib/custom-ca.ts b/src/lib/custom-ca.ts index d34833503..e95d0c890 100644 --- a/src/lib/custom-ca.ts +++ b/src/lib/custom-ca.ts @@ -4,7 +4,6 @@ * Reads CA bundles from (in priority order): * 1. `sentry cli defaults ca-cert` (stored path in SQLite) * 2. `NODE_EXTRA_CA_CERTS` env var - * 3. `SSL_CERT_FILE` env var * * Returns a `tls` options object for Bun's `fetch()`. On the Node.js npm * distribution, Node natively honors `NODE_EXTRA_CA_CERTS` so the extra @@ -23,6 +22,20 @@ import { getEnv } from "./env.js"; import { logger } from "./logger.js"; import { isSentrySaasUrl } from "./sentry-urls.js"; +/** + * Node 24+ exposes `tls.setDefaultCACertificates()` which modifies the + * process-wide CA trust store — including for `fetch()`. On Node 22 the + * function doesn't exist, and on Bun we use the per-request `tls.ca` + * option instead. + */ +const setDefaultCACertificates: ((certs: string[]) => void) | undefined = + // eslint-disable-next-line @typescript-eslint/no-require-imports + ( + require("node:tls") as { + setDefaultCACertificates?: (certs: string[]) => void; + } + ).setDefaultCACertificates; + const log = logger.withTag("tls"); /** Where the loaded CA came from */ @@ -79,11 +92,29 @@ function tryReadPem(path: string): string | undefined { return result.content; } +/** + * On Node 24+, inject the custom CA into the process-wide TLS trust store + * so Node's built-in `fetch()` (which ignores the Bun-specific `tls` option) + * also uses the custom CAs. On Node 22 this is a no-op; those users rely + * on `NODE_EXTRA_CA_CERTS` which Node handles natively. + */ +function injectIntoNodeTls(customPem: string): void { + if (typeof setDefaultCACertificates !== "function") { + return; + } + try { + setDefaultCACertificates([...rootCertificates, customPem]); + log.debug("Injected custom CA into Node.js TLS trust store"); + } catch (err) { + log.debug(`Failed to set Node.js default CA certificates: ${err}`); + } +} + /** * Resolve custom CA certificates. Runs once per process. * - * Tries sources in priority order: stored default, NODE_EXTRA_CA_CERTS, - * SSL_CERT_FILE. First readable PEM wins. + * Tries sources in priority order: stored default, NODE_EXTRA_CA_CERTS. + * First readable PEM wins. */ function resolve(): void { if (hasResolved) { @@ -108,11 +139,6 @@ function resolve(): void { source: "env", label: "NODE_EXTRA_CA_CERTS", }, - { - path: env.SSL_CERT_FILE?.trim() ?? "", - source: "env", - label: "SSL_CERT_FILE", - }, ]; for (const { path, source, label } of sources) { @@ -129,6 +155,7 @@ function resolve(): void { resolvedSource = source; resolvedLabel = label; log.debug(`Loaded CA certificates from ${label}: ${path}`); + injectIntoNodeTls(pem); return; } } diff --git a/src/lib/env-registry.ts b/src/lib/env-registry.ts index 9854eb732..6595eaeec 100644 --- a/src/lib/env-registry.ts +++ b/src/lib/env-registry.ts @@ -185,14 +185,6 @@ export const ENV_VAR_REGISTRY: readonly EnvVarEntry[] = [ example: "/path/to/corporate-ca.pem", selfHosted: true, }, - { - name: "SSL_CERT_FILE", - description: - "Fallback path to a PEM CA certificate bundle. " + - "Read when `NODE_EXTRA_CA_CERTS` is not set.", - example: "/etc/ssl/certs/ca-certificates.crt", - selfHosted: true, - }, // -- Display -- { name: "SENTRY_PLAIN_OUTPUT", diff --git a/test/lib/custom-ca.test.ts b/test/lib/custom-ca.test.ts index 73665d993..088a3617f 100644 --- a/test/lib/custom-ca.test.ts +++ b/test/lib/custom-ca.test.ts @@ -133,14 +133,11 @@ describe("custom CA loading", () => { "-----BEGIN CERTIFICATE-----\nMIIBxyz...\n-----END CERTIFICATE-----\n"; let savedNodeExtra: string | undefined; - let savedSslCert: string | undefined; beforeEach(() => { __resetForTests(); savedNodeExtra = process.env.NODE_EXTRA_CA_CERTS; - savedSslCert = process.env.SSL_CERT_FILE; delete process.env.NODE_EXTRA_CA_CERTS; - delete process.env.SSL_CERT_FILE; }); afterEach(() => { @@ -149,11 +146,6 @@ describe("custom CA loading", () => { } else { delete process.env.NODE_EXTRA_CA_CERTS; } - if (savedSslCert !== undefined) { - process.env.SSL_CERT_FILE = savedSslCert; - } else { - delete process.env.SSL_CERT_FILE; - } }); test("returns undefined when no CAs configured", () => { @@ -186,17 +178,6 @@ describe("custom CA loading", () => { expect(getCustomCaSource()).toBe("env"); }); - test("loads CA from SSL_CERT_FILE as fallback", () => { - const certPath = join(getConfigDir(), "ssl-cert.pem"); - writeFileSync(certPath, CERT_PEM); - process.env.SSL_CERT_FILE = certPath; - - const result = getCustomTlsOptions(); - expect(result).toBeDefined(); - expect(result?.tls.ca).toContain(CERT_PEM); - expect(getCustomCaSource()).toBe("env"); - }); - test("stored default takes priority over env vars", () => { const defaultPath = join(getConfigDir(), "default-ca.pem"); const envPath = join(getConfigDir(), "env-ca.pem"); @@ -213,21 +194,6 @@ describe("custom CA loading", () => { expect(getCustomCaSource()).toBe("default"); }); - test("NODE_EXTRA_CA_CERTS takes priority over SSL_CERT_FILE", () => { - const extraPath = join(getConfigDir(), "extra.pem"); - const sslPath = join(getConfigDir(), "ssl.pem"); - const SSL_PEM = - "-----BEGIN CERTIFICATE-----\nSSLONLY\n-----END CERTIFICATE-----\n"; - writeFileSync(extraPath, CERT_PEM); - writeFileSync(sslPath, SSL_PEM); - process.env.NODE_EXTRA_CA_CERTS = extraPath; - process.env.SSL_CERT_FILE = sslPath; - - const result = getCustomTlsOptions(); - expect(result?.tls.ca).toContain(CERT_PEM); - expect(result?.tls.ca).not.toContain("SSLONLY"); - }); - test("returns undefined when cert file does not exist", () => { process.env.NODE_EXTRA_CA_CERTS = "/nonexistent/path/ca.pem"; @@ -269,14 +235,11 @@ describe("warnIfSaasWithEnvCa", () => { "-----BEGIN CERTIFICATE-----\nMIIBxyz...\n-----END CERTIFICATE-----\n"; let savedNodeExtra: string | undefined; - let savedSslCert: string | undefined; beforeEach(() => { __resetForTests(); savedNodeExtra = process.env.NODE_EXTRA_CA_CERTS; - savedSslCert = process.env.SSL_CERT_FILE; delete process.env.NODE_EXTRA_CA_CERTS; - delete process.env.SSL_CERT_FILE; }); afterEach(() => { @@ -285,11 +248,6 @@ describe("warnIfSaasWithEnvCa", () => { } else { delete process.env.NODE_EXTRA_CA_CERTS; } - if (savedSslCert !== undefined) { - process.env.SSL_CERT_FILE = savedSslCert; - } else { - delete process.env.SSL_CERT_FILE; - } }); test("does not warn for stored default CAs targeting SaaS", () => { From 3095e45d2b2cbd101cc432ecc2bf26d7a63c8254 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 1 May 2026 18:44:29 +0000 Subject: [PATCH 13/13] chore: regenerate docs --- docs/src/content/docs/self-hosted.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/content/docs/self-hosted.md b/docs/src/content/docs/self-hosted.md index 1a06d2287..dea9a4129 100644 --- a/docs/src/content/docs/self-hosted.md +++ b/docs/src/content/docs/self-hosted.md @@ -78,7 +78,6 @@ If you pass a self-hosted Sentry URL as a command argument (e.g., an issue or ev | `SENTRY_ORG` | Default organization slug | | `SENTRY_PROJECT` | Default project slug (supports `org/project` format) | | `NODE_EXTRA_CA_CERTS` | Path to PEM file with additional CA certificates (for corporate proxies) | -| `SSL_CERT_FILE` | Fallback CA certificate bundle path | See [Configuration](./configuration/) for the full environment variable reference.