fix: improving our error messages when API keys aren't supplied

README.md

@@ -109,12 +109,12 @@ To ensure you're getting the latest features and security updates, we recommend
 
 ## Authentication
 
-For local CLI usage with a single project, you can authenticate `rdme` to your ReadMe project using `rdme login`. Once you follow the prompts and are successfully authenticated, your API key will be saved to a local configuration file (`~/.config/configstore/rdme-production.json`) and you won't have to provide the `--key` option to commands that require it.
+For local CLI usage with a single project, you can authenticate `rdme` to your ReadMe project using `rdme login`. Once you follow the prompts and are successfully authenticated, your API key will be saved to a local configuration file (`~/.config/configstore/rdme-production.json`) and you won't have to provide the `--key` flag to commands that require it.
 
 > [!WARNING]
-> For security reasons, we strongly recommend providing a project API key via the `--key` option in automations or CI environments (GitHub Actions, CircleCI, Travis CI, etc.). It's also recommended if you're working with multiple ReadMe projects to avoid accidentally overwriting existing data.
+> For security reasons, we strongly recommend providing a project API key via the `--key` flag in automations or CI environments (GitHub Actions, CircleCI, Travis CI, etc.). It's also recommended if you're working with multiple ReadMe projects to avoid accidentally overwriting existing data.
 
-You can also pass in your API key via environment variable. Here is the order of precedence when passing your API key into `rdme`:
+You can also pass in your API key via an environment variable. Here is the order of precedence when passing your API key into `rdme`:
 
 1. The `--key` option. If that isn't present, we look for...
 1. The `RDME_API_KEY` environment variable. If that isn't present, we look for...
@@ -166,7 +166,7 @@ rdme openapi validate --github
 
 This will run through the `openapi validate` command, ask you a few quick questions, and then automatically create a fully functional GitHub Actions workflow file for you. 🪄
 
