grain.social/grain

Switch repository

Loading…

What changes

Private galleries are served by grain-serve (crates/grain-serve/src/spaces/gallery.rs): social.grain.privateGallery.createPrivateGallery, addMember, getPrivateGallery, listMembers, listPrivateGalleries and listSharedPrivateGalleries, as their lexicons say. One gallery is one social.grain.privateGallery space, anchored on its author, keyed by the gallery's rkey. createPrivateGallery checks the author's PDS serves spaces, resolves every member first (one that cannot be found stops it with ActorNotFound before anything is made), makes the space, puts each member on its list to read (write: false), keeps an invite row for each, and writes the gallery, its photos (<rkey>-<n>) and items in one applyWrites, each record held to its lexicon first (InvalidRecord).

Who reads what. The gallery and its photos: its author and the members the author added, each with a credential the authority gives only to a member; anyone else gets 400 NotAuthorized, as the old server answered. The member list: the author alone (anyone else NotAuthorized), read with their session, less the author, with handle, display name and avatar from the index where it has them. listPrivateGalleries: the spaces the caller's PDS lists that are anchored on them, newest first. listSharedPrivateGalleries: the invite rows grain kept, each read with the caller's own credential before it is listed, so a gallery whose authority does not admit them is left out; the invite grants nothing. Tests check an outsider is refused the gallery, its picture and its member list, and that an invite row alone lists nothing.

/space-blob?space=&repo=&cid= (spaces/blob.rs) serves a picture a space record references, read with the caller's credential: 404 for a caller the space does not admit, as for a CID the repo's host has no blob for; no-store as below, the type read from the first bytes (JPEG, PNG, WebP, GIF, AVIF, else application/octet-stream), default-src 'none'; sandbox and nosniff.

How a reader is known (spaces/mod.rs). By the web's cookie or a service token, with the account's web session: a delegation token from its PDS, exchanged at the authority's for a credential, its PDS first checked to serve spaces (SpacesUnsupported otherwise). Or, for getPrivateGallery and /space-blob, by a delegation token an app sends in atproto-space-delegation, as knotless takes one: checked (grain::auth::verify_delegation), for the space asked, for the caller if one is named, and spent once here (InvalidToken otherwise), then exchanged. Credentials are kept on App by reader and space for one minute at most (REPROVE), then asked for again, so a member taken off a gallery stops reading it within that minute. A credential is dropped sooner when a host refuses it, or when its reader logs out.

Every answer of a space method is for its caller alone (spaces::routes). One layer covers every space route. For a query, it marks the answer with viewer::unshared, the way knotless marks a private read touched, so the answers layer from infra/headers sends Cache-Control: private, no-store and Vary: authorization, cookie. It also adds atproto-space-delegation to Vary. The picture is not a query, so the layer sends all of these headers for it directly.

write::with_session is write::call's session handling (refresh once on a refused access token, end the session on a second, out_of_scope) lifted out for spaces to share; call uses it unchanged.

From knotless

• spaces/mod.rs follows knotless-serve's private.rs and viewer.rs, as access.md's "Who can see a private repository" lays out:
  • every read is made as the asking viewer, with their own session or delegation token, and never as the server;
  • a delegation token is checked (verify_delegation), for the space asked, for the caller if one is named, and spent once (App::spend);
  • membership is proven again with a fresh credential every minute (REPROVE);
  • every answer that read a space is private, no-store (unshared);
  • logging out forgets the credentials read with (private::forget).
• blob.rs serves pictures, as knotless serves a private repository's raw files: unshared, under a policy that runs nothing.
• Divergences:
  • Live reads, no sync: knotless syncs each space a member reads into its own index (spaces.rs, space_sync.rs: registerNotify, oplog sync, a full read when a sync does not check out). Grain reads the author's repo at each request. The stack ports the old server's reads, which were live too. A sync like knotless's is the larger change and would come as its own step.
  • 400 NotAuthorized, not 404: knotless answers a non-member with a 404 worded as for a repository never made. Grain answers 400 NotAuthorized, because the lexicons declare that error and the web and apps act on it. /space-blob answers 404, as knotless does.
  • Invite rows: knotless lists a private repository to a member only once they have been proven a member there (read_by). Grain lists a gallery from the invite rows written when it was shared, and reads each with the member's credential before listing it. A member must be able to find a gallery they have never opened, which the lexicon's listSharedPrivateGalleries exists for. knotless's invites.rs is about runners, a different thing.

