Skip to content

Omit archived realms from _realm-auth and active-realm enumeration (CS-11665)#5358

Merged
lukemelia merged 2 commits into
mainfrom
cs-11665-enumeration-omits-archived-realms-from-_realm-auth-realm
Jun 29, 2026
Merged

Omit archived realms from _realm-auth and active-realm enumeration (CS-11665)#5358
lukemelia merged 2 commits into
mainfrom
cs-11665-enumeration-omits-archived-realms-from-_realm-auth-realm

Conversation

@lukemelia

@lukemelia lukemelia commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Archived realms must never surface in a user's active chooser / pickers, for non-owners and owners alike. Adds an archive filter to fetchUserPermissions so the realm enumeration path skips rows whose 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) look up a single realm by URL via fetchRealmPermissions, and fetchArchivedRealmsForOwner queries archived rows directly. Both bypass the enumeration filter so the archive flow stays reachable.

An includeArchived opt-in is exposed on fetchUserPermissions for callers that need archived realms in the enumeration; the explicit flag keeps the surface area honest.

Effect on federated search

multi-realm-authorization middleware uses the same fetchUserPermissions, so archived realms in a federated payload classify as "Insufficient permissions" and get 403'd before handle-search runs. No handler-level filter is needed in handle-search.

Tests

  • queries-test: archive realms across both UNION arms (owner direct + * public) and across onlyOwnRealms; assert default vs. includeArchived flips the result. 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.

Test plan

  • pnpm test in packages/realm-server (queries-test, realm-auth-test) — requires Docker for the test PG; will verify in CI.

@lukemelia lukemelia marked this pull request as ready for review June 29, 2026 17:02
@lukemelia lukemelia requested review from a team and jurgenwerk June 29, 2026 17:05
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>
@lukemelia lukemelia force-pushed the cs-11665-enumeration-omits-archived-realms-from-_realm-auth-realm branch from 013586f to f064e56 Compare June 29, 2026 17:19
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 includeArchived opt-in to fetchUserPermissions, defaulting to excluding archived realms from enumeration queries.
  • Thread the archive filter through both UNION arms (user permissions + public-read *) and the onlyOwnRealms branch.
  • Add realm-server tests covering archive → omitted → unarchive → re-enumerated behavior for both _realm-auth and fetchUserPermissions.

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.

Comment thread packages/runtime-common/db-queries/realm-permission-queries.ts
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   2h 33m 40s ⏱️
3 319 tests 3 304 ✅ 15 💤 0 ❌
3 338 runs  3 323 ✅ 15 💤 0 ❌

Results for commit 55901ae.

Realm Server Test Results

    1 files      1 suites   8m 38s ⏱️
1 673 tests 1 673 ✅ 0 💤 0 ❌
1 752 runs  1 752 ✅ 0 💤 0 ❌

Results for commit 55901ae.

@lukemelia lukemelia merged commit df385d6 into main Jun 29, 2026
66 of 67 checks passed
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants