diff --git a/crates/fastskill-core/src/storage/git_isolated.rs b/crates/fastskill-core/src/storage/git_isolated.rs index 2c436b28..bcdd4816 100644 --- a/crates/fastskill-core/src/storage/git_isolated.rs +++ b/crates/fastskill-core/src/storage/git_isolated.rs @@ -16,6 +16,12 @@ //! - is bounded by one overall deadline, a byte cap and a file cap, checked //! while git runs, and killed when any is crossed. //! +//! A caller that holds a credential for the repository -- a short-lived token +//! minted for this one clone, say -- passes it as +//! [`IsolatedCloneOptions::authorization`]. It reaches git through the clone's +//! environment as an `http..extraHeader`, scoped to the cloned URL, and +//! never through the command line, the URL or a file. +//! //! The result is the checked-out tree at one commit, with its `.git` removed. //! Symbolic links stay as they are in the repository: deciding whether to //! accept them is the caller's job, and @@ -79,6 +85,39 @@ pub struct IsolatedCloneOptions { pub follow_redirects: bool, /// Where to create the clone; the system temporary directory if `None`. pub parent_dir: Option, + /// The `Authorization` header git sends with its HTTP requests under the + /// cloned URL, if any. + pub authorization: Option, +} + +/// The value of an HTTP `Authorization` header for one clone, such as +/// `Basic `. Its `Debug` output never shows the value. +#[derive(Clone, PartialEq, Eq)] +pub struct CloneAuthorization(String); + +impl CloneAuthorization { + /// `value` as a header value: printable ASCII, so it can't end the header + /// or add another one. + /// + /// # Errors + /// + /// Fails if `value` is empty or holds anything but printable ASCII and + /// spaces. + pub fn new(value: impl Into) -> Result { + let value = value.into(); + if value.is_empty() || !value.bytes().all(|b| (b' '..=b'~').contains(&b)) { + return Err(ServiceError::Validation( + "an authorization header value must be non-empty printable ASCII".to_string(), + )); + } + Ok(Self(value)) + } +} + +impl std::fmt::Debug for CloneAuthorization { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("CloneAuthorization()") + } } impl Default for IsolatedCloneOptions { @@ -90,6 +129,7 @@ impl Default for IsolatedCloneOptions { allowed_protocols: vec!["https".to_string()], follow_redirects: false, parent_dir: None, + authorization: None, } } } @@ -148,6 +188,9 @@ pub async fn clone_isolated( let clone_args = build_isolated_clone_args(url, &tree, &home, branch, options); let mut clone = isolated_git(&home, &allow); clone.args(&clone_args); + if let Some(authorization) = &options.authorization { + authorize(&mut clone, url, authorization); + } let failed = |stderr: String| GitError::CloneFailed { url: safe_url.clone(), stderr, @@ -263,6 +306,19 @@ pub(crate) fn isolated_git(home: &Path, allowed_protocols: &str) -> Command { cmd } +/// Have `cmd` send `authorization` with its HTTP requests under `url`, through +/// git's environment (`GIT_CONFIG_COUNT`, git 2.31 or later), so the value is +/// never on the command line. Call after [`isolated_git`], which clears the +/// environment. +pub(crate) fn authorize(cmd: &mut Command, url: &str, authorization: &CloneAuthorization) { + cmd.env("GIT_CONFIG_COUNT", "1") + .env("GIT_CONFIG_KEY_0", format!("http.{url}.extraHeader")) + .env( + "GIT_CONFIG_VALUE_0", + format!("Authorization: {}", authorization.0), + ); +} + /// Run `cmd` until it exits, the deadline passes or `watched` crosses a cap. /// The outer error is a crossed limit; the inner one is git's failure, with /// the start of its stderr. diff --git a/crates/fastskill-core/src/storage/git_isolated/tests.rs b/crates/fastskill-core/src/storage/git_isolated/tests.rs index 2413eb95..ea4624b7 100644 --- a/crates/fastskill-core/src/storage/git_isolated/tests.rs +++ b/crates/fastskill-core/src/storage/git_isolated/tests.rs @@ -173,3 +173,48 @@ fn disk_usage_does_not_follow_links() { assert_eq!(files, 1); assert!(bytes < 1 << 20, "{bytes}"); } + +#[test] +fn authorization_reaches_git_through_its_environment_scoped_to_the_url() { + let home = Path::new("/w/home"); + let url = "https://example.com/acme/skills.git"; + let mut cmd = isolated_git(home, "https"); + let authorization = CloneAuthorization::new("Basic c2VjcmV0").unwrap(); + authorize(&mut cmd, url, &authorization); + let value = |name: &str| { + cmd.as_std() + .get_envs() + .find(|(key, _)| key.to_string_lossy() == name) + .and_then(|(_, value)| value.map(|v| v.to_string_lossy().into_owned())) + }; + assert_eq!(value("GIT_CONFIG_COUNT").as_deref(), Some("1")); + assert_eq!( + value("GIT_CONFIG_KEY_0").as_deref(), + Some("http.https://example.com/acme/skills.git.extraHeader") + ); + assert_eq!( + value("GIT_CONFIG_VALUE_0").as_deref(), + Some("Authorization: Basic c2VjcmV0") + ); + let options = IsolatedCloneOptions { + authorization: Some(authorization), + ..IsolatedCloneOptions::default() + }; + let args = build_isolated_clone_args(url, Path::new("/w/tree"), home, None, &options); + assert!(args.iter().all(|arg| !arg.contains("c2VjcmV0")), "{args:?}"); + assert!(!format!("{options:?}").contains("c2VjcmV0")); +} + +#[test] +fn authorization_values_are_one_printable_header_value() { + assert!(CloneAuthorization::new("Bearer abc.def-123").is_ok()); + for bad in [ + "", + "Basic a\r\nX-Other: 1", + "Basic a\nb", + "Basic \u{e9}", + "a\tb", + ] { + assert!(CloneAuthorization::new(bad).is_err(), "{bad:?}"); + } +} diff --git a/crates/fastskill-core/tests/git_isolated_auth_test.rs b/crates/fastskill-core/tests/git_isolated_auth_test.rs new file mode 100644 index 00000000..28167c7f --- /dev/null +++ b/crates/fastskill-core/tests/git_isolated_auth_test.rs @@ -0,0 +1,84 @@ +//! `clone_isolated` sends the caller's authorization with git's HTTP requests, +//! and nothing when it has none. A loopback listener records each request's +//! head and answers 404, so git stops after its first request. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use fastskill_core::storage::git_isolated::{ + clone_isolated, CloneAuthorization, IsolatedCloneOptions, +}; +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::sync::{Arc, Mutex}; +use tempfile::TempDir; + +/// A listener on a loopback port that keeps every request head it is sent. +fn recorder() -> (String, Arc>>) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let heads = Arc::new(Mutex::new(Vec::new())); + let kept = Arc::clone(&heads); + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut head = String::new(); + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 || line == "\r\n" { + break; + } + head.push_str(&line); + } + kept.lock().unwrap().push(head); + let _ = stream.write_all( + b"HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ); + } + }); + (base, heads) +} + +async fn clone_heads(authorization: Option) -> Vec { + let (base, heads) = recorder(); + let parent = TempDir::new().unwrap(); + let result = clone_isolated( + &format!("{base}/acme/skills.git"), + None, + &IsolatedCloneOptions { + allowed_protocols: vec!["http".to_string()], + parent_dir: Some(parent.path().to_path_buf()), + authorization, + ..IsolatedCloneOptions::default() + }, + ) + .await; + let error = result.expect_err("the listener serves no repository"); + assert!(!error.to_string().contains("c2VjcmV0"), "{error}"); + let heads = heads.lock().unwrap().clone(); + assert!(!heads.is_empty(), "git reached the listener"); + heads +} + +#[tokio::test] +async fn the_authorization_is_sent_with_every_request() { + let authorization = CloneAuthorization::new("Basic c2VjcmV0").unwrap(); + for head in clone_heads(Some(authorization)).await { + assert!(head.starts_with("GET /acme/skills.git/info/refs"), "{head}"); + assert!( + head.lines() + .any(|line| line.eq_ignore_ascii_case("authorization: Basic c2VjcmV0")), + "{head}" + ); + } +} + +#[tokio::test] +async fn without_an_authorization_none_is_sent() { + for head in clone_heads(None).await { + assert!( + !head.to_ascii_lowercase().contains("authorization:"), + "{head}" + ); + } +}