fix: one list of protected collections, and "projects" is on it

The runner and the webserver each kept their own list of the collections
SmartBotic runs on, and the two had drifted. The runner's named five; the
webserver's named ten; "projects" was in neither.

The runner's list is the one that decides what a workflow may touch, so a
workflow with defaultAccess read-write - a blanket yes to every name it
has not listed - could read and write projects, workflows, executions,
nodes, runners and workflow_groups. That is not a leak of data so much as
a way around the permission model: writing projects grants its owner
membership of any project, and writing workflows edits any workflow's
graph without passing a single check. Verified before fixing, with a
workflow that read both.

One list now, in storage/system_collections.hpp, with the tier attached
to the name rather than to whoever is asking. Workflows get nowhere near
any of it; the database page keeps the distinction it had, reading what
the services own but not writing it.

Ordering matters here and is the reason the first attempt failed its own
test. A workflow's private collection is protected too, so once it joined
this list the system check refused the workflow its own storage. The
ownership rule runs first: private storage is protected *from everybody
else*, and its owner is the exception, so asking "is this protected?"
before "is this mine?" locks a workflow out of its own data.

Only one live workflow uses defaultAccess read-write and it touches no
system collection, so nothing here changes what any existing workflow can
do - checked rather than assumed.

Verified: reading projects, workflows and users from a workflow with
defaultAccess read-write is refused by name, an insert into projects is
refused, and the control write to an ordinary collection in the same run
still succeeds - so this protects the system's data without turning
defaultAccess off. Through the database API projects reads as structural,
and both a document write and a collection delete answer 403. 67 passed,
0 failed, 2 skipped.

lib/storage/system_collections.hpp

@@ -0,0 +1,67 @@
+#pragma once
+
+#include <string>
+#include <unordered_set>
+
+#include "storage/workflow_collection.hpp"
+
+namespace smartbotic::storage {
+
+// The collections that belong to SmartBotic rather than to anybody's workflow.
+//
+// One list, read by both services. There used to be two - one in the runner
+// deciding what a workflow may touch, one in the webserver deciding what the
+// database page may touch - and they had drifted: the runner's named five
+// collections, the webserver's named ten, and "projects" was in neither. A
+// workflow with defaultAccess read-write could therefore read and rewrite
+// projects, workflows, executions, nodes and runners: it could grant its owner
+// membership of any project, or edit another workflow's graph, without going
+// past a single permission check.
+//
+// Two copies of a rule is one copy too many. Anything added here is protected
+// in both places at once, which is the only way the two stay in agreement.
+
+enum class CollectionProtection {
+    None,
+    // The services own these. Readable by an administrator looking after the
+    // data, but writing has to go through the endpoints that own them - a
+    // workflow document edited by hand sidesteps every check the workflow
+    // endpoints make, and a projects document edited by hand is a grant of
+    // access somebody gave themselves.
+    Structural,
+    // Password hashes, session tokens and encrypted credentials. Readable
+    // through an API means copyable out of it.
+    Secret,
+};
+
+inline CollectionProtection protectionForCollection(const std::string& collection) {
+    // A caller may address a collection with its project prefix. What it is
+    // does not depend on how it was spelled.
+    const std::string name = collectionBaseName(collection);
+
+    static const std::unordered_set<std::string> kSecret = {
+        "users", "sessions", "api_keys", "credentials", "collection_permissions"
+    };
+    static const std::unordered_set<std::string> kStructural = {
+        "projects", "workflows", "workflow_groups", "executions", "runners", "nodes"
+    };
+
+    if (kSecret.contains(name)) return CollectionProtection::Secret;
+    if (kStructural.contains(name)) return CollectionProtection::Structural;
+
+    // A workflow's own storage. Its owner reaches it through the ownership rule
+    // rather than through this list, and for everybody else it is as protected
+    // as anything above.
+    if (isWorkflowCollection(name)) return CollectionProtection::Structural;
+
+    return CollectionProtection::None;
+}
+
+// Whether a workflow is allowed near it at all. Workflows get no access to any
+// of these, at either tier: the distinction between "readable" and "writable"
+// is about an administrator using the database page, not about a workflow.
+inline bool isSystemCollection(const std::string& collection) {
+    return protectionForCollection(collection) != CollectionProtection::None;
+}
+
+}  // namespace smartbotic::storage

src/runner/collection_permissions.cpp

