fix(cli): reject a malformed --endpoint-url with a clear VALIDATION_ERROR (#19)

A malformed API endpoint produced an opaque or misleading failure instead
of a clear config error:

  --endpoint-url "not a url"   -> `Error: Invalid URL` (exit 1, a raw
                                  `new URL()` throw with no guidance)
  --endpoint-url "localhost:3000" (missing scheme, parses as scheme
                                  "localhost:") and "ftp://x" (wrong scheme)
                               -> `fetch failed` / Service unavailable,
                                  emitted only after a full retry+backoff
                                  cycle — looks like a network outage, not a
                                  config typo.

Add `assertValidEndpointUrl` in client-factory.ts and run it in both the
real and dry-run paths of `makeHttpClient`, on the resolved endpoint (so it
covers --endpoint-url, TESTSPRITE_API_URL, and the credentials file). A
malformed value now throws a typed VALIDATION_ERROR (exit 5) with an
actionable message.

Crucially, and unlike the `--target-url` SSRF guard, this does NOT reject
localhost or private hosts — the API endpoint legitimately points at a
self-hosted, local-dev, or mock backend. Only syntactically invalid values
(unparseable, or a non-http(s) scheme) are rejected, so existing
self-hosted/CI configs and the test suite's localhost mock backend are
unaffected.

Adds unit coverage for assertValidEndpointUrl and the two makeHttpClient
paths, plus subprocess regressions.

Author: Davidson3556

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);
+  });
+});

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),

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'], {