Эх сурвалжийг харах

fix(auth): deny scoped keys on collection-management routes (requireProjectManage)

A capability-scoped (publishable) key could previously create and delete
collections via requireProjectAccess alone. Introduce requireProjectManage,
which calls requireProjectAccess then throws 403 if k.scope is set. Wire it
into all four collection routes (POST create, GET list, GET by-name, DELETE).
Add regression test ApiFixture.ScopedKeyCannotManageCollections. Update spec.
Fszontagh 1 сар өмнө
parent
commit
105cbb5774

+ 4 - 2
docs/superpowers/specs/2026-06-15-scoped-api-keys-design.md

@@ -98,8 +98,10 @@ is rejected at create/patch (422).
 | `PATCH …/documents/{id}` (documents.cpp) | `update` |
 | `DELETE …/documents/{id}` (documents.cpp) | `delete` |
 
-Collection-management (`/collections` create/list/delete), project, key, settings,
-and stats routes remain **admin-only** and are never granted to a scoped key.
+Collection-management (`/collections` create/list/delete) and per-project routes
+stay gated by `requireProjectAccess`; additionally, **scoped keys are explicitly
+denied collection-management via `requireProjectManage`** (a scope cannot express
+collection ops). Project/key/settings/stats-global routes remain admin-only.
 (`read` = get-by-id only; withholding `list` means a key cannot enumerate a
 collection — e.g. `quotes` insert-only with no read/list means leads are never
 exfiltrable, and `designs` read-by-id requires already knowing the share id.)

+ 4 - 4
src/handlers/collections.cpp

@@ -10,7 +10,7 @@ void registerCollectionRoutes(ApiServer& s) {
 
     svr.Post(R"(/api/v1/projects/([^/]+)/collections)", [d](const httplib::Request& req, httplib::Response& res) {
         ApiKey k = requireKey(*d, req); std::string project = req.matches[1];
-        requireProjectAccess(k, project);
+        requireProjectManage(k, project);
         d->db.ensureProject(project); d->registry.ensure(project);
         auto body = bodyJson(req);
         std::string name = body.value("name", ""), kind = body.value("kind", "json");
@@ -38,7 +38,7 @@ void registerCollectionRoutes(ApiServer& s) {
 
     svr.Get(R"(/api/v1/projects/([^/]+)/collections)", [d](const httplib::Request& req, httplib::Response& res) {
         ApiKey k = requireKey(*d, req); std::string project = req.matches[1];
-        requireProjectAccess(k, project);
+        requireProjectManage(k, project);
 
         auto lower = [](std::string s) {
             std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { return (char)std::tolower(c); });
@@ -89,7 +89,7 @@ void registerCollectionRoutes(ApiServer& s) {
 
     svr.Get(R"(/api/v1/projects/([^/]+)/collections/([^/]+))", [d](const httplib::Request& req, httplib::Response& res) {
         ApiKey k = requireKey(*d, req); std::string project = req.matches[1], name = req.matches[2];
-        requireProjectAccess(k, project);
+        requireProjectManage(k, project);
         auto m = d->registry.get(project, name);
         if (!m) throw ApiError(ErrCode::NotFound, "not_found", "no such collection");
         nlohmann::json e = m->toJson();
@@ -101,7 +101,7 @@ void registerCollectionRoutes(ApiServer& s) {
 
     svr.Delete(R"(/api/v1/projects/([^/]+)/collections/([^/]+))", [d](const httplib::Request& req, httplib::Response& res) {
         ApiKey k = requireKey(*d, req); std::string project = req.matches[1], name = req.matches[2];
-        requireProjectAccess(k, project);
+        requireProjectManage(k, project);
         if (!d->registry.get(project, name)) throw ApiError(ErrCode::NotFound, "not_found", "no such collection");
         const std::string qc = qualify(project, name);
         // dropCollection() clears the collection's metadata but does NOT purge the

+ 5 - 0
src/server.cpp

@@ -38,6 +38,11 @@ void requireProjectAccess(const ApiKey& k, const std::string& project) {
     if (!k.canAccess(project))
         throw ApiError(ErrCode::Forbidden, "forbidden", "key not granted project '" + project + "'");
 }
+void requireProjectManage(const ApiKey& k, const std::string& project) {
+    requireProjectAccess(k, project);
+    if (k.scope)
+        throw ApiError(ErrCode::Forbidden, "forbidden", "scoped keys cannot manage collections");
+}
 
 void requireCapability(ServerDeps& d, const ApiKey& k, const httplib::Request& req,
                        const std::string& project, const std::string& collection, KeyOp op) {

+ 4 - 0
src/server.hpp

@@ -41,6 +41,10 @@ std::optional<ApiKey> resolveKey(ServerDeps& d, const httplib::Request& req);
 ApiKey requireKey(ServerDeps& d, const httplib::Request& req);              // throws Unauthorized
 void   requireAdmin(const ApiKey& k);                                      // throws Forbidden
 void   requireProjectAccess(const ApiKey& k, const std::string& project);  // throws Forbidden
+// Like requireProjectAccess, but additionally denies capability-scoped keys —
+// collection/project management is not expressible in a key scope, so a scoped
+// (publishable) key must never reach these routes.
+void   requireProjectManage(const ApiKey& k, const std::string& project);  // throws Forbidden
 void   requireCapability(ServerDeps& d, const ApiKey& k, const httplib::Request& req,
                          const std::string& project, const std::string& collection, KeyOp op);
 

+ 51 - 0
tests/test_api_integration.cpp

@@ -512,3 +512,54 @@ TEST_F(ApiFixture, EmbeddingCacheBypassedByNoCacheControlHeader) {
     countingMock.stop();
     countingThread.join();
 }
+
+// Security fix: scoped (publishable) keys must not be able to create or delete
+// collections even when granted the project. Collection-management is not expressible
+// in a key scope; a leaked publishable key must not be able to wipe the catalog.
+TEST_F(ApiFixture, ScopedKeyCannotManageCollections) {
+    // Build a capability-scoped key directly in the DB (key create API does not yet
+    // expose scope on POST — that is task U4). The key is granted project_ with a
+    // single data-level rule so it is a real scoped key (k.scope is populated).
+    svapi::ApiKey scopedKey;
+    scopedKey.id        = svapi::generateApiKey();
+    scopedKey.key       = svapi::generateApiKey();
+    scopedKey.label     = "test-scoped-coll";
+    scopedKey.projects  = {project_};
+    scopedKey.admin     = false;
+    scopedKey.createdAt = 0;
+    {
+        svapi::KeyScope sc;
+        svapi::KeyScopeRule rule;
+        rule.collection = "designs";
+        rule.ops        = {svapi::KeyOp::Insert};
+        sc.rules.push_back(std::move(rule));
+        scopedKey.scope = std::move(sc);
+    }
+    // Insert directly into the key collection so the server's KeyStore picks it up.
+    db_->client().upsert(keysColl_, scopedKey.toJson(), scopedKey.id);
+    keys_->bootstrap("");   // triggers reload() — bootstrap is idempotent if keys exist
+
+    const std::string base = "/api/v1/projects/" + project_ + "/collections";
+    auto scoped = http(scopedKey.key);
+
+    // POST /collections — scoped key must be denied (403)
+    auto createRes = scoped.Post(base.c_str(),
+        nlohmann::json{{"name", "scopedtest"}, {"kind", "json"}}.dump(), "application/json");
+    ASSERT_TRUE(createRes);
+    EXPECT_EQ(createRes->status, 403) << "scoped key must not create collections; body: " << createRes->body;
+
+    // DELETE /collections/<name> — scoped key must be denied (403)
+    auto delRes = scoped.Delete((base + "/any_collection").c_str());
+    ASSERT_TRUE(delRes);
+    EXPECT_EQ(delRes->status, 403) << "scoped key must not delete collections; body: " << delRes->body;
+
+    // Sanity: the fixture's unscoped admin key can still create a collection (201)
+    auto adminCreate = admin().Post(base.c_str(),
+        nlohmann::json{{"name", "scopedtest"}, {"kind", "json"}}.dump(), "application/json");
+    ASSERT_TRUE(adminCreate);
+    EXPECT_EQ(adminCreate->status, 201) << "unscoped admin key must still create collections";
+
+    // Cleanup
+    admin().Delete((base + "/scopedtest").c_str());
+    db_->client().remove(keysColl_, scopedKey.id);
+}