Skip to content

fix(graph): scope spanner tenant reads#148

Open
jonathanhaaswriter wants to merge 3 commits intofeat/graph-store-read-foundationfrom
feat/spanner-tenant-read-scope
Open

fix(graph): scope spanner tenant reads#148
jonathanhaaswriter wants to merge 3 commits intofeat/graph-store-read-foundationfrom
feat/spanner-tenant-read-scope

Conversation

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator

@jonathanhaaswriter jonathanhaaswriter commented Mar 24, 2026

Summary

  • mark SpannerGraphStore as tenant-scope-aware so tenant-configured reads can stay on the configured backend
  • enforce tenant visibility across Spanner node, edge, snapshot, and traversal reads instead of falling back to snapshot materialization
  • add TDD coverage for scoped Spanner reads and for CurrentSecurityGraphStoreForTenant preferring the configured Spanner backend

Validation

  • go test ./internal/graph ./internal/app
  • python3 scripts/devex.py run --files internal/app/app_graph_store_test.go internal/graph/store_spanner.go internal/graph/store_spanner_test.go

Stack

Base branch: feat/graph-store-read-foundation

@jonathanhaaswriter jonathanhaaswriter force-pushed the feat/graph-store-read-foundation branch from b271a42 to f80edf0 Compare March 25, 2026 15:28
@jonathanhaaswriter jonathanhaaswriter force-pushed the feat/spanner-tenant-read-scope branch from 8517988 to f1205c4 Compare March 25, 2026 15:30
@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator Author

Re-reviewed the current diff and I don't have any additional actionable findings right now.

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator Author

I think the tenant fail-closed contract regressed here. Once the configured Spanner store advertises tenant read scope, CurrentSecurityGraphStoreForTenant() prefers that path without the HasScopedNodesForTenant() guard the other tenant resolvers use. Because shared nodes are still visible under tenant scope, a tenant-missing request can return shared data instead of ErrStoreUnavailable.

Can we add the same tenant-presence check (or a Spanner equivalent) before returning the scoped store?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant