Merge pull request #2956 from github/mbg/start-proxy/validation-improvements

Improve JSON validation in `start-proxy` action

src/start-proxy.test.ts

@@ -6,6 +6,9 @@ import { setupTests } from "./testing-utils";
 
 setupTests(test);
 
+const toEncodedJSON = (data: any) =>
+  Buffer.from(JSON.stringify(data)).toString("base64");
+
 test("getCredentials prefers registriesCredentials over registrySecrets", async (t) => {
   const registryCredentials = Buffer.from(
     JSON.stringify([
@@ -26,9 +29,9 @@ test("getCredentials prefers registriesCredentials over registrySecrets", async
   t.is(credentials[0].host, "npm.pkg.github.com");
 });
 
-test("getCredentials throws error when credential missing host and url", async (t) => {
+test("getCredentials throws an error when configurations are not an array", async (t) => {
   const registryCredentials = Buffer.from(
-    JSON.stringify([{ type: "npm_registry", token: "abc" }]),
+    JSON.stringify({ type: "npm_registry", token: "abc" }),
   ).toString("base64");
 
   t.throws(
@@ -40,22 +43,66 @@ test("getCredentials throws error when credential missing host and url", async (
         undefined,
       ),
     {
-      message: "Invalid credentials - must specify host or url",
+      message:
+        "Expected credentials data to be an array of configurations, but it is not.",
     },
   );
 });
 
+test("getCredentials throws error when credential is not an object", async (t) => {
+  const testCredentials = [["foo"], [null]].map(toEncodedJSON);
+
+  for (const testCredential of testCredentials) {
+    t.throws(
+      () =>
+        startProxyExports.getCredentials(
+          getRunnerLogger(true),
+          undefined,
+          testCredential,
+          undefined,
+        ),
+      {
+        message: "Invalid credentials - must be an object",
+      },
+    );
+  }
+});
+
+test("getCredentials throws error when credential missing host and url", async (t) => {
+  const testCredentials = [
+    [{ type: "npm_registry", token: "abc" }],
+    [{ type: "npm_registry", token: "abc", host: null }],
+    [{ type: "npm_registry", token: "abc", url: null }],
+  ].map(toEncodedJSON);
+
+  for (const testCredential of testCredentials) {
+    t.throws(
+      () =>
+        startProxyExports.getCredentials(
+          getRunnerLogger(true),
+          undefined,
+          testCredential,
+          undefined,
+        ),
+      {
+        message: "Invalid credentials - must specify host or url",
+      },
+    );
+  }
+});
+
 test("getCredentials filters by language when specified", async (t) => {
   const mixedCredentials = [
     { type: "npm_registry", host: "npm.pkg.github.com", token: "abc" },
     { type: "maven_repository", host: "maven.pkg.github.com", token: "def" },
     { type: "nuget_feed", host: "nuget.pkg.github.com", token: "ghi" },
+    { type: "goproxy_server", host: "goproxy.example.com", token: "jkl" },
   ];
 
   const credentials = startProxyExports.getCredentials(
     getRunnerLogger(true),
     undefined,
-    Buffer.from(JSON.stringify(mixedCredentials)).toString("base64"),
+    toEncodedJSON(mixedCredentials),
     "java",
   );
   t.is(credentials.length, 1);
@@ -67,10 +114,9 @@ test("getCredentials returns all credentials when no language specified", async
     { type: "npm_registry", host: "npm.pkg.github.com", token: "abc" },
     { type: "maven_repository", host: "maven.pkg.github.com", token: "def" },
     { type: "nuget_feed", host: "nuget.pkg.github.com", token: "ghi" },
+    { type: "goproxy_server", host: "goproxy.example.com", token: "jkl" },
   ];
-  const credentialsInput = Buffer.from(
-    JSON.stringify(mixedCredentials),
-  ).toString("base64");
+  const credentialsInput = toEncodedJSON(mixedCredentials);
 
   const credentials = startProxyExports.getCredentials(
     getRunnerLogger(true),
@@ -78,7 +124,7 @@ test("getCredentials returns all credentials when no language specified", async
     credentialsInput,
     undefined,
   );
-  t.is(credentials.length, 3);
+  t.is(credentials.length, mixedCredentials.length);
 });
 
 test("getCredentials throws an error when non-printable characters are used", async (t) => {

src/start-proxy.ts

@@ -27,6 +27,15 @@ const LANGUAGE_TO_REGISTRY_TYPE: Record<Language, string> = {
   swift: "",
 } as const;
 
+/**
+ * Checks that `value` is neither `undefined` nor `null`.
+ * @param value The value to test.
+ * @returns Narrows the type of `value` to exclude `undefined` and `null`.
+ */
+function isDefined<T>(value: T | null | undefined): value is T {
+  return value !== undefined && value !== null;
+}
+
 // getCredentials returns registry credentials from action inputs.
 // It prefers `registries_credentials` over `registry_secrets`.
 // If neither is set, it returns an empty array.
@@ -63,17 +72,28 @@ export function getCredentials(
     throw new ConfigurationError("Invalid credentials format.");
   }
 
+  // Check that the parsed data is indeed an array.
+  if (!Array.isArray(parsed)) {
+    throw new ConfigurationError(
+      "Expected credentials data to be an array of configurations, but it is not.",
+    );
+  }
+
   const out: Credential[] = [];
   for (const e of parsed) {
+    if (e === null || typeof e !== "object") {
+      throw new ConfigurationError("Invalid credentials - must be an object");
+    }
+
     // Mask credentials to reduce chance of accidental leakage in logs.
-    if (e.password !== undefined) {
+    if (isDefined(e.password)) {
       core.setSecret(e.password);
     }
-    if (e.token !== undefined) {
+    if (isDefined(e.token)) {
       core.setSecret(e.token);
     }
 
-    if (e.url === undefined && e.host === undefined) {
+    if (!isDefined(e.url) && !isDefined(e.host)) {
       // The proxy needs one of these to work. If both are defined, the url has the precedence.
       throw new ConfigurationError(
         "Invalid credentials - must specify host or url",