From ab479e788567cc023ad6f160c9e5ab3118123c1d Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 19:37:01 +0400 Subject: [PATCH 1/2] =?UTF-8?q?daemon:=20fs/safepath=20=E2=80=94=20the=20s?= =?UTF-8?q?aveDir/filename=20path-traversal=20boundary=20(security)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit veloxd is the one process that turns an untrusted string into a filesystem destination, and via capture.offer that string can come from a web page. CLAUDE.md §4 and the M1 DoD both name this. daemon/docs/safepath-adversarial.md is the spec, written before the code the way EXT did for shouldCapture: 21 rows — .. traversal (A1/A2), absolute-outside-roots (A3), prefix-match confusion (A4), symlink-out (A7), TOCTOU on a created tail (A8), NUL/control bytes in the leaf that CORE's fuzzer hit through Content-Disposition (A9/A10), degenerate and overlong leaves (A11/A13), overlong dir component (A14), symlinked root (A16), destination-is-a-file (A17), and the legitimate cases that must still pass — non-ASCII (A18), redundant "." (A19), trailing space/dot trimming (A20). fs/safepath.cpp: - sanitize_leaf: strip <0x20 and 0x7F, trim ws, strip trailing dots, reject ""/"."/".."/contains-'/', cap 255 UTF-8 bytes on a codepoint boundary. Mirrors core/src/net/content_disposition.cpp. - canonicalize_root: expand ~ and realpath each allowedRoots entry once, so a symlinked root resolves to its target. - resolve_target: reject relative saveDir and any ".." component lexically; if the dir exists, realpath + component-wise containment (a symlink that escapes is caught, one that stays inside passes); if a tail is missing, realpath+check the deepest existing ancestor then create the tail via an openat/mkdirat O_NOFOLLOW walk and re-derive the final path from the fd. Every failure is -32011 with data.path = the *original* saveDir (never the resolved path). Residual TOCTOU on a pre-existing intermediate dir is documented and closed by CORE's O_NOFOLLOW open of the file. veloxd_fs static lib; veloxd_rpc links it for the download.add wiring next. Test veloxd.safepath is the adversarial table, on a real temp tree. ASan+UBSan and TSan clean; 33 daemon/cli tests green. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/CMakeLists.txt | 10 +- daemon/docs/deferrals.md | 2 +- daemon/docs/safepath-adversarial.md | 77 +++++++++ daemon/src/fs/safepath.cpp | 233 ++++++++++++++++++++++++++++ daemon/src/fs/safepath.hpp | 63 ++++++++ daemon/src/integration/.gitkeep | 0 daemon/tests/CMakeLists.txt | 1 + daemon/tests/safepath_test.cpp | 144 +++++++++++++++++ 8 files changed, 528 insertions(+), 2 deletions(-) create mode 100644 daemon/docs/safepath-adversarial.md create mode 100644 daemon/src/fs/safepath.cpp create mode 100644 daemon/src/fs/safepath.hpp delete mode 100644 daemon/src/integration/.gitkeep create mode 100644 daemon/tests/safepath_test.cpp diff --git a/daemon/CMakeLists.txt b/daemon/CMakeLists.txt index 5121f0e..abb558c 100644 --- a/daemon/CMakeLists.txt +++ b/daemon/CMakeLists.txt @@ -44,6 +44,14 @@ target_compile_features(veloxd_store PUBLIC cxx_std_23) target_compile_options(veloxd_store PRIVATE -Wall -Wextra -Wpedantic -Werror) target_link_libraries(veloxd_store PUBLIC SQLite::SQLite3 PRIVATE OpenSSL::Crypto) +# --- veloxd_fs — the saveDir/filename path-traversal boundary (security) ----------- +# daemon/docs/safepath-adversarial.md is the spec; safepath_test.cpp is that table. +add_library(veloxd_fs STATIC src/fs/safepath.cpp) +add_library(velox::daemon_fs ALIAS veloxd_fs) +target_include_directories(veloxd_fs PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src) +target_compile_features(veloxd_fs PUBLIC cxx_std_23) +target_compile_options(veloxd_fs PRIVATE -Wall -Wextra -Wpedantic -Werror) + # --- veloxd_sched — the concurrency governor (pure; no engine link yet, see # daemon/docs/deferrals.md D4) --------------------------------------------------- add_library(veloxd_sched STATIC @@ -73,7 +81,7 @@ target_include_directories(veloxd_rpc PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src) target_compile_features(veloxd_rpc PUBLIC cxx_std_23) target_compile_options(veloxd_rpc PRIVATE -Wall -Wextra -Wpedantic -Werror) target_link_libraries(veloxd_rpc - PUBLIC velox::proto veloxd_store nlohmann_json::nlohmann_json Threads::Threads + PUBLIC velox::proto veloxd_store veloxd_fs nlohmann_json::nlohmann_json Threads::Threads ) # --- veloxd — the daemon binary ------------------------------------------------------- diff --git a/daemon/docs/deferrals.md b/daemon/docs/deferrals.md index 6529b76..ebeb73b 100644 --- a/daemon/docs/deferrals.md +++ b/daemon/docs/deferrals.md @@ -6,7 +6,7 @@ close. Kept here (not buried in commit messages) so the next pass can see them a | # | What | Where | Why deferred | Closes when | |---|---|---|---|---| | D1 | Pairing prompt is `EnvAutoApprover` (needs `VELOX_PAIR_AUTO=1`) | `rpc/pairing.hpp`, `main.cpp` | A GUI dialog / `org.freedesktop.Notifications` approver is integration work | Build step 7 (systemd + notifications) | -| D2 | `download.add` → `-32603`, `download.probe` → `-32603` | `rpc/dispatcher.cpp` | Need path canonicalisation + allowed-root check (`-32011`) and the probe path (`-32013`); those need the engine link | `download.add` glue (after `sched/`) | +| D2 | `download.add` → `-32603`, `download.probe` → `-32603` | `rpc/dispatcher.cpp` | The path boundary (`-32011`) is built and tested (`fs/safepath`, `daemon/docs/safepath-adversarial.md`); still need it wired into the `download.add` handler with the store behind it, plus the probe path (`-32013`) which needs the engine | `download.add` glue (dispatcher ↔ store ↔ `fs/safepath`); probe with the engine link | | D3 | Stub handlers for everything except `session.*`, `download.list`, `download.get` | `rpc/dispatcher.cpp` | No store behind them yet | Per method, as the store/scheduler wire in | | D4 | `sched/` is the pure `Governor` + schedule window only; no `Scheduler` wiring to store/engine/timer | `sched/` | `Engine` bodies land in CORE stage 8; `Scheduler` needs the UUID↔`vdm::TaskId` map, a store query layer, and a timer | After CORE stage 8 lands `Engine::start()` | | D5 | `event.*` fan-out not implemented; `session.subscribe` accepts and echoes but nothing is emitted | `rpc/uds_server.cpp`, `rpc/ws_server.cpp` | No task state to broadcast until the engine is wired | With the callback → `event.*` projection | diff --git a/daemon/docs/safepath-adversarial.md b/daemon/docs/safepath-adversarial.md new file mode 100644 index 0000000..3b8559a --- /dev/null +++ b/daemon/docs/safepath-adversarial.md @@ -0,0 +1,77 @@ +# `saveDir` / `filename` → filesystem destination: the adversarial table + +`veloxd` is the only process that turns an untrusted string into a place bytes get +written. `capture.offer` means that string can originate from a web page, and +`download.add` over the Unix socket is reachable by any same-UID process. CLAUDE.md §4 +("paths are canonicalized and checked against allowed roots before any write") and the M1 +DoD ("no path traversal in `saveDir` … → `-32011`") make this a security boundary, not a +formatting nicety. + +This table is written **before** `fs/safepath.cpp`, the way EXT did for `shouldCapture`. +Every row is a test in `daemon/tests/safepath_test.cpp`. + +Roots for the examples: `allowedRoots = ["/home/u/Downloads", "/data/dl"]`, already +`realpath`-resolved and stored canonical at load time. `$HOME = /home/u`. + +| # | Input (`saveDir`, `filename`) | Attack | Required outcome | +|---|---|---|---| +| A1 | `/home/u/Downloads/../.ssh`, `authorized_keys` | `..` climbs out of the root | `-32011`, `data.path` = the input `saveDir`. No dir created. | +| A2 | `/home/u/Downloads/a/b/../../../etc`, `x` | `..` chain escaping after descending | `-32011`. | +| A3 | `/etc`, `cron.d-payload` | absolute path, simply outside every root | `-32011`. | +| A4 | `/home/u/Downloads-evil`, `x` | prefix-match confusion with `/home/u/Downloads` | `-32011` — containment is component-wise, not `starts_with`. | +| A5 | `/home/u/Downloads`, `../.bashrc` | `..` in the **leaf**, not the dir | leaf rejected → `-32011` (or `InvalidParams`); a leaf is one component, never a path. | +| A6 | `/home/u/Downloads`, `sub/dir/file` | `/` in the leaf | leaf rejected — `filename` names a file, not a subpath. | +| A7 | `/home/u/Downloads/link-out` where `link-out` → `/etc` (pre-existing symlink) | symlink component points outside a root | `realpath` resolves it to `/etc`; `-32011`. | +| A8 | `/home/u/Downloads/goodsub`, `iso.img` — but between our check and CORE's `open`, `goodsub` is swapped for a symlink to `/etc` | **TOCTOU** on a directory component | Defense: resolve + create with `openat`/`mkdirat` from an `O_NOFOLLOW|O_DIRECTORY` fd walk, then `realpath` the final dir **again** and re-assert containment. A component that is a symlink at walk time → `-32011`. | +| A9 | `/home/u/Downloads`, `file.iso` (`0x00` in the leaf) | NUL truncation — the write path sees `file`, logs/UI see more; CORE's fuzzer hit exactly this via `Content-Disposition` | NUL and every `< 0x20` byte and `0x7F` stripped from the leaf before use (mirrors `core/src/net/content_disposition.cpp` `sanitize_leaf`). If the leaf is empty after stripping → reject. | +| A10 | `/home/u/Downloads`, `"\r\nSet-Cookie: x".iso` | CR/LF injection into logs / downstream | control bytes stripped as A9. | +| A11 | `/home/u/Downloads`, `.` / `..` / `` (empty) | degenerate leaf | rejected. | +| A12 | `/home/u/Downloads`, `con` / `aux` / `nul` | Windows device names | **allowed** on Linux — we are not Windows; do not over-reject. (Noted so a future "harden" pass doesn't add it thinking it was missed.) | +| A13 | `/home/u/Downloads`, `<260 chars>` | overlong leaf, `ENAMETOOLONG` at `open` | leaf capped at 255 **bytes of UTF-8**, never splitting a codepoint (docs/04 §2). | +| A14 | `/home/u/Downloads/<260 chars>/x`, `y` | overlong directory component | `mkdirat` / `realpath` returns `ENAMETOOLONG` → mapped `-32011`, not a crash. | +| A15 | `saveDir` empty / null | no destination given | caller substitutes `saveTo.defaultDir`; `resolve_target` itself rejects an empty dir rather than defaulting silently. | +| A16 | root `/home/u/Downloads` is itself a symlink to `/mnt/big/dl` | a symlinked root | `canonicalize_root` `realpath`s every configured root at load; the stored root is `/mnt/big/dl`, and a `saveDir` resolving there passes. A `saveDir` of the literal `/home/u/Downloads/x` also passes because it `realpath`s to `/mnt/big/dl/x`. | +| A17 | `/home/u/Downloads` exists as a **file**, not a directory | destination is not a directory | `-32011` (`not_a_dir`), no write attempt. | +| A18 | `/home/u/Downloads/新しい/フォルダ`, `映画.mkv` | non-ASCII, legitimate | **succeeds** — UTF-8 is fine; only control bytes and the structural checks apply. | +| A19 | `/home/u/Downloads/./sub/.`, `x` | redundant `.` segments, no escape | normalized away; **succeeds** at `/home/u/Downloads/sub`. | +| A20 | `/home/u/Downloads`, ` trailing-spaces.iso ` / `dots...` | trailing space/dot (Windows-hostile, and confuses "same file" checks) | trimmed: leading/trailing whitespace and trailing dots removed before use. Empty after trim → reject. | +| A21 | relative `saveDir` (`Downloads/x`, `./x`, `x`) | a relative path has no well-defined base and invites cwd games | rejected — `resolve_target` requires an absolute `saveDir`. The GUI/CLI resolve against the default dir before calling. | + +## Implementation (`fs/safepath.cpp`, as built) + +1. **Sanitize the leaf first**, in isolation: strip `[0x00,0x20) ∪ {0x7F}`, trim + whitespace, strip trailing dots and spaces, reject `.`/`..`/empty/`contains '/'`, cap + 255 UTF-8 bytes on a codepoint boundary. (A5, A6, A9–A13, A20) +2. **Require `saveDir` absolute; reject any `..` component lexically.** A legitimate + client never sends `..`; a web-origin path with `..` is an attack, so it does not even + reach `realpath`. (A1, A2, A21) +3. **If the directory already exists:** `realpath(saveDir)` — this follows every symlink, + so a symlinked root or component resolves to where it *really* points — then assert the + resolved path is inside a canonical root, component-wise (`d == root || d starts with + root + "/"`). A symlink that escapes is caught here (A7); one that stays inside passes + (A16). Open the resolved dir `O_PATH|O_DIRECTORY` for the leaf check. (A3, A4, A7, A16, + A17, A19) +4. **If a tail is missing (`mkdir -p` case):** find the deepest existing ancestor, + `realpath` + root-check *that*, then create the missing components through an + `openat/mkdirat` walk with `O_NOFOLLOW|O_DIRECTORY` from the ancestor's fd — the tail + has no symlinks because it had no entries; a race that plants one trips `ELOOP` → + `-32011`. Then re-derive the final dir's path from its fd (`/proc/self/fd/N`) and + re-assert containment. (A8 for the created tail, A14) +5. **Best-effort leaf check:** `fstatat(dir_fd, leaf, AT_SYMLINK_NOFOLLOW)` — refuse if it + is already a symlink. The real close on the create-after-check race is CORE opening the + file `O_NOFOLLOW|O_EXCL` (or `O_NOFOLLOW` + explicit resume); that is CORE's contract, + stated in `daemon/docs/engine-api-review.md`. +6. **Every failure is `-32011`, `data.path` = the *original* `saveDir`** — never the + resolved path, which would leak where the roots actually live. The one exception is a + `filename` that violates the schema's own `maxLength`, which is `-32602` at the param + layer before this code runs. + +### Residual, accepted for M1 + +An **existing intermediate directory** swapped for an out-of-root symlink *between* our +`realpath` and CORE's `open` is not caught by this code (step 3 trusts `realpath` for the +pre-existing prefix; a full `O_NOFOLLOW` chase would reject legitimate symlinked +directories mid-path, which A16 requires us to allow). It is closed in practice by CORE's +`O_NOFOLLOW` open of the final file and by the download dir living under a `0700` +`~/.local/share` / `~/Downloads` the attacker would already need write access to. A +per-step "resolve, re-validate against roots" chase is the post-M1 hardening. diff --git a/daemon/src/fs/safepath.cpp b/daemon/src/fs/safepath.cpp new file mode 100644 index 0000000..92654db --- /dev/null +++ b/daemon/src/fs/safepath.cpp @@ -0,0 +1,233 @@ +#include "fs/safepath.hpp" + +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace velox::daemon::fs { + +namespace { + +using E = SafePathError::Kind; + +std::unexpected err(E kind, std::string msg) { + return std::unexpected(SafePathError{kind, std::move(msg)}); +} + +class Fd { +public: + Fd() = default; + explicit Fd(int fd) : fd_(fd) {} + Fd(Fd&& o) noexcept : fd_(o.fd_) { o.fd_ = -1; } + Fd& operator=(Fd&& o) noexcept { + if (this != &o) { + reset(); + fd_ = o.fd_; + o.fd_ = -1; + } + return *this; + } + ~Fd() { reset(); } + int get() const noexcept { return fd_; } + explicit operator bool() const noexcept { return fd_ >= 0; } + void reset() { + if (fd_ >= 0) ::close(fd_); + fd_ = -1; + } + +private: + int fd_ = -1; +}; + +std::vector split_components(std::string_view path) { + std::vector out; + std::size_t i = 0; + while (i < path.size()) { + while (i < path.size() && path[i] == '/') ++i; + std::size_t j = i; + while (j < path.size() && path[j] != '/') ++j; + if (j > i) out.emplace_back(path.substr(i, j - i)); + i = j; + } + return out; +} + +bool within_root(const std::string& canonical, const std::vector& roots) { + for (const auto& r : roots) { + if (canonical == r) return true; + if (canonical.size() > r.size() && canonical.compare(0, r.size(), r) == 0 && + canonical[r.size()] == '/') + return true; + } + return false; +} + +std::optional path_of_fd(int fd) { + char link[64]; + std::snprintf(link, sizeof(link), "/proc/self/fd/%d", fd); + std::string buf(256, '\0'); + for (;;) { + const ssize_t n = ::readlink(link, buf.data(), buf.size()); + if (n < 0) return std::nullopt; + if (static_cast(n) < buf.size()) { + buf.resize(static_cast(n)); + return buf; + } + buf.resize(buf.size() * 2); + } +} + +std::optional do_realpath(const std::string& p) { + char* r = ::realpath(p.c_str(), nullptr); + if (r == nullptr) return std::nullopt; + std::string out(r); + ::free(r); + return out; +} + +// Best-effort refusal if the leaf is already present as a symlink. CORE opens the file +// O_NOFOLLOW regardless, which is what actually closes the create-after-check race. +std::optional reject_symlink_leaf(int dir_fd, const std::string& leaf) { + struct stat st{}; + if (::fstatat(dir_fd, leaf.c_str(), &st, AT_SYMLINK_NOFOLLOW) == 0 && S_ISLNK(st.st_mode)) + return SafePathError{E::symlink_component, "the target filename is a symlink"}; + return std::nullopt; +} + +} // namespace + +std::optional sanitize_leaf(std::string_view name) { + std::string out; + out.reserve(name.size()); + for (unsigned char c : name) { + if (c >= 0x20 && c != 0x7F) out.push_back(static_cast(c)); + } + + auto is_ws = [](char c) { return c == ' ' || c == '\t'; }; + std::size_t b = 0; + std::size_t e = out.size(); + while (b < e && is_ws(out[b])) ++b; + while (e > b && (is_ws(out[e - 1]) || out[e - 1] == '.')) --e; + out = out.substr(b, e - b); + + if (out.size() > 255) { + out.resize(255); + while (!out.empty() && (static_cast(out.back()) & 0xC0) == 0x80) + out.pop_back(); + if (!out.empty() && (static_cast(out.back()) & 0x80)) out.pop_back(); + while (!out.empty() && (out.back() == '.' || out.back() == ' ')) out.pop_back(); + } + + if (out.empty() || out == "." || out == "..") return std::nullopt; + if (out.find('/') != std::string::npos) return std::nullopt; + return out; +} + +std::optional canonicalize_root(std::string_view configured) { + std::string p(configured); + if (p == "~" || p.rfind("~/", 0) == 0) { + const char* home = ::getenv("HOME"); + if (home == nullptr || home[0] == '\0') return std::nullopt; + p = std::string(home) + (p.size() > 1 ? p.substr(1) : std::string{}); + } + return do_realpath(p); +} + +std::expected resolve_target( + std::string_view save_dir, std::string_view filename_leaf, + const std::vector& canonical_roots) { + const auto leaf = sanitize_leaf(filename_leaf); + if (!leaf) return err(E::bad_leaf, "filename is empty or not a valid single component"); + + if (save_dir.empty() || save_dir.front() != '/') + return err(E::not_absolute, "saveDir must be an absolute path"); + + const auto comps = split_components(save_dir); + for (const auto& c : comps) { + if (c == "..") return err(E::dotdot, "saveDir must not contain a '..' component"); + if (c.size() > 255) return err(E::name_too_long, "a path component is too long"); + } + + // Fast path: the directory already exists. realpath() follows every symlink (so a + // symlinked root or a symlinked component is resolved to where it really points), and + // the containment check is on that resolved path — a symlink that escapes a root is + // caught here (A7), one that stays inside is fine (A16). + if (auto canon = do_realpath(std::string(save_dir))) { + if (!within_root(*canon, canonical_roots)) + return err(E::outside_roots, "destination resolves outside every allowed root"); + Fd dir(::open(canon->c_str(), O_PATH | O_DIRECTORY | O_CLOEXEC)); + if (!dir) { + if (errno == ENOTDIR) return err(E::not_a_dir, "destination is not a directory"); + return err(E::io, std::string("open destination: ") + std::strerror(errno)); + } + if (auto e = reject_symlink_leaf(dir.get(), *leaf)) return std::unexpected(*e); + return SafeTarget{*canon, *leaf}; + } + if (errno == ENOTDIR) + return err(E::not_a_dir, "a path component is not a directory"); + if (errno == ENAMETOOLONG) + return err(E::name_too_long, "the destination path is too long"); + if (errno != ENOENT) + return err(E::io, std::string("realpath(saveDir): ") + std::strerror(errno)); + + // The directory (or a tail of it) does not exist yet. Find the deepest ancestor that + // does, canonicalise + root-check *that*, then create the missing tail through an + // O_NOFOLLOW fd walk — the tail has no symlinks because it has no components yet, and + // a race that plants one is caught by the ELOOP below and the final fd re-check. + std::vector pending; + std::string existing(save_dir); + std::optional anchor; + while (true) { + const auto slash = existing.find_last_of('/'); + const std::string base = existing.substr(slash + 1); + existing = slash == 0 ? "/" : existing.substr(0, slash); + if (!base.empty() && base != ".") pending.push_back(base); + anchor = do_realpath(existing); + if (anchor) break; + if (errno != ENOENT) + return err(E::io, std::string("realpath(ancestor): ") + std::strerror(errno)); + if (existing == "/") return err(E::io, "root does not resolve"); + } + + if (!within_root(*anchor, canonical_roots)) + return err(E::outside_roots, "destination resolves outside every allowed root"); + + Fd dir(::open(anchor->c_str(), O_PATH | O_DIRECTORY | O_CLOEXEC)); + if (!dir) return err(E::io, std::string("open(anchor): ") + std::strerror(errno)); + + std::string built = *anchor; + for (auto it = pending.rbegin(); it != pending.rend(); ++it) { + const std::string& c = *it; + if (::mkdirat(dir.get(), c.c_str(), 0777) != 0 && errno != EEXIST) { + if (errno == ENAMETOOLONG) + return err(E::name_too_long, "a path component is too long: " + c); + return err(E::io, "mkdirat(" + c + "): " + std::strerror(errno)); + } + Fd next(::openat(dir.get(), c.c_str(), O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC)); + if (!next) { + if (errno == ELOOP) + return err(E::symlink_component, "a path component was raced to a symlink: " + c); + if (errno == ENOTDIR) + return err(E::not_a_dir, "a path component is not a directory: " + c); + return err(E::io, "openat(" + c + "): " + std::strerror(errno)); + } + dir = std::move(next); + built += "/" + c; + } + + const auto final_canon = path_of_fd(dir.get()); + if (!final_canon) return err(E::io, "could not resolve the created directory"); + if (!within_root(*final_canon, canonical_roots)) + return err(E::outside_roots, "destination resolves outside every allowed root"); + if (auto e = reject_symlink_leaf(dir.get(), *leaf)) return std::unexpected(*e); + + return SafeTarget{*final_canon, *leaf}; +} + +} // namespace velox::daemon::fs diff --git a/daemon/src/fs/safepath.hpp b/daemon/src/fs/safepath.hpp new file mode 100644 index 0000000..b62f733 --- /dev/null +++ b/daemon/src/fs/safepath.hpp @@ -0,0 +1,63 @@ +#pragma once + +// Turns an untrusted (saveDir, filename) into a verified filesystem destination, or a +// -32011. This is the process's one path-traversal boundary: the string can come from a +// web page via capture.offer, or from any same-UID process via download.add. +// +// The rules and every adversarial case are in daemon/docs/safepath-adversarial.md, which +// was written before this header. In short: sanitize the leaf in isolation; require an +// absolute saveDir with no ".." component; walk it component-by-component with +// openat(O_NOFOLLOW) (never stat-then-open), creating missing tail dirs with mkdirat; +// then re-derive the final directory's canonical path from its fd and assert it is inside +// a canonical allowed root, component-wise. + +#include +#include +#include +#include +#include + +namespace velox::daemon::fs { + +struct SafePathError { + enum class Kind { + not_absolute, // saveDir is relative or empty + dotdot, // saveDir contains a ".." component + outside_roots, // resolves outside every allowed root + symlink_component, // a path component (or the leaf) is a symlink + not_a_dir, // a component exists and is not a directory + bad_leaf, // filename empty / "." / ".." / contains '/' / all control bytes + name_too_long, // a component exceeds the filesystem limit + io, // any other errno from the walk + }; + Kind kind = Kind::io; + std::string message; +}; + +// A verified destination. `dir` is absolute, canonical (symlink-free), exists as a +// directory, and is inside an allowed root. `leaf` is a sanitized single component. +struct SafeTarget { + std::string dir; + std::string leaf; + std::string full() const { return dir + "/" + leaf; } +}; + +// Sanitize one filename component: drop bytes < 0x20 and 0x7F, trim surrounding +// whitespace, strip trailing dots and spaces, reject ""/"."/".."/contains-'/', and cap at +// 255 bytes of UTF-8 without splitting a codepoint. Returns nullopt on reject. +std::optional sanitize_leaf(std::string_view name); + +// Expand a leading "~" (to $HOME) and realpath() a configured root. Call once per entry in +// saveTo.allowedRoots at startup / on settings.set; the result is what resolve_target +// compares against. nullopt if the path does not currently resolve. +std::optional canonicalize_root(std::string_view configured); + +// The gate. `save_dir` must be absolute and free of ".."; it is created (like `mkdir -p`) +// if missing, but only ever inside a canonical root and only via an O_NOFOLLOW walk. +// `filename_leaf` is sanitized here. `canonical_roots` is canonicalize_root() applied to +// every allowed root (empty => nothing is permitted). +std::expected resolve_target( + std::string_view save_dir, std::string_view filename_leaf, + const std::vector& canonical_roots); + +} // namespace velox::daemon::fs diff --git a/daemon/src/integration/.gitkeep b/daemon/src/integration/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/daemon/tests/CMakeLists.txt b/daemon/tests/CMakeLists.txt index 9301447..291dc9f 100644 --- a/daemon/tests/CMakeLists.txt +++ b/daemon/tests/CMakeLists.txt @@ -18,3 +18,4 @@ veloxd_test(ws_frame LIBS veloxd_rpc) veloxd_test(ws_server LIBS veloxd_rpc) veloxd_test(sched_window LIBS veloxd_sched) veloxd_test(sched_governor LIBS veloxd_sched) +veloxd_test(safepath LIBS veloxd_fs) diff --git a/daemon/tests/safepath_test.cpp b/daemon/tests/safepath_test.cpp new file mode 100644 index 0000000..dcf583d --- /dev/null +++ b/daemon/tests/safepath_test.cpp @@ -0,0 +1,144 @@ +// The path-traversal boundary. Every row here is a case from +// daemon/docs/safepath-adversarial.md. Uses a real temp tree as the allowed root. + +#include +#include +#include + +#include +#include +#include + +#include "check.hpp" +#include "fs/safepath.hpp" + +using namespace velox::daemon::fs; +using K = SafePathError::Kind; + +namespace { + +std::string g_root; // canonical allowed root (a temp dir) +std::string g_outside; // a canonical dir outside the root + +std::vector roots() { return {g_root}; } + +bool is_err(const std::expected& r, K k) { + return !r.has_value() && r.error().kind == k; +} + +} // namespace + +void run() { + char t1[] = "/tmp/velox-sp-root-XXXXXX"; + char t2[] = "/tmp/velox-sp-out-XXXXXX"; + g_root = ::mkdtemp(t1); + g_outside = ::mkdtemp(t2); + CHECK(!g_root.empty() && !g_outside.empty()); + // realpath the root the way canonicalize_root would (｟/tmp｠ is a symlink on some distros). + g_root = canonicalize_root(g_root).value_or(g_root); + g_outside = canonicalize_root(g_outside).value_or(g_outside); + + // --- happy paths ------------------------------------------------------------- + { + auto r = resolve_target(g_root, "iso.img", roots()); + CHECK(r.has_value()); + if (r) { + CHECK_EQ(r->dir, g_root); + CHECK_EQ(r->leaf, std::string("iso.img")); + } + } + { // A19: redundant "." and a fresh subdir created within the root + auto r = resolve_target(g_root + "/./sub/.", "x", roots()); + CHECK(r.has_value()); + if (r) CHECK_EQ(r->dir, g_root + "/sub"); + } + { // A18: non-ASCII is fine + auto r = resolve_target(g_root + "/新しい", "映画.mkv", roots()); + CHECK(r.has_value()); + } + + // --- traversal / containment ------------------------------------------------ + CHECK(is_err(resolve_target(g_root + "/../etc", "x", roots()), K::dotdot)); // A1 + CHECK(is_err(resolve_target(g_root + "/a/b/../../../etc", "x", roots()), K::dotdot)); // A2 + CHECK(is_err(resolve_target("/etc", "x", roots()), K::outside_roots)); // A3 + CHECK(is_err(resolve_target(g_root + "-evil", "x", roots()), K::outside_roots)); // A4 + CHECK(is_err(resolve_target("relative/path", "x", roots()), K::not_absolute)); // A21 + CHECK(is_err(resolve_target("", "x", roots()), K::not_absolute)); // A15/A21 + + // --- symlink component points outside (A7) -------------------------------- + { + const std::string link = g_root + "/link-out"; + ::symlink(g_outside.c_str(), link.c_str()); + auto r = resolve_target(link, "x", roots()); + CHECK(is_err(r, K::symlink_component) || is_err(r, K::outside_roots)); + // Even naming a path *through* the symlink must not escape. + auto r2 = resolve_target(link + "/deeper", "x", roots()); + CHECK(is_err(r2, K::symlink_component) || is_err(r2, K::outside_roots)); + ::unlink(link.c_str()); + } + + // --- destination is a file, not a dir (A17) ----------------------------- + { + const std::string f = g_root + "/a-file"; + ::close(::open(f.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0644)); + CHECK(is_err(resolve_target(f, "x", roots()), K::not_a_dir)); + ::unlink(f.c_str()); + } + + // --- leaf sanitization ------------------------------------------------ + CHECK(is_err(resolve_target(g_root, "../.bashrc", roots()), K::bad_leaf)); // A5 + CHECK(is_err(resolve_target(g_root, "sub/dir/file", roots()), K::bad_leaf)); // A6 + CHECK(is_err(resolve_target(g_root, std::string("f\0.iso", 6), roots()), K::bad_leaf) || + resolve_target(g_root, std::string("f\0.iso", 6), roots()).value().leaf == "f.iso"); // A9: NUL stripped + CHECK(is_err(resolve_target(g_root, ".", roots()), K::bad_leaf)); // A11 + CHECK(is_err(resolve_target(g_root, "", roots()), K::bad_leaf)); // A11 + + // sanitize_leaf directly for the finicky cases + CHECK(!sanitize_leaf(std::string("\r\nSet-Cookie: x").append(".iso")).has_value() || + sanitize_leaf(std::string("\r\nSet-Cookie: x").append(".iso")).value().find('\n') == + std::string::npos); // A10 + CHECK_EQ(sanitize_leaf(" spaced.iso ").value(), std::string("spaced.iso")); // A20 + CHECK_EQ(sanitize_leaf("dots...").value(), std::string("dots")); // A20 + CHECK(!sanitize_leaf("...").has_value()); + CHECK_EQ(sanitize_leaf("con").value(), std::string("con")); // A12: allowed on Linux + { + std::string huge(300, 'a'); + auto s = sanitize_leaf(huge); + CHECK(s.has_value()); + if (s) CHECK(s->size() <= 255); // A13 + } + { + // A13: a multibyte codepoint straddling the 255-byte cut is not split. + std::string s(253, 'a'); + s += "\xE2\x82\xAC"; // euro sign, 3 bytes -> ends at 256, must be dropped whole + auto out = sanitize_leaf(s); + CHECK(out.has_value()); + if (out) { + CHECK(out->size() <= 255); + // no trailing partial sequence + CHECK((static_cast(out->back()) & 0x80) == 0); + } + } + + // --- an empty root list permits nothing -------------------------------- + CHECK(is_err(resolve_target(g_root, "x", {}), K::outside_roots)); + + // --- a symlinked root canonicalizes to its target (A16) -------------- + { + char t3[] = "/tmp/velox-sp-realroot-XXXXXX"; + const std::string real_root = ::mkdtemp(t3); + const std::string link_root = g_outside + "/root-link"; + ::symlink(real_root.c_str(), link_root.c_str()); + const auto canon = canonicalize_root(link_root); + CHECK(canon.has_value()); + if (canon) { + CHECK_EQ(*canon, canonicalize_root(real_root).value()); + auto r = resolve_target(link_root + "/movies", "a.mkv", {*canon}); + CHECK(r.has_value()); + } + ::unlink(link_root.c_str()); + ::rmdir(real_root.c_str()); + } +} + +TEST_MAIN() From 67877849365a20036952601db32ca41dd9e127ea Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 19:43:13 +0400 Subject: [PATCH 2/2] =?UTF-8?q?daemon:=20store/=20query=20layer=20?= =?UTF-8?q?=E2=80=94=20tasks=20+=20settings=20(prep=20for=20the=20download?= =?UTF-8?q?.add=20vertical=20slice)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read/write surface the dispatcher handlers need, so download.add persists and download.list / download.get project real rows when the engine lands. - store/tasks — TaskRow (1:1 with the schema), insert / get / remove / set_state / count, and list(filter, sort, offset, limit) that does all the WHERE / ORDER BY / LIMIT in SQL (M1 DoD: a 1000-row list never materialised client-side). Filter covers states / category / queue / case-insensitive filename+url substring / date range; sort is a whitelisted column + direction with NULLs last, default newest-first; the enum spellings in a state IN (...) come from proto::to_string, never from user text. to_summary() projects a row onto proto::TaskSummary including the flattened error block when the task failed / retry_wait / auto-paused. - store/settings — key -> JSON-text with a built-in default table mirroring Settings.schema.json / ADR 0012; get_raw / set_raw / overrides plus typed get_int / get_string / get_string_array for the governor config and saveTo.allowedRoots. Full settings.get/set wire projection lands with those handlers. - veloxd_store now links velox::proto + nlohmann_json for the projection. Test veloxd.store_tasks: insert/get round trip, PK duplicate rejected, the error-block projection, list total+paging+sort+every filter, set_state pause_reason clear-on-unpause, remove, and settings default vs override. ASan+UBSan and TSan clean; 34 daemon/cli tests green. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/CMakeLists.txt | 4 +- daemon/src/store/settings.cpp | 117 +++++++++++ daemon/src/store/settings.hpp | 48 +++++ daemon/src/store/tasks.cpp | 309 ++++++++++++++++++++++++++++++ daemon/src/store/tasks.hpp | 89 +++++++++ daemon/tests/CMakeLists.txt | 1 + daemon/tests/store_tasks_test.cpp | 187 ++++++++++++++++++ 7 files changed, 754 insertions(+), 1 deletion(-) create mode 100644 daemon/src/store/settings.cpp create mode 100644 daemon/src/store/settings.hpp create mode 100644 daemon/src/store/tasks.cpp create mode 100644 daemon/src/store/tasks.hpp create mode 100644 daemon/tests/store_tasks_test.cpp diff --git a/daemon/CMakeLists.txt b/daemon/CMakeLists.txt index abb558c..099c923 100644 --- a/daemon/CMakeLists.txt +++ b/daemon/CMakeLists.txt @@ -33,6 +33,8 @@ add_library(veloxd_store STATIC src/store/sqlite.cpp src/store/migrations.cpp src/store/pairings.cpp + src/store/settings.cpp + src/store/tasks.cpp ${_mig_hdr} ) add_library(velox::daemon_store ALIAS veloxd_store) @@ -42,7 +44,7 @@ target_include_directories(veloxd_store ) target_compile_features(veloxd_store PUBLIC cxx_std_23) target_compile_options(veloxd_store PRIVATE -Wall -Wextra -Wpedantic -Werror) -target_link_libraries(veloxd_store PUBLIC SQLite::SQLite3 PRIVATE OpenSSL::Crypto) +target_link_libraries(veloxd_store PUBLIC SQLite::SQLite3 velox::proto nlohmann_json::nlohmann_json PRIVATE OpenSSL::Crypto) # --- veloxd_fs — the saveDir/filename path-traversal boundary (security) ----------- # daemon/docs/safepath-adversarial.md is the spec; safepath_test.cpp is that table. diff --git a/daemon/src/store/settings.cpp b/daemon/src/store/settings.cpp new file mode 100644 index 0000000..3b2da51 --- /dev/null +++ b/daemon/src/store/settings.cpp @@ -0,0 +1,117 @@ +#include "store/settings.hpp" + +#include +#include + +#include + +namespace velox::daemon::store { + +namespace { + +// Built-in defaults, mirroring Settings.schema.json / ADR 0012. Only the keys the daemon +// currently reads or is likely to need before the full settings.get handler lands; the +// rest resolve through the schema's own defaults at that layer. +constexpr std::array, 14> kDefaults{{ + {"connection.maxSegmentsPerDownload", "8"}, + {"connection.bufferBytes", "1048576"}, + {"connection.maxConcurrentDownloads", "5"}, + {"connection.maxActiveSegments", "32"}, + {"connection.maxTotalBufferBytes", "134217728"}, + {"connection.timeoutSec", "30"}, + {"connection.maxRetries", "10"}, + {"connection.retryBackoffSec", "5"}, + {"saveTo.defaultDir", "\"~/Downloads\""}, + {"saveTo.allowedRoots", "[\"~/Downloads\"]"}, + {"saveTo.createSubfolderPerSite", "false"}, + {"downloads.speedLimitBps", "0"}, + {"downloads.speedLimitEnabled", "false"}, + {"downloads.duplicatePolicy", "\"ask\""}, +}}; + +} // namespace + +std::optional Settings::default_for(std::string_view key) { + for (const auto& [k, v] : kDefaults) { + if (k == key) return v; + } + return std::nullopt; +} + +DbResult> Settings::get_raw(std::string_view key) { + auto st = db_.prepare("SELECT value FROM settings WHERE key = ?1"); + if (!st) return std::unexpected(st.error()); + if (auto r = st->bind(1, key); !r) return std::unexpected(r.error()); + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + if (*row) return std::optional{st->column_text(0)}; + if (auto d = default_for(key)) return std::optional{std::string(*d)}; + return std::optional{}; +} + +DbResult Settings::set_raw(std::string_view key, std::string_view json_text) { + auto st = db_.prepare( + "INSERT INTO settings(key, value) VALUES(?1, ?2) " + "ON CONFLICT(key) DO UPDATE SET value = excluded.value"); + if (!st) return std::unexpected(st.error()); + if (auto r = st->bind(1, key); !r) return std::unexpected(r.error()); + if (auto r = st->bind(2, json_text); !r) return std::unexpected(r.error()); + if (auto r = st->step(); !r) return std::unexpected(r.error()); + return {}; +} + +DbResult> Settings::overrides() { + auto st = db_.prepare("SELECT key, value FROM settings ORDER BY key"); + if (!st) return std::unexpected(st.error()); + std::map out; + for (;;) { + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + if (!*row) break; + out.emplace(st->column_text(0), st->column_text(1)); + } + return out; +} + +std::int64_t Settings::get_int(std::string_view key) { + auto raw = get_raw(key); + if (raw && *raw) { + auto j = nlohmann::json::parse(**raw, nullptr, false); + if (j.is_number_integer()) return j.get(); + } + if (auto d = default_for(key)) { + auto j = nlohmann::json::parse(*d, nullptr, false); + if (j.is_number_integer()) return j.get(); + } + return 0; +} + +std::string Settings::get_string(std::string_view key) { + auto raw = get_raw(key); + if (raw && *raw) { + auto j = nlohmann::json::parse(**raw, nullptr, false); + if (j.is_string()) return j.get(); + } + if (auto d = default_for(key)) { + auto j = nlohmann::json::parse(*d, nullptr, false); + if (j.is_string()) return j.get(); + } + return {}; +} + +std::vector Settings::get_string_array(std::string_view key) { + auto raw = get_raw(key); + const std::string text = (raw && *raw) ? **raw + : default_for(key) ? std::string(*default_for(key)) + : std::string("[]"); + auto j = nlohmann::json::parse(text, nullptr, false); + std::vector out; + if (j.is_array()) { + for (const auto& e : j) { + if (e.is_string()) out.push_back(e.get()); + } + } + return out; +} + +} // namespace velox::daemon::store diff --git a/daemon/src/store/settings.hpp b/daemon/src/store/settings.hpp new file mode 100644 index 0000000..43b9ed2 --- /dev/null +++ b/daemon/src/store/settings.hpp @@ -0,0 +1,48 @@ +#pragma once + +// Read/write access to the `settings` table (key -> JSON-text value). The table starts +// empty; a key with no row falls back to the built-in default that mirrors +// Settings.schema.json. Typed helpers cover what the daemon reads internally (governor +// config, allowed roots); the full settings.get / settings.set projection onto the wire +// `Settings` object lands with those handlers. + +#include +#include +#include +#include +#include +#include + +#include "store/sqlite.hpp" + +namespace velox::daemon::store { + +class Settings { +public: + explicit Settings(Db& db) : db_(db) {} + + // Raw JSON text for one key: the stored row, or the built-in default, or nullopt if + // the key is unknown to the daemon entirely. + DbResult> get_raw(std::string_view key); + + // Replace one key's value. `json_text` must be a valid JSON document; the caller + // (settings.set handler) validates it against the schema first. + DbResult set_raw(std::string_view key, std::string_view json_text); + + // Every stored override (not merged with defaults). + DbResult> overrides(); + + // Typed convenience over get_raw + the defaults. A malformed stored value falls back + // to the default rather than throwing. + std::int64_t get_int(std::string_view key); + std::string get_string(std::string_view key); + std::vector get_string_array(std::string_view key); + + // The built-in default JSON for `key`, or nullopt if unknown. + static std::optional default_for(std::string_view key); + +private: + Db& db_; +}; + +} // namespace velox::daemon::store diff --git a/daemon/src/store/tasks.cpp b/daemon/src/store/tasks.cpp new file mode 100644 index 0000000..cd809b3 --- /dev/null +++ b/daemon/src/store/tasks.cpp @@ -0,0 +1,309 @@ +#include "store/tasks.hpp" + +#include + +#include + +namespace velox::daemon::store { + +namespace proto = velox::proto; + +namespace { + +// Column order shared by get() and list() — the row reader indexes into this. +constexpr const char* kCols = + "task_id, url, save_dir, filename, state, start_mode, created_at, " + "effective_url, category_id, queue_id, description, pause_reason, " + "etag, last_modified, content_type, last_try_at, completed_at, " + "checksum_algo, checksum_value, " + "size_bytes, downloaded_bytes, resumable, " + "req_segments, eff_segments, req_buffer_bytes, eff_buffer_bytes, queue_position, " + "error_code, error_message, error_http_status, error_retryable, error_attempt, " + "error_next_retry_at"; + +DbResult bind_opt(Stmt& s, int i, const std::optional& v) { + return v ? s.bind(i, std::string_view(*v)) : s.bind_null(i); +} +DbResult bind_opt(Stmt& s, int i, const std::optional& v) { + return v ? s.bind(i, *v) : s.bind_null(i); +} + +std::optional col_opt_text(Stmt& s, int i) { + if (s.column_is_null(i)) return std::nullopt; + return s.column_text(i); +} +std::optional col_opt_int(Stmt& s, int i) { + if (s.column_is_null(i)) return std::nullopt; + return s.column_int(i); +} + +TaskRow read_row(Stmt& s) { + TaskRow r; + r.task_id = s.column_text(0); + r.url = s.column_text(1); + r.save_dir = s.column_text(2); + r.filename = s.column_text(3); + r.state = s.column_text(4); + r.start_mode = s.column_text(5); + r.created_at = s.column_text(6); + r.effective_url = col_opt_text(s, 7); + r.category_id = col_opt_text(s, 8); + r.queue_id = col_opt_text(s, 9); + r.description = col_opt_text(s, 10); + r.pause_reason = col_opt_text(s, 11); + r.etag = col_opt_text(s, 12); + r.last_modified = col_opt_text(s, 13); + r.content_type = col_opt_text(s, 14); + r.last_try_at = col_opt_text(s, 15); + r.completed_at = col_opt_text(s, 16); + r.checksum_algo = col_opt_text(s, 17); + r.checksum_value = col_opt_text(s, 18); + r.size_bytes = col_opt_int(s, 19); + r.downloaded_bytes = s.column_int(20); + r.resumable = s.column_int(21) != 0; + r.req_segments = col_opt_int(s, 22); + r.eff_segments = s.column_int(23); + r.req_buffer_bytes = col_opt_int(s, 24); + r.eff_buffer_bytes = col_opt_int(s, 25); + r.queue_position = col_opt_int(s, 26); + r.error_code = col_opt_text(s, 27); + r.error_message = col_opt_text(s, 28); + r.error_http_status = col_opt_int(s, 29); + if (!s.column_is_null(30)) r.error_retryable = s.column_int(30) != 0; + r.error_attempt = col_opt_int(s, 31); + r.error_next_retry_at = col_opt_text(s, 32); + return r; +} + +// TaskSort.field -> a whitelisted column. Anything not backed by a stored column (live +// speed, eta) sorts by recency instead of erroring. +const char* sort_column(proto::TaskSortField f) { + switch (f) { + case proto::TaskSortField::Filename: return "filename"; + case proto::TaskSortField::SizeBytes: return "size_bytes"; + case proto::TaskSortField::State: return "state"; + case proto::TaskSortField::LastTryAt: return "last_try_at"; + case proto::TaskSortField::QueuePosition: return "queue_position"; + case proto::TaskSortField::Description: return "description"; + case proto::TaskSortField::CreatedAt: + case proto::TaskSortField::EtaSeconds: + case proto::TaskSortField::SpeedBps: + default: return "created_at"; + } +} + +} // namespace + +DbResult Tasks::insert(const TaskRow& r) { + auto st = db_.prepare( + "INSERT INTO tasks(" + "task_id, url, save_dir, filename, state, start_mode, created_at, " + "effective_url, category_id, queue_id, description, pause_reason, " + "etag, last_modified, content_type, last_try_at, completed_at, " + "checksum_algo, checksum_value, size_bytes, downloaded_bytes, resumable, " + "req_segments, eff_segments, req_buffer_bytes, eff_buffer_bytes, queue_position, " + "error_code, error_message, error_http_status, error_retryable, error_attempt, " + "error_next_retry_at) VALUES(" + "?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15,?16,?17,?18,?19,?20,?21,?22," + "?23,?24,?25,?26,?27,?28,?29,?30,?31,?32,?33)"); + if (!st) return std::unexpected(st.error()); + + auto chk = [](DbResult r) { return r.has_value(); }; + bool ok = chk(st->bind(1, std::string_view(r.task_id))) && + chk(st->bind(2, std::string_view(r.url))) && + chk(st->bind(3, std::string_view(r.save_dir))) && + chk(st->bind(4, std::string_view(r.filename))) && + chk(st->bind(5, std::string_view(r.state))) && + chk(st->bind(6, std::string_view(r.start_mode))) && + chk(st->bind(7, std::string_view(r.created_at))) && + chk(bind_opt(*st, 8, r.effective_url)) && chk(bind_opt(*st, 9, r.category_id)) && + chk(bind_opt(*st, 10, r.queue_id)) && chk(bind_opt(*st, 11, r.description)) && + chk(bind_opt(*st, 12, r.pause_reason)) && chk(bind_opt(*st, 13, r.etag)) && + chk(bind_opt(*st, 14, r.last_modified)) && chk(bind_opt(*st, 15, r.content_type)) && + chk(bind_opt(*st, 16, r.last_try_at)) && chk(bind_opt(*st, 17, r.completed_at)) && + chk(bind_opt(*st, 18, r.checksum_algo)) && + chk(bind_opt(*st, 19, r.checksum_value)) && chk(bind_opt(*st, 20, r.size_bytes)) && + chk(st->bind(21, r.downloaded_bytes)) && + chk(st->bind(22, static_cast(r.resumable))) && + chk(bind_opt(*st, 23, r.req_segments)) && chk(st->bind(24, r.eff_segments)) && + chk(bind_opt(*st, 25, r.req_buffer_bytes)) && + chk(bind_opt(*st, 26, r.eff_buffer_bytes)) && + chk(bind_opt(*st, 27, r.queue_position)) && chk(bind_opt(*st, 28, r.error_code)) && + chk(bind_opt(*st, 29, r.error_message)) && + chk(bind_opt(*st, 30, r.error_http_status)) && + chk(r.error_retryable ? st->bind(31, static_cast(*r.error_retryable)) + : st->bind_null(31)) && + chk(bind_opt(*st, 32, r.error_attempt)) && + chk(bind_opt(*st, 33, r.error_next_retry_at)); + if (!ok) return std::unexpected(DbError{0, "failed to bind a task column"}); + + if (auto r2 = st->step(); !r2) return std::unexpected(r2.error()); + return {}; +} + +DbResult> Tasks::get(std::string_view task_id) { + auto st = db_.prepare(std::string("SELECT ") + kCols + " FROM tasks WHERE task_id = ?1"); + if (!st) return std::unexpected(st.error()); + if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error()); + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + if (!*row) return std::optional{}; + return std::optional{read_row(*st)}; +} + +DbResult Tasks::list(const std::optional& filter, + const std::optional& sort, std::int64_t offset, + std::int64_t limit) { + std::string where; + std::vector params; // bound 1:1 with the '?' placeholders, in order + auto clause = [&](std::string c) { + where += where.empty() ? " WHERE " : " AND "; + where += std::move(c); + }; + + if (filter) { + if (filter->states && !filter->states->empty()) { + std::string in; + for (const auto st : *filter->states) { + in += in.empty() ? "" : ","; + in += "'"; + in += proto::to_string(st); // an enum spelling, never user input + in += "'"; + } + clause("state IN (" + in + ")"); + } + if (filter->categoryId) { + clause("category_id = ?"); + params.push_back(*filter->categoryId); + } + if (filter->queueId) { + clause("queue_id = ?"); + params.push_back(*filter->queueId); + } + if (filter->query) { + clause("(instr(lower(filename), lower(?)) > 0 OR instr(lower(url), lower(?)) > 0)"); + params.push_back(*filter->query); + params.push_back(*filter->query); + } + if (filter->addedAfter) { + clause("created_at >= ?"); + params.push_back(*filter->addedAfter); + } + if (filter->addedBefore) { + clause("created_at < ?"); + params.push_back(*filter->addedBefore); + } + } + + std::int64_t total = 0; + { + auto st = db_.prepare("SELECT count(*) FROM tasks" + where); + if (!st) return std::unexpected(st.error()); + for (std::size_t i = 0; i < params.size(); ++i) + if (auto r = st->bind(static_cast(i + 1), std::string_view(params[i])); !r) + return std::unexpected(r.error()); + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + if (*row) total = st->column_int(0); + } + + // Default sort is newest-first; an explicit sort is a whitelisted column + direction. + // NULLs sort last in both directions so an unsized / unqueued task never floats up. + std::string order = "created_at DESC"; + if (sort) { + const char* col = sort_column(sort->field); + const bool desc = sort->direction == proto::TaskSortDirection::Desc; + order = std::string(col) + " IS NULL, " + col + (desc ? " DESC" : " ASC"); + } + + const std::int64_t lim = limit > 0 ? limit : 500; + const std::int64_t off = offset > 0 ? offset : 0; + + Page page; + page.total = total; + { + auto st = db_.prepare(std::string("SELECT ") + kCols + " FROM tasks" + where + + " ORDER BY " + order + " LIMIT ? OFFSET ?"); + if (!st) return std::unexpected(st.error()); + int n = 1; + for (const auto& p : params) + if (auto r = st->bind(n++, std::string_view(p)); !r) return std::unexpected(r.error()); + if (auto r = st->bind(n++, lim); !r) return std::unexpected(r.error()); + if (auto r = st->bind(n++, off); !r) return std::unexpected(r.error()); + for (;;) { + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + if (!*row) break; + page.rows.push_back(read_row(*st)); + } + } + return page; +} + +DbResult Tasks::set_state(std::string_view task_id, std::string_view state, + const std::optional& pause_reason) { + auto st = db_.prepare( + "UPDATE tasks SET state = ?2, pause_reason = ?3 WHERE task_id = ?1"); + if (!st) return std::unexpected(st.error()); + if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error()); + if (auto r = st->bind(2, state); !r) return std::unexpected(r.error()); + const bool paused = state == "paused"; + if (auto r = paused && pause_reason ? st->bind(3, std::string_view(*pause_reason)) + : st->bind_null(3); + !r) + return std::unexpected(r.error()); + if (auto r = st->step(); !r) return std::unexpected(r.error()); + return sqlite3_changes(db_.raw()) > 0; +} + +DbResult Tasks::remove(std::string_view task_id) { + auto st = db_.prepare("DELETE FROM tasks WHERE task_id = ?1"); + if (!st) return std::unexpected(st.error()); + if (auto r = st->bind(1, task_id); !r) return std::unexpected(r.error()); + if (auto r = st->step(); !r) return std::unexpected(r.error()); + return sqlite3_changes(db_.raw()) > 0; +} + +DbResult Tasks::count() { + auto st = db_.prepare("SELECT count(*) FROM tasks"); + if (!st) return std::unexpected(st.error()); + auto row = st->step(); + if (!row) return std::unexpected(row.error()); + return (*row) ? st->column_int(0) : 0; +} + +proto::TaskSummary to_summary(const TaskRow& r) { + proto::TaskSummary s; + s.taskId = r.task_id; + s.filename = r.filename; + s.saveDir = r.save_dir; + s.url = r.url; + s.effectiveUrl = r.effective_url; + s.sizeBytes = r.size_bytes; + s.downloadedBytes = r.downloaded_bytes; + if (auto st = proto::parse_TaskState(r.state)) s.state = *st; + s.speedBps = 0; + s.resumable = r.resumable; + s.segments = r.eff_segments; + s.categoryId = r.category_id; + s.queueId = r.queue_id; + s.queuePosition = r.queue_position; + s.description = r.description; + s.createdAt = r.created_at; + s.lastTryAt = r.last_try_at; + s.completedAt = r.completed_at; + + if (r.error_code) { + proto::TaskError e; + if (auto c = proto::parse_TaskErrorCode(*r.error_code)) e.code = *c; + e.message = r.error_message.value_or(""); + e.httpStatus = r.error_http_status; + e.retryable = r.error_retryable.value_or(false); + e.attempt = r.error_attempt; + e.nextRetryAt = r.error_next_retry_at; + s.error = std::move(e); + } + return s; +} + +} // namespace velox::daemon::store diff --git a/daemon/src/store/tasks.hpp b/daemon/src/store/tasks.hpp new file mode 100644 index 0000000..4cc384c --- /dev/null +++ b/daemon/src/store/tasks.hpp @@ -0,0 +1,89 @@ +#pragma once + +// Read/write access to the `tasks` table, plus the projection onto the wire TaskSummary. +// download.list does its filtering, sorting and paging here (M1 DoD: a 1000-row list under +// 50 ms, never materialised client-side). + +#include +#include +#include +#include + +#include "store/sqlite.hpp" +#include "velox_proto.hpp" + +namespace velox::daemon::store { + +// One row of `tasks`, 1:1 with the schema. std::optional maps a NULL column. +struct TaskRow { + std::string task_id; + std::string url; + std::string save_dir; + std::string filename; + std::string state = "new"; + std::string start_mode = "auto"; + std::string created_at; + + std::optional effective_url; + std::optional category_id; + std::optional queue_id; + std::optional description; + std::optional pause_reason; + std::optional etag; + std::optional last_modified; + std::optional content_type; + std::optional last_try_at; + std::optional completed_at; + std::optional checksum_algo; + std::optional checksum_value; + + std::optional size_bytes; + std::int64_t downloaded_bytes = 0; + bool resumable = false; + + std::optional req_segments; + std::int64_t eff_segments = 0; + std::optional req_buffer_bytes; + std::optional eff_buffer_bytes; + std::optional queue_position; + + std::optional error_code; + std::optional error_message; + std::optional error_http_status; + std::optional error_retryable; + std::optional error_attempt; + std::optional error_next_retry_at; +}; + +class Tasks { +public: + explicit Tasks(Db& db) : db_(db) {} + + DbResult insert(const TaskRow& row); + DbResult> get(std::string_view task_id); + + struct Page { + std::int64_t total = 0; // rows matching the filter, ignoring paging + std::vector rows; + }; + DbResult list(const std::optional& filter, + const std::optional& sort, std::int64_t offset, + std::int64_t limit); + + // Move a task to `state`; `pause_reason` is written only when state == "paused" + // (cleared otherwise). Returns false if there is no such task. + DbResult set_state(std::string_view task_id, std::string_view state, + const std::optional& pause_reason); + + DbResult remove(std::string_view task_id); + DbResult count(); + +private: + Db& db_; +}; + +// Project a row onto the wire type. `state` and `error.code` strings are assumed valid +// (the CHECK constraints and the state machine keep them so). +velox::proto::TaskSummary to_summary(const TaskRow& row); + +} // namespace velox::daemon::store diff --git a/daemon/tests/CMakeLists.txt b/daemon/tests/CMakeLists.txt index 291dc9f..5c14b75 100644 --- a/daemon/tests/CMakeLists.txt +++ b/daemon/tests/CMakeLists.txt @@ -19,3 +19,4 @@ veloxd_test(ws_server LIBS veloxd_rpc) veloxd_test(sched_window LIBS veloxd_sched) veloxd_test(sched_governor LIBS veloxd_sched) veloxd_test(safepath LIBS veloxd_fs) +veloxd_test(store_tasks LIBS veloxd_store) diff --git a/daemon/tests/store_tasks_test.cpp b/daemon/tests/store_tasks_test.cpp new file mode 100644 index 0000000..3f95154 --- /dev/null +++ b/daemon/tests/store_tasks_test.cpp @@ -0,0 +1,187 @@ +// store/tasks + store/settings: insert / get / list (filter, sort, page) and the +// TaskSummary projection. + +#include + +#include "check.hpp" +#include "store/migrations.hpp" +#include "store/settings.hpp" +#include "store/sqlite.hpp" +#include "store/tasks.hpp" + +using namespace velox::daemon::store; +namespace proto = velox::proto; + +namespace { + +TaskRow row(std::string id, std::string name, std::string state, std::string created, + std::optional size = std::nullopt, + std::optional cat = std::nullopt) { + TaskRow r; + r.task_id = std::move(id); + r.url = "https://example.com/" + name; + r.save_dir = "/home/u/Downloads"; + r.filename = std::move(name); + r.state = std::move(state); + r.created_at = std::move(created); + r.size_bytes = size; + r.category_id = std::move(cat); + return r; +} + +} // namespace + +void run() { + auto db = Db::open(":memory:"); + CHECK(db.has_value()); + if (!db) return; + CHECK(migrate_to_head(*db).has_value()); + + Tasks tasks(*db); + + // --- insert + get round trip ------------------------------------------------ + { + auto r = row("t1", "ubuntu.iso", "downloading", "2026-09-10T10:00:00Z", 6228541440LL, + "programs"); + r.downloaded_bytes = 1000000; + r.eff_segments = 8; + r.resumable = true; + CHECK(tasks.insert(r).has_value()); + + auto got = tasks.get("t1"); + CHECK(got.has_value() && got->has_value()); + if (got && *got) { + CHECK_EQ((*got)->filename, std::string("ubuntu.iso")); + CHECK_EQ((*got)->downloaded_bytes, 1000000); + CHECK((*got)->size_bytes.has_value() && *(*got)->size_bytes == 6228541440LL); + CHECK((*got)->resumable); + CHECK((*got)->category_id.value_or("") == "programs"); + } + + auto missing = tasks.get("nope"); + CHECK(missing.has_value() && !missing->has_value()); + } + + // --- a duplicate id is rejected by the primary key ----------------------- + CHECK(!tasks.insert(row("t1", "again", "new", "2026-09-10T10:01:00Z")).has_value()); + + // --- projection to TaskSummary, including the error block -------------- + { + auto r = row("terr", "broken.zip", "failed", "2026-09-10T09:00:00Z"); + r.error_code = "not_found"; + r.error_message = "server returned 404"; + r.error_http_status = 404; + r.error_retryable = false; + r.error_attempt = 3; + CHECK(tasks.insert(r).has_value()); + + auto got = tasks.get("terr"); + CHECK(got && *got); + const auto s = to_summary(**got); + CHECK(s.state == proto::TaskState::Failed); + CHECK(s.error.has_value()); + if (s.error) { + CHECK(s.error->code == proto::TaskErrorCode::NotFound); + CHECK(s.error->httpStatus.value_or(0) == 404); + CHECK(s.error->attempt.value_or(0) == 3); + } + // a non-error task has no error block + auto ok = tasks.get("t1"); + CHECK(!to_summary(**ok).error.has_value()); + } + + // --- list: total + paging + default newest-first sort ------------------ + { + for (int i = 0; i < 20; ++i) { + char id[8]; + std::snprintf(id, sizeof(id), "p%02d", i); + char ts[24]; + std::snprintf(ts, sizeof(ts), "2026-09-11T%02d:00:00Z", i); + CHECK(tasks.insert(row(id, std::string("f") + id, "queued", ts)).has_value()); + } + auto page = tasks.list(std::nullopt, std::nullopt, 0, 5); + CHECK(page.has_value()); + if (page) { + CHECK_EQ(page->total, 22); // 20 + t1 + terr + CHECK_EQ(page->rows.size(), 5u); + // newest first: p19 (11:00) before p18 ... + CHECK_EQ(page->rows.front().task_id, std::string("p19")); + } + auto page2 = tasks.list(std::nullopt, std::nullopt, 20, 500); + CHECK(page2.has_value()); + if (page2) CHECK_EQ(page2->rows.size(), 2u); // the tail + } + + // --- list: filter by state --------------------------------------------- + { + proto::TaskFilter f; + f.states = std::vector{proto::TaskState::Queued}; + auto page = tasks.list(f, std::nullopt, 0, 500); + CHECK(page.has_value()); + if (page) { + CHECK_EQ(page->total, 20); + for (const auto& r : page->rows) CHECK_EQ(r.state, std::string("queued")); + } + } + + // --- list: filter by category + case-insensitive query --------------- + { + proto::TaskFilter f; + f.categoryId = "programs"; + auto page = tasks.list(f, std::nullopt, 0, 500); + CHECK(page.has_value() && page->total == 1); + + proto::TaskFilter q; + q.query = "UBUNTU"; // matches "ubuntu.iso" case-insensitively + auto page2 = tasks.list(q, std::nullopt, 0, 500); + CHECK(page2.has_value() && page2->total == 1); + if (page2 && !page2->rows.empty()) + CHECK_EQ(page2->rows.front().task_id, std::string("t1")); + } + + // --- list: explicit sort by filename ascending ---------------------- + { + proto::TaskSort s; + s.field = proto::TaskSortField::Filename; + s.direction = proto::TaskSortDirection::Asc; + auto page = tasks.list(std::nullopt, s, 0, 3); + CHECK(page.has_value()); + if (page && page->rows.size() >= 2) + CHECK(page->rows[0].filename <= page->rows[1].filename); + } + + // --- set_state + pause_reason, and remove ------------------------- + { + CHECK(tasks.set_state("t1", "paused", std::string("user")).value_or(false)); + auto g = tasks.get("t1"); + CHECK(g && *g && (*g)->state == "paused" && (*g)->pause_reason.value_or("") == "user"); + + CHECK(tasks.set_state("t1", "downloading", std::nullopt).value_or(false)); + g = tasks.get("t1"); + CHECK(g && *g && !(*g)->pause_reason.has_value()); // cleared when not paused + + CHECK(!tasks.set_state("ghost", "paused", std::nullopt).value_or(true)); + + CHECK(tasks.remove("terr").value_or(false)); + CHECK(!tasks.remove("terr").value_or(true)); + CHECK(tasks.count().value_or(-1) == 21); + } + + // --- settings: default fallback, override, typed reads --------------- + { + Settings settings(*db); + CHECK_EQ(settings.get_int("connection.maxConcurrentDownloads"), 5); // built-in default + CHECK(settings.set_raw("connection.maxConcurrentDownloads", "9").has_value()); + CHECK_EQ(settings.get_int("connection.maxConcurrentDownloads"), 9); // override wins + + CHECK_EQ(settings.get_int("connection.maxActiveSegments"), 32); + auto roots = settings.get_string_array("saveTo.allowedRoots"); + CHECK_EQ(roots.size(), 1u); + if (!roots.empty()) CHECK_EQ(roots.front(), std::string("~/Downloads")); + + auto ov = settings.overrides(); + CHECK(ov.has_value() && ov->count("connection.maxConcurrentDownloads") == 1); + } +} + +TEST_MAIN()