fix(plugin-security,spec): the end-user-facing [Security] Access denied gates stop showing developer copy (#7451) - #7471
Conversation
…l and capability denials (#7451) The three gates of the object CRUD middleware an ordinary non-admin principal reaches on ordinary business work now render their user-facing half through the shared operation-message catalog, in the caller's locale: - row-level pre-image write denial -> errors.record_access_denied (new key) - row-level CHECK post-image denial -> errors.record_change_not_allowed (new key) - capability AND-gate (ADR-0066 D3) -> errors.permission_denied (reused) Two new keys, not three: the unit is the SITUATION the user is in, not the gate that answered. A caller missing a CRUD bit and a caller missing a capability have one situation and one remedy, so they share a sentence; the row-level gates do not, because their users can act differently (ask the record's owner / change what they typed). Each gate keeps its previous sentence byte for byte on `developerMessage`, logged at the throw site and shipped on neither transport, per #7414's measurement. Enforcement, statuses, codes and every `details` payload are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
…ADR-0112 envelope (#7451) CI's full-suite run found a fifth consumption-radius pin that a phrase grep could not: `rejects.toThrow(/denied|permission/i)`, which was matching the SHAPE of the English copy rather than the refusal. The row-level gate's user half is now a localized catalog sentence containing neither word, so the regex went red on a change that altered nothing it was written to protect. Pinned properly instead: `code` + `statusCode` (never a bare throw), the user half against the catalog constant, and `developerMessage` for the one thing the regex was reaching for — which gate refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
Fixes #7451
The card asked for a classification before a patch, so that is what this PR leads with. Its first deliverable is which of the ~40
[Security] Access deniedtemplates a non-admin principal can reach on an ordinary business operation, measured rather than inferred from file names. Only that half is converted here; the admin-surface half is reported below as a follow-up recommendation, per the claim comment's scoping.Census, re-measured at the branch point
git grep -c "[Security] Access denied"at45cd354ec(the branch point), producers only — the matcher/doc occurrences inplugin-security/src/errors.ts(1),rest/src/rest-server.ts(3) andruntime/src/security/resolve-execution-context.ts(1) arestartsWithfallbacks and comments, not thrown copy, and the card correctly excludes them:2ef18070e)45cd354ec)plugin-security/src/security-plugin.tsdeveloperMessage, exactly as the card predictedplugin-security/src/delegated-admin-gate.tsplugin-security/src/suggested-audience-bindings.tsplugin-auth/src/invitation-role-cap.tsplugin-security/src/permission-set-projection.tsplugin-security/src/system-write-guard.tsplugin-security/src/predicate-guard.ts42 producer templates, 41 unconverted at the branch point. The census is otherwise unchanged from the card's: the three landings since (#7346 comments-only, #7400 probe scope, #7449 one template) moved only
security-plugin.ts, and only by #7449's conversion. Two test files also matched the grep (permission-denied-user-copy.test.ts,row-write-widener-composition.test.ts); they are consumers, counted separately below.The classification
The question asked at each site was which principal reaches this gate on an ordinary business operation — not which file it lives in.
Converted: end-user-facing, with measured pull
security-plugin.ts:1434row-level pre-image write denialusingpolicy excludesusing: 'assignee == current_user.email'on its own business objects (examples/app-showcase/src/security/permission-sets.ts:64);packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.tsalready drives it end to end over REST as a non-admin and asserts the English sentence off the response bodysecurity-plugin.ts:1786row-level CHECK post-image denialcheckpolicycheck: 'owner == current_user.email'(permission-sets.ts:95) with the comment "a contributor cannot reassign an invoice they own to someone else" — an ordinary CRM actionsecurity-plugin.ts:1180capability AND-gaterequiredPermissionsrequiredPermissionsis an authorable top-level object key (ADR-0066 D3,packages/spec/src/data/object.zod.ts:587); the card's own example is a business objectThe capability gate is the one whose audience is genuinely mixed, and it is worth stating rather than smoothing over: measured on today's shipped metadata, the only objects declaring object-level
requiredPermissionsaresys_scim_providerandsys_sso_provider, both gated onmanage_platform_settings, i.e. admin surface. It is still converted, because the mixed audience dissolves for this template specifically: the user-facing sentence it renders (permission_denied) is correct for both audiences, and the only facts that would distinguish them are capability IDs, which are internal vocabulary by construction. Nothing is lost either way — no ADR citation, no capability name — because the developer half is preserved verbatim.Not converted: admin surface (the card's own reading, confirmed)
delegated-admin-gate.ts(13),suggested-audience-bindings.ts(3),permission-set-projection.ts(1),system-write-guard.ts(1), andsecurity-plugin.ts's package-managed / permission-set admin-door family (:2457,:3314,:3339,:3376,:3389,:3456,:3482,:3500). Reached from an admin console by someone administering the system; most cite ADR clauses (ADR-0090 D5/D9, D6/D12, ADR-0091 D3, ADR-0103), and localizing them would destroy the citation without helping anyone. Recommended for a follow-up card as retain as developer copy, not as a translation backlog.Not converted: reachable by an end user, but NOT cleanly one situation
Reported rather than guessed, which is what the audience split exists for:
security-plugin.ts:3981(assertControlledByParentWrite) is end-user-facing with real pull —controlled_by_parentis declared byexamples/app-crm/src/objects/opportunity-line-item.object.tsand showcase'sinvoice/expense-report, so an ordinary user editing an invoice line meets it. But its singledeny()helper multiplexes six conditions under one sentence, and three of them are not access verdicts at all:'controlled_by_parent declared but no master_detail relation'(a metadata defect),'target record not found', and'detail record has no master reference'. Giving that one sentence user-facing copy would tell a user "you do not have access to the related record" when the truth is that their app is misconfigured — and would hide the metadata defect from the author. Splitting it is a real decision, not a copy edit.security-plugin.ts:1657(owner_idis system-managed, transfer grant) is a plausible end-user situation — record reassignment is a core CRM action — but has zero measured pull:allowTransferappears nowhere in either example app.security-plugin.ts:1069/:1140(fail-closed: permission subsystem unavailable, unresolvable posture) are end-user visible during degradation, but the situation is "the check could not run", not "you lack permission". Copy that says the latter would be false, and the right treatment likely touches more than copy.security-plugin.ts:953(public-form grant),:1891(tenant wall on a suppliedorganization_id),:1094/:1265(on-behalf-of delegation),predicate-guard.ts:122(query names a non-readable field),plugin-auth/invitation-role-cap.ts(3): reached through a misbuilt or forged client, a configured agent integration, or a team-administration action, not through ordinary business work.The three questions
Q2 — how many keys. Settled at claim time and applied as given: one key per situation, not per code and not per gate. Two new keys, not three, because the capability gate and the CRUD grant gate put the user in the same situation with the same remedy. The catalog module now carries that table in its header so the next consumer inherits the rule rather than the count.
Q1 — audience. Above.
Q3 — may the sentence name anything? Judged per site, and the answer was no three times for three different reasons:
permission_denied— PermissionDeniedError (403) message is shown verbatim to end users too — English-only, leaks the object API name and the internalpositionsvocabulary (sibling of #7307, different package) #7414's reasoning, unchanged; the capability gate adds only capability IDs, which are internal vocabulary.record_access_denied— this is the one that could have named honestly. The refused record is the one the caller just addressed by id, so unlike PermissionDeniedError (403) message is shown verbatim to end users too — English-only, leaks the object API name and the internalpositionsvocabulary (sibling of #7307, different package) #7414's gate there is no cascade-child ambiguity. It still names nothing, because the only spellings available at the throw site are the object's API name and an opaque row id; reaching a label needs the ladder whose last rung is the API name, which is exactly what must not reach a toast. The user already knows which record they clicked.record_change_not_allowed— the gate knows a post-image failed an authored predicate over the whole row, not which field carried the offending value. Naming the object without the field would send the user hunting.Two inheritances that did not happen
detailsand never a member. The dogfood test now measures that end to end: it asserts the localized sentence on a real REST body and that the serialized envelope contains neither[Security]norrow-level security.details.positionson the dispatcher transport is The runtime dispatcher serialises a PermissionDeniedError'sdetailsto the client, sopositions/permissionSetsreach the browser on the/datatransport #7450's question, untouched here.Enforcement is untouched
Same statuses, same codes, same decision logic. Every
detailspayload is asserted withtoEqualat all three gates — includingrequiredPermissions,missingPermissionsandrecordId.Verification
packages/spec/src/system/operation-message.test.ts: locale rendering, base-language match (jatoja-JP),de-DEfallback toen, overrides under the sharederrors.address, a throwing i18n service, and a developer-vocabulary absence sweep in every locale — each on top of a positive identity, with the loop guarded byexpect(locales.length).toBeGreaterThanOrEqual(4). Placeholder hygiene is asserted on the catalog entry, not the rendering, because rendering a removed key yields the bare key, which has no braces either.packages/plugins/plugin-security/src/security-denial-user-copy.test.ts, driving the realSecurityPluginmiddleware and resolving overrides through the realII18nService(FileI18nAdapter), never a stub: a stub is free to agree with whichever brace convention the test author had in mind, and this repo has two in flight (fix(plugin-audit): resolve reference titles inactivityMilestonessummary tokens (#7290) #7333).wherethroughmatchesFilterCondition— the producer's own matcher — rather than a hand-written one: the pre-image gate re-reads withwhere: { $and: [{ id }, rlsFilter] }, and a matcher blind to$andwould have made every denial case green while measuring the double's blindness. Itsupdate()/delete()open withassertEngineUpdateDispatch/assertEngineDeleteDispatch.maintoo, by construction, which is the point of a copy-only card.Reverse verification
Directions were written to a file and the tree committed before the first mutation, so the presumptions could not be edited to fit the result. All five matched:
messageanddeveloperMessageundefinedrecord_access_denieddeleted from all four locales, catalog layermessagecode/status/details)maintoo, by constructioncontrolled-by-parent-sharing.test.ts:3981pin stayed greenThe fourth row is the one worth reading: the template presumes "restore the limb, watch it go red", and for a copy-only card the enforcement pins cannot go red. Reported rather than dressed up as a passing revert test.
Consumption radius — and where grep was not enough
Grepped repo-wide before pushing. That found three pins. Two more were found only by running things, and both are worth recording because they name the limits of the grep:
security-plugin.test.ts:1174pinned a fragment (operation 'insert'), not a phrase — found by the package suite.service-automation/src/runas-system-stamping.integration.test.ts:308pinned a regex over the message shape,toThrow(/denied|permission/i)— found only by CI's full-suite run. It was never asserting this refusal; it matched because the English sentence happened to open with "Access denied", and the localized sentence contains neither word. A regex like that goes red on a change that alters nothing it was written to protect.All five re-spelled against the catalog constant rather than a literal, so the next copy edit needs no re-spell:
plugin-security/src/security-plugin.test.ts:1028(capability gate) and:1174(its per-operation narrowing);plugin-security/src/row-write-widener-composition.test.ts(expectRowLevelDenial);qa/dogfood/test/authored-row-write-scope.dogfood.test.ts(the end-to-end REST body);service-automation/src/runas-system-stamping.integration.test.ts(now the ADR-0112 envelope:code+statusCode, never a bare throw).None was weakened: each now asserts the user half against the constant and keeps the old sentence on
developerMessage, so "which gate answered" is still a measured fact rather than an inference from a generic 403 — a stronger discrimination than before, on two axes instead of one.