[3/4] feat(template): host-side COPY plan (archive rewrite to guest paths) by JoyboyBrian · Pull Request #73 · kvcache-ai/AgentENV

JoyboyBrian · 2026-07-29T20:53:17Z

What

Add a host-side COPY planner that reads an uploaded build-context archive and emits a bounded rewritten tar whose entries are mapped to their final guest paths.

This is layer 3 of the active COPY/ADD stack, replayed onto current main at 00ba6cb without #70.

Layer	PR	Branch	Scope
1/4	#71	`e2b-build/03-build-file-store`	Build-context archive store
2/4	#72	`e2b-build/04-upload-api`	Upload API and configuration
3/4	#73	`e2b-build/05-copy-plan`	Host-side COPY planning
4/4	#74	`e2b-build/06-copy-exec`	COPY/ADD execution, E2E, and documentation

Why

COPY/ADD requires Docker-compatible source matching and destination mapping without allowing an untrusted context archive to escape the intended guest paths. Keeping this logic host-side makes validation deterministic before anything is uploaded to the build sandbox.

Related: #28. The original PR remains the full concept/reference discussion.

Scope and non-goals

This layer adds only src/template/copy_plan.rs and its private module declaration. It has no runtime caller until #74, so the temporary dead_code allowance is removed by the next layer. Storage, HTTP upload, guest transfer/extraction, and build-spec wiring are outside this PR.

Exact layer diff: e2b-build/04-upload-api...e2b-build/05-copy-plan

Design

The archive is opened once and processed in two bounded passes.
The first pass normalizes/indexes paths and computes source-to-destination mappings.
The second pass streams selected entries into a rewritten tar with final absolute paths and requested ownership/mode metadata.
Traversal, unsafe links, unsupported entry types, invalid paths, excessive entry counts, and decompressed-byte budget violations are rejected.
Gzip contexts, glob expansion, file/directory destinations, and metadata overrides are covered by unit tests.

Compatibility and operations

Pure internal logic with no public API or configuration change.
No snapshot/storage format change.
No new crate or host dependency.
No behavior is enabled until [4/4] feat(template): execute COPY/ADD steps and document E2B builds #74 wires the planner into template execution.

Validation

The rewritten stack was validated on Linux against the #74 tip:

cargo fmt --all -- --check
cargo test --locked -p agentenv --lib template::
# 68 passed, 0 failed
cargo clippy --locked -p agentenv --lib -- -D warnings

This layer's focused tests cover path resolution, globs, destination semantics, metadata, gzip, traversal/link safety, unsupported types, and entry/byte budgets. GitHub CI validates this cumulative branch independently.

Reviewer notes

Archive path rewriting and decompression bounds are the primary security review surfaces. #70 is not required to merge this stack.

github-actions · 2026-07-29T21:07:54Z

🔍 OpenCodeReview found 21 issue(s) in this PR.

✅ Successfully posted inline: 20 comment(s)
❌ Failed to post inline: 1 comment(s)

[other · medium]

📄 `src/api/generated/src/server/mod.rs` (L1243-L1246)

⚠️ GitHub could not post this as an inline comment: Lines 1243-1246 could not be resolved (outside PR diff hunks)

A cancelled or panicking blocking task makes this request handler panic instead of returning a controlled error. The same applies to the serialization task below. Propagate JoinError (for example, map it to StatusCode::INTERNAL_SERVER_ERROR) rather than unwrapping request-path task results.

github-actions · 2026-07-29T21:07:56Z

+    let validation = tokio::task::spawn_blocking(move || {
+        templates_template_id_files_hash_get_validation(path_params)
+    })
+    .await
+    .unwrap();


[bug · medium]
This unwrap() can panic the request handler if the blocking task fails with a JoinError (for example, if it panics or is cancelled), instead of returning an HTTP error. The same .await.unwrap() pattern is repeated for JSON serialization in every response branch below. Propagate/map the JoinError to StatusCode::INTERNAL_SERVER_ERROR and log it; ideally fix the server-code generator so generated handlers consistently avoid these production-path panics.

Suggestion:

Suggested change

let validation = tokio::task::spawn_blocking(move || {

templates_template_id_files_hash_get_validation(path_params)

})

.await

.unwrap();

let validation = tokio::task::spawn_blocking(move || {

templates_template_id_files_hash_get_validation(path_params)

})

.await

.map_err(|e| {

error!(error = ?e, "validation task failed");

StatusCode::INTERNAL_SERVER_ERROR

})?;

github-actions · 2026-07-29T21:07:56Z

+    #[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,


[security · medium]
These two advertised resource limits are not consumed anywhere in src/ (only their declarations, validation, and tests reference them). Consequently, compressed archives are accepted without the configured expansion bound being applied, and builds have no aggregate context-size cap. Wire these values into archive planning/build execution before exposing the upload feature; otherwise operators may rely on controls that currently provide no protection.

