Scope nested artifact GET/DELETE to the path conversation

CHANGELOG.md

@@ -8,6 +8,12 @@ loosely while pre-1.0 (breaking changes can land on minor bumps).
 ## [Unreleased]
 
 ### Fixed
+- **Nested artifact routes honor `:cid`.** `GET`/`DELETE`
+  `/v1/conversations/:id/artifacts/:aid` (and `/raw`) 404 when the
+  conversation is missing or TUI-origin, or when the artifact belongs
+  to a different conversation. Matches the documented tenant+conversation
+  pair and the list/create prefix. Tenant-wide `/v1/artifacts/:aid`
+  is unchanged.
 - **Sandbox FIFO no longer deadlocks a tenant.** `/write` and `/read` of a
   named pipe used to block forever in `open()`. `/write` holds the
   per-tenant sandbox mutex across that open, so a workspace `mkfifo`

docs/api/artifacts/conversations-delete.md

@@ -30,7 +30,8 @@ curl -X DELETE \
 | Status | When | Body |
 |--------|------|------|
 | 401    | Missing / invalid bearer. | `{"error": "..."}` |
-| 404    | Id doesn't exist for this tenant + conversation pair. | `{"error": "artifact not found"}` |
+| 404    | Conversation is missing or TUI-origin. | `{"error": "conversation not found"}` |
+| 404    | Artifact missing or belongs to another conversation. | `{"error": "artifact not found"}` |
 
 ## See also
 

docs/api/artifacts/conversations-get.md

@@ -43,7 +43,8 @@ Field schemas: [Data model → ArtifactRecord](../../concepts/data-model.md#arti
 | Status | When | Body |
 |--------|------|------|
 | 401    | Missing / invalid bearer. | `{"error": "..."}` |
-| 404    | Id doesn't exist for this tenant + conversation pair. | `{"error": "artifact not found"}` |
+| 404    | Conversation is missing or TUI-origin. | `{"error": "conversation not found"}` |
+| 404    | Artifact missing or belongs to another conversation. | `{"error": "artifact not found"}` |
 
 ## See also
 

docs/api/artifacts/conversations-raw.md

@@ -43,7 +43,8 @@ When `If-None-Match` matches. Empty body, includes the same `ETag`.
 | Status | When | Body |
 |--------|------|------|
 | 401    | Missing / invalid bearer. | `{"error": "..."}` |
-| 404    | Id doesn't exist for this tenant + conversation pair, or the row's content is missing (data corruption — defensive). | `{"error": "artifact not found"}` or `{"error": "artifact content missing"}` |
+| 404    | Conversation is missing or TUI-origin. | `{"error": "conversation not found"}` |
+| 404    | Artifact missing, belongs to another conversation, or the row's content is missing (data corruption — defensive). | `{"error": "artifact not found"}` or `{"error": "artifact content missing"}` |
 
 ## See also
 

include/tenant_store.h

@@ -1039,14 +1039,20 @@ class TenantStore {
                                     const std::string& mime_type);
 
     // Metadata-only fetch — does NOT load the BLOB.  Use this for list
-    // pages, the JSON metadata endpoint, agent /list, etc.
+    // pages, the JSON metadata endpoint, agent /list, etc.  When
+    // `conversation_id > 0`, the row must also belong to that
+    // conversation (nested /v1/conversations/:cid/artifacts/:aid).
+    // `conversation_id == 0` is tenant-wide by id.
     std::optional<ArtifactRecord>
-    get_artifact_meta(int64_t tenant_id, int64_t id) const;
+    get_artifact_meta(int64_t tenant_id, int64_t id,
+                      int64_t conversation_id = 0) const;
 
     // BLOB fetch — separate so list paths don't pull megabytes.  Returns
-    // nullopt if the row doesn't exist for this tenant.
+    // nullopt if the row doesn't exist for this tenant (and, when
+    // `conversation_id > 0`, for that conversation).
     std::optional<std::string>
-    get_artifact_content(int64_t tenant_id, int64_t id) const;
+    get_artifact_content(int64_t tenant_id, int64_t id,
+                         int64_t conversation_id = 0) const;
 
     // Lookup by (tenant, conversation, path) — used by the agent
     // /read slash command to address artifacts the way they were
@@ -1065,7 +1071,12 @@ class TenantStore {
     std::vector<ArtifactRecord>
     list_artifacts_tenant(int64_t tenant_id, int limit) const;
 
-    bool delete_artifact(int64_t tenant_id, int64_t id);
+    // Tenant-scoped by default.  When `conversation_id > 0`, refuse (and
+    // do not nullify memory links) unless the row belongs to that
+    // conversation — so DELETE /v1/conversations/:cid/artifacts/:aid
+    // cannot remove a sibling thread's blob.
+    bool delete_artifact(int64_t tenant_id, int64_t id,
+                         int64_t conversation_id = 0);
 
     // SUM(size) — used by put_artifact for quota math and by HTTP
     // callers exposing "you have used X of Y" surfaces.

src/api_server.cpp

@@ -3466,20 +3466,35 @@ void handle_artifact_list_tenant(int fd, TenantStore& tenants, const Tenant& ten
     write_json_response(fd, 200, body);
 }
 
+// conversation_id == 0 is the tenant-wide /v1/artifacts/:aid surface.
+// Nested /v1/conversations/:cid/artifacts/:aid passes :cid so a
+// sibling-thread (or TUI-origin) id cannot be read or deleted through
+// the conversation-scoped path.  Missing / TUI conversations 404 like
+// list/create on the same prefix.
 void handle_artifact_get_meta(int fd, int64_t artifact_id,
-                                TenantStore& tenants, const Tenant& tenant) {
-    auto rec = tenants.get_artifact_meta(tenant.id, artifact_id);
+                                TenantStore& tenants, const Tenant& tenant,
+                                int64_t conversation_id = 0) {
+    if (conversation_id > 0 &&
+        !get_http_conversation(tenants, tenant.id, conversation_id)) {
+        return write_artifact_error(fd, 404, "conversation not found");
+    }
+    auto rec = tenants.get_artifact_meta(tenant.id, artifact_id, conversation_id);
     if (!rec) return write_artifact_error(fd, 404, "artifact not found");
     write_json_response(fd, 200, artifact_to_json(*rec));
 }
 
 // GET /v1/artifacts/:id/raw — content body with proper Content-Type +
 // ETag (= sha256) for conditional GETs.  Tenant-scoped lookup; cross-
-// tenant id surfaces as 404.
+// tenant id surfaces as 404.  Nested callers pass conversation_id.
 void handle_artifact_get_raw(int fd, int64_t artifact_id,
                               const HttpRequest& req,
-                              TenantStore& tenants, const Tenant& tenant) {
-    auto rec = tenants.get_artifact_meta(tenant.id, artifact_id);
+                              TenantStore& tenants, const Tenant& tenant,
+                              int64_t conversation_id = 0) {
+    if (conversation_id > 0 &&
+        !get_http_conversation(tenants, tenant.id, conversation_id)) {
+        return write_artifact_error(fd, 404, "conversation not found");
+    }
+    auto rec = tenants.get_artifact_meta(tenant.id, artifact_id, conversation_id);
     if (!rec) return write_artifact_error(fd, 404, "artifact not found");
 
     // ETag honors the strong-validator semantics — sha256 of the bytes.
@@ -3497,7 +3512,8 @@ void handle_artifact_get_raw(int fd, int64_t artifact_id,
         return;
     }
 
-    auto blob = tenants.get_artifact_content(tenant.id, artifact_id);
+    auto blob = tenants.get_artifact_content(tenant.id, artifact_id,
+                                              conversation_id);
     if (!blob) return write_artifact_error(fd, 404, "artifact content missing");
 
     std::ostringstream ss;
@@ -3512,8 +3528,13 @@ void handle_artifact_get_raw(int fd, int64_t artifact_id,
 }
 
 void handle_artifact_delete(int fd, int64_t artifact_id,
-                              TenantStore& tenants, const Tenant& tenant) {
-    if (!tenants.delete_artifact(tenant.id, artifact_id))
+                              TenantStore& tenants, const Tenant& tenant,
+                              int64_t conversation_id = 0) {
+    if (conversation_id > 0 &&
+        !get_http_conversation(tenants, tenant.id, conversation_id)) {
+        return write_artifact_error(fd, 404, "conversation not found");
+    }
+    if (!tenants.delete_artifact(tenant.id, artifact_id, conversation_id))
         return write_artifact_error(fd, 404, "artifact not found");
     auto body = jobj();
     body->as_object_mut()["deleted"] = jbool(true);
@@ -11636,13 +11657,16 @@ void ApiServer::handle_connection(int fd) {
                                                   "method not allowed\n");
                             return;
                         }
-                        return handle_artifact_get_raw(fd, aid, req, tenants_, *tenant);
+                        return handle_artifact_get_raw(fd, aid, req, tenants_,
+                                                       *tenant, id);
                     }
                     if (segs.size() == 5) {
                         if (req.method == "GET")
-                            return handle_artifact_get_meta(fd, aid, tenants_, *tenant);
+                            return handle_artifact_get_meta(fd, aid, tenants_,
+                                                           *tenant, id);
                         if (req.method == "DELETE")
-                            return handle_artifact_delete(fd, aid, tenants_, *tenant);
+                            return handle_artifact_delete(fd, aid, tenants_,
+                                                          *tenant, id);
                         write_plain_response(fd, 405, "Method Not Allowed",
                                               "method not allowed\n");
                         return;

src/tenant_store.cpp

@@ -3628,23 +3628,33 @@ TenantStore::put_artifact(int64_t tenant_id, int64_t conversation_id,
 }
 
 std::optional<ArtifactRecord>
-TenantStore::get_artifact_meta(int64_t tenant_id, int64_t id) const {
+TenantStore::get_artifact_meta(int64_t tenant_id, int64_t id,
+                                int64_t conversation_id) const {
     if (!db_) return std::nullopt;
-    Stmt q(db_, (std::string("SELECT ") + kArtifactMetaCols +
-                 " FROM tenant_artifacts WHERE tenant_id = ? AND id = ?;").c_str());
+    std::string sql = std::string("SELECT ") + kArtifactMetaCols +
+                      " FROM tenant_artifacts WHERE tenant_id = ? AND id = ?";
+    if (conversation_id > 0) sql += " AND conversation_id = ?";
+    sql += ";";
+    Stmt q(db_, sql.c_str());
     q.bind(1, tenant_id);
     q.bind(2, id);
+    if (conversation_id > 0) q.bind(3, conversation_id);
     if (q.step() != SQLITE_ROW) return std::nullopt;
     return row_to_artifact(q);
 }
 
 std::optional<std::string>
-TenantStore::get_artifact_content(int64_t tenant_id, int64_t id) const {
+TenantStore::get_artifact_content(int64_t tenant_id, int64_t id,
+                                   int64_t conversation_id) const {
     if (!db_) return std::nullopt;
-    Stmt q(db_, "SELECT content FROM tenant_artifacts "
-                 "WHERE tenant_id = ? AND id = ?;");
+    std::string sql = "SELECT content FROM tenant_artifacts "
+                      "WHERE tenant_id = ? AND id = ?";
+    if (conversation_id > 0) sql += " AND conversation_id = ?";
+    sql += ";";
+    Stmt q(db_, sql.c_str());
     q.bind(1, tenant_id);
     q.bind(2, id);
+    if (conversation_id > 0) q.bind(3, conversation_id);
     if (q.step() != SQLITE_ROW) return std::nullopt;
     const void* blob = sqlite3_column_blob(q.raw(), 0);
     int n = sqlite3_column_bytes(q.raw(), 0);
@@ -3700,8 +3710,15 @@ TenantStore::list_artifacts_tenant(int64_t tenant_id, int limit) const {
     return out;
 }
 
-bool TenantStore::delete_artifact(int64_t tenant_id, int64_t id) {
+bool TenantStore::delete_artifact(int64_t tenant_id, int64_t id,
+                                  int64_t conversation_id) {
     if (!db_) return false;
+    // Nested HTTP routes pass conversation_id so a mismatched :cid
+    // cannot nullify memory links or drop a sibling conversation's row.
+    if (conversation_id > 0 &&
+        !get_artifact_meta(tenant_id, id, conversation_id)) {
+        return false;
+    }
     // Soft cascade: nullify memory_entries.artifact_id referencing this
     // row before deleting it.  The schema-level FK couldn't be added by
     // ALTER TABLE (SQLite limitation), so we do the SET NULL ourselves.

tests/test_artifacts.cpp

@@ -279,6 +279,54 @@ TEST_CASE("artifacts are tenant-isolated") {
     CHECK(s.bytes_used_tenant(b) == 6);
 }
 
+TEST_CASE("id lookup and delete honor optional conversation_id") {
+    TempDb db; TenantStore s; s.open(db.path.string());
+    const int64_t tid = make_tenant(s, "acme");
+    const int64_t c1  = make_conversation(s, tid, "thread-1");
+    const int64_t c2  = make_conversation(s, tid, "thread-2");
+
+    auto p1 = s.put_artifact(tid, c1, "a.txt", "from-c1", "text/plain");
+    auto p2 = s.put_artifact(tid, c2, "b.txt", "from-c2", "text/plain");
+    REQUIRE(p1.record.has_value());
+    REQUIRE(p2.record.has_value());
+    const int64_t aid1 = p1.record->id;
+    const int64_t aid2 = p2.record->id;
+
+    // Tenant-wide (conversation_id == 0) still finds either row.
+    REQUIRE(s.get_artifact_meta(tid, aid1).has_value());
+    REQUIRE(s.get_artifact_content(tid, aid2).has_value());
+
+    // Matching conversation_id returns the row.
+    auto scoped = s.get_artifact_meta(tid, aid1, c1);
+    REQUIRE(scoped.has_value());
+    CHECK(scoped->path == "a.txt");
+    auto blob = s.get_artifact_content(tid, aid1, c1);
+    REQUIRE(blob.has_value());
+    CHECK(*blob == "from-c1");
+
+    // Sibling :cid is a miss — same 404 the nested HTTP route must emit.
+    CHECK_FALSE(s.get_artifact_meta(tid, aid1, c2).has_value());
+    CHECK_FALSE(s.get_artifact_content(tid, aid1, c2).has_value());
+    CHECK_FALSE(s.get_artifact_meta(tid, aid2, c1).has_value());
+
+    // Mismatched delete is a no-op: row and memory link stay put.
+    auto entry = s.create_entry(tid, "reference", "Refers", "", "", "[]", aid1);
+    REQUIRE(entry.artifact_id == aid1);
+    CHECK_FALSE(s.delete_artifact(tid, aid1, c2));
+    REQUIRE(s.get_artifact_meta(tid, aid1, c1).has_value());
+    auto still_linked = s.get_entry(tid, entry.id);
+    REQUIRE(still_linked.has_value());
+    CHECK(still_linked->artifact_id == aid1);
+
+    // Matching conversation_id deletes only that row.
+    REQUIRE(s.delete_artifact(tid, aid1, c1));
+    CHECK_FALSE(s.get_artifact_meta(tid, aid1).has_value());
+    REQUIRE(s.get_artifact_meta(tid, aid2, c2).has_value());
+    auto cleared = s.get_entry(tid, entry.id);
+    REQUIRE(cleared.has_value());
+    CHECK(cleared->artifact_id == 0);
+}
+
 TEST_CASE("deleting a conversation cascades to its artifacts") {
     TempDb db; TenantStore s; s.open(db.path.string());
     const int64_t tid = make_tenant(s, "acme");