diff --git a/.changeset/18509-identity-image-logo-nullish.md b/.changeset/18509-identity-image-logo-nullish.md new file mode 100644 index 00000000000..3ac87d0b3b5 --- /dev/null +++ b/.changeset/18509-identity-image-logo-nullish.md @@ -0,0 +1,35 @@ +--- +"@objectstack/spec": minor +--- + +`UserSchema.image` and `OrganizationSchema.logo` are declared `z.string().url().nullish()` — a URL string, `null`, or the key absent are all accepted — so the user and organization bodies this platform serves parse against the schemas it publishes (#18509). + +Both were `z.string().url().optional()`: a URL string or the key's absence, and `null` refused. Both columns are better-auth-owned and nullable — `sys_user.image` and `sys_organization.logo` are each `Field.url({ required: false })`, reaching SQLite as `varchar(255)` with `notnull=0` — and better-auth SELECTs them and serialises them present-and-null for a user who never set an avatar and an organization created without a logo. + +Measured through a real `AuthManager` (better-auth 1.7.3) over a real `ObjectQL` on a real `SqliteWasmDriver`, with the platform's own `sys_user` / `sys_organization` object definitions: + +``` +/auth/sign-up/email -> user.image = null +/auth/get-session -> user.image = null +/auth/organization/create -> logo = null +/auth/organization/list -> [0].logo = null +/auth/organization/get-full-organization + -> logo = null + -> members[].user.image = null + +UserSchema.safeParse() + -> [{ path: ["image"], code: "invalid_type", + message: "Invalid input: expected string, received null" }] +OrganizationSchema.safeParse() + -> [{ path: ["logo"], code: "invalid_type", + message: "Invalid input: expected string, received null" }, … ] +``` + +Those two paths now parse. + +- **Measured, not inferred.** #18509 exists because PR #18501's contract review named these two siblings as *not measured* rather than folding them into the `SessionUserSchema.image` ruling it had. The verdict here comes from the probe above, run the way that ruling's own evidence was taken; the analogy was only ever a reason to look. +- **The declaration was the thing that was wrong.** Prime Directive #12's default — fix the producer, never widen the consumer — rests on the premise it states out loud, that we own both ends. We do not: the nullable columns belong to a third-party model, so PD #12's own exit clause is the operative sentence. +- **A pure widening.** `.nullish()`, not `.nullable()`: the key's ABSENCE is a legal shape today, so `.nullable()` would retire a live shape as the price of admitting `null`. Every body legal before this change is still legal. +- **`.url()` is kept, and it does not fight `null`.** These two declarations carry `.url()`, which `SessionUserSchema.image` did not, so the question had to be answered rather than copied. `.nullish()` wraps the whole `z.string().url()`: `null` and `undefined` are separate branches the URL check never sees, while a present string is still required to be a well-formed URL. Of six inputs — absent, `null`, `''`, a URL, a non-URL, a number — exactly one row moves, and it is the ruled one. `''` and `'not-a-url'` are still refused. +- **No key is added or removed** — both keys were already authored and already published, so no authorable surface moves and nothing is retired. +- **`OrganizationSchema` is not made whole by this.** The same probe found `metadata` served present-and-null and `/auth/organization/create` omitting the required `updatedAt`. Those are separate defects with their own reasoning, filed separately rather than folded in; #18509 asked about `logo`. diff --git a/content/docs/references/identity/identity.mdx b/content/docs/references/identity/identity.mdx index f23523096dc..f67c02203fc 100644 --- a/content/docs/references/identity/identity.mdx +++ b/content/docs/references/identity/identity.mdx @@ -63,7 +63,7 @@ const result = AccountSchema.parse(data); | **email** | `string` | ✅ | User email address | | **emailVerified** | `boolean` | optional (default: `false`) | Whether email is verified | | **name** | `string` | optional | User display name | -| **image** | `string` | optional | Profile image URL | +| **image** | `string \| null` | optional | Profile image URL | | **createdAt** | `string` | ✅ | Account creation timestamp | | **updatedAt** | `string` | ✅ | Last update timestamp | diff --git a/content/docs/references/identity/organization.mdx b/content/docs/references/identity/organization.mdx index 20d734818c1..7eaf8c18f1d 100644 --- a/content/docs/references/identity/organization.mdx +++ b/content/docs/references/identity/organization.mdx @@ -85,7 +85,7 @@ const result = InvitationSchema.parse(data); | **id** | `string` | ✅ | Unique organization identifier | | **name** | `string` | ✅ | Organization display name | | **slug** | `string` | ✅ | Unique URL-friendly slug (lowercase alphanumeric, hyphens, underscores) | -| **logo** | `string` | optional | Organization logo URL | +| **logo** | `string \| null` | optional | Organization logo URL | | **metadata** | `Record` | optional | Custom metadata | | **createdAt** | `string` | ✅ | Organization creation timestamp | | **updatedAt** | `string` | ✅ | Last update timestamp | diff --git a/packages/spec/src/identity/identity.test.ts b/packages/spec/src/identity/identity.test.ts index 2df8180b0e7..a50cf9a42f5 100644 --- a/packages/spec/src/identity/identity.test.ts +++ b/packages/spec/src/identity/identity.test.ts @@ -70,6 +70,69 @@ describe('UserSchema', () => { }); }); +/** + * [#18509] `UserSchema.image` accepts `null` — the shape better-auth serves. + * + * `sys_user.image` is `Field.url({ required: false })`, which reaches SQLite as + * `image varchar(255)` with `notnull=0`. better-auth SELECTs that column and + * serialises it present-and-null for a user who never set an avatar, so + * `/auth/sign-up/email` and `/auth/get-session` both carry `"image": null`. + * Measured on a real `AuthManager` over ObjectQL + driver-sqlite-wasm; the + * evidence and its controls are in PR #18718's body. + * + * The whole accept set is pinned, not just the row that moved, so that a later + * flip to `.nullable()` (which would retire the legal absent-key shape) or a + * drop of `.url()` (which would start admitting `''` and `'not-a-url'`) goes + * red here rather than passing as "still accepts null". + */ +describe('[#18509] UserSchema.image accept set', () => { + const base = { + id: 'user_123', + email: 'test@example.com', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + }; + /** Issue paths, so a refusal is attributed to `image` and not to a neighbour. */ + const failedPaths = (value: unknown, present = true) => { + const input = present ? { ...base, image: value } : { ...base }; + const result = UserSchema.safeParse(input); + return result.success ? [] : result.error.issues.map((i) => i.path.join('.')); + }; + + it('accepts `null` — the value every /auth/* user body carries', () => { + expect(failedPaths(null)).toEqual([]); + }); + + it('still accepts the key being ABSENT — `.nullish()`, not `.nullable()`', () => { + expect(failedPaths(undefined, false)).toEqual([]); + }); + + it('still accepts a well-formed URL', () => { + expect(failedPaths('https://example.com/avatar.jpg')).toEqual([]); + }); + + it('still refuses a malformed URL — `.url()` keeps its force on the string branch', () => { + expect(failedPaths('not-a-url')).toEqual(['image']); + }); + + it('still refuses the empty string', () => { + expect(failedPaths('')).toEqual(['image']); + }); + + it('still refuses a non-string, non-null value', () => { + expect(failedPaths(42)).toEqual(['image']); + }); + + it('lit control: the instrument reports a neighbour when a neighbour is wrong', () => { + const { email: _dropped, ...withoutEmail } = base; + const result = UserSchema.safeParse({ ...withoutEmail, image: null }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.map((i) => i.path.join('.'))).toEqual(['email']); + } + }); +}); + describe('AccountSchema', () => { it('should accept valid OAuth account', () => { const account: Account = { diff --git a/packages/spec/src/identity/identity.zod.ts b/packages/spec/src/identity/identity.zod.ts index 5a91d73047b..d428d6e93fb 100644 --- a/packages/spec/src/identity/identity.zod.ts +++ b/packages/spec/src/identity/identity.zod.ts @@ -40,9 +40,40 @@ export const UserSchema = lazySchema(() => z.object({ name: z.string().optional().describe('User display name'), /** - * User's profile image URL - */ - image: z.string().url().optional().describe('Profile image URL'), + * User's profile image URL. + * + * `null` is accepted alongside a URL string and alongside the key being + * absent. better-auth owns this column, `sys_user.image` is declared + * `Field.url({ required: false })` and reaches SQLite as + * `image varchar(255)` with `notnull=0`, and better-auth SELECTs it and + * serialises it present-and-null for a user who never set an avatar. Measured + * on a real `AuthManager` over ObjectQL + driver-sqlite-wasm (#18509): the + * `/auth/sign-up/email` and `/auth/get-session` bodies both carry + * `"image": null`, and so does `members[].user.image` inside + * `/auth/organization/get-full-organization`. The declaration was the thing + * that was wrong — Prime Directive #12's default (fix the producer, never + * widen the consumer) rests on the premise it states out loud, that we own + * both ends, which does not hold for a third-party model. + * + * Same defect and same remedy as `SessionUserSchema.image` (#17235 / PR + * #18501, ruling batch #138 item 1), reached here by measurement rather than + * by analogy — #18509 exists precisely because that review refused to infer + * this key's verdict from that one. + * + * `.nullish()`, NOT `.nullable()`: the key's ABSENCE is a legal shape today, + * so `.nullable()` would retire a live shape as the price of admitting + * `null`. Pure widening only. + * + * `.url()` is KEPT, and it is not in tension with `null`. `.nullish()` wraps + * the whole `z.string().url()`, so `null` and `undefined` are separate + * branches the URL check never sees, while a present string is still required + * to be a well-formed URL. Measured: of the six inputs + * (absent / `null` / `''` / a URL / a non-URL / a number) exactly ONE moves, + * and it is the ruled one — `''` and `'not-a-url'` are still refused, which + * is why the empty-avatar-URL boundary note carried on #18509 does not become + * live here the way it would on a declaration without `.url()`. + */ + image: z.string().url().nullish().describe('Profile image URL'), /** * Account creation timestamp diff --git a/packages/spec/src/identity/organization.test.ts b/packages/spec/src/identity/organization.test.ts index a4abce6567e..adad18090a8 100644 --- a/packages/spec/src/identity/organization.test.ts +++ b/packages/spec/src/identity/organization.test.ts @@ -115,6 +115,97 @@ describe('OrganizationSchema', () => { }); }); +/** + * [#18509] `OrganizationSchema.logo` accepts `null` — the shape better-auth + * serves. + * + * `logo` is one of better-auth's own `sys_organization` columns, declared + * `Field.url({ required: false })` and reaching SQLite as `logo varchar(255)` + * with `notnull=0`. `/auth/organization/create`, `/auth/organization/list` and + * `/auth/organization/get-full-organization` all serve `"logo": null` for an + * organization created without one. Measured on a real `AuthManager` over + * ObjectQL + driver-sqlite-wasm; evidence and controls in PR #18718's body. + * + * The whole accept set is pinned, not just the row that moved — see the sibling + * block in `identity.test.ts` for why. + */ +describe('[#18509] OrganizationSchema.logo accept set', () => { + const base = { + id: 'org_123', + name: 'Acme Corporation', + slug: 'acme-corp', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + }; + const failedPaths = (value: unknown, present = true) => { + const input = present ? { ...base, logo: value } : { ...base }; + const result = OrganizationSchema.safeParse(input); + return result.success ? [] : result.error.issues.map((i) => i.path.join('.')); + }; + + it('accepts `null` — the value every organization body carries', () => { + expect(failedPaths(null)).toEqual([]); + }); + + it('still accepts the key being ABSENT — `.nullish()`, not `.nullable()`', () => { + expect(failedPaths(undefined, false)).toEqual([]); + }); + + it('still accepts a well-formed URL', () => { + expect(failedPaths('https://example.com/logo.png')).toEqual([]); + }); + + it('still refuses a malformed URL — `.url()` keeps its force on the string branch', () => { + expect(failedPaths('not-a-url')).toEqual(['logo']); + }); + + it('still refuses the empty string', () => { + expect(failedPaths('')).toEqual(['logo']); + }); + + it('still refuses a non-string, non-null value', () => { + expect(failedPaths(42)).toEqual(['logo']); + }); + + it('lit control: the instrument reports a neighbour when a neighbour is wrong', () => { + const { slug: _dropped, ...withoutSlug } = base; + const result = OrganizationSchema.safeParse({ ...withoutSlug, logo: null }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.map((i) => i.path.join('.'))).toEqual(['slug']); + } + }); + + /** + * ⛔ Scope fence, deliberately pinned as CURRENT behaviour rather than fixed: + * the same measurement found `metadata` served present-and-null and + * `/auth/organization/create` omitting the required `updatedAt`. Those are + * separate defects, filed separately — #18509 asked about `logo`. This pin + * exists so that the fence is visible and so that a later fix for either one + * has to come here and say so. + */ + it('does NOT (yet) accept a served body whole — metadata/updatedAt are separate cards', () => { + const served = { + id: 'org_123', + name: 'Acme Corporation', + slug: 'acme-corp', + logo: null, + metadata: null, + createdAt: '2026-01-01T00:00:00.000Z', + // `updatedAt` absent, exactly as `/auth/organization/create` serves it + }; + const result = OrganizationSchema.safeParse(served); + expect(result.success).toBe(false); + if (!result.success) { + // `logo` is gone from this list — that is this card's contribution. + expect(result.error.issues.map((i) => i.path.join('.')).sort()).toEqual([ + 'metadata', + 'updatedAt', + ]); + } + }); +}); + describe('MemberSchema', () => { it('should accept valid member data', () => { const member: Member = { diff --git a/packages/spec/src/identity/organization.zod.ts b/packages/spec/src/identity/organization.zod.ts index 62975609b0f..45036769f03 100644 --- a/packages/spec/src/identity/organization.zod.ts +++ b/packages/spec/src/identity/organization.zod.ts @@ -36,9 +36,41 @@ export const OrganizationSchema = lazySchema(() => z.object({ .describe('Unique URL-friendly slug (lowercase alphanumeric, hyphens, underscores)'), /** - * Organization logo URL - */ - logo: z.string().url().optional().describe('Organization logo URL'), + * Organization logo URL. + * + * `null` is accepted alongside a URL string and alongside the key being + * absent. `logo` is one of better-auth's own `sys_organization` columns, + * declared `Field.url({ required: false })` and reaching SQLite as + * `logo varchar(255)` with `notnull=0`; better-auth serialises it + * present-and-null for an organization created without one. Measured on a + * real `AuthManager` over ObjectQL + driver-sqlite-wasm (#18509): the + * `/auth/organization/create`, `/auth/organization/list` and + * `/auth/organization/get-full-organization` bodies all carry `"logo": null`. + * + * Same defect and same remedy as `SessionUserSchema.image` (#17235 / PR + * #18501, ruling batch #138 item 1), reached here by measurement rather than + * by analogy — #18509 exists precisely because that review refused to infer + * this key's verdict from that one. + * + * `.nullish()`, NOT `.nullable()`: the key's ABSENCE is a legal shape today, + * so `.nullable()` would retire a live shape as the price of admitting + * `null`. Pure widening only. + * + * `.url()` is KEPT, and it is not in tension with `null`. `.nullish()` wraps + * the whole `z.string().url()`, so `null` and `undefined` are separate + * branches the URL check never sees, while a present string is still required + * to be a well-formed URL. Measured: of the six inputs + * (absent / `null` / `''` / a URL / a non-URL / a number) exactly ONE moves, + * and it is the ruled one. + * + * ⚠️ This widening does NOT make a served organization body parse clean. + * The same measurement found two further divergences on this schema — + * `metadata` is served present-and-null, and `/auth/organization/create` + * omits `updatedAt`, which is declared required. Those are separate defects + * with their own reasoning and are filed separately rather than folded in + * here; #18509 asked about `logo`. + */ + logo: z.string().url().nullish().describe('Organization logo URL'), /** * Custom metadata for the organization