From d2496f8b0b409d3d92f631c36cff278e57ef2761 Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 15:30:03 +0200 Subject: [PATCH 1/2] refactor(cli): add shared Metrics HTTP command foundation Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/__tests__/commands/data.test.ts | 51 ++++ .../__tests__/features/data/dry-run.test.ts | 284 ++++++++++++++++++ .../cli/__tests__/network/metrics.test.ts | 194 ++++++++++++ packages/cli/bin/cli.ts | 2 + packages/cli/src/commands/data.ts | 16 + packages/cli/src/commands/index.ts | 1 + packages/cli/src/features/data/index.ts | 2 + packages/cli/src/features/data/output.ts | 26 ++ packages/cli/src/features/data/request.ts | 34 +++ packages/cli/src/network/index.ts | 2 + packages/cli/src/network/metrics-types.ts | 28 ++ packages/cli/src/network/metrics.ts | 30 ++ packages/cli/src/utils/api-error.ts | 11 + packages/cli/src/utils/index.ts | 1 + packages/cli/src/utils/telemetry.ts | 13 +- packages/core/__tests__/api/client.test.ts | 176 +++++++++++ packages/core/src/api/client.ts | 66 +++- packages/core/src/api/types.ts | 6 + 18 files changed, 927 insertions(+), 16 deletions(-) create mode 100644 packages/cli/__tests__/commands/data.test.ts create mode 100644 packages/cli/__tests__/features/data/dry-run.test.ts create mode 100644 packages/cli/__tests__/network/metrics.test.ts create mode 100644 packages/cli/src/commands/data.ts create mode 100644 packages/cli/src/features/data/index.ts create mode 100644 packages/cli/src/features/data/output.ts create mode 100644 packages/cli/src/features/data/request.ts create mode 100644 packages/cli/src/network/metrics-types.ts create mode 100644 packages/cli/src/network/metrics.ts create mode 100644 packages/cli/src/utils/api-error.ts 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..24132bbd --- /dev/null +++ b/packages/cli/src/features/data/request.ts @@ -0,0 +1,34 @@ +import type { ApiResponse } from '@spotify-confidence/core'; +import { printApiError } from '@utils/api-error.js'; +import { requireAuth } 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; + } + + const token = requireAuth(argv.profile as string | undefined); + if (!token) return; + + 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.' }, + }); + } +} 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; }; From d268d63ec827afa4bd29893be1845a7ba6f58922 Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 15:39:53 +0200 Subject: [PATCH 2/2] feat(cli): add warehouse list and get commands Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/README.md | 52 +- .../__tests__/commands/warehouse-read.test.ts | 747 ++++++++++++++++++ packages/cli/src/commands/warehouse.ts | 35 +- packages/cli/src/features/warehouse/index.ts | 1 + packages/cli/src/features/warehouse/read.ts | 43 + packages/cli/src/network/metrics-warehouse.ts | 168 ++++ 6 files changed, 1037 insertions(+), 9 deletions(-) create mode 100644 packages/cli/__tests__/commands/warehouse-read.test.ts create mode 100644 packages/cli/src/features/warehouse/read.ts create mode 100644 packages/cli/src/network/metrics-warehouse.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index fcfcb34b..603792b5 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -82,14 +82,50 @@ confidence quickstart # Launch the interactive setup wizard ### Data Warehouses -| Command | Description | -| ---------------------------------------------------- | -------------------------------------------------- | -| `confidence warehouse validate` | Validate warehouse configuration | -| `confidence warehouse create` | Create a data warehouse connection | -| `confidence warehouse connector create-flag-applied` | Create flag assignment data connector | -| `confidence warehouse connector create-event` | Create event data connector | -| `confidence warehouse assignment-table create` | Create an assignment table for experiment analysis | -| `confidence warehouse crypto-key create` | Create a crypto key (Snowflake) | +| Command | Description | +| ----------------------------------------------------- | -------------------------------------------------- | +| `confidence warehouse list` | List data warehouse connections | +| `confidence warehouse get --name dataWarehouses/` | Get a data warehouse connection | +| `confidence warehouse validate` | Validate warehouse configuration | +| `confidence warehouse create` | Create a data warehouse connection | +| `confidence warehouse connector create-flag-applied` | Create flag assignment data connector | +| `confidence warehouse connector create-event` | Create event data connector | +| `confidence warehouse assignment-table create` | Create an assignment table for experiment analysis | +| `confidence warehouse crypto-key create` | Create a crypto key (Snowflake) | + +#### Read warehouse connections + +Warehouse reads call the regional Metrics HTTP API directly (no MCP server required): + +```bash +confidence warehouse list --page-size 100 --json +confidence warehouse list --page-token '' --json +confidence warehouse list --all --profile production --output table +confidence warehouse get --name dataWarehouses/my-warehouse --json +confidence warehouse get --name dataWarehouses/my-warehouse --dry-run +``` + +`list` fetches one page by default and preserves `dataWarehouses`, `nextPageToken`, and +other response metadata inside the JSON `data` envelope. An omitted `dataWarehouses` +field in an empty protobuf JSON response is normalized to `[]`. `--page-size` must be a +positive integer at most 1000. `--all` follows pagination until exhausted, combines +warehouses in response order, and fails on repeated page tokens rather than looping. +A 1000-page safety limit also prevents endless aggregation if the server keeps +returning unique tokens; use single-page requests for larger listings. It can start +at `--page-token`; response metadata is merged with later pages taking precedence. `get` +requires a qualified `dataWarehouses/` name, not a bare ID or URL, and rejects a +response for a different warehouse. + +Both reads honor `--profile`, select EU/US from the authentication token, and support +`--json` / `--output`. `--dry-run` validates inputs and previews the request without +authentication or network access. Invalid inputs, malformed API responses, and API +errors fail with a nonzero exit code; no partial list is printed if pagination fails. +Public warehouse metadata is retained. Configuration output is restricted to known +public proto fields: input-only client secrets, AWS secret access keys, and unknown +configuration fields are omitted in every output format. Crypto key and service +account references remain visible; they are not raw credentials. + +Existing warehouse setup, creation, validation, and connector commands are unchanged. ### SDK diff --git a/packages/cli/__tests__/commands/warehouse-read.test.ts b/packages/cli/__tests__/commands/warehouse-read.test.ts new file mode 100644 index 00000000..f9dcdfaf --- /dev/null +++ b/packages/cli/__tests__/commands/warehouse-read.test.ts @@ -0,0 +1,747 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import yargs from 'yargs'; + +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; + +import { warehouseCommand } from '@commands/warehouse.js'; + +import { captureOutput } from '../helpers/capture.js'; +import { createRunner } from '../helpers/run-command.js'; + +const METRICS_EU = 'https://metrics.eu.confidence.dev/v1/dataWarehouses'; +const sut = createRunner(warehouseCommand); +const warehouse = { + name: 'dataWarehouses/analytics', + createTime: '2026-01-01T00:00:00Z', + updateTime: '2026-02-01T00:00:00Z', + creator: 'identities/alice', + updater: 'identities/bob', + owner: 'identities/team', + labels: { environment: 'production' }, + limits: { maxConcurrentQueries: '10' }, + config: { + bigQueryConfig: { gcpProjectId: 'project', dataset: 'confidence', bigQueryLocation: 'EU' }, + }, + futureMetadata: { state: 'READY' }, +}; + +function simulateOutputTTY(isTTY: boolean): Disposable { + const original = process.stdout.isTTY; + process.stdout.isTTY = isTTY; + return { + [Symbol.dispose]() { + process.stdout.isTTY = original; + }, + }; +} + +afterEach(() => vi.unstubAllEnvs()); + +describe('warehouse list/get output', () => { + it('prints one page with its continuation token and all public metadata in the JSON envelope', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + const page = { dataWarehouses: [warehouse], nextPageToken: 'next', totalSize: 12 }; + server.use(http.get(METRICS_EU, () => HttpResponse.json(page))); + + await sut(['warehouse', 'list', '--json']); + + expect(JSON.parse(output.stdout)).toEqual({ data: page }); + expect(output.stderr).toBe(''); + }); + + it('prints warehouse details with --output json', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(`${METRICS_EU}/analytics`, () => HttpResponse.json(warehouse))); + + await sut(['warehouse', 'get', '--name', warehouse.name, '--output', 'json']); + + expect(JSON.parse(output.stdout)).toEqual({ data: warehouse }); + }); + + it.each([['list'], ['get', '--name', warehouse.name]])( + 'renders public details in table output for %j', + async (...args) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(METRICS_EU, () => HttpResponse.json({ dataWarehouses: [warehouse] })), + http.get(`${METRICS_EU}/analytics`, () => HttpResponse.json(warehouse)), + ); + + await sut(['warehouse', ...args, '--output', 'table']); + + // Assert + expect(output.stdout).toContain(warehouse.name); + expect(output.stdout).toContain('project'); + expect(output.stdout).toContain('production'); + expect(output.stdout).toContain('Field'); + }, + ); + + it.each([ + { args: [], tty: false }, + { args: ['--json', '--output', 'table'], tty: true }, + ])('uses JSON when piped or explicitly requested: $args', async ({ args, tty }) => { + // Arrange + using _auth = prepareAuthTokens(); + using _tty = simulateOutputTTY(tty); + using output = captureOutput(); + server.use(http.get(METRICS_EU, () => HttpResponse.json({ dataWarehouses: [] }))); + + await sut(['warehouse', 'list', ...args]); + + expect(JSON.parse(output.stdout)).toEqual({ data: { dataWarehouses: [] } }); + }); +}); + +describe('warehouse read authentication and routing', () => { + it.each(['EU', 'US'])('uses the %s Metrics API and bearer authentication', async (region) => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const token = buildTestJwt({ 'https://confidence.dev/region': region }); + vi.stubEnv('CONFIDENCE_TOKEN', token); + server.use( + http.get( + `https://metrics.${region.toLowerCase()}.confidence.dev/v1/dataWarehouses`, + ({ request }) => + HttpResponse.json({ + dataWarehouses: [], + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + + await sut(['warehouse', 'list', '--json']); + + expect(JSON.parse(output.stdout).data.authorized).toBe(true); + }); + + it.each([['list'], ['get', '--name', warehouse.name]])( + 'uses a named profile for %j rather than the default token', + async (...args) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + const token = buildTestJwt({ 'https://confidence.dev/region': 'US' }); + const profileDir = join(process.env.CONFIDENCE_CONFIG_DIR!, 'profiles', 'production'); + mkdirSync(profileDir, { recursive: true }); + writeFileSync(join(profileDir, 'credentials.json'), JSON.stringify({ accessToken: token })); + server.use( + http.get('https://metrics.us.confidence.dev/v1/dataWarehouses', ({ request }) => + HttpResponse.json({ + dataWarehouses: [], + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + http.get('https://metrics.us.confidence.dev/v1/dataWarehouses/analytics', ({ request }) => + HttpResponse.json({ + ...warehouse, + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + + await sut(['warehouse', ...args, '--profile', 'production', '--json']); + + expect(JSON.parse(output.stdout).data.authorized).toBe(true); + }, + ); + + it('honors the API base URL override', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + vi.stubEnv('CONFIDENCE_API_BASE_URL', 'http://localhost:9876'); + server.use( + http.get('http://localhost:9876/v1/dataWarehouses/analytics', () => + HttpResponse.json(warehouse), + ), + ); + + await sut(['warehouse', 'get', '--name', warehouse.name, '--json']); + + expect(JSON.parse(output.stdout).data.name).toBe(warehouse.name); + }); + + it.each(['none', 'expired'] as const)( + 'fails without HTTP access when authentication is %s', + async (auth) => { + // Arrange + using _auth = prepareAuthTokens(auth); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(`${METRICS_EU}*`, () => { + requests++; + return HttpResponse.json({}); + }), + ); + + await sut(['warehouse', 'list']); + await sut(['warehouse', 'get', '--name', warehouse.name]); + + // Assert + expect(output.stderr).toContain(auth === 'none' ? 'Not logged in' : 'Token expired'); + expect(process.exitCode).toBe(1); + expect(requests).toBe(0); + expect(output.stdout).toBe(''); + }, + ); +}); + +describe('warehouse list pagination', () => { + it('passes pageSize and opaque pageToken safely and stops after one page by default', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + const pageToken = 'opaque+/=&? token'; + let requests = 0; + server.use( + http.get(METRICS_EU, ({ request }) => { + requests++; + return HttpResponse.json({ + dataWarehouses: [], + nextPageToken: 'next', + query: Object.fromEntries(new URL(request.url).searchParams), + }); + }), + ); + + await sut(['warehouse', 'list', '--page-size', '1000', '--page-token', pageToken, '--json']); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + dataWarehouses: [], + nextPageToken: 'next', + query: { pageSize: '1000', pageToken }, + }); + expect(requests).toBe(1); + }); + + it('only sends explicitly supplied pagination parameters', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(METRICS_EU, ({ request }) => + HttpResponse.json({ dataWarehouses: [], query: new URL(request.url).search }), + ), + ); + + await sut(['warehouse', 'list', '--json']); + + expect(JSON.parse(output.stdout).data.query).toBe(''); + }); + + it('follows --all from a starting token, including an empty intermediate page', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + const tokens: (string | null)[] = []; + const sizes: (string | null)[] = []; + server.use( + http.get(METRICS_EU, ({ request }) => { + const query = new URL(request.url).searchParams; + tokens.push(query.get('pageToken')); + sizes.push(query.get('pageSize')); + if (query.get('pageToken') === 'start') + return HttpResponse.json({ + dataWarehouses: [warehouse], + nextPageToken: 'second', + extra: 'retained', + state: 'first', + }); + if (query.get('pageToken') === 'second') + return HttpResponse.json({ nextPageToken: 'last' }); + return HttpResponse.json({ + dataWarehouses: [{ name: 'dataWarehouses/last' }], + state: 'last', + }); + }), + ); + + // Act + await sut([ + 'warehouse', + 'list', + '--all', + '--page-token', + 'start', + '--page-size', + '1', + '--json', + ]); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + dataWarehouses: [warehouse, { name: 'dataWarehouses/last' }], + nextPageToken: '', + extra: 'retained', + state: 'last', + }); + expect(tokens).toEqual(['start', 'second', 'last']); + expect(sizes).toEqual(['1', '1', '1']); + }); + + it.each([ + { args: [], body: {} }, + { args: ['--all'], body: { nextPageToken: '' } }, + ])( + 'normalizes omitted repeated fields in an empty protobuf JSON response: $args', + async ({ args, body }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(METRICS_EU, () => HttpResponse.json(body))); + + await sut(['warehouse', 'list', ...args, '--json']); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data: { ...body, dataWarehouses: [] } }); + expect(output.stderr).toBe(''); + }, + ); + + it('supports --all on an empty warehouse list', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(METRICS_EU, () => HttpResponse.json({ dataWarehouses: [], nextPageToken: '' })), + ); + + await sut(['warehouse', 'list', '--all', '--json']); + + expect(JSON.parse(output.stdout).data).toEqual({ dataWarehouses: [], nextPageToken: '' }); + }); + + it.each([ + { start: [], tokens: ['a', 'a'], calls: 2 }, + { start: [], tokens: ['a', 'b', 'a'], calls: 3 }, + { start: ['--page-token', 'a'], tokens: ['a'], calls: 1 }, + ])( + 'fails without partial output for a repeated pagination token: $tokens', + async ({ start, tokens, calls }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(METRICS_EU, () => + HttpResponse.json({ + dataWarehouses: [warehouse], + nextPageToken: tokens[requests++] ?? '', + }), + ), + ); + + await sut(['warehouse', 'list', '--all', ...start, '--json']); + + // Assert + expect(output.stderr).toContain('repeated page token'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + expect(requests).toBe(calls); + }, + ); + + it('bounds --all when the server never exhausts unique cursors', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(METRICS_EU, () => + HttpResponse.json({ dataWarehouses: [], nextPageToken: `page-${++requests}` }), + ), + ); + + await sut(['warehouse', 'list', '--all', '--json']); + + // Assert + expect(requests).toBe(1000); + expect(output.stderr).toContain('1000-page safety limit'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); + + it('does not print partial results when a later page fails', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(METRICS_EU, ({ request }) => + new URL(request.url).searchParams.has('pageToken') + ? HttpResponse.json( + { code: 7, message: 'Permission denied on page two' }, + { status: 403 }, + ) + : HttpResponse.json({ dataWarehouses: [warehouse], nextPageToken: 'next' }), + ), + ); + + await sut(['warehouse', 'list', '--all', '--json']); + + expect(output.stderr).toContain('Permission denied on page two'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); +}); + +describe('warehouse read validation and dry-run', () => { + it.each(['0', '-1', '1.5', '1001', 'NaN', 'Infinity', 'text'])( + 'rejects invalid --page-size %s before authentication, even in dry-run', + async (size) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + + await sut(['warehouse', 'list', '--page-size', size, '--dry-run']); + + // Assert + expect(output.stderr).toContain('--page-size must be a positive integer'); + expect(output.stderr).not.toContain('Not logged in'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }, + ); + + it.each([ + 'id', + '/dataWarehouses/id', + 'other/id', + 'dataWarehouses/', + 'dataWarehouses/a/child', + 'dataWarehouses/..', + 'dataWarehouses/a%2fb', + 'dataWarehouses/a?x=y', + 'dataWarehouses/a#b', + 'dataWarehouses/a\\b', + 'dataWarehouses/a b', + 'dataWarehouses/a\n', + 'dataWarehouses/a\u001b[31m', + 'https://evil.example/dataWarehouses/id', + '//evil.example/id', + ])('rejects unsafe or unqualified name %s before authentication', async (name) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + + await sut(['warehouse', 'get', '--name', name, '--dry-run']); + + // Assert + expect(output.stderr).toContain('Resource name must be dataWarehouses/'); + expect(output.stderr).not.toContain('Not logged in'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); + + it.each([ + ['list', '--page-size', '1', '--page-size', '2'], + ['list', '--page-token', 'a', '--page-token', 'b'], + ['list', '--page-token', ''], + ['list', '--page-token', ' '], + ['get', '--name', warehouse.name, '--name', 'dataWarehouses/other'], + ['get', '--name', ''], + ])('rejects ambiguous duplicate or empty scalar arguments: %j', async (...args) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + + await sut(['warehouse', ...args, '--dry-run']); + + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + expect(output.stderr).not.toContain('Not logged in'); + }); + + it('requires --name and rejects unimplemented read flags', async () => { + using _output = captureOutput(); + const parser = () => yargs().exitProcess(false).command(warehouseCommand); + + // Act / Assert + await expect(async () => parser().parseAsync(['warehouse', 'get'])).rejects.toThrow( + 'Missing required argument: name', + ); + await expect(async () => + parser().parseAsync(['warehouse', 'list', '--filter', 'x']), + ).rejects.toThrow('Unknown argument: filter'); + }); + + it.each([ + { + args: ['list', '--all', '--page-size', '5', '--page-token', 'start'], + preview: { + dryRun: true, + method: 'GET', + path: '/v1/dataWarehouses', + params: { pageSize: '5', pageToken: 'start' }, + all: true, + }, + }, + { + args: ['get', '--name', 'dataWarehouses/valid_ID-123'], + preview: { dryRun: true, method: 'GET', path: '/v1/dataWarehouses/valid_ID-123' }, + }, + ])( + 'previews a validated request without authentication or HTTP: $args', + async ({ args, preview }) => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(`${METRICS_EU}*`, () => { + requests++; + return HttpResponse.json({}); + }), + ); + + await sut(['warehouse', ...args, '--dry-run', '--json']); + + expect(JSON.parse(output.stdout)).toEqual({ data: preview }); + expect(output.stderr).toBe(''); + expect(requests).toBe(0); + }, + ); +}); + +describe('warehouse read failures', () => { + it.each([ + { action: ['list'], status: 403, message: 'Permission denied' }, + { action: ['get', '--name', warehouse.name], status: 403, message: 'Permission denied' }, + { action: ['get', '--name', warehouse.name], status: 404, message: 'Warehouse not found' }, + { action: ['list'], status: 500, message: 'Internal error' }, + ])( + 'reports HTTP $status for $action without printing response details', + async ({ action, status, message }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(`${METRICS_EU}*`, () => + HttpResponse.json( + { code: status, message, details: [{ clientSecret: 'details-secret-value' }] }, + { status }, + ), + ), + ); + + await sut(['warehouse', ...action, '--json']); + + // Assert + expect(output.stderr).toContain(message); + expect(output.stderr).not.toContain('details-secret-value'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }, + ); + + it('redacts bearer credentials and terminal controls from API errors', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + const credential = 'synthetic-bearer-credential'; + server.use( + http.get(METRICS_EU, () => + HttpResponse.json( + { code: 7, message: `Denied\n\u001b[31m Bearer ${credential}` }, + { status: 403 }, + ), + ), + ); + + await sut(['warehouse', 'list', '--json']); + + // Assert + expect(output.stderr).toContain('Denied'); + expect(output.stderr).not.toContain(credential); + expect(output.stderr).not.toContain('\u001b'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); + + it('reports network failures', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(METRICS_EU, () => HttpResponse.error())); + + await sut(['warehouse', 'list']); + + expect(output.stderr).toContain('Network error'); + expect(process.exitCode).toBe(1); + }); + + it.each( + [ + null, + [], + 'unexpected', + { dataWarehouses: null }, + { dataWarehouses: {} }, + { dataWarehouses: [null] }, + { dataWarehouses: [{ name: 'other/id' }] }, + { dataWarehouses: [], nextPageToken: 7 }, + { dataWarehouses: [{ name: warehouse.name, config: [] }] }, + { dataWarehouses: [{ name: warehouse.name, config: { bigQueryConfig: { dataset: 1 } } }] }, + ].map((body) => ({ body })), + )('fails in a controlled way on malformed list response $body', async ({ body }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(METRICS_EU, () => HttpResponse.json(body))); + + await sut(['warehouse', 'list', '--json']); + + expect(process.exitCode).toBe(1); + expect(output.stderr.length).toBeGreaterThan(0); + expect(output.stdout).toBe(''); + }); + + it.each( + [ + null, + [], + {}, + { name: 123 }, + { name: 'other/id' }, + { name: warehouse.name, config: { dataBricksConfig: 'raw-secret' } }, + { name: warehouse.name, config: { redshiftConfig: { credentialsSettings: [] } } }, + { name: warehouse.name, config: { redshiftConfig: { webIdentitySettings: 'raw-secret' } } }, + ].map((body) => ({ body })), + )('fails in a controlled way on malformed get response $body', async ({ body }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(`${METRICS_EU}/analytics`, () => HttpResponse.json(body))); + + await sut(['warehouse', 'get', '--name', warehouse.name, '--json']); + + expect(process.exitCode).toBe(1); + expect(output.stderr.length).toBeGreaterThan(0); + expect(output.stderr).not.toContain('raw-secret'); + expect(output.stdout).toBe(''); + }); + + it('rejects a get response for a different valid warehouse name', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(`${METRICS_EU}/analytics`, () => + HttpResponse.json({ ...warehouse, name: 'dataWarehouses/other' }), + ), + ); + + await sut(['warehouse', 'get', '--name', warehouse.name, '--json']); + + // Assert + expect(output.stderr).toContain('does not match the requested resource'); + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); + + it('reports non-JSON responses without printing raw response bodies', async () => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use(http.get(METRICS_EU, () => HttpResponse.text('not JSON'))); + + await sut(['warehouse', 'list']); + + expect(process.exitCode).toBe(1); + expect(output.stdout).toBe(''); + }); +}); + +describe('warehouse read credential redaction', () => { + const config = { + dataBricksConfig: { + host: 'https://databricks.example', + clientId: 'public-client', + clientSecret: 'databricks-secret-value', + futureCredentials: { password: 'future-secret-value' }, + }, + redshiftConfig: { + roleArn: 'arn:aws:iam::123:role/confidence', + database: 'analytics', + credentialsSettings: { + accessKeyId: 'public-access-key-id', + secretAccessKey: 'aws-secret-value', + sessionToken: 'session-secret-value', + }, + }, + snowflakeConfig: { + account: 'snowflake-account', + authenticationKey: 'cryptoKeys/warehouse', + privateKey: 'private-key-secret-value', + }, + bigQueryConfig: { + serviceAccount: 'confidence@example.iam.gserviceaccount.com', + serviceAccountKey: 'service-account-secret-value', + }, + unknownConfig: { token: 'unknown-secret-value' }, + }; + const safeConfig = { + dataBricksConfig: { host: 'https://databricks.example', clientId: 'public-client' }, + redshiftConfig: { + roleArn: 'arn:aws:iam::123:role/confidence', + database: 'analytics', + credentialsSettings: { accessKeyId: 'public-access-key-id' }, + }, + snowflakeConfig: { account: 'snowflake-account', authenticationKey: 'cryptoKeys/warehouse' }, + bigQueryConfig: { serviceAccount: 'confidence@example.iam.gserviceaccount.com' }, + }; + + it.each([ + { args: ['list'], format: ['--json'] }, + { args: ['list', '--all'], format: ['--output', 'table'] }, + { args: ['get', '--name', warehouse.name], format: ['--json'] }, + { args: ['get', '--name', warehouse.name], format: ['--output', 'table'] }, + ])( + 'omits credentials but retains public metadata for $args $format', + async ({ args, format }) => { + // Arrange + using _auth = prepareAuthTokens(); + using output = captureOutput(); + server.use( + http.get(METRICS_EU, () => + HttpResponse.json({ dataWarehouses: [{ ...warehouse, config }] }), + ), + http.get(`${METRICS_EU}/analytics`, () => HttpResponse.json({ ...warehouse, config })), + ); + + await sut(['warehouse', ...args, ...format]); + + // Assert + for (const secret of [ + 'databricks-secret-value', + 'future-secret-value', + 'aws-secret-value', + 'session-secret-value', + 'private-key-secret-value', + 'service-account-secret-value', + 'unknown-secret-value', + ]) { + expect(output.stdout + output.stderr).not.toContain(secret); + } + expect(output.stdout).toContain('cryptoKeys/warehouse'); + expect(output.stdout).toContain('public-client'); + expect(output.stdout).toContain('production'); + if (format.includes('--json')) { + const data = JSON.parse(output.stdout).data; + expect(args[0] === 'list' ? data.dataWarehouses[0] : data).toEqual({ + ...warehouse, + config: safeConfig, + }); + } + }, + ); +}); diff --git a/packages/cli/src/commands/warehouse.ts b/packages/cli/src/commands/warehouse.ts index 82dc5dd5..398dfcf8 100644 --- a/packages/cli/src/commands/warehouse.ts +++ b/packages/cli/src/commands/warehouse.ts @@ -8,6 +8,8 @@ import { createAssignmentTableCmd, createCryptoKeyCmd, runWarehouseSetup, + listWarehousesCmd, + getWarehouseCmd, } from '@features/warehouse/index.js'; import { safely } from '@utils/index.js'; @@ -41,6 +43,37 @@ export const warehouseCommand = { describe: 'Manage data warehouse connections', builder(yargs: Argv) { return yargs + .command( + 'list', + 'List data warehouses', + (y: Argv) => + y + .option('page-size', { + type: 'number', + describe: 'Maximum warehouses per page (1–1000)', + }) + .option('page-token', { + type: 'string', + describe: 'Start at this pagination token', + }) + .option('all', { + type: 'boolean', + default: false, + describe: 'Fetch all pages, starting at --page-token if provided', + }), + safely(listWarehousesCmd), + ) + .command( + 'get', + 'Get a data warehouse', + (y: Argv) => + y.option('name', { + type: 'string', + demandOption: true, + describe: 'Qualified resource name (dataWarehouses/)', + }), + safely(getWarehouseCmd), + ) .command( 'setup', 'Launch guided warehouse setup in your AI coding agent', @@ -176,7 +209,7 @@ export const warehouseCommand = { ) .demandCommand( 1, - 'Available actions: setup, validate, create, connector, assignment-table, crypto-key. Run "confidence warehouse --help" for details.', + 'Available actions: list, get, setup, validate, create, connector, assignment-table, crypto-key. Run "confidence warehouse --help" for details.', ) .strict(); }, diff --git a/packages/cli/src/features/warehouse/index.ts b/packages/cli/src/features/warehouse/index.ts index e33035a1..8a028965 100644 --- a/packages/cli/src/features/warehouse/index.ts +++ b/packages/cli/src/features/warehouse/index.ts @@ -5,3 +5,4 @@ export { createEventConnectionCmd } from './create-event-connection.js'; export { createAssignmentTableCmd } from './create-assignment-table.js'; export { createCryptoKeyCmd } from './create-crypto-key.js'; export { runWarehouseSetup } from './setup.js'; +export { listWarehousesCmd, getWarehouseCmd } from './read.js'; diff --git a/packages/cli/src/features/warehouse/read.ts b/packages/cli/src/features/warehouse/read.ts new file mode 100644 index 00000000..289e36c4 --- /dev/null +++ b/packages/cli/src/features/warehouse/read.ts @@ -0,0 +1,43 @@ +import { executeDataRequest } from '@features/data/request.js'; +import { + getWarehouse, + listWarehouses, + warehouseListParams, + type WarehouseListOptions, +} from '@network/metrics-warehouse.js'; +import { requireString, resourcePath } from '@network/metrics-types.js'; + +export async function listWarehousesCmd(argv: Record): Promise { + const pageSize = argv['page-size']; + if ( + pageSize !== undefined && + (typeof pageSize !== 'number' || !Number.isInteger(pageSize) || pageSize < 1 || pageSize > 1000) + ) { + throw new Error('--page-size must be a positive integer no greater than 1000.'); + } + const options: WarehouseListOptions = { + pageSize, + pageToken: + argv['page-token'] === undefined + ? undefined + : requireString(argv['page-token'], '--page-token'), + all: argv.all === true, + }; + await executeDataRequest(argv, (token) => listWarehouses(token, options), { + dryRun: true, + method: 'GET', + path: '/v1/dataWarehouses', + params: warehouseListParams(options), + all: options.all, + }); +} + +export async function getWarehouseCmd(argv: Record): Promise { + const name = requireString(argv.name, '--name'); + const path = resourcePath(name, 'dataWarehouses'); + await executeDataRequest(argv, (token) => getWarehouse(token, name), { + dryRun: true, + method: 'GET', + path, + }); +} diff --git a/packages/cli/src/network/metrics-warehouse.ts b/packages/cli/src/network/metrics-warehouse.ts new file mode 100644 index 00000000..3bae95cd --- /dev/null +++ b/packages/cli/src/network/metrics-warehouse.ts @@ -0,0 +1,168 @@ +import type { ApiResponse } from '@spotify-confidence/core'; + +import { metricsRequest } from './metrics.js'; +import { isRecord, requireRecord, requireString, resourcePath } from './metrics-types.js'; + +export type Warehouse = Record & { name: string }; +export type WarehousePage = Record & { + dataWarehouses: Warehouse[]; + nextPageToken?: string; +}; +export type WarehouseListOptions = { + pageSize?: number; + pageToken?: string; + all: boolean; +}; + +// Bound aggregation even when a broken server keeps returning unique continuation tokens. +const MAX_WAREHOUSE_PAGES = 1000; + +// Only expose public configuration fields from DataWarehouseConfig in metrics/v1/metrics.proto. +// In particular, never return Databricks clientSecret or Redshift secretAccessKey, +// even if a server accidentally includes these input-only fields in a read response. +const PUBLIC_CONFIG_FIELDS = { + bigQueryConfig: [ + 'gcpProjectId', + 'bigQueryLocation', + 'dataset', + 'serviceAccount', + 'processingProjectId', + 'interactiveProjectId', + 'exportJobsServiceAccount', + ], + dataBricksConfig: ['host', 'warehouseId', 'clientId', 'exposureSchema', 'resultsSchema'], + redshiftConfig: ['exposureSchema', 'region', 'roleArn', 'cluster', 'database', 'dataApiEndpoint'], + snowflakeConfig: [ + 'account', + 'user', + 'authenticationKey', + 'role', + 'warehouse', + 'exposureDatabase', + 'exposureSchema', + ], +} as const; + +function publicFields(value: unknown, fields: readonly string[]): Record { + const record = requireRecord(value, 'Warehouse configuration'); + const result: Record = {}; + for (const field of fields) { + if (record[field] !== undefined) { + if (typeof record[field] !== 'string') { + throw new Error('Invalid warehouse configuration response.'); + } + result[field] = record[field]; + } + } + return result; +} + +function publicConfig(value: unknown): Record { + const config = requireRecord(value, 'Warehouse config'); + const result: Record = {}; + for (const [kind, fields] of Object.entries(PUBLIC_CONFIG_FIELDS)) { + if (config[kind] !== undefined) result[kind] = publicFields(config[kind], fields); + } + if (isRecord(config.redshiftConfig)) { + const redshift = requireRecord(result.redshiftConfig); + if (config.redshiftConfig.credentialsSettings !== undefined) { + redshift.credentialsSettings = publicFields(config.redshiftConfig.credentialsSettings, [ + 'accessKeyId', + ]); + } + if (config.redshiftConfig.webIdentitySettings !== undefined) { + requireRecord(config.redshiftConfig.webIdentitySettings, 'Warehouse web identity settings'); + redshift.webIdentitySettings = {}; + } + } + return result; +} + +function parseWarehouse(value: unknown): Warehouse { + const warehouse = requireRecord(value, 'Data warehouse response'); + const name = requireString(warehouse.name, 'Data warehouse name'); + resourcePath(name, 'dataWarehouses'); + return { + ...warehouse, + name, + ...(warehouse.config === undefined ? {} : { config: publicConfig(warehouse.config) }), + }; +} + +function parsePage(value: unknown): WarehousePage { + const page = requireRecord(value, 'Warehouse list response'); + // Protobuf JSON omits empty repeated fields unless default values are emitted. + const warehouses = page.dataWarehouses === undefined ? [] : page.dataWarehouses; + if (!Array.isArray(warehouses)) { + throw new Error('Warehouse list response dataWarehouses must be an array.'); + } + if (page.nextPageToken !== undefined && typeof page.nextPageToken !== 'string') { + throw new Error('Warehouse list response nextPageToken must be a string.'); + } + return { + ...page, + dataWarehouses: warehouses.map(parseWarehouse), + ...(page.nextPageToken === undefined ? {} : { nextPageToken: page.nextPageToken }), + }; +} + +export function warehouseListParams(options: WarehouseListOptions): Record { + return { + ...(options.pageSize === undefined ? {} : { pageSize: String(options.pageSize) }), + ...(options.pageToken === undefined ? {} : { pageToken: options.pageToken }), + }; +} + +export async function listWarehouses( + token: string, + options: WarehouseListOptions, +): Promise> { + const seenTokens = new Set(); + if (options.pageToken) seenTokens.add(options.pageToken); + let pageToken = options.pageToken; + const warehouses: Warehouse[] = []; + let combined: WarehousePage = { dataWarehouses: warehouses }; + let pages = 0; + do { + if (pages >= MAX_WAREHOUSE_PAGES) { + throw new Error( + 'Warehouse pagination exceeded the 1000-page safety limit. Use single-page requests instead.', + ); + } + pages++; + const response = await metricsRequest(token, { + path: '/v1/dataWarehouses', + params: warehouseListParams({ ...options, pageToken }), + }); + if (!response.ok) return response; + const page = parsePage(response.data); + if (!options.all) return { ok: true, data: page }; + for (const warehouse of page.dataWarehouses) warehouses.push(warehouse); + combined = { + ...combined, + ...page, + dataWarehouses: warehouses, + nextPageToken: page.nextPageToken ?? '', + }; + pageToken = page.nextPageToken; + if (pageToken) { + if (seenTokens.has(pageToken)) { + throw new Error('Warehouse pagination returned a repeated page token.'); + } + seenTokens.add(pageToken); + } + } while (pageToken); + return { ok: true, data: combined }; +} + +export async function getWarehouse(token: string, name: string): Promise> { + const response = await metricsRequest(token, { + path: resourcePath(name, 'dataWarehouses'), + }); + if (!response.ok) return response; + const warehouse = parseWarehouse(response.data); + if (warehouse.name !== name) { + throw new Error('Data warehouse response name does not match the requested resource.'); + } + return { ok: true, data: warehouse }; +}