diff --git a/README.md b/README.md index 45d9204..40390bf 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,8 @@ Credential precedence (first match wins): 3. `~/.polylane/credentials.json` (OAuth, from `auth login` / `auth signup`) 4. `api_key` in `~/.polylane/config.json` (from `auth login --api-key`) +A command's own `--api-key` is not the Polylane key: `cloud connect --provider render --api-key ` and `cloud connect --provider triggerdev --api-key ` take the provider's key, and the Polylane credential comes from the layers above. + The environment variable outranks the credentials file so that a key exported in CI is never silently overridden by a stale OAuth token left on the runner. For account lifecycle operations beyond signup/login (reset password, update profile, delete account, notification settings) — use the web console. They're available via `polylane api call ` if you really need them from the CLI, but they're not first-class commands. diff --git a/src/args.ts b/src/args.ts index 8273961..d741ccd 100644 --- a/src/args.ts +++ b/src/args.ts @@ -1,4 +1,5 @@ import { type OptionDef, extractFlagName, hasValue } from './command'; +import type { GlobalFlags } from './types/flags'; import { CLIError } from './errors/base'; import { ExitCode } from './errors/codes'; @@ -152,3 +153,18 @@ export function parseFlags( return { flags, positional }; } + +// parseFlags folds global and command options into one record, so a command +// that declares a flag by the same name as a global one (`cloud connect +// --api-key` takes the provider's key; the global `--api-key` is the Polylane +// key) hands both meanings to the same key. The command's declaration wins: +// the value is the command's and never reaches the global layer, or a Render +// or Trigger.dev key would be sent as the Polylane credential. +export function globalFlagsOf(flags: Record, commandOptions: OptionDef[]): GlobalFlags { + const commandOwned = new Set(commandOptions.map((opt) => kebabToCamel(extractFlagName(opt.flag)))); + const global: Record = {}; + for (const [key, value] of Object.entries(flags)) { + if (!commandOwned.has(key)) global[key] = value; + } + return global as GlobalFlags; +} diff --git a/src/auth/resolver.ts b/src/auth/resolver.ts index 628cfe8..84709f1 100644 --- a/src/auth/resolver.ts +++ b/src/auth/resolver.ts @@ -8,21 +8,16 @@ import { ExitCode } from '../errors/codes'; // Precedence: --api-key flag > POLYLANE_API_KEY > ~/.polylane/credentials.json // (OAuth) > ~/.polylane/config.json api_key. The env var sits above the // credentials file on purpose: a CI runner exporting POLYLANE_API_KEY must not -// be silently overridden by a stale OAuth token left on disk. +// be silently overridden by a stale OAuth token left on disk. The loader +// records which layer supplied config.apiKey; argv is never consulted here, +// since a command's own `--api-key` (a provider key) is not the Polylane key. export async function resolveCredential(config: Config): Promise { - // 1. Flag-provided api key - if (process.argv.includes('--api-key') || process.argv.some((a) => a.startsWith('--api-key='))) { - if (config.apiKey) { - return { type: 'api-key', key: config.apiKey, source: 'flag' }; - } - } - - // 2. Env var - if (process.env.POLYLANE_API_KEY) { - return { type: 'api-key', key: process.env.POLYLANE_API_KEY, source: 'env' }; + // 1. Flag or env api key + if (config.apiKey && (config.apiKeySource === 'flag' || config.apiKeySource === 'env')) { + return { type: 'api-key', key: config.apiKey, source: config.apiKeySource }; } - // 3. OAuth credentials on disk + // 2. OAuth credentials on disk const stored = readCredentials(); if (stored) { if (isTokenExpiringSoon(stored)) { @@ -36,7 +31,7 @@ export async function resolveCredential(config: Config): Promise { } } - // 4. Config file + // 3. Config file if (config.apiKey) { return { type: 'api-key', key: config.apiKey, source: 'config' }; } diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 173557e..f6b2253 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -120,7 +120,9 @@ async function apiKeyLogin(config: Config, key: string): Promise { const name = user.forename ? `${user.forename}${user.surname ? ' ' + user.surname : ''}` : user.email ?? user.id; process.stderr.write(`\nSigned in as ${name} (${user.email ?? user.id})\n`); - const configWithKey: Config = { ...config, apiKey: key }; + // The key just accepted drives the rest of the sign-in ahead of any OAuth + // session left on disk, exactly as a global `--api-key` would. + const configWithKey: Config = { ...config, apiKey: key, apiKeySource: 'flag' }; const wsId = await selectWorkspace(configWithKey, user); writeConfigFile({ diff --git a/src/config/loader.ts b/src/config/loader.ts index f3565c8..deeef2f 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,5 +1,6 @@ import { CONFIG_FILE, ensureConfigDir } from './paths'; import { + type ApiKeySource, type Config, type RawConfig, DEFAULT_DOMAIN, @@ -69,6 +70,8 @@ export function loadConfig(flags: GlobalFlags): Config { const apiKey = flags.apiKey ?? env.POLYLANE_API_KEY ?? file.api_key; if (apiKey !== undefined) validateApiKey(apiKey); + const apiKeySource: ApiKeySource | undefined = + flags.apiKey !== undefined ? 'flag' : env.POLYLANE_API_KEY !== undefined ? 'env' : file.api_key !== undefined ? 'config' : undefined; const workspaceId = flags.workspace ?? env.POLYLANE_WORKSPACE_ID ?? file.workspace_id; if (workspaceId !== undefined) validateWorkspaceId(workspaceId); @@ -110,6 +113,7 @@ export function loadConfig(flags: GlobalFlags): Config { return { apiKey, + apiKeySource, domain, workspaceId, output, diff --git a/src/config/schema.ts b/src/config/schema.ts index b4c0dab..2d41cc0 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -4,6 +4,9 @@ import { ExitCode } from '../errors/codes'; export interface Config { apiKey?: string; + /** Which layer supplied `apiKey`; the resolver ranks a flag or env key above + * stored OAuth credentials and a config-file key below them. */ + apiKeySource?: ApiKeySource; domain: string; workspaceId?: string; output: OutputFormat; @@ -20,6 +23,8 @@ export interface Config { hints: boolean; } +export type ApiKeySource = 'flag' | 'env' | 'config'; + export interface RawConfig { api_key?: string; domain?: string; diff --git a/src/main.ts b/src/main.ts index 049a7c5..eaad348 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,4 +1,4 @@ -import { parseFlags, scanCommandPath } from './args'; +import { globalFlagsOf, parseFlags, scanCommandPath } from './args'; import { GLOBAL_OPTIONS } from './command'; import type { GlobalFlags } from './types/flags'; import { loadConfig } from './config/loader'; @@ -122,7 +122,7 @@ async function run(): Promise { command.options ?? [], GLOBAL_OPTIONS ); - const globalFlags = flags as GlobalFlags; + const globalFlags = globalFlagsOf(flags, command.options ?? []); const config = loadConfig(globalFlags); if (globalFlags.help) { diff --git a/test/args.test.ts b/test/args.test.ts index f8de3a6..9db2e74 100644 --- a/test/args.test.ts +++ b/test/args.test.ts @@ -1,6 +1,6 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; -import { parseFlags, scanCommandPath } from '../src/args'; +import { globalFlagsOf, parseFlags, scanCommandPath } from '../src/args'; import { GLOBAL_OPTIONS } from '../src/command'; import type { OptionDef } from '../src/command'; @@ -99,3 +99,26 @@ describe('parseFlags', () => { assert.deepEqual(positional, ['--something', 'else']); }); }); + +describe('globalFlagsOf', () => { + const colliding: OptionDef[] = [ + { flag: '--api-key ', description: 'provider key', type: 'string' }, + { flag: '--enabled', description: 'enabled', type: 'boolean' }, + ]; + + it('drops a flag the command declares itself, even when a global flag shares its name', () => { + const { flags } = parseFlags(['--api-key', 'rnd_x', '--workspace', 'ws_1', '--enabled'], colliding, GLOBAL_OPTIONS); + assert.deepEqual(globalFlagsOf(flags, colliding), { workspace: 'ws_1' }); + assert.equal(flags.apiKey, 'rnd_x'); + }); + + it('keeps a global flag for a command that does not declare it', () => { + const { flags } = parseFlags(['--api-key', 'sk_x', '--name', 'n'], commandOptions, GLOBAL_OPTIONS); + assert.deepEqual(globalFlagsOf(flags, commandOptions), { apiKey: 'sk_x' }); + }); + + it('passes everything through for a command with no options', () => { + const { flags } = parseFlags(['--api-key', 'sk_x', '--quiet'], [], GLOBAL_OPTIONS); + assert.deepEqual(globalFlagsOf(flags, []), { apiKey: 'sk_x', quiet: true }); + }); +}); diff --git a/test/connect-api-key-collision.test.ts b/test/connect-api-key-collision.test.ts new file mode 100644 index 0000000..f28b132 --- /dev/null +++ b/test/connect-api-key-collision.test.ts @@ -0,0 +1,119 @@ +import { describe, it, beforeEach, afterEach, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Command } from '../src/command'; +import type { GlobalFlags } from '../src/types/flags'; + +// HOME must point at a temp dir before any source module loads so the loader +// and the resolver read this test's files, not the developer's. +const tempHome = mkdtempSync(join(tmpdir(), 'polylane-api-key-collision-test-')); +process.env.HOME = tempHome; +after(() => rmSync(tempHome, { recursive: true, force: true })); + +const { parseFlags, globalFlagsOf } = await import('../src/args'); +const { GLOBAL_OPTIONS } = await import('../src/command'); +const { loadConfig } = await import('../src/config/loader'); +const { resolveCredential } = await import('../src/auth/resolver'); +const { cloudConnectCommand } = await import('../src/commands/cloud/connect'); +const { cloudListCommand } = await import('../src/commands/cloud/list'); + +const configDir = join(tempHome, '.polylane'); +const credentialsFile = join(configDir, 'credentials.json'); + +// What `polylane auth login` leaves behind: a valid OAuth session. +function writeLoginCredentials(): void { + mkdirSync(configDir, { recursive: true }); + writeFileSync( + credentialsFile, + JSON.stringify({ + access_token: 'oauth-from-auth-login', + refresh_token: 'refresh', + expires_at: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + token_type: 'Bearer', + scope: '', + }), + { mode: 0o600 } + ); +} + +// The same steps main.ts runs between the command lookup and the auth gate. +// `argv` is what follows the command path, and process.argv carries the whole +// invocation as it does in a real process. +async function resolveInvocation(command: Command, argv: string[]) { + process.argv = ['node', 'polylane', ...command.name.split(' '), ...argv]; + const { flags } = parseFlags(argv, command.options ?? [], GLOBAL_OPTIONS); + const config = loadConfig(globalFlagsOf(flags, command.options ?? []) as GlobalFlags); + const credential = await resolveCredential(config); + return { flags, config, credential }; +} + +describe('cloud connect --api-key is the provider key, not the Polylane key', () => { + const originalArgv = [...process.argv]; + const originalEnv = { ...process.env }; + + beforeEach(() => { + delete process.env.POLYLANE_API_KEY; + rmSync(credentialsFile, { force: true }); + }); + + afterEach(() => { + process.argv = [...originalArgv]; + process.env = { ...originalEnv, HOME: tempHome }; + }); + + for (const [provider, key] of [ + ['triggerdev', 'tr_prod_sk_x'], + ['render', 'rnd_x'], + ] as const) { + it(`${provider}: a signed-in user's OAuth session is used, and the ${provider} key reaches the command`, async () => { + writeLoginCredentials(); + + const { flags, config, credential } = await resolveInvocation(cloudConnectCommand, [ + '--provider', + provider, + '--api-key', + key, + ]); + + assert.equal(credential.type, 'oauth'); + assert.equal(credential.type === 'oauth' && credential.accessToken, 'oauth-from-auth-login'); + assert.equal(config.apiKey, undefined); + assert.equal(flags.apiKey, key); + }); + } + + it('the provider key is never tried as the Polylane credential when nothing else is set', async () => { + await assert.rejects( + resolveInvocation(cloudConnectCommand, ['--provider', 'triggerdev', '--api-key', 'tr_prod_sk_x']), + /Not signed in/ + ); + }); + + it('POLYLANE_API_KEY still authenticates connect while the provider key reaches the command', async () => { + writeLoginCredentials(); + process.env.POLYLANE_API_KEY = 'sk_from_env'; + + const { flags, credential } = await resolveInvocation(cloudConnectCommand, [ + '--provider', + 'triggerdev', + '--api-key', + 'tr_prod_sk_x', + ]); + + assert.equal(credential.type === 'api-key' && credential.key, 'sk_from_env'); + assert.equal(credential.type === 'api-key' && credential.source, 'env'); + assert.equal(flags.apiKey, 'tr_prod_sk_x'); + }); + + it('the global --api-key still authenticates a command without its own --api-key', async () => { + writeLoginCredentials(); + + const { config, credential } = await resolveInvocation(cloudListCommand, ['--api-key', 'sk_from_flag']); + + assert.equal(config.apiKey, 'sk_from_flag'); + assert.equal(credential.type === 'api-key' && credential.key, 'sk_from_flag'); + assert.equal(credential.type === 'api-key' && credential.source, 'flag'); + }); +}); diff --git a/test/loader.test.ts b/test/loader.test.ts index e40670d..c37a807 100644 --- a/test/loader.test.ts +++ b/test/loader.test.ts @@ -52,6 +52,26 @@ describe('loadConfig', () => { assert.equal(config.domain, 'api.prod.example.com'); }); + it('records which layer supplied the api key', () => { + assert.equal(loadConfig({} as GlobalFlags).apiKeySource, undefined); + + mkdirSync(configDir, { recursive: true }); + writeFileSync(configFile, JSON.stringify({ api_key: 'sk_file' })); + assert.deepEqual( + [loadConfig({} as GlobalFlags).apiKey, loadConfig({} as GlobalFlags).apiKeySource], + ['sk_file', 'config'] + ); + + process.env.POLYLANE_API_KEY = 'sk_env'; + assert.deepEqual( + [loadConfig({} as GlobalFlags).apiKey, loadConfig({} as GlobalFlags).apiKeySource], + ['sk_env', 'env'] + ); + + const fromFlag = loadConfig({ apiKey: 'sk_flag' } as GlobalFlags); + assert.deepEqual([fromFlag.apiKey, fromFlag.apiKeySource], ['sk_flag', 'flag']); + }); + it('parses timeout from env', () => { process.env.POLYLANE_TIMEOUT = '60'; const config = loadConfig({} as GlobalFlags); diff --git a/test/resolver.test.ts b/test/resolver.test.ts index 302c4d3..5a52948 100644 --- a/test/resolver.test.ts +++ b/test/resolver.test.ts @@ -34,25 +34,30 @@ function writeStaleCredentials(): void { } describe('resolveCredential precedence', () => { - const originalArgv = [...process.argv]; const originalEnv = { ...process.env }; beforeEach(() => { delete process.env.POLYLANE_API_KEY; - process.argv = originalArgv.filter((a) => !a.startsWith('--api-key')); rmSync(credentialsFile, { force: true }); }); afterEach(() => { - process.argv = [...originalArgv]; process.env = { ...originalEnv, HOME: tempHome }; }); + it('an env key set in the process but not recorded by the loader is not a credential', async () => { + writeStaleCredentials(); + process.env.POLYLANE_API_KEY = 'sk_from_env'; + + const cred = await resolveCredential(mockConfig()); + assert.equal(cred.type, 'oauth'); + }); + it('POLYLANE_API_KEY wins over a stale credentials.json', async () => { writeStaleCredentials(); process.env.POLYLANE_API_KEY = 'sk_from_env'; - const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_env' })); + const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_env', apiKeySource: 'env' })); assert.equal(cred.type, 'api-key'); assert.equal(cred.type === 'api-key' && cred.key, 'sk_from_env'); assert.equal(cred.type === 'api-key' && cred.source, 'env'); @@ -61,9 +66,8 @@ describe('resolveCredential precedence', () => { it('--api-key wins over POLYLANE_API_KEY and credentials.json', async () => { writeStaleCredentials(); process.env.POLYLANE_API_KEY = 'sk_from_env'; - process.argv.push('--api-key', 'sk_from_flag'); - const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_flag' })); + const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_flag', apiKeySource: 'flag' })); assert.equal(cred.type === 'api-key' && cred.key, 'sk_from_flag'); assert.equal(cred.type === 'api-key' && cred.source, 'flag'); }); @@ -71,13 +75,13 @@ describe('resolveCredential precedence', () => { it('credentials.json wins over the config file api_key', async () => { writeStaleCredentials(); - const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_config' })); + const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_config', apiKeySource: 'config' })); assert.equal(cred.type, 'oauth'); assert.equal(cred.type === 'oauth' && cred.accessToken, 'stale-oauth-token'); }); it('falls back to the config file api_key', async () => { - const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_config' })); + const cred = await resolveCredential(mockConfig({ apiKey: 'sk_from_config', apiKeySource: 'config' })); assert.equal(cred.type === 'api-key' && cred.key, 'sk_from_config'); assert.equal(cred.type === 'api-key' && cred.source, 'config'); });