Increase TypeScript strictness for policies

Before, the checks were all at runtime. Now they're at runtime and compile time. See [#369][0]. [0]: #369
helmetjs · Jun 25, 2022 · 7e98093 · 7e98093
1 parent 2df93b5
commit 7e98093
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Changed
+
+- **Breaking:** Where possible, increase TypeScript strictness around some strings. Only affects TypeScript users. See [#369](https://github.com/helmetjs/helmet/issues/369)
+
 ### Removed
 
 - **Breaking:** Dropped support for Node 12 and 13. Node 14+ is now required

diff --git a/middlewares/cross-origin-embedder-policy/index.ts b/middlewares/cross-origin-embedder-policy/index.ts
@@ -1,7 +1,7 @@
 import { IncomingMessage, ServerResponse } from "http";
 
 export interface CrossOriginEmbedderPolicyOptions {
-  policy?: string;
+  policy?: "require-corp" | "credentialless";
 }
 
 const ALLOWED_POLICIES = new Set(["require-corp", "credentialless"]);

diff --git a/middlewares/cross-origin-opener-policy/index.ts b/middlewares/cross-origin-opener-policy/index.ts
@@ -1,7 +1,7 @@
 import { IncomingMessage, ServerResponse } from "http";
 
 export interface CrossOriginOpenerPolicyOptions {
-  policy?: string;
+  policy?: "same-origin" | "same-origin-allow-popups" | "unsafe-none";
 }
 
 const ALLOWED_POLICIES = new Set([

diff --git a/middlewares/cross-origin-resource-policy/index.ts b/middlewares/cross-origin-resource-policy/index.ts
@@ -1,7 +1,7 @@
 import { IncomingMessage, ServerResponse } from "http";
 
 export interface CrossOriginResourcePolicyOptions {
-  policy?: string;
+  policy?: "same-origin" | "same-site" | "cross-origin";
 }
 
 const ALLOWED_POLICIES = new Set(["same-origin", "same-site", "cross-origin"]);

diff --git a/middlewares/referrer-policy/index.ts b/middlewares/referrer-policy/index.ts
@@ -1,10 +1,21 @@
 import { IncomingMessage, ServerResponse } from "http";
 
+type ReferrerPolicyToken =
+  | "no-referrer"
+  | "no-referrer-when-downgrade"
+  | "same-origin"
+  | "origin"
+  | "strict-origin"
+  | "origin-when-cross-origin"
+  | "strict-origin-when-cross-origin"
+  | "unsafe-url"
+  | "";
+
 export interface ReferrerPolicyOptions {
-  policy?: string | string[];
+  policy?: ReferrerPolicyToken | ReferrerPolicyToken[];
 }
 
-const ALLOWED_TOKENS = new Set<string>([
+const ALLOWED_TOKENS = new Set<ReferrerPolicyToken>([
   "no-referrer",
   "no-referrer-when-downgrade",
   "same-origin",
@@ -25,7 +36,7 @@ function getHeaderValueFromOptions({
     throw new Error("Referrer-Policy received no policy tokens");
   }
 
-  const tokensSeen = new Set<string>();
+  const tokensSeen = new Set<ReferrerPolicyToken>();
   tokens.forEach((token) => {
     if (!ALLOWED_TOKENS.has(token)) {
       throw new Error(

diff --git a/middlewares/x-permitted-cross-domain-policies/index.ts b/middlewares/x-permitted-cross-domain-policies/index.ts
@@ -1,7 +1,7 @@
 import { IncomingMessage, ServerResponse } from "http";
 
 export interface XPermittedCrossDomainPoliciesOptions {
-  permittedPolicies?: string;
+  permittedPolicies?: "none" | "master-only" | "by-content-type" | "all";
 }
 
 const ALLOWED_PERMITTED_POLICIES = new Set([

diff --git a/test/cross-origin-embedder-policy.test.ts b/test/cross-origin-embedder-policy.test.ts
@@ -18,7 +18,7 @@ describe("Cross-Origin-Embedder-Policy middleware", () => {
     );
   });
 
-  ["require-corp", "credentialless"].forEach((policy) => {
+  (["require-corp", "credentialless"] as const).forEach((policy) => {
     it(`sets "Cross-Origin-Embedder-Policy: ${policy}" when told to`, async () => {
       await check(crossOriginEmbedderPolicy({ policy }), {
         "cross-origin-embedder-policy": policy,
@@ -29,16 +29,16 @@ describe("Cross-Origin-Embedder-Policy middleware", () => {
   it("throws when setting the policy to an invalid value", () => {
     const invalidValues = [
       "",
-      "NONE",
-      "by-ftp-filename",
-      123 as any,
-      null as any,
-      new String("none") as any,
+      "foo",
+      "CREDENTIALLESS",
+      123,
+      null,
+      new String("credentialless"),
     ];
     for (const policy of invalidValues) {
-      expect(() => crossOriginEmbedderPolicy({ policy })).toThrow(
-        /^Cross-Origin-Embedder-Policy does not support /
-      );
+      expect(() =>
+        crossOriginEmbedderPolicy({ policy: policy as any })
+      ).toThrow(/^Cross-Origin-Embedder-Policy does not support /);
     }
   });
 });
diff --git a/test/cross-origin-opener-policy.test.ts b/test/cross-origin-opener-policy.test.ts
@@ -15,7 +15,7 @@ describe("Cross-Origin-Opener-Policy middleware", () => {
     );
   });
 
-  ["same-origin", "same-origin-allow-popups", "unsafe-none"].forEach(
+  (["same-origin", "same-origin-allow-popups", "unsafe-none"] as const).forEach(
     (policy) => {
       it(`sets "Cross-Origin-Opener-Policy: ${policy}" when told to`, async () => {
         await check(crossOriginOpenerPolicy({ policy }), {
@@ -28,14 +28,14 @@ describe("Cross-Origin-Opener-Policy middleware", () => {
   it("throws when setting the policy to an invalid value", () => {
     const invalidValues = [
       "",
-      "NONE",
-      "by-ftp-filename",
-      123 as any,
-      null as any,
-      new String("none") as any,
+      "foo",
+      "SAME-ORIGIN",
+      123,
+      null,
+      new String("same-origin"),
     ];
     for (const policy of invalidValues) {
-      expect(() => crossOriginOpenerPolicy({ policy })).toThrow(
+      expect(() => crossOriginOpenerPolicy({ policy: policy as any })).toThrow(
         /^Cross-Origin-Opener-Policy does not support /
       );
     }

diff --git a/test/cross-origin-resource-policy.test.ts b/test/cross-origin-resource-policy.test.ts
@@ -18,7 +18,7 @@ describe("Cross-Origin-Resource-Policy middleware", () => {
     );
   });
 
-  ["same-origin", "same-site", "cross-origin"].forEach((policy) => {
+  (["same-origin", "same-site", "cross-origin"] as const).forEach((policy) => {
     it(`sets "Cross-Origin-Resource-Policy: ${policy}" when told to`, async () => {
       await check(crossOriginResourcePolicy({ policy }), {
         "cross-origin-resource-policy": policy,
@@ -29,16 +29,16 @@ describe("Cross-Origin-Resource-Policy middleware", () => {
   it("throws when setting the policy to an invalid value", () => {
     const invalidValues = [
       "",
-      "NONE",
-      "by-ftp-filename",
-      123 as any,
-      null as any,
-      new String("none") as any,
+      "foo",
+      "CROSS-ORIGIN",
+      123,
+      null,
+      new String("none"),
     ];
     for (const policy of invalidValues) {
-      expect(() => crossOriginResourcePolicy({ policy })).toThrow(
-        /^Cross-Origin-Resource-Policy does not support /
-      );
+      expect(() =>
+        crossOriginResourcePolicy({ policy: policy as any })
+      ).toThrow(/^Cross-Origin-Resource-Policy does not support /);
     }
   });
 });
diff --git a/test/referrer-policy.test.ts b/test/referrer-policy.test.ts
@@ -17,17 +17,19 @@ describe("Referrer-Policy middleware", () => {
     });
   });
 
-  [
-    "no-referrer",
-    "no-referrer-when-downgrade",
-    "same-origin",
-    "origin",
-    "strict-origin",
-    "origin-when-cross-origin",
-    "strict-origin-when-cross-origin",
-    "unsafe-url",
-    "",
-  ].forEach((policy) => {
+  (
+    [
+      "no-referrer",
+      "no-referrer-when-downgrade",
+      "same-origin",
+      "origin",
+      "strict-origin",
+      "origin-when-cross-origin",
+      "strict-origin-when-cross-origin",
+      "unsafe-url",
+      "",
+    ] as const
+  ).forEach((policy) => {
     it(`can set the header to "${policy}" by specifying it as a string`, async () => {
       await check(referrerPolicy({ policy }), {
         "referrer-policy": policy,
@@ -48,12 +50,10 @@ describe("Referrer-Policy middleware", () => {
   });
 
   it("fails with a bad policy", () => {
-    expect(referrerPolicy.bind(null, { policy: "garbage" })).toThrow();
-    expect(referrerPolicy.bind(null, { policy: "sameorigin" })).toThrow();
-    expect(referrerPolicy.bind(null, { policy: 123 as any })).toThrow();
-    expect(referrerPolicy.bind(null, { policy: false as any })).toThrow();
-    expect(referrerPolicy.bind(null, { policy: null as any })).toThrow();
-    expect(referrerPolicy.bind(null, { policy: {} as any })).toThrow();
+    const invalidValues = ["foo", "sameorigin", "ORIGIN", 123, false, null, {}];
+    for (const policy of invalidValues) {
+      expect(referrerPolicy.bind(null, { policy: policy as any })).toThrow();
+    }
   });
 
   it("fails with an empty array", () => {

diff --git a/test/x-permitted-cross-domain-policies.test.ts b/test/x-permitted-cross-domain-policies.test.ts
@@ -18,7 +18,7 @@ describe("X-Permitted-Cross-Domain-Policies middleware", () => {
     );
   });
 
-  ["none", "master-only", "by-content-type", "all"].forEach(
+  (["none", "master-only", "by-content-type", "all"] as const).forEach(
     (permittedPolicies) => {
       it(`sets "X-Permitted-Cross-Domain-Policies: ${permittedPolicies}" when told to`, async () => {
         await check(xPermittedCrossDomainPolicies({ permittedPolicies }), {
@@ -33,13 +33,15 @@ describe("X-Permitted-Cross-Domain-Policies middleware", () => {
       "",
       "NONE",
       "by-ftp-filename",
-      123 as any,
-      null as any,
-      new String("none") as any,
+      123,
+      null,
+      new String("none"),
     ];
     for (const permittedPolicies of invalidValues) {
       expect(() =>
-        xPermittedCrossDomainPolicies({ permittedPolicies })
+        xPermittedCrossDomainPolicies({
+          permittedPolicies: permittedPolicies as any,
+        })
       ).toThrow(/^X-Permitted-Cross-Domain-Policies does not support /);
     }
   });