Просмотр исходного кода

Show members the workshop's own currency, tax and custom fields (#412)

* Show members the workshop's own currency, tax and custom fields

Ordinary pages read the workshop's currency, units, tax and labour defaults
through the call that guards every setting, so a member without the Settings
permission was refused on each page, shown built-in defaults instead, and
logged as a refusal every time. Those values now have their own read, limited
to a list of what is safe to show any member. Everything else stays behind
the Settings permission.

A record's custom fields follow the record: reading and filling them needs
what the work order or quote itself needs, and defining them stays a setting.

* Test the built-in Member role against the pages it may open

Every permission test in the suite covered an extreme: a member with no role
refused everywhere, or a member with every permission still kept out of the
owner's screens. Nothing signed in as the role the product actually hands
somebody at the desk and used the ordinary pages that role allows, which is
where both of the bugs this branch fixes were living.

The guard is not an assertion about currency, because a workshop on USD hides
the bug and a page that substitutes a default never turns red. It is that
using those pages writes no auth.permissionDenied row: a refusal on a page the
role is meant to reach is a page asking for something the role does not carry,
whatever the missing permission turns out to be. Two checks of what the person
sees sit on top of it, and one that Settings is still refused, so the first
test cannot be made to pass by widening the role.

Also fixes the sweep fixtures in the file manager tests, which aged files
against the real clock while handing the sweep a frozen NOW. From 20 September
a file put there "eight days old" measured 6.8 days from that NOW, under the
week the sweep requires, so all five sweepOrphanFiles tests quietly became
"swept nothing" and stayed red every day after.

The seeded fixtures now take a customer and a quote from one row each, rather
than from two unordered `limit 1` queries that could answer with two different
records.

* Ask only for the dashboard cards a role may read

The dashboard asked for every card and let each action refuse the ones the
viewer's role does not carry. Nothing on screen was wrong, since a refused
card is simply not drawn, but a member opening the page made twelve queries
that could not be answered and wrote twelve refusals to the audit log, every
visit.

The page now asks once what the role may read, by the same rules the actions
apply, and skips what it cannot have. A test reads the page and fails if a
card is gated on the wrong permission or not gated at all.

* Stop refusing members on the vehicle page and the work order

The vehicle page asked for its inspections and tire hotel panels whatever the
viewer's role, and the work order read its video meeting behind the Settings
permission although adding or removing one needs only the work order's own.
A member was refused on every vehicle and every job they opened, and could
put a video call on a job without ever being shown it.

The vehicle page now asks only for the panels the role may read, and the
meeting is read with the work order. The test that guards the dashboard's
gates now covers every page that asks first.
Bernt Christian Egeland 1 неделя назад
Родитель
Сommit
fab45068ac
31 измененных файлов с 1231 добавлено и 89 удалено
  1. 319 0
      e2e/specs/auth/member-role.spec.ts
  2. 162 21
      e2e/support/db.ts
  3. 144 0
      src/__tests__/features/custom-fields/record-permissions.test.ts
  4. 129 0
      src/__tests__/features/settings/member-readable-settings.test.ts
  5. 6 1
      src/__tests__/lib/files/manager.test.ts
  6. 177 0
      src/__tests__/lib/viewer-access.test.ts
  7. 2 2
      src/app/(authenticated)/billing/page.tsx
  8. 2 2
      src/app/(authenticated)/billing/recurring/page.tsx
  9. 2 2
      src/app/(authenticated)/calendar/page.tsx
  10. 2 2
      src/app/(authenticated)/customers/[id]/page.tsx
  11. 2 2
      src/app/(authenticated)/inspections/[id]/page.tsx
  12. 2 2
      src/app/(authenticated)/inventory/[id]/page.tsx
  13. 2 2
      src/app/(authenticated)/inventory/page.tsx
  14. 2 2
      src/app/(authenticated)/labor-presets/page.tsx
  15. 32 16
      src/app/(authenticated)/page.tsx
  16. 2 2
      src/app/(authenticated)/quotes/[id]/page.tsx
  17. 2 2
      src/app/(authenticated)/quotes/page.tsx
  18. 2 2
      src/app/(authenticated)/reminders/page.tsx
  19. 2 2
      src/app/(authenticated)/reports/page.tsx
  20. 2 2
      src/app/(authenticated)/tire-hotel/[id]/page.tsx
  21. 2 2
      src/app/(authenticated)/tire-hotel/forecast/page.tsx
  22. 2 2
      src/app/(authenticated)/tire-hotel/page.tsx
  23. 15 6
      src/app/(authenticated)/vehicles/[id]/page.tsx
  24. 2 2
      src/app/(authenticated)/work-orders/page.tsx
  25. 55 9
      src/features/custom-fields/Actions/customFieldActions.ts
  26. 1 1
      src/features/custom-fields/Components/CustomFieldsForm.tsx
  27. 13 1
      src/features/integrations/Actions/integrationActions.ts
  28. 27 0
      src/features/settings/Actions/settingsActions.ts
  29. 59 0
      src/features/settings/Lib/memberReadableSettings.ts
  30. 2 2
      src/features/vehicles/Components/service-page/ServiceRecordPage.tsx
  31. 58 0
      src/lib/viewer-access.ts

+ 319 - 0
e2e/specs/auth/member-role.spec.ts

