Skip to content

fix(plugin-hono-server): delegate permission-set resolution to the security service - #10035

Merged
os-elon merged 3 commits into
mainfrom
claude/issue-7616-delegate-permission-set-resolution
Aug 19, 2026
Merged

fix(plugin-hono-server): delegate permission-set resolution to the security service#10035
os-elon merged 3 commits into
mainfrom
claude/issue-7616-delegate-permission-set-resolution

Conversation

@os-elon

@os-elon os-elon commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7616

Step 2 of the card: /auth/me/permissions and /me/apps stop re-implementing
permission-set resolution and call the enforcement path's own resolution
instead. Step 1 (the contract method plus its exposure on the registered
security service) shipped in PR #7829 and is unchanged here — nothing in this
diff touches packages/spec or plugin-security.

What the endpoints used to do

Both handlers resolved the caller's sets themselves: composed the requested
names (positions ∪ explicit sets ∪ the deployment baseline), built their own
sys_permission_set DB loader, and called the permission evaluator directly
through the security.permissions handle. That is one rule in three copies —
the enforcement path's, and one per endpoint — and the contract's own header
names the consequence: a consumer that re-derives these answers locally "will
drift the moment the enforcement path changes."

It drifted three times, each divergence found only after it reached a user:
#7608 (both endpoints kept the fallback cliff the additive ADR-0090 D5 baseline
abolishes, so a member's first grant took them from 2 apps to 1), #7555 (an
app-declared isDefault set displaced member_default here rather than
composing with it), and #6334 (grant aggregation missed sys_user_position /
sys_position_permission_set entirely — retired by delegating to
resolveUserAuthzGrants, which is the precedent this extends one step).

Both call sites now call ISecurityService.resolvePermissionSetsForContext and
keep only the projection each surface needs.

The two absences — one dropped, one kept

The card's third constraint is that plugin-hono-server must not take a runtime
dependency on plugin-security, which is optional in the stacks these endpoints
serve. So the two absences are not the same absence:

absence disposition
SecurityPlugin present but too old to carry the method dropped — the method ships in @objectstack/spec@17.0.0, so the local copy that stood in for it is deleted rather than kept as a third path
No SecurityPlugin at all kept — the degraded branches are unchanged, reached through the service locator, never a package import

The contract declares the method optional, so the narrowing is also what the
type demands: the unguarded call does not compile. What changed is only what the
false branch does — it is now the degraded answer rather than a local
resolution.

Before/after on the wire — measured, not asserted

The card flagged that /me/apps deliberately projected a narrower column set
(name + systemPermissions + tabPermissions) than /auth/me/permissions,
and that delegating changes which columns load on both surfaces.

Measured by booting the real SecurityPlugin over a fake engine and running
both builds of plugin-hono-serverorigin/main and this branch — against
one service locator carrying everything a real stack registers, so each
build simply ignores the handles it does not read and both answers come out of
identical data.