Why

The third of the spaces stack: private galleries, which the web uses today, on what pulls 1 and 2 put in place.

How it was checked

crates/grain-serve/tests/spaces.rs, on the alpha: an author uploads a picture and makes a gallery for a member, refused first for a member who cannot be found, a bad key, a title of the wrong type and one too long; the author and the member read it (viewerIsAuthor each way, title, description, the photo, its CID and alt), an outsider is NotAuthorized, no caller 401, a pool's URI 400; the member reads the picture (bytes, type, private, no-store and its Vary), the outsider, a non-space and a CID with no blob 404; a query's answer private, no-store; a logout with credentials kept, after which the member still reads; the member list to the author with a profile from the index, NotAuthorized to the member; addMember by the author, refused to the member, of the author and of no one; the author's own list and the member's shared list, an outsider's shared list empty even with invite rows for the gallery and for no space. An app's read with the member's delegation token, refused when spent, unreadable, a header that is no token, for another space, sent with another account's cookie, and for an outsider at the exchange; a second token of the member's reads with the credential kept. addMember to a gallery never made is NotAuthorized. tests/write.rs, on the npm PDS (no spaces): getSpaceSupport false, createPrivateGallery SpacesUnsupported, too many members refused, a reader on it SpacesUnsupported. Unit tests for the spent tokens, the failures, the invite rows and their query plan, and the picture types. fmt and clippy here; the coverage gate and the answers check are left to the runner.

Differences from the old server

• A host failing is 502 UpstreamFailure, as grain-serve's other calls to a PDS answer, not 400 UpstreamFailed; a session that has ended is 401 AuthenticationRequired, not 400 SessionExpired.
• /space-blob serves the original, not a resized preset.
• The author reads their own gallery with a credential, as a member does, not with their session.
• The writes take the cookie from a page of the site only, as grain-serve's other writes do; a service token is refused (mobile private galleries are out of scope; nothing the apps call changed).
• The atproto-space-delegation path for apps is new.
• listSharedPrivateGalleries leaves out a gallery for any failure but an ended session, and so does the old one; it now says why only in the log.

Shortcomings

• A collection is read to 20 pages of 100, so a gallery of more than 2,000 photos is cut there.
• Nothing removes a member: the lexicons have no method for it.
• The credential cache is per process and unbounded but for expiry.
• The simplespace path (pds.js, zds) is not run here.
• createPrivateGallery is not atomic. It makes the space, puts the members on it and keeps the invite rows before the applyWrites that writes the gallery. If that last write fails, the space and invites are left with no gallery in them. A retry with the same rkey then asks the authority for a space that already exists; how the alpha answers that was not checked.
• listSharedPrivateGalleries applies limit to the invite rows before it drops the galleries the caller can no longer read, so a page can come back short.

Review

Start at spaces/mod.rs (who reads with what), then spaces/gallery.rs; spaces/blob.rs is short.

🤖 Generated with Claude Code

This pull serves private galleries from grain-serve: create, addMember, get, listMembers, and the two list methods. It also adds /space-blob and the delegation-token path for apps, and lifts with_session out of write::call. I read spaces/mod.rs, gallery.rs, blob.rs and with_session, and found nothing that must change. I did not read the 3,700-line diff in full, so the test files are unread.

The access rules match what the body says:

• The gallery, its photos and the picture are read only with a credential the authority gives to members.
• The member list is limited to the author.
• A delegation token is verified for the space and the caller, and spent once before it is exchanged.
• /space-blob answers 404 for non-members and sets private, revalidated caching with Vary and a sandbox CSP.

All four checks (check, rust, test, harness) were still queued when I looked. The coverage gate and the answers check haven't run, so read them before merging.

Two non-blocking points:

• Partial create: in createPrivateGallery the space, the member puts and the invite rows are all written before the applyWrites that makes the gallery. If that last call fails, the space and invites exist with no gallery, and a retry with the same rkey may fail on the existing space. I did not check how the authority answers a repeat create, so this is unconfirmed. A retry path, or at least a note under Shortcomings, would help.
• Limit before filtering: listSharedPrivateGalleries applies limit to the invite rows before dropping galleries the caller can no longer read. Pages can therefore come back short even when more readable galleries exist.