@@ -4,14 +4,6 @@
 namespace smartbotic::runner {
 
 // System collections that are always protected - no workflow access allowed
-const std::unordered_set<std::string> CollectionPermissions::SYSTEM_COLLECTIONS = {
-    "users",
-    "sessions",
-    "api_keys",
-    "credentials",
-    "collection_permissions"
-};
-
 std::string collectionAccessToString(CollectionAccess access) {
     switch (access) {
         case CollectionAccess::None: return "none";
@@ -116,7 +108,7 @@ CollectionAccess CollectionPermissions::getAccess(const std::string& collection)
 }
 
 bool CollectionPermissions::isSystemCollection(const std::string& collection) const {
-    return SYSTEM_COLLECTIONS.contains(collection);
+    return storage::isSystemCollection(collection);
 }
 
 std::unordered_map<std::string, CollectionAccess> CollectionPermissions::getPermissions() const {

src/runner/collection_permissions.hpp

@@ -5,6 +5,7 @@
 #include <unordered_set>
 #include <mutex>
 #include "storage/storage_client.hpp"
+#include "storage/system_collections.hpp"
 
 namespace smartbotic::runner {
 
@@ -38,7 +39,10 @@ public:
     // Get the access level for a collection
     CollectionAccess getAccess(const std::string& collection) const;
 
-    // Check if a collection is a system collection (always protected)
+    // Check if a collection is a system collection (always protected).
+    // The list lives in storage/system_collections.hpp so that the runner and
+    // the webserver cannot disagree about what is protected - they did, and
+    // "projects" fell through the gap between them.
     bool isSystemCollection(const std::string& collection) const;
 
     // Get all permission entries
@@ -58,9 +62,6 @@ private:
     std::unordered_map<std::string, CollectionAccess> permissions_;
     CollectionAccess default_access_ = CollectionAccess::None;
     mutable std::mutex mutex_;
-
-    // System collections that are always protected (no workflow access)
-    static const std::unordered_set<std::string> SYSTEM_COLLECTIONS;
 };
 
 } // namespace smartbotic::runner

src/runner/workflow_engine.cpp

@@ -1380,16 +1380,15 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
     // Helper to get per-workflow storage permission for a collection
     // Returns: "none", "read-only", or "read-write"
     auto getWorkflowAccess = [&workflow, this](const std::string& collection) -> std::string {
-        // System collections are always protected
-        if (collection_permissions_->isSystemCollection(collection)) {
-            return "none";
-        }
-
-        // A private collection is settled before the settings are consulted at
-        // all. Another workflow's is refused whatever the settings say - which
-        // matters most for defaultAccess, since a workflow set to read-write by
-        // default would otherwise reach every other workflow's private data,
-        // across projects, just by naming it.
+        // Private storage is settled first, ahead of both the system-collection
+        // check and the settings. Ahead of the system check because a workflow's
+        // own collection is protected *from everybody else* - its owner is the
+        // exception, and testing "is this protected?" before "is this mine?"
+        // locks a workflow out of its own data. Ahead of the settings because
+        // another workflow's must be refused whatever they say, which matters
+        // most for defaultAccess: a workflow set to read-write by default would
+        // otherwise reach every other workflow's data, across projects, just by
+        // naming it.
         if (storage::isWorkflowCollection(collection)) {
             if (!storage::isWorkflowCollectionOf(collection, workflow.id)) {
                 return "none";
@@ -1406,6 +1405,11 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
             return "read-write";
         }
 
+        // Everything SmartBotic runs on: no workflow goes near it.
+        if (collection_permissions_->isSystemCollection(collection)) {
+            return "none";
+        }
+
         // Check workflow settings for storage permissions
         if (workflow.settings.contains("storagePermissions")) {
             const auto& storage_perms = workflow.settings["storagePermissions"];

src/webserver/api/database_controller.cpp

@@ -1,5 +1,6 @@
 #include "database_controller.hpp"
 #include "logging/logger.hpp"
+#include "storage/system_collections.hpp"
 #include "storage/workflow_collection.hpp"
 #include <unordered_set>
 
@@ -98,33 +99,14 @@ void DatabaseController::registerRoutes(httplib::Server& server) {
 // ---------------------------------------------------------------------------
 
 DatabaseController::Protection DatabaseController::protectionFor(const std::string& collection) {
-    // A caller may address a collection with its project prefix. The protection
-    // is about which collection it is, not how it was spelled.
-    std::string name = collection;
-    const auto colon = name.rfind(':');
-    if (colon != std::string::npos) name = name.substr(colon + 1);
-
-    // Password hashes, session tokens and encrypted credentials. Readable
-    // through this API means copyable out of it.
-    static const std::unordered_set<std::string> SECRET = {
-        "users", "sessions", "api_keys", "credentials", "collection_permissions"
-    };
-    // The services read and write these themselves. Editing a workflow document
-    // by hand here would sidestep every check the workflow endpoints make, and
-    // dropping one would take the installation apart.
-    static const std::unordered_set<std::string> STRUCTURAL = {
-        "workflows", "workflow_groups", "executions", "runners", "nodes"
-    };
-
-    if (SECRET.contains(name)) return Protection::Secret;
-    if (STRUCTURAL.contains(name)) return Protection::Structural;
-
-    // A workflow's own collection. Readable here - an administrator looking
-    // after the data should be able to see what a workflow stored - but not
-    // writable, because the workflow is the only thing that should put data in
-    // it and dropping it is the workflow's own deletion, not a database edit.
-    if (storage::isWorkflowCollection(name)) return Protection::Structural;
-
+    // The list itself lives in storage/system_collections.hpp, shared with the
+    // runner. Keeping a second copy here is what let the two drift until
+    // "projects" was protected in neither.
+    switch (storage::protectionForCollection(collection)) {
+        case storage::CollectionProtection::Secret:     return Protection::Secret;
+        case storage::CollectionProtection::Structural: return Protection::Structural;
+        case storage::CollectionProtection::None:       return Protection::None;
+    }
     return Protection::None;
 }
 

tests/nodes/system-collections-refused.json

@@ -0,0 +1,90 @@
+{
+  "name": "verify-system-collections-refused",
+  "comment": "A workflow gets nowhere near the collections SmartBotic runs on, even with defaultAccess read-write. This is the setting that matters: a blanket yes to every unlisted name. Reading projects exposes who may do what; writing it is a grant of access somebody gave themselves. Writing workflows edits another workflow's graph without passing a single permission check.\n\nThis was open. The runner and the webserver each kept their own list of protected collections and the two had drifted - the runner named five, the webserver ten, and \"projects\" was in neither. The list is one list now, in storage/system_collections.hpp, so a name added there is protected in both places at once.\n\nThe control is an ordinary collection written under the same defaultAccess, which must still succeed: this test is about protecting the system's own data, not about turning defaultAccess off.",
+  "settings": {
+    "storagePermissions": {
+      "defaultAccess": "read-write"
+    }
+  },
+  "nodes": [
+    {
+      "id": "n1",
+      "name": "Trigger",
+      "type": "click-trigger",
+      "position": { "x": 0, "y": 0 },
+      "config": {}
+    },
+    {
+      "id": "projects",
+      "name": "Read who may do what",
+      "type": "storage-query",
+      "position": { "x": 0, "y": 100 },
+      "config": { "collectionSource": "manual", "collectionManual": "projects" }
+    },
+    {
+      "id": "workflows",
+      "name": "Read every workflow",
+      "type": "storage-query",
+      "position": { "x": 0, "y": 200 },
+      "config": { "collectionSource": "manual", "collectionManual": "workflows" }
+    },
+    {
+      "id": "users",
+      "name": "Read the password hashes",
+      "type": "storage-query",
+      "position": { "x": 0, "y": 300 },
+      "config": { "collectionSource": "manual", "collectionManual": "users" }
+    },
+    {
+      "id": "escalate",
+      "name": "Grant itself a project role",
+      "type": "storage-insert",
+      "position": { "x": 0, "y": 400 },
+      "config": {
+        "collectionSource": "manual",
+        "collectionManual": "projects",
+        "documentData": "{\"name\": \"trespass\"}"
+      }
+    },
+    {
+      "id": "control",
+      "name": "An ordinary collection still works",
+      "type": "storage-insert",
+      "position": { "x": 0, "y": 500 },
+      "config": {
+        "collectionSource": "manual",
+        "collectionManual": "wfprobe_control_collection",
+        "documentData": "{\"probe\": \"control\"}"
+      }
+    }
+  ],
+  "connections": [
+    { "sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "projects", "targetInput": "data" },
+    { "sourceNodeId": "projects", "sourceOutput": "main", "targetNodeId": "workflows", "targetInput": "data" },
+    { "sourceNodeId": "workflows", "sourceOutput": "main", "targetNodeId": "users", "targetInput": "data" },
+    { "sourceNodeId": "users", "sourceOutput": "main", "targetNodeId": "escalate", "targetInput": "data" },
+    { "sourceNodeId": "escalate", "sourceOutput": "main", "targetNodeId": "control", "targetInput": "data" }
+  ],
+  "expect": {
+    "projects": {
+      "status": "completed",
+      "output": { "error": "No read access to collection: projects" }
+    },
+    "workflows": {
+      "status": "completed",
+      "output": { "error": "No read access to collection: workflows" }
+    },
+    "users": {
+      "status": "completed",
+      "output": { "error": "No read access to collection: users" }
+    },
+    "escalate": {
+      "status": "completed",
+      "output": { "success": false, "error": "No write access to collection: projects" }
+    },
+    "control": {
+      "status": "completed",
+      "output": { "success": true }
+    }
+  }
+}