fixture /auth/me/permissions /me/apps
zero-grant member (baseline only) identical (5358 B) identical (11 B)
one-grant member, baseline ∪ explicit (#7608) identical (5506 B) identical (76 B)
granted set deactivated (active: false) identical identical
granted set carrying the object/field columns /me/apps never projected identical identical
no SecurityPlugin at all identical (151 B) identical (248 B)

No additional field reaches the wire on either endpoint. /me/apps now
loads the sets whole, and the columns it gained are read by nobody in that
handler — the body is the app list and only the app list. That is pinned
directly, including a JSON.stringify(body) assertion that no permission-set
column leaks through the filter.

Three states do change, and all three are states where the UI plane previously
disagreed with the data plane:

1. A deactivated set whose name matches a live position name. The plugin's
loader drops deactivated sys_permission_set rows; the local copies did not.
Core already filters deactivated sets out of context.permissions, so this is
reachable only through the second reader — a position name reused as a set name.
Before, that row granted billing.manage, the billing tab and object/field
access here while granting nothing on the data plane; after, nothing on both.
Narrowing, fail-closed, and a fourth instance of exactly the drift this card
documents.

2. A permission set with a malformed JSON column. /auth/me/permissions
parsed with a bare JSON.parse, so one malformed column threw inside its loader
and cost the member the whole set; the plugin's loader degrades that column
alone. Note /me/apps was unaffected because its narrower loader never parsed
the column that was malformed — the two copies in this one file already
disagreed with each other about the same row.

3. SecurityPlugin present, start() bailed. This is a third state the
two-absence framing does not name, and it is the one worth review. The plugin
registers security.permissions in init() but the security service only in
start() — which returns early on an engine that cannot take middleware, and
that same early return is before any middleware is registered. So a stack in
that state enforces nothing on the data plane. Before, these endpoints
answered with a restrictive map and 1 of 3 apps, computed against enforcement
that does not exist — so the access they reported as withheld was not being
withheld by anything, and the console was describing a policy no layer applied.
After, they degrade, which is what the degraded branch's own premise ("matches
server behaviour when SecurityPlugin isn't registered") asks for.

This is a widening, so it is called out rather than folded in. The argument for
it is the one just given, on its own merits — it is not a recorded ruling about
which fail-direction is worse, and no such ruling is cited here.

Tests

packages/plugins/plugin-hono-server: 20 files / 225 tests pass, up from
215 — plus typecheck clean.

New: current-user-endpoints-delegated-resolution.test.ts pins the delegation,
both absences separately, the third state above, the wire measurement, and the
contract's fail-closed stance on a thrown resolution (absence fails open, a
runtime failure must not).

The two existing fixtures were re-dispositioned rather than mass-edited. Both
resolved through security.permissions, so both moved to the published service.
In the #7608 file the additive rule itself now lives in plugin-security, so
the cases that asserted the call shape were replaced by delegation pins, and the
case that pinned this file's own baseline fallback chain was replaced outright —
that chain is deleted, and an assertion about it would have kept passing while
measuring nothing. What replaces it is the property that makes the deletion
true: the locator carries the security.* internal handles by default and the
endpoints must never ask for them, with the security.permissions double
throwing if called.

Reverse-verified: with current-user-endpoints.ts reverted to origin/main and
the tests kept, 21 cases fail across the three files, and exactly the four
absence pins stay green — correct, since those degraded branches exist on main
too. Restored, re-confirmed 225/225, and the tree is byte-identical to the
commit. The tests import ./current-user-endpoints by relative path, so vitest
compiles the source directly: no dist and no exports resolution is involved,
and no rebuild gates this ablation.

Gates

Re-derived with node scripts/pm/dispatch-gates.mjs over the real change set
(no path arguments) and run at 6eb75a85f: check:cross-package-test-inputs,
check:route-envelope, check:slot-lookup, check:test-source-alias,
check:type-source-resolution, check:changeset-gate-self-tests,
check:objectui-changeset, check:query-options-erasure,
check:engine-double-contract, check:where-matcher,
check:type-check-coverage, check:type-check-debt, check:nul-bytes,
check-adr-0087-registration, check-changeset-no-major,
check-empty-changeset, docs-audit/check-affected-docs, and pnpm lint
(repo-wide, clean).

The derivation added the changeset and test-file convention families the
dispatch list could not name. One of them caught a real defect in the new
fixture: its WHERE matcher read a $-prefixed combinator as a field name, which
yields a silent false indistinguishable from a row that did not match. Fixed
by refusing loudly, as the sibling fixtures do — second commit.

Ratchets: type-check-debt --re-measure re-measured 33 ledger entries on the
built closure — "none above its recorded number ... surplus: none".
where-matcher and slot-lookup both report "none new" with the baseline key
set verified. Nothing weakened, no baseline moved, no test skipped.

Not touched

The card's related item — the stale doc-comment in plugin-security's
resolveFallbackPermissionSets — is already current on main: PR #7829
corrected it. Verified, no edit needed, and that file is another lane's anyway.


Generated by Claude Code

