Skip to content

check the layer media type before flashing #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bennyz wants to merge 3 commits into
base: main
Choose a base branch
from
+185 −8
Open

check the layer media type before flashing #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7b3189
check the layer media type before flashing
bennyz Mar 18, 2026
e426134
Validate flashable media type in flash_from_oci path
bennyz Apr 8, 2026
e3f85ba
Improve artifact type test coverage in get_single_layer
bennyz Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/fls/oci/from_oci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,19 @@ pub async fn flash_from_oci(

// Get the layer to download
let layer = manifest.get_single_layer()?;

// Validate that the selected layer is a flashable disk image
if !layer
.media_type
.starts_with("application/vnd.automotive.disk")
{
return Err(format!(
"Layer media type '{}' is not a flashable disk image",
layer.media_type
)
.into());
}

let layer_size = layer.size;
let compression = layer.compression();

Expand Down
180 changes: 172 additions & 8 deletions src/fls/oci/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ pub struct ImageManifest {
#[serde(default)]
pub media_type: Option<String>,

/// Artifact type (OCI v1.1) - indicates the primary content type
#[serde(default)]
pub artifact_type: Option<String>,

/// Config blob descriptor
pub config: Descriptor,

Expand Down Expand Up @@ -130,19 +134,42 @@ impl Manifest {
}

/// Get the single layer from an image manifest
/// Returns error if there are no layers or multiple layers
///
/// Returns:
/// - the only layer for single-layer manifests
/// - for multi-layer manifests: artifactType match first, otherwise first automotive disk layer
/// - error when no suitable layer is found
pub fn get_single_layer(&self) -> Result<&Descriptor, String> {
match self {
Manifest::Image(ref m) => {
if m.layers.is_empty() {
Err("Manifest has no layers".to_string())
} else if m.layers.len() > 1 {
// For multi-layer images, we take the last layer
// which typically contains the actual content
Ok(m.layers.last().unwrap())
} else {
Ok(&m.layers[0])
return Err("Manifest has no layers".to_string());
}

if m.layers.len() == 1 {
return Ok(&m.layers[0]);
}

// If artifactType is set, find the layer matching it
if let Some(ref artifact_type) = m.artifact_type {
if let Some(layer) = m.layers.iter().find(|l| l.media_type == *artifact_type) {
return Ok(layer);
}
}

// Fall back to the first disk image layer
if let Some(layer) = m
.layers
.iter()
.find(|l| l.media_type.starts_with("application/vnd.automotive.disk"))
{
return Ok(layer);
}
Comment on lines +149 to +167
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate all successful paths against the flashable media types.

Line 145 returns the only layer before any media-type check, and Lines 149-153 will also accept any artifact_type match. That still lets a non-disk blob through this path, which is exactly what this PR is trying to prevent.

Minimal fix 
             Manifest::Image(ref m) => {
+                let is_flashable = |layer: &Descriptor| {
+                    layer.media_type.starts_with("application/vnd.automotive.disk")
+                };
+
                 if m.layers.is_empty() {
                     return Err("Manifest has no layers".to_string());
                 }
 
                 if m.layers.len() == 1 {
-                    return Ok(&m.layers[0]);
+                    let layer = &m.layers[0];
+                    return if is_flashable(layer) {
+                        Ok(layer)
+                    } else {
+                        Err(format!("Layer media type '{}' is not flashable", layer.media_type))
+                    };
                 }
 
                 // If artifactType is set, find the layer matching it
                 if let Some(ref artifact_type) = m.artifact_type {
-                    if let Some(layer) = m.layers.iter().find(|l| l.media_type == *artifact_type) {
+                    if let Some(layer) = m.layers.iter().find(|l| {
+                        l.media_type == *artifact_type && is_flashable(l)
+                    }) {
                         return Ok(layer);
                     }
                 }
 
                 // Fall back to the first disk image layer
                 if let Some(layer) = m
                     .layers
                     .iter()
-                    .find(|l| l.media_type.starts_with("application/vnd.automotive.disk"))
+                    .find(|l| is_flashable(l))
                 {
                     return Ok(layer);
                 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/fls/oci/manifest.rs` around lines 145 - 163, The early-return and
artifact_type match currently accept any layer regardless of media_type; update
the selection logic in manifest.rs so every successful path validates the
layer.media_type against the allowed flashable disk types (e.g., check
starts_with("application/vnd.automotive.disk") or an equivalent allowed list)
before returning; specifically, when m.layers.len() == 1 verify
m.layers[0].media_type is flashable, when matching via m.artifact_type ensure
the found layer also has a flashable media_type, and keep the existing fallback
that finds the first layer whose media_type starts_with the disk prefix.

Copy link
Copy Markdown
Member

@mangelajo mangelajo Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mangelajo, thanks for the confirmation! The fix is straightforward — every successful return path in get_single_layer needs to validate the layer's media type against the flashable disk prefix. Here's the suggested update:

pub fn get_single_layer(&self) -> Result<&Descriptor, String> {
    match self {
        Manifest::Image(ref m) => {
            let is_flashable = |layer: &Descriptor| {
                layer.media_type.starts_with("application/vnd.automotive.disk")
            };

            if m.layers.is_empty() {
                return Err("Manifest has no layers".to_string());
            }

            if m.layers.len() == 1 {
                let layer = &m.layers[0];
                return if is_flashable(layer) {
                    Ok(layer)
                } else {
                    Err(format!(
                        "Layer media type '{}' is not a flashable disk image",
                        layer.media_type
                    ))
                };
            }

            // If artifactType is set, find a layer matching it that is also flashable
            if let Some(ref artifact_type) = m.artifact_type {
                if let Some(layer) = m.layers.iter().find(|l| {
                    l.media_type == *artifact_type && is_flashable(l)
                }) {
                    return Ok(layer);
                }
            }

            // Fall back to the first disk image layer
            if let Some(layer) = m.layers.iter().find(|l| is_flashable(l)) {
                return Ok(layer);
            }

            Err(format!(
                "No disk image layer found among {} layers",
                m.layers.len()
            ))
        }
        Manifest::Index(_) => Err(
            "Cannot get layer from manifest index - need to resolve platform first".to_string(),
        ),
    }
}

Key changes vs the current PR:

  • Single-layer path: now rejects the layer if its media_type doesn't start with application/vnd.automotive.disk.
  • artifact_type path: the closure additionally requires is_flashable, so a matching artifact type alone is not enough.
  • Fallback path: already correct, just uses the shared is_flashable closure for consistency.

Would you like me to open this as a plan for code changes?

Copy link
Copy Markdown

@ambient-code ambient-code bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ab866a4. All three return paths in get_single_layer() now validate the media type using a shared is_flashable closure that checks for the application/vnd.automotive.disk prefix:

  1. Single layer path: now returns an error if the layer has a non-flashable media type
  2. artifact_type path: now requires both media_type == artifact_type AND is_flashable
  3. Fallback path: uses the same is_flashable closure (functionally unchanged, just refactored for consistency)

Also added unit tests covering: single flashable layer acceptance, single non-disk layer rejection, artifact_type-based selection, and no-match error.

Copy link
Copy Markdown

@ambient-code ambient-code bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — but with an important design adjustment: get_single_layer() is shared between the flash and extract paths. The extraction path (extract_files_from_oci_image_to_dir) legitimately needs to work with non-disk media types (e.g. tar+gzip layers), so adding flashable validation inside get_single_layer() broke the 5 oci_extract integration tests.

The fix instead:

  • Keeps get_single_layer() generic — single-layer manifests return the layer directly (callers decide what's valid)
  • Adds flashable validation in flash_from_oci() — the flash-specific path now rejects layers that don't start with application/vnd.automotive.disk
  • Multi-layer selection still prefers artifact_type match, then falls back to the first disk image layer

Also fixed the &&layer clippy lint (needless borrow).

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped: comment is from another GitHub bot.


Err(format!(
"No disk image layer found among {} layers",
m.layers.len()
))
}
Manifest::Index(_) => Err(
"Cannot get layer from manifest index - need to resolve platform first".to_string(),
Expand Down Expand Up @@ -256,6 +283,143 @@ mod tests {
}
}

#[test]
fn test_single_flashable_layer() {
let json = r#"{
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:config123",
"size": 100
},
"layers": [
{
"mediaType": "application/vnd.automotive.disk.raw",
"digest": "sha256:disk123",
"size": 9999
}
]
}"#;
let manifest = Manifest::parse(json.as_bytes(), None).unwrap();
let layer = manifest.get_single_layer().unwrap();
assert_eq!(layer.digest, "sha256:disk123");
}

#[test]
fn test_single_non_disk_layer_returned() {
// Single-layer manifests return the layer regardless of media type;
// callers (e.g. flash_from_oci) are responsible for validating
// flashable media types when appropriate.
let json = r#"{
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:config123",
"size": 100
},
"layers": [
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
"digest": "sha256:layer123",
"size": 5678
}
]
}"#;
let manifest = Manifest::parse(json.as_bytes(), None).unwrap();
let layer = manifest.get_single_layer().unwrap();
assert_eq!(layer.digest, "sha256:layer123");
}

#[test]
fn test_artifact_type_selection() {
// Two flashable layers: artifactType must pick the qcow2 layer,
// NOT the raw layer which appears first (and would be chosen by the fallback).
let json = r#"{
"schemaVersion": 2,
"artifactType": "application/vnd.automotive.disk.qcow2",
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:config123",
"size": 100
},
"layers": [
{
"mediaType": "application/vnd.automotive.disk.raw",
"digest": "sha256:disk_raw",
"size": 1000
},
{
"mediaType": "application/vnd.automotive.disk.qcow2",
"digest": "sha256:disk_qcow2",
"size": 9999
}
]
}"#;
let manifest = Manifest::parse(json.as_bytes(), None).unwrap();
let layer = manifest.get_single_layer().unwrap();
assert_eq!(layer.digest, "sha256:disk_qcow2");
}

#[test]
fn test_artifact_type_no_match_falls_back_to_disk_layer() {
// artifactType doesn't match any layer — should fall back to the first disk layer
let json = r#"{
"schemaVersion": 2,
"artifactType": "application/vnd.unknown.type",
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:config123",
"size": 100
},
"layers": [
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
"digest": "sha256:tar_layer",
"size": 1000
},
{
"mediaType": "application/vnd.automotive.disk.raw",
"digest": "sha256:disk1",
"size": 9999
}
]
}"#;
let manifest = Manifest::parse(json.as_bytes(), None).unwrap();
let layer = manifest.get_single_layer().unwrap();
assert_eq!(layer.digest, "sha256:disk1");
}

#[test]
fn test_no_flashable_layer_error() {
let json = r#"{
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:config123",
"size": 100
},
"layers": [
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
"digest": "sha256:layer1",
"size": 1000
},
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+zstd",
"digest": "sha256:layer2",
"size": 2000
}
]
}"#;
let manifest = Manifest::parse(json.as_bytes(), None).unwrap();
let err = manifest.get_single_layer().unwrap_err();
assert!(
err.contains("No disk image layer found"),
"Expected no-match error, got: {}",
err
);
}

#[test]
fn test_parse_index() {
let json = r#"{
Expand Down
Loading