Skip to content

feat(lws): type index and type search - #6650

Open
jeswr wants to merge 155 commits into
mainfrom
feat/lws
Open

jeswr wants to merge 155 commits into
mainfrom
feat/lws

Conversation

@jeswr

@jeswr jeswr commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Jesse · project thread

Before: LWS resources could be listed by container only; there was no way to find them by type.

After: the type index and QUERY type search are served, paged, with a type-search filter of at most 8 KiB so its page links fit a request head; the storage description lists the type index services.

Part of a five-PR stack that replaces the single LWS core PR, so each surface can reach a clean review on its own. Merge in order: 1 resources (#6728), 2 PATCH (#6729), 3 access grants and requests (#6730), 4 notifications (#6731), 5 search and the type index (#6650). The SAML (#6720) and OpenID Connect (#6721) follow-ups sit on #6650.

This PR used to hold the whole LWS core. Its earlier review history (Codex rounds 12 to 22) is in the commits and comments above; round 22's findings are fixed in the slice each belongs to (metadata size and Link target weighing in #6728, subscription expiry in #6731).

How: index.rs, its routes and its services in the storage description. A QUERY's filter is always its body (an empty one is no filter), a failed listing fails the index, and the walk keeps what it holds (URIs still to visit, the listing in hand, what it returns) within one budget (507 past it). A listing is read through Store::list_children_within: the embedded store counts the container's members from its graph's index before reading any, the HTTP store asks for one row past the bound and caps the response it reads, and a listing with more members than can lie below the container is refused, never cut short. A resource removed after its container was listed is skipped (its existence is checked under its lock), and a resource that is set aside, or becomes so while the walk waits for its lock, is skipped with what is under it rather than waited on. Each container is locked (shared, visibility-gated, see IriLocks::read) and checked to exist before its listing is read, so a recursive delete part way through is waited out rather than read half done, and a stuck one, which sets aside everything it holds, is skipped at once with everything queued under it. Every failure gets one generic 500 that names no resource. The index and search evaluate preconditions like any read, and one test walks every route: each that takes a body refuses a coded one with 415, and each that serves a representation answers If-Match and If-None-Match. The Touchstone index floor rises from 0 to 39; with the whole stack, Touchstone passes 193 and fails none (core 121 of 122, auth 22 of 31, index 39 of 39, webhook 11 of 11).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3


Generated by Claude Code

claude added 11 commits October 5, 2026 16:35
GET /.lws/types/index lists the types of every resource the client may
read; QUERY /.lws/types/search takes an application/lws-query+json CNF
filter over types and descriptive relations (Link headers and linkset
alike), refuses more than 32 groups with 422, and pages results at
page_size with stateless base64url page links dereferenced by GET.
Both are authorization-filtered per request and private, no-store.

describedby is a descriptive relation the search indexes, so it is no
longer dropped from a client's Link headers as structural.

Touchstone lws10-index: 39/39 passed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Implement LWS 1.0 core section 10 and the WebhookSubscription suite in
lws/notify.rs: subscription create (lws+json, type, topic URIs, inbox,
expires; read access to every topic required), listing as an LWS
container (own subscriptions, all for the owner), GET/DELETE by the
subscriber or owner (404 to anyone else), persistence through the Store.

Events are matched by topic coverage (a container topic covers
everything beneath it) and re-authorized at event time with the
subscriber's agent. Deliveries are Notification envelopes, signed per
RFC 9421 with an RFC 9530 content-digest and the notify key published
in the storage description, sent off the request path with retries on
5xx/unreachable and deactivation on 410 or 5 consecutive failures.
Without allow_insecure_fetch, inboxes must be https and resolve to
public addresses only; deliveries never follow redirects or proxies.

Touchstone: core/notifications 15/15, notifications/webhook 11/11.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
…t subjects

The LWS authorization server's token endpoint now exchanges subject tokens
for RFC 9068 access tokens to this storage, verified per suite the way the
Touchstone reference authorization server does:

- did:key (P-256 0x1200 / Ed25519 0xed multikeys; kid names the derived
  verification method), self-issued sub = iss = client_id, aud, exp, iat;
- controlled identifiers: the CID document's id, an authentication method
  (embedded or referenced) the subject controls, not revoked or expired;
- OpenID Connect: the subject's document names the issuer (lws:OpenIdProvider
  service, or Solid-OIDC solid:oidcIssuer in Turtle), discovery + JWKS,
  ES256/RS256/EdDSA, azp as the client.

