From 884336b10903ef8e2855b6198ffe84f2ce106f5c Mon Sep 17 00:00:00 2001 From: brunolnetto Date: Tue, 7 Jul 2026 19:57:17 -0300 Subject: [PATCH 1/6] feat(ops): implement MoveFile and CopyItem across all providers Closes #5 Adds MoveItem() virtual method to ICloudBackend interface. Implements server-side move/rename for all 6 providers: SharePoint: - PATCH /drives/{root}/items/{id} with name + parentReference - Cache invalidation on source and destination parent OneDrive: - PATCH /drives/{root}/items/{id} (same Graph API as SharePoint) - Also adds CopyItem() (was missing despite supports_server_side_copy=true) Google Drive: - GET /files/{id}?fields=parents to retrieve old parent IDs - PATCH with addParents/removeParents to avoid multi-parent artifacts Dropbox: - POST /files/move_v2 (symmetric to existing CopyItem implementation) SFTP: - libssh2_sftp_rename_ex() with OVERWRITE|ATOMIC|NATIVE flags VFS Agent: - POST /v1/move with {from, to} JSON body CloudFileSystem::MoveFile(): - Validates same-provider and same-root constraint - Resolves root (with cache), src item, dst parent - Delegates to backend MoveItem() - Invalidates cache for both src and dst paths Build: 0 errors, 0 warnings --- src/core/cloud_filesystem.cpp | 49 ++++++++++++++++++- src/include/core/cloud_backend.hpp | 8 +++ src/include/providers/dropbox_backend.hpp | 4 ++ src/include/providers/gdrive_backend.hpp | 4 ++ src/include/providers/onedrive_backend.hpp | 8 +++ src/include/providers/sftp_backend.hpp | 4 ++ src/include/providers/sharepoint_backend.hpp | 4 ++ src/include/providers/vfs_backend.hpp | 4 ++ src/providers/dropbox/dropbox_backend.cpp | 18 +++++++ src/providers/gdrive/gdrive_backend.cpp | 36 ++++++++++++++ src/providers/onedrive/onedrive_backend.cpp | 30 ++++++++++++ src/providers/sftp/sftp_backend.cpp | 19 +++++++ .../sharepoint/sharepoint_backend.cpp | 15 ++++++ src/providers/vfs/vfs_backend.cpp | 13 +++++ 14 files changed, 214 insertions(+), 2 deletions(-) diff --git a/src/core/cloud_filesystem.cpp b/src/core/cloud_filesystem.cpp index af98e00..d4e2fbe 100644 --- a/src/core/cloud_filesystem.cpp +++ b/src/core/cloud_filesystem.cpp @@ -442,8 +442,53 @@ void CloudFileSystem::RemoveFile(const string& path, optional_ptr) { cache_.InvalidateItem(b.Scheme(), path); } -void CloudFileSystem::MoveFile(const string&, const string&, optional_ptr) { - Throw("cloudfs: MoveFile not yet implemented"); +void CloudFileSystem::MoveFile(const string& src_url, const string& dst_url, + optional_ptr) { + auto& b = BackendFor(src_url); + if (!b.CanHandle(dst_url)) + Throw("cloudfs: move across providers is not supported"); + + std::string src_raw, src_path, dst_raw, dst_path, err; + Throw(!b.ParseUrl(src_url, src_raw, src_path, err) ? err : ""); + Throw(!b.ParseUrl(dst_url, dst_raw, dst_path, err) ? err : ""); + if (src_raw != dst_raw) + Throw("cloudfs: cross-root move not supported; source and destination must share the same " + "storage root"); + + // Resolve root (with cache) + std::string root_id; + if (!cache_.GetRootId(b.Scheme(), src_raw, root_id)) { + std::string tok; + Throw(!GetToken(b.Scheme(), tok, err) ? err : ""); + Throw(!b.ResolveRoot(src_raw, tok, root_id, err) ? err : ""); + cache_.PutRootId(b.Scheme(), src_raw, root_id); + } + std::string tok; + Throw(!GetToken(b.Scheme(), tok, err) ? err : ""); + + // Stat the source item + CloudItem src_item; + Throw(!b.Stat(root_id, src_path, tok, src_item, err) ? "source not found: " + err : ""); + + // Decompose destination into parent path + filename + auto last_slash = dst_path.rfind('/'); + std::string dst_name = + (last_slash == std::string::npos) ? dst_path : dst_path.substr(last_slash + 1); + std::string dst_parent_path = + (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); + + // Stat the destination parent folder + CloudItem dst_parent; + Throw(!b.Stat(root_id, dst_parent_path, tok, dst_parent, err) + ? "destination parent not found: " + err + : ""); + + // Perform server-side move + Throw(!b.MoveItem(root_id, src_item.id, dst_parent.id, dst_name, tok, err) ? err : ""); + + // Invalidate affected cache entries + cache_.InvalidateItem(b.Scheme(), src_url); + cache_.InvalidatePrefix(b.Scheme(), b.Scheme() + "://" + src_raw + dst_parent_path); } bool CloudFileSystem::ListFiles(const string& dir, diff --git a/src/include/core/cloud_backend.hpp b/src/include/core/cloud_backend.hpp index 8fc43bc..63641c7 100644 --- a/src/include/core/cloud_backend.hpp +++ b/src/include/core/cloud_backend.hpp @@ -146,6 +146,14 @@ class ICloudBackend { return false; } + // Optional server-side move/rename (avoids delete+re-upload for large files). + virtual bool MoveItem(const std::string& root, const std::string& src_item_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& access_token, std::string& err) { + err = Name() + ": server-side move not supported"; + return false; + } + // ── Root resolution ─────────────────────────────────────────────────────── // Resolve the root string (parsed by ParseUrl) to an internal drive/bucket ID. diff --git a/src/include/providers/dropbox_backend.hpp b/src/include/providers/dropbox_backend.hpp index 621d1c6..f8118b9 100644 --- a/src/include/providers/dropbox_backend.hpp +++ b/src/include/providers/dropbox_backend.hpp @@ -94,6 +94,10 @@ class DropboxBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + private: CloudItem ParseMetadata(const std::string& json) const; diff --git a/src/include/providers/gdrive_backend.hpp b/src/include/providers/gdrive_backend.hpp index b5c0595..f2a5c7f 100644 --- a/src/include/providers/gdrive_backend.hpp +++ b/src/include/providers/gdrive_backend.hpp @@ -88,6 +88,10 @@ class GDriveBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + private: // Path resolution: Google Drive has no native path API. // We walk the tree: root → child named X → child named Y → … diff --git a/src/include/providers/onedrive_backend.hpp b/src/include/providers/onedrive_backend.hpp index a771bc2..480b909 100644 --- a/src/include/providers/onedrive_backend.hpp +++ b/src/include/providers/onedrive_backend.hpp @@ -74,6 +74,14 @@ class OneDriveBackend : public ICloudBackend { const std::string& name, const std::string& token, CloudItem& out, std::string& err) override; + bool CopyItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + private: // Builds the Graph API base URL for a given root // root="me" → https://graph.microsoft.com/v1.0/me/drive diff --git a/src/include/providers/sftp_backend.hpp b/src/include/providers/sftp_backend.hpp index c3f6086..28a8708 100644 --- a/src/include/providers/sftp_backend.hpp +++ b/src/include/providers/sftp_backend.hpp @@ -100,6 +100,10 @@ class SFTPBackend : public ICloudBackend { const std::string& name, const std::string& token, CloudItem& out, std::string& err) override; + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + private: // Get or create an SSH+SFTP connection for a given root ("user@host:port") SSHConnection& GetConnection(const std::string& root, diff --git a/src/include/providers/sharepoint_backend.hpp b/src/include/providers/sharepoint_backend.hpp index 8da4a04..aeab3e9 100644 --- a/src/include/providers/sharepoint_backend.hpp +++ b/src/include/providers/sharepoint_backend.hpp @@ -74,6 +74,10 @@ class SharePointBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + private: // root = "https://tenant.sharepoint.com|/sites/SiteName" → drive_id struct SiteInfo { diff --git a/src/include/providers/vfs_backend.hpp b/src/include/providers/vfs_backend.hpp index 8316a17..2d8589d 100644 --- a/src/include/providers/vfs_backend.hpp +++ b/src/include/providers/vfs_backend.hpp @@ -95,6 +95,10 @@ class VFSBackend : public ICloudBackend { const std::string& name, const std::string& token, CloudItem& out, std::string& err) override; + bool MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) override; + // Check agent health — returns version string or error bool Ping(const std::string& root, const std::string& token, std::string& out_version, std::string& err); diff --git a/src/providers/dropbox/dropbox_backend.cpp b/src/providers/dropbox/dropbox_backend.cpp index ef0b394..a86acf0 100644 --- a/src/providers/dropbox/dropbox_backend.cpp +++ b/src/providers/dropbox/dropbox_backend.cpp @@ -278,4 +278,22 @@ bool DropboxBackend::CopyItem(const std::string&, const std::string& src_id, return true; } +bool DropboxBackend::MoveItem(const std::string&, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& tok, std::string& err) { + HttpRequest req; + req.method = "POST"; + req.SetBearerAuth(tok); + req.headers["Content-Type"] = "application/json"; + req.url = std::string(kApiBase) + "/files/move_v2"; + req.body = + "{\"from_path\":\"" + src_id + "\",\"to_path\":\"" + dst_parent_id + "/" + dst_name + "\"}"; + auto resp = http_.Execute(req); + if (!resp.ok()) { + err = "move failed " + std::to_string(resp.status); + return false; + } + return true; +} + } // namespace duckdb diff --git a/src/providers/gdrive/gdrive_backend.cpp b/src/providers/gdrive/gdrive_backend.cpp index 89f053b..257e557 100644 --- a/src/providers/gdrive/gdrive_backend.cpp +++ b/src/providers/gdrive/gdrive_backend.cpp @@ -220,4 +220,40 @@ bool GDriveBackend::CopyItem(const std::string&, const std::string& src_id, return true; } +bool GDriveBackend::MoveItem(const std::string&, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& tok, std::string& err) { + // Fetch current parents so we can pass removeParents (avoids duplicates in multi-parent drives) + std::string meta_url = + kApiBase + "/files/" + src_id + "?fields=parents&supportsAllDrives=true"; + auto meta_resp = http_.Get(meta_url, tok); + if (!meta_resp.ok()) { + err = "move: failed to fetch file metadata (" + std::to_string(meta_resp.status) + ")"; + return false; + } + // Extract first parent ID from JSON array ["id1","id2",...] + std::string old_parent; + auto parents_pos = meta_resp.body.find("\"parents\""); + if (parents_pos != std::string::npos) { + auto arr_start = meta_resp.body.find('[', parents_pos); + auto first_q = meta_resp.body.find('"', arr_start + 1); + auto second_q = meta_resp.body.find('"', first_q + 1); + if (first_q != std::string::npos && second_q != std::string::npos) + old_parent = meta_resp.body.substr(first_q + 1, second_q - first_q - 1); + } + + std::string url = kApiBase + "/files/" + src_id + "?supportsAllDrives=true"; + if (!old_parent.empty()) + url += "&removeParents=" + old_parent; + url += "&addParents=" + dst_parent_id; + + std::string body = "{\"name\":\"" + dst_name + "\"}"; + auto resp = http_.Patch(url, tok, body); + if (!resp.ok()) { + err = "move failed (" + std::to_string(resp.status) + ")"; + return false; + } + return true; +} + } // namespace duckdb diff --git a/src/providers/onedrive/onedrive_backend.cpp b/src/providers/onedrive/onedrive_backend.cpp index 9b41f34..cc57863 100644 --- a/src/providers/onedrive/onedrive_backend.cpp +++ b/src/providers/onedrive/onedrive_backend.cpp @@ -163,4 +163,34 @@ bool OneDriveBackend::CreateFolder(const std::string& root, const std::string& p return true; } +bool OneDriveBackend::CopyItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& tok, std::string& err) { + std::string url = + "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id + "/copy"; + std::string body = + "{\"parentReference\":{\"id\":\"" + dst_parent_id + "\"},\"name\":\"" + dst_name + "\"}"; + auto resp = http_.Post(url, tok, body); + if (resp.status != 202) { + err = "copy failed (" + std::to_string(resp.status) + ")"; + return false; + } + return true; +} + +bool OneDriveBackend::MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& tok, std::string& err) { + std::string url = + "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id; + std::string body = + "{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}"; + auto resp = http_.Patch(url, tok, body); + if (!resp.ok()) { + err = "move failed (" + std::to_string(resp.status) + ")"; + return false; + } + return true; +} + } // namespace duckdb diff --git a/src/providers/sftp/sftp_backend.cpp b/src/providers/sftp/sftp_backend.cpp index b819040..713c49b 100644 --- a/src/providers/sftp/sftp_backend.cpp +++ b/src/providers/sftp/sftp_backend.cpp @@ -396,4 +396,23 @@ bool SFTPBackend::CreateFolder(const std::string& root, const std::string& paren return true; } +bool SFTPBackend::MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) { + auto& conn = GetConnection(root, token, err); + if (!conn.IsValid()) + return false; + std::string dst_path = dst_parent_id + "/" + dst_name; + // LIBSSH2_SFTP_RENAME_OVERWRITE allows replacing an existing destination + int rc = libssh2_sftp_rename_ex(conn.sftp, src_id.c_str(), (unsigned int)src_id.size(), + dst_path.c_str(), (unsigned int)dst_path.size(), + LIBSSH2_SFTP_RENAME_OVERWRITE | LIBSSH2_SFTP_RENAME_ATOMIC | + LIBSSH2_SFTP_RENAME_NATIVE); + if (rc != 0) { + err = "sftp rename failed: " + src_id + " -> " + dst_path; + return false; + } + return true; +} + } // namespace duckdb diff --git a/src/providers/sharepoint/sharepoint_backend.cpp b/src/providers/sharepoint/sharepoint_backend.cpp index 7e6cbd3..eaf3fe4 100644 --- a/src/providers/sharepoint/sharepoint_backend.cpp +++ b/src/providers/sharepoint/sharepoint_backend.cpp @@ -229,4 +229,19 @@ bool SharePointBackend::CopyItem(const std::string& root, const std::string& src return true; } +bool SharePointBackend::MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& tok, std::string& err) { + std::string url = + "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id; + std::string body = + "{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}"; + auto resp = http_.Patch(url, tok, body); + if (!resp.ok()) { + err = "move failed (" + std::to_string(resp.status) + ")"; + return false; + } + return true; +} + } // namespace duckdb diff --git a/src/providers/vfs/vfs_backend.cpp b/src/providers/vfs/vfs_backend.cpp index 38a002f..0934d7c 100644 --- a/src/providers/vfs/vfs_backend.cpp +++ b/src/providers/vfs/vfs_backend.cpp @@ -146,6 +146,19 @@ bool VFSBackend::CreateFolder(const std::string& root, const std::string& parent return true; } +bool VFSBackend::MoveItem(const std::string& root, const std::string& src_id, + const std::string& dst_parent_id, const std::string& dst_name, + const std::string& token, std::string& err) { + std::string dst_path = dst_parent_id + "/" + dst_name; + std::string body = JsonUtil::MakeObject({{"from", src_id}, {"to", dst_path}}); + auto resp = http_.Post(root + "/v1/move", token, body); + if (!resp.ok()) { + err = "move failed (" + std::to_string(resp.status) + ")"; + return false; + } + return true; +} + bool VFSBackend::Ping(const std::string& root, const std::string& token, std::string& out_version, std::string& err) { auto resp = http_.Get(root + "/v1/ping", token); From d0baea47818b75402db2f0eafb19f5a5b33b70be Mon Sep 17 00:00:00 2001 From: brunolnetto Date: Tue, 7 Jul 2026 21:44:56 -0300 Subject: [PATCH 2/6] =?UTF-8?q?fix(ops):=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20MoveItem=20interface,=20JSON=20injection,=20Dropbox?= =?UTF-8?q?=20path,=20GDrive=20parents?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves all 6 review comments on PR #6: 1. dst_name empty validation (MoveFile): - Throw clear error when destination path ends with '/' (no filename) 2. MoveItem interface: CloudItem objects instead of bare string IDs - Dropbox move_v2 needs .path (not opaque id) for to_path - Graph/GDrive backends continue using .id (unchanged semantics) - Signature: MoveItem(root, const CloudItem& src, const CloudItem& dst_parent, ...) 3. JSON injection — add JsonUtil::EscapeJsonString() for dst_name in: - SharePoint, OneDrive, Google Drive, Dropbox MoveItem bodies 4. Dropbox MoveItem: from_path uses src_item.path, to_path uses dst_parent.path + '/' + dst_name (not opaque Dropbox IDs) 5. GDrive parent parsing: safe extraction of ALL parents from JSON array - Eliminate arr_start+1 overflow (was size_t wrap when '[' not found) - Remove ALL parents (not just first) to prevent multi-parent artifacts 6. Format: clang-format -style=LLVM applied to all changed files --- src/core/cloud_filesystem.cpp | 5 +- src/include/core/cloud_backend.hpp | 4 +- src/include/providers/dropbox_backend.hpp | 5 +- src/include/providers/gdrive_backend.hpp | 5 +- src/include/providers/onedrive_backend.hpp | 5 +- src/include/providers/sftp_backend.hpp | 5 +- src/include/providers/sharepoint_backend.hpp | 5 +- src/include/providers/vfs_backend.hpp | 5 +- src/providers/dropbox/dropbox_backend.cpp | 10 ++-- src/providers/gdrive/gdrive_backend.cpp | 48 ++++++++++++------- src/providers/onedrive/onedrive_backend.cpp | 11 ++--- src/providers/sftp/sftp_backend.cpp | 16 +++---- .../sharepoint/sharepoint_backend.cpp | 11 ++--- src/providers/vfs/vfs_backend.cpp | 8 ++-- 14 files changed, 76 insertions(+), 67 deletions(-) diff --git a/src/core/cloud_filesystem.cpp b/src/core/cloud_filesystem.cpp index d4e2fbe..578299b 100644 --- a/src/core/cloud_filesystem.cpp +++ b/src/core/cloud_filesystem.cpp @@ -477,6 +477,9 @@ void CloudFileSystem::MoveFile(const string& src_url, const string& dst_url, std::string dst_parent_path = (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); + if (dst_name.empty()) + Throw("cloudfs: destination path must include a filename, not a directory path"); + // Stat the destination parent folder CloudItem dst_parent; Throw(!b.Stat(root_id, dst_parent_path, tok, dst_parent, err) @@ -484,7 +487,7 @@ void CloudFileSystem::MoveFile(const string& src_url, const string& dst_url, : ""); // Perform server-side move - Throw(!b.MoveItem(root_id, src_item.id, dst_parent.id, dst_name, tok, err) ? err : ""); + Throw(!b.MoveItem(root_id, src_item, dst_parent, dst_name, tok, err) ? err : ""); // Invalidate affected cache entries cache_.InvalidateItem(b.Scheme(), src_url); diff --git a/src/include/core/cloud_backend.hpp b/src/include/core/cloud_backend.hpp index 63641c7..16ba5bf 100644 --- a/src/include/core/cloud_backend.hpp +++ b/src/include/core/cloud_backend.hpp @@ -147,8 +147,8 @@ class ICloudBackend { } // Optional server-side move/rename (avoids delete+re-upload for large files). - virtual bool MoveItem(const std::string& root, const std::string& src_item_id, - const std::string& dst_parent_id, const std::string& dst_name, + virtual bool MoveItem(const std::string& root, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& access_token, std::string& err) { err = Name() + ": server-side move not supported"; return false; diff --git a/src/include/providers/dropbox_backend.hpp b/src/include/providers/dropbox_backend.hpp index f8118b9..2d5dceb 100644 --- a/src/include/providers/dropbox_backend.hpp +++ b/src/include/providers/dropbox_backend.hpp @@ -94,9 +94,8 @@ class DropboxBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; private: CloudItem ParseMetadata(const std::string& json) const; diff --git a/src/include/providers/gdrive_backend.hpp b/src/include/providers/gdrive_backend.hpp index f2a5c7f..75d209d 100644 --- a/src/include/providers/gdrive_backend.hpp +++ b/src/include/providers/gdrive_backend.hpp @@ -88,9 +88,8 @@ class GDriveBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; private: // Path resolution: Google Drive has no native path API. diff --git a/src/include/providers/onedrive_backend.hpp b/src/include/providers/onedrive_backend.hpp index 480b909..1ab59bb 100644 --- a/src/include/providers/onedrive_backend.hpp +++ b/src/include/providers/onedrive_backend.hpp @@ -78,9 +78,8 @@ class OneDriveBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; private: // Builds the Graph API base URL for a given root diff --git a/src/include/providers/sftp_backend.hpp b/src/include/providers/sftp_backend.hpp index 28a8708..a0617b1 100644 --- a/src/include/providers/sftp_backend.hpp +++ b/src/include/providers/sftp_backend.hpp @@ -100,9 +100,8 @@ class SFTPBackend : public ICloudBackend { const std::string& name, const std::string& token, CloudItem& out, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; private: // Get or create an SSH+SFTP connection for a given root ("user@host:port") diff --git a/src/include/providers/sharepoint_backend.hpp b/src/include/providers/sharepoint_backend.hpp index aeab3e9..60b8b4b 100644 --- a/src/include/providers/sharepoint_backend.hpp +++ b/src/include/providers/sharepoint_backend.hpp @@ -74,9 +74,8 @@ class SharePointBackend : public ICloudBackend { const std::string& dst_parent_id, const std::string& dst_name, const std::string& token, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; private: // root = "https://tenant.sharepoint.com|/sites/SiteName" → drive_id diff --git a/src/include/providers/vfs_backend.hpp b/src/include/providers/vfs_backend.hpp index 2d8589d..0cdf421 100644 --- a/src/include/providers/vfs_backend.hpp +++ b/src/include/providers/vfs_backend.hpp @@ -95,9 +95,8 @@ class VFSBackend : public ICloudBackend { const std::string& name, const std::string& token, CloudItem& out, std::string& err) override; - bool MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, - const std::string& token, std::string& err) override; + bool MoveItem(const std::string& root, const CloudItem& src_item, const CloudItem& dst_parent, + const std::string& dst_name, const std::string& token, std::string& err) override; // Check agent health — returns version string or error bool Ping(const std::string& root, const std::string& token, std::string& out_version, diff --git a/src/providers/dropbox/dropbox_backend.cpp b/src/providers/dropbox/dropbox_backend.cpp index a86acf0..4e8dee0 100644 --- a/src/providers/dropbox/dropbox_backend.cpp +++ b/src/providers/dropbox/dropbox_backend.cpp @@ -278,16 +278,18 @@ bool DropboxBackend::CopyItem(const std::string&, const std::string& src_id, return true; } -bool DropboxBackend::MoveItem(const std::string&, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool DropboxBackend::MoveItem(const std::string&, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& tok, std::string& err) { + // Dropbox move_v2: from_path accepts opaque Dropbox ID ("id:...") but to_path must be a path. HttpRequest req; req.method = "POST"; req.SetBearerAuth(tok); req.headers["Content-Type"] = "application/json"; req.url = std::string(kApiBase) + "/files/move_v2"; - req.body = - "{\"from_path\":\"" + src_id + "\",\"to_path\":\"" + dst_parent_id + "/" + dst_name + "\"}"; + req.body = "{\"from_path\":\"" + JsonUtil::EscapeJsonString(src_item.path) + + "\",\"to_path\":\"" + JsonUtil::EscapeJsonString(dst_parent.path + "/" + dst_name) + + "\"}"; auto resp = http_.Execute(req); if (!resp.ok()) { err = "move failed " + std::to_string(resp.status); diff --git a/src/providers/gdrive/gdrive_backend.cpp b/src/providers/gdrive/gdrive_backend.cpp index 257e557..c4e751d 100644 --- a/src/providers/gdrive/gdrive_backend.cpp +++ b/src/providers/gdrive/gdrive_backend.cpp @@ -220,34 +220,46 @@ bool GDriveBackend::CopyItem(const std::string&, const std::string& src_id, return true; } -bool GDriveBackend::MoveItem(const std::string&, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool GDriveBackend::MoveItem(const std::string&, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& tok, std::string& err) { - // Fetch current parents so we can pass removeParents (avoids duplicates in multi-parent drives) + // Fetch current parents so we can remove ALL of them (avoids multi-parent artifacts). std::string meta_url = - kApiBase + "/files/" + src_id + "?fields=parents&supportsAllDrives=true"; + kApiBase + "/files/" + src_item.id + "?fields=parents&supportsAllDrives=true"; auto meta_resp = http_.Get(meta_url, tok); if (!meta_resp.ok()) { err = "move: failed to fetch file metadata (" + std::to_string(meta_resp.status) + ")"; return false; } - // Extract first parent ID from JSON array ["id1","id2",...] - std::string old_parent; - auto parents_pos = meta_resp.body.find("\"parents\""); - if (parents_pos != std::string::npos) { - auto arr_start = meta_resp.body.find('[', parents_pos); - auto first_q = meta_resp.body.find('"', arr_start + 1); - auto second_q = meta_resp.body.find('"', first_q + 1); - if (first_q != std::string::npos && second_q != std::string::npos) - old_parent = meta_resp.body.substr(first_q + 1, second_q - first_q - 1); + // Safely extract all parent IDs from JSON array ["id1","id2",...] + std::string remove_parents; + { + const auto& body = meta_resp.body; + auto p_pos = body.find("\"parents\""); + auto arr_start = (p_pos != std::string::npos) ? body.find('[', p_pos) : std::string::npos; + auto arr_end = + (arr_start != std::string::npos) ? body.find(']', arr_start) : std::string::npos; + for (size_t i = (arr_start != std::string::npos ? arr_start + 1 : body.size()); + arr_end != std::string::npos && i < arr_end;) { + auto q1 = body.find('"', i); + if (q1 == std::string::npos || q1 >= arr_end) + break; + auto q2 = body.find('"', q1 + 1); + if (q2 == std::string::npos || q2 >= arr_end) + break; + if (!remove_parents.empty()) + remove_parents += ","; + remove_parents += body.substr(q1 + 1, q2 - q1 - 1); + i = q2 + 1; + } } - std::string url = kApiBase + "/files/" + src_id + "?supportsAllDrives=true"; - if (!old_parent.empty()) - url += "&removeParents=" + old_parent; - url += "&addParents=" + dst_parent_id; + std::string url = kApiBase + "/files/" + src_item.id + "?supportsAllDrives=true"; + if (!remove_parents.empty()) + url += "&removeParents=" + remove_parents; // GDrive IDs are safe alphanumeric + url += "&addParents=" + dst_parent.id; - std::string body = "{\"name\":\"" + dst_name + "\"}"; + std::string body = "{\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + "\"}"; auto resp = http_.Patch(url, tok, body); if (!resp.ok()) { err = "move failed (" + std::to_string(resp.status) + ")"; diff --git a/src/providers/onedrive/onedrive_backend.cpp b/src/providers/onedrive/onedrive_backend.cpp index cc57863..da7cfa6 100644 --- a/src/providers/onedrive/onedrive_backend.cpp +++ b/src/providers/onedrive/onedrive_backend.cpp @@ -178,13 +178,12 @@ bool OneDriveBackend::CopyItem(const std::string& root, const std::string& src_i return true; } -bool OneDriveBackend::MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool OneDriveBackend::MoveItem(const std::string& root, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& tok, std::string& err) { - std::string url = - "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id; - std::string body = - "{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}"; + std::string url = "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_item.id; + std::string body = "{\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + + "\",\"parentReference\":{\"id\":\"" + dst_parent.id + "\"}}"; auto resp = http_.Patch(url, tok, body); if (!resp.ok()) { err = "move failed (" + std::to_string(resp.status) + ")"; diff --git a/src/providers/sftp/sftp_backend.cpp b/src/providers/sftp/sftp_backend.cpp index 713c49b..0a7c0d1 100644 --- a/src/providers/sftp/sftp_backend.cpp +++ b/src/providers/sftp/sftp_backend.cpp @@ -396,20 +396,20 @@ bool SFTPBackend::CreateFolder(const std::string& root, const std::string& paren return true; } -bool SFTPBackend::MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool SFTPBackend::MoveItem(const std::string& root, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& token, std::string& err) { auto& conn = GetConnection(root, token, err); if (!conn.IsValid()) return false; - std::string dst_path = dst_parent_id + "/" + dst_name; + std::string dst_path = dst_parent.id + "/" + dst_name; // LIBSSH2_SFTP_RENAME_OVERWRITE allows replacing an existing destination - int rc = libssh2_sftp_rename_ex(conn.sftp, src_id.c_str(), (unsigned int)src_id.size(), - dst_path.c_str(), (unsigned int)dst_path.size(), - LIBSSH2_SFTP_RENAME_OVERWRITE | LIBSSH2_SFTP_RENAME_ATOMIC | - LIBSSH2_SFTP_RENAME_NATIVE); + int rc = libssh2_sftp_rename_ex( + conn.sftp, src_item.id.c_str(), (unsigned int)src_item.id.size(), dst_path.c_str(), + (unsigned int)dst_path.size(), + LIBSSH2_SFTP_RENAME_OVERWRITE | LIBSSH2_SFTP_RENAME_ATOMIC | LIBSSH2_SFTP_RENAME_NATIVE); if (rc != 0) { - err = "sftp rename failed: " + src_id + " -> " + dst_path; + err = "sftp rename failed: " + src_item.id + " -> " + dst_path; return false; } return true; diff --git a/src/providers/sharepoint/sharepoint_backend.cpp b/src/providers/sharepoint/sharepoint_backend.cpp index eaf3fe4..50e081b 100644 --- a/src/providers/sharepoint/sharepoint_backend.cpp +++ b/src/providers/sharepoint/sharepoint_backend.cpp @@ -229,13 +229,12 @@ bool SharePointBackend::CopyItem(const std::string& root, const std::string& src return true; } -bool SharePointBackend::MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool SharePointBackend::MoveItem(const std::string& root, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& tok, std::string& err) { - std::string url = - "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id; - std::string body = - "{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}"; + std::string url = "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_item.id; + std::string body = "{\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + + "\",\"parentReference\":{\"id\":\"" + dst_parent.id + "\"}}"; auto resp = http_.Patch(url, tok, body); if (!resp.ok()) { err = "move failed (" + std::to_string(resp.status) + ")"; diff --git a/src/providers/vfs/vfs_backend.cpp b/src/providers/vfs/vfs_backend.cpp index 0934d7c..87234af 100644 --- a/src/providers/vfs/vfs_backend.cpp +++ b/src/providers/vfs/vfs_backend.cpp @@ -146,11 +146,11 @@ bool VFSBackend::CreateFolder(const std::string& root, const std::string& parent return true; } -bool VFSBackend::MoveItem(const std::string& root, const std::string& src_id, - const std::string& dst_parent_id, const std::string& dst_name, +bool VFSBackend::MoveItem(const std::string& root, const CloudItem& src_item, + const CloudItem& dst_parent, const std::string& dst_name, const std::string& token, std::string& err) { - std::string dst_path = dst_parent_id + "/" + dst_name; - std::string body = JsonUtil::MakeObject({{"from", src_id}, {"to", dst_path}}); + std::string dst_path = dst_parent.id + "/" + dst_name; + std::string body = JsonUtil::MakeObject({{"from", src_item.id}, {"to", dst_path}}); auto resp = http_.Post(root + "/v1/move", token, body); if (!resp.ok()) { err = "move failed (" + std::to_string(resp.status) + ")"; From cef62345e6396ac9410fcb5cfc0e3633a36749b6 Mon Sep 17 00:00:00 2001 From: Bruno Peixoto Date: Tue, 7 Jul 2026 22:25:19 -0300 Subject: [PATCH 3/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/providers/onedrive/onedrive_backend.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/providers/onedrive/onedrive_backend.cpp b/src/providers/onedrive/onedrive_backend.cpp index da7cfa6..9a0a736 100644 --- a/src/providers/onedrive/onedrive_backend.cpp +++ b/src/providers/onedrive/onedrive_backend.cpp @@ -168,8 +168,9 @@ bool OneDriveBackend::CopyItem(const std::string& root, const std::string& src_i const std::string& tok, std::string& err) { std::string url = "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id + "/copy"; - std::string body = - "{\"parentReference\":{\"id\":\"" + dst_parent_id + "\"},\"name\":\"" + dst_name + "\"}"; +std::string body = + "{\"parentReference\":{\"id\":\"" + JsonUtil::EscapeJsonString(dst_parent_id) + + "\"},\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + "\"}"; auto resp = http_.Post(url, tok, body); if (resp.status != 202) { err = "copy failed (" + std::to_string(resp.status) + ")"; From e2ea7221e82374e9eaefe65f1ee58425b535bc05 Mon Sep 17 00:00:00 2001 From: Bruno Peixoto Date: Tue, 7 Jul 2026 22:25:37 -0300 Subject: [PATCH 4/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/core/cloud_filesystem.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/cloud_filesystem.cpp b/src/core/cloud_filesystem.cpp index 578299b..23e9bc3 100644 --- a/src/core/cloud_filesystem.cpp +++ b/src/core/cloud_filesystem.cpp @@ -471,11 +471,12 @@ void CloudFileSystem::MoveFile(const string& src_url, const string& dst_url, Throw(!b.Stat(root_id, src_path, tok, src_item, err) ? "source not found: " + err : ""); // Decompose destination into parent path + filename - auto last_slash = dst_path.rfind('/'); - std::string dst_name = - (last_slash == std::string::npos) ? dst_path : dst_path.substr(last_slash + 1); - std::string dst_parent_path = - (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); +auto last_slash = dst_path.rfind('/'); +std::string dst_name = + (last_slash == std::string::npos) ? dst_path : dst_path.substr(last_slash + 1); +Throw(dst_name.empty() ? "cloudfs: destination must include a filename" : ""); +std::string dst_parent_path = + (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); if (dst_name.empty()) Throw("cloudfs: destination path must include a filename, not a directory path"); From b994b28fb9da947ab0f33b0fbbf648a290e43995 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Jul 2026 01:35:54 +0000 Subject: [PATCH 5/6] fix: address merge resolution review follow-ups --- src/include/providers/sftp_backend.hpp | 1 - src/providers/gdrive/gdrive_backend.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/include/providers/sftp_backend.hpp b/src/include/providers/sftp_backend.hpp index 2cadcac..aa606eb 100644 --- a/src/include/providers/sftp_backend.hpp +++ b/src/include/providers/sftp_backend.hpp @@ -115,7 +115,6 @@ class SFTPBackend : public ICloudBackend { std::string& err) override; bool AbortUpload(const CloudUploadSession& session, const std::string& token, std::string& err) override; - std::string& err) override; private: // Get or create an SSH+SFTP connection for a given root ("user@host:port") diff --git a/src/providers/gdrive/gdrive_backend.cpp b/src/providers/gdrive/gdrive_backend.cpp index c6ea3c2..c39223b 100644 --- a/src/providers/gdrive/gdrive_backend.cpp +++ b/src/providers/gdrive/gdrive_backend.cpp @@ -299,7 +299,7 @@ bool GDriveBackend::ListFolderRecursive(const std::string&, const std::string& f auto resp = http_.Get(url, tok); if (!resp.ok()) { - err = "list recursive failed " + std::to_string(resp.status); + err = "list recursive failed (" + std::to_string(resp.status) + ")"; return false; } for (auto& j : JsonUtil::GetArray(resp.body, "files")) { From 2b6b1264ba8cf171947b86cdaeb1c06aa4d533ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:34:25 +0000 Subject: [PATCH 6/6] fix(format): address clang-format style violations --- src/core/cloud_filesystem.cpp | 12 ++++++------ src/include/providers/gdrive_backend.hpp | 2 +- src/providers/onedrive/onedrive_backend.cpp | 6 +++--- test/test_suite.py | 0 4 files changed, 10 insertions(+), 10 deletions(-) mode change 100755 => 100644 test/test_suite.py diff --git a/src/core/cloud_filesystem.cpp b/src/core/cloud_filesystem.cpp index 4362650..31b68ca 100644 --- a/src/core/cloud_filesystem.cpp +++ b/src/core/cloud_filesystem.cpp @@ -474,12 +474,12 @@ void CloudFileSystem::MoveFile(const string& src_url, const string& dst_url, Throw(!b.Stat(root_id, src_path, tok, src_item, err) ? "source not found: " + err : ""); // Decompose destination into parent path + filename -auto last_slash = dst_path.rfind('/'); -std::string dst_name = - (last_slash == std::string::npos) ? dst_path : dst_path.substr(last_slash + 1); -Throw(dst_name.empty() ? "cloudfs: destination must include a filename" : ""); -std::string dst_parent_path = - (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); + auto last_slash = dst_path.rfind('/'); + std::string dst_name = + (last_slash == std::string::npos) ? dst_path : dst_path.substr(last_slash + 1); + Throw(dst_name.empty() ? "cloudfs: destination must include a filename" : ""); + std::string dst_parent_path = + (last_slash == std::string::npos || last_slash == 0) ? "/" : dst_path.substr(0, last_slash); if (dst_name.empty()) Throw("cloudfs: destination path must include a filename, not a directory path"); diff --git a/src/include/providers/gdrive_backend.hpp b/src/include/providers/gdrive_backend.hpp index c75ea6d..5ccc837 100644 --- a/src/include/providers/gdrive_backend.hpp +++ b/src/include/providers/gdrive_backend.hpp @@ -99,7 +99,7 @@ class GDriveBackend : public ICloudBackend { const std::string& dst_name, const std::string& token, std::string& err) override; bool AbortUpload(const CloudUploadSession& session, const std::string& token, - std::string& err) override; + std::string& err) override; private: // Path resolution: Google Drive has no native path API. diff --git a/src/providers/onedrive/onedrive_backend.cpp b/src/providers/onedrive/onedrive_backend.cpp index 9a4d913..76b56fd 100644 --- a/src/providers/onedrive/onedrive_backend.cpp +++ b/src/providers/onedrive/onedrive_backend.cpp @@ -168,9 +168,9 @@ bool OneDriveBackend::CopyItem(const std::string& root, const std::string& src_i const std::string& tok, std::string& err) { std::string url = "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id + "/copy"; - std::string body = - "{\"parentReference\":{\"id\":\"" + JsonUtil::EscapeJsonString(dst_parent_id) + - "\"},\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + "\"}"; + std::string body = "{\"parentReference\":{\"id\":\"" + + JsonUtil::EscapeJsonString(dst_parent_id) + "\"},\"name\":\"" + + JsonUtil::EscapeJsonString(dst_name) + "\"}"; auto resp = http_.Post(url, tok, body); if (resp.status != 202) { err = "copy failed (" + std::to_string(resp.status) + ")"; diff --git a/test/test_suite.py b/test/test_suite.py old mode 100755 new mode 100644