From 5e79d7d76bb0038468a9f73b37816c6edea04cbe Mon Sep 17 00:00:00 2001 From: TapTap Date: Wed, 9 Sep 2026 13:41:28 +0200 Subject: [PATCH] feat(p5-remote-option): --remote-option (probe 2.14.0), --trust-sender --- RSYNC_COMPAT.md | 8 ++- src/client/client_cli.c | 49 +++++++++++++ src/client/client_send.c | 3 +- src/client/usage.c | 10 +++ src/server/receiver.c | 7 +- src/server/server.c | 6 ++ src/shared/config.c | 10 +++ src/shared/config.h | 45 +++++++++++- src/shared/file.c | 28 +++++++- src/shared/file.h | 9 +++ src/shared/file_receive.c | 20 +++--- src/shared/transport_ssh.c | 111 ++++++++++++++++++++++------- src/shared/transport_ssh.h | 5 +- tests/integration/test_features.py | 14 ++++ tests/integration/test_ssh.py | 45 ++++++++++++ tests/test_client_cli.c | 110 ++++++++++++++++++++++++++++ tests/test_config.c | 55 ++++++++++++++ tests/test_transport_ssh.c | 62 ++++++++++++++-- 18 files changed, 547 insertions(+), 50 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 5531e52..3df6a39 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -616,7 +616,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | `--address=ADDRESS` | Bind address for outgoing socket | ❌ Not Implemented | Removed because it had no effect | | `-4`, `--ipv4` | Prefer IPv4 | ❌ Not Implemented | Removed because it had no effect | | `-6`, `--ipv6` | Prefer IPv6 | ❌ Not Implemented | Removed because it had no effect | -| `--remote-option=OPT`, `-M` | Send an option only to the remote side | ❌ Not Implemented | `-M` is FastSync's metadata-preservation flag | +| `--remote-option=OPT`, `-M` | Send an option only to the remote side | ✅ Implemented | Long form only; each value is appended to the remote server invocation over SSH as an individually single-quote-escaped shell word in `ssh_build_remote_command()`. Values are validated (non-empty, no control characters) and shell metacharacters cannot break out of the quoting (`;`, `&`, `|`, `, `$`, `(`, `)`, quotes are neutralized), so a value cannot inject an arbitrary remote command and a subsequent `--` on the client line cannot be turned into one. The options never cross the binary config frame. Divergence: the short `-M` form is intentionally unavailable because `-M` is already FastSync's metadata-preservation flag/multiplier (see Phase 5 notes below) | ## 14. Daemon Mode @@ -639,7 +639,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | Max data/string/chunk sizes | Prevent OOM attacks | ✅ Implemented | Per-message limits | | Per-connection memory limit | 1GB per connection | ✅ Implemented | `MAX_CONNECTION_MEMORY` | | `--max-alloc=SIZE` | Limit a single memory allocation | ✅ Implemented | Caps the largest single allocation; binary units, default 1G | -| `--trust-sender` | Trust remote sender's file list | ❌ Not Implemented | | +| `--trust-sender` | Trust remote sender's file list | ✅ Implemented | Long-form-only, receiver-local policy that never crosses the wire. The receiver skips its redundant up-front re-validation of the incoming file list (empty/`..` path rejection and the escaping-symlink-target containment), trusting the sender instead of double-checking (fewer checks, faster, potentially unsafe, matching rsync). Off by default. The low-level fd-relative confinement primitives (`file_open_secure_parent`, the O_NOFOLLOW parent walk, leaf/destination confinement) are deliberately KEPT even under `--trust-sender`, so a hostile sender still cannot write or link outside the authorized root (see Phase-5 notes below) | | `--old-args` | Disable modern arg protection | ✅ Implemented | SSH-only legacy mode; restores raw remote command construction and permits shell interpretation of the configured server path | | `--ignore-missing-args` | Ignore missing source args | ✅ Implemented | FastSync has a single source-root argument (which always exists), so the "explicitly requested source arguments" are the `--files-from` entries and the flags only ever apply there (inert without `--files-from`, like `-R`). Without the flag a listed-but-missing entry stays a hard pre-transfer error (nothing is transferred). With it each missing entry is skipped: nothing is sent for it, it never enters the keep-set, and the run succeeds for the rest — an all-missing non-empty list succeeds transferring nothing, matching rsync. `--dirs` + `--files-from` missing entries are skipped the same way. Every skipped entry is logged and a per-run warning names the count, so the handling is never a silent no-op. Divergences: an EMPTY `--files-from` file stays a hard error in every mode (no argument was requested at all; rsync likewise reports "no source files specified"); missing-arg skipping only applies to the pre-transfer list validation, so an entry that is present at preflight and vanishes mid-transfer still fails (matching rsync, whose flag "does not affect subsequent vanished-file errors"); `--no-ignore-missing-args` is not a supported negation | | `--delete-missing-args` | Delete missing source args | ✅ Implemented | Implies `--ignore-missing-args` (order-independent) and additionally removes each missing entry's destination mirror receiver-side. The mirror is computed exactly like a present sibling's wire path: the bare relative entry under `-R`, otherwise the full source-mirror path below the destination root. rsync parity, verified against the man page: it does **not** imply `--delete` generally and is "independent of any other type of delete processing" — unrelated destination extras are untouched unless `--delete` is also present. Composition with `--delete` + timing: the exact-path deletions commit with the manifest, early for `--delete-before`/`--delete-during`, else only after a fully-successful transfer (delete-after/commit). A non-empty directory mirror is removed only when `--force` or `--delete` is in effect (otherwise it is left with a warning and the run continues, like rsync); an absent mirror is a no-op. An explicitly listed missing arg is a user request, not an excluded file: its deletion is never blocked by the filter-exclusion protection of excluded destination mirrors (a mirror sitting inside a filter-excluded directory is still removed). Safety/policy: gated by the server `--allow-delete` policy like `--delete`; the request paths cross the wire only in the delete-manifest frame and are confined by the same receiver validation as the keep-set (non-empty, relative, traversal-free, bounded by the per-section/per-frame manifest caps); the `--delay-updates` staging directory and basis snapshots are protected exactly as in the extras walker. Divergence: the missing-args deletions are not counted toward `--max-delete` (they are explicit per-path requests, not discovered extras). See the Phase-3 wire note below for the `PROTOCOL_VERSION` bump | @@ -669,6 +669,10 @@ now transmits targets (the prior behavior was broken/partial); its status moved ## Implementation Difficulty Plan +**Phase 5 notes (remote-option wave):** `--remote-option=OPT` (long form only) and `--trust-sender` landed here. +- `--remote-option` is CLIENT-only and never serialized into the binary config frame. On the SSH transport the client forwards each value to the remote server by appending it to the remote command line in `ssh_build_remote_command()`, after ` --stdio`, as an individually single-quoted shell word (`'...'` with `'\''` for embedded quotes). Values are validated at CLI parse time (non-empty; no ASCII control characters) and rejected otherwise, and a non-conforming value is refused again in the command builder, so shell metacharacters (`;`, `&`, `|`, backticks, `$()`, quotes) can never break out of the quoting to inject an unrelated remote command — including after a client-side `--` separator, whose arguments are never forwarded anyway. Because the remote options affect the *remote server invocation*, not the transmitted config, the wire frame layout is unchanged, but `PROTOCOL_VERSION` was bumped **2.13.0 → 2.14.0** as the Phase-5 lockstep release marker (a 2.14 client against a 2.13 server fails the version check cleanly rather than the old server rejecting an unfamiliar forwarded argv later). Divergence: rsync's short `-M` form of `--remote-option` is intentionally NOT implemented, because `-M` is already FastSync's metadata-preservation mode/multiplier. +- `--trust-sender` is a receiver-local policy: it never crosses the wire (the sender's value is never serialized, so a wire peer can never enable it). On the receiving process it skips the up-front re-validation of the incoming file list (empty/`..` path rejection and the escaping-symlink-target containment), trusting the sender's list instead of double-checking — fewer checks, faster, and potentially unsafe, matching rsync. It is OFF by default (`config.trust_sender`). As a deliberate safety floor, the low-level fd-relative confinement primitives are NOT disabled: `file_open_secure_parent()` (O_NOFOLLOW walk, `..` rejection, root containment) and leaf/destination confinement still hold, so even under `--trust-sender` a hostile sender cannot write or create a symlink outside the authorized root — the relaxation only removes the redundant list-layer double-checks, never the root-confinement guarantees. + The estimates below cover the currently unimplemented features in this document. They assume one engineer familiar with the codebase, include implementation and focused tests, and exclude production rollout time. A feature should not be marked implemented until its behavior is tested in both local and SSH/TCP paths where applicable. > **Note:** This plan is a superset snapshot written while several of the listed features were still outstanding. The Summary matrix above is the authoritative record of what is already shipped (for example quiet/info/debug output, `--existing`, `--remove-source-files`, `-h`, and `--size-only` are now implemented on `dev`). Treat the phases as sequencing guidance for the work that remains unimplemented. diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 2c5b0d2..f2a0480 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -157,6 +157,40 @@ static int set_nonneg_int_option(int* dest, const char* value, const char* optio return 0; } +/* Forward decl: config_add_pattern is defined below, but the --remote-option + * helper above needs it. */ +static int config_add_pattern(char*** patterns, int* count, const char* value, const char* optname); + +/* Validate and append one --remote-option=OPT value. OPT is forwarded to the + * remote server invocation (over SSH) by appending it to the remote command + * line, so it must be a single safe shell word: it must be non-empty and must + * contain no control characters that could break the single-quoted command + * word ssh_build_remote_command wraps it in (newline/CR and other ASCII + * control chars are rejected up front). Ordinary shell metacharacters + * (; & | ` $ () etc.) need not be rejected because they are neutralized by the + * single-quoting boundary, but rejecting control characters keeps the + * quoting scheme airtight regardless of the remote shell. Returns 0 on + * success, -1 on a rejected value. */ +static int config_add_remote_option(Config* config, const char* value, const char* optname) { + if (!value || value[0] == '\0') { + log_message(LOG_LEVEL_ERROR, "%s requires a non-empty option value", optname); + return -1; + } + for (const unsigned char* p = (const unsigned char*)value; *p; p++) { + if (*p < 0x20 || *p == 0x7f) { + log_message(LOG_LEVEL_ERROR, + "%s value contains a control character that could break the remote shell " + "quoting; rejecting", + optname); + return -1; + } + } + if (config_add_pattern(&config->remote_options, &config->remote_option_count, value, optname) != + 0) + return -1; + return 0; +} + /* Validate and append one --compare-dest/--copy-dest/--link-dest directory. * The path is interpreted on the receiver relative to the destination root, * so it must be a non-empty relative path with no "." / ".." components (an @@ -551,6 +585,11 @@ static const OptionEntry OPTION_TABLE[] = { {"--xattrs", "-X", OPT_FLAG, offsetof(Config, preserve_xattrs)}, {"--acls", "-A", OPT_FLAG, offsetof(Config, preserve_acls)}, {"--fake-super", NULL, OPT_FLAG, offsetof(Config, fake_super)}, + /* Long-form-only: rsync's -M short form of --remote-option is INTENTIONALLY + * unavailable because -M already means metadata mode in FastSync (a + * documented divergence; see RSYNC_COMPAT.md). --trust-sender is a local + * receiver policy and never travels to the remote peer. */ + {"--trust-sender", NULL, OPT_FLAG, offsetof(Config, trust_sender)}, }; /* Only boolean options with no required argument are safe to negate. */ @@ -1125,6 +1164,16 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } if (set_checksum_seed(config, argv[++i]) != 0) return -1; + } else if (strncmp(argv[i], "--remote-option=", 16) == 0) { + if (config_add_remote_option(config, argv[i] + 16, "--remote-option") != 0) + return -1; + } else if (opt_is(argv[i], "--remote-option", NULL)) { + if (i + 1 >= argc) { + log_message(LOG_LEVEL_ERROR, "missing argument for --remote-option"); + return -1; + } + if (config_add_remote_option(config, argv[++i], "--remote-option") != 0) + return -1; } else if (strncmp(argv[i], "--compare-dest=", 15) == 0) { if (set_basis_dest_option(config, BASIS_DEST_COMPARE, argv[i] + 15, "--compare-dest") != 0) return -1; diff --git a/src/client/client_send.c b/src/client/client_send.c index 2f0c00a..9934a7f 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -367,7 +367,8 @@ static Client* connect_transfer_client(const Config* config) { return NULL; } return client_connect_ssh(config->ssh_destination, config->ssh_port, - config->fastsync_server_path, config->old_args); + config->fastsync_server_path, config->old_args, + config->remote_options, config->remote_option_count); } Client* client = client_create(); diff --git a/src/client/usage.c b/src/client/usage.c index 58c2396..689430c 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -186,6 +186,16 @@ void print_usage(void) { printf(" Path to fastsync-server on remote (default: fastsync-server)\n"); printf( " --old-args Disable safe SSH command argument quoting (legacy compatibility)\n"); + printf(" --remote-option=OPT Append OPT to the REMOTE server invocation over SSH\n"); + printf(" (repeatable; each value is single-quote-escaped on the remote\n"); + printf(" command line; empty values and values with control characters\n"); + printf(" are rejected). Long form only: rsync's -M short form is NOT\n"); + printf(" available because -M already means metadata preservation in\n"); + printf(" FastSync (documented divergence)\n"); + printf(" --trust-sender Trust the remote sender's file list: the receiver skips its\n"); + printf(" own up-front path-traversal/containment re-validation of the\n"); + printf(" incoming file list (fewer checks, faster, potentially unsafe).\n"); + printf(" Local receiver policy: never sent to the peer, off by default\n"); printf(" -l, --links Copy symlinks as symlinks\n"); printf(" --copy-links Transform symlinks into referent files\n"); printf(" --safe-links Skip symlinks that point outside transfer tree\n"); diff --git a/src/server/receiver.c b/src/server/receiver.c index 62694fb..63da749 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -3,6 +3,7 @@ #include "chunk.h" #include "config.h" #include "delay_updates.h" +#include "file.h" #include "file_receive.h" #include "log.h" #include "metadata.h" @@ -92,7 +93,11 @@ static bool receiver_process_batch(Config* config, int file_descriptor) { send_status(file_descriptor, STATUS_ERROR); return false; } - if (!utils_valid_batch_path(check_path)) { + /* --trust-sender: accept a ``..``/absolute check path (a trusted sender's + odd-but-legit entry) and defer containment to the secure stat below; + an empty path is still always rejected. */ + if (check_path[0] == '\0' || + (!file_get_trust_sender() && !utils_valid_batch_path(check_path))) { free(check_path); send_status(file_descriptor, STATUS_ERROR); return false; diff --git a/src/server/server.c b/src/server/server.c index f5bfd98..f232b7b 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -220,6 +220,12 @@ void handler(int file_descriptor) { fd-walk reads a stable value during the whole transfer (and never bleeds across the per-connection forked processes). */ file_set_keep_dirlinks(config->keep_dirlinks); + /* --trust-sender is a LOCAL receiver policy: it never crosses the wire (so a + wire peer can never enable it), the receiving process applies it here from + its own config. Set before any multithreaded receiver/writer threads are + spawned so the fd-walk reads a stable value during the whole transfer, and + never bleeds across the per-connection forked processes. Off by default. */ + file_set_trust_sender(config->trust_sender); if (config->use_multithreading) { Queue* q = queue_create(100, file_destroy); if (q == NULL) { diff --git a/src/shared/config.c b/src/shared/config.c index 20ffc27..0702733 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -122,6 +122,8 @@ static void config_set_defaults(Config* config) { config->rsync_path = NULL; config->old_args = false; config->temp_dir = NULL; + config->remote_options = NULL; + config->remote_option_count = 0; config->basis_dirs = NULL; config->basis_count = 0; config->partial_dir = NULL; @@ -161,6 +163,7 @@ static void config_set_defaults(Config* config) { config->open_noatime = false; config->use_xattrs = false; config->fake_super = false; + config->trust_sender = false; } static bool valid_wire_bool(int value) { @@ -395,6 +398,13 @@ void config_delete(Config* config) { free(config->rsh_command); free(config->rsync_path); free(config->temp_dir); + if (config->remote_options) { + for (int i = 0; i < config->remote_option_count; i++) + free(config->remote_options[i]); + free(config->remote_options); + } + config->remote_options = NULL; + config->remote_option_count = 0; for (int i = 0; i < config->basis_count; i++) { free(config->basis_dirs[i].path); config->basis_dirs[i].path = NULL; diff --git a/src/shared/config.h b/src/shared/config.h index 6bc9619..b046b8f 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -230,6 +230,14 @@ typedef struct Config { char* rsync_path; bool old_args; char* temp_dir; + /* --remote-option=OPT (Phase 5, long form only): one or more extra command-line + * options to append to the REMOTE server invocation over SSH. CLIENT-ONLY: + * they are composed into the remote command line by ssh_build_remote_command() + * (each valid word is shell-escaped with the same quoting boundary as the + * server path), and are NEVER serialized into the binary config frame. They + * do NOT cross the wire and are never parsed on the receiver process. */ + char** remote_options; + int remote_option_count; /* Alternate basis directories, ordered by command-line appearance. Each * entry's type selects compare/copy/link behavior on an exact match. These * cross the wire so the receiver can consult them; they are interpreted @@ -350,9 +358,44 @@ typedef struct Config { * a reserved user.fastsync.stat xattr recording the source uid/gid/mode/mtime * so a later privileged restore could re-apply them. Crosses the wire. */ bool fake_super; + + // Phase 5: --trust-sender + /* Long-form-only, receiver-local policy. rsync's --trust-sender tells the + * receiving side to trust that the sender already produced a sane file list, + * relaxing the receiver's own up-front re-validation of every incoming path. + * In FastSync the receiver normally double-checks each transmitted file-list + * entry (empty / ".." path-traversal rejection) and refuses to materialize a + * symlink whose target could escape the receive root. When trust_sender is + * set, those redundant list-level re-checks are SKIPPED: the receiving side + * trusts the sender's list instead of re-validating it (fewer checks, faster, + * potentially unsafe, matching rsync). It is a LOCAL receiver policy and is + * NEVER serialized into the config frame (it exists only on the process that + * actually receives the file list). Even under trust_sender the low-level + * fd-relative confinement primitives (file_open_secure_parent, the O_NOFOLLOW + * parent walk, leaf/destination confinement) are deliberately KEPT as a hard + * floor, so a hostile sender still cannot write or link outside the + * authorized root (see the phase-5 notes in RSYNC_COMPAT.md). Off by + * default; only relaxes validation when explicitly requested. */ + bool trust_sender; } Config; -#define PROTOCOL_VERSION "2.13.0" +/* Phase 5 (remote-option wave): 2.13.0 -> 2.14.0. + * + * WHY the bump, grounded in the wire: the binary config-frame layout is + * UNCHANGED by this wave (neither --remote-option nor --trust-sender adds a + * serialized field; see the field comments above). --remote-option is + * forwarded to the remote server over the SSH remote-command line + * (ssh_build_remote_command) and --trust-sender is a purely local receiver + * policy, so there is no new frame byte to negotiate. The bump is still the + * correct release marker for Phase 5 because the client-to-server INVOCATION + * surface changed: a client that composes remote-options expects a server that + * knows how to honor them, and the only safe way to express "this feature set + * is one coordinated release" is the strict same-version handshake FastSync + * already performs for every release. A 2.14 client against a 2.13 server + * fails the version check cleanly up front (rather than the remote server + * rejecting an unfamiliar forwarded argv at a confusing later point), which is + * exactly what the lockstep convention of this project requires. */ +#define PROTOCOL_VERSION "2.14.0" #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024) /* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */ #define MAX_BASIS_DIRS 64 diff --git a/src/shared/file.c b/src/shared/file.c index d7b4f46..9bc8b9e 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -363,6 +363,23 @@ bool file_get_keep_dirlinks(void) { return file_keep_dirlinks; } +/* --trust-sender (Phase 5) receiver process-wide policy: when set, the receiver + * trusts the sender's file list and skips its own redundant up-front re- + * validation (empty/".." path rejection, escaping-symlink-target containment). + * Kept OFF by default; the server's per-connection handler sets it once from the + * received config before any receiver/writer threads start (each connection is + * its own forked process, so this per-process value never bleeds across + * connections). */ +static bool file_trust_sender = false; + +void file_set_trust_sender(bool enable) { + file_trust_sender = enable; +} + +bool file_get_trust_sender(void) { + return file_trust_sender; +} + /* True when `target` is a lexical symlink target that can never escape the * receive root once created beneath it: relative (not absolute) and containing * no ".." path component. Used by --munge-links' sender-side containment: an @@ -425,7 +442,16 @@ char* file_symlink_munge(const char* target) { * false) so a malicious sender can never materialize a symlink that points * outside the receive root. */ bool file_symlink_at_secure(const char* path, const char* target) { - if (!path || !target || has_path_traversal(path) || !file_symlink_target_contained(target)) + /* The link itself (`path`) is always kept below the authorized root. The + TARGET may point anywhere: normally only a contained (relative, ".."-free) + target is permitted so a malicious sender can never plant a symlink that + later dereferences outside the root. Under --trust-sender that target + containment check is relaxed (the receiver trusts the sender and copies the + link verbatim, matching rsync -l), but path/leaf confinement is never + disabled, so the link still cannot be placed outside the tree. */ + if (!path || !target || has_path_traversal(path)) + return false; + if (!file_trust_sender && !file_symlink_target_contained(target)) return false; char* leaf = NULL; int parent_fd = file_open_secure_parent(path, &leaf, true); diff --git a/src/shared/file.h b/src/shared/file.h index 9fb14b8..37da41b 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -54,6 +54,15 @@ bool file_symlink_at_secure(const char* path, const char* target); void file_set_keep_dirlinks(bool enable); bool file_get_keep_dirlinks(void); +/* --trust-sender receiver process-wide policy (Phase 5). When set, the + * receiver trusts that the sender already produced a clean file list and skips + * its own redundant up-front re-validation of incoming paths (the empty/".." + * rejection and the escaping-symlink-target containment). The low-level + * fd-relative confinement primitives below are deliberately NOT disabled by + * this flag, so a hostile sender still cannot escape the authorized root. */ +void file_set_trust_sender(bool enable); +bool file_get_trust_sender(void); + /* A configured fd without a canonical identity deliberately rejects paths. */ bool file_set_authorized_root(int fd, const char* canonical_path); diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 692cf6b..a3d2acb 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -595,8 +595,12 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi could escape the receive root (absolute, or relative-with-"..") is never materialized. It is contained (the entry is skipped) rather than failing the whole transfer, so a hostile sender can inject a broken symlink but - can never redirect it outside the root. */ - if (ok && !file_symlink_target_contained(target)) + can never redirect it outside the root. --trust-sender deliberately + relaxes this receiver-side re-validation: a trusted sender's escaping + symlink target is copied verbatim (rsync -l parity). The low-level + leaf/destination confinement in file_symlink_at_secure still ensures the + link itself is placed inside the authorized root. */ + if (ok && !file_get_trust_sender() && !file_symlink_target_contained(target)) ok = false; if (!ok) { /* Skip the escaping/empty target (contained) rather than abort. */ @@ -2076,7 +2080,7 @@ File* file_receive(const Config* config, int file_descriptor) { char* path = receive_str(file_descriptor); if (path == NULL) return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { + if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) { char* escaped_path = output_escape(path, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid received file path: %s", escaped_path ? escaped_path : ""); @@ -2136,7 +2140,7 @@ File* file_receive_directory(int file_descriptor) { char* path = receive_str(file_descriptor); if (path == NULL) return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { + if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) { char* escaped_path = output_escape(path, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid received directory path: %s", escaped_path ? escaped_path : ""); @@ -2163,7 +2167,7 @@ File* file_receive_hardlink(int file_descriptor) { char* path = receive_str(file_descriptor); if (path == NULL) return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { + if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) { char* escaped_path = output_escape(path, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid received hard-link path: %s", escaped_path ? escaped_path : ""); @@ -2182,7 +2186,7 @@ File* file_receive_hardlink(int file_descriptor) { free(path); return NULL; } - if (target[0] == '\0' || has_path_traversal(target)) { + if (target[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(target))) { char* escaped = output_escape(target, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid hard-link target path: %s", escaped ? escaped : ""); @@ -2213,7 +2217,7 @@ File* file_receive_symlink(int file_descriptor, const Config* config) { char* path = receive_str(file_descriptor); if (path == NULL) return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { + if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) { char* escaped_path = output_escape(path, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid received symlink path: %s", escaped_path ? escaped_path : ""); @@ -2268,7 +2272,7 @@ File* file_receive_special(int file_descriptor) { char* path = receive_str(file_descriptor); if (path == NULL) return NULL; - if (path[0] == '\0' || has_path_traversal(path)) { + if (path[0] == '\0' || (!file_get_trust_sender() && has_path_traversal(path))) { char* escaped_path = output_escape(path, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "Invalid received special path: %s", escaped_path ? escaped_path : ""); diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index d63d920..25141d8 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -75,52 +75,108 @@ static int parse_remote_dest(const char* dest, RemoteDest* r) { return 0; } -char* ssh_build_remote_command(const char* server_path, bool old_args) { +char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, + int remote_option_count) { const char* path = server_path ? server_path : "fastsync-server"; const char* suffix = " --stdio"; + + /* Each --remote-option=OPT is appended after " --stdio" as one shell word, + escaped with the SAME single-quote boundary used for the server path. This + stays safe even in --old-args mode (which leaves the server path unquoted): + remote options are always single-quoted individually, so a value containing + shell metacharacters (; & | ` $ ()) can never break out of the quoting to + inject an unrelated remote command. Values are already validated at CLI + parse time (non-empty, no control characters); this layer only adds the + escaping boundary. */ size_t path_len = strlen(path); size_t suffix_len = strlen(suffix); + /* The base command (server path, quoted unless --old-args, then " --stdio"). */ + size_t command_len; if (old_args) { if (path_len > SIZE_MAX - suffix_len - 1) return NULL; - char* command = malloc(path_len + suffix_len + 1); - if (!command) + command_len = path_len + suffix_len + 1; + } else { + size_t quote_count = 0; + for (const char* p = path; *p; p++) + if (*p == '\'') + quote_count++; + if (path_len > SIZE_MAX - suffix_len - 4 || + quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4) return NULL; - memcpy(command, path, path_len); - memcpy(command + path_len, suffix, suffix_len + 1); - return command; + command_len = path_len + quote_count * 4 + suffix_len + 4; } - /* Quote the executable as one remote-shell word. This is the default safety boundary. */ - size_t quote_count = 0; - for (const char* p = path; *p; p++) - if (*p == '\'') - quote_count++; - if (path_len > SIZE_MAX - suffix_len - 4 || - quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4) - return NULL; - size_t command_len = path_len + quote_count * 4 + suffix_len + 4; - char* command = malloc(command_len + 1); + /* Add each remote option, escaped as one single-quoted word: + " ''", i.e. 1 leading space + 1 open quote + body (len + 3 per + embedded single quote) + 1 close quote = len + q*3 + 3 bytes. + Defense-in-depth against a non-conforming caller: never forward an empty + or control-character value, independent of the CLI validation. */ + for (int i = 0; i < remote_option_count; i++) { + const char* opt = remote_options[i]; + if (!opt || opt[0] == '\0') + return NULL; + size_t len = 0, q = 0; + for (const char* p = opt; *p; p++) { + /* Defense-in-depth: never forward a control character (newline/CR/etc.) + that could break the single-quoted shell word regardless of the remote + shell, independent of the CLI validation. */ + if ((unsigned char)*p < 0x20 || (unsigned char)*p == 0x7f) + return NULL; + if (*p == '\'') + q++; + len++; + } + if (len > SIZE_MAX - q * 3 || len + q * 3 + 3 > SIZE_MAX - command_len) + return NULL; + command_len += len + q * 3 + 3; + } + command_len += 1; /* NUL */ + + char* command = malloc(command_len); if (!command) return NULL; char* out = command; - *out++ = '\''; - for (const char* p = path; *p; p++) { - if (*p == '\'') { - memcpy(out, "'\\''", 4); - out += 4; - } else { - *out++ = *p; + if (old_args) { + memcpy(out, path, path_len); + out += path_len; + memcpy(out, suffix, suffix_len + 1); + out += suffix_len; + } else { + *out++ = '\''; + for (const char* p = path; *p; p++) { + if (*p == '\'') { + memcpy(out, "'\\''", 4); + out += 4; + } else { + *out++ = *p; + } } + *out++ = '\''; + memcpy(out, suffix, suffix_len + 1); + out += suffix_len; } - *out++ = '\''; - memcpy(out, suffix, suffix_len + 1); + for (int i = 0; i < remote_option_count; i++) { + const char* opt = remote_options[i]; + *out++ = ' '; + *out++ = '\''; + for (const char* p = opt; *p; p++) { + if (*p == '\'') { + memcpy(out, "'\\''", 4); + out += 4; + } else { + *out++ = *p; + } + } + *out++ = '\''; + } + *out = '\0'; return command; } Client* client_connect_ssh(const char* destination, int port, const char* server_path, - bool old_args) { + bool old_args, char* const* remote_options, int remote_option_count) { RemoteDest r; if (parse_remote_dest(destination, &r) != 0) { char* escaped = output_escape(destination, false); @@ -190,7 +246,8 @@ Client* client_connect_ssh(const char* destination, int port, const char* server char* ssh_argv[16]; int ac = 0; char port_str[16]; - char* remote_command = ssh_build_remote_command(server_path, old_args); + char* remote_command = + ssh_build_remote_command(server_path, old_args, remote_options, remote_option_count); if (!remote_command) ssh_child_setup_failed(exec_pipe[1]); ssh_argv[ac++] = "ssh"; diff --git a/src/shared/transport_ssh.h b/src/shared/transport_ssh.h index e46c687..d406efc 100644 --- a/src/shared/transport_ssh.h +++ b/src/shared/transport_ssh.h @@ -4,7 +4,8 @@ #include "transport_tcp.h" Client* client_connect_ssh(const char* destination, int port, const char* server_path, - bool old_args); -char* ssh_build_remote_command(const char* server_path, bool old_args); + bool old_args, char* const* remote_options, int remote_option_count); +char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, + int remote_option_count); #endif diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index fb853f0..d288883 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -187,6 +187,20 @@ def setup_test_data(): class TestDryRun: + def test_trust_sender_transfer_completes(self, shared_server): + """--trust-sender is a receiver-local policy (never sent to the peer). + A transfer run with it must still complete and produce byte-identical + results: the receiver keeps its low-level root confinement, so a normal + trusted transfer is unchanged.""" + clean_dir(DEST_DIR) + received = get_dest_received_dir(DEST_DIR, SOURCE_DIR) + result, _ = run_client(SOURCE_DIR, DEST_DIR, + flags=["--trust-sender"], port=shared_server.port) + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + mismatches, missing = verify_transfer(SOURCE_DIR, received) + assert not missing, f"Missing files: {missing[:5]}" + assert not mismatches, f"Mismatched files: {mismatches[:5]}" + def test_human_readable_dry_run(self): result, dur = run_client(SOURCE_DIR, DEST_DIR, flags=["-h", "--dry-run"]) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:100]}" diff --git a/tests/integration/test_ssh.py b/tests/integration/test_ssh.py index 931dbfe..eac239e 100644 --- a/tests/integration/test_ssh.py +++ b/tests/integration/test_ssh.py @@ -142,3 +142,48 @@ class TestSSHFeatures: def test_preallocate(self): r = _run_ssh_test("SSH Preallocate (--preallocate)", ["--preallocate"]) assert r["status"] == "Success", r["error"] + + def test_trust_sender(self): + r = _run_ssh_test("SSH Trust Sender (--trust-sender)", ["--trust-sender"]) + assert r["status"] == "Success", r["error"] + + def test_remote_option_reaches_server(self): + """--remote-option=OPT appends OPT to the remote server command line and + the server honors it. Over SSH the server is launched without + --allow-delete, so a bare --delete is inert (nothing is removed). If + --remote-option=--allow-delete really reaches the remote server, the + receiver's deletion policy becomes permissive and the stale destination + file IS removed. Asserting the file is gone is therefore a positive + proof the forwarded option was honored by the server.""" + src = SOURCE_DIR + if os.path.exists(src): + shutil.rmtree(src) + os.makedirs(src) + with open(os.path.join(src, "keep.txt"), "w") as f: + f.write("kept\n") + with open(os.path.join(src, "stale.txt"), "w") as f: + f.write("stale\n") + received = get_dest_received_dir(DEST_DIR, SOURCE_DIR) + + # Initial push so the destination mirrors the source. + clean_dir(DEST_DIR) + ssh_dest = f"localhost:{DEST_DIR}" + base = CLIENT_CMD + [src, ssh_dest, "--save-to-disk", + "--fastsync-server-path", os.path.join(BUILD_DIR, "server")] + first = subprocess.run(base, text=True, capture_output=True) + assert first.returncode == 0, f"initial push failed: {(first.stderr or first.stdout)[:200]}" + assert os.path.exists(os.path.join(received, "stale.txt")) + + # Remove stale.txt from the source and re-push with --delete + + # --remote-option=--allow-delete. Forwarding --allow-delete to the + # server is what makes the deletion actually happen. + os.remove(os.path.join(src, "stale.txt")) + second = subprocess.run(base + ["--delete", "--remote-option=--allow-delete"], + text=True, capture_output=True) + assert second.returncode == 0, \ + f"second push failed: {(second.stderr or second.stdout)[:200]}" + assert not os.path.exists(os.path.join(received, "stale.txt")), ( + "stale.txt still present: --allow-delete (forwarded via " + "--remote-option) did not reach the remote server" + ) + assert os.path.exists(os.path.join(received, "keep.txt")) diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 758c528..cc681f6 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -2480,6 +2480,109 @@ static void test_parse_args_devices_specials() { config_delete(cfg); } +/* --trust-sender parses; default is false (receiver-local policy, off). */ +static void test_parse_args_trust_sender_default_false() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "--source-dir", "/src", "--dest-dir", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0); + EXPECT_FALSE(cfg->trust_sender); + config_delete(cfg); +} + +static void test_parse_args_trust_sender() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "--trust-sender", "--source-dir", "/src", "--dest-dir", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->trust_sender); + config_delete(cfg); +} + +/* --remote-option=OPT is repeatable and stores each value in order. */ +static void test_parse_args_remote_option_multiple() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + EXPECT_EQ_INT(cfg->remote_option_count, 0); + char* argv[] = {"fastsync", + "--source-dir", + "/src", + "--dest-dir", + "/dst", + "--remote-option=--allow-delete", + "--remote-option=--verbose"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 7, argv, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->remote_option_count, 2); + EXPECT_EQ_STR(cfg->remote_options[0], "--allow-delete"); + EXPECT_EQ_STR(cfg->remote_options[1], "--verbose"); + config_delete(cfg); +} + +/* Space-separated form "--remote-option OPT" also parses. */ +static void test_parse_args_remote_option_space_form() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "--source-dir", "/src", "--dest-dir", + "/dst", "--remote-option", "-v"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 7, argv, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->remote_option_count, 1); + EXPECT_EQ_STR(cfg->remote_options[0], "-v"); + config_delete(cfg); +} + +/* A missing argument bare --remote-option is rejected. */ +static void test_parse_args_remote_option_missing_value() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "--source-dir", "/src", "--dest-dir", "/dst", "--remote-option"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), -1); + config_delete(cfg); +} + +/* An empty --remote-option value and a value with control characters is + * rejected (the value would break the remote shell quoting). */ +static void test_parse_args_remote_option_rejects_bad_values() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "--source-dir", "/src", "--dest-dir", "/dst", "--remote-option="}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), -1); + EXPECT_EQ_INT(cfg->remote_option_count, 0); + + char* argv2[] = {"fastsync", "--source-dir", "/src", "--dest-dir", + "/dst", "--remote-option", "--bad\noption"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 7, argv2, positional_args, &positional_count), -1); + EXPECT_EQ_INT(cfg->remote_option_count, 0); + config_delete(cfg); +} + +/* A short -M form must NOT be accepted as --remote-option: -M stays FastSync + * metadata mode (documented divergence). */ +static void test_parse_args_remote_option_no_short_M() { + Config* cfg = valid_client_config(); + EXPECT_NOT_NULL(cfg); + /* -M followed by a remote-option-looking word still means metadata mode. */ + char* argv[] = {"fastsync", "-M", "-v", "--source-dir", "/src", "--dest-dir", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 7, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_metadata); + EXPECT_EQ_INT(cfg->remote_option_count, 0); + config_delete(cfg); +} + void test_client_cli() { test_validate_config_required_paths(); test_parse_args_numeric_ids(); @@ -2608,4 +2711,11 @@ void test_client_cli() { test_parse_args_delete_policy_invalid_values(); test_parse_args_max_delete_inert_without_delete(); test_parse_args_missing_args_flags(); + test_parse_args_trust_sender_default_false(); + test_parse_args_trust_sender(); + test_parse_args_remote_option_multiple(); + test_parse_args_remote_option_space_form(); + test_parse_args_remote_option_missing_value(); + test_parse_args_remote_option_rejects_bad_values(); + test_parse_args_remote_option_no_short_M(); } diff --git a/tests/test_config.c b/tests/test_config.c index 6e6462c..f00aa2b 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1226,15 +1226,70 @@ static void test_config_phase4_xattr_wire_roundtrip() { } } +/* --trust-sender defaults to OFF (a receiver-local policy). */ +static void test_config_trust_sender_default_false() { + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + EXPECT_FALSE(cfg->trust_sender); + EXPECT_NULL(cfg->remote_options); + EXPECT_EQ_INT(cfg->remote_option_count, 0); + config_delete(cfg); +} + +/* --trust-sender and --remote-option are LOCAL to the process that sets them: + * they must never cross the wire. After a round-trip the receiver observes the + * neutral defaults (trust_sender=false, no remote options), even when the + * sender had them set. */ +static void test_config_local_only_fields_not_serialized() { + int p[2]; + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + io_set_fds(p[0], p[1]); + io_set_bwlimit(0); + + pid_t pid = fork(); + if (pid == 0) { + close(p[1]); + io_set_fds(p[0], p[0]); + Config* recv = config_receive(p[0]); + bool ok = recv != NULL && !recv->trust_sender && recv->remote_options == NULL && + recv->remote_option_count == 0; + config_delete(recv); + close(p[0]); + _exit(ok ? 0 : 1); + } + + close(p[0]); + io_set_fds(p[1], p[1]); + Config* send_cfg = config_create(); + EXPECT_NOT_NULL(send_cfg); + send_cfg->trust_sender = true; + /* remote_options is client-side state; populate it like the CLI would. */ + send_cfg->remote_options = malloc(sizeof(char*)); + send_cfg->remote_options[0] = str_dup("--allow-delete"); + send_cfg->remote_option_count = 1; + send_cfg->send_directory = str_dup("/src"); + send_cfg->receive_root_directory = str_dup("/dst"); + bool sent = config_send(p[1], send_cfg); + int status; + waitpid(pid, &status, 0); + close(p[1]); + config_delete(send_cfg); + + EXPECT_TRUE(sent); + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); +} + void test_config() { test_config_lifecycle(); test_config_ssh_dest(); test_config_ssh_dest_local_path(); test_config_ssh_dest_no_user(); + test_config_trust_sender_default_false(); test_pipeline_sender_lifecycle(); test_pipeline_receiver_lifecycle(); if (!is_running_under_valgrind()) { test_config_send_receive(); + test_config_local_only_fields_not_serialized(); test_config_send_receive_version_mismatch(); test_config_receive_truncated(); test_config_string_null_vs_empty_roundtrip(); diff --git a/tests/test_transport_ssh.c b/tests/test_transport_ssh.c index 7a2f3d6..03ea79b 100644 --- a/tests/test_transport_ssh.c +++ b/tests/test_transport_ssh.c @@ -4,13 +4,13 @@ static void test_ssh_connect_invalid_dest_no_colon() { /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh("invalid-destination-no-colon", 22, NULL, false); + Client* client = client_connect_ssh("invalid-destination-no-colon", 22, NULL, false, NULL, 0); EXPECT_NULL(client); } static void test_ssh_connect_invalid_dest_empty() { /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh("", 22, NULL, false); + Client* client = client_connect_ssh("", 22, NULL, false, NULL, 0); EXPECT_NULL(client); } @@ -21,7 +21,7 @@ static void test_ssh_connect_malformed() { setenv("PATH", "", 1); /* cppcheck-suppress constVariablePointer */ - Client* client = client_connect_ssh(":", 22, NULL, false); + Client* client = client_connect_ssh(":", 22, NULL, false, NULL, 0); if (saved_path) { setenv("PATH", saved_path, 1); @@ -36,7 +36,7 @@ static void test_ssh_connect_malformed() { /* Test client_connect_ssh with valid format but unreachable host. * The function launches ssh which will fail to connect, returns a Client. */ static void test_ssh_connect_unreachable() { - Client* client = client_connect_ssh("nonexistent.invalid:/remote/path", 22, NULL, false); + Client* client = client_connect_ssh("nonexistent.invalid:/remote/path", 22, NULL, false, NULL, 0); if (client != NULL) { client_disconnect(client); client_delete(client); @@ -45,23 +45,71 @@ static void test_ssh_connect_unreachable() { } static void test_ssh_remote_command_argument_modes() { - char* command = ssh_build_remote_command("fast sync; touch /tmp/pwned", false); + char* command = ssh_build_remote_command("fast sync; touch /tmp/pwned", false, NULL, 0); EXPECT_EQ_STR(command, "'fast sync; touch /tmp/pwned' --stdio"); free(command); - command = ssh_build_remote_command("fast'sync", false); + command = ssh_build_remote_command("fast'sync", false, NULL, 0); EXPECT_EQ_STR(command, "'fast'\\''sync' --stdio"); free(command); - command = ssh_build_remote_command("fast sync; touch /tmp/pwned", true); + command = ssh_build_remote_command("fast sync; touch /tmp/pwned", true, NULL, 0); EXPECT_EQ_STR(command, "fast sync; touch /tmp/pwned --stdio"); free(command); } +/* --remote-option=OPT appends OPT to the remote command line after " --stdio", + * each escaped as its own single-quoted shell word. Metacharacters that could + * break out of the quoting are neutralized (never injected), matching the + * ssh_build_remote_command safety boundary for the server path. */ +static void test_ssh_remote_command_with_remote_options() { + char* noop[] = {"--allow-delete"}; + char* command = ssh_build_remote_command("fastsync-server", false, noop, 1); + EXPECT_EQ_STR(command, "'fastsync-server' --stdio '--allow-delete'"); + free(command); + + /* Multiple options append in order, each as its own quoted word. */ + char* multi[] = {"-v", "--allow-delete"}; + command = ssh_build_remote_command("srv", false, multi, 2); + EXPECT_EQ_STR(command, "'srv' --stdio '-v' '--allow-delete'"); + free(command); + + /* A remote option containing a single quote and shell metacharacters is + escaped with the same "'\''" boundary, so it stays one word and cannot + break out into an arbitrary remote command. */ + char* val = strdup("--x=un'der; touch /tmp/pwned"); + char* dangerous[1] = {val}; + command = ssh_build_remote_command("srv", false, dangerous, 1); + EXPECT_EQ_STR(command, "'srv' --stdio '--x=un'\\''der; touch /tmp/pwned'"); + free(command); + free(val); + + /* --old-args leaves the server path unquoted but still quotes remote options. */ + command = ssh_build_remote_command("srv", true, multi, 2); + EXPECT_EQ_STR(command, "srv --stdio '-v' '--allow-delete'"); + free(command); +} + +/* The remote command builder refuses to forward an empty or control-character + * remote option (defense-in-depth independent of the CLI validation). */ +static void test_ssh_remote_command_rejects_bad_options() { + char* empty[] = {""}; + EXPECT_NULL(ssh_build_remote_command("srv", false, empty, 1)); + + char nl = '\n'; + char* newline[] = {&nl}; + EXPECT_NULL(ssh_build_remote_command("srv", false, newline, 1)); + + char* with_null[] = {NULL}; + EXPECT_NULL(ssh_build_remote_command("srv", false, with_null, 1)); +} + void test_transport_ssh() { test_ssh_connect_invalid_dest_no_colon(); test_ssh_connect_invalid_dest_empty(); test_ssh_connect_malformed(); test_ssh_connect_unreachable(); test_ssh_remote_command_argument_modes(); + test_ssh_remote_command_with_remote_options(); + test_ssh_remote_command_rejects_bad_options(); }