Documents are fetched under an SSRF policy (https only, no non-global
addresses, re-checked after redirects) unless allow_insecure_fetch is set.
Every refusal is an RFC 6749 error; bad subject tokens are invalid_request,
a foreign resource is invalid_target.

Also: EcKey::from_jwk ignores non-key JWK members (the private JWK it writes
carries alg/use/kid, which the p256 parser refused), and Jws gains EdDSA
verification.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
# Conflicts:
#	crates/sparq-lws-core/src/lws/jose.rs
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@jeswr jeswr self-assigned this Oct 5, 2026
claude added 3 commits October 5, 2026 16:55
The token endpoint now exchanges base64url SAML 2.0 assertions from identity
providers trusted via SOLID_SERVER_LWS_SAML_IDPS_FILE (PEM certificate, PEM
public key or public JWK per entity id). The assertion must be the document
root, carry exactly one enveloped signature whose single reference is the
assertion's unique ID with only enveloped + exclusive-c14n transforms, a SHA-2
digest and an rsa-sha256 / ecdsa-sha256 signature. Then Conditions validity,
an Audience naming this AS, and Subject/NameID (the LWS subject);
SubjectConfirmationData Recipient is the client.

Exclusive XML canonicalization 1.0 is implemented over a small quick-xml tree
(DOCTYPE and processing instructions refused). quick-xml 0.37 was already in
Cargo.lock. touchstone.sh generates an RSA IdP key with openssl, trusts it, and
declares SamlTrust with saml.idpKey.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
The lws-contrib dagger-workspace sparq cell sets SOLID_SERVER_OPEN_MODE=1, and its
harnesses run on private hosts, so open mode also allows insecure fetches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@github-actions github-actions Bot added area:ci bd migration label area:deps Dependency-management work area:docs bd migration label area:sparq-lws-core labels Oct 5, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 6755b0ef8a6c. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [high] Accept header bypasses root authorization — crates/sparq-lws-core/src/lws/resources.rs:65
    An unauthenticated GET / with Accept: application/lws+cid;q=0, application/lws+json skips authorization because the header contains LWS_CID. However, read() rejects the zero-quality description format and serves the protected root container listing, exposing member URIs and metadata.
    Determine the selected representation once and bypass authorization only when actually serving the public storage description.

  2. [high] Identity fetches can reach private endpoints — crates/sparq-lws-core/src/lws/subject_tokens.rs:775
    The HTTP client follows redirects before checking the final URL. An unauthenticated token-exchange request can name an attacker-controlled HTTPS subject that redirects to http://127.0.0.1:PORT/... or an internal service; the private request happens before rejection. Separately, DNS validation and connection resolution occur independently, allowing DNS rebinding.
    Validate each redirect before following it, enforce HTTPS at the client, and restrict the addresses actually used for connections.

  3. [high] JSON Patch copy permits exponential memory exhaustion — crates/sparq-lws-core/src/lws/resources.rs:1036
    Starting with {"a":["x"]}, repeatedly applying {"op":"copy","from":"/a","path":"/a/-"} approximately doubles the document each time. A small PATCH containing a few dozen operations can exhaust process memory because every copy deep-clones the value without checking expanded size. The request-body limit and storage quotas cannot prevent allocations made before writing.
    Enforce an intermediate document-size and allocation budget before cloning or inserting copied values.

  4. [high] Failed grant revocation is reported as successful — crates/sparq-lws-core/src/lws/access.rs:523
    DELETE ignores storage errors, removes the grant from memory, and returns 204. If the backend fails before removing the persisted record and containment edge, restarting the server reloads the supposedly revoked grant and restores access.
    Propagate persistence failures and report successful revocation only after durable deletion succeeds.

  5. [medium] Conditional mutations do not prevent lost updates — crates/sparq-lws-core/src/lws/resources.rs:938
    Preconditions are checked against separately fetched metadata, followed by an unconditional write. Two concurrent PUTs or PATCHes carrying the same If-Match can both pass and return success, with one overwriting the other. DELETE and linkset PATCH have the same separation.
    Make validation and mutation atomic through a backend compare-and-swap operation or appropriate shared locking.

  6. [medium] GET can return bytes with another version’s validators — crates/sparq-lws-core/src/lws/resources.rs:424
    GET fetches metadata first, then calls store.read(), which independently fetches current metadata and bytes. A concurrent replacement between those calls produces the new body with the old ETag, Last-Modified, and potentially Content-Type, corrupting cache and conditional-request behavior.
    Fetch bytes through the captured metadata using Store::read_at, or use one coherent resource snapshot.

  7. [medium] Modify-only permission exposes protected JSON values — crates/sparq-lws-core/src/lws/resources.rs:1040
    PATCH requires only Modify, yet a JSON Patch containing solely {"op":"test","path":"/pin","value":"1234"} returns 204 when the guessed value matches and 422 otherwise. An agent denied Read can therefore enumerate sensitive values without changing the document.
    Require read permission for content-observing patch operations and other content-dependent responses.

  8. [medium] Ignoring If-Range can corrupt resumed downloads — crates/sparq-lws-core/src/lws/resources.rs:444
    Range requests are served without evaluating If-Range. If a resource changes after a client downloads its prefix, requesting the remaining bytes with the old validator still returns 206 from the new version, allowing incompatible versions to be combined. RFC 9110 §13.1.5 requires ignoring Range when that validator does not match.
    Evaluate If-Range against the selected representation and return the complete body with 200 on mismatch.

