Omit archived realms from _realm-auth and active-realm enumeration (CS-11665)#5358
Merged
lukemelia merged 2 commits intoJun 29, 2026
Conversation
1 task
lukemelia
pushed a commit
that referenced
this pull request
Jun 29, 2026
The original CS-11666 plan filtered archived realms inside handle-search. That filter is subsumed by the enumeration filter added in CS-11665 (#5358): once a realm is archived, the requester has no permission for it per fetchUserPermissions, so multi-realm-authorization 403s the request before handle-search runs. Drop the redundant handler- level filter and the bulk fetchArchivedRealmURLs helper that fed it. Keep one integration test on the federated-search endpoint to pin the user-facing contract: a federated search payload that names an archived realm gets 403, and the same request restricted to active realms still searches normally. The test exercises the full stack (handle-search → multi-realm-auth → fetchUserPermissions → realm_metadata.archived_at) so a future refactor of the enumeration layer can't silently weaken it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an archive filter to fetchUserPermissions so the realm enumeration path skips realms with realm_metadata.archived_at IS NOT NULL by default. The filter is threaded into both arms of the false-branch UNION (the user's direct rows and the '*' public-read rows) and into the onlyOwnRealms branch, so every caller — _realm-auth, multi-realm authorization, the indexer sweep, publish-realm, definition-lookup, realm-readability, the screenshot/run-command tasks — sees only active realms. The archive-management endpoints (POST /_archive-realm, POST /_unarchive-realm, the upcoming GET /_archived-realms) intentionally look up a single realm by URL via fetchRealmPermissions / fetchArchivedRealmsForOwner instead, so they continue to reach archived realms despite the new default. Add an includeArchived flag for callers that need the old behavior; no caller in the tree currently sets it, but the explicit opt-in keeps the surface area honest. This simplifies CS-11666: multi-realm authorization now refuses archived realms in a federated payload (the user has no permission per the filtered enumeration), so handle-search's own archive filter becomes redundant. Tests: - queries-test: archive realms across both UNION arms (owner direct + '*' public) and across onlyOwnRealms; assert default vs. includeArchived. Round-trip: archive → omitted → unarchive → re-enumerated. - realm-auth-test: POST /_realm-auth returns the realm → archive → response omits it → unarchive → response includes it again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
013586f to
f064e56
Compare
lukemelia
pushed a commit
that referenced
this pull request
Jun 29, 2026
The original CS-11666 plan filtered archived realms inside handle-search. That filter is subsumed by the enumeration filter added in CS-11665 (#5358): once a realm is archived, the requester has no permission for it per fetchUserPermissions, so multi-realm-authorization 403s the request before handle-search runs. Drop the redundant handler- level filter and the bulk fetchArchivedRealmURLs helper that fed it. Keep one integration test on the federated-search endpoint to pin the user-facing contract: a federated search payload that names an archived realm gets 403, and the same request restricted to active realms still searches normally. The test exercises the full stack (handle-search → multi-realm-auth → fetchUserPermissions → realm_metadata.archived_at) so a future refactor of the enumeration layer can't silently weaken it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fetchUserPermissions' public arm returns every public realm in the DB, including the bootstrap realms in the seed template, so an exact key-set match fails in CI. Assert presence/absence of the four realms the test seeds in each enumeration mode instead, preserving the both-UNION-arms coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the realm-permissions enumeration path to omit archived realms by default, ensuring archived realms do not appear in active realm pickers and related flows (e.g. _realm-auth, multi-realm authorization), while still allowing explicit opt-in enumeration when needed.
Changes:
- Add an
includeArchivedopt-in tofetchUserPermissions, defaulting to excluding archived realms from enumeration queries. - Thread the archive filter through both UNION arms (user permissions + public-read
*) and theonlyOwnRealmsbranch. - Add realm-server tests covering archive → omitted → unarchive → re-enumerated behavior for both
_realm-authandfetchUserPermissions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/runtime-common/db-queries/realm-permission-queries.ts | Adds default archive filtering to realm enumeration and exposes includeArchived opt-in. |
| packages/realm-server/tests/realm-auth-test.ts | Verifies _realm-auth omits archived realms and re-includes them after unarchive. |
| packages/realm-server/tests/queries-test.ts | Verifies fetchUserPermissions default exclusion + includeArchived behavior across both UNION arms and onlyOwnRealms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
backspace
approved these changes
Jun 29, 2026
lukemelia
added a commit
that referenced
this pull request
Jun 30, 2026
The branch was cut before the sibling archive PRs (#5353 _archived-realms, #5358 _realm-auth archived exclusion / CS-11665) merged, so it carried an older realm-server that did not omit archived realms from fetchUserPermissions. With that exclusion absent, _realm-auth still enumerated archived realms as archived:false, and `boxel realm list` deduped the _archived-realms entries against them — leaving the archived marker off and breaking the three realm-list-archived integration tests. Merging main restores the exclusion (and the rest of main's archive-server work this branch had reverted).
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
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.
Summary
Archived realms must never surface in a user's active chooser / pickers, for non-owners and owners alike. Adds an archive filter to
fetchUserPermissionsso the realm enumeration path skips rows whoserealm_metadata.archived_at IS NOT NULLby default. The filter is threaded into both arms of thefalse-branch UNION (the user's direct rows and the*public-read rows) and into theonlyOwnRealmsbranch, so every caller —_realm-auth, multi-realm authorization, the indexer sweep, publish-realm, definition-lookup, realm-readability, the screenshot / run-command tasks — sees only active realms.The archive-management endpoints (
POST /_archive-realm,POST /_unarchive-realm) look up a single realm by URL viafetchRealmPermissions, andfetchArchivedRealmsForOwnerqueries archived rows directly. Both bypass the enumeration filter so the archive flow stays reachable.An
includeArchivedopt-in is exposed onfetchUserPermissionsfor callers that need archived realms in the enumeration; the explicit flag keeps the surface area honest.Effect on federated search
multi-realm-authorizationmiddleware uses the samefetchUserPermissions, so archived realms in a federated payload classify as "Insufficient permissions" and get 403'd beforehandle-searchruns. No handler-level filter is needed inhandle-search.Tests
queries-test: archive realms across both UNION arms (owner direct +*public) and acrossonlyOwnRealms; assert default vs.includeArchivedflips the result. Round-trip: archive → omitted → unarchive → re-enumerated.realm-auth-test:POST /_realm-authreturns the realm → archive → response omits it → unarchive → response includes it again.Test plan
pnpm testinpackages/realm-server(queries-test,realm-auth-test) — requires Docker for the test PG; will verify in CI.