Merge pull request #529 from mchangrh/sanitizeVideoID

ajayyy · web-flow · commit dc0bde0e36c6 · 2022-12-27T17:10:27.000-05:00
Sanitize videoID and minimum UserID length
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,6 +15,8 @@ jobs:
       # Initialization
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
+        with:
+          node-version: 18
       - run: npm install
       - name: Run Tests
         timeout-minutes: 5
diff --git a/.github/workflows/eslint.yml b/.github/workflows/eslint.yml
@@ -15,6 +15,8 @@ jobs:
       # Initialization
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
+        with:
+          node-version: 18
       - run: npm install
       - name: Run Tests
         timeout-minutes: 5
diff --git a/.github/workflows/generate-sqlite-base.yml b/.github/workflows/generate-sqlite-base.yml
@@ -16,6 +16,8 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
+        with:
+          node-version: 18
       - run: npm install
       - name: Set config
         run: |
diff --git a/.github/workflows/postgres-redis-ci.yml b/.github/workflows/postgres-redis-ci.yml
@@ -21,6 +21,8 @@ jobs:
       - name: Check running containers
         run: docker ps
       - uses: actions/setup-node@v3
+        with:
+          node-version: 18
       - run: npm install
       - name: Run Tests
         env:
diff --git a/ci.json b/ci.json
@@ -76,5 +76,6 @@
         "clientSecret": "testClientSecret",
         "redirectUri": "http://127.0.0.1/fake/callback"
     },
-    "minReputationToSubmitFiller": -1
+    "minReputationToSubmitFiller": -1,
+    "minUserIDLength": 0
 }
diff --git a/config.json.example b/config.json.example
@@ -66,5 +66,6 @@
         {
             "name": "vipUsers"
         }]
-    }
+    },
+    "minUserIDLength": 30 // minimum length of UserID to be accepted
 }
diff --git a/src/config.ts b/src/config.ts
@@ -166,7 +166,8 @@ addDefaults(config, {
     },
     gumroad: {
         productPermalinks: ["sponsorblock"]
-    }
+    },
+    minUserIDLength: 30
 });
 loadFromEnv(config);
 migrate(config);
diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts
@@ -22,6 +22,7 @@ import axios from "axios";
 import { vote } from "./voteOnSponsorTime";
 import { canSubmit } from "../utils/permissions";
 import { getVideoDetails, videoDetails } from "../utils/getVideoDetails";
