diff --git a/packages/cli/__tests__/commands/data.test.ts b/packages/cli/__tests__/commands/data.test.ts new file mode 100644 index 00000000..1bf9cd8e --- /dev/null +++ b/packages/cli/__tests__/commands/data.test.ts @@ -0,0 +1,51 @@ +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import yargs from 'yargs'; +import { prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { dataCommand } from '@commands/index.js'; + +function run(args: string[]) { + let output = ''; + const sut = yargs(args).scriptName('confidence').command(dataCommand).help().exitProcess(false); + return sut + .parseAsync(args, (_error: Error | undefined, _argv: unknown, text: string) => { + output = text; + }) + .then(() => output); +} + +describe('data command group', () => { + it('is registered in the real root help alongside existing commands', () => { + using _auth = prepareAuthTokens('none'); + const sut = fileURLToPath(new URL('../../bin/cli.ts', import.meta.url)); + + const output = execFileSync(process.execPath, ['--import', 'tsx', sut, '--help'], { + encoding: 'utf8', + env: { ...process.env, CONFIDENCE_TELEMETRY: 'false' }, + }); + + expect(output).toContain('confidence data'); + expect(output).toContain('confidence warehouse'); + expect(output).toContain('confidence flags'); + }); + + it('shows help without authentication or placeholder API subcommands', async () => { + using _auth = prepareAuthTokens('none'); + const sut = run; + + const output = await sut(['data', '--help']); + + expect(output).toContain('confidence data '); + expect(output).not.toMatch(/onboard|discover|proposal|workflow/iu); + }); + + it('shows guidance when invoked without a subcommand', async () => { + const sut = run; + expect(await sut(['data'])).toContain('confidence data --help'); + }); + + it('rejects unimplemented subcommands', async () => { + const sut = run; + expect(await sut(['data', 'invented'])).toContain('Unknown argument: invented'); + }); +}); diff --git a/packages/cli/__tests__/features/data/dry-run.test.ts b/packages/cli/__tests__/features/data/dry-run.test.ts new file mode 100644 index 00000000..ad31223c --- /dev/null +++ b/packages/cli/__tests__/features/data/dry-run.test.ts @@ -0,0 +1,284 @@ +import { mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { resetTelemetry } from '@spotify-confidence/core'; +import { executeDataRequest, printData } from '@features/data/index.js'; +import { metricsRequest } from '@network/metrics.js'; +import { requireRecord, requireString } from '@network/metrics-types.js'; +import { flushTelemetry, telemetryMiddleware } from '@utils/telemetry.js'; +import { captureOutput } from '../../helpers/capture.js'; + +const TEST_URL = 'https://metrics.eu.confidence.dev/v1/testResources'; + +function inputFile(contents: string) { + const dir = mkdtempSync(join(tmpdir(), 'confidence-data-test-')); + const path = join(dir, 'input.json'); + writeFileSync(path, contents); + return { + path, + [Symbol.dispose]() { + rmSync(dir, { recursive: true, force: true }); + }, + }; +} + +// A feature-like caller validates real input before the shared execution helper. +async function runValidatedFile(argv: Record) { + const input = requireRecord( + JSON.parse(readFileSync(argv['from-file'] as string, 'utf8')), + 'Input', + ); + requireString(input.displayName, 'displayName'); + await executeDataRequest( + argv, + (token) => metricsRequest(token, { path: '/v1/testResources', method: 'POST', body: input }), + { method: 'POST', path: '/v1/testResources', body: input }, + ); +} + +afterEach(() => { + resetTelemetry(); + vi.unstubAllEnvs(); +}); + +describe('Data dry-runs', () => { + it('previews validated input before auth with no HTTP or file writes', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + const configDir = process.env.CONFIDENCE_CONFIG_DIR!; + const before = readdirSync(configDir, { recursive: true }); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({}); + }), + ); + const sut = runValidatedFile; + + // Act + await sut({ 'dry-run': true, 'from-file': file.path, json: true, profile: '../invalid' }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ + data: { method: 'POST', path: '/v1/testResources', body: { displayName: 'Example' } }, + }); + expect(output.stderr).toBe(''); + expect(calls).toBe(0); + expect(readdirSync(configDir, { recursive: true })).toEqual(before); + expect(readFileSync(file.path, 'utf8')).toBe('{"displayName":"Example"}'); + }); + + it.each(['invalid JSON', '[]', '{"displayName":42}'])( + 'still rejects invalid local input %s before preview or auth', + async (contents) => { + using _auth = prepareAuthTokens('none'); + using file = inputFile(contents); + using output = captureOutput(); + const sut = runValidatedFile; + + await expect(sut({ 'dry-run': true, 'from-file': file.path, json: true })).rejects.toThrow(); + + expect(output.stdout).toBe(''); + expect(output.stderr).toBe(''); + }, + ); + + it('has a useful local preview when none is supplied', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({ 'dry-run': true, json: true }, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(JSON.parse(output.stdout)).toEqual({ data: { dryRun: true } }); + expect(calls).toBe(0); + }); +}); + +describe('Data request execution', () => { + it.each(['none', 'expired'] as const)( + 'fails without usable %s credentials before sending', + async (auth) => { + using _auth = prepareAuthTokens(auth); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({}, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(calls).toBe(0); + expect(output.stderr).toMatch(/Not logged in|Token expired/u); + expect(process.exitCode).toBe(1); + }, + ); + + it('sends once and preserves state, errors, and pagination in the JSON envelope', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + let calls = 0; + const data = { state: 'FAILED', error: { message: 'Resource failed' }, nextPageToken: 'next' }; + server.use( + http.post(TEST_URL, () => { + calls++; + return HttpResponse.json(data); + }), + ); + const previousExitCode = process.exitCode; + const sut = runValidatedFile; + + // Act + await sut({ 'from-file': file.path, json: true }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data }); + expect(output.stderr).toBe(''); + expect(process.exitCode).toBe(previousExitCode); + expect(calls).toBe(1); + }); + + it('uses the requested auth profile', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const profileDir = join(process.env.CONFIDENCE_CONFIG_DIR!, 'profiles', 'metrics'); + const token = buildTestJwt({ email: 'profile@example.com' }); + mkdirSync(profileDir, { recursive: true }); + writeFileSync(join(profileDir, 'credentials.json'), JSON.stringify({ accessToken: token })); + server.use( + http.get(TEST_URL, ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = executeDataRequest; + + // Act + await sut({ profile: 'metrics', json: true }, (value) => + metricsRequest(value, { path: '/v1/testResources' }), + ); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data: { authorized: true } }); + }); + + it('prints a sanitized failure, omits arbitrary details, and exits nonzero without replay', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let calls = 0; + server.use( + http.get(TEST_URL, () => { + calls++; + return HttpResponse.json( + { + code: 7, + message: 'Denied\n\u001b[31m Bearer private-credential', + details: [{ secret: 'private' }], + }, + { status: 403 }, + ); + }), + ); + const sut = executeDataRequest; + + // Act + await sut({ json: true }, (token) => metricsRequest(token, { path: '/v1/testResources' })); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('Denied'); + expect(output.stderr).not.toContain('private'); + expect(output.stderr).not.toContain('\u001b'); + expect(process.exitCode).toBe(1); + expect(calls).toBe(1); + }); + + it('reports thrown response validation errors as nonzero failures', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const sut = executeDataRequest; + + await sut({}, async () => { + throw new Error('Invalid resource response'); + }); + + expect(output.stderr).toContain('Invalid resource response'); + expect(process.exitCode).toBe(1); + }); + + it('renders nested resources as readable output when requested', () => { + using output = captureOutput(); + const sut = printData; + + sut({ name: 'testResources/a', status: { state: 'PENDING' } }, { output: 'table' }); + + expect(output.stdout).toContain('Field'); + expect(output.stdout).toContain('testResources/a'); + expect(output.stdout).toContain('PENDING'); + expect(output.stdout).not.toContain('[object Object]'); + }); +}); + +describe('dry-run telemetry suppression', () => { + it.each([ + ['data', 'entities', 'list'], + ['data', 'sql', 'preview'], + ['warehouse', 'list'], + ['warehouse', 'get'], + ])('sends no telemetry for %j even when telemetry is enabled', async (...command) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({ clientSecret: 'test' }); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': true }); + await flushTelemetry(); + + expect(calls).toBe(0); + }); + + it.each([ + { command: ['flags', 'list'], dryRun: true }, + { command: ['warehouse', 'create'], dryRun: true }, + { command: ['data', 'entities', 'list'], dryRun: false }, + ])('preserves existing telemetry behavior for %j', async ({ command, dryRun }) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + const steps: string[] = []; + server.use( + http.post('https://onboarding.confidence.dev/v1/agentTelemetryKey:acquire', () => + HttpResponse.json({ clientSecret: 'test' }), + ), + http.post('https://events.eu.confidence.dev/v1/events:publish', async ({ request }) => { + const body = (await request.json()) as { events: { payload: { step: string } }[] }; + steps.push(body.events[0].payload.step); + return HttpResponse.json({}); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': dryRun }); + await flushTelemetry(); + + expect(steps).toEqual([command.join('.')]); + }); +}); diff --git a/packages/cli/__tests__/network/metrics.test.ts b/packages/cli/__tests__/network/metrics.test.ts new file mode 100644 index 00000000..ea9cc088 --- /dev/null +++ b/packages/cli/__tests__/network/metrics.test.ts @@ -0,0 +1,194 @@ +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt } from '@spotify-confidence/testing/auth'; +import { metricsRequest } from '@network/metrics.js'; +import { isRecord, requireRecord, requireString, resourcePath } from '@network/metrics-types.js'; + +afterEach(() => vi.unstubAllEnvs()); + +describe('Metrics requests', () => { + it.each(['EU', 'US'])('routes to %s and authenticates the request', async (region) => { + const token = buildTestJwt({ 'https://confidence.dev/region': region }); + server.use( + http.get( + `https://metrics.${region.toLowerCase()}.confidence.dev/v1/testResources`, + ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(token, { path: '/v1/testResources' }); + + expect(result).toEqual({ ok: true, data: { authorized: true } }); + }); + + it('honors the API base override and sends body and explicit pagination parameters', async () => { + vi.stubEnv('CONFIDENCE_API_BASE_URL', 'http://localhost:9876'); + server.use( + http.post('http://localhost:9876/v1/testResources', async ({ request }) => + HttpResponse.json({ + body: await request.json(), + params: Object.fromEntries(new URL(request.url).searchParams), + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }); + + expect(result).toEqual({ + ok: true, + data: { + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }, + }); + }); + + it('preserves structured failures without retrying writes', async () => { + let calls = 0; + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use( + http.post('https://metrics.eu.confidence.dev/v1/testResources', () => { + calls++; + return HttpResponse.json(error, { status: 400 }); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { path: '/v1/testResources', method: 'POST' }); + + expect(result).toEqual({ ok: false, status: 400, error }); + expect(calls).toBe(1); + }); + + it('does not replay a redirected write or send credentials to its location', async () => { + let redirectedCalls = 0; + server.use( + http.post( + 'https://metrics.eu.confidence.dev/v1/testResources', + () => + new HttpResponse(null, { + status: 307, + headers: { Location: 'https://metrics.eu.confidence.dev/v1/redirected' }, + }), + ), + http.post('https://metrics.eu.confidence.dev/v1/redirected', () => { + redirectedCalls++; + return HttpResponse.json({}); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + expect(redirectedCalls).toBe(0); + }); + + it('passes cancellation through to the HTTP client', async () => { + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + signal: AbortSignal.abort(), + timeoutMs: 100, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + }); + + it.each([ + 'https://other.example/v1/testResources', + '//other.example/v1/testResources', + '/v1/../admin', + '/v1/%2e%2e/admin', + '/v1/testResources?secret=value', + '/v1/testResources#fragment', + '/v1/\\other.example', + ])('rejects unsafe API path %s before sending', async (path) => { + const sut = metricsRequest; + await expect(sut(buildTestJwt(), { path })).rejects.toThrow('local /v1/ API path'); + }); +}); + +describe('resource paths', () => { + it('builds a path from a qualified resource with exactly one ID', () => { + const sut = resourcePath; + expect(sut('testResources/abc-123_A', 'testResources')).toBe('/v1/testResources/abc-123_A'); + }); + + it.each([ + '', + 'abc', + '/testResources/abc', + 'https://evil.example/testResources/abc', + '//evil.example/testResources/abc', + 'other/abc', + 'testResources/', + 'testResources/.', + 'testResources/..', + 'testResources/../other', + 'testResources/abc/child', + 'testResources/%2e%2e', + 'testResources/%252fadmin', + 'testResources/a%2fb', + 'testResources/a?query=x', + 'testResources/a#fragment', + 'testResources/a\\b', + 'testResources/a b', + 'testResources/abc\n', + ])('rejects invalid resource name %j', (name) => { + const sut = resourcePath; + expect(() => sut(name, 'testResources')).toThrow('single valid ID'); + }); + + it.each(['../testResources', '/testResources', 'testResources/child', 'testResources?x=y'])( + 'rejects invalid collection %s', + (collection) => { + const sut = resourcePath; + expect(() => sut('testResources/abc', collection)).toThrow('Invalid resource collection'); + }, + ); +}); + +describe('response guards', () => { + it.each([null, [], 'text', 42, undefined].map((value) => ({ value })))( + 'rejects non-record value $value', + ({ value }) => { + const sut = requireRecord; + expect(isRecord(value)).toBe(false); + expect(() => sut(value, 'Response')).toThrow('Response must be an object'); + }, + ); + + it('preserves resource fields rather than discarding unknown ones', () => { + const resource = { name: 'testResources/a', state: 'FAILED', extra: { reason: 'example' } }; + const sut = requireRecord; + expect(sut(resource)).toBe(resource); + }); + + it.each(['', ' ', null, [], 42].map((value) => ({ value })))( + 'rejects empty or non-string value $value', + ({ value }) => { + const sut = requireString; + expect(() => sut(value, 'Name')).toThrow('Name must be a non-empty string'); + }, + ); + + it('does not silently rewrite strings', () => { + const sut = requireString; + expect(sut(' original ')).toBe(' original '); + }); +}); diff --git a/packages/cli/bin/cli.ts b/packages/cli/bin/cli.ts index 1e9f9157..f52a9689 100644 --- a/packages/cli/bin/cli.ts +++ b/packages/cli/bin/cli.ts @@ -20,6 +20,7 @@ import { pluginCommand, migrateCommand, warehouseCommand, + dataCommand, } from '../src/commands/index.js'; const cli = yargs(hideBin(process.argv)) @@ -88,6 +89,7 @@ const cli = yargs(hideBin(process.argv)) .command(pluginCommand) .command(migrateCommand) .command(warehouseCommand) + .command(dataCommand) .example('$0 login', 'Sign in to Confidence') .example('$0 flags setup', 'Set up feature flags in your project') .example('$0 docs search "feature flags"', 'Search the documentation') diff --git a/packages/cli/src/commands/data.ts b/packages/cli/src/commands/data.ts new file mode 100644 index 00000000..ec20479f --- /dev/null +++ b/packages/cli/src/commands/data.ts @@ -0,0 +1,16 @@ +import type { Argv } from 'yargs'; +import { noop } from '@spotify-confidence/shared-kernel'; + +/** Feature PRs register their subcommands in this group. */ +export const dataCommand = { + command: 'data', + describe: 'Work with Confidence data resources', + builder(yargs: Argv) { + return yargs + .usage('$0 data [options]') + .demandCommand(1, 'Run "confidence data --help" to see available commands.') + .strict() + .help(); + }, + handler: noop, +}; diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 84dd9b05..fbbee90a 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -13,4 +13,5 @@ export { pluginCommand } from './plugin.js'; export { updateCommand } from './update.js'; export { migrateCommand } from './migrate.js'; export { warehouseCommand } from './warehouse.js'; +export { dataCommand } from './data.js'; export type { GlobalFlags } from './types.js'; diff --git a/packages/cli/src/features/data/index.ts b/packages/cli/src/features/data/index.ts new file mode 100644 index 00000000..a4c9385e --- /dev/null +++ b/packages/cli/src/features/data/index.ts @@ -0,0 +1,2 @@ +export { printData } from './output.js'; +export { executeDataRequest } from './request.js'; diff --git a/packages/cli/src/features/data/output.ts b/packages/cli/src/features/data/output.ts new file mode 100644 index 00000000..06ba6ba0 --- /dev/null +++ b/packages/cli/src/features/data/output.ts @@ -0,0 +1,26 @@ +import { inspect } from 'node:util'; +import { isRecord } from '@network/metrics-types.js'; +import { extractFlags, formatJson, message, print, resolveFormat } from '@output/index.js'; + +/** Preserve the full response in JSON; render generic resources as key/value rows otherwise. */ +export function printData(data: unknown, argv: Record): void { + const flags = extractFlags(argv); + if (resolveFormat(flags) === 'json') { + message(formatJson(data)); + return; + } + + const fields = isRecord(data) ? data : { value: data }; + const rows = Object.entries(fields).map(([field, value]) => ({ + field, + value: typeof value === 'string' ? value : inspect(value, { depth: null, colors: false }), + })); + print({ + data: rows, + columns: [ + { key: 'field', header: 'Field' }, + { key: 'value', header: 'Value' }, + ], + flags, + }); +} diff --git a/packages/cli/src/features/data/request.ts b/packages/cli/src/features/data/request.ts new file mode 100644 index 00000000..6739ebcb --- /dev/null +++ b/packages/cli/src/features/data/request.ts @@ -0,0 +1,33 @@ +import type { ApiResponse } from '@spotify-confidence/core'; +import { printApiError } from '@utils/api-error.js'; +import { withAuth } from '@utils/auth.js'; +import { printData } from './output.js'; + +/** Callers must validate arguments and input before invoking this helper, including dry-runs. */ +export async function executeDataRequest( + argv: Record, + request: (token: string) => Promise>, + preview?: unknown, +): Promise { + if (argv['dry-run'] === true) { + printData(preview ?? { dryRun: true }, argv); + return; + } + + await withAuth(async (_argv, token) => { + try { + const result = await request(token); + if (!result.ok) { + printApiError(result); + return; + } + printData(result.data, argv); + } catch (err) { + printApiError({ + ok: false, + status: 0, + error: { code: 0, message: err instanceof Error ? err.message : 'Data request failed.' }, + }); + } + })(argv); +} diff --git a/packages/cli/src/network/index.ts b/packages/cli/src/network/index.ts index 609492b2..a63a9054 100644 --- a/packages/cli/src/network/index.ts +++ b/packages/cli/src/network/index.ts @@ -1,4 +1,6 @@ export { fetchLatestVersion } from './registry.js'; +export { metricsRequest } from './metrics.js'; +export { isRecord, requireRecord, requireString, resourcePath } from './metrics-types.js'; export { searchDocumentation, grepDocumentation, getFullSource } from './docs.js'; diff --git a/packages/cli/src/network/metrics-types.ts b/packages/cli/src/network/metrics-types.ts new file mode 100644 index 00000000..ac6f61dd --- /dev/null +++ b/packages/cli/src/network/metrics-types.ts @@ -0,0 +1,28 @@ +/** Small response/input guards; resource-specific validation belongs to each feature. */ +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export function requireRecord(value: unknown, label = 'Value'): Record { + if (!isRecord(value)) throw new Error(`${label} must be an object.`); + return value; +} + +export function requireString(value: unknown, label = 'Value'): string { + if (typeof value !== 'string' || value.trim().length === 0) { + throw new Error(`${label} must be a non-empty string.`); + } + return value; +} + +/** A qualified resource name must contain exactly one unescaped resource ID. */ +export function resourcePath(name: string, collection: string): string { + if (!/^[A-Za-z][A-Za-z0-9]*$/u.test(collection)) { + throw new Error('Invalid resource collection.'); + } + const prefix = `${collection}/`; + if (!name.startsWith(prefix) || !/^[A-Za-z0-9][A-Za-z0-9_-]*$/u.test(name.slice(prefix.length))) { + throw new Error(`Resource name must be ${collection}/ with a single valid ID.`); + } + return `/v1/${name}`; +} diff --git a/packages/cli/src/network/metrics.ts b/packages/cli/src/network/metrics.ts new file mode 100644 index 00000000..30b32781 --- /dev/null +++ b/packages/cli/src/network/metrics.ts @@ -0,0 +1,30 @@ +import { + apiRequest, + extractRegion, + type ApiRequestOptions, + type ApiResponse, +} from '@spotify-confidence/core'; + +/** Send one authenticated request to the token's regional Metrics API. */ +export async function metricsRequest( + token: string, + options: Omit, +): Promise> { + // Accept API paths, never absolute URLs or response-supplied pagination URLs. + if ( + !options.path.startsWith('/v1/') || + /[\\%?#\s]/u.test(options.path) || + options.path.split('/').some((part) => part === '.' || part === '..') + ) { + throw new Error('Metrics requests require a local /v1/ API path.'); + } + + return apiRequest({ + ...options, + token, + region: extractRegion(token), + service: 'metrics', + // Redirects could replay writes or send credentials to an API-supplied URL. + redirect: 'error', + }); +} diff --git a/packages/cli/src/utils/api-error.ts b/packages/cli/src/utils/api-error.ts new file mode 100644 index 00000000..4022ce40 --- /dev/null +++ b/packages/cli/src/utils/api-error.ts @@ -0,0 +1,11 @@ +import type { ApiFailure } from '@spotify-confidence/core'; +import { fail } from '@output/index.js'; + +/** Render only the API message, not arbitrary response bodies or error details. */ +export function printApiError(failure: ApiFailure): void { + const message = failure.error.message + .replace(/Bearer\s+\S+/giu, 'Bearer [redacted]') + .replace(/[\p{Cc}\p{Cf}]/gu, ' ') + .trim(); + fail(message || `API request failed (HTTP ${failure.status}).`); +} diff --git a/packages/cli/src/utils/index.ts b/packages/cli/src/utils/index.ts index 248d517a..90f45a7e 100644 --- a/packages/cli/src/utils/index.ts +++ b/packages/cli/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './auth.js'; +export * from './api-error.js'; export * from './mcp-error.js'; export * from './safely.js'; export * from './telemetry.js'; diff --git a/packages/cli/src/utils/telemetry.ts b/packages/cli/src/utils/telemetry.ts index 6733d36d..b0381671 100644 --- a/packages/cli/src/utils/telemetry.ts +++ b/packages/cli/src/utils/telemetry.ts @@ -2,7 +2,18 @@ import { initTelemetry, isTelemetryEnabled, getTelemetry, flush } from '@spotify export { flush as flushTelemetry }; -export function telemetryMiddleware(argv: { telemetry: boolean; _: (string | number)[] }) { +export function telemetryMiddleware(argv: { + telemetry: boolean; + _: (string | number)[]; + 'dry-run'?: boolean; +}) { + const [group, action] = argv._; + if ( + argv['dry-run'] === true && + (group === 'data' || (group === 'warehouse' && (action === 'list' || action === 'get'))) + ) { + return; + } if (!isTelemetryEnabled() || argv.telemetry === false) return; initTelemetry({ source: 'cli' }); diff --git a/packages/core/__tests__/api/client.test.ts b/packages/core/__tests__/api/client.test.ts index 981da382..77849826 100644 --- a/packages/core/__tests__/api/client.test.ts +++ b/packages/core/__tests__/api/client.test.ts @@ -1,7 +1,14 @@ import { http, HttpResponse, server } from '@spotify-confidence/testing'; import { resolveBaseUrl, apiRequest } from '@api/client.js'; +import type { ApiRequestOptions } from '@api/types.js'; const TEST_BASE = 'https://test-svc.eu.confidence.dev'; +const requestOptions: ApiRequestOptions = { + token: 'test-token', + region: 'EU', + service: 'test-svc', + path: '/v1/items', +}; beforeEach(() => { server.use( @@ -54,6 +61,175 @@ describe('resolveBaseUrl', () => { }); }); +describe('apiRequest response safety', () => { + it.each(['not JSON', '', 'private upstream error'])( + 'returns a controlled failure for malformed success body %j', + async (body) => { + server.use(http.get(`${TEST_BASE}/v1/items`, () => new HttpResponse(body))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 200, + error: { code: 200, message: 'Invalid JSON response from API.' }, + }); + }, + ); + + it.each( + [null, [], 'error', { message: {} }, { code: 'bad', message: 'invalid' }].map((body) => ({ + body, + })), + )('normalizes malformed error response $body', async ({ body }) => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json(body, { status: 503, statusText: 'Service Unavailable' }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 503, + error: { code: 503, message: 'Service Unavailable' }, + }); + }); + + it('does not expose non-JSON error bodies', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => new HttpResponse('upstream private details', { status: 502 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 502, + error: { code: 502, message: 'Bad Gateway' }, + }); + }); + + it('preserves structured API errors and details', async () => { + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use(http.get(`${TEST_BASE}/v1/items`, () => HttpResponse.json(error, { status: 400 }))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ ok: false, status: 400, error }); + }); + + it('discards malformed error details', async () => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json({ code: 3, message: 'Invalid input', details: 'bad' }, { status: 400 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 400, + error: { code: 3, message: 'Invalid input' }, + }); + }); + + it('returns a controlled failure if reading the body fails', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => + new HttpResponse( + new ReadableStream({ + start(controller) { + controller.error(new Error('Connection closed')); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + if (!result.ok) expect(result.error.message).toContain('Network error'); + }); +}); + +describe('apiRequest cancellation', () => { + it('honors an already cancelled caller signal', async () => { + const sut = apiRequest; + + const result = await sut({ ...requestOptions, signal: AbortSignal.abort() }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels before headers via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get(`${TEST_BASE}/v1/items`, async ({ request }) => { + if (mode === 'caller') controller.abort(); + await new Promise((resolve) => { + if (request.signal.aborted) resolve(); + else request.signal.addEventListener('abort', () => resolve(), { once: true }); + }); + return HttpResponse.json({}); + }), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels body consumption via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get( + `${TEST_BASE}/v1/items`, + ({ request }) => + new HttpResponse( + new ReadableStream({ + start(body) { + body.enqueue(new TextEncoder().encode('{')); + request.signal.addEventListener('abort', () => body.error(request.signal.reason), { + once: true, + }); + if (mode === 'caller') queueMicrotask(() => controller.abort()); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); +}); + describe('apiRequest', () => { it('sends an authorized GET request', async () => { const result = await apiRequest<{ auth: string }>({ diff --git a/packages/core/src/api/client.ts b/packages/core/src/api/client.ts index a4501eee..71aecdbf 100644 --- a/packages/core/src/api/client.ts +++ b/packages/core/src/api/client.ts @@ -25,32 +25,68 @@ export async function apiRequest(opts: ApiRequestOptions): Promise; + /** Cancels both the request and response body consumption. */ + signal?: AbortSignal; + /** Redirect policy. Defaults to fetch's existing follow behavior. */ + redirect?: 'follow' | 'error'; + /** Overall deadline in milliseconds, including response body consumption. Defaults to 30 seconds. */ + timeoutMs?: number; };