From 2edc8867b0fe25a8d7e3e3b23126ddf935233885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Miguel?= <36349314+vrmiguel@users.noreply.github.com> Date: Fri, 13 Dec 2024 18:31:02 -0300 Subject: [PATCH] Validate extension files (#844) --- cli/Cargo.toml | 4 +- cli/src/commands/containers.rs | 117 ++++++++++++++++++++++++++++++++- cli/src/v1.rs | 1 + 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index de9c8472..609441c5 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,8 +1,8 @@ [package] name = "pg-trunk" -version = "0.15.5" +version = "0.15.6" edition = "2021" -authors = ["Ian Stanton", "Vinícius Miguel"] +authors = ["Ian Stanton", "Vinícius Miguel", "David E. Wheeler"] description = "A package manager for PostgreSQL extensions" homepage = "https://pgt.dev" license = "Apache-2.0" diff --git a/cli/src/commands/containers.rs b/cli/src/commands/containers.rs index 7b5aef20..81763d00 100644 --- a/cli/src/commands/containers.rs +++ b/cli/src/commands/containers.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, Context}; +use anyhow::{bail, ensure, Context}; use bollard::container::{ CreateContainerOptions, DownloadFromContainerOptions, StartContainerOptions, }; @@ -7,6 +7,7 @@ use bollard::service::ExecInspectResponse; use bollard::Docker; use std::borrow::Cow; use std::collections::HashMap; +use std::ops::Not; use bollard::container::Config; use bollard::image::BuildImageOptions; @@ -164,9 +165,10 @@ pub async fn run_temporary_container( } pub struct ExtensionFiles { - /// Files stored in `pg_config --sharedir` + /// Files stored in `pg_config --sharedir`. Should store architecture-independent files, such as SQL files. sharedir: Vec, - /// Files stored in `pg_config --pkglibdir` + /// Files stored in `pg_config --pkglibdir`. Should store architecture-dependent files, such as + /// shared objects. pkglibdir: Vec, /// The parsed contents of the extension's control file, if it exists control_file: Option, @@ -228,6 +230,8 @@ pub async fn find_installed_extension_files( .any(|pattern| pattern.matches(&file_added)); if file_added.ends_with(".so") + || file_added.ends_with(".dll") + || file_added.ends_with(".dylib") || file_added.ends_with(".bc") || file_added.ends_with(".sql") || file_added.ends_with(".control") @@ -454,6 +458,7 @@ pub async fn package_installed_extension_files( let extension_files = find_installed_extension_files(&docker, container_id, &inclusion_patterns).await?; + validate_extension_files(&extension_files)?; let license_files = find_license_files(&docker, container_id).await?; let sharedir_list = extension_files.sharedir; @@ -622,6 +627,39 @@ pub async fn package_installed_extension_files( Ok(()) } +/// Validates the presence of essential files in a PostgreSQL extension. +/// +/// Returns an error if none of these files are found, indicating a potential issue +/// with the build or `install_command` used. +fn validate_extension_files(extension_files: &ExtensionFiles) -> anyhow::Result<()> { + let has_control_file = extension_files.control_file.is_some(); + let has_shared_object = extension_files.pkglibdir.iter().any(|filename| { + filename.ends_with(".so") || filename.ends_with(".dll") || filename.ends_with(".dylib") + }); + let has_sql = extension_files + .pkglibdir + .iter() + .chain(extension_files.sharedir.iter()) + .any(|filename| filename.ends_with(".sql")); + + // Base check: should at least have one of the three + ensure!( + has_control_file || has_shared_object || has_sql, + "Did not find a control file, a shared object or a SQL file. There is likely a problem with `install_command`." + ); + + if has_control_file { + ensure!( + has_sql, + "Extension has a control file but no SQL files, which is invalid." + ) + } else { + ensure!(has_shared_object && has_sql.not(), "Extension has no control file, therefore it should contain a shared object and no SQL files."); + } + + Ok(()) +} + /// Assumes `file_to_package.starts_with(sharedir)`. fn prepare_sharedir_file<'p>( sharedir: &str, @@ -742,3 +780,76 @@ pub async fn start_postgres(docker: &Docker, container_id: &str) -> anyhow::Resu Ok(()) } + +#[cfg(test)] +mod tests { + use crate::control_file::ControlFile; + + use super::{validate_extension_files, ExtensionFiles}; + + #[test] + fn validates_extension_files() { + // Valid: Contains files of the three kinds + validate_extension_files(&ExtensionFiles { + sharedir: vec![ + "pg_stat_statements--1.0--1.1.sql".into(), + "pg_stat_statements--1.1--1.2.sql".into(), + ], + pkglibdir: vec!["pg_stat_statements.so".into()], + control_file: Some(ControlFile { + directory: None, + module_pathname: Some("'$libdir/pg_stat_statements'".into()), + requires: None, + }), + }) + .unwrap(); + + // Valid: Extension with a control file and SQL files, but no shared objects + validate_extension_files(&ExtensionFiles { + sharedir: vec!["pgmq--1.4.4--1.4.5.sql".into(), "pgmq--1.4.5.sql".into()], + pkglibdir: vec![], + control_file: Some(ControlFile { + directory: None, + module_pathname: Some("'$libdir/pgmq'".into()), + requires: None, + }), + }) + .unwrap(); + + // Valid: Extension with a shared object but no control file + validate_extension_files(&ExtensionFiles { + sharedir: vec![], + pkglibdir: vec!["auth_delay.so".into()], + control_file: None, + }) + .unwrap(); + + // Invalid: Control file and no SQL file + validate_extension_files(&ExtensionFiles { + sharedir: vec![], + pkglibdir: vec!["nonesuch.so".into()], + control_file: Some(ControlFile { + directory: None, + module_pathname: Some("'$libdir/nonesuch'".into()), + requires: None, + }), + }) + .unwrap_err(); + + // Invalid: SQL files and no control file + validate_extension_files(&ExtensionFiles { + sharedir: vec!["nonesuch--1.4.4--1.4.5.sql".into(), "nonesuch.sql".into()], + pkglibdir: vec![], + control_file: None, + }) + .unwrap_err(); + + // Invalid: None of the three file kinds + validate_extension_files(&ExtensionFiles { + sharedir: vec![], + pkglibdir: vec![], + control_file: None, + }) + .unwrap_err(); + } +} diff --git a/cli/src/v1.rs b/cli/src/v1.rs index 92840105..675f4f5a 100644 --- a/cli/src/v1.rs +++ b/cli/src/v1.rs @@ -18,6 +18,7 @@ pub struct Extension { pub struct Download { pub link: String, pub pg_version: u8, + #[expect(unused)] pub platform: String, pub sha256: String, }