Removed the permission to read member custom field definitions

ref https://linear.app/ghost/issue/BER-3922/rename-the-code-behind-the-metafields-api-to-match-the-domain-language

Reading a field definition is no longer gated by a permission. A definition says
only that the site collects a shoe size, and every signed-in member is already
shown the whole list through Portal, so holding it back from staff protected
nothing that was not already on offer elsewhere.

It was also inverted. An integration holding member browse received members'
field values on the member payload, which never consults this permission, while
a request for the definitions describing those same values was refused. It could
read the data but not the schema, which is the wrong way round. The routes had
already drawn the line correctly and deliberately: reads are unflagged so Admin
can ask any site and get an empty list, and everything that changes something is
flagged. Only the permission layer had not caught up. What remains on the
resource is defining fields, which stays with the publisher.

The smaller change alongside it is one of order. Which namespace is being
written to now settles before who is asking, so a namespace nobody can define in
refuses on that ground whatever role the caller holds, rather than answering
with a missing permission that no one could have been granted. That is also
where namespace ownership belongs once an app owns a namespace of its own, since
the caller will then have to be that app rather than hold a role.

Claude-Session: https://claude.ai/code/session_01XFCbqgYHYhd9rZ5WXtqZyT

Author: rob-ghost

ghost/core/core/server/api/endpoints/member-metafields.ts

@@ -1,4 +1,5 @@
 import { actingContext, definitions } from '../../services/members-metafields';
+import { assertDefinable } from '../../services/members-metafields/namespaces';
 
 const permissionsService = require('../../services/permissions');
 
@@ -10,13 +11,30 @@ interface Frame {
   options: { namespace: string; key: string; context: unknown; [key: string]: unknown };
 }
 
-// With `permissions: true` the framework checks against the Bookshelf model named after
-// the resource. These fields have no Bookshelf model, so each handler asks the permissions
-// service directly.
+// Reading a definition needs no permission. A definition says only that the site collects
+// a shoe size, and every signed-in member is already shown the whole list, so there is
+// nothing here to keep from staff. Defining one is the publisher's own, and with
+// `permissions: true` the framework would check against the Bookshelf model named after
+// the resource — these fields have no Bookshelf model, so each writing handler asks the
+// permissions service directly.
 function canThis(frame: Frame) {
   return permissionsService.canThis(frame.options.context);
 }
 