Verdict: The PR is not safe to merge as is.

claude added 5 commits October 5, 2026 19:09
- Range: a multi-range, other-unit or malformed Range is ignored (200 with the
  whole body); only an unsatisfiable single range is 416. If-Range is honoured
  (strong ETag or exact Last-Modified), otherwise the Range is ignored.
- Containers asked for application/ld+json with the LWS profile answer with
  that media type.
- The linkset document carries server-managed metadata (up, type, and for a
  data resource self with format/size/modified), merged with the user links.
  Server-managed relations in a PATCH are ignored (read-only), and meta.links
  is re-derived from the stored user linkset so the two never drift. The
  linkset ETag is a digest of the whole document.
- PUT/PATCH leave the linkset alone unless Prefer: set-linkset (PUT replaces,
  PATCH adds; Preference-Applied echoed). Content-derived types still follow
  the content for the type index.
- A linkset PATCH announces Update; a Depth: infinity delete announces a
  Delete for every descendant.
- GET/HEAD/201 emit a rel="type" Link for each declared type.
- Malformed JSON Patch is 400; a well-formed patch that fails stays 422; a
  patch outgrowing 64 MiB (copy doubling) is 413.
- The storage description has an ETag and answers If-None-Match.
- Review findings: the root authorization bypass follows the negotiated
  representation, not a substring of Accept; test/copy/move patches need Read
  as well as Modify; GET reads bytes through the same metadata snapshot
  (read_at); conditional writes (PUT, PATCH, DELETE, linkset PATCH) and
  container metadata touches hold a per-IRI lock (single process only) so
  If-Match cannot lose updates.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Against w3c/lws-protocol main (ef02548):

