Improve validation (#48)

* add stricter username and slug handling

* add better validation patterns

* add color and icon validation

* refactor validation files for sidebar items

* refactor more validation

* move more validation to shared utility

* address comments

* fixes

* fix outdated mock

* address comments

* fix fallow CI

* fix

* fix font preload

* address changes, fixes

* fixes

Author: ntietje1

.github/workflows/ci.yml

@@ -11,6 +11,7 @@ concurrency:
 
 env:
   NODE_VERSION: 22
+  VITE_SITE_URL: 'http://localhost:3000'
 
 jobs:
   setup:
@@ -39,26 +40,18 @@ jobs:
       - run: vp run lint:convex # TODO: remove this once plugins exist for oxlint
 
   fallow:
+    if: github.event_name == 'pull_request'
     name: Fallow
     runs-on: ubuntu-latest
-    needs: [setup]
     steps:
       - uses: actions/checkout@v4
         with:
           fetch-depth: 0
-      - uses: voidzero-dev/setup-vp@v1
+      - uses: fallow-rs/fallow@v2
         with:
-          node-version: ${{ env.NODE_VERSION }}
-          cache: true
-      - run: vp install --frozen-lockfile
-      - name: Run Fallow
-        continue-on-error: true
-        run: |
-          if [ "${{ github.event_name }}" = "pull_request" ]; then
-            vp run fallow audit --base origin/main --format annotations --format json > fallow-report.json
-          else
-            vp run fallow --format json > fallow-report.json
-          fi
+          annotations: true
+          fail-on-issues: false
+          format: sarif
 
   react-compiler:
     name: React Compiler Health
@@ -90,7 +83,6 @@ jobs:
         if: github.event_name == 'pull_request'
         run: npx -y react-doctor@latest . --verbose --fail-on error --diff=${{ github.event.pull_request.base.ref }}
       - name: Run React Doctor (main)
-        id: doctor
         if: github.ref == 'refs/heads/main'
         run: npx -y react-doctor@latest . --verbose --fail-on error
 

.vscode/settings.json

@@ -1,11 +1,11 @@
 {
   "editor.defaultFormatter": "oxc.oxc-vscode",
   "oxc.fmt.configPath": "./vite.config.ts",
-  "editor.formatOnSave": true,
+  "editor.formatOnSave": false,
   "editor.formatOnSaveMode": "file",
   "editor.codeActionsOnSave": {
-    "source.fixAll.oxc": "explicit",
-    "source.fixAll.eslint": "explicit"
+    "source.fixAll.oxc": "never",
+    "source.fixAll.eslint": "never"
   },
 
   "eslint.enable": true,

convex/_generated/api.d.ts

@@ -66,10 +66,12 @@ import type * as canvases_functions_updateCanvas from "../canvases/functions/upd
 import type * as canvases_mutations from "../canvases/mutations.js";
 import type * as canvases_triggers from "../canvases/triggers.js";
 import type * as canvases_types from "../canvases/types.js";
+import type * as common_async from "../common/async.js";
 import type * as common_constants from "../common/constants.js";
 import type * as common_logger from "../common/logger.js";
 import type * as common_slug from "../common/slug.js";
 import type * as common_types from "../common/types.js";
+import type * as common_zod from "../common/zod.js";
 import type * as crons from "../crons.js";
 import type * as documentSnapshots_functions_createSnapshot from "../documentSnapshots/functions/createSnapshot.js";
 import type * as documentSnapshots_functions_getSnapshot from "../documentSnapshots/functions/getSnapshot.js";
@@ -155,7 +157,6 @@ import type * as sessions_functions_updateSession from "../sessions/functions/up
 import type * as sessions_mutations from "../sessions/mutations.js";
 import type * as sessions_queries from "../sessions/queries.js";
 import type * as sessions_types from "../sessions/types.js";
-import type * as sidebarItems_createParentTarget from "../sidebarItems/createParentTarget.js";
 import type * as sidebarItems_functions_claimPreviewGeneration from "../sidebarItems/functions/claimPreviewGeneration.js";
 import type * as sidebarItems_functions_collectDescendants from "../sidebarItems/functions/collectDescendants.js";
 import type * as sidebarItems_functions_defaultItemName from "../sidebarItems/functions/defaultItemName.js";
@@ -179,12 +180,18 @@ import type * as sidebarItems_schema_anySidebarItemValidator from "../sidebarIte
 import type * as sidebarItems_schema_anySidebarItemWithContentValidator from "../sidebarItems/schema/anySidebarItemWithContentValidator.js";
 import type * as sidebarItems_schema_sidebarItemsTable from "../sidebarItems/schema/sidebarItemsTable.js";
 import type * as sidebarItems_schema_validators from "../sidebarItems/schema/validators.js";
-import type * as sidebarItems_sharedValidation from "../sidebarItems/sharedValidation.js";
 import type * as sidebarItems_triggerTypes from "../sidebarItems/triggerTypes.js";
 import type * as sidebarItems_triggers from "../sidebarItems/triggers.js";
 import type * as sidebarItems_types_baseTypes from "../sidebarItems/types/baseTypes.js";
 import type * as sidebarItems_types_types from "../sidebarItems/types/types.js";
-import type * as sidebarItems_validation from "../sidebarItems/validation.js";
+import type * as sidebarItems_validation_access from "../sidebarItems/validation/access.js";
+import type * as sidebarItems_validation_color from "../sidebarItems/validation/color.js";
+import type * as sidebarItems_validation_icon from "../sidebarItems/validation/icon.js";
+import type * as sidebarItems_validation_move from "../sidebarItems/validation/move.js";
+import type * as sidebarItems_validation_name from "../sidebarItems/validation/name.js";
+import type * as sidebarItems_validation_orchestration from "../sidebarItems/validation/orchestration.js";
+import type * as sidebarItems_validation_parent from "../sidebarItems/validation/parent.js";
+import type * as sidebarItems_validation_slug from "../sidebarItems/validation/slug.js";
 import type * as sidebarShares_functions_getSidebarItemShares from "../sidebarShares/functions/getSidebarItemShares.js";
 import type * as sidebarShares_functions_getSidebarItemSharesForItem from "../sidebarShares/functions/getSidebarItemSharesForItem.js";
 import type * as sidebarShares_functions_getSidebarItemWithShares from "../sidebarShares/functions/getSidebarItemWithShares.js";
@@ -295,10 +302,12 @@ declare const fullApi: ApiFromModules<{
   "canvases/mutations": typeof canvases_mutations;
   "canvases/triggers": typeof canvases_triggers;
   "canvases/types": typeof canvases_types;
+  "common/async": typeof common_async;
   "common/constants": typeof common_constants;
   "common/logger": typeof common_logger;
   "common/slug": typeof common_slug;
   "common/types": typeof common_types;
+  "common/zod": typeof common_zod;
   crons: typeof crons;
   "documentSnapshots/functions/createSnapshot": typeof documentSnapshots_functions_createSnapshot;
   "documentSnapshots/functions/getSnapshot": typeof documentSnapshots_functions_getSnapshot;
@@ -384,7 +393,6 @@ declare const fullApi: ApiFromModules<{
   "sessions/mutations": typeof sessions_mutations;
   "sessions/queries": typeof sessions_queries;
   "sessions/types": typeof sessions_types;
-  "sidebarItems/createParentTarget": typeof sidebarItems_createParentTarget;
   "sidebarItems/functions/claimPreviewGeneration": typeof sidebarItems_functions_claimPreviewGeneration;
   "sidebarItems/functions/collectDescendants": typeof sidebarItems_functions_collectDescendants;
   "sidebarItems/functions/defaultItemName": typeof sidebarItems_functions_defaultItemName;
@@ -408,12 +416,18 @@ declare const fullApi: ApiFromModules<{
   "sidebarItems/schema/anySidebarItemWithContentValidator": typeof sidebarItems_schema_anySidebarItemWithContentValidator;
   "sidebarItems/schema/sidebarItemsTable": typeof sidebarItems_schema_sidebarItemsTable;
   "sidebarItems/schema/validators": typeof sidebarItems_schema_validators;
-  "sidebarItems/sharedValidation": typeof sidebarItems_sharedValidation;
   "sidebarItems/triggerTypes": typeof sidebarItems_triggerTypes;
   "sidebarItems/triggers": typeof sidebarItems_triggers;
   "sidebarItems/types/baseTypes": typeof sidebarItems_types_baseTypes;
   "sidebarItems/types/types": typeof sidebarItems_types_types;
-  "sidebarItems/validation": typeof sidebarItems_validation;
+  "sidebarItems/validation/access": typeof sidebarItems_validation_access;
+  "sidebarItems/validation/color": typeof sidebarItems_validation_color;
+  "sidebarItems/validation/icon": typeof sidebarItems_validation_icon;
+  "sidebarItems/validation/move": typeof sidebarItems_validation_move;
+  "sidebarItems/validation/name": typeof sidebarItems_validation_name;
+  "sidebarItems/validation/orchestration": typeof sidebarItems_validation_orchestration;
+  "sidebarItems/validation/parent": typeof sidebarItems_validation_parent;
+  "sidebarItems/validation/slug": typeof sidebarItems_validation_slug;
   "sidebarShares/functions/getSidebarItemShares": typeof sidebarShares_functions_getSidebarItemShares;
   "sidebarShares/functions/getSidebarItemSharesForItem": typeof sidebarShares_functions_getSidebarItemSharesForItem;
   "sidebarShares/functions/getSidebarItemWithShares": typeof sidebarShares_functions_getSidebarItemWithShares;

convex/_test/factories.helper.ts

@@ -1,7 +1,13 @@
 import { CAMPAIGN_MEMBER_ROLE, CAMPAIGN_MEMBER_STATUS } from '../campaigns/types'
+import type { SidebarItemColor } from '../sidebarItems/validation/color'
+import type { SidebarItemIconName } from '../sidebarItems/validation/icon'
 import { SIDEBAR_ITEM_LOCATION, SIDEBAR_ITEM_TYPES } from '../sidebarItems/types/baseTypes'
 import { SHARE_STATUS } from '../blockShares/types'
 import { slugify } from '../common/slug'
+import { assertCampaignSlug } from '../campaigns/validation'
+import { assertSidebarItemName } from '../sidebarItems/validation/name'
+import { assertSidebarItemSlug } from '../sidebarItems/validation/slug'
+import { assertUsername } from '../users/validation'
 import { makeYjsUpdateWithBlocks } from '../yjsSync/__tests__/makeYjsUpdate.helper'
 import type { TestBlock } from '../yjsSync/__tests__/makeYjsUpdate.helper'
 import type { TestConvex } from 'convex-test'
@@ -12,6 +18,7 @@ import type { PermissionLevel } from '../permissions/types'
 import type { ShareStatus } from '../blockShares/types'
 import type { BlockNoteId, BlockProps, BlockType, InlineContent } from '../blocks/types'
 import type { CustomBlock } from '../notes/editorSpecs'
+import type { SidebarItemName } from '../sidebarItems/validation/name'
 import { createHash } from 'crypto'
 
 type T = TestConvex<typeof schema>
@@ -65,16 +72,21 @@ export async function createUserProfile(
   }>,
 ) {
   const n = nextId()
+  const { username, ...rest } = overrides ?? {}
   const defaults = {
     authUserId: `auth-user-${n}`,
-    username: `user-${n}`,
+    username: assertUsername(`user-${n}`),
     email: `user-${n}@test.com`,
     emailVerified: null,
     name: `Test User ${n}`,
     profileImage: null,
     twoFactorEnabled: null,
   }
-  const data = { ...defaults, ...overrides }
+  const data = {
+    ...defaults,
+    ...rest,
+    ...(username !== undefined ? { username: assertUsername(username) } : {}),
+  }
   const id = await t.run(async (ctx) => {
     return await ctx.db.insert('userProfiles', data)
   })
@@ -93,15 +105,21 @@ export async function createCampaignWithDm(
   }>,
 ) {
   const n = nextId()
+  const { slug, ...rest } = overrides ?? {}
   const defaults = {
     name: `Campaign ${n}`,
     description: '',
     dmUserId: dmProfile._id,
-    slug: `campaign-${n}`,
+    slug: assertCampaignSlug(`campaign-${n}`),
     status: 'Active' as const,
     currentSessionId: null,
   }
-  const campaignData = { ...defaults, ...overrides, dmUserId: dmProfile._id }
+  const campaignData = {
+    ...defaults,
+    ...rest,
+    dmUserId: dmProfile._id,
+    ...(slug !== undefined ? { slug: assertCampaignSlug(slug) } : {}),
+  }
   const campaignId = await t.run(async (ctx) => {
     return await ctx.db.insert('campaigns', campaignData)
   })
@@ -142,9 +160,9 @@ export async function addPlayerToCampaign(
 const sidebarItemBase = (
   campaignId: Id<'campaigns'>,
   creatorProfileId: Id<'userProfiles'>,
-  name: string,
+  name: SidebarItemName,
 ): {
-  name: string
+  name: SidebarItemName
   slug: string
   campaignId: Id<'campaigns'>
   iconName: null
@@ -158,7 +176,7 @@ const sidebarItemBase = (
   previewUpdatedAt: null
 } & ReturnType<typeof commonFields> => ({
   name,
-  slug: slugify(name),
+  slug: assertSidebarItemSlug(slugify(name)),
   campaignId,
   iconName: null,
   color: null,
@@ -178,8 +196,8 @@ type CommonSidebarItemOverrides = Partial<{
   parentId: Id<'sidebarItems'> | null
   allPermissionLevel: PermissionLevel | null
   location: SidebarItemLocation
-  iconName: string | null
-  color: string | null
+  iconName: SidebarItemIconName | null
+  color: SidebarItemColor | null
   deletionTime: number | null
   deletedBy: Id<'userProfiles'> | null
   previewStorageId: Id<'_storage'> | null
@@ -201,15 +219,24 @@ async function insertSidebarItem(
   overrides?: CommonSidebarItemOverrides & Record<string, unknown>,
 ) {
   const n = nextId()
-  const name = overrides?.name ?? `${label} ${n}`
-
-  const { inheritShares, imageStorageId, storageId, ...sidebarOverrides } = overrides ?? {}
+  const name = assertSidebarItemName(overrides?.name ?? `${label} ${n}`)
+
+  const {
+    inheritShares,
+    imageStorageId,
+    storageId,
+    slug,
+    name: _name,
+    ...sidebarOverrides
+  } = overrides ?? {}
+  const validatedSlug =
+    slug !== undefined ? assertSidebarItemSlug(slug) : assertSidebarItemSlug(slugify(name))
 
   const sharedData = {
     ...sidebarItemBase(campaignId, creatorProfileId, name),
     type,
     ...sidebarOverrides,
-    slug: overrides?.slug ?? slugify(name),
+    slug: validatedSlug,
   }
 
   const extensionFields: Record<string, unknown> = {}

convex/auth/functions/onCreateUser.ts

@@ -1,4 +1,6 @@
 import { findUniqueSlug } from '../../common/slug'
+import { parseUsername } from '../../users/validation'
+import { USERNAME_MAX_LENGTH } from '../../users/constants'
 import type { MutationCtx } from '../../_generated/server'
 
 type AuthUserDoc = {
@@ -17,13 +19,20 @@ export async function onCreateUser(ctx: MutationCtx, user: AuthUserDoc): Promise
     user.name?.toLowerCase().replace(/\s+/g, '') ||
     `user${String(user._id).slice(-8)}`
 
-  const username = await findUniqueSlug(baseUsername, async (slug) => {
-    const conflict = await ctx.db
-      .query('userProfiles')
-      .withIndex('by_username', (q) => q.eq('username', slug))
-      .unique()
-    return conflict !== null
-  })
+  const username = await findUniqueSlug(
+    baseUsername,
+    async (slug) => {
+      const conflict = await ctx.db
+        .query('userProfiles')
+        .withIndex('by_username', (q) => q.eq('username', slug))
+        .unique()
+      return conflict !== null
+    },
+    {
+      maxLength: USERNAME_MAX_LENGTH,
+      isValidCandidate: (slug) => parseUsername(slug) !== null,
+    },
+  )
 
   await ctx.db.insert('userProfiles', {
     authUserId: String(user._id),

convex/blockShares/functions/setBlocksShareStatus.ts

@@ -1,5 +1,5 @@
 import { asyncMap } from 'convex-helpers'
-import { requireItemAccess } from '../../sidebarItems/validation'
+import { requireItemAccess } from '../../sidebarItems/validation/access'
 import { PERMISSION_LEVEL } from '../../permissions/types'
 import { logEditHistory } from '../../editHistory/log'
 import { EDIT_HISTORY_ACTION } from '../../editHistory/types'

convex/blockShares/functions/shareBlocks.ts

@@ -1,5 +1,5 @@
 import { asyncMap } from 'convex-helpers'
-import { requireItemAccess } from '../../sidebarItems/validation'
+import { requireItemAccess } from '../../sidebarItems/validation/access'
 import { PERMISSION_LEVEL } from '../../permissions/types'
 import { logEditHistory } from '../../editHistory/log'
 import { EDIT_HISTORY_ACTION } from '../../editHistory/types'

convex/blockShares/functions/unshareBlocks.ts

@@ -1,5 +1,5 @@
 import { asyncMap } from 'convex-helpers'
-import { requireItemAccess } from '../../sidebarItems/validation'
+import { requireItemAccess } from '../../sidebarItems/validation/access'
 import { PERMISSION_LEVEL } from '../../permissions/types'
 import { logEditHistory } from '../../editHistory/log'
 import { EDIT_HISTORY_ACTION } from '../../editHistory/types'

convex/blocks/functions/getBlockWithShares.ts

@@ -4,7 +4,7 @@ import { getCampaignMembers } from '../../campaigns/functions/getCampaignMembers
 import { getBlockSharesByBlock } from '../../blockShares/functions/getBlockSharesForBlock'
 import { PERMISSION_LEVEL } from '../../permissions/types'
 import { SHARE_STATUS } from '../../blockShares/types'
-import { checkItemAccess } from '../../sidebarItems/validation'
+import { checkItemAccess } from '../../sidebarItems/validation/access'
 import { findBlockByBlockNoteId } from './findBlockByBlockNoteId'
 import { getSidebarItem } from '../../sidebarItems/functions/getSidebarItem'
 import type { DmQueryCtx } from '../../functions'

convex/blocks/functions/getBlocksWithShares.ts

@@ -4,7 +4,7 @@ import { CAMPAIGN_MEMBER_ROLE } from '../../campaigns/types'
 import { getCampaignMembers } from '../../campaigns/functions/getCampaignMembers'
 import { PERMISSION_LEVEL } from '../../permissions/types'
 import { SHARE_STATUS } from '../../blockShares/types'
-import { checkItemAccess } from '../../sidebarItems/validation'
+import { checkItemAccess } from '../../sidebarItems/validation/access'
 import { findBlockByBlockNoteId } from './findBlockByBlockNoteId'
 import { getSidebarItem } from '../../sidebarItems/functions/getSidebarItem'
 import type { ShareStatus } from '../../blockShares/types'