+/**
+ * Settle the namespace before the caller.
+ *
+ * Which namespace is being written to decides whose authority applies, so it is resolved
+ * first; only once the publisher turns out to own it does a staff role become the
+ * question. The other order answers a request to define a field somewhere nobody owns
+ * with "you lack a permission", which sends the caller after a permission that would not
+ * have helped.
+ */
+async function canDefine(frame: Frame, action: (frame: Frame) => Promise<unknown>) {
+  assertDefinable(frame.options.namespace);
+  return action(frame);
+}
+
 const noCacheInvalidation = { cacheInvalidate: false };
 
 const controller = {
@@ -26,9 +44,7 @@ const controller = {
     headers: noCacheInvalidation,
     options: ['namespace', 'filter'],
     validation: { options: { namespace: { required: true } } },
-    permissions(frame: Frame) {
-      return canThis(frame).browse.member_custom_field();
-    },
+    permissions: false,
     query(frame: Frame) {
       return definitions!.browse({
         namespace: frame.options.namespace,
@@ -41,9 +57,7 @@ const controller = {
     headers: noCacheInvalidation,
     options: ['namespace', 'key'],
     validation: { options: { namespace: { required: true }, key: { required: true } } },
-    permissions(frame: Frame) {
-      return canThis(frame).read.member_custom_field(frame.options.key);
-    },
+    permissions: false,
     query(frame: Frame) {
       return definitions!.read(frame.options.namespace, frame.options.key);
     },
@@ -55,7 +69,7 @@ const controller = {
     options: ['namespace'],
     validation: { options: { namespace: { required: true } } },
     permissions(frame: Frame) {
-      return canThis(frame).add.member_custom_field();
+      return canDefine(frame, (f) => canThis(f).add.member_custom_field());
     },
     query(frame: Frame) {
       return definitions!.add(
@@ -71,7 +85,7 @@ const controller = {
     options: ['namespace'],
     validation: { options: { namespace: { required: true } } },
     permissions(frame: Frame) {
-      return canThis(frame).edit.member_custom_field();
+      return canDefine(frame, (f) => canThis(f).edit.member_custom_field());
     },
     query(frame: Frame) {
       return definitions!.reorder(
@@ -87,7 +101,7 @@ const controller = {
     options: ['namespace', 'key'],
     validation: { options: { namespace: { required: true }, key: { required: true } } },
     permissions(frame: Frame) {
-      return canThis(frame).edit.member_custom_field(frame.options.key);
+      return canDefine(frame, (f) => canThis(f).edit.member_custom_field(f.options.key));
     },
     query(frame: Frame) {
       return definitions!.edit(
@@ -105,7 +119,7 @@ const controller = {
     options: ['namespace', 'key'],
     validation: { options: { namespace: { required: true }, key: { required: true } } },
     permissions(frame: Frame) {
-      return canThis(frame).destroy.member_custom_field(frame.options.key);
+      return canDefine(frame, (f) => canThis(f).destroy.member_custom_field(f.options.key));
     },
     async query(frame: Frame) {
       await definitions!.destroy(

ghost/core/core/server/data/migrations/versions/6.63/2026-09-03-11-03-31-remove-member-custom-field-read-permissions.js

@@ -0,0 +1,30 @@
+const { combineTransactionalMigrations, createRemovePermissionMigration } = require('../../utils');
+
+const RESOURCE = 'member_custom_field';
+
+// Reading a field definition is no longer a permission. A definition says a site collects
+// a shoe size; every signed-in member already sees the whole list through Portal, so
+// gating staff behind a role protected nothing and inverted the two halves: an integration
+// holding `member: browse` received members' field values on the member payload, which
+// never consults this permission, while a request for the definitions describing those
+// values was refused.
+//
+// What remains on this resource is defining fields, which stays with the publisher.
+module.exports = combineTransactionalMigrations(
+  createRemovePermissionMigration(
+    {
+      name: 'Browse member custom fields',
+      action: 'browse',
+      object: RESOURCE,
+    },
+    ['Administrator', 'Admin Integration', 'Super Editor'],
+  ),
+  createRemovePermissionMigration(
+    {
+      name: 'Read member custom fields',
+      action: 'read',
+      object: RESOURCE,
+    },
+    ['Administrator', 'Admin Integration', 'Super Editor'],
+  ),
+);

ghost/core/core/server/data/schema/fixtures/fixtures.json

@@ -515,16 +515,6 @@
           "action_type": "destroy",
           "object_type": "label"
         },
-        {
-          "name": "Browse member custom fields",
-          "action_type": "browse",
-          "object_type": "member_custom_field"
-        },
-        {
-          "name": "Read member custom fields",
-          "action_type": "read",
-          "object_type": "member_custom_field"
-        },
         {
           "name": "Edit member custom fields",
           "action_type": "edit",
@@ -1069,7 +1059,6 @@
           "collection": "all",
           "recommendation": ["browse", "read"],
           "member": ["browse", "read", "add", "edit", "destroy"],
-          "member_custom_field": ["browse", "read"],
           "member_signin_url": "read",
           "offer": ["browse", "read"],
           "comment": "all",

ghost/core/core/server/services/members-metafields/definitions-service.ts

@@ -6,6 +6,7 @@ import { Metafield } from './models';
 import { FieldTypeSchema, type FieldType } from '@tryghost/metafield-types';
 import { CUSTOM_NAMESPACE } from '@tryghost/metafield-types/identity';
 import { metafieldCodec } from './codec';
+import { assertDefinable } from './namespaces';
 import { FIELD_STATUS, FieldStatusSchema } from './schema';
 import { ADMIN, readableFields, type Audience } from './access';
 import { activeFields, fieldByKey, inFieldOrder, type DefinitionQuery } from './queries';
@@ -133,15 +134,6 @@ export class MetafieldDefinitionsService {
     return namespace === CUSTOM_NAMESPACE;
   }
 
-  private assertDefinable(namespace: string): void {
-    if (!this.isStored(namespace)) {
-      throw new errors.ValidationError({
-        message: `Fields cannot be defined in the "${namespace}" namespace.`,
-        property: 'namespace',
-      });
-    }
-  }
-
   async browse(
     options: { namespace?: string; filter?: string } = {},
     audience: Audience = ADMIN,
@@ -192,7 +184,7 @@ export class MetafieldDefinitionsService {
    * key get distinct ones, exactly as if they had arrived as separate requests.
    */
   async add(context: RequestContext, namespace: string, input: unknown): Promise<Metafield[]> {
-    this.assertDefinable(namespace);
+    assertDefinable(namespace);
     const requestedCount = Array.isArray(input) ? input.length : 0;
 
     const parsed = AddFieldsInput.safeParse(input);
@@ -399,7 +391,7 @@ export class MetafieldDefinitionsService {
    * Returns every definition, archived included, matching what the request named.
    */
   async reorder(context: RequestContext, namespace: string, input: unknown): Promise<Metafield[]> {
-    this.assertDefinable(namespace);
+    assertDefinable(namespace);
     const parsed = ReorderInput.safeParse(input);
     if (!parsed.success) {
       const issue = parsed.error.issues[0];

ghost/core/core/server/services/members-metafields/namespaces.ts

@@ -0,0 +1,29 @@
+import errors from '@tryghost/errors';
+import { CUSTOM_NAMESPACE } from '@tryghost/metafield-types/identity';
+
+/**
+ * Who may define fields in a namespace.
+ *
+ * This is a property of the namespace, not of whoever is asking, so it is settled before
+ * any question about the caller. Asking the caller first gets the order wrong in a way
+ * that shows: a request to define a field in a namespace nobody owns would be refused for
+ * want of a permission, telling the caller to go and get one, when no permission would
+ * have helped.
+ *
+ * The publisher owns `custom` and nothing owns anything else yet. An app owning its own
+ * namespace resolves here too, and will not resolve to a permission at all: the caller
+ * has to *be* that app rather than hold a role, which is why the staff permission stays
+ * named after the publisher's fields rather than after metafields at large.
+ */
+export function definableByPublisher(namespace: string): boolean {
+  return namespace === CUSTOM_NAMESPACE;
+}
+
+export function assertDefinable(namespace: string): void {
+  if (!definableByPublisher(namespace)) {
+    throw new errors.ValidationError({
+      message: `Fields cannot be defined in the "${namespace}" namespace.`,
+      property: 'namespace',
+    });
+  }
+}

ghost/core/test/e2e-api/admin/member-custom-fields.test.ts

@@ -2230,7 +2230,7 @@ describe('Member Custom Fields Admin API', function () {
 
   describe('Authorization', function () {
     // The full role matrix is pinned in migration.test.js; here we only prove
-    // the endpoint enforces the permission — a role without it is rejected.
+    // the endpoint enforces the permission — a role without it may look but not touch.
     beforeAll(async function () {
       await agent.loginAsEditor();
     });
@@ -2239,8 +2239,9 @@ describe('Member Custom Fields Admin API', function () {
       await agent.loginAsOwner();
     });
 
-    it('forbids a role without permission from browsing', async function () {
-      await agent.get('members/metafields/custom/').expectStatus(403);
+    it('lets a role without permission read what the site collects', async function () {
+      await agent.get('members/metafields/custom/').expectStatus(200);
+      await agent.get('members/metafields/custom/company/').expectStatus(404);
     });
 
     it('forbids a role without permission from creating', async function () {
@@ -2256,6 +2257,17 @@ describe('Member Custom Fields Admin API', function () {
         .body({ members_metafields: [{ key: 'topic' }] })
         .expectStatus(403);
     });
+
+    it('answers for the namespace before it answers for the caller', async function () {
+      // Nobody can define a field here, so the namespace is the reason and the role is
+      // beside the point. Answering 403 would send this caller after a permission that
+      // would not have helped them.
+      const { body } = await agent
+        .post('members/metafields/shopify/')
+        .body({ members_metafields: [{ name: 'Topic', type: 'short_text' }] })
+        .expectStatus(422);
+      assert.match(body.errors[0].context, /shopify/);
+    });
   });
 
   // The flag governs whether a publisher can set custom fields up, not whether the rest of

ghost/core/test/integration/migrations/migration.test.js

@@ -87,7 +87,7 @@ describe('Migrations', function () {
     // Custom assertion to wrap all permissions
     function assertCompletePermissions(permissions) {
       // If you have to change this number, please add the relevant `assertHavePermission` checks below
-      assert.equal(permissions.length, 143);
+      assert.equal(permissions.length, 141);
 
       assertHavePermission(permissions, 'Export database', [
         'Administrator',
@@ -477,16 +477,6 @@ describe('Migrations', function () {
         'Super Editor',
       ]);
 
-      assertHavePermission(permissions, 'Browse member custom fields', [
-        'Administrator',
-        'Admin Integration',
-        'Super Editor',
-      ]);
-      assertHavePermission(permissions, 'Read member custom fields', [
-        'Administrator',
-        'Admin Integration',
-        'Super Editor',
-      ]);
       assertHavePermission(permissions, 'Add member custom fields', [
         'Administrator',
         'Admin Integration',

ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js

@@ -398,7 +398,7 @@ describe('Migration Fixture Utils', function () {
             const rolesAllStub = sinon.stub(models.Role, 'findAll').returns(Promise.resolve(dataMethodStub));
 
             const result = await fixtureManager.addFixturesForRelation(fixtures.relations[0]);
-            const FIXTURE_COUNT = 150;
+            const FIXTURE_COUNT = 149;
             assertExists(result);
             assert(_.isPlainObject(result));
             assert.equal(result.expected, FIXTURE_COUNT);

ghost/core/test/unit/server/data/schema/integrity.test.js

@@ -38,7 +38,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya
 describe('DB version integrity', function () {
   // Only these variables should need updating
   const currentSchemaHash = '29d413d3d965639382e8506d5db877d2';
-  const currentFixturesHash = '1727a789194847da33d68dd95301b416';
+  const currentFixturesHash = '5718e0d4eb037f159c312369e949829a';
   const currentSettingsHash = '6ea42a00cca61a1ba87f66eb6e25a78a';
   const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1';
 

ghost/core/test/utils/fixtures/fixtures.json

@@ -546,16 +546,6 @@
                     "action_type": "destroy",
                     "object_type": "label"
                 },
-                {
-                    "name": "Browse member custom fields",
-                    "action_type": "browse",
-                    "object_type": "member_custom_field"
-                },
-                {
-                    "name": "Read member custom fields",
-                    "action_type": "read",
-                    "object_type": "member_custom_field"
-                },
                 {
                     "name": "Edit member custom fields",
                     "action_type": "edit",
@@ -1261,7 +1251,6 @@
                     "recommendation": ["browse", "read"],
                     "member": ["browse", "read", "add", "edit", "destroy"],
                     "member_signin_url": "read",
-                    "member_custom_field": ["browse", "read"],
                     "offer": ["browse", "read"],
                     "comment": "all",
                     "gift_link": "manage"