Rollup of security fixes

.changeset/social-cows-yell.md

@@ -0,0 +1,5 @@
+---
+"@definitelytyped/header-parser": patch
+---
+
+Reject non-ranges in package dependencies

packages/dtslint/src/lint.ts

@@ -112,23 +112,35 @@ function getEslintOptions(
     ],
   };
 
+  // Always disable cascading .eslintrc.* discovery. Contributor-authored config files in
+  // types/<pkg>/ would otherwise be loaded by ESLint 8's legacy eslintrc engine, which
+  // resolves `extends` and `parser` (including in `overrides[]`) via
+  // createRequire(configFilePath).resolve(value) and require()s the result. Since dtslint has
+  // no file-extension allowlist, a contributor could ship a `.cjs` payload alongside
+  // `.eslintrc.json` and obtain arbitrary code execution in the lint process.
+  const baseOverrideConfig = {
+    plugins: ["@definitelytyped", "@typescript-eslint", "jsdoc"],
+    parser: "@typescript-eslint/parser",
+    parserOptions: {
+      project: true,
+      warnOnUnsupportedTypeScriptVersion: false,
+    },
+    ...overrideConfig,
+  };
+
   if (expectOnly) {
     return {
       useEslintrc: false,
-      overrideConfig: {
-        plugins: ["@definitelytyped", "@typescript-eslint", "jsdoc"],
-        parser: "@typescript-eslint/parser",
-        parserOptions: {
-          project: true,
-          warnOnUnsupportedTypeScriptVersion: false,
-        },
-        ...overrideConfig,
-      },
+      overrideConfig: baseOverrideConfig,
     };
   }
 
   return {
-    overrideConfig,
+    useEslintrc: false,
+    overrideConfig: {
+      ...baseOverrideConfig,
+      extends: ["plugin:@definitelytyped/all"],
+    },
   };
 }
 

packages/header-parser/src/index.ts

@@ -98,6 +98,13 @@ export function validatePackageJson(
       `${typesDirectoryName}'s package.json has bad "devDependencies": must include \`"@types/${typesDirectoryName}": "workspace:."\``,
     );
   }
+  // dependency version ranges
+  for (const depsKey of ["dependencies", "peerDependencies", "devDependencies"] as const) {
+    const deps = packageJson[depsKey];
+    if (deps && typeof deps === "object" && !Array.isArray(deps)) {
+      errors.push(...checkDependencyVersions(typesDirectoryName, depsKey, deps as Record<string, unknown>));
+    }
+  }
   // typesVersions
   if (needsTypesVersions) {
     assert.strictEqual(
@@ -475,10 +482,6 @@ export function checkPackageJsonDependencies(
 Please make a pull request to microsoft/DefinitelyTyped-tools adding it to \`packages/definitions-parser/allowedPackageJsonDependencies.txt\`.`;
       errors.push(`In ${path}: ${msg}`);
     }
-    const version = (dependencies as { [key: string]: unknown })[dependencyName];
-    if (typeof version !== "string") {
-      errors.push(`In ${path}: Dependency version for ${dependencyName} should be a string.`);
-    }
   }
   if (devDependencySelfName) {
     const selfDependency = (dependencies as { [key: string]: string | undefined })[devDependencySelfName];
@@ -492,3 +495,36 @@ Please make a pull request to microsoft/DefinitelyTyped-tools adding it to \`pac
   }
   return errors;
 }
+
+function checkDependencyVersions(
+  typesDirectoryName: string,
+  depsKey: "dependencies" | "peerDependencies" | "devDependencies",
+  dependencies: Record<string, unknown>,
+): string[] {
+  const errors: string[] = [];
+  for (const dependencyName of Object.keys(dependencies)) {
+    const version = dependencies[dependencyName];
+    if (typeof version !== "string") {
+      errors.push(
+        `${typesDirectoryName}'s package.json has bad "${depsKey}": version for ${dependencyName} should be a string.`,
+      );
+    } else if (version !== "workspace:." && !isValidRegistrySpec(version)) {
+      errors.push(
+        `${typesDirectoryName}'s package.json has bad "${depsKey}": version for ${dependencyName} (${JSON.stringify(
+          version,
+        )}) must be a valid semver range, dist-tag, or "workspace:.".`,
+      );
+    }
+  }
+  return errors;
+}
+
+// A registry dependency spec must be a valid semver range/version, or a dist-tag matching
+// this strict allowlist.
+const distTagRegex = /^[A-Za-z][A-Za-z0-9_-]*$/;
+function isValidRegistrySpec(spec: string): boolean {
+  const trimmed = spec.trim();
+  if (trimmed === "") return false;
+  if (semver.validRange(trimmed) !== null) return true;
+  return distTagRegex.test(trimmed);
+}

packages/header-parser/test/index.test.ts

