fix(rootfs): validate the layer key at the entry-path splice point

entry_path spliced the layer key into the cache filename raw: the
guard lived at the write/extract/prepare boundaries, so a library
consumer deriving paths through resolved_entry_path could get a
path outside the cache directory instead of the named error.
entry_path now returns Result and validates, prepare drops its now
redundant pre-check.
This commit is contained in:
vhaudiquet committed 2026-10-30 00:24:36 +01:00
1 parent 73eaeb57ac
commit 52d734063f
1 file changed
+29 -12
+29 -12
View File
@@ -211,17 +211,25 @@ impl RootfsCache {
/// The cache path for a prepared entry, keyed or not: a layer key /// The cache path for a prepared entry, keyed or not: a layer key
/// is spliced into the filename, so a layer and its base — and two /// is spliced into the filename, so a layer and its base — and two
/// layers of one base — live in separate entries, never overwrite /// layers of one base — live in separate entries, never overwrite
/// each other, and evict separately. /// each other, and evict separately. The key is validated here, at
pub fn entry_path(&self, source: &ImageSource, arch: &str, layer_key: Option<&str>) -> PathBuf { /// the splice point, so no caller can derive a path outside the
/// cache directory.
pub fn entry_path(
&self,
source: &ImageSource,
arch: &str,
layer_key: Option<&str>,
) -> Result<PathBuf> {
match layer_key { match layer_key {
None => self.tarball_path(source, arch), None => Ok(self.tarball_path(source, arch)),
Some(key) => { Some(key) => {
validate_layer_name(key)?;
let filename = generate_cache_filename(source, arch); let filename = generate_cache_filename(source, arch);
let stem = filename let stem = filename
.rsplit_once('.') .rsplit_once('.')
.map(|(stem, _)| stem) .map(|(stem, _)| stem)
.unwrap_or(&filename); .unwrap_or(&filename);
self.dir.join(format!("{stem}.{key}.tar.gz")) Ok(self.dir.join(format!("{stem}.{key}.tar.gz")))
} }
} }
} }
@@ -251,7 +259,7 @@ impl RootfsCache {
layer_key: Option<&str>, layer_key: Option<&str>,
) -> Result<PathBuf> { ) -> Result<PathBuf> {
let source = resolve_source(image, arch)?; let source = resolve_source(image, arch)?;
Ok(self.entry_path(&source, arch, layer_key)) self.entry_path(&source, arch, layer_key)
} }
/// Writes the layer pointer: `layers/<name>` holds the cache /// Writes the layer pointer: `layers/<name>` holds the cache
@@ -386,10 +394,7 @@ impl RootfsCache {
{ {
let arch = req.resolved_arch(); let arch = req.resolved_arch();
let source = resolve_source(&req.image, &arch)?; let source = resolve_source(&req.image, &arch)?;
if let Some(key) = &req.layer_key { let cache_path = self.entry_path(&source, &arch, req.layer_key.as_deref())?;
validate_layer_name(key)?;
}
let cache_path = self.entry_path(&source, &arch, req.layer_key.as_deref());
let marker = sidecar(&cache_path, PROVISIONED_SUFFIX); let marker = sidecar(&cache_path, PROVISIONED_SUFFIX);
if !req.no_cache && cache_path.exists() && marker.exists() { if !req.no_cache && cache_path.exists() && marker.exists() {
@@ -955,14 +960,26 @@ mod tests {
fn a_layer_key_splices_into_the_entry_name() { fn a_layer_key_splices_into_the_entry_name() {
let cache = RootfsCache::new(std::path::PathBuf::from("/cache")); let cache = RootfsCache::new(std::path::PathBuf::from("/cache"));
let source = crate::distro::parse_image_ref("alpine:3.23", "amd64").expect("parses"); let source = crate::distro::parse_image_ref("alpine:3.23", "amd64").expect("parses");
let plain = cache.entry_path(&source, "amd64", None); let plain = cache.entry_path(&source, "amd64", None).unwrap();
let keyed = cache.entry_path(&source, "amd64", Some("abc123")); let keyed = cache.entry_path(&source, "amd64", Some("abc123")).unwrap();
let other = cache.entry_path(&source, "amd64", Some("def456")); let other = cache.entry_path(&source, "amd64", Some("def456")).unwrap();
eprintln!("plain={plain:?} keyed={keyed:?} other={other:?}"); eprintln!("plain={plain:?} keyed={keyed:?} other={other:?}");
assert_ne!(plain, keyed, "the key changes the entry"); assert_ne!(plain, keyed, "the key changes the entry");
assert_ne!(keyed, other, "two keys are two entries"); assert_ne!(keyed, other, "two keys are two entries");
} }
#[test]
fn a_path_derivation_never_escapes_the_cache_directory() {
// The splice point validates: even a consumer that only
// derives paths (never writes) gets the named error instead
// of a path outside the cache directory.
let cache = RootfsCache::new("/cache");
let err = cache
.resolved_entry_path("alpine:3.23", "amd64", Some("../x"))
.unwrap_err();
assert!(err.to_string().contains("invalid layer name"), "{err}");
}
#[test] #[test]
fn layer_names_cannot_escape_the_cache_directory() { fn layer_names_cannot_escape_the_cache_directory() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();