diff --git a/src/lib/client-factory.test.ts b/src/lib/client-factory.test.ts index f18568e..f9fdc44 100644 --- a/src/lib/client-factory.test.ts +++ b/src/lib/client-factory.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import { DRY_RUN_API_KEY, DRY_RUN_BANNER, + assertValidEndpointUrl, emitDryRunBanner, makeHttpClient, resetDryRunBannerForTesting, @@ -12,6 +13,7 @@ import { REQUEST_TIMEOUT_MAX_MS, REQUEST_TIMEOUT_MIN_MS, } from './http.js'; +import { ApiError } from './errors.js'; const NO_CREDS_PATH = '/tmp/testsprite-cli-test-no-such-file-1234.ini'; @@ -284,3 +286,88 @@ describe('makeHttpClient — real path (regression)', () => { expect(stderrLines).not.toContain(DRY_RUN_BANNER); }); }); + +// --------------------------------------------------------------------------- +// assertValidEndpointUrl — endpoint syntax guard (NOT an SSRF guard) +// --------------------------------------------------------------------------- + +describe('assertValidEndpointUrl', () => { + it('accepts http(s) URLs, including private / localhost hosts (self-hosted, dev, mock)', () => { + for (const url of [ + 'https://api.testsprite.com', + 'http://localhost:3000', + 'http://127.0.0.1:8787', + 'https://testsprite.internal.example.com/api/cli/v1', + ]) { + expect(() => assertValidEndpointUrl(url)).not.toThrow(); + } + }); + + it('rejects an unparseable URL with a VALIDATION_ERROR (exit 5)', () => { + let caught: unknown; + try { + assertValidEndpointUrl('not a url'); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiError); + const apiErr = caught as ApiError; + expect(apiErr.code).toBe('VALIDATION_ERROR'); + expect(apiErr.exitCode).toBe(5); + expect(apiErr.nextAction).toContain('endpoint-url'); + }); + + it('rejects a missing scheme (parses as a bogus scheme) and a non-http(s) scheme', () => { + // `new URL('localhost:3000')` does not throw — it parses with protocol + // `localhost:`. Without the scheme check this would sail through and later + // fail as a retried "fetch failed". + for (const url of ['localhost:3000', 'ftp://example.com', 'file:///etc/hosts']) { + let caught: unknown; + try { + assertValidEndpointUrl(url); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiError); + expect((caught as ApiError).code).toBe('VALIDATION_ERROR'); + expect((caught as ApiError).exitCode).toBe(5); + } + }); +}); + +describe('makeHttpClient — endpoint validation', () => { + afterEach(() => { + resetDryRunBannerForTesting(); + }); + + it('throws a VALIDATION_ERROR (exit 5) on a malformed --endpoint-url under --dry-run', () => { + let caught: unknown; + try { + makeHttpClient( + { profile: 'default', output: 'json', debug: false, dryRun: true, endpointUrl: 'ftp://x' }, + { env: {} as NodeJS.ProcessEnv, credentialsPath: NO_CREDS_PATH, stderr: () => {} }, + ); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiError); + expect((caught as ApiError).exitCode).toBe(5); + }); + + it('rejects a malformed endpoint before the auth check on the real path', () => { + // No API key is configured, but the malformed endpoint is reported first + // (a deterministic config error) — exit 5, not exit 3. + let caught: unknown; + try { + makeHttpClient( + { profile: 'default', output: 'json', debug: false, dryRun: false, endpointUrl: 'nope' }, + { env: {} as NodeJS.ProcessEnv, credentialsPath: NO_CREDS_PATH, stderr: () => {} }, + ); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ApiError); + expect((caught as ApiError).code).toBe('VALIDATION_ERROR'); + expect((caught as ApiError).exitCode).toBe(5); + }); +}); diff --git a/src/lib/client-factory.ts b/src/lib/client-factory.ts index fad2fab..fa1f8dc 100644 --- a/src/lib/client-factory.ts +++ b/src/lib/client-factory.ts @@ -15,7 +15,7 @@ */ import { loadConfig } from './config.js'; import { defaultCredentialsPath } from './credentials.js'; -import { ApiError } from './errors.js'; +import { ApiError, localValidationError } from './errors.js'; import { facadeBaseUrl } from './facade.js'; import type { DebugEvent, FetchImpl } from './http.js'; import { @@ -139,15 +139,61 @@ function clampRequestTimeout(ms: number): number { return Math.min(Math.max(Math.round(ms), REQUEST_TIMEOUT_MIN_MS), REQUEST_TIMEOUT_MAX_MS); } +/** + * Validate that the resolved API endpoint is a syntactically valid http(s) + * URL before it is used to build requests. + * + * Unlike the `--target-url` guard in `target-url.ts`, this deliberately does + * NOT reject localhost or private addresses: the API endpoint legitimately + * points at a self-hosted, local-dev, or mock backend on a private host. It + * only catches a malformed value (unparseable, or a non-http(s) scheme) so the + * operator gets a fast, actionable VALIDATION_ERROR (exit 5) instead of an + * opaque `Invalid URL` (exit 1, a raw `new URL()` throw) or a misleading + * `fetch failed / Service temporarily unavailable` emitted only after a full + * retry-and-backoff cycle. + * + * The value can originate from `--endpoint-url`, `TESTSPRITE_API_URL`, or the + * credentials file `api_url`, so the message names all three rather than + * assuming the flag. + */ +export function assertValidEndpointUrl(rawUrl: string): void { + let parsed: URL; + try { + parsed = new URL(rawUrl); + } catch { + throw localValidationError( + 'endpoint-url', + `"${rawUrl}" is not a valid URL — provide an http(s) URL (e.g. https://api.testsprite.com) ` + + `via --endpoint-url, TESTSPRITE_API_URL, or the credentials file`, + undefined, + 'field', + ); + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + throw localValidationError( + 'endpoint-url', + `scheme "${parsed.protocol.replace(/:$/, '')}" is not supported — use http or https ` + + `(e.g. https://api.testsprite.com)`, + undefined, + 'field', + ); + } +} + export function makeHttpClient(opts: CommonOptions, deps: ClientFactoryDeps = {}): HttpClient { const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); const env = deps.env ?? process.env; const requestTimeoutMs = resolveRequestTimeoutMs(opts, env); if (opts.dryRun) { + const dryRunEndpoint = opts.endpointUrl ?? DRY_RUN_DEFAULT_ENDPOINT; + // Validate even under --dry-run so a typo in --endpoint-url is caught + // offline (no creds, no network). Validate BEFORE the banner so a rejected + // endpoint doesn't first announce a "sample response". + assertValidEndpointUrl(dryRunEndpoint); emitDryRunBanner(stderr); return new HttpClient({ - baseUrl: facadeBaseUrl(opts.endpointUrl ?? DRY_RUN_DEFAULT_ENDPOINT), + baseUrl: facadeBaseUrl(dryRunEndpoint), apiKey: DRY_RUN_API_KEY, fetchImpl: deps.fetchImpl ?? createDryRunFetch(), onDebug: opts.debug ? (event: DebugEvent) => stderr(formatDryRunDebug(event)) : undefined, @@ -163,6 +209,10 @@ export function makeHttpClient(opts: CommonOptions, deps: ClientFactoryDeps = {} env, credentialsPath, }); + // Catch a malformed endpoint (from --endpoint-url / TESTSPRITE_API_URL / + // credentials) before the auth check so a config typo surfaces as a clear + // VALIDATION_ERROR rather than an opaque URL throw or a retried "fetch failed". + assertValidEndpointUrl(config.apiUrl); if (!config.apiKey) throw ApiError.authRequired(); return new HttpClient({ baseUrl: facadeBaseUrl(config.apiUrl), diff --git a/test/cli.subprocess.test.ts b/test/cli.subprocess.test.ts index 8cbfa59..0d6f966 100644 --- a/test/cli.subprocess.test.ts +++ b/test/cli.subprocess.test.ts @@ -499,6 +499,35 @@ describe('project list subprocess', () => { }, 30_000); }); +describe('malformed --endpoint-url is rejected (exit 5), not retried as a network error', () => { + // Previously: a malformed endpoint surfaced either as an opaque `Invalid URL` + // (exit 1) or, for a missing/wrong scheme, as a `fetch failed` UNAVAILABLE + // only after a full retry-and-backoff cycle. Both are misleading config + // errors. Validation throws before any fetch, so no network is hit here even + // though a (dummy) key is configured. + + it('an unparseable endpoint exits 5 with a VALIDATION_ERROR naming endpoint-url', async () => { + const result = await runCli( + ['--output', 'json', '--endpoint-url', 'not a url', 'project', 'list'], + { TESTSPRITE_API_KEY: 'sk-subproc' }, + ); + expect(result.exitCode).toBe(5); + const parsed = JSON.parse(result.stderr) as { error: { code: string; nextAction: string } }; + expect(parsed.error.code).toBe('VALIDATION_ERROR'); + expect(parsed.error.nextAction).toContain('endpoint-url'); + }, 30_000); + + it('a non-http(s) scheme exits 5 instead of being retried as a network failure', async () => { + const result = await runCli( + ['--output', 'json', '--endpoint-url', 'ftp://example.com', 'project', 'list'], + { TESTSPRITE_API_KEY: 'sk-subproc' }, + ); + expect(result.exitCode).toBe(5); + const parsed = JSON.parse(result.stderr) as { error: { code: string } }; + expect(parsed.error.code).toBe('VALIDATION_ERROR'); + }, 30_000); +}); + describe('project get subprocess', () => { it('--output json returns the §6.1 Project shape', async () => { const result = await runCli(['--output', 'json', 'project', 'get', 'project_subproc'], {