From 2e872bc3df7410a451aa4b9777d3a1fccb047ad0 Mon Sep 17 00:00:00 2001 From: Justin Helmer Date: Thu, 17 Sep 2026 11:17:40 -0700 Subject: [PATCH] fix(auth): keep a command's own --api-key out of the Polylane credential `polylane cloud connect --provider triggerdev --api-key ` (and the Render form) failed with "Not signed in" right after a successful `polylane auth login`: the parser folds global and command options into one record, so the provider key landed in the global `apiKey`, the loader took it as the Polylane key, and the resolver's raw-argv scan for `--api-key` confirmed it. The API rejected the Trigger.dev key with a 401. The command's declaration now decides ownership: `globalFlagsOf` strips every flag the command declares before the globals reach the loader, so the provider key stays in the command's args and the Polylane credential comes from the env var, the OAuth session, or the config file. The loader records which layer supplied `apiKey` (`apiKeySource`) and the resolver reads that instead of scanning `process.argv`, so a `--api-key` anywhere on the line no longer relabels an env key as a flag key. Co-Authored-By: Claude Fable 5.1 --- README.md | 2 + src/args.ts | 16 ++++ src/auth/resolver.ts | 21 ++--- src/commands/auth/login.ts | 4 +- src/config/loader.ts | 4 + src/config/schema.ts | 5 ++ src/main.ts | 4 +- test/args.test.ts | 25 +++++- test/connect-api-key-collision.test.ts | 119 +++++++++++++++++++++++++ test/loader.test.ts | 20 +++++ test/resolver.test.ts | 20 +++-- 11 files changed, 215 insertions(+), 25 deletions(-) create mode 100644 test/connect-api-key-collision.test.ts 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'); });