diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b97d0a3..164f3b87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,13 @@ first added in the same Unreleased section is not an entry: edit that entry inst ### Changed - `tsk setup` names both installed skills, `tsk-cli` and `tsk-coordinator`, beside each agent's skills directory instead of printing only the `tsk-cli` file. +- A backup URL with a user name, such as `https://org@host/…`, no longer asks your credential manager. Drop `org@` from the URL ([credentials](https://gettsk.sh/docs/storage/#credentials)). ### Fixed - A bare token in an https backup URL is removed and masked even when the URL does not start at the scheme. - An open board shows the update notice as soon as the release check finishes, and a board left open checks again once a day. +- A token in a backup URL stays out of git's command line and the backup's local clone, so other local users cannot see it. ## v0.13.0 diff --git a/site/src/content/docs/docs/storage.md b/site/src/content/docs/docs/storage.md index 73db1b0c..49ba1615 100644 --- a/site/src/content/docs/docs/storage.md +++ b/site/src/content/docs/docs/storage.md @@ -273,6 +273,9 @@ Prefer ssh or a credential helper to a token in a backup URL. A URL such as `htt - `config.toml` keeps the URL as you wrote it, token included. - Backups never push the token: the pushed `config.toml` has it removed. +- git gets the token privately, never on its command line or in the backup's local clone. Other local users cannot see it in the process list. +- Any user name or token in the URL is the only login git tries for that target: your credential helper is not asked and stores nothing. +- For Azure DevOps or Bitbucket URLs that start with a user name, such as `https://org@dev.azure.com/org/p/_git/tsk-backup`, leave the `org@` part out of the backup URL. Your credential manager (Git Credential Manager, osxkeychain) then logs you in. - Every command prints the URL with its credentials as `***`, such as `https://***@github.com/me/tsk-backup.git`. `tsk backup remove` takes that form too, unless two targets share it: then give the number or the full URL. ## Restore diff --git a/src/backup.rs b/src/backup.rs index a2c98434..c3fa1549 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -770,9 +770,11 @@ pub fn run_probe(state_dir: &Path, spec: &str) -> Result<(), String> { .ok_or("no such target")?; let repos = repos_dir(state_dir); fsperm::ensure_private_dir(&repos).map_err(|error| error.to_string())?; - let mut command = git_command(&repos); - command.args(["ls-remote", "--heads", "--", &target.location]); - let found = match crate::git_base::bounded_process_output(command, "git", PROBE_WAIT) { + let found = match Remote::of(&target.location).and_then(|remote| { + let mut command = remote_command(&repos, &remote); + command.args(["ls-remote", "--heads", "--", &remote.url]); + run_git(command, PROBE_WAIT) + }) { Ok(output) if output.status.success() => "reached\n".to_string(), Ok(output) => format!( "error: {}\n", @@ -1184,7 +1186,7 @@ fn backup_to_git( let mut last_error = String::new(); for attempt in 0..PUSH_ATTEMPTS { if attempt > 0 { - git_ok(&clone.dir, &["fetch", "--quiet", "--prune", "origin"])?; + remote_git_ok(&clone, &["fetch", "--quiet", "--prune", "origin"])?; } let remote = format!("refs/remotes/origin/{}", clone.branch); let has_remote = git(&clone.dir, &["rev-parse", "--verify", "--quiet", &remote])? @@ -1255,8 +1257,8 @@ fn backup_to_git( if !ahead { return Ok(RunOutcome::UpToDate); } - let push = git( - &clone.dir, + let push = remote_git( + &clone, &[ "push", "--quiet", @@ -1377,6 +1379,7 @@ fn write_in_clone(clone: &Path, relative: &str, bytes: &[u8]) -> Result<(), Stri struct GitClone { dir: PathBuf, branch: String, + remote: Remote, } /// Clone the target under `backup-repos/` the first time, fetch it after. The branch is @@ -1392,15 +1395,19 @@ fn sync_clone(state_dir: &Path, target: &Target) -> Result { let repos = repos_dir(state_dir); fsperm::ensure_private_dir(&repos).map_err(|error| error.to_string())?; let dir = repos.join(target.id()); + let remote = Remote::of(&target.location)?; if dir.join(".git").is_dir() { - git_ok( - &dir, - &["remote", "set-url", "origin", "--", &target.location], - )?; - git_ok(&dir, &["fetch", "--quiet", "--prune", "origin"])?; + // Also rewrites a clone made before credentials stayed out of `.git/config`. + git_ok(&dir, &["remote", "set-url", "origin", "--", &remote.url])?; + let mut command = remote_command(&dir, &remote); + command.args(["fetch", "--quiet", "--prune", "origin"]); + let output = run_git(command, GIT_TIMEOUT)?; + if !output.status.success() { + return Err(git_failure("fetch", &output)); + } } else { let _ = fs::remove_dir_all(&dir); - let mut command = git_command(&repos); + let mut command = remote_command(&repos, &remote); command .args([ "clone", @@ -1409,10 +1416,10 @@ fn sync_clone(state_dir: &Path, target: &Target) -> Result { "--config", "core.symlinks=false", "--", - &target.location, + &remote.url, ]) .arg(&dir); - let output = crate::git_base::bounded_process_output(command, "git", GIT_TIMEOUT)?; + let output = run_git(command, GIT_TIMEOUT)?; if !output.status.success() { let _ = fs::remove_dir_all(&dir); return Err(git_failure("clone", &output)); @@ -1458,7 +1465,11 @@ fn sync_clone(state_dir: &Path, target: &Target) -> Result { .filter(|name| !name.is_empty()) }) .unwrap_or_else(|| "main".to_string()); - Ok(GitClone { dir, branch }) + Ok(GitClone { + dir, + branch, + remote, + }) } /// Every git call a backup makes. It runs in a detached child with no terminal, so it must @@ -1495,6 +1506,14 @@ fn git_command(dir: &Path) -> Command { ] { command.arg("-c").arg(setting); } + #[cfg(test)] + TEST_GIT.with(|test| { + if let Some((config, _)) = test.borrow().as_ref() { + for setting in config { + command.arg("-c").arg(setting); + } + } + }); command .env("GIT_TERMINAL_PROMPT", "0") .env("GCM_INTERACTIVE", "never"); @@ -1504,14 +1523,156 @@ fn git_command(dir: &Path) -> Command { command } +/// A shell credential helper that answers only for the target's host, with the user name and +/// password [`remote_command`] puts in its environment. It holds no secret itself: git's +/// command line is visible to every local user, its environment is not. +const CREDENTIAL_HELPER: &str = "!f() { test \"$1\" = get || return 0; h=; while IFS= read -r l; do case $l in host=*) h=${l#host=};; esac; done; if test \"$h\" = \"$TSK_BACKUP_HOST\"; then printf 'username=%s\\npassword=%s\\n' \"$TSK_BACKUP_USERNAME\" \"$TSK_BACKUP_PASSWORD\"; fi; }; f"; + +/// How git reaches a target: its URL without credentials, and the login the URL held. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Remote { + url: String, + login: Option, +} + +/// The user name and password of an http(s) URL, percent-decoded as git reads them, and the +/// host (with its port) they belong to. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Login { + host: String, + username: String, + password: String, +} + +impl Remote { + /// A bare `https://TOKEN@host` logs in as `TOKEN` with an empty password, as git would. + /// An ssh password is dropped: ssh never takes one from git. A login holding a line break + /// or NUL is refused: git's credential protocol is line based, so the rest of it would + /// read as another field or land in git's error message. + fn of(location: &str) -> Result { + let Some(url) = without_credentials(location) else { + return Ok(Self { + url: location.to_string(), + login: None, + }); + }; + let login = location.split_once("://").and_then(|(scheme, rest)| { + // The same scheme token [`credentials`] reads, so a URL it strips keeps its login. + let scheme = scheme_token(scheme); + let http = scheme.eq_ignore_ascii_case("http") || scheme.eq_ignore_ascii_case("https"); + let authority = rest.split(['/', '?', '#']).next()?; + let (userinfo, host) = authority.rsplit_once('@')?; + let (username, password) = userinfo.split_once(':').unwrap_or((userinfo, "")); + (http && !userinfo.is_empty()).then(|| Login { + host: host.to_string(), + username: percent_decoded(username), + password: percent_decoded(password), + }) + }); + if login.as_ref().is_some_and(|login| { + [&login.username, &login.password] + .iter() + .any(|part| part.contains(['\n', '\r', '\0'])) + }) { + return Err( + "the user name or password in the backup URL holds a line break or NUL, which git cannot take" + .into(), + ); + } + Ok(Self { url, login }) + } +} + +fn percent_decoded(text: &str) -> String { + let bytes = text.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + let hex = bytes + .get(index + 1..index + 3) + .filter(|_| bytes[index] == b'%') + .and_then(|hex| std::str::from_utf8(hex).ok()) + .and_then(|hex| u8::from_str_radix(hex, 16).ok()); + match hex { + Some(byte) => { + out.push(byte); + index += 3; + } + None => { + out.push(bytes[index]); + index += 1; + } + } + } + String::from_utf8_lossy(&out).into_owned() +} + +/// [`git_command`] for a call that reaches `remote`. A login goes to git through +/// [`CREDENTIAL_HELPER`] and the environment, never the command line or the clone's +/// `.git/config`, and is the only helper git asks: the user's own helpers would offer +/// another account's login first, or store this one. Without a login the user's helpers +/// answer as usual. +fn remote_command(dir: &Path, remote: &Remote) -> Command { + let mut command = git_command(dir); + if let Some(login) = &remote.login { + command + .args(["-c", "credential.helper="]) + .arg("-c") + .arg(format!("credential.helper={CREDENTIAL_HELPER}")) + .env("TSK_BACKUP_HOST", &login.host) + .env("TSK_BACKUP_USERNAME", &login.username) + .env("TSK_BACKUP_PASSWORD", &login.password); + } + command +} + +/// `-c` settings a test adds to every git call, and each call's arguments. +#[cfg(test)] +type TestGit = (Vec, Vec>); + +#[cfg(test)] +thread_local! { + /// Set by a test for git calls on its own thread. + static TEST_GIT: std::cell::RefCell> = const { std::cell::RefCell::new(None) }; +} + +fn run_git(command: Command, timeout: Duration) -> Result { + #[cfg(test)] + TEST_GIT.with(|test| { + if let Some((_, calls)) = test.borrow_mut().as_mut() { + calls.push( + command + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(), + ); + } + }); + crate::git_base::bounded_process_output(command, "git", timeout) +} + fn git(dir: &Path, args: &[&str]) -> Result { let mut command = git_command(dir); command.args(args); - crate::git_base::bounded_process_output(command, "git", GIT_TIMEOUT) + run_git(command, GIT_TIMEOUT) } fn git_ok(dir: &Path, args: &[&str]) -> Result { - let output = git(dir, args)?; + checked(args, git(dir, args)?) +} + +/// [`git`] on `clone` for a call that reaches its remote (see [`remote_command`]). +fn remote_git(clone: &GitClone, args: &[&str]) -> Result { + let mut command = remote_command(&clone.dir, &clone.remote); + command.args(args); + run_git(command, GIT_TIMEOUT) +} + +fn remote_git_ok(clone: &GitClone, args: &[&str]) -> Result { + checked(args, remote_git(clone, args)?) +} + +fn checked(args: &[&str], output: std::process::Output) -> Result { if output.status.success() { Ok(String::from_utf8_lossy(&output.stdout).into_owned()) } else { @@ -2862,7 +3023,7 @@ every = "daily" #[test] fn a_stored_error_never_holds_a_token() { - // git quotes the token outside a URL here. + // git would quote the token outside a URL here, had it been handed the token. let root = TempDir::new("stored-error"); let url = "git://me:S3CRET@127.0.0.1:9/x.git"; let state = state_with_target(&root.0, Path::new(url), ""); @@ -2876,7 +3037,7 @@ every = "daily" .expect("an error is recorded"); for error in [returned, stored, failure_message(&state).expect("shown")] { assert!(!error.contains("S3CRET"), "{error}"); - assert!(error.contains("***@127.0.0.1"), "{error}"); + assert!(error.contains("me@127.0.0.1"), "{error}"); } } @@ -3251,4 +3412,496 @@ every = "daily" ); assert_eq!(packs(), before, "no repack in a background run"); } + + /// Every file under `dir` that holds `needle`. + fn files_holding(dir: &Path, needle: &str) -> Vec { + let mut found = Vec::new(); + for entry in fs::read_dir(dir).expect("dir").flatten() { + let path = entry.path(); + if path.is_dir() { + found.extend(files_holding(&path, needle)); + } else if fs::read(&path).is_ok_and(|bytes| { + bytes + .windows(needle.len()) + .any(|window| window == needle.as_bytes()) + }) { + found.push(path); + } + } + found + } + + /// Runs `body` with git's calls on this thread recorded and `config` added to each, + /// returning every call's arguments. + fn recording_git(config: Vec, body: impl FnOnce()) -> Vec> { + TEST_GIT.with(|test| *test.borrow_mut() = Some((config, Vec::new()))); + body(); + TEST_GIT + .with(|test| test.borrow_mut().take()) + .map(|(_, calls)| calls) + .unwrap_or_default() + } + + #[test] + fn a_remote_keeps_its_login_out_of_the_url() { + let login = |host: &str, username: &str, password: &str| { + Some(Login { + host: host.into(), + username: username.into(), + password: password.into(), + }) + }; + assert_eq!( + Remote::of("https://me:s3cr%40et%2F@github.com:8443/me/b.git").expect("login"), + Remote { + url: "https://github.com:8443/me/b.git".into(), + login: login("github.com:8443", "me", "s3cr@et/"), + } + ); + assert_eq!( + Remote::of("https://ghp_TOKEN@github.com/me/b.git").expect("login"), + Remote { + url: "https://github.com/me/b.git".into(), + login: login("github.com", "ghp_TOKEN", ""), + } + ); + assert_eq!( + Remote::of(" https://ghp_TOKEN@github.com/me/b.git").expect("login"), + Remote { + url: " https://github.com/me/b.git".into(), + login: login("github.com", "ghp_TOKEN", ""), + } + ); + assert_eq!( + Remote::of("ssh://git:pw@host/x.git").expect("ssh"), + Remote { + url: "ssh://git@host/x.git".into(), + login: None, + } + ); + for plain in [ + "https://github.com/me/b.git", + "git@github.com:me/b.git", + "/srv/b.git", + ] { + assert_eq!( + Remote::of(plain).expect("plain"), + Remote { + url: plain.into(), + login: None, + } + ); + } + } + + #[test] + fn a_backup_never_puts_the_token_in_git_arguments_or_the_clone() { + let root = TempDir::new("token"); + let remote = bare_repo(&root.0); + let url = "https://me:s3cr%40et@127.0.0.1:1/b.git"; + let state = root.0.join("state"); + fs::create_dir_all(&state).expect("state"); + fs::write( + state.join("config.toml"), + format!("[[backup]]\nurl = {url:?}\n"), + ) + .expect("config"); + // The remote is served from disk under both spellings of its URL, so a build that + // still passes the token reaches it too and fails only on the token checks. + let config = ["https://127.0.0.1:1/b.git", url] + .iter() + .map(|https| format!("url.{}.insteadOf={https}", remote.display())) + .collect(); + let mut versions = Vec::new(); + let calls = recording_git(config, || { + assert_eq!( + run(&state, RunMode::One(0), at(DAY)).expect("run")[0].1, + Ok(RunOutcome::NoStore) + ); + write_store(&state, "a1"); + let results = run(&state, RunMode::Now, at(DAY + 1)).expect("run"); + assert!( + matches!(results[0].1, Ok(RunOutcome::Written(_))), + "{results:?}" + ); + let (listed, failures) = list_versions(&state, false).expect("versions"); + assert!(failures.is_empty(), "{failures:?}"); + versions = listed; + }); + assert!( + calls + .iter() + .any(|args| args.iter().any(|arg| arg == "clone")) + && calls + .iter() + .any(|args| args.iter().any(|arg| arg == "push")), + "{calls:?}" + ); + for args in &calls { + assert!( + !args + .iter() + .any(|arg| arg.contains("s3cr") && !arg.starts_with("url.")), + "token in git's arguments: {args:?}" + ); + } + assert_eq!(versions.len(), 1); + let repos = state.join(REPOS_DIR); + assert_eq!(files_holding(&repos, "s3cr"), Vec::::new()); + let clone = repos.join(Target::new(url.into()).id()); + assert_eq!( + git_in(&clone, &["remote", "get-url", "origin"]).trim(), + "https://127.0.0.1:1/b.git" + ); + } + + #[test] + fn an_old_clone_holding_the_token_is_rewritten_on_next_use() { + let root = TempDir::new("old-clone"); + let remote = bare_repo(&root.0); + let state = state_with_target(&root.0, &remote, ""); + run(&state, RunMode::One(0), at(DAY)).expect("run"); + // As a clone made before the fix: the target's URL with its token in `.git/config`. + let url = "https://me:s3cret@127.0.0.1:1/b.git"; + let clone = state + .join(REPOS_DIR) + .join(Target::new(remote.display().to_string()).id()); + let moved = state.join(REPOS_DIR).join(Target::new(url.into()).id()); + fs::rename(&clone, &moved).expect("move"); + git_in(&moved, &["remote", "set-url", "origin", url]); + fs::write( + state.join("config.toml"), + format!("[[backup]]\nurl = {url:?}\n"), + ) + .expect("config"); + // Nothing listens on port 1: the fetch fails, after the remote was rewritten. + let results = run(&state, RunMode::One(0), at(DAY + 1)).expect("run"); + assert!(results[0].1.is_err(), "{results:?}"); + let config = fs::read_to_string(moved.join(".git/config")).expect("config"); + assert!(!config.contains("s3cret"), "{config}"); + assert!( + config.contains("url = https://127.0.0.1:1/b.git"), + "{config}" + ); + } + + #[test] + fn the_credential_helper_gives_the_login_to_its_host_only() { + let root = TempDir::new("helper"); + let remote = Remote::of("https://me:s3cr%40et@example.com:8443/b.git").expect("login"); + let fill = |host: &str| { + let mut command = remote_command(&root.0, &remote); + command + .args(["credential", "fill"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()); + let mut child = command.spawn().expect("git"); + child + .stdin + .take() + .expect("stdin") + .write_all(format!("protocol=https\nhost={host}\n\n").as_bytes()) + .expect("write"); + let output = child.wait_with_output().expect("git"); + ( + output.status.success(), + String::from_utf8_lossy(&output.stdout).into_owned(), + ) + }; + let (filled, answer) = fill("example.com:8443"); + assert!(filled, "{answer}"); + assert!( + answer.contains("username=me\npassword=s3cr@et\n"), + "{answer}" + ); + let (filled, answer) = fill("elsewhere.example:8443"); + assert!(!filled && !answer.contains("s3cr"), "{answer}"); + } + + #[test] + fn the_users_own_helpers_answer_only_for_a_url_without_a_login() { + let root = TempDir::new("own-helper"); + let fill = |location: &str| { + let remote = Remote::of(location).expect("remote"); + let mut command = remote_command(&root.0, &remote); + command + .args(["credential", "fill"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()); + let mut child = command.spawn().expect("git"); + child + .stdin + .take() + .expect("stdin") + .write_all(b"protocol=https\nhost=example.com\n\n") + .expect("write"); + String::from_utf8_lossy(&child.wait_with_output().expect("git").stdout).into_owned() + }; + // The user's helpers, as their global config would set them: one for every URL and + // one for this host only. + for helper in ["credential.helper", "credential.https://example.com.helper"] { + let config = vec![format!( + "{helper}=!f() {{ echo username=mine; echo password=kept; }}; f" + )]; + let (plain, tokened, user_only) = ( + std::cell::RefCell::new(String::new()), + std::cell::RefCell::new(String::new()), + std::cell::RefCell::new(String::new()), + ); + recording_git(config, || { + *plain.borrow_mut() = fill("https://example.com/b.git"); + *tokened.borrow_mut() = fill("https://me:s3cret@example.com/b.git"); + *user_only.borrow_mut() = fill("https://alice@example.com/b.git"); + }); + let (plain, tokened, user_only) = ( + plain.into_inner(), + tokened.into_inner(), + user_only.into_inner(), + ); + // A user name alone is a login too (often a token): the user's helpers are never + // asked for its password. + assert!( + user_only.contains("username=alice\npassword=\n") && !user_only.contains("mine"), + "{helper}: {user_only}" + ); + assert!( + plain.contains("username=mine\npassword=kept\n"), + "{helper}: {plain}" + ); + assert!( + tokened.contains("username=me\npassword=s3cret\n") && !tokened.contains("mine"), + "{helper}: {tokened}" + ); + } + } + + /// A smart-http git server on 127.0.0.1 for the repositories under `root`, answering 401 + /// to any request without `user`:`password`, as a hosted remote does. Returns its port + /// and a ` ` line per request. + fn http_remote( + root: &Path, + user: &str, + password: &str, + ) -> (u16, std::sync::Arc>>) { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + use std::io::{BufRead, BufReader, Read}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("listen"); + let port = listener.local_addr().expect("addr").port(); + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let expected = format!("Basic {}", STANDARD.encode(format!("{user}:{password}"))); + let (root, served) = (root.to_path_buf(), log.clone()); + std::thread::spawn(move || { + for stream in listener.incoming().flatten() { + let mut reader = BufReader::new(stream.try_clone().expect("stream")); + let mut stream = stream; + let mut request = String::new(); + if reader.read_line(&mut request).is_err() { + continue; + } + let mut headers = std::collections::HashMap::new(); + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 || line.trim().is_empty() { + break; + } + if let Some((key, value)) = line.trim_end().split_once(": ") { + headers.insert(key.to_ascii_lowercase(), value.to_string()); + } + } + let header = |key: &str| headers.get(key).cloned().unwrap_or_default(); + let mut parts = request.split_whitespace(); + let (method, target) = ( + parts.next().unwrap_or("").to_string(), + parts.next().unwrap_or("").to_string(), + ); + let authorized = header("authorization") == expected; + served + .lock() + .expect("log") + .push(format!("{method} {target} {authorized}")); + if header("expect").eq_ignore_ascii_case("100-continue") { + let _ = stream.write_all(b"HTTP/1.1 100 Continue\r\n\r\n"); + } + let mut body = Vec::new(); + if header("transfer-encoding").eq_ignore_ascii_case("chunked") { + loop { + let mut size = String::new(); + let _ = reader.read_line(&mut size); + let size = usize::from_str_radix(size.trim(), 16).unwrap_or(0); + let mut chunk = vec![0; size + 2]; + let _ = reader.read_exact(&mut chunk); + if size == 0 { + break; + } + body.extend_from_slice(&chunk[..size]); + } + } else { + let length = header("content-length").parse().unwrap_or(0); + body.resize(length, 0); + let _ = reader.read_exact(&mut body); + } + if !authorized { + let _ = stream.write_all( + b"HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Basic realm=\"t\"\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ); + continue; + } + let (path, query) = target.split_once('?').unwrap_or((&target, "")); + let mut backend = Command::new("git") + .arg("http-backend") + .env("GIT_PROJECT_ROOT", &root) + .env("GIT_HTTP_EXPORT_ALL", "1") + .env("PATH_INFO", path) + .env("QUERY_STRING", query) + .env("REQUEST_METHOD", &method) + .env("CONTENT_TYPE", header("content-type")) + .env("CONTENT_LENGTH", body.len().to_string()) + .env("HTTP_CONTENT_ENCODING", header("content-encoding")) + .env("GIT_PROTOCOL", header("git-protocol")) + .env("REMOTE_USER", "me") + .env("REMOTE_ADDR", "127.0.0.1") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .expect("git http-backend"); + let mut stdin = backend.stdin.take().expect("stdin"); + let writer = std::thread::spawn(move || stdin.write_all(&body)); + let output = backend.wait_with_output().expect("http-backend"); + let _ = writer.join(); + let split = output + .stdout + .windows(4) + .position(|window| window == b"\r\n\r\n") + .unwrap_or(output.stdout.len()); + let (cgi, content) = output.stdout.split_at(split); + let content = content.get(4..).unwrap_or_default(); + let mut status = "200 OK".to_string(); + let mut response = String::new(); + for line in String::from_utf8_lossy(cgi).split("\r\n") { + match line.split_once(": ") { + Some((key, value)) if key.eq_ignore_ascii_case("status") => { + status = value.to_string() + } + Some(_) => response.push_str(&format!("{line}\r\n")), + None => {} + } + } + let _ = stream.write_all( + format!( + "HTTP/1.1 {status}\r\n{response}Content-Length: {}\r\nConnection: close\r\n\r\n", + content.len() + ) + .as_bytes(), + ); + let _ = stream.write_all(content); + } + }); + (port, log) + } + + #[test] + fn clone_probe_fetch_and_push_log_in_through_the_helper_alone() { + let root = TempDir::new("http-login"); + let remote = bare_repo(&root.0); + git_in(&remote, &["config", "http.receivepack", "true"]); + let (port, log) = http_remote(&root.0, "me", "s3cr@et"); + let url = format!("http://me:s3cr%40et@127.0.0.1:{port}/remote.git"); + let state = root.0.join("state"); + fs::create_dir_all(&state).expect("state"); + fs::write( + state.join("config.toml"), + format!("[[backup]]\nurl = {url:?}\n"), + ) + .expect("config"); + let calls = recording_git(Vec::new(), || { + // Clones the empty remote. + assert_eq!( + run(&state, RunMode::One(0), at(DAY)).expect("run")[0].1, + Ok(RunOutcome::NoStore) + ); + // `backup add`'s detached probe. + run_probe(&state, "0:1").expect("probe"); + assert_eq!( + fs::read_to_string(state.join(format!("{PROBE_FILE_PREFIX}1"))).expect("probe"), + "reached\n" + ); + // A push, then a fetch of the existing clone and another push. + for (index, title) in ["a1", "a2"].iter().enumerate() { + write_store(&state, title); + let results = run(&state, RunMode::Now, at(DAY + 1 + index as u64)).expect("run"); + assert!( + matches!(results[0].1, Ok(RunOutcome::Written(_))), + "{results:?}" + ); + } + }); + assert_eq!(commits(&remote), 2); + let log = log.lock().expect("log").clone(); + for (request, what) in [ + ( + "GET /remote.git/info/refs?service=git-upload-pack", + "clone and fetch", + ), + ("GET /remote.git/info/refs?service=git-receive-pack", "push"), + ("POST /remote.git/git-receive-pack", "push"), + ] { + assert!( + log.iter() + .filter(|line| line.starts_with(request) && line.ends_with(" true")) + .count() + >= 2, + "{what} logged in twice: {log:?}" + ); + } + assert!( + calls + .iter() + .any(|args| args.iter().any(|arg| arg == "ls-remote")), + "{calls:?}" + ); + for args in &calls { + assert!( + !args.iter().any(|arg| arg.contains("s3cr")), + "token in git's arguments: {args:?}" + ); + } + assert_eq!( + files_holding(&state.join(REPOS_DIR), "s3cr"), + Vec::::new() + ); + } + + #[test] + fn a_login_with_a_line_break_never_reaches_git() { + for url in [ + "https://me:a%0ATAIL@127.0.0.1:1/b.git", + "https://me:a%0DTAIL@127.0.0.1:1/b.git", + "https://me%00TAIL@127.0.0.1:1/b.git", + ] { + let root = TempDir::new("line-break"); + let state = root.0.join("state"); + fs::create_dir_all(&state).expect("state"); + fs::write( + state.join("config.toml"), + format!("[[backup]]\nurl = {url:?}\n"), + ) + .expect("config"); + let calls = recording_git(Vec::new(), || { + let results = run(&state, RunMode::One(0), at(DAY)).expect("run"); + let error = results[0].1.clone().expect_err("refused"); + assert!(error.contains("line break or NUL"), "{error}"); + assert!(!error.contains("TAIL"), "{error}"); + run_probe(&state, "0:1").expect("probe"); + let probe = + fs::read_to_string(state.join(format!("{PROBE_FILE_PREFIX}1"))).expect("probe"); + assert!( + probe.contains("line break or NUL") && !probe.contains("TAIL"), + "{probe}" + ); + }); + assert_eq!(calls, Vec::>::new(), "{url}"); + } + } } diff --git a/tests/cli_backup.rs b/tests/cli_backup.rs index 2439b7f6..d471ede9 100644 --- a/tests/cli_backup.rs +++ b/tests/cli_backup.rs @@ -333,8 +333,8 @@ fn a_row_number_is_not_sent_to_the_trash() { assert!(!message.contains("did you mean"), "{message}"); } -/// `tsk` with git told to fetch and push `url` from the local `repo`: a credential URL that -/// reaches a real repository without a network. +/// `tsk` with git told to fetch and push `url` from the local `repo`: a URL that reaches a +/// real repository without a network. git only ever sees a URL without its credentials. fn tsk_reaching(state: &Path, url: &str, repo: &str, args: &[&str]) -> Output { Command::new(env!("CARGO_BIN_EXE_tsk")) .args(args) @@ -358,7 +358,7 @@ fn credentials_in_a_target_url_are_never_printed() { let state = root.0.join("state"); add(&state, "kept private"); let run = |args: &[&str]| { - let output = tsk_reaching(&state, url, &repo, args); + let output = tsk_reaching(&state, "https://example.invalid/me/b.git", &repo, args); let text = format!("{}{}", stdout(&output), stderr(&output)); assert!( !text.contains("S3CRET"),