@@ -0,0 +1,319 @@
+import { expect, type Page, test } from '@playwright/test'
+import {
+  customFieldValue,
+  deleteCustomFields,
+  deletePersonWithEmail,
+  forgetWorkshopSetting,
+  membershipOf,
+  ownerOrganizationId,
+  permissionDenialsFor,
+  plantCustomField,
+  roleIdNamed,
+  seededTenantFixtures,
+  setWorkshopSetting,
+  type TenantFixtures,
+} from '../../support/db'
+import { settle } from '../../support/hydration'
+import { linkIn, waitForMail } from '../../support/mail'
+import { saveWorkOrder } from '../../support/work-order'
+
+/**
+ * The role the product hands somebody at the desk.
+ *
+ * Every other permission test in the suite covers an extreme: `roles.spec.ts`
+ * has a member with no role refused everywhere, `admin-only.spec.ts` has a
+ * member with every permission still kept out of the owner's screens, and the
+ * other fifty specs run as the owner, who skips the permission check
+ * altogether. Nothing signed in as the built-in Member role and used the
+ * ordinary pages that role is allowed to open, and two bugs lived in the gap.
+ * Both were silent: nineteen pages read the workshop's currency, units and tax
+ * through a call that needs `read:settings`, were refused, and fell back to the
+ * built-in defaults without a word, and a work order's custom fields needed the
+ * Settings permission too, so a Member saw none of them.
+ *
+ * The guard below is therefore not "assert the currency". A workshop whose
+ * currency is USD hides that bug, and a page that quietly substitutes a
+ * default never turns red. It is: using the pages this role may use writes no
+ * `auth.permissionDenied` row. A refusal on a page the role is meant to reach
+ * is, by definition, a page asking for something the role does not carry,
+ * whatever the missing permission turns out to be.
+ *
+ * Two narrow checks of what the person actually sees sit on top of it, because
+ * an audit table is not a user.
+ */
+
+test.describe.configure({ mode: 'serial' })
+
+// The colleague starts as a stranger with no session.
+test.use({ storageState: { cookies: [], origins: [] } })
+
+const stamp = Date.now()
+const MEMBER = `e2e-member-${stamp}@example.com`
+const PASSWORD = `E2e-pass-${stamp}`
+const FIELD_LABEL = `E2E Paint code ${stamp}`
+const CURRENCY_KEY = 'workshop.currencyCode'
+
+/**
+ * A currency that cannot be mistaken for the built-in fallback. `USD` is what
+ * every page substitutes when the read is refused, so a workshop on USD proves
+ * nothing; NOK prints "kr" and never "$".
+ */
+const CURRENCY = 'NOK'
+
+let organizationId = ''
+let fixtures: TenantFixtures
+let memberRoleId: string | null = null
+let currencyBefore: string | null = null
+let currencyChanged = false
+const plantedFields: string[] = []
+
+async function signInAsMember(page: Page) {
+  // The sign-in page sends anyone with a session straight on, so a fresh
+  // sign-in has to drop the old cookie first.
+  await page.context().clearCookies()
+  await page.goto('/auth/sign-in')
+  await page.locator('#email').fill(MEMBER)
+  await page.locator('#password').fill(PASSWORD)
+  await page.getByRole('button', { name: 'Sign In', exact: true }).click()
+  await page.waitForURL((url) => !url.pathname.startsWith('/auth'), { timeout: 30_000 })
+}
+
+test.afterAll(async () => {
+  // The workshop is left exactly as it was found: the currency back, the
+  // planted field and its values gone, the colleague gone. The Member role
+  // itself stays, because the product made it through its own button and it
+  // is the workshop's now.
+  if (currencyChanged) {
+    if (currencyBefore === null) await forgetWorkshopSetting(organizationId, CURRENCY_KEY)
+    else await setWorkshopSetting(organizationId, CURRENCY_KEY, currencyBefore)
+  }
+  await deleteCustomFields(plantedFields)
+  await deletePersonWithEmail(MEMBER)
+})
+
+test.describe('the built-in Member role', () => {
+  test('is invited with the workshop’s own Member role, and signs up from the mail', async ({
+    page,
+    browser,
+  }) => {
+    organizationId = await ownerOrganizationId()
+    fixtures = await seededTenantFixtures()
+
+    const owner = await browser.newContext({ storageState: 'e2e/.auth/owner.json' })
+    const ownerPage = await owner.newPage()
+    await ownerPage.goto('/settings/team')
+    await settle(ownerPage)
+
+    // The role has to be the one the app makes, not one assembled here: a
+    // permission list written in a test keeps passing after the real role
+    // changes underneath it. The team page offers to create the standard roles
+    // while either is missing, which is the path a workshop really takes.
+    const createRoles = ownerPage.getByRole('button', { name: 'Create the standard roles' })
+    if (await createRoles.isVisible().catch(() => false)) {
+      await createRoles.click()
+      await expect(ownerPage.getByText('Standard roles created')).toBeVisible({ timeout: 30_000 })
+    }
+    memberRoleId = await roleIdNamed(organizationId, 'Member')
+    expect(memberRoleId, 'the workshop has the built-in Member role').toBeTruthy()
+
+    await expect(async () => {
+      await ownerPage.getByRole('button', { name: 'Add', exact: true }).first().click()
+      await expect(ownerPage.getByText('Someone in the office')).toBeVisible({ timeout: 2_000 })
+    }).toPass({ timeout: 30_000 })
+    await ownerPage.getByText('Someone in the office').click()
+    await ownerPage.locator('#member-email').fill(MEMBER)
+
+    const dialog = ownerPage.getByRole('dialog').filter({ has: ownerPage.locator('#member-email') })
+    await dialog.getByRole('combobox').click()
+    await ownerPage.getByRole('option', { name: 'Member', exact: true }).click()
+    await ownerPage.getByRole('button', { name: 'Invite', exact: true }).click()
+    await expect(ownerPage.getByText(MEMBER).first()).toBeVisible({ timeout: 30_000 })
+    await owner.close()
+
+    const invitation = await waitForMail(MEMBER)
+    await page.goto(linkIn(invitation, /\/auth\/sign-up\?invite=/))
+    await page.locator('#name').fill('E2E Desk Colleague')
+    await page.locator('#email').fill(MEMBER)
+    await page.locator('#password').fill(PASSWORD)
+    await page.locator('#terms').click()
+    await page.getByRole('button', { name: /create account/i }).click()
+
+    // Inside the app, not at the door and not in onboarding: they joined a
+    // workshop that already exists.
+    await page.waitForURL((url) => !/^\/(auth|onboarding)/.test(url.pathname), { timeout: 30_000 })
+    await expect(page.getByRole('heading', { name: 'No access yet' })).toHaveCount(0)
+
+    // And they carry the built-in role, not an accidental blank membership.
+    const membership = await membershipOf(MEMBER, organizationId)
+    expect(membership.roleId).toBe(memberRoleId)
+    expect(membership.role).toBe('member')
+  })
+
+  test('is refused nothing on any page the role is meant to open', async ({ page }) => {
+    await signInAsMember(page)
+
+    /**
+     * Every page the Member role carries a permission for. Deliberately absent:
+     * settings, billing, reports, inspections and the tire hotel, which this
+     * role is meant to be refused and which the last test pins.
+     */
+    const pages: { url: string; shows: (page: Page) => Promise<void> }[] = [
+      { url: '/', shows: (p) => expect(p.getByText('Dashboard').first()).toBeVisible() },
+      {
+        url: '/work-orders',
+        shows: (p) => expect(p.getByText('All Work Orders').first()).toBeVisible(),
+      },
+      { url: '/vehicles', shows: (p) => expect(p.getByText('All Vehicles').first()).toBeVisible() },
+      {
+        url: `/vehicles/${fixtures.vehicleId}`,
+        // A page that rendered its own record, not an empty shell.
+        shows: (p) => expect(p.getByText(fixtures.vehiclePlate).first()).toBeVisible(),
+      },
+      {
+        url: `/vehicles/${fixtures.vehicleId}/service/${fixtures.serviceRecordId}`,
+        shows: (p) => expect(p.getByTestId('totals')).toBeVisible(),
+      },
+      {
+        url: '/customers',
+        shows: (p) => expect(p.getByText('All Customers').first()).toBeVisible(),
+      },
+      {
+        url: `/customers/${fixtures.customerId}`,
+        shows: (p) => expect(p.getByText(fixtures.customerName).first()).toBeVisible(),
+      },
+      { url: '/quotes', shows: (p) => expect(p.getByText('All Quotes').first()).toBeVisible() },
+      {
+        url: `/quotes/${fixtures.quoteId}`,
+        shows: (p) => expect(p.getByText(fixtures.quoteNumber).first()).toBeVisible(),
+      },
+      { url: '/inventory', shows: (p) => expect(p.getByText('All Parts').first()).toBeVisible() },
+      {
+        url: '/labor-presets',
+        shows: (p) => expect(p.getByText('Labor Presets').first()).toBeVisible(),
+      },
+      { url: '/work-board', shows: (p) => expect(p.getByText('Work Board').first()).toBeVisible() },
+      {
+        url: '/calendar',
+        shows: (p) => expect(p.getByRole('button', { name: 'Today' }).first()).toBeVisible(),
+      },
+    ]
+
+    // A second either side: the audit write is fire-and-forget, and the clock
+    // here is not the clock the row is stamped with.
+    const since = new Date(Date.now() - 1_000)
+    const refusals: string[] = []
+    let counted = 0
+
+    for (const { url, shows } of pages) {
+      await page.goto(url)
+      await settle(page)
+
+      // The refusal screen, an error boundary or an empty shell all mean the
+      // page did not open, and would leave the audit log innocently empty.
+      await expect(
+        page.getByRole('heading', { name: 'No access yet' }),
+        `${url} opens`
+      ).toHaveCount(0)
+      await shows(page)
+
+      await page.waitForTimeout(2_000)
+      const all = await permissionDenialsFor(MEMBER, since)
+      // Attributed to the page that was open when they appeared, which is what
+      // makes a failure here readable rather than a list of bare messages.
+      for (const message of all.slice(counted)) refusals.push(`${url} -> ${message}`)
+      counted = all.length
+    }
+
+    expect(
+      refusals,
+      'a page this role may open asked for a permission the role does not carry'
+    ).toEqual([])
+  })
+
+  test('sees the workshop’s own currency, not the built-in default', async ({ page, browser }) => {
+    currencyBefore = await setWorkshopSetting(organizationId, CURRENCY_KEY, CURRENCY)
+    currencyChanged = true
+
+    const jobUrl = `/vehicles/${fixtures.vehicleId}/service/${fixtures.serviceRecordId}`
+
+    // What the owner sees is the workshop's own answer, whatever the app's
+    // formatting happens to be. The Member has to see the same thing rather
+    // than a shape this test decided on.
+    const owner = await browser.newContext({ storageState: 'e2e/.auth/owner.json' })
+    const ownerPage = await owner.newPage()
+    await ownerPage.goto(jobUrl)
+    await settle(ownerPage)
+    const ownersTotals = (await ownerPage.getByTestId('totals').innerText()).trim()
+    await owner.close()
+
+    expect(ownersTotals, 'the workshop is on NOK for this test').toContain('kr')
+
+    await signInAsMember(page)
+    await page.goto(jobUrl)
+    await settle(page)
+    const membersTotals = (await page.getByTestId('totals').innerText()).trim()
+
+    expect(membersTotals).toBe(ownersTotals)
+    // Said plainly as well, because the comparison above would pass if both
+    // sides broke together.
+    expect(membersTotals).toContain('kr')
+    expect(membersTotals).not.toContain('$')
+    expect(membersTotals).not.toContain('USD')
+
+    // And on a list, which reads the currency through the same call.
+    await page.goto('/work-orders')
+    await settle(page)
+    const list = await page.getByRole('table').first().innerText()
+    expect(list).not.toContain('$')
+  })
+
+  test('sees a custom field on a work order, fills it in, and it stays', async ({ page }) => {
+    const fieldId = await plantCustomField(organizationId, 'service_record', FIELD_LABEL)
+    plantedFields.push(fieldId)
+
+    await signInAsMember(page)
+    const jobUrl = `/vehicles/${fixtures.vehicleId}/service/${fixtures.serviceRecordId}`
+    await page.goto(jobUrl)
+    await settle(page)
+
+    // The label is the whole point: a Member used to see no custom fields at
+    // all, so the workshop's own question never reached the person answering it.
+    const field = page.getByTestId(`custom-field-${fieldId}`)
+    await expect(field).toBeVisible()
+    await expect(field.getByText(FIELD_LABEL)).toBeVisible()
+
+    const value = `LY9C ${stamp}`
+    const input = field.getByRole('textbox')
+    await expect(async () => {
+      await input.fill(value)
+      await expect(input).toHaveValue(value)
+    }).toPass({ timeout: 30_000 })
+
+    await saveWorkOrder(page)
+
+    // Read back from the database as well as the page: the save used to be
+    // refused outright, and an input keeps what was typed into it either way.
+    await expect(async () => {
+      expect(await customFieldValue(fieldId, fixtures.serviceRecordId)).toBe(value)
+    }).toPass({ timeout: 30_000 })
+
+    await page.goto(jobUrl)
+    await settle(page)
+    await expect(page.getByTestId(`custom-field-${fieldId}`).getByRole('textbox')).toHaveValue(
+      value
+    )
+  })
+
+  test('is still kept out of the workshop’s settings', async ({ page }) => {
+    // The other half of the rule, and the guard against widening the role to
+    // make the first test pass: this table holds payment secrets, API keys and
+    // the licence token, and none of it is a Member's business.
+    await signInAsMember(page)
+
+    for (const url of ['/settings', '/settings/company']) {
+      await page.goto(url)
+      await settle(page)
+      await expect(page, `${url} is refused`).not.toHaveURL(new RegExp(`${url}$`))
+    }
+  })
+})

+ 162 - 21
e2e/support/db.ts

@@ -126,16 +126,36 @@ export interface TenantFixtures {
   quoteNumber: string
 }
 
