diff --git a/docs/docs/2-models.md b/docs/docs/2-models.md index 8c08df57..5ff0d307 100644 --- a/docs/docs/2-models.md +++ b/docs/docs/2-models.md @@ -156,6 +156,39 @@ query { The name of the field that ought to be used as display value, e.g. a `Post`'s `title`. +### `sensitiveDisplay` + +Set this when the display field holds sensitive data, e.g. a person's name or email address: + +```js +{ + name: 'User', + kind: 'entity', + displayField: 'fullName', + sensitiveDisplay: true, + // ... +} +``` + +Technical error messages then identify the entity by id alone instead of quoting the display value: + +``` +User 0b7e… is not deleted. // with sensitiveDisplay +User "Jane Doe" (0b7e…) is not deleted. // without +``` + +Those messages are returned to the client and written to server logs — for a person entity that means +a name (and, where the display field is built from one, an email address) ends up in both. Every +message built from a model's display value is covered, not just the one above: the already-deleted, +cannot-be-deleted-because-it-has, depends-on, cannot-restore-directly and not-found messages all use +the same helper. + +The display value is still used wherever it is shown on purpose — the client display query and the +delete dry-run payload an admin confirms against. This flag governs technical error messages only. + +Like `displayField`, it is not inherited by child models: set it on every model that declares a +sensitive `displayField`. + ### `defaultOrderBy` An array of orders with the same structure as the `orderBy` parameters in GraphQL queries. The implicit default order by is `[{ createdAt: 'DESC }]`. diff --git a/src/models/model-definitions.ts b/src/models/model-definitions.ts index 437df0dd..3f1d19c8 100644 --- a/src/models/model-definitions.ts +++ b/src/models/model-definitions.ts @@ -176,6 +176,22 @@ export type ModelDefinition = { restoreArgs?: readonly Field[]; }; displayField?: string; + + /** + * Set this when the display field holds sensitive data, e.g. a person's name or email address. + * Technical error messages then identify the entity by id alone (`User `) instead of + * quoting the display value (`User "Jane Doe (jane@example.com)" ()`), so that personal + * data cannot leak into errors returned to clients or into server logs. + * + * The display value is still used wherever it is shown on purpose — the client display query + * and the delete dry-run payload an admin confirms against. This flag governs technical error + * messages only. + * + * Like `displayField`, this is not inherited by child models: set it on every model that + * declares a sensitive `displayField`. + */ + sensitiveDisplay?: boolean; + defaultOrderBy?: readonly OrderBy[]; fields: readonly EntityFieldDefinition[]; diff --git a/src/models/models.ts b/src/models/models.ts index 8e4e78d0..881b1645 100644 --- a/src/models/models.ts +++ b/src/models/models.ts @@ -375,6 +375,7 @@ export class EntityModel extends Model { }; aggregatable?: boolean; displayField?: string; + sensitiveDisplay?: boolean; defaultOrderBy?: OrderBy[]; fields!: EntityField[]; diff --git a/src/resolvers/utils.ts b/src/resolvers/utils.ts index 1eab9720..226d8486 100644 --- a/src/resolvers/utils.ts +++ b/src/resolvers/utils.ts @@ -274,8 +274,13 @@ export const getColumnExpression = ( return getColumn(node, fieldName); }; +/** + * Identifies an entity in a technical error message. The display value is quoted to keep the message + * readable, unless the model marks its display as sensitive (`sensitiveDisplay`), in which case the + * id alone identifies the entity — these messages reach clients and server logs. + */ export const getTechnicalDisplay = (model: EntityModel, entity: Entity) => - model.displayField && entity[model.displayField] + model.displayField && !model.sensitiveDisplay && entity[model.displayField] ? `${model.name} "${entity[model.displayField]}" (${entity.id})` : entity.id ? `${model.name} ${entity.id}` diff --git a/tests/unit/technical-display.spec.ts b/tests/unit/technical-display.spec.ts new file mode 100644 index 00000000..e191c6ef --- /dev/null +++ b/tests/unit/technical-display.spec.ts @@ -0,0 +1,44 @@ +import { ModelDefinitions, Models } from '../../src/models'; +import { getTechnicalDisplay } from '../../src/resolvers/utils'; + +// Local models rather than the shared test models, so that adding an entity here +// does not churn every generated-schema snapshot. +const modelDefinitions: ModelDefinitions = [ + { + kind: 'entity', + name: 'Post', + displayField: 'title', + fields: [{ name: 'title', type: 'String' }], + }, + { + kind: 'entity', + name: 'Person', + displayField: 'fullName', + sensitiveDisplay: true, + fields: [{ name: 'fullName', type: 'String' }], + }, +]; + +const models = new Models(modelDefinitions); +const Post = models.getModel('Post', 'entity'); +const Person = models.getModel('Person', 'entity'); + +describe('getTechnicalDisplay', () => { + it('quotes the display value of an ordinary model', () => { + expect(getTechnicalDisplay(Post, { id: 'post-1', title: 'Hello' })).toBe('Post "Hello" (post-1)'); + }); + + it('leaves out a sensitive display value, keeping the id', () => { + expect(getTechnicalDisplay(Person, { id: 'person-1', fullName: 'Jane Doe (jane@example.com)' })).toBe( + 'Person person-1', + ); + }); + + it('falls back to the id when the entity has no display value', () => { + expect(getTechnicalDisplay(Post, { id: 'post-1' })).toBe('Post post-1'); + }); + + it('falls back to the model name when the entity has neither display value nor id', () => { + expect(getTechnicalDisplay(Person, {})).toBe('Person'); + }); +});