Skip to content

Fix ownership and CRDT scoping by database#187

Merged
farhan-syah merged 2 commits into
mainfrom
fix/drop-user-valid-owner-reassignment
Jul 20, 2026
Merged

Fix ownership and CRDT scoping by database#187
farhan-syah merged 2 commits into
mainfrom
fix/drop-user-valid-owner-reassignment

Conversation

@farhan-syah

@farhan-syah farhan-syah commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to the database-scoped ownership work already merged in ed928f4f. That commit re-keyed the owners map to include database_id but did not update every reader, which left ownership authorization broken and the cluster test crate uncompilable. This restores both.

Ownership authorization (435f3bb)

ed928f4f changed the owners-map key to {object_type}:{database_id}:{tenant_id}:{object_name}, but PermissionStore::check still looked it up with the unqualified collection_target string used for grants. The two shapes never match, so is_owner always returned false and owners silently lost implicit permissions on their own objects, falling through to explicit grants only. This is reached in production via authorize_collection_requirement.

check / check_function now take a database_id, and is_owner builds the key with owner_key rather than being handed a grant target. Owner keys and grant targets are documented as non-interchangeable so the two lookups don't get conflated again.

Three further sites looked up database-scoped owner rows with a hardcoded database 0:

  • ALTER COLLECTION … OWNER TO — the owner check, the get_collection, and the single-node fallback put_collection. For a collection outside the default database the fallback wrote the record into the wrong database.
  • DROP INDEX — ignored the database_id already present in its own signature.
  • SHOW PERMISSIONS ON <collection> — omitted the owner row outside the default database.

New test ownership_is_scoped_to_its_database pins both directions: an owner is recognised in their own database, and not in another database holding a same-named collection.

CRDT constraint snapshots (e4dffa0)

nodedb-cluster-tests did not compile against ed928f4f — three tests still destructured tuples that commit had widened, so stage 2 could not build. Fixed, along with a stale checkpoint-filename assertion and doc comments still describing the pre-ed928f4f scheme. The filename scheme itself is unchanged here.

TenantDataSnapshot::crdt_constraints moves from a 5-tuple to a named CrdtConstraintEntry struct. The tuple had crossed clippy's type_complexity threshold; a named type is the fix rather than an #[allow].

Validation

Two-stage suite green: stage 1 11971/11971, stage 2 310/310 (one flaky, lease_renews_before_expiry, passed on retry). Clippy clean.

Note: this branch is behind main and its runs did not include the CI commits merged since (c86abfe0, d990a85d, fe0e16cc). Stage 2 is worth re-running after merging main, as fe0e16cc touches the Calvin path.

Owner lookups previously keyed only on tenant_id, so a same-named
collection in another database within the same tenant would
incorrectly resolve to whichever owner happened to be recorded first.
Thread database_id through permission checks, namespace authorization,
and ownership DDL so ownership is correctly scoped per database.
CRDT constraint checkpoint filenames and tombstone lookups were keyed
by tenant and collection only, so a collection with the same name in
a different database within the same tenant could collide. Include
the database id in checkpoint filenames and replace the crdt_constraints
snapshot tuple with a named CrdtConstraintEntry struct carrying the
database id alongside tenant, collection, version, and constraints.
@farhan-syah
farhan-syah force-pushed the fix/drop-user-valid-owner-reassignment branch from e4dffa0 to cd6bd22 Compare July 20, 2026 09:54
@farhan-syah
farhan-syah merged commit ffaead5 into main Jul 20, 2026
1 check passed
@farhan-syah
farhan-syah deleted the fix/drop-user-valid-owner-reassignment branch July 20, 2026 09:55
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