Merge pull request #14026 from Budibase/revert-14020-revert-13993-fix/disallow-prohibited-columns

Bringing back backend column validation

packages/backend-core/src/db/constants.ts

@@ -1,14 +1,5 @@
-export const CONSTANT_INTERNAL_ROW_COLS = [
-  "_id",
-  "_rev",
-  "type",
-  "createdAt",
-  "updatedAt",
-  "tableId",
-] as const
-
-export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const
-
-export function isInternalColumnName(name: string): boolean {
-  return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name)
-}
+export {
+  CONSTANT_INTERNAL_ROW_COLS,
+  CONSTANT_EXTERNAL_ROW_COLS,
+  isInternalColumnName,
+} from "@budibase/shared-core"

packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte

@@ -17,6 +17,8 @@
     SWITCHABLE_TYPES,
     ValidColumnNameRegex,
     helpers,
+    CONSTANT_INTERNAL_ROW_COLS,
+    CONSTANT_EXTERNAL_ROW_COLS,
   } from "@budibase/shared-core"
   import { createEventDispatcher, getContext, onMount } from "svelte"
   import { cloneDeep } from "lodash/fp"
@@ -52,7 +54,6 @@
   const DATE_TYPE = FieldType.DATETIME
 
   const dispatch = createEventDispatcher()
-  const PROHIBITED_COLUMN_NAMES = ["type", "_id", "_rev", "tableId"]
   const { dispatch: gridDispatch, rows } = getContext("grid")
 
   export let field
@@ -487,20 +488,27 @@
       })
     }
     const newError = {}
+    const prohibited = externalTable
+      ? CONSTANT_EXTERNAL_ROW_COLS
+      : CONSTANT_INTERNAL_ROW_COLS
     if (!externalTable && fieldInfo.name?.startsWith("_")) {
       newError.name = `Column name cannot start with an underscore.`
     } else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) {
       newError.name = `Illegal character; must be alpha-numeric.`
-    } else if (PROHIBITED_COLUMN_NAMES.some(name => fieldInfo.name === name)) {
-      newError.name = `${PROHIBITED_COLUMN_NAMES.join(
+    } else if (
+      prohibited.some(
+        name => fieldInfo?.name?.toLowerCase() === name.toLowerCase()
+      )
+    ) {
+      newError.name = `${prohibited.join(
         ", "
-      )} are not allowed as column names`
+      )} are not allowed as column names - case insensitive.`
     } else if (inUse($tables.selected, fieldInfo.name, originalName)) {
       newError.name = `Column name already in use.`
     }
 
     if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) {
-      newError.subtype = `Auto Column requires a type`
+      newError.subtype = `Auto Column requires a type.`
     }
 
     if (fieldInfo.fieldName && fieldInfo.tableId) {

packages/server/src/api/routes/tests/table.spec.ts

@@ -276,6 +276,31 @@ describe.each([
         })
     })
 
+    isInternal &&
+      it("shouldn't allow duplicate column names", async () => {
+        const saveTableRequest: SaveTableRequest = {
+          ...basicTable(),
+        }
+        saveTableRequest.schema["Type"] = {
+          type: FieldType.STRING,
+          name: "Type",
+        }
+        // allow the "Type" column - internal columns aren't case sensitive
+        await config.api.table.save(saveTableRequest, {
+          status: 200,
+        })
+        saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" }
+        saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" }
+
+        await config.api.table.save(saveTableRequest, {
+          status: 400,
+          body: {
+            message:
+              'Column(s) "foo" are duplicated - check for other columns with these name (case in-sensitive)',
+          },
+        })
+      })
+
     it("should add a new column for an internal DB table", async () => {
       const saveTableRequest: SaveTableRequest = {
         ...basicTable(),

packages/server/src/sdk/app/tables/internal/index.ts

@@ -17,6 +17,7 @@ import { cloneDeep } from "lodash/fp"
 import isEqual from "lodash/isEqual"
 import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula"
 import { context } from "@budibase/backend-core"
+import { findDuplicateInternalColumns } from "@budibase/shared-core"
 import { getTable } from "../getters"
 import { checkAutoColumns } from "./utils"
 import * as viewsSdk from "../../views"
@@ -44,6 +45,17 @@ export async function save(
   if (hasTypeChanged(table, oldTable)) {
     throw new Error("A column type has changed.")
   }
+
+  // check for case sensitivity - we don't want to allow duplicated columns
+  const duplicateColumn = findDuplicateInternalColumns(table)
+  if (duplicateColumn.length) {
+    throw new Error(
+      `Column(s) "${duplicateColumn.join(
+        ", "
+      )}" are duplicated - check for other columns with these name (case in-sensitive)`
+    )
+  }
+
   // check that subtypes have been maintained
   table = checkAutoColumns(table, oldTable)
 

packages/shared-core/src/constants/index.ts

@@ -1,5 +1,6 @@
 export * from "./api"
 export * from "./fields"
+export * from "./rows"
 
 export const OperatorOptions = {
   Equals: {

packages/shared-core/src/constants/rows.ts

@@ -0,0 +1,14 @@
+export const CONSTANT_INTERNAL_ROW_COLS = [
+  "_id",
+  "_rev",
+  "type",
+  "createdAt",
+  "updatedAt",
+  "tableId",
+] as const
+
+export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const
+
+export function isInternalColumnName(name: string): boolean {
+  return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name)
+}

packages/shared-core/src/table.ts

@@ -1,4 +1,5 @@
-import { FieldType } from "@budibase/types"
+import { FieldType, Table } from "@budibase/types"
+import { CONSTANT_INTERNAL_ROW_COLS } from "./constants"
 
 const allowDisplayColumnByType: Record<FieldType, boolean> = {
   [FieldType.STRING]: true,
@@ -51,3 +52,27 @@ export function canBeDisplayColumn(type: FieldType): boolean {
 export function canBeSortColumn(type: FieldType): boolean {
   return !!allowSortColumnByType[type]
 }
+
+export function findDuplicateInternalColumns(table: Table): string[] {
+  // maintains the case of keys
+  const casedKeys = Object.keys(table.schema)
+  // get the column names
+  const uncasedKeys = casedKeys.map(colName => colName.toLowerCase())
+  // there are duplicates
+  const set = new Set(uncasedKeys)
+  let duplicates: string[] = []
+  if (set.size !== uncasedKeys.length) {
+    for (let key of set.keys()) {
+      const count = uncasedKeys.filter(name => name === key).length
+      if (count > 1) {
+        duplicates.push(key)
+      }
+    }
+  }
+  for (let internalColumn of CONSTANT_INTERNAL_ROW_COLS) {
+    if (casedKeys.find(key => key === internalColumn)) {
+      duplicates.push(internalColumn)
+    }
+  }
+  return duplicates
+}