From d38b43bac88acdef4295759d7dcc3d4c348d9575 Mon Sep 17 00:00:00 2001
From: Trevor Scheer <trevor.scheer@gmail.com>
Date: Wed, 30 Aug 2023 12:17:50 -0700
Subject: [PATCH] Merge pull request from GHSA-j5g3-5c8r-7qfx

Ref: https://github.com/apollographql/apollo-server/security/advisories/GHSA-j5g3-5c8r-7qfx

If a provided API key had characters that were invalid as
header values, usage reporting and schema reporting requests
would fail and log the API key.

This change implements two fixes to improve this:
* Trim the API key of any whitespace and log a warning
* Throw an error on startup if the key contains invalid characters
  after being `.trim()`med.
---
 .../apollo-server-core/src/ApolloServer.ts    |  2 +-
 .../src/__tests__/ApolloServerBase.test.ts    | 37 +++++++++++++++++++
 .../src/determineApolloConfig.ts              | 33 +++++++++++++++--
 3 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts
index a8117f1224f..db97245e3f4 100644
--- a/packages/apollo-server-core/src/ApolloServer.ts
+++ b/packages/apollo-server-core/src/ApolloServer.ts
@@ -214,7 +214,7 @@ export class ApolloServerBase<
       this.logger = loglevelLogger;
     }
 
-    this.apolloConfig = determineApolloConfig(apollo);
+    this.apolloConfig = determineApolloConfig(apollo, this.logger);
 
     if (gateway && (modules || schema || typeDefs || resolvers)) {
       throw new Error(
diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts
index 433ab8e7383..9c2b64fb62e 100644
--- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts
+++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts
@@ -84,6 +84,43 @@ describe('ApolloServerBase construction', () => {
       `"Apollo Server requires either an existing schema, modules or typeDefs"`,
     );
   });
+
+  it('throws when an API key is not a valid header value', () => {
+    expect(() => {
+      new ApolloServerBase({
+        typeDefs,
+        resolvers,
+        apollo: {
+          key: 'bar▒baz▒',
+        },
+      });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"The API key provided to Apollo Server contains characters which are invalid as HTTP header values. The following characters found in the key are invalid: ▒, ▒. Valid header values may only contain ASCII visible characters. If you think there is an issue with your key, please contact Apollo support."`,
+    );
+  });
+
+  it('trims whitespace from incoming API keys and logs a warning', () => {
+    const logger = {
+      debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
+      error: jest.fn(),
+    };
+    expect(() => {
+      new ApolloServerBase({
+        typeDefs,
+        resolvers,
+        apollo: {
+          key: 'barbaz\n',
+        },
+        logger,
+      });
+    }).not.toThrow();
+    expect(logger.warn).toHaveBeenCalledWith(
+      'The provided API key has unexpected leading or trailing whitespace. ' +
+        'Apollo Server will trim the key value before use.',
+    );
+  });
 });
 
 describe('ApolloServerBase start', () => {
diff --git a/packages/apollo-server-core/src/determineApolloConfig.ts b/packages/apollo-server-core/src/determineApolloConfig.ts
index 9c101d9c549..09660873e3c 100644
--- a/packages/apollo-server-core/src/determineApolloConfig.ts
+++ b/packages/apollo-server-core/src/determineApolloConfig.ts
@@ -1,10 +1,11 @@
-import type { ApolloConfig, ApolloConfigInput } from 'apollo-server-types';
+import type { ApolloConfig, ApolloConfigInput, Logger } from 'apollo-server-types';
 import createSHA from './utils/createSHA';
 
 // This function combines the `apollo` constructor argument and some environment
 // variables to come up with a full ApolloConfig.
 export function determineApolloConfig(
   input: ApolloConfigInput | undefined,
+  logger: Logger,
 ): ApolloConfig {
   const apolloConfig: ApolloConfig = {};
 
@@ -17,9 +18,21 @@ export function determineApolloConfig(
 
   // Determine key.
   if (input?.key) {
-    apolloConfig.key = input.key;
+    apolloConfig.key = input.key.trim();
   } else if (APOLLO_KEY) {
-    apolloConfig.key = APOLLO_KEY;
+    apolloConfig.key = APOLLO_KEY.trim();
+  }
+  if ((input?.key ?? APOLLO_KEY) !== apolloConfig.key) {
+    logger.warn(
+      'The provided API key has unexpected leading or trailing whitespace. ' +
+        'Apollo Server will trim the key value before use.',
+    );
+  }
+
+  // Assert API key is a valid header value, since it's going to be used as one
+  // throughout.
+  if (apolloConfig.key) {
+    assertValidHeaderValue(apolloConfig.key);
   }
 
   // Determine key hash.
@@ -65,3 +78,17 @@ export function determineApolloConfig(
 
   return apolloConfig;
 }
+
+function assertValidHeaderValue(value: string) {
+  // Ref: node-fetch@2.x `Headers` validation
+  // https://github.com/node-fetch/node-fetch/blob/9b9d45881e5ca68757077726b3c0ecf8fdca1f29/src/headers.js#L18
+  const invalidHeaderCharRegex = /[^\t\x20-\x7e\x80-\xff]/g;
+  if (invalidHeaderCharRegex.test(value)) {
+    const invalidChars = value.match(invalidHeaderCharRegex)!;
+    throw new Error(
+      `The API key provided to Apollo Server contains characters which are invalid as HTTP header values. The following characters found in the key are invalid: ${invalidChars.join(
+        ', ',
+      )}. Valid header values may only contain ASCII visible characters. If you think there is an issue with your key, please contact Apollo support.`,
+    );
+  }
+}