From 4679d20b99e5fecc190878f3ccde1e997fcb8cf3 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Fri, 2 Oct 2026 00:13:36 -0500 Subject: [PATCH] fix: make Spaces self reads collection-independent --- bench/README.md | 18 ++++++++++++++++++ bench/justfile | 4 ++++ bench/space_scopes.zig | 19 +++++++++++++++++++ src/internal/scopes.zig | 25 +++++++++++-------------- 4 files changed, 52 insertions(+), 14 deletions(-) create mode 100644 bench/space_scopes.zig diff --git a/bench/README.md b/bench/README.md index 0112894..d088bb3 100644 --- a/bench/README.md +++ b/bench/README.md @@ -461,3 +461,21 @@ Two `just bench write-profile 10 1000` runs after the lazy-MST change reported 2527.1-2698.4 ops/s, p95 9.3-9.8 ms. Average write time was split roughly across lock wait 23-24%, lazy repo loading 9-10%, staging 10%, commit build 7%, and SQLite/event persistence 50%. + +### Spaces self-read scope parity (2026-10-02) + +`just bench space-scopes` alternates member-list-style `read_self` checks with +writes outside the granted collection. One million checks accept 500,000 reads +and reject all 500,000 writes. Before the fix, ZDS rejected both. On the local +Apple Silicon host, the ReleaseFast loop measured 42 ns/check before and +50–60 ns/check after; the earlier exit represented incorrect authorization. + +The official PDS at `5b95b2f2723a2882824fbb0b819fd6aad884ef20` uses +[`read_self` for listMembers](https://github.com/bluesky-social/atproto/blob/5b95b2f2723a2882824fbb0b819fd6aad884ef20/packages/pds/src/api/com/atproto/simplespace/listMembers.ts) +and its scope matcher makes both read actions collection-independent. A local +probe of that revision's `SpacePermission.matches` accepted the same 500,000 +reads and rejected all writes (14 ns/check in Node, with an already parsed +grant; this is not directly comparable to ZDS's parse-and-match measurement). +The local Tranquil scope implementation has no Spaces permission matcher, so +there is no equivalent probe there. No storage or protocol primitive changed; +this authorization policy remains local to ZDS. diff --git a/bench/justfile b/bench/justfile index b88c704..2a4a019 100644 --- a/bench/justfile +++ b/bench/justfile @@ -105,3 +105,7 @@ write-profile callers="10" ops_per_caller="500": startup runs="5" history="10000": {{zig}} build -Doptimize=ReleaseSafe cd .. && python3 tools/startup_bench.py --runs {{runs}} --history {{history}} + +# benchmark OAuth space-level reads and collection-restricted writes +space-scopes: + {{zig}} run -O ReleaseFast --dep scopes -Mroot=../bench/space_scopes.zig -Mscopes=../src/internal/scopes.zig diff --git a/bench/space_scopes.zig b/bench/space_scopes.zig new file mode 100644 index 0000000..a94fb1d --- /dev/null +++ b/bench/space_scopes.zig @@ -0,0 +1,19 @@ +const std = @import("std"); +const scopes = @import("scopes"); + +pub fn main() void { + const grants = "space:fm.plyr.privateMedia?authority=did:plc:alice&skey=self&collection=fm.plyr.track&action=read&action=create&manage=update"; + const iterations = 1_000_000; + const start = std.Io.Clock.awake.now(std.Options.debug_io).toNanoseconds(); + var allowed: usize = 0; + for (0..iterations) |i| { + const action: scopes.SpaceAction = if (i % 2 == 0) .read_self else .create; + const collection: ?[]const u8 = if (i % 2 == 0) null else "app.other.thing"; + std.mem.doNotOptimizeAway(action); + const result = scopes.spaceAllows(grants, action, "fm.plyr.privateMedia", "did:plc:alice", "self", collection); + std.mem.doNotOptimizeAway(result); + allowed += @intFromBool(result); + } + const elapsed = std.Io.Clock.awake.now(std.Options.debug_io).toNanoseconds() - start; + std.debug.print("space scopes: {d} operations, {d} allowed, {d} ns/op\n", .{ iterations, allowed, @divTrunc(elapsed, iterations) }); +} diff --git a/src/internal/scopes.zig b/src/internal/scopes.zig index b40d9a9..f5b01e1 100644 --- a/src/internal/scopes.zig +++ b/src/internal/scopes.zig @@ -286,7 +286,7 @@ fn spaceScopeMatches( const allowed = param["skey=".len..]; if (!std.mem.eql(u8, allowed, "*") and !std.mem.eql(u8, allowed, skey)) return false; } else if (std.mem.startsWith(u8, param, "collection=")) { - if (action == .read or switch (action) { + if (action == .read or action == .read_self or switch (action) { .manage_create, .manage_update, .manage_delete => true, else => false, }) continue; @@ -299,7 +299,7 @@ fn spaceScopeMatches( return switch (action) { .manage_create, .manage_update, .manage_delete => saw_manage and manage_matches, .read => if (saw_action) action_matches else true, - .read_self => (if (saw_action) action_matches else true) and (if (saw_collection) collection_matches else true), + .read_self => if (saw_action) action_matches else true, .create, .update, .delete => (if (saw_action) action_matches else true) and saw_collection and collection_matches, }; } @@ -373,18 +373,15 @@ test "space scopes constrain type identity key action and collection" { } test "space read scopes allow repo-level self reads" { - // regression: repo-state endpoints (getLatestCommit, getRepoState, getRepo, - // getBlob) authorize read_self with a null collection; scopes without a - // collection restriction must allow that, and read implies read_self. - try std.testing.expect(spaceAllows("space:app.bulleted.space?action=read", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); - try std.testing.expect(spaceAllows("space:app.bulleted.space", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); - try std.testing.expect(spaceAllows("space:app.bulleted.space?action=read", .read_self, "app.bulleted.space", "did:plc:alice", "self", "app.bulleted.bullet")); - // collection-restricted scopes still deny whole-repo reads and self reads - // of other collections - try std.testing.expect(!spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); - try std.testing.expect(!spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read_self, "app.bulleted.space", "did:plc:alice", "self", "app.other.thing")); - // read_self never grants cross-repo read - try std.testing.expect(!spaceAllows("space:app.bulleted.space?action=read_self", .read, "app.bulleted.space", "did:plc:alice", "self", null)); + const scope = "space:fm.plyr.privateMedia?authority=did:plc:alice&skey=self&collection=fm.plyr.track&action=read&action=create&manage=update"; + try std.testing.expect(spaceAllows(scope, .read_self, "fm.plyr.privateMedia", "did:plc:alice", "self", null)); + try std.testing.expect(spaceAllows(scope, .read_self, "fm.plyr.privateMedia", "did:plc:alice", "self", "app.other.thing")); + try std.testing.expect(spaceAllows(scope, .read, "fm.plyr.privateMedia", "did:plc:alice", "self", null)); + try std.testing.expect(!spaceAllows(scope, .read_self, "fm.plyr.privateMedia", "did:plc:bob", "self", null)); + try std.testing.expect(!spaceAllows(scope, .read_self, "fm.plyr.privateMedia", "did:plc:alice", "other", null)); + try std.testing.expect(!spaceAllows(scope, .create, "fm.plyr.privateMedia", "did:plc:alice", "self", "app.other.thing")); + try std.testing.expect(spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read_self, "app.bulleted.space", "did:plc:alice", "self", null)); + try std.testing.expect(!spaceAllows("space:app.bulleted.space?collection=app.bulleted.bullet&action=read_self", .read, "app.bulleted.space", "did:plc:alice", "self", null)); } test "account scopes constrain attribute and action" { -- 2.51.2