Skip to content

fix(rest): meta app by-name answers a permission-denied envelope, not absence (#8013) - #8135

Merged
hotlong merged 1 commit into
mainfrom
claude/issue-8013-meta-app-by-name-denied-envelope
Aug 12, 2026
Merged

fix(rest): meta app by-name answers a permission-denied envelope, not absence (#8013)#8135
hotlong merged 1 commit into
mainfrom
claude/issue-8013-meta-app-by-name-denied-envelope

Conversation

@hotlong

Copy link
Copy Markdown
Contributor

Fixes#8013

Backend half of the ruling of 2026-08-12 (「接受你的全部建议。」). The console half is objectstack-ai/objectui#4252 and is blocked on this.

The code to branch on

code: "PERMISSION_DENIED" · status: 403, in the declared envelope:

{ "success": false, "error": { "code": "PERMISSION_DENIED", "message": "You do not have permission to open the 'finance' app." } }

PERMISSION_DENIED is the ADR-0112 standard catalog member for a generic authorization refusal — the ledger's own rule is that a generic condition takes the catalog rather than registering a synonym — and it is what standardErrorCodeForHttpStatus(403) answers. objectui#4252 reads body.error.code, which is the same accessor as the neighbouring absence answer, so the console needs one access path and not two.

What changed

GET /api/v1/meta/app/{name} collapsed three different refusals into one 404-equivalent, so an app the session may never open and an app that does not exist were byte-identical on the wire. The console has nothing to branch on, so it renders its only copy for an absent app — "it may still be publishing" — over a permanent authorization denial. Measured cost on objectui#4252: two acceptance-test batches spent chasing a "platform defect" that was a missing permission-set binding.

filterAppForUser returned a bare null for all three. It now delegates to filterAppForUserWithReason, which reports which gate fired; filterAppForUser is an unchanged thin wrapper over it, so the list route and the existing unit pins are untouched.

Exactly one of the three converts:

gatebeforeafter
requiredPermissions not heldabsence403 PERMISSION_DENIED
_unpublished (ADR-0045 §3)absenceabsence — unchanged
requiresService absent (ADR-0057 D10)absenceabsence — unchanged
name resolves to nothingabsenceabsence — unchanged
list route GET /meta/appsfilteredfiltered — unchanged

The reason comes from the branch that fired, never inferred from app == null at the call site. That distinction is the security boundary of the card:

Ordering is load-bearing for the same reason: _unpublished is judged first, so an app that is both unpublished and permission-gated reports absence.

Landing — confirmed as the card predicted

packages/rest/src/rest-server.ts, located by symbol: filterAppForUser and the by-name handler's app branch. The card asked to report a corrected landing if the by-name path resolved in metadata-protocol instead — it does not. The gate is re-derived in rest-server.ts, and the cached read path is excluded for isAppType, so every app-typed by-name request already takes the uncached branch that holds this gate. No metadata-protocol change, and no second file edited "to be safe".

Only the meta-apps region is touched, so this does not collide with #8071 (/security/suggested-bindings) in the same file. Rebased onto main before push.

Acceptance criteria

All four are pinned in packages/rest/src/meta-app-publish-gate.test.ts, which already owned this route's response-body facts. Every case asserts statusandcode (ADR-0112) — never "an error came back", since both answers under test are errors one apart.

  1. Session without the capability ⇒ 403 + PERMISSION_DENIED, no document and no leaked object names.
  2. Session with the capability ⇒ app served unchanged, navigation included.
  3. Nonexistent name ⇒ absence, in both shapes that reach this route: the stub's undefined, and the producer's declared RESOURCE_NOT_FOUND / 404 rejection that metadata-protocol actually throws.
  4. List route read and asserted absent for the unauthorized session — not "the endpoint still 200s". No authorized key, no PERMISSION_DENIED, no trace of the withheld app; and the holder still receives it.

Plus two partition pins: unpublished-and-permission-gated stays 404, and absent-service stays 404.

Reverse verification

Five reversals, each confirming the pin it targets goes red for the expected reason:

reversalpin that reddenedfailure text
R1 — restore the pre-fix single 404criterion 1expected 404 to be 403
R2 — drop the visible guard, infer the reason from nullcriterion 3expected 403 not to be 403
R2b — map the producer's miss onto a denialcriterion 3 (production shape)expected 403 to be 404
R3 — list route annotates authorized: false instead of droppingcriterion 4expected [ 'account', 'crm', 'finance', …(1) ] to not include 'finance'
R4 — deny the capability holder toocriterion 2expected 403 to be 200
R5 — naive "any null ⇒ 403"criterion 3 + both partition pins + the pre-existing #4829 pinexpected 403 to be 404 ×3, expected 403 not to be 403

R5 is the one that matters most: the naive implementation reddens four pins at once, including #4829's own SINGLE ITEM: the unpublished app 404s for a non-builder.

Criteria 2, 3 and 4 stay green under R1, and that is the correct direction rather than a gap — they pin behavior the fix deliberately leaves unchanged, so removing the fix cannot break them. Each has its own reversal above (R4, R2/R2b, R3).

Restored afterwards and confirmed byte-identical (git diff matches the saved patch exactly).

Gates

All run on the rebased tree (post-#8071, post-#8088), with the dependency closure built first.

  • pnpm --filter @objectstack/rest test101 files, 1678 tests, all passing.
  • pnpm --filter @objectstack/rest typecheck — clean.
  • pnpm check:type-check-debt — OK, "none above its recorded number"; @objectstack/rest stays at its recorded 155 and is absent from the surplus list, i.e. measured exactly at the ceiling. The ledger is not raised.
  • pnpm check:route-envelope — green including --self-test; rest-server.ts dialect counts unmoved at stringError 44 / siblingCode 75 (75 is fix(rest): one error envelope across the three /security/suggested-bindings routes (#7981) #8071's freshly-lowered baseline, which this change does not disturb). The refusal is written through the sharedsendError (@objectstack/types), imported as sendEnvelopeError because this module has a local sendError of its own — the sanitizing responder for thrown errors, which is a different thing. The body literal lives in the pinned SHARED_BUILDER, so no write site is added here.
  • node scripts/check-nul-bytes.mjs — OK, plus a wider self-scan of the changed files.

⛔ Out of scope, as the card's Links section requires: #7912 (filterAppForUser never consults enable.apiEnabled) is not folded in.


Generated by Claude Code

… absence
`GET /api/v1/meta/app/<name>` collapsed three different refusals into one
404-equivalent, so an app the session may never open and an app that does
not exist were byte-identical on the wire. The console has nothing to
branch on and renders its only copy for an absent app -- "it may still be
publishing" -- over a permanent authorization denial. Measured cost on
objectui#4252: two acceptance-test batches chasing a "platform defect"
that was a missing permission-set binding.
`filterAppForUser` now delegates to `filterAppForUserWithReason`, which
reports WHICH gate fired. Exactly one of them converts: an app that
EXISTS and whose `requiredPermissions` the session lacks answers 403
`PERMISSION_DENIED` (ADR-0112 standard catalog) in the declared envelope,
written through the shared `sendError` from `@objectstack/types`.
Everything else keeps answering absence, and the reason is taken from the
branch that fired rather than inferred from `null` at the call site:
- `_unpublished` -- ADR-0045 section 3 makes it externally unobservable,
and a 403 confirms existence (#4829 pinned 404-over-403);
- `requiresService` -- ADR-0057 D10 capability absence is a deployment
fact, not a denial to this caller;
- a nonexistent name -- converting it would make every app name on the
platform enumerable, a different and unruled change;
- the list route `GET /meta/apps` stays filtered exactly as-is, with no
`authorized: false` leakage.
All four acceptance criteria pinned in meta-app-publish-gate.test.ts on
`status` AND `code`, plus the two partition cases. Five reversals confirm
each pin goes red for its own reason; the naive "any null => 403" reddens
four at once, including #4829's own unpublished-app pin.
Fixes#8013
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 5:30pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/connect-mcp.mdx(via @objectstack/rest)
  • content/docs/api/error-handling-server.mdx(via @objectstack/rest)
  • content/docs/api/index.mdx(via @objectstack/rest)
  • content/docs/permissions/authentication.mdx(via @objectstack/rest)
  • content/docs/permissions/system-context.mdx(via packages/rest)
  • content/docs/plugins/index.mdx(via @objectstack/rest)
  • content/docs/plugins/packages.mdx(via @objectstack/rest)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/rest)
  • content/docs/protocol/kernel/i18n-standard.mdx(via packages/rest)

3 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/rest)
  • content/docs/releases/v12.mdx(via @objectstack/rest)
  • content/docs/releases/v17.mdx(via @objectstack/rest)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024): accepted. Enqueueing once CI is green.

Exactly one refusal converts, and each of the other four is refused on its own reasoning

This is the card's whole risk. Making an app's existence observable to a caller who may not open it is a deliberate, ruled disclosure — and it is licensed only for the by-name route on an app that exists and whose requiredPermissions the session lacks. The table holds that line, and every non-conversion is argued rather than left alone by default:

  • _unpublished stays absent — ADR-0045 §3 makes an unpublished app externally unobservable, and a 403 confirms existence. rest: filterAppForUser treats app hidden flag as builder-only access gate — built-in account app returns 404 for all normal users #4829 has pinned the 404-over-403 choice since it landed.
  • requiresService absent stays absent — an absent optional service is a deployment fact about the platform, not a statement about this caller. Nothing was denied to the session.
  • A nonexistent name stays absent — converting it would make every app name on the platform enumerable. Different change, unruled.
  • The list route stays filtered — no authorized key, no widening of the enumeration surface.

And the ordering is called out as load-bearing for the right reason: _unpublished is judged first, so an app that is both unpublished and permission-gated reports absence. Get that order backwards and the unpublished gate leaks through the new door.

The structural point underneath it all:

The reason comes from the branch that fired, never inferred from app == null at the call site.

That is what makes the partition real rather than incidental, and it is exactly what R5 proves.

R5 is the reversal that earns the suite

Five reversals, each aimed at a named pin — but R5, the naive "any null ⇒ 403", is the implementation a hurried developer would actually write, and it reddens four pins at once, including #4829's own pre-existing SINGLE ITEM: the unpublished app 404s for a non-builder. A suite that only tested "unauthorized gets 403" would have passed that implementation and shipped the enumeration hole.

Criterion 3 is pinned in both shapes that reach this route — the stub's undefinedand the producer's declared RESOURCE_NOT_FOUND/404 rejection that metadata-protocol actually throws. A single-shape pin would have covered the test double and missed production.

Criterion 4 is asserted the way I asked: the list response is read and the app asserted absent for the unauthorized session, not "the endpoint still 200s."

And criteria 2/3/4 staying green under R1 is explained rather than hidden — "they pin behavior the fix deliberately leaves unchanged, so removing the fix cannot break them" — with each carrying its own targeted reversal (R4, R2/R2b, R3). Reporting the greens and naming the property is the discipline; a suite where every pin reddens under one reversal is usually a suite testing one thing.

The landing question was answered, not assumed

The card asked you to report a corrected landing if the by-name path resolved in metadata-protocol. It does not — the gate is re-derived in rest-server.ts, and the cached read path is excluded for isAppType, so every app-typed by-name request already takes the uncached branch holding this gate. Reported with the reason, no metadata-protocol change, and no second file edited "to be safe." That last part matters: editing both places to be certain is how a file surface silently doubles.

The sendEnvelopeError import is a good catch — this module has its own local sendError (the sanitizing responder for thrown errors, a different thing), so aliasing the shared writer avoids a collision that would have been confusing rather than broken. Envelope counts unmoved at stringError 44 / siblingCode 75, correctly noting 75 is #8071's freshly-lowered baseline that you rebased onto.

For objectui#4252

The code to branch on is error.code: "PERMISSION_DENIED" / status: 403, and the choice is right: it is the ADR-0112 standard-catalog member for a generic authorization refusal rather than a registered synonym, it is what standardErrorCodeForHttpStatus(403) answers, and — the part that matters for the console — it sits at body.error.code, the same accessor as the neighbouring absence answer. One access path, not two. objectui#4252 is unblocked the moment this merges.

Not raised: @objectstack/rest measured exactly at its recorded 155 and absent from the surplus list — the repo's only zero-margin ceiling, held. 101 files / 1678 tests pass. #7912 correctly not folded in.

Flipping ready and enabling auto-merge once CI converges — both steps.


Generated by Claude Code

@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlong
hotlong marked this pull request as ready for review August 12, 2026 17:38
@hotlong
hotlong enabled auto-merge August 12, 2026 17:38
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 6ceffe0Aug 12, 2026
26 checks passed
@hotlong
hotlong deleted the claude/issue-8013-meta-app-by-name-denied-envelope branch August 12, 2026 17:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@hotlong@claude