From 11a84c8af2d394bb18d6375b0f9f06fa20113fb0 Mon Sep 17 00:00:00 2001 From: eastagiletracker <310448263+eastagiletracker@users.noreply.github.com> Date: Tue, 13 Oct 2026 07:23:12 +0000 Subject: [PATCH] fix: reject an --api-url that is not an http(s) URL, with exit 2 A base URL without a scheme was only noticed once a request was built. `api.databox.com` failed every command with a bare "TypeError: Invalid URL" (exit 1, the code for an API error), and `localhost:8080`, which parses as a URL with the scheme "localhost:", failed inside fetch as "Could not connect to API" with a request ID for a request that never left. `auth login --api-url` saved such a URL and exited 0, so every later command failed the same way. --api-url and DATABOX_API_URL are now checked in BaseCommand.init and in auth login, before any prompt, request or save, and exit 2. A bad URL already stored in the config exits 1, like an unreadable config, and names the command that replaces it. --- README.md | 4 +- src/base-command.ts | 22 +++++++- src/commands/auth/login.ts | 8 ++- src/lib/api-client.ts | 20 +++++++ test/base-command.test.ts | 96 ++++++++++++++++++++++++++++++++ test/commands/auth/login.test.ts | 36 ++++++++++++ test/lib/api-client.test.ts | 26 ++++++++- 7 files changed, 205 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index fac5776..4fca01a 100644 --- a/README.md +++ b/README.md @@ -275,8 +275,8 @@ A timed-out `dataset ingest` may still have been processed: check `databox datas | Exit code | Meaning | |-----------|---------| | `0` | Success. Answering anything but `y` or `yes` to a confirmation prompt, at a terminal or piped, also exits 0 after printing `Aborted.`. | -| `1` | The API returned an error (4xx or 5xx, including the rate limit, and `forbidden` when your role does not allow the command; see [Permissions](#permissions)). Also: no API key is configured, the stored config file is not valid JSON, the response was not JSON (usually a wrong `--api-url`), an update command was given no field to change, or `dataset ingest` was run at a terminal with no `--records` or `--file`. | -| `2` | The request was never sent, or never reached the API: an unknown flag, a value outside a flag's options, a malformed ID or JSON value, an ingest over the limits, or a network failure, timeout or redirect (the CLI does not follow a redirect: it would resend your API key). Also a command that would prompt when stdin is not a terminal and nothing is piped: a delete, purge or clear without `--force`, or `auth login` without `--api-key` (see [Authentication](#authentication)). | +| `1` | The API returned an error (4xx or 5xx, including the rate limit, and `forbidden` when your role does not allow the command; see [Permissions](#permissions)). Also: no API key is configured, the stored config file is not valid JSON or holds an API URL that is not `http://` or `https://`, the response was not JSON (usually a wrong `--api-url`), an update command was given no field to change, or `dataset ingest` was run at a terminal with no `--records` or `--file`. | +| `2` | The request was never sent, or never reached the API: an unknown flag, a value outside a flag's options, a malformed ID, `--api-url` or JSON value, an ingest over the limits, or a network failure, timeout or redirect (the CLI does not follow a redirect: it would resend your API key). Also a command that would prompt when stdin is not a terminal and nothing is piped: a delete, purge or clear without `--force`, or `auth login` without `--api-key` (see [Authentication](#authentication)). | | `130` | A prompt (a confirmation, or the API key at `auth login`) was interrupted with Ctrl-C. | ## Scripts and AI Agents diff --git a/src/base-command.ts b/src/base-command.ts index 061a00b..676f3e6 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -3,9 +3,9 @@ import { } from '@oclif/core' import { - ApiClient, ApiConnectionError, ApiRequestError, describeApiError, describeConnectionError, + ApiClient, ApiConnectionError, ApiRequestError, describeApiError, describeConnectionError, describeInvalidApiUrl, isApiUrl, } from './lib/api-client.js' -import {loadConfig} from './lib/config.js' +import {getConfigPath, loadConfig} from './lib/config.js' import {OUTPUT_FORMATS, OutputFormat, colorEnabled} from './lib/output.js' export type Flags = Interfaces.InferredFlags @@ -100,6 +100,12 @@ export abstract class BaseCommand ext const accountId = this.flags['account-id'] if (accountId) this.requireNumericId(accountId, '--account-id') + // A base URL fetch cannot use would otherwise fail only once the request is built: see isApiUrl(). + const apiUrl = this.flags['api-url'] + if (apiUrl !== undefined && !isApiUrl(apiUrl)) { + this.error(describeInvalidApiUrl(apiUrl), {exit: 2}) + } + // Every command that extends BaseCommand calls the API, so a missing key or an unreadable // config fails here too, not after the prompt. auth login, which runs without a key, // extends Command. @@ -109,9 +115,19 @@ export abstract class BaseCommand ext this.error('Not authenticated. Run "databox auth login" first.', {exit: 1}) } + // A URL saved before `auth login` checked it, or edited in by hand. Like an unreadable + // config, it is the stored config at fault, so exit 1; the --api-url above overrides it. + if (apiUrl === undefined && config.apiUrl !== undefined && !isApiUrl(config.apiUrl)) { + this.error( + `The API URL saved in ${getConfigPath()} ("${config.apiUrl}") is not an http:// or https:// URL. ` + + 'Run "databox auth login --api-url https://api.databox.com" to replace it.', + {exit: 1}, + ) + } + this.apiClient = new ApiClient({ apiKey, - baseUrl: this.flags['api-url'] ?? config.apiUrl, + baseUrl: apiUrl ?? config.apiUrl, trace: this.flags.verbose ? line => this.logToStderr(line) : undefined, version: this.config.version, }) diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index c414d21..4ff13b0 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -1,6 +1,6 @@ import {Command, Flags} from '@oclif/core' -import {ApiClient} from '../../lib/api-client.js' +import {ApiClient, describeInvalidApiUrl, isApiUrl} from '../../lib/api-client.js' import {loadConfig, saveConfig} from '../../lib/config.js' import {prompt, readPipedLine} from '../../lib/prompt.js' @@ -30,6 +30,12 @@ export default class Login extends Command { let apiKey = flags['api-key'] const apiUrl = flags['api-url'] + // Checked before the prompt and before anything is saved: a stored URL fetch cannot use + // would fail every later command, while this one still exited 0 with only a warning. + if (apiUrl !== undefined && !isApiUrl(apiUrl)) { + this.error(describeInvalidApiUrl(apiUrl), {exit: 2}) + } + // Off a terminal there is nobody to prompt, but the key can be piped in // (`pass show databox | databox auth login`). An empty first line or no input is refused. if (!apiKey && !process.stdin.isTTY) { diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 3b7d982..a445951 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -112,6 +112,26 @@ export function describeResponse(status: number, durationMs: number, requestId?: return lines } +/** + * Whether a value can be the API base URL: absolute, over http or https. Anything else fails only + * once a request is built: `api.databox.com` as a bare "TypeError: Invalid URL", and + * `localhost:8080`, which parses with the scheme "localhost:", as a connection error that blames + * the network for a request that never left. + */ +export function isApiUrl(value: string): boolean { + try { + const {protocol} = new URL(value) + return protocol === 'https:' || protocol === 'http:' + } catch { + return false + } +} + +/** The exit-2 message for an --api-url (or DATABOX_API_URL) that isApiUrl() refuses. */ +export function describeInvalidApiUrl(value: string): string { + return `--api-url must be an http:// or https:// URL, e.g. ${DEFAULT_BASE_URL}. Got "${value}" (from --api-url or DATABOX_API_URL).` +} + /** Whether fetch() rejected because of `redirect: 'error'`: Node reports it as a TypeError caused by "unexpected redirect". */ export function isRedirectRefusal(error: unknown): boolean { return error instanceof TypeError && error.cause instanceof Error && error.cause.message === 'unexpected redirect' diff --git a/test/base-command.test.ts b/test/base-command.test.ts index 07c2b9a..6344372 100644 --- a/test/base-command.test.ts +++ b/test/base-command.test.ts @@ -396,3 +396,99 @@ describe('base command: credentials', () => { expect(requests()).to.have.length(0) }) }) + +describe('base command: --api-url', () => { + const savedApiUrl = process.env.DATABOX_API_URL + + beforeEach(() => { + setupTestConfig(KEY) + mockApi([{ + method: 'GET', + path: '/v2/data-sources/10', + response: {data: DATA_SOURCE, requestId: 'test', status: 'success'}, + }]) + }) + + afterEach(() => { + if (savedApiUrl === undefined) delete process.env.DATABOX_API_URL + else process.env.DATABOX_API_URL = savedApiUrl + restoreApi() + cleanupTestConfig() + }) + + it('sends the request to the URL given', async () => { + const {error} = await runCommand(['data-source', 'get', '10', '--api-url', 'http://localhost:8080'], {root: process.cwd()}) + + expect(error).to.equal(undefined) + expect(requests()).to.have.length(1) + }) + + // Was a bare "TypeError: Invalid URL", exit 1, from inside the request. + it('rejects a URL without a scheme with exit 2, before any request', async () => { + const {error} = await runCommand(['data-source', 'get', '10', '--api-url', 'api.databox.com'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(error?.message).to.contain('--api-url must be an http:// or https:// URL') + expect(error?.message).to.contain('Got "api.databox.com"') + expect(requests()).to.have.length(0) + }) + + // Was "Could not connect to API. Check your internet connection." with a request ID that never left. + it('rejects a host:port without a scheme, which parses as a URL fetch cannot use', async () => { + const {error} = await runCommand(['data-source', 'get', '10', '--api-url', 'localhost:8080'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(error?.message).to.contain('Got "localhost:8080"') + expect(error?.message).to.not.contain('Request ID') + expect(requests()).to.have.length(0) + }) + + it('checks DATABOX_API_URL the same way', async () => { + process.env.DATABOX_API_URL = 'api.databox.com' + + const {error} = await runCommand(['data-source', 'get', '10'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(error?.message).to.contain('DATABOX_API_URL') + expect(requests()).to.have.length(0) + }) + + // No --force: the check has to fail before confirm() reads stdin. + it('rejects a bad URL before a destructive command prompts', async () => { + const {error} = await runCommand(['dataset', 'delete', '1', '--api-url', 'api.databox.com'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(error?.message).to.contain('--api-url must be an http:// or https:// URL') + expect(requests()).to.have.length(0) + }) + + it('reports a bad stored URL by path, with the command that replaces it', async () => { + fs.writeFileSync(getConfigPath(), JSON.stringify({apiKey: KEY, apiUrl: 'api.databox.com'})) + + const {error, stderr, stdout} = await runCommand(['data-source', 'get', '10'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(1) + expect(error?.message).to.contain(`The API URL saved in ${getConfigPath()} ("api.databox.com")`) + expect(error?.message).to.contain('databox auth login --api-url https://api.databox.com') + expect(`${error?.message}${stderr}${stdout}`).to.not.contain(KEY) + expect(requests()).to.have.length(0) + }) + + it('lets --api-url override a bad stored URL', async () => { + fs.writeFileSync(getConfigPath(), JSON.stringify({apiKey: KEY, apiUrl: 'api.databox.com'})) + + const {error} = await runCommand(['data-source', 'get', '10', '--api-url', 'https://api.databox.com'], {root: process.cwd()}) + + expect(error).to.equal(undefined) + expect(requests()).to.have.length(1) + }) + + it('still uses a good stored URL', async () => { + fs.writeFileSync(getConfigPath(), JSON.stringify({apiKey: KEY, apiUrl: 'https://staging.example.com'})) + + const {error} = await runCommand(['data-source', 'get', '10'], {root: process.cwd()}) + + expect(error).to.equal(undefined) + expect(requests()).to.have.length(1) + }) +}) diff --git a/test/commands/auth/login.test.ts b/test/commands/auth/login.test.ts index 1b19c58..331186b 100644 --- a/test/commands/auth/login.test.ts +++ b/test/commands/auth/login.test.ts @@ -52,6 +52,31 @@ describe('auth login', () => { expect(result.stderr).to.not.contain('Warning: Warning:') }) + it('saves the --api-url it was given', async () => { + mockApi([{ + method: 'GET', + path: '/v2/auth/validate-key', + response: {data: {}, requestId: 'test', status: 'success'}, + }]) + + const {error} = await runCommand(['auth', 'login', '--api-key', 'my-test-key', '--api-url', 'http://localhost:8080'], {root: process.cwd()}) + + expect(error).to.equal(undefined) + expect(JSON.parse(fs.readFileSync(getConfigPath(), 'utf8')).apiUrl).to.equal('http://localhost:8080') + }) + + // Was saved, with only a warning and exit 0, and then failed every later command. + it('refuses an --api-url without a scheme with exit 2, and saves nothing', async () => { + mockApi([]) + + const {error} = await runCommand(['auth', 'login', '--api-key', 'my-test-key', '--api-url', 'api.databox.com'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(error?.message).to.contain('--api-url must be an http:// or https:// URL') + expect(requests()).to.have.length(0) + expect(fs.existsSync(getConfigPath())).to.equal(false) + }) + describe('when stdin is not a terminal', () => { let restore: () => void const savedApiKey = process.env.DATABOX_API_KEY @@ -107,6 +132,17 @@ describe('auth login', () => { expect(fs.existsSync(getConfigPath())).to.equal(false) }) + it('refuses a bad --api-url before reading the piped key', async () => { + restore = pipeStdin('piped-test-key\n') + mockApi([]) + + const {error} = await runCommand(['auth', 'login', '--api-url', 'localhost:8080'], {root: process.cwd()}) + + expect(error?.oclif?.exit).to.equal(2) + expect(requests()).to.have.length(0) + expect(fs.existsSync(getConfigPath())).to.equal(false) + }) + it('uses --api-key without reading stdin', async () => { restore = pipeStdin('piped-test-key\n') mockApi([{ diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index b659b9f..9e7475a 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -1,7 +1,7 @@ import {expect} from 'chai' import { - ApiClient, ApiConnectionError, ApiRequestError, describeApiError, describeConnectionError, + ApiClient, ApiConnectionError, ApiRequestError, describeApiError, describeConnectionError, isApiUrl, } from '../../src/lib/api-client.js' import {mockApi, requests, restoreApi} from '../helpers.js' @@ -310,6 +310,30 @@ describe('ApiClient response bodies', () => { }) }) +describe('isApiUrl', () => { + it('accepts an absolute http or https URL', () => { + expect(isApiUrl('https://api.databox.com')).to.equal(true) + expect(isApiUrl('https://api.databox.com/')).to.equal(true) + expect(isApiUrl('http://localhost:8080')).to.equal(true) + }) + + it('refuses a host without a scheme, which new URL() cannot parse', () => { + expect(isApiUrl('api.databox.com')).to.equal(false) + expect(isApiUrl('')).to.equal(false) + }) + + // These parse, with the schemes "localhost:" and "api.databox.com:", so only fetch() would fail. + it('refuses a host:port without a scheme, which parses as some other scheme', () => { + expect(isApiUrl('localhost:8080')).to.equal(false) + expect(isApiUrl('api.databox.com:443')).to.equal(false) + }) + + it('refuses a scheme other than http and https', () => { + expect(isApiUrl('ftp://api.databox.com')).to.equal(false) + expect(isApiUrl('file:///etc/hosts')).to.equal(false) + }) +}) + describe('describeApiError', () => { it('renders code, message, field and request ID', () => { const error = new ApiRequestError('Title is required.', 400, [