enhancement!: restrict strapi prefixes in CTB (#20999)

This commit is contained in:
Ben Irvin 2024-08-21 13:36:12 +02:00 committed by GitHub
parent a2eac9d891
commit 8210c57d20
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 168 additions and 86 deletions

View File

@ -1,3 +1,4 @@
import * as builder from '../../../services/builder';
import { validateComponentInput, validateUpdateComponentInput } from '../component'; import { validateComponentInput, validateUpdateComponentInput } from '../component';
const componentValidation = { const componentValidation = {
@ -11,14 +12,7 @@ describe('Component validator', () => {
plugins: { plugins: {
'content-type-builder': { 'content-type-builder': {
services: { services: {
builder: { builder,
getReservedNames() {
return {
models: [],
attributes: ['thisIsReserved'],
};
},
},
}, },
}, },
}, },

View File

@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */ /* eslint-disable @typescript-eslint/ban-ts-comment */
import * as builder from '../../../services/builder';
import { import {
validateKind, validateKind,
validateUpdateContentTypeInput, validateUpdateContentTypeInput,
@ -12,14 +13,7 @@ describe('Content type validator', () => {
plugins: { plugins: {
'content-type-builder': { 'content-type-builder': {
services: { services: {
builder: { builder,
getReservedNames() {
return {
models: ['reserved-name'],
attributes: ['thisIsReserved'],
};
},
},
}, },
}, },
}, },
@ -49,7 +43,7 @@ describe('Content type validator', () => {
pluralName: 'tests', pluralName: 'tests',
displayName: 'Test', displayName: 'Test',
attributes: { attributes: {
thisIsReserved: { entryId: {
type: 'string', type: 'string',
default: '', default: '',
}, },
@ -62,13 +56,14 @@ describe('Content type validator', () => {
await validateUpdateContentTypeInput(data).catch((err) => { await validateUpdateContentTypeInput(data).catch((err) => {
expect(err).toMatchObject({ expect(err).toMatchObject({
name: 'ValidationError', name: 'ValidationError',
message: 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', message:
'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*',
details: { details: {
errors: [ errors: [
{ {
path: ['contentType', 'attributes', 'thisIsReserved'], path: ['contentType', 'attributes', 'entryId'],
message: message:
'Attribute keys cannot be one of __component, __contentType, thisIsReserved', 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*',
name: 'ValidationError', name: 'ValidationError',
}, },
], ],
@ -84,7 +79,7 @@ describe('Content type validator', () => {
pluralName: 'tests', pluralName: 'tests',
displayName: 'Test', displayName: 'Test',
attributes: { attributes: {
THIS_IS_RESERVED: { ENTRY_ID: {
type: 'string', type: 'string',
default: '', default: '',
}, },
@ -97,13 +92,14 @@ describe('Content type validator', () => {
await validateUpdateContentTypeInput(data).catch((err) => { await validateUpdateContentTypeInput(data).catch((err) => {
expect(err).toMatchObject({ expect(err).toMatchObject({
name: 'ValidationError', name: 'ValidationError',
message: 'Attribute keys cannot be one of __component, __contentType, thisIsReserved', message:
'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*',
details: { details: {
errors: [ errors: [
{ {
path: ['contentType', 'attributes', 'THIS_IS_RESERVED'], path: ['contentType', 'attributes', 'ENTRY_ID'],
message: message:
'Attribute keys cannot be one of __component, __contentType, thisIsReserved', 'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*',
name: 'ValidationError', name: 'ValidationError',
}, },
], ],
@ -119,8 +115,8 @@ describe('Content type validator', () => {
test.each(reservedNames)('Throws when reserved model names are used in %s', async (name) => { test.each(reservedNames)('Throws when reserved model names are used in %s', async (name) => {
const data = { const data = {
contentType: { contentType: {
singularName: name === 'singularName' ? 'reserved-name' : 'not-reserved-single', singularName: name === 'singularName' ? 'date-time' : 'not-reserved-single',
pluralName: name === 'pluralName' ? 'reserved-name' : 'not-reserved-plural', pluralName: name === 'pluralName' ? 'date-time' : 'not-reserved-plural',
displayName: 'Test', displayName: 'Test',
attributes: { attributes: {
notReserved: { notReserved: {
@ -136,12 +132,12 @@ describe('Content type validator', () => {
await validateUpdateContentTypeInput(data).catch((err) => { await validateUpdateContentTypeInput(data).catch((err) => {
expect(err).toMatchObject({ expect(err).toMatchObject({
name: 'ValidationError', name: 'ValidationError',
message: `Content Type name cannot be one of reserved-name`, message: `Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*`,
details: { details: {
errors: [ errors: [
{ {
path: ['contentType', name], path: ['contentType', name],
message: `Content Type name cannot be one of reserved-name`, message: `Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*`,
name: 'ValidationError', name: 'ValidationError',
}, },
], ],

View File

@ -146,8 +146,8 @@ const forbiddenContentTypeNameValidator = () => {
if (typeof value !== 'string') { if (typeof value !== 'string') {
return true; return true;
} }
// compare snake case to check the actual column names that will be used in the database
return reservedNames.every((reservedName) => snakeCase(reservedName) !== snakeCase(value)); return !getService('builder').isReservedModelName(value);
}, },
}; };
}; };

View File

@ -1,7 +1,7 @@
import { yup } from '@strapi/utils'; import { yup } from '@strapi/utils';
import _ from 'lodash'; import _ from 'lodash';
import { snakeCase } from 'lodash/fp'; import { snakeCase } from 'lodash/fp';
import { modelTypes, FORBIDDEN_ATTRIBUTE_NAMES, typeKinds } from '../../services/constants'; import { modelTypes, typeKinds } from '../../services/constants';
import { getService } from '../../utils'; import { getService } from '../../utils';
import { isValidKey, isValidCollectionName } from './common'; import { isValidKey, isValidCollectionName } from './common';
import { getTypeValidator } from './types'; import { getTypeValidator } from './types';
@ -78,22 +78,11 @@ const isConflictingKey = (key: string, attributes: Record<string, any>) => {
}; };
const isForbiddenKey = (key: string) => { const isForbiddenKey = (key: string) => {
const snakeCaseKey = snakeCase(key); return getService('builder').isReservedAttributeName(key);
const reservedNames = [
...FORBIDDEN_ATTRIBUTE_NAMES,
...getService('builder').getReservedNames().attributes,
];
return reservedNames.some((reserved) => {
return snakeCase(reserved) === snakeCaseKey;
});
}; };
const forbiddenValidator = () => { const forbiddenValidator = () => {
const reservedNames = [ const reservedNames = [...getService('builder').getReservedNames().attributes];
...FORBIDDEN_ATTRIBUTE_NAMES,
...getService('builder').getReservedNames().attributes,
];
return yup.mixed().test({ return yup.mixed().test({
name: 'forbiddenKeys', name: 'forbiddenKeys',

View File

@ -1,17 +1,7 @@
export const getReservedNames = () => { import { snakeCase } from 'lodash/fp';
return {
// use kebab case everywhere since singularName and pluralName are validated that way // use snake_case
models: [ export const reservedAttributes = [
'boolean',
'date',
'date-time',
'time',
'upload',
'document',
'then', // no longer an issue but still restricting for being a javascript keyword
],
// attributes are compared with snake_case(name), so only snake_case is needed here and camelCase + UPPER_CASE matches will still be caught
attributes: [
// TODO: these need to come from a centralized place so we don't break things accidentally in the future and can share them outside the CTB, for example on Strapi bootstrap prior to schema db sync // TODO: these need to come from a centralized place so we don't break things accidentally in the future and can share them outside the CTB, for example on Strapi bootstrap prior to schema db sync
// ID fields // ID fields
@ -34,11 +24,72 @@ export const getReservedNames = () => {
'localizations', 'localizations',
'meta', 'meta',
'locale', 'locale',
// TODO: remove these in favor of restricting the strapi_ prefix '__component',
'strapi', '__contentType',
'strapi_stage',
'strapi_assignee', // We support ending with * to denote prefixes
], 'strapi*',
'_strapi*',
'__strapi*',
];
// use snake_case
export const reservedModels = [
'boolean',
'date',
'date_time',
'time',
'upload',
'document',
'then', // no longer an issue but still restricting for being a javascript keyword
// We support ending with * to denote prefixes
'strapi*',
'_strapi*',
'__strapi*',
];
export const getReservedNames = () => {
return {
models: reservedModels,
attributes: reservedAttributes,
}; };
// strapi.db.getReservedNames(); };
// compare snake case to check the actual column names that will be used in the database
export const isReservedModelName = (name: string) => {
const snakeCaseName = snakeCase(name);
if (reservedModels.includes(snakeCaseName)) {
return true;
}
if (
reservedModels
.filter((key) => key.endsWith('*'))
.map((key) => key.slice(0, -1))
.some((prefix) => snakeCaseName.startsWith(prefix))
) {
return true;
}
return false;
};
// compare snake case to check the actual column names that will be used in the database
export const isReservedAttributeName = (name: string) => {
const snakeCaseName = snakeCase(name);
if (reservedAttributes.includes(snakeCaseName)) {
return true;
}
if (
reservedAttributes
.filter((key) => key.endsWith('*'))
.map((key) => key.slice(0, -1))
.some((prefix) => snakeCaseName.startsWith(prefix))
) {
return true;
}
return false;
}; };

View File

@ -36,8 +36,6 @@ export const DEFAULT_TYPES = [
export const VALID_UID_TARGETS = ['string', 'text'] as const; export const VALID_UID_TARGETS = ['string', 'text'] as const;
export const FORBIDDEN_ATTRIBUTE_NAMES = ['__component', '__contentType'] as const;
export const coreUids = { export const coreUids = {
STRAPI_USER: 'admin::user', STRAPI_USER: 'admin::user',
PREFIX: 'strapi::', PREFIX: 'strapi::',

View File

@ -8,6 +8,7 @@ const { createStrapiInstance } = require('api-tests/strapi');
const { createAuthRequest } = require('api-tests/request'); const { createAuthRequest } = require('api-tests/request');
const modelsUtils = require('api-tests/models'); const modelsUtils = require('api-tests/models');
const { createTestBuilder } = require('api-tests/builder'); const { createTestBuilder } = require('api-tests/builder');
const { kebabCase } = require('lodash/fp');
let strapi; let strapi;
let rq; let rq;
@ -48,6 +49,7 @@ describe('Content Type Builder - Content types', () => {
await restart(); await restart();
}); });
// TODO FIXME: this depends on all tests to run or else it throws an error
afterAll(async () => { afterAll(async () => {
const modelsUIDs = [ const modelsUIDs = [
'api::test-collection-type.test-collection-type', 'api::test-collection-type.test-collection-type',
@ -200,6 +202,58 @@ describe('Content Type Builder - Content types', () => {
}); });
}); });
test.each(['strapi', '_strapi', '__strapi'])(
'Cannot use %s prefix for content type name',
async (prefix) => {
const res = await rq({
method: 'POST',
url: '/content-type-builder/content-types',
body: {
contentType: {
displayName: 'unique string',
singularName: `${kebabCase(prefix)}-singular`,
pluralName: `${kebabCase(prefix)}-plural`,
attributes: {
[prefix]: {
type: 'string',
},
},
},
},
});
expect(res.statusCode).toBe(400);
expect(res.body).toEqual({
error: {
details: {
errors: [
{
message:
'Attribute keys cannot be one of id, document_id, created_at, updated_at, published_at, created_by_id, updated_by_id, created_by, updated_by, entry_id, status, localizations, meta, locale, __component, __contentType, strapi*, _strapi*, __strapi*',
name: 'ValidationError',
path: ['contentType', 'attributes', prefix],
},
{
message:
'Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*',
name: 'ValidationError',
path: ['contentType', 'singularName'],
},
{
message:
'Content Type name cannot be one of boolean, date, date_time, time, upload, document, then, strapi*, _strapi*, __strapi*',
name: 'ValidationError',
path: ['contentType', 'pluralName'],
},
],
},
message: '3 errors occurred',
name: 'ValidationError',
},
});
}
);
test('Cannot use same string for singularName and pluralName', async () => { test('Cannot use same string for singularName and pluralName', async () => {
const res = await rq({ const res = await rq({
method: 'POST', method: 'POST',