fs sandbox: fold_absolute_to_relative leaks Windows drive prefix on POSIX hosts (CI red on Linux/macOS)

[reinaldocrespo]

Summary

openads::platform::fold_absolute_to_relative (server filesystem sandbox, src/platform/fs_sandbox.cpp) does not fold a Windows-style client path on a non-Windows host. On Linux/macOS a client path like C:\dir\file (or C:/dir/file) is returned verbatim, so the C: drive prefix survives into the jailed path instead of being stripped.

This is currently failing CI on all three POSIX build configs (ubuntu-24.04 / ninja-clang, ubuntu-24.04 / ninja-clang / TLS, macos-14 / default) — the only red jobs on main:

tests/unit/fs_sandbox_test.cpp:33: ERROR: CHECK( rel.find("C:") == std::string::npos ) is NOT correct!

(It never surfaced until now because the feature had a separate -Werror compile break on clang/gcc; once that was fixed the build reached the tests and this assertion fires. It passes on the MSVC jobs because std::filesystem recognizes drive letters only on Windows.)

Root cause

std::string fold_absolute_to_relative(const std::string& client_path) {
    fs::path p(client_path);
    if (!p.is_absolute() && !p.has_root_name()) return client_path;  // <-- Linux: both false for "C:\..."
    ...
}

On Linux, fs::path("C:/app/data/t.txt") is neither absolute nor has a root name, so the guard returns the path unchanged. The drive letter is never dropped and the fold is a no-op.

Why it matters (beyond CI)

A cross-platform ADS client (Windows) talking to a POSIX OpenADS server with EnableFileFunc=1 sends Windows-style paths. Since the drive prefix is not folded, resolve_under_root composes an unexpected path — a potential jail-escape / path-confusion, not just a test failure. The fold should be host-independent.

Suggested fix

Normalize separators and strip a drive-letter prefix + leading root slashes explicitly, regardless of host, before handing the remainder to std::filesystem:

std::string fold_absolute_to_relative(const std::string& client_path) {
    std::string s = client_path;
    for (char& ch : s) if (ch == '\\') ch = '/';
    if (s.size() >= 2 &&
        ((s[0] >= 'A' && s[0] <= 'Z') || (s[0] >= 'a' && s[0] <= 'z')) &&
        s[1] == ':') {
        s.erase(0, 2);                 // drop "C:"
    }
    std::size_t b = s.find_first_not_of('/');
    if (b == std::string::npos)
        return fs::path(client_path).filename().string();
    s.erase(0, b);                     // drop leading root slashes
    fs::path p(s);
    if (p.is_absolute() || p.has_root_name()) p = p.relative_path();
    if (p.empty() || p == ".") return p.filename().string();
    return p.generic_string();
}

Verified locally this keeps the existing cases (sub/a.txt unchanged, ../outside.txt still caught by resolve_under_root, native Windows/Linux absolutes fold correctly) and makes fs_sandbox_test.cpp:33 pass on Linux/macOS.

Context

Feature landed in b06cf6a (feat(fs): server filesystem API). I only touched the surrounding compile/test issues to unblock the build for unrelated S4 work; leaving the sandbox logic itself to you since it's your feature and a security-sensitive path. Happy to open a PR with the above if useful.