From 41a13a0721febe2f17995e3dc09478dcd2a49ee6 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Thu, 20 Aug 2026 17:29:02 -0500 Subject: [PATCH] fix(spaces): authority self-authorization + recreate-after-delete Two correctness gaps found while chasing why a Bulletin board vanished after creation, both checked against the reference PDS, pds.js, and atproto-crates: - spacePolicyAllowsRequester now authorizes the space authority for its own space before any policy branch. Without it, a managing-app board (which is what Bulletin creates) required the managing app to be reachable and to authorize the owner via checkUserAccess for the owner to read their own data. The reference PDS (authorizeUser) and pds.js (handlers/auth.js) both short-circuit userDid == authority for exactly this reason ("must not be able to lock itself out"). atproto-crates instead always defers managing-app to checkUserAccess; Bulletin's self-follow authorizes the owner there anyway, so matching the reference is strictly safer. - createSpace on a previously deleted space returned 500 (the actor-state row left by the delete collided on its primary key, and the tombstone was never cleared). The lexicon says a deleted space may be created again; the reference upserts via ensureSpace and atproto-crates via ON CONFLICT DO UPDATE. Now createSpace refreshes the config, clears deleted_at, and reactivates the actor row. A live space still answers SpaceAlreadyExists. Smoke covers recreate-after-delete; unit test covers authority self-auth. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 10 ++++++++++ src/atproto/space.zig | 22 ++++++++++++++++++++++ src/storage/store.zig | 11 ++++++++++- tools/smoke-permissioned.sh | 18 ++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a24a92..f67fd72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,16 @@ Reconstructed from git history for everything up to `v0.1.1`; kept by hand from what let a zds account post a note on a board hosted elsewhere. Before, the only way to materialize was the pre-alpha `createSpace {did: }` call. - creating a record that already exists answers `RecordAlreadyExists` (was `NotPermitted`). + - the space authority is always authorized for its own space, before any policy check. A + `managing-app` board no longer needs the managing app reachable (or the owner to satisfy + its `checkUserAccess` rule) for the owner to read their own data. Matches the reference PDS + (`SimpleSpaceManager.authorizeUser`) and pds.js (`handlers/auth.js`); atproto-crates instead + always defers `managing-app` to `checkUserAccess`, which for Bulletin authorizes the owner + anyway via a self-follow. + - recreating a previously deleted space succeeds (per the `createSpace` lexicon) instead of a + 500: the config is refreshed and the tombstone cleared, and the actor row left by the delete + is reactivated rather than colliding. Matches the reference (`ensureSpace` upsert) and + atproto-crates (`ON CONFLICT DO UPDATE`). A live space still answers `SpaceAlreadyExists`. ## 0.3.0 — 2026-08-20 diff --git a/src/atproto/space.zig b/src/atproto/space.zig index ee259cf..bab1f7d 100644 --- a/src/atproto/space.zig +++ b/src/atproto/space.zig @@ -1599,6 +1599,11 @@ fn spacePolicyAllowsRequester( requester_did: []const u8, client_id: ?[]const u8, ) !bool { + // The authority is the only party that can reconfigure the space, so it must + // never be able to lock itself out — and reaching its own data must not + // depend on a managing app being reachable. Matches the reference PDS + // (SimpleSpaceManager.authorizeUser). + if (std.mem.eql(u8, requester_did, space.authority_did)) return true; if (std.mem.eql(u8, space.policy, "public")) return true; if (std.mem.eql(u8, space.policy, "member-list")) return store.simpleSpaceHasMember(space.uri, requester_did); if (std.mem.eql(u8, space.policy, "managing-app")) return managingAppAllowsRequester(allocator, space, requester_did, client_id); @@ -1849,6 +1854,23 @@ test "simplespace getSpace view matches the alpha lexicon" { ); } +test "the space authority is always authorized for its own space" { + const managing_app_space: store.SpaceConfig = .{ + .uri = "at://did:plc:auth/space/my.bulletin.board/self", + .authority_did = "did:plc:auth", + .space_type = "my.bulletin.board", + .skey = "self", + .managing_app = "did:web:unreachable.invalid#bulletin", + .policy = "managing-app", + .app_access_json = "{\"type\":\"open\"}", + .is_authority = true, + .deleted_at = null, + }; + // Returns before the managing-app branch would try to resolve the (here + // deliberately unreachable) service, so no allocator or store is touched. + try std.testing.expect(try spacePolicyAllowsRequester(std.testing.allocator, managing_app_space, "did:plc:auth", null)); +} + test "listRepoOps cursors are rev/idx" { const cursor = parseOplogCursor("3lx7abcdefg22/4").?; try std.testing.expectEqualStrings("3lx7abcdefg22", cursor.rev); diff --git a/src/storage/store.zig b/src/storage/store.zig index 7336c18..e0848e9 100644 --- a/src/storage/store.zig +++ b/src/storage/store.zig @@ -3711,11 +3711,15 @@ pub fn createSpace(allocator: std.mem.Allocator, input: CreateSpaceInput) !Space input.app_access_json, }); if (input.is_authority) { + // Recreating a previously deleted space is allowed (per the lexicon), so + // the authority's config is refreshed and the tombstone cleared. A live + // space never reaches here: the guard above returns SpaceAlreadyExists. try conn.exec( \\UPDATE permissioned_spaces \\SET managing_app = ?, \\ policy = ?, - \\ app_access_json = ? + \\ app_access_json = ?, + \\ deleted_at = NULL \\WHERE uri = ? , .{ input.managing_app, @@ -3729,9 +3733,14 @@ pub fn createSpace(allocator: std.mem.Allocator, input: CreateSpaceInput) !Space \\VALUES (?, ?, NULL, NULL) \\ON CONFLICT(space, repo_did) DO NOTHING , .{ uri, input.authority_did }); + // Upsert, not plain insert: a recreate lands on the actor row left behind by + // the prior delete, which must be reactivated rather than collide. try conn.exec( \\INSERT INTO permissioned_space_actor_state (space, actor_did, is_authority) \\VALUES (?, ?, ?) + \\ON CONFLICT(space, actor_did) DO UPDATE SET + \\ is_authority = excluded.is_authority, + \\ deleted_at = NULL , .{ uri, input.actor_did, @as(i64, if (input.is_authority) 1 else 0) }); if (input.is_authority) { try conn.exec( diff --git a/tools/smoke-permissioned.sh b/tools/smoke-permissioned.sh index 180350b..630136d 100755 --- a/tools/smoke-permissioned.sh +++ b/tools/smoke-permissioned.sh @@ -127,6 +127,24 @@ space_duplicate_status=$(curl -sS -o /tmp/zds-space-duplicate.json -w '%{http_co test "$space_duplicate_status" = "400" grep -q '"error":"SpaceAlreadyExists"' /tmp/zds-space-duplicate.json +# recreate-after-delete: a deleted space may be created again (per the lexicon), +# and the fresh config replaces the old one with the tombstone cleared. +recreate_space_uri="at://did:plc:permissionsmoke/space/fm.plyr.privateMedia/recreate" +encoded_recreate_space=$(printf '%s' "$recreate_space_uri" | jq -sRr @uri) +curl -fsS -X POST "$base/xrpc/com.atproto.simplespace.createSpace" \ + -H "authorization: Bearer $token" -H 'content-type: application/json' \ + --data '{"type":"fm.plyr.privateMedia","skey":"recreate","policy":{"$type":"com.atproto.simplespace.defs#publicPolicy"},"appAccess":{"$type":"com.atproto.simplespace.defs#open"}}' \ + | grep -q '"uri":"'"$recreate_space_uri"'"' +curl -fsS -X POST "$base/xrpc/com.atproto.simplespace.deleteSpace" \ + -H "authorization: Bearer $token" -H 'content-type: application/json' \ + --data "$(jq -nc --arg space "$recreate_space_uri" '{space:$space}')" | grep -q '{}' +recreate_after_delete=$(curl -fsS -X POST "$base/xrpc/com.atproto.simplespace.createSpace" \ + -H "authorization: Bearer $token" -H 'content-type: application/json' \ + --data '{"type":"fm.plyr.privateMedia","skey":"recreate","policy":{"$type":"com.atproto.simplespace.defs#managingAppPolicy","managingApp":"did:web:plyr.fm"},"appAccess":{"$type":"com.atproto.simplespace.defs#open"}}') +test "$recreate_after_delete" = '{"uri":"'"$recreate_space_uri"'"}' +recreate_get=$(curl -fsS -H "authorization: Bearer $token" "$base/xrpc/com.atproto.simplespace.getSpace?space=$encoded_recreate_space") +test "$recreate_get" = '{"uri":"'"$recreate_space_uri"'","policy":{"$type":"com.atproto.simplespace.defs#managingAppPolicy","managingApp":"did:web:plyr.fm"},"appAccess":{"$type":"com.atproto.simplespace.defs#open"}}' + space_get=$(curl -fsS -H "authorization: Bearer $token" "$base/xrpc/com.atproto.simplespace.getSpace?space=$encoded_space") test "$space_get" = '{"uri":"at://did:plc:permissionsmoke/space/fm.plyr.privateMedia/self","policy":{"$type":"com.atproto.simplespace.defs#managingAppPolicy","managingApp":"did:web:plyr.fm"},"appAccess":{"$type":"com.atproto.simplespace.defs#open"}}' -- 2.51.2