From 8606b6d7deaa15f6daf74e6c6c6c03f678d3720f Mon Sep 17 00:00:00 2001 From: aoruLola Date: Sun, 14 Jun 2026 02:30:07 +0800 Subject: [PATCH 1/6] fix: require confirmation before restoring deleted local files in ht revert When a checked-out asset is deleted locally (local_hash == None, base_hash == Some), the revert command now treats this as overwriting a local change, triggering the dangerous-operation prompt unless --yes is provided. Previously this case fell through to false, silently recreating the file without confirmation. Also adds the `ht revert` command to recover a single asset from a previous snapshot. --- crates/cli/src/cmd/mod.rs | 1 + crates/cli/src/cmd/revert.rs | 779 +++++++++++++++++++++++++++++++++++ crates/cli/src/main.rs | 3 + 3 files changed, 783 insertions(+) create mode 100644 crates/cli/src/cmd/revert.rs diff --git a/crates/cli/src/cmd/mod.rs b/crates/cli/src/cmd/mod.rs index 166cc9c..09f6ae0 100644 --- a/crates/cli/src/cmd/mod.rs +++ b/crates/cli/src/cmd/mod.rs @@ -13,6 +13,7 @@ pub(crate) mod log_cmd; pub(crate) mod login; pub(crate) mod remove; pub(crate) mod repo; +pub(crate) mod revert; pub(crate) mod rollback; pub(crate) mod save; pub(crate) mod server; diff --git a/crates/cli/src/cmd/revert.rs b/crates/cli/src/cmd/revert.rs new file mode 100644 index 0000000..416055c --- /dev/null +++ b/crates/cli/src/cmd/revert.rs @@ -0,0 +1,779 @@ +use std::fs; + +use anyhow::{anyhow, Context, Result}; +use clap::Args; + +use crate::utils::*; + +#[derive(Debug, Args)] +pub(crate) struct RevertArgs { + #[arg(long = "asset-path", help = "Repository asset path to revert")] + pub asset_path: String, + #[arg(long, help = "Repository id; defaults to the login profile repository")] + pub repo: Option, + #[arg( + long, + help = "Branch to revert from; defaults to the login profile branch" + )] + pub branch: Option, + #[arg(long = "to", help = "Optional changeset id to revert from")] + pub to_changeset_id: Option, + #[arg(long, help = "Skip confirmation prompts")] + pub yes: bool, + #[arg(long, help = "Keep the HyperTide lock after reverting")] + pub keep_lock: bool, +} + +pub(crate) async fn execute(args: RevertArgs) -> Result<()> { + let asset_path = normalize_revert_asset_path(&args.asset_path)?; + + let mut profile = load_profile()?; + let repo = resolve_repo(&profile, args.repo.as_deref())?; + let branch = args + .branch + .unwrap_or_else(|| profile.current_branch.clone()); + let mut workspace = load_workspace() + .context("workspace not initialized; run `ht checkout` before reverting assets")?; + if workspace.repo_id != repo || workspace.branch != branch { + return Err(anyhow!( + "workspace is bound to {}@{}, not {}@{}", + workspace.repo_id, + workspace.branch, + repo, + branch + )); + } + + let workspace_root = std::path::PathBuf::from(&workspace.workspace_root); + let target = resolve_workspace_target(&workspace_root, &asset_path)?; + let mut stage = load_stage().unwrap_or_else(|_| StageFile::default_for_branch(&branch)); + if stage.branch != branch { + stage = StageFile::default_for_branch(&branch); + } + + let has_staged_delta = stage.assets.iter().any(|asset| asset.path == asset_path); + let base_hash = workspace + .checked_out_assets + .iter() + .find(|asset| asset.path == asset_path) + .map(|asset| asset.blob_hash.clone()); + let local_hash = hash_local_asset(&workspace_root, &asset_path)?; + let overwrites_local_change = match (local_hash.as_deref(), base_hash.as_deref()) { + (Some(local), Some(base)) => local != base, + (Some(_), None) => true, + (None, Some(_)) => true, // local file was deleted — restoring it overwrites the user's uncommitted delete + (None, None) => false, + }; + if has_staged_delta || overwrites_local_change { + let mut actions = Vec::new(); + if has_staged_delta { + actions.push("remove staged delta"); + } + if overwrites_local_change { + actions.push("overwrite local file"); + } + confirm_dangerous( + &format!("revert {} ({})", asset_path, actions.join(", ")), + args.yes, + )?; + } + + let client = reqwest::Client::new(); + let snapshot = fetch_snapshot( + &client, + &mut profile, + &repo, + &branch, + args.to_changeset_id.as_deref(), + ) + .await?; + let asset = find_snapshot_asset(&snapshot, &asset_path)?.clone(); + let bytes = fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; + if let Some(parent) = target.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + fs::write(&target, &bytes).with_context(|| format!("failed to write {}", target.display()))?; + + let update = apply_revert_state( + &mut workspace, + &mut stage, + &asset.path, + &asset.blob_hash, + base_hash.as_deref(), + ); + workspace.last_synced_at = now_unix(); + save_workspace(&workspace)?; + save_stage(&stage)?; + + if !args.keep_lock { + if let Err(err) = send_lock_path_request("lock release", "release", &asset.path).await { + eprintln!( + "warning: reverted {}, but failed to release lock: {err}", + asset.path + ); + eprintln!( + "run `ht lock release --path {}` manually if needed", + asset.path + ); + } + } + + if json_output_enabled() { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "ok": true, + "asset_path": asset.path, + "restored_hash": asset.blob_hash, + }))? + ); + } else { + println!( + "reverted {} to {} on {}@{}{}", + asset.path, + snapshot + .changeset_id + .as_deref() + .unwrap_or(ROOT_BASE_CHANGESET_ID), + repo, + branch, + if update.removed_staged_delta { + " (removed staged delta)" + } else if update.staged_delta { + " (staged delta)" + } else { + "" + } + ); + } + Ok(()) +} + +#[derive(Debug, PartialEq, Eq)] +struct RevertStateUpdate { + removed_staged_delta: bool, + staged_delta: bool, +} + +fn normalize_revert_asset_path(asset_path: &str) -> Result { + let normalized = asset_path.trim().replace('\\', "/"); + let trimmed = normalized.as_str(); + if trimmed.is_empty() { + return Err(anyhow!("asset path cannot be empty")); + } + if trimmed.ends_with('/') || trimmed.ends_with('\\') { + return Err(anyhow!("directory revert is not supported: {asset_path}")); + } + if trimmed.contains('*') || trimmed.contains('?') { + return Err(anyhow!("wildcard revert is not supported: {asset_path}")); + } + Ok(normalized) +} + +fn remove_staged_asset(stage: &mut StageFile, asset_path: &str) -> bool { + let original_len = stage.assets.len(); + stage.assets.retain(|asset| asset.path != asset_path); + stage.assets.len() != original_len +} + +fn update_workspace_asset(workspace: &mut WorkspaceState, asset_path: &str, blob_hash: &str) { + if let Some(existing) = workspace + .checked_out_assets + .iter_mut() + .find(|asset| asset.path == asset_path) + { + existing.blob_hash = blob_hash.to_string(); + return; + } + + workspace.checked_out_assets.push(WorkspaceFile { + path: asset_path.to_string(), + blob_hash: blob_hash.to_string(), + }); +} + +fn apply_revert_state( + workspace: &mut WorkspaceState, + stage: &mut StageFile, + asset_path: &str, + blob_hash: &str, + base_hash: Option<&str>, +) -> RevertStateUpdate { + if base_hash == Some(blob_hash) { + update_workspace_asset(workspace, asset_path, blob_hash); + return RevertStateUpdate { + removed_staged_delta: remove_staged_asset(stage, asset_path), + staged_delta: false, + }; + } + + upsert_stage_asset(stage, asset_path, Some(blob_hash.to_string())); + RevertStateUpdate { + removed_staged_delta: false, + staged_delta: true, + } +} + +fn find_snapshot_asset<'a>(snapshot: &'a SyncResponse, asset_path: &str) -> Result<&'a SyncAsset> { + snapshot + .assets + .iter() + .find(|asset| asset.path == asset_path) + .ok_or_else(|| anyhow!("asset not found in target snapshot: {asset_path}")) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::utils::{ + save_profile, save_stage, save_workspace, AssetDelta, CliProfile, StageFile, StorageHash, + SyncAsset, SyncResponse, WorkspaceFile, WorkspaceState, + }; + use std::io::{Read, Write}; + use std::net::TcpListener; + use std::path::{Path, PathBuf}; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::{Arc, Mutex, OnceLock}; + use std::thread; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn cwd_lock() -> &'static tokio::sync::Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| tokio::sync::Mutex::new(())) + } + + fn unique_workspace(name: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(1); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("time") + .as_nanos(); + std::env::temp_dir() + .join("hypertide-cli-revert-e2e") + .join(format!( + "{}-{}-{}", + name, + nanos, + COUNTER.fetch_add(1, Ordering::Relaxed) + )) + } + + fn write_file(path: &Path, bytes: &[u8]) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).expect("parent dir"); + } + std::fs::write(path, bytes).expect("write file"); + } + + #[derive(Clone)] + struct FakeResponse { + status: &'static str, + content_type: &'static str, + body: Vec, + } + + struct FakeServer { + base_url: String, + requests: Arc>>, + handle: thread::JoinHandle<()>, + } + + impl FakeServer { + fn start(responses: Vec) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fake server"); + let base_url = format!("http://{}", listener.local_addr().expect("addr")); + let requests = Arc::new(Mutex::new(Vec::new())); + let request_log = Arc::clone(&requests); + let handle = thread::spawn(move || { + for response in responses { + let (mut stream, _) = listener.accept().expect("accept"); + let request = read_http_request(&mut stream); + request_log.lock().expect("request log").push(request); + let header = format!( + "HTTP/1.1 {}\r\ncontent-length: {}\r\ncontent-type: {}\r\nconnection: close\r\n\r\n", + response.status, + response.body.len(), + response.content_type + ); + stream.write_all(header.as_bytes()).expect("write header"); + stream.write_all(&response.body).expect("write body"); + } + }); + Self { + base_url, + requests, + handle, + } + } + + fn finish(self) -> Vec { + self.handle.join().expect("fake server thread"); + let requests = self.requests.lock().expect("request log"); + requests.clone() + } + } + + fn read_http_request(stream: &mut std::net::TcpStream) -> String { + let mut bytes = Vec::new(); + let mut buffer = [0_u8; 1024]; + loop { + let read = stream.read(&mut buffer).expect("read request"); + if read == 0 { + break; + } + bytes.extend_from_slice(&buffer[..read]); + if bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let request = String::from_utf8_lossy(&bytes).to_string(); + let content_length = request + .lines() + .find_map(|line| { + line.strip_prefix("content-length:") + .or_else(|| line.strip_prefix("Content-Length:")) + }) + .and_then(|value| value.trim().parse::().ok()) + .unwrap_or(0); + let header_len = bytes + .windows(4) + .position(|window| window == b"\r\n\r\n") + .expect("headers") + + 4; + while bytes.len() < header_len + content_length { + let read = stream.read(&mut buffer).expect("read body"); + if read == 0 { + break; + } + bytes.extend_from_slice(&buffer[..read]); + } + break; + } + } + String::from_utf8_lossy(&bytes).to_string() + } + + fn sync_response(repo: &str, branch: &str, changeset: &str, path: &str, hash: &str) -> Vec { + serde_json::to_vec(&serde_json::json!({ + "success": true, + "data": { + "repo_id": repo, + "branch": branch, + "changeset_id": changeset, + "assets": [{ + "asset_id": path, + "path": path, + "blob_hash": hash + }] + }, + "error": null + })) + .expect("sync json") + } + + fn lock_release_response(path: &str) -> Vec { + serde_json::to_vec(&serde_json::json!({ + "success": true, + "data": { + "file_path": path, + "owner_id": "owner", + "locked_at": "2026-06-11T00:00:00Z", + "lease_expires_at": null + }, + "error": null + })) + .expect("lock json") + } + + fn setup_revert_workspace( + root: &Path, + server_url: &str, + repo: &str, + branch: &str, + asset_path: &str, + base_hash: &str, + stage_assets: Vec, + ) { + std::fs::create_dir_all(root).expect("workspace root"); + save_profile(&CliProfile { + server: server_url.to_string(), + api_key: "test-key".to_string(), + api_key_direct: true, + access_token: None, + refresh_token: None, + access_token_expires_at: None, + current_repo: Some(repo.to_string()), + current_branch: branch.to_string(), + }) + .expect("save profile"); + save_workspace(&WorkspaceState { + repo_id: repo.to_string(), + branch: branch.to_string(), + workspace_root: root.to_string_lossy().to_string(), + base_changeset_id: Some("head-cs".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: asset_path.to_string(), + blob_hash: base_hash.to_string(), + }], + last_synced_at: 1, + }) + .expect("save workspace"); + save_stage(&StageFile { + branch: branch.to_string(), + base_changeset_id: Some("head-cs".to_string()), + assets: stage_assets, + }) + .expect("save stage"); + } + + #[test] + fn remove_staged_asset_removes_only_matching_path() { + let mut stage = StageFile { + branch: "main".to_string(), + base_changeset_id: Some("cs-0".to_string()), + assets: vec![ + AssetDelta { + path: "Content/A.uasset".to_string(), + blob_hash: Some("hash-a".to_string()), + }, + AssetDelta { + path: "Content/B.uasset".to_string(), + blob_hash: None, + }, + ], + }; + + assert!(remove_staged_asset(&mut stage, "Content/A.uasset")); + + assert_eq!(stage.assets.len(), 1); + assert_eq!(stage.assets[0].path, "Content/B.uasset"); + } + + #[test] + fn update_workspace_asset_replaces_existing_hash() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: Some("cs-0".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "old".to_string(), + }], + last_synced_at: 1, + }; + + update_workspace_asset(&mut workspace, "Content/A.uasset", "new"); + + assert_eq!(workspace.checked_out_assets.len(), 1); + assert_eq!(workspace.checked_out_assets[0].blob_hash, "new"); + } + + #[test] + fn update_workspace_asset_inserts_missing_asset() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: None, + checked_out_assets: Vec::new(), + last_synced_at: 1, + }; + + update_workspace_asset(&mut workspace, "Content/A.uasset", "hash-a"); + + assert_eq!(workspace.checked_out_assets.len(), 1); + assert_eq!(workspace.checked_out_assets[0].path, "Content/A.uasset"); + assert_eq!(workspace.checked_out_assets[0].blob_hash, "hash-a"); + } + + #[test] + fn apply_revert_state_stages_blob_when_target_differs_from_workspace_base() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: Some("cs-head".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "head-hash".to_string(), + }], + last_synced_at: 1, + }; + let mut stage = StageFile::default_for_branch("main"); + + let update = apply_revert_state( + &mut workspace, + &mut stage, + "Content/A.uasset", + "old-hash", + Some("head-hash"), + ); + + assert_eq!( + update, + RevertStateUpdate { + removed_staged_delta: false, + staged_delta: true, + } + ); + assert_eq!(workspace.checked_out_assets[0].blob_hash, "head-hash"); + assert_eq!(stage.assets.len(), 1); + assert_eq!(stage.assets[0].path, "Content/A.uasset"); + assert_eq!(stage.assets[0].blob_hash.as_deref(), Some("old-hash")); + } + + #[test] + fn apply_revert_state_clears_stage_when_target_matches_workspace_base() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: Some("cs-head".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "head-hash".to_string(), + }], + last_synced_at: 1, + }; + let mut stage = StageFile { + branch: "main".to_string(), + base_changeset_id: Some("cs-head".to_string()), + assets: vec![AssetDelta { + path: "Content/A.uasset".to_string(), + blob_hash: Some("local-hash".to_string()), + }], + }; + + let update = apply_revert_state( + &mut workspace, + &mut stage, + "Content/A.uasset", + "head-hash", + Some("head-hash"), + ); + + assert_eq!( + update, + RevertStateUpdate { + removed_staged_delta: true, + staged_delta: false, + } + ); + assert_eq!(workspace.checked_out_assets[0].blob_hash, "head-hash"); + assert!(stage.assets.is_empty()); + } + + #[tokio::test(flavor = "current_thread")] + async fn revert_execute_e2e_stages_old_snapshot_blob_and_encodes_sync_query() { + let _guard = cwd_lock().lock().await; + let original_dir = std::env::current_dir().expect("current dir"); + let root = unique_workspace("stage-old-snapshot"); + std::fs::create_dir_all(&root).expect("workspace dir"); + std::env::set_current_dir(&root).expect("set cwd"); + + let repo = "repo"; + let branch = "feature-a"; + let changeset = "cs-old"; + let asset_path = "Content/A.uasset"; + let base_bytes = b"head version"; + let old_bytes = b"old version"; + let base_hash = StorageHash::hash_bytes(base_bytes); + let old_hash = StorageHash::hash_bytes(old_bytes); + write_file(&root.join(asset_path), base_bytes); + let server = FakeServer::start(vec![ + FakeResponse { + status: "200 OK", + content_type: "application/json", + body: sync_response(repo, branch, changeset, asset_path, &old_hash), + }, + FakeResponse { + status: "200 OK", + content_type: "application/octet-stream", + body: old_bytes.to_vec(), + }, + FakeResponse { + status: "200 OK", + content_type: "application/json", + body: lock_release_response(asset_path), + }, + ]); + setup_revert_workspace( + &root, + &server.base_url, + repo, + branch, + asset_path, + &base_hash, + vec![], + ); + + let result = execute(RevertArgs { + asset_path: " Content\\A.uasset ".to_string(), + repo: Some(repo.to_string()), + branch: Some(branch.to_string()), + to_changeset_id: Some(changeset.to_string()), + yes: true, + keep_lock: false, + }) + .await; + + std::env::set_current_dir(&original_dir).expect("restore cwd"); + result.expect("revert executes"); + assert_eq!( + std::fs::read(root.join(asset_path)).expect("asset"), + old_bytes + ); + let workspace: WorkspaceState = + serde_json::from_slice(&std::fs::read(root.join(".hypertide/workspace.json")).unwrap()) + .unwrap(); + assert_eq!(workspace.checked_out_assets[0].blob_hash, base_hash); + let stage: StageFile = + serde_json::from_slice(&std::fs::read(root.join(".hypertide/stage.json")).unwrap()) + .unwrap(); + assert_eq!(stage.assets.len(), 1); + assert_eq!(stage.assets[0].path, asset_path); + assert_eq!( + stage.assets[0].blob_hash.as_deref(), + Some(old_hash.as_str()) + ); + + let requests = server.finish(); + assert!( + requests[0].starts_with("GET /v2/sync/"), + "request should target sync endpoint: {}", + requests[0] + ); + assert!( + requests[0].contains("branch="), + "request should include branch: {}", + requests[0] + ); + assert!( + requests[0].contains("to_changeset_id="), + "request should include to_changeset_id: {}", + requests[0] + ); + assert!(requests[1].starts_with(&format!("GET /v2/storage/download/{old_hash} "))); + assert!(requests[2].starts_with("POST /v2/locks/release ")); + assert!(requests[2].contains(r#""file_path":"Content/A.uasset""#)); + } + + #[tokio::test(flavor = "current_thread")] + async fn revert_execute_e2e_clears_stage_when_snapshot_matches_workspace_base() { + let _guard = cwd_lock().lock().await; + let original_dir = std::env::current_dir().expect("current dir"); + let root = unique_workspace("clear-stage"); + std::fs::create_dir_all(&root).expect("workspace dir"); + std::env::set_current_dir(&root).expect("set cwd"); + + let repo = "repo"; + let branch = "main"; + let asset_path = "Content/A.uasset"; + let base_bytes = b"head version"; + let local_bytes = b"local dirty version"; + let base_hash = StorageHash::hash_bytes(base_bytes); + write_file(&root.join(asset_path), local_bytes); + let server = FakeServer::start(vec![ + FakeResponse { + status: "200 OK", + content_type: "application/json", + body: sync_response(repo, branch, "head-cs", asset_path, &base_hash), + }, + FakeResponse { + status: "200 OK", + content_type: "application/octet-stream", + body: base_bytes.to_vec(), + }, + ]); + setup_revert_workspace( + &root, + &server.base_url, + repo, + branch, + asset_path, + &base_hash, + vec![AssetDelta { + path: asset_path.to_string(), + blob_hash: Some(StorageHash::hash_bytes(local_bytes)), + }], + ); + + let result = execute(RevertArgs { + asset_path: asset_path.to_string(), + repo: None, + branch: None, + to_changeset_id: None, + yes: true, + keep_lock: true, + }) + .await; + + std::env::set_current_dir(&original_dir).expect("restore cwd"); + result.expect("revert executes"); + assert_eq!( + std::fs::read(root.join(asset_path)).expect("asset"), + base_bytes + ); + let workspace: WorkspaceState = + serde_json::from_slice(&std::fs::read(root.join(".hypertide/workspace.json")).unwrap()) + .unwrap(); + assert_eq!(workspace.checked_out_assets[0].blob_hash, base_hash); + let stage: StageFile = + serde_json::from_slice(&std::fs::read(root.join(".hypertide/stage.json")).unwrap()) + .unwrap(); + assert!(stage.assets.is_empty()); + + let requests = server.finish(); + assert_eq!(requests.len(), 2); + assert!(requests[0].starts_with("GET /v2/sync/repo?branch=main ")); + assert!(requests[1].starts_with(&format!("GET /v2/storage/download/{base_hash} "))); + } + + #[test] + fn find_snapshot_asset_errors_when_target_missing() { + let snapshot = SyncResponse { + repo_id: "repo".to_string(), + branch: "main".to_string(), + changeset_id: Some("cs-1".to_string()), + assets: vec![SyncAsset { + asset_id: None, + path: "Content/Other.uasset".to_string(), + blob_hash: "hash-other".to_string(), + }], + }; + + let error = find_snapshot_asset(&snapshot, "Content/A.uasset").unwrap_err(); + + assert!(error + .to_string() + .contains("asset not found in target snapshot")); + } + + #[test] + fn deleted_local_file_treated_as_overwrite() { + // When local file is deleted (local_hash == None) but the base has a hash, + // overwrites_local_change should be true so the dangerous-operation prompt fires. + let local_hash: Option<&str> = None; + let base_hash: Option<&str> = Some("abc123"); + let overwrites_local_change = match (local_hash, base_hash) { + (Some(local), Some(base)) => local != base, + (Some(_), None) => true, + (None, Some(_)) => true, + (None, None) => false, + }; + assert!( + overwrites_local_change, + "local deletion should count as a local change" + ); + } + + #[test] + fn validate_asset_path_rejects_empty_directory_and_wildcards() { + assert!(normalize_revert_asset_path("").is_err()); + assert!(normalize_revert_asset_path("Content/Foo/").is_err()); + assert!(normalize_revert_asset_path("Content/*.uasset").is_err()); + assert_eq!( + normalize_revert_asset_path(" Content\\A.uasset ").unwrap(), + "Content/A.uasset" + ); + } +} diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 2ab4727..99ec7c7 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -55,6 +55,8 @@ enum Command { Log(cmd::log_cmd::LogArgs), #[command(about = "Submit a rollback changeset")] Rollback(cmd::rollback::RollbackArgs), + #[command(about = "Recover a single asset from a previous snapshot")] + Revert(cmd::revert::RevertArgs), #[command(about = "Sync local metadata to a branch snapshot")] Sync(cmd::sync::SyncArgs), #[command(about = "Materialize branch assets into the workspace")] @@ -103,6 +105,7 @@ async fn main() -> Result<()> { Command::Submit(args) => cmd::submit::execute(args).await, Command::Log(args) => cmd::log_cmd::execute(args).await, Command::Rollback(args) => cmd::rollback::execute(args).await, + Command::Revert(args) => cmd::revert::execute(args).await, Command::Sync(args) => cmd::sync::execute(args).await, Command::Checkout(args) => cmd::checkout::execute(args).await, Command::Status(args) => cmd::status::execute(args).await, From ee163affcc2eaae3e08480c12ed6eb8aa196128e Mon Sep 17 00:00:00 2001 From: aoruLola Date: Sun, 14 Jun 2026 02:40:45 +0800 Subject: [PATCH 2/6] fix: support staging deletions and preserve asset IDs in ht revert 1. Stage deletions when target snapshot lacks the asset: - When reverting to a changeset before an asset was added, the command now stages a deletion (blob_hash: None) and removes the local file instead of erroring. 2. Preserve snapshot asset IDs when staging reverts: - Add asset_id field to AssetDelta with serde skip_serializing_if - Pass asset_id from snapshot entries through apply_revert_state and upsert_stage_asset into the staged delta - On submit the server defaults a missing asset ID to the path, so this preserves stable asset IDs from API/checkpoint flows --- crates/cli/src/cmd/add.rs | 2 +- crates/cli/src/cmd/remove.rs | 2 +- crates/cli/src/cmd/revert.rs | 200 +++++++++++++++++++++++------------ crates/cli/src/utils.rs | 16 ++- 4 files changed, 149 insertions(+), 71 deletions(-) diff --git a/crates/cli/src/cmd/add.rs b/crates/cli/src/cmd/add.rs index 9504d0c..bc92c4a 100644 --- a/crates/cli/src/cmd/add.rs +++ b/crates/cli/src/cmd/add.rs @@ -41,7 +41,7 @@ pub(crate) async fn execute(args: AddArgs) -> Result<()> { if stage.branch != branch { stage = StageFile::default_for_branch(&branch); } - upsert_stage_asset(&mut stage, &path, Some(blob)); + upsert_stage_asset(&mut stage, &path, Some(blob), None); save_stage(&stage)?; println!("staged {} asset(s) on {}", stage.assets.len(), stage.branch); Ok(()) diff --git a/crates/cli/src/cmd/remove.rs b/crates/cli/src/cmd/remove.rs index 4f945e0..5f01241 100644 --- a/crates/cli/src/cmd/remove.rs +++ b/crates/cli/src/cmd/remove.rs @@ -20,7 +20,7 @@ pub(crate) async fn execute(args: RemoveArgs) -> Result<()> { if stage.branch != branch { stage = StageFile::default_for_branch(&branch); } - upsert_stage_asset(&mut stage, &args.asset_path, None); + upsert_stage_asset(&mut stage, &args.asset_path, None, None); save_stage(&stage)?; println!( "staged delete for {} on {} ({} asset(s) staged)", diff --git a/crates/cli/src/cmd/revert.rs b/crates/cli/src/cmd/revert.rs index 416055c..498ff8a 100644 --- a/crates/cli/src/cmd/revert.rs +++ b/crates/cli/src/cmd/revert.rs @@ -87,66 +87,121 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { args.to_changeset_id.as_deref(), ) .await?; - let asset = find_snapshot_asset(&snapshot, &asset_path)?.clone(); - let bytes = fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; - if let Some(parent) = target.parent() { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create {}", parent.display()))?; - } - fs::write(&target, &bytes).with_context(|| format!("failed to write {}", target.display()))?; - - let update = apply_revert_state( - &mut workspace, - &mut stage, - &asset.path, - &asset.blob_hash, - base_hash.as_deref(), - ); - workspace.last_synced_at = now_unix(); - save_workspace(&workspace)?; - save_stage(&stage)?; - if !args.keep_lock { - if let Err(err) = send_lock_path_request("lock release", "release", &asset.path).await { - eprintln!( - "warning: reverted {}, but failed to release lock: {err}", - asset.path - ); - eprintln!( - "run `ht lock release --path {}` manually if needed", - asset.path + let snapshot_asset = find_snapshot_asset(&snapshot, &asset_path); + + let _update = match snapshot_asset { + Some(asset) => { + let bytes = fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; + if let Some(parent) = target.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + fs::write(&target, &bytes) + .with_context(|| format!("failed to write {}", target.display()))?; + + let update = apply_revert_state( + &mut workspace, + &mut stage, + &asset_path, + Some(&asset.blob_hash), + base_hash.as_deref(), + asset.asset_id.clone(), ); + + if !args.keep_lock { + if let Err(err) = + send_lock_path_request("lock release", "release", &asset_path).await + { + eprintln!( + "warning: reverted {}, but failed to release lock: {err}", + asset_path + ); + eprintln!( + "run `ht lock release --path {}` manually if needed", + asset_path + ); + } + } + + if json_output_enabled() { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "ok": true, + "asset_path": asset_path, + "restored_hash": asset.blob_hash, + }))? + ); + } else { + println!( + "reverted {} to {} on {}@{}{}", + asset_path, + snapshot + .changeset_id + .as_deref() + .unwrap_or(ROOT_BASE_CHANGESET_ID), + repo, + branch, + if update.removed_staged_delta { + " (removed staged delta)" + } else if update.staged_delta { + " (staged delta)" + } else { + "" + } + ); + } + + update } - } + None => { + // Asset not found in target snapshot — stage a deletion + if target.exists() { + fs::remove_file(&target) + .with_context(|| format!("failed to delete {}", target.display()))?; + } - if json_output_enabled() { - println!( - "{}", - serde_json::to_string_pretty(&serde_json::json!({ - "ok": true, - "asset_path": asset.path, - "restored_hash": asset.blob_hash, - }))? - ); - } else { - println!( - "reverted {} to {} on {}@{}{}", - asset.path, - snapshot - .changeset_id - .as_deref() - .unwrap_or(ROOT_BASE_CHANGESET_ID), - repo, - branch, - if update.removed_staged_delta { - " (removed staged delta)" - } else if update.staged_delta { - " (staged delta)" + let update = apply_revert_state( + &mut workspace, + &mut stage, + &asset_path, + None, + base_hash.as_deref(), + None, + ); + + if json_output_enabled() { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "ok": true, + "asset_path": asset_path, + "restored_hash": null, + "staged_deletion": true, + }))? + ); } else { - "" + println!( + "reverted {} to absent on {}@{}{}", + asset_path, + repo, + branch, + if update.staged_delta { + " (staged deletion)" + } else { + "" + } + ); } - ); - } + + update + } + }; + + workspace.last_synced_at = now_unix(); + save_workspace(&workspace)?; + save_stage(&stage)?; Ok(()) } @@ -197,30 +252,37 @@ fn apply_revert_state( workspace: &mut WorkspaceState, stage: &mut StageFile, asset_path: &str, - blob_hash: &str, + blob_hash: Option<&str>, base_hash: Option<&str>, + asset_id: Option, ) -> RevertStateUpdate { - if base_hash == Some(blob_hash) { - update_workspace_asset(workspace, asset_path, blob_hash); + if base_hash == blob_hash { + if let Some(hash) = blob_hash { + update_workspace_asset(workspace, asset_path, hash); + } return RevertStateUpdate { removed_staged_delta: remove_staged_asset(stage, asset_path), staged_delta: false, }; } - upsert_stage_asset(stage, asset_path, Some(blob_hash.to_string())); + upsert_stage_asset( + stage, + asset_path, + blob_hash.map(|h| h.to_string()), + asset_id, + ); RevertStateUpdate { removed_staged_delta: false, staged_delta: true, } } -fn find_snapshot_asset<'a>(snapshot: &'a SyncResponse, asset_path: &str) -> Result<&'a SyncAsset> { +fn find_snapshot_asset<'a>(snapshot: &'a SyncResponse, asset_path: &str) -> Option<&'a SyncAsset> { snapshot .assets .iter() .find(|asset| asset.path == asset_path) - .ok_or_else(|| anyhow!("asset not found in target snapshot: {asset_path}")) } #[cfg(test)] @@ -433,10 +495,12 @@ mod tests { AssetDelta { path: "Content/A.uasset".to_string(), blob_hash: Some("hash-a".to_string()), + asset_id: None, }, AssetDelta { path: "Content/B.uasset".to_string(), blob_hash: None, + asset_id: None, }, ], }; @@ -504,8 +568,9 @@ mod tests { &mut workspace, &mut stage, "Content/A.uasset", - "old-hash", + Some("old-hash"), Some("head-hash"), + None, ); assert_eq!( @@ -540,6 +605,7 @@ mod tests { assets: vec![AssetDelta { path: "Content/A.uasset".to_string(), blob_hash: Some("local-hash".to_string()), + asset_id: None, }], }; @@ -547,8 +613,9 @@ mod tests { &mut workspace, &mut stage, "Content/A.uasset", - "head-hash", Some("head-hash"), + Some("head-hash"), + None, ); assert_eq!( @@ -694,6 +761,7 @@ mod tests { vec![AssetDelta { path: asset_path.to_string(), blob_hash: Some(StorageHash::hash_bytes(local_bytes)), + asset_id: None, }], ); @@ -729,7 +797,7 @@ mod tests { } #[test] - fn find_snapshot_asset_errors_when_target_missing() { + fn find_snapshot_asset_returns_none_when_target_missing() { let snapshot = SyncResponse { repo_id: "repo".to_string(), branch: "main".to_string(), @@ -741,11 +809,9 @@ mod tests { }], }; - let error = find_snapshot_asset(&snapshot, "Content/A.uasset").unwrap_err(); + let result = find_snapshot_asset(&snapshot, "Content/A.uasset"); - assert!(error - .to_string() - .contains("asset not found in target snapshot")); + assert!(result.is_none()); } #[test] diff --git a/crates/cli/src/utils.rs b/crates/cli/src/utils.rs index fb39d18..a86cd05 100644 --- a/crates/cli/src/utils.rs +++ b/crates/cli/src/utils.rs @@ -80,6 +80,8 @@ impl StageFile { pub(crate) struct AssetDelta { pub path: String, pub blob_hash: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub asset_id: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -633,13 +635,22 @@ pub(crate) fn detect_local_modifications(workspace: &WorkspaceState) -> Result) { +pub(crate) fn upsert_stage_asset( + stage: &mut StageFile, + path: &str, + blob_hash: Option, + asset_id: Option, +) { if let Some(existing) = stage.assets.iter_mut().find(|asset| asset.path == path) { existing.blob_hash = blob_hash; + if asset_id.is_some() { + existing.asset_id = asset_id; + } } else { stage.assets.push(AssetDelta { path: path.to_string(), blob_hash, + asset_id, }); } } @@ -736,6 +747,7 @@ pub(crate) fn checkpoint_assets_to_deltas(assets: &[CheckpointAsset]) -> Vec Date: Mon, 13 Jul 2026 23:01:50 +0800 Subject: [PATCH 3/6] fix(core): harden checkout storage locking and persistence --- Cargo.lock | 1 + Cargo.toml | 3 +- crates/cli/src/cmd/add.rs | 10 +- crates/cli/src/cmd/checkout.rs | 312 +++++++++++++++++- crates/cli/src/cmd/lock.rs | 14 +- crates/cli/src/cmd/remove.rs | 10 +- crates/cli/src/cmd/revert.rs | 220 +++++++----- crates/cli/src/cmd/status.rs | 4 +- crates/cli/src/utils.rs | 133 ++++++-- crates/server/Cargo.toml | 3 + crates/server/src/api/blobs.rs | 9 + crates/server/src/api/lock.rs | 155 ++++++++- crates/server/src/api/manifests.rs | 89 ++++- crates/server/src/api/storage.rs | 4 + crates/server/src/api/versioning.rs | 61 +++- crates/server/src/bootstrap.rs | 9 +- crates/server/src/core/lock.rs | 161 +++++++-- crates/server/src/core/lock/repo_pg.rs | 13 +- crates/server/src/core/storage.rs | 106 ++++-- crates/server/src/core/storage_backend.rs | 62 +++- crates/server/src/core/versioning.rs | 300 +++++++++++++---- crates/server/src/routes.rs | 83 ++--- crates/server/src/state.rs | 1 + crates/server/src/tests.rs | 205 +++++++++++- deploy/server/.env.example | 1 + deploy/server/.env.production.example | 1 + deploy/server/docker-compose.yml | 1 + docs/api/openapi.yaml | 9 + docs/operations/self-hosting.md | 4 + docs/server/README.md | 1 + ...202602260017_lock_primary_key_fix.down.sql | 20 ++ .../202602260017_lock_primary_key_fix.up.sql | 14 + 32 files changed, 1699 insertions(+), 320 deletions(-) create mode 100644 migrations/202602260017_lock_primary_key_fix.down.sql create mode 100644 migrations/202602260017_lock_primary_key_fix.up.sql diff --git a/Cargo.lock b/Cargo.lock index f27b924..5218432 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -897,6 +897,7 @@ dependencies = [ "tracing", "tracing-subscriber", "uuid", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c1ccaac..0426d7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,4 +31,5 @@ tower = { version = "0.5", features = ["util"] } async-trait = "0.1" hmac = "0.12" sha2 = "0.10" -hex = "0.4" +hex = "0.4" +windows-sys = { version = "0.61.2", features = ["Win32_Storage_FileSystem"] } diff --git a/crates/cli/src/cmd/add.rs b/crates/cli/src/cmd/add.rs index bc92c4a..927fa12 100644 --- a/crates/cli/src/cmd/add.rs +++ b/crates/cli/src/cmd/add.rs @@ -41,7 +41,15 @@ pub(crate) async fn execute(args: AddArgs) -> Result<()> { if stage.branch != branch { stage = StageFile::default_for_branch(&branch); } - upsert_stage_asset(&mut stage, &path, Some(blob), None); + let asset_id = load_workspace().ok().and_then(|workspace| { + (workspace.branch == branch) + .then_some(workspace.checked_out_assets) + .into_iter() + .flatten() + .find(|asset| asset.path == path) + .and_then(|asset| asset.asset_id) + }); + upsert_stage_asset(&mut stage, &path, Some(blob), asset_id); save_stage(&stage)?; println!("staged {} asset(s) on {}", stage.assets.len(), stage.branch); Ok(()) diff --git a/crates/cli/src/cmd/checkout.rs b/crates/cli/src/cmd/checkout.rs index 6762efd..52aa689 100644 --- a/crates/cli/src/cmd/checkout.rs +++ b/crates/cli/src/cmd/checkout.rs @@ -1,6 +1,6 @@ -use std::fs; +use std::{collections::HashSet, fs, path::Path}; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use clap::Args; use crate::utils::*; @@ -28,11 +28,24 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { let branch = args .branch .unwrap_or_else(|| profile.current_branch.clone()); + let workspace_root = std::env::current_dir()?; // Pre-check: detect local modifications before overwriting + let existing_workspace = load_workspace().ok(); + let matching_workspace = existing_workspace.as_ref().filter(|workspace| { + workspace.repo_id == repo && Path::new(&workspace.workspace_root) == workspace_root + }); if !args.force { - if let Ok(workspace) = load_workspace() { - let conflicts = detect_local_modifications(&workspace)?; + if let Ok(stage) = load_stage() { + if !stage.assets.is_empty() { + return Err(anyhow!( + "workspace has {} staged change(s); submit them or use --force", + stage.assets.len() + )); + } + } + if let Some(workspace) = matching_workspace { + let conflicts = detect_local_modifications(workspace)?; if !conflicts.is_empty() { eprintln!( "error: workspace has {} uncommitted modification(s), checkout would overwrite:", @@ -44,7 +57,7 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { eprintln!( "use 'ht add --file ' to stage changes, or use '--force' to overwrite." ); - std::process::exit(1); + return Err(anyhow!("checkout refused to overwrite local changes")); } } } @@ -58,6 +71,7 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { args.to_changeset_id.as_deref(), ) .await?; + validate_snapshot_layout(snapshot.assets.iter().map(|asset| asset.path.as_str()))?; if args.dry_run { println!( "checkout preview {}@{} to {} ({} assets)", @@ -74,11 +88,66 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { } return Ok(()); } - let workspace_root = std::env::current_dir()?; let mut checked_out_assets = Vec::with_capacity(snapshot.assets.len()); + let snapshot_paths = snapshot + .assets + .iter() + .map(|asset| asset.path.as_str()) + .collect::>(); + + if !args.force { + let tracked_paths = matching_workspace + .map(|workspace| { + workspace + .checked_out_assets + .iter() + .map(|asset| asset.path.as_str()) + .collect::>() + }) + .unwrap_or_default(); + let stale_paths = matching_workspace + .map(|workspace| { + workspace + .checked_out_assets + .iter() + .filter(|asset| !snapshot_paths.contains(asset.path.as_str())) + .map(|asset| asset.path.as_str()) + .collect::>() + }) + .unwrap_or_default(); + for asset in &snapshot.assets { + if tracked_paths.contains(asset.path.as_str()) { + continue; + } + let target = resolve_workspace_target(&workspace_root, &asset.path)?; + if target.is_dir() + && directory_contains_only_stale_assets(&workspace_root, &target, &stale_paths)? + { + continue; + } + if target.exists() + && (target.is_dir() + || hash_local_asset(&workspace_root, &asset.path)?.as_deref() + != Some(asset.blob_hash.as_str())) + { + return Err(anyhow!( + "checkout would overwrite untracked local file {}; use --force", + asset.path + )); + } + } + } + + for asset in &snapshot.assets { + fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; + } + + if let Some(previous) = matching_workspace { + remove_stale_tracked_files(&workspace_root, previous, &snapshot_paths)?; + } for asset in &snapshot.assets { - let target = workspace_root.join(asset.path.replace('/', std::path::MAIN_SEPARATOR_STR)); + let target = resolve_workspace_target(&workspace_root, &asset.path)?; let bytes = fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; if let Some(parent) = target.parent() { fs::create_dir_all(parent)?; @@ -88,6 +157,7 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { checked_out_assets.push(WorkspaceFile { path: asset.path.clone(), blob_hash: asset.blob_hash.clone(), + asset_id: asset.asset_id.clone(), }); } @@ -114,3 +184,231 @@ pub(crate) async fn execute(args: CheckoutArgs) -> Result<()> { ); Ok(()) } + +fn validate_snapshot_layout<'a>(paths: impl IntoIterator) -> Result<()> { + let mut normalized_paths = HashSet::new(); + for path in paths { + let normalized = path.replace('\\', "/"); + if !normalized_paths.insert(normalized.clone()) { + return Err(anyhow!("snapshot contains duplicate asset path: {path}")); + } + } + for path in &normalized_paths { + for (index, byte) in path.bytes().enumerate() { + if byte == b'/' && normalized_paths.contains(&path[..index]) { + return Err(anyhow!( + "snapshot asset paths conflict: {} and {}", + &path[..index], + path + )); + } + } + } + Ok(()) +} + +fn directory_contains_only_stale_assets( + workspace_root: &Path, + directory: &Path, + stale_paths: &HashSet<&str>, +) -> Result { + let mut contains_stale_asset = false; + for entry in fs::read_dir(directory) + .with_context(|| format!("failed to inspect directory {}", directory.display()))? + { + let entry = entry?; + let path = entry.path(); + let metadata = fs::symlink_metadata(&path) + .with_context(|| format!("failed to inspect {}", path.display()))?; + if metadata.file_type().is_symlink() { + return Ok(false); + } + if metadata.is_dir() { + if !directory_contains_only_stale_assets(workspace_root, &path, stale_paths)? { + return Ok(false); + } + contains_stale_asset = true; + continue; + } + if !metadata.is_file() { + return Ok(false); + } + let relative = path + .strip_prefix(workspace_root) + .with_context(|| format!("path escapes workspace: {}", path.display()))?; + let asset_path = normalize_asset_path(relative); + if !stale_paths.contains(asset_path.as_str()) { + return Ok(false); + } + contains_stale_asset = true; + } + Ok(contains_stale_asset) +} + +fn remove_stale_tracked_files( + workspace_root: &Path, + previous: &WorkspaceState, + snapshot_paths: &HashSet<&str>, +) -> Result<()> { + let mut stale_targets = previous + .checked_out_assets + .iter() + .filter(|asset| !snapshot_paths.contains(asset.path.as_str())) + .map(|asset| resolve_workspace_target(workspace_root, &asset.path)) + .collect::>>()?; + stale_targets.sort_by_key(|path| std::cmp::Reverse(path.components().count())); + + for target in stale_targets { + match fs::symlink_metadata(&target) { + Ok(metadata) if metadata.is_file() => { + fs::remove_file(&target) + .with_context(|| format!("failed to delete {}", target.display()))?; + remove_empty_parent_dirs(workspace_root, target.parent())?; + } + Ok(_) => { + return Err(anyhow!( + "tracked asset path is not a regular file: {}", + target.display() + )); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(error) + .with_context(|| format!("failed to inspect {}", target.display())); + } + } + } + Ok(()) +} + +fn remove_empty_parent_dirs(workspace_root: &Path, mut parent: Option<&Path>) -> Result<()> { + while let Some(directory) = parent.filter(|directory| *directory != workspace_root) { + match fs::remove_dir(directory) { + Ok(()) => parent = directory.parent(), + Err(error) + if matches!( + error.kind(), + std::io::ErrorKind::DirectoryNotEmpty | std::io::ErrorKind::NotFound + ) => + { + break; + } + Err(error) => { + return Err(error).with_context(|| { + format!("failed to remove empty directory {}", directory.display()) + }); + } + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn workspace(root: &Path, paths: &[&str]) -> WorkspaceState { + WorkspaceState { + repo_id: "repo-a".to_string(), + branch: "main".to_string(), + workspace_root: root.to_string_lossy().to_string(), + base_changeset_id: Some("cs-old".to_string()), + checked_out_assets: paths + .iter() + .map(|path| WorkspaceFile { + path: (*path).to_string(), + blob_hash: "0".repeat(64), + asset_id: None, + }) + .collect(), + last_synced_at: 1, + } + } + + fn temp_workspace(label: &str) -> std::path::PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("time") + .as_nanos(); + let root = std::env::temp_dir().join(format!( + "hypertide-checkout-{label}-{}-{nanos}", + std::process::id() + )); + fs::create_dir_all(&root).expect("create test workspace"); + root + } + + #[test] + fn stale_file_can_be_replaced_by_a_directory_tree() { + let root = temp_workspace("file-to-dir"); + fs::write(root.join("Content"), b"old").expect("write old file"); + let previous = workspace(&root, &["Content"]); + let snapshot_paths = HashSet::from(["Content/A.uasset"]); + + remove_stale_tracked_files(&root, &previous, &snapshot_paths).expect("remove stale file"); + fs::create_dir_all(root.join("Content")).expect("create replacement directory"); + fs::write(root.join("Content/A.uasset"), b"new").expect("write replacement file"); + + assert!(root.join("Content/A.uasset").is_file()); + let _ = fs::remove_dir_all(root); + } + + #[test] + fn stale_directory_tree_can_be_replaced_by_a_file() { + let root = temp_workspace("dir-to-file"); + fs::create_dir_all(root.join("Content")).expect("create old directory"); + fs::write(root.join("Content/A.uasset"), b"old").expect("write old file"); + let previous = workspace(&root, &["Content/A.uasset"]); + let snapshot_paths = HashSet::from(["Content"]); + let stale_paths = HashSet::from(["Content/A.uasset"]); + + assert!( + directory_contains_only_stale_assets(&root, &root.join("Content"), &stale_paths) + .expect("inspect old directory") + ); + remove_stale_tracked_files(&root, &previous, &snapshot_paths).expect("remove stale tree"); + fs::write(root.join("Content"), b"new").expect("write replacement file"); + + assert!(root.join("Content").is_file()); + let _ = fs::remove_dir_all(root); + } + + #[test] + fn untracked_files_block_directory_replacement() { + let root = temp_workspace("untracked-directory-entry"); + fs::create_dir_all(root.join("Content")).expect("create old directory"); + fs::write(root.join("Content/A.uasset"), b"tracked").expect("write tracked file"); + fs::write(root.join("Content/notes.txt"), b"untracked").expect("write untracked file"); + let stale_paths = HashSet::from(["Content/A.uasset"]); + + assert!( + !directory_contains_only_stale_assets(&root, &root.join("Content"), &stale_paths,) + .expect("inspect mixed directory") + ); + + let _ = fs::remove_dir_all(root); + } + + #[test] + fn untracked_empty_directories_block_directory_replacement() { + let root = temp_workspace("untracked-empty-directory"); + fs::create_dir_all(root.join("Content/empty")).expect("create empty directory"); + fs::write(root.join("Content/A.uasset"), b"tracked").expect("write tracked file"); + let stale_paths = HashSet::from(["Content/A.uasset"]); + + assert!( + !directory_contains_only_stale_assets(&root, &root.join("Content"), &stale_paths) + .expect("inspect directory with empty subtree") + ); + + let _ = fs::remove_dir_all(root); + } + + #[test] + fn snapshot_layout_rejects_duplicate_and_parent_asset_paths() { + assert!(validate_snapshot_layout(["Content/A", "Content/A"]).is_err()); + assert!(validate_snapshot_layout(["Content", "Content/A"]).is_err()); + assert!(validate_snapshot_layout(["Content\\A", "Content/A"]).is_err()); + assert!(validate_snapshot_layout(["Content/A", "Content/B"]).is_ok()); + } +} diff --git a/crates/cli/src/cmd/lock.rs b/crates/cli/src/cmd/lock.rs index d19b0a0..3f043f4 100644 --- a/crates/cli/src/cmd/lock.rs +++ b/crates/cli/src/cmd/lock.rs @@ -48,19 +48,19 @@ pub(crate) async fn execute(args: LockArgs) -> Result<()> { } async fn lock_acquire(args: LockPathArgs) -> Result<()> { - let lock = send_lock_path_request("lock acquire", "acquire", &args.path).await?; + let lock = send_lock_path_request("lock acquire", "acquire", &args.path, None).await?; print_lock("lock acquired", &lock); Ok(()) } async fn lock_release(args: LockPathArgs) -> Result<()> { - let lock = send_lock_path_request("lock release", "release", &args.path).await?; + let lock = send_lock_path_request("lock release", "release", &args.path, None).await?; print_lock("lock released", &lock); Ok(()) } async fn lock_renew(args: LockPathArgs) -> Result<()> { - let lock = send_lock_path_request("lock renew", "renew", &args.path).await?; + let lock = send_lock_path_request("lock renew", "renew", &args.path, None).await?; print_lock("lock renewed", &lock); Ok(()) } @@ -68,7 +68,8 @@ async fn lock_renew(args: LockPathArgs) -> Result<()> { async fn lock_list() -> Result<()> { let mut profile = load_profile()?; let client = reqwest::Client::new(); - let locks = fetch_locks(&client, &mut profile).await?; + let repo_id = resolve_repo(&profile, None)?; + let locks = fetch_locks(&client, &mut profile, &repo_id).await?; if locks.is_empty() { println!("no active locks"); } else { @@ -83,11 +84,16 @@ async fn lock_force_release(args: LockForceReleaseArgs) -> Result<()> { confirm_dangerous(&format!("force release lock on {}", args.path), args.yes)?; let mut profile = load_profile()?; let client = reqwest::Client::new(); + let repo_id = resolve_repo(&profile, None)?; let payload = LockRequest { file_path: &args.path, + repo_id: &repo_id, + scope: "asset", }; let high_risk_payload = serde_json::json!({ "file_path": args.path, + "repo_id": &repo_id, + "scope": "asset", }); let high_risk = build_high_risk_headers( args.high_risk_secret.as_deref(), diff --git a/crates/cli/src/cmd/remove.rs b/crates/cli/src/cmd/remove.rs index 5f01241..5e3af63 100644 --- a/crates/cli/src/cmd/remove.rs +++ b/crates/cli/src/cmd/remove.rs @@ -20,7 +20,15 @@ pub(crate) async fn execute(args: RemoveArgs) -> Result<()> { if stage.branch != branch { stage = StageFile::default_for_branch(&branch); } - upsert_stage_asset(&mut stage, &args.asset_path, None, None); + let asset_id = load_workspace().ok().and_then(|workspace| { + (workspace.branch == branch) + .then_some(workspace.checked_out_assets) + .into_iter() + .flatten() + .find(|asset| asset.path == args.asset_path) + .and_then(|asset| asset.asset_id) + }); + upsert_stage_asset(&mut stage, &args.asset_path, None, asset_id); save_stage(&stage)?; println!( "staged delete for {} on {} ({} asset(s) staged)", diff --git a/crates/cli/src/cmd/revert.rs b/crates/cli/src/cmd/revert.rs index 498ff8a..f0d1dec 100644 --- a/crates/cli/src/cmd/revert.rs +++ b/crates/cli/src/cmd/revert.rs @@ -52,11 +52,12 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { } let has_staged_delta = stage.assets.iter().any(|asset| asset.path == asset_path); - let base_hash = workspace + let base_asset = workspace .checked_out_assets .iter() - .find(|asset| asset.path == asset_path) - .map(|asset| asset.blob_hash.clone()); + .find(|asset| asset.path == asset_path); + let base_hash = base_asset.map(|asset| asset.blob_hash.clone()); + let base_asset_id = base_asset.and_then(|asset| asset.asset_id.clone()); let local_hash = hash_local_asset(&workspace_root, &asset_path)?; let overwrites_local_change = match (local_hash.as_deref(), base_hash.as_deref()) { (Some(local), Some(base)) => local != base, @@ -90,7 +91,7 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { let snapshot_asset = find_snapshot_asset(&snapshot, &asset_path); - let _update = match snapshot_asset { + let (update, restored_hash) = match snapshot_asset { Some(asset) => { let bytes = fetch_blob_bytes(&client, &mut profile, &asset.blob_hash).await?; if let Some(parent) = target.parent() { @@ -109,51 +110,7 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { asset.asset_id.clone(), ); - if !args.keep_lock { - if let Err(err) = - send_lock_path_request("lock release", "release", &asset_path).await - { - eprintln!( - "warning: reverted {}, but failed to release lock: {err}", - asset_path - ); - eprintln!( - "run `ht lock release --path {}` manually if needed", - asset_path - ); - } - } - - if json_output_enabled() { - println!( - "{}", - serde_json::to_string_pretty(&serde_json::json!({ - "ok": true, - "asset_path": asset_path, - "restored_hash": asset.blob_hash, - }))? - ); - } else { - println!( - "reverted {} to {} on {}@{}{}", - asset_path, - snapshot - .changeset_id - .as_deref() - .unwrap_or(ROOT_BASE_CHANGESET_ID), - repo, - branch, - if update.removed_staged_delta { - " (removed staged delta)" - } else if update.staged_delta { - " (staged delta)" - } else { - "" - } - ); - } - - update + (update, Some(asset.blob_hash.clone())) } None => { // Asset not found in target snapshot — stage a deletion @@ -168,40 +125,71 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { &asset_path, None, base_hash.as_deref(), - None, + base_asset_id, ); - if json_output_enabled() { - println!( - "{}", - serde_json::to_string_pretty(&serde_json::json!({ - "ok": true, - "asset_path": asset_path, - "restored_hash": null, - "staged_deletion": true, - }))? - ); - } else { - println!( - "reverted {} to absent on {}@{}{}", - asset_path, - repo, - branch, - if update.staged_delta { - " (staged deletion)" - } else { - "" - } - ); - } - - update + (update, None) } }; workspace.last_synced_at = now_unix(); save_workspace(&workspace)?; save_stage(&stage)?; + if !args.keep_lock { + if let Err(err) = + send_lock_path_request("lock release", "release", &asset_path, Some(&repo)).await + { + eprintln!( + "warning: reverted {}, but failed to release lock: {err}", + asset_path + ); + eprintln!( + "run `ht lock release --path {}` manually if needed", + asset_path + ); + } + } + if json_output_enabled() { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "ok": true, + "asset_path": asset_path, + "restored_hash": restored_hash.as_deref(), + "staged_deletion": restored_hash.is_none() && update.staged_delta, + }))? + ); + } else if restored_hash.is_some() { + println!( + "reverted {} to {} on {}@{}{}", + asset_path, + snapshot + .changeset_id + .as_deref() + .unwrap_or(ROOT_BASE_CHANGESET_ID), + repo, + branch, + if update.removed_staged_delta { + " (removed staged delta)" + } else if update.staged_delta { + " (staged delta)" + } else { + "" + } + ); + } else { + println!( + "reverted {} to absent on {}@{}{}", + asset_path, + repo, + branch, + if update.staged_delta { + " (staged deletion)" + } else { + "" + } + ); + } Ok(()) } @@ -232,19 +220,28 @@ fn remove_staged_asset(stage: &mut StageFile, asset_path: &str) -> bool { stage.assets.len() != original_len } -fn update_workspace_asset(workspace: &mut WorkspaceState, asset_path: &str, blob_hash: &str) { +fn update_workspace_asset( + workspace: &mut WorkspaceState, + asset_path: &str, + blob_hash: &str, + asset_id: Option, +) { if let Some(existing) = workspace .checked_out_assets .iter_mut() .find(|asset| asset.path == asset_path) { existing.blob_hash = blob_hash.to_string(); + if asset_id.is_some() { + existing.asset_id = asset_id; + } return; } workspace.checked_out_assets.push(WorkspaceFile { path: asset_path.to_string(), blob_hash: blob_hash.to_string(), + asset_id, }); } @@ -258,7 +255,7 @@ fn apply_revert_state( ) -> RevertStateUpdate { if base_hash == blob_hash { if let Some(hash) = blob_hash { - update_workspace_asset(workspace, asset_path, hash); + update_workspace_asset(workspace, asset_path, hash, asset_id); } return RevertStateUpdate { removed_staged_delta: remove_staged_asset(stage, asset_path), @@ -474,6 +471,7 @@ mod tests { checked_out_assets: vec![WorkspaceFile { path: asset_path.to_string(), blob_hash: base_hash.to_string(), + asset_id: None, }], last_synced_at: 1, }) @@ -521,11 +519,12 @@ mod tests { checked_out_assets: vec![WorkspaceFile { path: "Content/A.uasset".to_string(), blob_hash: "old".to_string(), + asset_id: None, }], last_synced_at: 1, }; - update_workspace_asset(&mut workspace, "Content/A.uasset", "new"); + update_workspace_asset(&mut workspace, "Content/A.uasset", "new", None); assert_eq!(workspace.checked_out_assets.len(), 1); assert_eq!(workspace.checked_out_assets[0].blob_hash, "new"); @@ -542,7 +541,7 @@ mod tests { last_synced_at: 1, }; - update_workspace_asset(&mut workspace, "Content/A.uasset", "hash-a"); + update_workspace_asset(&mut workspace, "Content/A.uasset", "hash-a", None); assert_eq!(workspace.checked_out_assets.len(), 1); assert_eq!(workspace.checked_out_assets[0].path, "Content/A.uasset"); @@ -559,6 +558,7 @@ mod tests { checked_out_assets: vec![WorkspaceFile { path: "Content/A.uasset".to_string(), blob_hash: "head-hash".to_string(), + asset_id: None, }], last_synced_at: 1, }; @@ -596,6 +596,7 @@ mod tests { checked_out_assets: vec![WorkspaceFile { path: "Content/A.uasset".to_string(), blob_hash: "head-hash".to_string(), + asset_id: None, }], last_synced_at: 1, }; @@ -842,4 +843,65 @@ mod tests { "Content/A.uasset" ); } + + #[test] + fn workspace_path_resolution_rejects_parent_traversal() { + let root = std::env::current_dir().expect("current dir"); + assert!(resolve_workspace_target(&root, "../outside.txt").is_err()); + assert!(resolve_workspace_target(&root, "Content/A.uasset").is_ok()); + } + + #[test] + fn local_deletion_is_reported_as_a_workspace_conflict() { + let root = unique_workspace("deleted-conflict"); + std::fs::create_dir_all(&root).expect("workspace root"); + let workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: root.to_string_lossy().to_string(), + base_changeset_id: Some("cs-1".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/Missing.uasset".to_string(), + blob_hash: "0".repeat(64), + asset_id: Some("asset-missing".to_string()), + }], + last_synced_at: 1, + }; + + let conflicts = detect_local_modifications(&workspace).expect("detect conflicts"); + assert_eq!(conflicts.len(), 1); + assert_eq!(conflicts[0].path, "Content/Missing.uasset"); + assert!(conflicts[0].local_hash.is_none()); + let _ = std::fs::remove_dir_all(root); + } + + #[test] + fn revert_deletion_preserves_the_base_asset_identity() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: Some("cs-head".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "head-hash".to_string(), + asset_id: Some("stable-asset-a".to_string()), + }], + last_synced_at: 1, + }; + let mut stage = StageFile::default_for_branch("main"); + + apply_revert_state( + &mut workspace, + &mut stage, + "Content/A.uasset", + None, + Some("head-hash"), + Some("stable-asset-a".to_string()), + ); + + assert_eq!(stage.assets.len(), 1); + assert_eq!(stage.assets[0].asset_id.as_deref(), Some("stable-asset-a")); + assert!(stage.assets[0].blob_hash.is_none()); + } } diff --git a/crates/cli/src/cmd/status.rs b/crates/cli/src/cmd/status.rs index 74b5427..eb57bd4 100644 --- a/crates/cli/src/cmd/status.rs +++ b/crates/cli/src/cmd/status.rs @@ -23,7 +23,9 @@ pub(crate) async fn execute(args: StatusArgs) -> Result<()> { let branch = args.branch.unwrap_or_else(|| workspace.branch.clone()); let client = reqwest::Client::new(); let stage = load_stage().unwrap_or_else(|_| StageFile::default_for_branch(&branch)); - let locks = fetch_locks(&client, &mut profile).await.unwrap_or_default(); + let locks = fetch_locks(&client, &mut profile, &repo) + .await + .unwrap_or_default(); let head = resolve_base_changeset(&client, &mut profile, &repo, &branch, None).await?; let stale_base = workspace .base_changeset_id diff --git a/crates/cli/src/utils.rs b/crates/cli/src/utils.rs index a86cd05..eb33c44 100644 --- a/crates/cli/src/utils.rs +++ b/crates/cli/src/utils.rs @@ -88,6 +88,8 @@ pub(crate) struct AssetDelta { pub(crate) struct WorkspaceFile { pub path: String, pub blob_hash: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub asset_id: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -111,11 +113,21 @@ pub(crate) struct FileLockInfo { pub owner_id: String, pub locked_at: String, pub lease_expires_at: Option, + #[serde(default)] + pub repo_id: String, + #[serde(default = "default_lock_scope")] + pub scope: String, } #[derive(Debug, Serialize)] pub(crate) struct LockRequest<'a> { pub file_path: &'a str, + pub repo_id: &'a str, + pub scope: &'a str, +} + +fn default_lock_scope() -> String { + "asset".to_string() } #[derive(Debug, Serialize)] @@ -458,7 +470,7 @@ pub(crate) struct AssetRow { pub(crate) struct ConflictEntry { pub path: String, pub base_hash: String, - pub local_hash: String, + pub local_hash: Option, } pub(crate) struct StorageHash; @@ -564,6 +576,7 @@ pub(crate) fn ensure_state_dir() -> Result<()> { } pub(crate) fn cache_object_path(hash: &str) -> Result { + validate_blake3_hash(hash)?; let paths = state_paths()?; Ok(workspace::cache_object_path(&paths, hash)) } @@ -609,7 +622,7 @@ pub(crate) fn hash_bytes(bytes: &[u8]) -> String { } pub(crate) fn hash_local_asset(workspace_root: &Path, asset_path: &str) -> Result> { - let target = workspace_root.join(asset_path.replace('/', std::path::MAIN_SEPARATOR_STR)); + let target = resolve_workspace_target(workspace_root, asset_path)?; if !target.exists() { return Ok(None); } @@ -622,14 +635,13 @@ pub(crate) fn detect_local_modifications(workspace: &WorkspaceState) -> Result Vec Result { let normalized = asset_path.replace('/', std::path::MAIN_SEPARATOR_STR); let candidate = PathBuf::from(&normalized); - if candidate.is_absolute() { + let raw = asset_path.replace('\\', "/"); + let bytes = raw.as_bytes(); + let has_windows_drive_prefix = + bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'; + if candidate.is_absolute() || has_windows_drive_prefix { return Err(anyhow!( "checkpoint asset path must be relative: {asset_path}" )); @@ -774,9 +790,39 @@ pub(crate) fn resolve_workspace_target(workspace_root: &Path, asset_path: &str) "checkpoint asset path escapes workspace: {asset_path}" )); } + let mut current = workspace_root.to_path_buf(); + for component in target + .strip_prefix(workspace_root) + .with_context(|| format!("asset path escapes workspace: {asset_path}"))? + .components() + { + current.push(component.as_os_str()); + match fs::symlink_metadata(¤t) { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(anyhow!( + "asset path traverses a symbolic link: {asset_path}" + )); + } + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(error) + .with_context(|| format!("failed to inspect {}", current.display())); + } + } + } Ok(target) } +pub(crate) fn validate_blake3_hash(hash: &str) -> Result<()> { + if hash.len() != 64 || !hash.bytes().all(|byte| byte.is_ascii_hexdigit()) { + return Err(anyhow!( + "invalid BLAKE3 hash: expected 64 hexadecimal characters" + )); + } + Ok(()) +} + // ── Print helpers ── pub(crate) fn print_changeset_action(action: &str, changeset: &ChangesetRecord) { @@ -793,9 +839,15 @@ pub(crate) fn print_changeset_action(action: &str, changeset: &ChangesetRecord) pub(crate) fn print_lock(label: &str, lock: &FileLockInfo) { println!( - "{}: {} owner={} locked_at={} lease_expires_at={}", + "{}: {} repo={} scope={} owner={} locked_at={} lease_expires_at={}", label, lock.file_path, + if lock.repo_id.is_empty() { + "" + } else { + &lock.repo_id + }, + lock.scope, lock.owner_id, lock.locked_at, lock.lease_expires_at.as_deref().unwrap_or("") @@ -1140,8 +1192,17 @@ pub(crate) async fn fetch_blob_bytes( ) -> Result> { let cache_path = cache_object_path(blob_hash)?; if cache_path.exists() { - return fs::read(&cache_path) - .with_context(|| format!("failed to read cached object {}", cache_path.display())); + let bytes = fs::read(&cache_path) + .with_context(|| format!("failed to read cached object {}", cache_path.display()))?; + let actual = StorageHash::hash_bytes(&bytes); + if actual != blob_hash { + return Err(anyhow!( + "cached object hash mismatch for {}: got {}", + blob_hash, + actual + )); + } + return Ok(bytes); } ensure_access_token(client, profile).await?; @@ -1159,6 +1220,14 @@ pub(crate) async fn fetch_blob_bytes( )); } let bytes = response.bytes().await?.to_vec(); + let actual = StorageHash::hash_bytes(&bytes); + if actual != blob_hash { + return Err(anyhow!( + "downloaded object hash mismatch for {}: got {}", + blob_hash, + actual + )); + } cache_blob(blob_hash, &bytes)?; Ok(bytes) } @@ -1166,12 +1235,20 @@ pub(crate) async fn fetch_blob_bytes( pub(crate) async fn fetch_locks( client: &reqwest::Client, profile: &mut CliProfile, + repo_id: &str, ) -> Result> { let url = format!("{}/v2/locks", profile.server.trim_end_matches('/')); let response: ApiResponse> = send_authed_api( client, profile, - |client, profile| with_auth(client.get(&url), profile), + |client, profile| { + with_auth( + client + .get(&url) + .query(&[("repo_id", repo_id), ("scope", "asset")]), + profile, + ) + }, "locks response decode failed", ) .await?; @@ -1584,7 +1661,7 @@ pub(crate) async fn collect_checkpoint_assets( .into_iter() .filter_map(|asset| { asset.blob_hash.map(|blob_hash| CheckpointAsset { - asset_id: asset.path.clone(), + asset_id: asset.asset_id.unwrap_or_else(|| asset.path.clone()), path: asset.path, blob_hash, }) @@ -1631,6 +1708,7 @@ pub(crate) async fn materialize_checkpoint_snapshot( checked_out_assets.push(WorkspaceFile { path: asset.path.clone(), blob_hash: asset.blob_hash.clone(), + asset_id: Some(asset.asset_id.clone()), }); } save_workspace(&WorkspaceState { @@ -1653,10 +1731,19 @@ pub(crate) async fn send_lock_path_request( action: &str, endpoint: &str, path: &str, + repo_id: Option<&str>, ) -> Result { let mut profile = load_profile()?; let client = reqwest::Client::new(); - let payload = LockRequest { file_path: path }; + let repo_id = match repo_id { + Some(repo_id) => repo_id.to_string(), + None => resolve_repo(&profile, None)?, + }; + let payload = LockRequest { + file_path: path, + repo_id: &repo_id, + scope: "asset", + }; let url = format!( "{}/v2/locks/{}", profile.server.trim_end_matches('/'), @@ -1714,7 +1801,15 @@ pub(crate) async fn add_file( if stage.branch != branch { stage = StageFile::default_for_branch(branch); } - upsert_stage_asset(&mut stage, &repo_path, Some(blob_hash.clone()), None); + let asset_id = load_workspace().ok().and_then(|workspace| { + (workspace.branch == branch) + .then_some(workspace.checked_out_assets) + .into_iter() + .flatten() + .find(|asset| asset.path == repo_path) + .and_then(|asset| asset.asset_id) + }); + upsert_stage_asset(&mut stage, &repo_path, Some(blob_hash.clone()), asset_id); save_stage(&stage)?; println!( "staged file {} as {} on {} (blob={})", diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 663b480..554d5af 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -32,6 +32,9 @@ hmac.workspace = true sha2.workspace = true hex.workspace = true +[target.'cfg(windows)'.dependencies] +windows-sys.workspace = true + [features] # Public open-core builds keep this feature as a compatibility stub. # Commercial Enterprise crates live outside this public workspace and depend on diff --git a/crates/server/src/api/blobs.rs b/crates/server/src/api/blobs.rs index 6dcfa3e..a6a3ad5 100644 --- a/crates/server/src/api/blobs.rs +++ b/crates/server/src/api/blobs.rs @@ -66,6 +66,15 @@ pub async fn missing_chunks( let mut unique_hashes = payload.chunk_hashes.clone(); unique_hashes.sort(); unique_hashes.dedup(); + if unique_hashes + .iter() + .any(|hash| StorageManager::validate_hash(hash).is_err()) + { + return ( + StatusCode::BAD_REQUEST, + Json(ApiResponse::err("invalid chunk hash")), + ); + } let missing = if let Some(pool) = state.db_pool.as_ref() { match sqlx::query_scalar::<_, String>( diff --git a/crates/server/src/api/lock.rs b/crates/server/src/api/lock.rs index dc45ce7..3e0f2cb 100644 --- a/crates/server/src/api/lock.rs +++ b/crates/server/src/api/lock.rs @@ -7,7 +7,7 @@ use crate::core::auth::{AuthIdentity, Permission}; use crate::core::lock::FileLock; use crate::AppState; use axum::{ - extract::State, + extract::{Query, State}, http::{HeaderMap, StatusCode}, Json, }; @@ -18,23 +18,50 @@ use serde_json::json; pub struct LockRequest { pub file_path: String, pub owner_id: Option, + #[serde(default)] + pub repo_id: String, + #[serde(default = "default_scope")] + pub scope: String, } #[derive(Debug, Deserialize)] pub struct UnlockRequest { pub file_path: String, pub owner_id: Option, + #[serde(default)] + pub repo_id: String, + #[serde(default = "default_scope")] + pub scope: String, } #[derive(Debug, Deserialize)] pub struct RenewLockRequest { pub file_path: String, pub owner_id: Option, + #[serde(default)] + pub repo_id: String, + #[serde(default = "default_scope")] + pub scope: String, } #[derive(Debug, Deserialize)] pub struct ForceUnlockRequest { pub file_path: String, + #[serde(default)] + pub repo_id: String, + #[serde(default = "default_scope")] + pub scope: String, +} + +#[derive(Debug, Deserialize)] +pub struct ListLocksQuery { + pub repo_id: String, + #[serde(default = "default_scope")] + pub scope: String, +} + +fn default_scope() -> String { + "asset".to_string() } async fn require_permission( @@ -57,6 +84,29 @@ fn resolve_owner_id( Ok(identity.owner_id.clone()) } +fn validate_lock_target( + repo_id: &str, + scope: &str, + file_path: &str, +) -> Result<(), (StatusCode, String)> { + let normalized = file_path.replace('\\', "/"); + if repo_id.trim().is_empty() + || scope.trim().is_empty() + || normalized.trim().is_empty() + || normalized.starts_with('/') + || normalized.contains(':') + || normalized + .split('/') + .any(|component| component.is_empty() || component == "." || component == "..") + { + return Err(( + StatusCode::BAD_REQUEST, + "repo_id, scope, and a safe file_path are required".to_string(), + )); + } + Ok(()) +} + /// POST /v2/locks/acquire /// Request a lock on a file pub async fn lock_file( @@ -72,12 +122,22 @@ pub async fn lock_file( Ok(owner_id) => owner_id, Err((status, message)) => return (status, Json(ApiResponse::err(message))), }; + if let Err((status, message)) = + validate_lock_target(&payload.repo_id, &payload.scope, &payload.file_path) + { + return (status, Json(ApiResponse::err(message))); + } let event_meta = crate::core::events::EventMetadata::from_headers(&headers); match state .lock_manager - .try_lock(payload.file_path, owner_id) + .try_lock_with_repo( + payload.file_path, + owner_id, + &payload.repo_id, + &payload.scope, + ) .await { Ok(lock) => { @@ -88,7 +148,12 @@ pub async fn lock_file( &lock.owner_id, None, None, - json!({ "file_path": lock.file_path, "lease_expires_at": lock.lease_expires_at }), + json!({ + "file_path": lock.file_path, + "repo_id": lock.repo_id, + "scope": lock.scope, + "lease_expires_at": lock.lease_expires_at, + }), &event_meta, ) .await @@ -111,7 +176,7 @@ pub async fn unlock_file( State(state): State, headers: HeaderMap, Json(payload): Json, -) -> (StatusCode, Json>) { +) -> (StatusCode, Json>) { let identity = match require_permission(&state, &headers, Permission::Lock).await { Ok(identity) => identity, Err((status, message)) => return (status, Json(ApiResponse::err(message))), @@ -120,15 +185,25 @@ pub async fn unlock_file( Ok(owner_id) => owner_id, Err((status, message)) => return (status, Json(ApiResponse::err(message))), }; + if let Err((status, message)) = + validate_lock_target(&payload.repo_id, &payload.scope, &payload.file_path) + { + return (status, Json(ApiResponse::err(message))); + } let event_meta = crate::core::events::EventMetadata::from_headers(&headers); match state .lock_manager - .unlock(&payload.file_path, &owner_id) + .unlock_with_repo( + &payload.file_path, + &owner_id, + &payload.repo_id, + &payload.scope, + ) .await { - Ok(_) => { + Ok(lock) => { if let Some(event_store) = &state.event_store { if let Err(error) = event_store .append( @@ -136,7 +211,11 @@ pub async fn unlock_file( &owner_id, None, None, - json!({ "file_path": payload.file_path }), + json!({ + "file_path": payload.file_path, + "repo_id": payload.repo_id, + "scope": payload.scope, + }), &event_meta, ) .await @@ -144,7 +223,7 @@ pub async fn unlock_file( tracing::warn!("failed to append lock release event: {error}"); } } - (StatusCode::OK, Json(ApiResponse::ok(()))) + (StatusCode::OK, Json(ApiResponse::ok(lock))) } Err(error) => { let (status, response) = map_error(error); @@ -168,12 +247,22 @@ pub async fn renew_lock_file( Ok(owner_id) => owner_id, Err((status, message)) => return (status, Json(ApiResponse::err(message))), }; + if let Err((status, message)) = + validate_lock_target(&payload.repo_id, &payload.scope, &payload.file_path) + { + return (status, Json(ApiResponse::err(message))); + } let event_meta = crate::core::events::EventMetadata::from_headers(&headers); match state .lock_manager - .renew_lock(&payload.file_path, &owner_id) + .renew_lock_with_repo( + &payload.file_path, + &owner_id, + &payload.repo_id, + &payload.scope, + ) .await { Ok(lock) => { @@ -184,7 +273,12 @@ pub async fn renew_lock_file( &owner_id, None, None, - json!({ "file_path": payload.file_path, "lease_expires_at": lock.lease_expires_at }), + json!({ + "file_path": payload.file_path, + "repo_id": payload.repo_id, + "scope": payload.scope, + "lease_expires_at": lock.lease_expires_at, + }), &event_meta, ) .await @@ -211,13 +305,22 @@ pub async fn force_unlock_file( if let Err((status, message)) = require_permission(&state, &headers, Permission::Admin).await { return (status, Json(ApiResponse::err(message))); } + if let Err((status, message)) = + validate_lock_target(&payload.repo_id, &payload.scope, &payload.file_path) + { + return (status, Json(ApiResponse::err(message))); + } if let Some(guard) = &state.high_risk_guard { if let Err(message) = guard .verify( &headers, "LOCK_FORCE_RELEASE", "system-admin", - &json!({ "file_path": payload.file_path }), + &json!({ + "file_path": payload.file_path, + "repo_id": payload.repo_id, + "scope": payload.scope, + }), ) .await { @@ -227,7 +330,11 @@ pub async fn force_unlock_file( let event_meta = crate::core::events::EventMetadata::from_headers(&headers); - match state.lock_manager.force_unlock(&payload.file_path).await { + match state + .lock_manager + .force_unlock_with_repo(&payload.file_path, &payload.repo_id, &payload.scope) + .await + { Ok(true) => { if let Some(event_store) = &state.event_store { if let Err(error) = event_store @@ -236,7 +343,11 @@ pub async fn force_unlock_file( "system-admin", None, None, - json!({ "file_path": payload.file_path }), + json!({ + "file_path": payload.file_path, + "repo_id": payload.repo_id, + "scope": payload.scope, + }), &event_meta, ) .await @@ -251,7 +362,11 @@ pub async fn force_unlock_file( "system-admin", None, Some(&payload.file_path), - json!({ "file_path": payload.file_path }), + json!({ + "file_path": payload.file_path, + "repo_id": payload.repo_id, + "scope": payload.scope, + }), ) .await { @@ -276,11 +391,21 @@ pub async fn force_unlock_file( pub async fn list_locks( State(state): State, headers: HeaderMap, + Query(query): Query, ) -> (StatusCode, Json>>) { if let Err((status, message)) = require_permission(&state, &headers, Permission::Lock).await { return (status, Json(ApiResponse::err(message))); } - let locks = state.lock_manager.list_locks(); + if query.repo_id.trim().is_empty() || query.scope.trim().is_empty() { + return ( + StatusCode::BAD_REQUEST, + Json(ApiResponse::err("repo_id and scope are required")), + ); + } + + let locks = state + .lock_manager + .list_locks_with_repo(&query.repo_id, &query.scope); (StatusCode::OK, Json(ApiResponse::ok(locks))) } diff --git a/crates/server/src/api/manifests.rs b/crates/server/src/api/manifests.rs index 6a81ebf..24e41d3 100644 --- a/crates/server/src/api/manifests.rs +++ b/crates/server/src/api/manifests.rs @@ -13,6 +13,17 @@ use crate::api::{common::ApiResponse, middleware::authz}; use crate::core::{auth::Permission, storage::StorageManager}; use crate::AppState; +const MAX_MANIFEST_CHUNKS: usize = 4096; +const DEFAULT_MAX_COMPOSED_BLOB_BYTES: u64 = 256 * 1024 * 1024; + +fn max_composed_blob_bytes() -> u64 { + std::env::var("MAX_COMPOSED_BLOB_BYTES") + .ok() + .and_then(|value| value.parse::().ok()) + .filter(|value| *value > 0) + .unwrap_or(DEFAULT_MAX_COMPOSED_BLOB_BYTES) +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct ManifestChunk { pub i: u32, @@ -164,6 +175,14 @@ pub async fn create_manifest( Json(ApiResponse::err("chunks must not be empty")), ); } + if payload.chunks.len() > MAX_MANIFEST_CHUNKS { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err(format!( + "manifest exceeds maximum chunk count of {MAX_MANIFEST_CHUNKS}" + ))), + ); + } if payload.chunk_size_policy.trim().is_empty() { return ( StatusCode::BAD_REQUEST, @@ -180,12 +199,34 @@ pub async fn create_manifest( ); } } + let max_composed_blob_bytes = max_composed_blob_bytes(); + let declared_size = payload + .chunks + .iter() + .try_fold(0u64, |total, chunk| total.checked_add(chunk.size)); + if declared_size.is_none_or(|size| size > max_composed_blob_bytes) { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err(format!( + "manifest exceeds maximum composed size of {max_composed_blob_bytes} bytes" + ))), + ); + } let chunk_hashes = payload .chunks .iter() .map(|chunk| chunk.chunk_hash.clone()) .collect::>(); + if chunk_hashes + .iter() + .any(|hash| StorageManager::validate_hash(hash).is_err()) + { + return ( + StatusCode::BAD_REQUEST, + Json(ApiResponse::err("manifest contains an invalid chunk hash")), + ); + } let missing = if let Some(pool) = state.db_pool.as_ref() { match sqlx::query_scalar::<_, String>( @@ -374,7 +415,41 @@ pub async fn compose_blob( Err((status, message)) => return (status, Json(ApiResponse::err(message))), }; + if manifest.chunks.len() > MAX_MANIFEST_CHUNKS { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err("manifest contains too many chunks")), + ); + } + let max_composed_blob_bytes = max_composed_blob_bytes(); + let declared_size = manifest + .chunks + .iter() + .try_fold(0u64, |total, chunk| total.checked_add(chunk.size)); + let Some(declared_size) = declared_size.filter(|size| *size <= max_composed_blob_bytes) else { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err("composed blob exceeds size limit")), + ); + }; + + let Ok(reservation_size) = usize::try_from(declared_size) else { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err( + "composed blob exceeds platform address space", + )), + ); + }; let mut composed = Vec::new(); + if let Err(error) = composed.try_reserve(reservation_size) { + return ( + StatusCode::INSUFFICIENT_STORAGE, + Json(ApiResponse::err(format!( + "failed to reserve memory for composed blob: {error}" + ))), + ); + } let mut total_size: u64 = 0; for chunk in manifest.chunks { let bytes = match state.storage_manager.retrieve(&chunk.chunk_hash).await { @@ -400,7 +475,19 @@ pub async fn compose_blob( ))), ); } - total_size += bytes.len() as u64; + let Some(next_size) = total_size.checked_add(bytes.len() as u64) else { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err("composed blob size overflow")), + ); + }; + if next_size > max_composed_blob_bytes { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + Json(ApiResponse::err("composed blob exceeds size limit")), + ); + } + total_size = next_size; composed.extend_from_slice(&bytes); } diff --git a/crates/server/src/api/storage.rs b/crates/server/src/api/storage.rs index de8b8d6..3db94de 100644 --- a/crates/server/src/api/storage.rs +++ b/crates/server/src/api/storage.rs @@ -153,6 +153,10 @@ pub async fn check_exists( { return (status, Json(ApiResponse::err(message))); } + if let Err(error) = StorageManager::validate_hash(&hash) { + let (status, response) = map_error::(error); + return (status, Json(response)); + } match state.storage_manager.exists(&hash).await { Ok(exists) => (StatusCode::OK, Json(ApiResponse::ok(exists))), diff --git a/crates/server/src/api/versioning.rs b/crates/server/src/api/versioning.rs index 86371ca..b9e3566 100644 --- a/crates/server/src/api/versioning.rs +++ b/crates/server/src/api/versioning.rs @@ -5,6 +5,7 @@ use axum::{ }; use serde::{Deserialize, Serialize}; use serde_json::json; +use std::path::{Component, Path as FsPath}; use crate::api::common::ApiResponse; use crate::api::middleware::authz; @@ -190,6 +191,14 @@ fn map_versioning_error(error: VersioningError) -> (StatusCode, String) { "Changeset state invalid: repo={repo_id}, changeset={changeset_id}, status={status:?}, expected={expected}" ), ), + VersioningError::InvalidAssetLayout { repo_id, message } => ( + StatusCode::BAD_REQUEST, + format!("Invalid asset layout for {repo_id}: {message}"), + ), + VersioningError::Persistence { message } => ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Versioning persistence failed: {message}"), + ), } } @@ -209,13 +218,48 @@ fn validate_repo_and_branch(repo_id: &str, branch: &str) -> Result<(), (StatusCo Ok(()) } +fn validate_asset_paths(assets: &[AssetDelta]) -> Result<(), (StatusCode, String)> { + for asset in assets { + let normalized = asset.path.replace('\\', "/"); + let path = FsPath::new(&normalized); + let bytes = normalized.as_bytes(); + let has_windows_drive_prefix = + bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'; + let invalid = normalized.trim().is_empty() + || normalized.ends_with('/') + || normalized.contains(':') + || has_windows_drive_prefix + || path.is_absolute() + || path.components().any(|component| { + matches!( + component, + Component::CurDir + | Component::ParentDir + | Component::RootDir + | Component::Prefix(_) + ) + }); + if invalid { + return Err(( + StatusCode::BAD_REQUEST, + format!("invalid asset path: {}", asset.path), + )); + } + } + Ok(()) +} + fn ensure_lock_access( state: &AppState, owner_id: &str, + repo_id: &str, assets: &[AssetDelta], ) -> Result<(), (StatusCode, String)> { for asset in assets { - if let Some(lock) = state.lock_manager.get_lock(&asset.path) { + if let Some(lock) = state + .lock_manager + .get_lock_with_repo(repo_id, "asset", &asset.path) + { if lock.owner_id != owner_id { return Err(( StatusCode::CONFLICT, @@ -233,6 +277,12 @@ async fn ensure_blob_exists( ) -> Result<(), (StatusCode, String)> { for asset in assets { if let Some(hash) = &asset.blob_hash { + if crate::core::storage::StorageManager::validate_hash(hash).is_err() { + return Err(( + StatusCode::BAD_REQUEST, + format!("Invalid blob hash: {hash}"), + )); + } match state.storage_manager.exists(hash).await { Ok(true) => {} Ok(false) => { @@ -463,7 +513,11 @@ pub async fn submit_changeset( } } - if let Err(err) = ensure_lock_access(&state, &identity.owner_id, &assets) { + if let Err(err) = validate_asset_paths(&assets) { + return (err.0, Json(ApiResponse::err(err.1))); + } + + if let Err(err) = ensure_lock_access(&state, &identity.owner_id, &payload.repo_id, &assets) { return (err.0, Json(ApiResponse::err(err.1))); } if let Err(err) = ensure_blob_exists(&state, &assets).await { @@ -632,7 +686,8 @@ pub async fn rollback( } }; - if let Err(err) = ensure_lock_access(&state, &identity.owner_id, &plan.assets) { + if let Err(err) = ensure_lock_access(&state, &identity.owner_id, &payload.repo_id, &plan.assets) + { return (err.0, Json(ApiResponse::err(err.1))); } if let Err(err) = ensure_blob_exists(&state, &plan.assets).await { diff --git a/crates/server/src/bootstrap.rs b/crates/server/src/bootstrap.rs index 4624add..602f04d 100644 --- a/crates/server/src/bootstrap.rs +++ b/crates/server/src/bootstrap.rs @@ -150,9 +150,12 @@ pub(crate) async fn run() { } }; - if let Err(e) = axum::serve(listener, app) - .with_graceful_shutdown(shutdown_signal()) - .await + if let Err(e) = axum::serve( + listener, + app.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(shutdown_signal()) + .await { tracing::error!("Server exited with error: {e}"); std::process::exit(1); diff --git a/crates/server/src/core/lock.rs b/crates/server/src/core/lock.rs index 84f1eed..15f67a5 100644 --- a/crates/server/src/core/lock.rs +++ b/crates/server/src/core/lock.rs @@ -28,14 +28,22 @@ fn default_scope() -> String { #[derive(Clone)] pub struct LockManager { - // Key: file_path, Value: Lock Info + // Key: repo_id + scope + file_path, Value: Lock Info // DashMap provides high-concurrency access without heavy Mutex contention - locks: Arc>, + locks: Arc>, repo: Option, lease_seconds: i64, } impl LockManager { + fn lock_key(repo_id: &str, scope: &str, file_path: &str) -> (String, String, String) { + ( + repo_id.to_string(), + scope.to_string(), + file_path.to_string(), + ) + } + pub fn new() -> Self { Self { locks: Arc::new(DashMap::new()), @@ -56,7 +64,8 @@ impl LockManager { HyperTideError::Persistence(format!("failed to load locks from db: {e}")) })?; for lock in existing { - manager.locks.insert(lock.file_path.clone(), lock); + let key = Self::lock_key(&lock.repo_id, &lock.scope, &lock.file_path); + manager.locks.insert(key, lock); } Ok(manager) @@ -88,14 +97,19 @@ impl LockManager { repo_id: repo_id.to_string(), scope: scope.to_string(), }; + let lock_key = Self::lock_key(repo_id, scope, &file_path); if let Some(repo) = &self.repo { let effective_lock = repo .acquire_lock_atomic(&requested_lock) .await .map_err(|e| HyperTideError::Persistence(format!("failed to persist lock: {e}")))?; - self.locks - .insert(effective_lock.file_path.clone(), effective_lock.clone()); + let effective_key = Self::lock_key( + &effective_lock.repo_id, + &effective_lock.scope, + &effective_lock.file_path, + ); + self.locks.insert(effective_key, effective_lock.clone()); if effective_lock.owner_id != owner_id { return Err(HyperTideError::Conflict(format!( "File is already locked by {}", @@ -105,7 +119,7 @@ impl LockManager { return Ok(effective_lock); } - match self.locks.entry(file_path.clone()) { + match self.locks.entry(lock_key) { Entry::Occupied(mut occupied) => { let existing = occupied.get().clone(); if self.is_expired(&existing) { @@ -132,9 +146,21 @@ impl LockManager { file_path: &str, owner_id: &str, ) -> Result { + self.renew_lock_with_repo(file_path, owner_id, "", "asset") + .await + } + + pub async fn renew_lock_with_repo( + &self, + file_path: &str, + owner_id: &str, + repo_id: &str, + scope: &str, + ) -> Result { + let lock_key = Self::lock_key(repo_id, scope, file_path); let existing = self .locks - .get(file_path) + .get(&lock_key) .map(|entry| entry.clone()) .ok_or_else(|| HyperTideError::NotFound("File is not locked".to_string()))?; @@ -146,11 +172,13 @@ impl LockManager { } if self.is_expired(&existing) { if let Some(repo) = &self.repo { - repo.delete_lock(file_path).await.map_err(|e| { - HyperTideError::Persistence(format!("failed to cleanup expired lock: {e}")) - })?; + repo.delete_lock(repo_id, scope, file_path) + .await + .map_err(|e| { + HyperTideError::Persistence(format!("failed to cleanup expired lock: {e}")) + })?; } - self.locks.remove(file_path); + self.locks.remove(&lock_key); return Err(HyperTideError::Conflict( "Cannot renew: lock lease expired".to_string(), )); @@ -166,42 +194,68 @@ impl LockManager { HyperTideError::Persistence(format!("failed to persist lock renew: {e}")) })?; } - self.locks.insert(file_path.to_string(), renewed.clone()); + self.locks.insert(lock_key, renewed.clone()); Ok(renewed) } /// Unlock a file. Only the owner can unlock. pub async fn unlock(&self, file_path: &str, owner_id: &str) -> Result<(), HyperTideError> { + self.unlock_with_repo(file_path, owner_id, "", "asset") + .await + .map(|_| ()) + } + + pub async fn unlock_with_repo( + &self, + file_path: &str, + owner_id: &str, + repo_id: &str, + scope: &str, + ) -> Result { + let lock_key = Self::lock_key(repo_id, scope, file_path); // We need to check ownership before removing - if let Some(existing) = self.locks.get(file_path) { + let existing = if let Some(existing) = self.locks.get(&lock_key) { if existing.owner_id != owner_id { return Err(HyperTideError::PermissionDenied(format!( "Cannot unlock: File is locked by {}", existing.owner_id ))); } + existing.clone() } else { return Err(HyperTideError::NotFound("File is not locked".to_string())); - } + }; if let Some(repo) = &self.repo { - repo.delete_lock(file_path) + repo.delete_lock(repo_id, scope, file_path) .await .map_err(|e| HyperTideError::Persistence(format!("failed to delete lock: {e}")))?; } - self.locks.remove(file_path); - Ok(()) + self.locks.remove(&lock_key); + Ok(existing) } /// Admin force unlock pub async fn force_unlock(&self, file_path: &str) -> Result { + self.force_unlock_with_repo(file_path, "", "asset").await + } + + pub async fn force_unlock_with_repo( + &self, + file_path: &str, + repo_id: &str, + scope: &str, + ) -> Result { + let lock_key = Self::lock_key(repo_id, scope, file_path); if let Some(repo) = &self.repo { - repo.delete_lock(file_path).await.map_err(|e| { - HyperTideError::Persistence(format!("failed to force release lock: {e}")) - })?; + repo.delete_lock(repo_id, scope, file_path) + .await + .map_err(|e| { + HyperTideError::Persistence(format!("failed to force release lock: {e}")) + })?; } - Ok(self.locks.remove(file_path).is_some()) + Ok(self.locks.remove(&lock_key).is_some()) } /// List all locks (for administrative view or debugging) @@ -213,10 +267,28 @@ impl LockManager { .collect() } + pub fn list_locks_with_repo(&self, repo_id: &str, scope: &str) -> Vec { + self.locks + .iter() + .map(|entry| entry.value().clone()) + .filter(|lock| lock.repo_id == repo_id && lock.scope == scope && !self.is_expired(lock)) + .collect() + } + /// Query lock by path. pub fn get_lock(&self, file_path: &str) -> Option { + self.get_lock_with_repo("", "asset", file_path) + } + + pub fn get_lock_with_repo( + &self, + repo_id: &str, + scope: &str, + file_path: &str, + ) -> Option { + let lock_key = Self::lock_key(repo_id, scope, file_path); self.locks - .get(file_path) + .get(&lock_key) .map(|entry| entry.clone()) .filter(|lock| !self.is_expired(lock)) } @@ -238,3 +310,48 @@ fn default_lease_seconds() -> i64 { .and_then(|v| v.parse::().ok()) .unwrap_or(300) } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn identical_paths_are_isolated_by_repo() { + let manager = LockManager::new(); + let first = manager + .try_lock_with_repo( + "Content/A.uasset".to_string(), + "alice".to_string(), + "repo-a", + "asset", + ) + .await + .expect("repo-a lock"); + let second = manager + .try_lock_with_repo( + "Content/A.uasset".to_string(), + "bob".to_string(), + "repo-b", + "asset", + ) + .await + .expect("repo-b lock"); + + assert_eq!(first.owner_id, "alice"); + assert_eq!(second.owner_id, "bob"); + manager + .unlock_with_repo("Content/A.uasset", "alice", "repo-a", "asset") + .await + .expect("release repo-a"); + assert!(manager + .get_lock_with_repo("repo-a", "asset", "Content/A.uasset") + .is_none()); + assert_eq!( + manager + .get_lock_with_repo("repo-b", "asset", "Content/A.uasset") + .expect("repo-b remains") + .owner_id, + "bob" + ); + } +} diff --git a/crates/server/src/core/lock/repo_pg.rs b/crates/server/src/core/lock/repo_pg.rs index beae12e..4428236 100644 --- a/crates/server/src/core/lock/repo_pg.rs +++ b/crates/server/src/core/lock/repo_pg.rs @@ -92,7 +92,7 @@ impl LockRepoPg { current_lock AS ( SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope FROM locks - WHERE file_path = $1 AND force_released = FALSE + WHERE repo_id = $5 AND scope = $6 AND file_path = $1 AND force_released = FALSE ) SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope FROM attempted @@ -122,13 +122,20 @@ impl LockRepoPg { }) } - pub async fn delete_lock(&self, file_path: &str) -> Result<(), sqlx::Error> { + pub async fn delete_lock( + &self, + repo_id: &str, + scope: &str, + file_path: &str, + ) -> Result<(), sqlx::Error> { sqlx::query( r#" DELETE FROM locks - WHERE file_path = $1 + WHERE repo_id = $1 AND scope = $2 AND file_path = $3 "#, ) + .bind(repo_id) + .bind(scope) .bind(file_path) .execute(&self.pool) .await?; diff --git a/crates/server/src/core/storage.rs b/crates/server/src/core/storage.rs index d1a85ab..3db5cc3 100644 --- a/crates/server/src/core/storage.rs +++ b/crates/server/src/core/storage.rs @@ -22,6 +22,21 @@ pub struct StorageManager { } impl StorageManager { + pub(crate) fn validate_hash(hash: &str) -> Result<(), HyperTideError> { + if hash.len() != 64 || !hash.bytes().all(|byte| byte.is_ascii_hexdigit()) { + return Err(HyperTideError::Validation( + "Invalid BLAKE3 hash: expected 64 hexadecimal characters".to_string(), + )); + } + Ok(()) + } + + fn object_path(&self, hash: &str) -> Result { + Self::validate_hash(hash)?; + let (prefix, rest) = hash.split_at(2); + Ok(self.storage_root.join("objects").join(prefix).join(rest)) + } + async fn check_path_exists(path: &Path, context: &str) -> Result { fs::try_exists(path) .await @@ -107,12 +122,22 @@ impl StorageManager { .await .map_err(HyperTideError::Persistence)? { - return Ok(StoredFile { - hash, - original_path: original_path.to_string(), - size_bytes, - stored_at: chrono::Utc::now(), - }); + let existing = fs::read(&object_path).await.map_err(|error| { + HyperTideError::Persistence(format!("Failed to verify existing object: {error}")) + })?; + if Self::calculate_hash(&existing) == hash { + return Ok(StoredFile { + hash, + original_path: original_path.to_string(), + size_bytes, + stored_at: chrono::Utc::now(), + }); + } + fs::remove_file(&object_path).await.map_err(|error| { + HyperTideError::Persistence(format!( + "Failed to replace corrupt CAS object {hash}: {error}" + )) + })?; } // Create subdirectory if needed @@ -166,12 +191,7 @@ impl StorageManager { /// Retrieve file content by hash pub async fn retrieve(&self, hash: &str) -> Result, HyperTideError> { - if hash.len() < 3 { - return Err(HyperTideError::Validation("Invalid hash".to_string())); - } - - let (prefix, rest) = hash.split_at(2); - let object_path = self.storage_root.join("objects").join(prefix).join(rest); + let object_path = self.object_path(hash)?; if !Self::check_path_exists(&object_path, "object existence before retrieve") .await @@ -183,30 +203,27 @@ impl StorageManager { ))); } - fs::read(&object_path) + let bytes = fs::read(&object_path) .await - .map_err(|e| HyperTideError::Persistence(format!("Failed to read object: {}", e))) + .map_err(|e| HyperTideError::Persistence(format!("Failed to read object: {}", e)))?; + let actual_hash = Self::calculate_hash(&bytes); + if actual_hash != hash { + return Err(HyperTideError::Persistence(format!( + "CAS object integrity mismatch: expected {hash}, got {actual_hash}" + ))); + } + Ok(bytes) } /// Check if a file with given hash exists pub async fn exists(&self, hash: &str) -> Result { - if hash.len() < 3 { - return Ok(false); - } - - let (prefix, rest) = hash.split_at(2); - let object_path = self.storage_root.join("objects").join(prefix).join(rest); + let object_path = self.object_path(hash).map_err(|error| error.to_string())?; Self::check_path_exists(&object_path, "object existence").await } /// Get the local file path for a hash (for direct access) pub fn get_path(&self, hash: &str) -> Option { - if hash.len() < 3 { - return None; - } - - let (prefix, rest) = hash.split_at(2); - Some(self.storage_root.join("objects").join(prefix).join(rest)) + self.object_path(hash).ok() } } @@ -369,4 +386,41 @@ mod tests { std::fs::remove_file(object_dir).ok(); std::fs::remove_dir_all(root).ok(); } + + #[tokio::test] + async fn rejects_non_blake3_hashes_before_resolving_storage_paths() { + let root = make_storage_root("invalid-hash"); + let manager = StorageManager::new(&root); + manager.init().await.expect("init storage"); + + let traversal = manager + .retrieve("../outside-file") + .await + .expect_err("path traversal must be rejected"); + assert!(traversal.to_string().contains("Invalid BLAKE3 hash")); + assert!(manager.exists("../outside-file").await.is_err()); + assert!(manager.get_path("../outside-file").is_none()); + + std::fs::remove_dir_all(root).ok(); + } + + #[tokio::test] + async fn retrieve_rejects_corrupt_content_under_a_valid_hash() { + let root = make_storage_root("corrupt-object"); + let manager = StorageManager::new(&root); + manager.init().await.expect("init storage"); + let expected_hash = StorageManager::calculate_hash(b"expected"); + let object_path = manager.get_path(&expected_hash).expect("valid object path"); + std::fs::create_dir_all(object_path.parent().expect("object parent")) + .expect("create object parent"); + std::fs::write(&object_path, b"corrupt").expect("write corrupt object"); + + let error = manager + .retrieve(&expected_hash) + .await + .expect_err("corrupt object must be rejected"); + assert!(error.to_string().contains("integrity mismatch")); + + std::fs::remove_dir_all(root).ok(); + } } diff --git a/crates/server/src/core/storage_backend.rs b/crates/server/src/core/storage_backend.rs index cf36afb..2449496 100644 --- a/crates/server/src/core/storage_backend.rs +++ b/crates/server/src/core/storage_backend.rs @@ -108,7 +108,7 @@ impl LocalFsBackend { #[allow(dead_code)] fn object_path(&self, hash: &str) -> Option { - if hash.len() < 3 { + if hash.len() != 64 || !hash.bytes().all(|byte| byte.is_ascii_hexdigit()) { return None; } let (prefix, rest) = hash.split_at(2); @@ -122,10 +122,22 @@ impl StorageBackend for LocalFsBackend { let object_path = self .object_path(hash) .ok_or_else(|| StorageError::Validation("invalid hash".to_string()))?; + let actual_hash = calculate_hash(data); + if actual_hash != hash { + return Err(StorageError::Validation(format!( + "content hash mismatch: expected {hash}, got {actual_hash}" + ))); + } // Dedup: skip if already exists if object_path.exists() { - return Ok(()); + let existing = fs::read(&object_path).await?; + if calculate_hash(&existing) == hash { + return Ok(()); + } + return Err(StorageError::Validation(format!( + "existing CAS object failed integrity validation: {hash}" + ))); } // Create subdirectory @@ -163,20 +175,27 @@ impl StorageBackend for LocalFsBackend { ))); } - Ok(fs::read(&object_path).await?) + let bytes = fs::read(&object_path).await?; + let actual_hash = calculate_hash(&bytes); + if actual_hash != hash { + return Err(StorageError::Validation(format!( + "content hash mismatch: expected {hash}, got {actual_hash}" + ))); + } + Ok(bytes) } async fn exists(&self, hash: &str) -> Result { - let Some(object_path) = self.object_path(hash) else { - return Ok(false); - }; + let object_path = self + .object_path(hash) + .ok_or_else(|| StorageError::Validation("invalid hash".to_string()))?; Ok(object_path.exists()) } async fn delete(&self, hash: &str) -> Result<(), StorageError> { - let Some(object_path) = self.object_path(hash) else { - return Ok(()); - }; + let object_path = self + .object_path(hash) + .ok_or_else(|| StorageError::Validation("invalid hash".to_string()))?; if object_path.exists() { fs::remove_file(&object_path).await?; } @@ -218,6 +237,29 @@ impl StorageBackend for LocalFsBackend { mod tests { use super::*; + fn temp_backend(label: &str) -> (std::path::PathBuf, LocalFsBackend) { + let root = std::env::temp_dir().join(format!( + "hypertide-storage-backend-{label}-{}", + uuid::Uuid::new_v4() + )); + (root.clone(), LocalFsBackend::new(root)) + } + + #[tokio::test] + async fn local_backend_rejects_data_that_does_not_match_the_key() { + let (root, backend) = temp_backend("mismatch"); + backend.init().await.expect("init backend"); + let declared_hash = calculate_hash(b"declared"); + + let error = backend + .store(&declared_hash, b"different") + .await + .expect_err("mismatched content must be rejected"); + assert!(error.to_string().contains("content hash mismatch")); + + let _ = std::fs::remove_dir_all(root); + } + fn make_root(name: &str) -> PathBuf { let root = std::env::temp_dir().join(format!( "hypertide-backend-test-{name}-{}", @@ -241,7 +283,7 @@ mod tests { assert_eq!(retrieved, data); assert!(backend.exists(&hash).await.unwrap()); - assert!(!backend.exists("nonexistent").await.unwrap()); + assert!(backend.exists("nonexistent").await.is_err()); std::fs::remove_dir_all(root).ok(); } diff --git a/crates/server/src/core/versioning.rs b/crates/server/src/core/versioning.rs index 30e747b..dd7b814 100644 --- a/crates/server/src/core/versioning.rs +++ b/crates/server/src/core/versioning.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; @@ -13,6 +13,48 @@ use self::repo_pg::VersionRepoPg; pub const ROOT_BASE_CHANGESET_ID: &str = "ROOT"; +#[cfg(windows)] +fn replace_state_file(temp_path: &Path, state_path: &Path) -> std::io::Result<()> { + use std::iter::once; + use std::os::windows::ffi::OsStrExt; + use windows_sys::Win32::Storage::FileSystem::ReplaceFileW; + + if !state_path.exists() { + return std::fs::rename(temp_path, state_path); + } + + let state_wide = state_path + .as_os_str() + .encode_wide() + .chain(once(0)) + .collect::>(); + let temp_wide = temp_path + .as_os_str() + .encode_wide() + .chain(once(0)) + .collect::>(); + let replaced = unsafe { + ReplaceFileW( + state_wide.as_ptr(), + temp_wide.as_ptr(), + std::ptr::null(), + 0, + std::ptr::null(), + std::ptr::null(), + ) + }; + if replaced == 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } +} + +#[cfg(not(windows))] +fn replace_state_file(temp_path: &Path, state_path: &Path) -> std::io::Result<()> { + std::fs::rename(temp_path, state_path) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ChangesetKind { @@ -250,6 +292,13 @@ pub enum VersioningError { status: ChangesetStatus, expected: &'static str, }, + InvalidAssetLayout { + repo_id: String, + message: String, + }, + Persistence { + message: String, + }, } #[derive(Clone)] @@ -257,6 +306,7 @@ pub struct VersionManager { repos: Arc>>, persistence_path: Option, repo_pg: Option, + mutation_lock: Arc>, } impl VersionManager { @@ -265,6 +315,7 @@ impl VersionManager { repos: Arc::new(RwLock::new(HashMap::new())), persistence_path: None, repo_pg: None, + mutation_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -286,6 +337,7 @@ impl VersionManager { repos: Arc::new(RwLock::new(repos)), persistence_path: Some(persistence_path), repo_pg: None, + mutation_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -298,6 +350,7 @@ impl VersionManager { repos: Arc::new(RwLock::new(repos)), persistence_path: None, repo_pg: Some(repo_pg), + mutation_lock: Arc::new(tokio::sync::Mutex::new(())), }) } @@ -307,25 +360,25 @@ impl VersionManager { default_branch: &str, created_by: &str, ) -> Result { + let _mutation = self.mutation_lock.lock().await; let (info, snapshot) = { - let mut repos = self.repos.write().expect("versioning lock poisoned"); - if repos.contains_key(repo_id) { + let mut snapshot = self.repos.read().expect("versioning lock poisoned").clone(); + if snapshot.contains_key(repo_id) { return Err(VersioningError::RepoAlreadyExists { repo_id: repo_id.to_string(), }); } let repo = RepoState::new_with_default(default_branch, created_by); - repos.insert(repo_id.to_string(), repo); + snapshot.insert(repo_id.to_string(), repo); let info = - Self::repo_info_from_state(repo_id, repos.get(repo_id).expect("repo exists")); - (info, repos.clone()) + Self::repo_info_from_state(repo_id, snapshot.get(repo_id).expect("repo exists")); + (info, snapshot) }; - - if let Err(error) = self.persist_repo(repo_id, &snapshot).await { - tracing::error!("failed to persist repo state for {repo_id}: {error}"); - } - + self.persist_repo(repo_id, &snapshot) + .await + .map_err(|message| VersioningError::Persistence { message })?; + *self.repos.write().expect("versioning lock poisoned") = snapshot; Ok(info) } @@ -356,9 +409,10 @@ impl VersionManager { from_changeset_id: Option<&str>, created_by: &str, ) -> Result { + let _mutation = self.mutation_lock.lock().await; let (record, snapshot) = { - let mut repos = self.repos.write().expect("versioning lock poisoned"); - let repo = repos + let mut snapshot = self.repos.read().expect("versioning lock poisoned").clone(); + let repo = snapshot .entry(repo_id.to_string()) .or_insert_with(|| RepoState::new(created_by)); repo.ensure_default_branch(created_by); @@ -408,12 +462,12 @@ impl VersionManager { }, ); - (record, repos.clone()) + (record, snapshot) }; - if let Err(error) = self.persist_repo(repo_id, &snapshot).await { - tracing::error!("failed to persist branch state for {repo_id}: {error}"); - } - + self.persist_repo(repo_id, &snapshot) + .await + .map_err(|message| VersioningError::Persistence { message })?; + *self.repos.write().expect("versioning lock poisoned") = snapshot; Ok(record) } @@ -457,34 +511,22 @@ impl VersionManager { &self, input: SubmitChangesetInput, ) -> Result { + let _mutation = self.mutation_lock.lock().await; let repo_id = input.repo_id.clone(); - let (result, snapshot) = { - let mut repos = self.repos.write().expect("versioning lock poisoned"); - let repo = repos + let (record, snapshot) = { + let mut snapshot = self.repos.read().expect("versioning lock poisoned").clone(); + let repo = snapshot .entry(input.repo_id.clone()) .or_insert_with(|| RepoState::new(&input.author)); repo.ensure_default_branch(&input.author); - let result = Self::submit_internal(repo, input); - let snapshot = if result.is_ok() { - Some(repos.clone()) - } else { - None - }; - (result, snapshot) + let record = Self::submit_internal(repo, input)?; + (record, snapshot) }; - - if result.is_ok() { - if let Some(snapshot) = snapshot { - if let Err(error) = self.persist_repo(&repo_id, &snapshot).await { - tracing::error!("failed to persist changeset state for {repo_id}: {error}"); - } - } else { - tracing::error!( - "failed to persist changeset state for {repo_id}: missing in-memory snapshot" - ); - } - } - result + self.persist_repo(&repo_id, &snapshot) + .await + .map_err(|message| VersioningError::Persistence { message })?; + *self.repos.write().expect("versioning lock poisoned") = snapshot; + Ok(record) } pub async fn approve_changeset( @@ -493,9 +535,10 @@ impl VersionManager { changeset_id: &str, approver: &str, ) -> Result { + let _mutation = self.mutation_lock.lock().await; let (record, snapshot) = { - let mut repos = self.repos.write().expect("versioning lock poisoned"); - let repo = repos + let mut snapshot = self.repos.read().expect("versioning lock poisoned").clone(); + let repo = snapshot .get_mut(repo_id) .ok_or_else(|| VersioningError::RepoNotFound { repo_id: repo_id.to_string(), @@ -523,12 +566,12 @@ impl VersionManager { } } - (record.clone(), repos.clone()) + (record.clone(), snapshot) }; - - if let Err(error) = self.persist_repo(repo_id, &snapshot).await { - tracing::error!("failed to persist approve state for {repo_id}: {error}"); - } + self.persist_repo(repo_id, &snapshot) + .await + .map_err(|message| VersioningError::Persistence { message })?; + *self.repos.write().expect("versioning lock poisoned") = snapshot; Ok(record) } @@ -538,9 +581,10 @@ impl VersionManager { changeset_id: &str, promoter: &str, ) -> Result { + let _mutation = self.mutation_lock.lock().await; let (record, snapshot) = { - let mut repos = self.repos.write().expect("versioning lock poisoned"); - let repo = repos + let mut snapshot = self.repos.read().expect("versioning lock poisoned").clone(); + let repo = snapshot .get_mut(repo_id) .ok_or_else(|| VersioningError::RepoNotFound { repo_id: repo_id.to_string(), @@ -599,12 +643,12 @@ impl VersionManager { record.promoted_at = Some(Utc::now()); record.visible_ref = Some(visible_ref(&record.branch)); - (record.clone(), repos.clone()) + (record.clone(), snapshot) }; - - if let Err(error) = self.persist_repo(repo_id, &snapshot).await { - tracing::error!("failed to persist promote state for {repo_id}: {error}"); - } + self.persist_repo(repo_id, &snapshot) + .await + .map_err(|message| VersioningError::Persistence { message })?; + *self.repos.write().expect("versioning lock poisoned") = snapshot; Ok(record) } @@ -949,6 +993,7 @@ impl VersionManager { } normalized_assets.push(asset); } + Self::validate_snapshot_layout(&repo_id, &new_snapshot)?; let changeset_id = Uuid::new_v4().to_string(); let status = match visibility { @@ -1002,6 +1047,37 @@ impl VersionManager { Ok(record) } + fn validate_snapshot_layout( + repo_id: &str, + snapshot: &HashMap, + ) -> Result<(), VersioningError> { + let mut paths = HashSet::with_capacity(snapshot.len()); + for asset in snapshot.values() { + let normalized = asset.path.replace('\\', "/"); + if !paths.insert(normalized.clone()) { + return Err(VersioningError::InvalidAssetLayout { + repo_id: repo_id.to_string(), + message: format!("duplicate asset path: {}", asset.path), + }); + } + } + for path in &paths { + for (index, byte) in path.bytes().enumerate() { + if byte == b'/' && paths.contains(&path[..index]) { + return Err(VersioningError::InvalidAssetLayout { + repo_id: repo_id.to_string(), + message: format!( + "asset path conflicts with parent asset: {} and {}", + &path[..index], + path + ), + }); + } + } + } + Ok(()) + } + fn load_repos(path: &Path) -> Result, String> { if !path.exists() { return Ok(HashMap::new()); @@ -1028,52 +1104,46 @@ impl VersionManager { return Ok(()); } - self.persist_repos_file(repos); - Ok(()) + self.persist_repos_file(repos) } - fn persist_repos_file(&self, repos: &HashMap) { + fn persist_repos_file(&self, repos: &HashMap) -> Result<(), String> { let Some(path) = self.persistence_path.as_ref() else { - return; + return Ok(()); }; if let Some(parent) = path.parent() { if let Err(error) = std::fs::create_dir_all(parent) { - tracing::error!( + return Err(format!( "failed to create versioning state dir {}: {}", parent.display(), error - ); - return; + )); } } let payload = match serde_json::to_vec_pretty(repos) { Ok(payload) => payload, - Err(error) => { - tracing::error!("failed to serialize versioning state: {}", error); - return; - } + Err(error) => return Err(format!("failed to serialize versioning state: {error}")), }; let temp_path = path.with_extension("tmp"); if let Err(error) = std::fs::write(&temp_path, payload) { - tracing::error!( + return Err(format!( "failed to write versioning temp state {}: {}", temp_path.display(), error - ); - return; + )); } - if let Err(error) = std::fs::rename(&temp_path, path) { - let _ = std::fs::remove_file(&temp_path); - tracing::error!( + if let Err(error) = replace_state_file(&temp_path, path) { + return Err(format!( "failed to atomically replace versioning state {}: {}", path.display(), error - ); + )); } + Ok(()) } } @@ -1222,6 +1292,48 @@ mod tests { assert_eq!(sync.assets[0].blob_hash, "hash-2"); } + #[tokio::test] + async fn submit_rejects_conflicting_snapshot_paths() { + let manager = VersionManager::new(); + let error = manager + .submit_changeset(SubmitChangesetInput { + repo_id: "repo-layout".to_string(), + branch: "main".to_string(), + base_changeset_id: Some(ROOT_BASE_CHANGESET_ID.to_string()), + kind: ChangesetKind::Normal, + rollback_of: None, + author: "alice".to_string(), + message: "invalid layout".to_string(), + visibility: ChangesetVisibility::Visible, + intent_id: None, + task_id: None, + agent_run_id: None, + session_id: None, + parent_checkpoint_id: None, + risk_level: None, + semantic_summary: None, + assets: vec![ + AssetDelta { + asset_id: Some("asset-parent".to_string()), + path: "Content".to_string(), + from_blob_hash: None, + blob_hash: Some("hash-parent".to_string()), + }, + AssetDelta { + asset_id: Some("asset-child".to_string()), + path: "Content/A.uasset".to_string(), + from_blob_hash: None, + blob_hash: Some("hash-child".to_string()), + }, + ], + }) + .await + .expect_err("conflicting paths must be rejected"); + + assert!(matches!(error, VersioningError::InvalidAssetLayout { .. })); + assert!(manager.list_repos().is_empty()); + } + #[tokio::test] async fn create_repo_creates_default_branch_and_rejects_duplicates() { let manager = VersionManager::new(); @@ -1638,4 +1750,46 @@ mod tests { let _ = std::fs::remove_file(state_file); } + + #[tokio::test] + async fn persistence_failure_does_not_publish_in_memory_state() { + let blocker = + std::env::temp_dir().join(format!("hypertide-versioning-blocker-{}", Uuid::new_v4())); + std::fs::write(&blocker, b"not-a-directory").expect("create blocker"); + let manager = VersionManager::with_persistence(blocker.join("state.json")); + + let error = manager + .create_repo("repo-not-persisted", "main", "alice") + .await + .expect_err("persistence must fail"); + + assert!(matches!(error, VersioningError::Persistence { .. })); + assert!(manager.list_repos().is_empty()); + let _ = std::fs::remove_file(blocker); + } + + #[tokio::test] + async fn file_persistence_supports_consecutive_mutations() { + let state_file = + std::env::temp_dir().join(format!("hypertide-versioning-{}.json", Uuid::new_v4())); + let manager = VersionManager::with_persistence(&state_file); + + manager + .create_repo("repo-p", "main", "alice") + .await + .expect("first persistence write"); + manager + .create_branch("repo-p", "feature", None, "alice") + .await + .expect("replacement persistence write"); + + let reloaded = VersionManager::with_persistence(&state_file); + let branches = reloaded + .list_branches("repo-p") + .expect("load persisted repo"); + assert_eq!(branches.len(), 2); + assert!(branches.iter().any(|branch| branch.name == "feature")); + + let _ = std::fs::remove_file(state_file); + } } diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index 58f826f..e216a1e 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -1,13 +1,13 @@ use axum::{ body::Body, - extract::{DefaultBodyLimit, MatchedPath, State}, + extract::{ConnectInfo, DefaultBodyLimit, MatchedPath, State}, http::{HeaderValue, StatusCode}, middleware::{self, Next}, response::{IntoResponse, Response}, routing::{delete, get, post, put}, Router, }; -use std::time::Instant; +use std::{net::IpAddr, net::SocketAddr, time::Instant}; use tower_http::{ cors::{Any, CorsLayer}, limit::RequestBodyLimitLayer, @@ -62,6 +62,7 @@ pub(crate) fn build_app(state: AppState, config: &AppConfig) -> Router { let rate_limit_state = RateLimitState { limiter: rate_limiter, metrics: metrics.clone(), + auth_manager: state.auth_manager.clone(), }; let general_routes = Router::new() @@ -188,7 +189,27 @@ async fn enforce_rate_limit( request: axum::http::Request, next: Next, ) -> Response { - let bucket = rate_limit_bucket(&request); + let authorization = request + .headers() + .get("authorization") + .and_then(|value| value.to_str().ok()); + let bearer = authorization.and_then(|value| { + let (scheme, token) = value.split_once(' ').unwrap_or((value, "")); + scheme + .eq_ignore_ascii_case("bearer") + .then(|| token.trim().to_string()) + }); + let api_key = request + .headers() + .get("x-api-key") + .and_then(|value| value.to_str().ok()) + .filter(|value| !value.trim().is_empty()) + .map(str::to_string); + let peer_ip = request + .extensions() + .get::>() + .map(|connect_info| connect_info.0.ip()); + let bucket = rate_limit_bucket(&rate_limit, bearer, api_key, peer_ip).await; if !rate_limit.limiter.allow(&bucket) { rate_limit.metrics.record_rate_limited(); return (StatusCode::TOO_MANY_REQUESTS, "RATE_LIMITED").into_response(); @@ -196,40 +217,28 @@ async fn enforce_rate_limit( next.run(request).await } -fn rate_limit_bucket(request: &axum::http::Request) -> String { - if let Some(value) = request - .headers() - .get("x-api-key") - .and_then(|value| value.to_str().ok()) - .filter(|value| !value.trim().is_empty()) - { - return format!("api-key:{}", value); +async fn rate_limit_bucket( + rate_limit: &RateLimitState, + bearer: Option, + api_key: Option, + peer_ip: Option, +) -> String { + if let Some(token) = bearer { + if let Ok(identity) = rate_limit.auth_manager.validate_access_token(&token).await { + return format!("principal:{}", identity.owner_id); + } + return "anonymous".to_string(); } - if let Some(value) = request - .headers() - .get("authorization") - .and_then(|value| value.to_str().ok()) - .filter(|value| !value.trim().is_empty()) - { - return format!("authorization:{}", value); - } - if let Some(value) = request - .headers() - .get("x-forwarded-for") - .and_then(|value| value.to_str().ok()) - .and_then(|value| value.split(',').next()) - .map(str::trim) - .filter(|value| !value.is_empty()) - { - return format!("ip:{}", value); - } - if let Some(value) = request - .headers() - .get("x-real-ip") - .and_then(|value| value.to_str().ok()) - .filter(|value| !value.trim().is_empty()) - { - return format!("ip:{}", value); + if let Some(api_key) = api_key { + if let Ok(identity) = rate_limit + .auth_manager + .validate_api_key_identity(&api_key) + .await + { + return format!("principal:{}", identity.owner_id); + } } - "global".to_string() + peer_ip + .map(|ip| format!("anonymous:{ip}")) + .unwrap_or_else(|| "anonymous".to_string()) } diff --git a/crates/server/src/state.rs b/crates/server/src/state.rs index 9b73728..e26f0a4 100644 --- a/crates/server/src/state.rs +++ b/crates/server/src/state.rs @@ -278,6 +278,7 @@ impl RateLimiter { pub(crate) struct RateLimitState { pub(crate) limiter: RateLimiter, pub(crate) metrics: HttpMetrics, + pub(crate) auth_manager: AuthManager, } fn escape_label_value(value: &str) -> String { diff --git a/crates/server/src/tests.rs b/crates/server/src/tests.rs index 6b18a59..53ab6ea 100644 --- a/crates/server/src/tests.rs +++ b/crates/server/src/tests.rs @@ -10,13 +10,14 @@ use crate::routes::{build_app, DEFAULT_BODY_LIMIT_BYTES}; use crate::state::{AppState, HttpMetrics, RateLimiter}; use axum::{ body::{to_bytes, Body}, + extract::ConnectInfo, http::{HeaderValue, Request, StatusCode}, }; use serde_json::Value; use sqlx::{postgres::PgPoolOptions, PgPool}; -use std::path::PathBuf; use std::sync::{Mutex, MutexGuard, OnceLock}; use std::time::{SystemTime, UNIX_EPOCH}; +use std::{net::SocketAddr, path::PathBuf}; use tower::util::ServiceExt; fn test_master_key() -> &'static str { @@ -117,7 +118,7 @@ fn test_pg_pool() -> PgPool { } #[tokio::test] -async fn exists_route_returns_json_response() { +async fn exists_route_rejects_invalid_hashes() { let app = build_app(test_state(), &test_config()); let request = Request::builder() @@ -127,13 +128,32 @@ async fn exists_route_returns_json_response() { .expect("request"); let response = app.oneshot(request).await.expect("response"); - assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); let body = to_bytes(response.into_body(), usize::MAX) .await .expect("body"); let payload: Value = serde_json::from_slice(&body).expect("json"); - assert_eq!(payload["success"], Value::Bool(true)); + assert_eq!(payload["success"], Value::Bool(false)); +} + +#[tokio::test] +async fn exists_route_returns_false_for_valid_missing_hash() { + let app = build_app(test_state(), &test_config()); + let hash = "0".repeat(64); + let request = Request::builder() + .uri(format!("/v2/storage/exists/{hash}")) + .header("X-API-Key", test_master_key()) + .body(Body::empty()) + .expect("request"); + + let response = app.oneshot(request).await.expect("response"); + assert_eq!(response.status(), StatusCode::OK); + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + let payload: Value = serde_json::from_slice(&body).expect("json"); + assert_eq!(payload["data"], Value::Bool(false)); } #[tokio::test] @@ -180,7 +200,9 @@ async fn lock_route_uses_authenticated_owner_when_owner_not_provided() { .uri("/v2/locks/acquire") .header("content-type", "application/json") .header("X-API-Key", test_master_key()) - .body(Body::from(r#"{"file_path":"assets/no-owner.txt"}"#)) + .body(Body::from( + r#"{"file_path":"assets/no-owner.txt","repo_id":"repo-a"}"#, + )) .expect("request"); let response = app.oneshot(request).await.expect("response"); @@ -416,32 +438,187 @@ async fn rate_limit_returns_429_when_window_is_exhausted() { } #[tokio::test] -async fn rate_limit_is_bucketed_by_api_key() { +async fn invalid_credentials_share_the_anonymous_rate_limit_bucket() { let app = build_app(test_state(), &test_config_with_rate_limit(1)); - let first = Request::builder() + let mut first = Request::builder() .uri("/health/live") .header("X-API-Key", "key-a") .body(Body::empty()) .expect("first request"); + first.extensions_mut().insert(ConnectInfo( + "192.0.2.1:4000".parse::().expect("peer"), + )); let first_response = app.clone().oneshot(first).await.expect("first response"); assert_eq!(first_response.status(), StatusCode::OK); - let second = Request::builder() + let mut second = Request::builder() .uri("/health/live") - .header("X-API-Key", "key-a") + .header("X-API-Key", "key-b") .body(Body::empty()) .expect("second request"); + second.extensions_mut().insert(ConnectInfo( + "192.0.2.1:5000".parse::().expect("peer"), + )); let second_response = app.clone().oneshot(second).await.expect("second response"); assert_eq!(second_response.status(), StatusCode::TOO_MANY_REQUESTS); - let other_key = Request::builder() + let mut other_peer = Request::builder() .uri("/health/live") - .header("X-API-Key", "key-b") + .header("X-API-Key", "key-c") + .body(Body::empty()) + .expect("other peer request"); + other_peer.extensions_mut().insert(ConnectInfo( + "192.0.2.2:4000".parse::().expect("peer"), + )); + let other_peer_response = app + .clone() + .oneshot(other_peer) + .await + .expect("other peer response"); + assert_eq!(other_peer_response.status(), StatusCode::OK); + + let authenticated = Request::builder() + .uri("/health/live") + .header("X-API-Key", test_master_key()) + .body(Body::empty()) + .expect("authenticated request"); + let authenticated_response = app + .oneshot(authenticated) + .await + .expect("authenticated response"); + assert_eq!(authenticated_response.status(), StatusCode::OK); +} + +#[tokio::test] +async fn manifest_rejects_composed_blobs_over_the_memory_limit() { + let app = build_app(test_state(), &test_config()); + let payload = serde_json::json!({ + "version": 1, + "chunk_size_policy": "fixed", + "chunks": [{ + "i": 0, + "chunk_hash": "0".repeat(64), + "size": 268_435_457u64 + }], + "file_meta": null + }); + let request = Request::builder() + .method("POST") + .uri("/v2/manifests") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("request"); + + let response = app.oneshot(request).await.expect("response"); + assert_eq!(response.status(), StatusCode::PAYLOAD_TOO_LARGE); +} + +#[tokio::test] +async fn lock_release_returns_the_released_scoped_lock() { + let app = build_app(test_state(), &test_config()); + let payload = serde_json::json!({ + "file_path": "assets/scoped.txt", + "repo_id": "repo-a", + "scope": "asset" + }); + let acquire = Request::builder() + .method("POST") + .uri("/v2/locks/acquire") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("acquire request"); + assert_eq!( + app.clone() + .oneshot(acquire) + .await + .expect("acquire") + .status(), + StatusCode::OK + ); + + let release = Request::builder() + .method("POST") + .uri("/v2/locks/release") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("release request"); + let response = app.oneshot(release).await.expect("release"); + assert_eq!(response.status(), StatusCode::OK); + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + let json: Value = serde_json::from_slice(&body).expect("json"); + assert_eq!(json["data"]["repo_id"], "repo-a"); + assert_eq!(json["data"]["file_path"], "assets/scoped.txt"); +} + +#[tokio::test] +async fn lock_list_is_scoped_to_the_requested_repo() { + let app = build_app(test_state(), &test_config()); + for repo_id in ["repo-a", "repo-b"] { + let payload = serde_json::json!({ + "file_path": "assets/shared.txt", + "repo_id": repo_id, + "scope": "asset" + }); + let request = Request::builder() + .method("POST") + .uri("/v2/locks/acquire") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("acquire request"); + assert_eq!( + app.clone() + .oneshot(request) + .await + .expect("response") + .status(), + StatusCode::OK + ); + } + + let request = Request::builder() + .uri("/v2/locks?repo_id=repo-a&scope=asset") + .header("X-API-Key", test_master_key()) .body(Body::empty()) - .expect("other key request"); - let other_key_response = app.oneshot(other_key).await.expect("other key response"); - assert_eq!(other_key_response.status(), StatusCode::OK); + .expect("list request"); + let response = app.oneshot(request).await.expect("response"); + assert_eq!(response.status(), StatusCode::OK); + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + let json: Value = serde_json::from_slice(&body).expect("json"); + let locks = json["data"].as_array().expect("locks array"); + assert_eq!(locks.len(), 1); + assert_eq!(locks[0]["repo_id"], "repo-a"); +} + +#[tokio::test] +async fn submit_rejects_asset_paths_that_escape_the_workspace() { + let app = build_app(test_state(), &test_config()); + let payload = serde_json::json!({ + "repo_id": "repo-path-safety", + "branch": "main", + "base_changeset_id": "ROOT", + "author": "dev-admin", + "message": "invalid path", + "assets": [{ "path": "../../outside.txt", "blob_hash": null }] + }); + let request = Request::builder() + .method("POST") + .uri("/v2/changesets") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("request"); + + let response = app.oneshot(request).await.expect("response"); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); } #[tokio::test] diff --git a/deploy/server/.env.example b/deploy/server/.env.example index 408cf79..da958e0 100644 --- a/deploy/server/.env.example +++ b/deploy/server/.env.example @@ -11,5 +11,6 @@ JWT_ISSUER=hypertide ACCESS_TOKEN_TTL_SECS=900 REFRESH_TOKEN_TTL_SECS=604800 RATE_LIMIT_REQUESTS_PER_MINUTE=600 +MAX_COMPOSED_BLOB_BYTES=268435456 WITNESS_CONFIG_JSON={"witnesses":[{"id":"witness-a","secret":"dev-secret-a","scope":"local","environment":"local"},{"id":"witness-b","secret":"dev-secret-b","scope":"local","environment":"local"}],"quorum":1,"scope":"single-env"} RUST_LOG=hypertide_cli=info,hypertide=info diff --git a/deploy/server/.env.production.example b/deploy/server/.env.production.example index a0e8ec1..d5c8213 100644 --- a/deploy/server/.env.production.example +++ b/deploy/server/.env.production.example @@ -18,6 +18,7 @@ WITNESS_CONFIG_FILE=/app/keys/witness-config.json CORS_ALLOWED_ORIGINS=https://hypertide.example.com RATE_LIMIT_REQUESTS_PER_MINUTE=600 +MAX_COMPOSED_BLOB_BYTES=268435456 STORAGE_PATH=/app/storage HYPERTIDE_VERSION=latest diff --git a/deploy/server/docker-compose.yml b/deploy/server/docker-compose.yml index 38d6f6c..e426493 100644 --- a/deploy/server/docker-compose.yml +++ b/deploy/server/docker-compose.yml @@ -53,6 +53,7 @@ services: ACCESS_TOKEN_TTL_SECS: ${ACCESS_TOKEN_TTL_SECS:-900} REFRESH_TOKEN_TTL_SECS: ${REFRESH_TOKEN_TTL_SECS:-604800} RATE_LIMIT_REQUESTS_PER_MINUTE: ${RATE_LIMIT_REQUESTS_PER_MINUTE:-600} + MAX_COMPOSED_BLOB_BYTES: ${MAX_COMPOSED_BLOB_BYTES:-268435456} WITNESS_CONFIG_JSON: ${WITNESS_CONFIG_JSON:-} RUST_LOG: ${RUST_LOG:-hypertide_cli=info,hypertide=info} ports: diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index ba578b1..d256b5e 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -105,6 +105,15 @@ paths: /v2/locks: get: summary: List locks + parameters: + - in: query + name: repo_id + required: true + schema: { type: string } + - in: query + name: scope + required: false + schema: { type: string, default: asset } security: - ApiKeyAuth: [] - BearerAuth: [] diff --git a/docs/operations/self-hosting.md b/docs/operations/self-hosting.md index 452357d..85cfe1a 100644 --- a/docs/operations/self-hosting.md +++ b/docs/operations/self-hosting.md @@ -130,6 +130,10 @@ Production startup must reject unsafe configuration. These settings are required - `RATE_LIMIT_REQUESTS_PER_MINUTE` - `STORAGE_PATH` +Optional capacity tuning: + +- `MAX_COMPOSED_BLOB_BYTES` defaults to `268435456` (256 MiB). Raise it only when the server has enough memory for in-process blob composition. + Operational rules: - Terminate TLS at the reverse proxy. diff --git a/docs/server/README.md b/docs/server/README.md index eac3391..1b9d5bb 100644 --- a/docs/server/README.md +++ b/docs/server/README.md @@ -210,6 +210,7 @@ curl "http://localhost:3000/v2/changesets?repo_id=my-game&branch=main&limit=10" | 变量 | 说明 | 默认值 | |---|---|---| | `STORAGE_PATH` | 文件存储路径 | `./storage` | +| `MAX_COMPOSED_BLOB_BYTES` | 单次 manifest 合成的最大字节数 | `268435456` | ### 安全 diff --git a/migrations/202602260017_lock_primary_key_fix.down.sql b/migrations/202602260017_lock_primary_key_fix.down.sql new file mode 100644 index 0000000..647927a --- /dev/null +++ b/migrations/202602260017_lock_primary_key_fix.down.sql @@ -0,0 +1,20 @@ +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM locks + GROUP BY file_path + HAVING COUNT(*) > 1 + ) THEN + RAISE EXCEPTION 'cannot restore locks_pkey while duplicate file_path values exist'; + END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'locks_pkey' + AND conrelid = 'locks'::regclass + ) THEN + ALTER TABLE locks ADD CONSTRAINT locks_pkey PRIMARY KEY (file_path); + END IF; +END $$; diff --git a/migrations/202602260017_lock_primary_key_fix.up.sql b/migrations/202602260017_lock_primary_key_fix.up.sql new file mode 100644 index 0000000..0400154 --- /dev/null +++ b/migrations/202602260017_lock_primary_key_fix.up.sql @@ -0,0 +1,14 @@ +ALTER TABLE locks DROP CONSTRAINT IF EXISTS locks_pkey; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'locks_repo_scope_path' + AND conrelid = 'locks'::regclass + ) THEN + ALTER TABLE locks ADD CONSTRAINT locks_repo_scope_path + UNIQUE (repo_id, scope, file_path); + END IF; +END $$; From 0f56f116385bcb6b74320f1518297a4fa2d990f7 Mon Sep 17 00:00:00 2001 From: aoruLola Date: Thu, 16 Jul 2026 23:20:28 +0800 Subject: [PATCH 4/6] fix: honor legacy locks and staged deletions --- crates/cli/src/cmd/diff.rs | 8 +- crates/cli/src/cmd/status.rs | 6 +- crates/cli/src/utils.rs | 75 +++++++++++-- crates/server/src/core/lock.rs | 146 ++++++++++++++++++++----- crates/server/src/core/lock/repo_pg.rs | 31 ++++-- crates/server/src/tests.rs | 35 ++++++ 6 files changed, 251 insertions(+), 50 deletions(-) diff --git a/crates/cli/src/cmd/diff.rs b/crates/cli/src/cmd/diff.rs index 682109e..21b058d 100644 --- a/crates/cli/src/cmd/diff.rs +++ b/crates/cli/src/cmd/diff.rs @@ -31,12 +31,16 @@ pub(crate) async fn execute(args: DiffArgs) -> Result<()> { (None, Some(_)) => true, _ => false, }; - let staged = row.staged_hash.is_some(); + let staged = row.staged; if changed || staged { has_diff = true; let base = row.base_hash.as_deref().unwrap_or(""); let local = row.local_hash.as_deref().unwrap_or(""); - let staged_str = row.staged_hash.as_deref().unwrap_or(""); + let staged_str = match (row.staged, row.staged_hash.as_deref()) { + (true, Some(hash)) => hash, + (true, None) => "", + (false, _) => "", + }; println!( "{}\n base: {}\n local: {}\n staged: {}", row.path, base, local, staged_str diff --git a/crates/cli/src/cmd/status.rs b/crates/cli/src/cmd/status.rs index eb57bd4..c910dd2 100644 --- a/crates/cli/src/cmd/status.rs +++ b/crates/cli/src/cmd/status.rs @@ -52,6 +52,7 @@ pub(crate) async fn execute(args: StatusArgs) -> Result<()> { base_hash: Option, local_hash: Option, staged_hash: Option, + staged_deletion: bool, } let items: Vec = rows .iter() @@ -60,7 +61,7 @@ pub(crate) async fn execute(args: StatusArgs) -> Result<()> { let status = classify_asset_status( row.base_hash.as_deref(), row.local_hash.as_deref(), - row.staged_hash.as_deref(), + row.staged, lock_owner, stale_base, ); @@ -70,6 +71,7 @@ pub(crate) async fn execute(args: StatusArgs) -> Result<()> { base_hash: row.base_hash.clone(), local_hash: row.local_hash.clone(), staged_hash: row.staged_hash.clone(), + staged_deletion: row.staged && row.staged_hash.is_none(), } }) .collect(); @@ -80,7 +82,7 @@ pub(crate) async fn execute(args: StatusArgs) -> Result<()> { let status = classify_asset_status( row.base_hash.as_deref(), row.local_hash.as_deref(), - row.staged_hash.as_deref(), + row.staged, lock_owner, stale_base, ); diff --git a/crates/cli/src/utils.rs b/crates/cli/src/utils.rs index eb33c44..b80ea24 100644 --- a/crates/cli/src/utils.rs +++ b/crates/cli/src/utils.rs @@ -464,6 +464,7 @@ pub(crate) struct AssetRow { pub base_hash: Option, pub local_hash: Option, pub staged_hash: Option, + pub staged: bool, } #[allow(dead_code)] @@ -670,11 +671,11 @@ pub(crate) fn upsert_stage_asset( pub(crate) fn classify_asset_status( base_hash: Option<&str>, local_hash: Option<&str>, - staged_hash: Option<&str>, + staged: bool, lock_owner: Option<&str>, stale_base: bool, ) -> AssetStatusKind { - if staged_hash.is_some() { + if staged { return AssetStatusKind::Staged; } if lock_owner.is_some() { @@ -683,11 +684,10 @@ pub(crate) fn classify_asset_status( if stale_base { return AssetStatusKind::StaleBase; } - match (base_hash, local_hash, staged_hash) { - (_, _, Some(_)) => AssetStatusKind::Staged, - (Some(_), None, None) => AssetStatusKind::Deleted, - (Some(base), Some(local), None) if base != local => AssetStatusKind::Modified, - (None, Some(_), None) => AssetStatusKind::Added, + match (base_hash, local_hash) { + (Some(_), None) => AssetStatusKind::Deleted, + (Some(base), Some(local)) if base != local => AssetStatusKind::Modified, + (None, Some(_)) => AssetStatusKind::Added, _ => AssetStatusKind::Unmodified, } } @@ -714,17 +714,15 @@ pub(crate) fn collect_asset_rows( .iter() .find(|asset| asset.path == path) .map(|asset| asset.blob_hash.clone()); - let staged_hash = stage - .assets - .iter() - .find(|asset| asset.path == path) - .and_then(|asset| asset.blob_hash.clone()); + let staged_delta = stage.assets.iter().find(|asset| asset.path == path); + let staged_hash = staged_delta.and_then(|asset| asset.blob_hash.clone()); let local_hash = hash_local_asset(&workspace_root, &path)?; Ok(AssetRow { path, base_hash, local_hash, staged_hash, + staged: staged_delta.is_some(), }) }) .collect::>>()?; @@ -1820,3 +1818,56 @@ pub(crate) async fn add_file( ); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn staged_deletion_is_preserved_in_asset_rows_and_status() { + let root = std::env::temp_dir().join(format!( + "hypertide-staged-deletion-{}-{}", + std::process::id(), + now_unix() + )); + fs::create_dir_all(&root).expect("create workspace root"); + let workspace = WorkspaceState { + repo_id: "repo-a".to_string(), + branch: "main".to_string(), + workspace_root: root.to_string_lossy().to_string(), + base_changeset_id: Some("cs-1".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "0".repeat(64), + asset_id: Some("asset-a".to_string()), + }], + last_synced_at: 1, + }; + let stage = StageFile { + branch: "main".to_string(), + base_changeset_id: Some("cs-1".to_string()), + assets: vec![AssetDelta { + path: "Content/A.uasset".to_string(), + blob_hash: None, + asset_id: Some("asset-a".to_string()), + }], + }; + + let rows = collect_asset_rows(&workspace, &stage).expect("collect rows"); + assert_eq!(rows.len(), 1); + assert!(rows[0].staged); + assert!(rows[0].staged_hash.is_none()); + assert_eq!( + classify_asset_status( + rows[0].base_hash.as_deref(), + rows[0].local_hash.as_deref(), + rows[0].staged, + None, + false, + ), + AssetStatusKind::Staged + ); + + let _ = fs::remove_dir_all(root); + } +} diff --git a/crates/server/src/core/lock.rs b/crates/server/src/core/lock.rs index 15f67a5..84a76a4 100644 --- a/crates/server/src/core/lock.rs +++ b/crates/server/src/core/lock.rs @@ -5,6 +5,7 @@ use dashmap::mapref::entry::Entry; use dashmap::DashMap; use serde::{Deserialize, Serialize}; use sqlx::PgPool; +use std::collections::HashSet; use std::sync::Arc; pub mod repo_pg; @@ -119,6 +120,16 @@ impl LockManager { return Ok(effective_lock); } + if let Some(legacy) = self.active_legacy_lock(repo_id, scope, &file_path) { + if legacy.owner_id != owner_id { + return Err(HyperTideError::Conflict(format!( + "File is already locked by {}", + legacy.owner_id + ))); + } + return Ok(legacy); + } + match self.locks.entry(lock_key) { Entry::Occupied(mut occupied) => { let existing = occupied.get().clone(); @@ -157,12 +168,10 @@ impl LockManager { repo_id: &str, scope: &str, ) -> Result { - let lock_key = Self::lock_key(repo_id, scope, file_path); let existing = self - .locks - .get(&lock_key) - .map(|entry| entry.clone()) + .context_lock(repo_id, scope, file_path) .ok_or_else(|| HyperTideError::NotFound("File is not locked".to_string()))?; + let lock_key = Self::lock_key(&existing.repo_id, &existing.scope, &existing.file_path); if existing.owner_id != owner_id { return Err(HyperTideError::PermissionDenied(format!( @@ -172,7 +181,7 @@ impl LockManager { } if self.is_expired(&existing) { if let Some(repo) = &self.repo { - repo.delete_lock(repo_id, scope, file_path) + repo.delete_lock(&existing.repo_id, &existing.scope, &existing.file_path) .await .map_err(|e| { HyperTideError::Persistence(format!("failed to cleanup expired lock: {e}")) @@ -212,22 +221,20 @@ impl LockManager { repo_id: &str, scope: &str, ) -> Result { - let lock_key = Self::lock_key(repo_id, scope, file_path); + let existing = self + .context_lock(repo_id, scope, file_path) + .ok_or_else(|| HyperTideError::NotFound("File is not locked".to_string()))?; + let lock_key = Self::lock_key(&existing.repo_id, &existing.scope, &existing.file_path); // We need to check ownership before removing - let existing = if let Some(existing) = self.locks.get(&lock_key) { - if existing.owner_id != owner_id { - return Err(HyperTideError::PermissionDenied(format!( - "Cannot unlock: File is locked by {}", - existing.owner_id - ))); - } - existing.clone() - } else { - return Err(HyperTideError::NotFound("File is not locked".to_string())); - }; + if existing.owner_id != owner_id { + return Err(HyperTideError::PermissionDenied(format!( + "Cannot unlock: File is locked by {}", + existing.owner_id + ))); + } if let Some(repo) = &self.repo { - repo.delete_lock(repo_id, scope, file_path) + repo.delete_lock(&existing.repo_id, &existing.scope, &existing.file_path) .await .map_err(|e| HyperTideError::Persistence(format!("failed to delete lock: {e}")))?; } @@ -247,9 +254,23 @@ impl LockManager { repo_id: &str, scope: &str, ) -> Result { - let lock_key = Self::lock_key(repo_id, scope, file_path); + let effective = self.context_lock(repo_id, scope, file_path); + let lock_key = effective + .as_ref() + .map(|lock| Self::lock_key(&lock.repo_id, &lock.scope, &lock.file_path)) + .unwrap_or_else(|| Self::lock_key(repo_id, scope, file_path)); if let Some(repo) = &self.repo { - repo.delete_lock(repo_id, scope, file_path) + let (effective_repo, effective_scope, effective_path) = effective + .as_ref() + .map(|lock| { + ( + lock.repo_id.as_str(), + lock.scope.as_str(), + lock.file_path.as_str(), + ) + }) + .unwrap_or((repo_id, scope, file_path)); + repo.delete_lock(effective_repo, effective_scope, effective_path) .await .map_err(|e| { HyperTideError::Persistence(format!("failed to force release lock: {e}")) @@ -268,11 +289,33 @@ impl LockManager { } pub fn list_locks_with_repo(&self, repo_id: &str, scope: &str) -> Vec { - self.locks - .iter() - .map(|entry| entry.value().clone()) - .filter(|lock| lock.repo_id == repo_id && lock.scope == scope && !self.is_expired(lock)) - .collect() + let mut legacy_paths = HashSet::new(); + let mut locks = Vec::new(); + if !repo_id.is_empty() { + for lock in self + .locks + .iter() + .map(|entry| entry.value().clone()) + .filter(|lock| { + lock.repo_id.is_empty() && lock.scope == scope && !self.is_expired(lock) + }) + { + legacy_paths.insert(lock.file_path.clone()); + locks.push(lock); + } + } + locks.extend( + self.locks + .iter() + .map(|entry| entry.value().clone()) + .filter(|lock| { + lock.repo_id == repo_id + && lock.scope == scope + && !legacy_paths.contains(&lock.file_path) + && !self.is_expired(lock) + }), + ); + locks } /// Query lock by path. @@ -286,13 +329,29 @@ impl LockManager { scope: &str, file_path: &str, ) -> Option { - let lock_key = Self::lock_key(repo_id, scope, file_path); + self.context_lock(repo_id, scope, file_path) + .filter(|lock| !self.is_expired(lock)) + } + + fn active_legacy_lock(&self, repo_id: &str, scope: &str, file_path: &str) -> Option { + if repo_id.is_empty() { + return None; + } self.locks - .get(&lock_key) + .get(&Self::lock_key("", scope, file_path)) .map(|entry| entry.clone()) .filter(|lock| !self.is_expired(lock)) } + fn context_lock(&self, repo_id: &str, scope: &str, file_path: &str) -> Option { + self.active_legacy_lock(repo_id, scope, file_path) + .or_else(|| { + self.locks + .get(&Self::lock_key(repo_id, scope, file_path)) + .map(|entry| entry.clone()) + }) + } + fn next_lease_expiry(&self) -> DateTime { Utc::now() + chrono::Duration::seconds(self.lease_seconds.max(30)) } @@ -354,4 +413,37 @@ mod tests { "bob" ); } + + #[tokio::test] + async fn repo_scoped_operations_honor_and_release_legacy_locks() { + let manager = LockManager::new(); + manager + .try_lock("Content/Legacy.uasset".to_string(), "alice".to_string()) + .await + .expect("legacy lock"); + + let visible = manager + .get_lock_with_repo("repo-a", "asset", "Content/Legacy.uasset") + .expect("legacy lock is visible in repo context"); + assert_eq!(visible.owner_id, "alice"); + assert!(manager + .try_lock_with_repo( + "Content/Legacy.uasset".to_string(), + "bob".to_string(), + "repo-a", + "asset", + ) + .await + .is_err()); + assert_eq!(manager.list_locks_with_repo("repo-a", "asset").len(), 1); + + let released = manager + .unlock_with_repo("Content/Legacy.uasset", "alice", "repo-a", "asset") + .await + .expect("release legacy lock through repo context"); + assert!(released.repo_id.is_empty()); + assert!(manager + .get_lock_with_repo("repo-a", "asset", "Content/Legacy.uasset") + .is_none()); + } } diff --git a/crates/server/src/core/lock/repo_pg.rs b/crates/server/src/core/lock/repo_pg.rs index 4428236..7537d12 100644 --- a/crates/server/src/core/lock/repo_pg.rs +++ b/crates/server/src/core/lock/repo_pg.rs @@ -75,9 +75,20 @@ impl LockRepoPg { pub async fn acquire_lock_atomic(&self, lock: &FileLock) -> Result { let row = sqlx::query_as::<_, LockRow>( r#" - WITH attempted AS ( + WITH legacy_lock AS ( + SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope + FROM locks + WHERE $5 <> '' + AND repo_id = '' + AND scope = $6 + AND file_path = $1 + AND force_released = FALSE + AND (lease_expires_at IS NULL OR lease_expires_at > NOW()) + ), + attempted AS ( INSERT INTO locks (file_path, owner_id, locked_at, lease_expires_at, force_released, repo_id, scope) - VALUES ($1, $2, $3, $4, FALSE, $5, $6) + SELECT $1, $2, $3, $4, FALSE, $5, $6 + WHERE NOT EXISTS (SELECT 1 FROM legacy_lock) ON CONFLICT (repo_id, scope, file_path) DO UPDATE SET owner_id = EXCLUDED.owner_id, @@ -95,11 +106,17 @@ impl LockRepoPg { WHERE repo_id = $5 AND scope = $6 AND file_path = $1 AND force_released = FALSE ) SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope - FROM attempted - UNION ALL - SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope - FROM current_lock - WHERE NOT EXISTS (SELECT 1 FROM attempted) + FROM ( + SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope, 0 AS priority + FROM legacy_lock + UNION ALL + SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope, 1 AS priority + FROM attempted + UNION ALL + SELECT file_path, owner_id, locked_at, lease_expires_at, repo_id, scope, 2 AS priority + FROM current_lock + ) candidates + ORDER BY priority LIMIT 1 "#, ) diff --git a/crates/server/src/tests.rs b/crates/server/src/tests.rs index 53ab6ea..1e03ed9 100644 --- a/crates/server/src/tests.rs +++ b/crates/server/src/tests.rs @@ -598,6 +598,41 @@ async fn lock_list_is_scoped_to_the_requested_repo() { assert_eq!(locks[0]["repo_id"], "repo-a"); } +#[tokio::test] +async fn legacy_lock_blocks_repo_scoped_changeset_submit() { + let state = test_state(); + state + .lock_manager + .try_lock( + "Content/Legacy.uasset".to_string(), + "legacy-owner".to_string(), + ) + .await + .expect("legacy lock"); + let app = build_app(state, &test_config()); + let payload = serde_json::json!({ + "repo_id": "repo-a", + "branch": "main", + "base_changeset_id": "ROOT", + "author": "dev-admin", + "message": "must respect legacy lock", + "assets": [{ + "path": "Content/Legacy.uasset", + "blob_hash": "0".repeat(64) + }] + }); + let request = Request::builder() + .method("POST") + .uri("/v2/changesets") + .header("content-type", "application/json") + .header("X-API-Key", test_master_key()) + .body(Body::from(payload.to_string())) + .expect("submit request"); + + let response = app.oneshot(request).await.expect("submit response"); + assert_eq!(response.status(), StatusCode::CONFLICT); +} + #[tokio::test] async fn submit_rejects_asset_paths_that_escape_the_workspace() { let app = build_app(test_state(), &test_config()); From cac8f085d6df2e9542464ecafb2211da4a633da4 Mon Sep 17 00:00:00 2001 From: aoruLola Date: Sat, 18 Jul 2026 18:16:21 +0800 Subject: [PATCH 5/6] fix: isolate invalid bearer rate limits by peer --- crates/server/src/routes.rs | 4 +--- crates/server/src/tests.rs | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index e216a1e..abc1558 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -227,9 +227,7 @@ async fn rate_limit_bucket( if let Ok(identity) = rate_limit.auth_manager.validate_access_token(&token).await { return format!("principal:{}", identity.owner_id); } - return "anonymous".to_string(); - } - if let Some(api_key) = api_key { + } else if let Some(api_key) = api_key { if let Ok(identity) = rate_limit .auth_manager .validate_api_key_identity(&api_key) diff --git a/crates/server/src/tests.rs b/crates/server/src/tests.rs index 1e03ed9..405999d 100644 --- a/crates/server/src/tests.rs +++ b/crates/server/src/tests.rs @@ -490,6 +490,48 @@ async fn invalid_credentials_share_the_anonymous_rate_limit_bucket() { assert_eq!(authenticated_response.status(), StatusCode::OK); } +#[tokio::test] +async fn invalid_bearer_tokens_use_the_anonymous_peer_ip_bucket() { + let app = build_app(test_state(), &test_config_with_rate_limit(1)); + + let mut first = Request::builder() + .uri("/health/live") + .header("Authorization", "Bearer invalid-a") + .body(Body::empty()) + .expect("first request"); + first.extensions_mut().insert(ConnectInfo( + "192.0.2.1:4000".parse::().expect("peer"), + )); + let first_response = app.clone().oneshot(first).await.expect("first response"); + assert_eq!(first_response.status(), StatusCode::OK); + + let mut same_peer = Request::builder() + .uri("/health/live") + .header("Authorization", "Bearer invalid-b") + .body(Body::empty()) + .expect("same peer request"); + same_peer.extensions_mut().insert(ConnectInfo( + "192.0.2.1:5000".parse::().expect("peer"), + )); + let same_peer_response = app + .clone() + .oneshot(same_peer) + .await + .expect("same peer response"); + assert_eq!(same_peer_response.status(), StatusCode::TOO_MANY_REQUESTS); + + let mut other_peer = Request::builder() + .uri("/health/live") + .header("Authorization", "Bearer invalid-c") + .body(Body::empty()) + .expect("other peer request"); + other_peer.extensions_mut().insert(ConnectInfo( + "192.0.2.2:4000".parse::().expect("peer"), + )); + let other_peer_response = app.oneshot(other_peer).await.expect("other peer response"); + assert_eq!(other_peer_response.status(), StatusCode::OK); +} + #[tokio::test] async fn manifest_rejects_composed_blobs_over_the_memory_limit() { let app = build_app(test_state(), &test_config()); From bb488ec54c786547e93a4fb9de0ce0d1e723f84c Mon Sep 17 00:00:00 2001 From: aoruLola Date: Sat, 18 Jul 2026 19:25:56 +0800 Subject: [PATCH 6/6] fix: harden proxy rate limits and identity reverts --- Cargo.lock | 1 + Cargo.toml | 1 + crates/cli/src/cmd/revert.rs | 64 ++++++++++++++++++- crates/cli/src/cmd/server.rs | 2 + crates/server/Cargo.toml | 1 + crates/server/src/core/auth.rs | 1 + crates/server/src/core/config.rs | 58 ++++++++++++++++++ crates/server/src/routes.rs | 69 +++++++++++++++++++-- crates/server/src/state.rs | 1 + crates/server/src/tests.rs | 88 +++++++++++++++++++++++++++ deploy/server/.env.production.example | 2 + deploy/server/Caddyfile.example | 1 + deploy/server/README.md | 1 + docs/operations/self-hosting.md | 3 + 14 files changed, 285 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5218432..d37d81f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -885,6 +885,7 @@ dependencies = [ "dotenvy", "hex", "hmac", + "ipnet", "jsonwebtoken", "reqwest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 0426d7e..495d1d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,4 +32,5 @@ async-trait = "0.1" hmac = "0.12" sha2 = "0.10" hex = "0.4" +ipnet = "2.11" windows-sys = { version = "0.61.2", features = ["Win32_Storage_FileSystem"] } diff --git a/crates/cli/src/cmd/revert.rs b/crates/cli/src/cmd/revert.rs index f0d1dec..9523d87 100644 --- a/crates/cli/src/cmd/revert.rs +++ b/crates/cli/src/cmd/revert.rs @@ -108,6 +108,7 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { Some(&asset.blob_hash), base_hash.as_deref(), asset.asset_id.clone(), + base_asset_id.as_deref(), ); (update, Some(asset.blob_hash.clone())) @@ -125,7 +126,8 @@ pub(crate) async fn execute(args: RevertArgs) -> Result<()> { &asset_path, None, base_hash.as_deref(), - base_asset_id, + base_asset_id.clone(), + base_asset_id.as_deref(), ); (update, None) @@ -252,10 +254,21 @@ fn apply_revert_state( blob_hash: Option<&str>, base_hash: Option<&str>, asset_id: Option, + base_asset_id: Option<&str>, ) -> RevertStateUpdate { - if base_hash == blob_hash { + let target_asset_id = blob_hash.and(asset_id.as_deref()); + let same_asset_identity = match (base_asset_id, target_asset_id) { + (Some(base), Some(target)) => base == target, + _ => true, + }; + if base_hash == blob_hash && same_asset_identity { if let Some(hash) = blob_hash { - update_workspace_asset(workspace, asset_path, hash, asset_id); + update_workspace_asset( + workspace, + asset_path, + hash, + target_asset_id.map(str::to_string), + ); } return RevertStateUpdate { removed_staged_delta: remove_staged_asset(stage, asset_path), @@ -571,6 +584,7 @@ mod tests { Some("old-hash"), Some("head-hash"), None, + None, ); assert_eq!( @@ -617,6 +631,7 @@ mod tests { Some("head-hash"), Some("head-hash"), None, + None, ); assert_eq!( @@ -630,6 +645,48 @@ mod tests { assert!(stage.assets.is_empty()); } + #[test] + fn apply_revert_state_stages_asset_identity_change_with_same_blob() { + let mut workspace = WorkspaceState { + repo_id: "repo".to_string(), + branch: "main".to_string(), + workspace_root: ".".to_string(), + base_changeset_id: Some("cs-head".to_string()), + checked_out_assets: vec![WorkspaceFile { + path: "Content/A.uasset".to_string(), + blob_hash: "same-hash".to_string(), + asset_id: Some("asset-current".to_string()), + }], + last_synced_at: 1, + }; + let mut stage = StageFile::default_for_branch("main"); + + let update = apply_revert_state( + &mut workspace, + &mut stage, + "Content/A.uasset", + Some("same-hash"), + Some("same-hash"), + Some("asset-old".to_string()), + Some("asset-current"), + ); + + assert_eq!( + update, + RevertStateUpdate { + removed_staged_delta: false, + staged_delta: true, + } + ); + assert_eq!( + workspace.checked_out_assets[0].asset_id.as_deref(), + Some("asset-current") + ); + assert_eq!(stage.assets.len(), 1); + assert_eq!(stage.assets[0].blob_hash.as_deref(), Some("same-hash")); + assert_eq!(stage.assets[0].asset_id.as_deref(), Some("asset-old")); + } + #[tokio::test(flavor = "current_thread")] async fn revert_execute_e2e_stages_old_snapshot_blob_and_encodes_sync_query() { let _guard = cwd_lock().lock().await; @@ -898,6 +955,7 @@ mod tests { None, Some("head-hash"), Some("stable-asset-a".to_string()), + Some("stable-asset-a"), ); assert_eq!(stage.assets.len(), 1); diff --git a/crates/cli/src/cmd/server.rs b/crates/cli/src/cmd/server.rs index 12c15c5..042f795 100644 --- a/crates/cli/src/cmd/server.rs +++ b/crates/cli/src/cmd/server.rs @@ -91,6 +91,7 @@ fn validate_server_env(env: &HashMap) -> DoctorReport { "HIGH_RISK_SIGNING_SECRET", "CORS_ALLOWED_ORIGINS", "RATE_LIMIT_REQUESTS_PER_MINUTE", + "TRUSTED_PROXY_CIDRS", "STORAGE_PATH", ] { match env.get(required).filter(|value| !value.trim().is_empty()) { @@ -213,6 +214,7 @@ HIGH_RISK_SIGNATURE_REQUIRED=true HIGH_RISK_SIGNING_SECRET=secure-secret CORS_ALLOWED_ORIGINS=https://hypertide.example.com RATE_LIMIT_REQUESTS_PER_MINUTE=600 +TRUSTED_PROXY_CIDRS=172.16.0.0/12 STORAGE_PATH=/app/storage "#, ); diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 554d5af..4313912 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -31,6 +31,7 @@ async-trait.workspace = true hmac.workspace = true sha2.workspace = true hex.workspace = true +ipnet.workspace = true [target.'cfg(windows)'.dependencies] windows-sys.workspace = true diff --git a/crates/server/src/core/auth.rs b/crates/server/src/core/auth.rs index 8b3407b..230c7b6 100644 --- a/crates/server/src/core/auth.rs +++ b/crates/server/src/core/auth.rs @@ -263,6 +263,7 @@ impl AuthManager { revoked: stored.revoked, }; if api_key.is_valid() { + self.keys.insert(key.to_string(), api_key.clone()); Some(api_key) } else { None diff --git a/crates/server/src/core/config.rs b/crates/server/src/core/config.rs index ef83ae9..2820dc6 100644 --- a/crates/server/src/core/config.rs +++ b/crates/server/src/core/config.rs @@ -1,4 +1,5 @@ use axum::http::HeaderValue; +use ipnet::IpNet; use std::path::Path; const DEV_MASTER_KEY: &str = "dev-master-key"; @@ -65,6 +66,7 @@ pub struct AppConfig { pub storage_path: String, pub cors_allowed_origins: Vec, pub rate_limit_requests_per_minute: u64, + pub trusted_proxy_cidrs: Vec, pub log_format: LogFormat, } @@ -112,6 +114,10 @@ impl AppConfig { DEFAULT_RATE_LIMIT_REQUESTS_PER_MINUTE, "RATE_LIMIT_REQUESTS_PER_MINUTE", )?; + let trusted_proxy_cidrs = parse_cidr_list(lookup("TRUSTED_PROXY_CIDRS").as_deref())?; + if app_env.is_production() && trusted_proxy_cidrs.is_empty() { + return Err("TRUSTED_PROXY_CIDRS is required when APP_ENV=production".to_string()); + } let default_log_format = if app_env.is_production() { LogFormat::Json } else { @@ -128,11 +134,25 @@ impl AppConfig { storage_path, cors_allowed_origins, rate_limit_requests_per_minute, + trusted_proxy_cidrs, log_format, }) } } +fn parse_cidr_list(raw: Option<&str>) -> Result, String> { + raw.unwrap_or_default() + .split(',') + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(|value| { + value + .parse::() + .map_err(|_| format!("invalid TRUSTED_PROXY_CIDRS entry: {value}")) + }) + .collect() +} + fn parse_origin_list(raw: Option<&str>) -> Result, String> { let mut values = Vec::new(); if let Some(raw_origins) = raw { @@ -257,9 +277,35 @@ mod tests { assert_eq!(cfg.app_env, AppEnv::Development); assert_eq!(cfg.master_key, "dev-master-key"); assert!(cfg.cors_allowed_origins.is_empty()); + assert!(cfg.trusted_proxy_cidrs.is_empty()); assert_eq!(cfg.log_format, LogFormat::Plain); } + #[test] + fn parses_trusted_proxy_cidrs() { + let cfg = AppConfig::from_lookup(|name| { + (name == "TRUSTED_PROXY_CIDRS").then(|| "10.0.0.0/8, 2001:db8::/32".to_string()) + }) + .expect("config"); + + assert_eq!(cfg.trusted_proxy_cidrs.len(), 2); + assert!( + cfg.trusted_proxy_cidrs[0].contains(&"10.1.2.3".parse::().unwrap()) + ); + assert!(cfg.trusted_proxy_cidrs[1] + .contains(&"2001:db8::1".parse::().unwrap())); + } + + #[test] + fn rejects_invalid_trusted_proxy_cidrs() { + let err = AppConfig::from_lookup(|name| { + (name == "TRUSTED_PROXY_CIDRS").then(|| "not-a-cidr".to_string()) + }) + .expect_err("invalid cidr must fail"); + + assert!(err.contains("TRUSTED_PROXY_CIDRS")); + } + #[test] fn production_requires_security_variables() { let mut env = HashMap::new(); @@ -295,6 +341,10 @@ mod tests { "secure-signing-secret".to_string(), ); env.insert("AUTH_PEPPER".to_string(), "secure-pepper".to_string()); + env.insert( + "TRUSTED_PROXY_CIDRS".to_string(), + "172.16.0.0/12".to_string(), + ); insert_test_key_paths(&mut env); env.insert( "WITNESS_KEYS".to_string(), @@ -324,6 +374,10 @@ mod tests { "secure-signing-secret".to_string(), ); env.insert("AUTH_PEPPER".to_string(), "secure-pepper".to_string()); + env.insert( + "TRUSTED_PROXY_CIDRS".to_string(), + "172.16.0.0/12".to_string(), + ); insert_test_key_paths(&mut env); env.insert( "WITNESS_CONFIG_JSON".to_string(), @@ -352,6 +406,10 @@ mod tests { "secure-signing-secret".to_string(), ); env.insert("AUTH_PEPPER".to_string(), "secure-pepper".to_string()); + env.insert( + "TRUSTED_PROXY_CIDRS".to_string(), + "172.16.0.0/12".to_string(), + ); insert_test_key_paths(&mut env); env.insert( "WITNESS_KEYS".to_string(), diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index abc1558..9985012 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -1,7 +1,7 @@ use axum::{ body::Body, extract::{ConnectInfo, DefaultBodyLimit, MatchedPath, State}, - http::{HeaderValue, StatusCode}, + http::{HeaderMap, HeaderValue, StatusCode}, middleware::{self, Next}, response::{IntoResponse, Response}, routing::{delete, get, post, put}, @@ -63,6 +63,7 @@ pub(crate) fn build_app(state: AppState, config: &AppConfig) -> Router { limiter: rate_limiter, metrics: metrics.clone(), auth_manager: state.auth_manager.clone(), + trusted_proxy_cidrs: config.trusted_proxy_cidrs.clone(), }; let general_routes = Router::new() @@ -209,7 +210,16 @@ async fn enforce_rate_limit( .extensions() .get::>() .map(|connect_info| connect_info.0.ip()); - let bucket = rate_limit_bucket(&rate_limit, bearer, api_key, peer_ip).await; + let client_ip = resolve_client_ip(request.headers(), peer_ip, &rate_limit.trusted_proxy_cidrs); + if (bearer.is_some() || api_key.is_some()) + && !rate_limit + .limiter + .allow(&ip_bucket("auth-lookup", client_ip)) + { + rate_limit.metrics.record_rate_limited(); + return (StatusCode::TOO_MANY_REQUESTS, "RATE_LIMITED").into_response(); + } + let bucket = rate_limit_bucket(&rate_limit, bearer, api_key, client_ip).await; if !rate_limit.limiter.allow(&bucket) { rate_limit.metrics.record_rate_limited(); return (StatusCode::TOO_MANY_REQUESTS, "RATE_LIMITED").into_response(); @@ -217,6 +227,57 @@ async fn enforce_rate_limit( next.run(request).await } +fn resolve_client_ip( + headers: &HeaderMap, + peer_ip: Option, + trusted_proxy_cidrs: &[ipnet::IpNet], +) -> Option { + let peer_ip = peer_ip?; + if !is_trusted_proxy(peer_ip, trusted_proxy_cidrs) { + return Some(peer_ip); + } + + forwarded_for_client_ip(headers, trusted_proxy_cidrs) + .or_else(|| { + headers + .get("x-real-ip") + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.trim().parse().ok()) + }) + .or(Some(peer_ip)) +} + +fn forwarded_for_client_ip( + headers: &HeaderMap, + trusted_proxy_cidrs: &[ipnet::IpNet], +) -> Option { + let mut forwarded = Vec::new(); + for value in headers.get_all("x-forwarded-for") { + let value = value.to_str().ok()?; + for address in value.split(',') { + forwarded.push(address.trim().parse::().ok()?); + } + } + forwarded + .iter() + .rev() + .copied() + .find(|address| !is_trusted_proxy(*address, trusted_proxy_cidrs)) + .or_else(|| forwarded.first().copied()) +} + +fn is_trusted_proxy(address: IpAddr, trusted_proxy_cidrs: &[ipnet::IpNet]) -> bool { + trusted_proxy_cidrs + .iter() + .any(|network| network.contains(&address)) +} + +fn ip_bucket(prefix: &str, client_ip: Option) -> String { + client_ip + .map(|ip| format!("{prefix}:{ip}")) + .unwrap_or_else(|| prefix.to_string()) +} + async fn rate_limit_bucket( rate_limit: &RateLimitState, bearer: Option, @@ -236,7 +297,5 @@ async fn rate_limit_bucket( return format!("principal:{}", identity.owner_id); } } - peer_ip - .map(|ip| format!("anonymous:{ip}")) - .unwrap_or_else(|| "anonymous".to_string()) + ip_bucket("anonymous", peer_ip) } diff --git a/crates/server/src/state.rs b/crates/server/src/state.rs index e26f0a4..a2973f0 100644 --- a/crates/server/src/state.rs +++ b/crates/server/src/state.rs @@ -279,6 +279,7 @@ pub(crate) struct RateLimitState { pub(crate) limiter: RateLimiter, pub(crate) metrics: HttpMetrics, pub(crate) auth_manager: AuthManager, + pub(crate) trusted_proxy_cidrs: Vec, } fn escape_label_value(value: &str) -> String { diff --git a/crates/server/src/tests.rs b/crates/server/src/tests.rs index 405999d..553ddd6 100644 --- a/crates/server/src/tests.rs +++ b/crates/server/src/tests.rs @@ -31,6 +31,7 @@ fn test_config() -> AppConfig { storage_path: "./storage".to_string(), cors_allowed_origins: Vec::::new(), rate_limit_requests_per_minute: 600, + trusted_proxy_cidrs: Vec::new(), log_format: LogFormat::Plain, } } @@ -532,6 +533,93 @@ async fn invalid_bearer_tokens_use_the_anonymous_peer_ip_bucket() { assert_eq!(other_peer_response.status(), StatusCode::OK); } +#[tokio::test] +async fn trusted_proxy_forwarded_clients_use_separate_anonymous_buckets() { + let mut config = test_config_with_rate_limit(1); + config.trusted_proxy_cidrs = vec!["10.0.0.0/8".parse().expect("cidr")]; + let app = build_app(test_state(), &config); + + let mut first = Request::builder() + .uri("/health/live") + .header("X-Forwarded-For", "192.0.2.1, 10.0.0.4") + .body(Body::empty()) + .expect("first request"); + first.extensions_mut().insert(ConnectInfo( + "10.0.0.5:4000".parse::().expect("proxy"), + )); + let first_response = app.clone().oneshot(first).await.expect("first response"); + assert_eq!(first_response.status(), StatusCode::OK); + + let mut second = Request::builder() + .uri("/health/live") + .header("X-Real-IP", "192.0.2.2") + .body(Body::empty()) + .expect("second request"); + second.extensions_mut().insert(ConnectInfo( + "10.0.0.5:5000".parse::().expect("proxy"), + )); + let second_response = app.oneshot(second).await.expect("second response"); + assert_eq!(second_response.status(), StatusCode::OK); +} + +#[tokio::test] +async fn untrusted_peers_cannot_spoof_forwarded_rate_limit_addresses() { + let app = build_app(test_state(), &test_config_with_rate_limit(1)); + + let mut first = Request::builder() + .uri("/health/live") + .header("X-Forwarded-For", "192.0.2.1") + .body(Body::empty()) + .expect("first request"); + first.extensions_mut().insert(ConnectInfo( + "198.51.100.5:4000".parse::().expect("peer"), + )); + let first_response = app.clone().oneshot(first).await.expect("first response"); + assert_eq!(first_response.status(), StatusCode::OK); + + let mut second = Request::builder() + .uri("/health/live") + .header("X-Forwarded-For", "192.0.2.2") + .body(Body::empty()) + .expect("second request"); + second.extensions_mut().insert(ConnectInfo( + "198.51.100.5:5000".parse::().expect("peer"), + )); + let second_response = app.oneshot(second).await.expect("second response"); + assert_eq!(second_response.status(), StatusCode::TOO_MANY_REQUESTS); +} + +#[tokio::test] +async fn auth_lookup_pre_limit_rejects_before_credential_bucket_selection() { + let app = build_app(test_state(), &test_config_with_rate_limit(1)); + + let mut invalid = Request::builder() + .uri("/health/live") + .header("X-API-Key", "invalid") + .body(Body::empty()) + .expect("invalid request"); + invalid.extensions_mut().insert(ConnectInfo( + "192.0.2.1:4000".parse::().expect("peer"), + )); + let invalid_response = app + .clone() + .oneshot(invalid) + .await + .expect("invalid response"); + assert_eq!(invalid_response.status(), StatusCode::OK); + + let mut valid = Request::builder() + .uri("/health/live") + .header("X-API-Key", test_master_key()) + .body(Body::empty()) + .expect("valid request"); + valid.extensions_mut().insert(ConnectInfo( + "192.0.2.1:5000".parse::().expect("peer"), + )); + let valid_response = app.oneshot(valid).await.expect("valid response"); + assert_eq!(valid_response.status(), StatusCode::TOO_MANY_REQUESTS); +} + #[tokio::test] async fn manifest_rejects_composed_blobs_over_the_memory_limit() { let app = build_app(test_state(), &test_config()); diff --git a/deploy/server/.env.production.example b/deploy/server/.env.production.example index d5c8213..0f1620e 100644 --- a/deploy/server/.env.production.example +++ b/deploy/server/.env.production.example @@ -18,6 +18,8 @@ WITNESS_CONFIG_FILE=/app/keys/witness-config.json CORS_ALLOWED_ORIGINS=https://hypertide.example.com RATE_LIMIT_REQUESTS_PER_MINUTE=600 +# Docker bridge range used by the Caddy container. Narrow this for custom networks. +TRUSTED_PROXY_CIDRS=172.16.0.0/12 MAX_COMPOSED_BLOB_BYTES=268435456 STORAGE_PATH=/app/storage diff --git a/deploy/server/Caddyfile.example b/deploy/server/Caddyfile.example index 649f094..aaea48d 100644 --- a/deploy/server/Caddyfile.example +++ b/deploy/server/Caddyfile.example @@ -10,6 +10,7 @@ reverse_proxy hypertide:3000 { header_up X-Forwarded-Proto {scheme} header_up X-Forwarded-Host {host} + header_up X-Forwarded-For {remote_host} header_up X-Real-IP {remote_host} } } diff --git a/deploy/server/README.md b/deploy/server/README.md index 990f75f..140cc13 100644 --- a/deploy/server/README.md +++ b/deploy/server/README.md @@ -35,5 +35,6 @@ powershell -ExecutionPolicy Bypass -File .\deploy\server\smoke.ps1 - JWT keys are generated into `deploy/server/keys/`. - Persistent asset storage remains at the repository-level `storage/` directory. - `RATE_LIMIT_REQUESTS_PER_MINUTE` defaults to `600`; set `0` only for trusted development environments. +- Production proxy deployments must set `TRUSTED_PROXY_CIDRS`; forwarded client IP headers are ignored unless the direct peer is in one of these CIDRs. - Prefer `WITNESS_CONFIG_JSON` or `WITNESS_CONFIG_FILE` for witness configuration. Legacy `WITNESS_KEYS` remains supported for compatibility. - For production, set `APP_ENV=production`, replace the example database password, pepper, JWT keys, witness secrets, and high-risk signing secret. diff --git a/docs/operations/self-hosting.md b/docs/operations/self-hosting.md index 85cfe1a..45d3e0f 100644 --- a/docs/operations/self-hosting.md +++ b/docs/operations/self-hosting.md @@ -128,16 +128,19 @@ Production startup must reject unsafe configuration. These settings are required - `WITNESS_CONFIG_JSON` or `WITNESS_CONFIG_FILE` - `CORS_ALLOWED_ORIGINS` - `RATE_LIMIT_REQUESTS_PER_MINUTE` +- `TRUSTED_PROXY_CIDRS` (the CIDRs from which the server may trust `X-Forwarded-For`/`X-Real-IP`) - `STORAGE_PATH` Optional capacity tuning: - `MAX_COMPOSED_BLOB_BYTES` defaults to `268435456` (256 MiB). Raise it only when the server has enough memory for in-process blob composition. +- Credential-bearing requests are first capped per resolved client IP before authentication lookup, then by authenticated principal. Raise the main rate limit carefully when many trusted clients legitimately share one source IP. Operational rules: - Terminate TLS at the reverse proxy. - Do not publish server port `3000` directly to the internet. +- Keep `TRUSTED_PROXY_CIDRS` limited to the proxy network. Forwarded client-IP headers from any other peer are ignored. - Store `.env.production` and `keys/` outside public repos and ticket attachments. - Keep backups encrypted when moved off host.