Repository navigation
Conversation
…xpressions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ymous-grant-flag-acting-user-key-and-realmjson # Conflicts: # packages/realm-server/tests/test-module-timings.json
…eration that reads the caller once A grant whose budget is spent stays eligible at admission, so a request only it would admit is answered by its charge as over the limit rather than as unadmitted. A grant that names the caller who isn't signed in on an operation that reads actor() is warned about once, against the operation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ymous-grant-flag-acting-user-key-and-realmjson # Conflicts: # packages/realm-server/tests/test-module-timings.json
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28497a967d
ℹ️ 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".
| id: computeContentHash( | ||
| stableStringify([ | ||
| card, | ||
| resolved, | ||
| operation, | ||
| outcome?.canonical ?? null, | ||
| ]) ?? '', |
There was a problem hiding this comment.
When two grants have the same policy card, target type, operation, and where but different blocklist, rate-limit, or actingUser expressions, this hash gives them the same ID. Admission later represents allowed grants as a set of IDs, so one allowed duplicate also makes a blocked duplicate eligible, while the rate limiter stores both grants in the same database bucket; the gate can consequently select the blocked grant's acting user and the grants spend each other's allowance. Include the anonymous-control expressions, plus a stable duplicate discriminator if necessary, in the identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Addressed in fc7c3c9a. The compiled identity includes the grant path, which is unique for each policy-grant position; grants with identical predicates and different anonymous controls therefore have separate eligibility entries and rate-limit buckets.
| if (outcome === 'holds' && judged && (await admitted(candidate, judged))) { | ||
| return candidate; |
There was a problem hiding this comment.
When multiple anonymous write grants have predicates that hold and an actingUser reading instance(), the first grant can fail its acting-user check here while a later grant admits the write. The trace has already recorded both predicates as held, however, and explain() subsequently selects the first grant whose trace outcome is held; it therefore reports the failed grant as admittedBy even though the later grant actually authorizes the write. Preserve the candidate that passes admitted() or distinguish an acting-user failure in the trace.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 8aa458fd. The pending-write rehearsal returns the exact grant that passed its predicate and acting-user check, and explain uses that grant for admittedBy.
There was a problem hiding this comment.
🟡 Changes recommended
Critical correctness issues remain in grant identity, capability scoping, and acting-user consistency.
6 open findings
What changed in this PR
This PR moves anonymous-access controls from realm-level settings into policy grants, adding BXL expressions for admission, blocklists, acting users, rate limits, and policy-card access.
Changes:
- Adds grant-based anonymous admission and per-grant rate limiting.
- Extends BXL, dispatch, search, telemetry, and explanation handling.
- Updates schemas, migrations, tests, catalog data, and documentation.
Unresolved blockers include grant identity collisions, anonymous capability-scope bypasses, and inconsistent acting-user evaluation.
| File | Summary |
|---|---|
packages/runtime-common/realm.ts |
Integrates grant-based anonymous admission. |
packages/runtime-common/helpers/const.ts |
Updates realm-info fixtures. |
packages/runtime-common/card-operations/types.ts |
Adds policy and explanation types. |
packages/runtime-common/card-operations/transforms.ts |
Extends transform context typing. |
packages/runtime-common/card-operations/telemetry.ts |
Updates anonymous telemetry. |
packages/runtime-common/card-operations/policy-query.ts |
Applies anonymous search filters. |
packages/runtime-common/card-operations/index.ts |
Exports new APIs. |
packages/runtime-common/card-operations/grant-expressions.ts |
Compiles and evaluates grant expressions. |
packages/runtime-common/card-operations/gate.ts |
Applies anonymous grant eligibility. |
packages/runtime-common/card-operations/explain.ts |
Reports grant-level behavior. |
packages/runtime-common/card-operations/dispatch.ts |
Carries anonymous request scope. |
packages/runtime-common/card-operations/anonymous-request.ts |
Tracks grants and acting users. |
packages/runtime-common/card-operations/acting-users.ts |
Removes superseded acting-user logic. |
packages/runtime-common/anonymous-rate-limiter.ts |
Implements per-grant counters. |
packages/runtime-common/anonymous-admission.ts |
Handles grant admission and charging. |
packages/runtime-common/anonymous-access.ts |
Updates shared anonymous-access utilities. |
packages/realm-server/tests/realm-policy-anonymous-search-test.ts |
Tests anonymous searches. |
packages/realm-server/tests/realm-policy-anonymous-read-test.ts |
Tests reads, blocklists, and limits. |
packages/realm-server/tests/realm-policy-anonymous-named-operation-test.ts |
Tests anonymous named operations. |
packages/realm-server/tests/realm-policy-anonymous-explain-test.ts |
Tests grant explanations. |
packages/realm-server/tests/realm-anonymous-access-test.ts |
Updates anonymous-access coverage. |
packages/realm-server/tests/helpers/index.ts |
Updates test helpers. |
packages/realm-server/tests/catalog-test-subset-test.ts |
Updates catalog assertions. |
packages/realm-server/tests/anonymous-rate-limiter-test.ts |
Tests grant-isolated limits. |
packages/realm-server/tests/anonymous-access-test.ts |
Tests expression parsing and settlement. |
packages/realm-server/main.ts |
Configures platform anonymous limits. |
packages/realm-server/handlers/handle-search.ts |
Passes admission into searches. |
packages/postgres/migrations/1791466220205_anonymous-grant-rate-limits.js |
Adds grant-level rate-limit storage. |
packages/host/tests/integration/realm-policy-test.gts |
Tests policy editing behavior. |
packages/host/tests/helpers/index.gts |
Supports configurable test limits. |
packages/host/config/schema/1791466220205_schema.sql |
Updates the SQLite schema. |
packages/catalog/test-subset.json |
Pins the catalog revision. |
packages/bxl/tests/unit/fixtures/function-coverage/request-context.ts |
Tests policy request context. |
packages/bxl/tests/boxel/authoring-skill-claims.ts |
Updates authoring claims. |
packages/bxl/src/transform.ts |
Adds policy context typing. |
packages/bxl/src/jqtools/evaluate/runtimeState.ts |
Adds policy runtime state. |
packages/bxl/src/bxl/profiles/function-safety.ts |
Registers policy() as request context. |
packages/bxl/src/bxl/compiler/readable-syntax.ts |
Supports policy key arguments. |
packages/bxl/src/bxl/bridge/request-context-native.ts |
Implements policy() accessors. |
packages/base/realm-config.gts |
Removes obsolete realm-level fields. |
.claude/skills/operations-diagnosis/SKILL.md |
Documents the new operational model. |
🧠 Review effort: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let id = grant.anonymous?.id; | ||
| if (id !== undefined && !this.#admittedGrants.includes(id)) { | ||
| this.#admittedGrants.push(id); | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in fc7c3c9a. Grant identities include their policy path, so otherwise identical grants retain independent rate-limit buckets.
| let anonymousRequest = | ||
| invocation.anonymousRequest ?? | ||
| new AnonymousRequest(actingUserResolver(core)); |
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in fc7c3c9a. Capability checks carry the request-wide anonymous admission into their operation scope, so they consider only eligible grants.
| if (scope.caller.kind === 'anonymous') { | ||
| await scope.anonymousRequest.admittedThrough(grant, instance); |
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in fc7c3c9a. Target-dependent acting-user resolutions are cached by grant and target for the request, so admission and write attribution use the same resolution.
| id: computeContentHash( | ||
| stableStringify([ | ||
| card, | ||
| resolved, | ||
| operation, |
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in fc7c3c9a. Grant identities include their policy path, so otherwise identical grants retain independent rate-limit buckets.
| `INSERT INTO anonymous_grant_rate_limits | ||
| (realm_url, grant_id, client_ip, window_start, window_seconds, count, expires_at) |
There was a problem hiding this comment.
[Claude Code 🤖] The mixed-version rollout needs an explicit compatibility gate or a dual-counter migration. I am leaving this thread open because a permanent second global counter would change the per-grant limit contract.
There was a problem hiding this comment.
there arent any live consumers of permissions policies in production so the deployment should be fine
There was a problem hiding this comment.
[Claude Code 🤖] Confirmed. With no live permission-policy consumers, the release has no mixed-policy traffic to preserve across the counter namespaces.
habdelra
left a comment
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Independent cross-check complete. One blocking regression is noted inline: moving an unchanged grant resets its anonymous rate-limit bucket. I found no additional verified requirement or coverage gaps.
| resolved, | ||
| operation, | ||
| outcome?.canonical ?? null, | ||
| grantPath, |
There was a problem hiding this comment.
[Claude Code 🤖] Blocking — regression: including grantPath makes a grant's rate-limit identity change when an editor inserts or reorders grants, so its in-window bucket is reset even though the grant itself is unchanged. The compile regression test that moves two grants currently fails for exactly this reason. Please use a persistent, position-independent discriminator that still keeps otherwise-identical grants separate, and retain the move regression coverage.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 996535a. The limiter identity now hashes the grant's position-independent policy behavior plus its acting-user, blocklist, and rate-limit expressions; moving the unchanged grant retains its bucket, while differing traffic controls remain separate. The regression test now covers both cases.
|
[Claude Code 🤖] The published blocking review finding is resolved. No remaining blocking findings from this review. |
richardhjtan
left a comment
richardhjtan
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] One blocking finding, inline: the anonymous opt-in matches any "anonymous" literal in where, so a grant that compares a card field with that word opens to visitors.
|
[Claude Code 🤖] The blocking finding is resolved in |
…ymous opt-in - A grant whose rate limit an address has used up admits only what no grant with budget left does, at the gate and in a search's scope, and the charge goes to it only then. Overlapping grants no longer leave a later grant's budget unusable. - A capability check from a caller a public-read ACL let in carries the admission made for its writes, so per-grant blocklists apply to its pairs. - `actor() != "anonymous"` guards a signed-in grant instead of opting it in; the signed-in reading of every predicate settles actor() before it is compiled into a search filter. - The cheap pre-compile check no longer misses an escaped "anonymous". - New `acting-user-from-submitted-card` warning for a create grant whose actingUser reads instance(). - Telemetry keeps a null actor for callers who aren't signed in; stale dashboard text, comments and the unused NO_ACTING_USER removed; skill text matches the opt-in and charging rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Grafana previewPreview deployed for 1 dashboard in the staging Grafana.
Dashboards: Preview is torn down automatically when this PR is closed or merged. (Run: https://github.com/cardstack/boxel/actions/runs/38146980728) |
Observability diff (vs staging)Show diffdiff --git a/tmp/remote-canon.vamWK5/dashboards/boxel-status/policy-decisions.json b/tmp/committed-canon.kO6iz6/dashboards/boxel-status/policy-decisions.json
index 6024ce3..e1dcd27 100644
--- a/tmp/remote-canon.vamWK5/dashboards/boxel-status/policy-decisions.json
+++ b/tmp/committed-canon.kO6iz6/dashboards/boxel-status/policy-decisions.json
@@ -9,20 +9,7 @@
},
"spec": {
"annotations": {
- "list": [
- {
- "builtIn": 1,
- "datasource": {
- "type": "grafana",
- "uid": "-- Grafana --"
- },
- "enable": true,
- "hide": true,
- "iconColor": "rgba(0, 211, 255, 1)",
- "name": "Annotations & Alerts",
- "type": "dashboard"
- }
- ]
+ "list": []
},
"description": "Operations-permission policy decisions, search-lane scoping, snapshot reads, compiles and their costs, from the boxel:operations channel. See the operations-diagnosis skill for how to read it.",
"editable": true,
@@ -1655,7 +1642,7 @@
"type": "loki",
"uid": "loki"
},
- "description": "blocklist: the realm's anonymousBlocklist names the address. ip-undetermined: no address reached the realm, usually a hop misconfiguration. blocklist-invalid: an entry isn't an address or range, which closes the realm to every such caller.",
+ "description": "blocklist: the blocklist of every grant that could admit the caller names the address. ip-undetermined: no address reached the realm, usually a hop misconfiguration. blocklist-invalid: a grant's blocklist isn't a list of addresses and ranges, or didn't evaluate, which closes that grant to every such caller.",
"fieldConfig": {
"defaults": {
"custom": {
@@ -1783,7 +1770,7 @@
"type": "loki",
"uid": "loki"
},
- "description": "A: admitted writes by the first user they were made as. B: refused writes whose grants named no one who may write the realm, by the failure (key-missing, not-a-matrix-id, no-write).",
+ "description": "A: admitted writes by the first user they were made as. B: refused writes whose grants named no one who may write the realm, by the failure (expression-failed, not-a-matrix-id, no-write).",
"fieldConfig": {
"defaults": {
"custom": {
@@ -1856,7 +1843,7 @@
}
],
"refresh": "1m",
- "schemaVersion": 42,
+ "schemaVersion": 41,
"tags": [
"policy",
"permissions",
@@ -1867,7 +1854,7 @@
{
"hide": 2,
"name": "env",
- "query": "staging",
+ "query": "__ENV__",
"skipUrlSync": true,
"type": "constant"
},
(Run: https://github.com/cardstack/boxel/actions/runs/38146980625) |
A spent grant is left out of a type's scope only where a grant with budget left scopes that type, so a type only spent grants scope answers 429 rather than dropping its rows. Comments on charging, budget-first ordering and the cheap pre-compile check say what the code does; the operations skill states the search rule exactly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Important
Merge order: cardstack/boxel-catalog#825, then this PR.
The catalog PR adds the
OperationGrantfields this PR's tests use. Merge it first, then re-run this PR'sCatalog Test Subsetcheck (the pin already points at the catalog PR's head) and merge this one.Merges after: cardstack/boxel-catalog#825
A realm's policy used to open itself to callers who aren't signed in through a boolean on the grant, while the realm being governed decided in its own
realm.jsonhow hard those callers could hit it and which addresses it refused. That was two places with two owners, and a policy author couldn't see either one from the card they were writing.Now the grant says all of it. A caller who isn't signed in has
actor() == "anonymous", a value no Matrix id can take, and a grant admits one only when itswherecomparesactor()with that text in a way that can hold for one.actor() != "anonymous"is a guard on a signed-in grant, not an opt-in. Besidewherethe grant carries four BXL expressions, parsed in the samepolicyprofile and evaluated per request:actingUser: who a visitor's write is made as.blocklist: the addresses the grant refuses.rateLimitRequestsandrateLimitWindowSeconds: the grant's rate limit.Each one can read the governed realm's settings with
realmConfig(), read the policy card's own fields with the newpolicy()accessor, or be written out as a constant. A realm that wants to keep the values to itself still can, and a policy that wants them visible on the card can do that instead.Opting in through
whererather than a flag keeps this safe to land. A grant that never comparesactor()with"anonymous"never starts admitting anybody, however permissive its condition would be for one:TRUEstays closed, and so doactor() != .ownerIdand.submittedAs == "anonymous".Where each expression is evaluated has three consequences:
instance()oractor(): a blocked or over-limit caller has to get the same answer whatever card it names, a refusal must not cost a card read, and a search must be checked once however many rows it returns.actingUseris settled per write once the target is known, so it alone may readinstance().anonymous_grant_rate_limitskeys a window on the realm, the admitting grant and the caller's address, so two grants in one realm never spend each other's allowance. Where two grants admit the same card, one whose budget is spent admits it only where no grant with budget left does, so a visitor keeps going through the other until both are spent. A search is scoped the same way type by type: a type a grant with budget left scopes leaves out the spent ones, and a type only spent grants scope answers 429.A grant that sets no limit, or whose expression produces none, falls back to
BOXEL_ANONYMOUS_RATE_LIMIT. Every realm now reports that value in its info, so the grant's edit form can show what applies when the field is left empty.An
actingUserthat readsinstance()on a create grant compiles with anacting-user-from-submitted-cardwarning: there the card is the one the visitor sends, so the visitor picks which writer the card is created as.A capability check from a visitor whom a public-read ACL let in is judged through the grants the visitor's address may use, as the write itself would be, so a grant's blocklist shapes the answer too.
explainreports the new shape. The explanation carries the platform's limit. Each grant that names"anonymous"carries its expressions and what each one produced: the acting user or why there is none, the blocklist entries or why the blocklist closes the grant, and the limit with where each half came from.Bumping the source realm's
updated_atusesCURRENT_TIMESTAMPon SQLite, wherenow()doesn't exist.The old realm-level fields are gone from
RealmConfig. Writing them intorealm.jsonnow does nothing, and there's a test for exactly that. The realm-wideanonymous_rate_limitstable stays for the rollout and is dropped in CS-13706. The boxel-cli plugin's copy of the authoring skills is refreshed in CS-13707, once the boxel-skills rewrite merges.One thing worth a second look:
anonymous-grant-reads-actorsurvives in a narrower form. It used to fire for two different things, a grant'swherereadingactor()and the granted operation's program reading it. The first is now the opt-in itself, so that half is gone. The second is still real, because dispatch refuses an operation whose transform readsactor()to a caller who has none. The code and its message now describe only the operation, and a grant it fires on isn't also told itsactingUseris never used.🤖 Generated with Claude Code
Preview panels
Policy edit view:
Realm Config edit view: