feat(state): owning accessors + pinned resolveLink — fix cross-node UAF (lifetime Phase 1)

inc/cvc/core/state.h

@@ -28,10 +28,12 @@
 #include <boost/algorithm/string/trim.hpp>
 #include <boost/chrono.hpp>
 #include <boost/date_time/posix_time/posix_time_types.hpp>
+#include <boost/enable_shared_from_this.hpp>
 #include <boost/foreach.hpp>
 #include <boost/function.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/property_tree/ptree.hpp>
+#include <boost/shared_ptr.hpp>
 #include <boost/thread/condition_variable.hpp>
 #include <cstddef>
 #include <cvc/core/app.h>
@@ -147,7 +149,14 @@ template <typename T> class state_future {
 // 01/12/2014 -- Joe R. -- Added init_funcs and json()
 // 01/13/2014 -- Joe R. -- Removing notifyXmlRpc() once and for all.
 // 12/08/2025 -- Added futures API for async value retrieval.
-class state {
+//
+// enable_shared_from_this (boost variant, matching state_ptr = boost::shared_ptr<state>): every
+// live node is already shared_ptr-owned (the root via instancePtr(), children via the _children
+// map), so a node can hand out its own owning pointer. This backs the owning accessors
+// (findDescendantShared / sharedChild / handle) that let a caller PIN a node across a concurrent
+// sweepExpired() — the bare state& / state* that operator() / findDescendant return do NOT keep the
+// node alive, so a concurrent structural mutation can free it under the caller (a cross-node UAF).
+class state : public boost::enable_shared_from_this<state> {
 public:
   typedef boost::shared_ptr<state> state_ptr;
   typedef std::map<std::string, state_ptr> child_map;
@@ -510,6 +519,14 @@ class state {
   struct link_resolution {
     link_resolution_kind kind = link_resolution_kind::resolved;
     state *target = nullptr;
+    // Owning pin of `target` (non-null exactly when `target` is): resolveLink walks and returns the
+    // terminal held as a state_ptr, so a caller that keeps `target_owned` (or the whole result)
+    // alive cannot have the resolved node freed under it by a concurrent sweepExpired().
+    state_ptr target_owned;
+    // Owning pins of the resolved terminal's ancestor chain (root's child .. terminal), so a caller
+    // that MUTATES `target` (writes walk its _parent chain) is safe against a concurrent sweep.
+    // Empty when the start node was itself the terminal (a non-link resolveLink caller).
+    std::vector<state_ptr> target_pins;
     std::vector<std::string> visited; // ordered absolute paths
     std::size_t hops = 0;
   };
@@ -605,6 +622,53 @@ class state {
   // Useful for link resolution and any other read-only navigation.
   state *findDescendant(const std::string &path);
 
+  // Normalize a state path to its canonical dot-separated form (trim; drop
+  // leading/trailing/duplicate separators). A pure STRING operation — unlike fullName(), it never
+  // walks the _parent chain, so it is the safe way to obtain a node's canonical absolute path when
+  // the node may have been orphaned by a concurrent sweep (used by the state:// resolver to build
+  // its canonical URI).
+  static std::string normalize_path(const std::string &path);
+
+  // Owning analogues of findDescendant() and operator(): they return the map's shared_ptr rather
+  // than a bare state* / state&, so the returned node stays alive for as long as the caller holds
+  // the returned state_ptr (or a `handle` wrapping it) — a concurrent sweepExpired() can then only
+  // UNLINK the node from its parent, never free it under the caller. Use these (not the bare
+  // accessors) whenever a node is read/used on a thread that a tree-wide sweep could run against
+  // (e.g. a compute-pool worker resolving `state://…`). NOTE: the pin covers the returned node's
+  // own storage (value/data/children/mutex); it does NOT pin the node's ancestor chain, so
+  // fullName() / parentName() on a node whose ancestor was concurrently swept is still unsafe —
+  // avoid those on a pinned-but-possibly-orphaned node (see
+  // docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md).
+  //
+  // findDescendantShared: read-only, returns a null state_ptr when any segment is absent.
+  // sharedChild: create-or-get (like operator()), returns the pinning state_ptr for the terminal.
+  //
+  // The optional `pins` out-vector collects an owning state_ptr for EVERY node on the resolved path
+  // (root's child .. the returned node), so a caller can keep the whole ANCESTOR CHAIN alive. Pass
+  // it when the caller will MUTATE the returned node (value()/data() internally call fullName() and
+  // parent()->childChanged(), which walk _parent — a leaf-only pin leaves those ancestors exposed
+  // to a concurrent sweep). Pass nullptr for a bare leaf pin — a read via the parent-free getters,
+  // or a caller that never walks _parent, needs no more.
+  state_ptr findDescendantShared(const std::string &path, std::vector<state_ptr> *pins = nullptr);
+  state_ptr sharedChild(const std::string &childname = std::string(),
+                        std::vector<state_ptr> *pins = nullptr);
+
+  // RAII sugar: a movable handle that reads like a node (operator-> / operator*) while pinning it
+  // alive. `handle h = node.sharedChild("a.b"); h->value("x");` keeps a.b alive for h's lifetime.
+  class handle {
+  public:
+    handle() = default;
+    explicit handle(state_ptr p) : _p(std::move(p)) {}
+    state *operator->() const { return _p.get(); }
+    state &operator*() const { return *_p; }
+    state *get() const { return _p.get(); }
+    const state_ptr &ptr() const { return _p; }
+    explicit operator bool() const { return static_cast<bool>(_p); }
+
+  private:
+    state_ptr _p;
+  };
+
   // -------- Phase 8 slice 6: pull-on-demand remote link resolution --------
   //
   // resolveRemote() extends resolveLink() by consulting the

src/cvc/ariadne/uri_state.cpp

@@ -27,18 +27,39 @@ std::string channel_of(const std::string &query) {
   return ch;
 }
 
+// The effective (link-followed) node, PINNED, plus its canonical absolute path. `node` is held as
+// an owning state_ptr and `path` is captured from strings we already hold (the addressed path, or
+// resolveLink's visited chain) — never a post-resolution fullName() on `node`, which would walk the
+// node's _parent chain and UAF if a concurrent sweepExpired() orphaned it (the confirmed cross-node
+// hazard for a `state://` resolve on a compute-pool worker).
+struct effective {
+  cvc::state::state_ptr node;
+  // The effective node PLUS its ancestor chain, kept alive so a write (value()/data() walk the
+  // node's _parent chain via fullName()/childChanged()) is safe against a concurrent sweep. For a
+  // followed transparent link this is the TARGET's chain; otherwise the addressed node's chain.
+  std::vector<cvc::state::state_ptr> pins;
+  std::string path;
+};
+
 // Follow a TRANSPARENT link to its terminal target; a non-link / opaque / broken / cyclic
 // transparent link stays put (resolvedValue's fallback). SHARED by read and write so both address
 // the identical node — crucially even a DEFAULT (non-writable) transparent link: a read follows it,
 // so a write must follow it too, or a store would be shadowed on the link node and unreadable via
 // the same URI (the write-through routing in state::value() only fires for a WRITABLE link).
-cvc::state *effective_node(cvc::state *node) {
+// `addressed_pins` is the addressed node's own ancestor chain (from the caller's navigation), used
+// when the node is not a followed link; `addressed_path` is its normalized `state://` path.
+effective effective_node(cvc::state::state_ptr node,
+                         std::vector<cvc::state::state_ptr> addressed_pins,
+                         const std::string &addressed_path) {
   if (node && node->isLink() && node->linkMode() == cvc::state::link_mode::transparent) {
     const cvc::state::link_resolution lr = node->resolveLink();
-    if (lr.kind == cvc::state::link_resolution_kind::resolved && lr.target)
-      return lr.target;
+    if (lr.kind == cvc::state::link_resolution_kind::resolved && lr.target_owned)
+      // target_owned pins the terminal and target_pins its ancestor chain; visited.back() is its
+      // absolute path captured during the walk (safe) — no fullName() on a possibly-orphaned node.
+      return {lr.target_owned, lr.target_pins,
+              lr.visited.empty() ? addressed_path : lr.visited.back()};
   }
-  return node;
+  return {std::move(node), std::move(addressed_pins), addressed_path};
 }
 
 // The `?value` / `?data` channels are served; `?children` and anything else are not (they need the
@@ -56,26 +77,35 @@ UriResult state_resolve(cvc::state &root, const Uri &u) {
                 "' is not served by the built-in state handler (only '?value' / '?data'); register "
                 "a custom handler for '?children'"};
 
-  // Navigate WITHOUT creating nodes; an empty path is the root itself.
-  cvc::state *node = u.path.empty() ? &root : root.findDescendant(u.path);
+  // Navigate WITHOUT creating nodes; an empty path is the root itself. PIN the addressed node AND
+  // its ancestor chain so a concurrent sweepExpired() (on the scheduler/pycvc thread) cannot free
+  // it — or an ancestor it walks — while this resolve, which may run on a compute-pool worker,
+  // reads its value/data (parent-free getters) and, for a transparent link, resolves through it
+  // (resolveLink touches the start node's fullName). Canonical is built from the normalized path
+  // STRING, never a fullName() on the returned node.
+  std::vector<cvc::state::state_ptr> chain;
+  cvc::state::state_ptr node =
+      u.path.empty() ? root.shared_from_this() : root.findDescendantShared(u.path, &chain);
   if (!node)
     return {false, std::string(), std::string(), "ari: state node '" + u.path + "' not found"};
-  cvc::state *eff = effective_node(node); // read follows a transparent link to its target
-  const std::string canonical = "state://" + eff->fullName() + "?" + channel;
+  const effective eff =
+      effective_node(node, std::move(chain),
+                     cvc::state::normalize_path(u.path)); // follows a transparent link
+  const std::string canonical = "state://" + eff.path + "?" + channel;
 
   if (channel == "data") {
     // The data channel carries a raw string/byte blob (what state_store writes here, and what the
     // §13.9 HTTP cache parks on a node). Typed data() payloads (a value_t / geometry) are a
     // different consumer (state-data-get) and are not byte-serialized here.
-    const boost::any d = eff->data();
+    const boost::any d = eff.node->data();
     if (const std::string *s = boost::any_cast<std::string>(&d))
       return {true, *s, canonical, std::string()};
     if (d.empty())
       return {true, std::string(), canonical, std::string()}; // empty data -> empty content
     return {false, std::string(), std::string(),
             "ari: state '" + u.path + "?data' holds a non-string payload (not byte-serializable)"};
   }
-  return {true, eff->value(), canonical, std::string()};
+  return {true, eff.node->value(), canonical, std::string()};
 }
 
 // §13.10 the write analogue: store `content` into the addressed node's `?value` (default) or
@@ -89,12 +119,21 @@ StoreResult state_store(cvc::state &root, const Uri &u, const std::string &conte
     return {false, std::string(),
             "ari: state channel '?" + channel +
                 "' is not writable by the built-in state handler (only '?value' / '?data')"};
-  cvc::state *eff = effective_node(u.path.empty() ? &root : &root(u.path));
+  // sharedChild CREATES the path if absent (a store may target a not-yet-existing node) and pins
+  // the WHOLE chain (into `chain`). Unlike the read, the write below MUST pin ancestors:
+  // value()/data() internally call fullName() and parent()->childChanged(), which walk the node's
+  // _parent chain — a leaf-only pin would leave those ancestors exposed to a concurrent sweep (a
+  // use-after-free). The pins (eff.pins for a followed link, else `chain`) are held alive across
+  // the setter call below.
+  std::vector<cvc::state::state_ptr> chain;
+  cvc::state::state_ptr node =
+      u.path.empty() ? root.shared_from_this() : root.sharedChild(u.path, &chain);
+  const effective eff = effective_node(node, std::move(chain), cvc::state::normalize_path(u.path));
   if (channel == "data")
-    eff->data(boost::any(content)); // store the bytes as a string blob on the data channel
+    eff.node->data(boost::any(content)); // store the bytes as a string blob on the data channel
   else
-    eff->value(content);
-  return {true, "state://" + eff->fullName() + "?" + channel, std::string()};
+    eff.node->value(content);
+  return {true, "state://" + eff.path + "?" + channel, std::string()};
 }
 
 } // namespace

src/cvc/core/state.cpp

@@ -784,6 +784,50 @@ state &state::operator()(const std::string &childname) {
   return (*child)(join(keys, SEPARATOR));
 }
 
+// state::sharedChild
+// -----------------
+// Owning create-or-get analogue of operator(): walks (creating as needed) the child path, holding
+// each hop as a state_ptr, and returns the terminal PINNED. Same create/lock semantics as
+// operator() (child under the parent's lock, released before descending); the difference is the
+// return type — the caller gets an owning pointer that keeps the node alive across a concurrent
+// sweepExpired(), where operator()'s bare state& would dangle.
+state::state_ptr state::sharedChild(const std::string &childname, std::vector<state_ptr> *pins) {
+  using namespace boost::algorithm;
+
+  boost::this_thread::interruption_point();
+
+  std::vector<std::string> keys;
+  split(keys, childname, is_any_of(SEPARATOR));
+  BOOST_FOREACH (std::string &key, keys)
+    trim(key);
+  while (!keys.empty() && keys.front().empty())
+    keys.erase(keys.begin());
+
+  state_ptr cur = shared_from_this();
+  BOOST_FOREACH (std::string &key, keys) {
+    if (key.empty())
+      continue; // skip empty interior/trailing segments, as operator() does via its leading-drop
+    state_ptr next;
+    {
+      boost::mutex::scoped_lock lock(cur->_mutex);
+      auto it = cur->_children.find(key);
+      if (it != cur->_children.end() && it->second) {
+        next = it->second;
+      } else {
+        next.reset(new state(cur->_ctx, key, cur.get()));
+        cur->_children[key] = next;
+        cur->_lastMod = boost::posix_time::microsec_clock::universal_time();
+        cur->_initialized = true;
+      }
+    }
+    cur = next;
+    if (pins)
+      pins->push_back(
+          cur); // pin every hop so the caller can safely walk the result's _parent chain
+  }
+  return cur;
+}
+
 // ----------------
 // state::linkTo / clearLink / isLink / linkTarget / resolveLink
 // ----------------
@@ -999,6 +1043,38 @@ state *state::findDescendant(const std::string &path) {
   return cur;
 }
 
+state::state_ptr state::findDescendantShared(const std::string &path,
+                                             std::vector<state_ptr> *pins) {
+  using namespace boost::algorithm;
+  std::string normalized = normalize_state_path(path);
+  if (normalized.empty())
+    return shared_from_this();
+  std::vector<std::string> keys;
+  split(keys, normalized, is_any_of(SEPARATOR));
+  state_ptr cur = shared_from_this();
+  for (auto &k : keys) {
+    trim(k);
+    if (k.empty())
+      continue;
+    state_ptr next;
+    {
+      boost::mutex::scoped_lock lock(cur->_mutex);
+      auto it = cur->_children.find(k);
+      if (it == cur->_children.end() || !it->second)
+        return state_ptr();
+      next = it->second; // copy the OWNING shared_ptr — pins this hop past a concurrent sweep
+    }
+    cur = next;
+    if (pins)
+      pins->push_back(
+          cur); // pin every hop so the caller can safely walk the result's _parent chain
+  }
+  return cur;
+}
+
+// static
+std::string state::normalize_path(const std::string &path) { return normalize_state_path(path); }
+
 state::link_resolution state::resolveLink(std::size_t hop_budget) {
   link_resolution result;
 
@@ -1009,7 +1085,17 @@ state::link_resolution state::resolveLink(std::size_t hop_budget) {
   // when multi-tree lands the key extends to (tree_id, path).
   std::unordered_set<std::string> seen;
 
-  state *cur = this;
+  // Pin the walk: hold each hop as a state_ptr (start via shared_from_this(), each subsequent hop
+  // via findDescendantShared) so a concurrent sweepExpired() cannot free the node we read the link
+  // target from, and collect the terminal's ancestor chain into result.target_pins so a caller that
+  // MUTATES the terminal (a write walks its _parent chain) is safe too. Each hop's path comes from
+  // the target STRING (normalize_path == that node's fullName, since the target is absolute) rather
+  // than a fullName() _parent walk. The one remaining _parent walk is the START node's fullName()
+  // just below; it is safe when the caller pinned the addressed node's chain (the state:// resolver
+  // does), and is otherwise the caller's responsibility (a resolvedValue/resolvedData caller must
+  // not hold an orphaned start). See docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md.
+  state_ptr cur_owned = shared_from_this();
+  state *cur = cur_owned.get();
   // Record the starting node's path so cycles that loop back to
   // the start (including a self-link) are detected as cycles
   // rather than mistakenly classified as "resolved".
@@ -1026,6 +1112,9 @@ state::link_resolution state::resolveLink(std::size_t hop_budget) {
       // Terminal node: not a link.
       result.kind = (cur == this) ? link_resolution_kind::none : link_resolution_kind::resolved;
       result.target = cur;
+      result.target_owned = cur_owned; // pin the resolved terminal for the caller
+      // result.target_pins already holds this terminal's ancestor chain (set on the hop that
+      // reached it); empty iff the start node was itself the terminal.
       return result;
     }
 
@@ -1035,24 +1124,29 @@ state::link_resolution state::resolveLink(std::size_t hop_budget) {
       return result;
     }
 
-    state *next = root.findDescendant(target);
-    if (next == nullptr) {
+    std::vector<state_ptr> hop_pins;
+    state_ptr next_owned = root.findDescendantShared(target, &hop_pins);
+    if (!next_owned) {
       result.kind = link_resolution_kind::broken;
       result.target = nullptr;
       result.visited.push_back(target);
       return result;
     }
     ++result.hops;
 
-    std::string next_path = next->fullName();
+    // The target is an app-root-absolute path, so its normalized form IS next_owned->fullName() —
+    // use the string to avoid walking next_owned's (unpinned) ancestor chain.
+    std::string next_path = normalize_state_path(target);
     if (!seen.insert(next_path).second) {
       result.kind = link_resolution_kind::cycle_detected;
       result.target = nullptr;
       result.visited.push_back(next_path);
       return result;
     }
     result.visited.push_back(next_path);
-    cur = next;
+    result.target_pins = std::move(hop_pins); // this hop's chain; kept iff it becomes the terminal
+    cur_owned = next_owned;
+    cur = cur_owned.get();
   }
 }
 
@@ -1176,6 +1270,12 @@ state::send_message_result state::sendMessage(const std::string &payload,
     case link_resolution_kind::resolved:
     case link_resolution_kind::none:
       target = lr.target;
+      // Take the resolved absolute path from the walk while `lr` (and its pins) are still alive.
+      // Calling target->fullName() AFTER `lr` is dropped would walk the resolved terminal's _parent
+      // chain unpinned — safe today (sendMessage is scheduler-thread only) but a latent cross-node
+      // hazard if this ever runs off-thread; visited.back() is already that absolute path.
+      if (!lr.visited.empty())
+        r.resolved_path = lr.visited.back();
       break;
     case link_resolution_kind::broken:
       r.status = send_message_result::status_kind::broken_link;
@@ -1194,7 +1294,8 @@ state::send_message_result state::sendMessage(const std::string &payload,
       return r;
     }
   }
-  r.resolved_path = target->fullName();
+  if (r.resolved_path.empty())
+    r.resolved_path = target->fullName(); // non-link node (target == this): its own path
 
   // 2. Find the default shard for this app context. With no
   // shard registered (common in unit tests of pure-state code)