diff --git a/README.md b/README.md index 349fe3db..9cf433d2 100644 --- a/README.md +++ b/README.md @@ -105,18 +105,21 @@ polymorfa doctor polymorfa doctor --profile work --json ``` -`doctor` runs five checks and exits with code 1 if any fails: - -| Check | What it does | -| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `node` | Requires Node.js 22 or newer. | -| `config` | Loads and validates the profile config and reports the active profile. | -| `keyring` | Reads the selected profile's entry from the operating-system keyring. A locked or unavailable keyring fails, or warns when an environment credential is in use. | -| `api` | Calls the unauthenticated `/ping` on the configured base URL, without retries. | -| `credential` | For a server API key, lists projects as an authenticated ping. `401` fails; `403` warns that the key authenticated but project listing was denied. Project and client tokens are reported as present but are not validated remotely. | - -Environment credentials take precedence over the active profile, as for other -commands. Output never contains credential values. +`doctor` runs five checks and exits with code 1 if any fails or credential +validation is skipped. A successful run needs one server API key. It does not +change credentials or configuration. + +| Check | What it does | +| ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `node` | Requires Node.js 22 or newer. | +| `config` | Loads and validates the profile config and reports the active profile. | +| `keyring` | Reads the selected profile's entry from the operating-system keyring. A locked or unavailable keyring fails, or warns when an environment credential is in use. | +| `api` | Calls the unauthenticated `/ping` on the configured base URL, without retries. | +| `credential` | For a server API key, lists projects as an authenticated ping. `401` fails; `403` warns that the key authenticated but project listing was denied. Missing credentials fail. Project and client tokens are reported as present, but remote validation is skipped and the command exits 1. | + +An explicit `--profile` selects that profile. Otherwise, one environment +credential takes precedence over the active profile; multiple credential +environment variables fail as ambiguous. Output never contains credential values. ## QuickLinks diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 11a66257..67b15372 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -5,7 +5,7 @@ import {runDoctor} from '../services/doctor.js' export default class Doctor extends BaseCommand { static override description = - 'Check Node.js, config, keyring access, API reachability, and API key validity.' + 'Check Node.js, config, keyring access, API reachability, and server API key validity. Exits 1 if validation fails or is skipped.' static override examples = [ '<%= config.bin %> doctor', diff --git a/src/services/doctor.ts b/src/services/doctor.ts index 649dec6a..35c5cbcf 100644 --- a/src/services/doctor.ts +++ b/src/services/doctor.ts @@ -64,12 +64,12 @@ export async function runDoctor( } const envKey = nonEmpty(deps.env.POLYMORFA_API_KEY) - const envOther = - nonEmpty(deps.env.POLYMORFA_PROJECT_TOKEN) ?? - nonEmpty(deps.env.POLYMORFA_CLIENT_TOKEN) - const useEnv = - nonEmpty(deps.profile) === undefined && - (envKey !== undefined || envOther !== undefined) + const envProjectToken = nonEmpty(deps.env.POLYMORFA_PROJECT_TOKEN) + const envClientToken = nonEmpty(deps.env.POLYMORFA_CLIENT_TOKEN) + const envCredentialCount = [envKey, envProjectToken, envClientToken].filter( + (value) => value !== undefined, + ).length + const useEnv = nonEmpty(deps.profile) === undefined && envCredentialCount > 0 const profileName = nonEmpty(deps.profile) ?? config?.activeProfile const profile = profileName === undefined ? undefined : config?.profiles[profileName] @@ -120,11 +120,11 @@ export async function runDoctor( }) } if (useEnv) { - credential = envKey ?? envOther + credential = envKey ?? envProjectToken ?? envClientToken kind = envKey !== undefined ? 'apiKey' - : nonEmpty(deps.env.POLYMORFA_PROJECT_TOKEN) !== undefined + : envProjectToken !== undefined ? 'projectToken' : 'clientToken' } @@ -151,18 +151,30 @@ export async function runDoctor( } checks.push( - await credentialCheck({ - baseUrl, - credential, - kind, - lookup: deps.lookup ?? listProjectsPing, - reachable, - source: useEnv ? 'environment' : `profile ${profileName ?? '-'}`, - timeoutMs: deps.timeoutMs, - }), + useEnv && envCredentialCount > 1 + ? { + name: 'credential', + status: 'fail', + detail: + 'Environment credentials are ambiguous. Set only one of POLYMORFA_API_KEY, POLYMORFA_PROJECT_TOKEN, or POLYMORFA_CLIENT_TOKEN, or select --profile.', + } + : await credentialCheck({ + baseUrl, + credential, + kind, + lookup: deps.lookup ?? listProjectsPing, + reachable, + source: useEnv ? 'environment' : `profile ${profileName ?? '-'}`, + timeoutMs: deps.timeoutMs, + }), ) - return {checks, ok: checks.every((check) => check.status !== 'fail')} + return { + checks, + ok: checks.every( + (check) => check.status !== 'fail' && check.status !== 'skip', + ), + } } function nodeCheck(version: string): DoctorCheck { @@ -188,7 +200,7 @@ async function credentialCheck(input: { if (input.credential === undefined || input.kind === undefined) { return { name: 'credential', - status: 'warn', + status: 'fail', detail: 'No credential is configured. Set POLYMORFA_API_KEY or run polymorfa auth login --with-api-key.', } @@ -204,7 +216,7 @@ async function credentialCheck(input: { return { name: 'credential', status: 'skip', - detail: `A ${input.kind === 'projectToken' ? 'project token' : 'client token'} from ${input.source} is present; doctor validates only server API keys remotely.`, + detail: `A ${input.kind === 'projectToken' ? 'project token' : 'client token'} from ${input.source} is present but was not validated. Doctor needs a server API key to verify authentication; this token's command access depends on the command.`, } } try { diff --git a/test/commands/doctor.test.ts b/test/commands/doctor.test.ts new file mode 100644 index 00000000..5a005d20 --- /dev/null +++ b/test/commands/doctor.test.ts @@ -0,0 +1,92 @@ +import {runCommand} from '@oclif/test' +import {mkdtemp, rm} from 'node:fs/promises' +import {tmpdir} from 'node:os' +import {join} from 'node:path' +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, +} from 'vitest' + +import { + startHttpServer, + withEnvironment, + type TestHttpServer, +} from '../helpers/http-server.js' + +const API_KEY = + 'pmfa_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' + +describe('doctor command exit status', () => { + let server: TestHttpServer + let directory: string + + beforeAll(async () => { + server = await startHttpServer({body: {success: true, data: []}}) + directory = await mkdtemp(join(tmpdir(), 'polymorfa-cli-doctor-')) + }) + + afterAll(async () => { + await server.close() + await rm(directory, {force: true, recursive: true}) + }) + + beforeEach(() => { + process.exitCode = undefined + }) + + afterEach(() => { + process.exitCode = undefined + }) + + async function doctor(env: Record) { + return withEnvironment( + { + POLYMORFA_API_KEY: undefined, + POLYMORFA_CLIENT_TOKEN: undefined, + POLYMORFA_PROJECT_TOKEN: undefined, + POLYMORFA_BASE_URL: server.baseUrl, + POLYMORFA_CONFIG_DIR: directory, + ...env, + }, + () => runCommand('doctor --json'), + ) + } + + test('returns success for an authenticated server API key', async () => { + const result = await doctor({POLYMORFA_API_KEY: API_KEY}) + expect(result.error, result.stderr).toBeUndefined() + expect(JSON.parse(result.stdout)).toMatchObject({ok: true}) + expect(process.exitCode).toBeUndefined() + expect(server.requests.map((request) => request.url)).toContain( + '/platform/projects', + ) + }) + + test('returns failure when credential validation is skipped', async () => { + const result = await doctor({POLYMORFA_CLIENT_TOKEN: 'pmfa_ct_example'}) + expect(result.error, result.stderr).toBeUndefined() + expect(JSON.parse(result.stdout)).toMatchObject({ok: false}) + expect(process.exitCode).toBe(1) + }) + + test('returns failure for ambiguous credentials without using either', async () => { + const requestsBefore = server.requests.length + const result = await doctor({ + POLYMORFA_API_KEY: API_KEY, + POLYMORFA_PROJECT_TOKEN: 'private-project-token', + }) + expect(result.error, result.stderr).toBeUndefined() + expect(JSON.parse(result.stdout)).toMatchObject({ok: false}) + expect(process.exitCode).toBe(1) + expect( + server.requests.slice(requestsBefore).map((request) => request.url), + ).toEqual(['/ping']) + expect(result.stdout).not.toContain(API_KEY) + expect(result.stdout).not.toContain('private-project-token') + }) +}) diff --git a/test/services/doctor.test.ts b/test/services/doctor.test.ts index f389bcd1..67337e1d 100644 --- a/test/services/doctor.test.ts +++ b/test/services/doctor.test.ts @@ -84,7 +84,7 @@ describe('doctor', () => { }, }), ) - expect(status(report)).toMatchObject({credential: 'warn', keyring: 'fail'}) + expect(status(report)).toMatchObject({credential: 'fail', keyring: 'fail'}) expect(report.ok).toBe(false) }) @@ -195,10 +195,76 @@ describe('doctor', () => { expect(report.ok).toBe(true) }) + test('fails when no credential can be used', async () => { + const lookup = vi.fn() + const report = await runDoctor( + deps({ + credentials: { + delete: () => Promise.resolve(), + get: () => Promise.resolve(undefined), + set: () => Promise.resolve(), + }, + lookup, + profiles: { + load: () => Promise.resolve({profiles: {}, version: 2}), + save: () => Promise.resolve(), + }, + }), + ) + expect(status(report)).toMatchObject({credential: 'fail', keyring: 'ok'}) + expect(report.ok).toBe(false) + expect(lookup).not.toHaveBeenCalled() + }) + + test('rejects ambiguous environment credentials without remote validation', async () => { + const lookup = vi.fn() + const report = await runDoctor( + deps({ + env: { + POLYMORFA_API_KEY: KEY, + POLYMORFA_PROJECT_TOKEN: 'secret-project', + }, + lookup, + }), + ) + expect(status(report).credential).toBe('fail') + expect(report.ok).toBe(false) + expect(lookup).not.toHaveBeenCalled() + expect(JSON.stringify(report)).not.toContain(KEY) + expect(JSON.stringify(report)).not.toContain('secret-project') + }) + + test('an explicit profile ignores ambient environment credentials', async () => { + const lookup = vi.fn(() => Promise.resolve([])) + const report = await runDoctor( + deps({ + env: { + POLYMORFA_API_KEY: 'secret-api', + POLYMORFA_CLIENT_TOKEN: 'secret-client', + }, + lookup, + profile: 'work', + }), + ) + expect(report.ok).toBe(true) + expect(lookup).toHaveBeenCalledWith( + expect.objectContaining({credential: KEY}), + ) + }) + test('does not remotely validate client tokens', async () => { const report = await runDoctor( deps({env: {POLYMORFA_CLIENT_TOKEN: 'pmfa_ct_x'}}), ) expect(status(report).credential).toBe('skip') + expect(report.ok).toBe(false) + }) + + test('does not report success for an unvalidated project token', async () => { + const report = await runDoctor( + deps({env: {POLYMORFA_PROJECT_TOKEN: 'pmfa_pt_x'}}), + ) + expect(status(report).credential).toBe('skip') + expect(report.ok).toBe(false) }) })