@@ -99,6 +99,75 @@ describe("validatePackageJson", () => {
   it("works with old-version packages", () => {
     expect(Array.isArray(validatePackageJson("hapi", { ...pkgJson, version: "16.6.9999" }, []))).toBeFalsy();
   });
+  it("requires dependency versions to be valid semver ranges, dist-tags, or 'workspace:.'", () => {
+    expect(
+      validatePackageJson(
+        "hapi",
+        { ...pkgJson, dependencies: { ...(pkgJson.dependencies as object), joi: "not a range" } },
+        [],
+      ),
+    ).toEqual([
+      `hapi's package.json has bad "dependencies": version for joi ("not a range") must be a valid semver range, dist-tag, or "workspace:.".`,
+    ]);
+  });
+  it.each([
+    ["file:./local.tgz"],
+    ["./local.tgz"],
+    ["local.tgz"],
+    ["foo.tar.gz"],
+    ["git+https://example.com/x.git"],
+    ["git+ssh://git@example.com:x/y.git"],
+    ["git@example.com:x/y.git"],
+    ["https://example.com/x.tgz"],
+    ["http://example.com/x.tgz"],
+    ["user/repo"],
+    ["user/repo#branch"],
+    ["npm:other@^1"],
+    ["~/local"],
+    ["../local"],
+  ])("rejects non-registry dependency spec %p", (bad) => {
+    const result = validatePackageJson(
+      "hapi",
+      { ...pkgJson, dependencies: { ...(pkgJson.dependencies as object), joi: bad } },
+      [],
+    );
+    expect(Array.isArray(result)).toBe(true);
+    expect(result as string[]).toContainEqual(
+      `hapi's package.json has bad "dependencies": version for joi (${JSON.stringify(
+        bad,
+      )}) must be a valid semver range, dist-tag, or "workspace:.".`,
+    );
+  });
+  it.each([["latest"], ["next"], ["beta"], ["rc"], ["canary"], ["experimental"], ["nightly"]])(
+    "allows dist-tag %p as a dependency version",
+    (tag) => {
+      expect(
+        Array.isArray(
+          validatePackageJson(
+            "hapi",
+            { ...pkgJson, dependencies: { ...(pkgJson.dependencies as object), joi: tag } },
+            [],
+          ),
+        ),
+      ).toBeFalsy();
+    },
+  );
+  it("allows 'workspace:.' as a dependency version", () => {
+    expect(
+      Array.isArray(
+        validatePackageJson(
+          "hapi",
+          { ...pkgJson, dependencies: { ...(pkgJson.dependencies as object), joi: "workspace:." } },
+          [],
+        ),
+      ),
+    ).toBeFalsy();
+  });
+  it("requires dependency versions to be strings", () => {
+    expect(validatePackageJson("hapi", { ...pkgJson, peerDependencies: { foo: 5 } }, [])).toEqual([
+      `hapi's package.json has bad "peerDependencies": version for foo should be a string.`,
+    ]);
+  });
 });
 
 describe("makeTypesVersionsForPackageJson", () => {

packages/mergebot/src/_tests/discussions.test.ts

@@ -25,4 +25,18 @@ describe(extractNPMReference, () => {
   test.concurrent.each(eventActions)("(%s, %s) is %s", async (title, result) => {
     expect(extractNPMReference({ title })).toEqual(result);
   });
+
+  const invalid = [
+    "[Pkg: foo] inject", // space disallowed
+    "[node @attacker] hi", // space + invalid char
+    "[FOO] uppercase not allowed in npm names",
+    "[../etc/passwd] traversal",
+    "[]", // empty
+    "[ leading-space]",
+    "[trailing-space ]",
+    "[has\nnewline]",
+  ];
+  test.concurrent.each(invalid)("rejects invalid title %p", async (title) => {
+    expect(extractNPMReference({ title })).toBeUndefined();
+  });
 });

packages/mergebot/src/_tests/fixtures/38979/_response.json

@@ -38,7 +38,9 @@
         "state": "OPEN",
         "headRefOid": "222334139e52fc16369464cfb5dc95c82f71192f",
         "changedFiles": 72,
-        "commitIds": {},
+        "commitIds": {
+          "totalCount": 0
+        },
         "timelineItems": {
           "nodes": [
             {
@@ -835,7 +837,8 @@
               "__typename": "PullRequestReview"
             }
           ],
-          "__typename": "PullRequestReviewConnection"
+          "__typename": "PullRequestReviewConnection",
+          "totalCount": 21
         },
         "commits": {
           "totalCount": 24,
@@ -2485,7 +2488,8 @@
             }
           ],
           "__typename": "ProjectV2ItemConnection"
-        }
+        },
+        "baseRefOid": "master"
       },
       "__typename": "Repository"
     }

packages/mergebot/src/_tests/fixtures/38979/derived.json

