Merge pull request #69 from mollersuite/proper-validation

Proper validation via Zod

package-lock.json

@@ -26,7 +26,8 @@
         "multer": "^1.4.5-lts.1",
         "node-fetch": "^3.3.2",
         "nodemailer": "^6.9.3",
-        "typescript": "^5.2.2"
+        "typescript": "^5.2.2",
+        "zod": "^3.23.5"
       },
       "devDependencies": {
         "@sveltejs/vite-plugin-svelte": "^2.4.6",
@@ -1941,6 +1942,14 @@
       "engines": {
         "node": ">=0.4"
       }
+    },
+    "node_modules/zod": {
+      "version": "3.23.5",
+      "resolved": "https://registry.npmjs.org/zod/-/zod-3.23.5.tgz",
+      "integrity": "sha512-fkwiq0VIQTksNNA131rDOsVJcns0pfVUjHzLrNBiF/O/Xxb5lQyEXkhZWcJ7npWsYlvs+h0jFWXXy4X46Em1JA==",
+      "funding": {
+        "url": "https://github.com/sponsors/colinhacks"
+      }
     }
   }
 }

package.json

@@ -35,7 +35,8 @@
     "multer": "^1.4.5-lts.1",
     "node-fetch": "^3.3.2",
     "nodemailer": "^6.9.3",
-    "typescript": "^5.2.2"
+    "typescript": "^5.2.2",
+    "zod": "^3.23.5"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "^2.4.6",

src/server/index.ts

@@ -9,7 +9,7 @@ import { fileURLToPath } from "url"
 import { dirname } from "path"
 import config from "./lib/config.js"
 
