diff --git a/config/default.toml b/config/default.toml index 3dab300b..0c8555a0 100644 --- a/config/default.toml +++ b/config/default.toml @@ -79,6 +79,30 @@ peer_discovery_refresh_interval_secs = 5 # Timeout for custom extension HTTP calls, in milliseconds. # timeout_ms = 5000 +[template_build] +# Build-context upload settings backing the E2B SDK's COPY support +# (GET /templates/{templateID}/files/{hash} plus the returned upload URL). +# Maximum accepted size for one uploaded build-context archive, in MiB. +# files_max_upload_mib = 1024 +# Maximum size one build-context archive may expand to once decompressed, in MiB. +# files_max_context_mib = 4096 +# Cap on the combined on-disk size of all build-context archives one build spec +# may reference, in MiB. +# files_max_build_context_mib = 4096 +# How long an issued upload URL stays valid, in seconds. +# files_url_ttl_secs = 3600 +# How long one build-context upload request may run before the server gives up +# and responds 408, in seconds. +# files_upload_timeout_secs = 300 +# Optional external base URL used when building upload URLs. Defaults to +# "http://{Host header}" of the upload-link request, which matches +# direct-node and bundled-gateway deployments. +# Any TLS-terminated, gateway-fronted, or multi-hop deployment MUST set this to +# the external origin clients reach: the fallback derives the URL from the +# request Host header with plain http, and that upload URL carries a bearer +# token in its query string. +# public_base_url = "https://agentenv.example.com" + [cluster] # Shared gRPC endpoint for cluster-level services such as scheduler heartbeat # reporting and P2P peer discovery (e.g. "http://127.0.0.1:9090"). diff --git a/docs/src/integration/e2b.md b/docs/src/integration/e2b.md index 6a6db325..f767fd20 100644 --- a/docs/src/integration/e2b.md +++ b/docs/src/integration/e2b.md @@ -97,6 +97,34 @@ sandbox.beta_pause() sandbox.kill() ``` +### Template builds + +The SDK's template builder works against AgentENV, including Dockerfiles with `COPY`: + +```python +import asyncio +from e2b import AsyncTemplate, Template + +template = Template(file_context_path=".").from_dockerfile( + """ + FROM ubuntu:24.04 + COPY requirements.txt /opt/app/requirements.txt + RUN apt-get update && apt-get install -y python3 + """ +) +asyncio.run(AsyncTemplate.build(template=template, alias="my-template")) +``` + +How `COPY` works: for each `COPY` instruction the SDK requests an upload link (`GET /templates/{templateID}/files/{hash}`), `PUT`s a tar archive of the matching context files to the returned bearer upload URL, and references the archive by `filesHash` when it starts the build. AgentENV stores the archives in the snapshot repository (shared across nodes) and extracts them inside the build sandbox. + +Requirements and behavior notes: + +- The base image must provide `/bin/bash` (already required for `RUN` steps) and `tar` for `COPY` steps. +- Copied files are owned by `root:root` like Docker's `COPY` default. `COPY --chown=user:group` resolves names against the image's own `/etc/passwd` and `/etc/group` and applies only to the files the copy creates; an unknown user fails the build. +- Write directory destinations with a trailing slash (`COPY app.py /opt/`). Docker's special case of copying a single file onto an existing directory named without a trailing slash (`COPY app.py /opt`) is not supported and fails the build with a clear error. +- Rebuilding an existing alias is allowed: the alias keeps pointing at the previous template while the new build runs and moves to the new template when the build commits (E2B semantics). The previous template stays addressable by ID. A failed rebuild leaves the alias untouched. +- Any TLS-terminated, gateway-fronted, or multi-hop deployment must set `template_build.public_base_url` (or `AENV_TEMPLATE_BUILD_PUBLIC_BASE_URL`) to the external origin clients reach, for example `https://agentenv.example.com`. When it is unset, the upload URL is derived from the request `Host` header with plain `http`, and because that URL carries a bearer token in its query string the upload either fails against an HTTPS-only listener or sends the token and the whole build context in cleartext. Direct-node and bundled-gateway deployments can keep the default. + ## E2B CLI AgentENV is compatible with the E2B CLI, but we recommend using the diff --git a/src/api/build_files.rs b/src/api/build_files.rs new file mode 100644 index 00000000..d911fbe5 --- /dev/null +++ b/src/api/build_files.rs @@ -0,0 +1,287 @@ +//! Hand-written upload endpoint for template build-context archives. +//! +//! `GET /templates/{templateID}/files/{hash}` (generated API) hands the E2B +//! SDK a bearer URL pointing here; the SDK then `PUT`s a tar archive with no +//! authentication headers. The durable random token embedded in the URL is +//! therefore the credential, and this route stays outside the generated +//! router so the archive can stream to disk instead of buffering in memory. + +use std::time::Duration; + +use axum::extract::{Path, Request, State}; +use axum::http::StatusCode; +use axum::response::{IntoResponse, Response}; +use axum::routing::put; +use axum::{Json, Router}; +use futures::StreamExt; +use tokio::io::AsyncWriteExt; +use tracing::{debug, warn}; + +use agentenv_http_server::models; + +use super::ApiImpl; +use crate::cfg::ConfigManager; +use crate::snapshot::repository::build_files::is_valid_build_files_hash; + +pub(crate) fn router(api_impl: I) -> Router +where + I: AsRef + Clone + Send + Sync + 'static, +{ + Router::new() + .route( + "/templates/{template_id}/files/{hash}/content", + put(upload_build_archive::), + ) + .with_state(api_impl) +} + +struct UploadQuery { + expires: i64, + token: String, +} + +fn parse_upload_query(query: Option<&str>) -> Option { + let query = query?; + let mut expires: Option = None; + let mut token: Option = None; + for (key, value) in url::form_urlencoded::parse(query.as_bytes()) { + match key.as_ref() { + "expires" => expires = value.parse().ok(), + "token" => token = Some(value.into_owned()), + _ => {} + } + } + Some(UploadQuery { + expires: expires?, + token: token?, + }) +} + +fn error_response(code: StatusCode, message: impl Into) -> Response { + ( + code, + Json(models::Error::new(code.as_u16() as i32, message.into())), + ) + .into_response() +} + +async fn upload_build_archive( + State(api_impl): State, + Path((template_id, hash)): Path<(String, String)>, + request: Request, +) -> Response +where + I: AsRef + Clone + Send + Sync + 'static, +{ + let api: &ApiImpl = api_impl.as_ref(); + + if !is_valid_build_files_hash(&hash) { + return error_response( + StatusCode::BAD_REQUEST, + format!("invalid build files hash '{hash}'"), + ); + } + let Some(store) = api.snapshot_manager().template_build_files() else { + return error_response( + StatusCode::BAD_REQUEST, + "the configured snapshot backend does not support build-context uploads", + ); + }; + let Some(query) = parse_upload_query(request.uri().query()) else { + return error_response( + StatusCode::UNAUTHORIZED, + "upload URL is missing the expires/token query parameters", + ); + }; + + // Verification does not consume the grant: consumption happens only after + // the archive has been durably published, so an upload that fails while + // streaming, staging, or storing the body stays retryable with this URL. + let now_unix = chrono::Utc::now().timestamp(); + let authorized = match store + .verify_upload_grant(&query.token, &template_id, &hash, query.expires, now_unix) + .await + { + Ok(authorized) => authorized, + Err(error) => { + warn!(error = %error, "failed to verify build-file upload grant"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to validate upload grant", + ); + } + }; + if !authorized { + return error_response( + StatusCode::UNAUTHORIZED, + "upload grant is invalid, expired, or already used; request a fresh upload link", + ); + } + + let max_bytes = ConfigManager::global_config() + .template_build + .files_max_upload_mib + .saturating_mul(1024 * 1024); + let upload_timeout = Duration::from_secs( + ConfigManager::global_config() + .template_build + .files_upload_timeout_secs, + ); + + // `staged` is the drop guard that removes the staging file on every early + // return below, so it must stay bound for the rest of the handler. + let staged = match tokio::task::spawn_blocking(tempfile::NamedTempFile::new).await { + Ok(Ok(staged)) => staged, + Ok(Err(error)) => { + warn!(error = %error, "failed to create staging file for build archive"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to stage build archive", + ); + } + Err(error) => { + warn!(error = %error, "failed to join staging file creation for build archive"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to stage build archive", + ); + } + }; + let staged_path = staged.path().to_path_buf(); + + let mut file = match tokio::fs::File::create(&staged_path).await { + Ok(file) => file, + Err(error) => { + warn!(error = %error, "failed to open staging file for build archive"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to stage build archive", + ); + } + }; + + let consume_body = async { + let mut total: u64 = 0; + let mut stream = request.into_body().into_data_stream(); + while let Some(chunk) = stream.next().await { + let chunk = match chunk { + Ok(chunk) => chunk, + Err(error) => { + debug!(error = %error, "build archive upload stream aborted"); + return Err(error_response( + StatusCode::BAD_REQUEST, + "failed to read the uploaded archive body", + )); + } + }; + total += chunk.len() as u64; + if total > max_bytes { + return Err(error_response( + StatusCode::PAYLOAD_TOO_LARGE, + format!("build archive exceeds the configured limit of {max_bytes} bytes"), + )); + } + if let Err(error) = file.write_all(&chunk).await { + warn!(error = %error, "failed to write staged build archive"); + return Err(error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to stage build archive", + )); + } + } + if let Err(error) = file.flush().await { + warn!(error = %error, "failed to flush staged build archive"); + return Err(error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to stage build archive", + )); + } + Ok(total) + }; + + let total = match tokio::time::timeout(upload_timeout, consume_body).await { + Ok(Ok(total)) => total, + Ok(Err(response)) => return response, + Err(_) => { + debug!(template_id, hash, "build archive upload timed out"); + return error_response( + StatusCode::REQUEST_TIMEOUT, + format!( + "build archive upload did not complete within {} seconds", + upload_timeout.as_secs() + ), + ); + } + }; + drop(file); + + // Publishing before the grant is consumed keeps a failed store retryable + // with the same URL. An unclaimed replay reaching this point is harmless: + // the token authorizes exactly this template_id/hash and `import` is + // first-write-wins, so it can neither publish a different key nor change + // what is already stored. + // + // `hash` is the cache key the SDK computed for this build context, not a + // digest of the received bytes that the server verified. + if let Err(error) = store.import(&hash, &staged_path).await { + warn!(error = %error, hash, "failed to import build archive"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to store build archive; the upload can be retried with the same link", + ); + } + + // The archive is published, so the claim only enforces single-use: the + // atomic remove/delete picks a single winner among concurrent replays, and + // a replay that loses the race is rejected even though the archive it + // uploaded is stored. `now_unix` is the timestamp taken before the body was + // read, so a slow but authorized upload is not rejected for aging past the + // TTL. + let claimed = match store + .claim_upload_grant(&query.token, &template_id, &hash, query.expires, now_unix) + .await + { + Ok(claimed) => claimed, + Err(error) => { + warn!(error = %error, "failed to claim build-file upload grant"); + return error_response( + StatusCode::INTERNAL_SERVER_ERROR, + "failed to validate upload grant", + ); + } + }; + if !claimed { + return error_response( + StatusCode::UNAUTHORIZED, + "upload grant is invalid, expired, or already used; request a fresh upload link", + ); + } + + debug!( + template_id, + hash, + bytes = total, + "stored build-context archive" + ); + StatusCode::OK.into_response() +} + +#[cfg(test)] +mod tests { + use super::parse_upload_query; + + #[test] + fn upload_query_parses_bearer_token_and_expiry() { + let query = parse_upload_query(Some("expires=1234&token=upload-token")) + .expect("query should parse"); + assert_eq!(query.expires, 1234); + assert_eq!(query.token, "upload-token"); + } + + #[test] + fn upload_query_requires_both_fields() { + assert!(parse_upload_query(Some("expires=1234")).is_none()); + assert!(parse_upload_query(Some("token=upload-token")).is_none()); + assert!(parse_upload_query(None).is_none()); + } +} diff --git a/src/api/generated/src/apis/templates.rs b/src/api/generated/src/apis/templates.rs index a25ac144..fc727885 100644 --- a/src/api/generated/src/apis/templates.rs +++ b/src/api/generated/src/apis/templates.rs @@ -64,6 +64,22 @@ pub enum TemplatesTemplateIdDeleteResponse { Status500_ServerError(models::Error), } +#[derive(Debug, PartialEq, Serialize, Deserialize)] +#[must_use] +#[allow(clippy::large_enum_variant)] +pub enum TemplatesTemplateIdFilesHashGetResponse { + /// Successfully returned the upload link + Status201_SuccessfullyReturnedTheUploadLink(models::TemplateBuildFileUpload), + /// Bad request + Status400_BadRequest(models::Error), + /// Authentication error + Status401_AuthenticationError(models::Error), + /// Not found + Status404_NotFound(models::Error), + /// Server error + Status500_ServerError(models::Error), +} + #[derive(Debug, PartialEq, Serialize, Deserialize)] #[must_use] #[allow(clippy::large_enum_variant)] @@ -187,6 +203,19 @@ pub trait Templates: path_params: &models::TemplatesTemplateIdDeletePathParams, ) -> Result; + /// Template build file upload link. + /// + /// TemplatesTemplateIdFilesHashGet - GET /templates/{templateID}/files/{hash} + async fn templates_template_id_files_hash_get( + &self, + + method: &Method, + host: &Host, + cookies: &CookieJar, + claims: &Self::Claims, + path_params: &models::TemplatesTemplateIdFilesHashGetPathParams, + ) -> Result; + /// List template builds. /// /// TemplatesTemplateIdGet - GET /templates/{templateID} diff --git a/src/api/generated/src/models.rs b/src/api/generated/src/models.rs index 8d871403..134bd3e5 100644 --- a/src/api/generated/src/models.rs +++ b/src/api/generated/src/models.rs @@ -249,6 +249,13 @@ pub struct TemplatesTemplateIdDeletePathParams { pub template_id: String, } +#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize, validator::Validate)] +#[cfg_attr(feature = "conversion", derive(frunk::LabelledGeneric))] +pub struct TemplatesTemplateIdFilesHashGetPathParams { + pub template_id: String, + pub hash: String, +} + #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize, validator::Validate)] #[cfg_attr(feature = "conversion", derive(frunk::LabelledGeneric))] pub struct TemplatesTemplateIdGetPathParams { @@ -7649,6 +7656,157 @@ impl std::convert::TryFrom for header::IntoHeaderValue, +} + +impl TemplateBuildFileUpload { + #[allow(clippy::new_without_default, clippy::too_many_arguments)] + pub fn new(present: bool) -> TemplateBuildFileUpload { + TemplateBuildFileUpload { present, url: None } + } +} + +/// Converts the TemplateBuildFileUpload value to the Query Parameters representation (style=form, explode=false) +/// specified in https://swagger.io/docs/specification/serialization/ +/// Should be implemented in a serde serializer +impl std::fmt::Display for TemplateBuildFileUpload { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let params: Vec> = vec![ + Some("present".to_string()), + Some(self.present.to_string()), + self.url + .as_ref() + .map(|url| ["url".to_string(), url.to_string()].join(",")), + ]; + + write!( + f, + "{}", + params.into_iter().flatten().collect::>().join(",") + ) + } +} + +/// Converts Query Parameters representation (style=form, explode=false) to a TemplateBuildFileUpload value +/// as specified in https://swagger.io/docs/specification/serialization/ +/// Should be implemented in a serde deserializer +impl std::str::FromStr for TemplateBuildFileUpload { + type Err = String; + + fn from_str(s: &str) -> std::result::Result { + /// An intermediate representation of the struct to use for parsing. + #[derive(Default)] + #[allow(dead_code)] + struct IntermediateRep { + pub present: Vec, + pub url: Vec, + } + + let mut intermediate_rep = IntermediateRep::default(); + + // Parse into intermediate representation + let mut string_iter = s.split(','); + let mut key_result = string_iter.next(); + + while key_result.is_some() { + let val = match string_iter.next() { + Some(x) => x, + None => { + return std::result::Result::Err( + "Missing value while parsing TemplateBuildFileUpload".to_string(), + ); + } + }; + + if let Some(key) = key_result { + #[allow(clippy::match_single_binding)] + match key { + #[allow(clippy::redundant_clone)] + "present" => intermediate_rep.present.push( + ::from_str(val).map_err(|x| x.to_string())?, + ), + #[allow(clippy::redundant_clone)] + "url" => intermediate_rep.url.push( + ::from_str(val).map_err(|x| x.to_string())?, + ), + _ => { + return std::result::Result::Err( + "Unexpected key while parsing TemplateBuildFileUpload".to_string(), + ); + } + } + } + + // Get the next key + key_result = string_iter.next(); + } + + // Use the intermediate representation to return the struct + std::result::Result::Ok(TemplateBuildFileUpload { + present: intermediate_rep + .present + .into_iter() + .next() + .ok_or_else(|| "present missing in TemplateBuildFileUpload".to_string())?, + url: intermediate_rep.url.into_iter().next(), + }) + } +} + +// Methods for converting between header::IntoHeaderValue and HeaderValue + +#[cfg(feature = "server")] +impl std::convert::TryFrom> for HeaderValue { + type Error = String; + + fn try_from( + hdr_value: header::IntoHeaderValue, + ) -> std::result::Result { + let hdr_value = hdr_value.to_string(); + match HeaderValue::from_str(&hdr_value) { + std::result::Result::Ok(value) => std::result::Result::Ok(value), + std::result::Result::Err(e) => std::result::Result::Err(format!( + r#"Invalid header value for TemplateBuildFileUpload - value: {hdr_value} is invalid {e}"# + )), + } + } +} + +#[cfg(feature = "server")] +impl std::convert::TryFrom for header::IntoHeaderValue { + type Error = String; + + fn try_from(hdr_value: HeaderValue) -> std::result::Result { + match hdr_value.to_str() { + std::result::Result::Ok(value) => { + match ::from_str(value) { + std::result::Result::Ok(value) => { + std::result::Result::Ok(header::IntoHeaderValue(value)) + } + std::result::Result::Err(err) => std::result::Result::Err(format!( + r#"Unable to convert header value '{value}' into TemplateBuildFileUpload - {err}"# + )), + } + } + std::result::Result::Err(e) => std::result::Result::Err(format!( + r#"Unable to convert header: {hdr_value:?} to string: {e}"# + )), + } + } +} + #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize, validator::Validate)] #[cfg_attr(feature = "conversion", derive(frunk::LabelledGeneric))] pub struct TemplateBuildInfo { diff --git a/src/api/generated/src/server/mod.rs b/src/api/generated/src/server/mod.rs index 766300a6..e5439fbf 100644 --- a/src/api/generated/src/server/mod.rs +++ b/src/api/generated/src/server/mod.rs @@ -111,6 +111,10 @@ where "/templates/{template_id}/builds/{build_id}/status", get(templates_template_id_builds_build_id_status_get::), ) + .route( + "/templates/{template_id}/files/{hash}", + get(templates_template_id_files_hash_get::), + ) .route("/v2/sandboxes", get(v2_sandboxes_get::)) .route("/v2/templates", get(v2_templates_get::)) .route( @@ -4160,6 +4164,174 @@ where }) } +#[tracing::instrument(skip_all)] +fn templates_template_id_files_hash_get_validation( + path_params: models::TemplatesTemplateIdFilesHashGetPathParams, +) -> std::result::Result<(models::TemplatesTemplateIdFilesHashGetPathParams,), ValidationErrors> { + path_params.validate()?; + + Ok((path_params,)) +} +/// TemplatesTemplateIdFilesHashGet - GET /templates/{templateID}/files/{hash} +#[tracing::instrument(skip_all)] +async fn templates_template_id_files_hash_get( + method: Method, + TypedHeader(host): TypedHeader, + cookies: CookieJar, + headers: HeaderMap, + Path(path_params): Path, + State(api_impl): State, +) -> Result +where + I: AsRef + Send + Sync, + A: apis::templates::Templates + + apis::ApiKeyAuthHeader + + apis::ApiAuthBasic + + Send + + Sync, + E: std::fmt::Debug + Send + Sync + 'static, +{ + // Authentication + let claims_in_header = api_impl + .as_ref() + .extract_claims_from_header(&headers, "X-Team-ID") + .await; + let claims_in_auth_header = api_impl + .as_ref() + .extract_claims_from_auth_header(apis::BasicAuthKind::Bearer, &headers, "authorization") + .await; + let claims = None.or(claims_in_header).or(claims_in_auth_header); + let Some(claims) = claims else { + return response_with_status_code_only(StatusCode::UNAUTHORIZED); + }; + + #[allow(clippy::redundant_closure)] + let validation = tokio::task::spawn_blocking(move || { + templates_template_id_files_hash_get_validation(path_params) + }) + .await + .unwrap(); + + let Ok((path_params,)) = validation else { + return Response::builder() + .status(StatusCode::BAD_REQUEST) + .body(Body::from(validation.unwrap_err().to_string())) + .map_err(|_| StatusCode::BAD_REQUEST); + }; + + let result = api_impl + .as_ref() + .templates_template_id_files_hash_get(&method, &host, &cookies, &claims, &path_params) + .await; + + let mut response = Response::builder(); + + let resp = match result { + Ok(rsp) => match rsp { + apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status201_SuccessfullyReturnedTheUploadLink + (body) + => { + let mut response = response.status(201); + { + let mut response_headers = response.headers_mut().unwrap(); + response_headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json")); + } + + let body_content = tokio::task::spawn_blocking(move || + serde_json::to_vec(&body).map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + })).await.unwrap()?; + response.body(Body::from(body_content)) + }, + apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status400_BadRequest + (body) + => { + let mut response = response.status(400); + { + let mut response_headers = response.headers_mut().unwrap(); + response_headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json")); + } + + let body_content = tokio::task::spawn_blocking(move || + serde_json::to_vec(&body).map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + })).await.unwrap()?; + response.body(Body::from(body_content)) + }, + apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status401_AuthenticationError + (body) + => { + let mut response = response.status(401); + { + let mut response_headers = response.headers_mut().unwrap(); + response_headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json")); + } + + let body_content = tokio::task::spawn_blocking(move || + serde_json::to_vec(&body).map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + })).await.unwrap()?; + response.body(Body::from(body_content)) + }, + apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status404_NotFound + (body) + => { + let mut response = response.status(404); + { + let mut response_headers = response.headers_mut().unwrap(); + response_headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json")); + } + + let body_content = tokio::task::spawn_blocking(move || + serde_json::to_vec(&body).map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + })).await.unwrap()?; + response.body(Body::from(body_content)) + }, + apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status500_ServerError + (body) + => { + let mut response = response.status(500); + { + let mut response_headers = response.headers_mut().unwrap(); + response_headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json")); + } + + let body_content = tokio::task::spawn_blocking(move || + serde_json::to_vec(&body).map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + })).await.unwrap()?; + response.body(Body::from(body_content)) + }, + }, + Err(why) => { + // Application code returned an error. This should not happen, as the implementation should + // return a valid response. + return api_impl.as_ref().handle_error(&method, &host, &cookies, why).await; + }, + }; + + resp.map_err(|e| { + error!(error = ?e); + StatusCode::INTERNAL_SERVER_ERROR + }) +} + #[tracing::instrument(skip_all)] fn templates_template_id_get_validation( path_params: models::TemplatesTemplateIdGetPathParams, diff --git a/src/api/impls/mod.rs b/src/api/impls/mod.rs index 1e1d50bc..454a9d29 100644 --- a/src/api/impls/mod.rs +++ b/src/api/impls/mod.rs @@ -59,6 +59,10 @@ impl ApiImpl { Arc::clone(&self.orchestrator) } + pub(crate) fn snapshot_manager(&self) -> &SnapshotManager { + &self.snapshot_manager + } + pub(crate) fn proxy_client(&self) -> &ProxyClient { &self.proxy_client } diff --git a/src/api/impls/template.rs b/src/api/impls/template.rs index bf88f6a1..3c8727b5 100644 --- a/src/api/impls/template.rs +++ b/src/api/impls/template.rs @@ -17,6 +17,7 @@ use super::template_helpers::{ }; use super::ApiImpl; use crate::image::ResolvedBlockImage; +use crate::snapshot::repository::build_files::is_valid_build_files_hash; use crate::snapshot::{ CommandContext, SnapshotAlias, SnapshotId, SnapshotListFilter, SnapshotRecord, SnapshotSource, TemplateBuildErrorReason, TemplateBuildStatus, @@ -319,6 +320,100 @@ impl Templates<()> for ApiImpl { } } + async fn templates_template_id_files_hash_get( + &self, + _method: &Method, + host: &Host, + _cookies: &CookieJar, + _claims: &Self::Claims, + path_params: &models::TemplatesTemplateIdFilesHashGetPathParams, + ) -> Result { + let template_id = &path_params.template_id; + let hash = &path_params.hash; + + if !is_valid_build_files_hash(hash) { + return Ok( + TemplatesTemplateIdFilesHashGetResponse::Status400_BadRequest(Self::error( + 400, + format!("invalid build files hash '{hash}'"), + )), + ); + } + let Some(store) = self.snapshot_manager.template_build_files() else { + return Ok( + TemplatesTemplateIdFilesHashGetResponse::Status400_BadRequest(Self::error( + 400, + "the configured snapshot backend does not support build-context uploads", + )), + ); + }; + + match self.snapshot_manager.get(template_id).await { + Ok(Some(_)) => {} + Ok(None) => { + return Ok(TemplatesTemplateIdFilesHashGetResponse::Status404_NotFound( + Self::error(404, format!("template {template_id} not found")), + )); + } + Err(err) => { + return Ok( + TemplatesTemplateIdFilesHashGetResponse::Status500_ServerError( + Self::snapshot_manager_error(&err), + ), + ); + } + } + + let present = match store.exists(hash).await { + Ok(present) => present, + Err(err) => { + return Ok( + TemplatesTemplateIdFilesHashGetResponse::Status500_ServerError(Self::error( + 500, + format!("failed to check build archive: {err}"), + )), + ); + } + }; + let config = &crate::cfg::ConfigManager::global_config().template_build; + let expires = chrono::Utc::now() + .timestamp() + .saturating_add(i64::try_from(config.files_url_ttl_secs).unwrap_or(i64::MAX)); + let token = match store.create_upload_grant(template_id, hash, expires).await { + Ok(token) => token, + Err(err) => { + return Ok( + TemplatesTemplateIdFilesHashGetResponse::Status500_ServerError(Self::error( + 500, + format!("failed to prepare upload link: {err}"), + )), + ); + } + }; + + // The SDK PUTs to this URL with a bare HTTP client (no auth headers), + // so the durable bearer token in the query string is the credential. + // Reusing the Host header keeps the URL valid across gateway and + // direct-node access. + let base = config + .public_base_url + .clone() + .unwrap_or_else(|| format!("http://{host}")); + let url = format!( + "{}/templates/{template_id}/files/{hash}/content?expires={expires}&token={token}", + base.trim_end_matches('/'), + ); + + Ok( + TemplatesTemplateIdFilesHashGetResponse::Status201_SuccessfullyReturnedTheUploadLink( + models::TemplateBuildFileUpload { + present, + url: Some(url), + }, + ), + ) + } + async fn templates_get( &self, _method: &Method, diff --git a/src/api/impls/template_helpers.rs b/src/api/impls/template_helpers.rs index 35bb32a7..d4a61ca6 100644 --- a/src/api/impls/template_helpers.rs +++ b/src/api/impls/template_helpers.rs @@ -1,6 +1,7 @@ use agentenv_http_server::models; use crate::cfg::ConfigManager; +use crate::snapshot::repository::build_files::is_valid_build_files_hash; use crate::snapshot::{SnapshotAlias, SnapshotId, SnapshotRecord}; use crate::template::TemplateBuildSpec; use crate::types::SandboxResources; @@ -124,23 +125,32 @@ pub(super) fn template_build_start_base_source( } } +/// Upper bound on a COPY/ADD source pattern. The pattern is matched against +/// every entry name of an uploaded archive, so its length directly bounds the +/// per-entry matching cost. +const MAX_COPY_SRC_BYTES: usize = 4096; + fn apply_e2b_template_step( mut spec: TemplateBuildSpec, step: &models::TemplateStep, ) -> Result { - if step + let args = step.args.as_deref().unwrap_or_default(); + let step_type = step.r_type.to_ascii_uppercase(); + let carries_files_hash = step .files_hash - .as_ref() - .is_some_and(|hash| !hash.trim().is_empty()) - { + .as_deref() + .map(str::trim) + .is_some_and(|hash| !hash.is_empty()); + if carries_files_hash && !matches!(step_type.as_str(), "COPY" | "ADD") { return Err(models::Error::new( 400, - "template build filesHash/COPY support is not implemented yet".to_string(), + format!( + "{} template step must not carry a filesHash; only COPY and ADD consume build context archives", + step.r_type + ), )); } - - let args = step.args.as_deref().unwrap_or_default(); - match step.r_type.to_ascii_uppercase().as_str() { + match step_type.as_str() { "RUN" => { let Some(cmd) = args.first().filter(|cmd| !cmd.trim().is_empty()) else { return Err(models::Error::new( @@ -222,11 +232,98 @@ fn apply_e2b_template_step( spec = spec.label(key.to_string(), pair[1].clone()); } } + // The E2B SDK resolves ADD like COPY client-side (local files only) + // and sends both with a filesHash referencing the uploaded archive. "COPY" | "ADD" => { - return Err(models::Error::new( - 400, - format!("{} template steps are not supported yet", step.r_type), - )); + let Some(files_hash) = step + .files_hash + .as_deref() + .map(str::trim) + .filter(|hash| !hash.is_empty()) + else { + return Err(models::Error::new( + 400, + format!( + "{} template steps require a filesHash referencing an uploaded build context archive", + step.r_type + ), + )); + }; + // The hash is an opaque cache key that becomes part of an archive + // path, so it must be shape-checked before the build starts. + if !is_valid_build_files_hash(files_hash) { + return Err(models::Error::new( + 400, + format!( + "{} template step filesHash '{files_hash}' is not a valid build files hash", + step.r_type + ), + )); + } + let src = args + .first() + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .ok_or_else(|| { + models::Error::new( + 400, + format!("{} template step requires a source argument", step.r_type), + ) + })?; + if src.len() > MAX_COPY_SRC_BYTES { + return Err(models::Error::new( + 400, + format!( + "{} template step source argument exceeds {MAX_COPY_SRC_BYTES} bytes", + step.r_type + ), + )); + } + let dest = args + .get(1) + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .ok_or_else(|| { + models::Error::new( + 400, + format!( + "{} template step requires a destination argument", + step.r_type + ), + ) + })?; + let user = args + .get(2) + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .map(str::to_string); + let mode = args + .get(3) + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .map(|value| { + let mode = u32::from_str_radix(value, 8).map_err(|_| { + models::Error::new( + 400, + format!( + "{} template step mode '{value}' is not a valid octal mode", + step.r_type + ), + ) + })?; + if mode > 0o7777 { + return Err(models::Error::new( + 400, + format!( + "{} template step mode '{value}' exceeds the maximum octal mode 7777", + step.r_type + ), + )); + } + Ok(mode) + }) + .transpose()?; + spec = spec.copy(src, dest, files_hash, user, mode); } other => { return Err(models::Error::new( @@ -241,9 +338,31 @@ fn apply_e2b_template_step( #[cfg(test)] mod tests { - use super::{template_build_start_base_source, TemplateBuildStartBaseSource}; + use super::{ + apply_e2b_template_step, template_build_start_base_source, TemplateBuildStartBaseSource, + MAX_COPY_SRC_BYTES, + }; + use crate::template::TemplateBuildSpec; use agentenv_http_server::models; + const HASH: &str = "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"; + + fn step(r_type: &str, args: &[&str]) -> models::TemplateStep { + let mut step = models::TemplateStep::new(r_type.to_string()); + step.args = Some(args.iter().map(|arg| (*arg).to_string()).collect()); + step + } + + fn copy_step(args: &[&str]) -> models::TemplateStep { + let mut step = step("COPY", args); + step.files_hash = Some(HASH.to_string()); + step + } + + fn apply(step: &models::TemplateStep) -> Result { + apply_e2b_template_step(TemplateBuildSpec::new(), step) + } + #[test] fn start_base_source_defaults_when_not_specified() { let body = models::TemplateBuildStartV2::new(); @@ -278,4 +397,57 @@ mod tests { ) ); } + + #[test] + fn step_type_is_matched_case_insensitively() { + apply(&step("workdir", &["/app"])).expect("lowercase step type should apply"); + } + + #[test] + fn files_hash_outside_copy_and_add_is_rejected() { + let mut run = step("RUN", &["echo hi"]); + run.files_hash = Some(HASH.to_string()); + + let err = apply(&run).expect_err("a RUN step must not carry a filesHash"); + assert_eq!(err.code, 400); + assert!(err.message.contains("filesHash"), "{}", err.message); + + // A blank filesHash stays acceptable: the SDK omits it as an empty + // string for non-COPY steps. + run.files_hash = Some(" ".to_string()); + apply(&run).expect("a blank filesHash should be ignored"); + } + + #[test] + fn copy_step_rejects_out_of_range_mode() { + apply(©_step(&["src", "/dest", "", "0755"])).expect("a valid mode should apply"); + + let err = apply(©_step(&["src", "/dest", "", "10000"])) + .expect_err("a mode above 7777 should be rejected"); + assert_eq!(err.code, 400); + assert!(err.message.contains("10000"), "{}", err.message); + } + + #[test] + fn copy_step_rejects_a_malformed_files_hash() { + let mut copy = copy_step(&["src", "/dest"]); + copy.files_hash = Some("../../etc/passwd".to_string()); + + let err = apply(©).expect_err("a malformed filesHash should be rejected"); + assert_eq!(err.code, 400); + assert!(err.message.contains("../../etc/passwd"), "{}", err.message); + } + + #[test] + fn copy_step_rejects_an_oversized_source_pattern() { + let oversized = "a".repeat(MAX_COPY_SRC_BYTES + 1); + let err = apply(©_step(&[oversized.as_str(), "/dest"])) + .expect_err("an oversized source should be rejected"); + assert_eq!(err.code, 400); + assert!( + err.message.contains(&MAX_COPY_SRC_BYTES.to_string()), + "{}", + err.message + ); + } } diff --git a/src/api/mod.rs b/src/api/mod.rs index 6cae7863..86bb400a 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -1,3 +1,4 @@ +mod build_files; mod impls; mod proxy; pub mod server; diff --git a/src/api/openapi.yml b/src/api/openapi.yml index f752ee8e..1f305eaa 100644 --- a/src/api/openapi.yml +++ b/src/api/openapi.yml @@ -898,6 +898,18 @@ components: type: boolean description: Whether the step should be forced to run regardless of the cache + TemplateBuildFileUpload: + description: Upload link for one build context archive, addressed by its files hash + required: + - present + properties: + present: + type: boolean + description: Whether the archive for this hash is already stored + url: + type: string + description: URL the client should PUT the tar archive to + TemplateBuildRequestV3: properties: name: @@ -2141,6 +2153,42 @@ paths: "500": $ref: "#/components/responses/500" + /templates/{templateID}/files/{hash}: + get: + summary: Template build file upload link + description: Get an upload link for a tar archive containing build context files for one COPY step + tags: [templates] + security: + - AccessTokenAuth: [] + - ApiKeyAuth: [] + - AuthProviderBearerAuth: [] + AuthProviderTeamAuth: [] + - AdminApiKeyAuth: [] + AdminTeamAuth: [] + parameters: + - $ref: "#/components/parameters/templateID" + - in: path + name: hash + required: true + schema: + type: string + description: Hash of the build context files + responses: + "201": + description: Successfully returned the upload link + content: + application/json: + schema: + $ref: "#/components/schemas/TemplateBuildFileUpload" + "400": + $ref: "#/components/responses/400" + "401": + $ref: "#/components/responses/401" + "404": + $ref: "#/components/responses/404" + "500": + $ref: "#/components/responses/500" + /templates/aliases/{alias}: get: summary: Check template alias diff --git a/src/api/proxy.rs b/src/api/proxy.rs index e9069d44..8472ee28 100644 --- a/src/api/proxy.rs +++ b/src/api/proxy.rs @@ -220,7 +220,18 @@ where I: AsRef + Send + Sync, { if !has_routing_header(request.headers()) { - return StatusCode::NOT_FOUND.into_response(); + // Unmatched control-plane route: return the API error envelope so + // JSON clients surface "route not found" instead of failing to parse + // an empty 404 body. + let error = agentenv_http_server::models::Error::new( + 404, + format!( + "route not found: {} {}", + request.method(), + request.uri().path() + ), + ); + return (StatusCode::NOT_FOUND, axum::Json(error)).into_response(); } let forward_path = request.uri().path().to_owned(); with_route_source( @@ -2013,6 +2024,24 @@ mod tests { .unwrap(); assert_eq!(response.status(), StatusCode::NOT_FOUND); + let content_type = response + .headers() + .get(axum::http::header::CONTENT_TYPE) + .and_then(|value| value.to_str().ok()) + .unwrap_or_default() + .to_owned(); + assert!( + content_type.starts_with("application/json"), + "unexpected content-type: {content_type}" + ); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let payload: Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(payload["code"], 404); + let message = payload["message"].as_str().unwrap(); + assert!( + message.contains("route not found: GET /nonexistent/path"), + "unexpected message: {message}" + ); } #[tokio::test] diff --git a/src/api/server.rs b/src/api/server.rs index 2a738898..a1a39016 100644 --- a/src/api/server.rs +++ b/src/api/server.rs @@ -1,6 +1,6 @@ use axum::{middleware, routing::get, Router}; -use super::{proxy, ApiImpl}; +use super::{build_files, proxy, ApiImpl}; use crate::observability::prometheus; use agentenv_http_server::apis; use agentenv_observability::metrics_handler; @@ -23,9 +23,10 @@ where { // Keep the generated control-plane API as the primary router, then merge in // the hand-written `/proxy/*` entrypoints needed for the temporary reverse - // proxy contract. + // proxy contract and the streaming build-context upload endpoint. agentenv_http_server::server::new::(api_impl.clone()) .merge(proxy::router(api_impl.clone())) + .merge(build_files::router(api_impl.clone())) .route("/metrics", get(metrics_handler)) .layer(middleware::from_fn_with_state( api_impl, diff --git a/src/cfg.rs b/src/cfg.rs index baf32196..533b107f 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -113,6 +113,8 @@ pub struct AppConfig { pub network: NetworkConfig, #[config(nested)] pub custom_extension: CustomExtensionConfig, + #[config(nested)] + pub template_build: TemplateBuildConfig, } #[derive(Debug, Deserialize, Clone, Config)] @@ -475,6 +477,43 @@ pub struct CustomExtensionConfig { pub timeout_ms: u64, } +/// Settings for the template build-context upload path used by the E2B SDK's +/// `COPY` support (`GET /templates/{templateID}/files/{hash}` plus the upload +/// URL it returns). +#[derive(Debug, Config, Clone)] +pub struct TemplateBuildConfig { + /// Maximum accepted size for one uploaded build-context archive, in MiB. + #[config(default = 1024u64)] + pub files_max_upload_mib: u64, + /// Maximum size one build-context archive may expand to once + /// decompressed, in MiB. This bounds what a compressed upload can cost + /// the node that runs the build. + #[config(default = 4096u64)] + pub files_max_context_mib: u64, + /// Cap on the combined on-disk size of all build-context archives one + /// build spec may reference, in MiB. + #[config(default = 4096u64)] + pub files_max_build_context_mib: u64, + /// How long an issued upload URL stays valid, in seconds. + #[config(default = 3600u64)] + pub files_url_ttl_secs: u64, + /// How long one build-context upload request may run before the server + /// gives up and responds 408, in seconds. + #[config(default = 300u64)] + pub files_upload_timeout_secs: u64, + /// Optional external base URL (e.g. "https://agentenv.example.com") used + /// when building upload URLs. When unset, upload URLs reuse the Host + /// header of the upload-link request with plain http, which matches + /// direct-node and bundled-gateway deployments. + /// + /// Any TLS-terminated, gateway-fronted, or multi-hop deployment MUST set + /// this to the external origin clients reach: the fallback derives the URL + /// from the request Host header with plain http, and that upload URL + /// carries a bearer token in its query string. + #[config(env = "AENV_TEMPLATE_BUILD_PUBLIC_BASE_URL", parse_env = parse_trimmed_string)] + pub public_base_url: Option, +} + #[derive(Debug, Config, Clone)] pub struct P2pConfig { #[config(default = false)] @@ -750,6 +789,17 @@ impl AppConfig { self.cluster.normalize(); self.sandbox_proxy.normalize()?; + // An env var exported empty means unset, matching the custom-extension + // URL handling; validation and the upload-URL builder then agree on + // the exact value in use. + self.template_build.public_base_url = self + .template_build + .public_base_url + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_owned); + Ok(()) } @@ -778,6 +828,70 @@ impl AppConfig { } self.validate_memory_snapshot_background_download()?; self.validate_overlaybd_global_config_paths()?; + self.validate_template_build()?; + Ok(()) + } + + /// Reject template build-context settings that would only fail later, at + /// upload-link time: a base URL that cannot produce a usable upload URL, + /// or a TTL that makes the `now + ttl` expiry arithmetic overflow or the + /// grant effectively unexpirable. + fn validate_template_build(&self) -> Result<()> { + // 7 days. Upload grants are single-use credentials in a query string, + // so a longer window is always a misconfiguration. + const MAX_URL_TTL_SECS: u64 = 604_800; + let cfg = &self.template_build; + if cfg.files_url_ttl_secs == 0 { + bail!("template_build.files_url_ttl_secs must be > 0"); + } + if cfg.files_url_ttl_secs > MAX_URL_TTL_SECS { + bail!( + "template_build.files_url_ttl_secs must be <= {MAX_URL_TTL_SECS} (got {})", + cfg.files_url_ttl_secs + ); + } + if cfg.files_upload_timeout_secs == 0 { + bail!("template_build.files_upload_timeout_secs must be > 0"); + } + // An upload slower than the grant TTL would stage the whole body and + // then lose the grant to expiry-based pruning at claim time. + if cfg.files_upload_timeout_secs > cfg.files_url_ttl_secs { + bail!( + "template_build.files_upload_timeout_secs ({}) must be <= \ + files_url_ttl_secs ({})", + cfg.files_upload_timeout_secs, + cfg.files_url_ttl_secs + ); + } + if cfg.files_max_upload_mib == 0 { + bail!("template_build.files_max_upload_mib must be > 0"); + } + if cfg.files_max_context_mib == 0 { + bail!("template_build.files_max_context_mib must be > 0"); + } + if cfg.files_max_build_context_mib == 0 { + bail!("template_build.files_max_build_context_mib must be > 0"); + } + if let Some(base_url) = cfg.public_base_url.as_deref() { + let parsed = url::Url::parse(base_url).with_context(|| { + format!( + "invalid template_build.public_base_url {base_url:?}: must be an absolute \ + http/https URL" + ) + })?; + if !matches!(parsed.scheme(), "http" | "https") { + bail!( + "invalid template_build.public_base_url {base_url:?}: scheme must be http or \ + https" + ); + } + if parsed.query().is_some() { + bail!("invalid template_build.public_base_url {base_url:?}: must have no query"); + } + if parsed.fragment().is_some() { + bail!("invalid template_build.public_base_url {base_url:?}: must have no fragment"); + } + } Ok(()) } @@ -1171,6 +1285,115 @@ mod tests { assert!(config.validate().is_err()); } + #[test] + fn template_build_defaults_pass_validation() { + let config = AppConfig::default(); + assert_eq!(config.template_build.files_url_ttl_secs, 3600); + assert_eq!(config.template_build.files_upload_timeout_secs, 300); + assert_eq!(config.template_build.files_max_build_context_mib, 4096); + assert!(config.template_build.public_base_url.is_none()); + config.validate().expect("default config passes"); + } + + #[test] + fn validate_accepts_absolute_template_build_public_base_url() { + let mut config = AppConfig::default(); + config.template_build.public_base_url = Some("https://agentenv.example.com".to_string()); + + config.validate().expect("https base url passes"); + } + + #[test] + fn validate_rejects_template_build_public_base_url_with_query() { + let mut config = AppConfig::default(); + config.template_build.public_base_url = + Some("https://agentenv.example.com/?token=abc".to_string()); + + let err = config.validate().unwrap_err(); + let message = err.to_string(); + assert!(message.contains("public_base_url"), "{message}"); + assert!(message.contains("must have no query"), "{message}"); + } + + #[test] + fn validate_rejects_template_build_public_base_url_without_scheme() { + let mut config = AppConfig::default(); + config.template_build.public_base_url = Some("agentenv.example.com".to_string()); + + let err = config.validate().unwrap_err(); + assert!( + err.to_string().contains("public_base_url"), + "unexpected error: {err}" + ); + } + + #[test] + fn validate_bounds_template_build_files_url_ttl() { + let mut config = AppConfig::default(); + config.template_build.files_url_ttl_secs = 0; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("template_build.files_url_ttl_secs must be > 0"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.template_build.files_url_ttl_secs = 604_801; + assert!(config.validate().is_err()); + + let mut config = AppConfig::default(); + config.template_build.files_url_ttl_secs = 604_800; + config.validate().expect("max ttl passes"); + } + + #[test] + fn validate_bounds_template_build_upload_timeout() { + let mut config = AppConfig::default(); + config.template_build.files_upload_timeout_secs = 0; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("template_build.files_upload_timeout_secs must be > 0"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.template_build.files_upload_timeout_secs = 3601; + let err = config.validate().unwrap_err(); + assert!( + err.to_string().contains("must be <= files_url_ttl_secs"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.template_build.files_max_upload_mib = 0; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("template_build.files_max_upload_mib must be > 0"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.template_build.files_max_context_mib = 0; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("template_build.files_max_context_mib must be > 0"), + "unexpected error: {err}" + ); + + let mut config = AppConfig::default(); + config.template_build.files_max_build_context_mib = 0; + let err = config.validate().unwrap_err(); + assert!( + err.to_string() + .contains("template_build.files_max_build_context_mib must be > 0"), + "unexpected error: {err}" + ); + } + #[test] fn overlaybd_converter_cache_version_includes_tool_version() { let config = AppConfig::default(); diff --git a/src/sandbox/backend.rs b/src/sandbox/backend.rs index ad04b094..8b67944d 100644 --- a/src/sandbox/backend.rs +++ b/src/sandbox/backend.rs @@ -366,4 +366,18 @@ pub trait SandboxExecutor: Send { ) -> Result { self.executor()?.start_process(cmd, args, opts).await } + + /// Upload a local file into the sandbox at `guest_path` as `username`. + /// + /// Streams through envd's files API, so the sandbox must be running. + async fn upload_file( + &self, + local_path: &std::path::Path, + guest_path: &str, + username: &str, + ) -> Result<()> { + self.executor()? + .upload_file(local_path, guest_path, username) + .await + } } diff --git a/src/sandbox/envd.rs b/src/sandbox/envd.rs index 36f8e43f..b33209cd 100644 --- a/src/sandbox/envd.rs +++ b/src/sandbox/envd.rs @@ -79,6 +79,41 @@ impl EnvdInstance { } } + /// Uploads a local file into the guest at `guest_path` via envd's files + /// API. `username` selects the guest account envd writes as; template + /// builds pass "root" because plain OCI images have no other account. + #[tracing::instrument(skip(self, local_path))] + pub(crate) async fn upload_file( + &self, + local_path: &std::path::Path, + guest_path: &str, + username: &str, + ) -> Result<()> { + use envd::http_client::apis::files_api; + + match files_api::files_post( + &self.config, + Some(guest_path), + Some(username), + None, + None, + Some(local_path.to_path_buf()), + ) + .await + { + Ok(_) => Ok(()), + // envd currently returns a successful text/plain response for + // this endpoint, while the generated client attempts to decode + // every 2xx response as JSON. The upload has completed by the + // time this response-body error is produced. + Err(envd::http_client::apis::Error::Serde(error)) => { + debug!(%error, "ignoring envd upload response-body decoding error"); + Ok(()) + } + Err(error) => Err(anyhow::Error::new(error).context("upload file to sandbox via envd")), + } + } + #[tracing::instrument(skip(self, env_vars))] pub(crate) async fn init( &self, diff --git a/src/sandbox/process.rs b/src/sandbox/process.rs index dad3a82d..972b7220 100644 --- a/src/sandbox/process.rs +++ b/src/sandbox/process.rs @@ -218,6 +218,18 @@ impl<'a> Executor<'a> { self.start_process_inner(cmd, args, opts, true).await } + /// Upload a local file into the sandbox at `guest_path` as `username`. + pub async fn upload_file( + &self, + local_path: &std::path::Path, + guest_path: &str, + username: &str, + ) -> Result<()> { + self.envd_instance + .upload_file(local_path, guest_path, username) + .await + } + /// Start a process via the envd gRPC client and return a [`ProcessHandle`]. async fn start_process_inner( &self, diff --git a/src/snapshot/manager.rs b/src/snapshot/manager.rs index 634bf25d..5167f354 100644 --- a/src/snapshot/manager.rs +++ b/src/snapshot/manager.rs @@ -84,6 +84,14 @@ impl SnapshotManager { self.repository.create(record).await } + /// Returns the shared build-context archive store, when the configured + /// repository backend provides one. + pub fn template_build_files( + &self, + ) -> Option> { + self.repository.template_build_files() + } + #[tracing::instrument(skip(self, metadata, manifest), fields(snapshot_id = %metadata.id))] pub async fn publish( &self, diff --git a/src/snapshot/repository/backends/oss/build_files.rs b/src/snapshot/repository/backends/oss/build_files.rs new file mode 100644 index 00000000..2ac034dd --- /dev/null +++ b/src/snapshot/repository/backends/oss/build_files.rs @@ -0,0 +1,169 @@ +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use async_trait::async_trait; + +use super::client::OssClient; +use crate::snapshot::repository::build_files::{ + generate_upload_token, is_valid_build_files_hash, is_valid_upload_token, + TemplateBuildFileStore, TemplateBuildUploadGrant, +}; +use crate::snapshot::repository::{RepositoryError, RepositoryResult}; + +const BUILD_FILES_PREFIX: &str = "template-build-files"; + +/// Build-context archive store backed by the OSS repository bucket. +/// +/// Layout: `template-build-files/{hash}.tar` plus durable bearer grants under +/// `template-build-files/upload-grants/`. Retention is delegated to bucket +/// lifecycle rules; archives are cache entries the SDK re-uploads when absent. +pub(crate) struct OssTemplateBuildFileStore { + client: Arc, +} + +impl OssTemplateBuildFileStore { + pub(crate) fn new(client: Arc) -> Arc { + Arc::new(Self { client }) + } + + fn archive_key(hash: &str) -> RepositoryResult { + if !is_valid_build_files_hash(hash) { + return Err(RepositoryError::InvalidRequest { + reason: format!("invalid build files hash '{hash}'"), + }); + } + Ok(format!("{BUILD_FILES_PREFIX}/{hash}.tar")) + } + + fn grant_key(token: &str) -> Option { + is_valid_upload_token(token) + .then(|| format!("{BUILD_FILES_PREFIX}/upload-grants/{token}.json")) + } + + /// Reads a grant record, mapping an absent object to `None`. + async fn read_grant(&self, key: &str) -> RepositoryResult> { + let bytes = match self.client.get_bytes(key).await { + Ok(bytes) => bytes, + Err(error) if OssClient::is_not_found_error(&error) => return Ok(None), + Err(error) => return Err(RepositoryError::backend("read upload grant", error)), + }; + serde_json::from_slice(&bytes) + .map(Some) + .map_err(|error| RepositoryError::backend("parse upload grant", error)) + } +} + +#[async_trait] +impl TemplateBuildFileStore for OssTemplateBuildFileStore { + async fn exists(&self, hash: &str) -> RepositoryResult { + let key = Self::archive_key(hash)?; + self.client + .exists(&key) + .await + .map_err(|error| RepositoryError::backend("check build archive", error)) + } + + async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()> { + let key = Self::archive_key(hash)?; + // Archives are immutable so a repeat upload cannot change what an + // in-flight build reads. This fast path is not atomic against a + // concurrent import: the loser's bytes are dropped, and since the hash + // is a caller-supplied cache key rather than a verified digest, which + // racing upload wins is undefined — first-write-wins stability, not + // content authenticity. + if self + .client + .exists(&key) + .await + .map_err(|error| RepositoryError::backend("check build archive", error))? + { + return Ok(()); + } + self.client + .put_file(&key, staged) + .await + .map_err(|error| RepositoryError::backend("upload build archive", error)) + } + + async fn materialize( + &self, + hash: &str, + scratch_dir: &Path, + ) -> RepositoryResult> { + let key = Self::archive_key(hash)?; + let dest = scratch_dir.join(format!("{hash}.tar")); + match self.client.get_to_file(&key, &dest).await { + Ok(_) => Ok(Some(dest)), + Err(error) if OssClient::is_not_found_error(&error) => Ok(None), + Err(error) => Err(RepositoryError::backend("download build archive", error)), + } + } + + async fn create_upload_grant( + &self, + template_id: &str, + hash: &str, + expires_unix: i64, + ) -> RepositoryResult { + let token = generate_upload_token(); + let key = Self::grant_key(&token).expect("generated token is valid"); + let grant = serde_json::to_vec(&TemplateBuildUploadGrant::new( + template_id, + hash, + expires_unix, + )) + .map_err(|error| RepositoryError::backend("serialize upload grant", error))?; + self.client + .put_bytes(&key, grant) + .await + .map_err(|error| RepositoryError::backend("write upload grant", error))?; + Ok(token) + } + + async fn verify_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult { + let Some(key) = Self::grant_key(token) else { + return Ok(false); + }; + // Deliberately does not delete the object: verification must leave the + // upload URL usable for a retry. + let Some(grant) = self.read_grant(&key).await? else { + return Ok(false); + }; + Ok(grant.authorizes(template_id, hash, expires_unix, now_unix)) + } + + async fn claim_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult { + let Some(key) = Self::grant_key(token) else { + return Ok(false); + }; + let Some(grant) = self.read_grant(&key).await? else { + return Ok(false); + }; + if !grant.authorizes(template_id, hash, expires_unix, now_unix) { + return Ok(false); + } + // Consume the grant so the upload URL cannot be replayed. S3-compatible + // stores offer no conditional delete, so simultaneous replays of one + // token can both observe the grant; archives are immutable, which is + // what keeps that from mattering. + self.client + .delete(&key) + .await + .map_err(|error| RepositoryError::backend("consume upload grant", error))?; + Ok(true) + } +} diff --git a/src/snapshot/repository/backends/oss/mod.rs b/src/snapshot/repository/backends/oss/mod.rs index c3adede3..1578424c 100644 --- a/src/snapshot/repository/backends/oss/mod.rs +++ b/src/snapshot/repository/backends/oss/mod.rs @@ -1,3 +1,4 @@ +mod build_files; mod client; mod config; mod layout; diff --git a/src/snapshot/repository/backends/oss/repository.rs b/src/snapshot/repository/backends/oss/repository.rs index 06f747ef..b01d187a 100644 --- a/src/snapshot/repository/backends/oss/repository.rs +++ b/src/snapshot/repository/backends/oss/repository.rs @@ -42,6 +42,7 @@ pub(crate) struct OssSnapshotRepository { client: Arc, snapshot_image_storage: SnapshotImageStoragePolicy, acr_exporter: AcrDiskImageExporter, + build_files: Arc, } const MAX_ALIAS_BIND_ATTEMPTS: usize = 5; @@ -51,10 +52,12 @@ impl OssSnapshotRepository { client: Arc, snapshot_image_storage: SnapshotImageStoragePolicy, ) -> Self { + let build_files = super::build_files::OssTemplateBuildFileStore::new(Arc::clone(&client)); Self { client, snapshot_image_storage, acr_exporter: AcrDiskImageExporter::new(), + build_files, } } @@ -148,6 +151,15 @@ fn fallback_to_object_storage_would_mix_sources( #[async_trait] impl SnapshotRepository for OssSnapshotRepository { + fn template_build_files( + &self, + ) -> Option> { + Some(Arc::clone(&self.build_files) + as Arc< + dyn crate::snapshot::repository::TemplateBuildFileStore, + >) + } + async fn create(&self, record: SnapshotRecord) -> RepositoryResult { if !matches!(record.source, SnapshotSource::Template { .. }) { return Err(RepositoryError::InvalidRequest { @@ -164,25 +176,28 @@ impl SnapshotRepository for OssSnapshotRepository { reason: format!("snapshot '{}' already exists", record.id), }); } + // When the alias already points at a live snapshot, leave the binding + // untouched so the existing template keeps resolving while the new + // build runs; a successful publish moves the alias to the new snapshot + // (E2B rebuild semantics). + let mut bind_on_create = true; if let Some(alias) = record.alias.as_ref() { if let Some(existing) = self.load_alias_target(alias.as_ref()).await? { if existing != record.id && self.snapshot_exists(&existing).await? { - return Err(RepositoryError::AliasConflict { - alias: alias.to_string(), - existing, - new_id: record.id.clone(), - }); + bind_on_create = false; } } } self.write_record(&record).await?; - if let Some(alias) = record.alias.as_ref() { - if let Err(error) = self.bind_alias(alias.as_ref(), &record.id).await { - let _ = self - .client - .delete(&OssSnapshotArtifactLayout::record_key(&record.id)) - .await; - return Err(error); + if bind_on_create { + if let Some(alias) = record.alias.as_ref() { + if let Err(error) = self.bind_alias(alias.as_ref(), &record.id, false).await { + let _ = self + .client + .delete(&OssSnapshotArtifactLayout::record_key(&record.id)) + .await; + return Err(error); + } } } Ok(record) @@ -287,26 +302,63 @@ impl SnapshotRepository for OssSnapshotRepository { disk_publications: disk_publications.clone(), }; - // 5. Bind alias (if present) with conflict detection. + // 5. Commit the record before moving the alias. This prevents an + // alias from ever resolving to a snapshot whose catalog record + // has not been published yet. The tradeoff is a crash window: + // dying after the record write but before `bind_alias` leaves a + // committed record whose `alias` field names an alias that still + // resolves to the previous snapshot, so readers of `record.alias` + // (listings) may observe the stale claim until the next rebind. + // Nothing reconciles that state automatically. + let previous_record = self.read_record(id).await?; + let previous_alias_target = if let Some(alias) = metadata.alias.as_ref() { + match self.load_alias_target(alias.as_ref()).await? { + Some(existing) if self.snapshot_exists(&existing).await? => Some(existing), + _ => None, + } + } else { + None + }; + let record = self + .write_committed_record( + metadata.id.clone(), + metadata.alias.clone(), + metadata.resources, + committed, + metadata.source.clone(), + ) + .await?; + + // 6. Move the alias only after the new record is readable. If the + // bind fails, restore the pending record and old alias. if let Some(ref alias) = metadata.alias { - if let Err(e) = self.bind_alias(alias.as_ref(), id).await { - // Best-effort rollback. Content-addressed managed layers are intentionally left - // in place; they are shared across snapshots and require separate GC. - if let Err(error) = self.client.delete_prefix(&layout.artifact_prefix()).await { - warn!(snapshot_id = %id, error = %error, "failed to roll back snapshot artifacts after alias bind failure"); + if let Err(error) = self.bind_alias(alias.as_ref(), id, true).await { + self.restore_alias_after_failed_bind( + alias.as_ref(), + id, + previous_alias_target.as_ref(), + ) + .await; + self.restore_record_after_failed_publish(id, previous_record.as_ref()) + .await; + return Err(error); + } + if let Some(previous_id) = previous_alias_target + .as_ref() + .filter(|previous_id| *previous_id != id) + { + if let Err(error) = self.clear_record_alias(previous_id, alias.as_ref()).await { + warn!( + alias = %alias, + previous_snapshot_id = %previous_id, + error = %error, + "failed to clear previous snapshot alias metadata" + ); } - return Err(e); } } - self.write_committed_record( - metadata.id.clone(), - metadata.alias.clone(), - metadata.resources, - committed, - metadata.source.clone(), - ) - .await + Ok(record) } .await; @@ -584,7 +636,8 @@ impl OssSnapshotRepository { /// Instead the algorithm is: /// 1. Read the current alias target. /// 2. If it already points to `id`, return success (idempotent). - /// 3. If it points to a live snapshot, return `AliasConflict`. + /// 3. If it points to a live snapshot: with `rebind` move the alias to + /// `id` (E2B rebuild semantics), otherwise return `AliasConflict`. /// 4. If it points to a deleted snapshot, remove the stale alias. /// 5. Write our binding unconditionally. /// 6. Read back and verify we won the race. If someone else wrote a @@ -595,7 +648,7 @@ impl OssSnapshotRepository { /// interval between our write and the subsequent read. This is weaker /// than a true CAS but sufficient for the current deployment model /// where concurrent publishes for the *same alias* are rare. - async fn bind_alias(&self, alias: &str, id: &SnapshotId) -> RepositoryResult<()> { + async fn bind_alias(&self, alias: &str, id: &SnapshotId, rebind: bool) -> RepositoryResult<()> { let key = validated_alias_key(alias)?; let payload = serde_json::to_vec(id) .map_err(|e| RepositoryError::backend("serialize alias binding", e))?; @@ -608,18 +661,19 @@ impl OssSnapshotRepository { } let still_exists = self.snapshot_exists(&existing_id).await?; - if still_exists { + if still_exists && !rebind { return Err(RepositoryError::AliasConflict { alias: alias.to_string(), existing: existing_id, new_id: id.clone(), }); } - - self.client - .delete(&key) - .await - .map_err(|e| RepositoryError::backend("delete stale alias", e))?; + if !still_exists { + self.client + .delete(&key) + .await + .map_err(|e| RepositoryError::backend("delete stale alias", e))?; + } } // Step 5: write our binding (unconditional — OSS does not @@ -669,6 +723,95 @@ impl OssSnapshotRepository { }) } + async fn restore_record_after_failed_publish( + &self, + id: &SnapshotId, + previous_record: Option<&SnapshotRecord>, + ) { + let result = match previous_record { + Some(record) => self.write_record(record).await, + None => self + .client + .delete(&OssSnapshotArtifactLayout::record_key(id)) + .await + .map_err(|error| RepositoryError::backend("remove failed snapshot record", error)), + }; + if let Err(error) = result { + warn!(snapshot_id = %id, error = %error, "failed to restore snapshot record after publish failure"); + } + } + + async fn restore_alias_after_failed_bind( + &self, + alias: &str, + id: &SnapshotId, + previous_id: Option<&SnapshotId>, + ) { + let current = match self.load_alias_target(alias).await { + Ok(current) => current, + Err(error) => { + warn!(alias, snapshot_id = %id, error = %error, "failed to inspect alias during publish rollback"); + return; + } + }; + // Skip when a concurrent publisher already moved the alias elsewhere. + // This only narrows the lost-update window: like `bind_alias`, the + // rollback cannot be atomic on a store without conditional writes, so a + // publisher that rebinds between this read and the write below is still + // clobbered. + if current.as_ref() != Some(id) { + return; + } + + let key = match validated_alias_key(alias) { + Ok(key) => key, + Err(error) => { + warn!(alias, snapshot_id = %id, error = %error, "failed to validate alias during publish rollback"); + return; + } + }; + let result = + match previous_id { + Some(previous_id) => match serde_json::to_vec(previous_id) { + Ok(payload) => { + self.client.put_bytes(&key, payload).await.map_err(|error| { + RepositoryError::backend("restore alias binding", error) + }) + } + Err(error) => Err(RepositoryError::backend( + "serialize restored alias binding", + error, + )), + }, + None => self.client.delete(&key).await.map_err(|error| { + RepositoryError::backend("remove failed alias binding", error) + }), + }; + if let Err(error) = result { + warn!(alias, snapshot_id = %id, error = %error, "failed to restore alias after publish failure"); + } + } + + /// Clears the alias field on the record that previously owned a rebound + /// alias so template listings do not report the moved name twice. + /// + /// Only `moved_alias` is cleared; a previous owner that already claims a + /// different name keeps it. + async fn clear_record_alias(&self, id: &SnapshotId, moved_alias: &str) -> RepositoryResult<()> { + if let Some(mut previous) = self.read_record(id).await? { + let claims_moved_alias = previous + .alias + .as_ref() + .is_some_and(|alias| alias.as_ref() == moved_alias); + if claims_moved_alias { + previous.alias = None; + previous.updated_at_unix_ms = now_unix_ms(); + self.write_record(&previous).await?; + } + } + Ok(()) + } + async fn export_managed_disk_image( &self, image_config_path: &Path, diff --git a/src/snapshot/repository/backends/posixfs/backend.rs b/src/snapshot/repository/backends/posixfs/backend.rs index 4dc7f7ba..713a4961 100644 --- a/src/snapshot/repository/backends/posixfs/backend.rs +++ b/src/snapshot/repository/backends/posixfs/backend.rs @@ -7,11 +7,13 @@ use tokio::task; use super::super::shared_runtime_cache_root; use super::artifacts::{CollectedBuiltArtifacts, PosixFsArtifactStore}; +use super::build_files::PosixFsTemplateBuildFileStore; use super::catalog::PosixFsCatalogStore; use super::runtime::PosixFsRuntimeResolver; use crate::image::cache::{local_image_services_from_global_config, OverlaybdLayerStore}; use crate::sandbox::FirecrackerSnapshotManifest; use crate::snapshot::artifact_cache::LocalArtifactCache; +use crate::snapshot::repository::build_files::TemplateBuildFileStore; use crate::snapshot::repository::interfaces::{SnapshotRepository, SnapshotRuntimeResolver}; use crate::snapshot::repository::{RepositoryError, RepositoryResult, SnapshotListFilter}; use crate::snapshot::types::{ @@ -72,9 +74,11 @@ impl PosixFsBackend { let runtime_cache_root = runtime_cache_root.unwrap_or_else(|| cache_root.join("runtime")); let catalog_store = Arc::new(PosixFsCatalogStore::new(root.clone())); let artifact_store = Arc::new(PosixFsArtifactStore::new(root.clone())); + let build_files = PosixFsTemplateBuildFileStore::new(&root); let repository: Arc = Arc::new(PosixFsSnapshotRepository::new( catalog_store, artifact_store, + build_files, )); let runtime_resolver: Arc = Arc::new( PosixFsRuntimeResolver::new(root, runtime_cache_root, store, cache), @@ -111,16 +115,19 @@ impl PosixFsBackend { pub(crate) struct PosixFsSnapshotRepository { catalog_store: Arc, artifact_store: Arc, + build_files: Arc, } impl PosixFsSnapshotRepository { pub(crate) fn new( catalog_store: Arc, artifact_store: Arc, + build_files: Arc, ) -> Self { Self { catalog_store, artifact_store, + build_files, } } @@ -237,6 +244,10 @@ impl SnapshotRepository for PosixFsSnapshotRepository { .await } + fn template_build_files(&self) -> Option> { + Some(Arc::clone(&self.build_files) as Arc) + } + async fn publish( &self, metadata: SnapshotPublishMetadata, @@ -399,6 +410,7 @@ mod tests { PosixFsSnapshotRepository::new( Arc::new(PosixFsCatalogStore::new(root.to_path_buf())), Arc::new(PosixFsArtifactStore::new(root.to_path_buf())), + super::super::build_files::PosixFsTemplateBuildFileStore::new(root), ) } @@ -474,37 +486,162 @@ mod tests { } #[tokio::test] - async fn failed_commit_cleans_uncommitted_snapshot_directory() { + async fn publish_rebinds_existing_alias_to_new_snapshot() { let tempdir = TempDir::new().expect("tempdir should exist"); - let repository_root = tempdir.path().to_path_buf(); let repository = test_backend(tempdir.path()).repository(); let first_id = SnapshotId::generate(); let local_artifacts = seed_built_snapshot(tempdir.path()); - let first_metadata = sample_metadata(first_id.clone(), Some("conflict")); repository - .publish(first_metadata, local_artifacts) + .publish( + sample_metadata(first_id.clone(), Some("rebind")), + local_artifacts, + ) .await .expect("first publish should work"); let second_id = SnapshotId::generate(); let local_artifacts = seed_built_snapshot(tempdir.path()); - let err = repository + repository .publish( - sample_metadata(second_id.clone(), Some("conflict")), + sample_metadata(second_id.clone(), Some("rebind")), local_artifacts, ) .await - .expect_err("second publish should fail"); + .expect("second publish should rebind the alias"); + + let resolved = repository + .resolve_alias("rebind") + .await + .expect("resolve should work") + .expect("alias should resolve"); + assert_eq!(resolved, second_id, "alias should move to the new snapshot"); + + let previous = repository + .get(first_id.to_string().as_str()) + .await + .expect("get should work") + .expect("previous snapshot should stay addressable by id"); + assert_eq!( + previous.alias, None, + "previous snapshot should lose the rebound alias" + ); + } + + #[tokio::test] + async fn failed_publish_keeps_previous_alias_and_removes_snapshot_dir() { + let tempdir = TempDir::new().expect("tempdir should exist"); + let repository = test_backend(tempdir.path()).repository(); + + let first_id = SnapshotId::generate(); + let local_artifacts = seed_built_snapshot(tempdir.path()); + repository + .publish( + sample_metadata(first_id.clone(), Some("rebind")), + local_artifacts, + ) + .await + .expect("first publish should work"); + + let second_id = SnapshotId::generate(); + let broken_artifacts = seed_built_snapshot(tempdir.path()); + // `import_built_artifacts` copies `vm_state.bin` first, so removing it + // fails the publish before any catalog state is committed. + fs::remove_file(&broken_artifacts.vm_state.path).expect("remove seeded vm state"); + repository + .publish( + sample_metadata(second_id.clone(), Some("rebind")), + broken_artifacts, + ) + .await + .expect_err("publish should fail when the vm state artifact is missing"); - assert!(matches!(err, RepositoryError::AliasConflict { .. })); assert!( - !repository_root + !tempdir + .path() .join("snapshots") .join(second_id.to_string()) .exists(), - "failed publish should not leave a committed revision directory" + "failed publish should not leave a snapshot directory behind" ); + + let resolved = repository + .resolve_alias("rebind") + .await + .expect("resolve should work") + .expect("alias should still resolve"); + assert_eq!( + resolved, first_id, + "alias should stay bound to the previously committed snapshot" + ); + + let previous = repository + .get(first_id.to_string().as_str()) + .await + .expect("get should work") + .expect("previous snapshot should stay addressable by id"); + assert_eq!( + previous.alias.as_ref().map(ToString::to_string), + Some("rebind".to_string()), + "previous snapshot should keep the alias after a failed rebind" + ); + } + + #[tokio::test] + async fn create_keeps_existing_alias_until_new_build_commits() { + let tempdir = TempDir::new().expect("tempdir should exist"); + let repository = test_backend(tempdir.path()).repository(); + + let committed_id = SnapshotId::generate(); + let local_artifacts = seed_built_snapshot(tempdir.path()); + repository + .publish( + sample_metadata(committed_id.clone(), Some("stable")), + local_artifacts, + ) + .await + .expect("publish should work"); + + let waiting = SnapshotRecord::template_waiting( + SnapshotId::generate(), + Some(SnapshotAlias::parse("stable").expect("alias should parse")), + crate::types::SandboxResources { + cpu_count: 1, + memory_mib: 256, + disk_size_mib: 0, + }, + ); + let waiting_id = waiting.id.clone(); + repository + .create(waiting) + .await + .expect("create with an existing alias should be allowed"); + + let resolved = repository + .resolve_alias("stable") + .await + .expect("resolve should work") + .expect("alias should resolve"); + assert_eq!( + resolved, committed_id, + "alias should keep pointing at the committed snapshot while the rebuild is pending" + ); + + let local_artifacts = seed_built_snapshot(tempdir.path()); + repository + .publish( + sample_metadata(waiting_id.clone(), Some("stable")), + local_artifacts, + ) + .await + .expect("publishing the rebuild should rebind the alias"); + + let resolved = repository + .resolve_alias("stable") + .await + .expect("resolve should work") + .expect("alias should resolve"); + assert_eq!(resolved, waiting_id, "alias should move after commit"); } #[tokio::test] diff --git a/src/snapshot/repository/backends/posixfs/build_files.rs b/src/snapshot/repository/backends/posixfs/build_files.rs new file mode 100644 index 00000000..c486ee8b --- /dev/null +++ b/src/snapshot/repository/backends/posixfs/build_files.rs @@ -0,0 +1,726 @@ +use std::fs; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; + +use async_trait::async_trait; +use tokio::task; +use tracing::{debug, warn}; + +use crate::snapshot::repository::build_files::{ + generate_upload_token, is_valid_build_files_hash, is_valid_upload_token, + TemplateBuildFileStore, TemplateBuildUploadGrant, +}; +use crate::snapshot::repository::{RepositoryError, RepositoryResult}; + +/// How long imported build-context archives and upload grants are retained. +/// Archives are cache entries keyed by content hash; the SDK re-uploads any +/// archive that has been pruned, so expiry only costs one extra upload. +/// Grants expire after `template_build.files_url_ttl_secs` anyway, so this +/// only bounds how long the spent grant files linger on disk. +const BUILD_FILE_RETENTION: Duration = Duration::from_secs(7 * 24 * 60 * 60); + +const GRANTS_DIR_NAME: &str = "upload-grants"; + +/// Build-context archive store rooted on the shared POSIX repository. +/// +/// Layout: `{repository_root}/template-build-files/{hash}.tar` plus durable +/// upload grants under `upload-grants/`. Both live on the shared filesystem, +/// so every node observes the same archives and verifies the same upload URLs. +pub(crate) struct PosixFsTemplateBuildFileStore { + root: PathBuf, +} + +impl PosixFsTemplateBuildFileStore { + pub(crate) fn new(repository_root: &Path) -> Arc { + Arc::new(Self { + root: repository_root.join("template-build-files"), + }) + } + + fn archive_path(&self, hash: &str) -> RepositoryResult { + if !is_valid_build_files_hash(hash) { + return Err(RepositoryError::InvalidRequest { + reason: format!("invalid build files hash '{hash}'"), + }); + } + Ok(self.root.join(format!("{hash}.tar"))) + } + + fn ensure_root(root: &Path) -> RepositoryResult<()> { + fs::create_dir_all(root).map_err(|error| { + RepositoryError::backend( + format!("create template build files dir '{}'", root.display()), + error, + ) + }) + } + + /// Removes archives whose modification time is older than the retention + /// window. Runs opportunistically on import and scans a bounded number of + /// entries per call; failures only log. + fn prune_expired(root: &Path) { + let cutoff = SystemTime::now() - BUILD_FILE_RETENTION; + Self::prune_dir_older_than(root, "tar", cutoff); + } + + /// Removes upload grants that have passed their own `expires_unix`. Runs + /// opportunistically whenever a new grant is written, so the grants + /// directory stays bounded by upload-link traffic; the scan is bounded per + /// call and drains the backlog over successive requests, and failures only + /// log. + /// + /// Pruning by the record rather than by mtime keeps grants alive for + /// exactly their TTL even when `template_build.files_url_ttl_secs` is + /// configured beyond the retention window. + fn prune_expired_grants(root: &Path) { + let cutoff = SystemTime::now() - BUILD_FILE_RETENTION; + let now_unix = chrono::Utc::now().timestamp(); + Self::prune_dir(&Self::grants_dir(root), "json", |path, modified| { + match fs::read(path) + .ok() + .and_then(|bytes| serde_json::from_slice::(&bytes).ok()) + { + Some(grant) => grant.expires_unix < now_unix, + // Unparseable leftovers fall back to the mtime rule. + None => modified.is_some_and(|modified| modified < cutoff), + } + }); + } + + fn prune_dir_older_than(dir: &Path, extension: &str, cutoff: SystemTime) { + Self::prune_dir(dir, extension, |_, modified| { + modified.is_some_and(|modified| modified < cutoff) + }); + } + + /// Pruning is opportunistic and bounded: at most `MAX_PRUNE_SCAN` matching + /// entries are inspected per call, so the cost a request pays stays + /// constant no matter how many records the directory holds. Anything left + /// over is reclaimed by later calls. + fn prune_dir( + dir: &Path, + extension: &str, + is_expired: impl Fn(&Path, Option) -> bool, + ) { + const MAX_PRUNE_SCAN: usize = 256; + + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + let mut scanned: usize = 0; + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().is_none_or(|ext| ext != extension) { + continue; + } + if scanned >= MAX_PRUNE_SCAN { + break; + } + scanned += 1; + let modified = entry + .metadata() + .and_then(|metadata| metadata.modified()) + .ok(); + if is_expired(&path, modified) { + if let Err(error) = fs::remove_file(&path) { + warn!( + path = %path.display(), + error = %error, + "failed to prune expired template build file" + ); + } else { + debug!(path = %path.display(), "pruned expired template build file"); + } + } + } + } + + fn grants_dir(root: &Path) -> PathBuf { + root.join(GRANTS_DIR_NAME) + } + + fn grant_path(root: &Path, token: &str) -> Option { + is_valid_upload_token(token).then(|| Self::grants_dir(root).join(format!("{token}.json"))) + } + + /// Reads a grant record, mapping an absent file to `None`. + fn read_grant(path: &Path) -> RepositoryResult> { + let bytes = match fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(RepositoryError::backend("read upload grant", error)), + }; + serde_json::from_slice(&bytes) + .map(Some) + .map_err(|error| RepositoryError::backend("parse upload grant", error)) + } + + /// Best-effort mtime refresh, so retention means "unused for the window" + /// and an archive a build is still reading stays outside the prune + /// horizon. Read-only repository mounts must keep working, so failures + /// only log. + fn touch(path: &Path) { + let refreshed = fs::File::options() + .write(true) + .open(path) + .and_then(|file| file.set_times(fs::FileTimes::new().set_modified(SystemTime::now()))); + if let Err(error) = refreshed { + debug!( + path = %path.display(), + error = %error, + "failed to refresh build archive mtime" + ); + } + } + + fn write_grant( + root: &Path, + template_id: &str, + hash: &str, + expires_unix: i64, + ) -> RepositoryResult { + let grants_dir = Self::grants_dir(root); + fs::create_dir_all(&grants_dir).map_err(|error| { + RepositoryError::backend( + format!("create upload grants dir '{}'", grants_dir.display()), + error, + ) + })?; + Self::prune_expired_grants(root); + let bytes = serde_json::to_vec(&TemplateBuildUploadGrant::new( + template_id, + hash, + expires_unix, + )) + .map_err(|error| RepositoryError::backend("serialize upload grant", error))?; + + for _ in 0..3 { + let token = generate_upload_token(); + let path = Self::grant_path(root, &token).expect("generated token is valid"); + match fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(mut file) => { + file.write_all(&bytes) + .and_then(|()| file.sync_all()) + .map_err(|error| { + let _ = fs::remove_file(&path); + RepositoryError::backend("write upload grant", error) + })?; + return Ok(token); + } + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(RepositoryError::backend("create upload grant", error)), + } + } + Err(RepositoryError::Backend { + message: "failed to allocate a unique upload grant token".to_string(), + source: None, + }) + } +} + +#[async_trait] +impl TemplateBuildFileStore for PosixFsTemplateBuildFileStore { + async fn exists(&self, hash: &str) -> RepositoryResult { + let path = self.archive_path(hash)?; + task::spawn_blocking(move || -> RepositoryResult { + match fs::metadata(&path) { + Ok(_) => Ok(true), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(RepositoryError::backend( + format!("stat build archive '{}'", path.display()), + error, + )), + } + }) + .await + .map_err(|error| RepositoryError::backend("join build file exists task", error))? + } + + async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()> { + let final_path = self.archive_path(hash)?; + let root = self.root.clone(); + let staged = staged.to_path_buf(); + task::spawn_blocking(move || -> RepositoryResult<()> { + // Archives are immutable: the hash addresses the content, so a + // repeat upload cannot change what an in-flight build reads. + if final_path.exists() { + return Ok(()); + } + Self::ensure_root(&root)?; + Self::prune_expired(&root); + // Copy into the store filesystem first (the staged file usually + // lives on node-local tmp), then link it into place within the + // store directory so readers only ever observe complete archives. + let store_staged = root.join(format!(".import-{}.tmp", uuid::Uuid::new_v4())); + fs::copy(&staged, &store_staged).map_err(|error| { + let _ = fs::remove_file(&store_staged); + RepositoryError::backend("copy build archive into store", error) + })?; + // The archive is only ever published once, so its data must reach + // stable storage before the name does: a directory entry that + // outlives the bytes would pin a truncated archive forever behind + // the `exists` fast path. + fs::File::open(&store_staged) + .and_then(|file| file.sync_all()) + .map_err(|error| { + let _ = fs::remove_file(&store_staged); + RepositoryError::backend("sync build archive", error) + })?; + // Link rather than rename so a concurrent import cannot replace an + // archive a running build is already reading: the first writer + // wins and everyone else observes `AlreadyExists`. + let published = match fs::hard_link(&store_staged, &final_path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => Ok(()), + Err(error) => Err(RepositoryError::backend("publish build archive", error)), + }; + if published.is_ok() { + // Best effort: filesystems that reject a directory fsync must + // keep working, and a lost entry only costs one re-upload. + if let Err(error) = fs::File::open(&root).and_then(|dir| dir.sync_all()) { + debug!( + path = %root.display(), + error = %error, + "failed to sync build archive store directory" + ); + } + } + let _ = fs::remove_file(&store_staged); + published + }) + .await + .map_err(|error| RepositoryError::backend("join build file import task", error))? + } + + async fn materialize( + &self, + hash: &str, + _scratch_dir: &Path, + ) -> RepositoryResult> { + let path = self.archive_path(hash)?; + task::spawn_blocking(move || -> RepositoryResult> { + match fs::metadata(&path) { + Ok(_) => { + Self::touch(&path); + Ok(Some(path)) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(RepositoryError::backend( + format!("stat build archive '{}'", path.display()), + error, + )), + } + }) + .await + .map_err(|error| RepositoryError::backend("join build file materialize task", error))? + } + + async fn create_upload_grant( + &self, + template_id: &str, + hash: &str, + expires_unix: i64, + ) -> RepositoryResult { + let root = self.root.clone(); + let template_id = template_id.to_string(); + let hash = hash.to_string(); + task::spawn_blocking(move || Self::write_grant(&root, &template_id, &hash, expires_unix)) + .await + .map_err(|error| RepositoryError::backend("join create upload grant task", error))? + } + + async fn verify_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult { + let Some(path) = Self::grant_path(&self.root, token) else { + return Ok(false); + }; + let template_id = template_id.to_string(); + let hash = hash.to_string(); + task::spawn_blocking(move || -> RepositoryResult { + // Reads only: the grant file must survive so an upload that fails + // before the archive is stored can be retried with the same URL. + let Some(grant) = Self::read_grant(&path)? else { + return Ok(false); + }; + Ok(grant.authorizes(&template_id, &hash, expires_unix, now_unix)) + }) + .await + .map_err(|error| RepositoryError::backend("join verify upload grant task", error))? + } + + async fn claim_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult { + let Some(path) = Self::grant_path(&self.root, token) else { + return Ok(false); + }; + let template_id = template_id.to_string(); + let hash = hash.to_string(); + task::spawn_blocking(move || -> RepositoryResult { + let Some(grant) = Self::read_grant(&path)? else { + return Ok(false); + }; + if !grant.authorizes(&template_id, &hash, expires_unix, now_unix) { + return Ok(false); + } + // Consume the grant. `remove_file` succeeds for exactly one + // caller, so it is the claim: concurrent replays of the same + // token lose the race and are rejected. + match fs::remove_file(&path) { + Ok(()) => Ok(true), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(RepositoryError::backend("consume upload grant", error)), + } + }) + .await + .map_err(|error| RepositoryError::backend("join claim upload grant task", error))? + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + const HASH: &str = "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"; + + fn staged_file(dir: &Path, contents: &[u8]) -> PathBuf { + let path = dir.join("staged.tar"); + fs::write(&path, contents).expect("write staged file"); + path + } + + #[tokio::test] + async fn import_then_exists_and_materialize() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + assert!(!store.exists(HASH).await.expect("exists should work")); + assert_eq!( + store + .materialize(HASH, tempdir.path()) + .await + .expect("materialize should work"), + None + ); + + let staged = staged_file(tempdir.path(), b"tar-bytes"); + store + .import(HASH, &staged) + .await + .expect("import should work"); + + assert!(store.exists(HASH).await.expect("exists should work")); + let materialized = store + .materialize(HASH, tempdir.path()) + .await + .expect("materialize should work") + .expect("archive should exist"); + assert_eq!( + fs::read(materialized).expect("read materialized"), + b"tar-bytes" + ); + } + + #[tokio::test] + async fn import_rejects_invalid_hash() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + let staged = staged_file(tempdir.path(), b"tar-bytes"); + + let err = store + .import("../escape", &staged) + .await + .expect_err("invalid hash should fail"); + assert!(matches!(err, RepositoryError::InvalidRequest { .. })); + } + + #[tokio::test] + async fn writing_a_grant_prunes_expired_grant_files() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + let fresh_token = store + .create_upload_grant("template", HASH, i64::MAX) + .await + .expect("fresh grant should be created"); + + // Plant a grant file that predates the retention window. + let grants_dir = tempdir + .path() + .join("template-build-files") + .join("upload-grants"); + let stale_path = grants_dir.join(format!("{}.json", generate_upload_token())); + fs::write(&stale_path, b"{}").expect("write stale grant"); + let stale_mtime = SystemTime::now() - BUILD_FILE_RETENTION - Duration::from_secs(60); + let stale_file = fs::File::options() + .write(true) + .open(&stale_path) + .expect("open stale grant"); + stale_file + .set_times(fs::FileTimes::new().set_modified(stale_mtime)) + .expect("set stale mtime"); + drop(stale_file); + + store + .create_upload_grant("template", HASH, i64::MAX) + .await + .expect("new grant should be created"); + + assert!(!stale_path.exists(), "expired grant file should be pruned"); + assert!( + store + .claim_upload_grant(&fresh_token, "template", HASH, i64::MAX, 0) + .await + .expect("validation should work"), + "unexpired grants must survive pruning" + ); + } + + #[tokio::test] + async fn upload_grant_is_shared_across_instances() { + let tempdir = TempDir::new().expect("tempdir"); + let first = PosixFsTemplateBuildFileStore::new(tempdir.path()); + let second = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + // A mismatched or expired claim leaves the grant usable. + let token = first + .create_upload_grant("template", HASH, 1000) + .await + .expect("grant should be created"); + assert!(!second + .claim_upload_grant(&token, "other", HASH, 1000, 999) + .await + .expect("mismatched grant should be rejected")); + assert!(!second + .claim_upload_grant(&token, "template", HASH, 1000, 1001) + .await + .expect("expired grant should be rejected")); + assert!(second + .claim_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("grant issued by another instance should claim")); + } + + #[tokio::test] + async fn upload_grant_is_single_use() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + let token = store + .create_upload_grant("template", HASH, 1000) + .await + .expect("grant should be created"); + + assert!(store + .claim_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("first claim should succeed")); + assert!( + !store + .claim_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("replay should be rejected"), + "an upload URL must not be replayable" + ); + } + + #[tokio::test] + async fn archives_are_immutable_once_stored() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + let first = staged_file(tempdir.path(), b"original"); + store.import(HASH, &first).await.expect("first import"); + + let replacement = tempdir.path().join("replacement.tar"); + fs::write(&replacement, b"replaced").expect("write replacement"); + store + .import(HASH, &replacement) + .await + .expect("repeat import should be accepted"); + + // Two imports racing for a hash neither has stored yet must both + // succeed; the loser's hard link hits AlreadyExists and is dropped. + // A fresh hash keeps both calls off the exists() fast path. + const FRESH_HASH: &str = "f00ff00ff00ff00ff00ff00ff00ff00f"; + let concurrent = tempdir.path().join("concurrent.tar"); + fs::write(&concurrent, b"concurrent").expect("write concurrent"); + let (left, right) = tokio::join!( + store.import(FRESH_HASH, &replacement), + store.import(FRESH_HASH, &concurrent) + ); + left.expect("concurrent import should be accepted"); + right.expect("concurrent import should be accepted"); + let winner = store + .materialize(FRESH_HASH, tempdir.path()) + .await + .expect("materialize should work") + .expect("archive should exist"); + let winner_bytes = fs::read(winner).expect("read winner"); + assert!( + winner_bytes == b"replaced" || winner_bytes == b"concurrent", + "stored bytes must come from one of the racing imports" + ); + + let materialized = store + .materialize(HASH, tempdir.path()) + .await + .expect("materialize should work") + .expect("archive should exist"); + assert_eq!( + fs::read(materialized).expect("read materialized"), + b"original", + "a stored archive must never be replaced underneath a build" + ); + + let leftovers = fs::read_dir(tempdir.path().join("template-build-files")) + .expect("read store dir") + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".import-")) + .count(); + assert_eq!(leftovers, 0, "import must not leak staging files"); + } + + #[tokio::test] + async fn materialize_refreshes_the_archive_mtime() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + let staged = staged_file(tempdir.path(), b"tar-bytes"); + store.import(HASH, &staged).await.expect("import"); + + let archive = tempdir + .path() + .join("template-build-files") + .join(format!("{HASH}.tar")); + let stale = SystemTime::now() - BUILD_FILE_RETENTION - Duration::from_secs(60); + let file = fs::File::options() + .write(true) + .open(&archive) + .expect("open archive"); + file.set_times(fs::FileTimes::new().set_modified(stale)) + .expect("set stale mtime"); + drop(file); + + store + .materialize(HASH, tempdir.path()) + .await + .expect("materialize should work") + .expect("archive should exist"); + + let modified = fs::metadata(&archive) + .and_then(|metadata| metadata.modified()) + .expect("read archive mtime"); + assert!( + modified > stale, + "materializing an archive must keep it outside the prune horizon" + ); + } + + #[tokio::test] + async fn verifying_a_grant_does_not_consume_it() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + let token = store + .create_upload_grant("template", HASH, 1000) + .await + .expect("grant should be created"); + + for _ in 0..2 { + assert!( + store + .verify_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("verification should work"), + "verification must not consume the grant" + ); + } + assert!(!store + .verify_upload_grant(&token, "other", HASH, 1000, 999) + .await + .expect("mismatched grant should be rejected")); + + // An upload that failed after verification can still be retried. + assert!(store + .claim_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("claim should succeed")); + assert!( + !store + .verify_upload_grant(&token, "template", HASH, 1000, 999) + .await + .expect("verification should work"), + "a consumed grant must no longer verify" + ); + } + + #[tokio::test] + async fn concurrent_claims_pick_a_single_winner() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + let token = store + .create_upload_grant("template", HASH, 1000) + .await + .expect("grant should be created"); + + let (left, right) = tokio::join!( + store.claim_upload_grant(&token, "template", HASH, 1000, 999), + store.claim_upload_grant(&token, "template", HASH, 1000, 999) + ); + let claims = [ + left.expect("claim should work"), + right.expect("claim should work"), + ]; + assert_eq!( + claims.iter().filter(|claimed| **claimed).count(), + 1, + "exactly one concurrent claim may win" + ); + } + + #[tokio::test] + async fn grants_are_pruned_once_their_own_expiry_passes() { + let tempdir = TempDir::new().expect("tempdir"); + let store = PosixFsTemplateBuildFileStore::new(tempdir.path()); + + // Expired long ago in grant terms, but freshly written on disk, so the + // mtime rule alone would keep it for the whole retention window. + let expired_token = store + .create_upload_grant("template", HASH, 1000) + .await + .expect("grant should be created"); + let expired_path = tempdir + .path() + .join("template-build-files") + .join("upload-grants") + .join(format!("{expired_token}.json")); + assert!(expired_path.exists()); + + store + .create_upload_grant("template", HASH, i64::MAX) + .await + .expect("new grant should be created"); + + assert!( + !expired_path.exists(), + "a grant past its own expiry should be pruned" + ); + } +} diff --git a/src/snapshot/repository/backends/posixfs/catalog.rs b/src/snapshot/repository/backends/posixfs/catalog.rs index 6c36d6bd..76b7486c 100644 --- a/src/snapshot/repository/backends/posixfs/catalog.rs +++ b/src/snapshot/repository/backends/posixfs/catalog.rs @@ -6,6 +6,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use serde::de::DeserializeOwned; use serde::Serialize; +use tracing::warn; use super::layout::PosixFsSnapshotArtifactLayout; use crate::snapshot::repository::SnapshotListFilter; @@ -79,9 +80,9 @@ impl PosixFsCatalogStore { /// /// Flow: /// 1. acquire the alias lock when an alias is present - /// 2. bind the alias - /// 3. write the commit marker - /// 4. write the committed snapshot record + /// 2. write the commit marker + /// 3. write the committed snapshot record + /// 4. atomically bind the alias as the final visible operation pub(crate) fn commit_publish( &self, session: &PublishSession, @@ -90,25 +91,30 @@ impl PosixFsCatalogStore { ) -> RepositoryResult { let now = now_unix_ms(); let snapshot_id = metadata.id.clone(); + let previous_record = self.load_record_by_id_unlocked(&snapshot_id)?; let write_result = if let Some(alias) = metadata.alias.as_ref() { self.with_alias_lock(alias, |store| { let record = store.committed_record_unlocked(&metadata, committed.clone(), now)?; let alias_path = PosixFsSnapshotArtifactLayout::alias_path(&store.root, alias); - if let Some(existing) = store.load_alias_target(alias)? { - if existing != snapshot_id { - if store.load_record_by_id_unlocked(&existing)?.is_some() { - return Err(RepositoryError::AliasConflict { - alias: alias.to_string(), - existing, - new_id: snapshot_id.clone(), - }); - } - store.remove_file_if_exists(&alias_path)?; - } - } - store.write_json(&alias_path, &snapshot_id)?; + let existing = store.load_alias_target(alias)?; store.write_commit_marker(&session.snapshot_id)?; store.write_committed_record_unlocked(&record)?; + // `write_json` uses an atomic rename. Keeping this as the final + // fallible operation means a failed rebuild leaves the old + // alias binding untouched. The tradeoff is a crash window: dying + // after the record write but before the alias write leaves a + // committed record whose `alias` field names an alias that still + // resolves to the previous snapshot, so readers of `record.alias` + // (listings) may observe the stale claim until the next rebind. + store.write_json(&alias_path, &snapshot_id)?; + + if let Some(existing) = existing.filter(|existing| existing != &snapshot_id) { + // The previous snapshot stays addressable by id, so running + // sandboxes and explicit id references keep working. Alias + // metadata cleanup is best effort because the binding has + // already moved successfully. + store.clear_moved_alias_on_previous_record(&existing, alias.as_ref(), now); + } Ok(record) }) } else { @@ -123,17 +129,7 @@ impl PosixFsCatalogStore { match write_result { Ok(record) => Ok(record), Err(error) => { - if let Some(alias) = metadata.alias.as_ref() { - let _ = self.with_alias_lock(alias, |store| { - let alias_path = - PosixFsSnapshotArtifactLayout::alias_path(&store.root, alias); - if store.load_alias_target(alias)?.as_ref() == Some(&snapshot_id) { - store.remove_file_if_exists(&alias_path)?; - } - Ok(()) - }); - } - let _ = self.cleanup_uncommitted_snapshot_dir(&session.snapshot_id); + self.rollback_failed_publish(&session.snapshot_id, previous_record.as_ref()); Err(error) } } @@ -164,12 +160,35 @@ impl PosixFsCatalogStore { if let Some(alias) = record.alias.as_ref() { self.with_alias_lock(alias, |store| { - store.ensure_alias_available(alias, &record.id)?; store.write_record_unlocked(&record)?; - store.write_json( - &PosixFsSnapshotArtifactLayout::alias_path(&store.root, alias), - &record.id, - ) + let alias_path = PosixFsSnapshotArtifactLayout::alias_path(&store.root, alias); + let bind = (|| -> RepositoryResult<()> { + match store.load_alias_target(alias)? { + // The alias currently points at a live snapshot. Leave the + // binding untouched so the existing template keeps resolving + // while the new build runs; a successful commit moves the + // alias to the new snapshot (E2B rebuild semantics). + Some(existing) + if existing != record.id + && store.load_record_by_id_unlocked(&existing)?.is_some() => + { + Ok(()) + } + Some(existing) if existing != record.id => { + store.remove_file_if_exists(&alias_path)?; + store.write_json(&alias_path, &record.id) + } + _ => store.write_json(&alias_path, &record.id), + } + })(); + if let Err(error) = bind { + // Keep creation all-or-nothing under the alias lock: a record + // that survives a failed binding claims the alias in listings + // with nothing left to reconcile it. + let _ = store.remove_file_if_exists(&store.record_path(&record.id)); + return Err(error); + } + Ok(()) })?; } else { self.write_record_unlocked(&record)?; @@ -376,6 +395,67 @@ impl PosixFsCatalogStore { self.write_record_unlocked(&record) } + /// Clears `moved_alias` from the record that owned it before a rebind. + /// + /// Best effort: the alias binding has already moved, so a failure here only + /// leaves stale alias metadata on the previous owner's record. + fn clear_moved_alias_on_previous_record( + &self, + previous_id: &SnapshotId, + moved_alias: &str, + now: i64, + ) { + // Lock order is alias lock first, then record lock; nothing takes them + // in the reverse order today. + let _guard = match self.acquire_record_lock(previous_id) { + Ok(guard) => guard, + Err(error) => { + warn!( + alias = %moved_alias, + previous_snapshot_id = %previous_id, + error = %error, + "failed to lock previous snapshot record for alias cleanup" + ); + return; + } + }; + + let mut previous = match self.load_record_by_id_unlocked(previous_id) { + Ok(Some(previous)) => previous, + Ok(None) => return, + Err(error) => { + warn!( + alias = %moved_alias, + previous_snapshot_id = %previous_id, + error = %error, + "failed to load previous snapshot alias metadata" + ); + return; + } + }; + + // Only clear the alias this publish actually moved; the previous owner + // may already claim a different name. + let claims_moved_alias = previous + .alias + .as_ref() + .is_some_and(|alias| alias.as_ref() == moved_alias); + if !claims_moved_alias { + return; + } + + previous.alias = None; + previous.updated_at_unix_ms = now; + if let Err(error) = self.write_record_unlocked(&previous) { + warn!( + alias = %moved_alias, + previous_snapshot_id = %previous_id, + error = %error, + "failed to clear previous snapshot alias metadata" + ); + } + } + fn read_json(&self, path: &Path) -> RepositoryResult where T: DeserializeOwned, @@ -498,6 +578,19 @@ impl PosixFsCatalogStore { self.remove_dir_if_exists(&snapshot_layout.snapshot_dir()) } + fn rollback_failed_publish(&self, id: &SnapshotId, previous_record: Option<&SnapshotRecord>) { + if let Err(error) = self.remove_dir_if_exists(&self.layout(id).snapshot_dir()) { + warn!(snapshot_id = %id, error = %error, "failed to remove snapshot artifacts after publish failure"); + } + let restore_result = match previous_record { + Some(record) => self.write_record_unlocked(record), + None => self.remove_file_if_exists(&self.record_path(id)), + }; + if let Err(error) = restore_result { + warn!(snapshot_id = %id, error = %error, "failed to restore snapshot record after publish failure"); + } + } + fn load_record_by_id_unlocked( &self, id: &SnapshotId, @@ -626,28 +719,6 @@ impl PosixFsCatalogStore { action(self) } - fn ensure_alias_available( - &self, - alias: &SnapshotAlias, - new_id: &SnapshotId, - ) -> RepositoryResult<()> { - let alias_path = PosixFsSnapshotArtifactLayout::alias_path(&self.root, alias); - if let Some(existing) = self.load_alias_target(alias)? { - if &existing == new_id { - return Ok(()); - } - if self.load_record_by_id_unlocked(&existing)?.is_some() { - return Err(RepositoryError::AliasConflict { - alias: alias.to_string(), - existing, - new_id: new_id.clone(), - }); - } - self.remove_file_if_exists(&alias_path)?; - } - Ok(()) - } - fn write_record_unlocked(&self, record: &SnapshotRecord) -> RepositoryResult<()> { self.write_json(&self.record_path(&record.id), record) } diff --git a/src/snapshot/repository/backends/posixfs/mod.rs b/src/snapshot/repository/backends/posixfs/mod.rs index 09ce72af..f61204dc 100644 --- a/src/snapshot/repository/backends/posixfs/mod.rs +++ b/src/snapshot/repository/backends/posixfs/mod.rs @@ -1,5 +1,6 @@ mod artifacts; mod backend; +mod build_files; mod catalog; mod layout; mod runtime; diff --git a/src/snapshot/repository/build_files.rs b/src/snapshot/repository/build_files.rs new file mode 100644 index 00000000..f693af92 --- /dev/null +++ b/src/snapshot/repository/build_files.rs @@ -0,0 +1,197 @@ +use std::path::{Path, PathBuf}; + +use async_trait::async_trait; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine; +use serde::{Deserialize, Serialize}; + +use super::errors::RepositoryResult; + +/// Number of random bytes in an upload bearer token. +pub const UPLOAD_TOKEN_LEN: usize = 32; + +/// Durable authorization record for one build-context upload URL. +/// +/// Grants live in the same shared repository as build archives. That makes a +/// URL issued by one node verifiable by any other node without coordinating a +/// deployment-wide in-memory signing secret. +#[derive(Debug, Deserialize, Serialize)] +pub struct TemplateBuildUploadGrant { + pub template_id: String, + pub hash: String, + pub expires_unix: i64, +} + +impl TemplateBuildUploadGrant { + pub fn new(template_id: &str, hash: &str, expires_unix: i64) -> Self { + Self { + template_id: template_id.to_string(), + hash: hash.to_string(), + expires_unix, + } + } + + pub fn authorizes( + &self, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> bool { + now_unix <= expires_unix + && self.expires_unix == expires_unix + && self.template_id == template_id + && self.hash == hash + } +} + +/// Durable store for template build-context archives. +/// +/// The E2B SDK resolves every `COPY` step through +/// `GET /templates/{templateID}/files/{hash}` and then `PUT`s a tar archive of +/// the matching context files to the returned URL. This store owns those +/// archives, addressed by the SDK-computed content hash, so that: +/// +/// - any node can answer the upload-link request (`exists`), +/// - any node can accept the upload (`import`), and +/// - the node that runs the build can read the archive back (`materialize`). +/// +/// Implementations must place the archives in storage shared by all nodes of +/// the deployment, mirroring the visibility rules of committed snapshots. +#[async_trait] +pub trait TemplateBuildFileStore: Send + Sync { + /// Returns whether an archive for `hash` is already stored. + async fn exists(&self, hash: &str) -> RepositoryResult; + + /// Imports a fully written local file as the archive for `hash`. + /// + /// Implementations must publish atomically: concurrent readers never + /// observe a partially imported archive. `hash` is the cache key supplied + /// by the authenticated caller, not a digest the store verifies, so + /// immutability here means first-write-wins stability rather than content + /// authenticity: importing a hash that is already stored keeps the stored + /// archive, so an in-flight build can never observe its build context + /// change underneath it. + async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()>; + + /// Materializes the archive for `hash` as a node-local file. + /// + /// `scratch_dir` is a caller-owned directory the implementation may use + /// for downloads; implementations backed by a shared filesystem may return + /// the shared path directly. Callers must treat the returned file as + /// read-only. Returns `None` when no archive is stored for `hash`. + async fn materialize( + &self, + hash: &str, + scratch_dir: &Path, + ) -> RepositoryResult>; + + /// Creates a durable bearer grant for one upload URL and returns its + /// URL-safe token. + async fn create_upload_grant( + &self, + template_id: &str, + hash: &str, + expires_unix: i64, + ) -> RepositoryResult; + + /// Verifies a durable bearer grant without consuming it, returning + /// whether it authorizes this upload. + /// + /// Verification never removes the grant, so a request that fails before + /// the archive is stored can be retried with the same upload URL. Callers + /// must `claim_upload_grant` after publishing the archive, so a failed + /// publication leaves the URL retryable. + async fn verify_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult; + + /// Claims a durable bearer grant, returning whether it authorized this + /// upload. + /// + /// Grants are single-use: a successful claim consumes the grant, so an + /// upload URL cannot be replayed within its TTL. Implementations must + /// make the claim itself the atomic step wherever the backend offers an + /// atomic primitive (a POSIX filesystem does, via rename/unlink), so + /// concurrent requests carrying the same token cannot both succeed. + /// S3-compatible backends have no conditional delete and therefore + /// degrade to best-effort single-use within the grant TTL; archive + /// immutability is what keeps a lost race from mattering: both uploads are + /// bound to the same (template_id, hash), and `import` is first-write-wins, + /// so neither can change an archive that is already stored — which upload + /// wins a first store is undefined. + async fn claim_upload_grant( + &self, + token: &str, + template_id: &str, + hash: &str, + expires_unix: i64, + now_unix: i64, + ) -> RepositoryResult; +} + +/// Returns whether `hash` is acceptable as a build-file content hash. +/// +/// The E2B SDK sends a lowercase hex SHA-256, but the value is treated as an +/// opaque cache key; this only enforces a path- and URL-safe shape. +pub fn is_valid_build_files_hash(hash: &str) -> bool { + (16..=128).contains(&hash.len()) && hash.bytes().all(|b| b.is_ascii_hexdigit()) +} + +/// Generates a cryptographically random URL-safe upload bearer token. +pub fn generate_upload_token() -> String { + let mut token = [0u8; UPLOAD_TOKEN_LEN]; + rand::fill(&mut token); + URL_SAFE_NO_PAD.encode(token) +} + +/// Returns whether `token` has the exact shape generated for upload grants. +pub fn is_valid_upload_token(token: &str) -> bool { + URL_SAFE_NO_PAD + .decode(token) + .is_ok_and(|decoded| decoded.len() == UPLOAD_TOKEN_LEN) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hash_validation_accepts_sha256_hex() { + assert!(is_valid_build_files_hash( + "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" + )); + assert!(is_valid_build_files_hash("ABCDEF0123456789")); + } + + #[test] + fn hash_validation_rejects_path_unsafe_values() { + assert!(!is_valid_build_files_hash("")); + assert!(!is_valid_build_files_hash("short")); + assert!(!is_valid_build_files_hash("../../../../etc/passwd")); + assert!(!is_valid_build_files_hash("deadbeef/deadbeef")); + assert!(!is_valid_build_files_hash(&"a".repeat(129))); + } + + #[test] + fn upload_token_has_expected_shape() { + let token = generate_upload_token(); + assert!(is_valid_upload_token(&token)); + assert!(!is_valid_upload_token("not-a-valid-token")); + } + + #[test] + fn upload_grant_is_bound_to_request_and_expiry() { + let grant = TemplateBuildUploadGrant::new("tmpl", "aabbccddeeff0011", 1000); + assert!(grant.authorizes("tmpl", "aabbccddeeff0011", 1000, 999)); + assert!(!grant.authorizes("tmpl", "aabbccddeeff0011", 1000, 1001)); + assert!(!grant.authorizes("other", "aabbccddeeff0011", 1000, 999)); + assert!(!grant.authorizes("tmpl", "aabbccddeeff0012", 1000, 999)); + assert!(!grant.authorizes("tmpl", "aabbccddeeff0011", 2000, 999)); + } +} diff --git a/src/snapshot/repository/interfaces.rs b/src/snapshot/repository/interfaces.rs index 16c387f4..604b07a6 100644 --- a/src/snapshot/repository/interfaces.rs +++ b/src/snapshot/repository/interfaces.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use async_trait::async_trait; +use super::build_files::TemplateBuildFileStore; use super::errors::RepositoryResult; use crate::sandbox::FirecrackerSnapshotManifest; use crate::snapshot::types::{ @@ -173,6 +174,15 @@ pub trait SnapshotRepository: Send + Sync { id: &SnapshotId, reason: TemplateBuildErrorReason, ) -> RepositoryResult<()>; + + /// Returns the shared store for template build-context archives. + /// + /// Returns `None` when this backend does not support build-context + /// uploads; the template files API then reports the capability as + /// unavailable instead of failing at build time. + fn template_build_files(&self) -> Option> { + None + } } #[async_trait] diff --git a/src/snapshot/repository/mod.rs b/src/snapshot/repository/mod.rs index 788c94e8..aa45e140 100644 --- a/src/snapshot/repository/mod.rs +++ b/src/snapshot/repository/mod.rs @@ -1,6 +1,8 @@ pub mod backends; +pub mod build_files; pub mod errors; pub mod interfaces; +pub use build_files::TemplateBuildFileStore; pub use errors::{RepositoryError, RepositoryResult}; pub use interfaces::{SnapshotListFilter, SnapshotRepository, SnapshotRuntimeResolver}; diff --git a/src/template/build_spec.rs b/src/template/build_spec.rs index 8e23e18b..3d5957f2 100644 --- a/src/template/build_spec.rs +++ b/src/template/build_spec.rs @@ -25,13 +25,41 @@ pub(crate) struct TemplateBuildStep { #[derive(Clone, Debug)] pub(crate) enum TemplateBuildStepKind { - Run { cmd: String }, - Env { key: String, value: String }, - Workdir { path: PathBuf }, - User { value: String }, - ExposedPort { port: String }, - Volume { path: String }, - Label { key: String, value: String }, + Run { + cmd: String, + }, + Env { + key: String, + value: String, + }, + Workdir { + path: PathBuf, + }, + User { + value: String, + }, + ExposedPort { + port: String, + }, + Volume { + path: String, + }, + Label { + key: String, + value: String, + }, + /// Copies files from an uploaded build-context archive into the rootfs. + /// + /// `files_hash` addresses the archive in the repository's template + /// build-file store; `user` and `mode` mirror Docker's `--chown` / + /// `--chmod` flags. + Copy { + src: String, + dest: String, + files_hash: String, + user: Option, + mode: Option, + }, } impl TemplateBuildStep { @@ -84,6 +112,24 @@ impl TemplateBuildStep { }, } } + + pub(crate) fn copy( + src: impl Into, + dest: impl Into, + files_hash: impl Into, + user: Option, + mode: Option, + ) -> Self { + Self { + kind: TemplateBuildStepKind::Copy { + src: src.into(), + dest: dest.into(), + files_hash: files_hash.into(), + user, + mode, + }, + } + } } #[derive(Clone, Debug, Default)] @@ -167,6 +213,35 @@ impl TemplateBuildSpec { self } + /// Appends a build step copying files from an uploaded build-context + /// archive (addressed by `files_hash`) into the rootfs. + pub fn copy( + mut self, + src: impl Into, + dest: impl Into, + files_hash: impl Into, + user: Option, + mode: Option, + ) -> Self { + self.steps + .push(TemplateBuildStep::copy(src, dest, files_hash, user, mode)); + self + } + + /// Returns the distinct build-context archive hashes referenced by COPY steps. + pub(crate) fn referenced_build_file_hashes(steps: &[TemplateBuildStep]) -> Vec { + let mut seen: std::collections::HashSet<&str> = std::collections::HashSet::new(); + let mut hashes: Vec = Vec::new(); + for step in steps { + if let TemplateBuildStepKind::Copy { files_hash, .. } = &step.kind { + if seen.insert(files_hash.as_str()) { + hashes.push(files_hash.clone()); + } + } + } + hashes + } + /// Appends an `apt-get install` build step for the provided packages. pub fn apt(mut self, packages: I) -> Self where diff --git a/src/template/builder.rs b/src/template/builder.rs index c934e154..ea8cf059 100644 --- a/src/template/builder.rs +++ b/src/template/builder.rs @@ -18,6 +18,9 @@ use crate::snapshot::{ }; use crate::types::SandboxResources; +/// Bounds the node disk cost of one build spec. +const MAX_BUILD_ARCHIVES: usize = 32; + #[derive(Clone)] /// Coordinates template-builder flows over committed snapshots. pub struct TemplateBuilder { @@ -100,9 +103,12 @@ impl TemplateBuilder { async fn execute_and_publish( &self, snapshot_manager: &SnapshotManager, - context: TemplateBuildContext, + mut context: TemplateBuildContext, operation: &'static str, ) -> TemplatePipelineResult { + context.build_archives = + Self::materialize_build_archives(snapshot_manager, &context).await?; + info!("executing template build"); let build_execution = match TemplateBuildRunner::new().execute(&context) { Ok(execution) => execution, @@ -151,6 +157,87 @@ impl TemplateBuilder { } impl TemplateBuilder { + /// Fetches every build-context archive referenced by COPY steps into + /// node-local files before the build sandbox starts. + async fn materialize_build_archives( + snapshot_manager: &SnapshotManager, + context: &TemplateBuildContext, + ) -> TemplatePipelineResult> { + let hashes = TemplateBuildSpec::referenced_build_file_hashes(&context.steps); + if hashes.is_empty() { + return Ok(std::collections::HashMap::new()); + } + if hashes.len() > MAX_BUILD_ARCHIVES { + return Err(TemplateBuildError::invalid_input(format!( + "template build references {} distinct build context archives, which exceeds the \ + limit of {MAX_BUILD_ARCHIVES}", + hashes.len() + )) + .into()); + } + + let Some(store) = snapshot_manager.template_build_files() else { + return Err(TemplateBuildError::invalid_input( + "the configured snapshot backend does not support the build-context uploads required by COPY steps", + ) + .into()); + }; + + let scratch = context.local_dir().join("build-archives"); + tokio::fs::create_dir_all(&scratch).await.map_err(|error| { + TemplateBuildError::with_source("create build archive scratch dir", error) + })?; + + let max_context_mib = ConfigManager::global_config() + .template_build + .files_max_build_context_mib; + let max_total_bytes = max_context_mib.saturating_mul(1024 * 1024); + + let mut archives = std::collections::HashMap::new(); + let mut total_bytes: u64 = 0; + for hash in hashes { + match store.materialize(&hash, &scratch).await { + Ok(Some(path)) => { + let size = tokio::fs::metadata(&path) + .await + .map_err(|error| { + TemplateBuildError::with_source( + format!("read build context archive '{hash}' metadata"), + error, + ) + })? + .len(); + total_bytes = total_bytes.saturating_add(size); + if total_bytes > max_total_bytes { + return Err(TemplateBuildError::invalid_input(format!( + "template build context archives total at least {total_bytes} bytes, \ + which exceeds the template_build.files_max_build_context_mib limit of \ + {max_context_mib} MiB" + )) + .into()); + } + archives.insert(hash, path); + } + Ok(None) => { + return Err(TemplateBuildError::invalid_input(format!( + "build context archive '{hash}' has not been uploaded; request an upload \ + link via GET /templates/{{templateID}}/files/{hash} and upload the \ + archive before starting the build" + )) + .into()); + } + Err(error) => { + return Err(TemplateBuildError::with_source( + format!("fetch build context archive '{hash}'"), + error, + ) + .into()); + } + } + } + Ok(archives) + } + fn build_failure_reason(error: &AnyhowError) -> TemplateBuildErrorReason { error .chain() @@ -209,6 +296,7 @@ impl TemplateBuilder { resources, workspace, steps: spec.steps().to_vec(), + build_archives: std::collections::HashMap::new(), base, cpu_config_json: self.current_cpu_config(), }) @@ -261,6 +349,7 @@ impl TemplateBuilder { resources, workspace, steps: spec.steps().to_vec(), + build_archives: std::collections::HashMap::new(), base: TemplateBuildBase::Snapshot { base_snapshot: Box::new(base_snapshot.clone()), }, @@ -377,6 +466,33 @@ mod tests { )); } + #[tokio::test] + async fn materialize_build_archives_rejects_too_many_archives() { + let tempdir = TempDir::new().expect("tempdir"); + let (manager, snapshot_manager) = test_parts(tempdir.path()); + let runnable = RunnableSnapshot::mock(); + let mut spec = TemplateBuildSpec::new(); + for index in 0..=MAX_BUILD_ARCHIVES { + spec = spec.copy("./src", "/app", format!("hash-{index}"), None, None); + } + let context = manager + .prepare_snapshot_base_context( + &spec, + crate::snapshot::SnapshotId::generate(), + &runnable, + ) + .expect("snapshot-base preparation should succeed"); + + let err = TemplateBuilder::materialize_build_archives(&snapshot_manager, &context) + .await + .expect_err("too many referenced archives should be rejected"); + + assert!(matches!( + err, + crate::template::TemplatePipelineError::Build(TemplateBuildError::InvalidInput { .. }) + )); + } + #[tokio::test] async fn build_snapshot_base_rejects_overriding_rootfs_base() { let manager = TemplateBuilder::new(); diff --git a/src/template/copy_plan.rs b/src/template/copy_plan.rs new file mode 100644 index 00000000..f5302a9f --- /dev/null +++ b/src/template/copy_plan.rs @@ -0,0 +1,1190 @@ +//! Host-side planning for template `COPY` steps. +//! +//! The E2B SDK uploads one tar archive per `COPY` instruction whose entry +//! paths are relative to the build context (the glob in `src` is already +//! resolved by the SDK). This module rewrites that archive so every entry +//! carries its final absolute guest path per Docker `COPY` semantics; the +//! build sandbox then only needs a single `tar -xpf archive -C /`. +//! +//! The rewrite runs in two passes so an archive is never held in memory: the +//! first pass indexes entry paths to compute the mapping, the second streams +//! each entry's bytes straight into the rewritten archive. Both passes read +//! from a single open file handle so the source cannot be replaced or unlinked +//! between them. +//! +//! Ownership is written into the rewritten headers rather than applied with a +//! post-extract `chown`, so a copy can only ever change the files it creates. +//! For the same reason the destination root itself never gets an archive +//! entry: `tar -xp` restores mode and ownership onto directory members that +//! already exist. + +use std::fs::File; +use std::io::{BufReader, Read, Seek, SeekFrom, Write}; +use std::path::Path; + +use anyhow::{bail, Context, Result}; + +/// Upper bound on entries in one build-context archive. The indexing pass +/// keeps one normalized path per entry, so this bounds that allocation +/// independently of the byte budget: an archive of a million empty files is +/// tiny but path-heavy. Real build contexts are orders of magnitude smaller. +const MAX_ARCHIVE_ENTRIES: usize = 200_000; + +/// Numeric ownership applied to every entry of one copy. +#[derive(Clone, Copy, Debug)] +pub(crate) struct CopyOwnership { + pub(crate) uid: u64, + pub(crate) gid: u64, +} + +/// Inputs for rewriting one `COPY` step's archive. +pub(crate) struct CopyRequest<'a> { + pub(crate) source_tar: &'a Path, + pub(crate) src: &'a str, + pub(crate) dest: &'a str, + pub(crate) workdir: &'a str, + pub(crate) mode: Option, + /// Ownership requested by `--chown`, already resolved to numeric ids + /// inside the build sandbox. `None` keeps Docker's root:root default. + pub(crate) ownership: Option, + /// Budget for the decompressed archive, bounding both the rewritten file + /// on the host and what a single upload can expand to. + pub(crate) max_total_bytes: u64, +} + +/// Summary of a rewritten copy archive. +#[derive(Debug)] +pub(crate) struct CopyPlan { + /// Number of file/dir/symlink entries written to the rewritten archive. + pub(crate) entry_count: usize, + /// Total file bytes written to the rewritten archive. + pub(crate) total_bytes: u64, + /// Resolved absolute guest path of the copy destination root. + pub(crate) dest_root: String, + /// Whether a directory entry for `dest_root` itself was dropped. When set, + /// the guest has to create that directory (with the requested ownership + /// and mode) before extraction, but only if it does not already exist. + pub(crate) skipped_dest_root: bool, + /// Whether the copy treats `dest_root` as a directory. Archives without a + /// directory member for the root (file-only uploads) still need the guest + /// to create a missing destination with the requested metadata. + pub(crate) dest_is_dir: bool, +} + +/// One archive entry as seen by the indexing pass. +struct EntryIndex { + /// Normalized context-relative path ("dir/file.txt"). + path: String, + is_dir: bool, +} + +fn is_glob_pattern(src: &str) -> bool { + src.contains(['*', '?', '[']) +} + +/// One member of a `[...]` character class. +enum ClassItem { + Char(char), + Range(char, char), +} + +/// One matchable unit inside a single path segment of a glob pattern. +enum GlobToken { + Star, + Any, + Literal(char), + Class { + negated: bool, + items: Vec, + }, +} + +/// Splits one pattern segment into tokens. An unterminated `[` is a literal. +fn tokenize_segment(pattern: &[char]) -> Vec { + let mut tokens = Vec::new(); + let mut i = 0; + while i < pattern.len() { + match pattern[i] { + '*' => { + tokens.push(GlobToken::Star); + i += 1; + } + '?' => { + tokens.push(GlobToken::Any); + i += 1; + } + '[' => match pattern[i + 1..].iter().position(|&c| c == ']') { + None => { + tokens.push(GlobToken::Literal('[')); + i += 1; + } + Some(end) => { + let class = &pattern[i + 1..i + 1 + end]; + let (negated, class) = match class.first() { + Some('!') | Some('^') => (true, &class[1..]), + _ => (false, class), + }; + let mut items = Vec::new(); + let mut j = 0; + while j < class.len() { + if j + 2 < class.len() && class[j + 1] == '-' { + items.push(ClassItem::Range(class[j], class[j + 2])); + j += 3; + } else { + items.push(ClassItem::Char(class[j])); + j += 1; + } + } + tokens.push(GlobToken::Class { negated, items }); + i += end + 2; + } + }, + c => { + tokens.push(GlobToken::Literal(c)); + i += 1; + } + } + } + tokens +} + +/// Whether a single-character token accepts `c`. +fn token_matches(token: &GlobToken, c: char) -> bool { + match token { + GlobToken::Star => false, + GlobToken::Any => true, + GlobToken::Literal(expected) => *expected == c, + GlobToken::Class { negated, items } => { + let hit = items.iter().any(|item| match item { + ClassItem::Char(ch) => *ch == c, + ClassItem::Range(low, high) => *low <= c && c <= *high, + }); + hit != *negated + } + } +} + +/// Matches one segment with a single backtrack point per `*`, which keeps the +/// worst case quadratic instead of the exponential blowup a naive recursive +/// matcher has on patterns such as `*a*a*a*a*b`. +fn match_segment(tokens: &[GlobToken], value: &[char]) -> bool { + let mut token_idx = 0usize; + let mut value_idx = 0usize; + let mut last_star: Option = None; + let mut last_star_value = 0usize; + + while value_idx < value.len() { + if token_idx < tokens.len() { + if matches!(tokens[token_idx], GlobToken::Star) { + last_star = Some(token_idx); + last_star_value = value_idx; + token_idx += 1; + continue; + } + if token_matches(&tokens[token_idx], value[value_idx]) { + token_idx += 1; + value_idx += 1; + continue; + } + } + // Mismatch: let the most recent `*` swallow one more character. + let Some(star_idx) = last_star else { + return false; + }; + token_idx = star_idx + 1; + last_star_value += 1; + value_idx = last_star_value; + } + + tokens[token_idx..] + .iter() + .all(|token| matches!(token, GlobToken::Star)) +} + +/// Minimal fnmatch-style matcher covering `*`, `?` and `[...]` (no `**`), +/// mirroring the Python `glob` patterns the SDK resolves client-side. +/// +/// Matching is segment-wise like Go's `path/filepath.Match`, which is what +/// Docker uses for `COPY` sources: none of the wildcards ever match `/`, so a +/// pattern and a value with different segment counts never match. +fn glob_match(pattern: &str, value: &str) -> bool { + let mut pattern_segments = pattern.split('/'); + let mut value_segments = value.split('/'); + loop { + match (pattern_segments.next(), value_segments.next()) { + (None, None) => return true, + (Some(pattern_segment), Some(value_segment)) => { + let pattern_segment: Vec = pattern_segment.chars().collect(); + let value_segment: Vec = value_segment.chars().collect(); + if !match_segment(&tokenize_segment(&pattern_segment), &value_segment) { + return false; + } + } + _ => return false, + } + } +} + +/// Normalizes a context-relative source pattern ("./a/b/" -> "a/b"). +fn normalize_src(src: &str) -> String { + let mut src = src.trim(); + while let Some(stripped) = src.strip_prefix("./") { + src = stripped; + } + src.trim_end_matches('/').to_string() +} + +/// Joins `path` onto `base` and lexically normalizes the result into an +/// absolute guest path. Absolute `path` values replace `base` entirely, which +/// is how Docker resolves both `WORKDIR` and `COPY` destinations. +pub(crate) fn resolve_guest_path(base: &str, path: &str) -> Result { + let joined = if path.starts_with('/') { + path.to_string() + } else { + let base = if base.trim().is_empty() { "/" } else { base }; + if !base.starts_with('/') { + bail!("cannot resolve '{path}' against non-absolute base '{base}'"); + } + format!("{}/{}", base.trim_end_matches('/'), path) + }; + + let mut parts: Vec<&str> = Vec::new(); + for part in joined.split('/') { + match part { + "" | "." => {} + ".." => { + if parts.pop().is_none() { + bail!("path '{path}' escapes the filesystem root"); + } + } + part => parts.push(part), + } + } + Ok(format!("/{}", parts.join("/"))) +} + +fn join_abs(base: &str, rel: &str) -> String { + if rel.is_empty() { + base.to_string() + } else if base == "/" { + format!("/{rel}") + } else { + format!("{base}/{rel}") + } +} + +fn base_name(path: &str) -> &str { + path.rsplit('/').next().unwrap_or(path) +} + +/// Normalizes one archive entry path and rejects escapes. +fn normalize_entry_path(raw: &Path) -> Result { + let mut parts: Vec = Vec::new(); + for component in raw.components() { + match component { + std::path::Component::Normal(part) => { + // Lossy conversion would collapse distinct non-UTF-8 names + // onto one replacement-character name, silently overwriting. + let Some(part) = part.to_str() else { + bail!( + "non-UTF-8 path component in archive entry '{}'", + raw.display() + ); + }; + parts.push(part.to_string()); + } + std::path::Component::CurDir => {} + other => bail!( + "unsupported path component {:?} in archive entry '{}'", + other, + raw.display() + ), + } + } + if parts.is_empty() { + bail!("empty path in archive entry"); + } + Ok(parts.join("/")) +} + +/// The uploaded archive, opened once and read twice. +/// +/// Holding the handle across both passes means the two passes provably see the +/// same inode: a concurrent unlink or replacement of the path cannot make the +/// second pass read a different archive than the one the mapping was built +/// from. +struct SourceArchive { + file: File, + gzip: bool, + /// Hard cap on the bytes any one pass may pull out of the reader. + budget: u64, +} + +impl SourceArchive { + fn open(source_tar: &Path, max_total_bytes: u64) -> Result { + let mut file = File::open(source_tar) + .with_context(|| format!("open build context archive '{}'", source_tar.display()))?; + let mut magic = [0u8; 2]; + let gzip = match file.read(&mut magic) { + Ok(2) => magic == [0x1f, 0x8b], + _ => false, + }; + // Every indexed entry charges at least its own 512-byte header against + // the payload budget, so the configured limit already bounds how many + // entries an archive can hold. + let max_entries = (max_total_bytes / 512 + 1).min(MAX_ARCHIVE_ENTRIES as u64); + Ok(Self { + file, + gzip, + // The tar crate buffers GNU long-name and PAX records whole before + // any per-entry budget check can run, so the reader itself has to + // be capped. The slack allows 1 KiB of framing for every entry the + // budget can hold plus the end-of-archive terminator, which keeps + // long-name-heavy archives acceptable while keeping the raw stream + // proportional to the caller's payload budget at any setting. + budget: max_total_bytes + .saturating_add(max_entries.saturating_mul(1024)) + .saturating_add(1024), + }) + } + + /// Starts one pass over the archive from offset 0. + fn pass(&self) -> Result>> { + let mut file = self + .file + .try_clone() + .context("reopen build context archive")?; + file.seek(SeekFrom::Start(0)) + .context("rewind build context archive")?; + + let reader: Box = if self.gzip { + Box::new(flate2::read::GzDecoder::new(BufReader::new(file)).take(self.budget)) + } else { + Box::new(BufReader::new(file).take(self.budget)) + }; + Ok(tar::Archive::new(reader)) + } +} + +/// Context for a per-entry read failure. +/// +/// The reader is capped, so an entry that declares more bytes than the budget +/// allows surfaces here as a truncated-archive error rather than an unbounded +/// allocation; naming the limit keeps that case diagnosable. +fn entry_read_context(max_total_bytes: u64) -> String { + format!( + "read build context archive entry; the archive must stay within the configured \ + limit of {max_total_bytes} bytes" + ) +} + +fn check_entry_type(entry_type: tar::EntryType) -> Result<()> { + match entry_type { + tar::EntryType::Regular + | tar::EntryType::Directory + | tar::EntryType::Symlink + | tar::EntryType::GNUSparse => Ok(()), + // Metadata-only companion entries (long names, pax headers) are + // consumed by the tar crate itself and never surface here. + other => bail!("unsupported entry type {other:?} in build context archive"), + } +} + +/// First pass: index entry paths and enforce the archive budgets without +/// reading any file contents. +fn read_entry_index(source: &SourceArchive, max_total_bytes: u64) -> Result> { + let mut archive = source.pass()?; + let mut index = Vec::new(); + let mut total_bytes = 0u64; + + for entry in archive + .entries() + .context("read build context archive entries")? + { + let entry = entry.with_context(|| entry_read_context(max_total_bytes))?; + let entry_type = entry.header().entry_type(); + check_entry_type(entry_type)?; + + // Count the entry's own header block and trailing padding: an archive + // of many empty files still costs real bytes to stream. + let padded_size = entry + .size() + .checked_next_multiple_of(512) + .unwrap_or(u64::MAX); + total_bytes = total_bytes.saturating_add(512).saturating_add(padded_size); + if total_bytes > max_total_bytes { + bail!( + "build context archive expands beyond the configured limit of \ + {max_total_bytes} bytes" + ); + } + if index.len() >= MAX_ARCHIVE_ENTRIES { + bail!("build context archive holds more than {MAX_ARCHIVE_ENTRIES} entries"); + } + + index.push(EntryIndex { + path: normalize_entry_path(&entry.path().context("entry path")?)?, + is_dir: entry_type == tar::EntryType::Directory, + }); + } + + if index.is_empty() { + bail!("build context archive contains no files"); + } + Ok(index) +} + +/// Final guest paths for every indexed entry, plus what the guest still has to +/// do for the destination root itself. +struct MappedEntries { + /// Positionally aligned with the entry index; `None` marks an entry the + /// rewrite drops. + targets: Vec>, + /// Resolved absolute destination root. + dest_root: String, + /// Whether a directory entry for `dest_root` itself was dropped. + skipped_dest_root: bool, + /// Whether the copy treats `dest_root` as a directory. + dest_is_dir: bool, +} + +/// Computes the final absolute guest path for every indexed entry. +fn map_entries( + index: &[EntryIndex], + src: &str, + dest_raw: &str, + workdir: &str, +) -> Result { + let src = normalize_src(src); + let dest_is_dir_hint = dest_raw.ends_with('/') + || dest_raw.ends_with("/.") + || dest_raw == "." + || dest_raw.is_empty(); + let dest = resolve_guest_path(workdir, if dest_raw.is_empty() { "." } else { dest_raw })?; + + let copy_whole_context = src.is_empty() || src == "."; + // Docker gives a wildcard that resolves to exactly one regular file the + // same destination semantics as a literal single-file source. A matched + // directory always contributes its own member, so `!is_dir` is what keeps + // directory sources on the recursive path. + let single_file_src = !copy_whole_context + && index.len() == 1 + && !index[0].is_dir + && (index[0].path == src || glob_match(&src, &index[0].path)); + + let mapped: Vec = if single_file_src { + vec![if dest_is_dir_hint { + // The base name has to come from the resolved entry: the source + // may be a pattern, which is never a valid path component. + join_abs(&dest, base_name(&index[0].path)) + } else { + dest.clone() + }] + } else if copy_whole_context || !is_glob_pattern(&src) { + // Directory source: Docker copies the directory *contents* into dest. + let mut mapped = Vec::with_capacity(index.len()); + for entry in index { + let rel = if copy_whole_context { + entry.path.as_str() + } else if entry.path == src { + "" + } else if let Some(rel) = entry.path.strip_prefix(&format!("{src}/")) { + rel + } else { + bail!( + "archive entry '{}' does not belong to COPY source '{}'", + entry.path, + src + ); + }; + mapped.push(join_abs(&dest, rel)); + } + mapped + } else { + // Glob source: every matched top-level item lands inside dest. Matched + // files keep their base name; matched directories contribute their + // contents (Docker treats each matched directory like a directory + // source). + let mut mapped = Vec::with_capacity(index.len()); + for entry in index { + let mut components = entry.path.split('/'); + let mut prefix = String::new(); + let mut matched_root: Option = None; + for component in components.by_ref() { + if prefix.is_empty() { + prefix.push_str(component); + } else { + prefix.push('/'); + prefix.push_str(component); + } + if glob_match(&src, &prefix) { + matched_root = Some(prefix.clone()); + break; + } + } + let Some(root) = matched_root else { + bail!( + "archive entry '{}' does not match COPY source pattern '{}'", + entry.path, + src + ); + }; + let rel = entry + .path + .strip_prefix(&root) + .map(|rest| rest.trim_start_matches('/')) + .unwrap_or(""); + mapped.push(if rel.is_empty() && !entry.is_dir { + join_abs(&dest, base_name(&root)) + } else { + join_abs(&dest, rel) + }); + } + mapped + }; + + // A directory source (and every glob-matched directory) maps its own root + // onto dest. Emitting a header for it would make the guest's `tar -xp` + // restore mode and ownership onto a pre-existing destination directory, + // which a copy must never touch; the guest creates it instead when absent. + let mut skipped_dest_root = false; + let targets = mapped + .into_iter() + .zip(index) + .map(|(target, entry)| { + if entry.is_dir && target == dest { + skipped_dest_root = true; + None + } else { + Some(target) + } + }) + .collect(); + + Ok(MappedEntries { + targets, + dest_root: dest, + skipped_dest_root, + // An explicit directory destination ("dest/") is a directory even for + // a single-file copy, and the guest still has to create it when it is + // missing: no archive member covers the destination root itself. + dest_is_dir: !single_file_src || dest_is_dir_hint, + }) +} + +/// Rewrites the SDK context archive into `output` with final absolute guest +/// paths, the requested ownership, and the optional mode override applied. +pub(crate) fn plan_copy_archive(request: &CopyRequest<'_>, output: &Path) -> Result { + let source = SourceArchive::open(request.source_tar, request.max_total_bytes)?; + let index = read_entry_index(&source, request.max_total_bytes)?; + let mapped = map_entries(&index, request.src, request.dest, request.workdir)?; + + let out_file = File::create(output) + .with_context(|| format!("create rewritten copy archive '{}'", output.display()))?; + let mut builder = tar::Builder::new(out_file); + let mut entry_count = 0usize; + let mut total_bytes = 0u64; + let mut seen = 0usize; + + // Second pass: stream each entry's bytes into the rewritten archive. + let mut archive = source.pass()?; + for entry in archive + .entries() + .context("read build context archive entries")? + { + let mut entry = entry.with_context(|| entry_read_context(request.max_total_bytes))?; + let Some(target) = mapped.targets.get(seen) else { + bail!("build context archive changed while it was being rewritten"); + }; + seen += 1; + let Some(target) = target else { + continue; + }; + + let relative = target.trim_start_matches('/'); + if relative.is_empty() { + // The destination root itself ("/"); parents always exist. + continue; + } + + let entry_type = entry.header().entry_type(); + check_entry_type(entry_type)?; + let link_name = entry + .link_name() + .context("entry link name")? + .map(|link| link.into_owned()); + let mut header = entry.header().clone(); + let (uid, gid) = request + .ownership + .map_or((0, 0), |owner| (owner.uid, owner.gid)); + header.set_uid(uid); + header.set_gid(gid); + // Clear the name fields so the numeric ids above are authoritative. + // GNU tar prefers uname/gname when they resolve in the target image, + // so leaving the uploader's account names in place could hand files + // to an unrelated guest account. + header + .set_username("") + .and_then(|()| header.set_groupname("")) + .with_context(|| format!("clear ownership names on entry '{target}'"))?; + if let Some(mode) = request.mode { + header.set_mode(mode); + } + + match entry_type { + tar::EntryType::Directory => { + header.set_size(0); + builder + .append_data(&mut header, format!("{relative}/"), std::io::empty()) + .with_context(|| format!("write directory entry '{target}'"))?; + } + tar::EntryType::Symlink => { + let link = link_name.context("symlink entry is missing its target")?; + header.set_size(0); + builder + .append_link(&mut header, relative, &link) + .with_context(|| format!("write symlink entry '{target}'"))?; + } + _ => { + let size = entry.size(); + header.set_size(size); + // A GNU sparse entry is read back expanded, so the rewritten + // entry is a plain regular file. + header.set_entry_type(tar::EntryType::Regular); + builder + .append_data(&mut header, relative, &mut entry) + .with_context(|| format!("write file entry '{target}'"))?; + total_bytes += size; + } + } + entry_count += 1; + } + + if seen != mapped.targets.len() { + bail!("build context archive changed while it was being rewritten"); + } + + let mut out_file = builder.into_inner().context("finish rewritten archive")?; + out_file.flush().context("flush rewritten archive")?; + + Ok(CopyPlan { + entry_count, + total_bytes, + dest_root: mapped.dest_root, + skipped_dest_root: mapped.skipped_dest_root, + dest_is_dir: mapped.dest_is_dir, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeMap; + use tempfile::TempDir; + + const NO_LIMIT: u64 = u64::MAX; + + fn request<'a>( + source_tar: &'a Path, + src: &'a str, + dest: &'a str, + workdir: &'a str, + ) -> CopyRequest<'a> { + CopyRequest { + source_tar, + src, + dest, + workdir, + mode: None, + ownership: None, + max_total_bytes: NO_LIMIT, + } + } + + fn build_source_tar(dir: &Path, entries: &[(&str, Option<&str>)]) -> std::path::PathBuf { + // (path, Some(contents)) = file, (path, None) = directory + let tar_path = dir.join("source.tar"); + let file = File::create(&tar_path).expect("create tar"); + let mut builder = tar::Builder::new(file); + for (path, contents) in entries { + let mut header = tar::Header::new_gnu(); + header.set_uid(501); + header.set_gid(20); + match contents { + Some(data) => { + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o644); + header.set_size(data.len() as u64); + builder + .append_data(&mut header, path, data.as_bytes()) + .expect("append file"); + } + None => { + header.set_entry_type(tar::EntryType::Directory); + header.set_mode(0o755); + header.set_size(0); + builder + .append_data(&mut header, format!("{path}/"), std::io::empty()) + .expect("append dir"); + } + } + } + builder.finish().expect("finish tar"); + tar_path + } + + struct Rewritten { + kind: tar::EntryType, + uid: u64, + gid: u64, + mode: u32, + contents: String, + } + + fn rewritten_entries(path: &Path) -> BTreeMap { + let mut archive = tar::Archive::new(File::open(path).expect("open rewritten")); + let mut out = BTreeMap::new(); + for entry in archive.entries().expect("entries") { + let mut entry = entry.expect("entry"); + let path = entry.path().expect("path").to_string_lossy().into_owned(); + let kind = entry.header().entry_type(); + let uid = entry.header().uid().expect("uid"); + let gid = entry.header().gid().expect("gid"); + let mode = entry.header().mode().expect("mode"); + let mut contents = String::new(); + entry.read_to_string(&mut contents).expect("read"); + out.insert( + path, + Rewritten { + kind, + uid, + gid, + mode, + contents, + }, + ); + } + out + } + + #[test] + fn single_file_to_absolute_file_dest() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("hello.txt", Some("hello\n"))]); + let out = dir.path().join("out.tar"); + + let plan = + plan_copy_archive(&request(&tar, "hello.txt", "/hello.txt", "/"), &out).expect("plan"); + + assert_eq!(plan.entry_count, 1); + assert_eq!(plan.total_bytes, 6); + assert!( + !plan.dest_is_dir, + "a single-file dest needs no directory preparation" + ); + let entries = rewritten_entries(&out); + let entry = &entries["hello.txt"]; + assert_eq!(entry.kind, tar::EntryType::Regular); + assert_eq!(entry.uid, 0, "ownership must default to root"); + assert_eq!(entry.gid, 0); + assert_eq!(entry.contents, "hello\n"); + } + + #[test] + fn single_file_to_directory_dest() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("requirements.txt", Some("e2b\n"))]); + let out = dir.path().join("out.tar"); + + let plan = plan_copy_archive(&request(&tar, "requirements.txt", "/home/user/", "/"), &out) + .expect("plan"); + + assert_eq!(plan.dest_root, "/home/user"); + assert!( + plan.dest_is_dir, + "an explicit directory destination must be prepared by the guest" + ); + assert!(rewritten_entries(&out).contains_key("home/user/requirements.txt")); + } + + #[test] + fn relative_dest_resolves_against_workdir() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("config.py", Some("x = 1\n"))]); + let out = dir.path().join("out.tar"); + + plan_copy_archive(&request(&tar, "config.py", "conf/app.py", "/srv"), &out).expect("plan"); + + assert!(rewritten_entries(&out).contains_key("srv/conf/app.py")); + } + + #[test] + fn directory_source_copies_contents_into_dest() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar( + dir.path(), + &[ + ("app", None), + ("app/main.py", Some("print()\n")), + ("app/sub", None), + ("app/sub/util.py", Some("pass\n")), + ], + ); + let out = dir.path().join("out.tar"); + + let plan = + plan_copy_archive(&request(&tar, "app", "/opt/service", "/"), &out).expect("plan"); + + assert_eq!(plan.dest_root, "/opt/service"); + assert!( + plan.skipped_dest_root, + "the destination root must be left to the guest" + ); + assert!(plan.dest_is_dir); + let entries = rewritten_entries(&out); + assert!( + !entries.contains_key("opt/service/"), + "a header for the destination root would rewrite its metadata" + ); + assert!(entries.contains_key("opt/service/main.py")); + assert!(entries.contains_key("opt/service/sub/")); + assert!(entries.contains_key("opt/service/sub/util.py")); + } + + #[test] + fn whole_context_source_copies_everything() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar( + dir.path(), + &[ + ("a.txt", Some("a")), + ("sub", None), + ("sub/b.txt", Some("b")), + ], + ); + let out = dir.path().join("out.tar"); + + plan_copy_archive(&request(&tar, ".", "/workspace", "/"), &out).expect("plan"); + + let entries = rewritten_entries(&out); + assert!(entries.contains_key("workspace/a.txt")); + assert!(entries.contains_key("workspace/sub/b.txt")); + } + + #[test] + fn glob_source_places_matches_by_base_name() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar( + dir.path(), + &[("one.txt", Some("1")), ("two.txt", Some("2"))], + ); + let out = dir.path().join("out.tar"); + + let plan = plan_copy_archive(&request(&tar, "*.txt", "/data/", "/"), &out).expect("plan"); + + assert_eq!(plan.entry_count, 2); + let entries = rewritten_entries(&out); + assert!(entries.contains_key("data/one.txt")); + assert!(entries.contains_key("data/two.txt")); + } + + #[test] + fn glob_matching_one_file_renames_onto_a_file_dest() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("one.txt", Some("1"))]); + let out = dir.path().join("out.tar"); + + let plan = + plan_copy_archive(&request(&tar, "*.txt", "/renamed.txt", "/"), &out).expect("plan"); + + assert!( + !plan.dest_is_dir, + "a wildcard resolving to one file renames like a literal source" + ); + assert!(rewritten_entries(&out).contains_key("renamed.txt")); + } + + #[test] + fn glob_matching_one_file_keeps_its_name_under_a_directory_dest() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("one.txt", Some("1"))]); + let out = dir.path().join("out.tar"); + + plan_copy_archive(&request(&tar, "*.txt", "/data/", "/"), &out).expect("plan"); + + // The base name comes from the matched entry, never from the pattern. + assert!(rewritten_entries(&out).contains_key("data/one.txt")); + } + + #[test] + fn glob_matching_directory_copies_its_contents() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar( + dir.path(), + &[ + ("pkg-a", None), + ("pkg-a/lib.py", Some("a")), + ("pkg-b", None), + ("pkg-b/lib.py", Some("b")), + ], + ); + let out = dir.path().join("out.tar"); + + let plan = + plan_copy_archive(&request(&tar, "pkg-*", "/opt/pkgs", "/"), &out).expect("plan"); + + // Docker merges contents of every matched directory into dest; the + // second lib.py overwrites the first at extract time. + let entries = rewritten_entries(&out); + assert!(entries.contains_key("opt/pkgs/lib.py")); + assert_eq!(plan.dest_root, "/opt/pkgs"); + assert!(plan.skipped_dest_root); + assert!(!entries.contains_key("opt/pkgs/")); + } + + #[test] + fn mode_override_applies_to_entries() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("run.sh", Some("#!/bin/sh\n"))]); + let out = dir.path().join("out.tar"); + + let mut req = request(&tar, "run.sh", "/usr/local/bin/run.sh", "/"); + req.mode = Some(0o755); + plan_copy_archive(&req, &out).expect("plan"); + + assert_eq!(rewritten_entries(&out)["usr/local/bin/run.sh"].mode, 0o755); + } + + #[test] + fn ownership_is_written_into_entry_headers() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar( + dir.path(), + &[("app", None), ("app/main.py", Some("print()\n"))], + ); + let out = dir.path().join("out.tar"); + + let mut req = request(&tar, "app", "/opt/service", "/"); + req.ownership = Some(CopyOwnership { + uid: 1000, + gid: 2000, + }); + plan_copy_archive(&req, &out).expect("plan"); + + // Every created entry carries the requested ownership, and nothing + // outside the archive can be affected. + for entry in rewritten_entries(&out).values() { + assert_eq!(entry.uid, 1000); + assert_eq!(entry.gid, 2000); + } + } + + #[test] + fn gzip_archives_are_accepted() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("hello.txt", Some("hi"))]); + let gz_path = dir.path().join("source.tar.gz"); + let mut encoder = flate2::write::GzEncoder::new( + File::create(&gz_path).expect("create gz"), + flate2::Compression::fast(), + ); + std::io::copy(&mut File::open(&tar).expect("open tar"), &mut encoder).expect("compress"); + encoder.finish().expect("finish gz"); + let out = dir.path().join("out.tar"); + + let plan = plan_copy_archive(&request(&gz_path, "hello.txt", "/hello.txt", "/"), &out) + .expect("plan"); + assert_eq!(plan.entry_count, 1); + } + + #[test] + fn rejects_archives_over_the_decompressed_budget() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("big.txt", Some("0123456789"))]); + let out = dir.path().join("out.tar"); + + let mut req = request(&tar, "big.txt", "/big.txt", "/"); + req.max_total_bytes = 4; + let err = plan_copy_archive(&req, &out).expect_err("oversized archive must fail"); + assert!(err.to_string().contains("expands beyond")); + } + + #[test] + fn rejects_truncated_long_name_records() { + let dir = TempDir::new().expect("tempdir"); + let inner = build_source_tar(dir.path(), &[("small.txt", Some("s"))]); + + // A GNU long-name record declaring far more bytes than it carries. + // The tar crate buffers such a record whole; the capped reader bounds + // that allocation and the overdeclared record fails as truncated + // instead of being served the rest of the stream as name bytes. The + // cap scales with the configured budget, so the 64 KiB limit below + // bounds the allocation at KiB rather than MiB scale. + let mut header = tar::Header::new_gnu(); + let long_link = b"././@LongLink"; + header.as_gnu_mut().expect("gnu header").name[..long_link.len()].copy_from_slice(long_link); + header.set_entry_type(tar::EntryType::GNULongName); + header.set_mode(0o644); + header.set_size(0o77777777777); + header.set_cksum(); + + let mut bytes = Vec::new(); + bytes.extend_from_slice(header.as_bytes()); + bytes.extend_from_slice(&[0u8; 512]); + bytes.extend_from_slice(&std::fs::read(&inner).expect("read inner tar")); + let tar_path = dir.path().join("longname.tar"); + std::fs::write(&tar_path, &bytes).expect("write tar"); + let out = dir.path().join("out.tar"); + + let mut req = request(&tar_path, "small.txt", "/small.txt", "/"); + req.max_total_bytes = 64 * 1024; + let err = plan_copy_archive(&req, &out).expect_err("overdeclared long-name must fail"); + assert!(err.to_string().contains("configured limit")); + } + + #[test] + fn counts_entry_framing_against_the_budget() { + let dir = TempDir::new().expect("tempdir"); + // Twenty empty files carry zero payload bytes but 512 bytes of tar + // framing each, which the index pass must charge to the budget. + let entries: Vec<(String, Option<&str>)> = (0..20) + .map(|i| (format!("empty-{i}.txt"), Some(""))) + .collect(); + let entries: Vec<(&str, Option<&str>)> = entries + .iter() + .map(|(name, content)| (name.as_str(), *content)) + .collect(); + let tar = build_source_tar(dir.path(), &entries); + let out = dir.path().join("out.tar"); + + let mut req = request(&tar, ".", "/ctx/", "/"); + req.max_total_bytes = 4 * 1024; + let err = plan_copy_archive(&req, &out).expect_err("framing must exhaust the budget"); + assert!(err.to_string().contains("expands beyond")); + } + + #[test] + fn rejects_entries_escaping_the_root() { + let dir = TempDir::new().expect("tempdir"); + let tar_path = dir.path().join("evil.tar"); + let file = File::create(&tar_path).expect("create tar"); + let mut builder = tar::Builder::new(file); + // `append_data` refuses to write `..` paths, so craft the header + // manually the way a hostile client would. + let mut header = tar::Header::new_gnu(); + let evil_path = b"../../etc/passwd"; + header.as_gnu_mut().expect("gnu header").name[..evil_path.len()].copy_from_slice(evil_path); + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o644); + header.set_size(4); + header.set_cksum(); + builder + .append(&header, "pwn\n".as_bytes()) + .expect("append raw entry"); + builder.finish().expect("finish"); + let out = dir.path().join("out.tar"); + + let err = plan_copy_archive(&request(&tar_path, "passwd", "/tmp/x", "/"), &out) + .expect_err("path escape must fail"); + assert!(err.to_string().contains("unsupported path component")); + } + + #[test] + fn rejects_non_utf8_entry_names() { + let dir = TempDir::new().expect("tempdir"); + let tar_path = dir.path().join("latin1.tar"); + let file = File::create(&tar_path).expect("create tar"); + let mut builder = tar::Builder::new(file); + let mut header = tar::Header::new_gnu(); + // A latin-1 name; lossy conversion would silently rename it and + // collapse distinct names onto one replacement-character path. + let raw_name = b"caf\xe9.txt"; + header.as_gnu_mut().expect("gnu header").name[..raw_name.len()].copy_from_slice(raw_name); + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o644); + header.set_size(1); + header.set_cksum(); + builder + .append(&header, "x".as_bytes()) + .expect("append raw entry"); + builder.finish().expect("finish"); + let out = dir.path().join("out.tar"); + + let err = plan_copy_archive(&request(&tar_path, ".", "/ctx/", "/"), &out) + .expect_err("non-UTF-8 entry name must fail"); + assert!(err.to_string().contains("non-UTF-8 path component")); + } + + #[test] + fn rejects_dest_escaping_the_root() { + let dir = TempDir::new().expect("tempdir"); + let tar = build_source_tar(dir.path(), &[("a.txt", Some("a"))]); + let out = dir.path().join("out.tar"); + + let err = plan_copy_archive(&request(&tar, "a.txt", "../../x", "/"), &out) + .expect_err("dest escape must fail"); + assert!(err.to_string().contains("escapes the filesystem root")); + } + + #[test] + fn rejects_empty_archive() { + let dir = TempDir::new().expect("tempdir"); + let tar_path = dir.path().join("empty.tar"); + let file = File::create(&tar_path).expect("create tar"); + tar::Builder::new(file).finish().expect("finish"); + let out = dir.path().join("out.tar"); + + let err = plan_copy_archive(&request(&tar_path, "x", "/x", "/"), &out) + .expect_err("empty archive must fail"); + assert!(err.to_string().contains("no files")); + } + + #[test] + fn guest_path_resolution_follows_docker_semantics() { + assert_eq!( + resolve_guest_path("/srv", "app").expect("relative"), + "/srv/app" + ); + assert_eq!( + resolve_guest_path("/srv/app", "/opt").expect("absolute"), + "/opt" + ); + assert_eq!( + resolve_guest_path("/srv/app", "../lib").expect("parent"), + "/srv/lib" + ); + assert_eq!(resolve_guest_path("", "opt").expect("empty base"), "/opt"); + assert!(resolve_guest_path("relative", "app").is_err()); + assert!(resolve_guest_path("/", "../escape").is_err()); + } + + #[test] + fn glob_match_basics() { + assert!(glob_match("*.txt", "a.txt")); + assert!(!glob_match("*.txt", "a.txt.bak")); + assert!(glob_match("data?", "data1")); + assert!(glob_match("[ab]*", "b12")); + assert!(!glob_match("[!ab]*", "b12")); + assert!(glob_match("pkg-*", "pkg-a")); + } + + #[test] + fn glob_wildcards_never_cross_a_separator() { + assert!(!glob_match("*.txt", "sub/a.txt")); + assert!(!glob_match("src?nested", "src/nested")); + assert!(!glob_match("[sa]rc", "src/nested")); + assert!(glob_match("src/*.rs", "src/main.rs")); + assert!(!glob_match("src/*.rs", "src/nested/main.rs")); + assert!(!glob_match("src/*", "src")); + } + + #[test] + fn glob_match_stays_polynomial_on_pathological_patterns() { + // The previous recursive matcher took minutes on this input. + assert!(!glob_match("*a*a*a*a*a*a*a*a*b", &"a".repeat(64))); + assert!(glob_match( + "*a*a*a*a*a*a*a*a*b", + &format!("{}b", "a".repeat(64)) + )); + } +} diff --git a/src/template/mod.rs b/src/template/mod.rs index 57800f46..53aaabd6 100644 --- a/src/template/mod.rs +++ b/src/template/mod.rs @@ -1,5 +1,6 @@ mod build_spec; mod builder; +mod copy_plan; mod errors; mod runner; mod step_executor; diff --git a/src/template/runner.rs b/src/template/runner.rs index 1879f11e..ff7c5dde 100644 --- a/src/template/runner.rs +++ b/src/template/runner.rs @@ -64,6 +64,9 @@ pub(crate) struct TemplateBuildContext { // Keep the tempdir owner alive for the duration of build + publish. pub workspace: TempDir, pub steps: Vec, + /// Node-local archives for COPY steps, keyed by files hash. Populated by + /// the builder before execution. + pub build_archives: std::collections::HashMap, pub base: TemplateBuildBase, pub cpu_config_json: Option, } @@ -197,6 +200,7 @@ impl TemplateBuildRunner { let worker_span = tracing::debug_span!("template_build_sandbox", sandbox_id = %sandbox_id); let step_executor = self.step_executor.clone(); let steps = context.steps.clone(); + let build_archives = context.build_archives.clone(); let output_dir = context.local_dir().to_path_buf(); let initial_context = context.initial_context.clone(); let startup = context.startup.clone(); @@ -220,7 +224,7 @@ impl TemplateBuildRunner { debug!("template build sandbox started"); let build_context = step_executor - .execute(&sandbox, &steps, initial_context) + .execute(&sandbox, &steps, initial_context, &build_archives) .await?; let startup = prepare_startup(startup, override_startup, &build_context); run_startup_commands(&sandbox, startup.as_ref()).await?; diff --git a/src/template/step_executor.rs b/src/template/step_executor.rs index a920f552..df91e885 100644 --- a/src/template/step_executor.rs +++ b/src/template/step_executor.rs @@ -1,13 +1,42 @@ use std::collections::HashMap; +use std::path::PathBuf; use anyhow::{Context, Result}; +use shell_util::shell_quote; use tracing::debug; use super::build_spec::{TemplateBuildStep, TemplateBuildStepKind}; +use super::copy_plan::{plan_copy_archive, resolve_guest_path, CopyOwnership, CopyRequest}; use super::errors::{command_output_suffix, TemplateBuildFailure}; +use crate::cfg::ConfigManager; use crate::sandbox::{ProcessOpts, SandboxExecutor}; use crate::snapshot::CommandContext; +/// Validates a Docker-style `--chown` value (`user`, `uid`, `user:group`). +/// +/// The value ends up in a shell command inside the build sandbox (quoted), so +/// this stays conservative rather than mirroring every libc name rule. +fn is_valid_chown_spec(spec: &str) -> bool { + fn valid_part(part: &str) -> bool { + // A leading '-' would be read as an option by the `id` lookup below. + !part.is_empty() + && !part.starts_with('-') + && part + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-' | b'.')) + } + let mut parts = spec.split(':'); + let (user, group, extra) = (parts.next(), parts.next(), parts.next()); + if extra.is_some() { + return false; + } + match (user, group) { + (Some(user), None) => valid_part(user), + (Some(user), Some(group)) => valid_part(user) && valid_part(group), + _ => false, + } +} + #[derive(Clone, Debug, Default)] pub(crate) struct TemplateStepExecutor; @@ -17,7 +46,7 @@ impl TemplateStepExecutor { } #[tracing::instrument( - skip(self, sandbox, steps, initial_context), + skip(self, sandbox, steps, initial_context, build_archives), fields(step_count = steps.len()) )] pub(crate) async fn execute( @@ -25,6 +54,7 @@ impl TemplateStepExecutor { sandbox: &impl SandboxExecutor, steps: &[TemplateBuildStep], initial_context: CommandContext, + build_archives: &HashMap, ) -> Result { let mut context = initial_context; @@ -35,7 +65,46 @@ impl TemplateStepExecutor { context = context.with_env_var(key.clone(), value.clone()); } TemplateBuildStepKind::Workdir { path } => { - context = context.with_workdir(path.to_string_lossy()); + // Docker resolves a relative WORKDIR against the current + // one; only absolute values replace it outright. + let path = path.to_string_lossy(); + let resolved = + resolve_guest_path(&context.workdir, &path).with_context(|| { + TemplateBuildFailure::with_step( + format!("build step failed: invalid workdir '{path}'"), + format!("WORKDIR {path}"), + ) + })?; + // Docker creates the directory. A bare `mkdir -p` matches + // the classic builder: idempotent on an existing directory + // without touching its metadata, and ENOTDIR on a file. + // No cwd, because the current workdir may not exist. + let script = format!("set -eu\nmkdir -p -- {}\n", shell_quote(&resolved)); + let output = sandbox + .run_command_with_opts( + "/bin/bash", + &["-lc", &script], + &ProcessOpts::default(), + ) + .await + .with_context(|| { + TemplateBuildFailure::with_step( + "build step failed: create workdir", + format!("WORKDIR {path}"), + ) + })?; + if output.exit_code != 0 { + return Err(TemplateBuildFailure::with_step( + format!( + "build step failed: creating the workdir exited with status {}{}", + output.exit_code, + command_output_suffix(&output.stdout, &output.stderr) + ), + format!("WORKDIR {path}"), + ) + .into()); + } + context = context.with_workdir(resolved); } TemplateBuildStepKind::User { value } => { context = context.with_user(Some(value.clone())); @@ -63,6 +132,25 @@ impl TemplateStepExecutor { self.run_step(sandbox, &context.workdir, &context.env_vars, cmd) .await?; } + TemplateBuildStepKind::Copy { + src, + dest, + files_hash, + user, + mode, + } => { + self.copy_step( + sandbox, + &context, + build_archives, + src, + dest, + files_hash, + user.as_deref(), + *mode, + ) + .await?; + } } } debug!("template build steps completed"); @@ -70,6 +158,209 @@ impl TemplateStepExecutor { Ok(context) } + /// Applies one COPY step: rewrites the uploaded context archive to final + /// absolute guest paths on the host, streams it into the sandbox via + /// envd, and extracts it at `/` inside the guest. + #[allow(clippy::too_many_arguments)] + async fn copy_step( + &self, + sandbox: &impl SandboxExecutor, + context: &CommandContext, + build_archives: &HashMap, + src: &str, + dest: &str, + files_hash: &str, + user: Option<&str>, + mode: Option, + ) -> Result<()> { + let step_label = format!("COPY {src} {dest}"); + let with_step = |message: String| TemplateBuildFailure::with_step(message, &step_label); + + let archive = build_archives.get(files_hash).ok_or_else(|| { + with_step(format!( + "build step failed: build context archive '{files_hash}' has not been uploaded" + )) + })?; + + if let Some(user) = user { + if !is_valid_chown_spec(user) { + return Err( + with_step(format!("build step failed: invalid COPY user '{user}'")).into(), + ); + } + } + + // Resolve --chown against the image's own accounts, like Docker, and + // bake the numeric result into the archive headers. Applying it after + // extraction with `chown -R` would also rewrite pre-existing files + // under the destination. + let ownership = match user { + Some(user) => Some(self.resolve_ownership(sandbox, user, &step_label).await?), + None => None, + }; + + let rewritten = tempfile::Builder::new() + .prefix("agentenv-copy-") + .suffix(".tar") + .tempfile() + .context("create rewritten copy archive")?; + let plan = plan_copy_archive( + &CopyRequest { + source_tar: archive, + src, + dest, + workdir: &context.workdir, + mode, + ownership, + max_total_bytes: ConfigManager::global_config() + .template_build + .files_max_context_mib + .saturating_mul(1024 * 1024), + }, + rewritten.path(), + ) + .map_err(|error| with_step(format!("build step failed: {error:#}")))?; + debug!( + files_hash, + entries = plan.entry_count, + bytes = plan.total_bytes, + "prepared copy archive" + ); + + let guest_archive = format!("/tmp/.agentenv-copy-{}.tar", uuid::Uuid::new_v4()); + sandbox + .upload_file(rewritten.path(), &guest_archive, "root") + .await + .with_context(|| with_step("build step failed: upload build context".to_string()))?; + + // The plan never emits a header for the destination root, so a + // pre-existing destination keeps its metadata. Create it here when it + // is missing so a fresh destination still gets the requested + // ownership; `tar -C /` auto-creates any missing intermediate + // directory. File-only archives carry no directory member for the + // root, hence the `dest_is_dir` gate rather than `skipped_dest_root` + // alone. + let mut script = String::new(); + if plan.skipped_dest_root || plan.dest_is_dir { + let dest_root = shell_quote(&plan.dest_root); + script.push_str(&format!("if [ ! -e {dest_root} ]; then\n")); + script.push_str(&format!(" mkdir -p -- {dest_root} || exit 1\n")); + if let Some(owner) = ownership { + script.push_str(&format!( + " chown {}:{} -- {dest_root} || exit 1\n", + owner.uid, owner.gid + )); + } + // `--chmod` applies to copied content only. `skipped_dest_root` + // means the archive itself carried the destination directory, so + // the mode is the one that entry would have received; a directory + // synthesized for a single-file copy is not copied content and + // must not take the file's mode. + if let Some(mode) = mode.filter(|_| plan.skipped_dest_root) { + script.push_str(&format!(" chmod {mode:o} -- {dest_root} || exit 1\n")); + } + script.push_str("fi\n"); + } + // `--no-overwrite-dir` (GNU tar) keeps extraction from restoring mode + // and ownership onto directories that already exist in the image; + // directories tar creates still receive the archive header's metadata. + script.push_str(&format!( + "tar -xp --no-overwrite-dir -f {archive} -C /\nrc=$?\nrm -f {archive}\nif [ $rc -ne 0 ]; then exit $rc; fi\n", + archive = shell_quote(&guest_archive), + )); + + let output = sandbox + .run_command_with_opts("/bin/bash", &["-lc", &script], &ProcessOpts::default()) + .await + .with_context(|| with_step("build step failed".to_string()))?; + if output.exit_code != 0 { + let message = format!( + "build step failed: extracting the build context exited with status {}{}", + output.exit_code, + command_output_suffix(&output.stdout, &output.stderr) + ); + return Err(with_step(message).into()); + } + Ok(()) + } + + /// Resolves a `--chown` spec to numeric ids inside the build sandbox. + /// + /// Docker resolves names against the image's own `/etc/passwd` and + /// `/etc/group`, so the lookup has to happen in the guest. A failed lookup + /// fails the step the way Docker does rather than silently falling back to + /// root. + async fn resolve_ownership( + &self, + sandbox: &impl SandboxExecutor, + user: &str, + step_label: &str, + ) -> Result { + let (user_part, group_part) = user.split_once(':').unwrap_or((user, "")); + let script = format!( + r#"set -eu +case "{user}" in + *[!0-9]*) uid=$(id -u -- "{user}"); ugid=$(id -g -- "{user}") ;; + *) uid="{user}"; ugid="{user}" ;; +esac +if [ -n "{group}" ]; then + case "{group}" in + *[!0-9]*) gid=$(awk -F: -v n="{group}" '$1==n{{print $3; f=1}} END{{exit !f}}' /etc/group) ;; + *) gid="{group}" ;; + esac +else + gid="$ugid" +fi +printf '%s %s\n' "$uid" "$gid" +"#, + user = user_part, + group = group_part, + ); + + let output = sandbox + .run_command_with_opts("/bin/bash", &["-lc", &script], &ProcessOpts::default()) + .await + .with_context(|| { + TemplateBuildFailure::with_step( + "build step failed: resolve COPY ownership".to_string(), + step_label, + ) + })?; + if output.exit_code != 0 { + // A nonzero exit also covers a missing group, a `/etc/group` the + // lookup cannot read, and a guest without `id`/`awk`, so the + // message must not assert that the user is absent. + return Err(TemplateBuildFailure::with_step( + format!( + "build step failed: could not resolve COPY ownership '{user}' in the image; \ + the user or group may not exist{}", + command_output_suffix(&output.stdout, &output.stderr) + ), + step_label, + ) + .into()); + } + + let parsed = output + .stdout + .split_whitespace() + .map(str::parse::) + .collect::, _>>() + .ok() + .filter(|ids| ids.len() == 2); + let Some(ids) = parsed else { + return Err(TemplateBuildFailure::with_step( + format!("build step failed: could not resolve COPY user '{user}'"), + step_label, + ) + .into()); + }; + Ok(CopyOwnership { + uid: ids[0], + gid: ids[1], + }) + } + async fn run_step( &self, sandbox: &impl SandboxExecutor, @@ -103,14 +394,20 @@ impl TemplateStepExecutor { #[cfg(test)] mod tests { + use std::collections::HashMap; + use std::sync::Mutex; + use anyhow::{anyhow, Result}; use async_trait::async_trait; + use shell_util::shell_quote; use super::TemplateStepExecutor; use crate::sandbox::{Executor, ProcessHandle, ProcessOpts, ProcessOutput, SandboxExecutor}; use crate::snapshot::CommandContext; use crate::template::build_spec::TemplateBuildStep; + /// Sandbox that fails any exec, so steps expected to stay host-side prove + /// they never touch the guest. struct NoopSandbox; #[async_trait(?Send)] @@ -136,9 +433,96 @@ mod tests { } } + /// One recorded exec: command, arguments, and working directory. + type RecordedCommand = (String, Vec, Option); + + /// Records every command a step issues and replays a canned result. + #[derive(Default)] + struct RecordingSandbox { + commands: Mutex>, + /// Stdout every recorded command reports back. + stdout: String, + /// Exit code every recorded command reports back. + exit_code: i32, + } + + impl RecordingSandbox { + fn commands(&self) -> Vec { + self.commands + .lock() + .expect("commands mutex should not be poisoned") + .clone() + } + } + + #[async_trait(?Send)] + impl SandboxExecutor for RecordingSandbox { + fn executor(&self) -> Result> { + Err(anyhow!("not used by this test")) + } + async fn run_command_with_opts( + &self, + cmd: &str, + args: &[&str], + opts: &ProcessOpts, + ) -> Result { + self.commands + .lock() + .expect("commands mutex should not be poisoned") + .push(( + cmd.to_string(), + args.iter().map(|arg| (*arg).to_string()).collect(), + opts.cwd.clone(), + )); + Ok(ProcessOutput { + stdout: self.stdout.clone(), + stderr: String::new(), + exit_code: self.exit_code, + }) + } + async fn start_process( + &self, + _cmd: &str, + _args: &[&str], + _opts: &ProcessOpts, + ) -> Result { + Err(anyhow!("not used by this test")) + } + async fn upload_file( + &self, + _local_path: &std::path::Path, + _guest_path: &str, + _username: &str, + ) -> Result<()> { + Ok(()) + } + } + + /// Writes a one-file build context archive and returns its path. + fn single_file_archive(dir: &std::path::Path) -> std::path::PathBuf { + let tar_path = dir.join("context.tar"); + let mut builder = + tar::Builder::new(std::fs::File::create(&tar_path).expect("create context tar")); + let contents = b"e2b\n"; + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o644); + header.set_size(contents.len() as u64); + builder + .append_data(&mut header, "requirements.txt", &contents[..]) + .expect("append file"); + builder.finish().expect("finish context tar"); + tar_path + } + async fn run(steps: Vec) -> CommandContext { TemplateStepExecutor::new() - .execute(&NoopSandbox, &steps, CommandContext::default()) + .execute( + &NoopSandbox, + &steps, + CommandContext::default(), + &HashMap::new(), + ) .await .expect("steps should execute without error") } @@ -153,7 +537,12 @@ mod tests { async fn user_step_overrides_base_image_user() { let initial = CommandContext::default().with_user(Some("root".to_string())); let ctx = TemplateStepExecutor::new() - .execute(&NoopSandbox, &[TemplateBuildStep::user("zzz")], initial) + .execute( + &NoopSandbox, + &[TemplateBuildStep::user("zzz")], + initial, + &HashMap::new(), + ) .await .unwrap(); assert_eq!(ctx.user.as_deref(), Some("zzz")); @@ -178,12 +567,117 @@ mod tests { &NoopSandbox, &[TemplateBuildStep::exposed_port("8080")], initial, + &HashMap::new(), ) .await .unwrap(); assert_eq!(ctx.exposed_ports, vec!["8080"]); } + #[tokio::test] + async fn copy_step_fails_without_uploaded_archive() { + let err = TemplateStepExecutor::new() + .execute( + &NoopSandbox, + &[TemplateBuildStep::copy( + "hello.txt", + "/hello.txt", + "aabbccddeeff0011", + None, + None, + )], + CommandContext::default(), + &HashMap::new(), + ) + .await + .expect_err("missing archive should fail the step"); + assert!(err.to_string().contains("has not been uploaded")); + } + + #[tokio::test] + async fn copy_step_prepares_a_directory_dest_without_chmod() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let mut archives = HashMap::new(); + archives.insert( + "aabbccddeeff0011".to_string(), + single_file_archive(dir.path()), + ); + let sandbox = RecordingSandbox { + stdout: "1000 2000\n".to_string(), + ..RecordingSandbox::default() + }; + + TemplateStepExecutor::new() + .execute( + &sandbox, + &[TemplateBuildStep::copy( + "requirements.txt", + "/home/user/", + "aabbccddeeff0011", + Some("1000:2000".to_string()), + Some(0o600), + )], + CommandContext::default(), + &archives, + ) + .await + .expect("copy step should execute"); + + let commands = sandbox.commands(); + let script = &commands.last().expect("extraction command").1[1]; + let dest_root = shell_quote("/home/user"); + assert!( + script.contains(&format!("mkdir -p -- {dest_root}")), + "a missing directory destination must be created: {script}" + ); + assert!( + script.contains(&format!("chown 1000:2000 -- {dest_root}")), + "a created destination must carry the requested ownership: {script}" + ); + assert!( + !script.contains("chmod"), + "--chmod applies to copied content, not to a synthesized destination: {script}" + ); + } + + #[tokio::test] + async fn failed_ownership_lookup_does_not_claim_the_user_is_absent() { + let sandbox = RecordingSandbox { + exit_code: 1, + ..RecordingSandbox::default() + }; + + let err = TemplateStepExecutor::new() + .resolve_ownership(&sandbox, "root:missing-group", "COPY a b") + .await + .expect_err("a failed ownership lookup must fail the step"); + + let message = err.to_string(); + assert!( + message.contains("could not resolve COPY ownership 'root:missing-group'"), + "unexpected message: {message}" + ); + assert!( + !message.contains("does not exist"), + "the same exit also covers a missing group or an unusable lookup: {message}" + ); + } + + #[test] + fn chown_spec_validation() { + assert!(super::is_valid_chown_spec("user")); + assert!(super::is_valid_chown_spec("user:group")); + assert!(super::is_valid_chown_spec("1000:1000")); + assert!(super::is_valid_chown_spec("www-data")); + assert!(!super::is_valid_chown_spec("")); + assert!(!super::is_valid_chown_spec("user:")); + assert!(!super::is_valid_chown_spec("user:group:extra")); + assert!(!super::is_valid_chown_spec("user name")); + assert!(!super::is_valid_chown_spec("user;rm -rf /")); + assert!(!super::is_valid_chown_spec("-r")); + assert!(!super::is_valid_chown_spec("user:-g")); + } + #[tokio::test] async fn volume_deduplicates() { let ctx = run(vec![ @@ -203,7 +697,50 @@ mod tests { #[tokio::test] async fn workdir_step_updates_workdir() { - let ctx = run(vec![TemplateBuildStep::workdir("/workspace")]).await; + let sandbox = RecordingSandbox::default(); + let ctx = TemplateStepExecutor::new() + .execute( + &sandbox, + &[TemplateBuildStep::workdir("/workspace")], + CommandContext::default(), + &HashMap::new(), + ) + .await + .expect("steps should execute without error"); + assert_eq!(ctx.workdir, "/workspace"); + let commands = sandbox.commands(); + assert_eq!(commands.len(), 1); + assert!( + commands[0].1[1].contains(&shell_quote("/workspace")), + "WORKDIR must create the directory: {}", + commands[0].1[1] + ); + } + + #[tokio::test] + async fn workdir_step_resolves_relative_paths() { + let sandbox = RecordingSandbox::default(); + let ctx = TemplateStepExecutor::new() + .execute( + &sandbox, + &[ + TemplateBuildStep::workdir("/a"), + TemplateBuildStep::workdir("b"), + ], + CommandContext::default(), + &HashMap::new(), + ) + .await + .expect("steps should execute without error"); + + assert_eq!(ctx.workdir, "/a/b"); + let commands = sandbox.commands(); + assert_eq!(commands.len(), 2); + assert!( + commands[1].1[1].contains(&shell_quote("/a/b")), + "the second WORKDIR must create the resolved path: {}", + commands[1].1[1] + ); } }