+import * as youtubeID from "../utils/youtubeID";
 
 type CheckResult = {
     pass: boolean,
@@ -185,15 +186,23 @@ async function checkUserActiveWarning(userID: string): Promise<CheckResult> {
 }
 
 async function checkInvalidFields(videoID: VideoID, userID: UserID, hashedUserID: HashedUserID
-    , segments: IncomingSegment[], videoDurationParam: number, userAgent: string): Promise<CheckResult> {
+    , segments: IncomingSegment[], videoDurationParam: number, userAgent: string, service: Service): Promise<CheckResult> {
     const invalidFields = [];
     const errors = [];
     if (typeof videoID !== "string" || videoID?.length == 0) {
         invalidFields.push("videoID");
     }
-    if (typeof userID !== "string" || userID?.length < 30) {
+    if (service === Service.YouTube && config.mode !== "test") {
+        const sanitizedVideoID = youtubeID.validate(videoID) ? videoID : youtubeID.sanitize(videoID);
+        if (!youtubeID.validate(sanitizedVideoID)) {
+            invalidFields.push("videoID");
+            errors.push("YouTube videoID could not be extracted");
+        }
+    }
+    const minLength = config.minUserIDLength;
+    if (typeof userID !== "string" || userID?.length < minLength) {
         invalidFields.push("userID");
-        if (userID?.length < 30) errors.push(`userID must be at least 30 characters long`);
+        if (userID?.length < minLength) errors.push(`userID must be at least ${minLength} characters long`);
     }
     if (!Array.isArray(segments) || segments.length == 0) {
         invalidFields.push("segments");
@@ -484,7 +493,7 @@ export async function postSkipSegments(req: Request, res: Response): Promise<Res
     //hash the userID
     const userID = await getHashCache(paramUserID || "");
 
-    const invalidCheckResult = await checkInvalidFields(videoID, paramUserID, userID, segments, videoDurationParam, userAgent);
+    const invalidCheckResult = await checkInvalidFields(videoID, paramUserID, userID, segments, videoDurationParam, userAgent, service);
     if (!invalidCheckResult.pass) {
         return res.status(invalidCheckResult.errorCode).send(invalidCheckResult.errorMessage);
     }
diff --git a/src/routes/voteOnSponsorTime.ts b/src/routes/voteOnSponsorTime.ts
@@ -325,7 +325,7 @@ export async function vote(ip: IPAddress, UUID: SegmentUUID, paramUserID: UserID
         return { status: 400 };
     }
     // Ignore this vote, invalid
-    if (paramUserID.length < 30 && config.mode !== "test") {
+    if (paramUserID.length < config.minUserIDLength) {
         return { status: 200 };
     }
 
diff --git a/src/types/config.model.ts b/src/types/config.model.ts
@@ -94,7 +94,8 @@ export interface SBSConfig {
     }
     gumroad: {
         productPermalinks: string[],
-    }
+    },
+    minUserIDLength: number
 }
 
 export interface WebhookConfig {
diff --git a/src/utils/youtubeID.ts b/src/utils/youtubeID.ts
@@ -0,0 +1,26 @@
+import { VideoID } from "../types/segments.model";
+
+const idRegex = new RegExp(/([0-9A-Za-z_-]{11})/); // group to always be index 1
+const exclusiveIdegex = new RegExp(`^${idRegex.source}$`);
+// match /c/, /channel/, /@channel, full UUIDs
+const negativeRegex = new RegExp(/(\/(channel|c)\/.+)|(\/@.+)|([a-f0-9]{64,65})|(youtube\.com\/clip\/)/);
+const urlRegex = new RegExp(`(?:v=|/|youtu.be/)${idRegex.source}(?:|/|[?&]t=\\d+s?)>?(?:\\s|$)`);
+const negateIdRegex = new RegExp(/(?:[^0-9A-Za-z_-]*?)/);
+const looseEndsRegex = new RegExp(`${negateIdRegex.source}${idRegex.source}${negateIdRegex.source}`);
+
+export const validate = (id: string): boolean => exclusiveIdegex.test(id);
+
+export const sanitize = (id: string): VideoID | null => {
+    // first decode URI
+    id = decodeURIComponent(id);
+    // strict matching
+    const strictMatch = id.match(exclusiveIdegex)?.[1];
+    const urlMatch = id.match(urlRegex)?.[1];
+    // return match, if not negative, return looseMatch
+    const looseMatch = id.match(looseEndsRegex)?.[1];
+    return strictMatch ? (strictMatch as VideoID)
+        : negativeRegex.test(id) ? null
+            : urlMatch ? (urlMatch as VideoID)
+                : looseMatch ? (looseMatch as VideoID)
+                    : null;
+};
diff --git a/test.json b/test.json
@@ -64,5 +64,6 @@
         "clientSecret": "testClientSecret",
         "redirectUri": "http://127.0.0.1/fake/callback"
     },
-    "minReputationToSubmitFiller": -1
+    "minReputationToSubmitFiller": -1,
+    "minUserIDLength": 0
 }
diff --git a/test/cases/environment.ts b/test/cases/environment.ts
@@ -0,0 +1,12 @@
+import assert from "assert";
+import { config } from "../../src/config";
+
+describe("environment", () => {
+    it("minUserIDLength should be < 10", () => {
+        assert(config.minUserIDLength < 10);
+    });
+    it("nodeJS major version should be >= 16", () => {
+        const [major] = process.versions.node.split(".").map(i => parseInt(i));
+        assert(major >= 16);
+    });
+});
diff --git a/test/cases/validateVideoIDs.ts b/test/cases/validateVideoIDs.ts
@@ -0,0 +1,125 @@
+import assert from "assert";
+import { client } from "../utils/httpClient";
+import { config } from "../../src/config";
+import sinon from "sinon";
+import { sanitize } from "../../src/utils/youtubeID";
+
+// videoID array
+const badVideoIDs = [
+    ["null", "< 11"],
+    ["dQw4w9WgXc?", "invalid characters"],
+    ["https://www.youtube.com/clip/UgkxeLPGsmKnMdm46DGml_0aa0aaAAAAA00a", "clip URL"],
+    ["https://youtube.com/channel/UCaAa00aaaAA0a0a0AaaAAAA", "channel ID (UC)"],
+    ["https://www.youtube.com/@LinusTechTips", "channel @username"],
+    ["https://www.youtube.com/@GamersNexus", "channel @username"],
+    ["https://www.youtube.com/c/LinusTechTips", "custom channel /c/"],
+    ["https://www.youtube.com/c/GamersNexus", "custom channel /c/"],
+    ["https://www.youtube.com/", "home/ page URL"],
+    ["03224876b002487796379942f199bc22ffac46157ad2488119bccc7b03c55430","UUID"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ&t=16s#requiredSegment=03224876b002487796379942f199bc22ffac46157ad2488119bccc7b03c55430", "full #requiredSegments uuid"],
+    ["","empty videoID"]
+
+];
+const goodVideoIDs = [
+    ["dQw4w9WgXcQ", "standalone videoID"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ", "?watch link"],
+    ["http://www.youtube.com/watch?v=dQw4w9WgXcQ", "http link"],
+    ["www.youtube.com/watch?v=dQw4w9WgXcQ", "no protocol link"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ?t=2", "trailing &t parameter"],
+    ["https://youtu.be/dQw4w9WgXcQ","youtu.be"],
+    ["youtu.be/dQw4w9WgXcQ","no protocol youtu.be"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ&t=16s#requiredSegment=00000000000","#requiredsegment link"],
+    ["https://www.youtube.com/embed/dQw4w9WgXcQ?wmode=transparent&rel=0&autohide=1&showinfo=1&fs=1&enablejsapi=0&theme=light", "long embedded link"],
+    ["http://m.youtube.com/watch?v=dQw4w9WgXcQ&app=m&persist_app=1", "force persist desktop"],
+    ["http://m.youtube.com/watch?v=dQw4w9WgXcQ", "mobile"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=PL8mG-AaA0aAa0AAa0A0A-aAaaA00aaAa0","/watch&list"],
+    ["https://www.youtube.com/embed/dQw4w9WgXcQ?list=PL8mG-AaA0aAa0AAa0A0A-aAaaA00aaAa0","/embed/video"],
+    ["dQw4w9WgXcQ\n", "escaped newline"],
+    ["dQw4w9WgXcQ\t", "escaped tab"],
+    ["%20dQw4w9WgXcQ%20%20%20", "urlencoded"],
+    ["https://sb.ltn.fi/video/dQw4w9WgXcQ/","sbltnfi link"],
+    ["https://www.youtube.com/watch?v=dQw4w9WgXcQ#t=0m10s", "anchor as t parameter"],
+];
+const edgeVideoIDs = [
+    ["https://www.youtube.com/embed/videoseries?list=PL8mG-Aaa0aAa1AAa0A0A-a0aaA00aaAa0", "/videoseries"],
+    ["https://www.youtube.com/embed/playlist?list=PL8mG-Aaa0aAa1AAa0A0A-a0aaA00aaAa0", "/playlist"],
+    ["PL8mG-Aaa0aAa1AAa0A0A-a0aaA00aaAa0", "playlist ID"],
+    ["UgkxeLPGsmKnMdm46DGml_0aa0aaAAAAA00a","clip ID"],
+    ["https://www.youtube.com/GamersNexus", "channel custom URL"],
+    ["https://www.youtube.com/LinusTechTips", "channel custom URL"],
+];
+const targetVideoID = "dQw4w9WgXcQ";
+
+// tests
+describe("YouTube VideoID validation - failing tests", () => {
+    for (const testCase of badVideoIDs) {
+        it(`Should error on invalid videoID - ${testCase[1]}`, () => {
+            assert.equal(sanitize(testCase[0]), null);
+        });
+    }
+});
+describe("YouTube VideoID validation - passing tests", () => {
+    for (const testCase of goodVideoIDs) {
+        it(`Should be able to sanitize good videoID - ${testCase[1]}`, () => {
+            assert.equal(sanitize(testCase[0]), targetVideoID);
+        });
+    }
+});
+describe("YouTube VideoID validation - edge cases tests", () => {
+    for (const testCase of edgeVideoIDs) {
+        it(`edge cases produce bad results - ${testCase[1]}`, () => {
+            assert.ok(sanitize(testCase[0]));
+        });
+    }
+});
+
+// stubs
+const mode = "production";
+let stub: sinon.SinonStub;
+
+// constants
+const endpoint = "/api/skipSegments";
+const userID = "postVideoID_user1";
+const expectedError = `No valid videoID. YouTube videoID could not be extracted`;
+
+
+// helper functions
+const postSkipSegments = (videoID: string) => client({
+    method: "POST",
+    url: endpoint,
+    params: {
+        videoID,
+        startTime: Math.random(),
+        endTime: 10,
+        userID,
+        service: "YouTube",
+        category: "sponsor"
+    }
+});
+
+describe("VideoID Validation - postSkipSegments", () => {
+    before(() => stub = sinon.stub(config, "mode").value(mode));
+    after(() => stub.restore());
+
+    it("Should return production mode if stub worked", (done) => {
+        assert.strictEqual(config.mode, mode);
+        done();
+    });
+
+    it(`Should return 400 for invalid videoID`, (done) => {
+        postSkipSegments("123456").then(res => {
+            assert.strictEqual(res.status, 400);
+            assert.strictEqual(res.data, expectedError);
+            done();
+        })
+            .catch(err => done(err));
+    });
+
+    it(`Should return 200 for valid videoID`, (done) => {
+        postSkipSegments("dQw4w9WgXcQ").then(res => {
+            assert.strictEqual(res.status, 200);
+            done();
+        })
+            .catch(err => done(err));
+    });
+});

Original file line number	Diff line number	Diff line change
`@@ -76,5 +76,6 @@`
`76`	`76`	`"clientSecret": "testClientSecret",`
`77`	`77`	`"redirectUri": "http://127.0.0.1/fake/callback"`
`78`	`78`	`},`
`79`		`- "minReputationToSubmitFiller": -1`
	`79`	`+ "minReputationToSubmitFiller": -1,`
	`80`	`+ "minUserIDLength": 0`
`80`	`81`	`}`
Original file line number	Diff line number	Diff line change
`@@ -66,5 +66,6 @@`
`66`	`66`	`{`
`67`	`67`	`"name": "vipUsers"`
`68`	`68`	`}]`
`69`		`- }`
	`69`	`+ },`
	`70`	`+ "minUserIDLength": 30 // minimum length of UserID to be accepted`
`70`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -325,7 +325,7 @@ export async function vote(ip: IPAddress, UUID: SegmentUUID, paramUserID: UserID`
`325`	`325`	`return { status: 400 };`
`326`	`326`	`}`
`327`	`327`	`// Ignore this vote, invalid`
`328`		`- if (paramUserID.length < 30 && config.mode !== "test") {`
	`328`	`+ if (paramUserID.length < config.minUserIDLength) {`
`329`	`329`	`return { status: 200 };`
`330`	`330`	`}`
`331`	`331`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,8 @@ export interface SBSConfig {`
`94`	`94`	`}`
`95`	`95`	`gumroad: {`
`96`	`96`	`productPermalinks: string[],`
`97`		`- }`
	`97`	`+ },`
	`98`	`+ minUserIDLength: number`
`98`	`99`	`}`
`99`	`100`
`100`	`101`	`export interface WebhookConfig {`