fix(plugin-security,spec): PERMISSION_DENIED stops showing developer copy to end users (#7414) - #7449
Merged
Merged
Conversation
…copy to end users (#7414) The 403 refusal is correct and its transport is fine; the message was not. `Error.message` is the body's human-readable string on every transport and Console renders it verbatim in a toast, so a business user in a localized app got an English sentence naming an object API name they had never seen and ending in `positions [org_member, everyone]`. The error now carries two messages because it has two audiences: - `message` — the user's half, rendered in `ExecutionContext.locale` through the SHARED operation-message catalog #7307 built (`errors.permission_denied`). It names no object, no operation and no position, in any shipped locale. - `developerMessage` — the developer's half, the previous sentence byte for byte. LOGGED at the throw site rather than shipped: unlike the 409, this body carries no structured API names, so shipping it would have added a disclosure instead of removing one. It is a sibling of `details`, never a member of it, because `details` is what the dispatcher serialises. `code` / `statusCode` / `details` unchanged; one PERMISSION_DENIED (ADR-0112), two sentences. 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
|
github-actions
Bot
added
size/l
documentation
This was referenced Aug 10, 2026
Contributor
📓 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:
|
…'s dispatch predicates (#7414) `check:engine-double-contract` caught the new fixture's `delete()`/`update()` being looser than `ObjectQL`'s — a fake that accepts call shapes the engine refuses collects greens the producer would not. Both verbs now open with `assertEngineDeleteDispatch` / `assertEngineUpdateDispatch` from `@objectstack/metadata-core`. Also corrects the flag on the catalog's placeholder case: reverse verification measured it going RED on a reverted catalog (it reads the ENTRY, not the rendering), so the comment claiming it cannot bite was wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
…sage (#7414) `share-links-enforcement-context.test.ts` drives the REAL security gate and asserted `toContain('Access denied')` on the message. That string is now the developer half (logged, not shipped); the message is the catalog's user-facing sentence. Re-spelled against `BUILTIN_OPERATION_MESSAGES.en.permission_denied` so the case keeps its stated purpose — proving an authorization answer reached the client rather than being swallowed into the generic internal-error string — without needing another re-spell on the next copy edit. Found by CI, not by the local sweep: the rule's consumption radius is wider than the edited package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
os-zhuang
marked this pull request as ready for review
August 10, 2026 13:51
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7414
The 403 the object-CRUD gate throws is correct and its transport is fine. What reached the end user was the problem:
Error.messageis the body's human-readable string on every transport (mapDataError'sbody.error, the dispatcher'serror.message) and Console renders it verbatim in a toast, so an operator in a fully localized app readEnglish-only, naming a table they have never seen, ending in internal authorization vocabulary that reads as a contradiction to someone who does hold rights on the record they clicked. And it is not confined to obviously-unauthorized actions:
cascadeDeleteRelationsre-authorises every cascade child independently, so an ordinary parent delete the app deliberately granted can surface a 403 naming a child object the operator never addressed.The mechanism is #7307's, not a second one
This card is the second consumer of the shared operation-message catalog PR #7423 built, not a new remedy for the same defect class:
packages/spec/src/system/operation-message.tsrenderOperationMessage(),BUILTIN_OPERATION_MESSAGES, theerrors.override address, the four-rung resolution ladder, the "a misbehaving i18n service must not turn the refusal into a 500" guardOPERATION_MESSAGE_FALLBACK_LOCALEenlast resortmessage= user's half inExecutionContext.locale;developerMessage= the previous sentence byte for byteOnly the catalog content is new: one key,
permission_denied, in all four shipped locales. No new exports —packages/spec's api-surface / export-origins / authorable-surface baselines are untouched, which is why this PR carries no regenerated artifacts (verified by running those gates, below).One deliberate divergence in the sentence itself
delete_restrictednames the objects because the user must know which related records block them — that is the action they can take. An object-permission refusal gives the user nothing to act on by naming the object, and on a cascade the object the gate refuses is a child they never addressed. Sopermission_deniedtakes no placeholders at all, and the label ladder (bundle → declared label → API name) is deliberately not used here: its last rung is the API name, which is exactly what must not reach the toast. Reported rather than papered over — the dispatch for this card presumed a label ladder, and it does not apply.Where the structured fields actually land — measured, and the answer changed the design
PR #7423 justified shipping
developerMessageover the wire with: "It discloses nothing the envelope did not already carry:dependentObjectandobjectare API names on the same body." The issue's own "Expected" section asserts the same for this error — thatoperation/object/positions"are the right channel … and they are already there". That is only half true, and the half that is false is the one this decision rests on.error.detailsreach the client?@objectstack/rest(mapDataError,rest-server.ts:726-737){ error, code, object? };error.detailsis never read, and thatobjectis the object the route named@objectstack/runtimedispatcher (http-dispatcher.ts:1945-1951)this.error(e.message, 403, { code: 'PERMISSION_DENIED', ...(e.details ?? {}) })→buildApiError→error.detailsSo on the REST transport the positions, the operation and (on a cascade) the child object's API name reach a client through nothing but this message. Adding a
developerMessagethere would have been a new disclosure of internal authorization vocabulary, introduced by a card whose purpose was to reduce disclosure. One error class cannot honestly carry a per-transport disclosure policy, so:developerMessageis not shipped on either transport. It is logged at the throw site (ctx.logger.warn), where an app builder debugging a 403 already looks, in English regardless of the caller's locale.details, never a member of it —detailsis the field the dispatcher serialises.packages/rest/src/rest.test.tspins this positively, with the reasoning inline, so the next reader mirroring DELETE_RESTRICTED (409) message is shown verbatim to end users: English-only, leaks API names, and contains developer-facing advice #7307 here fails loudly instead of quietly widening the envelope.The second row of that table deserves a card of its own:
positionsandpermissionSetsalready reach the browser today on the dispatcher transport, which this copy fix cannot and must not change (enforcement and the structured payload are explicitly out of scope). Filed separately as #7450.Anchors re-derived at the branch point
Branch point
2ef18070e(= PR #7423's squash). The triage anchorsecurity-plugin.ts:1160still held exactly:PR #7346 (comments) and PR #7400 (
checkAuthoredRowWriteprobe scope) are both landed and untouched by this change.Verification
packages/spec/src/system/operation-message.test.ts: locale rendering,ja→ja-JPbase-language match,de-DE→enfallback, override undererrors.permission_denied, a throwing i18n service, and an absence guard overpositions/permissionSets/[Security]/Access denied/operationin every locale — each asserted on top of a positive identity so an empty string cannot satisfy it.packages/plugins/plugin-security/src/permission-denied-user-copy.test.ts, driving the realSecurityPluginmiddleware, and resolving overrides through the realII18nService(FileI18nAdapterfrom@objectstack/service-i18n, added as a devDependency) rather than a hand-writtent. A stub is free to agree with a producer that disagrees with the shipped implementation, and this repo has two brace conventions in flight (fix(plugin-audit): resolve reference titles inactivityMilestonessummary tokens (#7290) #7333).code/statusCode/ the wholedetailspayload asserted unchanged. Flagged in the test file as non-regression guards rather than revert-detectors: they are green onmaintoo, by construction, which is the point of a copy-only card.whereonfind, so the bootstrap permission setsSecurityPlugin.start()seeds were all handed back to thedbLoaderfor any unresolved name — includingadmin_full_access— and the gate admitted the delete under 12 assertions written to see it refused. The double now honours the predicate, and the comment abovemakeEnginesays why.Not done, deliberately
:1160.plugin-securitycarries ~40 other[Security] Access deniedsentences (delegated-admin gate, row-level gate, predicate guard, package-managed guards). They share the defect class, but the issue measured and named this one, and a 40-template sweep is a different review. Filed as The other ~40[Security] Access deniedsentences are still English-only developer copy shown verbatim to end users (same class as #7414, one template fixed) #7451.