Declared custom fields in the Admin API member schema - #29457
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
WalkthroughCustom fields are declared in member API schemas and preserved during validation. Member services remove them when the feature is disabled, reject non-empty values during creation, and handle edits consistently. Custom-field writes enforce key-count and key-length limits. Tests cover schema preservation, feature-flag behavior, malformed and empty payloads, prototype-pollution cases, and oversized inputs. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 52s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 59s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 3m | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 38s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 22s | View ↗ |
nx run-many -t test:unit -p ghost,@tryghost/adm... |
✅ Succeeded | 31s | View ↗ |
nx run-many -t lint -p ghost,@tryghost/admin-ap... |
✅ Succeeded | 23s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 8s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-20 22:00:58 UTC
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29457 +/- ##
==========================================
- Coverage 74.53% 74.51% -0.03%
==========================================
Files 1602 1602
Lines 140399 140450 +51
Branches 17066 17055 -11
==========================================
+ Hits 104652 104658 +6
- Misses 34693 34774 +81
+ Partials 1054 1018 -36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`:
- Around line 472-477: Update the custom_fields validation condition in the
member creation service to guard against null before calling Object.keys.
Preserve the existing validation error for defined, non-empty custom_fields
while allowing null to avoid the TypeError and 500 response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f778b903-16d2-4854-a6b5-e4416a2a6c54
📒 Files selected for processing (7)
ghost/core/core/server/api/endpoints/utils/validators/input/members.jsghost/core/core/server/services/members-custom-fields/values-service.tsghost/core/core/server/services/members/members-api/services/member-bread-service.jsghost/core/test/e2e-api/admin/member-custom-fields.test.tspackages/admin-api-schema/src/schemas/members-edit.jsonpackages/admin-api-schema/src/schemas/members.jsonpackages/admin-api-schema/test/api.test.ts
2e48c31 to
80ebc53
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/test/e2e-api/admin/member-custom-fields.test.ts`:
- Around line 842-850: Update the overlong-key assertion in the “rejects a key
too long to name a field” test to use the serialized error’s message when
context is null, matching the adjacent tests’ nullish fallback before checking
the length limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45ff4e33-ad99-4214-a530-43962499613c
📒 Files selected for processing (8)
ghost/core/core/server/api/endpoints/utils/validators/input/members.jsghost/core/core/server/services/members-custom-fields/index.tsghost/core/core/server/services/members-custom-fields/values-service.tsghost/core/core/server/services/members/members-api/services/member-bread-service.jsghost/core/test/e2e-api/admin/member-custom-fields.test.tspackages/admin-api-schema/src/schemas/members-edit.jsonpackages/admin-api-schema/src/schemas/members.jsonpackages/admin-api-schema/test/api.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/admin-api-schema/src/schemas/members.json
- packages/admin-api-schema/test/api.test.ts
- packages/admin-api-schema/src/schemas/members-edit.json
- ghost/core/core/server/services/members/members-api/services/member-bread-service.js
Resolving the keys in a custom field values write binds one query parameter per key, so a large enough object exceeded the driver's parameter limit and surfaced as a 500 carrying the generated SQL. A write cannot name more fields than the site is allowed to define, so the number of keys is bounded by the same operator setting that bounds definitions rather than by a number invented in the value path: it is injected as a getter by the module that already reads it for the definitions service, so both paths share one ceiling and it can change between requests. A key is bounded in turn by the width of the column it is minted into, so no key a site could have minted is refused and a longer one cannot name a field that exists. That bound describes what a values object is rather than a step in resolving one, so it is expressed in the schema the input is parsed against, which both verbs share, and a key too wide for the column is refused as malformed input rather than looked up and reported as unknown. The unknown-field error names the key it could not resolve, so keeping an overlong key out of the lookup also keeps it out of the response.
ref https://linear.app/ghost/issue/BER-3801/declare-custom-fields-in-admin-api-schema-and-remove-the The member schemas set additionalProperties:false, so ajv silently stripped custom_fields out of the body before it reached the service, which was worked around by wrapping the shared members input validator. Declaring the property removes the need for that wrap. It is declared without a shape because only the site's field-type catalog knows what a given key accepts, and constraining it here would both split validation across two layers that could disagree and change behaviour for existing callers, since the key reaches the schema on every site regardless of whether the flag is on. The flag gate therefore moves into the service, where a flag-off site drops the key exactly as a Ghost predating the feature would. Because any JSON value now reaches the create path, the not-supported-on-create refusal asks the values service whether the body names a value, and it answers exactly as resolving an edit would: both verbs share one parse, so a body malformed enough to be refused on edit is refused on create rather than accepted and dropped, and a well-formed body that names no value is created as though custom_fields had never been sent, including an object carrying only a __proto__ key, which names nothing once parsed. The question is only asked when the key is present, so a create that never mentions custom fields does not reach for the collaborator to be told nothing is there.
80ebc53 to
0e39151
Compare

Refs BER-3801
Problem
The member schemas in the Admin API schema package reject unknown properties, and the validator is configured to strip them rather than error. Custom field values sent on a member body were therefore removed before they ever reached the service — silently, with nothing in the response to explain where they went. The previous workaround stashed the values across validation and restored them afterwards, which kept the feature working but left the API describing itself inaccurately: the endpoint accepted something its own schema said did not exist.
Solution
Custom fields are now declared on the member schemas directly, so the key survives validation on its own terms and the stash-and-restore workaround is gone. The declaration is deliberately permissive and constrains nothing about the value. Only the site's own field-type catalog knows what any given key accepts, and splitting that judgement across two layers invites them to disagree; validating in one place keeps the rules honest. It also matters because the schema describes the endpoint rather than any one site, so a shape it judged would be judged everywhere, including on sites that have never turned the feature on and that accepted anything under this key before it existed.
Because the schema no longer decides whether values get through, the feature flag does. The service drops the key when the feature is off, on both create and edit, so a flag-off site behaves exactly as a Ghost that predates the feature: the write succeeds, everything else applies, and the values are ignored rather than rejected. With the flag on, setting values while creating a member is still refused outright, since that vertical has not been built yet and refusing is more legible than accepting and quietly discarding. An empty object is allowed through on create so clients whose serializer always emits the key can still create members.
There is also a ceiling on the number of keys accepted in a single write. Resolving keys binds one query parameter each, so a large enough object exceeded the database driver's parameter limit and surfaced as a server error carrying the generated SQL. The limit sits far above any workable number of field definitions and well below the lowest driver limit, so its only job is to turn an absurd payload into a clean rejection.