-const app = new Hono()
+const app = new Hono({strict: false})
 
 app.get(
     "/static/assets/*",
@@ -78,7 +78,7 @@ apiRouter.loadAPIMethods().then(() => {
         app.fetch(
             new Request(
                 new URL(
-                    "/api/v1/info",
+                    "/api/v1",
                     ctx.req.raw.url
                 ).href,
                 ctx.req.raw

src/server/lib/files.ts

@@ -8,22 +8,24 @@ import config, { Configuration } from "./config.js"
 import "dotenv/config"
 
 import * as Accounts from "./accounts.js"
+import { z } from "zod"
+import * as schemas from "./schemas/files.js"
+import { issuesToMessage } from "./middleware.js"
 
-export let id_check_regex = /[A-Za-z0-9_\-\.\!\=\:\&\$\,\+\;\@\~\*\(\)\']+/
 export let alphanum = Array.from(
     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"
 )
 
 // bad solution but whatever
 
-export type FileVisibility = "public" | "anonymous" | "private"
+export type FileVisibility = z.infer<typeof schemas.FileVisibility>
 
 /**
  * @description Generates an alphanumeric string, used for files
  * @param length Length of the ID
  * @returns a random alphanumeric string
  */
-export function generateFileId(length: number = 5) {
+export function generateFileId(length: number = 5): z.infer<typeof schemas.FileId> {
     let fid = ""
     for (let i = 0; i < length; i++) {
         fid += alphanum[crypto.randomInt(0, alphanum.length)]
@@ -31,35 +33,7 @@ export function generateFileId(length: number = 5) {
     return fid
 }
 
-/**
+export type FilePointer = z.infer<typeof schemas.FilePointer>
- * @description Assert multiple conditions... this exists out of pure laziness
- * @param conditions
- */
-
-function multiAssert(
-    conditions: Map<boolean, { message: string; status: number }>
-) {
-    for (let [cond, err] of conditions.entries()) {
-        if (cond) return err
-    }
-}
-
-export type FileUploadSettings = Partial<Pick<FilePointer, "mime" | "owner">> &
-    Pick<FilePointer, "mime" | "filename"> & { uploadId?: string }
-
-export interface FilePointer {
-    filename: string
-    mime: string
-    messageids: string[]
-    owner?: string
-    sizeInBytes?: number
-    tag?: string
-    visibility?: FileVisibility
-    reserved?: boolean
-    chunkSize?: number
-    lastModified?: number
-    md5?: string
-}
 
 export interface StatusCodeError {
     status: number
@@ -471,8 +445,8 @@ export class UploadStream extends Writable {
             visibility: ogf
                 ? ogf.visibility
                 : this.owner 
-                  ? Accounts.getFromId(this.owner)?.defaultFileVisibility
+                    && Accounts.getFromId(this.owner)?.defaultFileVisibility 
-                  : undefined,
+                    || "public",
             // so that json.stringify doesnt include tag:undefined
             ...((ogf || {}).tag ? { tag: ogf.tag } : {}),
 
@@ -527,12 +501,11 @@ export class UploadStream extends Writable {
             return this.destroy(
                 new WebError(400, "duplicate attempt to set upload ID")
             )
-        if (
+
-            !id ||
+        let check = schemas.FileId.safeParse(id);
-            id.match(id_check_regex)?.[0] != id ||
+
-            id.length > this.files.config.maxUploadIdLength
+        if (!check.success)
-        )
+            return this.destroy(new WebError(400, issuesToMessage(check.error.issues)))
-            return this.destroy(new WebError(400, "invalid file ID"))
 
         if (this.files.files[id] && this.files.files[id].owner != this.owner)
             return this.destroy(new WebError(403, "you don't own this file"))

src/server/lib/middleware.ts

@@ -3,6 +3,7 @@ import type { Context, Handler as RequestHandler } from "hono"
 import ServeError from "../lib/errors.js"
 import * as auth from "./auth.js"
 import { setCookie } from "hono/cookie"
+import { z } from "zod"
 
 /**
  * @description Middleware which adds an account, if any, to ctx.get("account")
@@ -37,7 +38,6 @@ export const requiresAdmin: RequestHandler = function (ctx, next) {
  * @param tokenPermissions Permissions which your route requires.
  * @returns Express middleware
  */
-
 export const requiresPermissions = function (
     ...tokenPermissions: auth.TokenPermission[]
 ): RequestHandler {
@@ -93,6 +93,18 @@ export const assertAPI = function (
     }
 }
 
+export const issuesToMessage = function(issues: z.ZodIssue[]) {
+    return issues.map(e => `${e.path}: ${e.code} :: ${e.message}`).join("; ")
+}
+
+export const scheme = function(scheme: z.ZodTypeAny): RequestHandler {
+    return async function(ctx, next) {
+        let chk = scheme.safeParse(await ctx.req.json())
+        if (chk.success) return next()
+        else return ServeError(ctx, 400, issuesToMessage(chk.error.issues))
+    }
+}
+
 // Not really middleware but a utility
 
 export const login = (ctx: Context, account: string) => setCookie(ctx, "auth", auth.create(account, 3 * 24 * 60 * 60 * 1000), {
@@ -101,21 +113,3 @@ export const login = (ctx: Context, account: string) => setCookie(ctx, "auth", a
     secure: true,
     httpOnly: true
 })
-
-type SchemeType = "array" | "object" | "string" | "number" | "boolean"
-
-interface SchemeObject {
-    type: "object"
-    children: {
-        [key: string]: SchemeParameter
-    }
-}
-
-interface SchemeArray {
-    type: "array"
-    children:
-        | SchemeParameter /* All children of the array must be this type */
-        | SchemeParameter[] /* Array must match this pattern */
-}
-
-type SchemeParameter = SchemeType | SchemeObject | SchemeArray

src/server/lib/schemas/accounts.ts

@@ -0,0 +1,21 @@
+import {z} from "zod"
+import { FileVisibility } from "./files.js"
+
+export const StringPassword = z.string().min(8,"password must be at least 8 characters")
+export const Password =
+    z.object({
+        hash: z.string(),
+        salt: z.string()
+    })
+export const Username =
+    z.string().min(3, "username too short").max(20, "username too long").regex(/[A-Za-z0-9_\-\.]+/, "username contains invalid characters")
+export const Account =
+    z.object({
+        id: z.string(),
+        username: Username,
+        email: z.optional(z.string().email("must be an email")),
+        password: Password,
+        files: z.array(z.string()),
+        admin: z.boolean(),
+        defaultFileVisibility: FileVisibility
+    })

src/server/lib/schemas/files.ts

@@ -0,0 +1,21 @@
+import {z} from "zod"
+import config from "../config.js"
+
+export const FileId = z.string()
+    .regex(/[A-Za-z0-9_\-\.\!\=\:\&\$\,\+\;\@\~\*\(\)\']+/,"file ID uses invalid characters")
+    .max(config.maxUploadIdLength,"file ID too long")
+    .min(1, "you... *need* a file ID")
+export const FileVisibility = z.enum(["public", "anonymous", "private"])
+export const FileTag = z.string().toLowerCase().max(30, "tag length too long")
+export const FilePointer = z.object({
+    filename: z.string().max(256, "filename too long"),
+    mime: z.string().max(256, "mimetype too long"),
+    messageids: z.array(z.string()),
+    owner: z.optional(z.string()),
+    sizeInBytes: z.optional(z.number()),
+    tag: z.optional(FileTag),
+    visibility: z.optional(FileVisibility).default("public"),
+    chunkSize: z.optional(z.number()),
+    lastModified: z.optional(z.number()),
+    md5: z.optional(z.string())
+})

src/server/lib/schemas/index.ts

@@ -0,0 +1,2 @@
+export * as AccountSchemas from "./accounts.js"
+export * as FileSchemas from "./files.js"

src/server/routes/api/v0/authRoutes.ts

@@ -14,8 +14,7 @@ import config from "../../../lib/config.js"
 import ServeError from "../../../lib/errors.js"
 import Files, {
     FileVisibility,
-    generateFileId,
+    generateFileId
-    id_check_regex,
 } from "../../../lib/files.js"
 
 import { writeFile } from "fs/promises"

src/server/routes/api/v0/fileApiRoutes.ts

@@ -68,10 +68,6 @@ export default function (files: Files) {
 
                 let fp = files.files[e]
 
-                if (fp.reserved) {
-                    return
-                }
-
                 switch (body.action) {
                     case "delete":
                         files.unlink(e, true)

src/server/routes/api/v1/account.ts

@@ -5,21 +5,25 @@ import { getCookie, setCookie } from "hono/cookie"
 
 // Libs
 
-import Files, { id_check_regex } from "../../../lib/files.js"
+import Files from "../../../lib/files.js"
 import * as Accounts from "../../../lib/accounts.js"
 import * as auth from "../../../lib/auth.js"
 import {
     assertAPI,
     getAccount,
+    issuesToMessage,
     login,
     noAPIAccess,
     requiresAccount,
     requiresPermissions,
+    scheme,
 } from "../../../lib/middleware.js"
 import ServeError from "../../../lib/errors.js"
 import { CodeMgr, sendMail } from "../../../lib/mail.js"
 
 import Configuration from "../../../lib/config.js"
+import { AccountSchemas, FileSchemas } from "../../../lib/schemas/index.js"
+import { z } from "zod"
 
 const router = new Hono<{
     Variables: {
@@ -40,8 +44,7 @@ type Message = [200 | 400 | 401 | 403 | 429 | 501, string]
 // @Jack5079 make typings better if possible
 
 type Validator<
-    T extends keyof Partial<Accounts.Account>,
+    T extends keyof Partial<Accounts.Account>
-    ValueNotNull extends boolean,
 > =
     /**
      * @param actor The account performing this action
@@ -52,44 +55,33 @@ type Validator<
         actor: Accounts.Account,
         target: Accounts.Account,
         params: UserUpdateParameters &
-            (ValueNotNull extends true
+            {
-                ? {
                 [K in keyof Pick<
                     UserUpdateParameters,
                     T
                 >]-?: UserUpdateParameters[K]
-                  }
+            },
-                : {}),
         ctx: Context
     ) => Accounts.Account[T] | Message
 
-// this type is so stupid stg
+type SchemedValidator<
-type ValidatorWithSettings<T extends keyof Partial<Accounts.Account>> =
+    T extends keyof Partial<Accounts.Account>
-    | {
+> = {
-          acceptsNull: true
+    validator: Validator<T>,
-          validator: Validator<T, false>
+    schema: z.ZodTypeAny
-      }
+}
-    | {
-          acceptsNull?: false
-          validator: Validator<T, true>
-      }
 
 const validators: {
-    [T in keyof Partial<Accounts.Account>]:
+    [T in keyof Partial<Accounts.Account>]: SchemedValidator<T>
-        | Validator<T, true>
-        | ValidatorWithSettings<T>
 } = {
-    defaultFileVisibility(actor, target, params) {
+    defaultFileVisibility: {
-        if (
+        schema: FileSchemas.FileVisibility,
-            ["public", "private", "anonymous"].includes(
+        validator: (actor, target, params) => {
-                params.defaultFileVisibility
-            )
-        )
             return params.defaultFileVisibility
-        else return [400, "invalid file visibility"]
+        }
     },
     email: {
-        acceptsNull: true,
+        schema: AccountSchemas.Account.shape.email.optional(),
         validator: (actor, target, params, ctx) => {
             if (
                 !params.currentPassword || // actor on purpose here to allow admins
@@ -109,8 +101,8 @@ const validators: {
                 return undefined
             }
 
-            if (typeof params.email !== "string")
+            if (!z.string().email().safeParse(typeof params.email).success)
-                return [400, "email must be string"]
+                return [400, "bad email"]
             if (actor.admin) return params.email
 
             // send verification email
@@ -142,7 +134,9 @@ const validators: {
             return [200, "please check your inbox"]
         },
     },
-    password(actor, target, params) {
+    password: {
+        schema: AccountSchemas.StringPassword,
+        validator: (actor, target, params) => {
             if (
                 !params.currentPassword || // actor on purpose here to allow admins
                 (params.currentPassword &&
@@ -150,9 +144,6 @@ const validators: {
             )
                 return [401, "current password incorrect"]
 
-        if (typeof params.password != "string" || params.password.length < 8)
-            return [400, "password must be 8 characters or longer"]
-
             if (target.email) {
                 sendMail(
                     target.email,
@@ -164,8 +155,11 @@ const validators: {
             }
 
             return Accounts.password.hash(params.password)
+        }
     },
-    username(actor, target, params) {
+    username: {
+        schema: AccountSchemas.Username,
+        validator: (actor, target, params) => {
             if (
                 !params.currentPassword || // actor on purpose here to allow admins
                 (params.currentPassword &&
@@ -173,25 +167,9 @@ const validators: {
             )
                 return [401, "current password incorrect"]
 
-        if (
-            typeof params.username != "string" ||
-            params.username.length < 3 ||
-            params.username.length > 20
-        )
-            return [
-                400,
-                "username must be between 3 and 20 characters in length",
-            ]
-
             if (Accounts.getFromUsername(params.username))
                 return [400, "account with this username already exists"]
 
-        if (
-            (params.username.match(/[A-Za-z0-9_\-\.]+/) || [])[0] !=
-            params.username
-        )
-            return [400, "username has invalid characters"]
-
             if (target.email) {
                 sendMail(
                     target.email,
@@ -203,45 +181,15 @@ const validators: {
             }
 
             return params.username
+        }
     },
-    customCSS: {
+    admin: {
-        acceptsNull: true,
+        schema: z.boolean(),
         validator: (actor, target, params) => {
-            if (
-                !params.customCSS ||
-                (params.customCSS.match(id_check_regex)?.[0] ==
-                    params.customCSS &&
-                    params.customCSS.length <= Configuration.maxUploadIdLength)
-            )
-                return params.customCSS
-            else return [400, "bad file id"]
-        },
-    },
-    embed(actor, target, params) {
-        if (typeof params.embed !== "object")
-            return [400, "must use an object for embed"]
-        if (params.embed.color === undefined) {
-            params.embed.color = target.embed?.color
-        } else if (
-            !(
-                (params.embed.color.toLowerCase().match(/[a-f0-9]+/)?.[0] ==
-                    params.embed.color.toLowerCase() &&
-                    params.embed.color.length == 6) ||
-                params.embed.color == null
-            )
-        )
-            return [400, "bad embed color"]
-
-        if (params.embed.largeImage === undefined) {
-            params.embed.largeImage = target.embed?.largeImage
-        } else params.embed.largeImage = Boolean(params.embed.largeImage)
-
-        return params.embed
-    },
-    admin(actor, target, params) {
             if (actor.admin && !target.admin) return params.admin
             else if (!actor.admin) return [400, "cannot promote yourself"]
             else return [400, "cannot demote an admin"]
+        }
     },
 }
 
@@ -272,7 +220,10 @@ function isMessage(object: any): object is Message {
 }
 
 export default function (files: Files) {
-    router.post("/", async (ctx) => {
+    router.post("/", scheme(z.object({
+        username: AccountSchemas.Username,
+        password: AccountSchemas.StringPassword
+    })), async (ctx) => {
         const body = await ctx.req.json()
         if (!Configuration.accounts.registrationEnabled) {
             return ServeError(ctx, 403, "account registration disabled")
@@ -290,28 +241,6 @@ export default function (files: Files) {
             )
         }
 
-        if (body.username.length < 3 || body.username.length > 20) {
-            return ServeError(
-                ctx,
-                400,
-                "username must be over or equal to 3 characters or under or equal to 20 characters in length"
-            )
-        }
-
-        if (
-            (body.username.match(/[A-Za-z0-9_\-\.]+/) || [])[0] != body.username
-        ) {
-            return ServeError(ctx, 400, "username contains invalid characters")
-        }
-
-        if (body.password.length < 8) {
-            return ServeError(
-                ctx,
-                400,
-                "password must be 8 characters or longer"
-            )
-        }
-
         return Accounts.create(body.username, body.password)
             .then((account) => {
                 login(ctx, account)
@@ -352,23 +281,11 @@ export default function (files: Files) {
                         `the ${x} parameter cannot be set or is not a valid parameter`,
                     ] as Message
 
-                let validator = (
+                let validator = validators[x]!
-                    typeof validators[x] == "object"
-                        ? validators[x]
-                        : {
-                              validator: validators[x] as Validator<
-                                  typeof x,
-                                  false
-                              >,
-                              acceptsNull: false,
-                          }
-                ) as ValidatorWithSettings<typeof x>
 
-                if (!validator.acceptsNull && !v)
+                let check = validator.schema.safeParse(v)
-                    return [
+                if (!check.success)
-                        400,
+                    return [400, issuesToMessage(check.error.issues)]
-                        `the ${x} validator does not accept null values`,
-                    ] as Message
 
                 return [
                     x,
@@ -437,7 +354,7 @@ export default function (files: Files) {
         })
     })
 
-    router.get("/css", async (ctx) => {
+    router.get("/:user/css", async (ctx) => {
         let acc = ctx.get("account")
         if (acc?.customCSS) return ctx.redirect(`/file/${acc.customCSS}`)
         else return ctx.text("")

src/server/routes/api/v1/api.json

@@ -4,7 +4,10 @@
     "mount": [
         "account",
         "session",
-        "info",
+        {
+            "file": "index",
+            "to": "/"
+        },
         {
             "file": "file/index",
             "to": "/file"

src/server/routes/api/v1/session.ts

@@ -6,15 +6,18 @@ import { getCookie, setCookie } from "hono/cookie"
 
 // Libs
 
-import Files, { id_check_regex } from "../../../lib/files.js"
+import Files from "../../../lib/files.js"
 import * as Accounts from "../../../lib/accounts.js"
 import * as auth from "../../../lib/auth.js"
 import {
     getAccount,
     login,
-    requiresAccount
+    requiresAccount,
+    scheme
 } from "../../../lib/middleware.js"
 import ServeError from "../../../lib/errors.js"
+import { AccountSchemas } from "../../../lib/schemas/index.js"
+import { z } from "zod"
 
 const router = new Hono<{
     Variables: {
@@ -25,15 +28,11 @@ const router = new Hono<{
 router.use(getAccount)
 
 export default function (files: Files) {
-    router.post("/", async (ctx, res) => {
+    router.post("/",scheme(z.object({
+        username: AccountSchemas.Username,
+        password: AccountSchemas.StringPassword
+    })), async (ctx) => {
         const body = await ctx.req.json()
-        if (
-            typeof body.username != "string" ||
-            typeof body.password != "string"
-        ) {
-            ServeError(ctx, 400, "please provide a username or password")
-            return
-        }
 
         if (auth.validate(getCookie(ctx, "auth")!)) {
             ServeError(ctx, 400, "you are already logged in")

src/server/tools/cli.ts

@@ -1,19 +1,19 @@
 import fs from "fs"
 import { stat } from "fs/promises"
-import Files from "./lib/files.js"
+import Files from "../lib/files.js"
 import { program } from "commander"
 import { basename } from "path"
 import { Writable } from "node:stream"
-import config from "./lib/config.js"
+import config from "../lib/config.js"
-import pkg from "../../package.json" assert { type: "json" }
+import pkg from "../../../package.json" assert { type: "json" }
 import { fileURLToPath } from "url"
 import { dirname } from "path"
 
 // init data
 
 const __dirname = dirname(fileURLToPath(import.meta.url))
-if (!fs.existsSync(__dirname + "/../../.data/"))
+if (!fs.existsSync(__dirname + "/../../../.data/"))
-    fs.mkdirSync(__dirname + "/../../.data/")
+    fs.mkdirSync(__dirname + "/../../../.data/")
 
 // discord
 let files = new Files(config)