fix(spaces): protocol hardening, performance and a smaller server surface #133

Closed
opened 2026-09-24 12:39:09 +00:00 by Grandiras · 2 comments
Owner

Part of the v0.7.0 update cycle (wave 6). Follow-up to #113: normative hardening, performance, and a smaller public server surface for Spaces.

Protocol hardening (spaces research G14–G18, G22, G23)

  • DID-document rules from proposal 0016 (2026-09-02): an entry named #atproto_space / #atproto_space_host that exists but is malformed is an error, not a fallback. #atproto_space_host must have type AtprotoSpaceHost and an absolute https endpoint. Fall back only when the entry is absent.
  • Server token keys: restrict kid to #atproto / #atproto_space, and decide explicitly whether a missing kid is accepted.
  • DPoP: reject ath on the credential exchange, and optionally pin ES256.
  • SpaceRepoCar.Verify:
    • each index key must be exactly one NSID/rkey, both validated;
    • each record block must decode as a CBOR map;
    • index CIDs must be dag-cbor.
  • Space-type declarations: collections must not contain *; follow the proposal on name length.
  • Space and SimpleSpace models adopt the open-union and extension-data pattern from #115.
  • Docs: sync.getBlob does not serve space blobs.

Performance

  • Cache verified space credentials keyed by SHA-256(jwt) until min(exp, DID-cache TTL). Keep the per-request exp, subject and jti checks. Cache the imported JWK keys. Today each space read spends about 350 µs of CPU on crypto before touching data.
  • SpaceCredentialProvider:
    • per-space single-flight instead of one lock held across the network mint;
    • dispose superseded credentials, which currently grow without bound;
    • use the caching identity resolver.
  • SpaceSyncer recovery pre-sizes from Content-Length and parses the buffer without an extra copy.
  • EF stores use upserts (ExecuteUpdateAsync first).
  • The replay-store sweep runs off the request path.
  • Cache client metadata and JWKS per client_id.

Smaller surface

  • Make the endpoint classes and query-parameter types internal, using primary constructors.
  • Remove single-implementation interfaces with no test seam value.
  • Remove the 7 hand-written DI lambdas where TryAddSingleton<T>() suffices.
  • Remove the unused public members (research list).
  • Trim InMemorySpaceStoreWarning to the replay store.
  • Share the paging helper.
  • Replace LengthLimitedStream with the shared bounded reader.

CHANGELOG Security/Changed/Breaking changes as appropriate.

Part of the v0.7.0 update cycle (wave 6). Follow-up to #113: normative hardening, performance, and a smaller public server surface for Spaces. ## Protocol hardening (spaces research G14–G18, G22, G23) - [ ] DID-document rules from proposal 0016 (2026-09-02): an entry named `#atproto_space` / `#atproto_space_host` that exists but is malformed is an **error, not a fallback**. `#atproto_space_host` must have type `AtprotoSpaceHost` and an absolute https endpoint. Fall back only when the entry is absent. - [ ] Server token keys: restrict `kid` to `#atproto` / `#atproto_space`, and decide explicitly whether a missing `kid` is accepted. - [ ] DPoP: reject `ath` on the credential exchange, and optionally pin `ES256`. - [ ] `SpaceRepoCar.Verify`: - each index key must be exactly one `NSID/rkey`, both validated; - each record block must decode as a CBOR map; - index CIDs must be dag-cbor. - [ ] Space-type declarations: `collections` must not contain `*`; follow the proposal on `name` length. - [ ] Space and SimpleSpace models adopt the open-union and extension-data pattern from #115. - [ ] Docs: `sync.getBlob` does not serve space blobs. ## Performance - [ ] Cache verified space credentials keyed by `SHA-256(jwt)` until `min(exp, DID-cache TTL)`. Keep the per-request `exp`, subject and `jti` checks. Cache the imported JWK keys. Today each space read spends about 350 µs of CPU on crypto before touching data. - [ ] `SpaceCredentialProvider`: - per-space single-flight instead of one lock held across the network mint; - dispose superseded credentials, which currently grow without bound; - use the caching identity resolver. - [ ] `SpaceSyncer` recovery pre-sizes from `Content-Length` and parses the buffer without an extra copy. - [ ] EF stores use upserts (`ExecuteUpdateAsync` first). - [ ] The replay-store sweep runs off the request path. - [ ] Cache client metadata and JWKS per `client_id`. ## Smaller surface - [ ] Make the endpoint classes and query-parameter types `internal`, using primary constructors. - [ ] Remove single-implementation interfaces with no test seam value. - [ ] Remove the 7 hand-written DI lambdas where `TryAddSingleton<T>()` suffices. - [ ] Remove the unused public members (research list). - [ ] Trim `InMemorySpaceStoreWarning` to the replay store. - [ ] Share the paging helper. - [ ] Replace `LengthLimitedStream` with the shared bounded reader. CHANGELOG `Security`/`Changed`/`Breaking changes` as appropriate.
Author
Owner

Add to scope. This came out of #113, which was validated against a local spaces-alpha PDS (@atproto/dev-env@0.0.0-spaces-alpha-20260915165437).

Per-account signing for space service auth. The reference implementation requires the service-auth iss on notifyWrite to be the writer (or the space authority). It also requires the managing-app checkUserAccess call to come from the authority. The SDK signs every outbound call as its own service DID, which causes two failures:

  • A reference authority refuses write notifications from SDK-hosted repos.
  • A bulletin-style managing app refuses access checks from an SDK authority, unless the SDK's service DID is the authority DID.

A fix needs a way to sign as the account whose repo the SDK hosts. That means an account-key signer seam, for example ISpaceAccountSigner resolved per DID, used by SpaceWriteNotifier and SimpleSpaceAccessPolicy, with the service key as the fallback. Add integration coverage against the alpha image.

Add to scope. This came out of #113, which was validated against a local spaces-alpha PDS (`@atproto/dev-env@0.0.0-spaces-alpha-20260915165437`). **Per-account signing for space service auth.** The reference implementation requires the service-auth `iss` on `notifyWrite` to be the **writer** (or the space authority). It also requires the managing-app `checkUserAccess` call to come from the **authority**. The SDK signs every outbound call as its own service DID, which causes two failures: - A reference authority refuses write notifications from SDK-hosted repos. - A bulletin-style managing app refuses access checks from an SDK authority, unless the SDK's service DID *is* the authority DID. A fix needs a way to sign as the account whose repo the SDK hosts. That means an account-key signer seam, for example `ISpaceAccountSigner` resolved per DID, used by `SpaceWriteNotifier` and `SimpleSpaceAccessPolicy`, with the service key as the fallback. Add integration coverage against the alpha image.
Author
Owner

Also from the #149 review, which is non-blocking: LexiconTypeRegistry.RegisterUnionVariant accepts a variant for a [AtProtoUnion(Closed = true)] base, and the read path consults the registry before the closed check. Either reject that or document it as a deliberate override, and add a test. Handle it when the Space/SimpleSpace unions move to the open-union pattern.

Also from the #149 review, which is non-blocking: `LexiconTypeRegistry.RegisterUnionVariant` accepts a variant for a `[AtProtoUnion(Closed = true)]` base, and the read path consults the registry before the closed check. Either reject that or document it as a deliberate override, and add a test. Handle it when the Space/SimpleSpace unions move to the open-union pattern.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/ATProto.NET#133
No description provided.