github-actions · 2026-07-29T21:07:56Z

+    #[config(env = "AENV_TEMPLATE_BUILD_PUBLIC_BASE_URL", parse_env = parse_trimmed_string)]
+    pub public_base_url: Option<String>,


[security · high]
Leaving this optional makes the security-sensitive default depend on the request Host: the upload-link implementation builds http://{host}/...?...token=..., and the SDK then follows that URL with the build archive. If an untrusted client or intermediary can influence Host, it can redirect both the bearer token and source archive to an attacker-controlled origin; the fixed http scheme also exposes them on TLS-fronted deployments. Require an explicit external base URL whenever this endpoint is enabled, or derive the origin only from trusted/allowlisted proxy configuration rather than the request header.

github-actions · 2026-07-29T21:07:56Z

+            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() {


[security · medium]
The value is documented as an external origin, but this validation accepts URL userinfo such as https://user:password@example.com. The downstream builder returns the base verbatim in every upload URL, exposing those configured credentials to API clients and logs. Reject non-empty usernames/passwords (and consider explicitly defining whether path prefixes are supported) before accepting the base URL.

Suggestion:

Suggested change

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() {

if !matches!(parsed.scheme(), "http" | "https") {

bail!(

"invalid template_build.public_base_url {base_url:?}: scheme must be http or \

https"

);

}

if !parsed.username().is_empty() || parsed.password().is_some() {

bail!(

"invalid template_build.public_base_url {base_url:?}: must have no credentials"

);

}

if parsed.query().is_some() {

github-actions · 2026-07-29T21:07:56Z

+            let record = self
+                .write_committed_record(
+                    metadata.id.clone(),
+                    metadata.alias.clone(),
+                    metadata.resources,
+                    committed,
+                    metadata.source.clone(),
+                )
+                .await?;


[other · high]
There is a durable inconsistency window after this await: cancellation or process termination before bind_alias leaves a committed record claiming the alias while the alias still resolves to the previous snapshot. list() reads record aliases directly, and no recovery path reconciles this state, so misleading duplicate alias claims can persist indefinitely. Please make publication recoverable/transactional (for example, persist an explicit pending-alias state and reconcile it on startup/read), rather than relying only on rollback for returned errors.

github-actions · 2026-07-29T21:07:56Z

+    /// archive, so an in-flight build can never observe its build context
+    /// change underneath it.
+    async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()>;


[security · high]
This contract permits a globally shared, first-write-wins cache entry to be published without checking that the archive actually represents hash. The upload route passes authenticated caller-controlled bytes directly here, and both backends reuse the entry across templates, so a malformed or malicious upload can permanently poison a known cache key and later builds will materialize the wrong context. Validate the archive against the SDK's canonical content-hash algorithm before publication (and reject mismatches); also constrain accepted keys to the actual lowercase SHA-256 format rather than 16–128 hex characters. If the SDK hash is not a digest of the tar bytes, verification needs to reproduce its canonical context hashing rather than hashing the raw tar stream.

Suggestion:

Suggested change

/// archive, so an in-flight build can never observe its build context

/// change underneath it.

async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()>;

/// archive, so an in-flight build can never observe its build context

/// change underneath it. Before publishing, implementations must verify

/// that `staged` represents the canonical build context identified by

/// `hash` and reject a mismatch.

async fn import(&self, hash: &str, staged: &Path) -> RepositoryResult<()>;

github-actions · 2026-07-29T21:07:56Z

+    async fn create_upload_grant(
+        &self,
+        template_id: &str,
+        hash: &str,
+        expires_unix: i64,
+    ) -> RepositoryResult<String>;


[security · medium]
The grant lifecycle has no operation or explicit contract for deleting grants that expire without a successful claim. This occurs routinely when clients request a link but skip the PUT (and the current link endpoint creates a grant even when the archive is already present). The POSIX backend happens to prune opportunistically, but the OSS backend only delegates retention to an externally configured bucket lifecycle; without that optional policy, bearer-grant objects accumulate indefinitely and allow storage/metadata exhaustion. Add an implementation-independent expiry cleanup contract/API, or make backend initialization install/require a lifecycle rule for the upload-grant prefix.

github-actions · 2026-07-29T21:07:56Z

+    } 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());


[bug · medium]
A glob that resolves to multiple source items is accepted even when dest_raw does not end in /. Docker requires a multi-source COPY destination to be an explicitly directory-form path (ending in /); for example, COPY *.txt /data should fail rather than silently creating /data and placing all matches beneath it. Track the distinct matched roots and reject multiple roots when dest_is_dir_hint is false.

github-actions · 2026-07-29T21:07:56Z

+            mapped.push(if rel.is_empty() && !entry.is_dir {
+                join_abs(&dest, base_name(&root))
+            } else {
+                join_abs(&dest, rel)
+            });


[performance · medium]
Each target owns another copy of the full destination prefix. Because dest is caller-controlled and its length is not charged against max_total_bytes, a long destination combined with many empty entries can amplify a small allowed archive into entry_count * dest.len() memory during planning (and long-name records can similarly make the rewritten archive exceed the documented budget). Store only per-entry relative mappings and join while streaming, or explicitly bound/charge generated target-path bytes and rewritten tar framing.

github-actions · 2026-07-29T21:07:56Z

+        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;
+        };


