Skip to content

Commit 24e7a4a

Browse files
committed
fix(security): bound fetch buffering + decompression, fix utf8 panic, reject scheme downgrade
Harden the OCI/Helm/git fetch paths against malicious registries and poisoned environments: - oci_transport: char-boundary-safe error-body truncation (was a byte slice that panicked when a multibyte char straddled byte 300); cap response bodies via a streamed counting reader with early Content-Length reject; new ResponseTooLarge + BodyRead error variants. Manifests capped at 32 MiB, blobs at 1 GiB. - oci_fetcher: reject before download when the manifest declares an oversize layer; cap decompressed tar output at 2 GiB via CappedReader (the verified digest covers compressed bytes only — a small valid-digest gzip could otherwise fill the disk). - helm_repo_fetcher: same body + decompression caps; reject an http:// tarball URL served by an index fetched over https:// (scheme-downgrade guard). - git_fetcher: pin ssl_verify=true on every gix connection via set_transport_options so ambient GIT_SSL_NO_VERIFY can't disable cert validation on the first (pre-pin) clone; defense-in-depth single- normal-component guard on tree-entry names before dest.join. Finding #5 was fixed in code (force_tls_verification), not merely documented — gix exposes per-connection transport options that win over the env-derived config.
1 parent 6c4f5fc commit 24e7a4a

5 files changed

Lines changed: 704 additions & 76 deletions

File tree

crates/akua-core/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,10 @@ tar = { version = "0.4", optional = true }
116116
# not apply them to the TLS client in gix 0.66. gix-transport enables curl's
117117
# default OpenSSL feature, so `git-fetch` also enables `curl/static-ssl` to make
118118
# release builds use vendored OpenSSL instead of target system OpenSSL packages.
119+
# Because this transport honours `GIT_SSL_NO_VERIFY` from the ambient
120+
# environment, `git_fetcher::force_tls_verification` pins `ssl_verify = true` on
121+
# every connection before the handshake so a poisoned env cannot disable cert
122+
# validation on the first (pre-pin) clone.
119123
gix = { version = "0.66", default-features = false, features = ["blocking-http-transport-curl-rustls", "max-performance-safe", "worktree-mutation"], optional = true }
120124
curl = { version = "0.4", optional = true, features = ["static-ssl"] }
121125
p256 = { version = "0.13", default-features = false, features = ["ecdsa", "std", "pem", "pkcs8"], optional = true }