@@ -11,6 +11,8 @@
   "isFirstContribution": false,
   "tooManyFiles": false,
   "hugeChange": false,
+  "tooManyCommits": false,
+  "tooManyReviews": false,
   "popularityLevel": "Critical",
   "pkgInfo": [
     {

packages/mergebot/src/_tests/fixtures/43136/_response.json

@@ -37,7 +37,9 @@
         "state": "OPEN",
         "headRefOid": "e6863537248bbfee8f0ef8c636bb00c25cf40b96",
         "changedFiles": 2,
-        "commitIds": {},
+        "commitIds": {
+          "totalCount": 0
+        },
         "timelineItems": {
           "nodes": [
             {
@@ -129,7 +131,8 @@
               }
             }
           ],
-          "__typename": "PullRequestReviewConnection"
+          "__typename": "PullRequestReviewConnection",
+          "totalCount": 2
         },
         "commits": {
           "totalCount": 2,
@@ -326,7 +329,8 @@
             }
           ],
           "__typename": "ProjectV2ItemConnection"
-        }
+        },
+        "baseRefOid": "master"
       },
       "__typename": "Repository"
     }

packages/mergebot/src/_tests/fixtures/43136/derived.json

@@ -11,6 +11,8 @@
   "isFirstContribution": true,
   "tooManyFiles": false,
   "hugeChange": false,
+  "tooManyCommits": false,
+  "tooManyReviews": false,
   "popularityLevel": "Critical",
   "pkgInfo": [
     {

packages/mergebot/src/_tests/fixtures/43144/_response.json

@@ -33,7 +33,9 @@
         "state": "OPEN",
         "headRefOid": "f1f5c4bb0ae553f56766882f6458d2e22baa87c7",
         "changedFiles": 2,
-        "commitIds": {},
+        "commitIds": {
+          "totalCount": 0
+        },
         "timelineItems": {
           "nodes": [
             {
@@ -96,7 +98,8 @@
               }
             }
           ],
-          "__typename": "PullRequestReviewConnection"
+          "__typename": "PullRequestReviewConnection",
+          "totalCount": 1
         },
         "commits": {
           "totalCount": 3,
@@ -267,7 +270,8 @@
             }
           ],
           "__typename": "ProjectV2ItemConnection"
-        }
+        },
+        "baseRefOid": "master"
       },
       "__typename": "Repository"
     }

packages/mergebot/src/_tests/fixtures/43144/derived.json

@@ -11,6 +11,8 @@
   "isFirstContribution": false,
   "tooManyFiles": false,
   "hugeChange": false,
+  "tooManyCommits": false,
+  "tooManyReviews": false,
   "popularityLevel": "Well-liked by everyone",
   "pkgInfo": [
     {

packages/mergebot/src/_tests/fixtures/43151/_response.json

@@ -37,7 +37,9 @@
         "state": "OPEN",
         "headRefOid": "bb6d3150b485cd203d265e06ca910262256e523e",
         "changedFiles": 4,
-        "commitIds": {},
+        "commitIds": {
+          "totalCount": 0
+        },
         "timelineItems": {
           "nodes": [
             {
@@ -61,7 +63,8 @@
         },
         "reviews": {
           "nodes": [],
-          "__typename": "PullRequestReviewConnection"
+          "__typename": "PullRequestReviewConnection",
+          "totalCount": 0
         },
         "commits": {
           "totalCount": 1,
@@ -211,7 +214,8 @@
             }
           ],
           "__typename": "ProjectV2ItemConnection"
-        }
+        },
+        "baseRefOid": "master"
       },
       "__typename": "Repository"
     }

packages/mergebot/src/_tests/fixtures/43151/derived.json

@@ -11,6 +11,8 @@
   "isFirstContribution": false,
   "tooManyFiles": false,
   "hugeChange": false,
+  "tooManyCommits": false,
+  "tooManyReviews": false,
   "popularityLevel": "Well-liked by everyone",
   "pkgInfo": [
     {

packages/mergebot/src/_tests/fixtures/43160/_response.json

@@ -37,7 +37,9 @@
         "state": "OPEN",
         "headRefOid": "6d5d2a85b41d287f97c9d331a9ff6a9824e2f1ff",
         "changedFiles": 1,
-        "commitIds": {},
+        "commitIds": {
+          "totalCount": 0
+        },
         "timelineItems": {
           "nodes": [
             {
@@ -86,7 +88,8 @@
               }
             }
           ],
-          "__typename": "PullRequestReviewConnection"
+          "__typename": "PullRequestReviewConnection",
+          "totalCount": 1
         },
         "commits": {
           "totalCount": 1,
@@ -204,7 +207,8 @@
             }
           ],
           "__typename": "ProjectV2ItemConnection"
-        }
+        },
+        "baseRefOid": "master"
       },
       "__typename": "Repository"
     }