From 3b0a2606ee269e166f43f2adf505d0aacabaec8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 18 Aug 2026 03:29:36 +0900 Subject: [PATCH 1/4] feat(agent): expose active package policy Expose the validated active package-broker policy through the shared authenticated GET /v1/policy route. Return a structured unavailable error without leaking policy source or file-security details. This requires now-policy-api and now-policy-server-template 0.4.0 from Devolutions/now-libraries#93 before the change can ship. Issue: Devolutions/now-libraries#93 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/now-package-broker/src/auth.rs | 19 +- crates/now-package-broker/src/server/mod.rs | 238 ++++++++++++++++++-- 2 files changed, 238 insertions(+), 19 deletions(-) diff --git a/crates/now-package-broker/src/auth.rs b/crates/now-package-broker/src/auth.rs index bc770d149..65b5515d4 100644 --- a/crates/now-package-broker/src/auth.rs +++ b/crates/now-package-broker/src/auth.rs @@ -31,6 +31,10 @@ impl PipeClient { /// unauthenticated work a connection flood can trigger. pub(crate) fn from_connected_pipe(server: &NamedPipeServer) -> anyhow::Result { let process_id = connected_pipe_client_process_id(server).context("failed to query pipe client process id")?; + Self::from_process_id(process_id) + } + + fn from_process_id(process_id: u32) -> anyhow::Result { let process = Process::get_by_pid(process_id, PROCESS_QUERY_LIMITED_INFORMATION) .with_context(|| format!("failed to open pipe client process {process_id}"))?; let executable_path = process @@ -50,6 +54,11 @@ impl PipeClient { }) } + #[cfg(test)] + pub(crate) fn from_current_process() -> anyhow::Result { + Self::from_process_id(std::process::id()) + } + /// Security identifier of the authenticated pipe client user, captured at connect. pub(crate) fn user_sid(&self) -> &Sid { &self.user_sid @@ -61,7 +70,7 @@ impl PipeClient { skip_signature_validation: bool, ) -> anyhow::Result<()> { self.validate_client_context(&request.client)?; - self.validate_signature(skip_signature_validation) + self.validate_connection(skip_signature_validation) } pub(crate) fn validate_status_request( @@ -70,7 +79,7 @@ impl PipeClient { skip_signature_validation: bool, ) -> anyhow::Result<()> { self.validate_client_context(&request.client)?; - self.validate_signature(skip_signature_validation) + self.validate_connection(skip_signature_validation) } pub(crate) fn validate_cancel_request( @@ -79,7 +88,7 @@ impl PipeClient { skip_signature_validation: bool, ) -> anyhow::Result<()> { self.validate_client_context(&request.client)?; - self.validate_signature(skip_signature_validation) + self.validate_connection(skip_signature_validation) } fn validate_client_context(&self, client: &ClientContext) -> anyhow::Result<()> { @@ -87,7 +96,7 @@ impl PipeClient { self.validate_executable_path(&client.client_executable_path) } - fn validate_signature(&self, skip_signature_validation: bool) -> anyhow::Result<()> { + pub(crate) fn validate_connection(&self, skip_signature_validation: bool) -> anyhow::Result<()> { if signature_validation_skipped(skip_signature_validation) { warn!("DEBUG MODE: Skipping package broker client signature validation"); return Ok(()); @@ -364,7 +373,7 @@ mod tests { user_sid: client_user_sid(), }; - assert!(client.validate_signature(true).is_err()); + assert!(client.validate_connection(true).is_err()); } } diff --git a/crates/now-package-broker/src/server/mod.rs b/crates/now-package-broker/src/server/mod.rs index 6f1e65b20..bd5640097 100644 --- a/crates/now-package-broker/src/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -11,8 +11,8 @@ use now_policy_api::{ CancelRequest, CancelResponse, CancelResponseKind, CapabilitiesResponse, CapabilitiesResponseKind, Decision, DecisionInfo, Elevation, ErrorCode, ErrorResponse, EvaluationResponse, EvaluationResponseKind, ExecutionResponse, ExecutionResponseKind, HealthResponse, HealthResponseKind, HealthStatus, ManagerCapability, ManagerName, - OperationStatus, OperationSubmission, PackageRequest, Scope, StatusRequest, StatusResponse, StatusResponseKind, - Transport, + OperationStatus, OperationSubmission, PackageRequest, PolicyResponse, PolicyResponseKind, Scope, StatusRequest, + StatusResponse, StatusResponseKind, Transport, }; use now_policy_server_template::{MAX_REQUEST_BODY_BYTES, PackageBrokerServer, SharedPackageBrokerServer}; use tracing::{info, trace, warn}; @@ -115,6 +115,17 @@ impl PackageBrokerServer for BrokerConnection { self.state.capabilities(self.client.user_sid()).await } + async fn policy(&self) -> Result { + self.client + .validate_connection(self.state.skip_signature_validation) + .map_err(|error| { + warn!(error = format!("{error:#}"), "Rejected package broker policy request"); + error_response(ErrorCode::Unauthorized, "pipe client authentication failed") + })?; + + self.state.policy_response() + } + async fn evaluate(&self, request: PackageRequest) -> Result { self.client .validate_request(&request, self.state.skip_signature_validation) @@ -163,6 +174,25 @@ impl PackageBrokerServer for BrokerConnection { } impl BrokerState { + fn active_policy(&self) -> Result, ErrorResponse> { + let guard = self.policy.read().expect("policy lock poisoned"); + guard + .as_ref() + .map(Arc::clone) + .ok_or_else(|| error_response(ErrorCode::BrokerPaused, "policy file is unavailable or corrupted")) + } + + fn policy_response(&self) -> Result { + let policy = self.active_policy()?; + + Ok(PolicyResponse { + response_kind: PolicyResponseKind, + response_version: api_version(), + server: server_context(), + policy: (*policy).clone(), + }) + } + async fn health(&self) -> HealthResponse { let policy_guard = self.policy.read().expect("policy lock poisoned"); let (status, policy_id) = match policy_guard.as_ref() { @@ -419,18 +449,7 @@ impl BrokerState { } let received_at = Utc::now(); - let policy = { - let guard = self.policy.read().expect("policy lock poisoned"); - match guard.as_ref() { - Some(policy) => Arc::clone(policy), - None => { - return Err(error_response( - ErrorCode::BrokerPaused, - "policy file is unavailable or corrupted", - )); - } - } - }; + let policy = self.active_policy()?; if let Some(reason) = policy_validity_failure(&policy, received_at) { warn!(%reason, "Rejecting request: policy outside validity window"); @@ -521,12 +540,15 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; + use axum::body::{Body, to_bytes}; + use axum::http::{Method, Request, StatusCode}; use chrono::Utc; use now_policy::{ PackageBrokerPolicy, PolicyEnforcement, PolicyMetadata, PolicySchemaUri, ResourceId, RulePrecedence, SemanticVersion, }; use now_policy_api as api; + use tower_service::Service as _; use super::*; use crate::executor::{ExecutionOutput, OperationCanceled, ProcessStartedCallback}; @@ -601,6 +623,194 @@ mod tests { } } + fn shared_state(policy: Option) -> Arc { + let mut state = state(); + state.policy = RwLock::new(policy.map(Arc::new)); + Arc::new(state) + } + + async fn route_request(state: Arc, method: Method, uri: &str) -> axum::response::Response { + let client = PipeClient::from_current_process().expect("capture current test process"); + let mut router = build_router_for_client(state, client); + router + .call( + Request::builder() + .method(method) + .uri(uri) + .body(Body::empty()) + .expect("valid test request"), + ) + .await + .expect("router is infallible") + } + + async fn response_json(response: axum::response::Response) -> serde_json::Value { + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("read response body"); + serde_json::from_slice(&body).expect("response is valid JSON") + } + + #[cfg(feature = "dev-skip-broker-signature")] + #[tokio::test] + async fn policy_route_serializes_active_policy_with_empty_rules() { + let expected = permissive_policy(); + let response = route_request(shared_state(Some(expected.clone())), Method::GET, "/v1/policy").await; + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers().get("content-type").unwrap(), "application/json"); + + let response: PolicyResponse = + serde_json::from_value(response_json(response).await).expect("deserialize policy response"); + assert_eq!(response.response_kind, PolicyResponseKind); + assert_eq!(&*response.response_version, api::API_VERSION_STR); + assert_eq!(response.server.transport, Transport::HttpNamedPipe); + assert_eq!( + serde_json::to_value(response.policy).unwrap(), + serde_json::to_value(expected).unwrap() + ); + } + + #[cfg(feature = "dev-skip-broker-signature")] + #[tokio::test] + async fn policy_route_serializes_full_policy_matches_and_constraints() { + let expected = + now_policy::schema::parse_policy_json(include_str!("../assets/samples/corporate-allowlist.policy.json")) + .expect("sample policy is valid"); + let response = route_request(shared_state(Some(expected.clone())), Method::GET, "/v1/policy").await; + + assert_eq!(response.status(), StatusCode::OK); + + let response: PolicyResponse = + serde_json::from_value(response_json(response).await).expect("deserialize policy response"); + assert_eq!( + serde_json::to_value(response.policy).unwrap(), + serde_json::to_value(expected).unwrap() + ); + } + + #[cfg(feature = "dev-skip-broker-signature")] + #[tokio::test] + async fn policy_route_returns_structured_service_unavailable_without_active_policy() { + let response = route_request(shared_state(None), Method::GET, "/v1/policy").await; + + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + + let body = response_json(response).await; + let error: ErrorResponse = serde_json::from_value(body.clone()).expect("deserialize error response"); + assert_eq!(error.code, ErrorCode::BrokerPaused); + assert_eq!(error.message, "policy file is unavailable or corrupted"); + assert!(error.details.is_empty()); + assert!(body.get("Policy").is_none()); + } + + #[cfg(not(feature = "dev-skip-broker-signature"))] + #[tokio::test] + async fn policy_route_rejects_unsigned_client() { + let response = route_request(shared_state(Some(permissive_policy())), Method::GET, "/v1/policy").await; + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + + let body = response_json(response).await; + let error: ErrorResponse = serde_json::from_value(body.clone()).expect("deserialize error response"); + assert_eq!(error.code, ErrorCode::Unauthorized); + assert_eq!(error.message, "pipe client authentication failed"); + assert!(body.get("Policy").is_none()); + } + + #[cfg(feature = "dev-skip-broker-signature")] + #[tokio::test] + async fn policy_route_preserves_existing_routes_and_method_restrictions() { + let state = shared_state(Some(permissive_policy())); + + for uri in ["/v1/health", "/v1/capabilities"] { + let response = route_request(Arc::clone(&state), Method::GET, uri).await; + assert_eq!(response.status(), StatusCode::OK, "unexpected status for {uri}"); + } + + let response = route_request(Arc::clone(&state), Method::HEAD, "/v1/policy").await; + assert_eq!(response.status(), StatusCode::OK); + assert!( + to_bytes(response.into_body(), usize::MAX) + .await + .expect("read HEAD response") + .is_empty() + ); + + for method in [ + Method::POST, + Method::PUT, + Method::PATCH, + Method::DELETE, + Method::OPTIONS, + Method::TRACE, + Method::CONNECT, + ] { + let response = route_request(Arc::clone(&state), method.clone(), "/v1/policy").await; + assert_eq!( + response.status(), + StatusCode::METHOD_NOT_ALLOWED, + "unexpected status for {method}" + ); + } + + let response = route_request(state, Method::GET, "/v1/not-a-route").await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[test] + fn concurrent_policy_replacement_returns_only_complete_snapshots() { + let policy_a = permissive_policy(); + let mut policy_b = + now_policy::schema::parse_policy_json(include_str!("../assets/samples/corporate-allowlist.policy.json")) + .expect("sample policy is valid"); + policy_b.metadata.id = ResourceId::from("replacement-policy"); + policy_b.metadata.revision = 42; + + let current_policy_json = serde_json::to_value(&policy_a).unwrap(); + let replacement_policy_json = serde_json::to_value(&policy_b).unwrap(); + let policy_a = Arc::new(policy_a); + let policy_b = Arc::new(policy_b); + let state = shared_state(None); + *state.policy.write().expect("policy lock") = Some(Arc::clone(&policy_a)); + + const READER_COUNT: usize = 4; + const ITERATIONS: usize = 1_000; + let barrier = Arc::new(std::sync::Barrier::new(READER_COUNT + 1)); + + std::thread::scope(|scope| { + for _ in 0..READER_COUNT { + let state = Arc::clone(&state); + let barrier = Arc::clone(&barrier); + let current_policy_json = ¤t_policy_json; + let replacement_policy_json = &replacement_policy_json; + scope.spawn(move || { + barrier.wait(); + for _ in 0..ITERATIONS { + let response = state.policy_response().expect("active policy response"); + let actual = serde_json::to_value(response.policy).unwrap(); + assert!( + actual == *current_policy_json || actual == *replacement_policy_json, + "response mixed two policy snapshots" + ); + std::thread::yield_now(); + } + }); + } + + barrier.wait(); + for index in 0..ITERATIONS { + let replacement = if index % 2 == 0 { + Arc::clone(&policy_b) + } else { + Arc::clone(&policy_a) + }; + *state.policy.write().expect("policy lock") = Some(replacement); + std::thread::yield_now(); + } + }); + } + fn request() -> PackageRequest { PackageRequest { request_kind: api::PackageRequestKind, From 4c6dc53ac0419de67cebacf07dc105dc249341ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 18 Aug 2026 13:38:09 +0900 Subject: [PATCH 2/4] fix(agent): hide policy source details Return a generic policy-unavailable message so clients cannot infer whether the active policy is file-backed, missing, or corrupt. Issue: Devolutions/now-libraries#93 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/now-package-broker/src/server/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/now-package-broker/src/server/mod.rs b/crates/now-package-broker/src/server/mod.rs index bd5640097..9e40ac524 100644 --- a/crates/now-package-broker/src/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -179,7 +179,7 @@ impl BrokerState { guard .as_ref() .map(Arc::clone) - .ok_or_else(|| error_response(ErrorCode::BrokerPaused, "policy file is unavailable or corrupted")) + .ok_or_else(|| error_response(ErrorCode::BrokerPaused, "active policy is unavailable")) } fn policy_response(&self) -> Result { @@ -699,7 +699,7 @@ mod tests { let body = response_json(response).await; let error: ErrorResponse = serde_json::from_value(body.clone()).expect("deserialize error response"); assert_eq!(error.code, ErrorCode::BrokerPaused); - assert_eq!(error.message, "policy file is unavailable or corrupted"); + assert_eq!(error.message, "active policy is unavailable"); assert!(error.details.is_empty()); assert!(body.get("Policy").is_none()); } From 03b3710f8a0c28f8b872fde636b219cd7af20e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 26 Aug 2026 21:19:02 +0900 Subject: [PATCH 3/4] fix(agent): align policy contract integration Adopt the final shared server trait and keep policy-domain conversions owned by the broker after the compatibility feature removal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- crates/now-package-broker/Cargo.toml | 4 +- .../src/evaluator/matching.rs | 65 +++++++++++++++++-- crates/now-package-broker/src/server/mod.rs | 7 +- .../src/server/responses.rs | 6 +- 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/crates/now-package-broker/Cargo.toml b/crates/now-package-broker/Cargo.toml index 379c5024a..273b11d97 100644 --- a/crates/now-package-broker/Cargo.toml +++ b/crates/now-package-broker/Cargo.toml @@ -31,8 +31,8 @@ hyper = { version = "1", features = ["http1", "server"] } hyper-util = { version = "0.1", features = ["tokio", "server", "server-auto", "service"] } notify = { version = "7", default-features = false } now-policy = "0.2" -now-policy-api = { version = "0.3", features = ["policy-compat"] } -now-policy-server-template = { version = "0.3", features = ["policy-compat"] } +now-policy-api = "0.3" +now-policy-server-template = "0.3" parking_lot = "0.12" regex = "1" semver = "1" diff --git a/crates/now-package-broker/src/evaluator/matching.rs b/crates/now-package-broker/src/evaluator/matching.rs index 213ae8d08..325375c4a 100644 --- a/crates/now-package-broker/src/evaluator/matching.rs +++ b/crates/now-package-broker/src/evaluator/matching.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use now_policy::{Architecture, Elevation, ManagerName, Operation, PolicyRule, Scope}; -use now_policy_api::PackageRequest; +use now_policy_api::{self as api, PackageRequest}; use super::RequestFlags; use super::constraints::constraints_pass; @@ -18,16 +18,16 @@ pub(super) fn rule_matches( ) -> bool { let m = &rule.match_criteria; - operations_match(request.operation.into(), &m.operations) - && managers_match(request.manager.into(), &m.managers) + operations_match(policy_operation(request.operation), &m.operations) + && managers_match(policy_manager(request.manager), &m.managers) && wildcard_any(&request.source.name, &m.sources) && wildcard_any(&request.package.id, &m.package_identifiers) && m.package_names.is_empty() && string_in_set(effective_version, &m.versions) && version_range_matches(effective_version, &m.version_range) - && scopes_match(request.options.scope.map(Into::into), &m.scopes) - && architectures_match(request.package.architecture.map(Into::into), &m.architectures) - && elevation_match(request.client.requested_elevation.into(), &m.elevation) + && scopes_match(request.options.scope.map(policy_scope), &m.scopes) + && architectures_match(request.package.architecture.map(policy_architecture), &m.architectures) + && elevation_match(policy_elevation(request.client.requested_elevation), &m.elevation) && bool_in_set(request.options.interactive, &m.interactive) && bool_in_set(request.options.skip_hash_check, &m.skip_hash_check) && bool_in_set(request.options.pre_release, &m.pre_release) @@ -39,6 +39,59 @@ pub(super) fn rule_matches( && constraints_pass(&rule.constraints, request, flags) } +fn policy_operation(operation: api::Operation) -> Operation { + match operation { + api::Operation::Install => Operation::Install, + api::Operation::Update => Operation::Update, + api::Operation::Uninstall => Operation::Uninstall, + } +} + +fn policy_manager(manager: api::ManagerName) -> ManagerName { + match manager { + api::ManagerName::Winget => ManagerName::Winget, + api::ManagerName::PowerShell => ManagerName::PowerShell, + api::ManagerName::PowerShell7 => ManagerName::PowerShell7, + api::ManagerName::Apt => ManagerName::Apt, + api::ManagerName::Bun => ManagerName::Bun, + api::ManagerName::Cargo => ManagerName::Cargo, + api::ManagerName::Chocolatey => ManagerName::Chocolatey, + api::ManagerName::Dnf => ManagerName::Dnf, + api::ManagerName::Dotnet => ManagerName::Dotnet, + api::ManagerName::Flatpak => ManagerName::Flatpak, + api::ManagerName::Homebrew => ManagerName::Homebrew, + api::ManagerName::Npm => ManagerName::Npm, + api::ManagerName::Pacman => ManagerName::Pacman, + api::ManagerName::Pip => ManagerName::Pip, + api::ManagerName::Scoop => ManagerName::Scoop, + api::ManagerName::Snap => ManagerName::Snap, + api::ManagerName::Vcpkg => ManagerName::Vcpkg, + } +} + +fn policy_scope(scope: api::Scope) -> Scope { + match scope { + api::Scope::User => Scope::User, + api::Scope::Machine => Scope::Machine, + } +} + +fn policy_architecture(architecture: api::Architecture) -> Architecture { + match architecture { + api::Architecture::X86 => Architecture::X86, + api::Architecture::X64 => Architecture::X64, + api::Architecture::Arm64 => Architecture::Arm64, + api::Architecture::Neutral => Architecture::Neutral, + } +} + +fn policy_elevation(elevation: api::Elevation) -> Elevation { + match elevation { + api::Elevation::Standard => Elevation::Standard, + api::Elevation::Elevated => Elevation::Elevated, + } +} + fn operations_match(op: Operation, allowed: &BTreeSet) -> bool { allowed.is_empty() || allowed.contains(&op) } diff --git a/crates/now-package-broker/src/server/mod.rs b/crates/now-package-broker/src/server/mod.rs index 9e40ac524..96c4865d4 100644 --- a/crates/now-package-broker/src/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -115,7 +115,7 @@ impl PackageBrokerServer for BrokerConnection { self.state.capabilities(self.client.user_sid()).await } - async fn policy(&self) -> Result { + async fn active_policy(&self) -> Result { self.client .validate_connection(self.state.skip_signature_validation) .map_err(|error| { @@ -477,7 +477,10 @@ impl BrokerState { let effective_decision = if audit_mode { Decision::Allow } else { - decision.decision.into() + match decision.decision { + now_policy::Decision::Allow => Decision::Allow, + now_policy::Decision::Deny => Decision::Deny, + } }; let reason = if audit_mode && decision.decision != now_policy::Decision::Allow { diff --git a/crates/now-package-broker/src/server/responses.rs b/crates/now-package-broker/src/server/responses.rs index 3899f8759..02758756f 100644 --- a/crates/now-package-broker/src/server/responses.rs +++ b/crates/now-package-broker/src/server/responses.rs @@ -5,7 +5,7 @@ use now_policy::PolicyDocument; use now_policy_api::{ API_VERSION_STR, ApiVersion, Architecture, ErrorCode, ErrorResponse, ErrorResponseKind, ManagerCapability, ManagerName, Operation, OperationDiagnostics, PackageRequest, RequestSummary, ResourceId, ResponsePolicyInfo, - RuleId, Scope, ServerContext, Transport, + RuleId, Scope, SemanticVersion, ServerContext, Transport, }; use crate::operation_tracker::OperationTracker; @@ -182,9 +182,9 @@ pub(super) fn request_summary(request: &PackageRequest) -> RequestSummary { pub(super) fn policy_info(policy: &PolicyDocument) -> ResponsePolicyInfo { ResponsePolicyInfo { - id: policy.metadata.id.clone().into(), + id: ResourceId(policy.metadata.id.0.clone()), revision: policy.metadata.revision, - policy_version: policy.policy_version.clone().into(), + policy_version: SemanticVersion(policy.policy_version.0.clone()), } } From ab7591b5a8efe00c9a084d295cbbeda95fea1a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 26 Aug 2026 21:43:29 +0900 Subject: [PATCH 4/4] build(agent): refresh policy dependency lock Record the registry graph after removing the obsolete policy compatibility features so locked CI can resolve the manifest consistently. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Cargo.lock | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 874292004..65e79b80d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4802,7 +4802,6 @@ checksum = "fa0817fd85c0a6b0173e2b837fa2b369be684c93c11fed1b5182284021411839" dependencies = [ "chrono", "derive_more", - "now-policy", "schemars", "semver", "serde", @@ -4820,7 +4819,6 @@ dependencies = [ "aide", "async-trait", "axum 0.8.9", - "now-policy", "now-policy-api", "schemars", "serde",