diff --git a/server/db/migrations-pg.ts b/server/db/migrations-pg.ts index e6ca8ee65..d129ffaa2 100644 --- a/server/db/migrations-pg.ts +++ b/server/db/migrations-pg.ts @@ -1147,4 +1147,13 @@ export const pgMigrations: Migration[] = [ alter table collab_documents add column generation text not null default ''; `, }, + { + // See migrations-sqlite.ts:024 — clears display names that are just the + // account's email address, because author bindings render them publicly. + id: '024_clear_email_display_names', + sql: ` + update users set display_name = '' + where trim(lower(display_name)) = trim(lower(email)); + `, + }, ] diff --git a/server/db/migrations-sqlite.ts b/server/db/migrations-sqlite.ts index 734a53fb3..a4c5cdd74 100644 --- a/server/db/migrations-sqlite.ts +++ b/server/db/migrations-sqlite.ts @@ -1211,4 +1211,17 @@ export const sqliteMigrations: Migration[] = [ alter table collab_documents add column generation text not null default ''; `, }, + { + // `display_name` is rendered on PUBLIC pages through author bindings, and + // setup used to default it to the account's email address — so every + // install created before this has an address sitting in a public field. + // Clear it only where it is exactly the address; a name somebody actually + // chose is left alone. Admin surfaces already fall back to the email for + // their own display, so an empty value costs nothing there. + id: '024_clear_email_display_names', + sql: ` + update users set display_name = '' + where trim(lower(display_name)) = trim(lower(email)); + `, + }, ] diff --git a/server/handlers/cms/setup.ts b/server/handlers/cms/setup.ts index c9cea4613..9581fe11d 100644 --- a/server/handlers/cms/setup.ts +++ b/server/handlers/cms/setup.ts @@ -54,12 +54,16 @@ export async function handleSetupRoutes(req: Request, db: DbClient): Promise { it('offers author display fields without exposing user ids as binding fields', () => { + // `author` is declared, because it is the name an author reaches for and + // an undeclared key is exactly how a binding to a non-field goes unnoticed. expect(DataRowsSource.fields).toContainEqual({ - id: 'authorName', + id: 'author', label: 'Author name', }) + expect(DataRowsSource.fields).toContainEqual({ + id: 'authorName', + label: 'Author name (alias)', + }) expect(DataRowsSource.fields).toContainEqual({ id: 'authorRoleName', label: 'Author role', diff --git a/src/__tests__/publisher/authorBindingLeak.test.ts b/src/__tests__/publisher/authorBindingLeak.test.ts new file mode 100644 index 000000000..bb27b8bbd --- /dev/null +++ b/src/__tests__/publisher/authorBindingLeak.test.ts @@ -0,0 +1,57 @@ +/** + * A binding must never publish internal shape. + * + * `{currentEntry.author}` used to resolve to the `PublicDataUserReference` + * object, and the token interpolator JSON-stringified any non-scalar "so + * mistakes stay visible" — an author-facing debug affordance running in the + * public publish path. A single template binding therefore published + * `{"displayName":"owner@example.com","roleSlug":"owner","roleName":"Owner"}` + * into live HTML, and into every screenshot taken of it. + * + * Two independent guards keep that shut, and both are asserted here: the + * people keys are leaf strings, and a non-scalar value renders as nothing + * rather than as JSON. + */ +import { describe, expect, it } from 'bun:test' +import { interpolateTokens } from '@core/templates/tokenInterpolation' +import type { TemplateContext } from '@core/templates/contextFrames' + +function contextWithEntry(fields: Record): TemplateContext { + return { entryStack: [{ id: 'row_1', fields }] } as unknown as TemplateContext +} + +describe('author bindings never publish internal shape', () => { + it('renders the display name for {currentEntry.author}', () => { + const ctx = contextWithEntry({ author: 'Rosa Ferrandis' }) + expect(interpolateTokens('By {currentEntry.author}', ctx)).toBe('By Rosa Ferrandis') + }) + + it('renders nothing for a binding that resolves to an object', () => { + const ctx = contextWithEntry({ + author: { displayName: 'owner@example.com', roleSlug: 'owner', roleName: 'Owner' }, + }) + const out = interpolateTokens('By {currentEntry.author}', ctx) + + expect(out).toBe('By ') + expect(out).not.toContain('@') + expect(out).not.toContain('roleSlug') + expect(out).not.toContain('displayName') + }) + + it('renders nothing for a binding that resolves to an array', () => { + const ctx = contextWithEntry({ tags: ['a', 'b'] }) + expect(interpolateTokens('{currentEntry.tags}', ctx)).toBe('') + }) + + it('still honours an author-supplied fallback when the value is non-scalar', () => { + const ctx = contextWithEntry({ author: { displayName: 'owner@example.com' } }) + expect(interpolateTokens('{currentEntry.author|Anonymous}', ctx)).toBe('Anonymous') + }) + + it('leaves scalar bindings untouched', () => { + const ctx = contextWithEntry({ title: 'El inventario', year: 2025, pardoned: false }) + expect(interpolateTokens('{currentEntry.title} ({currentEntry.year})', ctx)) + .toBe('El inventario (2025)') + expect(interpolateTokens('{currentEntry.pardoned}', ctx)).toBe('false') + }) +}) diff --git a/src/__tests__/server/loopPrefetch.test.ts b/src/__tests__/server/loopPrefetch.test.ts index 4d315cc2b..32842b94a 100644 --- a/src/__tests__/server/loopPrefetch.test.ts +++ b/src/__tests__/server/loopPrefetch.test.ts @@ -49,24 +49,22 @@ describe('loopPrefetch', () => { createdAt: '2026-05-01T10:02:00.000Z', }) + // Every people key is a LEAF. `author` used to be the reference object, + // so a template binding it published `{"displayName":…,"roleSlug":…}` + // straight into the page. expect(item.fields).toMatchObject({ - author: { - displayName: 'Author Name', - roleSlug: 'editor', - roleName: 'Editor', - }, + author: 'Author Name', authorName: 'Author Name', authorRoleName: 'Editor', authorRoleSlug: 'editor', - publishedBy: { - displayName: 'Publisher Name', - roleSlug: 'admin', - roleName: 'Admin', - }, + publishedBy: 'Publisher Name', publishedByName: 'Publisher Name', publishedByRoleName: 'Admin', publishedByRoleSlug: 'admin', }) + for (const key of ['author', 'publishedBy']) { + expect(typeof item.fields[key]).not.toBe('object') + } expect('authorUserId' in item.fields).toBe(false) expect('authorId' in item.fields).toBe(false) expect('publishedByUserId' in item.fields).toBe(false) diff --git a/src/__tests__/server/setupDisplayName.test.ts b/src/__tests__/server/setupDisplayName.test.ts new file mode 100644 index 000000000..aee62024e --- /dev/null +++ b/src/__tests__/server/setupDisplayName.test.ts @@ -0,0 +1,72 @@ +/** + * A display name is never silently the account's email address. + * + * `display_name` is rendered on PUBLIC pages through author bindings, and both + * setup and `createUser` used to default it to the email. So a fresh install + * had the owner's address sitting in a publicly-bindable field before anyone + * had written a line of content, and the only way to change it was the account + * page after the fact. + * + * Setup now takes an optional `displayName`, and an unset name stays empty. + */ +import { describe, expect, it } from 'bun:test' +import type { DbClient } from '../../../server/db' +import { handleCmsRequest } from '../../../server/handlers/cms' +import { createUser, findUserByEmail } from '../../../server/repositories/users' +import { createTestDb } from '../helpers/createTestDb' + +const EMAIL = 'owner@example.com' +const PASSWORD = 'long-enough-password' + +async function runSetup(db: DbClient, body: Record): Promise { + return await handleCmsRequest( + new Request('http://localhost/admin/api/cms/setup', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ siteName: 'Test', email: EMAIL, password: PASSWORD, ...body }), + }), + db, + ) +} + +describe('setup display name', () => { + it('leaves the display name empty when none is given', async () => { + const { db } = await createTestDb() + expect((await runSetup(db, {})).status).toBe(201) + + const owner = await findUserByEmail(db, EMAIL) + expect(owner?.displayName).toBe('') + expect(owner?.displayName).not.toContain('@') + }) + + it('stores the display name given at setup', async () => { + const { db } = await createTestDb() + expect((await runSetup(db, { displayName: 'Rosa Ferrandis' })).status).toBe(201) + + expect((await findUserByEmail(db, EMAIL))?.displayName).toBe('Rosa Ferrandis') + }) + + it('trims a whitespace-only display name to empty rather than the email', async () => { + const { db } = await createTestDb() + expect((await runSetup(db, { displayName: ' ' })).status).toBe(201) + + expect((await findUserByEmail(db, EMAIL))?.displayName).toBe('') + }) +}) + +describe('createUser display name', () => { + it('does not fall back to the email address', async () => { + const { db } = await createTestDb() + await runSetup(db, {}) + + const created = await createUser(db, { + email: 'editor@example.com', + displayName: '', + passwordHash: 'x', + roleId: 'admin', + }) + + expect(created.displayName).toBe('') + expect(created.displayName).not.toContain('@') + }) +}) diff --git a/src/admin/preauth/AdminPreAuthForm.module.css b/src/admin/preauth/AdminPreAuthForm.module.css index 9d15f0728..37699825d 100644 --- a/src/admin/preauth/AdminPreAuthForm.module.css +++ b/src/admin/preauth/AdminPreAuthForm.module.css @@ -43,6 +43,12 @@ font-weight: 600; } +/* Secondary note inside a field label — lighter than the label it follows. */ +.hint { + color: var(--text-subtle); + font-weight: 400; +} + @keyframes instaticAdminSpin { to { transform: rotate(360deg); diff --git a/src/admin/preauth/AdminPreAuthForm.tsx b/src/admin/preauth/AdminPreAuthForm.tsx index c0b4377aa..12950bd28 100644 --- a/src/admin/preauth/AdminPreAuthForm.tsx +++ b/src/admin/preauth/AdminPreAuthForm.tsx @@ -68,6 +68,7 @@ export function AdminPreAuthForm({ onAuthenticated, }: AdminPreAuthFormProps) { const [siteName, setSiteName] = useState('My Site') + const [displayName, setDisplayName] = useState('') const [email, setEmail] = useState('') const [password, setPassword] = useState('') const [mfaCode, setMfaCode] = useState('') @@ -75,6 +76,7 @@ export function AdminPreAuthForm({ const [error, setError] = useState(initialError) const siteNameId = useId() + const displayNameId = useId() const emailId = useId() const passwordId = useId() const mfaCodeId = useId() @@ -86,7 +88,7 @@ export function AdminPreAuthForm({ return } await runAuthAction(async () => { - await setupCms({ siteName, email, password }) + await setupCms({ siteName, email, password, displayName }) await loginCms({ email, password }) onAuthenticated(await getCurrentCmsUser()) }, 'Setup failed', setSubmitting, setError) @@ -167,16 +169,32 @@ export function AdminPreAuthForm({ /> ) : phase === 'setup' && ( - + <> + + + {/* Optional, and public: this is what author bindings render on + published pages. Left blank they render nothing — which is + why it is offered here rather than only on the account page. */} + + )} {phase !== 'mfa' && ( diff --git a/src/core/data/publicDataUser.ts b/src/core/data/publicDataUser.ts index ba852c423..24ea45126 100644 --- a/src/core/data/publicDataUser.ts +++ b/src/core/data/publicDataUser.ts @@ -1,8 +1,15 @@ /** * Public-facing user reference attached to published data rows. * - * Strips internal-only ids and email addresses, retaining only the bits the - * publisher and frontend templates need to render bylines safely. + * Drops internal-only ids and the `email` field, retaining only the bits the + * publisher and frontend templates need to render bylines. + * + * It does NOT guarantee the result is free of email addresses: `displayName` + * carries whatever the account has set, and setup used to default that to the + * address. Two things keep it out of public HTML now — a display name is empty + * until someone sets one (`createUser`), and the binding frames expose the + * display name as a leaf string rather than this object, because a non-scalar + * binding used to be serialised wholesale into the page. */ interface PublicDataUserReference { diff --git a/src/core/loops/sources/dataRows.ts b/src/core/loops/sources/dataRows.ts index edef240c3..ab96d8151 100644 --- a/src/core/loops/sources/dataRows.ts +++ b/src/core/loops/sources/dataRows.ts @@ -164,12 +164,12 @@ function rowToLoopItem( versionNumber: Number(row.version_number), tableId: row.table_id, tableSlug: row.table_slug, - // People - author, + // People — display names, never the reference object. See loopPrefetch. + author: author?.displayName ?? null, authorName: author?.displayName ?? null, authorRoleSlug: author?.roleSlug ?? null, authorRoleName: author?.roleName ?? null, - publishedBy, + publishedBy: publishedBy?.displayName ?? null, publishedByName: publishedBy?.displayName ?? null, publishedByRoleSlug: publishedBy?.roleSlug ?? null, publishedByRoleName: publishedBy?.roleName ?? null, @@ -503,7 +503,8 @@ export const DataRowsSource: LoopEntitySource = { fields: [ { id: 'slug', label: 'Slug' }, { id: 'title', label: 'Title (post-type)' }, - { id: 'authorName', label: 'Author name' }, + { id: 'author', label: 'Author name' }, + { id: 'authorName', label: 'Author name (alias)' }, { id: 'authorRoleName', label: 'Author role' }, { id: 'body', label: 'Body (post-type, markdown)', format: 'html' }, { id: 'featuredMedia', label: 'Featured media (post-type)', format: 'media' }, diff --git a/src/core/persistence/cmsAuth.ts b/src/core/persistence/cmsAuth.ts index 6f8a3d681..f655820db 100644 --- a/src/core/persistence/cmsAuth.ts +++ b/src/core/persistence/cmsAuth.ts @@ -11,6 +11,8 @@ interface CmsSetupInput { siteName: string email: string password: string + /** The owner's public name. Omitted or empty means author bindings render nothing. */ + displayName?: string } interface CmsLoginInput { diff --git a/src/core/templates/tokenInterpolation.ts b/src/core/templates/tokenInterpolation.ts index 6946abc99..7314c14c2 100644 --- a/src/core/templates/tokenInterpolation.ts +++ b/src/core/templates/tokenInterpolation.ts @@ -276,14 +276,18 @@ export function interpolateTokens(input: string, context: TemplateRenderDataCont } else if (typeof rawValue === 'number' || typeof rawValue === 'boolean') { out += String(rawValue) } else { - // Objects / arrays — fall back to JSON for visibility. Real-world - // bindings should target leaf fields; surfacing the JSON keeps - // mistakes visible instead of silent. - try { - out += JSON.stringify(rawValue) - } catch { - // ignore - } + // Objects / arrays resolve to NOTHING, deliberately. + // + // This branch used to JSON.stringify the value "so mistakes stay + // visible". That is an author-facing debug affordance running in the + // public publish path: `{currentEntry.author}` against a reference + // object published the account's display name, role slug and role name + // into live HTML. A binding that misses is a blank, never a dump of + // whatever internal shape happened to sit behind the key — the frame + // cannot know which of its values are safe to print. + // + // Treated as missing, so an author-supplied fallback still applies. + if (seg.fallback !== undefined) out += seg.fallback } } return out