Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions src/lib/client-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it } from 'vitest';
import {
DRY_RUN_API_KEY,
DRY_RUN_BANNER,
assertValidEndpointUrl,
emitDryRunBanner,
makeHttpClient,
resetDryRunBannerForTesting,
Expand All @@ -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';

Expand Down Expand Up @@ -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);
});
});
54 changes: 52 additions & 2 deletions src/lib/client-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down
29 changes: 29 additions & 0 deletions test/cli.subprocess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'], {
Expand Down