From 54c33a22c9aa68790a91f34662622e7938debd5b Mon Sep 17 00:00:00 2001 From: eareimu Date: Fri, 17 Jul 2026 14:31:46 +0800 Subject: [PATCH 1/4] fix(pishoo): restore identity access policy fallback --- CHANGELOG.md | 7 + pishoo/src/policy.rs | 353 +++++++++++++++++++++++++++++------ pishoo/src/service/source.rs | 6 +- 3 files changed, 306 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6163cb1..e813e33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +### Fixed + +- pishoo identity services without an explicit `access_rules` directive once + again load the identity profile's `db/access.db`; only a genuinely absent + implicit database falls back to an empty policy, while explicit or damaged + databases fail service preparation. + ## [0.8.0-beta.5] - 2026-07-16 ### Added diff --git a/pishoo/src/policy.rs b/pishoo/src/policy.rs index eb16c92..0a7ba61 100644 --- a/pishoo/src/policy.rs +++ b/pishoo/src/policy.rs @@ -1,12 +1,16 @@ -use std::sync::Arc; +use std::{path::PathBuf, sync::Arc}; -use dhttp::access::{ - db::{ - evaluator::LocationRulesDatabase, - service::{error::EnsureStoreError, location_service::LocationService}, +use dhttp::{ + access::{ + db::{ + self, AccessDbError, + evaluator::LocationRulesDatabase, + service::{error::EnsureStoreError, location_service::LocationService}, + }, + matcher::LocationRulesMatcher, + policy::LocationRuleEvaluator, }, - matcher::LocationRulesMatcher, - policy::LocationRuleEvaluator, + home::identity::IdentityProfile, }; use snafu::{ResultExt, Snafu}; @@ -36,45 +40,154 @@ pub enum PolicyError { #[snafu(display("failed to connect access_rules database `{uri}`"))] ConnectDb { uri: String, source: sea_orm::DbErr }, - #[snafu(display("failed to validate access_rules database `{uri}`"))] + #[snafu(display( + "failed to open default access_rules database `{}`", + path.display() + ))] + OpenDefaultDb { + path: PathBuf, + source: Box, + }, + + #[snafu(display( + "failed to inspect default access_rules database `{}`", + path.display() + ))] + InspectDefaultDb { + path: PathBuf, + source: std::io::Error, + }, + + #[snafu(display("failed to validate access_rules database `{location}`"))] ValidateDb { - uri: String, + location: String, source: EnsureStoreError, }, } -pub async fn load_policy_bundle(uri: Option<&str>) -> Result { - let Some(uri) = uri else { - return Ok(PolicyBundle::default()); - }; - - let db = sea_orm::Database::connect(uri) - .await - .context(ConnectDbSnafu { - uri: uri.to_string(), - })?; +async fn database_policy_bundle( + db: sea_orm::DatabaseConnection, + location: String, +) -> Result { LocationService::new(&db) .ensure_store() .await - .context(ValidateDbSnafu { - uri: uri.to_string(), - })?; + .context(ValidateDbSnafu { location })?; Ok(PolicyBundle { location_rules: Arc::new(LocationRulesDatabase::new(db)), }) } +fn recover_missing_default_database( + path: PathBuf, + missing_source: AccessDbError, + inspection: Result<(), std::io::Error>, +) -> Result { + match inspection { + Err(source) if source.kind() == std::io::ErrorKind::NotFound => { + tracing::debug!( + path = %path.display(), + "default access_rules database is absent; using empty policy" + ); + Ok(PolicyBundle::default()) + } + Err(source) => Err(PolicyError::InspectDefaultDb { path, source }), + Ok(()) => Err(PolicyError::OpenDefaultDb { + path, + source: Box::new(missing_source), + }), + } +} + +pub async fn load_policy_bundle( + explicit_uri: Option<&str>, + identity_profile: Option<&IdentityProfile>, +) -> Result { + if let Some(uri) = explicit_uri { + let db = sea_orm::Database::connect(uri) + .await + .context(ConnectDbSnafu { + uri: uri.to_string(), + })?; + return database_policy_bundle(db, uri.to_string()).await; + } + + let Some(identity_profile) = identity_profile else { + return Ok(PolicyBundle::default()); + }; + + let path = identity_profile.access_db_path(); + let db = match db::open_access_database(identity_profile).await { + Ok(db) => db, + Err(source @ AccessDbError::MissingStore { .. }) => { + return recover_missing_default_database( + path.clone(), + source, + std::fs::metadata(&path).map(|_| ()), + ); + } + Err(source) => { + return Err(PolicyError::OpenDefaultDb { + path, + source: Box::new(source), + }); + } + }; + + database_policy_bundle(db, path.display().to_string()).await +} + #[cfg(test)] mod tests { - use dhttp::access::{ - action::RequestAction, - db::evaluator::LocationRulesDatabase, - expr::atomics::{AtomicLocationRuleExpr, EvalError}, - policy::LocationRuleRequest, + use std::{ + io, + path::{Path, PathBuf}, + time::{SystemTime, UNIX_EPOCH}, + }; + + use dhttp::{ + access::{ + action::RequestAction, + db::{AccessDbError, evaluator::LocationRulesDatabase}, + expr::atomics::{AtomicLocationRuleExpr, EvalError}, + policy::LocationRuleRequest, + }, + home::identity::IdentityProfile, }; use sea_orm::{ConnectionTrait, DatabaseBackend, Statement}; + struct TestProfile { + root: PathBuf, + profile: IdentityProfile, + } + + impl TestProfile { + fn new(label: &str) -> Self { + let root = std::env::temp_dir().join(format!( + "pishoo-policy-{label}-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock should follow Unix epoch") + .as_nanos() + )); + let profile = IdentityProfile::try_from(root.join("alice.dhttp.net")) + .expect("test profile path should contain a DHTTP name"); + std::fs::create_dir_all(profile.path()).expect("create test profile"); + Self { root, profile } + } + + fn profile(&self) -> &IdentityProfile { + &self.profile + } + } + + impl Drop for TestProfile { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.root); + } + } + struct TestRequest; impl LocationRuleRequest for TestRequest { @@ -136,53 +249,175 @@ mod tests { .expect("insert root rule"); } - #[tokio::test] - async fn policy_bundle_uses_live_database_evaluator() { - let path = std::env::temp_dir().join(format!( - "pishoo-policy-live-db-{}.db", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - let uri = format!("sqlite://{}?mode=rwc", path.display()); - let db = sea_orm::Database::connect(&uri) + async fn create_access_database(path: &Path, action: i32) -> String { + std::fs::create_dir_all(path.parent().expect("access DB path should have a parent")) + .expect("create access DB parent"); + let create_uri = format!("sqlite://{}?mode=rwc", path.display()); + let db = sea_orm::Database::connect(&create_uri) .await - .expect("create access db"); + .expect("create access DB"); create_minimal_access_schema(&db).await; - replace_root_rule(&db, 0, "*?").await; + replace_root_rule(&db, action, "*?").await; drop(db); + format!("sqlite://{}?mode=rw", path.display()) + } + + async fn evaluated_action(bundle: &super::PolicyBundle) -> RequestAction { + bundle + .location_rules + .evaluate("/", &TestRequest) + .await + .expect("policy should decide") + .action + } + + #[tokio::test] + async fn identity_default_policy_is_loaded_and_remains_live() { + let test_profile = TestProfile::new("profile-live"); + let path = test_profile.profile().access_db_path(); + let uri = create_access_database(&path, 0).await; + + let bundle = super::load_policy_bundle(None, Some(test_profile.profile())) + .await + .expect("profile default access DB should load"); + assert_eq!(evaluated_action(&bundle).await, RequestAction::Allow); + + let db = sea_orm::Database::connect(&uri) + .await + .expect("reopen profile default access DB"); + replace_root_rule(&db, 1, "*?").await; + + assert_eq!(evaluated_action(&bundle).await, RequestAction::Deny); + let _type_check: Option = None; + } - let uri = format!("sqlite://{}?mode=rw", path.display()); - let bundle = super::load_policy_bundle(Some(&uri)) + #[tokio::test] + async fn missing_identity_default_policy_falls_back_to_empty() { + let test_profile = TestProfile::new("profile-missing"); + + let bundle = super::load_policy_bundle(None, Some(test_profile.profile())) .await - .expect("load policy bundle"); - let request = TestRequest; + .expect("missing implicit access DB should be recoverable"); - assert_eq!( + assert!( bundle .location_rules - .evaluate("/", &request) + .evaluate("/", &TestRequest) .await - .expect("policy should decide") - .action, - RequestAction::Allow + .is_err(), + "empty policy must not synthesize a root rule" ); + } - let db = sea_orm::Database::connect(&uri).await.expect("reopen db"); - replace_root_rule(&db, 1, "*?").await; + #[tokio::test] + async fn identity_default_directory_is_not_recoverable() { + let test_profile = TestProfile::new("profile-directory"); + std::fs::create_dir_all(test_profile.profile().access_db_path()) + .expect("create directory at access DB path"); + + let error = super::load_policy_bundle(None, Some(test_profile.profile())) + .await + .expect_err("an existing non-file access path must fail"); + + assert!(matches!(error, super::PolicyError::OpenDefaultDb { .. })); + } - assert_eq!( + #[test] + fn identity_default_metadata_permission_error_is_not_recoverable() { + let path = PathBuf::from("/profile/db/access.db"); + let missing = AccessDbError::MissingStore { path: path.clone() }; + + let error = super::recover_missing_default_database( + path, + missing, + Err(io::Error::new(io::ErrorKind::PermissionDenied, "denied")), + ) + .expect_err("metadata permission errors must fail"); + + assert!(matches!( + error, + super::PolicyError::InspectDefaultDb { source, .. } + if source.kind() == io::ErrorKind::PermissionDenied + )); + } + + #[tokio::test] + async fn corrupt_identity_default_database_is_not_recoverable() { + let test_profile = TestProfile::new("profile-corrupt"); + let path = test_profile.profile().access_db_path(); + std::fs::create_dir_all(path.parent().expect("access DB path should have a parent")) + .expect("create access DB parent"); + std::fs::write(&path, b"not sqlite").expect("write corrupt access DB"); + + let error = super::load_policy_bundle(None, Some(test_profile.profile())) + .await + .expect_err("corrupt profile access DB must fail"); + + assert!(matches!(error, super::PolicyError::OpenDefaultDb { .. })); + } + + #[tokio::test] + async fn identity_default_database_without_schema_is_not_recoverable() { + let test_profile = TestProfile::new("profile-schema"); + let path = test_profile.profile().access_db_path(); + std::fs::create_dir_all(path.parent().expect("access DB path should have a parent")) + .expect("create access DB parent"); + let uri = format!("sqlite://{}?mode=rwc", path.display()); + drop( + sea_orm::Database::connect(&uri) + .await + .expect("create schema-less SQLite DB"), + ); + + let error = super::load_policy_bundle(None, Some(test_profile.profile())) + .await + .expect_err("schema-less profile access DB must fail validation"); + + assert!(matches!(error, super::PolicyError::ValidateDb { .. })); + } + + #[tokio::test] + async fn invalid_explicit_uri_never_falls_back_to_valid_profile_default() { + let test_profile = TestProfile::new("explicit-invalid"); + create_access_database(&test_profile.profile().access_db_path(), 0).await; + let missing_uri = format!( + "sqlite://{}?mode=rw", + test_profile.root.join("explicit-missing.db").display() + ); + + let error = super::load_policy_bundle(Some(&missing_uri), Some(test_profile.profile())) + .await + .expect_err("explicit URI failure must terminate selection"); + + assert!(matches!(error, super::PolicyError::ConnectDb { .. })); + } + + #[tokio::test] + async fn valid_explicit_uri_wins_over_valid_profile_default() { + let test_profile = TestProfile::new("explicit-wins"); + create_access_database(&test_profile.profile().access_db_path(), 0).await; + let explicit_uri = create_access_database(&test_profile.root.join("explicit.db"), 1).await; + + let bundle = super::load_policy_bundle(Some(&explicit_uri), Some(test_profile.profile())) + .await + .expect("explicit access DB should load"); + + assert_eq!(evaluated_action(&bundle).await, RequestAction::Deny); + } + + #[tokio::test] + async fn direct_server_without_explicit_uri_keeps_empty_policy() { + let bundle = super::load_policy_bundle(None, None) + .await + .expect("direct server without access_rules should keep empty policy"); + + assert!( bundle .location_rules - .evaluate("/", &request) + .evaluate("/", &TestRequest) .await - .expect("policy should see updated DB") - .action, - RequestAction::Deny + .is_err(), + "direct server must not synthesize profile rules" ); - - let _ = std::fs::remove_file(path); - let _type_check: Option = None; } } diff --git a/pishoo/src/service/source.rs b/pishoo/src/service/source.rs index 9dcab05..5112ed5 100644 --- a/pishoo/src/service/source.rs +++ b/pishoo/src/service/source.rs @@ -187,7 +187,11 @@ impl TypedServerSource { .effective() .as_ref() .map(|uri| uri.0.as_str()); - let policy = crate::policy::load_policy_bundle(access_rules_uri) + let identity_profile = match self.server_config.identity() { + ServerIdentity::Profile(profile) => Some(profile), + ServerIdentity::Direct { .. } => None, + }; + let policy = crate::policy::load_policy_bundle(access_rules_uri, identity_profile) .await .context(prepare_server_update_error::PolicySnafu { name: self.name.to_string(), From 6739bbe862bd5546ff84f8eebf68dce582ab6978 Mon Sep 17 00:00:00 2001 From: eareimu Date: Fri, 17 Jul 2026 21:01:41 +0800 Subject: [PATCH 2/4] fix(pishoo): use dhttp as the default worker group --- CHANGELOG.md | 6 ++++++ README.md | 4 ++-- pishoo/src/config/worker_target.rs | 19 ++++++++++++++++++- xtask/deb/common/etc/dhttp/pishoo.conf | 2 +- xtask/deb/pishoo-common.postinst | 2 +- xtask/release/rpm/package.sh | 2 +- xtask/tests/worker_group_contract.rs | 10 ++++++++++ 7 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 xtask/tests/worker_group_contract.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index e813e33..c336ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Changed + +- Non-macOS default worker discovery now uses the `dhttp` group, and Linux + DEB/RPM installation hooks ensure that group exists instead of creating the + legacy `pishoo` group. macOS continues to use `_www`. + ### Fixed - pishoo identity services without an explicit `access_rules` directive once diff --git a/README.md b/README.md index f367f57..b9f86cd 100644 --- a/README.md +++ b/README.md @@ -24,8 +24,8 @@ identity services load identity profiles through the DHTTP home API. Use `pishoo -c ` only for a standalone config file. Explicit config mode does not infer a DHTTP home, does not load identity profile `server.conf` files, -and does not enumerate the default `pishoo` group when `workers` and `groups` are -absent. +and does not enumerate the platform default worker group (`dhttp` on non-macOS, +`_www` on macOS) when `workers` and `groups` are absent. ### 启动反向代理 diff --git a/pishoo/src/config/worker_target.rs b/pishoo/src/config/worker_target.rs index 92668cb..038a8d1 100644 --- a/pishoo/src/config/worker_target.rs +++ b/pishoo/src/config/worker_target.rs @@ -16,7 +16,7 @@ pub struct WorkerTarget { const DEFAULT_GROUPS: &[&str] = &["_www"]; #[cfg(not(target_os = "macos"))] -const DEFAULT_GROUPS: &[&str] = &["pishoo"]; +const DEFAULT_GROUPS: &[&str] = &["dhttp"]; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum WorkerDiscoveryMode { @@ -323,3 +323,20 @@ pub fn resolve_worker_targets( } Ok(resolved) } + +#[cfg(test)] +mod tests { + use super::DEFAULT_GROUPS; + + #[cfg(target_os = "macos")] + #[test] + fn macos_default_worker_group_is_www() { + assert_eq!(DEFAULT_GROUPS, &["_www"]); + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn non_macos_default_worker_group_is_dhttp() { + assert_eq!(DEFAULT_GROUPS, &["dhttp"]); + } +} diff --git a/xtask/deb/common/etc/dhttp/pishoo.conf b/xtask/deb/common/etc/dhttp/pishoo.conf index 4387ddf..4b10570 100644 --- a/xtask/deb/common/etc/dhttp/pishoo.conf +++ b/xtask/deb/common/etc/dhttp/pishoo.conf @@ -4,5 +4,5 @@ pishoo { # When no workers or groups are specified in default global-home mode, # pishoo enumerates all users in the platform default worker group. - # Linux DEB/RPM packages use the "pishoo" group; macOS/Homebrew uses "_www". + # Linux DEB/RPM packages use the "dhttp" group; macOS/Homebrew uses "_www". } diff --git a/xtask/deb/pishoo-common.postinst b/xtask/deb/pishoo-common.postinst index 204c3b7..ee19693 100755 --- a/xtask/deb/pishoo-common.postinst +++ b/xtask/deb/pishoo-common.postinst @@ -2,7 +2,7 @@ set -e case "$1" in configure) - addgroup --system --quiet pishoo || true + addgroup --system --quiet dhttp || true install -d -m 0755 /etc/dhttp ;; esac diff --git a/xtask/release/rpm/package.sh b/xtask/release/rpm/package.sh index 3708ae6..8cf9a0e 100755 --- a/xtask/release/rpm/package.sh +++ b/xtask/release/rpm/package.sh @@ -115,7 +115,7 @@ install -D -m 0644 %{SOURCE2} %{buildroot}%{_unitdir}/pishoo.service %{_unitdir}/pishoo.service %pre -getent group pishoo >/dev/null || groupadd --system pishoo || : +getent group dhttp >/dev/null || groupadd --system dhttp || : %post %systemd_post pishoo.service diff --git a/xtask/tests/worker_group_contract.rs b/xtask/tests/worker_group_contract.rs new file mode 100644 index 0000000..3232372 --- /dev/null +++ b/xtask/tests/worker_group_contract.rs @@ -0,0 +1,10 @@ +const DEB_POSTINST: &str = include_str!("../deb/pishoo-common.postinst"); +const RPM_PACKAGE_SCRIPT: &str = include_str!("../release/rpm/package.sh"); + +#[test] +fn linux_packages_create_the_dhttp_group() { + assert!(DEB_POSTINST.contains("addgroup --system --quiet dhttp")); + assert!(!DEB_POSTINST.contains("addgroup --system --quiet pishoo")); + assert!(RPM_PACKAGE_SCRIPT.contains("groupadd --system dhttp")); + assert!(!RPM_PACKAGE_SCRIPT.contains("groupadd --system pishoo")); +} From 86ec81b418844a73116c9673f8b9f0a05fa06034 Mon Sep 17 00:00:00 2001 From: eareimu Date: Fri, 17 Jul 2026 20:20:55 +0800 Subject: [PATCH 3/4] chore: prepare pishoo v0.8.0-beta.6 --- CHANGELOG.md | 11 +++ Cargo.lock | 2 +- pishoo/Cargo.toml | 2 +- xtask/Cargo.lock | 4 +- xtask/Cargo.toml | 2 +- xtask/release.toml | 6 +- xtask/tests/release_contract.rs | 123 ++++++++++++++++++++++++++++++++ 7 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 xtask/tests/release_contract.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c336ed2..2909fcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,16 @@ ## Unreleased +## [0.8.0-beta.6] - 2026-07-17 + ### Changed - Non-macOS default worker discovery now uses the `dhttp` group, and Linux DEB/RPM installation hooks ensure that group exists instead of creating the legacy `pishoo` group. macOS continues to use `_www`. +- `pishoo-common` now follows the current pishoo source version. Linux packages + render it as `0.8.0~beta.6-1`, while pishoo accepts common packages from the + last published `0.5.1-1` through its current package version. ### Fixed @@ -15,6 +20,12 @@ implicit database falls back to an empty policy, while explicit or damaged databases fail service preparation. +### Components + +- `pishoo` v0.8.0-beta.6 +- `gateway` v0.8.0-beta.5 (unchanged) +- `pishoo-common` v0.8.0-beta.6 (`0.8.0~beta.6-1` for DEB/RPM) + ## [0.8.0-beta.5] - 2026-07-16 ### Added diff --git a/Cargo.lock b/Cargo.lock index 1884c6a..b143f71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2952,7 +2952,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "pishoo" -version = "0.8.0-beta.5" +version = "0.8.0-beta.6" dependencies = [ "axum", "clap", diff --git a/pishoo/Cargo.toml b/pishoo/Cargo.toml index 6f2cf3a..5521092 100644 --- a/pishoo/Cargo.toml +++ b/pishoo/Cargo.toml @@ -4,7 +4,7 @@ name = "pishoo" description = "modern, secure, QUIC-powered web/proxy engine" homepage = "https://www.dhttp.net" -version = "0.8.0-beta.5" +version = "0.8.0-beta.6" edition = "2024" license = "Apache-2.0" diff --git a/xtask/Cargo.lock b/xtask/Cargo.lock index aeee347..bc37bd6 100644 --- a/xtask/Cargo.lock +++ b/xtask/Cargo.lock @@ -938,8 +938,8 @@ dependencies = [ [[package]] name = "genmeta-xtask-release" -version = "0.2.0-beta.7" -source = "git+https://github.com/genmeta/genmeta-xtask-release.git?tag=v0.2.0-beta.7#30572027307f45bdc8ea817ea30d784bdd2cc995" +version = "0.2.0-beta.10" +source = "git+https://github.com/genmeta/genmeta-xtask-release.git?tag=v0.2.0-beta.10#f6e56b52dffc5e027db6831f6ea19397c12b892b" dependencies = [ "aws-credential-types", "aws-sdk-s3", diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 5e65816..425617f 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -8,5 +8,5 @@ publish = false resolver = "3" [dependencies] -genmeta-xtask-release = { git = "https://github.com/genmeta/genmeta-xtask-release.git", tag = "v0.2.0-beta.8", version = "0.2.0-beta.8" } +genmeta-xtask-release = { git = "https://github.com/genmeta/genmeta-xtask-release.git", tag = "v0.2.0-beta.10", version = "0.2.0-beta.10" } snafu = "0.9" diff --git a/xtask/release.toml b/xtask/release.toml index 8222ee1..61645ad 100644 --- a/xtask/release.toml +++ b/xtask/release.toml @@ -30,7 +30,7 @@ architecture = "target" dockerfile = "xtask/release/deb/Dockerfile" [package.pishoo.deb.requires.pishoo-common.version] -">=" = { from = "dependency" } +">=" = { value = "0.5.1-1" } "<=" = { from = "self" } @@ -40,7 +40,7 @@ architecture = "target" dockerfile = "xtask/release/rpm/Dockerfile" [package.pishoo.rpm.requires.pishoo-common.version] -">=" = { from = "dependency" } +">=" = { value = "0.5.1-1" } "<=" = { from = "self" } @@ -55,7 +55,7 @@ value = "/opt/homebrew/etc/dhttp" value = "/usr/local/etc/dhttp" [package.pishoo-common] -version = "0.5.1" +version = "0.8.0-beta.6" description = "Common files for pishoo" license = "Apache-2.0" homepage = "https://dhttp.net" diff --git a/xtask/tests/release_contract.rs b/xtask/tests/release_contract.rs new file mode 100644 index 0000000..9224e36 --- /dev/null +++ b/xtask/tests/release_contract.rs @@ -0,0 +1,123 @@ +use std::path::{Path, PathBuf}; + +use genmeta_xtask_release::{ + contract::{ + ReleaseContract, VersionBoundSource, VersionBoundSourceContract, load_release_contract, + }, + package::{PackageVersion, resolve_metadata}, + requires::{linux_requirement_entries, resolve_requires_for}, + system::PackageSystem, +}; + +fn repository_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("xtask should live under the repository root") + .to_path_buf() +} + +fn release_contract(root: &Path) -> ReleaseContract { + load_release_contract(&root.join("xtask/release.toml")) + .expect("gateway release contract should load") +} + +#[test] +fn pishoo_common_package_version_follows_pishoo() { + let root = repository_root(); + let contract = release_contract(&root); + let pishoo = + resolve_metadata(&contract, "pishoo", &root).expect("pishoo metadata should resolve"); + let common = resolve_metadata(&contract, "pishoo-common", &root) + .expect("pishoo-common metadata should resolve"); + + assert_eq!(common.source_version, pishoo.source_version); + + let common_contract = contract + .package("pishoo-common") + .expect("pishoo-common contract should exist"); + let deb = common_contract + .deb + .as_ref() + .expect("pishoo-common should have a deb branch"); + let rpm = common_contract + .rpm + .as_ref() + .expect("pishoo-common should have an rpm branch"); + + assert_eq!( + PackageVersion::deb(common.source_version.clone(), deb.revision.clone()) + .expect("pishoo-common deb version should compose") + .as_string(), + "0.8.0~beta.6-1" + ); + assert_eq!( + PackageVersion::rpm(common.source_version, rpm.release.clone()) + .expect("pishoo-common rpm version should compose") + .as_string(), + "0.8.0~beta.6-1" + ); +} + +#[test] +fn pishoo_linux_requirements_keep_published_floor_and_current_ceiling() { + let root = repository_root(); + let contract = release_contract(&root); + let pishoo = contract.package("pishoo").expect("pishoo should exist"); + + for (system, branch) in [ + ( + PackageSystem::Deb, + pishoo + .deb + .as_ref() + .expect("pishoo should have a deb branch") + .requires + .get("pishoo-common") + .expect("pishoo deb should require pishoo-common"), + ), + ( + PackageSystem::Rpm, + pishoo + .rpm + .as_ref() + .expect("pishoo should have an rpm branch") + .requires + .get("pishoo-common") + .expect("pishoo rpm should require pishoo-common"), + ), + ] { + assert_eq!( + branch.version.minimum, + Some(VersionBoundSourceContract::Literal("0.5.1-1".to_owned())) + ); + assert_eq!( + branch.version.maximum, + Some(VersionBoundSourceContract::Source( + VersionBoundSource::SelfPackage + )) + ); + + let requirements = resolve_requires_for(&contract, &root, "pishoo", system) + .expect("pishoo requirements should resolve"); + let common = requirements + .get("pishoo-common") + .expect("pishoo-common bounds should resolve"); + assert_eq!(common.minimum.as_deref(), Some("0.5.1-1")); + assert_eq!(common.maximum.as_deref(), Some("0.8.0~beta.6-1")); + + let entries = linux_requirement_entries(system, "pishoo-common", common.clone()) + .expect("pishoo-common requirement entries should render"); + let expected = match system { + PackageSystem::Deb => vec![ + "pishoo-common (>= 0.5.1-1)", + "pishoo-common (<= 0.8.0~beta.6-1)", + ], + PackageSystem::Rpm => vec![ + "pishoo-common >= 0.5.1-1", + "pishoo-common <= 0.8.0~beta.6-1", + ], + PackageSystem::Brew | PackageSystem::Scoop => unreachable!(), + }; + assert_eq!(entries, expected); + } +} From bcd7613cf79d328a40d335cafdc8d23c3b1cebc5 Mon Sep 17 00:00:00 2001 From: eareimu Date: Sat, 18 Jul 2026 00:06:57 +0800 Subject: [PATCH 4/4] fix(release): harden pishoo group setup hooks --- xtask/deb/control | 2 +- xtask/deb/pishoo-common.postinst | 10 +- xtask/release/rpm/package.sh | 10 +- xtask/tests/worker_group_contract.rs | 241 ++++++++++++++++++++++++++- 4 files changed, 259 insertions(+), 4 deletions(-) diff --git a/xtask/deb/control b/xtask/deb/control index ca1a80a..12815d2 100644 --- a/xtask/deb/control +++ b/xtask/deb/control @@ -8,7 +8,7 @@ Homepage: https://www.dhttp.net Package: pishoo-common Architecture: all Multi-Arch: foreign -Depends: ${misc:Depends} +Depends: adduser, ${misc:Depends} Description: modern, secure, QUIC-powered web/proxy engine - common files Pishoo ("Prosperity Guardian Beast") is a powerful proxy server optimized for HTTP/3 and end-to-end encrypted communication. Built for privacy and security diff --git a/xtask/deb/pishoo-common.postinst b/xtask/deb/pishoo-common.postinst index ee19693..53592a1 100755 --- a/xtask/deb/pishoo-common.postinst +++ b/xtask/deb/pishoo-common.postinst @@ -2,7 +2,15 @@ set -e case "$1" in configure) - addgroup --system --quiet dhttp || true + if getent group dhttp >/dev/null; then + : + else + status=$? + if [ "$status" -ne 2 ]; then + exit "$status" + fi + addgroup --system --quiet dhttp + fi install -d -m 0755 /etc/dhttp ;; esac diff --git a/xtask/release/rpm/package.sh b/xtask/release/rpm/package.sh index 8cf9a0e..778aca7 100755 --- a/xtask/release/rpm/package.sh +++ b/xtask/release/rpm/package.sh @@ -115,7 +115,15 @@ install -D -m 0644 %{SOURCE2} %{buildroot}%{_unitdir}/pishoo.service %{_unitdir}/pishoo.service %pre -getent group dhttp >/dev/null || groupadd --system dhttp || : +if getent group dhttp >/dev/null; then + : +else + status=\$? + if [ "\$status" -ne 2 ]; then + exit "\$status" + fi + groupadd --system dhttp +fi %post %systemd_post pishoo.service diff --git a/xtask/tests/worker_group_contract.rs b/xtask/tests/worker_group_contract.rs index 3232372..1454fc6 100644 --- a/xtask/tests/worker_group_contract.rs +++ b/xtask/tests/worker_group_contract.rs @@ -1,10 +1,249 @@ +use std::{ + fs, + os::unix::fs::PermissionsExt, + path::{Path, PathBuf}, + process::{Command, Output}, + sync::atomic::{AtomicU64, Ordering}, +}; + const DEB_POSTINST: &str = include_str!("../deb/pishoo-common.postinst"); +const DEB_CONTROL: &str = include_str!("../deb/control"); const RPM_PACKAGE_SCRIPT: &str = include_str!("../release/rpm/package.sh"); +static NEXT_TEST_DIR: AtomicU64 = AtomicU64::new(0); + +struct GroupHook { + name: &'static str, + script: String, + configure_argument: bool, + create_invocation: &'static str, +} + +struct TestDir(PathBuf); + +impl TestDir { + fn new(label: &str) -> Self { + let sequence = NEXT_TEST_DIR.fetch_add(1, Ordering::Relaxed); + let path = + std::env::temp_dir().join(format!("pishoo-{label}-{}-{sequence}", std::process::id())); + fs::create_dir(&path).expect("test directory should be created"); + Self(path) + } + + fn path(&self) -> &Path { + &self.0 + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn generated_rpm_preinstall() -> String { + let test_dir = TestDir::new("rpm-spec"); + let bin = test_dir.path().join("bin"); + let out = test_dir.path().join("out"); + fs::create_dir(&bin).expect("fake command directory should be created"); + write_fake_command(&bin, "rpmbuild", "exit 0"); + + let xtask_root = Path::new(env!("CARGO_MANIFEST_DIR")); + let repository_root = xtask_root + .parent() + .expect("xtask should live under the repository root"); + let path = std::env::var_os("PATH").expect("tests should have PATH"); + let output = Command::new("/bin/bash") + .arg(xtask_root.join("release/rpm/package.sh")) + .env( + "PATH", + format!("{}:{}", bin.display(), path.to_string_lossy()), + ) + .env("XTASK_RELEASE_PACKAGE_ID", "pishoo-common") + .env("XTASK_RELEASE_PACKAGE_VERSION", "0.8.0~beta.6-1") + .env("XTASK_RELEASE_SOURCE_VERSION", "0.8.0-beta.6") + .env("XTASK_RELEASE_REPO_ROOT", repository_root) + .env("XTASK_RELEASE_OUT_DIR", &out) + .output() + .expect("rpm package script should execute"); + assert!( + output.status.success(), + "rpm common spec generation failed: {output:?}" + ); + + let spec = fs::read_to_string(out.join("rpmbuild/SPECS/pishoo-common.spec")) + .expect("rpm common spec should be generated"); + spec.split_once("\n%pre\n") + .expect("rpm common spec should define %pre") + .1 + .split_once("\n%post\n") + .expect("rpm common spec should define %post after %pre") + .0 + .to_owned() +} + +fn group_hooks() -> [GroupHook; 2] { + [ + GroupHook { + name: "deb", + script: DEB_POSTINST.to_owned(), + configure_argument: true, + create_invocation: "addgroup --system --quiet dhttp\n", + }, + GroupHook { + name: "rpm", + script: generated_rpm_preinstall(), + configure_argument: false, + create_invocation: "groupadd --system dhttp\n", + }, + ] +} + +fn write_fake_command(bin: &Path, name: &str, body: &str) { + let path = bin.join(name); + fs::write(&path, format!("#!/bin/sh\nset -eu\n{body}\n")) + .expect("fake command should be written"); + let mut permissions = fs::metadata(&path) + .expect("fake command metadata should exist") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(path, permissions).expect("fake command should be executable"); +} + +fn run_group_hook(hook: &GroupHook, getent_status: i32, create_status: i32) -> (Output, String) { + let test_dir = TestDir::new(hook.name); + let bin = test_dir.path().join("bin"); + fs::create_dir(&bin).expect("fake command directory should be created"); + let log = test_dir.path().join("commands.log"); + + write_fake_command( + &bin, + "getent", + "printf 'getent %s\\n' \"$*\" >> \"$HOOK_LOG\"\nexit \"$GETENT_STATUS\"", + ); + for command in ["addgroup", "groupadd"] { + write_fake_command( + &bin, + command, + &format!("printf '{command} %s\\n' \"$*\" >> \"$HOOK_LOG\"\nexit \"$CREATE_STATUS\""), + ); + } + write_fake_command( + &bin, + "install", + "printf 'install %s\\n' \"$*\" >> \"$HOOK_LOG\"", + ); + + let mut command = Command::new("/bin/sh"); + command + .arg("-eu") + .arg("-c") + .arg(&hook.script) + .arg(hook.name) + .env_clear() + .env("PATH", &bin) + .env("HOOK_LOG", &log) + .env("GETENT_STATUS", getent_status.to_string()) + .env("CREATE_STATUS", create_status.to_string()); + if hook.configure_argument { + command.arg("configure"); + } + let output = command.output().expect("group hook should execute"); + let commands = fs::read_to_string(log).unwrap_or_default(); + (output, commands) +} + #[test] -fn linux_packages_create_the_dhttp_group() { +fn linux_group_hooks_create_the_dhttp_group_without_masking_failures() { assert!(DEB_POSTINST.contains("addgroup --system --quiet dhttp")); assert!(!DEB_POSTINST.contains("addgroup --system --quiet pishoo")); + assert!( + !DEB_POSTINST.lines().any(|line| { + line.contains("addgroup --system --quiet dhttp") && line.contains("||") + }) + ); assert!(RPM_PACKAGE_SCRIPT.contains("groupadd --system dhttp")); assert!(!RPM_PACKAGE_SCRIPT.contains("groupadd --system pishoo")); + assert!( + !RPM_PACKAGE_SCRIPT + .lines() + .any(|line| line.contains("groupadd --system dhttp") && line.contains("||")) + ); +} + +#[test] +fn linux_common_packages_depend_on_their_group_creation_tools() { + let common = DEB_CONTROL + .split_once("Package: pishoo-common\n") + .expect("debian control should define pishoo-common") + .1 + .split_once("\n\nPackage:") + .expect("pishoo-common should be followed by another package") + .0; + + assert!( + common + .lines() + .any(|line| line == "Depends: adduser, ${misc:Depends}") + ); + assert!(RPM_PACKAGE_SCRIPT.contains("Requires(pre): shadow-utils")); +} + +#[test] +fn linux_group_hooks_skip_creation_when_dhttp_already_exists() { + for hook in group_hooks() { + let (output, commands) = run_group_hook(&hook, 0, 91); + assert!( + output.status.success(), + "{} hook failed: {output:?}", + hook.name + ); + assert!(commands.contains("getent group dhttp\n"), "{commands}"); + assert!(!commands.contains("addgroup "), "{commands}"); + assert!(!commands.contains("groupadd "), "{commands}"); + } +} + +#[test] +fn linux_group_hooks_create_dhttp_when_it_is_absent() { + for hook in group_hooks() { + let (output, commands) = run_group_hook(&hook, 2, 0); + assert!( + output.status.success(), + "{} hook failed: {output:?}", + hook.name + ); + assert!(commands.contains("getent group dhttp\n"), "{commands}"); + assert!(commands.contains(hook.create_invocation), "{commands}"); + } +} + +#[test] +fn linux_group_hooks_propagate_group_creation_failures() { + for hook in group_hooks() { + let (output, commands) = run_group_hook(&hook, 2, 42); + assert_eq!( + output.status.code(), + Some(42), + "{} hook: {output:?}", + hook.name + ); + assert!(commands.contains(hook.create_invocation), "{commands}"); + } +} + +#[test] +fn linux_group_hooks_propagate_getent_failures() { + for hook in group_hooks() { + let (output, commands) = run_group_hook(&hook, 3, 0); + assert_eq!( + output.status.code(), + Some(3), + "{} hook: {output:?}", + hook.name + ); + assert!(commands.contains("getent group dhttp\n"), "{commands}"); + assert!(!commands.contains("addgroup "), "{commands}"); + assert!(!commands.contains("groupadd "), "{commands}"); + } }