2e8fe285

Cover the control-socket overflow that broke a sync outright

a73x 2026-09-06 08:53

Commit message

Cover the control-socket overflow that broke a sync outright

tests/sync_ssh_connection_test.rs

@@ @@ -1,7 +1,41 @@ @@
+mod common;
+use std::path::Path;
+use std::process::Output;
+use common::ServerHarness;
+/// Set a repo up with an `srv` SSH remote, collab refspec, autosync off and one
+/// issue to push, then run one sync with `TMPDIR` pointed at `tmp`.
+fn sync_with_tmpdir(harness: &ServerHarness, name: &str, tmp: &Path) -> Output {
+    let key = harness.ssh_client_key();
+    harness.push_head();
+    let repo = harness.work_repo();
+    let url = harness.ssh_url_for(harness.repo_name());
+    repo.git(&["remote", "add", "srv", &url]);
+    repo.git(&[
+        "config",
+        "--add",
+        "remote.srv.fetch",
+        "+refs/collab/*:refs/collab/sync/srv/*",
+    ]);
+    repo.git(&["config", "collab.autoSync", "false"]);
+    repo.issue_open(name);
+    let ssh = format!(
+        "ssh -i {} -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+         -o IdentitiesOnly=yes -o BatchMode=yes -o ConnectTimeout=5",
+        key.display()
+    );
+    repo.cli_command()
+        .args(["sync", "--remote", "srv"])
+        .env("GIT_SSH_COMMAND", &ssh)
+        .env("TMPDIR", tmp)
+        .output()
+        .expect("failed to run git-collab sync")
+}
+/// A sync fetches and then pushes. Run as two independent `git` invocations
+/// those are two SSH connections, each paying a handshake, an auth round and
+/// whatever the network charges. The push should ride the connection the
@@ @@ -59,3 +93,61 @@ fn a_sync_authenticates_to_the_server_once() { @@
+        "one sync should log in once, saw {logins} logins:\n{log}"
+    );
+/// A `TMPDIR` too long to hold a control socket must cost the *sharing*, not
+/// the sync.
+///
+/// This is the regression test for the bug the sharing test above was masking
+/// (fixed 2026-09-06): `ssh_share::socket_dir` budgeted for the 40-character
+/// `%C` hash but not for the `.XXXXXXXXXXXXXXXX` ssh appends while bringing the
+/// master up, so a directory of 51-59 bytes passed the guard, ssh refused the
+/// path it was handed, and `git fetch` died with exit 128. A sync that cannot
+/// share has to fall back to two connections and still finish.
+#[test]
+fn a_tmpdir_too_long_for_a_control_socket_still_syncs() {
+    let harness = ServerHarness::new("long-tmpdir");
+    // The length matters, and "long" is not enough: a *very* long TMPDIR was
+    // refused by the old budget too, so a fixture like that passes either way
+    // and proves nothing. The bug lived in a band — paths the old budget
+    // accepted and ssh then refused — and this fixture has to sit in it.
+    //
+    // The socket directory is `<TMPDIR>/git-collab-ssh-<pid>`. At 34 bytes of
+    // TMPDIR that directory is 55-57 bytes for any pid of 5 to 7 digits, so
+    // the old budget (dir + "/" + 40) came to 96-98 and passed, while the name
+    // ssh actually opens (another 17 bytes) is 113-115 and exceeds every
+    // platform's `sun_path`. Built under /tmp so the length is this test's
+    // choice and not the environment's.
+    const TARGET_TMPDIR_LEN: usize = 34;
+    let root = tempfile::TempDir::new_in("/tmp").expect("temp dir under /tmp");
+    let pad = TARGET_TMPDIR_LEN
+        .checked_sub(root.path().as_os_str().len() + "/".len())
+        .expect("temp root is already longer than the target length");
+    let long = root.path().join("d".repeat(pad));
+    std::fs::create_dir_all(&long).unwrap();
+    assert_eq!(
+        long.as_os_str().len(),
+        TARGET_TMPDIR_LEN,
+        "this fixture only reproduces the bug at exactly this length"
+    );
+    let output = sync_with_tmpdir(&harness, "Synced without sharing", &long);
+    assert!(
+        output.status.success(),
+        "a sync that cannot share a connection must still succeed, got {:?}:\n{}{}",
+        output.status.code(),
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr),
+    );
+    // And it really did decline to share: fetch and push logged in separately.
+    // Asserting the count, not just success, keeps this from passing for the
+    // wrong reason if sharing ever starts working under a long path.
+    let log = harness.server_log();
+    let logins = log.matches("Public key auth accepted").count();
+    assert_eq!(
+        logins, 2,
+        "expected the unshared fallback to log in twice, saw {logins}:\n{log}"
+    );
+}