+/** A customer and a quote, each taken whole so its id and its words agree. */
+async function pairs(
+  db: Client,
+  organizationId: string
+): Promise<Pick<TenantFixtures, 'customerId' | 'customerName' | 'quoteId' | 'quoteNumber'>> {
+  const customer = await db.query<{ id: string; name: string }>(
+    `select id, name from customers where "organizationId" = $1 order by "createdAt", id limit 1`,
+    [organizationId]
+  )
+  if (!customer.rows[0]) throw new Error('the seeded workshop has no customer')
+
+  const quote = await db.query<{ id: string; quoteNumber: string }>(
+    `select id, "quoteNumber" from quotes
+      where "organizationId" = $1 and "quoteNumber" is not null and "quoteNumber" <> ''
+      order by "createdAt", id limit 1`,
+    [organizationId]
+  )
+  if (!quote.rows[0]) throw new Error('the seeded workshop has no numbered quote')
+
+  return {
+    customerId: customer.rows[0].id,
+    customerName: customer.rows[0].name,
+    quoteId: quote.rows[0].id,
+    quoteNumber: quote.rows[0].quoteNumber,
+  }
+}
+
 export async function seededTenantFixtures(): Promise<TenantFixtures> {
   const organizationId = await ownerOrganizationId()
   return withDb(async (db) => {
-    const one = async (sql: string): Promise<string> => {
-      const result = await db.query<{ id: string }>(sql, [organizationId])
-      const id = result.rows[0]?.id
-      if (!id) throw new Error(`the seeded workshop has nothing for: ${sql}`)
-      return id
-    }
-
     /**
      * A vehicle and one of its own jobs, from one row.
      *
@@ -170,20 +190,12 @@ export async function seededTenantFixtures(): Promise<TenantFixtures> {
       vehicleId: job.vehicleId,
       serviceRecordId: job.serviceRecordId,
       vehiclePlate: job.licensePlate,
-      customerId: await one(`select id from customers where "organizationId" = $1 limit 1`),
-      quoteId: await one(
-        `select id from quotes
-          where "organizationId" = $1 and "quoteNumber" is not null and "quoteNumber" <> ''
-          limit 1`
-      ),
-      customerName: await one(
-        `select name as id from customers where "organizationId" = $1 limit 1`
-      ),
-      quoteNumber: await one(
-        `select "quoteNumber" as id from quotes
-          where "organizationId" = $1 and "quoteNumber" is not null and "quoteNumber" <> ''
-          limit 1`
-      ),
+      // Id and words from one row each, for the same reason the vehicle and
+      // its job come from one row: `limit 1` without an order is not a
+      // promise, and two queries for "a customer" can answer with two
+      // different customers. That way round the id opens one record and the
+      // name that is searched for on it belongs to another.
+      ...(await pairs(db, organizationId)),
     }
   })
 }
@@ -1455,3 +1467,132 @@ export async function removePlantedVehicle(planted: PlantedVehicle): Promise<voi
     await db.query('delete from vehicles where id = $1', [planted.vehicleId])
   })
 }
+
+/**
+ * Every permission refusal logged for one person since a moment in time.
+ *
+ * `withAuth` writes an `auth.permissionDenied` row whenever a role is short of
+ * what an action asked for, which makes the audit log the one place that says
+ * what a page quietly wanted and did not get. A refusal on a page the role is
+ * meant to reach is, by definition, a bug: the page renders anyway, falls back
+ * to a built-in default, and says nothing about it.
+ *
+ * The write is fire-and-forget, so give it a moment to land before counting.
+ */
+export async function permissionDenialsFor(email: string, since: Date): Promise<string[]> {
+  return withDb(async (db) => {
+    const result = await db.query<{ message: string }>(
+      `select coalesce(a.message, a.action) as message
+         from audit_logs a
+         join users u on u.id = a."userId"
+        where lower(u.email) = lower($1)
+          and a.action = 'auth.permissionDenied'
+          and a.timestamp >= $2
+        order by a.timestamp`,
+      [email, since]
+    )
+    return result.rows.map((row) => row.message)
+  })
+}
+
+/**
+ * Sets one of a workshop's settings directly, returning what was there before
+ * (null when the key had never been saved), so a spec can put it back.
+ *
+ * The row needs an owner: `app_settings.userId` is not nullable, so a key the
+ * workshop has never saved is attributed to whoever owns the workshop.
+ */
+export async function setWorkshopSetting(
+  organizationId: string,
+  key: string,
+  value: string
+): Promise<string | null> {
+  return withDb(async (db) => {
+    const before = await db.query<{ value: string }>(
+      `select value from app_settings where "organizationId" = $1 and key = $2`,
+      [organizationId, key]
+    )
+    await db.query(
+      `insert into app_settings (id, key, value, "userId", "organizationId")
+       values (gen_random_uuid()::text, $2, $3,
+               (select "userId" from organization_members
+                 where "organizationId" = $1 and role = 'owner' limit 1),
+               $1)
+       on conflict ("organizationId", key) do update set value = excluded.value`,
+      [organizationId, key, value]
+    )
+    return before.rows[0]?.value ?? null
+  })
+}
+
+/**
+ * A custom field on one kind of record, made here so the spec owns it.
+ *
+ * `name` is the key the app stores values under and `label` is what a person
+ * reads, so both are stamped: the point of the field is that its label shows
+ * up on the record, and a name left over from an earlier run would collide on
+ * `(organizationId, name, entityType)`.
+ */
+export async function plantCustomField(
+  organizationId: string,
+  entityType: 'service_record' | 'quote',
+  name: string
+): Promise<string> {
+  return withDb(async (db) => {
+    const result = await db.query<{ id: string }>(
+      `insert into custom_field_definitions
+         (id, name, label, "fieldType", "entityType", "sortOrder", "isActive",
+          "createdAt", "updatedAt", "userId", "organizationId")
+       values (gen_random_uuid()::text, $2, $2, 'text', $3, 0, true, now(), now(),
+               (select "userId" from organization_members
+                 where "organizationId" = $1 and role = 'owner' limit 1),
+               $1)
+       returning id`,
+      [organizationId, name, entityType]
+    )
+    return result.rows[0].id
+  })
+}
+
+/** Removes planted field definitions, and the values written into them. */
+export async function deleteCustomFields(ids: string[]): Promise<void> {
+  if (ids.length === 0) return
+  await withDb(async (db) => {
+    await db.query(`delete from custom_field_values where "fieldId" = any($1::text[])`, [ids])
+    await db.query(`delete from custom_field_definitions where id = any($1::text[])`, [ids])
+  })
+}
+
+/**
+ * The value stored in one custom field for one record, or null.
+ *
+ * Read back to prove a save from the browser reached the database rather than
+ * only the input it was typed into.
+ */
+export async function customFieldValue(fieldId: string, entityId: string): Promise<string | null> {
+  return withDb(async (db) => {
+    const result = await db.query<{ value: string }>(
+      `select value from custom_field_values where "fieldId" = $1 and "entityId" = $2`,
+      [fieldId, entityId]
+    )
+    return result.rows[0]?.value ?? null
+  })
+}
+
+/**
+ * A workshop's own role by name, as `createDefaultRoles` made it.
+ *
+ * The built-in Member role is what the product really hands somebody at the
+ * desk, so a spec about that role has to use that row rather than build an
+ * equivalent permission list by hand: a list assembled in the test would keep
+ * passing after the real role changed underneath it.
+ */
+export async function roleIdNamed(organizationId: string, name: string): Promise<string | null> {
+  return withDb(async (db) => {
+    const result = await db.query<{ id: string }>(
+      `select id from roles where "organizationId" = $1 and name = $2 limit 1`,
+      [organizationId, name]
+    )
+    return result.rows[0]?.id ?? null
+  })
+}

+ 144 - 0
src/__tests__/features/custom-fields/record-permissions.test.ts

@@ -0,0 +1,144 @@
+/**
+ * @vitest-environment node
+ *
+ * A record's custom fields follow the record, not the Settings permission.
+ *
+ * Defining a field is a setting. Filling one in is part of the work order or
+ * the quote, like its title. All of it used to need Settings, so a Member who
+ * could edit a work order saw none of the workshop's custom fields on it,
+ * could not have saved one, and was refused on every page load.
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const db = vi.hoisted(() => ({
+  customFieldDefinition: { findMany: vi.fn() },
+  customFieldValue: { findMany: vi.fn(), upsert: vi.fn() },
+  serviceRecord: { count: vi.fn() },
+  quote: { count: vi.fn() },
+  $transaction: vi.fn(),
+}))
+vi.mock('@/lib/db', () => ({ db }))
+
+const seenOptions = vi.hoisted(() => [] as { requiredPermissions?: unknown }[])
+vi.mock('@/lib/with-auth', () => ({
+  withAuth: async (
+    action: (ctx: { userId: string; organizationId: string }) => unknown,
+    options: { requiredPermissions?: unknown } = {}
+  ) => {
+    seenOptions.push(options)
+    try {
+      return { success: true, data: await action({ userId: 'u-1', organizationId: 'org-1' }) }
+    } catch (error) {
+      return { success: false, error: (error as Error).message }
+    }
+  },
+}))
+vi.mock('next/cache', () => ({ revalidatePath: vi.fn() }))
+vi.mock('@/lib/features', () => ({ requireFeature: vi.fn() }))
+
+import {
+  getCustomFieldValues,
+  getFieldDefinitions,
+  saveCustomFieldValues,
+} from '@/features/custom-fields/Actions/customFieldActions'
+import { PermissionAction, PermissionSubject } from '@/lib/permissions'
+
+const needed = () => seenOptions.at(-1)?.requiredPermissions
+
+beforeEach(() => {
+  seenOptions.length = 0
+  db.customFieldDefinition.findMany.mockReset().mockResolvedValue([{ id: 'f-1', defaultValue: '' }])
+  db.customFieldValue.findMany.mockReset().mockResolvedValue([])
+  db.customFieldValue.upsert.mockReset().mockReturnValue('upsert')
+  db.serviceRecord.count.mockReset().mockResolvedValue(1)
+  db.quote.count.mockReset().mockResolvedValue(1)
+  db.$transaction.mockReset().mockResolvedValue([])
+})
+
+describe("reading a record's custom fields", () => {
+  it('needs what reading the work order needs', async () => {
+    const result = await getCustomFieldValues('job-1', 'service_record')
+    expect(result.success).toBe(true)
+    expect(needed()).toEqual([
+      { action: PermissionAction.READ, subject: PermissionSubject.SERVICES },
+    ])
+  })
+
+  it('needs what reading the quote needs', async () => {
+    await getCustomFieldValues('q-1', 'quote')
+    expect(needed()).toEqual([{ action: PermissionAction.READ, subject: PermissionSubject.QUOTES }])
+  })
+
+  it("draws the form for one kind of record on that record's permission", async () => {
+    await getFieldDefinitions('service_record')
+    expect(needed()).toEqual([
+      { action: PermissionAction.READ, subject: PermissionSubject.SERVICES },
+    ])
+  })
+
+  it('keeps the whole list, which is the settings screen, behind Settings', async () => {
+    await getFieldDefinitions()
+    expect(needed()).toEqual([
+      { action: PermissionAction.READ, subject: PermissionSubject.SETTINGS },
+    ])
+  })
+})
+
+describe('filling them in', () => {
+  it('needs what saving the work order needs', async () => {
+    const result = await saveCustomFieldValues('job-1', 'service_record', { 'f-1': 'Blue' })
+    expect(result.success).toBe(true)
+    expect(needed()).toEqual([
+      { action: PermissionAction.UPDATE, subject: PermissionSubject.SERVICES },
+    ])
+    expect(db.customFieldValue.upsert).toHaveBeenCalledTimes(1)
+  })
+
+  it('needs what saving the quote needs', async () => {
+    await saveCustomFieldValues('q-1', 'quote', { 'f-1': 'Blue' })
+    expect(needed()).toEqual([
+      { action: PermissionAction.UPDATE, subject: PermissionSubject.QUOTES },
+    ])
+  })
+})
+
+describe("a record that is not this workshop's", () => {
+  it('cannot be read', async () => {
+    db.serviceRecord.count.mockResolvedValue(0)
+
+    const result = await getCustomFieldValues('somebody-elses-job', 'service_record')
+
+    expect(result).toEqual({ success: false, error: 'Record not found' })
+    expect(db.serviceRecord.count).toHaveBeenCalledWith({
+      where: { id: 'somebody-elses-job', organizationId: 'org-1' },
+    })
+    expect(db.customFieldValue.findMany).not.toHaveBeenCalled()
+  })
+
+  it('cannot have values attached to it', async () => {
+    db.quote.count.mockResolvedValue(0)
+
+    const result = await saveCustomFieldValues('somebody-elses-quote', 'quote', { 'f-1': 'x' })
+
+    expect(result).toEqual({ success: false, error: 'Record not found' })
+    expect(db.customFieldValue.upsert).not.toHaveBeenCalled()
+    expect(db.$transaction).not.toHaveBeenCalled()
+  })
+})
+
+describe('a kind of record nobody defined', () => {
+  it('falls back to the strictest permission, and is refused anyway', async () => {
+    const read = await getCustomFieldValues('x', 'customer')
+    expect(needed()).toEqual([
+      { action: PermissionAction.READ, subject: PermissionSubject.SETTINGS },
+    ])
+    expect(read).toEqual({ success: false, error: 'Unknown record type' })
+
+    const saved = await saveCustomFieldValues('x', 'customer', { 'f-1': 'x' })
+    expect(needed()).toEqual([
+      { action: PermissionAction.UPDATE, subject: PermissionSubject.SETTINGS },
+    ])
+    expect(saved).toEqual({ success: false, error: 'Unknown record type' })
+    expect(db.customFieldValue.upsert).not.toHaveBeenCalled()
+  })
+})

+ 129 - 0
src/__tests__/features/settings/member-readable-settings.test.ts

@@ -0,0 +1,129 @@
+/**
+ * @vitest-environment node
+ *
+ * What a Member may read without the Settings permission.
+ *
+ * A Member was refused `read:settings` on every ordinary page, because every
+ * page asked for the workshop's currency through the same call that guards
+ * its payment secrets. The page fell back to built-in defaults without saying
+ * so, and each refusal wrote an audit row. The fix is a second, narrow read;
+ * these tests are what keeps it narrow.
+ */
+import { readdirSync, readFileSync, statSync } from 'node:fs'
+import { join } from 'node:path'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const findMany = vi.hoisted(() => vi.fn())
+vi.mock('@/lib/db', () => ({ db: { appSetting: { findMany } } }))
+
+const seenOptions = vi.hoisted(() => [] as unknown[])
+vi.mock('@/lib/with-auth', () => ({
+  withAuth: async (
+    action: (ctx: { userId: string; organizationId: string }) => unknown,
+    options?: unknown
+  ) => {
+    seenOptions.push(options)
+    return { success: true, data: await action({ userId: 'u-1', organizationId: 'org-1' }) }
+  },
+}))
+vi.mock('next/cache', () => ({ revalidatePath: vi.fn() }))
+
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
+import { MEMBER_READABLE_SETTINGS } from '@/features/settings/Lib/memberReadableSettings'
+import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
+
+beforeEach(() => {
+  findMany.mockReset().mockResolvedValue([])
+  seenOptions.length = 0
+})
+
+describe('the list of what a member may read', () => {
+  it('holds nothing that looks like a secret', () => {
+    const secretish =
+      /secret|password|passwd|token|api[_.-]?key|private|webhook|credential|smtp|licen[sc]e/i
+    const names = Object.entries(SETTING_KEYS)
+      .filter(([, value]) => MEMBER_READABLE_SETTINGS.has(value))
+      .flatMap(([name, value]) => [name, value])
+    expect(names.filter((name) => secretish.test(name))).toEqual([])
+  })
+
+  it('leaves every secret in the table out', () => {
+    for (const secret of [
+      SETTING_KEYS.PAYMENT_STRIPE_SECRET_KEY,
+      SETTING_KEYS.PAYMENT_STRIPE_WEBHOOK_SECRET,
+      SETTING_KEYS.PAYMENT_VIPPS_CLIENT_SECRET,
+      SETTING_KEYS.PAYMENT_PAYPAL_CLIENT_SECRET,
+      SETTING_KEYS.LICENSE_TOKEN,
+      SETTING_KEYS.AI_API_KEY,
+    ]) {
+      expect(MEMBER_READABLE_SETTINGS.has(secret)).toBe(false)
+    }
+  })
+
+  it('covers every key an ordinary page asks for', () => {
+    // A page that asks for a key that is not listed gets it back missing and
+    // renders a default, which is the bug this replaced. So the pages are
+    // read, and what they ask for is checked against the list.
+    const asked = new Map<string, string>()
+    const walk = (dir: string) => {
+      for (const entry of readdirSync(dir)) {
+        const path = join(dir, entry)
+        if (statSync(path).isDirectory()) {
+          if (entry !== '__tests__' && entry !== 'generated') walk(path)
+        } else if (/\.tsx?$/.test(entry)) {
+          const source = readFileSync(path, 'utf8')
+          for (const call of source.matchAll(/getDisplaySettings\(\s*\[([\s\S]*?)\]\s*\)/g)) {
+            for (const key of call[1].matchAll(/SETTING_KEYS\.([A-Z0-9_]+)/g))
+              asked.set(key[1], path)
+          }
+        }
+      }
+    }
+    walk(join(process.cwd(), 'src'))
+
+    expect(asked.size).toBeGreaterThan(10)
+    const unlisted = [...asked].filter(
+      ([name]) => !MEMBER_READABLE_SETTINGS.has(SETTING_KEYS[name as keyof typeof SETTING_KEYS])
+    )
+    expect(unlisted).toEqual([])
+  })
+})
+
+describe('reading them', () => {
+  it('needs membership of the workshop and nothing more', async () => {
+    await getDisplaySettings([SETTING_KEYS.CURRENCY_CODE])
+    expect(seenOptions).toEqual([undefined])
+  })
+
+  it("returns the workshop's own values, read for this workshop only", async () => {
+    findMany.mockResolvedValue([{ key: SETTING_KEYS.CURRENCY_CODE, value: 'NOK' }])
+
+    const result = await getDisplaySettings([SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.UNIT_SYSTEM])
+
+    expect(result).toEqual({ success: true, data: { [SETTING_KEYS.CURRENCY_CODE]: 'NOK' } })
+    expect(findMany).toHaveBeenCalledWith({
+      where: {
+        organizationId: 'org-1',
+        key: { in: [SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.UNIT_SYSTEM] },
+      },
+      select: { key: true, value: true },
+    })
+  })
+
+  it('never asks the database for a key that is not on the list', async () => {
+    await getDisplaySettings([
+      SETTING_KEYS.CURRENCY_CODE,
+      SETTING_KEYS.PAYMENT_STRIPE_SECRET_KEY,
+      SETTING_KEYS.AI_API_KEY,
+    ])
+
+    expect(findMany).toHaveBeenCalledTimes(1)
+    expect(findMany.mock.calls[0][0].where.key.in).toEqual([SETTING_KEYS.CURRENCY_CODE])
+  })
+
+  it('reads nothing at all when every key asked for is private', async () => {
+    const result = await getDisplaySettings([SETTING_KEYS.PAYMENT_STRIPE_SECRET_KEY])
+    expect(result).toEqual({ success: true, data: {} })
+    expect(findMany).not.toHaveBeenCalled()
+  })
+})

+ 6 - 1
src/__tests__/lib/files/manager.test.ts

@@ -94,7 +94,12 @@ async function put(root: string, org: string, folder: string, name: string, ageH
   const file = path.join(dir, name)
   await writeFile(file, 'bytes')
   if (ageHours) {
-    const when = new Date(Date.now() - ageHours * 3600_000)
+    // Aged against NOW, not the real clock. The sweep is handed NOW, so a file
+    // aged from today drifts a day further from it every day that passes: on
+    // 20 September a file put here "eight days old" was 6.8 days old as the
+    // sweep measured it, under the week it requires, and every sweep test
+    // quietly became "swept nothing".
+    const when = new Date(NOW - ageHours * 3600_000)
     await utimes(file, when, when)
   }
   return file

+ 177 - 0
src/__tests__/lib/viewer-access.test.ts

@@ -0,0 +1,177 @@
+/**
+ * @vitest-environment node
+ *
+ * Asking what a role may read before asking for it.
+ *
+ * A Member opening the dashboard was refused twelve times per visit, because
+ * the page asked for every card and let `withAuth` turn down the ones the role
+ * does not carry. Nothing on screen was wrong, so nothing looked broken: the
+ * cost was twelve wasted queries and twelve "permission denied" rows in the
+ * audit log, every time, for every member. The page now asks here first.
+ *
+ * Two things have to stay true. This must agree with `withAuth` about who
+ * reads what, or it hides a card somebody is entitled to. And the dashboard's
+ * gates must name the permission each action really needs, or the refusals
+ * come straight back.
+ */
+import { readFileSync, readdirSync, statSync } from 'node:fs'
+import { join } from 'node:path'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const getAuthContext = vi.hoisted(() => vi.fn())
+const getCachedMembership = vi.hoisted(() => vi.fn())
+vi.mock('@/lib/get-auth-context', () => ({ getAuthContext }))
+vi.mock('@/lib/cached-session', () => ({ getCachedMembership }))
+
+import { MEMBER_PERMISSIONS } from '@/features/team/Lib/technicianRole'
+import { PermissionSubject } from '@/lib/permissions'
+import { getViewerAccess, readIfAllowed } from '@/lib/viewer-access'
+
+const person = (over: Record<string, unknown> = {}) => ({
+  userId: 'u-1',
+  organizationId: 'org-1',
+  role: 'member',
+  isSuperAdmin: false,
+  isAdmin: false,
+  ...over,
+})
+const role = (...granted: [string, string][]) => ({
+  customRole: { permissions: granted.map(([action, subject]) => ({ action, subject })) },
+})
+
+beforeEach(() => {
+  getAuthContext.mockReset()
+  getCachedMembership.mockReset().mockResolvedValue(null)
+})
+
+describe('who reads what', () => {
+  it('is everything for an owner, an admin, an admin role and a super admin', async () => {
+    // `isAdmin` on the auth context is exactly the set `withAuth` waves through.
+    getAuthContext.mockResolvedValue(person({ isAdmin: true }))
+    const access = await getViewerAccess()
+    expect(access.reads(PermissionSubject.TIRE_HOTEL)).toBe(true)
+    expect(access.reads(PermissionSubject.SETTINGS)).toBe(true)
+    // Their role is never even looked up.
+    expect(getCachedMembership).not.toHaveBeenCalled()
+  })
+
+  it('is what the role was given, for anybody else', async () => {
+    getAuthContext.mockResolvedValue(person())
+    getCachedMembership.mockResolvedValue(role(['read', 'vehicles'], ['update', 'inspections']))
+    const access = await getViewerAccess()
+
+    expect(access.reads(PermissionSubject.VEHICLES)).toBe(true)
+    // Being allowed to change something is not being allowed to read it here:
+    // the action asks for `read`, so that is what is checked.
+    expect(access.reads(PermissionSubject.INSPECTIONS)).toBe(false)
+    expect(access.reads(PermissionSubject.TIRE_HOTEL)).toBe(false)
+  })
+
+  it('is nothing for a member with no role, and for nobody at all', async () => {
+    getAuthContext.mockResolvedValue(person())
+    expect((await getViewerAccess()).reads(PermissionSubject.DASHBOARD)).toBe(false)
+
+    getAuthContext.mockResolvedValue(null)
+    expect((await getViewerAccess()).reads(PermissionSubject.DASHBOARD)).toBe(false)
+  })
+})
+
+describe('a card the role may not read', () => {
+  it('is not asked for, and answers the way a refusal does', async () => {
+    const call = vi.fn(async () => ({ success: true, data: ['a job'] }))
+    const result = await readIfAllowed({ reads: () => false }, PermissionSubject.TIRE_HOTEL, call)
+
+    expect(call).not.toHaveBeenCalled()
+    expect(result).toEqual({ success: false, error: 'Insufficient permissions', forbidden: true })
+  })
+
+  it('is asked for as usual when the role may read it', async () => {
+    const call = vi.fn(async () => ({ success: true, data: ['a job'] }))
+    const result = await readIfAllowed({ reads: () => true }, PermissionSubject.TIRE_HOTEL, call)
+    expect(result).toEqual({ success: true, data: ['a job'] })
+  })
+})
+
+describe('every page that asks first', () => {
+  const root = join(process.cwd(), 'src')
+
+  const sources: string[] = []
+  const pages: { path: string; source: string }[] = []
+  const walk = (dir: string, into: (path: string, source: string) => void) => {
+    for (const entry of readdirSync(dir)) {
+      const path = join(dir, entry)
+      if (statSync(path).isDirectory()) walk(path, into)
+      else if (/\.tsx?$/.test(entry)) into(path, readFileSync(path, 'utf8'))
+    }
+  }
+  walk(join(root, 'features'), (_path, source) => sources.push(source))
+  walk(join(root, 'app'), (path, source) => {
+    if (source.includes('readIfAllowed(access,'))
+      pages.push({ path: path.slice(root.length + 1), source })
+  })
+
+  /** The subject an action must be allowed to READ, or null when it asks for none. */
+  function subjectNeededBy(action: string): string | null {
+    for (const source of sources) {
+      const at = source.search(new RegExp(`export (async )?function ${action}\\b`))
+      if (at < 0) continue
+      // Up to the end of this function, not the next export: a helper that
+      // follows it may need something this action does not.
+      const end = source.indexOf('\n}\n', at)
+      const body = source.slice(at, end < 0 ? undefined : end)
+      const inline = body.match(
+        /PermissionAction\.READ,\s*subject:\s*PermissionSubject\.([A-Z_]+)/
+      )?.[1]
+      if (inline) return inline
+      // Some files name the permission once and refer to it:
+      // `requiredPermissions: READ`, with `const READ = [{ … }]` elsewhere.
+      const named = body.match(/requiredPermissions:\s*([A-Za-z_]\w*)\b/)?.[1]
+      if (!named) return null
+      return (
+        source.match(
+          new RegExp(
+            `const ${named} = \\[\\s*\\{\\s*action:\\s*PermissionAction\\.READ,\\s*subject:\\s*PermissionSubject\\.([A-Z_]+)`
+          )
+        )?.[1] ?? null
+      )
+    }
+    throw new Error(`${action} was not found under src/features`)
+  }
+
+  it('covers the dashboard and the vehicle page', () => {
+    expect(pages.map((page) => page.path).sort()).toEqual([
+      'app/(authenticated)/page.tsx',
+      'app/(authenticated)/vehicles/[id]/page.tsx',
+    ])
+  })
+
+  it('gates each call on the permission its action really needs', () => {
+    const wrong = pages.flatMap(({ path, source }) =>
+      [...source.matchAll(/readIfAllowed\(access, S\.([A-Z_]+), \(\) =>\s*(\w+)\(/g)]
+        .filter(([, subject, action]) => subjectNeededBy(action) !== subject)
+        .map(([, subject, action]) => `${path}: ${action} is gated on ${subject}`)
+    )
+    expect(wrong).toEqual([])
+  })
+
+  it('asks for nothing outside what a Member reads without asking first', () => {
+    // The built-in Member role reads these and nothing else. A call that needs
+    // any other subject is one a Member is refused, so it has to be gated.
+    const memberReads = new Set<string>(
+      MEMBER_PERMISSIONS.filter((p) => p.action === 'read').map((p) => p.subject)
+    )
+    const ungated = pages.flatMap(({ path, source }) => {
+      // Up to the line that closes the list, not the first `])`, which can
+      // belong to a call inside it.
+      const start = source.indexOf('await Promise.all([')
+      const block = source.slice(start, source.indexOf('\n  ])', start))
+      return [...block.matchAll(/^\s{4}(get\w+)\(/gm)]
+        .map(([, action]) => ({ action, subject: subjectNeededBy(action) }))
+        .filter(
+          ({ subject }) => subject !== null && !memberReads.has(String(subject).toLowerCase())
+        )
+        .map(({ action, subject }) => `${path}: ${action} needs ${subject} and is not gated`)
+    })
+    expect(ungated).toEqual([])
+  })
+})

+ 2 - 2
src/app/(authenticated)/billing/page.tsx

@@ -1,6 +1,6 @@
 import { resolveListSort } from '@/lib/list-sort-preference.server'
 import { getBillingHistory } from '@/features/billing/Actions/billingActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { ListPage } from '@/components/list-page'
@@ -41,7 +41,7 @@ export default async function BillingPage({
       sortBy,
       sortOrder,
     }),
-    getSettings([SETTING_KEYS.CURRENCY_CODE]),
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE]),
   ])
 
   const settings = settingsResult.success && settingsResult.data ? settingsResult.data : {}

+ 2 - 2
src/app/(authenticated)/billing/recurring/page.tsx

@@ -1,5 +1,5 @@
 import { getRecurringInvoices } from '@/features/billing/Actions/recurringInvoiceActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { ListPage } from '@/components/list-page'
@@ -17,7 +17,7 @@ export default async function RecurringInvoicesPage() {
 
   const [result, settingsResult, vehicles] = await Promise.all([
     getRecurringInvoices(),
-    getSettings([SETTING_KEYS.CURRENCY_CODE]),
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE]),
     db.vehicle.findMany({
       where: { organizationId: membership.organizationId, isArchived: false },
       select: {

+ 2 - 2
src/app/(authenticated)/calendar/page.tsx

@@ -1,7 +1,7 @@
 import { getCalendarEvents } from '@/features/calendar/Actions/calendarActions'
 import { getVehicles } from '@/features/vehicles/Actions/vehicleActions'
 import { getCustomersList } from '@/features/customers/Actions/customerActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { getAuthContext } from '@/lib/get-auth-context'
@@ -34,7 +34,7 @@ export default async function CalendarPage({
   const date = parseDateKey(params.date) ?? parseDateKey(todayStr) ?? new Date()
 
   const messageChannels = ctx ? await getAvailableChannels(ctx.organizationId) : []
-  const settingsResult = await getSettings([
+  const settingsResult = await getDisplaySettings([
     SETTING_KEYS.CURRENCY_CODE,
     SETTING_KEYS.WORKBOARD_WEEK_START_DAY,
   ])

+ 2 - 2
src/app/(authenticated)/customers/[id]/page.tsx

@@ -5,7 +5,7 @@ import {
   getCustomerQuotes,
   getCustomersList,
 } from '@/features/customers/Actions/customerActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { getConversation } from '@/features/sms/Actions/smsActions'
 import { getTelegramConversation } from '@/features/telegram/Actions/telegramActions'
@@ -22,7 +22,7 @@ export default async function CustomerDetailPage({ params }: { params: Promise<{
   const [result, settingsResult, layoutData, customersResult, invoicesResult, quotesResult] =
     await Promise.all([
       getCustomer(id),
-      getSettings([SETTING_KEYS.UNIT_SYSTEM]),
+      getDisplaySettings([SETTING_KEYS.UNIT_SYSTEM]),
       getLayoutData(),
       getCustomersList(),
       getCustomerInvoices(id),

+ 2 - 2
src/app/(authenticated)/inspections/[id]/page.tsx

@@ -3,7 +3,7 @@ import {
   getInspection,
   getInspectionTechnicians,
 } from '@/features/inspections/Actions/inspectionActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import {
   InspectionPageClient,
@@ -31,7 +31,7 @@ export default async function InspectionDetailPage({
     authContext?.organizationId ? getFeatures(authContext.organizationId) : null,
     getCommonDefectNotes(id),
     getInspectionTechnicians(),
-    getSettings([SETTING_KEYS.WORKSHOP_ADDRESS]),
+    getDisplaySettings([SETTING_KEYS.WORKSHOP_ADDRESS]),
   ])
 
   return (

+ 2 - 2
src/app/(authenticated)/inventory/[id]/page.tsx

@@ -4,7 +4,7 @@ import {
   getStockMovementsPaginated,
 } from '@/features/inventory/Actions/getStockMovements'
 import { getInventoryCategories } from '@/features/inventory/Actions/inventoryActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { formatDateTime } from '@/lib/format'
@@ -32,7 +32,7 @@ export default async function InventoryPartDetailPage({
       reason: sp.reason,
     }),
     getInventoryCategories(),
-    getSettings([
+    getDisplaySettings([
       SETTING_KEYS.CURRENCY_CODE,
       SETTING_KEYS.INVENTORY_MARKUP_MULTIPLIER,
       SETTING_KEYS.UNIT_SYSTEM,

+ 2 - 2
src/app/(authenticated)/inventory/page.tsx

@@ -4,7 +4,7 @@ import {
   getInventoryCategories,
   hasAnyReorderPoint,
 } from '@/features/inventory/Actions/inventoryActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { InventoryClient } from './inventory-client'
 import { PageHeader } from '@/components/page-header'
@@ -39,7 +39,7 @@ export default async function InventoryPage({
       lowStock: params.lowStock === '1',
     }),
     getInventoryCategories(),
-    getSettings([
+    getDisplaySettings([
       SETTING_KEYS.CURRENCY_CODE,
       SETTING_KEYS.INVENTORY_MARKUP_MULTIPLIER,
       SETTING_KEYS.INVENTORY_DEFAULT_UNIT,

+ 2 - 2
src/app/(authenticated)/labor-presets/page.tsx

@@ -1,7 +1,7 @@
 import { resolveListSort } from '@/lib/list-sort-preference.server'
 import { getInventoryPartsList } from '@/features/inventory/Actions/inventoryActions'
 import { getLaborPresetsPaginated } from '@/features/labor-presets/Actions/laborPresetActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { LaborPresetsClient } from './labor-presets-client'
 import { PageHeader } from '@/components/page-header'
@@ -31,7 +31,7 @@ export default async function LaborPresetsPage({
       sortBy: sort.sortBy,
       sortOrder: sort.sortOrder,
     }),
-    getSettings([SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.DEFAULT_LABOR_RATE]),
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.DEFAULT_LABOR_RATE]),
     // For the "import from inventory" picker in the preset form. A user
     // without inventory read permission simply gets no picker.
     getInventoryPartsList(),

+ 32 - 16
src/app/(authenticated)/page.tsx

@@ -3,7 +3,7 @@ import {
   getDashboardStats,
   getUpcomingReminders,
 } from '@/features/vehicles/Actions/dashboardActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import {
   getVehiclesDueForService,
@@ -14,6 +14,8 @@ import { getQuoteRequests } from '@/features/inspections/Actions/quoteRequestAct
 import { getPendingServiceRequests } from '@/features/customers/Actions/customerActions'
 import { getQuoteResponses } from '@/features/quotes/Actions/quoteResponseActions'
 import { getAuthContext } from '@/lib/get-auth-context'
+import { PermissionSubject } from '@/lib/permissions'
+import { getViewerAccess, readIfAllowed } from '@/lib/viewer-access'
 import { getFeatures } from '@/lib/features'
 import { getRecentSmsThreads } from '@/features/sms/Actions/smsActions'
 import { getNotifications } from '@/features/notifications/Actions/notificationActions'
@@ -39,6 +41,14 @@ export default async function DashboardPage() {
   // that cannot have a portal never runs it, and the card on the second.
   const portalAllowed = features?.customerPortal ?? false
 
+  // What this person's role may read, asked once. Each card below is only
+  // asked for when it can be answered: a refused call is a wasted query and a
+  // "permission denied" row in the audit log, and a Member opening this page
+  // used to write twelve of them. What is drawn does not change, because a
+  // refused card was never drawn.
+  const access = await getViewerAccess()
+  const S = PermissionSubject
+
   const [
     result,
     settingsResult,
@@ -59,28 +69,34 @@ export default async function DashboardPage() {
     serviceRequestsResult,
     inspectionsDueResult,
   ] = await Promise.all([
-    getDashboardStats(),
-    getSettings([
+    readIfAllowed(access, S.DASHBOARD, () => getDashboardStats()),
+    getDisplaySettings([
       SETTING_KEYS.CURRENCY_CODE,
       SETTING_KEYS.UNIT_SYSTEM,
       SETTING_KEYS.PORTAL_ENABLED,
     ]),
-    getUpcomingReminders(),
-    getVehiclesDueForService(),
-    getDismissedMaintenanceVehicles(),
-    getInspectionsPaginated({ status: 'in_progress', pageSize: 5 }),
-    getInspectionsPaginated({ status: 'completed', pageSize: 5 }),
-    getQuoteRequests(),
-    getQuoteResponses(),
-    smsEnabled ? getRecentSmsThreads(0, 5) : Promise.resolve(null),
+    readIfAllowed(access, S.DASHBOARD, () => getUpcomingReminders()),
+    readIfAllowed(access, S.VEHICLES, () => getVehiclesDueForService()),
+    readIfAllowed(access, S.VEHICLES, () => getDismissedMaintenanceVehicles()),
+    readIfAllowed(access, S.INSPECTIONS, () =>
+      getInspectionsPaginated({ status: 'in_progress', pageSize: 5 })
+    ),
+    readIfAllowed(access, S.INSPECTIONS, () =>
+      getInspectionsPaginated({ status: 'completed', pageSize: 5 })
+    ),
+    readIfAllowed(access, S.INSPECTIONS, () => getQuoteRequests()),
+    readIfAllowed(access, S.QUOTES, () => getQuoteResponses()),
+    smsEnabled && access.reads(S.CUSTOMERS) ? getRecentSmsThreads(0, 5) : Promise.resolve(null),
     getNotifications(),
     getRecentAuditLogs(10),
-    getRecentObservations(),
-    getMyActiveJobs(),
+    readIfAllowed(access, S.VEHICLES, () => getRecentObservations()),
+    readIfAllowed(access, S.SERVICES, () => getMyActiveJobs()),
     getOnboardingChecklist(),
-    getTireHotelSummary(),
-    portalAllowed ? getPendingServiceRequests() : Promise.resolve(null),
-    getInspectionsDueSummary(),
+    readIfAllowed(access, S.TIRE_HOTEL, () => getTireHotelSummary()),
+    portalAllowed && access.reads(S.CUSTOMERS)
+      ? getPendingServiceRequests()
+      : Promise.resolve(null),
+    readIfAllowed(access, S.VEHICLES, () => getInspectionsDueSummary()),
   ])
 
   const [layoutUser, widgetRows] = auth

+ 2 - 2
src/app/(authenticated)/quotes/[id]/page.tsx

@@ -1,5 +1,5 @@
 import { getQuote } from '@/features/quotes/Actions/quoteActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import {
   readWarrantyDefaults,
@@ -18,7 +18,7 @@ export default async function QuoteDetailPage({ params }: { params: Promise<{ id
   const { id } = await params
   const [result, settingsResult, presetsResult, inventoryResult, authContext] = await Promise.all([
     getQuote(id),
-    getSettings([
+    getDisplaySettings([
       SETTING_KEYS.CURRENCY_CODE,
       SETTING_KEYS.DEFAULT_TAX_RATE,
       SETTING_KEYS.TAX_ENABLED,

+ 2 - 2
src/app/(authenticated)/quotes/page.tsx

@@ -1,6 +1,6 @@
 import { resolveListSort } from '@/lib/list-sort-preference.server'
 import { getQuotesPaginated } from '@/features/quotes/Actions/quoteActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { QuotesClient } from './quotes-client'
 import { PageHeader } from '@/components/page-header'
@@ -32,7 +32,7 @@ export default async function QuotesPage({
       sortBy: sort.sortBy,
       sortOrder: sort.sortOrder,
     }),
-    getSettings([SETTING_KEYS.CURRENCY_CODE]),
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE]),
   ])
 
   if (!result.success || !result.data) {

+ 2 - 2
src/app/(authenticated)/reminders/page.tsx

@@ -1,7 +1,7 @@
 import { getTranslations } from 'next-intl/server'
 import { getAllReminders } from '@/features/vehicles/Actions/reminderActions'
 import { getVehicles } from '@/features/vehicles/Actions/vehicleActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { RemindersPageClient } from '@/features/vehicles/Components/RemindersPageClient'
 import { PageHeader } from '@/components/page-header'
@@ -10,7 +10,7 @@ export default async function RemindersPage() {
   const [remindersResult, vehiclesResult, settingsResult] = await Promise.all([
     getAllReminders(),
     getVehicles(),
-    getSettings([SETTING_KEYS.UNIT_SYSTEM]),
+    getDisplaySettings([SETTING_KEYS.UNIT_SYSTEM]),
   ])
 
   if (!remindersResult.success || !remindersResult.data) {

+ 2 - 2
src/app/(authenticated)/reports/page.tsx

@@ -1,4 +1,4 @@
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { UpgradePrompt } from '@/components/upgrade-prompt'
@@ -44,7 +44,7 @@ export default async function ReportsPage() {
     )
   }
 
-  const settingsResult = await getSettings([
+  const settingsResult = await getDisplaySettings([
     SETTING_KEYS.CURRENCY_CODE,
     SETTING_KEYS.INVOICE_PRIMARY_COLOR,
   ])

+ 2 - 2
src/app/(authenticated)/tire-hotel/[id]/page.tsx

@@ -7,7 +7,7 @@ import { getTireSet } from '@/features/tire-hotel/Actions/tireSetActions'
 import { getLocationOptions } from '@/features/tire-hotel/Actions/storageActions'
 import { getJobsForSet } from '@/features/tire-hotel/Actions/tireJobActions'
 import { getAttachmentsForSet } from '@/features/tire-hotel/Actions/attachmentActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { TireSetClient } from './tire-set-client'
@@ -32,7 +32,7 @@ export default async function TireSetPage({ params }: { params: Promise<{ id: st
       getLocationOptions(),
       getJobsForSet(id),
       getAttachmentsForSet(id),
-      getSettings([
+      getDisplaySettings([
         SETTING_KEYS.UNIT_SYSTEM,
         SETTING_KEYS.CURRENCY_CODE,
         SETTING_KEYS.TIRE_HOTEL_DEFAULT_SEASONAL_PRICE,

+ 2 - 2
src/app/(authenticated)/tire-hotel/forecast/page.tsx

@@ -2,7 +2,7 @@ import { notFound } from 'next/navigation'
 import { getCachedMembership, getCachedSession } from '@/lib/cached-session'
 import { getTireHotelSettings } from '@/features/tire-hotel/Lib/tireHotelSettings'
 import { getSetsForForecast } from '@/features/tire-hotel/Actions/tireSetActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { PageHeader } from '@/components/page-header'
 import { ForecastClient } from './forecast-client'
@@ -17,7 +17,7 @@ export default async function TireForecastPage() {
 
   const [result, settingsResult] = await Promise.all([
     getSetsForForecast(),
-    getSettings([SETTING_KEYS.UNIT_SYSTEM]),
+    getDisplaySettings([SETTING_KEYS.UNIT_SYSTEM]),
   ])
 
   const data = result.success && result.data ? result.data : { sets: [], total: 0, shown: 0 }

+ 2 - 2
src/app/(authenticated)/tire-hotel/page.tsx

@@ -4,7 +4,7 @@ import { getCachedMembership, getCachedSession } from '@/lib/cached-session'
 import { getTireHotelSettings } from '@/features/tire-hotel/Lib/tireHotelSettings'
 import { getLocationOptions } from '@/features/tire-hotel/Actions/storageActions'
 import { getTireSetsPaginated } from '@/features/tire-hotel/Actions/tireSetActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { totalFree as sumFree } from '@/features/tire-hotel/Lib/capacity'
 import { PageHeader } from '@/components/page-header'
@@ -42,7 +42,7 @@ export default async function TireHotelPage({
       sortOrder: single('sortOrder') === 'asc' ? 'asc' : 'desc',
     }),
     getLocationOptions(),
-    getSettings([SETTING_KEYS.UNIT_SYSTEM]),
+    getDisplaySettings([SETTING_KEYS.UNIT_SYSTEM]),
     db.vehicle.findMany({
       where: { organizationId, isArchived: false },
       orderBy: { updatedAt: 'desc' },

+ 15 - 6
src/app/(authenticated)/vehicles/[id]/page.tsx

@@ -3,7 +3,7 @@ import { getVehicle } from '@/features/vehicles/Actions/vehicleActions'
 import { getServiceRecordsPaginated } from '@/features/vehicles/Actions/serviceActions'
 import { getNotesPaginated } from '@/features/vehicles/Actions/noteActions'
 import { getCustomersList } from '@/features/customers/Actions/customerActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { getVehiclePredictedMileage } from '@/features/vehicles/Actions/predictedMaintenanceActions'
 import { getVehicleInspections } from '@/features/inspections/Actions/inspectionActions'
@@ -18,6 +18,8 @@ import { getTireSetsForVehicle } from '@/features/tire-hotel/Actions/tireJobActi
 import { VehicleDetailClient } from './vehicle-detail-client'
 import { PageHeader } from '@/components/page-header'
 import { redirect } from 'next/navigation'
+import { PermissionSubject } from '@/lib/permissions'
+import { getViewerAccess, readIfAllowed } from '@/lib/viewer-access'
 
 export default async function VehicleDetailPage({
   params,
@@ -46,6 +48,13 @@ export default async function VehicleDetailPage({
   const findingsPage = Number(sp.findingsPage) || 1
   const findingsPageSize = Number(sp.findingsPageSize) || 10
 
+  // The inspections and tire hotel panels belong to permissions of their own.
+  // A role without them is not shown the panels, and is not asked for their
+  // data either: a refused call is a wasted query and a refusal in the audit
+  // log on every vehicle somebody opens (lib/viewer-access).
+  const access = await getViewerAccess()
+  const S = PermissionSubject
+
   const [
     result,
     customersResult,
@@ -63,17 +72,17 @@ export default async function VehicleDetailPage({
     getCustomersList(),
     getServiceRecordsPaginated(id, { page, pageSize, search, type }),
     getNotesPaginated(id, { page: notesPage, pageSize: notesPageSize }),
-    getSettings([SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.UNIT_SYSTEM]),
-    getSettings([
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE, SETTING_KEYS.UNIT_SYSTEM]),
+    getDisplaySettings([
       SETTING_KEYS.PREDICTED_MAINTENANCE_ENABLED,
       SETTING_KEYS.MAINTENANCE_SERVICE_INTERVAL,
       SETTING_KEYS.MAINTENANCE_APPROACHING_THRESHOLD,
     ]),
-    getVehicleInspections(id),
-    getTemplates(),
+    readIfAllowed(access, S.INSPECTIONS, () => getVehicleInspections(id)),
+    readIfAllowed(access, S.INSPECTIONS, () => getTemplates()),
     getVehicleQuotes(id),
     getVehicleFindings(id, { page: findingsPage, pageSize: findingsPageSize }),
-    getTireSetsForVehicle(id),
+    readIfAllowed(access, S.TIRE_HOTEL, () => getTireSetsForVehicle(id)),
   ])
 
   if (!result.success || !result.data) {

+ 2 - 2
src/app/(authenticated)/work-orders/page.tsx

@@ -3,7 +3,7 @@ import { resolveListColumns } from '@/lib/list-columns.server'
 import { WORK_ORDER_COLUMNS } from '@/lib/list-columns'
 import { getTranslations } from 'next-intl/server'
 import { getWorkOrders } from '@/features/vehicles/Actions/serviceActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { getVehicles } from '@/features/vehicles/Actions/vehicleActions'
 import { getCustomersList } from '@/features/customers/Actions/customerActions'
@@ -43,7 +43,7 @@ export default async function WorkOrdersPage({
       sortOrder: sort.sortOrder,
       due: params.due,
     }),
-    getSettings([SETTING_KEYS.CURRENCY_CODE]),
+    getDisplaySettings([SETTING_KEYS.CURRENCY_CODE]),
     getVehicles(),
     getCustomersList(),
     getAuthContext(),

+ 55 - 9
src/features/custom-fields/Actions/customFieldActions.ts

@@ -4,6 +4,8 @@ import { db } from '@/lib/db'
 import { withAuth } from '@/lib/with-auth'
 import {
   createFieldDefinitionSchema,
+  entityTypes,
+  type EntityType,
   updateFieldDefinitionSchema,
 } from '../Schema/customFieldSchema'
 import { revalidatePath } from 'next/cache'
@@ -11,6 +13,48 @@ import { PermissionAction, PermissionSubject } from '@/lib/permissions'
 import { requireFeature } from '@/lib/features'
 import { clearedToNull } from '@/lib/clearable'
 
+/**
+ * Whose permission a record's custom fields fall under: the record's own.
+ *
+ * Every one of these used to need the Settings permission, because defining a
+ * field is a setting. But filling one in is not: it is part of the work order
+ * or the quote, the same as its title. A Member who may edit a work order saw
+ * none of the workshop's custom fields on it and could not have saved one,
+ * and was refused on every page load besides. Defining fields stays behind
+ * Settings; reading and filling them follows the record.
+ */
+const RECORD_SUBJECT: Record<EntityType, PermissionSubject> = {
+  service_record: PermissionSubject.SERVICES,
+  quote: PermissionSubject.QUOTES,
+}
+
+function isEntityType(value: unknown): value is EntityType {
+  return typeof value === 'string' && (entityTypes as readonly string[]).includes(value)
+}
+
+/** The permission a call about one record's fields needs. An unknown kind needs Settings. */
+function recordPermission(entityType: unknown, action: PermissionAction) {
+  return [
+    {
+      action,
+      subject: isEntityType(entityType) ? RECORD_SUBJECT[entityType] : PermissionSubject.SETTINGS,
+    },
+  ]
+}
+
+/**
+ * The record has to be this workshop's. These actions are handed a bare id,
+ * and a value row is keyed by it, so without this a caller could attach
+ * values to a record that is not theirs.
+ */
+async function assertOwnRecord(entityId: string, entityType: EntityType, organizationId: string) {
+  const found =
+    entityType === 'quote'
+      ? await db.quote.count({ where: { id: entityId, organizationId } })
+      : await db.serviceRecord.count({ where: { id: entityId, organizationId } })
+  if (found === 0) throw new Error('Record not found')
+}
+
 export async function getFieldDefinitions(entityType?: string) {
   return withAuth(
     async ({ organizationId }) => {
@@ -24,7 +68,11 @@ export async function getFieldDefinitions(entityType?: string) {
       })
     },
     {
-      requiredPermissions: [{ action: PermissionAction.READ, subject: PermissionSubject.SETTINGS }],
+      // The fields of one kind of record are read to draw that record's form.
+      // The whole list, across kinds, is the settings screen.
+      requiredPermissions: entityType
+        ? recordPermission(entityType, PermissionAction.READ)
+        : [{ action: PermissionAction.READ, subject: PermissionSubject.SETTINGS }],
     }
   )
 }
@@ -143,6 +191,8 @@ export async function deleteFieldDefinition(fieldId: string) {
 export async function getCustomFieldValues(entityId: string, entityType: string) {
   return withAuth(
     async ({ organizationId }) => {
+      if (!isEntityType(entityType)) throw new Error('Unknown record type')
+      await assertOwnRecord(entityId, entityType, organizationId)
       const definitions = await db.customFieldDefinition.findMany({
         where: { organizationId, entityType, isActive: true },
         orderBy: [{ sortOrder: 'asc' }, { createdAt: 'asc' }],
@@ -167,9 +217,7 @@ export async function getCustomFieldValues(entityId: string, entityType: string)
         value: valuesMap[def.id] !== undefined ? valuesMap[def.id] : (def.defaultValue ?? ''),
       }))
     },
-    {
-      requiredPermissions: [{ action: PermissionAction.READ, subject: PermissionSubject.SETTINGS }],
-    }
+    { requiredPermissions: recordPermission(entityType, PermissionAction.READ) }
   )
 }
 
@@ -180,6 +228,8 @@ export async function saveCustomFieldValues(
 ) {
   return withAuth(
     async ({ organizationId }) => {
+      if (!isEntityType(entityType)) throw new Error('Unknown record type')
+      await assertOwnRecord(entityId, entityType, organizationId)
       const definitions = await db.customFieldDefinition.findMany({
         where: { organizationId, entityType, isActive: true },
       })
@@ -201,10 +251,6 @@ export async function saveCustomFieldValues(
       await db.$transaction(ops)
       return { saved: true }
     },
-    {
-      requiredPermissions: [
-        { action: PermissionAction.UPDATE, subject: PermissionSubject.SETTINGS },
-      ],
-    }
+    { requiredPermissions: recordPermission(entityType, PermissionAction.UPDATE) }
   )
 }

+ 1 - 1
src/features/custom-fields/Components/CustomFieldsForm.tsx

@@ -136,7 +136,7 @@ export function CustomFieldsForm({
         const val = values[field.id] ?? field.value ?? ''
         const hasError = !!errors[field.id]
         return (
-          <div key={field.id} className="space-y-1.5">
+          <div key={field.id} data-testid={`custom-field-${field.id}`} className="space-y-1.5">
             {field.fieldType !== 'checkbox' && (
               <Label className={cn(hasError && 'text-destructive')}>
                 {field.label}

+ 13 - 1
src/features/integrations/Actions/integrationActions.ts

@@ -981,6 +981,18 @@ const SERVICE_PERMISSION = [
   { action: PermissionAction.UPDATE, subject: PermissionSubject.SERVICES },
 ]
 
+/**
+ * Seeing the meeting on a work order is part of seeing the work order. It
+ * used to need the Settings permission, because connections are a setting,
+ * while adding or removing the meeting needed only the work order's own: so a
+ * member could put a video call on a job and never be shown it, and was
+ * refused on every work order they opened. Nothing here is a secret: a link
+ * already on the job, and the names of the services that could add one.
+ */
+const SERVICE_READ_PERMISSION = [
+  { action: PermissionAction.READ, subject: PermissionSubject.SERVICES },
+]
+
 export interface ServiceVideoCall {
   /** The link on the work order, from whichever connection put it there. */
   link: {
@@ -1043,7 +1055,7 @@ export async function getServiceVideoCall(serviceRecordId: string) {
         .map((m) => ({ connectorId: m.id, name: m.name, provider: m.meetingProvider }))
       return { link, providers }
     },
-    { requiredPermissions: READ_PERMISSION }
+    { requiredPermissions: SERVICE_READ_PERMISSION }
   )
 }
 

+ 27 - 0
src/features/settings/Actions/settingsActions.ts

@@ -8,6 +8,7 @@ import { PermissionAction, PermissionSubject } from '@/lib/permissions'
 import { demoGuardSettingKey } from '@/lib/demo'
 import { assertOwnUploads } from '@/lib/upload-url'
 import { armFeatureHints } from '../Lib/armFeatureHints'
+import { MEMBER_READABLE_SETTINGS } from '../Lib/memberReadableSettings'
 import { requireFeature } from '@/lib/features'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import { releaseReplacedSettingFiles, settingValuesBefore } from '@/lib/files/settings'
@@ -26,6 +27,32 @@ export async function getSetting(key: SettingKey) {
   )
 }
 
+/**
+ * How the workshop's work is shown: currency, units, tax and labour defaults,
+ * the address on a document. Readable by every member, because a member who
+ * may open a work order has to see it in the workshop's own currency.
+ *
+ * Only what is named in MEMBER_READABLE_SETTINGS is ever returned. Anything
+ * else asked for is left out rather than refused, so a page that asks for one
+ * key too many still renders, and that key simply is not there: it is read
+ * through `getSettings`, behind the Settings permission, or not at all.
+ */
+export async function getDisplaySettings(keys: SettingKey[]) {
+  return withAuth(async ({ organizationId }) => {
+    const allowed = keys.filter((key) => MEMBER_READABLE_SETTINGS.has(key))
+    if (allowed.length === 0) return {} as Record<string, string>
+
+    const settings = await db.appSetting.findMany({
+      where: { organizationId, key: { in: allowed } },
+      select: { key: true, value: true },
+    })
+
+    const map: Record<string, string> = {}
+    for (const s of settings) map[s.key] = s.value
+    return map
+  })
+}
+
 export async function getSettings(keys?: SettingKey[]) {
   return withAuth(
     async ({ userId, organizationId }) => {

+ 59 - 0
src/features/settings/Lib/memberReadableSettings.ts

@@ -0,0 +1,59 @@
+import { SETTING_KEYS, type SettingKey } from '../Schema/settingsSchema'
+import { WARRANTY_SETTING_KEYS } from './warrantyDefaults'
+
+/**
+ * What every member of a workshop may read, whatever their role.
+ *
+ * These are not "settings" in the sense the Settings permission guards. They
+ * are how the workshop's own work is shown: its currency, its units, its tax
+ * rate, its address on a document. A member who may open a work order has to
+ * see it in the workshop's currency, and they were not: every ordinary page
+ * read these through `getSettings`, which needs `read:settings`, so a Member
+ * was refused on every page load, the page quietly fell back to the built-in
+ * defaults (the wrong currency, the wrong tax rate, no labour rate), and each
+ * refusal wrote a row to the audit log. Eight thousand in one afternoon.
+ *
+ * The permission stays exactly where it was for everything else, because the
+ * same table holds payment secrets, API keys and the licence token. This is a
+ * list of what is safe, not a list of what is secret: a key that is not named
+ * here is not readable this way, so a new setting is private until somebody
+ * decides otherwise.
+ */
+export const MEMBER_READABLE_SETTINGS: ReadonlySet<SettingKey> = new Set<SettingKey>([
+  // Money and measures, as shown on every list and document.
+  SETTING_KEYS.CURRENCY_CODE,
+  SETTING_KEYS.UNIT_SYSTEM,
+  SETTING_KEYS.DATE_FORMAT,
+  SETTING_KEYS.TIME_FORMAT,
+  SETTING_KEYS.TIMEZONE,
+  SETTING_KEYS.WORKBOARD_WEEK_START_DAY,
+
+  // The defaults a new line, quote or invoice starts from.
+  SETTING_KEYS.TAX_ENABLED,
+  SETTING_KEYS.DEFAULT_TAX_RATE,
+  SETTING_KEYS.DEFAULT_LABOR_RATE,
+  SETTING_KEYS.INVOICE_DUE_DAYS,
+  SETTING_KEYS.PARTS_DEFAULT_MARKUP_PERCENT,
+  SETTING_KEYS.PARTS_MARKUP_APPLIES_TO_INVENTORY,
+  SETTING_KEYS.INVENTORY_MARKUP_MULTIPLIER,
+  SETTING_KEYS.INVENTORY_DEFAULT_UNIT,
+  SETTING_KEYS.LOW_STOCK_DEFAULT_THRESHOLD,
+  SETTING_KEYS.TIRE_HOTEL_DEFAULT_SEASONAL_PRICE,
+  ...WARRANTY_SETTING_KEYS,
+
+  // What a vehicle's page works out from its history.
+  SETTING_KEYS.PREDICTED_MAINTENANCE_ENABLED,
+  SETTING_KEYS.MAINTENANCE_SERVICE_INTERVAL,
+  SETTING_KEYS.MAINTENANCE_APPROACHING_THRESHOLD,
+
+  // How a document looks, and who it says it is from. All of it is printed on
+  // what the customer receives.
+  SETTING_KEYS.INVOICE_ACTIVE_DESIGN,
+  SETTING_KEYS.INVOICE_PRIMARY_COLOR,
+  SETTING_KEYS.WORKSHOP_ADDRESS,
+  SETTING_KEYS.WORKSHOP_EMAIL,
+  SETTING_KEYS.WORKSHOP_PHONE,
+
+  // Whether the share and portal buttons are offered at all.
+  SETTING_KEYS.PORTAL_ENABLED,
+])

+ 2 - 2
src/features/vehicles/Components/service-page/ServiceRecordPage.tsx

@@ -2,7 +2,7 @@ import { parseTaxComponents } from '@/lib/tax-components'
 import { getServiceRecord } from '@/features/vehicles/Actions/serviceActions'
 import { getServiceVideoCall } from '@/features/integrations/Actions/integrationActions'
 import { getWorkBays } from '@/features/workboard/Actions/workBayActions'
-import { getSettings } from '@/features/settings/Actions/settingsActions'
+import { getDisplaySettings } from '@/features/settings/Actions/settingsActions'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
 import {
   readWarrantyDefaults,
@@ -68,7 +68,7 @@ export async function ServiceRecordPage({
     initialLayout,
   ] = await Promise.all([
     getServiceRecord(serviceId),
-    getSettings([
+    getDisplaySettings([
       SETTING_KEYS.CURRENCY_CODE,
       SETTING_KEYS.UNIT_SYSTEM,
       SETTING_KEYS.DEFAULT_TAX_RATE,

+ 58 - 0
src/lib/viewer-access.ts

@@ -0,0 +1,58 @@
+import { getCachedMembership } from './cached-session'
+import { getAuthContext } from './get-auth-context'
+import { hasPermission, PermissionAction, type PermissionSubject } from './permissions'
+import type { ActionResult } from './with-auth'
+
+/**
+ * What the person looking at a page may read, asked once, before the page
+ * asks for anything.
+ *
+ * A page that shows several things at once (the dashboard shows sixteen) used
+ * to ask for all of them and let `withAuth` refuse the ones this role may not
+ * have. That is correct on screen, because a refused card is simply not
+ * drawn, and wrong everywhere else: each refusal is a query that was never
+ * going to be answered and a "permission denied" row in the audit log. A
+ * Member opening the dashboard wrote twelve of them per visit, which buries
+ * the one refusal an owner would actually want to see.
+ *
+ * So the page asks here first and does not make the call. The rules are the
+ * ones `withAuth` applies, in the same order: a super admin, an owner, an
+ * admin and an admin role read everything; anybody else reads what their role
+ * was given, and somebody with no role reads nothing.
+ *
+ * This decides what to ask for, never what is allowed. `withAuth` still
+ * guards every action, so a page that gets this wrong is refused as before.
+ */
+export interface ViewerAccess {
+  reads(subject: PermissionSubject): boolean
+}
+
+export async function getViewerAccess(): Promise<ViewerAccess> {
+  const auth = await getAuthContext()
+  if (!auth) return { reads: () => false }
+  if (auth.isAdmin) return { reads: () => true }
+
+  const membership = await getCachedMembership(auth.userId)
+  const granted = membership?.customRole?.permissions ?? []
+  return {
+    reads: (subject) => hasPermission(granted, { action: PermissionAction.READ, subject }),
+  }
+}
+
+/**
+ * Makes the call when the viewer may read `subject`, and otherwise answers as
+ * `withAuth` would have, without the query and without the audit row. The
+ * page handles both the same way, which is the point.
+ */
+export function readIfAllowed<T extends ActionResult<unknown>>(
+  access: ViewerAccess,
+  subject: PermissionSubject,
+  call: () => Promise<T>
+): Promise<T> {
+  if (access.reads(subject)) return call()
+  return Promise.resolve({
+    success: false,
+    error: 'Insufficient permissions',
+    forbidden: true,
+  } as T)
+}