x,edgraph: harden reserved-namespace registration and value-lock delete coverage#9754
Open
matthewmcneely wants to merge 2 commits into
Open
x,edgraph: harden reserved-namespace registration and value-lock delete coverage#9754matthewmcneely wants to merge 2 commits into
matthewmcneely wants to merge 2 commits into
Conversation
Two RegisterReservedNamespace misuse modes that previously surfaced only at mutation time, both now caught at init(): - A namespace-qualified name (containing NsSeparator) never matched the bare lookups, so a value-locked predicate registered in qualified form stayed publicly writable (fail open). Reject any qualified name in PredicatePrefix/Predicates/Types/ValueLocked at registration. - Two namespaces (or a re-init) claiming the same name silently last-writer-wins, letting import order pick the TrustMarker. Panic on a duplicate predicate/type/value-lock instead. Adds tests for the qualified-name and duplicate paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…* * gap The value-lock guard runs only on the /mutate set+del NQuad path. Two delete vectors bypassed it: - Alter DropAttr builds its own delete edge and calls ApplyMutations directly. Apply the same TrustMarker check there, so a value-locked predicate can't be dropped by an external or admin caller lacking the owning service's marker. - A 'S * *' delete reaches the guard with the wildcard predicate (matching no value lock), and the wildcard expands post-Raft in worker where the request context is gone, so it can't be enforced per-predicate at validation. Documented the boundary at the guard call; bulk subject deletes that remove a value-locked predicate are gated by ACL predicate-level permissions instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #9753 (the reserved-namespace registry), addressing two hardening points from review of that work.
Registration input validation (
x/keys.go)RegisterReservedNamespacenow panics atinit()on two misconfigurations that previously surfaced only at mutation time:PredicatePrefix/Predicates/Types/ValueLocked.TrustMarker. Now panics.Value-lock delete coverage (
edgraph/server.go)The value-lock guard (
newReservedPredicateGuard) ran only on the/mutateset+del NQuad path. Two delete vectors bypassed it:Alter DropAttrbuilds its own delete edge and callsApplyMutationsdirectly. It now applies the sameTrustMarkercheck, so a value-locked predicate can't be dropped by a caller lacking the owning service's marker.S * *delete reaches the guard with the wildcard predicate (matching no value lock), and the wildcard is expanded post-Raft inworkerwhere the request context is gone, so it can't be enforced per-predicate at validation. Documented the boundary at the guard; bulk subject deletes that remove a value-locked predicate are gated by ACL predicate-level permissions instead.Tests / safety
x/reserved_namespace_test.gocovers the qualified-name and duplicate rejections. TheDropAttrenforcement is exercised by an integration test in the consuming downstream (the value lock has no in-tree consumer, since the registry is empty in a stock build). With no registration the registry stays empty and behavior is identical to before.