[bug · medium]
The two-pass consistency check verifies only the entry count. Keeping one file handle pins the inode but does not make its contents immutable: another writable handle can replace/reorder entries or change their types between passes, after which second-pass entries receive mappings computed for different first-pass entries. Compare each second-pass normalized path/type with its EntryIndex before using the positional target, or snapshot/lock the input before pass one.

…nfig

github-actions · 2026-08-07T05:41:40Z

+# 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


[bug · high]
These two settings are exposed as hard limits, but the current implementation never reads either field (they are only defaulted and validated in cfg.rs). As a result, changing these values in default.toml does not bound decompressed archive size or the aggregate build-context footprint, contrary to the documented security/resource guarantees. Either enforce them in the archive validation/materialization/build-planning path or remove them until enforcement exists.

github-actions · 2026-08-07T05:41:40Z

+    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",
+        );
+    }


[security · high]
Claim the grant before publishing, or make grant consumption and publication a single atomic repository operation. Concurrent requests can both pass verify_upload_grant; because the supplied hash is explicitly not validated against the body, the request that wins first-write-wins import can publish arbitrary bytes and then lose claim_upload_grant, while the other request wins the claim but cannot replace that archive. Thus a replay that ultimately receives 401 can determine the stored build context. If retryability is required, introduce an atomic claim/lease state that can be released on upload failure rather than authorizing publication before ownership is established.

github-actions · 2026-08-07T05:41:40Z

+    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);


[security · high]
This endpoint returns a durable bearer upload URL, but the current auth implementation treats any non-empty X-Team-ID, X-API-Key, admin token, or bearer credential as valid, and the endpoint implementation ignores claims when looking up template_id. Consequently, an unauthenticated caller can supply an arbitrary non-empty header and obtain an upload grant for any existing template. Validate the credential and carry tenant/team identity in the claims, then authorize template_id against those claims before issuing the grant.

github-actions · 2026-08-07T05:41:40Z