Nothing in this round must change. Since my last review the head moved by one commit plus a merge of spaces/support. That commit changes when a refused scope ends a session: a session asked for the space scopes is never treated as behind, so a failed probe of a PDS can't end it. I only read the commit message, not its diff, so this approval rests on that message and on my earlier read of the gallery code.

My earlier review asked for nothing blocking. Neither non-blocking point was addressed in the commits since: the partial create in createPrivateGallery, and limit applied before listSharedPrivateGalleries drops unreadable galleries. At least a line under Shortcomings would help.

All four checks (check, rust, test, harness) are still queued on the new head. The earlier runs were cancelled, so nothing has passed yet. Read the results, and the coverage gate and answers check the body leaves to the runner, before merging.

Nothing in the new commit needs to change, so I'm approving this round. The head moved by one commit, 16f3b73. I read the cache-control layer and the credential handling it adds. I did not read the test diff.

The commit does two things:

• Cache headers: every space route now answers private, no-store with Vary: cookie, authorization, atproto-space-delegation, from one layer. The picture route is covered too.
• Credential lifetime: a credential is read with for 60 seconds at most (REPROVE) before the authority is asked again, so someone taken off a gallery stops reading it within a minute. A credential is also dropped when a host refuses it, and login::logout calls app.spaces.log_out(&did) to forget a reader's credentials.

Both earlier non-blocking points (the non-atomic createPrivateGallery, and limit applied before listSharedPrivateGalleries drops unreadable galleries) are now listed under Shortcomings, which is as much as I was asking for.

All four checks (check, rust, test, harness) are still queued on this head. Read the results, and the coverage gate and answers check the body leaves to the runner, before merging.

Nothing in the new commit needs to change, so I'm approving this round. The head moved by one commit, c3bfce3, and I read its whole diff.

• /space-blob: a CID the repo's host has no blob for (BlobNotFound) now answers 404, the same as a caller the space doesn't admit. A refused read still drops the credential. The credential and read errors go through one path, so a non-member and a missing picture look the same. That is what you want here.
• spaces::refused: it replaces the repeated "forget the credential on denial, then fail" code in gallery.rs and blob.rs. The behaviour is unchanged.
• Views: the optional fields (gallery, handle, displayName, avatar, authorHandle) now go through views::put. This is a refactor, and the member and gallery views keep the same shape.
• Tests: the new cases cover a picture that isn't there, addMember on a space that was never made, and a second delegation token being read with.

The two earlier non-blocking points, the non-atomic create and limit applied before filtering, are listed under Shortcomings.

All four checks (check, rust, test, harness) are still queued on this head, so nothing has passed yet. Read the results, and the coverage gate and answers check the body leaves to the runner, before merging.

Nothing in this round must change, so I'm approving. Since my last review the head moved by three commits plus a merge of batch/one, which brings in other branches' work.

• a4376d8: space queries now mark themselves with viewer::unshared. The shared answers layer then sends private, no-store with Vary: authorization, cookie. The spaces layer adds atproto-space-delegation to Vary, and sends the full set directly for the picture route. This matches the body and keeps the earlier cache guarantee. I read this only from the commit message and the body, not the diff.
• 6210e92 and 421b83b: these are test-only changes. They cover a delegation header that is no token, and a logout with credentials kept, after which the member still reads. I did not read the test diff.

The two earlier non-blocking points, the non-atomic create and limit applied before filtering, are listed under Shortcomings, which is enough.

All four checks (check, rust, test, harness) are still queued on this head, so nothing has run. Read the results, and the coverage gate and answers check the body leaves to the runner, before merging.

Landed on feature/rust at 92c202b, fast-forwarded with spaces/pools (Pools are read and written through grain-serve), which contains this head (67d8d95) and passed every check against 164794a. Review rounds ran out, so this was merged on the last approval (421b83b). The only change since then is 67d8d95, which was read before merging and doesn't change behaviour. Also read before merging: credentials and refusals are kept per reader and per space; the one cache shared between viewers is a group's public, unauthenticated listing; every write goes through write::writer (same-site cookie, no service token); and the spaces routes alone carry private, no-store.