Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions crosslink/src/commands/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,12 @@ pub fn init(

// Generate SSH key unless opted out
if !no_key {
let keys_dir = crosslink_dir.join("keys");
// GH#610: store keys under the main repo's `.crosslink/keys/` so
// they outlive `git worktree remove` of an agent worktree. The
// worktree-scoped `user.signingkey` on the hub-cache (which is
// also rooted in the main repo) then references a stable path.
let host_crosslink = signing::host_crosslink_dir(crosslink_dir);
let keys_dir = host_crosslink.join("keys");
match signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) {
Ok(keypair) => {
// Store relative path from .crosslink/
Expand Down Expand Up @@ -382,9 +387,20 @@ pub fn init(
// On --force re-runs, key-gen errors with "already exists".
// Reuse the on-disk key; otherwise agent.json loses
// ssh_key_path and shared_writer silently disables signing.
// GH#610: prefer the host-side keys dir (where new agents
// store their keys) and fall back to the worktree-local
// path for legacy agents whose keys predate the relocation.
let rel_path = format!("keys/{agent_id}_ed25519");
let private_path = crosslink_dir.join(&rel_path);
let public_path = crosslink_dir.join(format!("keys/{agent_id}_ed25519.pub"));
let host_private = host_crosslink.join(&rel_path);
let host_public = host_crosslink.join(format!("keys/{agent_id}_ed25519.pub"));
let (private_path, public_path) = if host_private.exists() && host_public.exists() {
(host_private, host_public)
} else {
(
crosslink_dir.join(&rel_path),
crosslink_dir.join(format!("keys/{agent_id}_ed25519.pub")),
)
};
if private_path.exists() && public_path.exists() {
if let (Ok(fp), Ok(pub_key)) = (
signing::get_key_fingerprint(&public_path),
Expand Down Expand Up @@ -602,7 +618,9 @@ pub fn bootstrap(

// Step 5: Generate SSH key unless opted out
if !no_key && config.ssh_key_path.is_none() {
let keys_dir = crosslink_dir.join("keys");
// GH#610: see `init` above — keys live under the main repo's
// `.crosslink/keys/` so they survive agent-worktree cleanup.
let keys_dir = signing::host_crosslink_dir(&crosslink_dir).join("keys");
match signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) {
Ok(keypair) => {
let rel_path = format!("keys/{agent_id}_ed25519");
Expand Down
7 changes: 6 additions & 1 deletion crosslink/src/commands/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,12 @@ fn init_agent_identity(crosslink_dir: &Path, agent_id: &str) -> Result<()> {
// AgentConfig::init defaults to role=driver.
let mut config = crate::identity::AgentConfig::init(crosslink_dir, agent_id, None)?;

let keys_dir = crosslink_dir.join("keys");
// GH#610: route through `host_crosslink_dir` for consistency with
// agent init/bootstrap. `crosslink init` always runs in the main
// repo, so this is a no-op in practice — but if any future
// refactor moves this into a worktree context, key storage stays
// pinned to the main repo and survives `git worktree remove`.
let keys_dir = crate::signing::host_crosslink_dir(crosslink_dir).join("keys");
match crate::signing::generate_agent_key(&keys_dir, agent_id, &config.machine_id) {
Ok(keypair) => {
config.ssh_key_path = Some(format!("keys/{agent_id}_ed25519"));
Expand Down
11 changes: 10 additions & 1 deletion crosslink/src/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,16 @@ pub struct AgentConfig {
/// autonomous agent worktrees. Missing field defaults to `driver`.
#[serde(default)]
pub role: AgentRole,
/// Path to SSH private key, relative to .crosslink/ (e.g. "`keys/agent_ed25519`").
/// Path to SSH private key, relative to the **main repo's**
/// `.crosslink/` (e.g. "`keys/agent_ed25519`").
///
/// GH#610: new agents store keys under the main repo's
/// `.crosslink/keys/` so they survive `git worktree remove` of a
/// kickoff agent worktree. Legacy agents (pre-#610) wrote this
/// path relative to the worktree's own `.crosslink/`; the
/// resolver in `sync::trust::resolve_agent_key` tries the host
/// path first and falls back to the worktree path for legacy
/// keys.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub ssh_key_path: Option<String>,
/// SSH public key fingerprint (e.g. "SHA256:...").
Expand Down
130 changes: 130 additions & 0 deletions crosslink/src/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,37 @@ pub enum SignatureVerification {

// ── Key generation ──────────────────────────────────────────────────

/// Resolve the *host* (main-repo) crosslink directory for storing
/// agent signing keys.
///
/// Agent keys live under the main repo's `.crosslink/keys/` rather
/// than each agent worktree's own `.crosslink/keys/` so that the
/// key file survives `git worktree remove` when a kickoff agent is
/// cleaned up (GH#610). Without this, the worktree-scoped
/// `user.signingkey` reference on the hub-cache outlives the file
/// it points at, breaking every subsequent `crosslink sync` until
/// a human re-points it (see also GH#565 for the C-path repair
/// that catches the residual breakage).
///
/// Behaviour:
///
/// - When `crosslink_dir` is inside a git worktree, walks up via
/// [`crate::utils::resolve_main_repo_root`] and returns
/// `<main-repo>/.crosslink`.
/// - When `crosslink_dir` is already the main repo's crosslink dir
/// (the driver case), returns it unchanged.
/// - When git can't resolve a repo (e.g. unit-test temp dirs with
/// no `git init`), returns `crosslink_dir` unchanged so callers
/// continue to behave as if there were no worktree.
#[must_use]
pub fn host_crosslink_dir(crosslink_dir: &Path) -> PathBuf {
let worktree_root = crosslink_dir.parent().unwrap_or(crosslink_dir);
crate::utils::resolve_main_repo_root(worktree_root).map_or_else(
|| crosslink_dir.to_path_buf(),
|root| root.join(".crosslink"),
)
}

/// Generate a new Ed25519 SSH key pair for an agent.
///
/// Keys are stored at `{keys_dir}/{agent_id}_ed25519` (.pub for public).
Expand Down Expand Up @@ -1127,6 +1158,105 @@ mod tests {
assert!(read_public_key(&path).is_err());
}

// ── host_crosslink_dir (GH#610) ──────────────────────────────────

/// In a temp dir with no git metadata, `resolve_main_repo_root`
/// returns `None`; `host_crosslink_dir` falls back to the input.
#[test]
fn test_host_crosslink_dir_no_git_returns_input() {
let dir = tempdir().unwrap();
let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();
let resolved = host_crosslink_dir(&crosslink_dir);
assert_eq!(resolved, crosslink_dir);
}

/// In a plain (non-worktree) git repo, `host_crosslink_dir`
/// returns the same `.crosslink/` it was given.
#[test]
fn test_host_crosslink_dir_main_repo_is_identity() {
let dir = tempdir().unwrap();
Command::new("git")
.current_dir(dir.path())
.args(["init", "-b", "main"])
.output()
.unwrap();
let crosslink_dir = dir.path().join(".crosslink");
std::fs::create_dir_all(&crosslink_dir).unwrap();
let resolved = host_crosslink_dir(&crosslink_dir);
// Canonicalize both sides — macOS resolves /tmp through /private/tmp,
// and `resolve_main_repo_root` canonicalises internally.
let expected = crosslink_dir.canonicalize().unwrap_or(crosslink_dir);
let actual = resolved.canonicalize().unwrap_or(resolved);
assert_eq!(actual, expected);
}

/// In a linked git worktree, `host_crosslink_dir` walks back up to
/// the main repo's `.crosslink/` (the whole point of the helper).
#[test]
fn test_host_crosslink_dir_worktree_resolves_to_main() {
let main = tempdir().unwrap();
let main_root = main.path();

// Build a main repo with one commit so `git worktree add` has
// something to branch from.
Command::new("git")
.current_dir(main_root)
.args(["init", "-b", "main"])
.output()
.unwrap();
Command::new("git")
.current_dir(main_root)
.args(["config", "user.email", "t@t.local"])
.output()
.unwrap();
Command::new("git")
.current_dir(main_root)
.args(["config", "user.name", "t"])
.output()
.unwrap();
std::fs::write(main_root.join("a"), "x").unwrap();
Command::new("git")
.current_dir(main_root)
.args(["add", "."])
.output()
.unwrap();
Command::new("git")
.current_dir(main_root)
.args(["commit", "-m", "init", "--no-gpg-sign"])
.output()
.unwrap();

// The main repo's crosslink dir — the answer we expect.
let main_crosslink = main_root.join(".crosslink");
std::fs::create_dir_all(&main_crosslink).unwrap();

// Add a linked worktree under the main repo on a fresh branch.
let wt_root = main_root.join(".worktrees").join("agent-abcd");
Command::new("git")
.current_dir(main_root)
.args([
"worktree",
"add",
"-b",
"agent-test",
wt_root.to_str().unwrap(),
])
.output()
.unwrap();
let wt_crosslink = wt_root.join(".crosslink");
std::fs::create_dir_all(&wt_crosslink).unwrap();

let resolved = host_crosslink_dir(&wt_crosslink);
// Canonicalise so /tmp ↔ /private/tmp on macOS doesn't trip us.
let expected = main_crosslink.canonicalize().unwrap_or(main_crosslink);
let actual = resolved.canonicalize().unwrap_or(resolved);
assert_eq!(
actual, expected,
"worktree's host_crosslink_dir should resolve to the main repo's .crosslink/"
);
}

// Integration tests requiring ssh-keygen on PATH
#[test]
fn test_generate_agent_key() {
Expand Down
113 changes: 113 additions & 0 deletions crosslink/src/sync/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3570,6 +3570,119 @@ fn test_repair_stale_signingkey_cache_missing_is_noop() {
assert!(!repaired);
}

// ------------------------------------------------------------------
// resolve_agent_key — GH#610: agent keys live in main repo, with
// a legacy fallback to the worktree-local path.
// ------------------------------------------------------------------

/// When no git metadata is present, `host_crosslink_dir` returns the input
/// unchanged. With the key only at the worktree-local path, the resolver
/// falls back to it — i.e. the legacy pre-#610 layout still works.
#[test]
fn test_resolve_agent_key_legacy_worktree_path_fallback() {
let dir = tempdir().unwrap();
let wt_crosslink = dir.path().join(".crosslink");
let keys_dir = wt_crosslink.join("keys");
std::fs::create_dir_all(&keys_dir).unwrap();
let key_file = keys_dir.join("legacy_ed25519");
std::fs::write(&key_file, "legacy-key-bytes").unwrap();

let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/legacy_ed25519");
assert_eq!(
resolved, key_file,
"no host-side key + worktree-local key → resolver picks the worktree path"
);
}

/// When neither the host-side nor the worktree-local path has the file,
/// the resolver returns the worktree-relative path. The caller is then
/// responsible for checking `.exists()` — preserves the pre-fix
/// "key missing → caller disables signing" contract.
#[test]
fn test_resolve_agent_key_neither_path_exists_returns_worktree_path() {
let dir = tempdir().unwrap();
let wt_crosslink = dir.path().join(".crosslink");
std::fs::create_dir_all(&wt_crosslink).unwrap();

let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/ghost_ed25519");
assert_eq!(resolved, wt_crosslink.join("keys/ghost_ed25519"));
assert!(!resolved.exists(), "precondition for this test");
}

/// End-to-end #610: with a real main repo and a linked worktree, an
/// agent.json living in the worktree resolves `ssh_key_path` to the
/// main repo's `.crosslink/keys/`. This is the contract that lets
/// `configure_signing` (called from the worktree's context) bake an
/// absolute main-repo path into the hub-cache's worktree-scoped
/// `user.signingkey`, so the reference keeps pointing at a real file
/// after `git worktree remove` of the agent worktree.
#[test]
fn test_resolve_agent_key_worktree_picks_main_repo_keys() {
let main = tempdir().unwrap();
let main_root = main.path();

// Bootstrap a real git repo so `resolve_main_repo_root` succeeds.
for args in [
vec!["init", "-b", "main"],
vec!["config", "user.email", "t@t.local"],
vec!["config", "user.name", "t"],
] {
Command::new("git")
.current_dir(main_root)
.args(&args)
.output()
.unwrap();
}
std::fs::write(main_root.join("README.md"), "x").unwrap();
Command::new("git")
.current_dir(main_root)
.args(["add", "."])
.output()
.unwrap();
Command::new("git")
.current_dir(main_root)
.args(["commit", "-m", "init", "--no-gpg-sign"])
.output()
.unwrap();

// The post-#610 key lives in the MAIN repo's .crosslink/keys/.
let main_crosslink = main_root.join(".crosslink");
let main_keys = main_crosslink.join("keys");
std::fs::create_dir_all(&main_keys).unwrap();
let main_key = main_keys.join("agent_ed25519");
std::fs::write(&main_key, "real-key-bytes").unwrap();

// Linked worktree the agent would have run in. agent.json lives
// here and references `keys/agent_ed25519` — relative to *main*
// per the post-#610 semantics. The worktree's own .crosslink
// intentionally has no `keys/` directory.
let wt_root = main_root.join(".worktrees").join("agent-test");
Command::new("git")
.current_dir(main_root)
.args([
"worktree",
"add",
"-b",
"agent-test",
wt_root.to_str().unwrap(),
])
.output()
.unwrap();
let wt_crosslink = wt_root.join(".crosslink");
std::fs::create_dir_all(&wt_crosslink).unwrap();

let resolved = crate::sync::trust::resolve_agent_key(&wt_crosslink, "keys/agent_ed25519");
let expected = main_key.canonicalize().unwrap_or(main_key);
let actual = resolved.canonicalize().unwrap_or(resolved);
assert_eq!(
actual, expected,
"with a main-repo key present, the resolver prefers it over the worktree path \
— this is what lets `configure_signing` bake an absolute main-repo path into \
the hub-cache's worktree-scoped `user.signingkey` so the reference survives \
`git worktree remove` of the agent worktree (GH#610)"
);
}

// ============================================================================
// GH#586: structured push-failure classifier
// ============================================================================
Expand Down
22 changes: 20 additions & 2 deletions crosslink/src/sync/trust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,24 @@ fn expand_tilde(path: &str) -> PathBuf {
PathBuf::from(path)
}

/// Resolve an agent's SSH private key path from the relative form stored in
/// `agent.json` (`ssh_key_path`).
///
/// GH#610: new agents store their key under the *main repo's*
/// `.crosslink/keys/` so it survives `git worktree remove` of a kickoff
/// agent worktree. Legacy agents (created before that change) have their
/// key inside the worktree's own `.crosslink/keys/`. Try the host path
/// first; fall back to the worktree-local path so existing deployments
/// keep signing until the legacy worktree is cleaned up.
pub(super) fn resolve_agent_key(worktree_crosslink_dir: &Path, rel_key: &str) -> PathBuf {
let host = crate::signing::host_crosslink_dir(worktree_crosslink_dir);
let host_path = host.join(rel_key);
if host_path.exists() {
return host_path;
}
worktree_crosslink_dir.join(rel_key)
}

/// Best guess whether a `user.signingkey` value is a filesystem path rather
/// than literal key material (e.g. an inline `ssh-ed25519 AAAA...` line).
///
Expand Down Expand Up @@ -115,7 +133,7 @@ impl SyncManager {
// attribution is distinct.
if let Some(agent) = AgentConfig::load(crosslink_dir)? {
if let (Some(rel_key), Some(_)) = (&agent.ssh_key_path, &agent.ssh_fingerprint) {
let private_key = self.crosslink_dir.join(rel_key);
let private_key = resolve_agent_key(&self.crosslink_dir, rel_key);
if private_key.exists() {
signing::configure_git_ssh_signing(
&self.cache_dir,
Expand Down Expand Up @@ -165,7 +183,7 @@ impl SyncManager {
// hasn't set `user.signingkey` in the main repo.
if let Some(agent) = AgentConfig::load(crosslink_dir)? {
if let (Some(rel_key), Some(_)) = (&agent.ssh_key_path, &agent.ssh_fingerprint) {
let private_key = self.crosslink_dir.join(rel_key);
let private_key = resolve_agent_key(&self.crosslink_dir, rel_key);
if private_key.exists() {
signing::configure_git_ssh_signing(
&self.cache_dir,
Expand Down
Loading