+                                                apis::templates::TemplatesTemplateIdFilesHashGetResponse::Status201_SuccessfullyReturnedTheUploadLink
+                                                    (body)
+                                                => {
+                                                  let mut response = response.status(201);


[bug · medium]
This is a GET that retrieves upload metadata, but it reports 201 Created. That status communicates creation of a new resource (normally with a Location header), can conflict with generated client expectations, and makes ordinary GET caching behavior less predictable. Define the success response as 200 OK in the OpenAPI operation and regenerate this handler.

github-actions · 2026-08-07T05:41:40Z

+        _claims: &Self::Claims,
+        path_params: &models::TemplatesTemplateIdFilesHashGetPathParams,
+    ) -> Result<TemplatesTemplateIdFilesHashGetResponse, ()> {
+        let template_id = &path_params.template_id;


[security · high]
The generated route authenticates the request, but this implementation ignores _claims and only verifies that the template ID exists. If claims carry tenant/team/role scope, any authenticated caller can request another template's link, observe global archive presence, and mint a grant bound to that template/hash. Apply the same ownership/tenant authorization used by other template operations before calling exists or create_upload_grant; otherwise this endpoint must be explicitly restricted to a globally trusted principal.

github-actions · 2026-08-07T05:41:41Z

+    /// 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.


[security · high]
Separating verify → publish → claim does not ensure that the request which wins the grant also supplies the stored archive. Two concurrent requests can both verify and race through first-write-wins import; on POSIX, one request may publish the bytes while the other wins the later atomic claim, and a crash after import leaves the token replayable. With unverified archive contents this makes cache contents nondeterministic. Redesign this as a backend operation that atomically reserves/claims the grant before publication, with rollback/retry state for failed uploads, or stage uploads per token and only promote the claimed winner.

github-actions · 2026-08-07T05:41:41Z

+pub fn is_valid_upload_token(token: &str) -> bool {
+    URL_SAFE_NO_PAD
+        .decode(token)
+        .is_ok_and(|decoded| decoded.len() == UPLOAD_TOKEN_LEN)
+}


[performance · medium]
This validates attacker-controlled query input by decoding it into an allocated Vec before checking its expected size. An oversized token therefore causes proportional decoding work and an additional allocation on every grant lookup. Reject anything whose encoded length is not 43 first, then decode into a fixed [u8; UPLOAD_TOKEN_LEN] buffer (and ensure the HTTP request-target is bounded).

Suggestion:

Suggested change

pub fn is_valid_upload_token(token: &str) -> bool {

URL_SAFE_NO_PAD

.decode(token)

.is_ok_and(|decoded| decoded.len() == UPLOAD_TOKEN_LEN)

}

pub fn is_valid_upload_token(token: &str) -> bool {

const ENCODED_UPLOAD_TOKEN_LEN: usize = 43;

if token.len() != ENCODED_UPLOAD_TOKEN_LEN {

return false;

}

let mut decoded = [0_u8; UPLOAD_TOKEN_LEN];

URL_SAFE_NO_PAD

.decode_slice(token, &mut decoded)

.is_ok_and(|len| len == UPLOAD_TOKEN_LEN)

}

github-actions · 2026-08-07T05:41:41Z

+        tar::EntryType::Regular
+        | tar::EntryType::Directory
+        | tar::EntryType::Symlink
+        | tar::EntryType::GNUSparse => Ok(()),


[bug · medium]
Valid build-context tar archives may encode repeated inodes as hard-link (EntryType::Link) entries (for example, Python's tarfile does this by default), so a COPY containing hard-linked files will fail here even though Docker accepts such contexts. Support hard links by resolving their archive link name through the same source-to-destination mapping (and validating that it refers to an indexed member), or deliberately dereference them into regular entries during the rewrite.

github-actions · 2026-08-07T05:41:41Z

+        let mut header = entry.header().clone();
+        let (uid, gid) = request
+            .ownership
+            .map_or((0, 0), |owner| (owner.uid, owner.gid));


[security · medium]
The configured budget is enforced only against the input archive in read_entry_index; the rewritten archive is never size-checked. A source archive near the limit can gain additional GNU/PAX name records and padding when absolute destination paths are appended, so output can exceed max_total_bytes despite the request documentation promising that the budget bounds the rewritten host file and single upload expansion. Track the bytes emitted by the builder (including headers/framing), or reject based on a conservative output-size estimate before writing.

github-actions · 2026-08-07T05:41:41Z

+                builder
+                    .append_link(&mut header, relative, &link)
+                    .with_context(|| format!("write symlink entry '{target}'"))?;


[security · high]
The symlink target is copied verbatim from the uploaded archive, including absolute and ..-relative targets. The documented guest extraction is tar -xpf archive -C /; if a later archive member is placed beneath this symlink, extraction can follow it and write outside the intended COPY destination (and an absolute link can expose or redirect paths in the sandbox). Validate/rewrite symlink targets so they remain within the allowed guest tree, or extract with a mechanism that rejects symlink traversal before writing subsequent members.

JoyboyBrian · 2026-08-09T23:14:01Z

Closing: this targets the server-side template-builder COPY path, which is deferred together with stock E2B SDK COPY compatibility (#141). The planning and destination-resolution ideas and relevant tests may be adapted for the native workflow (#147), after the open review findings are addressed and revalidated in that implementation.

github-actions Bot reviewed Jul 29, 2026

View reviewed changes

JoyboyBrian added 3 commits August 6, 2026 21:04

feat(snapshot): template build-context archive store (posixfs + oss)

81a8edb

feat(api): E2B build-context upload endpoints and [template_build] co…

d11d80a

…nfig

feat(template): host-side COPY plan (archive rewrite to guest paths)

a5377b6

JoyboyBrian force-pushed the e2b-build/05-copy-plan branch from 0306de5 to a5377b6 Compare August 7, 2026 04:59

JoyboyBrian changed the title ~~[5/6] feat(template): host-side COPY plan (archive rewrite to guest paths)~~ [3/4] feat(template): host-side COPY plan (archive rewrite to guest paths) Aug 7, 2026

github-actions Bot reviewed Aug 7, 2026

View reviewed changes

This was referenced Aug 7, 2026

RFC: per-step OverlayBD layer caching and build-context transport for E2B template builds #141

Closed

Design: native single-stage Dockerfile builds orchestrated by aenv #147

Open

JoyboyBrian closed this Aug 9, 2026

		#[config(env = "AENV_TEMPLATE_BUILD_PUBLIC_BASE_URL", parse_env = parse_trimmed_string)]
		pub public_base_url: Option<String>,

Uh oh!

Conversation

JoyboyBrian commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Scope and non-goals

Design

Compatibility and operations

Validation

Reviewer notes

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📄 src/api/generated/src/server/mod.rs (L1243-L1246)

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

JoyboyBrian commented Aug 9, 2026

Uh oh!

Reviewers

Assignees

JoyboyBrian commented Jul 29, 2026 •

edited

Loading

github-actions Bot commented Jul 29, 2026 •

edited

Loading

📄 `src/api/generated/src/server/mod.rs` (L1243-L1246)