-You can see examples featuring the latest version in [our docs](https://docs.readme.com/main/docs/rdme#github-actions-examples). We recommend [configuring Dependabot to keep your actions up-to-date](https://docs.github.com/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot). 
+You can see examples featuring the latest version in [our docs](https://docs.readme.com/main/docs/rdme#github-actions-examples). We recommend [configuring Dependabot to keep your actions up-to-date](https://docs.github.com/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot).
 
 -->
 

src/lib/getCurrentConfig.ts

@@ -2,6 +2,15 @@ import type { Hook } from '@oclif/core';
 
 import configstore from './configstore.js';
 
+export function normalizeAPIKey(value: string | undefined): string | undefined {
+  if (value === undefined) {
+    return undefined;
+  }
+
+  const trimmed = value.trim();
+  return trimmed.length > 0 ? trimmed : undefined;
+}
+
 /**
  * Retrieves stored user data values from env variables or configstore,
  * with env variables taking precedent
@@ -12,16 +21,20 @@ export default function getCurrentConfig(this: Hook.Context): {
   project?: string;
 } {
   const apiKey = (() => {
-    if (process.env.RDME_API_KEY) {
+    const rdmeAPIKey = normalizeAPIKey(process.env.RDME_API_KEY);
+    if (rdmeAPIKey) {
       this.debug('using RDME_API_KEY env var for api key');
-      return process.env.RDME_API_KEY;
-    } else if (process.env.README_API_KEY) {
+      return rdmeAPIKey;
+    }
+
+    const readmeAPIKey = normalizeAPIKey(process.env.README_API_KEY);
+    if (readmeAPIKey) {
       this.debug('using README_API_KEY env var for api key');
-      return process.env.README_API_KEY;
+      return readmeAPIKey;
     }
 
     this.debug('falling back to configstore value for api key');
-    return configstore.get<string>('apiKey');
+    return normalizeAPIKey(configstore.get<string>('apiKey'));
   })();
 
   const email = (() => {

src/lib/hooks/prerun.ts

@@ -2,63 +2,63 @@
 import type { Hook } from '@oclif/core';
 
 import { Flags } from '@oclif/core';
-import chalk from 'chalk';
 
 import configstore from '../configstore.js';
 import { keyFlag } from '../flags.js';
-import getCurrentConfig from '../getCurrentConfig.js';
-import isCI, { isTest } from '../isCI.js';
+import getCurrentConfig, { normalizeAPIKey } from '../getCurrentConfig.js';
+import isCI from '../isCI.js';
 import { info } from '../logger.js';
 import loginFlow from '../loginFlow.js';
 
 const hook: Hook.Prerun = async function run(options) {
   this.debug('configstore location:', configstore.path);
   if (options.Command?.flags?.key) {
     this.debug('current command has --key flag');
-    if (isTest()) {
-      options.Command.flags.key = keyFlag;
-    } else {
-      options.Command.flags.key = Flags.string({
-        // `parse` is run if the user passes in a `--key` flag
-        parse: async input => {
-          this.debug('--key flag detected in parse function');
-          const { email, project } = getCurrentConfig.call(this);
-          // We only want to log this if the API key is stored in the configstore, **not** in an env var.
-          if (input && configstore.get('apiKey') === input) {
-            info(
-              `🔑 ${chalk.green(email)} is currently logged in, using the stored API key for this project: ${chalk.blue(
-                project,
-              )}`,
-              { includeEmojiPrefix: false },
-            );
-          }
+    options.Command.flags.key = Flags.string({
+      summary: keyFlag.summary,
+      required: keyFlag.required,
+      description: keyFlag.description,
+      parse: async input => {
+        this.debug('--key flag detected in parse function');
+        const trimmed = input === undefined || input === null ? '' : String(input).trim();
+        if (!trimmed) {
+          throw new Error('No project API key was specified.');
+        }
 
-          return input;
-        },
-        // `default` is run if no `--key` flag is passed
-        default: async () => {
-          this.debug('no --key flag detected, running default function');
-          const { apiKey } = getCurrentConfig.call(this);
-          // if the user is passing an API key via env var or configstore, use that
-          if (apiKey) {
-            this.debug('api key found in config, returning');
-            return apiKey;
-          }
+        return trimmed;
+      },
 
-          if (isCI()) {
-            throw new Error('No project API key provided. Please use `--key`.');
-          }
+      // `default` is run if no `--key` flag is passed
+      default: async () => {
+        this.debug('no --key flag detected, running default function');
+        const { apiKey } = getCurrentConfig.call(this);
 
-          // if in non-CI and the user hasn't passed in a key, we prompt them to log in
-          info("Looks like you're missing a ReadMe API key, let's fix that! 🦉", { includeEmojiPrefix: false });
-          const result = await loginFlow.call(this);
-          info(result, { includeEmojiPrefix: false });
+        // if the user is passing an API key via an env var or configstore, use that
+        if (apiKey) {
+          this.debug('api key found in config, returning');
+          return apiKey;
+        }
 
-          // loginFlow sets the configstore value, so let's use that
-          return configstore.get('apiKey');
-        },
-      });
-    }
+        if (isCI()) {
+          throw new Error(
+            'No project API key was provided. Please provide one with `--key` or the `RDME_API_KEY` or `README_API_KEY` environment variables.',
+          );
+        }
+
+        // if in non-CI and the user hasn't passed in a key, we prompt them to log in
+        info("Looks like you're missing a ReadMe API key, let's fix that! 🦉", { includeEmojiPrefix: false });
+        const result = await loginFlow.call(this);
+        info(result, { includeEmojiPrefix: false });
+
+        // loginFlow sets the configstore value, so let's use that
+        const storedKey = normalizeAPIKey(configstore.get<string>('apiKey'));
+        if (!storedKey) {
+          throw new Error("We couldn't find your API key.");
+        }
+
+        return storedKey;
+      },
+    });
   } else {
     this.debug('current command does not have --key flag');
   }

test/commands/openapi/upload.test.ts

@@ -5,9 +5,10 @@ import fs from 'node:fs/promises';
 import nock from 'nock';
 import prompts from 'prompts';
 import slugify from 'slugify';
-import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import Command from '../../../src/commands/openapi/upload.js';
+import configstore from '../../../src/lib/configstore.js';
 import petstore from '../../__fixtures__/petstore-simple-weird-version.json' with { type: 'json' };
 import { getAPIv2Mock, getAPIv2MockForGHA } from '../../helpers/get-api-mock.js';
 import { githubActionsEnv } from '../../helpers/git-mock.js';
@@ -42,6 +43,47 @@ describe('rdme openapi upload', () => {
 
       expect(result).toMatchSnapshot();
     });
+
+    describe('API key validation', () => {
+      it('should error when `--key` is empty', async () => {
+        const result = await run(['--branch', branch, filename, '--key', '']);
+
+        expect(result.error?.message).toContain('No project API key was specified.');
+      });
+
+      it('should error when `--key` is whitespace-only', async () => {
+        const result = await run(['--branch', branch, filename, '--key', '   ']);
+
+        expect(result.error?.message).toContain('No project API key was specified.');
+      });
+
+      describe('in CI without env or configstore key', () => {
+        const originalGet = configstore.get.bind(configstore);
+
+        beforeEach(() => {
+          vi.stubEnv('TEST_RDME_CI', 'true');
+          vi.stubEnv('RDME_API_KEY', '');
+          vi.stubEnv('README_API_KEY', '');
+          vi.spyOn(configstore, 'get').mockImplementation((storeKey: string) => {
+            if (storeKey === 'apiKey') return;
+            return originalGet(storeKey);
+          });
+        });
+
+        afterEach(() => {
+          vi.unstubAllEnvs();
+          vi.restoreAllMocks();
+        });
+
+        it('should error with guidance when no API key is available', async () => {
+          const result = await run(['--branch', branch, filename]);
+
+          expect(result.error?.message).toMatchInlineSnapshot(
+            `"No project API key was provided. Please provide one with \`--key\` or the \`RDME_API_KEY\` or \`README_API_KEY\` environment variables."`,
+          );
+        });
+      });
+    });
   });
 
   describe('given that the API definition is a local file', () => {

test/helpers/oclif.ts

@@ -20,7 +20,6 @@ export const mockVersion = '7.0.0';
  * @see {@link https://oclif.io/docs/testing}
  */
 export function setupOclifConfig() {
-  // https://stackoverflow.com/a/61829368
   const root = path.join(new URL('.', import.meta.url).pathname, '.');
 
   return Config.load({
@@ -39,9 +38,16 @@ export function setupOclifConfig() {
 export function runCommand(Command: CommandClass) {
   return async function runCommandAgainstArgs(args?: string[]) {
     const oclifConfig = await setupOclifConfig();
-    // @ts-expect-error currently we have mismatching return types in our commands.
-    // we can fix this later but it's not a priority right now.
-    return captureOutput<string>(() => Command.run(args, oclifConfig), { testNodeEnv });
+    return captureOutput<string>(
+      async () => {
+        await oclifConfig.runHook('prerun', { argv: args ?? [], Command });
+
+        // @ts-expect-error currently we have mismatching return types in our commands.
+        // we can fix this later but it's not a priority right now.
+        return Command.run(args ?? [], oclifConfig);
+      },
+      { testNodeEnv },
+    );
   };
 }
 

test/lib/getCurrentConfig.test.ts

@@ -0,0 +1,16 @@
+import { describe, expect, it } from 'vitest';
+
+import { normalizeAPIKey } from '../../src/lib/getCurrentConfig.js';
+
+describe('#normalizeAPIKey()', () => {
+  it('returns undefined for missing or whitespace-only values', () => {
+    expect(normalizeAPIKey('')).toBeUndefined();
+    expect(normalizeAPIKey('   ')).toBeUndefined();
+    expect(normalizeAPIKey('\t\n')).toBeUndefined();
+  });
+
+  it('returns trimmed non-empty strings', () => {
+    expect(normalizeAPIKey('  rdme_abc  ')).toBe('rdme_abc');
+    expect(normalizeAPIKey('x')).toBe('x');
+  });
+});