claude added 2 commits August 19, 2026 17:00
…curity service (#7616)

`/auth/me/permissions` and `/me/apps` re-implemented the caller's permission-set
resolution locally — composing the requested names, building their own
`sys_permission_set` DB loader and calling the evaluator directly. That made one
rule three copies, and it drifted from the enforcement path three times.

Both handlers now call `ISecurityService.resolvePermissionSetsForContext` on the
`security` service and keep only the projection each surface needs. The fallback
for a SecurityPlugin too old to carry the method is deleted (it ships in
@objectstack/spec@17.0.0); the degraded branches for a stack with NO
SecurityPlugin stay, reached through the service locator so no runtime dependency
on plugin-security is taken.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
…fake WHERE matcher

`check:where-matcher` judges every hand-written WHERE matcher in a test file:
reading a `$`-prefixed combinator as a field name yields a silent `false`, which
is indistinguishable from a row that legitimately did not match, so a case can
pass for the wrong reason. Refuse loudly instead, matching the convention the
sibling fixtures in this package already follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-hono-server, touching 13 documentable anchor(s).

15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx (via sys_permission_set (literal))
  • content/docs/kernel/services-checklist.mdx (via security.permissions (literal))
  • content/docs/permissions/access-recipes.mdx (via member_default (literal))
  • content/docs/permissions/administrator-guide.mdx (via member_default (literal))
  • content/docs/permissions/attachments-access.mdx (via member_default (literal))
  • content/docs/permissions/authorization.mdx (via sys_permission_set (literal))
  • content/docs/permissions/delegated-administration.mdx (via sys_permission_set (literal))
  • content/docs/permissions/index.mdx (via member_default (literal))
  • content/docs/permissions/permission-metadata.mdx (via member_default (literal))
  • content/docs/permissions/permission-sets.mdx (via member_default (literal), sys_permission_set (literal))
  • content/docs/permissions/permissions-matrix.mdx (via member_default (literal))
  • content/docs/permissions/positions.mdx (via member_default (literal))
  • content/docs/permissions/profiles.mdx (via member_default (literal))
  • content/docs/permissions/sharing-rules.mdx (via member_default (literal))
  • content/docs/ui/audience-based-interfaces.mdx (via member_default (literal))

6 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via member_default (literal), security.permissions (literal))
  • content/docs/releases/v12.mdx (via sys_permission_set (literal))
  • content/docs/releases/v13.mdx (via member_default (literal), sys_permission_set (literal))
  • content/docs/releases/v14.mdx (via member_default (literal))
  • content/docs/releases/v15.mdx (via member_default (literal), sys_permission_set (literal))
  • content/docs/releases/v17.mdx (via member_default (literal), sys_permission_set (literal))

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.

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d2bafad9f56f6ebc896270b65773af3f5a460079 — the merge of head 39144a231e7f331a55430483a5257a03b995868d into base 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d2bafad9f56f6ebc896270b65773af3f5a460079 && git checkout d2bafad9f56f6ebc896270b65773af3f5a460079
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96f 39144a231e7f331a55430483a5257a03b995868d && git checkout -B drift-repro 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96f && git merge --no-ff 39144a231e7f331a55430483a5257a03b995868d

node scripts/docs-audit/affected-docs.mjs --json 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 09b880b0ce9e2a9b140666ab4a2f75b0c55fe96f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…third-state comment

The comment argued that the bailed-start widening is right because the previous
answer was "the fail-direction #7608 names as the worse one". #7608 says the
opposite: it calls the UI-under-reporting direction the MILDER reading of its own
defect — "so it presents as 'the console hides things the API allows' rather than
as an exposure" — offered as the reassuring contrast to an exposure, not as a
grading of one direction against the other.

The argument itself is unaffected and stands on its own merits: in that state the
plugin registered no middleware, so the access the endpoint reported as withheld
was not being withheld by anything. It is a judgement under review, not a
recorded ruling, and the comment now says so.

Comment-only. No behaviour, no assertion, no conclusion changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@os-elon
os-elon marked this pull request as ready for review August 19, 2026 18:03
@os-elon
os-elon added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit c1731d0 Aug 19, 2026
26 checks passed
@os-elon
os-elon deleted the claude/issue-7616-delegate-permission-set-resolution branch August 19, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants