Skip to content

fix(security): an organization-less permission-set row grants again — #11121 revoked standing access silently - #11610

Merged
os-zhuang merged 1 commit into
mainfrom
fix/permission-set-residue-fallback
Aug 24, 2026
Merged

fix(security): an organization-less permission-set row grants again — #11121 revoked standing access silently#11610
os-zhuang merged 1 commit into
mainfrom
fix/permission-set-residue-fallback

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

What broke

#11121 tenant-scoped the request-time permission-set loader so two organizations
holding a row for the same name stop answering each other's requests. It shipped
the other half as a COMMENT:

// One row per NAME: this organization's own where it has one, an// organization-less leftover only where it does not.constown=resolveOwnOrganizationRow(...).own;// ← never a residueif(own)byName.set(name,own);

resolveOwnOrganizationRow is written for SEEDERS, and its docstring says so
plainly: an organization-less row "is reported separately and never returned
as the organization's own row
" — because reading it as "already seeded" is the
silent no-op the per-organization catalog exists to prevent. Enforcement needs
the opposite reading. The loader took the seeder's answer.

What it cost

Every walled deployment carrying pre-#11121sys_permission_set rows — or any
row authored without a tenant, which includes admin-UI-authored sets — silently
lost those grants on upgrade. The asymmetry is observable on a single row:

planeread pathafter #11121
system_permissions, tab_permissionsresolve-authz-context, unscoped, by idstill applied
object_permissions, admin_scopethis loadersilently stopped

One row, two enforcement planes, opposite verdicts, and the only signal was a
boot WARN about "leftovers" whose text states the organization's catalog is
complete.

The same commit states the no-silent-revocation rule twice — in
per-organization-catalog.ts's header ("grant TARGETS … deleting them revokes
standing access with no signal at the moment of loss") and at
resolve-authz-context.ts:537-540, where the sys_position read deliberately
keeps org-less rows for exactly this reason. This restores it in the third
place.

Found by

cloud's apps/ee-group-showcase dogfood suites, which had been failing four
ADR-0111 / ADR-0105 assertions on cloud main while turbo replayed them from
cache (cloud#1636). The suites hand-seed their catalog rows as system with no
tenant, so those rows land as residue the moment they are written — an exact
model of the customer case.

The change

One line of behaviour: own ?? organizationLessResidue. Preference order is
unchanged, so #11121's cross-tenant fix is intact — this organization's own row
still WINS wherever it exists; a leftover is consulted only in its absence.

Three cases added to resolve-permission-sets-for-context.pin.test.ts, which
#11121's own suite did not cover (it covers seeding and the sys_position
sweep):

  • an organization-less row resolves for a caller who has an organization, and
    resolves WHOLE (the dropped columns are asserted, not just presence);
  • when both exist, this organization's own row wins and the residue's broader
    grant does not leak in behind it;
  • the single-posture carve-out is untouched.

Reverting the one-line fix reddens exactly the first case, with the message
"the grant vanished for a caller in an organization — this is the silent
revocation". Verified both directions locally (8 passed with the fix; 1 failed /
7 passed without).

…11121 residue)
The per-organization loader read resolveOwnOrganizationRow().own alone,
which is never a residue once an organization is supplied — so every
walled deployment carrying pre-#11121 rows silently lost those grants on
upgrade, while the SAME row's system/tab permissions kept applying
through the unscoped by-id read.
The helper is written for seeders, where not seeing a residue as 'already
seeded' is the point; enforcement needs the opposite reading. This
implements the fallback the loader's own comment already promised. Own
still beats residue, so #11121's cross-tenant fix is intact.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 14 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 644ad5043ec0374003b574b4b863023ba91e01e1packageMentionDocs.

@os-zhuang
os-zhuang enabled auto-merge August 24, 2026 08:33
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-zhuang
os-zhuang added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit 502ff8bAug 24, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the fix/permission-set-residue-fallback branch August 24, 2026 09:06
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

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang