Skip to content

security(runtime): namespace isolation for get_edge, merge_note, traversal roots (#548 #567 #568 #569)#599

Merged
ohdearquant merged 12 commits into
mainfrom
show/khive-issue-sweep/rt-security
May 31, 2026
Merged

security(runtime): namespace isolation for get_edge, merge_note, traversal roots (#548 #567 #568 #569)#599
ohdearquant merged 12 commits into
mainfrom
show/khive-issue-sweep/rt-security

Conversation

@ohdearquant
Copy link
Copy Markdown
Owner

Summary

Closes four namespace-isolation gaps in khive-runtime where ID-based operations could leak or act on records outside the caller's namespace. All four route through the single ensure_namespace helper (fail-closed: foreign → NotFound/empty, absent → Ok(None)).

Issue Fix
#569 Refactor ensure_namespace to static (record_ns, caller_ns); route all ID-path checks through it — get_entity/delete_entity/update_entity + the previously-inline note/event checks in resolve, delete_note, edit_note (each preserving its return convention).
#548 get_edge now preflights graph_edges.namespace via SQL before the graph-store call; foreign edge id → Err(NotFound), absent → Ok(None).
#567 merge_note fetches both into_id/from_id and namespace-checks each before the blocking SQL transaction; either foreign id aborts with Err(NotFound).
#568 Root filtering via substrate_exists_in_ns across traverse, neighbors_with_query, bfs_traverse, shortest_path — foreign roots filtered before storage expansion (closes the include_roots:true echo leak).

Tests

Three new regression tests, each verified fail-closed (pre-fix behavior differs from the assertion):

  • get_edge_cross_namespace_returns_not_found
  • merge_note_cross_namespace_either_id_returns_not_found
  • traverse_foreign_namespace_root_yields_no_expansion

Plus the existing guard tests (resolve_prefix_invisible_across_namespaces, delete_note_cross_namespace_returns_mismatch_error) stay green unchanged.

cargo test --workspace0 failed · cargo clippy --workspace --all-targets -- -D warningsclean. No ADR-002 / schema / migration edits.

Scope

Part of the khive issue-sweep. Do not merge — awaiting Ocean's review. Each commit references its issue number(s).

🤖 Generated with Claude Code

@ohdearquant ohdearquant enabled auto-merge (squash) May 31, 2026 12:32
ohdearquant and others added 11 commits May 31, 2026 08:36
…per (#569)

Change ensure_namespace from an instance method (taking &self, &NamespaceToken, Uuid)
to a two-argument associated function (record_ns: &str, caller_ns: &str) so every
impl block can call it as Self::ensure_namespace without needing a runtime reference.
Update all three call sites in operations.rs and curation.rs to use the new signature.
Behavior is unchanged — wrong-namespace still returns RuntimeError::NotFound.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a namespace preflight to get_edge: query graph_edges for the namespace
of the requested edge id before calling the graph store. A foreign-namespace
edge id now returns RuntimeError::NotFound (fail-closed) instead of leaking
the edge payload. An absent id still returns Ok(None).

Also update substrate_exists_in_ns to treat Err(NotFound) from get_edge as
"not in this namespace" (Ok(false)) rather than propagating the error, so
traversal root filtering continues to work correctly after get_edge hardens.
Make substrate_exists_in_ns pub(crate) for graph_traversal.rs access (#568).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…567)

Before spawning the blocking merge transaction, fetch both into_id and from_id
from the note store and verify each note's namespace matches the caller's token
via ensure_namespace. Either foreign id now returns RuntimeError::NotFound before
any data is read or mutated, closing the cross-namespace merge path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Filter foreign-namespace roots before storage expansion in three traversal
entry points:

- neighbors_with_query: return empty vec if node_id is not visible in caller ns
- traverse: drain roots, keep only those visible in caller ns; return empty vec
  if no roots survive the filter (prevents storage from echoing include_roots)
- bfs_traverse: return empty vec if start node is foreign
- shortest_path: return None if either from or to is foreign (checked before
  the from==to fast path to avoid echoing a foreign self-path)

All four paths delegate to substrate_exists_in_ns which already treats
Err(NotFound) from get_edge as false (added in #548 commit).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…567, #568)

Add three denial tests that prove each fixed path is now fail-closed:

- get_edge_cross_namespace_returns_not_found (#548): creates an edge in ns-a,
  verifies it is visible from ns-a, asserts Err(NotFound) when fetched from ns-b.

- merge_note_cross_namespace_either_id_returns_not_found (#567): creates notes
  in ns-a and ns-b, asserts Err(NotFound) when into_id is foreign and again
  when from_id is foreign, so both positions of the preflight are covered.

- traverse_foreign_namespace_root_yields_no_expansion (#568): creates an a->b
  edge in ns-a with include_roots=true, traverses from ns-b using a.id as root,
  asserts the result is empty (storage never receives the foreign root id).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… ensure_namespace (#569)

Three remaining inline namespace comparisons now delegate to Self::ensure_namespace:

- operations.rs resolve (~1572,1580): `note.namespace == ns` and `event.namespace == ns`
  → `Self::ensure_namespace(&note.namespace, ns).is_ok()` / `&event.namespace`
  Preserves the filter-not-error semantics (foreign records are silently skipped).

- operations.rs delete_note (~1609): `note.namespace != ns`
  → `Self::ensure_namespace(&note.namespace, ns).is_err()`
  Preserves the Ok(false) convention (ADR-007 — foreign indistinguishable from absent).

- curation.rs edit_note (~461): `note.namespace != token.namespace().as_str()`
  → `Self::ensure_namespace(&note.namespace, token.namespace().as_str())?`
  Preserves the propagated-Err convention.

No behavior change. cargo test --workspace: 0 failures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#548)

ADR-007 requires absent and foreign-namespace IDs to be indistinguishable.
Map ensure_namespace failure to Ok(None) instead of propagating Err(NotFound).
Rename regression test to get_edge_cross_namespace_returns_none and add the
absent-vs-foreign equivalence assertion.
…le (#567)

NoteStore::get_note is ID-only (no namespace filter); ensure_note_kind could
disclose foreign note existence and kind before runtime denial. Replace with
runtime.resolve(token, id) which routes through ensure_namespace, returning
None for absent and foreign-namespace notes alike.

Add regression test ensure_note_kind_rejects_foreign_note_before_kind_check
verifying opaque NotFound for all expected_kind values against a foreign note.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ohdearquant ohdearquant force-pushed the show/khive-issue-sweep/rt-security branch from 79a006c to 7e6aa19 Compare May 31, 2026 12:36
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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