From e1bb2e923375ee8f01e78a9c6026356c95bc17ff Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 15:33:54 +0200 Subject: [PATCH] chore(p8h): reconcile ssh old-args docs, secret-file perms docs; fix test cppcheck --- RSYNC_COMPAT.md | 6 +++--- src/shared/transport_ssh.h | 9 ++++++--- tests/test_file.c | 3 +++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index e8c19ef..44f4baa 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -611,7 +611,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | Flag | Rsync Description | FastSync Status | Notes | |------|-------------------|-----------------|-------| | `-e`, `--rsh=COMMAND` | Remote shell to use | ✅ Implemented | `-e`/`--rsh` (and `--rsh=COMMAND`) select the remote-shell program used to build the SSH child argv, overriding the default `ssh`. The command is whitespace-split into the leading argv words so rsync's `-e "ssh -p 2222"` works; the standard `-o` family, an optional `-p` port, `user@host` and the quoted remote command (`fastsync-server --stdio`) follow. Stored in the `rsh_command` config field. **Client-only, never crosses the wire** (it is a launch concern, not a handshake property) | -| `--rsync-path=PROGRAM` | rsync binary on remote | ✅ Implemented | Alias for `--fastsync-server-path`: both write the `fastsync_server_path` config field used as the remote-side server program (quoted as one remote-shell word unless `--old-args`), which CROSSES the wire as before. Kept separate from `--rsh`, which names the local connecting program | +| `--rsync-path=PROGRAM` | rsync binary on remote | ✅ Implemented | Alias for `--fastsync-server-path`: both write the `fastsync_server_path` config field used as the remote-side server program (always quoted as one remote-shell word), which CROSSES the wire as before. Kept separate from `--rsh`, which names the local connecting program | | `--port=PORT` | Alternate daemon port | ✅ Implemented | rsync's daemon-port flag maps to the client-side `server_port` config field: a client connects to a TCP/TLS server (incl. `host::module/path` daemon destinations) with `--server-port`, and the `fastsync-server --daemon` listener's port is taken from its config's `port` key (default 873) or overridden by `--dparam port=` / `-p` | | `--sockopts=OPTIONS` | Custom TCP options | ✅ Implemented | Comma-separated allowlist of `OPT=VAL` applied via `setsockopt` after `socket()` before `connect()`/`bind()`. Only `TCP_NODELAY`, `SO_KEEPALIVE`, `SO_REUSEADDR` (0/1) and `SO_RCVBUF`/`SO_SNDBUF` (byte count) are accepted; an unknown option name or a bad value is rejected up front, never silently ignored. A value is required for every option (`OPT=VAL`; a bare name is an error). Applied to the outgoing TCP and TLS client socket; absent by default. `SockOptEntry`/`sockopts` config fields. Local socket concern: never crosses the wire | | `--blocking-io` | Use blocking I/O for remote shell | ✅ Implemented | With `--blocking-io` the SSH-transport socketpair socket is left without `SO_RCVTIMEO`/`SO_SNDTIMEO`, so the transfer blocks naturally; by default it gets the same read/write timeout as the TCP transport (see `--timeout`). `blocking_io` config bool. **Client-only, never crosses the wire** | @@ -629,7 +629,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | `--config=FILE` | Alternate rsyncd.conf file | ✅ Implemented | Wave A: selects the daemon config file. Default when omitted (in `--daemon` mode): `~/.config/fastsync/fastsyncd.conf` if it exists, else `/etc/fastsyncd.conf`. The grammar is FastSync-native (documented in the Daemon Mode notes below) and strictly rejects unknown keys so a typo can never silently change what a module serves; requires `--daemon` | | `--dparam=OVERRIDE` | Override global daemon config | ✅ Implemented | Wave A: overrides one global scalar from the command line (`--dparam port=8734` and `--dparam=KEY=VALUE` both work). Limited to the global scalar keys the grammar defines (`port`, `motd file`, `address`); keys are case-insensitive and unknown keys/invalid values are rejected. Requires `--daemon` | | `--no-detach` | Don't detach from parent | ✅ Implemented | Wave A: with `--daemon`, keeps the listener in the foreground (what integration tests use). Without it the daemonizes (fork/setsid, stdio redirected to /dev/null) after the listening socket is bound. Requires `--daemon` | -| `--password-file=FILE` | Read daemon password from file | ✅ Implemented | Wave B daemon auth. Client: `--password-file` supplies `user:password` for a `host::module/path` destination (the username is taken from this file, so `user@host::module` stays rejected). Server (`fastsync-server --daemon --password-file FILE`): the credential store that modules with `auth users` are verified against. Only a SHA-256 digest of the password ever crosses the wire or is stored server-side; the literal password never appears in logs. See the Daemon Mode notes below for the file formats and the plaintext/TLS caveat | +| `--password-file=FILE` | Read daemon password from file | ✅ Implemented | Wave B daemon auth. Client: `--password-file` supplies `user:password` for a `host::module/path` destination (the username is taken from this file, so `user@host::module` stays rejected). Server (`fastsync-server --daemon --password-file FILE`): the credential store that modules with `auth users` are verified against. Only a SHA-256 digest of the password ever crosses the wire or is stored server-side; the literal password never appears in logs. The file must be private to its owner: both the client and server refuse to load a `--password-file`/`--early-input` that grants any group/other permission bit (mode 0600), mirroring the TLS private-key check. See the Daemon Mode notes below for the file formats and the plaintext/TLS caveat | | `--early-input=FILE` | Use FILE for daemon early exec | ✅ Implemented | Server-only (requires `--daemon`): a second credential-store file, same `user:SHA256HEX` grammar as `--password-file`, read before the listener accepts connections (a secrets-manager / process-substitution source). Its entries layer over `--password-file`: identical entries dedupe, a conflicting secret for the same user is a startup error. A daemon whose modules declare `auth users` must be given at least one of the two, or it refuses to start (fail closed) | **Daemon Mode notes (Wave A, protocol 2.15.0; Wave B auth, Wave C MOTD, no bump):** FastSync daemon mode is supported in FastSync's own protocol/config grammar, not rsync's SMB/daemon option encoding. @@ -657,7 +657,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | 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 | ✅ 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 | +| `--old-args` | Disable modern arg protection | ✅ Implemented | SSH-only; accepted for CLI compatibility but is now a **documented no-op**: FastSync always single-quote-escapes the remote server path and each `--remote-option` value (`ssh_build_remote_command`), so a metacharacter-bearing `--rsync-path` can never be interpreted by the remote shell. The flag no longer disables that quoting (the old raw-construction behavior was an injection foot-gun and is removed); the safety-relevant behavior is identical either way | | `--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 | diff --git a/src/shared/transport_ssh.h b/src/shared/transport_ssh.h index 7458961..08908ce 100644 --- a/src/shared/transport_ssh.h +++ b/src/shared/transport_ssh.h @@ -6,10 +6,13 @@ Client* client_connect_ssh(const char* destination, int port, const char* server_path, bool old_args, const char* rsh_command, bool blocking_io, char* const* remote_options, int remote_option_count); -/* Build the escaped remote-shell command string (the server program path quoted - * as one remote-shell word unless --old-args, followed by ` --stdio` and each +/* Build the escaped remote-shell command string (the server program path always + * quoted as one remote-shell word, followed by ` --stdio` and each * --remote-option value appended as an individually single-quoted shell word). - * Every --remote-option value is individually escaped with the '\'' sequence and + * `old_args` is accepted for CLI/ABI compatibility but no longer disables + * quoting: the path is always escaped so a metacharacter-bearing + * --rsync-path can never be interpreted by the remote shell. Every + * --remote-option value is individually escaped with the '\'' sequence and * values with empty/control characters are rejected at the CLI parse layer. */ char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, int remote_option_count); diff --git a/tests/test_file.c b/tests/test_file.c index e7088c3..fd715fc 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -1429,8 +1429,10 @@ static void test_keep_dirlinks_secure_open_impl() { int parent_fd = file_open_secure_parent(path, &leaf, false); EXPECT_TRUE(parent_fd >= 0); EXPECT_NOT_NULL(leaf); + // cppcheck-suppress knownConditionTrueFalse if (leaf) EXPECT_EQ_STR(leaf, "file.txt"); + // cppcheck-suppress knownConditionTrueFalse if (parent_fd >= 0) { struct stat st; EXPECT_EQ_INT(fstat(parent_fd, &st), 0); @@ -1444,6 +1446,7 @@ static void test_keep_dirlinks_secure_open_impl() { leaf = NULL; parent_fd = file_open_secure_parent(path, &leaf, false); EXPECT_TRUE(parent_fd >= 0); + // cppcheck-suppress knownConditionTrueFalse if (parent_fd >= 0) { struct stat st; EXPECT_EQ_INT(fstat(parent_fd, &st), 0);