Repository navigation
Conversation
A realm's own _search admits a caller who isn't signed in where its policy opens query to such callers, scoped by the anonymous grants that don't read actor(), and counts the search once scoped. _federated-search admits each unreadable realm that opens query to the caller's address, counts each realm's budget separately, and marks the result incomplete where a realm turns the caller away. Capability checks and explain answer search questions for such callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 108d380313
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mounted = await resolveRealmsForFederatedRequest( | ||
| locate.reconciler, | ||
| asked, | ||
| ); |
There was a problem hiding this comment.
Cap anonymous fan-out before mounting realms
A credential-less caller can submit an ad-hoc federated request naming an arbitrarily large set of registered private realms; the only realm-count limit is client-side (packages/runtime-common/search-bounds.ts:34-35), while this path concurrently lazy-mounts every realm whose config may name a policy before determining whether any policy actually permits anonymous queries. Consequently, an unauthenticated request can force widespread realm startup and policy compilation—even when all of those policies ultimately deny it—creating a resource-exhaustion path and defeating lazy mounting. Bound this admission fan-out or use a cheap pre-mount anonymous-admission signal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. Anonymous admission is now bounded before anything is mounted:
- It runs only for an ad-hoc search whose payload names a type some grant could be on.
- Archived realms are dropped first.
- A request naming more distinct private realms than
MAX_REALMS_PER_SEARCH_REQUESTadmits none and gets today's 401, so a caller who isn't signed in can make the server mount and ask at most that many realms per request, the same bound a client search obeys. - A realm whose
realm.jsonnames no policy is still never mounted.
Pinned by "a search naming more realms than a search may fan out to is told to authenticate, and no realm is asked".
| (await realm.countAnonymousSearch(anonymous.request, caller)) | ||
| .kind !== 'counted' |
There was a problem hiding this comment.
Parse the query before charging anonymous realms
For a federated payload with a valid item.on anchor but another invalid member, such as a malformed page, searchInvocation supplies types and this call charges the anonymous budget; only afterward does respond call parseSearchEntryQueryFromPayload and return 400. Repeated rejected requests can therefore exhaust the address's allowance without executing a valid search, unlike the realm-local path which marks the request served only after parsing and scoping. Validate the complete search payload before charging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. For a request that authenticated nobody, handle-search now parses the whole search payload before policyAccess runs. A malformed one gets the same 400 the later parse gives (now shared as respondToInvalidSearch), and no realm counts it. Pinned by "a malformed search is refused before any realm counts it".
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Independent review of this PR's own diff (head 108d380, against the anonymous-reads base).
Security: no blocking issues found. I checked:
- Grant filtering.
grantFiltersdrops every grant that doesn't opt in, and every grant whosewherereadsactor(), for a principal with no actor. If one got through,lowerQueryOperationwould throwactor-requiredanyway, so the rule holds twice over. - Federated path. Only realms that admit the caller become grant candidates (filtered in the middleware and checked again by
anonymous.callers.get(url)inpolicyAccess). - Blocked addresses. A blocked or undetermined address is never admitted.
- Named queries. A named query on
_federated-searchgets a 401. The realm-route difference is noted inline. 'refuse'endpoints._federated-info,-typesand-index-countsstill send the same 401 message, with no mount and no admission.- The anonymous principal elsewhere. It reaches no other
SearchPrincipalconsumer in a widening way.resolveNamedQuerytreats it as no actor. The capability check's query lane follows the same grant rule asanonymousSummary. Explain treats onlyactor.kind === 'user'as a user.realm-authoritypaths are unchanged. - Client address. It comes from the
clientAddressmiddleware, which strips spoofed headers, and the federated request is built from the same headers. - Response to unauthenticated callers. It doesn't distinguish a realm with a policy from one without (both get the same 401). Only timing differs.
Cost (DoS). For an unauthenticated request that names N realms it can't read, the middleware now:
- reads
realm.jsonfor each realm that isn't mounted; - mounts every realm whose
realm.jsonnames a policy; - runs a
remainingbudget probe for each realm whose policy opensquery.
None of that is charged when the search scopes nothing, for example when the filter has no type anchor. The set of mounts equals what a signed-in caller's search already mounts in policyAccess, and N is unbounded on both paths. So the cost is comparable to the existing signed-in path. The new marginal cost is one uncounted DB read per opted-in realm per request. That seems acceptable. One optional improvement: skip admission when searchInvocation(payload) has no types, since nothing could be scoped then.
Charging. Each scoped realm search is charged exactly once, after scoping, against that realm's own budget. Denied scope costs nothing. A realm that turns the caller away or can't read its count is failed, which gives meta.incomplete. The realm's own route keeps its 429. Re-reading the body in handle-search is safe because it's cached in REQUEST_BODY_STATE. Signed-in requests and realms without anonymous grants take exactly the old path: anonymousCallers is undefined, there's no extra request build, and caller is undefined throughout policyAccess.
Findings (inline):
- Low: an archived realm is asked for admission before the archived filter runs, which turns a 401 into an empty 200.
- Low: an exception from one realm's
admitAnonymousSearchfails the whole federated request with a 500. - Low: a named query on a realm's own
_searchis now admitted and answered (200 with no rows, a 400, oractor-required) instead of the 401 that_federated-searchgives. - Question: anonymous ad-hoc searches serve each row's full link closure and HTML.
- Nit: a duplicated realm is charged twice.
- Nit: a duplicated sentence in the
grantFilterscomment.
Comment accuracy (outside the diff). handle-search.ts around lines 225–236 still says "No policy admits a request that authenticated nobody" and "the request has no one a policy could admit". That's now only true of named queries, so it's worth rewording.
Test gaps:
- an archived realm that opts in;
- an anonymous call to
_federated-info(a'refuse'endpoint) naming a realm that opts in, to pin the unchanged 401; - a mix of a public realm and an opted-in realm in one federated search;
- a named query on the realm's own
_search; - the capability check's negative cases (LIBRARY, and a type covered only by the actor grant);
- a fixture with a link, to pin what
included[]carries for anonymous callers.
Fixture IPs are all RFC 5737, and I found no ticket or PR references in code comments.
| ]; | ||
| anonymousCallers = | ||
| unreadableRealms === 'carry' && !isNamedQueryPayload(payload) | ||
| ? await admitAnonymousSearch(realmsRequiringAuth, request, { |
There was a problem hiding this comment.
[Claude Code 🤖] Low: an archived realm can admit the caller here, which turns the old 401 into an empty 200.
admitAnonymousSearch runs over realmsRequiringAuth, and that list includes archived realms. archivedRealmURLs only filters grantCandidates further down (line 341). So when the only realm that admits the caller is archived, anonymousCallers.size > 0, the request skips the 401, and grantCandidates then drops that realm. The caller gets a 200 with no rows where it used to get a 401. Along the way the sealed realm is mounted, its budget is probed with remaining, and a blocked or rate-limited telemetry line is recorded for a realm that will never be searched. It also tells an unauthenticated caller that the archived realm's policy opens query.
Suggested fix: run archivedRealmURLs on realmsRequiringAuth first and only ask the realms that aren't archived. The archivedRealmURLs call at line 341 can then reuse that set. A test with an archived realm that opts in would pin the behavior.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. Archived realms are filtered out before admission (notArchived), so an archived realm is never mounted, asked, counted or recorded, and a request whose only admitting realm is archived still gets the 401. Pinned by "an archived realm admits nobody to a search".
| let admitted = new Map<string, AnonymousCaller>(); | ||
| await Promise.all( | ||
| mounted.map(async (realm, index) => { | ||
| let caller = await realm?.admitAnonymousSearch(request); |
There was a problem hiding this comment.
[Claude Code 🤖] Low: if one realm throws during admission, the whole federated request fails with a 500.
Each step before this call is guarded per realm: mayNameRealmPolicy catches its errors, and resolveRealmsForFederatedRequest uses allSettled. realm.admitAnonymousSearch is not guarded. It reads getRealmPolicy, getAnonymousAdmission and getAnonymousAccess, and only the rate limiter's error is caught inside it. If any of those throws, Promise.all rejects and the middleware throws, which fails the request for public realms and other admitting realms too. policyAccess in handle-search handles the same situation by counting the realm as failed.
Suggested fix: wrap each realm's call in a try/catch that logs the error and leaves that realm out of the map. That realm then admits nobody, which matches "a realm that won't mount admits nobody".
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. Each realm's admitAnonymousSearch is caught on its own. A realm that throws is logged and admits nobody, and the other realms still answer, which matches how policyAccess treats a realm that can't be judged.
| if ( | ||
| anonymous && | ||
| caller && | ||
| (await realm.countAnonymousSearch(anonymous.request, caller)) |
There was a problem hiding this comment.
[Claude Code 🤖] Nit: a realm named twice in realms is charged twice for one search.
parseRealmsFromPayload doesn't remove duplicates. named.filter(candidates.has) keeps them, and so does realmsThatMayNamePolicy. mountRealms reuses the mount, but this Promise.all still runs policyQueryScope and countAnonymousSearch once for each entry. The cost falls only on the caller's own address, so it isn't a security issue. It does break "counted one unit per realm search". Removing duplicates from the candidates before policyAccess (or from asked here) fixes it.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. The grant candidates handed to policyAccess are de-duplicated, and so is the admission list in the middleware, so a realm named twice is asked and counted once. Pinned by "a realm named twice is counted once".
| APPLIES_ARCHIVED_SEAL, | ||
| { | ||
| ...APPLIES_ARCHIVED_SEAL, | ||
| anonymous: ANONYMOUS_SEARCH, |
There was a problem hiding this comment.
[Claude Code 🤖] Low: a named query on a realm's own _search is admitted and answered, while _federated-search refuses it with a 401.
This dispatch admits any unauthenticated QUERY whenever the realm's policy opens query, whatever the payload is. searchEntriesResponse then calls resolveNamedQuery with the anonymous principal, and #policyQueryScope judges the named operation, which no anonymous grant can cover, so the result is denied. The caller gets one of three answers:
- a 200 with no rows;
- a 400 explaining what is wrong with the declaration;
- a 401
actor-requiredif the declaration readsactor().
Before this change the answer was always the plain 401. The federated middleware refuses the same payload outright (isNamedQueryPayload), and the description says only an ad-hoc search under query is ever opened. Even with no rows, an unauthenticated caller can make the realm resolve declarations for free.
Suggested fix: in searchEntriesResponse, answer requestContext.anonymousCaller with the unauthenticated 401 when isNamedQueryPayload(payload), before resolveNamedQuery runs. Then add a realm-route case next to the federated named-query test.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. A realm's own _search answers a named-query payload from a caller admitted without a session with the unauthenticated 401 before the declaration is read, matching _federated-search. Pinned by "a named query on a realm's own search tells such a caller to authenticate".
| }; | ||
| // A search of the realm: what a `query` grant opens. It is counted once its | ||
| // grants have scoped it, whatever rows it finds. | ||
| const ANONYMOUS_SEARCH: AnonymousDispatch = { |
There was a problem hiding this comment.
[Claude Code 🤖] Question / decision: anonymous search results carry each row's full link closure.
An ad-hoc search declares no link strategy, so effectiveLinkStrategy(undefined, …) is full. Under the current rule that "a grant on a query admits everything its rows' representations carry" (see realm-policy-query-reach-test's default case), each row's whole closure in included[] and its prerendered HTML go to anyone. That includes linked cards (for example an Author linked from a published Article) that no anonymous read grant admits. For signed-in callers this exposure reached a named user. With this change it reaches the public internet, and an ad-hoc search has no declaration that could narrow it.
Is that the intended contract for anonymous query grants? If it is, it would help to state it in the authoring skill and pin it with a fixture that has a link. If it isn't, one option is to cap anonymous searches at ids.
There was a problem hiding this comment.
[Claude Code 🤖] Intended: an anonymous search serves what a signed-in search through the same grant serves. A query grant admits its rows with each row's whole link closure, the same way a granted read serves the whole representation (security invariant 7 in the policy design). How far a grant reaches past its rows is an authoring constraint, documented at the top of policy-query.ts. A narrower reach is declared on a named query's links, for every caller alike, since a response's shape never depends on how its caller was authorized. I've asked for the authoring skill's anonymous section (boxel-skills#174) to tell authors to check what an opted-in type links to before opening it. No code change.
| // grant that opts in to such callers matches, and never one whose filter reads | ||
| // the caller, which has no one to stand for. | ||
| // | ||
| // Each filter is in the grammar the engine runs, each comparison in it kept from judging a card whose type reads |
There was a problem hiding this comment.
[Claude Code 🤖] Nit: this sentence repeats "in the grammar the engine runs" from the paragraph above, and the line runs past the wrap width. Could the two paragraphs be merged, or this one rewrapped?
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 02a6396f12. The two paragraphs are merged and rewrapped.
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…d queries A request that authenticated nobody asks at most as many unarchived realms as a search may fan out to, and only for an ad-hoc search that names a type. A realm that throws while admitting admits nobody. The query is parsed before any realm counts it, a realm named twice is counted once, and a named query on a realm's own _search is told to authenticate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Host Test Results 1 files ± 0 1 suites ±0 6m 59s ⏱️ - 1h 46m 4s Results for commit 78b7a94. ± Comparison against earlier commit e29864e. Realm Server Test Results 1 files ±0 316 suites ±0 2h 3m 4s ⏱️ + 5m 29s Results for commit 78b7a94. ± Comparison against earlier commit e29864e. |
…reads-through-a-grant-that-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…reads-through-a-grant-that-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that # Conflicts: # packages/runtime-common/realm.ts
…reads-through-a-grant-that-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
…hat-opts-in' into cs-13578-policy-admit-anonymous-searches-through-a-query-grant-that
Important
Merge order (each must merge before the next):
Compile a policy grant's opt-in to callers who aren't signed in #6534: compile a grant's anonymous opt-in (merged)Policy: admit anonymous reads through a grant that opts in, rate-limited per realm and address #6568(CS-13536): anonymous reads (merged)4a. Policy: let a grant on a declared operation admit a caller who isn't signed in #6579 (CS-13605): anonymous named operations, stacked on Policy: admit anonymous writes as a user the realm's config names #6577. boxel-skills#174's §12 describes it
4b. Policy: explain how a realm treats a caller who isn't signed in #6581 (CS-13609): explaining a visitor's refusal, stacked on Policy: let a grant on a declared operation admit a caller who isn't signed in #6579. boxel-skills#174's §10/§12 describe it
Add the judgment calls to the realm-policy-authoring skill boxel-skills#175(CS-13108): merged, and merged into boxel-skills#174. This section is §12, Refusals §13, its section §14 and the checklist §15.5a.
Say that no policy grant reaches a path the realm ignores boxel-skills#178(CS-13336): merged, and merged into boxel-skills#1747a. Say that a policy's rules are not secret from the writers of other realms boxel-skills#181 (CS-13398): a realm-policy-authoring §9 bullet, stacked on z-index handling for modals opening modals #179, merges right after it
Every PR in this order carries this callout. Whoever opens one of the PRs above tells the owners of the others, and each owner updates its own callout.
In service of CS-13521, which lets a realm's policy admit a caller who isn't signed in: an
OperationGranton a base operation opts in withanonymous: true, an anonymous write is made as a user the governed realm names, and the governed realm limits and blocks such callers by address. Anonymous reads switched that on for reads. This PR does the same for searches.What a search composes for a caller who isn't signed in.
SearchPrincipalgains{ kind: 'anonymous' }.policyQueryScopejudges such a principal only by the matching grants that opt in to anonymous callers, and never by one whosewherereadsactor(), since that would have no one to stand for. Everything else about scoping is unchanged: only grants on the invoked operation that compiled a filter, confined per type, and never the realm's authorization cards. A named query is granted by its own name, which no grant can open to such callers (anonymous-not-base-operation), so only an ad-hoc search underqueryis ever opened, and a named-query payload from such a caller is answered with the unauthenticated 401 on both routes. A visitor's search, like a visitor's read, serves each admitted row's whole link closure, as it does for a signed-in caller through the same grant.A realm's own
_search. The route is tagged with thequeryoperation, so the realm admits a caller who isn't signed in exactly as it admits a reader: its policy opensqueryto such callers, the address is determined and not on the blocklist, and the address has budget left. Anything else gets today's 401. An admitted search runs with the anonymous grants' filters composed in, and is counted one unit once those grants have scoped it, whatever rows it finds. A search no grant scopes answers no rows and costs nothing. Over the limit, the answer is the 429 the read routes give._federated-search. A request that authenticated nobody and names a realm it can't read is no longer refused outright. The middleware asks each such realm, throughRealm.admitAnonymousSearch, whether it admits the caller. Only an ad-hoc search that names a type is asked about. Archived realms are never asked. A request naming more distinct private realms thanMAX_REALMS_PER_SEARCH_REQUEST(the client fan-out bound) admits none, so an unauthenticated request can make the server mount and ask at most that many realms. A realm whoserealm.jsonnames no policy isn't mounted to ask, and a realm that throws while answering admits nobody without failing the others. A realm that admits the caller becomes a grant candidate, with the caller it admitted, and the rest contribute no rows, as they would for a signed-in caller. If no realm admits the caller, the answer is today's 401. A named query is never opened this way. Inhandle-search, the query is parsed before any realm counts it, a realm named twice is counted once, and each admitted realm counts the search against the caller's budget there, once its grants have scoped it (Realm.countAnonymousSearch). A realm whose budget is spent, or whose count can't be read, is counted failed: its rows are withheld, the result saysmeta.incomplete, and the other realms still answer. Realms never share a budget.Capability checks and explain. The capability check's query lane answers a caller who isn't signed in as the search would, through the same principal. Explain judges such a caller's search question by the grants that opt in to them where the policy opens the operation, and answers
actor-requiredeverywhere else.Telemetry. Each realm records its own
anonymous-requestline for a search (admitted,infra,blockedorrate-limited), as it does for reads. A federated search records one line per realm it reads, with that realm's limit and count.Tests:
realm-policy-anonymous-search-test.tscovers:actor()admitting nothing;meta.incomplete;The filter, capabilities and explain policy tests run unchanged.
🤖 Generated with Claude Code