- Access Profile: target is optional (an untargeted policy covers the
  grant's storage); target type matchers DataResource / Container /
  StorageResource, values non-recursive; access documents need the LWS
  @context (400 otherwise); storage must be a URI.
- Tokens: sub must be a URI; the token endpoint refuses a subject or
  client that is not one. SAML NameID must be a URI and Recipient is
  required (no issuer fallback); OIDC azp must be a URI.
- Key rotation: SOLID_SERVER_LWS_AS_PREVIOUS_KEY_FILE (public or private
  JWK) is published in the JWKS and accepted by kid.
- Grant, request and subscription services behave as containers:
  negotiated, paged listings with ETag, Vary and up/type/linkset links,
  read-only linksets, links on 201s, ETag on a subscription GET.
- Notifications: a new request notifies the owner's inbox when the
  owner's JSON(-LD) identity document names one; a new grant notifies
  the inboxes of the requests by or for its assignees.
- Type index paged with first/prev/next/last; type search gains
  prev/last.
- Review fixes: identity fetches follow no redirects automatically,
  check each hop, connect only to public addresses via a shared
  resolver and use no proxy; grant, request and subscription deletes
  report store failures as 500 and keep the record until the durable
  delete succeeds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

All eight findings from the Codex review of 6755b0e are addressed in ac95cea:

  1. Root authorization bypass: the bypass now applies only when the negotiated representation is the storage description (resources.rs, with a test for lws+cid;q=0).
  2. Identity-fetch SSRF: no automatic redirects, each hop validated, no proxy, connections only to public addresses through a filtering resolver shared with webhook delivery (subject_tokens.rs, mod.rs).
  3. JSON Patch copy amplification: a 64 MiB budget on the intermediate document, checked before every clone or insert; over budget is 413.
  4. Revocation durability: a failed grant, request or subscription delete returns 500 and keeps the record in memory and storage.
  5. Lost updates: a per-IRI lock in LwsState held across the precondition check and the write for PUT, PATCH, DELETE and linkset PATCH. The Store has no compare-and-swap, so this is per process, as the code comment says.
  6. GET coherence: the body is read with read_at from the same metadata snapshot that supplies ETag and Last-Modified.
  7. Modify-only content probing: patches containing test, copy or move also require Read.
  8. If-Range: honoured for strong ETags and exact dates; a mismatch serves the full 200.

Touchstone still passes every applicable test (core 121/122, auth 31/31, index 39/39, webhook 11/11).


Generated by Claude Code

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 4e9ad5115035. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [high] Identity fetches permit SSRF through redirects and DNS rebinding — crates/sparq-lws-core/src/lws/subject_tokens.rs:775
    The HTTP client follows redirects before fetch validates the final URL. An unauthenticated token-exchange request can name an attacker-controlled HTTPS subject that redirects to http://127.0.0.1/... or a private address; the internal GET occurs before rejection. DNS checks also resolve separately from the client’s connection, allowing rebinding.
    Disable automatic redirects, validate every destination before connecting, and enforce the address policy in the connection resolver.

  2. [high] Holder-of-key SAML assertions are accepted without proof of possession — crates/sparq-lws-core/src/lws/saml.rs:112
    Validation never checks SubjectConfirmation/@Method or its key material. A valid assertion whose only confirmation method is holder-of-key is therefore exchanged for a bearer access token by anyone possessing the assertion, without demonstrating possession of the required private key. This defeats the SAML holder-of-key requirements.
    Require a fully validated supported confirmation method; reject holder-of-key assertions until proof verification is implemented.

  3. [high] Recursive deletion bypasses descendants’ permissions — crates/sparq-lws-core/src/lws/resources.rs:1293
    Authorization checks only the requested container. remove then deletes every descendant without checking its permissions. An agent granted delete solely on /shared/ can send DELETE /shared/ with Depth: infinity and erase resources owned by other agents, despite grants being explicitly non-recursive. The LWS recursive-delete example assumes permissions for all contents.
    Validate delete permission across the entire subtree before removing anything, and protect that validation against concurrent changes.

  4. [high] Concurrent mutations can overwrite resources and transfer ownership — crates/sparq-lws-core/src/lws/resources.rs:841
    Slug availability checks and creation are separate operations. Two agents with only create permission on a shared container can concurrently select the same unused Slug; create_in_container replaces existing child metadata, so both requests succeed and the later creator metadata gives one agent full access to the other’s resource. The same unprotected check/write pattern lets concurrent If-Match updates both succeed against one old ETag, losing an update.
    Make name reservation and conditional mutations atomic, using store-level checks or shared mutation synchronization covering authorization, validation, and commit.

  5. [high] Accept negotiation bypasses root-listing authorization — crates/sparq-lws-core/src/lws/resources.rs:65
    An anonymous GET / with Accept: application/lws+cid;q=0, application/lws+json skips authorization because the header contains application/lws+cid. read subsequently honors its zero quality value and returns the protected root container listing, including resource identifiers, sizes, and modification times. Pagination exposes subsequent pages too.
    Select the representation once and bypass authorization only when the response actually serves the public storage description.

  6. [high] Separate SAML audience restrictions are incorrectly combined with OR — crates/sparq-lws-core/src/lws/saml.rs:92
    The verifier flattens all descendant Audience elements and accepts any match. A signed assertion with one AudienceRestriction including this server and another excluding it is accepted, although SAML requires every restriction to hold independently. A credential not valid for this authorization server can consequently produce access tokens.
    Evaluate each direct AudienceRestriction separately, requiring a matching audience in every restriction, and reject unsupported conditions.

  7. [high] Failed grant revocations report success and can reappear after restart — crates/sparq-lws-core/src/lws/access.rs:523
    Grant deletion discards the store error, removes the in-memory grant, and returns 204. If a durable backend fails before deleting the stored record, the owner believes access was revoked, but AccessStore::load restores the grant on restart and the assignee regains access.
    Return an error unless durable deletion succeeds, and coordinate durable and in-memory revocation so success guarantees persistence.

  8. [high] LWS mode silently removes application-level DoS protections — crates/sparq-lws-core/src/main.rs:1149
    The LWS branch discards overload_config and returns a router containing only CORS middleware. Configured admission limits, per-IP rate limiting, request timeouts, and body limits therefore stop applying. Dispatch buffers up to 64 MiB before authentication, so unauthenticated clients can hold many slow uploads or exhaust memory despite the configured protections.
    Apply the shared overload protections to the LWS router and make its body reader enforce the configured ceiling.

  9. [medium] Stale If-Range validators still produce partial responses — crates/sparq-lws-core/src/lws/resources.rs:444
    Range handling never evaluates If-Range. After a resource changes, a client resuming a download with Range: bytes=100- and the old ETag receives 206 containing bytes from the new version. Combining those bytes with its cached prefix corrupts the download.
    Evaluate If-Range against the selected representation and return the full 200 response when it fails, as required by RFC 9110 §13.1.5.

Verdict: The PR is unsafe to merge as is.

- SAML: every SubjectConfirmation must be bearer (holder-of-key and
  sender-vouches are refused); each needs SubjectConfirmationData with a
  future NotOnOrAfter and a URI Recipient (the client), and several must
  agree on the client.
- SAML: each AudienceRestriction must name this server (ANDed, not ORed);
  ProxyRestriction is ignored, OneTimeUse and unknown conditions refused.
- Recursive DELETE locks the whole subtree (longest IRI first), then
  requires Delete on every descendant before removing anything; it
  removes exactly the checked set.
- POST holds the container's lock from the free-name check through the
  create and creator metadata, so two same-Slug POSTs never share an IRI.
- The LWS router gets the Solid overload stack (rate limit, admission,
  timeout, body limit, probes outside) via app::with_overload_layers, and
  the dispatcher reads at most SOLID_SERVER_MAX_BODY_BYTES (413 above).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
claude added 3 commits October 9, 2026 05:20
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3

# Conflicts:
#	crates/sparq-lws-core/src/lws/resources.rs
…t, skip removed resources

The embedded store reads a bounded listing from the container graph's own index: it counts the members exactly, refuses a listing that cannot fit unread, and stops at the byte bound. The HTTP store asks for one row past the bound and refuses when it arrives, reading the response only up to what such a listing could take. A listing with more members than can lie below the container is refused, never cut short. Stores and backends that cannot bound the read refuse the call. The index walk checks each resource still exists under its lock before visiting it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@jeswr

jeswr commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head b1ee4015b3bb. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Index walks wait on resources set aside after failed rollback — crates/sparq-lws-core/src/lws/index.rs:316
    When rollback keeps failing, LwsState::set_aside retains the affected resources’ exclusive locks while recovery retries. The new walk never checks is_set_aside, so every index or search reaching one waits until recovery or the request timeout—even when the caller cannot read that resource. Repeated requests can occupy all admission slots, defeating the existing prompt refusal for these resources.
    Detect set-aside resources before listing or waiting for their locks and fail the whole request promptly with a generic error.

Requested checks

  • Prior memory finding: closed. Embedded listing counts exactly before scanning; HTTP caps its response read; trait defaults refuse unsupported bounded reads.
  • Prior truncation finding: closed. Embedded and memory backends refuse excess rows; HTTP requests LIMIT n+1 and refuses the extra row. No bound-induced partial listing is returned as complete.
  • Prior deleted-resource finding: closed. Existence is checked under the shared resource lock, and absent resources are skipped.

Verified by code inspection; tests were not run in the read-only checkout.

Verdict: Fix the set-aside lock handling before merging.

claude added 8 commits October 9, 2026 06:00
RFC 9264 section 4.2.4.2 gives an internationalized target attribute one or
more value objects; an empty array is now refused with 422 through both
PATCH formats, leaving the metadata as it was.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
…ounded

- LwsState::visible is the one check every route makes; a stuck change
  sets aside its resources and the containers listing them, PUT's included.
- Set-aside marks are counted per IRI, so one recovery finishing does not
  reveal what another is still putting back.
- What set-aside changes hold is counted; at 256 MiB new writes get 503.
- PUT and DELETE hand their listing locks to the task running their store
  calls, and that task touches the container, so cancelling the request
  can neither release the listing early nor leave the container's date
  behind.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
…f force

A grant's create stores a durable unsettled mark first and removes it once
the grant is stored and in force; a revocation stores the mark, takes the
grant out of force, then removes the record (set aside when that keeps
failing). AccessStore::load never puts a marked record in force and removes
it, so a restart after a create whose outcome was unknown, or after a
revocation that did not land, cannot bring the grant back.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Carries the set-aside rework up and fixes the slice's review findings with it:
- A subscription to a linkset topic is checked as its resource (existence
  and authorization) under the resource's shared lock.
- prepare_at_most bounds the deliveries it prepares, not the subscriptions;
  DELETE's shared bound can no longer underflow.
- Update is announced by the task that runs the write (hold_locks), so a
  cancelled PUT or PATCH still notifies.
- A delivery's wait for a resource lock gives up once the resource is set
  aside, and the subscription is checked again after the lock is taken.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
LwsState::read_visible takes a resource's shared lock unless the resource
is not visible, or stops being visible while the lock is waited for. The
type index and search walk skips such a resource (and what is under it),
and a notification delivery uses the same check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@jeswr

jeswr commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head e1e27b07073f. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Container expansion precedes the visibility lock — crates/sparq-lws-core/src/lws/index.rs:296

    The walk reads and queues a container’s children before calling read_visible. During a recursive DELETE, it can capture a partially removed listing, then wait for the container lock. If rollback succeeds, restored members remain absent from that snapshot, so the search returns an incomplete 200 after recovery.

    If rollback is instead set aside, read_visible skips the container but leaves its children queued. Untouched descendants retain their exclusive subtree locks without necessarily appearing in the remaining undo entries that determine visibility. Visiting one can therefore wait indefinitely; unlocked descendants can also be returned from the supposedly skipped subtree.

    Acquire the visible container lock and establish existence before reading its listing. Also abandon queued descendants when an ancestor becomes set aside, including while waiting for a descendant’s lock; cover both successful recovery and stuck recursive rollback.

Requested checks

  • Round-5 waiting finding: Not fully closed. Directly marked resources are skipped, but the retained descendant-lock case above still hangs.
  • Recovery correctness: No persistent index cache survives recovery, but an in-flight walk can still omit restored resources.
  • Set-aside existence: Subtree exclusion is incomplete, so descendants can reveal a container that the walk skipped as set aside.
  • Review was static; tests were not run.

Verdict: Fix the container traversal race before merging.

claude added 11 commits October 9, 2026 09:01
A recursive DELETE whose rollback was set aside hid only what it had
removed, while its recovery task held the locks of the whole subtree.
What the delete had not reached stayed visible, so a request for it, or
a walk over it, waited on a lock that would not come back until the
rollback finished. Every resource the delete locked is now set aside
with it (Undo::Locked, nothing to put back), so they are answered 503 at
once and skipped by walks.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
…hange in flight

- IriLocks owns the set-aside marks: lock, read, try_lock and try_read
  give a guard only while the IRI is visible, re-checked once held, and a
  wait gives up as soon as the IRI is set aside (woken by a Notify). A
  request already queued on a lock is answered 503 rather than waiting on
  a recovery; every route that holds a lock got it this way.
- A container listing reads each member under its shared lock; a change
  in flight to one makes the read retry (bounded, then 503) once that
  change is over, without the container's lock held. A member's removal
  holds its container exclusively, so a removal that may yet be put back
  is never listed.
- A container's date counts as behind from before a change's locks are
  released until the touch after it lands (touching/touched count).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
# Conflicts:
#	crates/sparq-lws-core/src/lws/resources.rs
The unsettled mark was a separate stored key: a failed create left it
behind (taking a store slot, out of any listing), a mark that could not
be cleared acknowledged a create the boot would then remove, and a
create's mark cleanup could race a revocation of the same grant.

There is no separate mark now. A grant is created under an unsettled
content type and stored again under its own type before it is put in
force; one whose second write is not known to land is refused and
removed. A revocation stores the record as unsettled (best effort; a
full store does not stop it), takes it out of force and removes it. The
boot removes any record stored as unsettled. Each record's own lock is
held across its create and its revocation, so the two never interleave,
and every lock the access services take is the visibility-gated one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Adapts the notify slice to the visibility-gated locks (deliveries and
cancellations take them; the polling wait is gone) and fixes a deadlock:
a conditional subscription checked its topics while holding the
subscriptions container exclusively, so a topic naming that container or
its linkset waited on the lock it held. Topics are now checked before
the container is held.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
Adapts the search slice to the visibility-gated locks and fixes the
container walk: each resource is now locked (shared, visibility-gated)
and checked to exist before its listing is read, so a recursive delete
part way through is waited out rather than read half done, and a stuck
one (which now sets aside everything it holds) is skipped at once, with
everything queued under it. LwsState::read_visible and its polling are
gone; IriLocks::read does the same for every caller.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gz4YZq3SwTb7XN8z1JL2C3
@jeswr

jeswr commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head fbedd39b4233. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Requested checks

The round-6 medium is closed: readable acquires the visibility-gated read lock, checks existence, then lists and queues children (index.rs:291–307). The recursive-delete regression test covers waiting for rollback and skipping set-aside subtrees. Tests were inspected, not executed.

Verdict: The PR is safe to merge as is based on this review.

jeswr commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

Held with #6730: this slice builds on its grant and request code (inbox delivery for grants and requests, the access snapshots that deliveries and the index check against), so it cannot land before #6730, which waits on #6750 for durable revocation and create recovery. The review of this head can still go ahead.


Generated by Claude Code

@jeswr
jeswr force-pushed the feat/lws-notify branch 2 times, most recently from 41ad37d to 84a2325 Compare October 11, 2026 00:01
Base automatically changed from feat/lws-notify to main October 11, 2026 00:10

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci bd migration label area:deps Dependency-management work area:docs bd migration label area:sparq-lws-core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants