diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9516f76..dd269e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,9 +6,9 @@ name: ci on: push: - branches: [main] + branches: [main, dev] pull_request: - branches: [main] + branches: [main, dev] workflow_dispatch: jobs: @@ -52,6 +52,31 @@ jobs: working-directory: src-tauri run: cargo check --workspace + - name: Rust library and CLI tests + working-directory: src-tauri + run: | + cargo test --locked -p faro --lib + cargo test --locked -p faro-cli + + - name: Setup Python for S3 regression audit + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: S3 CLI regression audit (local emulator) + run: | + python -m pip install 'moto[server]==5.1.12' + cargo build --locked --manifest-path src-tauri/Cargo.toml -p faro-cli + python scripts/audit-s3-cli.py + + - name: Upload S3 audit results + if: always() + uses: actions/upload-artifact@v4 + with: + name: s3-cli-audit + path: src-tauri/target/s3-cli-audit.json + if-no-files-found: ignore + - name: cargo clippy (warnings allowed for now) working-directory: src-tauri run: cargo clippy --workspace -- -D warnings -A clippy::all diff --git a/docs/plans/26_backend-correctness-and-coverage.md b/docs/plans/26_backend-correctness-and-coverage.md new file mode 100644 index 0000000..a3263af --- /dev/null +++ b/docs/plans/26_backend-correctness-and-coverage.md @@ -0,0 +1,186 @@ +# Plan 26 — Backend correctness and coverage + +Status: planned. S3 fixes and regression coverage are tracked in PR #34; the +other work below is not implemented by that PR. This plan follows the connection +types exposed in New connection, plus WordPress and local disk. A connection +appearing in the picker is not evidence that every app or CLI operation works. + +## Outcome + +For each backend, state which operations are supported, verify them through the +actual app transfer runner and CLI, and return an explicit unsupported error for +the rest. Prioritize preventing wrong-target writes and deletion before adding +features. Keep real-provider checks distinct from emulator and mock results. + +## Evidence at the start + +These are code findings unless a reproduction is explicitly listed: + +| Backend | Current evidence | Immediate implication | +| --- | --- | --- | +| S3 | 55 CLI emulator checks, three live emulator transfer tests; see `docs/s3-testing.md` | Keep this suite running in CI. Wasabi and other providers still need separate smoke tests. | +| Google Drive | `find_child` selects the first matching name; `delete` ignores the recursive flag and uses permanent deletion | Reproduce duplicate-name selection and nonrecursive folder deletion first. | +| Google Drive | App uploads buffer the full file; CLI upload and download explicitly reject Drive | Large-file handling and CLI parity are separate missing features. | +| Google Drive | Basic mock roundtrip covers nested paths, token exchange/refresh and CRUD, but uploads directly through HTTP | It does not prove the real transfer runner, CLI, or browser authorization works. | +| Azure Blob | Non-S3 object backend treats mkdir as a successful no-op and renames one object rather than a prefix tree | Empty-folder and directory-rename semantics need an explicit contract and tests. | +| Azure Blob | An ignored large-object/resume test exists | Run it with Azurite, then add namespace and CLI checks. S3 results do not establish Azure correctness. | +| SFTP | Recursive delete uses following `metadata` on the starting path | Reproduce deleting a symlink to an external directory and ensure target contents survive. | +| FTP | Recursive delete replaces listing errors with an empty list | Reproduce denied child listings and prevent partial deletion after incomplete traversal. | +| FTP/SFTP | Some overwrite probes use `is_ok()` as existence | Test denial and disconnect separately from missing files. | +| Other backends | Several mock roundtrips exist, but no complete operation matrix has been run in this audit | Inventory before claiming support or treating untested behavior as a bug. | + +Relevant sources: `src-tauri/src/session/gdrive.rs`, +`src-tauri/src/remotefs/{gdrive,object,sftp,ftp}.rs`, +`src-tauri/src/transfer/{mod,live_tests}.rs`, and +`src-tauri/faro-cli/src/main.rs`. + +## Delivery order + +### 1. Google Drive safety and test harness + +Start with `src-tauri/tests/gdrive_mock.py` and extend it with deterministic +fixtures and request logs. Keep test profiles, tokens, and data isolated from +the user's normal profile store. Do not exercise destructive cases on real data. + +1. Duplicate sibling files and folders: never silently choose an arbitrary ID + for download, overwrite, rename, delete, or sync. Initially fail ambiguous + path resolution with an actionable error; design ID-based selection for the + UI as a separate enhancement. Test both cold and warmed resolver caches. +2. Deletion: a nonrecursive delete of a nonempty folder must fail without any + mutation. Define recoverable trash versus explicit permanent delete, and + expose that distinction to the caller before changing the default. Verify + recursive traversal and failure behavior for whichever contract is chosen. +3. Errors: revoked access, permission denial, malformed metadata, failed child + listing, and token-refresh failure must not become empty folders or missing + files. A failed source scan must never create a mirror deletion plan. +4. Pagination: multiple pages, empty intermediate pages, malformed/repeated + continuation tokens, and an error on a later page. Return no partial-success + result on an incomplete listing. +5. Cache invalidation: external rename, move, deletion, and same-name replacement + after the cache is populated must not redirect an operation to a stale ID. + +Acceptance: each reproduced safety defect has a failing regression before its +fix, then passes through `GDriveFs` and relevant shared scan/sync paths. Assert +untouched sibling IDs, bytes, and mutation logs as well as returned errors. + +### 2. Azure namespace and transfer audit + +Provide a one-command disposable Azurite lab with a seeded container, test +credentials, automatic teardown, and a JSON result report. Exercise: + +- Implicit prefixes, explicit directory markers, empty folders, zero-byte files, + file/prefix name collisions, spaces, Unicode, percent signs and trailing spaces. +- Paginated listings, missing versus unreadable prefixes, recursive deletion + boundaries, directory rename, and preservation of source data on copy failure. +- Upload/download byte equality, large block uploads, range downloads, cancel, + connection loss, changed-source resume, and cleanup after failure. +- CLI copy destinations, diff/hash errors, additive and mirror sync, and the + actual app transfer runner. Match supported behavior rather than blindly + assuming Azure has S3's API or marker semantics. + +Acceptance: publish the pass/fail/unsupported matrix. Fix reproduced defects; +operations without a reliable implementation must fail explicitly rather than +return success. Keep actual Azure authentication, permissions and account +configuration as separate real-service checks outside the emulator claim. + +### 3. Google Drive transfer completeness + +After safety is covered: + +- Replace whole-file buffering with a bounded-memory upload path. Implement and + verify the provider's resumable upload flow, including interruption, expired + upload sessions, token expiry, and cancel/retry behavior without duplicate files. +- Wire CLI upload/download into a shared tested implementation. Verify `cp`, + sync, hash/diff and error exit codes; retain explicit rejection until supported. +- Specify Google Docs/Sheets/Slides export names and formats, shortcut handling, + Shared Drive access and permissions, and unusual/duplicate names that cannot + map losslessly to a local path. Do not turn unsupported items into empty files. +- Validate concurrent token refresh, revoked authorization, rate limiting and + transient server errors. Retry mutations only when duplicate side effects + can be prevented or detected. + +Acceptance: real runner and CLI tests verify bytes, bounded buffering and request +sequences. Provider-specific features remain marked unverified until exercised +against a disposable real-account fixture. Browser consent and refresh-token +persistence need an explicit real-account smoke test. + +### 4. FTP, FTPS and SFTP safety + +Create disposable servers and fault fixtures for symlinks, unreadable children, +hidden files, empty directories, Unicode and legacy encodings, overwrite policy, +disconnects, failed saves and transfers. Extend the existing live transfer tests. +Cover FTP MLSD/LIST fallbacks, active/passive mode and connection limits; FTPS +certificate rejection/acceptance; SSH password/key authentication, changed host +keys, reconnect and command exit codes. Verify both app runner and CLI. + +Acceptance: no traversal outside the selected tree, no partial-scan deletion, +and no false success. Failed downloads preserve existing local files, and failed +remote saves have documented, tested recovery behavior. + +### 5. Cover the remaining connection picker + +The following are audit targets, not claims of confirmed defects or support. + +| Backend | Starting fixture | Provider-specific cases | +| --- | --- | --- | +| GCS | Object-store contract tests and a disposable service fixture | Credential failures, exact keys, markers/prefixes, pagination, changed-object resume | +| Dropbox | Existing Dropbox mock | Cursor pagination, moves, upload sessions, name conflicts, revoked access | +| OneDrive | Existing OneDrive mock | Item IDs, next links, name conflicts, shortcuts, upload sessions, token expiry | +| Box | Existing Box mock | Duplicate/conflicting names, versions, pagination, permissions, large uploads | +| WebDAV | Local server plus fault responses | Multistatus errors, encoded paths, MOVE conflicts, locks, auth, partial listings | +| HTTP | Local static/range server | Read-only capability enforcement, redirects, absent/wrong lengths, unsupported ranges, changed source | +| Shopify | Existing Shopify mock | Resource pagination, writable versus read-only resources, rate limits, API errors | +| HubSpot | Existing HubSpot mock | Cursor pagination, object/file distinctions, permissions, partial API failures | +| Dynamics 365 | Existing Dynamics mock | Next links, entity metadata, permissions, partial API failures | +| Faro Agent | Disposable agent process | Pairing/auth, path boundaries, permissions, reconnect, ranged writes, platform differences | +| WordPress | Existing WordPress test fixture | Media and resource capabilities, permissions, pagination, blocked endpoints, failed writes | +| Local disk | Temporary directories on each OS | Case collisions, reserved names, long paths, locked files, full disk, symlinks | + +For each backend, inventory every CLI match arm and every UI capability first. +List/create/rename/delete support does not establish transfer, preview, search, +editor or sync support. Never generalize a test passing on one provider to all +providers that share a Rust trait. + +## Shared operation matrix and test rules + +Track connection/auth, browse/pagination, stat, mkdir, upload, download, +overwrite/skip/rename, move, nonrecursive/recursive delete, preview, editor save, +search, disk usage, hash/diff, additive/mirror sync, cancellation and recovery. +Maintain separate columns for app, CLI, mock/emulator evidence and real service. +Use statuses: verified, failing, unsupported, not tested. Attach a test name or +result artifact to each verified cell. + +Every supported writable backend must pass these shared safety cases: + +1. Missing, denied, canceled, truncated and disconnected operations are distinct; + none produces a success notification or successful CLI exit accidentally. +2. Exact file bytes and sibling contents survive failed transfers and mutations. +3. Mirror cannot delete after an incomplete scan or failed required copy. +4. Empty folders, hidden files and filename collisions have explicit semantics. +5. Retry/cancel does not leave duplicate work or report a half-completed operation + as finished; cleanup limitations are documented and tested. + +Use ephemeral roots and synthetic credentials. Reports must not contain tokens, +real customer paths, or file contents. Test harnesses fail when required fixtures +are missing; an early return from an ignored test is not a passing integration +result. Bound all waits and inject faults deterministically. + +## CI and release evidence + +- PR checks: unit/contract tests, TypeScript/build, and lightweight local + emulator/mock audits; keep assertions tied to bytes and side effects. +- Extend OS coverage for local paths, credential storage and atomic replacement. +- Put expensive large-transfer and reconnect tests in a separately runnable + workflow with bounded timeouts and saved reports. Do not silently omit them + from a release validation claim. +- Real-service smoke tests run only with dedicated disposable accounts and + explicit credentials. They are separate evidence for OAuth, IAM and service + behavior that local fixtures cannot establish. + +## First implementation PR + +Keep the first follow-up focused: extend the Drive mock for duplicate names, +nonrecursive deletion and denied listings; add regressions; fix ambiguous +resolution and deletion safety; run through the shared sync scanner; document +remaining unsupported features. Do not combine that safety PR with CLI transfer +support, Shared Drive support or an upload-engine rewrite. diff --git a/docs/plans/5_additional-backends.md b/docs/plans/5_additional-backends.md index 8816ac1..f3d14b6 100644 --- a/docs/plans/5_additional-backends.md +++ b/docs/plans/5_additional-backends.md @@ -6,10 +6,11 @@ Faro today speaks SFTP, FTP/FTPS, S3-compatible object storage, **Azure Blob** (both via `object_store`), local disk, and the Faro Agent — each a `RemoteFs` impl in `src-tauri/src/remotefs/` (`sftp.rs`, `ftp.rs`, `object.rs`, `local.rs`, `agent.rs`). Because browse, transfer, directory sync, and the -continuous folder-sync engine all sit on that one trait, **every new backend -inherits all of them for free** — proven by the Agent, which slotted in and -immediately worked with the explorer and transfers. This plan widens the -connection list, cheapest-first. +continuous folder-sync engine use common interfaces, a new backend can reuse +their orchestration. Each operation still needs provider-specific implementation +and verification; sharing a trait does not establish feature parity. See +[Plan 26](26_backend-correctness-and-coverage.md) for the safety audit and app/CLI +coverage requirements. This plan widens the connection list, cheapest-first. Each backend is the same recipe: diff --git a/docs/s3-testing.md b/docs/s3-testing.md new file mode 100644 index 0000000..6e9e88d --- /dev/null +++ b/docs/s3-testing.md @@ -0,0 +1,230 @@ +# S3 testing and CLI regression coverage + +The confirmed audit defects are fixed in this checkout. The expanded CLI audit +passes **55/55 checks**, including injected failures. The app transfer runner +also passes directory, Unicode, and 200 MiB multipart/resume tests. Changes +are tracked in [PR #34](https://github.com/jhd3197/Faro/pull/34); this document +does not imply a released build contains them. + +Final validation on Windows: 246 library tests passed (24 ignored; the known +unrelated `ssh_keygen_accepts_generated_key` Windows ACL test was filtered), +7 CLI unit tests passed, all 3 live S3 transfer tests passed, and TypeScript +`tsc --noEmit` passed. The live tests include 200 MiB multipart/ranged transfers +and restart after the remote source changes during a pause. + +No AWS account, Wasabi account, Docker, or real credentials are needed. The lab +uses [Moto's S3 HTTP server](https://docs.getmoto.org/en/5.1.12/docs/server_mode.html) +on `127.0.0.1`. Data lives in memory and disappears when the server stops. +Faro connects through its normal S3 client and signs requests with dummy keys. + +## Start the lab (Windows / PowerShell) + +From the repository root, install into an isolated, git-ignored environment: + +```powershell +python -m venv src-tauri/target/s3-lab-venv +src-tauri/target/s3-lab-venv/Scripts/python.exe -m pip install "moto[server]==5.1.12" boto3 +src-tauri/target/s3-lab-venv/Scripts/python.exe scripts/s3-lab.py +``` + +On macOS/Linux use `python3` to create the environment and its `bin/python` +instead of `Scripts/python.exe`. Use `--port 5010` if 5009 is occupied, or +`--check` to verify the API behavior and stop immediately. Stop the foreground +server with Ctrl+C. + +Create an S3 connection in Faro using the custom endpoint: + +| Field | Value | +| --- | --- | +| Endpoint | `http://127.0.0.1:5009` | +| Bucket | `faro-test` | +| Region | `us-east-1` | +| Access key / username | `faro-test` | +| Secret key / password | `faro-test-secret` | +| Remote path | `/issue-33` | + +Download `issue-33` into a fresh local folder. It should contain five files, +including a real zero-byte `empty.txt`, nested folders, a folder with spaces and +an ampersand, and an empty `marked/empty` folder. There should be no transfer +rows for the directory markers and no `object head` errors. Use a build from +this checkout to test the fix; an already-installed release may still have it. + +## Automated checks + +The wire-format tests start a tiny local HTTP fixture and exercise Faro's actual +S3 XML parser; they run without Moto or cloud credentials: + +```powershell +cargo test --manifest-path src-tauri/Cargo.toml -p faro --lib remotefs::object::tests +``` + +With the lab running, test the same directory listings the GUI uses and copy +every discovered file through Faro's real transfer runner, verifying bytes: + +```powershell +$env:FARO_LIVE_S3 = 'http://127.0.0.1:5009:faro-test:faro-test:faro-test-secret' +cargo test --manifest-path src-tauri/Cargo.toml -p faro --lib live_s3_directory_markers -- --ignored --nocapture +``` + +The existing broader transfer test uploads/downloads 200 MiB, checks multipart +ETags and ranged downloads, then replaces a paused download's source to check +that it restarts safely. It uses temporary local files and unique S3 keys: + +```powershell +cargo test --manifest-path src-tauri/Cargo.toml -p faro --lib live_s3_parallel_round_trip -- --ignored --nocapture +``` + +On Windows, prefer rustup's Cargo if Chocolatey shadows it: +`$env:Path = "$env:USERPROFILE\.cargo\bin;$env:Path"`. + +## Why the directory error happens + +S3 represents directories as key prefixes. Some tools also create zero-byte +objects ending in `/`, called folder markers. Listing `issue-33/marked/` with +delimiter `/` returns its marker in `Contents` alongside child objects, and +subdirectories in `CommonPrefixes`. + +The `object_store` 0.11.2 parser strips the marker's trailing slash. Faro used +to classify this returned object as a file, then request metadata for +`issue-33/marked`, which is a different, nonexistent key. That explains an +error per marked directory while its child files download successfully. + +The S3 namespace adapter now retains raw keys and trailing-slash markers for +listing, rename, mkdir, and deletion, using the storage library's SigV4 signer. +A directory's own marker is excluded from its children. Directory paths retain +a trailing slash so they can be distinguished from same-named file objects. +Streaming file operations use `Path::parse` to preserve names without encoding +them a second time. Real zero-byte files remain downloadable, and HEAD errors +are not suppressed. + +The lab checks the distinction directly: HEAD of `marked` returns 404, HEAD of +`marked/` returns 200, and GET of `marked/hello.txt` returns its exact contents. +See [AWS's prefix documentation](https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html) +and [HeadObject reference](https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html). + +## Unicode and special-character keys + +The lab seeds `/key-encoding/café.txt`. File metadata, downloads, uploads, +listing, editing, preview reads, and hashes now parse existing keys instead of +percent-encoding their names. The CLI audit covers Unicode, literal `%`, and +`#`, including a marked directory rename with a Unicode/percent filename. + +Run the app transfer regression with the lab running: + +```powershell +cargo test --manifest-path src-tauri/Cargo.toml -p faro --lib live_s3_unicode_key -- --ignored --nocapture +``` + +## What the lab proves + +It exercises S3 HTTP listing/metadata/download behavior and Faro's transfer +code. Moto is an emulator: it does not establish compatibility with every +Wasabi behavior, real IAM permissions, TLS setup, or network failure mode. +The report is consistent with this reproduced marker bug, but confirmation +against the reporter's actual bucket still requires a Wasabi retest. + +### Initial investigation, before the broader fixes (October 8, 2026) + +- Before the fix: two marker regression tests failed; the ordinary-prefix test passed. +- After the fix: all three wire-format tests passed. +- Live directory copy: all five files matched exactly, the empty directory was + preserved, and a genuinely missing file still returned an error. +- Live 200 MiB round trip: multipart upload, ranged download, and restart after + changing a paused source all passed. +- Unicode reproduction: failed as described above, after direct SDK retrieval + of the original key succeeded. This was subsequently fixed. +- Full library suite: 244 passed, 24 ignored, one failed. The failure was + `keys::tests::ssh_keygen_accepts_generated_key`: Windows OpenSSH rejected a + temporary private key because its inherited ACL allowed `CodexSandboxUsers`. + No SSH-key code was changed for this investigation. + +## Repeatable CLI audit + +The baseline audit on checkout `09814d7` (local version 1.3.38, with the first +#33 marker fix) had 30 passing checks and 18 unmet expectations. The completed +fix now passes all 55 checks, including seven additional rename and failure +cases. Some checks verify explicit unsupported-operation errors; passing the +audit does not mean every command supports every backend. + +Repeat it from the repository root: + +```powershell +cargo build --manifest-path src-tauri/Cargo.toml -p faro-cli +src-tauri/target/s3-lab-venv/Scripts/python.exe scripts/audit-s3-cli.py +``` + +The script starts and stops its own Moto server on a free loopback port, +creates dummy profiles under a temporary `FARO_DATA_DIR`, and checks actual S3 +keys and local bytes. It also uses a separate deterministic HTTP 403 fixture +to test an unreadable S3 source. It never reads the user's saved profiles or +connects to a real bucket. Exit 1 means expectations failed; evidence is saved +to `src-tauri/target/s3-cli-audit.json` (git-ignored). Temporary test files and +both servers are cleaned up automatically. + +### Verified working in the CLI + +- Saved-profile loading and marker-aware directory listing. +- Paginated listing of 1,005 objects. +- Ordinary file upload/download, zero-byte files, spaces and ampersands. +- A 17 MiB multipart upload and download, with byte equality checked. +- Ordinary file rename and delete. +- Recursive deletion of an implicit prefix, preserving a similarly named + neighboring prefix (`delete/` versus `delete-other/`). +- Sync dry-run without writes, nested push/pull, successful mirror deletion + in both directions, and pulling files from marker-backed directories. +- Hash diff for ordinary keys: identical trees and different equal-size files. +- Name search, opted-in content search, and hash-based duplicate detection. +- Nonzero errors for missing downloads, a denied individual deletion, and an + explicitly denied directory listing. +- Clear rejection of SSH `exec`, remote-to-remote `cp`, and `cp --recursive`. + +### Historical findings and implemented fixes + +| Priority | Finding | Reproduction and consequence | Code | +| --- | --- | --- | --- | +| 1 | Mirror sync must reject incomplete or unreadable sources | A 403 while listing the S3 source was treated as an empty tree. Pull with `--mirror` deleted a local `keep.txt` and exited 0. A nonexistent local source likewise caused push with `--mirror` to delete the remote `keep.txt`. | `src-tauri/src/scan.rs` (`walk_tree`, discarded listing errors), `src-tauri/src/sync.rs` (`plan_indexed`) | +| 1 | Exact object identity must survive deletion | `rm lab:/collision/ --recursive` deleted the separate object `collision` and left `collision/child.txt`. Deleting `whitespace/report.txt ` (trailing space) deleted `whitespace/report.txt` instead. | `src-tauri/src/remotefs/object.rs` (`normalize_prefix`, `delete`) | +| 2 | Key encoding must preserve stored names | Downloads of `café.txt`, `100%.txt`, and `hash#.txt` failed. Uploading `café.txt` created the literal key `caf%C3%A9.txt`. Listing a Unicode prefix returned an empty successful result. | `Path::from` calls throughout object-backed operations, including CLI upload/download | +| 2 | Failure must affect CLI exit status | `diff --hash` compared `AAA` with `BBB` under a Unicode key, recorded `hashError`, still classified it as `same`, and exited 0. Mirror sync whose deletion returned 403 printed a warning followed by `Sync complete.` and exited 0. | `src-tauri/src/diff.rs` (`hash_pass`), `src-tauri/faro-cli/src/main.rs` (`cmd_diff`, `cmd_sync`) | +| 2 | `cp` must honor the destination filename | Uploading `small.txt` to `rename-copy/different.txt` created `rename-copy/small.txt`. Downloading to `download-renamed.txt` created a directory of that name containing `small.txt`. Both exited 0. These argument-handling problems are not specific to S3. | `src-tauri/faro-cli/src/main.rs` (`cmd_cp`, `download_file`) | +| 3 | Directory-marker handling must extend beyond browsing | Recursive removal left `delete-marked/` and `delete-marked/empty/`. Name search reported a folder marker as `isDir: false`. | `src-tauri/src/remotefs/object.rs` (`delete`), `src-tauri/src/search.rs` (`name_object`) | +| 3 | Directory feature support needs an explicit contract | Prefix rename failed with 404. `mkdir` intentionally returned success without creating anything. Sync copied the files but omitted an empty marker-backed directory. | Object adapter `rename`/`create_dir`; file-only sync plan | + +Every finding in the table is now covered by passing regression checks. The +shared scanner rejects missing/unreadable source roots and child-listing errors; +only a missing destination root can be treated as empty. Exact-key operations +preserve trailing spaces and distinguish files from directory prefixes. + +Sync plans now include empty directories, and the app sync dialog displays +them. The shared app/Bridge executor propagates deletion errors and waits for +successful copies before applying mirror deletions. CLI mirror deletion and +hash-read failures return nonzero status. Explicit `cp` destination filenames +are honored; use a trailing slash for a remote destination directory. For local +destinations, an existing directory or trailing separator means a directory; +otherwise the argument is the destination filename. + +Reported CLI download failures clean up a staged file without touching the +original destination. A complete download is synced and atomically placed. +Multipart upload errors and Ctrl+C request an abort, and abort failures are +reported. The fault suite verifies truncated downloads, multipart abort after +a rejected part, unreadable child directories, and denied hash reads. + +### Parity and remaining validation + +CLI `cp` remains file-only; use `sync` for directory trees. Remote-to-remote +`cp` and SSH `exec` on S3 are explicitly rejected. CLI transfers remain separate +from the app's transfer manager: persistent CLI pause/resume and parallel-range +scheduling have not been added. Abrupt process termination or loss of network +access during cleanup can still leave temporary files or multipart uploads. + +S3 directory rename consists of copies followed by deletions, not an atomic +transaction. Existing destination prefixes and overlapping source/destination +prefixes are rejected; all copies must succeed before source deletion starts. +Concurrent changes and versioned-bucket behavior need provider-specific tests. + +Still untested: actual Wasabi and other vendors, real IAM/credential discovery +and session tokens, TLS, object locks, encryption/KMS, throttling, and exhaustive +GUI/Agent Bridge/editor/preview workflows. The frontend type-checks and the app +transfer runner is exercised directly; no full native GUI end-to-end test was +performed. Moto success is evidence for tested semantics, not certification of +every provider or bucket configuration. diff --git a/scripts/audit-s3-cli.py b/scripts/audit-s3-cli.py new file mode 100644 index 0000000..fd2b88b --- /dev/null +++ b/scripts/audit-s3-cli.py @@ -0,0 +1,332 @@ +"""Black-box S3 CLI audit. Uses only an ephemeral Moto server and dummy profiles. + +Run with the Python environment from docs/s3-testing.md, after building faro-cli. +Exit 1 means at least one desired behavior failed; JSON preserves the evidence. +""" + +import argparse +import hashlib +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import json +import logging +import os +from pathlib import Path +import subprocess +import tempfile +import threading +from urllib.parse import urlsplit, parse_qs + +import boto3 +from botocore.config import Config +from moto.server import ThreadedMotoServer + + +def main(): + root = Path(__file__).resolve().parents[1] + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--cli", type=Path, default=root / "src-tauri/target/debug" / ("faro-cli.exe" if os.name == "nt" else "faro-cli")) + parser.add_argument("--report", type=Path, default=root / "src-tauri/target/s3-cli-audit.json") + args = parser.parse_args() + cli = args.cli.resolve() + if not cli.is_file(): + parser.error(f"build faro-cli first: {cli}") + logging.getLogger("werkzeug").setLevel(logging.ERROR) + server = ThreadedMotoServer(ip_address="127.0.0.1", port=0, verbose=False) + server.start() + results = [] + try: + host, port = server.get_host_and_port() + endpoint = f"http://{host}:{port}" + s3 = boto3.client("s3", endpoint_url=endpoint, region_name="us-east-1", + aws_access_key_id="test", aws_secret_access_key="test", + config=Config(s3={"addressing_style": "path"}, proxies={})) + bucket = "faro-cli-audit" + s3.create_bucket(Bucket=bucket) + + def put(key, body=b"payload\n"): + s3.put_object(Bucket=bucket, Key=key, Body=body) + + def keys(prefix=""): + return sorted(obj["Key"] for page in s3.get_paginator("list_objects_v2").paginate(Bucket=bucket, Prefix=prefix) for obj in page.get("Contents", [])) + + def body(key): + return s3.get_object(Bucket=bucket, Key=key)["Body"].read() + + with tempfile.TemporaryDirectory(prefix="faro-s3-cli-") as scratch: + work = Path(scratch) + profile = dict(id="lab", name="lab", protocol="s3", host=host, port=port, + username="test", auth=dict(kind="password", password="test"), + bucket=bucket, region="us-east-1", endpoint=endpoint) + (work / "profiles.json").write_text(json.dumps([profile]), encoding="utf-8") + env = dict(os.environ, FARO_DATA_DIR=str(work), NO_PROXY="127.0.0.1,localhost") + + def run(*argv): + p = subprocess.run([str(cli), *map(str, argv)], env=env, cwd=work, + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=60) + return dict(command=list(map(str, argv)), code=p.returncode, stdout=p.stdout, stderr=p.stderr) + + def check(name, argv, verify, expected_code=0): + result = {} + try: + result = run(*argv) + ok = result["code"] == expected_code and verify(result) + record = dict(name=name, passed=bool(ok), **result) + except Exception as error: + record = dict(name=name, passed=False, error=str(error), **result) + results.append(record) + print(f"{'PASS' if record['passed'] else 'FAIL'} {name}", flush=True) + return record + + def local(name, data=b"payload\n"): + path = work / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + return path + + check("profiles: isolated S3 profile loads", ["profiles", "list"], lambda r: "lab" in r["stdout"]) + put("browse/", b"") + put("browse/nested/", b"") + put("browse/nested/hello.txt") + put("browse/empty.txt", b"") + check("ls: markers excluded, empty file retained", ["ls", "lab:/browse", "--bytes"], + lambda r: len(r["stdout"].splitlines()) == 2 and "nested" in r["stdout"] and "empty.txt" in r["stdout"]) + for n in range(1005): + put(f"pagination/{n:04}.txt", b"x") + check("ls: all 1005 objects across pages", ["ls", "lab:/pagination"], lambda r: len(r["stdout"].splitlines()) == 1005) + + small = local("small.txt") + check("cp: ordinary upload", ["cp", small, "lab:/copy/small.txt"], lambda r: body("copy/small.txt") == small.read_bytes()) + down = work / "download" + down.mkdir() + check("cp: ordinary download to directory", ["cp", "lab:/copy/small.txt", down], lambda r: (down / "small.txt").read_bytes() == small.read_bytes()) + empty = local("empty.txt", b"") + check("cp: zero-byte upload", ["cp", empty, "lab:/copy/empty.txt"], lambda r: body("copy/empty.txt") == b"") + check("cp: zero-byte download", ["cp", "lab:/copy/empty.txt", down], lambda r: (down / "empty.txt").read_bytes() == b"") + spaced = local("hello world.txt") + check("cp: spaces and ampersand", ["cp", spaced, "lab:/spaces & symbols/hello world.txt"], lambda r: body("spaces & symbols/hello world.txt") == spaced.read_bytes()) + big = local("large.bin", bytes(range(256)) * (17 * 4096)) + check("cp: 17 MiB multipart upload", ["cp", big, "lab:/copy/large.bin"], lambda r: hashlib.sha256(body("copy/large.bin")).digest() == hashlib.sha256(big.read_bytes()).digest()) + check("cp: 17 MiB download", ["cp", "lab:/copy/large.bin", down], lambda r: (down / "large.bin").read_bytes() == big.read_bytes()) + check("cp: explicit remote destination filename", ["cp", small, "lab:/rename-copy/different.txt"], lambda r: keys("rename-copy/") == ["rename-copy/different.txt"]) + exact = work / "download-renamed.txt" + check("cp: explicit local destination filename", ["cp", "lab:/copy/small.txt", exact], lambda r: exact.is_file() and exact.read_bytes() == small.read_bytes()) + check("cp: missing object is an error", ["cp", "lab:/copy/missing.txt", down], lambda r: "404" in r["stderr"], expected_code=1) + check("cp: remote-to-remote explicitly unsupported", ["cp", "lab:/copy/small.txt", "lab:/other.txt"], lambda r: "not yet supported" in r["stderr"], expected_code=1) + check("cp: recursive option explicitly unavailable", ["cp", "--recursive", "lab:/browse", down], lambda r: "unexpected argument" in r["stderr"], expected_code=2) + + for name in ["café.txt", "100%.txt", "hash#.txt"]: + put(f"special/{name}") + check(f"cp: download exact key {name}", ["cp", f"lab:/special/{name}", down], lambda r, n=name: (down / n).read_bytes() == b"payload\n") + unicode_file = local("café.txt") + check("cp: upload preserves Unicode key", ["cp", unicode_file, "lab:/unicode-upload/café.txt"], lambda r: keys("unicode-upload/") == ["unicode-upload/café.txt"]) + put("café/child.txt") + check("ls: Unicode prefix", ["ls", "lab:/café"], lambda r: "child.txt" in r["stdout"]) + + check("mv: ordinary file", ["mv", "lab:/copy/small.txt", "lab:/copy/moved.txt"], lambda r: "copy/small.txt" not in keys("copy/") and body("copy/moved.txt") == b"payload\n") + check("rm: ordinary file", ["rm", "lab:/copy/moved.txt"], lambda r: "copy/moved.txt" not in keys("copy/")) + put("move-dir/child.txt") + check("mv: directory prefix", ["mv", "lab:/move-dir", "lab:/moved-dir"], lambda r: keys("move-dir/") == [] and body("moved-dir/child.txt") == b"payload\n") + put("rename-marked", b"separate file") + put("rename-marked/", b"") + put("rename-marked/empty/", b"") + put("rename-marked/café%.txt") + check("mv: marked directory preserves Unicode and empty folders", ["mv", "lab:/rename-marked/", "lab:/rename-result/"], + lambda r: keys("rename-marked/") == [] and body("rename-marked") == b"separate file" and keys("rename-result/") == ["rename-result/", "rename-result/café%.txt", "rename-result/empty/"]) + check("mv: overlapping directory rename is rejected", ["mv", "lab:/rename-result/", "lab:/rename-result/inside/"], + lambda r: len(keys("rename-result/")) == 3, expected_code=1) + put("delete/child.txt") + put("delete-other/keep.txt") + check("rm: recursive implicit prefix and sibling boundary", ["rm", "lab:/delete", "--recursive"], lambda r: keys("delete/") == [] and keys("delete-other/") == ["delete-other/keep.txt"]) + put("delete-marked/", b"") + put("delete-marked/empty/", b"") + put("delete-marked/child.txt") + check("rm: recursive marker directory fully removed", ["rm", "lab:/delete-marked", "--recursive"], lambda r: keys("delete-marked/") == []) + put("collision", b"keep me") + put("collision/child.txt") + check("rm: trailing-slash directory preserves same-named object", ["rm", "lab:/collision/", "--recursive"], lambda r: keys("collision") == ["collision"] and body("collision") == b"keep me") + put("whitespace/report.txt ", b"remove me") + put("whitespace/report.txt", b"keep me") + check("rm: exact trailing-space key preserves neighboring object", ["rm", "lab:/whitespace/report.txt "], lambda r: keys("whitespace/") == ["whitespace/report.txt"] and body("whitespace/report.txt") == b"keep me") + check("mkdir: actually creates an empty directory", ["mkdir", "lab:/created-empty"], lambda r: keys("created-empty") == ["created-empty/"]) + + push = local("push/a.txt", b"one\n").parent + local("push/nested/b.txt", b"two\n") + local("push/empty.txt", b"") + check("sync: dry run leaves bucket unchanged", ["sync", push, "lab:/sync", "--dry-run"], lambda r: keys("sync/") == []) + check("sync: nested push", ["sync", push, "lab:/sync"], lambda r: [k for k in keys("sync/") if not k.endswith('/')] == ["sync/a.txt", "sync/empty.txt", "sync/nested/b.txt"] and body("sync/nested/b.txt") == b"two\n") + pull = work / "pull" + pull.mkdir() + check("sync: nested pull", ["sync", pull, "lab:/sync", "--direction", "pull"], lambda r: all((pull / p.relative_to(push)).read_bytes() == p.read_bytes() for p in push.rglob("*") if p.is_file())) + put("sync/extra.txt") + check("sync: mirror push removes destination-only file", ["sync", push, "lab:/sync", "--mirror"], lambda r: "sync/extra.txt" not in keys("sync/")) + local("pull/extra.txt") + check("sync: mirror pull removes destination-only file", ["sync", pull, "lab:/sync", "--mirror", "--direction", "pull"], lambda r: not (pull / "extra.txt").exists()) + marked = work / "marked-pull" + marked.mkdir() + put("browse/empty-dir/", b"") + check("sync: pull marked directory files", ["sync", marked, "lab:/browse", "--direction", "pull"], lambda r: (marked / "nested/hello.txt").read_bytes() == b"payload\n" and (marked / "empty.txt").read_bytes() == b"") + check("sync: pull preserves empty marker folders", ["sync", marked, "lab:/browse", "--direction", "pull"], lambda r: (marked / "empty-dir").is_dir()) + check("diff: identical trees with hashes", ["diff", push, "lab:/sync", "--hash", "--json"], lambda r: '"hashError"' not in r["stdout"]) + put("sync/a.txt", b"TWO\n") + check("diff: detects equal-size content changes", ["diff", push, "lab:/sync", "--hash", "--json"], lambda r: '"different"' in r["stdout"], expected_code=1) + uni_diff = local("unicode-diff/café.txt", b"AAA").parent + put("unicode-diff/café.txt", b"BBB") + check("diff: Unicode hash comparison fails closed", ["diff", uni_diff, "lab:/unicode-diff", "--hash", "--json"], lambda r: True, expected_code=1) + check("search: name matches", ["search", "lab:/sync", "*.txt", "--json"], lambda r: len(json.loads(r["stdout"])["hits"]) == 3) + check("search: content with opt-in", ["search", "lab:/sync", "two", "--content", "--content-remote", "--json"], lambda r: len(json.loads(r["stdout"])["hits"]) >= 1) + check("search: folder markers classified as directories", ["search", "lab:/browse", "nested", "--json"], lambda r: all(h["isDir"] for h in json.loads(r["stdout"])["hits"])) + put("dedupe/a.txt", b"same") + put("dedupe/b.txt", b"same") + check("dedupe: hashes find identical files", ["dedupe", "lab:/dedupe", "--hash", "--json"], lambda r: "a.txt" in r["stdout"] and "b.txt" in r["stdout"]) + check("exec: S3 clearly rejected", ["exec", "lab", "echo hello"], lambda r: "SSH/SFTP" in r["stderr"], expected_code=1) + + # Resource-policy denies are checked by Moto. Verify denial directly + # first so a missing emulator permission feature cannot look like a + # successful negative-path check. + put("denied/stale.txt") + s3.put_bucket_policy(Bucket=bucket, Policy=json.dumps({ + "Version": "2012-10-17", "Statement": [{ + "Effect": "Deny", "Principal": "*", "Action": "s3:DeleteObject", + "Resource": f"arn:aws:s3:::{bucket}/denied/*", + }], + })) + try: + s3.delete_object(Bucket=bucket, Key="denied/stale.txt") + except s3.exceptions.ClientError as error: + if error.response["ResponseMetadata"]["HTTPStatusCode"] != 403: + raise + else: + raise AssertionError("Moto did not enforce the test's DeleteObject deny") + check("rm: access denial is reported", ["rm", "lab:/denied/stale.txt"], lambda r: "403" in r["stderr"] and keys("denied/") == ["denied/stale.txt"], expected_code=1) + empty_source = work / "empty-source" + empty_source.mkdir() + check("sync: mirror deletion failure returns nonzero", ["sync", empty_source, "lab:/denied", "--mirror"], lambda r: keys("denied/") == ["denied/stale.txt"], expected_code=1) + s3.delete_bucket_policy(Bucket=bucket) + + put("missing-local-source/keep.txt", b"must survive") + check("sync: missing local source cannot mirror-delete remote files", + ["sync", work / "does-not-exist", "lab:/missing-local-source", "--mirror"], + lambda r: keys("missing-local-source/") == ["missing-local-source/keep.txt"], expected_code=1) + + # A deterministic S3 HTTP 403 fixture exercises read failures + # independently of Moto's coverage of IAM ListBucket conditions. + class DeniedListing(BaseHTTPRequestHandler): + def do_GET(self): + xml = b"AccessDeniedListing denied by audit fixture" + self.send_response(403) + self.send_header("Content-Type", "application/xml") + self.send_header("Content-Length", str(len(xml))) + self.end_headers() + self.wfile.write(xml) + + def log_message(self, *_args): + pass + + denied_server = ThreadingHTTPServer(("127.0.0.1", 0), DeniedListing) + denied_thread = threading.Thread(target=denied_server.serve_forever, daemon=True) + denied_thread.start() + protected = local("protected-pull/keep.txt", b"must survive") + try: + blocked = dict(profile, id="blocked", name="blocked", endpoint=f"http://127.0.0.1:{denied_server.server_port}") + (work / "profiles.json").write_text(json.dumps([profile, blocked]), encoding="utf-8") + check("ls: explicit 403 listing is reported", ["ls", "blocked:/"], lambda r: "403" in r["stderr"], expected_code=1) + check("sync: unreadable S3 source cannot mirror-delete local files", + ["sync", protected.parent, "blocked:/", "--direction", "pull", "--mirror"], + lambda r: protected.is_file() and protected.read_bytes() == b"must survive", expected_code=1) + finally: + denied_server.shutdown() + denied_server.server_close() + denied_thread.join() + + class TransferFaults(BaseHTTPRequestHandler): + aborted = False + + def respond(self, status, data=b"", **headers): + self.send_response(status) + self.send_header("Content-Length", str(len(data))) + for name, value in headers.items(): + self.send_header(name.replace('_', '-'), value) + self.end_headers() + self.wfile.write(data) + + def do_GET(self): + parsed = urlsplit(self.path) + if "list-type" in parse_qs(parsed.query): + prefix = parse_qs(parsed.query).get("prefix", [""])[0] + if prefix == "hash/": + self.respond(200, b'hash/a.txt32026-10-08T00:00:00Ztest') + elif prefix == "tree/": + self.respond(200, b'tree/unreadable/') + else: + self.respond(403, b'AccessDenied') + elif parsed.path.endswith("partial.txt"): + self.send_response(200) + self.send_header("Content-Length", "100") + self.send_header("Last-Modified", "Thu, 08 Oct 2026 00:00:00 GMT") + self.send_header("ETag", '"test"') + self.send_header("Connection", "close") + self.end_headers() + self.wfile.write(b"partial") + self.close_connection = True + else: + self.respond(403, b'AccessDenied') + + def do_POST(self): + self.respond(200, b'faro-cli-auditlarge.binaudit-upload') + + def do_PUT(self): + self.close_connection = True + self.respond(403, b'AccessDenied', Connection="close") + + def do_DELETE(self): + type(self).aborted = "uploadId=audit-upload" in self.path + self.respond(204) + + def log_message(self, *_args): + pass + + fault_server = ThreadingHTTPServer(("127.0.0.1", 0), TransferFaults) + fault_thread = threading.Thread(target=fault_server.serve_forever, daemon=True) + fault_thread.start() + try: + fault = dict(profile, id="fault", name="fault", endpoint=f"http://127.0.0.1:{fault_server.server_port}") + (work / "profiles.json").write_text(json.dumps([profile, fault]), encoding="utf-8") + original = local("fault-download/partial.txt", b"original contents") + check("cp: truncated download preserves existing destination", ["cp", "fault:/partial.txt", original], + lambda r: original.read_bytes() == b"original contents" and not list(original.parent.glob("*.faro-part")), expected_code=1) + new_target = original.parent / "new.txt" + check("cp: truncated download leaves no partial destination", ["cp", "fault:/partial.txt", new_target], + lambda r: not new_target.exists() and not list(original.parent.glob("*.faro-part")), expected_code=1) + check("cp: failed multipart upload is aborted", ["cp", big, "fault:/large.bin"], + lambda r: TransferFaults.aborted, expected_code=1) + hash_root = local("fault-hash/a.txt", b"AAA").parent + check("diff: denied hash read returns nonzero", ["diff", hash_root, "fault:/hash", "--hash", "--json"], + lambda r: "hashError" in r["stdout"], expected_code=1) + nested_keep = local("fault-tree/keep.txt", b"must survive") + check("sync: unreadable child cannot produce a partial mirror plan", ["sync", nested_keep.parent, "fault:/tree", "--direction", "pull", "--mirror"], + lambda r: nested_keep.read_bytes() == b"must survive", expected_code=1) + finally: + fault_server.shutdown() + fault_server.server_close() + fault_thread.join() + + # Preserve postconditions for false-success findings after the + # disposable server and local work directory are cleaned up. + results.append(dict(name="evidence: resulting keys and paths", passed=True, + observation_only=True, + keys={p: keys(p) for p in ["rename-copy/", "unicode-upload/", "delete-marked/", "collision", "whitespace/", "denied/", "created-empty", "missing-local-source/"]}, + unreadable_source_local_file_preserved=protected.exists(), + explicit_local_destination_is_directory=exact.is_dir(), + explicit_local_destination_children=[p.name for p in exact.iterdir()] if exact.is_dir() else [])) + finally: + server.stop() + args.report.parent.mkdir(parents=True, exist_ok=True) + args.report.write_text(json.dumps(dict(cli=str(cli), results=results), indent=2, ensure_ascii=False), encoding="utf-8") + checks = [r for r in results if not r.get("observation_only")] + failures = sum(not r["passed"] for r in checks) + print(f"\n{len(checks) - failures} passed, {failures} failed. Evidence: {args.report}") + return bool(failures) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/s3-lab.py b/scripts/s3-lab.py new file mode 100644 index 0000000..d340453 --- /dev/null +++ b/scripts/s3-lab.py @@ -0,0 +1,76 @@ +"""Disposable, loopback-only S3 lab for Faro issue #33. See docs/s3-testing.md.""" + +import argparse +import logging +import threading + +import boto3 +from botocore.config import Config +from botocore.exceptions import ClientError +from moto.server import ThreadedMotoServer + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--port", type=int, default=5009) + parser.add_argument("--check", action="store_true", help="check API semantics and exit") + args = parser.parse_args() + logging.getLogger("werkzeug").setLevel(logging.WARNING) + server = ThreadedMotoServer(ip_address="127.0.0.1", port=args.port, verbose=False) + server.start() + try: + host, port = server.get_host_and_port() + endpoint = f"http://{host}:{port}" + client = boto3.client( + "s3", endpoint_url=endpoint, region_name="us-east-1", + aws_access_key_id="faro-test", aws_secret_access_key="faro-test-secret", + config=Config(s3={"addressing_style": "path"}, proxies={}), + ) + bucket = "faro-test" + client.create_bucket(Bucket=bucket) + objects = { + "issue-33/": b"", + "issue-33/marked/": b"", + "issue-33/marked/hello.txt": b"hello from marked folder\n", + "issue-33/marked/nested/": b"", + "issue-33/marked/nested/data.txt": b"nested payload\n", + "issue-33/marked/empty/": b"", + "issue-33/implicit/hello.txt": b"hello from implicit folder\n", + "issue-33/empty.txt": b"", + "issue-33/spaces & symbols/hello world.txt": b"spaced filename\n", + "key-encoding/café.txt": b"coffee\n", + } + for key, body in objects.items(): + client.put_object(Bucket=bucket, Key=key, Body=body) + + listing = client.list_objects_v2(Bucket=bucket, Prefix="issue-33/marked/", Delimiter="/") + assert "issue-33/marked/" in [obj["Key"] for obj in listing["Contents"]] + print("LIST marked/: contains its own trailing-slash folder marker", flush=True) + for key in ["issue-33/marked", "issue-33/implicit", "issue-33/missing.txt"]: + try: + client.head_object(Bucket=bucket, Key=key) + except ClientError as error: + status = error.response["ResponseMetadata"]["HTTPStatusCode"] + assert status == 404, (key, status) + print(f"HEAD {key}: {status} (expected)", flush=True) + else: + raise AssertionError(f"HEAD unexpectedly succeeded: {key}") + assert client.head_object(Bucket=bucket, Key="issue-33/marked/")["ContentLength"] == 0 + assert client.get_object(Bucket=bucket, Key="issue-33/marked/hello.txt")["Body"].read() == objects["issue-33/marked/hello.txt"] + print("HEAD marked/: 200; GET marked/hello.txt: exact bytes verified", flush=True) + print(f"\nFaro S3 connection:\n Endpoint: {endpoint}\n Bucket: {bucket}\n" + " Region: us-east-1\n Access key: faro-test\n Secret key: faro-test-secret\n" + " Remote path: /issue-33\n", flush=True) + print(f"PowerShell: $env:FARO_LIVE_S3='{endpoint}:{bucket}:faro-test:faro-test-secret'", flush=True) + if not args.check: + print("Lab running. Ctrl+C stops it and discards all test objects.", flush=True) + try: + threading.Event().wait() + except KeyboardInterrupt: + pass + finally: + server.stop() + + +if __name__ == "__main__": + main() diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 0515055..7a54cea 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1552,6 +1552,7 @@ dependencies = [ "async-trait", "base64 0.22.1", "bytes", + "chrono", "cloud-filter", "dirs 5.0.1", "encoding_rs", @@ -1655,6 +1656,7 @@ dependencies = [ "reqwest 0.12.28", "russh", "serde_json", + "tempfile", "tokio", "ureq", ] @@ -4354,6 +4356,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f7649a7b4df05aed9ea7ec6f628c67c9953a43869b8bc50929569b2999d443fe" dependencies = [ "memchr", + "serde", ] [[package]] diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index f9a8356..93bf026 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -94,7 +94,8 @@ object_store = { version = "0.11", features = ["aws", "azure", "gcp"] } bytes = "1" # Importers — quick-xml for FileZilla's sitemanager.xml. -quick-xml = "0.36" +quick-xml = { version = "0.36", features = ["serialize"] } +chrono = { version = "0.4", default-features = false, features = ["std"] } # Shared connection index (faro.db). The `bundled` feature compiles SQLite from # source so there's no system-SQLite dependency. One DB under app_data_dir holds diff --git a/src-tauri/faro-cli/Cargo.toml b/src-tauri/faro-cli/Cargo.toml index 09a165b..fa2beaf 100644 --- a/src-tauri/faro-cli/Cargo.toml +++ b/src-tauri/faro-cli/Cargo.toml @@ -43,6 +43,7 @@ ureq = { version = "2", default-features = false, features = ["json", "tls", "gz # Misc anyhow = "1" +tempfile = "3" serde_json = "1" dirs = "5" base64 = "0.22" diff --git a/src-tauri/faro-cli/src/main.rs b/src-tauri/faro-cli/src/main.rs index b55eee7..faa2af8 100644 --- a/src-tauri/faro-cli/src/main.rs +++ b/src-tauri/faro-cli/src/main.rs @@ -65,6 +65,7 @@ enum Cmd { /// Copy a file from one location to another. Cp { source: String, + /// Destination filename or directory. End remote directories with `/`. destination: String, }, @@ -994,14 +995,18 @@ async fn cmd_cp(store: &ProfileStore, src: &str, dst: &str) -> Result<()> { let profile = find_profile(store, &profile_name).await?; let session = open_session(&profile).await?; let dest_parent = parent_of(&dest); - let local_name = basename(&local_path); - upload_file(&session, &local_path, &dest_parent, &local_name).await?; + let name = if dest.ends_with('/') { basename(&local_path) } else { basename(&dest) }; + let parent = if dest.ends_with('/') { dest.trim_end_matches('/').to_string() } else { dest_parent }; + upload_file(&session, &local_path, &parent, &name).await?; } // download: remote → local - (Target::Remote { profile_name, path: src }, Target::Local(local_dir)) => { + (Target::Remote { profile_name, path: src }, Target::Local(destination)) => { let profile = find_profile(store, &profile_name).await?; let session = open_session(&profile).await?; - download_file(&session, &src, &local_dir).await?; + let target = if std::path::Path::new(&destination).is_dir() || destination.ends_with(['/', '\\']) { + PathBuf::from(&destination).join(basename(&src)) + } else { PathBuf::from(&destination) }; + download_file_to(&session, &src, &target).await?; } // remote → remote: not supported across profiles in v1.1. (Target::Remote { .. }, Target::Remote { .. }) => { @@ -1130,11 +1135,17 @@ async fn cmd_sync( return Ok(()); } - if plan.copies.is_empty() && plan.deletes.is_empty() { + if plan.copies.is_empty() && plan.deletes.is_empty() && plan.directories.is_empty() { eprintln!("Already in sync."); return Ok(()); } + for directory in &plan.directories { + match direction { + Dir::Push => remote_fs.create_dir(directory).await?, + Dir::Pull => tokio::fs::create_dir_all(directory).await?, + } + } let bar = ProgressBar::new(plan.copies.len() as u64); bar.set_style( ProgressStyle::with_template("{bar:30.cyan/blue} {pos:>3}/{len:3} {msg}") @@ -1163,9 +1174,7 @@ async fn cmd_sync( Dir::Push => remote_fs.as_ref(), Dir::Pull => local_fs.as_ref(), }; - if let Err(e) = fs.delete(&d.path, false).await { - eprintln!("{} {}: {e}", warn("delete failed"), d.path); - } + fs.delete(&d.path, false).await.with_context(|| format!("sync deletion failed: {}", d.path))?; } eprintln!("Sync complete."); Ok(()) @@ -1264,6 +1273,9 @@ async fn cmd_diff( // Conventional diff exit code: 0 when identical, 1 when the trees differ. let s = &result.summary; let differ = s.only_in_a + s.only_in_b + s.different; + if result.entries.iter().any(|e| e.hash_error.is_some()) { + bail!("content comparison incomplete: one or more files could not be hashed"); + } std::process::exit(if differ == 0 { 0 } else { 1 }) } @@ -1312,7 +1324,11 @@ fn print_diff_human(result: &faro_lib::diff::DiffResult, all: bool) { } if shown == 0 { - eprintln!("Trees are identical."); + if result.entries.iter().any(|e| e.hash_error.is_some()) { + eprintln!("Comparison incomplete: some files could not be hashed."); + } else { + eprintln!("Trees are identical."); + } } let s = &result.summary; @@ -3254,7 +3270,7 @@ async fn upload_object( ) -> Result<()> { use tokio::io::AsyncReadExt; let key = remote_path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let mut file = tokio::fs::File::open(local_path).await?; let meta = file.metadata().await?; if meta.len() <= 16 * 1024 * 1024 { @@ -3267,36 +3283,59 @@ async fn upload_object( bar.inc(meta.len()); } else { let mut upload = obj.store.put_multipart(&p).await?; - let mut buf = vec![0u8; 8 * 1024 * 1024]; - loop { - let mut filled = 0; - while filled < buf.len() { - let n = file.read(&mut buf[filled..]).await?; - if n == 0 { - break; + let result: Result<()> = tokio::select! { + result = async { + let mut buf = vec![0u8; 8 * 1024 * 1024]; + loop { + let mut filled = 0; + while filled < buf.len() { + let n = file.read(&mut buf[filled..]).await?; + if n == 0 { + break; + } + filled += n; + } + if filled == 0 { + break; + } + let chunk = bytes::Bytes::copy_from_slice(&buf[..filled]); + upload.put_part(chunk.into()).await?; + bar.inc(filled as u64); + if filled < buf.len() { + break; + } } - filled += n; - } - if filled == 0 { - break; - } - let chunk = bytes::Bytes::copy_from_slice(&buf[..filled]); - upload.put_part(chunk.into()).await?; - bar.inc(filled as u64); - if filled < buf.len() { - break; + upload.complete().await?; + Ok(()) + } => result, + _ = tokio::signal::ctrl_c() => Err(anyhow!("upload canceled")), + }; + if let Err(error) = result { + if let Err(abort) = upload.abort().await { + return Err(error.context(format!("multipart cleanup also failed: {abort}"))); } + return Err(error); } - upload.complete().await?; } Ok(()) } async fn download_file(session: &Session, remote_path: &str, local_dir: &str) -> Result<()> { - let local_dir_path = PathBuf::from(local_dir); - tokio::fs::create_dir_all(&local_dir_path).await.ok(); let name = basename(remote_path); - let final_path = local_dir_path.join(&name); + download_file_to(session, remote_path, &PathBuf::from(local_dir).join(name)).await +} + +async fn download_file_to(session: &Session, remote_path: &str, final_path: &std::path::Path) -> Result<()> { + let name = basename(remote_path); + if let Some(parent) = final_path.parent().filter(|p| !p.as_os_str().is_empty()) { + tokio::fs::create_dir_all(parent).await?; + } + let target = final_path.to_path_buf(); + let parent = target.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(std::path::Path::new(".")); + // RAII removes the staged copy on errors. Persist replaces the destination + // atomically only after the complete download is flushed and synced. + let staged = tempfile::Builder::new().prefix(".faro-cli-").suffix(".faro-part").tempfile_in(parent)?; + let final_path = staged.path().to_path_buf(); let bar = make_bar(0, &name); @@ -3336,16 +3375,20 @@ async fn download_file(session: &Session, remote_path: &str, local_dir: &str) -> Session::Object(obj) => { use tokio::io::AsyncWriteExt; let key = remote_path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let get = obj.store.get(&p).await?; + let expected = get.meta.size as u64; + let mut received = 0u64; let mut file = tokio::fs::File::create(&final_path).await?; let mut stream = get.into_stream(); - while let Some(chunk) = stream.next().await { + while let Some(chunk) = tokio::time::timeout(std::time::Duration::from_secs(60), stream.next()).await.context("download stalled")? { let chunk = chunk?; + received += chunk.len() as u64; file.write_all(&chunk).await?; bar.inc(chunk.len() as u64); } file.flush().await?; + if received != expected { bail!("incomplete download: received {received} of {expected} bytes"); } } Session::Webdav(_) | Session::Http(_) @@ -3362,6 +3405,8 @@ async fn download_file(session: &Session, remote_path: &str, local_dir: &str) -> Session::Agent(_) => anyhow::bail!("faro-agent connections are not supported in the CLI"), } + staged.as_file().sync_all()?; + staged.persist(&target).map_err(|e| e.error).with_context(|| format!("place download {}", target.display()))?; bar.finish_with_message(format!("downloaded {name}")); Ok(()) } diff --git a/src-tauri/src/bridge.rs b/src-tauri/src/bridge.rs index af91829..5483610 100644 --- a/src-tauri/src/bridge.rs +++ b/src-tauri/src/bridge.rs @@ -3947,13 +3947,14 @@ async fn op_sync( "deleteCount": delete_count, "totalBytes": total_bytes, "copies": copies, + "directories": plan.directories, "deletes": deletes, "listTruncated": copy_count > SYNC_PLAN_MAX_ENTRIES || delete_count > SYNC_PLAN_MAX_ENTRIES, }), ); } - if copy_count == 0 && delete_count == 0 { + if copy_count == 0 && delete_count == 0 && plan.directories.is_empty() { // Nothing to do — don't raise an approval prompt for a no-op. state .log( @@ -3988,6 +3989,7 @@ async fn op_sync( total_bytes, delete_count, ); + let summary = format!("{summary}; create {} folders", plan.directories.len()); if let Err(resp) = gate( app, state, diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 0d1156a..74104b3 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -1483,6 +1483,13 @@ pub(crate) async fn execute_sync_plan( let mut ids = Vec::new(); let policy = crate::transfer::OverwritePolicy::Overwrite; + for directory in &plan.directories { + match plan.direction { + SyncDirection::LocalToRemote => remote_fs.create_dir(directory).await?, + SyncDirection::RemoteToLocal => tokio::fs::create_dir_all(directory).await?, + } + } + for copy in plan.copies { let dest_parent = parent_of(©.destination_path); let id = match plan.direction { @@ -1512,16 +1519,27 @@ pub(crate) async fn execute_sync_plan( ids.push(id); } - // Apply Mirror deletes after queueing transfers. We don't gate on - // transfer completion — the user already confirmed the plan — but we - // do execute deletes serially in this call so the function only - // returns once the destination is in its final shape. + // Do not remove destination files if a replacement copy failed. + if !plan.deletes.is_empty() { + for id in &ids { + loop { + let transfer = transfers.get(id).await.ok_or_else(|| anyhow::anyhow!("sync transfer disappeared"))?; + match transfer.status { + crate::transfer::TransferStatus::Done | crate::transfer::TransferStatus::Skipped => break, + crate::transfer::TransferStatus::Error | crate::transfer::TransferStatus::Canceled => { + anyhow::bail!("sync copy failed; mirror deletions were not applied"); + } + _ => tokio::time::sleep(std::time::Duration::from_millis(250)).await, + } + } + } + } for d in plan.deletes { let fs: &dyn RemoteFs = match plan.direction { SyncDirection::LocalToRemote => remote_fs.as_ref(), SyncDirection::RemoteToLocal => local_fs.as_ref(), }; - let _ = fs.delete(&d.path, false).await; // best-effort + fs.delete(&d.path, false).await?; } Ok(ids) diff --git a/src-tauri/src/diff.rs b/src-tauri/src/diff.rs index 8e2b6a5..dcf609d 100644 --- a/src-tauri/src/diff.rs +++ b/src-tauri/src/diff.rs @@ -387,7 +387,7 @@ async fn hash_ssh(ssh: &Arc, path: &str) -> Result { async fn hash_object(obj: &Arc, path: &str) -> Result { use futures::StreamExt; let key = path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let get = obj .store .get(&p) diff --git a/src-tauri/src/diskscan.rs b/src-tauri/src/diskscan.rs index de04e34..3e912d9 100644 --- a/src-tauri/src/diskscan.rs +++ b/src-tauri/src/diskscan.rs @@ -481,7 +481,27 @@ async fn scan_object( use object_store::path::Path as ObjPath; use object_store::ObjectStore; - let prefix_raw = root.trim().trim_matches('/'); + if obj.profile.protocol == "s3" { + let prefix = root.trim_matches('/'); + let prefix = if prefix.is_empty() { String::new() } else { format!("{prefix}/") }; + let listing = crate::session::object::s3_namespace::S3Namespace::new(&obj.profile)?.list(&prefix, false).await?; + let mut tree = scan::ScanTree::default(); + for o in listing.objects { + if info.cancel.is_cancelled() { anyhow::bail!("scan canceled"); } + if o.key.ends_with('/') { continue; } + let rel = o.key.strip_prefix(&prefix).context("S3 key outside scan prefix")?.to_string(); + tree.files.insert(rel, scan::ScanEntry { + absolute: format!("/{}", o.key), size: o.size, + modified: chrono::DateTime::parse_from_rfc3339(&o.last_modified)?.timestamp(), etag: o.etag, + }); + } + info.files.store(tree.files.len(), Ordering::Relaxed); + info.bytes.store(tree.files.values().map(|f| f.size).sum(), Ordering::Relaxed); + emit_progress(info, app); + return Ok(tree); + } + + let prefix_raw = root.trim_matches('/'); let prefix = if prefix_raw.is_empty() || prefix_raw == "." { String::new() } else { @@ -490,7 +510,7 @@ async fn scan_object( let prefix_path = if prefix.is_empty() { None } else { - Some(ObjPath::from(prefix.as_str())) + Some(ObjPath::parse(prefix.as_str())?) }; let mut stream = obj.store.list(prefix_path.as_ref()); diff --git a/src-tauri/src/editor.rs b/src-tauri/src/editor.rs index e3dae08..db05420 100644 --- a/src-tauri/src/editor.rs +++ b/src-tauri/src/editor.rs @@ -355,7 +355,7 @@ async fn download_to( Session::Object(obj) => { use futures::StreamExt; let key = remote_path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let get = obj .store .get(&p) @@ -576,7 +576,7 @@ pub(crate) async fn upload_from( } Session::Object(obj) => { let key = remote_path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let mut file = tokio::fs::File::open(&local).await?; let mut buf = Vec::with_capacity(size as usize); file.read_to_end(&mut buf).await?; diff --git a/src-tauri/src/foldersync.rs b/src-tauri/src/foldersync.rs index fb2d983..e963613 100644 --- a/src-tauri/src/foldersync.rs +++ b/src-tauri/src/foldersync.rs @@ -527,7 +527,7 @@ async fn reconcile_inner( let warning = apply_safety(&mut plan, pair, source_available); - if plan.copies.is_empty() && plan.deletes.is_empty() { + if plan.copies.is_empty() && plan.deletes.is_empty() && plan.directories.is_empty() { // Nothing to transfer — but still snapshot the source so a first run // seeds the index (later same-size edits become detectable) and a delete // prunes its row. @@ -700,6 +700,11 @@ fn apply_safety(plan: &mut SyncPlan, pair: &SyncPair, source_available: bool) -> if !pair.exclude.is_empty() { plan.copies.retain(|c| !is_excluded(&c.relative, &pair.exclude)); plan.deletes.retain(|d| !is_excluded(&d.relative, &pair.exclude)); + let destination_root = match pair.direction { + SyncDirection::LocalToRemote => &plan.remote_root, + SyncDirection::RemoteToLocal => &plan.local_root, + }; + plan.directories.retain(|d| !is_excluded(&crate::scan::relative_of(destination_root, d), &pair.exclude)); plan.total_bytes = plan.copies.iter().map(|c| c.size).sum(); } diff --git a/src-tauri/src/preview.rs b/src-tauri/src/preview.rs index 3b7eb95..d46c751 100644 --- a/src-tauri/src/preview.rs +++ b/src-tauri/src/preview.rs @@ -241,7 +241,7 @@ pub(crate) async fn read_head(session: Option<&Session>, path: &str, max: u64) - } Some(Session::Object(obj)) => { let key = path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let get = obj.store.get(&p).await.with_context(|| format!("get {key}"))?; collect_capped(get.into_stream().map(|r| r.map_err(anyhow::Error::from)), max).await } diff --git a/src-tauri/src/remotefs/local.rs b/src-tauri/src/remotefs/local.rs index d82f23d..8df92df 100644 --- a/src-tauri/src/remotefs/local.rs +++ b/src-tauri/src/remotefs/local.rs @@ -14,10 +14,8 @@ impl RemoteFs for LocalFs { .with_context(|| format!("read_dir {}", p.display()))?; let mut out = Vec::new(); while let Some(entry) = rd.next_entry().await? { - let meta = match entry.metadata().await { - Ok(m) => m, - Err(_) => continue, - }; + let meta = entry.metadata().await + .with_context(|| format!("stat {}", entry.path().display()))?; let kind = if meta.is_dir() { FileKind::Directory } else if meta.file_type().is_symlink() { diff --git a/src-tauri/src/remotefs/object.rs b/src-tauri/src/remotefs/object.rs index 9d8d7d6..ecb54a6 100644 --- a/src-tauri/src/remotefs/object.rs +++ b/src-tauri/src/remotefs/object.rs @@ -8,8 +8,8 @@ use object_store::ObjectStore; use std::sync::Arc; /// RemoteFs implementation for any object_store-backed session (S3, R2, B2, -/// Azure Blob, …). Semantics are uniform: no real directories, no chmod, -/// rename = copy + delete (object_store handles that for us). +/// Azure Blob, …). S3 directories use exact trailing-slash marker keys; +/// rename is copy then delete. Other stores retain their implicit prefixes. pub struct ObjectFs { session: Arc, } @@ -20,8 +20,11 @@ impl ObjectFs { } } +#[cfg(test)] +mod tests; + fn normalize_prefix(raw: &str) -> String { - let trimmed = raw.trim().trim_matches('/'); + let trimmed = raw.trim_matches('/'); if trimmed.is_empty() || trimmed == "." { String::new() } else { @@ -65,10 +68,37 @@ fn entry_for_prefix(prefix: &str) -> DirEntry { impl RemoteFs for ObjectFs { async fn list_dir(&self, path: &str) -> Result> { let prefix = normalize_prefix(path); + if self.session.profile.protocol == "s3" { + let api = crate::session::object::s3_namespace::S3Namespace::new(&self.session.profile)?; + let prefix = if prefix.is_empty() { prefix } else { format!("{prefix}/") }; + let listing = api.list(&prefix, true).await?; + if !prefix.is_empty() && listing.objects.is_empty() && listing.prefixes.is_empty() { + return Err(std::io::Error::new(std::io::ErrorKind::NotFound, format!("S3 prefix {prefix} does not exist")).into()); + } + let mut entries = Vec::new(); + for p in listing.prefixes { + let mut entry = entry_for_prefix(&p.prefix); + // Keep the slash to distinguish a directory from a same-named file. + entry.path.push('/'); + entries.push(entry); + } + for o in listing.objects { + if o.key == prefix { continue; } + if o.key.ends_with('/') { + let mut entry = entry_for_prefix(&o.key); + entry.path.push('/'); + entries.push(entry); + } else { + entries.push(entry_for_object(&o.key, o.size, + Some(chrono::DateTime::parse_from_rfc3339(&o.last_modified)?.timestamp()), o.etag)); + } + } + return Ok(entries); + } let prefix_path = if prefix.is_empty() { None } else { - Some(ObjPath::from(prefix.as_str())) + Some(ObjPath::parse(prefix.as_str())?) }; let listing = self @@ -84,6 +114,13 @@ impl RemoteFs for ObjectFs { out.push(entry_for_prefix(cp.as_ref())); } for obj in listing.objects { + // ListObjects(prefix="dir/", delimiter="/") can include the + // directory marker "dir/" itself. object_store::Path removes its + // trailing slash, so it arrives as "dir". It is not a child file: + // queuing it would HEAD "dir" and fail (issue #33). + if prefix_path.as_ref() == Some(&obj.location) { + continue; + } let modified = obj.last_modified.timestamp(); out.push(entry_for_object( obj.location.as_ref(), @@ -96,8 +133,33 @@ impl RemoteFs for ObjectFs { } async fn rename(&self, from: &str, to: &str) -> Result<()> { - let src = ObjPath::from(normalize_prefix(from)); - let dst = ObjPath::from(normalize_prefix(to)); + if self.session.profile.protocol == "s3" { + let api = crate::session::object::s3_namespace::S3Namespace::new(&self.session.profile)?; + let src = normalize_prefix(from); + let dst = normalize_prefix(to); + if src.is_empty() || dst.is_empty() { anyhow::bail!("cannot rename the bucket root"); } + if src == dst { return Ok(()); } + if !from.ends_with('/') && api.exists(&src).await? { + api.copy(&src, &dst).await?; + return api.delete(&src).await; + } + let src = format!("{src}/"); + let dst = format!("{dst}/"); + if dst.starts_with(&src) || src.starts_with(&dst) { anyhow::bail!("directory rename paths overlap"); } + let listing = api.list(&src, false).await?; + if listing.objects.is_empty() { anyhow::bail!("source directory does not exist: {src}"); } + if !api.list(&dst, false).await?.objects.is_empty() { anyhow::bail!("destination directory already exists: {dst}"); } + // Finish all copies before deleting any source key. A failed copy + // leaves the originals available for recovery. + for o in &listing.objects { + let relative = o.key.strip_prefix(&src).context("S3 returned a key outside the requested prefix")?; + api.copy(&o.key, &format!("{dst}{relative}")).await?; + } + for o in listing.objects { api.delete(&o.key).await?; } + return Ok(()); + } + let src = ObjPath::parse(normalize_prefix(from))?; + let dst = ObjPath::parse(normalize_prefix(to))?; self.session .store .rename(&src, &dst) @@ -108,8 +170,28 @@ impl RemoteFs for ObjectFs { async fn delete(&self, path: &str, recursive: bool) -> Result<()> { let key = normalize_prefix(path); - let target = ObjPath::from(key.as_str()); - let is_object = self.session.store.head(&target).await.is_ok(); + if self.session.profile.protocol == "s3" { + let api = crate::session::object::s3_namespace::S3Namespace::new(&self.session.profile)?; + if !key.is_empty() && !path.ends_with('/') && api.exists(&key).await? { + return api.delete(&key).await; + } + if !recursive { anyhow::bail!("{path} is a directory or missing; use recursive for directories"); } + let prefix = if key.is_empty() { key } else { format!("{key}/") }; + let listing = api.list(&prefix, false).await?; + // Complete the listing before deleting: list errors cannot silently + // produce an incomplete successful deletion. + for o in listing.objects { + if !o.key.starts_with(&prefix) { anyhow::bail!("S3 returned a key outside the requested prefix"); } + api.delete(&o.key).await?; + } + return Ok(()); + } + let target = ObjPath::parse(key.as_str())?; + let is_object = match self.session.store.head(&target).await { + Ok(_) => true, + Err(object_store::Error::NotFound { .. }) => false, + Err(e) => return Err(e.into()), + }; if is_object { self.session @@ -136,7 +218,14 @@ impl RemoteFs for ObjectFs { Ok(()) } - async fn create_dir(&self, _path: &str) -> Result<()> { + async fn create_dir(&self, path: &str) -> Result<()> { + if self.session.profile.protocol == "s3" { + let key = normalize_prefix(path); + if !key.is_empty() { + return crate::session::object::s3_namespace::S3Namespace::new(&self.session.profile)? + .mkdir(&format!("{key}/")).await; + } + } // Object stores have no folders. Accepted as a no-op so the UI's // mkdir flow doesn't error; the prefix appears once the first // object lands in it. @@ -152,7 +241,7 @@ impl RemoteFs for ObjectFs { can_chmod: false, can_symlink: false, can_rename: true, - has_directories: false, + has_directories: self.session.profile.protocol == "s3", has_shell: false, has_commands: false, // Object stores expose an ETag per object — an opaque change token diff --git a/src-tauri/src/remotefs/object/tests.rs b/src-tauri/src/remotefs/object/tests.rs new file mode 100644 index 0000000..1613732 --- /dev/null +++ b/src-tauri/src/remotefs/object/tests.rs @@ -0,0 +1,95 @@ +use super::*; +use crate::profiles::ConnectionProfile; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; + +// Exercise S3 HTTP listings: an in-memory ObjectStore cannot represent +// trailing-slash marker keys because object_store::Path strips that slash. +async fn listing(path: &str, contents: &str) -> Vec { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let endpoint = format!("http://{}", listener.local_addr().unwrap()); + let xml = format!("{contents}"); + let server = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut request = Vec::new(); + let mut buf = [0; 1024]; + while !request.windows(4).any(|w| w == b"\r\n\r\n") { + let n = socket.read(&mut buf).await.unwrap(); + assert!(n > 0); + request.extend_from_slice(&buf[..n]); + } + assert!(request.starts_with(b"GET /faro-test?")); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/xml\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{xml}", + xml.len() + ); + socket.write_all(response.as_bytes()).await.unwrap(); + }); + let profile: ConnectionProfile = serde_json::from_value(serde_json::json!({ + "id": "s3-test", "name": "S3 test", "protocol": "s3", + "host": "127.0.0.1", "port": 0, "username": "test", + "auth": {"kind": "password", "password": "test"}, + "bucket": "faro-test", "region": "us-east-1", "endpoint": endpoint + })) + .unwrap(); + let session = crate::session::object::object_connect(&profile) + .await + .unwrap(); + let entries = tokio::time::timeout( + std::time::Duration::from_secs(5), + ObjectFs::new(Arc::new(session)).list_dir(path), + ) + .await + .unwrap() + .unwrap(); + server.await.unwrap(); + entries +} + +fn object(key: &str, size: usize) -> String { + format!("{key}{size}2026-10-08T00:00:00Ztest") +} + +#[tokio::test] +async fn s3_folder_marker_is_not_a_downloadable_file() { + let xml = format!( + "{}{}photos/nested/", + object("photos/", 0), + object("photos/empty.txt", 0) + ); + let entries = listing("/photos/", &xml).await; + assert_eq!( + entries.len(), + 2, + "the directory's own marker must be excluded: {entries:?}" + ); + assert!(entries + .iter() + .any(|e| e.path == "/photos/nested/" && e.kind == FileKind::Directory)); + assert!(entries + .iter() + .any(|e| e.path == "/photos/empty.txt" && e.kind == FileKind::File && e.size == 0)); +} + +#[tokio::test] +async fn s3_empty_marker_directory_has_no_children() { + assert!(listing("/photos/empty", &object("photos/empty/", 0)) + .await + .is_empty()); +} + +#[tokio::test] +async fn s3_implicit_prefix_and_same_named_file_are_both_preserved() { + let xml = format!( + "{}photos/", + object("photos", 5) + ); + let entries = listing("/", &xml).await; + assert_eq!(entries.len(), 2); + assert!(entries + .iter() + .any(|e| e.kind == FileKind::File && e.size == 5)); + assert!(entries.iter().any(|e| e.kind == FileKind::Directory)); + let entries = listing("/photos", &object("photos/image.jpg", 12)).await; + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].path, "/photos/image.jpg"); +} diff --git a/src-tauri/src/scan.rs b/src-tauri/src/scan.rs index a35d785..3414b34 100644 --- a/src-tauri/src/scan.rs +++ b/src-tauri/src/scan.rs @@ -13,11 +13,11 @@ //! fallback for all of them; the strategy hook is the [`ScanOptions`] the caller //! passes. -use std::collections::{BTreeMap, VecDeque}; +use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use anyhow::Result; +use anyhow::{Context, Result}; use futures::stream::{FuturesUnordered, StreamExt}; use crate::remotefs::{FileKind, RemoteFs}; @@ -66,10 +66,11 @@ pub struct ScanEntry { } /// A flattened file tree keyed by relative path (POSIX `/`) from the scan root. -/// Directories are implicit — only files are tracked, exactly as sync needs. +/// Explicit directories retain empty folders when a sync plan is executed. #[derive(Debug, Default, Clone)] pub struct ScanTree { pub files: BTreeMap, + pub directories: BTreeSet, } /// Knobs for a walk. `Default` is what the sync poller wants: full concurrency, @@ -91,10 +92,20 @@ pub async fn walk_tree(fs: &dyn RemoteFs, root: &str) -> Result { walk(fs, root, &ScanOptions::default(), |_| {}).await } +/// Only a missing destination root may be treated as empty. An unreadable +/// source or any failed child listing must prevent planning a destructive sync. +pub async fn destination_tree(fs: &dyn RemoteFs, root: &str) -> Result { + match fs.list_dir(root).await { + Err(e) if e.downcast_ref::().is_some_and(|e| e.kind() == std::io::ErrorKind::NotFound) => Ok(ScanTree::default()), + Err(e) => Err(e), + Ok(_) => walk_tree(fs, root).await, + } +} + /// Recursively list every file under `root`. Bounded-concurrency BFS: up to /// `opts.concurrency` `list_dir` calls are in flight at once. Unreadable -/// directories are tolerated (skipped), symlinks are not followed — both -/// preserving the behaviour of the walk this replaced. `on_progress` fires once +/// directories abort the scan; callers must never mistake partial results for +/// a complete source tree. Symlinks are not followed. `on_progress` fires once /// per completed directory listing. pub async fn walk( fs: &dyn RemoteFs, @@ -114,24 +125,26 @@ pub async fn walk( loop { if opts.cancel.is_cancelled() { - break; + anyhow::bail!("directory scan canceled"); } // Keep the pipeline full: top up in-flight listings from the queue. while inflight.len() < limit { match queue.pop_front() { Some(dir) => inflight.push(async move { - let res = fs.list_dir(&dir).await; - res + let res = fs.list_dir(&dir).await.with_context(|| format!("list directory {dir}")); + (dir, res) }), None => break, } } // Empty queue *and* nothing in flight ⇒ the tree is fully walked. - let Some(res) = inflight.next().await else { + let Some((dir, res)) = inflight.next().await else { break; }; dirs_scanned += 1; - if let Ok(entries) = res { + { + let entries = res?; + tree.directories.insert(relative_of(&normalized_root, dir.trim_end_matches('/'))); for entry in entries { match entry.kind { FileKind::Directory => queue.push_back(entry.path.clone()), diff --git a/src-tauri/src/search.rs b/src-tauri/src/search.rs index badd21c..5f8a8cc 100644 --- a/src-tauri/src/search.rs +++ b/src-tauri/src/search.rs @@ -559,6 +559,7 @@ async fn name_walk( sink: &mut HitSink<'_>, ) -> usize { let matcher = compiled.name.as_ref().expect("name search compiles a name matcher"); + let normalized_root = root.trim_end_matches('/').to_string(); let limit = scan::DEFAULT_CONCURRENCY; @@ -749,13 +750,39 @@ async fn name_object( cancel: &CancelToken, ) -> Result> { let matcher = compiled.name.as_ref().expect("name search compiles a name matcher"); - let prefix_raw = root.trim().trim_matches('/'); + if obj.profile.protocol == "s3" { + let prefix = root.trim_matches('/'); + let prefix = if prefix.is_empty() { String::new() } else { format!("{prefix}/") }; + let listing = crate::session::object::s3_namespace::S3Namespace::new(&obj.profile)?.list(&prefix, false).await?; + let mut entries = std::collections::BTreeMap::new(); + for o in listing.objects { + let rel = o.key.strip_prefix(&prefix).context("S3 key outside search prefix")?; + if rel.is_empty() { continue; } + let dir = rel.ends_with('/'); + let rel = rel.trim_end_matches('/'); + entries.insert((rel.to_string(), dir), o.size); + for (i, _) in rel.match_indices('/') { + entries.entry((rel[..i].to_string(), true)).or_insert(0); + } + } + let mut hits = Vec::new(); + for ((rel, dir), size) in entries { + if cancel.is_cancelled() || hits.len() >= query.max_results { break; } + let name = basename(&rel); + if matcher.matches(name) && (dir || compiled.filters.passes(name)) { + let suffix = if dir { "/" } else { "" }; + hits.push(SearchHit::name(format!("/{prefix}{rel}{suffix}"), rel, dir, size)); + } + } + return Ok(hits); + } + let prefix_raw = root.trim_matches('/'); let prefix = if prefix_raw.is_empty() || prefix_raw == "." { String::new() } else { prefix_raw.to_string() }; - let prefix_path = (!prefix.is_empty()).then(|| object_store::path::Path::from(prefix.as_str())); + let prefix_path = if prefix.is_empty() { None } else { Some(object_store::path::Path::parse(prefix.as_str())?) }; let mut stream = obj.store.list(prefix_path.as_ref()); let mut hits = Vec::new(); @@ -1072,7 +1099,7 @@ async fn read_ssh(ssh: &SshSession, path: &str, max: u64) -> Result> { async fn read_object(obj: &ObjectSession, path: &str, max: u64) -> Result> { let key = path.trim_start_matches('/'); - let p = object_store::path::Path::from(key); + let p = object_store::path::Path::parse(key)?; let get = obj.store.get(&p).await.with_context(|| format!("get {key}"))?; let mut stream = get.into_stream(); let mut buf = Vec::new(); diff --git a/src-tauri/src/session/object.rs b/src-tauri/src/session/object.rs index 5419be7..878b55b 100644 --- a/src-tauri/src/session/object.rs +++ b/src-tauri/src/session/object.rs @@ -8,6 +8,8 @@ use std::sync::Arc; use std::time::Duration; use uuid::Uuid; +pub mod s3_namespace; + /// A blob/object-store session. The same struct backs every cloud object /// store (S3, R2, B2, Azure Blob) — only the builder differs, and the /// resulting Arc hides the implementation from everything diff --git a/src-tauri/src/session/object/s3_namespace.rs b/src-tauri/src/session/object/s3_namespace.rs new file mode 100644 index 0000000..176dda7 --- /dev/null +++ b/src-tauri/src/session/object/s3_namespace.rs @@ -0,0 +1,239 @@ +//! Exact-key S3 namespace operations. object_store::Path intentionally strips +//! trailing slashes, so it cannot represent folder markers for PUT/COPY/DELETE. +//! File streaming stays in object_store; signing uses its public SigV4 signer. +use crate::profiles::{AuthMethod, ConnectionProfile}; +use anyhow::{bail, Context, Result}; +use object_store::aws::{AwsAuthorizer, AwsCredential}; +use reqwest::{Client, Method, StatusCode}; +use serde::Deserialize; +use std::time::Duration; +use url::Url; + +pub struct S3Namespace { + client: Client, + bucket_url: Url, + bucket: String, + region: String, + credentials: AwsCredential, +} + +#[derive(Default, Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct Listing { + #[serde(default, rename = "Contents")] + pub objects: Vec, + #[serde(default, rename = "CommonPrefixes")] + pub prefixes: Vec, + #[serde(default)] + next_continuation_token: Option, + #[serde(default)] + is_truncated: bool, + #[serde(default)] + encoding_type: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct Object { + pub key: String, + pub size: u64, + pub last_modified: String, + #[serde(rename = "ETag")] + pub etag: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct Prefix { + pub prefix: String, +} + +impl S3Namespace { + pub fn new(profile: &ConnectionProfile) -> Result { + let bucket = profile.bucket.clone().context("missing S3 bucket")?; + let region = profile.region.clone().unwrap_or_else(|| "us-east-1".into()); + let endpoint = profile + .endpoint + .as_deref() + .filter(|s| !s.is_empty()) + .map(str::to_owned) + .unwrap_or_else(|| { + let suffix = if region.starts_with("cn-") { + "amazonaws.com.cn" + } else { + "amazonaws.com" + }; + format!("https://s3.{region}.{suffix}") + }); + let mut bucket_url = Url::parse(&endpoint).context("invalid S3 endpoint")?; + bucket_url + .path_segments_mut() + .map_err(|_| anyhow::anyhow!("invalid S3 endpoint"))? + .pop_if_empty() + .push(&bucket); + let AuthMethod::Password { password } = &profile.auth else { + bail!("S3 requires password authentication") + }; + Ok(Self { + client: Client::builder() + .connect_timeout(Duration::from_secs(10)) + .timeout(Duration::from_secs(60)) + .redirect(reqwest::redirect::Policy::none()) + .build()?, + bucket_url, + bucket, + region, + credentials: AwsCredential { + key_id: profile.username.clone(), + secret_key: password.clone(), + token: None, + }, + }) + } + + fn object_url(&self, key: &str) -> Result { + // Validate without using the normalized return value: the trailing slash + // is significant. Reject dot segments rather than letting Url erase them. + if !key.is_empty() { + object_store::path::Path::parse(key)?; + } + let mut url = self.bucket_url.clone(); + if !key.is_empty() { + url.path_segments_mut().unwrap().extend(key.split('/')); + } + Ok(url) + } + + async fn request( + &self, + method: Method, + url: Url, + copy_source: Option<&str>, + ) -> Result { + let mut builder = self.client.request(method.clone(), url); + if method == Method::PUT { + builder = builder.body(Vec::new()); + } + if let Some(source) = copy_source { + let encoded: String = url::form_urlencoded::byte_serialize( + format!("/{}/{source}", self.bucket).as_bytes(), + ) + .collect::() + .replace('+', "%20"); + builder = builder.header("x-amz-copy-source", encoded); + } + let mut request = builder.build()?; + AwsAuthorizer::new(&self.credentials, "s3", &self.region).authorize(&mut request, None); + let response = self + .client + .execute(request) + .await + .context("S3 namespace request")?; + if response.status() == StatusCode::NOT_FOUND { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "S3 object or bucket not found", + ) + .into()); + } + Ok(response + .error_for_status() + .context("S3 namespace response")?) + } + + pub async fn list(&self, prefix: &str, delimiter: bool) -> Result { + let mut listing = Listing::default(); + let mut token: Option = None; + loop { + let mut url = self.bucket_url.clone(); + { + let mut query = url.query_pairs_mut(); + query + .append_pair("list-type", "2") + .append_pair("encoding-type", "url") + .append_pair("prefix", prefix); + if delimiter { + query.append_pair("delimiter", "/"); + } + if let Some(t) = &token { + query.append_pair("continuation-token", t); + } + } + let xml = self.request(Method::GET, url, None).await?.text().await?; + let mut page: Listing = quick_xml::de::from_str(&xml).context("decode S3 listing")?; + if page.encoding_type.as_deref() == Some("url") { + for object in &mut page.objects { + object.key = percent_encoding::percent_decode_str(&object.key) + .decode_utf8()? + .into_owned(); + } + for prefix in &mut page.prefixes { + prefix.prefix = percent_encoding::percent_decode_str(&prefix.prefix) + .decode_utf8()? + .into_owned(); + } + } + listing.objects.extend(page.objects); + listing.prefixes.extend(page.prefixes); + if !page.is_truncated { + break; + } + let next = page + .next_continuation_token + .context("truncated S3 listing has no continuation token")?; + if token.as_ref() == Some(&next) { + bail!("S3 listing repeated a continuation token"); + } + token = Some(next); + } + Ok(listing) + } + + pub async fn exists(&self, key: &str) -> Result { + match self + .request(Method::HEAD, self.object_url(key)?, None) + .await + { + Ok(_) => Ok(true), + Err(e) + if e.downcast_ref::() + .is_some_and(|e| e.kind() == std::io::ErrorKind::NotFound) => + { + Ok(false) + } + Err(e) => Err(e), + } + } + + pub async fn mkdir(&self, key: &str) -> Result<()> { + self.request(Method::PUT, self.object_url(key)?, None) + .await?; + Ok(()) + } + + pub async fn delete(&self, key: &str) -> Result<()> { + self.request(Method::DELETE, self.object_url(key)?, None) + .await?; + Ok(()) + } + + pub async fn copy(&self, from: &str, to: &str) -> Result<()> { + let xml = self + .request(Method::PUT, self.object_url(to)?, Some(from)) + .await? + .text() + .await?; + // S3 CopyObject may return HTTP 200 with an embedded error. + #[derive(Deserialize)] + struct CopySuccess { + #[serde(rename = "ETag")] + etag: String, + } + let result: CopySuccess = + quick_xml::de::from_str(&xml).context("S3 copy did not return a successful result")?; + if result.etag.is_empty() { + bail!("S3 copy returned an empty ETag"); + } + Ok(()) + } +} diff --git a/src-tauri/src/sync.rs b/src-tauri/src/sync.rs index dc46dc5..d390208 100644 --- a/src-tauri/src/sync.rs +++ b/src-tauri/src/sync.rs @@ -57,6 +57,8 @@ pub struct SyncPlan { pub local_root: String, pub remote_root: String, pub copies: Vec, + #[serde(default)] + pub directories: Vec, pub deletes: Vec, pub total_bytes: u64, } @@ -117,12 +119,20 @@ pub(crate) async fn plan_indexed( index: Option<&std::collections::HashMap>, source_signal: crate::remotefs::ChangeSignal, ) -> Result<(SyncPlan, crate::scan::ScanTree)> { - let local_tree = crate::scan::walk_tree(local_fs, local_root).await?; - let remote_tree = crate::scan::walk_tree(remote_fs, remote_root).await?; + let (local_tree, remote_tree) = match direction { + SyncDirection::LocalToRemote => ( + crate::scan::walk_tree(local_fs, local_root).await?, + crate::scan::destination_tree(remote_fs, remote_root).await?, + ), + SyncDirection::RemoteToLocal => ( + crate::scan::destination_tree(local_fs, local_root).await?, + crate::scan::walk_tree(remote_fs, remote_root).await?, + ), + }; // Borrow both trees only for the diff, then move the source tree out to hand // back to the caller (moving while `source`/`dest` still borrow would fail). - let (copies, deletes, total_bytes) = { + let (copies, deletes, directories, total_bytes) = { let (source, dest, dest_root) = match direction { SyncDirection::LocalToRemote => (&local_tree, &remote_tree, remote_root), SyncDirection::RemoteToLocal => (&remote_tree, &local_tree, local_root), @@ -169,7 +179,14 @@ pub(crate) async fn plan_indexed( } } } - (copies, deletes, total_bytes) + // A real filesystem cannot hold both file "a" and directory "a/". + // Refuse before copying or deleting when an object tree has a collision. + if source.directories.iter().any(|d| source.files.contains_key(d)) { + anyhow::bail!("source contains a file and directory with the same name"); + } + let directories = source.directories.difference(&dest.directories) + .map(|rel| if rel.is_empty() { dest_root.to_string() } else { join(dest_root, rel) }).collect(); + (copies, deletes, directories, total_bytes) }; let source_tree = match direction { @@ -184,6 +201,7 @@ pub(crate) async fn plan_indexed( local_root: local_root.to_string(), remote_root: remote_root.to_string(), copies, + directories, deletes, total_bytes, }, @@ -253,6 +271,30 @@ mod index_tests { .unwrap() } + #[tokio::test] + async fn missing_source_never_becomes_a_mirror_delete_plan() { + let dst = scratch("missing-source"); + std::fs::write(dst.join("keep.txt"), b"keep").unwrap(); + let absent = dst.join("not-present"); + let result = plan(&LocalFs, &LocalFs, absent.to_str().unwrap(), dst.to_str().unwrap(), + SyncDirection::LocalToRemote, SyncStrategy::Mirror).await; + assert!(result.is_err()); + assert_eq!(std::fs::read(dst.join("keep.txt")).unwrap(), b"keep"); + std::fs::remove_dir_all(dst).unwrap(); + } + + #[tokio::test] + async fn missing_destination_and_empty_directories_are_planned() { + let src = scratch("empty-directories"); + std::fs::create_dir_all(src.join("empty/nested")).unwrap(); + let dst = src.with_extension("destination"); + let planned = plan_stateless(&src, &dst).await; + assert_eq!(planned.directories.len(), 3, "root plus both empty folders"); + assert!(planned.copies.is_empty()); + assert!(!dst.exists(), "planning must not create the destination"); + std::fs::remove_dir_all(src).unwrap(); + } + async fn plan_with( src: &Path, dst: &Path, diff --git a/src-tauri/src/transfer/live_tests.rs b/src-tauri/src/transfer/live_tests.rs index 78e24b7..6d2e1ae 100644 --- a/src-tauri/src/transfer/live_tests.rs +++ b/src-tauri/src/transfer/live_tests.rs @@ -451,6 +451,71 @@ async fn live_s3_parallel_round_trip() { object_round_trip("FARO_LIVE_S3").await; } +/// Run against scripts/s3-lab.py. Uses the same RemoteFs listing as the +/// directory queue and the real transfer runner for every discovered file. +#[tokio::test] +#[ignore = "requires scripts/s3-lab.py and FARO_LIVE_S3"] +async fn live_s3_directory_markers() { + let session = object_session("FARO_LIVE_S3").await.expect("set FARO_LIVE_S3"); + let fs = fs_for_session(&session); + let expected = std::collections::BTreeMap::from([ + ("/issue-33/marked/hello.txt", "hello from marked folder\n"), + ("/issue-33/marked/nested/data.txt", "nested payload\n"), + ("/issue-33/implicit/hello.txt", "hello from implicit folder\n"), + ("/issue-33/empty.txt", ""), + ("/issue-33/spaces & symbols/hello world.txt", "spaced filename\n"), + ]); + let dir = scratch("s3-directories"); + let mgr = Arc::new(TransferManager::new()); + let mut dirs = vec!["/issue-33".to_string()]; + let mut seen = std::collections::BTreeSet::new(); + let mut files = std::collections::BTreeSet::new(); + while let Some(path) = dirs.pop() { + assert!(seen.insert(path.clone()), "directory cycle: {path}"); + assert!(seen.len() <= 10, "unexpected directory tree"); + let local_dir = dir.join(path.trim_start_matches('/')); + std::fs::create_dir_all(&local_dir).unwrap(); + for entry in fs.list_dir(&path).await.unwrap() { + if entry.kind == crate::remotefs::FileKind::Directory { + dirs.push(entry.path); + } else { + assert!(expected.contains_key(entry.path.as_str()), "unexpected file: {entry:?}"); + assert!(files.insert(entry.path.clone()), "duplicate file"); + let (t, _, _) = tokio::time::timeout( + Duration::from_secs(30), download(&mgr, &session, &entry.path, &local_dir), + ).await.expect("download timed out"); + assert_eq!(t.status, TransferStatus::Done, "{}: {:?}", entry.path, t.error); + assert_eq!(std::fs::read(local_dir.join(&entry.name)).unwrap(), expected[entry.path.as_str()].as_bytes()); + } + } + } + assert_eq!(files.len(), expected.len()); + assert!(dir.join("issue-33/marked/empty").is_dir(), "empty marker folder preserved"); + // The fix must not turn genuine missing-file errors into successful copies. + let error = remote_size(&session, "/issue-33/missing.txt").await.unwrap_err(); + assert!(error.to_string().contains("object head")); + println!("5 files copied with exact bytes; empty folder preserved; missing file still errors"); + std::fs::remove_dir_all(&dir).unwrap(); +} + +/// Regression for exact Unicode keys, through metadata and the real downloader. +#[tokio::test] +#[ignore = "requires scripts/s3-lab.py and FARO_LIVE_S3"] +async fn live_s3_unicode_key() { + let session = object_session("FARO_LIVE_S3").await.expect("set FARO_LIVE_S3"); + let Session::Object(obj) = &*session else { unreachable!() }; + let key = object_store::path::Path::parse("key-encoding/café.txt").unwrap(); + let bytes = obj.store.get(&key).await.unwrap().bytes().await.unwrap(); + assert_eq!(bytes.as_ref(), b"coffee\n", "the original S3 key is readable"); + assert_eq!(remote_size(&session, "/key-encoding/café.txt").await.unwrap(), 7); + let dir = scratch("s3-unicode"); + let mgr = Arc::new(TransferManager::new()); + let (t, _, _) = download(&mgr, &session, "/key-encoding/café.txt", &dir).await; + assert_eq!(t.status, TransferStatus::Done, "{:?}", t.error); + assert_eq!(std::fs::read(dir.join("café.txt")).unwrap(), b"coffee\n"); + std::fs::remove_dir_all(dir).unwrap(); +} + #[tokio::test] #[ignore] async fn live_azure_parallel_round_trip() { diff --git a/src-tauri/src/transfer/mod.rs b/src-tauri/src/transfer/mod.rs index 0abcfd1..01e7d1c 100644 --- a/src-tauri/src/transfer/mod.rs +++ b/src-tauri/src/transfer/mod.rs @@ -511,7 +511,7 @@ fn join_remote(dir: &str, name: &str) -> String { } fn basename(path: &str) -> String { - path.rsplit(|c| c == '/' || c == '\\') + path.trim_end_matches(['/', '\\']).rsplit(|c| c == '/' || c == '\\') .next() .unwrap_or(path) .to_string() @@ -1069,7 +1069,7 @@ impl TransferManager { v } - async fn get(&self, id: &str) -> Option { + pub(crate) async fn get(&self, id: &str) -> Option { self.transfers.lock().await.get(id).cloned() } @@ -2401,7 +2401,7 @@ impl TransferManager { self.update(id, |t| t.status = TransferStatus::Transferring) .await; let key = remote_path.trim_start_matches('/').to_string(); - let p = object_store::path::Path::from(key.as_str()); + let p = object_store::path::Path::parse(key.as_str())?; let local = local_identity(local_path).await?; let size = local.size; let mut file = tokio::fs::File::open(local_path) @@ -3512,7 +3512,7 @@ pub(crate) async fn remote_size(session: &Arc, path: &str) -> Result { let key = path.trim_start_matches('/').to_string(); - let p = object_store::path::Path::from(key.as_str()); + let p = object_store::path::Path::parse(key.as_str())?; let meta = obj .store .head(&p) @@ -3585,24 +3585,32 @@ async fn remote_resolve( } Session::Object(obj) => { let key = initial_remote.trim_start_matches('/').to_string(); - let probe = object_store::path::Path::from(key.as_str()); - let exists = obj.store.head(&probe).await.is_ok(); + let probe = object_store::path::Path::parse(key.as_str())?; + if matches!(policy, OverwritePolicy::Overwrite) { + return Ok((initial_remote.to_string(), false)); + } + let exists = match obj.store.head(&probe).await { + Ok(_) => true, + Err(object_store::Error::NotFound { .. }) => false, + Err(e) => return Err(e.into()), + }; Ok(match policy { OverwritePolicy::Overwrite => (initial_remote.to_string(), false), OverwritePolicy::Skip => (initial_remote.to_string(), exists), OverwritePolicy::Rename if !exists => (initial_remote.to_string(), false), OverwritePolicy::Rename => { - let mut candidate = initial_remote.to_string(); for i in 1..=999 { let (stem, ext) = split_ext(initial_remote); - candidate = format!("{stem}_{i}{ext}"); + let candidate = format!("{stem}_{i}{ext}"); let key = candidate.trim_start_matches('/'); - let p = object_store::path::Path::from(key); - if obj.store.head(&p).await.is_err() { - break; + let p = object_store::path::Path::parse(key)?; + match obj.store.head(&p).await { + Err(object_store::Error::NotFound { .. }) => return Ok((candidate, false)), + Err(e) => return Err(e.into()), + Ok(_) => {}, } } - (candidate, false) + anyhow::bail!("no unused object name found after 999 attempts") } }) } @@ -4716,7 +4724,7 @@ mod tests { async fn put_object(store: &object_store::memory::InMemory, key: &str, data: &[u8]) { use object_store::ObjectStore; store - .put(&object_store::path::Path::from(key), bytes::Bytes::copy_from_slice(data).into()) + .put(&object_store::path::Path::parse(key).unwrap(), bytes::Bytes::copy_from_slice(data).into()) .await .unwrap(); } diff --git a/src-tauri/src/transfer/sources.rs b/src-tauri/src/transfer/sources.rs index 2c657b7..897030a 100644 --- a/src-tauri/src/transfer/sources.rs +++ b/src-tauri/src/transfer/sources.rs @@ -90,7 +90,7 @@ pub async fn remote_identity(session: &Arc, path: &str) -> Result Arc::new(SftpSource::new(ssh.clone(), path_s, ident.size)), Session::Object(obj) => Arc::new(ObjectSource { session: obj.clone(), - key: object_store::path::Path::from(path.trim_start_matches('/')), + key: object_store::path::Path::parse(path.trim_start_matches('/'))?, etag: ident.etag.clone(), }), Session::Webdav(dav) => { diff --git a/src/components/SyncDialog.tsx b/src/components/SyncDialog.tsx index 8aeebf7..8cff8c3 100644 --- a/src/components/SyncDialog.tsx +++ b/src/components/SyncDialog.tsx @@ -90,7 +90,7 @@ export function SyncDialog({ localPath, remotePath, onClose }: Props) { }; const totalOps = - (plan?.copies.length ?? 0) + (plan?.deletes.length ?? 0); + (plan?.copies.length ?? 0) + (plan?.deletes.length ?? 0) + (plan?.directories?.length ?? 0); return (
@@ -312,12 +312,19 @@ function PlanSummary({ plan }: { plan: SyncPlan }) { const both = plan.copies.length > 0 && plan.deletes.length > 0; return (
- {plan.copies.length === 0 && plan.deletes.length === 0 ? ( + {plan.copies.length === 0 && plan.deletes.length === 0 && !plan.directories?.length ? (
Everything is already in sync.
) : (
+ {!!plan.directories?.length && ( + }> + {plan.directories.slice(0, 200).map((directory) => ( + {directory} + ))} + + )} {plan.copies.length > 0 && (