crates/akua-core/src/git_fetcher.rs

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@
1515
//! implementation. Sandbox posture in CLAUDE.md + security-model.md
1616
//! stays intact.
1717
//!
18+
//! ## TLS posture
19+
//!
20+
//! gix's curl-rustls transport derives its TLS options from git config,
21+
//! which layers in `GIT_SSL_NO_VERIFY` / `GIT_SSL_CAINFO` as
22+
//! environment overrides. Left to the default, a poisoned environment
23+
//! could disable certificate validation on the first clone — a TOFU
24+
//! MITM window before any commit is pinned. [`force_tls_verification`]
25+
//! pins `ssl_verify = true` on every connection *before* the handshake,
26+
//! so neither half of a hostile env (`*_NO_VERIFY` nor a swapped CA
27+
//! bundle) is ever consulted.
28+
//!
1829
//! ## Scope
1930
//!
2031
//! - Public HTTPS + `file://` URLs. Public github / gitlab / gitea
@@ -228,16 +239,21 @@ fn clone_bare(
228239
url: url.to_string(),
229240
detail: e.to_string(),
230241
})?;
231-
if let Some(map) = auth {
232-
// `Arc` over `clone()` because `configure_connection` and
233-
// `set_credentials` are both `FnMut` — refcount bumps beat
234-
// re-cloning the map per connection / per 401 retry.
235-
let map: std::sync::Arc<HostAuthMap> = std::sync::Arc::new(map.clone());
236-
prep = prep.configure_connection(move |conn| {
237-
set_connection_credentials(conn, std::sync::Arc::clone(&map));
238-
Ok(())
239-
});
240-
}
242+
// Always install a connection configurator so we can pin TLS
243+
// verification ON regardless of ambient env (see
244+
// `force_tls_verification`); the credential helper is only added
245+
// when the caller supplied an auth map.
246+
let map = auth.map(|m| std::sync::Arc::new(m.clone()));
247+
prep = prep.configure_connection(move |conn| {
248+
force_tls_verification(conn);
249+
if let Some(map) = &map {
250+
// `Arc` over `clone()` because `configure_connection` and
251+
// `set_credentials` are both `FnMut` — refcount bumps beat
252+
// re-cloning the map per connection / per 401 retry.
253+
set_connection_credentials(conn, std::sync::Arc::clone(map));
254+
}
255+
Ok(())
256+
});
241257
let (repo, _) = prep
242258
.fetch_only(gix::progress::Discard, &gix::interrupt::IS_INTERRUPTED)
243259
.map_err(|e| GitFetchError::Clone {
@@ -277,6 +293,7 @@ fn refresh_bare(
277293
url: url.to_string(),
278294
detail: e.to_string(),
279295
})?;
296+
force_tls_verification(&mut conn);
280297
if let Some(map) = auth {
281298
set_connection_credentials(&mut conn, std::sync::Arc::new(map.clone()));
282299
}
@@ -296,6 +313,36 @@ fn refresh_bare(
296313
Ok(())
297314
}
298315

316+
/// Pin TLS certificate verification ON for this connection, defeating
317+
/// any ambient `GIT_SSL_NO_VERIFY` / `http.sslNoVerify` in the
318+
/// environment.
319+
///
320+
/// gix's curl-rustls transport derives its `http::Options` from the
321+
/// repo's git config, which layers in `GIT_SSL_NO_VERIFY` /
322+
/// `GIT_SSL_CAINFO` as environment overrides (see
323+
/// `gix::config::tree::gitoxide::Http::SSL_NO_VERIFY`). A poisoned
324+
/// environment could therefore disable certificate validation on the
325+
/// first `akua add` — a TOFU MITM window before any digest/commit is
326+
/// pinned. We pre-seed the connection's transport options with a
327+
/// `ssl_verify: true` `http::Options` *before* the handshake. Because
328+
/// `Connection::prepare_fetch` only derives options from config when
329+
/// `transport_options` is still `None`, our explicit value wins and the
330+
/// env-derived `ssl_verify = false` is never consulted.
331+
///
332+
/// `ssl_ca_info` is intentionally left `None` (curl's default trust
333+
/// store) — we drop any env-supplied CA bundle along with the
334+
/// env-supplied no-verify, so neither half of a poisoned env applies.
335+
fn force_tls_verification<T>(conn: &mut gix::remote::Connection<'_, '_, T>)
336+
where
337+
T: gix::protocol::transport::client::Transport,
338+
{
339+
let opts = gix::protocol::transport::client::http::Options {
340+
ssl_verify: true,
341+
..Default::default()
342+
};
343+
conn.set_transport_options(Box::new(opts));
344+
}
345+
299346
#[allow(clippy::result_large_err)]
300347
fn set_connection_credentials<'a, T>(
301348
conn: &mut gix::remote::Connection<'a, '_, T>,
@@ -456,6 +503,25 @@ fn materialize_checkout(
456503
Ok(())
457504
}
458505

506+
/// Is `name` a single, normal path component safe to `join` onto a
507+
/// checkout dir? Rejects empty, `.`/`..`, anything containing a path
508+
/// separator, and absolute paths. Mirrors the path-escape posture in
509+
/// CLAUDE.md: user-reachable code never constructs an escape string.
510+
fn is_safe_component(name: &str) -> bool {
511+
if name.is_empty() || name == "." || name == ".." {
512+
return false;
513+
}
514+
if name.contains('/') || name.contains('\\') {
515+
return false;
516+
}
517+
// A single normal component never resolves to anything but itself.
518+
let mut comps = Path::new(name).components();
519+
matches!(
520+
(comps.next(), comps.next()),
521+
(Some(std::path::Component::Normal(_)), None)
522+
)
523+
}
524+
459525
/// Walk a git tree and materialize every blob to disk. Simple
460526
/// recursive traversal — no deltified tree walking, so bigger repos
461527
/// take more I/O; fine for helm-chart-scale trees (~100 files).
@@ -470,6 +536,16 @@ fn write_tree(repo: &gix::Repository, tree_id: gix::ObjectId, dest: &Path) -> Re
470536
for entry in tree_ref.entries.iter() {
471537
let name = std::str::from_utf8(entry.filename)
472538
.map_err(|e| format!("non-utf8 filename in `{tree_id}`: {e}"))?;
539+
// Defense-in-depth: gix already validates tree-entry names, but
540+
// we still hold each name to a single normal path component
541+
// before `dest.join` so a crafted/​corrupt tree can't escape the
542+
// checkout dir (`..`, an embedded `/`, or an absolute path would
543+
// otherwise let `join` walk outside `dest`).
544+
if !is_safe_component(name) {
545+
return Err(format!(
546+
"unsafe tree-entry name `{name}` in `{tree_id}` (must be a single normal path component)"
547+
));
548+
}
473549
let target = dest.join(name);
474550
match entry.mode.kind() {
475551
gix::object::tree::EntryKind::Tree => {
@@ -604,6 +680,22 @@ mod tests {
604680
assert_eq!(RefSpec::Rev("abcdef1234".into()).label(), "abcdef1234");
605681
}
606682

683+
#[test]
684+
fn is_safe_component_accepts_normal_names_rejects_escapes() {
685+
assert!(is_safe_component("Chart.yaml"));
686+
assert!(is_safe_component("values.yaml"));
687+
assert!(is_safe_component("templates"));
688+
// Escapes / non-single-component names must be rejected.
689+
assert!(!is_safe_component(""));
690+
assert!(!is_safe_component("."));
691+
assert!(!is_safe_component(".."));
692+
assert!(!is_safe_component("a/b"));
693+
assert!(!is_safe_component("../escape"));
694+
assert!(!is_safe_component("/etc/passwd"));
695+
assert!(!is_safe_component("sub/../../escape"));
696+
assert!(!is_safe_component("a\\b"));
697+
}
698+
607699
#[test]
608700
fn bare_repo_path_is_deterministic_and_sanitized() {
609701
let root = Path::new("/cache");

0 commit comments

Comments
 (0)