Fixes the findings from a 7-agent audit (security, C correctness, code quality, refactoring, repo hygiene, docs accuracy, tech debt), each independently re-verified against source. No wire change (PROTOCOL_VERSION stays 2.28.0).
Security
--temp-dir confinement — file_open_temp_dir() now resolves the opened fd (/proc/self/fd + realpath) and requires it within the authorized receive root; a planted symlink escaping the root is rejected (TOCTOU-safe, fail-closed). Regression test plants an escaping symlink.
setuid/setgid/sticky gating — client-controlled special bits are masked when super-user activities are not permitted (SUPER_MODE_OFF), across files, symlinks, dirs, special nodes, --chmod, and --fake-super. Daemon umask(022) instead of 0.
Credentials hardening — O_NOFOLLOW on secret files (symlinked store fails closed), with a literal /dev/fd/<n> / /proc/self/fd/<n> exemption so process substitution keeps working; FIFO reads now bound-wait (~3 s) so a connected-but-silent FIFO fails instead of hanging, while slow writers still work.
Correctness
Compression ceiling raised from 100 MiB to the 256 MiB protocol whole-file limit (-z on 100–256 MiB files now works; bomb guard retained).
--bwlimit now paces --sendfile on plaintext TCP (previously ignored; TLS path was already paced); sendfile poll() retries EINTR; SSL_read length clamped to INT_MAX.
--partial-dir implies --partial (rsync parity, empirically verified); --inplace + --partial-dir is now rejected like rsync.
Filter modifiers — x rejected everywhere; e/n/w/- accepted and consumed on merge/dir-merge (no longer leak into the merge filename) and rejected on non-merge rules. Row reclassified ✅ → ⚠️ (modifier semantics remain unimplemented).
Signal handlers made async-signal-safe (sigaction; server handler only _exits).
Refactors
Dead filter_rules_apply removed; four set_error helpers unified into utils_set_error; path_is_within deduped; Config**→Config*; dead --old-args plumbing removed; shared constants deduped; printf format attributes added and -Wformat-signedness enabled.
Strict -Werror build clean; clang-format 18 clean; cppcheck clean; unit 45/45 (ASan, UBSan, valgrind); full integration 883 passed; differential parity 62 passed; README-consistency test passes. Two independent reviews (comprehensive + C memory/thread-safety) found the credentials FIFO regression and the filter merge-modifier over-rejection — both fixed and re-verified.
## Codebase audit cycle — security, correctness, refactors, docs
Fixes the findings from a 7-agent audit (security, C correctness, code quality, refactoring, repo hygiene, docs accuracy, tech debt), each independently re-verified against source. **No wire change** (`PROTOCOL_VERSION` stays 2.28.0).
### Security
- **`--temp-dir` confinement** — `file_open_temp_dir()` now resolves the opened fd (`/proc/self/fd` + `realpath`) and requires it within the authorized receive root; a planted symlink escaping the root is rejected (TOCTOU-safe, fail-closed). Regression test plants an escaping symlink.
- **setuid/setgid/sticky gating** — client-controlled special bits are masked when super-user activities are not permitted (`SUPER_MODE_OFF`), across files, symlinks, dirs, special nodes, `--chmod`, and `--fake-super`. Daemon `umask(022)` instead of `0`.
- **Credentials hardening** — `O_NOFOLLOW` on secret files (symlinked store fails closed), with a literal `/dev/fd/<n>` / `/proc/self/fd/<n>` exemption so process substitution keeps working; FIFO reads now bound-wait (~3 s) so a connected-but-silent FIFO fails instead of hanging, while slow writers still work.
### Correctness
- **Compression ceiling** raised from 100 MiB to the 256 MiB protocol whole-file limit (`-z` on 100–256 MiB files now works; bomb guard retained).
- **`--bwlimit` now paces `--sendfile`** on plaintext TCP (previously ignored; TLS path was already paced); sendfile `poll()` retries EINTR; `SSL_read` length clamped to `INT_MAX`.
- **`--partial-dir` implies `--partial`** (rsync parity, empirically verified); `--inplace` + `--partial-dir` is now rejected like rsync.
- **Filter modifiers** — `x` rejected everywhere; `e`/`n`/`w`/`-` accepted and consumed on `merge`/`dir-merge` (no longer leak into the merge filename) and rejected on non-merge rules. Row reclassified ✅ → ⚠️ (modifier semantics remain unimplemented).
- **`config_create()` OOM leak**, **`errno`-after-`free`**, **`mutex_progress` leak**, **unchecked `str_dup` null-deref**, **`MAX_FILTER_RULES` client-side enforcement**, **unknown wire `Status` rejection**.
- **Signal handlers** made async-signal-safe (`sigaction`; server handler only `_exit`s).
### Refactors
Dead `filter_rules_apply` removed; four `set_error` helpers unified into `utils_set_error`; `path_is_within` deduped; `Config**`→`Config*`; dead `--old-args` plumbing removed; shared constants deduped; `printf` format attributes added and `-Wformat-signedness` enabled.
### Docs / repo hygiene
README build deps (zlib/lz4), `--delete` default, scanner order, `--progress`/`--stats`, undocumented flags; AGENTS deps/sanitizer/setpriv; RSYNC_COMPAT tally + rows; CHANGELOG/HANDOFF. Removed tracked scratch `test_data-manual/` and extended `.gitignore`.
### Verification
Strict `-Werror` build clean; clang-format 18 clean; cppcheck clean; unit 45/45 (ASan, UBSan, valgrind); full integration 883 passed; differential parity 62 passed; README-consistency test passes. Two independent reviews (comprehensive + C memory/thread-safety) found the credentials FIFO regression and the filter merge-modifier over-rejection — both fixed and re-verified.
MAX_DECOMPRESSED_SIZE was 100 MiB while the receiver advertises and the
sender compresses whole files up to MAX_RECEIVE_WHOLE_FILE_SIZE (256 MiB),
so -z on a 100-256 MiB regular file failed with 'Declared decompressed
size exceeds 104857600 bytes'. Define the internal bomb-guard ceiling in
terms of the protocol constant so the two bounds cannot drift, and add
unit coverage for a 130 MiB payload (accepted) and an over-ceiling
declared size (still rejected).
rsync 3.4.1 resolves --partial-dir after option parsing and sets keep_partial,
so --partial-dir=DIR alone retains an interrupted transfer's partial file.
FastSync only used the partial dir when --partial was also given, silently
discarding it otherwise.
Set Config->partial in cli_finalize_config whenever partial_dir is set.
Following rsync, an explicit --no-partial does NOT win (verified on rsync
3.4.1 in either option order); --inplace is guarded because it writes the
destination in place with no partial staging.
Tests: CLI unit coverage for the implication/precedence/inplace guard, and a
deterministic integration case that blocks the final install (non-empty
directory at the destination) and asserts the staged partial survives under
--partial-dir alone.
Three receiver security fixes from the audit:
1. --temp-dir symlink escape (High): file_open_temp_dir() opened the
client-controlled scratch dir with a bare open(), so a symlink planted
under the receive root let a peer redirect receiver scratch files
outside the authorized root. The opened dir is now judged by the REAL
path of its fd (via /proc/self/fd), and any target outside the
authorized receive root is refused with a logged error (EACCES). An
in-root symlink (the EXDEV cross-filesystem fallback case) still works,
and the no-root local batch path is unchanged.
2. setuid/setgid/sticky under SUPER_MODE_OFF (High): the special bits were
applied under --perms (and via --chmod) even when the connection forbade
super-user activities. FileAttrPolicy gains super_permitted, set by
file_attr_policy_from_config() from privilege_super_mode_permitted();
metadata_mode_for_policy(), the symlink path, the special-node creation
path, and the deferred directory-mode apply now strip the special bits
when it is false. Exact rsync semantics are preserved when permitted.
3. daemon umask (Low): daemonize() forced umask(0), so implied parent
directories created without -p were world-writable 0777. Set the
conventional daemon umask 022 instead (rsync never forces 0); -p/-a mode
preservation is unaffected because it restores modes via fchmod.
Tests: new unit tests for file_open_temp_dir confinement and the
masked/unmasked special-bit policy (incl. the --chmod path), a daemon
world-writable-dir regression test, an integration escape test, and a
root-only integration test asserting special bits are masked without
--allow-super. The old cross-filesystem test encoded the vulnerable
behavior (symlink target outside the root) and is replaced by the escape
test; the EXDEV fallback code is retained for in-root links.
- multiprocessing: destroy mutex_progress on the dir_entries_mutex
init-failure path (init >= 7); drop bogus log_perror
- delete_plan: capture errno before free() in apply_deferred_path
- queue/array_list: log_message instead of log_perror for non-errno
conditions
- protocol: reject unknown wire Status values via status_is_valid() in
receive_status, receive_status_timed and the keepalive reader; declare
protocol_receive_status_timed in protocol.h
- protocol: %llu for unsigned long long debug counters
- tests: out-of-range status rejection test
The standalone filter_rule_parse() already rejected the rsync xattr-name
'x' modifier, but the list parser used by --filter/-f silently dropped the
flag for merge/dir-merge rules (and relied on a second parse for plain
rules). Reject it explicitly in filter_list_parse_append_depth() with the
same diagnostic, so '-x', 'merge,x' and 'dir-merge,x' all fail cleanly.
Also reject the unimplemented rsync merge modifiers 'e', 'n' and 'w'
instead of folding them into the pattern, which previously produced
misleading errors such as "could not read merge file 'n file'". Only a
token made up solely of modifier characters is treated as a modifier run,
so glued patterns ('-newfile', '-e2e') and mixed tokens ("H,!secret")
keep their historical parsing.
Adds tests/test_filter.c with focused rejection and supported-syntax
cases.
Client: replace the non-async-signal-safe signal(3) call inside
client_signal_handler() with a precomputed SIG_DFL sigaction(2), which is
on the POSIX async-signal-safe list. The handler stays installed while a
transfer is armed so a repeated Ctrl-C still leads to a graceful abort
rather than a hard kill mid-cleanup.
Server: cleanup() now only calls _exit(2) (async-signal-safe). The former
server_delete()/daemon_conf_free()/credentials_free() teardown called
free()/close()/SSL_CTX_free() from signal context, which can deadlock or
corrupt the heap if the signal lands inside malloc/free. Handlers are
installed with sigaction(2) instead of signal(3). The normal shutdown
path in main() still performs the full teardown; the signal path relies on
process exit to reclaim the parent daemon's socket, anonymous shared
mapping and heap (no named/persistent parent resource is left behind).
secret_file_open() previously opened --password-file/--early-input with
plain O_RDONLY, so a symlinked path was followed before the owner/mode
fstat gate ran, and an empty/planted FIFO could block fgets forever.
Open with O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC (mirroring the dummy-key
sidecar): ELOOP now fails closed, and a writer-less FIFO yields EOF/EAGAIN
instead of hanging. Clear O_NONBLOCK again for regular files, where it is
a no-op, so their stdio read path is unchanged.
file.c: drop the duplicate <fcntl.h>/<unistd.h> includes (kept the first
occurrences).
Tests: a symlinked password file is rejected, and a writer-less named
FIFO fails cleanly without hanging.
secret_file_open() opened secret files with O_NONBLOCK and only cleared it
for S_ISREG, so on a FIFO/process-substitution source (--password-file
<(...), --early-input <(...)) fgets() failed immediately with EAGAIN when
the writer had not yet produced data, breaking slow producers.
Keep O_NONBLOCK at open() (a writer-less FIFO must not block the open) and
route all three readers through a new secret_read_line() helper. It
accumulates a line across reads and, on EAGAIN/EWOULDBLOCK (or a partial
line) with no newline and no EOF, clearerr()s and polls for readability
against one overall CLOCK_MONOTONIC deadline of
CREDENTIAL_FIFO_READ_TIMEOUT_MS (3000 ms); on timeout or a real read error
it fails with a clear message. EOF finishes normally. Regular files are
left blocking and read exactly as before.
Handles a line split across several write()s and keeps the owner/mode
fstat gate, O_NOFOLLOW and the /dev/fd/N exception unchanged.
The earlier modifier-rejection change rejected e/n/w on all rules, but rsync
3.4.1 accepts them (plus the '-' merge-only modifier) on merge and dir-merge
rules. Restrict the rejection to non-merge rules and consume the merge-file
modifiers (e/n/w/-) so they no longer leak into the merge filename.
- is_merge_rule()/is_merge_modifier_char() gate the merge-only modifiers.
- scan vs consume sets: e/n/w still count as modifier-run chars on every rule
(pure tokens like -new/-press stay rejected), but are only consumed on merge
rules, preserving mixed-token parsing such as H,!secret -> ecret.
- '-' is accepted/consumed only on merge/dir-merge (e.g. dir-merge,- .rules).
- x remains rejected everywhere with its dedicated message.
- e/n/w/- semantics remain unimplemented and are documented as accepted-but-
ignored in filter.h.
Tests: split the merge forms out of the rejection test into a new acceptance
test asserting the merge file is read and dir_merge_names keeps the modifier-
free basename; non-merge pure-modifier forms still rejected.
Update the docs for the audit follow-up fixes:
- --filter merge modifiers e/n/w/- are now accepted-and-consumed on
merge/dir-merge rules (rejected on non-merge, x rejected everywhere);
their semantics stay unimplemented, so the --filter row moves to Caveat
and the tally becomes 119/11/27 = 157.
- --inplace + --partial-dir is rejected with rsync's message.
- secret_file_open() O_NOFOLLOW (symlinked credential paths fail closed;
fd-backed paths exempt) and ~3 s bound-wait on FIFO reads.
- AGENTS setpriv wording corrected to the collected instance count.
- CHANGELOG [Unreleased] audit section extended with the follow-ups.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Codebase audit cycle — security, correctness, refactors, docs
Fixes the findings from a 7-agent audit (security, C correctness, code quality, refactoring, repo hygiene, docs accuracy, tech debt), each independently re-verified against source. No wire change (
PROTOCOL_VERSIONstays 2.28.0).Security
--temp-dirconfinement —file_open_temp_dir()now resolves the opened fd (/proc/self/fd+realpath) and requires it within the authorized receive root; a planted symlink escaping the root is rejected (TOCTOU-safe, fail-closed). Regression test plants an escaping symlink.SUPER_MODE_OFF), across files, symlinks, dirs, special nodes,--chmod, and--fake-super. Daemonumask(022)instead of0.O_NOFOLLOWon secret files (symlinked store fails closed), with a literal/dev/fd/<n>//proc/self/fd/<n>exemption so process substitution keeps working; FIFO reads now bound-wait (~3 s) so a connected-but-silent FIFO fails instead of hanging, while slow writers still work.Correctness
-zon 100–256 MiB files now works; bomb guard retained).--bwlimitnow paces--sendfileon plaintext TCP (previously ignored; TLS path was already paced); sendfilepoll()retries EINTR;SSL_readlength clamped toINT_MAX.--partial-dirimplies--partial(rsync parity, empirically verified);--inplace+--partial-diris now rejected like rsync.xrejected everywhere;e/n/w/-accepted and consumed onmerge/dir-merge(no longer leak into the merge filename) and rejected on non-merge rules. Row reclassified ✅ → ⚠️ (modifier semantics remain unimplemented).config_create()OOM leak,errno-after-free,mutex_progressleak, uncheckedstr_dupnull-deref,MAX_FILTER_RULESclient-side enforcement, unknown wireStatusrejection.sigaction; server handler only_exits).Refactors
Dead
filter_rules_applyremoved; fourset_errorhelpers unified intoutils_set_error;path_is_withindeduped;Config**→Config*; dead--old-argsplumbing removed; shared constants deduped;printfformat attributes added and-Wformat-signednessenabled.Docs / repo hygiene
README build deps (zlib/lz4),
--deletedefault, scanner order,--progress/--stats, undocumented flags; AGENTS deps/sanitizer/setpriv; RSYNC_COMPAT tally + rows; CHANGELOG/HANDOFF. Removed tracked scratchtest_data-manual/and extended.gitignore.Verification
Strict
-Werrorbuild clean; clang-format 18 clean; cppcheck clean; unit 45/45 (ASan, UBSan, valgrind); full integration 883 passed; differential parity 62 passed; README-consistency test passes. Two independent reviews (comprehensive + C memory/thread-safety) found the credentials FIFO regression and the filter merge-modifier over-rejection — both fixed and re-verified.The standalone filter_rule_parse() already rejected the rsync xattr-name 'x' modifier, but the list parser used by --filter/-f silently dropped the flag for merge/dir-merge rules (and relied on a second parse for plain rules). Reject it explicitly in filter_list_parse_append_depth() with the same diagnostic, so '-x', 'merge,x' and 'dir-merge,x' all fail cleanly. Also reject the unimplemented rsync merge modifiers 'e', 'n' and 'w' instead of folding them into the pattern, which previously produced misleading errors such as "could not read merge file 'n file'". Only a token made up solely of modifier characters is treated as a modifier run, so glued patterns ('-newfile', '-e2e') and mixed tokens ("H,!secret") keep their historical parsing. Adds tests/test_filter.c with focused rejection and supported-syntax cases.