Cherry-Sync drives rsync and ssh against user-supplied paths, some of which may be untrusted (typed by a careless user, expanded from an empty shell variable, or — eventually — read from a remote file list). This document is the single source of truth on the tool's security posture: the invariants every change must hold, the trust boundaries we do and don't defend, and the catalog of concerns we've found so far.
It is deliberately principle-first: it records threats and the invariants that counter them, not the code that implements them — so a reviewer reasons from the threat model rather than from our current implementation. New concerns join the catalog as they surface; closed ones are held closed by regression tests.
These are non-negotiable. A change that breaks one is a bug, even if every test still passes.
- No shell, ever. Always invoke external commands with
exec.Commandand an argument slice. Never build a command string and hand it tosh -c(orbash -c, etc.). Withexec.Commandthe argv goes straight toexecve, so shell metacharacters (;, backticks,$(…),&&, quotes) in a path are inert — they reachrsyncas literal bytes in one argument. This is why the classic "shell injection" class does not apply to us, and it must stay that way. - An end-of-options (
--) separator before user paths.rsyncparses options anywhere on its command line, so a path beginning with-would otherwise be read as an option. The danger is concrete: rsync's remote-shell option (-e/--rsh) executes an arbitrary command on the far side. A--end-of-options marker immediately before the user-supplied paths forecloses this — everything after it is a path, never an option. - Validate every path operand before use. At minimum, reject empty strings: an empty path becomes
"" + "/"="/", pointingrsyncat the filesystem root. Validation belongs at the input boundary; surface failures as a usage error (exit 2), not a partial run. - NUL-delimit any file list handed to rsync. When a transfer is driven from a list of paths, delimit it with NUL bytes rather than newlines. The default newline delimiter lets a filename containing a newline (legal on Unix) smuggle extra entries into the list. (Not yet implemented — see the catalog.)
What we defend, and what we explicitly don't:
- Local invocation is trusted-ish, but inputs are not. We assume the person running
csyncis authorized, but we do not assume their arguments are well-formed. Untrusted-input handling (the invariants above) is about robustness and avoiding footguns, not about defending against a hostile local user — who could just runrsyncdirectly. - SSH is the trusted transport. We rely on the user's existing
sshconfiguration and host keys. We do not add our own authentication, key management, or host-key verification on top; that isssh's job. - A hostile remote host is out of scope. If the remote is compromised, it can return misleading dry-run output or malicious file contents. We surface
rsync's errors verbatim rather than swallowing them, but we do not attempt to sandbox or validate what the remote sends. A user syncing with a box they don't control is outside our threat model. - No secret handling.
csyncstores no credentials and prints no secrets; authentication is delegated entirely tossh.
- rsync argument injection via leading-dash paths. A source or destination beginning with
-parsed as an rsync option — worst case, a remote-shell option executing an arbitrary command on the far side. Closed by the end-of-options separator invariant, and held closed by a behavioral test that fails if the separator is removed. - Empty path → filesystem root. An empty path operand would expand to
"/", retargeting the operation at the root of the filesystem. Closed by the path-validation invariant (empty-string rejection), held closed by unit and behavioral regression tests. The operand normalization ininternal/operandpreserves this invariant: a path that reduces to nothing after resolving a~home shortcut or collapsing trailing slashes (a bare~, ahost:with no path, or a run of slashes) becomes"."— the directory itself — never the empty string. - Shell injection. Not applicable: we use
exec.Commandwith an argv slice, neversh -c. Recorded here so the question doesn't get re-litigated — the defense is the "No shell, ever" invariant, not a patch. - Filter-rule metacharacter in a deletion path. A selected deletion is applied with an rsync filter rule (
--delete --include=<path> … --exclude='*'), in which*,?, and[are glob metacharacters — an unescaped one in a filename would make the include match, and--deleteremove, files the user never chose. The filenames themselves still reach rsync as inert argv entries (the "No shell, ever" invariant holds), but rsync's own pattern matcher reinterprets them. Closed for now by dropping any delete candidate whose name contains such a character — and any leading space, which the*deletingitemize parser can't reliably tell apart from the code column's padding — at detection, so it is never offered or applied; held closed by unit and behavioral tests. Making those names removable (escaping the metacharacters, parsing the leading-space case by fixed column width) is deferred to a tracked issue.
- Newline in a filename smuggling into the transfer list. Harmless on the current compare-only path, but the v0.1 transfer phase will drive
rsyncfrom a list of paths, newline-delimited by default. Mitigation: NUL-delimited paths (see invariant). Address when the transfer phase lands. - Whitespace-only / all-blank paths. We currently reject only the exactly empty string.
" "becomes" /", which is not filesystem root, so we left it alone to avoid rejecting legitimate-if-weird paths. Revisit if it proves to be a real footgun. - Path literally
--. With the end-of-options separator in place, a user path of--resolves to a literal path named--. Believed harmless; noted for completeness.
gosec runs as a deterministic gate in the lefthook pre-push hook — static analysis, no LLM, no session bias, and it can't be forgotten because it gates the push itself. It flags any exec.Command with variable arguments as G204.
The single rsync invocation with variable arguments carries a justified, per-site #nosec G204: its safety rests on the invariants above and is proven by a behavioral test that goes red if the end-of-options separator is removed.
Suppress G204 only per-site, with a justification that points at the reasoning. Never disable it globally — that would blind the gate to the same pattern everywhere else in the tree.