From f8ac8cbd4dff7c687d63d7929187329c2b9e79b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= Date: Sun, 20 Sep 2026 15:37:37 +0000 Subject: [PATCH 1/3] FM-600: Proxmox accounts, TLS fingerprint trust, and discovery --- Cargo.lock | 9 + crates/fleet-api/src/lib.rs | 19 + crates/fleet-api/src/operations.rs | 6 + crates/fleet-api/src/proxmox.rs | 579 ++++++++++++ crates/fleet-api/tests/apply_surface.rs | 1 + crates/fleet-api/tests/checkout_surface.rs | 1 + crates/fleet-api/tests/contract.rs | 3 + crates/fleet-api/tests/frogenv_surface.rs | 2 + crates/fleet-api/tests/mise_surface.rs | 1 + crates/fleet-api/tests/skills_surface.rs | 1 + crates/fleet-application/src/authz.rs | 19 +- crates/fleet-application/src/lib.rs | 1 + crates/fleet-application/src/proxmox.rs | 860 ++++++++++++++++++ crates/fleet-application/tests/proxmox.rs | 614 +++++++++++++ crates/fleet-auth/tests/authz_adapter.rs | 2 +- crates/fleet-controller/Cargo.toml | 1 + crates/fleet-controller/src/lib.rs | 12 +- crates/fleet-controller/src/main.rs | 15 + crates/fleet-controller/src/proxmox_store.rs | 320 +++++++ .../fleet-controller/tests/browser_guard.rs | 1 + crates/fleet-controller/tests/gateway.rs | 1 + crates/fleet-controller/tests/machines.rs | 1 + .../fleet-controller/tests/node_enrollment.rs | 1 + crates/fleet-controller/tests/onboarding.rs | 1 + crates/fleet-controller/tests/proxmox.rs | 405 +++++++++ crates/fleet-controller/tests/serve.rs | 17 +- crates/fleet-controller/tests/tailnet.rs | 1 + .../migrations/0017_proxmox_accounts.sql | 15 + crates/fleet-storage-sqlite/src/lib.rs | 2 + crates/fleet-storage-sqlite/src/proxmox.rs | 124 +++ crates/fleetctl/src/lib.rs | 288 ++++++ crates/fleetctl/tests/cli.rs | 125 +++ crates/fleetd/tests/node_install.rs | 1 + .../fleet-provider-proxmox/Cargo.toml | 10 + .../fleet-provider-proxmox/src/lib.rs | 733 ++++++++++++++- .../fleet-provider-proxmox/tests/pin_live.rs | 104 +++ packages/api-client/openapi.json | 841 ++++++++++++++++- packages/api-client/src/generated/fleet.ts | 643 +++++++++++++ 38 files changed, 5742 insertions(+), 38 deletions(-) create mode 100644 crates/fleet-api/src/proxmox.rs create mode 100644 crates/fleet-application/src/proxmox.rs create mode 100644 crates/fleet-application/tests/proxmox.rs create mode 100644 crates/fleet-controller/src/proxmox_store.rs create mode 100644 crates/fleet-controller/tests/proxmox.rs create mode 100644 crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql create mode 100644 crates/fleet-storage-sqlite/src/proxmox.rs create mode 100644 crates/providers/fleet-provider-proxmox/tests/pin_live.rs diff --git a/Cargo.lock b/Cargo.lock index e18fdba..0382916 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -622,6 +622,7 @@ dependencies = [ "fleet-provider-frogenv", "fleet-provider-git", "fleet-provider-mise", + "fleet-provider-proxmox", "fleet-provider-skills-manager", "fleet-provider-ssh", "fleet-provider-tailscale", @@ -723,8 +724,15 @@ dependencies = [ name = "fleet-provider-proxmox" version = "0.1.0" dependencies = [ + "async-trait", "fleet-application", "fleet-core", + "reqwest", + "rustls", + "serde", + "serde_json", + "sha2 0.10.9", + "tokio", ] [[package]] @@ -2287,6 +2295,7 @@ version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ + "log", "once_cell", "ring", "rustls-pki-types", diff --git a/crates/fleet-api/src/lib.rs b/crates/fleet-api/src/lib.rs index 0da41c5..53047e0 100644 --- a/crates/fleet-api/src/lib.rs +++ b/crates/fleet-api/src/lib.rs @@ -23,6 +23,7 @@ pub mod node; pub mod onboarding; pub mod operations; pub mod projects; +pub mod proxmox; pub mod ready; pub mod skills; pub mod system; @@ -119,6 +120,12 @@ pub const API_BASE_PATH: &str = "/api/v1"; tailnet::CorrelationCandidateDto, tailnet::ImportTailnetDeviceRequest, tailnet::TailnetStatusDto, + proxmox::ConfirmProxmoxFingerprintRequest, + proxmox::CreateProxmoxAccountRequest, + proxmox::ProxmoxAccountDto, + proxmox::ProxmoxDiscoveryDto, + proxmox::ProxmoxFingerprintDto, + proxmox::ProxmoxResourceDto, node::CreateEnrollmentTokenRequest, node::EnrollmentTokenCreatedDto, node::EnrollmentTokenDto, @@ -146,6 +153,10 @@ pub const API_BASE_PATH: &str = "/api/v1"; name = "tailnet", description = "Optional Tailscale discovery: correlated tailnet devices and the import handoff into the onboarding flow. Correlation is evidence only; Fleet identity never derives from Tailscale." ), + ( + name = "proxmox", + description = "Proxmox accounts, TLS fingerprint trust, and cluster discovery. The token secret is write-only; discovery is locked until the host fingerprint is confirmed." + ), ( name = "nodes", description = "Node enrollment and identity: enrollment tokens, node state, and revocation. \ @@ -208,6 +219,14 @@ pub fn api(state: Arc) -> (Router, utoipa::openapi::OpenAp .routes(routes!(tailnet::clear_tailnet)) .routes(routes!(tailnet::list_tailnet_devices)) .routes(routes!(tailnet::import_tailnet_device)) + .routes(routes!( + proxmox::list_proxmox_accounts, + proxmox::create_proxmox_account + )) + .routes(routes!(proxmox::delete_proxmox_account)) + .routes(routes!(proxmox::observe_proxmox_fingerprint)) + .routes(routes!(proxmox::confirm_proxmox_fingerprint)) + .routes(routes!(proxmox::discover_proxmox_cluster)) .with_state(state), ) .split_for_parts(); diff --git a/crates/fleet-api/src/operations.rs b/crates/fleet-api/src/operations.rs index 1460e8d..87797b3 100644 --- a/crates/fleet-api/src/operations.rs +++ b/crates/fleet-api/src/operations.rs @@ -52,6 +52,10 @@ pub struct ApiState { /// The project use cases, when the controller was composed with a /// database; `None` only in document/test states. pub projects: Option>, + /// The Proxmox use cases, when the controller was composed with a + /// database, a secret store, and the provider wired; `None` only in + /// document/test states. + pub proxmox: Option>, } impl std::fmt::Debug for ApiState { @@ -65,6 +69,7 @@ impl std::fmt::Debug for ApiState { .field("onboarding", &self.onboarding) .field("tailnet", &self.tailnet) .field("projects", &self.projects) + .field("proxmox", &self.proxmox) .finish() } } @@ -236,6 +241,7 @@ impl ApiState { onboarding: None, tailnet: None, projects: None, + proxmox: None, } } } diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs new file mode 100644 index 0000000..93fadbb --- /dev/null +++ b/crates/fleet-api/src/proxmox.rs @@ -0,0 +1,579 @@ +//! The Proxmox surface: account management with the TLS trust flow and +//! cluster discovery (FM-600). +//! +//! This adapter decides nothing about trust; it translates HTTP into the +//! application's use cases and their outcomes into the public envelopes. +//! The token secret is write-only: it arrives in the create request and is +//! never returned by any endpoint. Discovery is a read; account mutations +//! are audited through the application layer. + +use std::sync::Arc; + +use axum::{ + Extension, Json, + extract::{Path, State}, + http::StatusCode, +}; +use fleet_application::proxmox::{NewProxmoxAccount, ProxmoxAccount, ProxmoxUseCaseError}; +use fleet_core::{CorrelationId, ErrorCode, PublicError, RetryClass}; +use serde::{Deserialize, Serialize}; +use std::str::FromStr as _; +use utoipa::ToSchema; + +use crate::envelope::{Page, PageInfo, Resource}; +use crate::error::{ApiError, ApiErrorResponse}; + +/// Extracts the Proxmox use cases from the API state, or answers with the +/// standard envelope when the controller was composed without one. +fn proxmox_or_error( + state: &crate::operations::ApiState, + correlation_id: CorrelationId, +) -> Result, ApiErrorResponse> { + state.proxmox.clone().ok_or_else(|| { + let public = PublicError::new( + ErrorCode::from_str("machine_unavailable") + .expect("the literal is valid error code syntax"), + "the Proxmox surface is not wired; the controller needs its database and secret store", + RetryClass::Backoff, + ); + ApiError::new(&public, correlation_id).with_status(StatusCode::SERVICE_UNAVAILABLE) + }) +} + +/// Maps a Proxmox use-case outcome onto the public error envelope, once. +fn map_proxmox_error( + error: &ProxmoxUseCaseError, + correlation_id: CorrelationId, +) -> ApiErrorResponse { + let (status, code, retry): (StatusCode, &str, RetryClass) = match error { + ProxmoxUseCaseError::Denied(_) => (StatusCode::FORBIDDEN, "denied", RetryClass::Never), + ProxmoxUseCaseError::NotFound { .. } => { + (StatusCode::NOT_FOUND, "not_found", RetryClass::Never) + } + ProxmoxUseCaseError::Conflict { .. } => { + (StatusCode::CONFLICT, "conflict", RetryClass::Never) + } + ProxmoxUseCaseError::Invalid { .. } => ( + StatusCode::BAD_REQUEST, + "invalid_request", + RetryClass::Never, + ), + ProxmoxUseCaseError::UnconfirmedTrust { .. } => ( + StatusCode::CONFLICT, + "proxmox_unconfirmed", + RetryClass::Never, + ), + ProxmoxUseCaseError::NoSecret { .. } => { + (StatusCode::CONFLICT, "proxmox_no_secret", RetryClass::Never) + } + ProxmoxUseCaseError::Source(fleet_application::proxmox::ProxmoxSourceError::Auth) => { + (StatusCode::BAD_GATEWAY, "proxmox_auth", RetryClass::Backoff) + } + ProxmoxUseCaseError::Source( + fleet_application::proxmox::ProxmoxSourceError::FingerprintMismatch { .. }, + ) => ( + StatusCode::CONFLICT, + "proxmox_fingerprint_mismatch", + RetryClass::Never, + ), + ProxmoxUseCaseError::Source(_) => ( + StatusCode::BAD_GATEWAY, + "proxmox_source", + RetryClass::Backoff, + ), + ProxmoxUseCaseError::Credentials(_) | ProxmoxUseCaseError::Backend { .. } => ( + StatusCode::INTERNAL_SERVER_ERROR, + "internal", + RetryClass::Backoff, + ), + }; + let message = match error { + ProxmoxUseCaseError::Backend { .. } => { + "the request could not be completed; the detail is in the controller log".to_owned() + } + other => other.to_string(), + }; + let public = PublicError::new( + ErrorCode::from_str(code).expect("the literal is valid error code syntax"), + message, + retry, + ); + ApiError::new(&public, correlation_id).with_status(status) +} + +/// One configured Proxmox account. The token secret is never here. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxAccountDto { + /// The account's identity. + pub id: String, + /// The operator-facing name. + pub name: String, + /// The PVE host. + pub host: String, + /// The API port. + pub port: u16, + /// The API token id (`user@realm!tokenname`), not secret on its own. + pub token_id: String, + /// The trust state: `unconfirmed` until the fingerprint is pinned. + pub fingerprint_state: String, + /// The pinned fingerprint, once confirmed. + pub fingerprint: Option, + /// When the account was created. + pub created_at: i64, +} + +impl From for ProxmoxAccountDto { + fn from(account: ProxmoxAccount) -> Self { + Self { + id: account.id, + name: account.name, + host: account.host, + port: account.port, + token_id: account.token_id, + fingerprint_state: match account.fingerprint { + Some(_) => "confirmed".to_owned(), + None => "unconfirmed".to_owned(), + }, + fingerprint: account.fingerprint, + created_at: account.created_at, + } + } +} + +/// One normalized discovery observation. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxResourceDto { + /// The normalized kind: `node`, `qemu`, `lxc`, `qemu-template`, or + /// `storage`. + pub kind: String, + /// The cluster-visible id. + pub id: String, + /// The hosting node, when the resource has one. + pub node: Option, + /// The VMID, when the resource has one. + pub vmid: Option, + /// The display name, when carried. + pub name: Option, + /// The PVE status string, when carried. + pub status: Option, + /// The account that observed the resource. + pub account_id: String, + /// The PVE version the observation came from. + pub pve_version: String, + /// When the observation was taken. + pub observed_at: i64, +} + +/// The discovery snapshot. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxDiscoveryDto { + /// The account that produced the snapshot. + pub account_id: String, + /// The PVE version seen. + pub pve_version: String, + /// The normalized resources. + pub resources: Vec, + /// The per-resource normalization warnings. + pub warnings: Vec, + /// The count the API reported. + pub reported_count: usize, + /// When the snapshot was taken. + pub observed_at: i64, +} + +/// The create-account request. The token secret is write-only. +#[derive(Debug, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct CreateProxmoxAccountRequest { + /// The operator-facing name. + pub name: String, + /// The PVE host (IP or DNS name). + pub host: String, + /// The API port; 8006 when omitted. + pub port: Option, + /// The API token id (`user@realm!tokenname`). + pub token_id: String, + /// The API token secret (write-only). + pub token_secret: String, +} + +/// The confirm-trust request: the fingerprint the caller observed. +#[derive(Debug, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ConfirmProxmoxFingerprintRequest { + /// The SHA-256 fingerprint as observed (colons optional). + pub fingerprint: String, +} + +/// Lists the configured accounts. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal or backend failure. +#[utoipa::path( + get, + path = "/proxmox/accounts", + tag = "proxmox", + operation_id = "listProxmoxAccounts", + responses( + ( + status = 200, + description = "The configured accounts, newest first.", + body = Page + ), + ( + status = 403, + description = "The caller may not read the Proxmox surface.", + body = crate::error::ApiError + ), + ) +)] +pub async fn list_proxmox_accounts( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, +) -> Result>, ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let accounts = proxmox + .list(state.authorizer.as_ref(), &principal) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + let items: Vec = accounts.into_iter().map(Into::into).collect(); + Ok(Json(Page { + page: PageInfo { + next_cursor: None, + limit: items.len().try_into().unwrap_or(u32::MAX), + }, + items, + })) +} + +/// Registers an account and stores its token secret. The account starts +/// `unconfirmed`: discovery stays locked until the fingerprint is confirmed. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, conflict, or backend +/// failure. +#[utoipa::path( + post, + path = "/proxmox/accounts", + tag = "proxmox", + operation_id = "createProxmoxAccount", + request_body = CreateProxmoxAccountRequest, + responses( + ( + status = 201, + description = "The account was created; confirm its fingerprint before discovery.", + body = Resource + ), + ( + status = 400, + description = "The request is malformed.", + body = crate::error::ApiError + ), + ( + status = 403, + description = "The caller may not configure the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 409, + description = "The account name is taken.", + body = crate::error::ApiError + ), + ) +)] +pub async fn create_proxmox_account( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Json(request): Json, +) -> Result<(StatusCode, Json>), ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let account = proxmox + .create( + state.authorizer.as_ref(), + &principal, + NewProxmoxAccount { + name: request.name, + host: request.host, + port: request.port, + token_id: request.token_id, + }, + &request.token_secret, + ) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok((StatusCode::CREATED, Json(Resource::new(account.into())))) +} + +/// Removes an account and its secret. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unknown account, or a +/// backend failure. +#[utoipa::path( + delete, + path = "/proxmox/accounts/{accountId}", + tag = "proxmox", + operation_id = "deleteProxmoxAccount", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ), + responses( + ( + status = 204, + description = "The account was removed." + ), + ( + status = 403, + description = "The caller may not configure the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 404, + description = "The account does not exist.", + body = crate::error::ApiError + ), + ) +)] +pub async fn delete_proxmox_account( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path(account_id): Path, +) -> Result { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + proxmox + .delete(state.authorizer.as_ref(), &principal, &account_id) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok(StatusCode::NO_CONTENT) +} + +/// Captures the host's certificate fingerprint without sending any +/// credential. The report is the input to the confirm step. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unknown account, or an +/// unreachable host. +#[utoipa::path( + post, + path = "/proxmox/accounts/{accountId}/observe", + tag = "proxmox", + operation_id = "observeProxmoxFingerprint", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ), + responses( + ( + status = 200, + description = "The observed fingerprint; confirm it to trust the host.", + body = Resource + ), + ( + status = 403, + description = "The caller may not read the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 404, + description = "The account does not exist.", + body = crate::error::ApiError + ), + ( + status = 502, + description = "The host is unreachable.", + body = crate::error::ApiError + ), + ) +)] +pub async fn observe_proxmox_fingerprint( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path(account_id): Path, +) -> Result>, ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let observed = proxmox + .observe(state.authorizer.as_ref(), &principal, &account_id) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok(Json(Resource::new(ProxmoxFingerprintDto { + account_id, + fingerprint: observed, + }))) +} + +/// The observed fingerprint report. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxFingerprintDto { + /// The account the fingerprint was observed for. + pub account_id: String, + /// The host certificate's SHA-256 fingerprint. + pub fingerprint: String, +} + +/// Pins the confirmed fingerprint as the account's trust anchor. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unknown account, or a +/// malformed fingerprint. +#[utoipa::path( + post, + path = "/proxmox/accounts/{accountId}/confirm", + tag = "proxmox", + operation_id = "confirmProxmoxFingerprint", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ), + request_body = ConfirmProxmoxFingerprintRequest, + responses( + ( + status = 200, + description = "The fingerprint is pinned; discovery is unlocked.", + body = Resource + ), + ( + status = 400, + description = "The fingerprint is malformed.", + body = crate::error::ApiError + ), + ( + status = 403, + description = "The caller may not configure the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 404, + description = "The account does not exist.", + body = crate::error::ApiError + ), + ) +)] +pub async fn confirm_proxmox_fingerprint( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path(account_id): Path, + Json(request): Json, +) -> Result>, ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let account = proxmox + .confirm( + state.authorizer.as_ref(), + &principal, + &account_id, + &request.fingerprint, + ) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok(Json(Resource::new(account.into()))) +} + +/// Discovers the cluster through one trusted account. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unconfirmed account, or +/// a source failure. +#[utoipa::path( + get, + path = "/proxmox/accounts/{accountId}/discovery", + tag = "proxmox", + operation_id = "discoverProxmoxCluster", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ), + responses( + ( + status = 200, + description = "The discovery snapshot, availability-honest.", + body = Resource + ), + ( + status = 403, + description = "The caller may not read the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 404, + description = "The account does not exist.", + body = crate::error::ApiError + ), + ( + status = 409, + description = "The account's trust is unconfirmed or its fingerprint was refused.", + body = crate::error::ApiError + ), + ) +)] +pub async fn discover_proxmox_cluster( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path(account_id): Path, +) -> Result>, ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let discovery = proxmox + .discover( + state.authorizer.as_ref(), + &principal, + &account_id, + fleet_core::SystemClock::now_unix_millis(), + ) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok(Json(Resource::new(ProxmoxDiscoveryDto { + account_id: discovery.account_id, + pve_version: discovery.pve_version, + resources: discovery + .resources + .into_iter() + .map(|resource| ProxmoxResourceDto { + kind: resource.kind, + id: resource.id, + node: resource.node, + vmid: resource.vmid, + name: resource.name, + status: resource.status, + account_id: resource.account_id, + pve_version: resource.pve_version, + observed_at: resource.observed_at, + }) + .collect(), + warnings: discovery.warnings, + reported_count: discovery.reported_count, + observed_at: discovery.observed_at, + }))) +} diff --git a/crates/fleet-api/tests/apply_surface.rs b/crates/fleet-api/tests/apply_surface.rs index b93abaa..cd497cb 100644 --- a/crates/fleet-api/tests/apply_surface.rs +++ b/crates/fleet-api/tests/apply_surface.rs @@ -355,6 +355,7 @@ fn state_for(authorizer: Arc) -> Arc (axum::Router, Arc) { onboarding: None, tailnet: None, projects: None, + proxmox: None, }); ( router(state).layer(axum::Extension(fleet_api::ActingPrincipal { @@ -411,6 +412,7 @@ fn operation_state(authorizer: Arc) -> onboarding: None, tailnet: None, projects: None, + proxmox: None, }) } @@ -840,6 +842,7 @@ fn machine_state( onboarding: None, tailnet: None, projects: None, + proxmox: None, }) } diff --git a/crates/fleet-api/tests/frogenv_surface.rs b/crates/fleet-api/tests/frogenv_surface.rs index 0913a7f..e7374ce 100644 --- a/crates/fleet-api/tests/frogenv_surface.rs +++ b/crates/fleet-api/tests/frogenv_surface.rs @@ -344,6 +344,7 @@ fn state_for(authorizer: Arc) -> Arc) -> Arc "apply.execute", Permission::SourceFetch => "source.fetch", Permission::SourceActivate => "source.activate", + Permission::ProxmoxRead => "proxmox.read", + Permission::ProxmoxConfig => "proxmox.config", } } @@ -258,7 +269,9 @@ impl Permission { | Permission::ProjectsReady | Permission::ApplyExecute | Permission::SourceFetch - | Permission::SourceActivate => true, + | Permission::SourceActivate + | Permission::ProxmoxRead + | Permission::ProxmoxConfig => true, } } @@ -281,7 +294,9 @@ impl Permission { | Permission::ProjectsRead | Permission::ProjectsCreate | Permission::SourceFetch - | Permission::SourceActivate => false, + | Permission::SourceActivate + | Permission::ProxmoxRead + | Permission::ProxmoxConfig => false, Permission::MachineReadSensitive | Permission::OperationCancel | Permission::SecretRead diff --git a/crates/fleet-application/src/lib.rs b/crates/fleet-application/src/lib.rs index a570ee1..92901dc 100644 --- a/crates/fleet-application/src/lib.rs +++ b/crates/fleet-application/src/lib.rs @@ -15,6 +15,7 @@ pub mod onboarding; pub mod operation; pub mod planner; pub mod project; +pub mod proxmox; pub mod ready; pub mod source; pub mod tailnet; diff --git a/crates/fleet-application/src/proxmox.rs b/crates/fleet-application/src/proxmox.rs new file mode 100644 index 0000000..96a4e6b --- /dev/null +++ b/crates/fleet-application/src/proxmox.rs @@ -0,0 +1,860 @@ +//! The Proxmox integration use cases: accounts with verified TLS trust and +//! cluster discovery as normalized observations (FM-600; FM-S08). +//! +//! The trust model is the FM-S08 decision: a PVE host presents its own +//! cluster CA, so Fleet pins the host certificate's SHA-256 fingerprint. +//! `observe` captures a host's fingerprint without sending any credential — +//! the transport refuses the handshake after capture — and `confirm` pins +//! it through an explicit authorized step (the FM-201 SSH trust flow, over +//! TLS). A pinned host whose certificate changes refuses every call until +//! the pin is re-confirmed, and the mismatch is reported with both +//! fingerprints as evidence. +//! +//! Accounts are multi-account by design. The API token lives in Fleet's +//! encrypted secret store behind the [`ProxmoxCredentialStore`] port and is +//! resolved just in time at the source boundary; it never appears in audit +//! metadata, error details, or debug output. +//! +//! Discovery is read-only: cluster resources normalize into +//! [`ProxmoxResource`] observations with provenance and time, per-resource +//! failures isolate into warnings instead of dropping the snapshot, and +//! nothing here mutates a PVE host. +#![warn(missing_docs)] + +use std::fmt; +use std::sync::Arc; + +use async_trait::async_trait; +use serde::{Deserialize, Serialize}; + +use crate::authz::{AccessRequest, ActingPrincipal, Authorizer, Decision, Permission, authorize}; +use crate::operation::AuditPort; +use fleet_core::SensitiveString; + +/// Binds one credential-carrying call to one account, resolving the secret +/// just in time. +pub struct BoundRequest { + /// The account making the call. + pub account: ProxmoxAccount, + /// The token secret, resolved from the encrypted store. + pub secret: SensitiveString, + /// The pinned fingerprint, when the account is trusted. + pub pinned_fingerprint: Option, +} + +impl fmt::Debug for BoundRequest { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("BoundRequest") + .field("account", &self.account) + .field("secret", &"") + .field("pinned_fingerprint", &self.pinned_fingerprint) + .finish() + } +} + +/// One configured Proxmox account. The credential is a *reference* — the +/// value lives in the encrypted store and never travels with the record. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxAccount { + /// The account's stable identity. + pub id: String, + /// The operator-facing name, unique among accounts. + pub name: String, + /// The PVE host (IP or DNS name). + pub host: String, + /// The API port; 8006 in the common case. + pub port: u16, + /// The API token id (`user@realm!tokenname`), not secret on its own. + pub token_id: String, + /// The pinned host-certificate fingerprint, once confirmed. + pub fingerprint: Option, + /// When the account was created (epoch millis). + pub created_at: i64, +} + +/// The fingerprint state of an account, as the trust flow reports it. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FingerprintState { + /// No fingerprint is pinned: the account exists but cannot call yet. + Unconfirmed, + /// A fingerprint is pinned and every call verifies against it. + Confirmed, +} + +/// A credential-store failure that is safe to print. +#[derive(Debug)] +pub enum CredentialStoreError { + /// The store is unreadable or unwritable. + Backend { + /// The bounded detail. + detail: String, + }, +} + +impl fmt::Display for CredentialStoreError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Backend { detail } => write!(f, "the secret store failed: {detail}"), + } + } +} + +impl std::error::Error for CredentialStoreError {} + +/// The credential store port: where each account's API-token secret lives +/// between calls. The composition root implements this over Fleet's +/// encrypted secret store with per-account record names. +#[async_trait] +pub trait ProxmoxCredentialStore: fmt::Debug + Send + Sync { + /// Resolves the account's token secret, when stored. + /// + /// # Errors + /// + /// Fails when the store cannot be read. + async fn load(&self, account_id: &str) -> Result, CredentialStoreError>; + /// Stores (replacing any previous) the account's token secret. + /// + /// # Errors + /// + /// Fails when the store cannot be written. + async fn store(&self, account_id: &str, secret: &str) -> Result<(), CredentialStoreError>; + /// Removes the account's token secret. + /// + /// # Errors + /// + /// Fails when the store cannot be written. + async fn clear(&self, account_id: &str) -> Result<(), CredentialStoreError>; +} + +/// A discovery-source failure that is safe to print. Fingerprints and +/// statuses travel here; tokens never do. +#[derive(Debug)] +pub enum ProxmoxSourceError { + /// The pinned fingerprint was refused, with the observed fingerprint as + /// evidence. The pin did its job: the connection died at the handshake. + FingerprintMismatch { + /// The observed leaf fingerprint. + observed: String, + /// The pinned fingerprint. + pinned: String, + }, + /// The API token was refused (401): the credential is wrong or revoked. + Auth, + /// The token lacks the privilege (403). + Forbidden { + /// The bounded, redacted detail. + detail: String, + }, + /// Any other HTTP outcome. + Http { + /// The status. + status: u16, + /// The bounded detail. + detail: String, + }, + /// The API answered, but not with something Fleet can interpret. + InvalidPayload { + /// The bounded detail. + detail: String, + }, + /// Transport-level failure (DNS, TCP, timeout). + Connect { + /// The bounded, redacted detail. + detail: String, + }, + /// The credential store failed. + Credentials(CredentialStoreError), +} + +impl fmt::Display for ProxmoxSourceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FingerprintMismatch { observed, pinned } => write!( + f, + "the host certificate's fingerprint {observed} does not match the pinned {pinned}" + ), + Self::Auth => write!(f, "the API token was refused (401)"), + Self::Forbidden { detail } => { + write!(f, "the token lacks the privilege (403): {detail}") + } + Self::Http { status, detail } => write!(f, "the API answered {status}: {detail}"), + Self::InvalidPayload { detail } => { + write!(f, "the API's payload is not interpretable: {detail}") + } + Self::Connect { detail } => write!(f, "the connection failed: {detail}"), + Self::Credentials(error) => write!(f, "{error}"), + } + } +} + +impl std::error::Error for ProxmoxSourceError {} + +/// One normalized discovery observation: a node, guest, template, or +/// storage seen through one account, with provenance. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxResource { + /// The normalized kind: `node`, `qemu`, `lxc`, `qemu-template`, or + /// `storage`. + pub kind: String, + /// The cluster-visible id, e.g. `node/pve`, `qemu/101`. + pub id: String, + /// The hosting node, when the resource has one. + pub node: Option, + /// The VMID, when the resource has one. + pub vmid: Option, + /// The display name, when carried. + pub name: Option, + /// The PVE status string, when carried. + pub status: Option, + /// The account that observed the resource. + pub account_id: String, + /// The PVE version the observation came from. + pub pve_version: String, + /// When the observation was taken (epoch millis). + pub observed_at: i64, +} + +/// The discovery snapshot: every resource the cluster reported, plus the +/// honest record of what failed normalization. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProxmoxDiscovery { + /// The account that produced the snapshot. + pub account_id: String, + /// The PVE version seen. + pub pve_version: String, + /// The normalized resources. + pub resources: Vec, + /// The per-resource normalization warnings. A partial failure never + /// drops the snapshot. + pub warnings: Vec, + /// The count the API reported, for honesty about isolation. + pub reported_count: usize, + /// When the snapshot was taken (epoch millis). + pub observed_at: i64, +} + +/// The account record port: durable account state. +#[async_trait] +pub trait ProxmoxAccountPort: fmt::Debug + Send + Sync { + /// Creates an account, minting its identity. + /// + /// # Errors + /// + /// Fails when the name is taken or the backend errors. + async fn create(&self, account: &NewProxmoxAccount) -> Result; + /// Reads one account. + /// + /// # Errors + /// + /// Fails when unknown or the backend errors. + async fn get(&self, id: &str) -> Result; + /// Lists accounts, newest first. + /// + /// # Errors + /// + /// Fails when the backend errors. + async fn list(&self) -> Result, String>; + /// Records the confirmed fingerprint, or clears it with `None`. + /// + /// # Errors + /// + /// Fails when unknown or the backend errors. + async fn set_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result; + /// Removes the account. + /// + /// # Errors + /// + /// Fails when unknown or the backend errors. + async fn delete(&self, id: &str) -> Result<(), String>; +} + +/// A creation request for one account. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct NewProxmoxAccount { + /// The operator-facing name. + pub name: String, + /// The PVE host (IP or DNS name). + pub host: String, + /// The API port; 8006 when omitted. + pub port: Option, + /// The API token id (`user@realm!tokenname`). + pub token_id: String, +} + +/// A use-case rejection, mapped onto public API errors by the adapter. +#[derive(Debug)] +pub enum ProxmoxUseCaseError { + /// The caller may not perform the action. + Denied(Decision), + /// The request is malformed. + Invalid { + /// What is wrong. + detail: String, + }, + /// The addressed account does not exist. + NotFound { + /// What was not found. + what: String, + }, + /// The account name is taken. + Conflict { + /// The conflict detail. + detail: String, + }, + /// The account has no pinned fingerprint yet, so no credential-carrying + /// call may go out. Confirm the fingerprint first — this is the + /// explicit-trust gate, not an error to work around. + UnconfirmedTrust { + /// The account name. + account: String, + }, + /// The account's token secret is not in the store. + NoSecret { + /// The account name. + account: String, + }, + /// The discovery source refused or failed. + Source(ProxmoxSourceError), + /// The credential store failed. + Credentials(CredentialStoreError), + /// A port failed. + Backend { + /// Where. + context: &'static str, + /// The failure detail. + detail: String, + }, +} + +impl fmt::Display for ProxmoxUseCaseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Denied(decision) => write!(f, "denied: {decision}"), + Self::Invalid { detail } => write!(f, "invalid request: {detail}"), + Self::NotFound { what } => write!(f, "not found: {what}"), + Self::Conflict { detail } => write!(f, "conflict: {detail}"), + Self::UnconfirmedTrust { account } => write!( + f, + "the host certificate for account {account} is not confirmed; observe and confirm the fingerprint first" + ), + Self::NoSecret { account } => write!( + f, + "the API token for account {account} is not in the secret store; re-create the account" + ), + Self::Source(error) => write!(f, "{error}"), + Self::Credentials(error) => write!(f, "{error}"), + Self::Backend { context, detail } => { + write!(f, "proxmox {context} failed: {detail}") + } + } + } +} + +impl std::error::Error for ProxmoxUseCaseError {} + +/// The discovery port. The provider implements this over the PVE API; +/// tests implement it over recorded fixtures. +#[async_trait] +pub trait ProxmoxDiscoverPort: fmt::Debug + Send + Sync { + /// Discovers the cluster through one bound account. + /// + /// The transport refuses credential-free trust probes by design, so a + /// discovery without a pinned fingerprint reports + /// [`ProxmoxSourceError::Connect`] upstream; the use case never reaches + /// this port without one. + /// + /// # Errors + /// + /// Fails with [`ProxmoxSourceError`]. + async fn discover( + &self, + account: &ProxmoxAccount, + secret: &SensitiveString, + ) -> Result; +} + +/// The provider's own discovery shape, before application-layer enrichment. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct RawDiscovery { + /// The PVE version seen. + pub version: String, + /// The normalized resources, without provenance. + pub resources: Vec, + /// The per-resource normalization warnings. + pub warnings: Vec, + /// The count the API reported. + pub reported_count: usize, +} + +/// The trust-probe port: captures a host's fingerprint without credentials. +/// The provider implements this with the observe-only TLS policy; tests +/// implement it over canned fingerprints. +#[async_trait] +pub trait ProxmoxTrustProbe: fmt::Debug + Send + Sync { + /// Observes the host's certificate fingerprint. No credential is sent: + /// the handshake is refused after capture, by construction. + /// + /// # Errors + /// + /// Fails when the host is unreachable or presents no certificate. + async fn observe(&self, host: &str, port: u16) -> Result; +} + +/// The Proxmox account/discovery use cases. +#[derive(Debug)] +pub struct ProxmoxAccounts { + accounts: Arc, + credentials: Arc, + discovery: Arc, + trust: Arc, + audit: Arc, +} + +impl ProxmoxAccounts { + /// Composes the service from its ports. + #[must_use] + pub fn new( + accounts: Arc, + credentials: Arc, + discovery: Arc, + trust: Arc, + audit: Arc, + ) -> Self { + Self { + accounts, + credentials, + discovery, + trust, + audit, + } + } + + /// Lists the configured accounts with their trust states. + /// + /// # Errors + /// + /// Fails on denial or a backend failure. + pub async fn list( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + ) -> Result, ProxmoxUseCaseError> { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxRead, + resource: None, + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + self.accounts + .list() + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + }) + } + + /// Registers an account and stores its token secret. The secret goes + /// into the encrypted store and is never echoed, logged, or audited; + /// the audit event names the account and the token id only. The new + /// account starts `Unconfirmed`: discovery stays locked until the + /// fingerprint is confirmed. + /// + /// # Errors + /// + /// Fails on denial, a malformed request, a name conflict, or a backend + /// failure. + pub async fn create( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + new: NewProxmoxAccount, + token_secret: &str, + ) -> Result { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxConfig, + resource: None, + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + validate_name(&new.name)?; + validate_host(&new.host)?; + validate_token_id(&new.token_id)?; + if token_secret.is_empty() || token_secret.len() > 256 { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the token secret must be 1..=256 characters".to_owned(), + }); + } + let account = self.accounts.create(&new).await.map_err(|detail| { + if is_taken(&detail) { + ProxmoxUseCaseError::Conflict { detail } + } else { + ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + } + } + })?; + // A failed secret write must not leave an account that pretends to + // be usable: the account is removed again. A clear of a record that + // was never written succeeds. + if let Err(error) = self.credentials.store(&account.id, token_secret).await { + let _ = self.accounts.delete(&account.id).await; + return Err(ProxmoxUseCaseError::Backend { + context: "credentials", + detail: error.to_string(), + }); + } + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(&account.id), + "proxmox_account_created", + // The token id contains "token", which the audit guard + // structurally rejects — and should: the account name carries + // the provenance, the credential material stays out entirely. + None, + ) + .await?; + Ok(account) + } + + /// Removes an account and its secret. Fleet keeps no other trace. + /// + /// # Errors + /// + /// Fails on denial, an unknown account, or a backend failure. + pub async fn delete( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + ) -> Result<(), ProxmoxUseCaseError> { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxConfig, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + let account = self.require_account(account_id).await?; + self.accounts + .delete(account_id) + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + })?; + // The secret's removal is best effort: the record is gone from the + // account surface either way, and a stuck store must not make the + // account undeletable. The detail is logged at the boundary. + if let Err(error) = self.credentials.clear(account_id).await { + let _ = error; + } + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(account_id), + "proxmox_account_deleted", + Some(("name", account.name.as_str())), + ) + .await?; + Ok(()) + } + + /// Captures the host's certificate fingerprint without sending any + /// credential. The report is the input to `confirm`; Fleet never pins + /// implicitly. + /// + /// # Errors + /// + /// Fails on denial, an unknown account, or an unreachable/anonymous + /// host. + pub async fn observe( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + ) -> Result { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxRead, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + let account = self.require_account(account_id).await?; + self.trust + .observe(&account.host, account.port) + .await + .map_err(ProxmoxUseCaseError::Source) + } + + /// Pins the confirmed fingerprint. Only a fingerprint this principal + /// observed through `observe` may be confirmed: the caller supplies it + /// explicitly, and it becomes the account's only trust anchor. + /// + /// # Errors + /// + /// Fails on denial, an unknown account, or a malformed fingerprint. + pub async fn confirm( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + fingerprint: &str, + ) -> Result { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxConfig, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + self.require_account(account_id).await?; + let normalized = normalize_fingerprint(fingerprint); + if normalized.len() != 64 || !normalized.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the fingerprint must be a SHA-256 digest (colons optional)".to_owned(), + }); + } + let account = self + .accounts + .set_fingerprint(account_id, Some(normalized)) + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + })?; + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(account_id), + "proxmox_fingerprint_confirmed", + Some(("fingerprint", account.fingerprint.as_deref().unwrap_or(""))), + ) + .await?; + Ok(account) + } + + /// Discovers the cluster through one trusted account. The snapshot is + /// availability-honest: a fingerprint mismatch, an auth failure, or a + /// privilege failure is the reported state — never an empty list. + /// + /// # Errors + /// + /// Fails on denial, an unconfirmed account, a missing secret, or any + /// source failure. + pub async fn discover( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + now: i64, + ) -> Result { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxRead, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + let account = self.require_account(account_id).await?; + // The explicit-trust gate: without a confirmed fingerprint no + // credential-carrying call leaves Fleet. This is the acceptance + // criterion, not a convenience check. + let Some(_pinned) = account.fingerprint.clone() else { + return Err(ProxmoxUseCaseError::UnconfirmedTrust { + account: account.name.clone(), + }); + }; + let secret = self + .credentials + .load(account_id) + .await + .map_err(ProxmoxUseCaseError::Credentials)? + .ok_or_else(|| ProxmoxUseCaseError::NoSecret { + account: account.name.clone(), + })?; + let raw = self + .discovery + .discover(&account, &SensitiveString::new(secret)) + .await + .map_err(ProxmoxUseCaseError::Source)?; + let resources = raw + .resources + .into_iter() + .map(|mut resource| { + resource.account_id.clone_from(&account.id); + resource.pve_version.clone_from(&raw.version); + resource.observed_at = now; + resource + }) + .collect(); + Ok(ProxmoxDiscovery { + account_id: account.id, + pve_version: raw.version, + resources, + warnings: raw.warnings, + reported_count: raw.reported_count, + observed_at: now, + }) + } + + async fn require_account(&self, id: &str) -> Result { + self.accounts.get(id).await.map_err(|detail| { + if detail.contains("not found") { + ProxmoxUseCaseError::NotFound { + what: format!("account {id}"), + } + } else { + ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + } + } + }) + } + + async fn audit_event( + &self, + principal: &ActingPrincipal, + action: Permission, + account_id: Option<&str>, + event: &str, + fact: Option<(&str, &str)>, + ) -> Result<(), ProxmoxUseCaseError> { + let mut metadata = crate::audit::AuditMetadata::default(); + metadata + .insert("event", event) + .map_err(|error| ProxmoxUseCaseError::Backend { + context: "audit", + detail: error.to_string(), + })?; + if let Some((key, value)) = fact { + metadata + .insert(key, value) + .map_err(|error| ProxmoxUseCaseError::Backend { + context: "audit", + detail: error.to_string(), + })?; + } + self.audit + .record_intent(&crate::audit::AuditIntent { + actor: principal.id.clone(), + action: action.id().to_owned(), + resource: account_id.map(str::to_owned), + decision: Decision::allow(), + correlation_id: None, + operation_id: None, + metadata, + }) + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "audit", + detail, + }) + } +} + +/// Normalizes a fingerprint for comparison. Exposed for the adapter layer's +/// echo handling. +#[must_use] +pub fn normalize_fingerprint(value: &str) -> String { + value.replace(':', "").to_uppercase() +} + +fn validate_name(name: &str) -> Result<(), ProxmoxUseCaseError> { + let count = name.chars().count(); + if count == 0 || count > 128 { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the name must be 1..=128 characters".to_owned(), + }); + } + Ok(()) +} + +fn validate_host(host: &str) -> Result<(), ProxmoxUseCaseError> { + let count = host.chars().count(); + if count == 0 || count > 253 { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the host must be 1..=253 characters".to_owned(), + }); + } + if host.starts_with("http") { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the host is a bare host or IP, not a URL".to_owned(), + }); + } + Ok(()) +} + +fn validate_token_id(token_id: &str) -> Result<(), ProxmoxUseCaseError> { + let count = token_id.chars().count(); + if count == 0 || count > 128 { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the token id must be 1..=128 characters".to_owned(), + }); + } + if !token_id.contains('@') || !token_id.contains('!') { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the token id must look like user@realm!tokenname".to_owned(), + }); + } + Ok(()) +} + +/// The storage adapter names the conflict; the application recognizes the +/// class without matching on adapter strings beyond this marker. +fn is_taken(detail: &str) -> bool { + detail.contains("taken") || detail.contains("UNIQUE") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fingerprints_normalize_for_comparison() { + assert_eq!(normalize_fingerprint("dc:2c:11"), "DC2C11"); + assert_eq!(normalize_fingerprint("ABCD"), "ABCD"); + } + + #[test] + fn token_ids_require_the_user_realm_token_shape() { + assert!(validate_token_id("root@pam!GLM-AGENT").is_ok()); + assert!(validate_token_id("root@pam").is_err()); + assert!(validate_token_id("GLM-AGENT").is_err()); + assert!(validate_token_id("").is_err()); + } + + #[test] + fn hosts_are_bare_not_urls() { + assert!(validate_host("192.168.68.223").is_ok()); + assert!(validate_host("pve.localdomain").is_ok()); + assert!(validate_host("https://pve:8006").is_err()); + assert!(validate_host("").is_err()); + } +} diff --git a/crates/fleet-application/tests/proxmox.rs b/crates/fleet-application/tests/proxmox.rs new file mode 100644 index 0000000..a7f6511 --- /dev/null +++ b/crates/fleet-application/tests/proxmox.rs @@ -0,0 +1,614 @@ +//! The Proxmox use cases over fakes: the explicit-trust gate (observe → +//! confirm → discover), credential secrecy, and the honest failure +//! taxonomy. + +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use fleet_application::audit::{AuditIntent, AuditOutcome}; +use fleet_application::authz::{AccessRequest, ActingPrincipal, Authorizer, Decision, ReasonId}; +use fleet_application::operation::AuditPort; +use fleet_application::proxmox::{ + CredentialStoreError, NewProxmoxAccount, ProxmoxAccountPort, ProxmoxAccounts, + ProxmoxCredentialStore, ProxmoxDiscoverPort, ProxmoxSourceError, ProxmoxTrustProbe, + ProxmoxUseCaseError, RawDiscovery, +}; +use fleet_core::SensitiveString; + +const NOW: i64 = 1_800_000_000_000; + +/// A full SHA-256-shaped fingerprint for the trust-flow tests. +const FP: &str = "DC2C116EC9C7EA618AA4E41EFB9BDEE4AA3D81EB16388F2B360AABE283A76498"; + +fn principal() -> ActingPrincipal { + ActingPrincipal { + id: "anonymous-lan-admin".to_owned(), + } +} + +#[derive(Debug, Default)] +struct AllowAll; + +impl Authorizer for AllowAll { + fn decide(&self, _request: AccessRequest<'_>) -> Decision { + Decision::allow() + } +} + +#[derive(Debug, Default)] +struct DenyAll; + +impl Authorizer for DenyAll { + fn decide(&self, _request: AccessRequest<'_>) -> Decision { + Decision::deny(ReasonId::UnknownPrincipal) + } +} + +/// The account port over an in-memory map, mirroring the SQLite adapter's +/// semantics (unique names, not-found details). +#[derive(Debug, Default)] +struct FakeAccounts { + accounts: Mutex>, +} + +impl FakeAccounts { + fn find(&self, id: &str) -> Option { + self.accounts + .lock() + .unwrap() + .iter() + .find(|account| account.id == id) + .cloned() + } +} + +#[async_trait] +impl ProxmoxAccountPort for FakeAccounts { + async fn create( + &self, + new: &NewProxmoxAccount, + ) -> Result { + if self + .accounts + .lock() + .unwrap() + .iter() + .any(|account| account.name == new.name) + { + return Err(format!("the account name {:?} is already taken", new.name)); + } + let account = fleet_application::proxmox::ProxmoxAccount { + id: format!("acc-{}", self.accounts.lock().unwrap().len() + 1), + name: new.name.clone(), + host: new.host.clone(), + port: new.port.unwrap_or(8006), + token_id: new.token_id.clone(), + fingerprint: None, + created_at: NOW, + }; + self.accounts.lock().unwrap().push(account.clone()); + Ok(account) + } + + async fn get(&self, id: &str) -> Result { + self.find(id) + .ok_or_else(|| format!("account {id} not found")) + } + + async fn list(&self) -> Result, String> { + Ok(self.accounts.lock().unwrap().clone()) + } + + async fn set_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result { + let mut accounts = self.accounts.lock().unwrap(); + let account = accounts + .iter_mut() + .find(|account| account.id == id) + .ok_or_else(|| format!("account {id} not found"))?; + account.fingerprint = fingerprint; + Ok(account.clone()) + } + + async fn delete(&self, id: &str) -> Result<(), String> { + let mut accounts = self.accounts.lock().unwrap(); + let before = accounts.len(); + accounts.retain(|account| account.id != id); + if accounts.len() == before { + return Err(format!("account {id} not found")); + } + Ok(()) + } +} + +/// The credential store over a map, recording what was stored and cleared. +#[derive(Debug, Default)] +struct FakeCredentials { + secrets: Mutex>, + cleared: Mutex>, +} + +#[async_trait] +impl ProxmoxCredentialStore for FakeCredentials { + async fn load(&self, account_id: &str) -> Result, CredentialStoreError> { + Ok(self.secrets.lock().unwrap().get(account_id).cloned()) + } + + async fn store(&self, account_id: &str, secret: &str) -> Result<(), CredentialStoreError> { + self.secrets + .lock() + .unwrap() + .insert(account_id.to_owned(), secret.to_owned()); + Ok(()) + } + + async fn clear(&self, account_id: &str) -> Result<(), CredentialStoreError> { + self.secrets.lock().unwrap().remove(account_id); + self.cleared.lock().unwrap().push(account_id.to_owned()); + Ok(()) + } +} + +/// The discovery port over canned results and failures. +#[derive(Debug, Default)] +struct FakeDiscovery { + result: Mutex>>, + calls: Mutex>, +} + +impl FakeDiscovery { + fn with(result: Result) -> Arc { + Arc::new(Self { + result: Mutex::new(Some(result)), + calls: Mutex::new(Vec::new()), + }) + } +} + +#[async_trait] +impl ProxmoxDiscoverPort for FakeDiscovery { + async fn discover( + &self, + account: &fleet_application::proxmox::ProxmoxAccount, + _secret: &SensitiveString, + ) -> Result { + self.calls.lock().unwrap().push(account.id.clone()); + self.result + .lock() + .unwrap() + .take() + .expect("a discovery answer was prepared") + } +} + +/// The trust probe over a canned fingerprint. +#[derive(Debug, Default)] +struct FakeProbe { + fingerprint: Mutex>, +} + +impl FakeProbe { + fn with(fingerprint: &str) -> Arc { + Arc::new(Self { + fingerprint: Mutex::new(Some(fingerprint.to_owned())), + }) + } +} + +#[async_trait] +impl ProxmoxTrustProbe for FakeProbe { + async fn observe(&self, _host: &str, _port: u16) -> Result { + self.fingerprint + .lock() + .unwrap() + .clone() + .ok_or_else(|| ProxmoxSourceError::Connect { + detail: "unreachable".to_owned(), + }) + } +} + +/// The audit sink over a vector, for secrecy assertions. +#[derive(Debug, Default)] +struct FakeAudit { + intents: Mutex>, +} + +#[async_trait] +impl AuditPort for FakeAudit { + async fn record_intent(&self, intent: &AuditIntent) -> Result<(), String> { + self.intents.lock().unwrap().push(intent.clone()); + Ok(()) + } + + async fn record_outcome( + &self, + _operation_id: &str, + _outcome: AuditOutcome, + ) -> Result<(), String> { + Ok(()) + } +} + +fn discovery_ok() -> RawDiscovery { + RawDiscovery { + version: "9.2.2".to_owned(), + resources: vec![fleet_application::proxmox::ProxmoxResource { + kind: "node".to_owned(), + id: "node/pve".to_owned(), + node: None, + vmid: None, + name: Some("pve".to_owned()), + status: Some("online".to_owned()), + account_id: String::new(), + pve_version: String::new(), + observed_at: 0, + }], + warnings: Vec::new(), + reported_count: 1, + } +} + +fn service( + discovery: Arc, + probe: Arc, +) -> (ProxmoxAccounts, Arc) { + let audit = Arc::new(FakeAudit::default()); + ( + ProxmoxAccounts::new( + Arc::new(FakeAccounts::default()), + Arc::new(FakeCredentials::default()), + discovery, + probe, + audit.clone(), + ), + audit, + ) +} + +async fn create_account(proxmox: &ProxmoxAccounts) -> fleet_application::proxmox::ProxmoxAccount { + proxmox + .create( + &AllowAll, + &principal(), + NewProxmoxAccount { + name: "pve-main".to_owned(), + host: "192.168.68.223".to_owned(), + port: Some(8006), + token_id: "root@pam!GLM-AGENT".to_owned(), + }, + "the-token-secret-material", + ) + .await + .expect("the account is well formed") +} + +#[tokio::test] +async fn discovery_is_locked_until_the_fingerprint_is_confirmed() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + let error = proxmox + .discover(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::UnconfirmedTrust { .. }), + "{error}" + ); +} + +#[tokio::test] +async fn observe_confirm_then_discover_walks_the_trust_flow() { + let (proxmox, audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); + let account = create_account(&proxmox).await; + + // Observe: the fingerprint arrives without any credential sent. + let observed = proxmox + .observe(&AllowAll, &principal(), &account.id) + .await + .unwrap(); + assert_eq!(observed, FP); + + // Confirm: the account becomes trusted, fingerprint normalized. + let account = proxmox + .confirm( + &AllowAll, + &principal(), + &account.id, + &fleet_application::proxmox::normalize_fingerprint(FP).to_lowercase(), + ) + .await + .unwrap(); + assert_eq!(account.fingerprint.as_deref(), Some(FP)); + + // Discover: the snapshot lands with provenance. + let snapshot = proxmox + .discover(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap(); + assert_eq!(snapshot.pve_version, "9.2.2"); + assert_eq!(snapshot.resources.len(), 1); + assert_eq!(snapshot.resources[0].account_id, account.id); + assert_eq!(snapshot.resources[0].pve_version, "9.2.2"); + assert_eq!(snapshot.resources[0].observed_at, NOW); + assert_eq!(snapshot.observed_at, NOW); + + // The trust flow is audited as account mutations. + let intents = audit.intents.lock().unwrap(); + assert!( + intents.iter().any(|intent| { + intent + .metadata + .entries() + .any(|(key, value)| key == "event" && value == "proxmox_fingerprint_confirmed") + }), + "{intents:?}" + ); +} + +#[tokio::test] +async fn a_fingerprint_mismatch_is_reported_as_evidence_not_an_empty_list() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Err(ProxmoxSourceError::FingerprintMismatch { + observed: FP.to_owned(), + pinned: "DD".repeat(32), + })), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + proxmox + .confirm(&AllowAll, &principal(), &account.id, FP) + .await + .unwrap(); + let error = proxmox + .discover(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + match error { + ProxmoxUseCaseError::Source(ProxmoxSourceError::FingerprintMismatch { + observed, + pinned, + }) => { + assert_eq!(observed, FP); + assert_eq!(pinned, "DD".repeat(32)); + } + other => panic!("expected a fingerprint mismatch, got {other}"), + } +} + +#[tokio::test] +async fn auth_and_privilege_failures_are_honest_states() { + for source_error in [ + ProxmoxSourceError::Auth, + ProxmoxSourceError::Forbidden { + detail: "no permission".to_owned(), + }, + ProxmoxSourceError::Connect { + detail: "timeout".to_owned(), + }, + ] { + let (proxmox, _audit) = service( + FakeDiscovery::with(Err(source_error)), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + proxmox + .confirm(&AllowAll, &principal(), &account.id, FP) + .await + .unwrap(); + let error = proxmox + .discover(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + assert!(matches!(error, ProxmoxUseCaseError::Source(_)), "{error}"); + } +} + +#[tokio::test] +async fn the_token_secret_never_surfaces_in_audit_or_errors() { + let (proxmox, audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + proxmox + .confirm(&AllowAll, &principal(), &account.id, FP) + .await + .unwrap(); + let _ = proxmox + .discover(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap(); + for intent in audit.intents.lock().unwrap().iter() { + let rendered = format!("{intent:?}"); + assert!( + !rendered.contains("the-token-secret-material"), + "the secret leaked into audit: {rendered}" + ); + } +} + +#[tokio::test] +async fn a_failed_secret_write_removes_the_account() { + // A store that refuses writes: the account must not survive half-made. + #[derive(Debug)] + struct RefusingStore; + #[async_trait] + impl ProxmoxCredentialStore for RefusingStore { + async fn load(&self, _account_id: &str) -> Result, CredentialStoreError> { + Ok(None) + } + async fn store( + &self, + _account_id: &str, + _secret: &str, + ) -> Result<(), CredentialStoreError> { + Err(CredentialStoreError::Backend { + detail: "the disk is full".to_owned(), + }) + } + async fn clear(&self, _account_id: &str) -> Result<(), CredentialStoreError> { + Ok(()) + } + } + let audit = Arc::new(FakeAudit::default()); + let proxmox = ProxmoxAccounts::new( + Arc::new(FakeAccounts::default()), + Arc::new(RefusingStore), + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + audit, + ); + let error = proxmox + .create( + &AllowAll, + &principal(), + NewProxmoxAccount { + name: "pve-main".to_owned(), + host: "192.168.68.223".to_owned(), + port: None, + token_id: "root@pam!GLM-AGENT".to_owned(), + }, + "the-token-secret-material", + ) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::Backend { .. }), + "{error}" + ); + let accounts = proxmox.list(&AllowAll, &principal()).await.unwrap(); + assert!(accounts.is_empty(), "the half-made account is gone"); +} + +#[tokio::test] +async fn deleting_an_account_clears_its_secret() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + proxmox + .delete(&AllowAll, &principal(), &account.id) + .await + .unwrap(); + assert!( + proxmox + .list(&AllowAll, &principal()) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test] +async fn a_denied_caller_never_reaches_the_ports() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + let error = proxmox.list(&DenyAll, &principal()).await.unwrap_err(); + assert!(matches!(error, ProxmoxUseCaseError::Denied(_))); + let error = proxmox + .discover(&DenyAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + assert!(matches!(error, ProxmoxUseCaseError::Denied(_))); +} + +#[tokio::test] +async fn malformed_accounts_are_refused_before_any_write() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + for new in [ + NewProxmoxAccount { + name: String::new(), + host: "192.168.68.223".to_owned(), + port: None, + token_id: "root@pam!GLM-AGENT".to_owned(), + }, + NewProxmoxAccount { + name: "pve-main".to_owned(), + host: "https://192.168.68.223".to_owned(), + port: None, + token_id: "root@pam!GLM-AGENT".to_owned(), + }, + NewProxmoxAccount { + name: "pve-main".to_owned(), + host: "192.168.68.223".to_owned(), + port: None, + token_id: "no-shape".to_owned(), + }, + ] { + let error = proxmox + .create(&AllowAll, &principal(), new, "secret") + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::Invalid { .. }), + "{error}" + ); + } + assert!( + proxmox + .list(&AllowAll, &principal()) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test] +async fn a_conflicting_account_name_is_a_conflict() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + create_account(&proxmox).await; + let error = proxmox + .create( + &AllowAll, + &principal(), + NewProxmoxAccount { + name: "pve-main".to_owned(), + host: "10.0.0.1".to_owned(), + port: None, + token_id: "root@pam!OTHER".to_owned(), + }, + "another-secret", + ) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::Conflict { .. }), + "{error}" + ); +} + +#[tokio::test] +async fn confirm_refuses_a_malformed_fingerprint() { + let (proxmox, _audit) = service( + FakeDiscovery::with(Ok(discovery_ok())), + FakeProbe::with("AA:BB"), + ); + let account = create_account(&proxmox).await; + for fingerprint in ["", "nothex", "A".repeat(63).as_str(), &"G".repeat(64)] { + let error = proxmox + .confirm(&AllowAll, &principal(), &account.id, fingerprint) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::Invalid { .. }), + "{error}" + ); + } +} diff --git a/crates/fleet-auth/tests/authz_adapter.rs b/crates/fleet-auth/tests/authz_adapter.rs index f50c9da..d19c3b9 100644 --- a/crates/fleet-auth/tests/authz_adapter.rs +++ b/crates/fleet-auth/tests/authz_adapter.rs @@ -103,7 +103,7 @@ fn every_catalog_action_has_a_unique_stable_id_and_a_risk_ruling() { assert!(Permission::MachineReadSensitive.is_risky()); assert!(!Permission::SystemRead.is_risky()); // The catalog is the complete vocabulary the adapter permits. - assert_eq!(Permission::ALL.len(), 36); + assert_eq!(Permission::ALL.len(), 38); } #[test] diff --git a/crates/fleet-controller/Cargo.toml b/crates/fleet-controller/Cargo.toml index 02df0e7..5634e0b 100644 --- a/crates/fleet-controller/Cargo.toml +++ b/crates/fleet-controller/Cargo.toml @@ -23,6 +23,7 @@ fleet-schema = { path = "../../schemas" } fleet-provider-mise = { version = "0.1.0", path = "../providers/fleet-provider-mise" } fleet-provider-skills-manager = { version = "0.1.0", path = "../providers/fleet-provider-skills-manager" } fleet-provider-tailscale = { path = "../providers/fleet-provider-tailscale" } +fleet-provider-proxmox = { path = "../providers/fleet-provider-proxmox" } fleet-secrets = { version = "0.1.0", path = "../fleet-secrets" } fleet-storage-sqlite = { version = "0.1.0", path = "../fleet-storage-sqlite" } futures-util = "0.3.31" diff --git a/crates/fleet-controller/src/lib.rs b/crates/fleet-controller/src/lib.rs index a45d0f3..3b0689d 100644 --- a/crates/fleet-controller/src/lib.rs +++ b/crates/fleet-controller/src/lib.rs @@ -20,6 +20,7 @@ pub mod install; pub mod mise; pub mod node_crypto; pub mod onboard; +pub mod proxmox_store; pub mod ready; pub mod skills; pub mod source; @@ -133,6 +134,7 @@ fn api_state( onboarding: Option>, tailnet: Option>, projects: Option>, + proxmox: Option>, ) -> fleet_api::operations::ApiState { let authorizer: std::sync::Arc = std::sync::Arc::new(fleet_auth::LanAllowAllAuthorizer); @@ -155,6 +157,7 @@ fn api_state( onboarding, tailnet, projects, + proxmox, }; } // Without a store there is nothing to serve: the state's backends answer @@ -170,6 +173,7 @@ fn api_state( onboarding: None, tailnet: None, projects: None, + proxmox: None, } } @@ -273,6 +277,7 @@ pub fn build_router( onboarding: Option<&Arc>, tailnet: Option<&Arc>, projects: Option<&Arc>, + proxmox: Option<&Arc>, ) -> Router { let probe = Probe { web_dist_ready: settings.web_dist.join("index.html").is_file(), @@ -284,6 +289,7 @@ pub fn build_router( onboarding.cloned(), tailnet.cloned(), projects.cloned(), + proxmox.cloned(), )); let shell = shell(settings).fallback(fleet_api::router(api_state.clone())); let mut router = Router::new() @@ -361,6 +367,7 @@ async fn readyz(State(probe): State) -> (StatusCode, String) { /// # Errors /// /// Fails if the listener cannot be bound or the server stops on an I/O error. +#[allow(clippy::too_many_arguments)] pub async fn serve( settings: Settings, db: Option, @@ -368,11 +375,12 @@ pub async fn serve( onboarding: Option>, tailnet: Option>, projects: Option>, + proxmox: Option>, shutdown: impl Future + Send + 'static, ) -> io::Result<()> { let listener = tokio::net::TcpListener::bind(settings.listen).await?; serve_on( - listener, settings, db, services, onboarding, tailnet, projects, shutdown, + listener, settings, db, services, onboarding, tailnet, projects, proxmox, shutdown, ) .await } @@ -393,6 +401,7 @@ pub async fn serve_on( onboarding: Option>, tailnet: Option>, projects: Option>, + proxmox: Option>, shutdown: impl Future + Send + 'static, ) -> io::Result<()> { eprintln!("{}", fleet_auth::TrustMode::TrustedLan.warning()); @@ -429,6 +438,7 @@ pub async fn serve_on( onboarding.as_ref(), tailnet.as_ref(), projects.as_ref(), + proxmox.as_ref(), ) .into_make_service_with_connect_info::(), ) diff --git a/crates/fleet-controller/src/main.rs b/crates/fleet-controller/src/main.rs index f8d4e44..3cb6f81 100644 --- a/crates/fleet-controller/src/main.rs +++ b/crates/fleet-controller/src/main.rs @@ -347,6 +347,20 @@ fn run_serve(config: fleet_config::ControllerConfig) -> ExitCode { )), std::sync::Arc::new(fleet_storage_sqlite::AuditSink::new(store.pool().clone())), )); + // The Proxmox surface composes over the store, the secret store, + // and the provider's pinned-fingerprint transport. Without a secret + // store it serves the standard "unavailable" envelope. + let proxmox = secrets.as_ref().map(|secrets| { + std::sync::Arc::new(fleet_controller::proxmox_store::compose_proxmox( + store.pool().clone(), + secrets.clone(), + std::sync::Arc::new( + fleet_provider_proxmox::ReqwestPveTransport::new() + .expect("the PVE transport must build"), + ), + std::sync::Arc::new(fleet_storage_sqlite::AuditSink::new(store.pool().clone())), + )) + }); let served = serve( settings, pool, @@ -354,6 +368,7 @@ fn run_serve(config: fleet_config::ControllerConfig) -> ExitCode { Some(onboarding), tailnet, Some(projects), + proxmox, shutdown_signal(), ) .await; diff --git a/crates/fleet-controller/src/proxmox_store.rs b/crates/fleet-controller/src/proxmox_store.rs new file mode 100644 index 0000000..2b6425c --- /dev/null +++ b/crates/fleet-controller/src/proxmox_store.rs @@ -0,0 +1,320 @@ +//! The Proxmox integration's composition: the credential store over the +//! encrypted secret store, the trust probe and discovery source over the +//! provider, and the use-case wiring (FM-600). +//! +//! Each account's API token lives as one secret record named +//! `proxmox/` inside Fleet's encrypted store — the same store +//! every controller credential uses. The value is resolved just in time at +//! the discovery boundary; nothing else in the controller reads it, and +//! deleting the account deletes the record. +//! +//! The trust probe composes the provider's observe-only TLS policy: the +//! handshake is refused after the fingerprint is captured, so no credential +//! can be sent during a probe. Discovery composes the pinned-verifier +//! transport; an account without a confirmed fingerprint never reaches it. + +use std::sync::Arc; + +use async_trait::async_trait; +use fleet_application::proxmox::{ + ProxmoxCredentialStore, ProxmoxDiscoverPort, ProxmoxSourceError, ProxmoxTrustProbe, + RawDiscovery, +}; +use fleet_core::SensitiveString; +use fleet_provider_proxmox::{ProxmoxSource as _, PveCredentials, PveHttpRequest, PveTransport}; +use fleet_secrets::{SecretStore, SecretValue}; + +/// The secret-record name prefix for account tokens. The account id +/// completes it. +pub const SECRET_PREFIX: &str = "proxmox/"; + +fn secret_name(account_id: &str) -> String { + format!("{SECRET_PREFIX}{account_id}") +} + +/// The credential store over the secret store. +#[derive(Debug)] +pub struct SecretBackedProxmoxCredentials { + secrets: Arc, +} + +impl SecretBackedProxmoxCredentials { + /// Composes the store over the controller's secret store. + #[must_use] + pub fn new(secrets: Arc) -> Self { + Self { secrets } + } + + async fn record_id(&self, name: &str) -> Result, String> { + Ok(self + .secrets + .list() + .await + .map_err(|error| format!("the secret store is unreadable: {error}"))? + .into_iter() + .find(|record| record.name == name) + .map(|record| record.id)) + } +} + +#[async_trait] +impl ProxmoxCredentialStore for SecretBackedProxmoxCredentials { + async fn load( + &self, + account_id: &str, + ) -> Result, fleet_application::proxmox::CredentialStoreError> { + let name = secret_name(account_id); + let Some(id) = self.record_id(&name).await.map_err(|detail| { + fleet_application::proxmox::CredentialStoreError::Backend { detail } + })? + else { + return Ok(None); + }; + let value = self.secrets.resolve(&id).await.map_err(|error| { + fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("the secret record {name:?} is unreadable: {error}"), + } + })?; + String::from_utf8(value.expose().to_vec()) + .map(Some) + .map_err( + |_| fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("the secret record {name:?} is not UTF-8"), + }, + ) + } + + async fn store( + &self, + account_id: &str, + secret: &str, + ) -> Result<(), fleet_application::proxmox::CredentialStoreError> { + let name = secret_name(account_id); + if let Some(id) = self.record_id(&name).await.map_err(|detail| { + fleet_application::proxmox::CredentialStoreError::Backend { detail } + })? { + return self + .secrets + .update(&id, SecretValue::new(secret.as_bytes().to_vec())) + .await + .map(|_| ()) + .map_err( + |error| fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("cannot update {name:?}: {error}"), + }, + ); + } + match self + .secrets + .create(&name, SecretValue::new(secret.as_bytes().to_vec())) + .await + { + Ok(_) => Ok(()), + Err(fleet_secrets::SecretError::DuplicateName { .. }) => { + let id = + self.record_id(&name) + .await + .map_err(|detail| { + fleet_application::proxmox::CredentialStoreError::Backend { detail } + })? + .ok_or_else(|| { + fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("cannot store {name:?}: vanished after the race"), + } + })?; + self.secrets + .update(&id, SecretValue::new(secret.as_bytes().to_vec())) + .await + .map(|_| ()) + .map_err( + |error| fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("cannot update {name:?}: {error}"), + }, + ) + } + Err(other) => Err(fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("cannot store {name:?}: {other}"), + }), + } + } + + async fn clear( + &self, + account_id: &str, + ) -> Result<(), fleet_application::proxmox::CredentialStoreError> { + let name = secret_name(account_id); + if let Some(id) = self.record_id(&name).await.map_err(|detail| { + fleet_application::proxmox::CredentialStoreError::Backend { detail } + })? { + self.secrets.delete(&id).await.map_err(|error| { + fleet_application::proxmox::CredentialStoreError::Backend { + detail: format!("cannot delete {name:?}: {error}"), + } + })?; + } + Ok(()) + } +} + +/// The trust probe over the provider's observe-only TLS policy: no +/// credential exists in this path at all. +#[derive(Debug)] +pub struct ProviderTrustProbe { + transport: Arc, +} + +impl ProviderTrustProbe { + /// Composes the probe over a transport. + #[must_use] + pub fn new(transport: Arc) -> Self { + Self { transport } + } +} + +#[async_trait] +impl ProxmoxTrustProbe for ProviderTrustProbe { + async fn observe(&self, host: &str, port: u16) -> Result { + // A probe carries no credential: the token fields are placeholders + // that are never sent, because the observe policy refuses the + // handshake before any HTTP request is completed. + let request = PveHttpRequest { + host: host.to_owned(), + port, + path: "/api2/json/version".to_owned(), + pinned_fingerprint: None, + credentials: Arc::new(PveCredentials { + token_id: "observe-only".to_owned(), + token: SensitiveString::new("observe-only"), + }), + }; + match self.transport.execute(request).await { + Err(fleet_provider_proxmox::PveTransportError::ObserveRefused { observed }) => { + Ok(observed) + } + Err(fleet_provider_proxmox::PveTransportError::NoCertificate) => { + Err(ProxmoxSourceError::Connect { + detail: "the host presented no certificate".to_owned(), + }) + } + Err(other) => Err(ProxmoxSourceError::Connect { + detail: other.to_string(), + }), + // A host that accepts the observe probe cannot exist: the + // policy refuses every handshake. If a future transport changes + // that, refuse here rather than trusting silently. + Ok(_) => Err(ProxmoxSourceError::Connect { + detail: "the observe probe must refuse; refusing to trust".to_owned(), + }), + } + } +} + +/// The discovery source over the provider client. +#[derive(Debug)] +pub struct ProviderDiscovery { + client: fleet_provider_proxmox::ProxmoxClient, +} + +impl ProviderDiscovery { + /// Composes the source over the provider client. + #[must_use] + pub fn new(client: fleet_provider_proxmox::ProxmoxClient) -> Self { + Self { client } + } +} + +#[async_trait] +impl ProxmoxDiscoverPort for ProviderDiscovery { + async fn discover( + &self, + account: &fleet_application::proxmox::ProxmoxAccount, + secret: &SensitiveString, + ) -> Result { + let Some(pinned) = account.fingerprint.clone() else { + // The use case gates this; a source call without a pin is a + // composition defect. Refuse loudly rather than probing with a + // credential. + return Err(ProxmoxSourceError::Connect { + detail: "the account has no confirmed fingerprint; refusing to send credentials" + .to_owned(), + }); + }; + let request = PveHttpRequest { + host: account.host.clone(), + port: account.port, + path: "/api2/json/cluster/resources".to_owned(), + pinned_fingerprint: Some(pinned.clone()), + credentials: Arc::new(PveCredentials { + token_id: account.token_id.clone(), + token: SensitiveString::new(secret.expose().to_owned()), + }), + }; + match self.client.discover(request).await { + Ok(discovery) => Ok(RawDiscovery { + version: discovery.version.clone(), + resources: discovery + .resources + .into_iter() + .map(|resource| fleet_application::proxmox::ProxmoxResource { + kind: resource.kind, + id: resource.id, + node: resource.node, + vmid: resource.vmid, + name: resource.name, + status: resource.status, + account_id: account.id.clone(), + pve_version: discovery.version.clone(), + observed_at: 0, + }) + .collect(), + warnings: discovery.warnings, + reported_count: discovery.reported_count, + }), + Err(fleet_provider_proxmox::PveApiError::Auth) => Err(ProxmoxSourceError::Auth), + Err(fleet_provider_proxmox::PveApiError::Forbidden { detail }) => { + Err(ProxmoxSourceError::Forbidden { detail }) + } + Err(fleet_provider_proxmox::PveApiError::Http { status, detail }) => { + Err(ProxmoxSourceError::Http { status, detail }) + } + Err(fleet_provider_proxmox::PveApiError::InvalidPayload { detail }) => { + Err(ProxmoxSourceError::InvalidPayload { detail }) + } + Err(fleet_provider_proxmox::PveApiError::Transport( + fleet_provider_proxmox::PveTransportError::FingerprintMismatch { observed, pinned }, + )) => Err(ProxmoxSourceError::FingerprintMismatch { + observed, + pinned: pinned.unwrap_or(pinned_placeholder()), + }), + Err(fleet_provider_proxmox::PveApiError::Transport(other)) => { + Err(ProxmoxSourceError::Connect { + detail: other.to_string(), + }) + } + } + } +} + +fn pinned_placeholder() -> String { + // Unreachable in practice: the discovery request always pins. Kept for + // exhaustive matching without a panic path. + String::new() +} + +/// Composes the Proxmox use cases over its ports. +#[must_use] +pub fn compose_proxmox( + pool: sqlx::SqlitePool, + secrets: Arc, + transport: Arc, + audit: Arc, +) -> fleet_application::proxmox::ProxmoxAccounts { + let client = fleet_provider_proxmox::ProxmoxClient::new(transport.clone()); + fleet_application::proxmox::ProxmoxAccounts::new( + Arc::new(fleet_storage_sqlite::ProxmoxAccountRepository::new(pool)), + Arc::new(SecretBackedProxmoxCredentials::new(secrets)), + Arc::new(ProviderDiscovery::new(client)), + Arc::new(ProviderTrustProbe::new(transport)), + audit, + ) +} diff --git a/crates/fleet-controller/tests/browser_guard.rs b/crates/fleet-controller/tests/browser_guard.rs index bc10cba..0dfea4c 100644 --- a/crates/fleet-controller/tests/browser_guard.rs +++ b/crates/fleet-controller/tests/browser_guard.rs @@ -44,6 +44,7 @@ async fn router_with_db() -> (axum::Router, Vec) { None, None, None, + None, ); (router, vec![dist, dir]) } diff --git a/crates/fleet-controller/tests/gateway.rs b/crates/fleet-controller/tests/gateway.rs index 548905e..855dd11 100644 --- a/crates/fleet-controller/tests/gateway.rs +++ b/crates/fleet-controller/tests/gateway.rs @@ -88,6 +88,7 @@ async fn harness() -> Harness { None, None, None, + None, ); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); diff --git a/crates/fleet-controller/tests/machines.rs b/crates/fleet-controller/tests/machines.rs index 2fe24d6..ab790fa 100644 --- a/crates/fleet-controller/tests/machines.rs +++ b/crates/fleet-controller/tests/machines.rs @@ -63,6 +63,7 @@ async fn harness() -> Harness { None, None, None, + None, ); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); diff --git a/crates/fleet-controller/tests/node_enrollment.rs b/crates/fleet-controller/tests/node_enrollment.rs index 89b9d50..e48f0ad 100644 --- a/crates/fleet-controller/tests/node_enrollment.rs +++ b/crates/fleet-controller/tests/node_enrollment.rs @@ -84,6 +84,7 @@ async fn controller() -> TestController { None, None, None, + None, ); TestController { _dist: dist, diff --git a/crates/fleet-controller/tests/onboarding.rs b/crates/fleet-controller/tests/onboarding.rs index 28bc4fc..2e83f3c 100644 --- a/crates/fleet-controller/tests/onboarding.rs +++ b/crates/fleet-controller/tests/onboarding.rs @@ -193,6 +193,7 @@ async fn harness() -> Harness { Some(&onboarding), None, None, + None, ); let listener = TokioListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); diff --git a/crates/fleet-controller/tests/proxmox.rs b/crates/fleet-controller/tests/proxmox.rs new file mode 100644 index 0000000..ba838d9 --- /dev/null +++ b/crates/fleet-controller/tests/proxmox.rs @@ -0,0 +1,405 @@ +//! The Proxmox surface end to end: a real controller router, a secret +//! store, and a fake provider transport over recorded PVE fixtures — +//! create, observe, confirm, discover, and delete, with the trust gate +//! enforced at the surface. + +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use fleet_controller::Settings; +use fleet_controller::build_router; +use fleet_controller::proxmox_store::compose_proxmox; +use fleet_provider_proxmox::{PveHttpRequest, PveHttpResponse, PveTransport, PveTransportError}; +use fleet_secrets::SecretStore; +use fleet_storage_sqlite::Store; +use serde_json::{Value, json}; +use tokio::net::TcpListener as TokioListener; + +const VERSION_BODY: &str = + r#"{"data":{"release":"9.2","version":"9.2.2","repoid":"b9984c6d90a4bd80"}}"#; + +const RESOURCES_BODY: &str = r#"{"data":[ + {"id":"node/pve","type":"node","status":"online","maxcpu":16,"maxmem":67342831616}, + {"id":"qemu/100","type":"qemu","node":"pve","vmid":100,"name":"dev-01","status":"running","template":0}, + {"id":"qemu/101","type":"qemu","node":"pve","vmid":101,"name":"fleet-test-01","status":"stopped","template":0}, + {"id":"qemu/900","type":"qemu","vmid":900,"template":1,"status":"stopped"}, + {"id":"sdn/zone1","type":"sdn"} +]}"#; + +/// The pinned fingerprint the fake transport accepts. +const FP: &str = "DC2C116EC9C7EA618AA4E41EFB9BDEE4AA3D81EB16388F2B360AABE283A76498"; + +#[derive(Debug)] +struct FixedTransport { + behavior: Mutex, +} + +#[derive(Debug, Clone, Copy)] +enum Behavior { + /// Observe captures the fingerprint and refuses; pinned calls succeed. + Normal, + /// Pinned calls report a fingerprint mismatch with the observed value. + Mismatch, +} + +impl FixedTransport { + fn normal() -> Arc { + Arc::new(Self { + behavior: Mutex::new(Behavior::Normal), + }) + } + + fn mismatch() -> Arc { + Arc::new(Self { + behavior: Mutex::new(Behavior::Mismatch), + }) + } +} + +#[async_trait] +impl PveTransport for FixedTransport { + async fn execute(&self, request: PveHttpRequest) -> Result { + let behavior = *self.behavior.lock().unwrap(); + match (&request.pinned_fingerprint, behavior) { + (None, _) => Err(PveTransportError::ObserveRefused { + observed: FP.to_owned(), + }), + (Some(pinned), Behavior::Mismatch) => Err(PveTransportError::FingerprintMismatch { + observed: "AA:".repeat(31) + "AA", + pinned: Some(pinned.clone()), + }), + (Some(_), Behavior::Normal) => { + let body = if request.path.contains("/version") { + VERSION_BODY + } else { + RESOURCES_BODY + }; + Ok(PveHttpResponse { + status: 200, + body: body.as_bytes().to_vec(), + }) + } + } + } +} + +struct Harness { + _dist: tempfile::TempDir, + _store_dir: tempfile::TempDir, + _key_dir: tempfile::TempDir, + address: std::net::SocketAddr, + shutdown: Option>, +} + +async fn harness_with(transport: Arc) -> Harness { + let dist = tempfile::tempdir().unwrap(); + std::fs::write(dist.path().join("index.html"), "fleet").unwrap(); + let store_dir = tempfile::tempdir().unwrap(); + let store = Store::open(&store_dir.path().join("fleet.db")) + .await + .unwrap(); + let key_dir = tempfile::tempdir().unwrap(); + let key_path = key_dir.path().join("master.key"); + std::fs::write( + &key_path, + "1 0a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20212223242526272829\n", + ) + .unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600)).unwrap(); + } + let secrets = Arc::new(SecretStore::open(store.pool().clone(), &key_path).unwrap()); + let proxmox = Arc::new(compose_proxmox( + store.pool().clone(), + secrets, + transport, + Arc::new(fleet_storage_sqlite::AuditSink::new(store.pool().clone())), + )); + + let settings = Settings { + listen: "127.0.0.1:0".parse().unwrap(), + web_dist: dist.path().to_path_buf(), + artifacts_dir: None, + }; + let router = build_router( + &settings, + Some(store.pool().clone()), + None, + None, + None, + None, + Some(&proxmox), + ); + let listener = TokioListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel::<()>(); + tokio::spawn(async move { + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(async { + let _ = shutdown_rx.await; + }) + .await + .expect("the test server must serve"); + }); + Harness { + _dist: dist, + _store_dir: store_dir, + _key_dir: key_dir, + address, + shutdown: Some(shutdown_tx), + } +} + +async fn harness() -> Harness { + harness_with(FixedTransport::normal()).await +} + +impl Drop for Harness { + fn drop(&mut self) { + if let Some(shutdown) = self.shutdown.take() { + let _ = shutdown.send(()); + } + } +} + +impl Harness { + async fn get(&self, path: &str) -> (axum::http::StatusCode, Value) { + raw(self.address, "GET", path, None).await + } + + async fn post(&self, path: &str, body: Value) -> (axum::http::StatusCode, Value) { + raw(self.address, "POST", path, Some(body)).await + } + + async fn delete(&self, path: &str) -> (axum::http::StatusCode, Value) { + raw(self.address, "DELETE", path, None).await + } +} + +/// One raw HTTP request over TCP; the test asserts on bodies, not clients. +async fn raw( + address: std::net::SocketAddr, + method: &str, + path: &str, + body: Option, +) -> (axum::http::StatusCode, Value) { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + let mut stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let mut head = format!("{method} {path} HTTP/1.1\r\nHost: test\r\n"); + if let Some(body) = &body { + head.push_str(&format!( + "Content-Type: application/json\r\nContent-Length: {}\r\n", + body.to_string().len() + )); + } + head.push_str("Connection: close\r\n\r\n"); + stream.write_all(head.as_bytes()).await.unwrap(); + if let Some(body) = &body { + stream.write_all(body.to_string().as_bytes()).await.unwrap(); + } + let mut response = Vec::new(); + stream.read_to_end(&mut response).await.unwrap(); + let text = String::from_utf8_lossy(&response); + let (head, rest) = text.split_once("\r\n\r\n").expect("an HTTP response"); + let status = head + .lines() + .next() + .and_then(|line| line.split_whitespace().nth(1)) + .and_then(|code| code.parse::().ok()) + .map(axum::http::StatusCode::from_u16) + .unwrap() + .unwrap(); + let body = if rest.trim().is_empty() { + Value::Null + } else { + serde_json::from_str(rest).unwrap_or(Value::Null) + }; + (status, body) +} + +#[tokio::test] +async fn the_proxmox_surface_walks_create_observe_confirm_discover_delete() { + let harness = harness().await; + + // Empty at first. + let (status, body) = harness.get("/api/v1/proxmox/accounts").await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + assert_eq!(body["items"].as_array().unwrap().len(), 0); + + // Create: the secret is accepted and never echoed. + let (status, body) = harness + .post( + "/api/v1/proxmox/accounts", + json!({ + "name": "pve-main", + "host": "192.168.68.223", + "port": 8006, + "tokenId": "root@pam!GLM-AGENT", + "tokenSecret": "the-token-secret-material" + }), + ) + .await; + assert_eq!(status, axum::http::StatusCode::CREATED, "{body}"); + assert_eq!(body["data"]["fingerprintState"], "unconfirmed"); + let rendered = body.to_string(); + assert!( + !rendered.contains("the-token-secret-material"), + "the secret is write-only: {rendered}" + ); + let account_id = body["data"]["id"].as_str().unwrap().to_owned(); + + // Discovery is locked before trust. + let (status, body) = harness + .get(&format!("/api/v1/proxmox/accounts/{account_id}/discovery")) + .await; + assert_eq!(status, axum::http::StatusCode::CONFLICT, "{body}"); + assert_eq!(body["code"], "proxmox_unconfirmed"); + + // Observe: the fingerprint arrives; no credential is ever sent. + let (status, body) = harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/observe"), + json!({}), + ) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + assert_eq!(body["data"]["fingerprint"], FP); + + // Confirm: trust is pinned. + let (status, body) = harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/confirm"), + json!({"fingerprint": FP}), + ) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + assert_eq!(body["data"]["fingerprintState"], "confirmed"); + + // Discover: the normalized snapshot, sdn isolated as a warning-free skip. + let (status, body) = harness + .get(&format!("/api/v1/proxmox/accounts/{account_id}/discovery")) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + assert_eq!(body["data"]["pveVersion"], "9.2.2"); + let resources = body["data"]["resources"].as_array().unwrap(); + assert_eq!(resources.len(), 4, "{body}"); + assert!( + resources + .iter() + .any(|resource| resource["kind"] == "qemu-template" && resource["vmid"] == 900) + ); + assert!( + resources.iter().all(|resource| resource["kind"] != "sdn"), + "non-resource types are skipped, not coerced" + ); + assert_eq!(body["data"]["reportedCount"], 5); + + // Delete: the account and its secret are gone. + let (status, _) = harness + .delete(&format!("/api/v1/proxmox/accounts/{account_id}")) + .await; + assert_eq!(status, axum::http::StatusCode::NO_CONTENT); + let (status, body) = harness.get("/api/v1/proxmox/accounts").await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + assert_eq!(body["items"].as_array().unwrap().len(), 0); +} + +#[tokio::test] +async fn a_mismatched_fingerprint_is_reported_with_both_values() { + let harness = harness_with(FixedTransport::mismatch()).await; + let (status, body) = harness + .post( + "/api/v1/proxmox/accounts", + json!({ + "name": "pve-main", + "host": "192.168.68.223", + "tokenId": "root@pam!GLM-AGENT", + "tokenSecret": "the-token-secret-material" + }), + ) + .await; + assert_eq!(status, axum::http::StatusCode::CREATED, "{body}"); + let account_id = body["data"]["id"].as_str().unwrap().to_owned(); + harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/confirm"), + json!({"fingerprint": FP}), + ) + .await; + let (status, body) = harness + .get(&format!("/api/v1/proxmox/accounts/{account_id}/discovery")) + .await; + assert_eq!(status, axum::http::StatusCode::CONFLICT, "{body}"); + assert_eq!(body["code"], "proxmox_fingerprint_mismatch"); + let message = body["message"].as_str().unwrap_or_default(); + assert!(message.contains("AA:"), "{message}"); + assert!(message.contains("DC2C"), "{message}"); +} + +#[tokio::test] +async fn a_malformed_create_request_is_refused_publicly() { + let harness = harness().await; + let (status, body) = harness + .post( + "/api/v1/proxmox/accounts", + json!({ + "name": "pve-main", + "host": "https://192.168.68.223", + "tokenId": "root@pam!GLM-AGENT", + "tokenSecret": "the-token-secret-material" + }), + ) + .await; + assert_eq!(status, axum::http::StatusCode::BAD_REQUEST, "{body}"); + assert_eq!(body["code"], "invalid_request"); + + // A URL-shaped host is refused: the host is a bare host or IP. + let (status, body) = harness + .post( + "/api/v1/proxmox/accounts", + json!({ + "name": "pve-main", + "host": "192.168.68.223", + "tokenId": "not-a-token-id", + "tokenSecret": "the-token-secret-material" + }), + ) + .await; + assert_eq!(status, axum::http::StatusCode::BAD_REQUEST, "{body}"); + assert_eq!(body["code"], "invalid_request"); +} + +#[tokio::test] +async fn a_duplicate_account_name_conflicts() { + let harness = harness().await; + let payload = json!({ + "name": "pve-main", + "host": "192.168.68.223", + "tokenId": "root@pam!GLM-AGENT", + "tokenSecret": "the-token-secret-material" + }); + let (status, _) = harness + .post("/api/v1/proxmox/accounts", payload.clone()) + .await; + assert_eq!(status, axum::http::StatusCode::CREATED); + let (status, body) = harness.post("/api/v1/proxmox/accounts", payload).await; + assert_eq!(status, axum::http::StatusCode::CONFLICT, "{body}"); + assert_eq!(body["code"], "conflict"); +} + +#[tokio::test] +async fn an_unknown_account_refuses_with_not_found() { + let harness = harness().await; + let (status, body) = harness + .get("/api/v1/proxmox/accounts/acc-missing/discovery") + .await; + assert_eq!(status, axum::http::StatusCode::NOT_FOUND, "{body}"); + assert_eq!(body["code"], "not_found"); + let (status, _) = harness.delete("/api/v1/proxmox/accounts/acc-missing").await; + assert_eq!(status, axum::http::StatusCode::NOT_FOUND); +} diff --git a/crates/fleet-controller/tests/serve.rs b/crates/fleet-controller/tests/serve.rs index 2dd2885..bb1a64a 100644 --- a/crates/fleet-controller/tests/serve.rs +++ b/crates/fleet-controller/tests/serve.rs @@ -50,9 +50,19 @@ async fn spawn( .expect("bound listener must report its address"); let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); tokio::spawn(async move { - serve_on(listener, settings, db, None, None, None, None, async { - let _ = shutdown_rx.await; - }) + serve_on( + listener, + settings, + db, + None, + None, + None, + None, + None, + async { + let _ = shutdown_rx.await; + }, + ) .await .expect("the server must serve without I/O errors"); }); @@ -158,6 +168,7 @@ async fn graceful_shutdown_stops_the_server_and_releases_the_listener() { None, None, None, + None, async { let _ = shutdown_rx.await; }, diff --git a/crates/fleet-controller/tests/tailnet.rs b/crates/fleet-controller/tests/tailnet.rs index 91d261d..6d2cea4 100644 --- a/crates/fleet-controller/tests/tailnet.rs +++ b/crates/fleet-controller/tests/tailnet.rs @@ -141,6 +141,7 @@ async fn harness() -> Harness { None, Some(&tailnet), None, + None, ); let listener = TokioListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); diff --git a/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql new file mode 100644 index 0000000..fef5ab0 --- /dev/null +++ b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql @@ -0,0 +1,15 @@ +-- FM-600: Proxmox accounts — multi-account API-token hosts with TLS trust +-- state. The token secret never lives here: accounts reference Fleet's +-- encrypted secret store by account id. The fingerprint is the pinned +-- SHA-256 of the host certificate, empty until the trust step confirms it. +CREATE TABLE proxmox_accounts ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + host TEXT NOT NULL, + port INTEGER NOT NULL, + token_id TEXT NOT NULL, + fingerprint TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL +) STRICT; + +CREATE INDEX proxmox_accounts_created ON proxmox_accounts (created_at DESC); diff --git a/crates/fleet-storage-sqlite/src/lib.rs b/crates/fleet-storage-sqlite/src/lib.rs index e046460..2488b44 100644 --- a/crates/fleet-storage-sqlite/src/lib.rs +++ b/crates/fleet-storage-sqlite/src/lib.rs @@ -18,6 +18,7 @@ pub mod nodes; pub mod onboarding; pub mod operations; pub mod projects; +pub mod proxmox; pub mod source; pub use audit::{AuditLedger, AuditSink}; @@ -26,6 +27,7 @@ pub use nodes::NodeRepository; pub use onboarding::OnboardingRepository; pub use operations::OperationRepository; pub use projects::ProjectRepository; +pub use proxmox::ProxmoxAccountRepository; pub use source::SourceRepository; use std::path::{Path, PathBuf}; diff --git a/crates/fleet-storage-sqlite/src/proxmox.rs b/crates/fleet-storage-sqlite/src/proxmox.rs new file mode 100644 index 0000000..766180e --- /dev/null +++ b/crates/fleet-storage-sqlite/src/proxmox.rs @@ -0,0 +1,124 @@ +//! The Proxmox account repository: the SQLite implementation of the +//! application's [`ProxmoxAccountPort`]. +//! +//! Accounts reference their token secret through the encrypted secret +//! store by account id; nothing here ever touches secret values. The +//! fingerprint column is the pinned SHA-256 of the host certificate, empty +//! until the trust step confirms it. + +use async_trait::async_trait; +use sqlx::Row as _; +use sqlx::SqlitePool; +use uuid::Uuid; + +use fleet_application::proxmox::{NewProxmoxAccount, ProxmoxAccount, ProxmoxAccountPort}; + +/// The Proxmox account repository over a pool. +#[derive(Debug)] +pub struct ProxmoxAccountRepository { + pool: SqlitePool, +} + +impl ProxmoxAccountRepository { + /// Creates a repository over the store's pool. + #[must_use] + pub fn new(pool: SqlitePool) -> Self { + Self { pool } + } + + fn row_to_account(row: &sqlx::sqlite::SqliteRow) -> ProxmoxAccount { + let fingerprint: String = row.get("fingerprint"); + ProxmoxAccount { + id: row.get("id"), + name: row.get("name"), + host: row.get("host"), + port: u16::try_from(row.get::("port")).unwrap_or(8006), + token_id: row.get("token_id"), + fingerprint: (!fingerprint.is_empty()).then_some(fingerprint), + created_at: row.get("created_at"), + } + } +} + +#[async_trait] +impl ProxmoxAccountPort for ProxmoxAccountRepository { + async fn create(&self, account: &NewProxmoxAccount) -> Result { + let id = Uuid::now_v7().to_string(); + let now = fleet_core::SystemClock::now_unix_millis(); + let port = account.port.unwrap_or(8006); + let result = sqlx::query( + "INSERT INTO proxmox_accounts (id, name, host, port, token_id, fingerprint, created_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, '', ?6)", + ) + .bind(&id) + .bind(&account.name) + .bind(&account.host) + .bind(i64::from(port)) + .bind(&account.token_id) + .bind(now) + .execute(&self.pool) + .await; + match result { + Ok(_) => self.get(&id).await, + Err(error) if is_unique_violation(&error) => Err(format!( + "the account name {:?} is already taken", + account.name + )), + Err(error) => Err(format!("create failed: {error}")), + } + } + + async fn get(&self, id: &str) -> Result { + sqlx::query("SELECT id, name, host, port, token_id, fingerprint, created_at FROM proxmox_accounts WHERE id = ?1") + .bind(id) + .fetch_optional(&self.pool) + .await + .map_err(|error| format!("get failed: {error}"))? + .map(|row| Self::row_to_account(&row)) + .ok_or_else(|| format!("account {id} not found")) + } + + async fn list(&self) -> Result, String> { + let rows = sqlx::query("SELECT id, name, host, port, token_id, fingerprint, created_at FROM proxmox_accounts ORDER BY created_at DESC") + .fetch_all(&self.pool) + .await + .map_err(|error| format!("list failed: {error}"))?; + Ok(rows.iter().map(Self::row_to_account).collect()) + } + + async fn set_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result { + let value = fingerprint.unwrap_or_default(); + sqlx::query("UPDATE proxmox_accounts SET fingerprint = ?2 WHERE id = ?1") + .bind(id) + .bind(&value) + .execute(&self.pool) + .await + .map_err(|error| format!("set_fingerprint failed: {error}"))?; + self.get(id).await + } + + async fn delete(&self, id: &str) -> Result<(), String> { + let result = sqlx::query("DELETE FROM proxmox_accounts WHERE id = ?1") + .bind(id) + .execute(&self.pool) + .await + .map_err(|error| format!("delete failed: {error}"))?; + if result.rows_affected() == 0 { + return Err(format!("account {id} not found")); + } + Ok(()) + } +} + +fn is_unique_violation(error: &sqlx::Error) -> bool { + matches!( + error + .as_database_error() + .map(sqlx::error::DatabaseError::kind), + Some(sqlx::error::ErrorKind::UniqueViolation) + ) +} diff --git a/crates/fleetctl/src/lib.rs b/crates/fleetctl/src/lib.rs index 036fae3..0d5fc16 100644 --- a/crates/fleetctl/src/lib.rs +++ b/crates/fleetctl/src/lib.rs @@ -490,6 +490,43 @@ pub enum Command { /// The SSH port; 22 when omitted. port: Option, }, + /// List the configured Proxmox accounts. + ProxmoxAccounts, + /// Create a Proxmox account. The token secret is read from standard + /// input and never placed in a process argument. + ProxmoxCreate { + /// The operator-facing account name. + name: String, + /// The PVE host (IP or DNS name). + host: String, + /// The API port; 8006 when omitted. + port: Option, + /// The API token id (`user@realm!tokenname`). + token_id: String, + }, + /// Remove a Proxmox account and its secret. + ProxmoxDelete { + /// The account's identity. + account_id: String, + }, + /// Observe a host's certificate fingerprint without sending any + /// credential. + ProxmoxObserve { + /// The account's identity. + account_id: String, + }, + /// Confirm the observed fingerprint as the account's trust anchor. + ProxmoxConfirm { + /// The account's identity. + account_id: String, + /// The fingerprint as observed (colons optional). + fingerprint: String, + }, + /// Discover the cluster through one trusted account. + ProxmoxDiscover { + /// The account's identity. + account_id: String, + }, /// Start the audited "Install Fleet Node" bootstrap on an agentless /// machine: download the checksummed service package on the node, /// install the systemd service, enroll, and wait for the gateway @@ -628,6 +665,7 @@ pub fn parse(args: &[String]) -> Result { parse_install_node(machine_id, rest, &url)? } ["tailnet", verb, rest @ ..] => parse_tailnet_command(verb, rest)?, + ["proxmox", verb, rest @ ..] => parse_proxmox_command(verb, rest)?, ["projects", verb, rest @ ..] => parse_projects_command(verb, rest)?, _ => return Err(CliError { message: usage() }), }; @@ -1034,6 +1072,112 @@ fn parse_tailnet_command(verb: &str, rest: &[&str]) -> Result } } +/// Parses one `fleetctl proxmox` subcommand. +#[allow(clippy::too_many_lines)] +fn parse_proxmox_command(verb: &str, rest: &[&str]) -> Result { + match verb { + "accounts" => match rest { + [] => Ok(Command::ProxmoxAccounts), + _ => Err(CliError { message: usage() }), + }, + "create" => { + let mut name = None; + let mut host = None; + let mut port = None; + let mut token_id = None; + let mut flags = rest.iter().copied(); + while let Some(flag) = flags.next() { + match flag { + "--name" => { + name = Some( + flags + .next() + .ok_or_else(|| CliError { + message: "--name requires a value".to_owned(), + })? + .to_owned(), + ); + } + "--host" => { + host = Some( + flags + .next() + .ok_or_else(|| CliError { + message: "--host requires a value".to_owned(), + })? + .to_owned(), + ); + } + "--port" => { + let value = flags.next().ok_or_else(|| CliError { + message: "--port requires a value".to_owned(), + })?; + port = Some(value.parse().map_err(|_| CliError { + message: format!("--port must be a number, not {value:?}"), + })?); + } + "--token-id" => { + token_id = Some( + flags + .next() + .ok_or_else(|| CliError { + message: "--token-id requires a value".to_owned(), + })? + .to_owned(), + ); + } + other => { + return Err(CliError { + message: format!( + "unknown flag {other:?}; see the usage below\n\n{}", + usage() + ), + }); + } + } + } + Ok(Command::ProxmoxCreate { + name: name.ok_or_else(|| CliError { + message: "--name is required".to_owned(), + })?, + host: host.ok_or_else(|| CliError { + message: "--host is required".to_owned(), + })?, + port, + token_id: token_id.ok_or_else(|| CliError { + message: "--token-id is required".to_owned(), + })?, + }) + } + "delete" => match rest { + [account_id] => Ok(Command::ProxmoxDelete { + account_id: (*account_id).to_owned(), + }), + _ => Err(CliError { message: usage() }), + }, + "observe" => match rest { + [account_id] => Ok(Command::ProxmoxObserve { + account_id: (*account_id).to_owned(), + }), + _ => Err(CliError { message: usage() }), + }, + "confirm" => match rest { + [account_id, "--fingerprint", fingerprint] => Ok(Command::ProxmoxConfirm { + account_id: (*account_id).to_owned(), + fingerprint: (*fingerprint).to_owned(), + }), + _ => Err(CliError { message: usage() }), + }, + "discover" => match rest { + [account_id] => Ok(Command::ProxmoxDiscover { + account_id: (*account_id).to_owned(), + }), + _ => Err(CliError { message: usage() }), + }, + _ => Err(CliError { message: usage() }), + } +} + fn usage() -> String { format!( "Usage: fleetctl [--url ] [--socket ] [--output json|text] \n\nCommands:\n status\n system\n operations list [--limit ]\n operations get \n operations cancel \n machines list [--tag ] [--group ] [--capability ] [--status ] [--limit ]\n machines get \n machines onboard create --user --host [--port ] [--name ] [--description ] [--tag ]... [--group ]... --auth agent|identity-file [--identity ]\n machines onboard list [--limit ]\n machines onboard get \n machines onboard test [--wait] [--timeout ]\n machines onboard discover [--wait] [--timeout ]\n machines onboard confirm --fingerprint \n machines onboard add \n machines onboard cancel \n projects list [--remote-prefix

] [--name-substring ] [--limit ]\n projects get \n projects create --remote --name [--description ]\n projects update --name [--description ]\n projects delete \n projects discover --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects record (the discovery result is read from stdin)\n projects ready --root [--dry-run] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects clone --root [--branch ] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects pull --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects status --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects write-config --root --file --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (contents from stdin)\n skills probe --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--artifact-url --artifact-sha256 ] [--wait] [--timeout ]\n skills deploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n skills undeploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n frogenv status|setup|login|request|sync --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n frogenv run --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n mise inventory|status --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise install --tool --version --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise exec --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n apply --plan-id --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (the plan JSON is read from stdin)\n tailnet status\n tailnet configure --client-id (the client secret is read from stdin)\n tailnet clear\n tailnet devices [--limit ]\n tailnet import --user [--port ]\n machines install-node --endpoint --auth agent|identity-file [--identity ] [--artifact-url --artifact-sha256 ] [--controller-url ] [--install-timeout ] [--connect-timeout ] [--wait] [--timeout ]\n\n`status` prefers the node's local socket (default {DEFAULT_SOCKET}); `--url` is the explicit direct-controller override. Other commands talk to the controller, which defaults to {DEFAULT_URL}." @@ -1505,6 +1649,12 @@ fn render(invocation: &Invocation, payload: &Value) -> String { | Command::TailnetConfigure { .. } | Command::TailnetClear | Command::TailnetDevices { .. } => render_tailnet(Some(payload)), + Command::ProxmoxAccounts + | Command::ProxmoxCreate { .. } + | Command::ProxmoxDelete { .. } + | Command::ProxmoxObserve { .. } + | Command::ProxmoxConfirm { .. } + | Command::ProxmoxDiscover { .. } => render_proxmox(Some(payload)), _ => render_text(Some(payload)), }, } @@ -1953,6 +2103,61 @@ fn request_for(command: &Command) -> Result { .unwrap_or_default(), None, ), + Command::ProxmoxAccounts => ( + reqwest::Method::GET, + "/api/v1/proxmox/accounts".to_owned(), + Vec::new(), + None, + ), + Command::ProxmoxCreate { + name, + host, + port, + token_id, + } => { + let mut body = serde_json::json!({ + "name": name, + "host": host, + "tokenId": token_id, + "tokenSecret": read_stdin_line("the API token secret")?, + }); + if let Some(port) = port { + body["port"] = serde_json::json!(port); + } + ( + reqwest::Method::POST, + "/api/v1/proxmox/accounts".to_owned(), + Vec::new(), + Some(body), + ) + } + Command::ProxmoxDelete { account_id } => ( + reqwest::Method::DELETE, + format!("/api/v1/proxmox/accounts/{account_id}"), + Vec::new(), + None, + ), + Command::ProxmoxObserve { account_id } => ( + reqwest::Method::POST, + format!("/api/v1/proxmox/accounts/{account_id}/observe"), + Vec::new(), + None, + ), + Command::ProxmoxConfirm { + account_id, + fingerprint, + } => ( + reqwest::Method::POST, + format!("/api/v1/proxmox/accounts/{account_id}/confirm"), + Vec::new(), + Some(serde_json::json!({ "fingerprint": fingerprint })), + ), + Command::ProxmoxDiscover { account_id } => ( + reqwest::Method::GET, + format!("/api/v1/proxmox/accounts/{account_id}/discovery"), + Vec::new(), + None, + ), Command::TailnetImport { node_id, user, @@ -2535,6 +2740,89 @@ fn render_project_deleted() -> String { "project removed".to_owned() } +/// Renders the Proxmox surface as human text; exposed for contract tests. +#[doc(hidden)] +#[must_use] +pub fn render_proxmox_for_test(value: &Value) -> String { + render_proxmox(Some(value)) +} + +fn render_proxmox(value: Option<&Value>) -> String { + let Some(value) = value else { + return String::new(); + }; + // Discovery: a snapshot with resources and honest warnings. + if let Some(resources) = value.get("resources").and_then(Value::as_array) { + let mut lines = vec![format!( + "{:<18} {:<10} {:<28} {:<20} {}", + "KIND", "VMID", "ID", "NAME", "STATUS" + )]; + for resource in resources { + lines.push(format!( + "{:<18} {:<10} {:<28} {:<20} {}", + resource["kind"].as_str().unwrap_or("-"), + resource["vmid"] + .as_u64() + .map_or_else(|| "-".to_owned(), |v| v.to_string()), + resource["id"].as_str().unwrap_or("-"), + resource["name"].as_str().unwrap_or("-"), + resource["status"].as_str().unwrap_or("-"), + )); + } + if resources.is_empty() { + lines.push("(no resources reported)".to_owned()); + } + if let Some(warnings) = value.get("warnings").and_then(Value::as_array) + && !warnings.is_empty() + { + lines.push(String::new()); + lines.push("warnings:".to_owned()); + for warning in warnings { + if let Some(text) = warning.as_str() { + lines.push(format!(" {text}")); + } + } + } + if let Some(version) = value.get("pveVersion").and_then(Value::as_str) { + lines.insert(0, format!("PVE {version}")); + } + return lines.join("\n"); + } + // Accounts list: one row per account with its trust state. + if let Some(items) = value.get("items").and_then(Value::as_array) { + let mut lines = vec![format!( + "{:<38} {:<24} {:<32} {}", + "ID", "NAME", "TOKEN ID", "TRUST" + )]; + for account in items { + lines.push(format!( + "{:<38} {:<24} {:<32} {}", + account["id"].as_str().unwrap_or("-"), + account["name"].as_str().unwrap_or("-"), + account["tokenId"].as_str().unwrap_or("-"), + account["fingerprintState"].as_str().unwrap_or("-"), + )); + } + if items.is_empty() { + lines.push("(no Proxmox accounts)".to_owned()); + } + return lines.join("\n"); + } + // Single account / fingerprint / deletion answers: key facts only. + let mut lines = Vec::new(); + for (key, val) in value.as_object().into_iter().flatten() { + let rendered = match val { + Value::String(text) => text.clone(), + other => other.to_string(), + }; + lines.push(format!("{key}: {rendered}")); + } + if lines.is_empty() { + lines.push("account removed".to_owned()); + } + lines.join("\n") +} + /// Renders the tailnet surface as human text; exposed for contract tests. #[doc(hidden)] #[must_use] diff --git a/crates/fleetctl/tests/cli.rs b/crates/fleetctl/tests/cli.rs index c3ac863..1807821 100644 --- a/crates/fleetctl/tests/cli.rs +++ b/crates/fleetctl/tests/cli.rs @@ -262,6 +262,7 @@ fn fleetctl_talks_to_a_real_controller() { None, None, None, + None, ); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); @@ -357,6 +358,7 @@ fn fleetctl_machines_read_a_real_controller() { None, None, None, + None, ); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); @@ -520,6 +522,7 @@ async fn an_explicit_url_sends_status_straight_to_the_controller() { None, None, None, + None, ); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); @@ -809,6 +812,7 @@ fn fleetctl_onboards_a_real_controller() { Some(&onboarding), None, None, + None, ); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); @@ -1866,3 +1870,124 @@ fn parsing_refuses_the_undocumented_apply_forms() { ); } } + +#[test] +fn text_output_renders_proxmox_accounts_and_discoveries() { + let page = json!({ + "items": [ + {"id": "acc-1", "name": "pve-main", "host": "192.168.68.223", "port": 8006, + "tokenId": "root@pam!GLM-AGENT", "fingerprintState": "confirmed", + "fingerprint": "DC2C…6498", "createdAt": 1} + ], + "page": {"limit": 50, "nextCursor": null} + }); + let text = fleetctl::render_proxmox_for_test(&page); + assert!(text.contains("ID"), "{text}"); + assert!(text.contains("pve-main"), "{text}"); + assert!(text.contains("confirmed"), "{text}"); + + let empty = json!({"items": [], "page": {"limit": 50, "nextCursor": null}}); + let text = fleetctl::render_proxmox_for_test(&empty); + assert!(text.contains("no Proxmox accounts"), "{text}"); + + let discovery = json!({ + "accountId": "acc-1", + "pveVersion": "9.2.2", + "resources": [ + {"kind": "node", "id": "node/pve", "vmid": null, "name": "pve", "status": "online"}, + {"kind": "qemu", "id": "qemu/100", "vmid": 100, "name": "dev-01", "status": "running"} + ], + "warnings": ["resource #2: the entry carries no id"], + "reportedCount": 3, + "observedAt": 1 + }); + let text = fleetctl::render_proxmox_for_test(&discovery); + assert!(text.contains("PVE 9.2.2"), "{text}"); + assert!(text.contains("node/pve"), "{text}"); + assert!(text.contains("qemu/100"), "{text}"); + assert!(text.contains("resource #2"), "{text}"); +} + +#[test] +fn parsing_walks_the_proxmox_forms() { + let args: Vec = ["proxmox", "accounts"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxAccounts + )); + + let args: Vec = [ + "proxmox", + "create", + "--name", + "pve-main", + "--host", + "192.168.68.223", + "--port", + "8006", + "--token-id", + "root@pam!GLM-AGENT", + ] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxCreate { .. } + )); + + let args: Vec = ["proxmox", "delete", "acc-1"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxDelete { .. } + )); + + let args: Vec = ["proxmox", "observe", "acc-1"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxObserve { .. } + )); + + let args: Vec = ["proxmox", "confirm", "acc-1", "--fingerprint", "DC2C"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxConfirm { .. } + )); + + let args: Vec = ["proxmox", "discover", "acc-1"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxDiscover { .. } + )); +} + +#[test] +fn parsing_refuses_the_undocumented_proxmox_forms() { + for args in [ + vec!["proxmox", "create", "--name", "pve-main"], + vec!["proxmox", "confirm", "acc-1"], + vec!["proxmox", "discover"], + ] { + let args: Vec = args.iter().map(ToString::to_string).collect(); + let error = fleetctl::parse(&args).unwrap_err(); + assert!( + error.message.contains("Usage") || error.message.contains("is required"), + "{error}" + ); + } +} diff --git a/crates/fleetd/tests/node_install.rs b/crates/fleetd/tests/node_install.rs index 8d04998..960c887 100644 --- a/crates/fleetd/tests/node_install.rs +++ b/crates/fleetd/tests/node_install.rs @@ -180,6 +180,7 @@ async fn harness() -> Harness { None, None, None, + None, ); let listener = TokioListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); diff --git a/crates/providers/fleet-provider-proxmox/Cargo.toml b/crates/providers/fleet-provider-proxmox/Cargo.toml index 55dff09..8b89fcf 100644 --- a/crates/providers/fleet-provider-proxmox/Cargo.toml +++ b/crates/providers/fleet-provider-proxmox/Cargo.toml @@ -7,8 +7,18 @@ repository.workspace = true publish.workspace = true [dependencies] +async-trait = "0.1.92" fleet-application = { path = "../../fleet-application" } fleet-core = { path = "../../fleet-core" } +reqwest = { version = "0.12.24", default-features = false, features = ["rustls-tls", "json"] } +rustls = { version = "0.23", default-features = false, features = ["ring", "std", "tls12", "logging"] } +serde = { version = "1", features = ["derive"] } +serde_json = "1" +sha2 = "0.10" +tokio = { version = "1", features = ["rt", "macros"] } + +[dev-dependencies] +tokio = { version = "1", features = ["rt", "macros"] } [lints] workspace = true diff --git a/crates/providers/fleet-provider-proxmox/src/lib.rs b/crates/providers/fleet-provider-proxmox/src/lib.rs index a191689..8f7af66 100644 --- a/crates/providers/fleet-provider-proxmox/src/lib.rs +++ b/crates/providers/fleet-provider-proxmox/src/lib.rs @@ -1,6 +1,731 @@ -//! proxmox provider adapter boundary. - +//! The Proxmox provider: discovery over the documented PVE REST API, with +//! TLS fingerprint pinning as the only trust model (FM-600; FM-S08). +//! +//! The FM-S08 spike chose the fallback: a small `reqwest` transport with a +//! custom rustls verifier that pins the host certificate's SHA-256 +//! fingerprint, and typed provider DTOs translated at this boundary. PVE +//! hosts present their own cluster CA, so system trust fails; the pinned +//! fingerprint is the trust. Verification is never disabled: an unpinned +//! host is probed with an observe-only verifier that refuses the handshake +//! *after* capturing the leaf fingerprint — no credentials are sent, no +//! session is established, and the reported fingerprint is the honest input +//! to the confirm step (the FM-201 SSH trust flow, over TLS). +//! +//! This crate never logs, stores, or serializes the API token: it arrives +//! per call inside redacting types and is dropped with the request. Errors +//! are caller-safe — statuses, bounded details, and fingerprints, never +//! tokens. +//! +//! Decoding is tolerant on purpose: PVE's Perl API answers loose and null +//! shapes (`data: null`, missing fields, stringly numbers at the edges). +//! Every normalized field is optional and bounded; a payload beyond the +//! bounds is a payload error, not silent truncation. #![warn(missing_docs)] -/// Skeleton marker proving that the provider crate is loadable. -pub const SKELETON: &str = "fleet-provider-proxmox"; +use std::fmt; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use fleet_core::SensitiveString; +use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; +use rustls::crypto::CryptoProvider; +use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; +use rustls::{DigitallySignedStruct, SignatureScheme}; +use sha2::Digest; + +/// The default PVE API port. +pub const DEFAULT_PORT: u16 = 8006; +/// The request timeout applied to every call. +pub const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); +/// The maximum response body the transport accepts. Discovery payloads are +/// bounded lists; anything larger is refused rather than materialized. +pub const MAX_BODY_BYTES: usize = 8 * 1024 * 1024; + +/// The API token: the token id (`user@realm!tokenname`) and the secret +/// value. The secret is zeroizing and redacted by construction; the pair is +/// cloned rarely (into one request) and never serialized, logged, or +/// audited. +#[derive(Debug)] +pub struct PveCredentials { + /// The token id, as PVE's `Authorization: PVEAPIToken==` + /// header names it. Not secret on its own. + pub token_id: String, + /// The token secret value, redacted. + pub token: SensitiveString, +} + +/// A transport request: one API call against one PVE host. +#[derive(Clone, Debug)] +pub struct PveHttpRequest { + /// The host (IP or DNS name) without scheme or port. + pub host: String, + /// The port; [`DEFAULT_PORT`] in the common case. + pub port: u16, + /// The URL path under `/api2/json`, starting with `/`. + pub path: String, + /// The pinned fingerprint, when the caller confirmed one. `None` means + /// observe-only: the handshake is refused after capture. + pub pinned_fingerprint: Option, + /// The credentials for the call. + pub credentials: Arc, +} + +/// A transport response: status and bounded body. +#[derive(Clone, Debug)] +pub struct PveHttpResponse { + /// The HTTP status. + pub status: u16, + /// The response body, bounded by [`MAX_BODY_BYTES`]. + pub body: Vec, +} + +/// A transport failure that is safe to print. TLS fingerprint facts travel +/// here; tokens never do. +#[derive(Debug)] +pub enum PveTransportError { + /// The handshake was refused for a fingerprint mismatch, with the + /// observed leaf fingerprint (hex, colon-separated, like PVE's own + /// display) and the pinned value. + FingerprintMismatch { + /// The observed leaf certificate's SHA-256 fingerprint. + observed: String, + /// The fingerprint the caller pinned, when any. + pinned: Option, + }, + /// The observe-only probe: the handshake was deliberately refused after + /// capturing the fingerprint, so no credentials could be sent. The + /// observed fingerprint is the report. + ObserveRefused { + /// The observed leaf certificate's SHA-256 fingerprint. + observed: String, + }, + /// The host presented no certificate to pin. This is a TLS-level + /// anomaly; refuse it. + NoCertificate, + /// Transport-level failure (DNS, TCP, timeout), with a bounded detail. + Connect { + /// The bounded, redacted detail. + detail: String, + }, + /// The response exceeded the body bound. + BodyTooLarge { + /// The limit that was exceeded. + limit: usize, + }, +} + +impl fmt::Display for PveTransportError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FingerprintMismatch { observed, pinned } => match pinned { + Some(pinned) => write!( + f, + "the host certificate's fingerprint {observed} does not match the pinned {pinned}" + ), + None => write!( + f, + "the host certificate's fingerprint {observed} is not pinned" + ), + }, + Self::ObserveRefused { observed } => write!( + f, + "the trust step refused the handshake after observing the fingerprint {observed}" + ), + Self::NoCertificate => write!(f, "the host presented no certificate"), + Self::Connect { detail } => write!(f, "the connection failed: {detail}"), + Self::BodyTooLarge { limit } => { + write!(f, "the response exceeds the {limit}-byte bound") + } + } + } +} + +impl std::error::Error for PveTransportError {} + +/// The transport port. The real implementation speaks TLS with the pinned +/// rustls verifier; tests record fixtures. +#[async_trait] +pub trait PveTransport: fmt::Debug + Send + Sync { + /// Executes one request. + /// + /// # Errors + /// + /// Fails with [`PveTransportError`]; HTTP statuses travel inside the + /// response. + async fn execute(&self, request: PveHttpRequest) -> Result; +} + +/// The reqwest-backed transport: rustls with the pinned-fingerprint +/// verifier, bounded timeouts, and no redirect surprises. +#[derive(Debug)] +pub struct ReqwestPveTransport; + +impl ReqwestPveTransport { + /// Builds the transport. + /// + /// # Errors + /// + /// Fails when the HTTP client cannot be built. + pub fn new() -> Result { + // A throwaway client proves the TLS feature set resolves in this + // exact dependency graph; each call builds its own client over a + // fresh pinned verifier (the policy is per-call, not per-transport). + reqwest::Client::builder() + .timeout(REQUEST_TIMEOUT) + .build() + .map_err(|error| format!("cannot build the HTTP client: {error}"))?; + Ok(Self) + } +} + +impl Default for ReqwestPveTransport { + fn default() -> Self { + Self::new().expect("the PVE transport must build") + } +} + +/// What the verifier should do with the leaf certificate it sees. +enum TlsPolicy { + /// Capture the fingerprint, then refuse: the trust probe. No HTTP + /// request is ever completed, so credentials are never sent. + Observe, + /// Accept only the leaf whose SHA-256 matches the pinned value. + Pin(String), +} + +/// The verifier shared with the rustls session. It never disables +/// verification: every path either matches the pin or refuses. The leaf +/// fingerprint it observed lands in `captured`, which is how the transport +/// reports trust facts on refusal — reqwest's own error chain does not +/// carry them. +struct PinningVerifier { + policy: TlsPolicy, + provider: Arc, + captured: Arc>>, +} + +impl PinningVerifier { + /// Formats a digest the way PVE and the legacy client display it: + /// uppercase colon-separated hex. + fn fingerprint(digest: &[u8; 32]) -> String { + digest + .iter() + .map(|byte| format!("{byte:02X}")) + .collect::>() + .join(":") + } +} + +impl fmt::Debug for PinningVerifier { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.policy { + TlsPolicy::Observe => f.write_str("PinningVerifier(observe)"), + TlsPolicy::Pin(_) => f.write_str("PinningVerifier(pinned)"), + } + } +} + +impl ServerCertVerifier for PinningVerifier { + fn verify_server_cert( + &self, + end_entity: &CertificateDer<'_>, + _intermediates: &[CertificateDer<'_>], + _server_name: &ServerName<'_>, + _ocsp: &[u8], + _now: UnixTime, + ) -> Result { + let mut hasher = sha2::Sha256::new(); + hasher.update(end_entity.as_ref()); + let digest: [u8; 32] = hasher.finalize().into(); + let observed = Self::fingerprint(&digest); + *self + .captured + .lock() + .expect("the capture lock is not poisoned") = Some(observed.clone()); + match &self.policy { + TlsPolicy::Observe => Err(rustls::Error::General( + "fleet observe-only trust probe: refusing after capture".to_owned(), + )), + TlsPolicy::Pin(pinned) + if normalize_fingerprint(pinned) == normalize_fingerprint(&observed) => + { + Ok(ServerCertVerified::assertion()) + } + TlsPolicy::Pin(_) => Err(rustls::Error::General( + "fleet trust: the certificate does not match the pinned fingerprint".to_owned(), + )), + } + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls12_signature( + message, + cert, + dss, + &self.provider.signature_verification_algorithms, + ) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls13_signature( + message, + cert, + dss, + &self.provider.signature_verification_algorithms, + ) + } + + fn supported_verify_schemes(&self) -> Vec { + self.provider + .signature_verification_algorithms + .supported_schemes() + } +} + +/// Normalizes a fingerprint for comparison: strips colons and uppercases. +/// Both the stored pin and PVE's displayed form reach this. +#[must_use] +pub fn normalize_fingerprint(value: &str) -> String { + value.replace(':', "").to_uppercase() +} + +#[async_trait] +impl PveTransport for ReqwestPveTransport { + async fn execute(&self, request: PveHttpRequest) -> Result { + let policy = match &request.pinned_fingerprint { + Some(pinned) => TlsPolicy::Pin(normalize_fingerprint(pinned)), + None => TlsPolicy::Observe, + }; + let provider = Arc::new(rustls::crypto::ring::default_provider()); + let captured = Arc::new(Mutex::new(None)); + let verifier = Arc::new(PinningVerifier { + policy, + provider: provider.clone(), + captured: Arc::clone(&captured), + }); + let mut config = rustls::ClientConfig::builder_with_provider(provider) + .with_protocol_versions(&[&rustls::version::TLS13]) + .map_err(|error| PveTransportError::Connect { + detail: error.to_string(), + })? + .dangerous() + .with_custom_certificate_verifier(verifier) + .with_no_client_auth(); + config.alpn_protocols = Vec::new(); + + let client = reqwest::Client::builder() + .timeout(REQUEST_TIMEOUT) + .redirect(reqwest::redirect::Policy::none()) + .use_preconfigured_tls(config) + .build() + .map_err(|error| PveTransportError::Connect { + detail: format!("the TLS configuration was rejected: {error}"), + })?; + let url = format!("https://{}:{}{}", request.host, request.port, request.path); + let response = client + .get(&url) + .header( + "Authorization", + format!( + "PVEAPIToken={}={}", + request.credentials.token_id, + request.credentials.token.expose() + ), + ) + .send() + .await + .map_err(|error| { + // The verifier's refusal surfaces as an opaque connect + // error; the trust facts live in the capture the verifier + // wrote before refusing. + let observed = captured + .lock() + .expect("the capture lock is not poisoned") + .clone(); + match (observed, request.pinned_fingerprint.as_deref()) { + (Some(observed), Some(_)) => PveTransportError::FingerprintMismatch { + observed, + pinned: request.pinned_fingerprint.clone(), + }, + (Some(observed), None) => PveTransportError::ObserveRefused { observed }, + (None, _) => PveTransportError::Connect { + detail: error.to_string(), + }, + } + })?; + let status = u16::from(response.status()); + let body = response + .bytes() + .await + .map_err(|error| PveTransportError::Connect { + detail: error.to_string(), + })?; + if body.len() > MAX_BODY_BYTES { + return Err(PveTransportError::BodyTooLarge { + limit: MAX_BODY_BYTES, + }); + } + Ok(PveHttpResponse { + status, + body: body.to_vec(), + }) + } +} + +/// A payload failure: the API answered, but not with something Fleet can +/// interpret. +#[derive(Debug)] +pub enum PveApiError { + /// The credentials were refused (401). + Auth, + /// The caller lacks the privilege (403), with the bounded detail. + Forbidden { + /// The bounded, redacted detail. + detail: String, + }, + /// Any other HTTP outcome. + Http { + /// The status. + status: u16, + /// The bounded detail. + detail: String, + }, + /// The body was not interpretable. + InvalidPayload { + /// The bounded detail. + detail: String, + }, + /// The transport failed. + Transport(PveTransportError), +} + +impl fmt::Display for PveApiError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Auth => write!(f, "the API token was refused (401)"), + Self::Forbidden { detail } => { + write!(f, "the token lacks the privilege (403): {detail}") + } + Self::Http { status, detail } => write!(f, "the API answered {status}: {detail}"), + Self::InvalidPayload { detail } => { + write!(f, "the API's payload is not interpretable: {detail}") + } + Self::Transport(error) => write!(f, "{error}"), + } + } +} + +impl std::error::Error for PveApiError {} + +/// One normalized cluster resource: a node, a QEMU guest, an LXC container, +/// a storage, or a template. Provenance and time attach at the application +/// layer; this is the provider's own shape. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveResource { + /// The normalized kind: `node`, `qemu`, `lxc`, `storage`, or + /// `qemu-template`. + pub kind: String, + /// The cluster-visible id, e.g. `node/pve`, `qemu/101`. + pub id: String, + /// The hosting node, when the resource has one. + pub node: Option, + /// The VMID, when the resource has one. + pub vmid: Option, + /// The display name, when carried. + pub name: Option, + /// The PVE status string (`running`, `stopped`, `online`, …), when + /// carried. + pub status: Option, +} + +/// The discovery result: the API version seen and the normalized resources. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveDiscovery { + /// The PVE version string, e.g. `9.2.2`. + pub version: String, + /// The normalized resources, per-resource failures isolated away. + pub resources: Vec, + /// The resources that failed normalization, as bounded per-resource + /// warnings. A partial failure never drops the whole snapshot. + pub warnings: Vec, + /// The resource count before isolation, for honesty about loss. + pub reported_count: usize, +} + +/// The discovery port. The provider implements this over the PVE API; +/// tests implement it over recorded fixtures. +#[async_trait] +pub trait ProxmoxSource: fmt::Debug + Send + Sync { + /// Discovers the cluster's resources through one account. + /// + /// # Errors + /// + /// Fails with [`PveApiError`] on auth, privilege, HTTP, payload, or + /// transport failures. Per-resource normalization failures are isolated + /// into the result's warnings instead. + async fn discover(&self, request: PveHttpRequest) -> Result; +} + +/// The provider client: transport plus normalization. Stateless — every +/// call carries its own endpoint and credentials. +pub struct ProxmoxClient { + transport: Arc, +} + +impl fmt::Debug for ProxmoxClient { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProxmoxClient") + .field("transport", &self.transport) + .finish() + } +} + +impl ProxmoxClient { + /// Composes the client over a transport. + #[must_use] + pub fn new(transport: Arc) -> Self { + Self { transport } + } + + async fn call(&self, request: PveHttpRequest) -> Result { + let response = self + .transport + .execute(request) + .await + .map_err(PveApiError::Transport)?; + match response.status { + 401 => Err(PveApiError::Auth), + 403 => Err(PveApiError::Forbidden { + detail: bounded_body(&response.body), + }), + status if (400..600).contains(&status) => Err(PveApiError::Http { + status, + detail: bounded_body(&response.body), + }), + _ => { + let value: serde_json::Value = + serde_json::from_slice(&response.body).map_err(|error| { + PveApiError::InvalidPayload { + detail: format!("the body is not JSON: {error}"), + } + })?; + // PVE wraps every response in `{"data": ...}`; `data: null` + // and a missing `data` both mean "empty". + Ok(value + .get("data") + .cloned() + .unwrap_or(serde_json::Value::Null)) + } + } + } +} + +/// A bounded, credential-free body excerpt for error details. +fn bounded_body(body: &[u8]) -> String { + let text = String::from_utf8_lossy(body); + fleet_core::redact_url_credentials(&fleet_core::flatten_control_characters( + &text.chars().take(256).collect::(), + )) +} + +#[async_trait] +impl ProxmoxSource for ProxmoxClient { + async fn discover(&self, request: PveHttpRequest) -> Result { + // The version first: it anchors provenance and proves the trust. + let version_request = PveHttpRequest { + path: "/api2/json/version".to_owned(), + ..request.clone() + }; + let version_data = self.call(version_request).await?; + let version = version_data + .get("version") + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .chars() + .take(32) + .collect::(); + if version.is_empty() { + return Err(PveApiError::InvalidPayload { + detail: "the version payload carries no version string".to_owned(), + }); + } + + let resources_request = PveHttpRequest { + path: "/api2/json/cluster/resources".to_owned(), + ..request.clone() + }; + let data = self.call(resources_request).await?; + let entries = match data { + serde_json::Value::Array(entries) => entries, + // `data: null` is an empty cluster: honest, not an error. + serde_json::Value::Null => Vec::new(), + other => { + return Err(PveApiError::InvalidPayload { + detail: format!( + "the resources payload is not a list (it is a {})", + type_name_of(&other) + ), + }); + } + }; + let reported_count = entries.len(); + let mut resources = Vec::new(); + let mut warnings = Vec::new(); + // Per-resource isolation: one malformed entry warns; the rest land. + for (index, entry) in entries.into_iter().enumerate() { + match normalize_resource(&entry) { + Ok(Some(resource)) => resources.push(resource), + Ok(None) => {} + Err(detail) => warnings.push(format!("resource #{index}: {detail}")), + } + } + Ok(PveDiscovery { + version, + resources, + warnings, + reported_count, + }) + } +} + +fn type_name_of(value: &serde_json::Value) -> &'static str { + match value { + serde_json::Value::Null => "null", + serde_json::Value::Bool(_) => "boolean", + serde_json::Value::Number(_) => "number", + serde_json::Value::String(_) => "string", + serde_json::Value::Array(_) => "list", + serde_json::Value::Object(_) => "object", + } +} + +/// Normalizes one cluster-resources entry. `Ok(None)` skips a non-resource +/// row without warning; `Err` warns. +fn normalize_resource(entry: &serde_json::Value) -> Result, String> { + let id = entry + .get("id") + .and_then(serde_json::Value::as_str) + .ok_or_else(|| "the entry carries no id".to_owned())? + .chars() + .take(128) + .collect::(); + let pve_type = entry + .get("type") + .and_then(serde_json::Value::as_str) + .ok_or_else(|| format!("entry {id} carries no type"))?; + let is_template = entry + .get("template") + .and_then(serde_json::Value::as_i64) + .unwrap_or(0) + == 1; + let kind = match (pve_type, is_template) { + ("node", _) => "node", + ("qemu", false) => "qemu", + ("qemu", true) => "qemu-template", + ("lxc", _) => "lxc", + ("storage", _) => "storage", + ("sdn" | "pool", _) => return Ok(None), + (other, _) => { + return Err(format!( + "entry {id} has an unrecognized type {other:?} (reported honestly, not coerced)" + )); + } + }; + let node = entry + .get("node") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(128).collect()); + let vmid = entry.get("vmid").and_then(|value| { + value + .as_u64() + .map(|value| u32::try_from(value).unwrap_or(0)) + }); + let name = entry + .get("name") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(256).collect()); + let status = entry + .get("status") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(64).collect()); + Ok(Some(PveResource { + kind: kind.to_owned(), + id, + node, + vmid, + name, + status, + })) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fingerprints_normalize_for_comparison() { + let pve_form = "DC:2C:11:6E:C9:C7:EA:61:8A:A4:E4:1E:FB:9B:DE:E4:AA:3D:81:EB:16:38:8F:2B:36:0A:AB:E2:83:A7:64:98"; + let bare = pve_form.replace(':', ""); + assert_eq!( + normalize_fingerprint(pve_form), + normalize_fingerprint(&bare) + ); + assert_eq!(normalize_fingerprint("ab:cd"), "ABCD"); + } + + #[test] + fn nodes_and_guests_normalize_and_odd_types_warn() { + let entry = serde_json::json!({ + "id": "qemu/101", "type": "qemu", "node": "pve", "vmid": 101, + "name": "dev-01", "status": "running", "template": 0 + }); + let resource = normalize_resource(&entry).unwrap().unwrap(); + assert_eq!(resource.kind, "qemu"); + assert_eq!(resource.vmid, Some(101)); + + let template = serde_json::json!({ + "id": "qemu/900", "type": "qemu", "template": 1, "status": "stopped" + }); + assert_eq!( + normalize_resource(&template).unwrap().unwrap().kind, + "qemu-template" + ); + + let node = serde_json::json!({"id": "node/pve", "type": "node", "status": "online"}); + let resource = normalize_resource(&node).unwrap().unwrap(); + assert_eq!(resource.kind, "node"); + assert_eq!(resource.vmid, None); + + let sdn = serde_json::json!({"id": "sdn/zone1", "type": "sdn"}); + assert!(normalize_resource(&sdn).unwrap().is_none()); + + let mystery = serde_json::json!({"id": "weird/1", "type": "mystery"}); + let error = normalize_resource(&mystery).unwrap_err(); + assert!(error.contains("unrecognized type"), "{error}"); + } + + #[test] + fn null_and_missing_envelopes_mean_empty() { + let body = serde_json::json!({"data": null}); + assert!(body.get("data").cloned().unwrap_or_default().is_null()); + let body = serde_json::json!({}); + assert!( + body.get("data") + .cloned() + .unwrap_or(serde_json::Value::Null) + .is_null() + ); + } +} diff --git a/crates/providers/fleet-provider-proxmox/tests/pin_live.rs b/crates/providers/fleet-provider-proxmox/tests/pin_live.rs new file mode 100644 index 0000000..4e398f7 --- /dev/null +++ b/crates/providers/fleet-provider-proxmox/tests/pin_live.rs @@ -0,0 +1,104 @@ +//! Live trust verification against the integration PVE host. Skipped +//! without `FLEET_PVE_LIVE=1` and the `PROXMOX_*` environment; the CI gate +//! never depends on it. + +use fleet_core::SensitiveString; +use fleet_provider_proxmox::{ + ProxmoxClient, ProxmoxSource as _, PveCredentials, PveHttpRequest, PveTransport, + PveTransportError, ReqwestPveTransport, +}; +use std::sync::Arc; + +fn live() -> Option<(String, String, String, String, String)> { + if std::env::var("FLEET_PVE_LIVE").ok()?.trim() != "1" { + return None; + } + Some(( + std::env::var("PROXMOX_HOST").ok()?, + std::env::var("PROXMOX_PORT").unwrap_or_else(|_| "8006".to_owned()), + std::env::var("PROXMOX_TOKEN_ID").ok()?, + std::env::var("PROXMOX_API_KEY").ok()?, + std::env::var("PROXMOX_FINGERPRINT").ok()?, + )) +} + +#[tokio::test] +async fn pinned_transport_converses_with_the_live_host() { + let Some((host, port, token_id, key, fingerprint)) = live() else { + return; + }; + let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let request = PveHttpRequest { + host, + port: port.parse().unwrap(), + path: "/api2/json/version".to_owned(), + pinned_fingerprint: Some(fingerprint), + credentials: Arc::new(PveCredentials { + token_id, + token: SensitiveString::new(key), + }), + }; + let client = ProxmoxClient::new(transport); + let discovery = client.discover(request).await.unwrap(); + assert!(!discovery.version.is_empty()); + assert!(!discovery.resources.is_empty()); + assert!( + discovery.resources.iter().any(|r| r.kind == "node"), + "a live cluster lists at least its node" + ); +} + +#[tokio::test] +async fn a_wrong_fingerprint_is_refused_at_the_handshake() { + let Some((host, port, token_id, key, _)) = live() else { + return; + }; + let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let request = PveHttpRequest { + host, + port: port.parse().unwrap(), + path: "/api2/json/version".to_owned(), + // All-zero: never the real certificate. + pinned_fingerprint: Some("00".repeat(32)), + credentials: Arc::new(PveCredentials { + token_id, + token: SensitiveString::new(key), + }), + }; + let error = transport.execute(request).await.unwrap_err(); + match error { + PveTransportError::FingerprintMismatch { observed, pinned } => { + assert_eq!(pinned, Some("00".repeat(32))); + assert_eq!(observed.len(), 32 * 2 + 31, "{observed}"); + } + other => panic!("expected a fingerprint mismatch, got {other:?}"), + } +} + +#[tokio::test] +async fn an_unpinned_host_is_observed_not_conversed_with() { + let Some((host, port, token_id, key, fingerprint)) = live() else { + return; + }; + let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let request = PveHttpRequest { + host, + port: port.parse().unwrap(), + path: "/api2/json/version".to_owned(), + pinned_fingerprint: None, + credentials: Arc::new(PveCredentials { + token_id, + token: SensitiveString::new(key), + }), + }; + let error = transport.execute(request).await.unwrap_err(); + match error { + PveTransportError::ObserveRefused { observed } => { + assert_eq!( + fleet_provider_proxmox::normalize_fingerprint(&observed), + fleet_provider_proxmox::normalize_fingerprint(&fingerprint) + ); + } + other => panic!("expected an observe refusal, got {other:?}"), + } +} diff --git a/packages/api-client/openapi.json b/packages/api-client/openapi.json index 3a4f2ca..2068ef5 100644 --- a/packages/api-client/openapi.json +++ b/packages/api-client/openapi.json @@ -1911,6 +1911,343 @@ } } }, + "/api/v1/proxmox/accounts": { + "get": { + "tags": [ + "proxmox" + ], + "summary": "Lists the configured accounts.", + "description": "# Errors\n\nReturns the public error envelope on refusal or backend failure.", + "operationId": "listProxmoxAccounts", + "responses": { + "200": { + "description": "The configured accounts, newest first.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Page_ProxmoxAccountDto" + } + } + } + }, + "403": { + "description": "The caller may not read the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + }, + "post": { + "tags": [ + "proxmox" + ], + "summary": "Registers an account and stores its token secret. The account starts\n`unconfirmed`: discovery stays locked until the fingerprint is confirmed.", + "description": "# Errors\n\nReturns the public error envelope on refusal, conflict, or backend\nfailure.", + "operationId": "createProxmoxAccount", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CreateProxmoxAccountRequest" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "The account was created; confirm its fingerprint before discovery.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Resource_ProxmoxAccountDto" + } + } + } + }, + "400": { + "description": "The request is malformed.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "403": { + "description": "The caller may not configure the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "409": { + "description": "The account name is taken.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, + "/api/v1/proxmox/accounts/{accountId}": { + "delete": { + "tags": [ + "proxmox" + ], + "summary": "Removes an account and its secret.", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unknown account, or a\nbackend failure.", + "operationId": "deleteProxmoxAccount", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "description": "The account was removed." + }, + "403": { + "description": "The caller may not configure the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "404": { + "description": "The account does not exist.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, + "/api/v1/proxmox/accounts/{accountId}/confirm": { + "post": { + "tags": [ + "proxmox" + ], + "summary": "Pins the confirmed fingerprint as the account's trust anchor.", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unknown account, or a\nmalformed fingerprint.", + "operationId": "confirmProxmoxFingerprint", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ConfirmProxmoxFingerprintRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "The fingerprint is pinned; discovery is unlocked.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Resource_ProxmoxAccountDto" + } + } + } + }, + "400": { + "description": "The fingerprint is malformed.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "403": { + "description": "The caller may not configure the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "404": { + "description": "The account does not exist.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, + "/api/v1/proxmox/accounts/{accountId}/discovery": { + "get": { + "tags": [ + "proxmox" + ], + "summary": "Discovers the cluster through one trusted account.", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unconfirmed account, or\na source failure.", + "operationId": "discoverProxmoxCluster", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The discovery snapshot, availability-honest.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Resource_ProxmoxDiscoveryDto" + } + } + } + }, + "403": { + "description": "The caller may not read the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "404": { + "description": "The account does not exist.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "409": { + "description": "The account's trust is unconfirmed or its fingerprint was refused.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, + "/api/v1/proxmox/accounts/{accountId}/observe": { + "post": { + "tags": [ + "proxmox" + ], + "summary": "Captures the host's certificate fingerprint without sending any\ncredential. The report is the input to the confirm step.", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unknown account, or an\nunreachable host.", + "operationId": "observeProxmoxFingerprint", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The observed fingerprint; confirm it to trust the host.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Resource_ProxmoxFingerprintDto" + } + } + } + }, + "403": { + "description": "The caller may not read the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "404": { + "description": "The account does not exist.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "502": { + "description": "The host is unreachable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, "/api/v1/system": { "get": { "tags": [ @@ -2520,6 +2857,19 @@ } } }, + "ConfirmProxmoxFingerprintRequest": { + "type": "object", + "description": "The confirm-trust request: the fingerprint the caller observed.", + "required": [ + "fingerprint" + ], + "properties": { + "fingerprint": { + "type": "string", + "description": "The SHA-256 fingerprint as observed (colons optional)." + } + } + }, "CorrelatedDeviceDto": { "type": "object", "description": "One tailnet device with its Fleet-machine candidates (evidence only).", @@ -2774,6 +3124,43 @@ } } }, + "CreateProxmoxAccountRequest": { + "type": "object", + "description": "The create-account request. The token secret is write-only.", + "required": [ + "name", + "host", + "tokenId", + "tokenSecret" + ], + "properties": { + "host": { + "type": "string", + "description": "The PVE host (IP or DNS name)." + }, + "name": { + "type": "string", + "description": "The operator-facing name." + }, + "port": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "The API port; 8006 when omitted.", + "minimum": 0 + }, + "tokenId": { + "type": "string", + "description": "The API token id (`user@realm!tokenname`)." + }, + "tokenSecret": { + "type": "string", + "description": "The API token secret (write-only)." + } + } + }, "DiscoveredCheckoutDto": { "type": "object", "description": "One discovered checkout, as the discovery operation reported it.", @@ -4311,7 +4698,73 @@ "updatedAt": { "type": "integer", "format": "int64", - "description": "Last update, in epoch milliseconds." + "description": "Last update, in epoch milliseconds." + } + } + }, + "description": "The items on this page, in the endpoint's documented order." + }, + "page": { + "$ref": "#/components/schemas/PageInfo", + "description": "Where this page sits in the result set." + } + } + }, + "Page_ProjectDto": { + "type": "object", + "description": "A page of resources.\n\nThe concrete schema for a list endpoint appears when that endpoint does;\n[`PageInfo`] is the part of the shape that is fixed for every one of them.", + "required": [ + "items", + "page" + ], + "properties": { + "items": { + "type": "array", + "items": { + "type": "object", + "description": "A project as the detail view displays it.", + "required": [ + "id", + "remote", + "name", + "description", + "checkouts", + "createdAt", + "updatedAt" + ], + "properties": { + "checkouts": { + "type": "array", + "items": { + "$ref": "#/components/schemas/CheckoutFactDto" + }, + "description": "The observed checkouts across machines, newest observation first." + }, + "createdAt": { + "type": "integer", + "format": "int64", + "description": "Creation time (epoch milliseconds)." + }, + "description": { + "type": "string", + "description": "Operator notes." + }, + "id": { + "type": "string", + "description": "The project's identity." + }, + "name": { + "type": "string", + "description": "The display name." + }, + "remote": { + "type": "string", + "description": "The normalized remote." + }, + "updatedAt": { + "type": "integer", + "format": "int64", + "description": "Last mutation (epoch milliseconds)." } } }, @@ -4323,7 +4776,7 @@ } } }, - "Page_ProjectDto": { + "Page_ProxmoxAccountDto": { "type": "object", "description": "A page of resources.\n\nThe concrete schema for a list endpoint appears when that endpoint does;\n[`PageInfo`] is the part of the shape that is fixed for every one of them.", "required": [ @@ -4335,49 +4788,54 @@ "type": "array", "items": { "type": "object", - "description": "A project as the detail view displays it.", + "description": "One configured Proxmox account. The token secret is never here.", "required": [ "id", - "remote", "name", - "description", - "checkouts", - "createdAt", - "updatedAt" + "host", + "port", + "tokenId", + "fingerprintState", + "createdAt" ], "properties": { - "checkouts": { - "type": "array", - "items": { - "$ref": "#/components/schemas/CheckoutFactDto" - }, - "description": "The observed checkouts across machines, newest observation first." - }, "createdAt": { "type": "integer", "format": "int64", - "description": "Creation time (epoch milliseconds)." + "description": "When the account was created." }, - "description": { + "fingerprint": { + "type": [ + "string", + "null" + ], + "description": "The pinned fingerprint, once confirmed." + }, + "fingerprintState": { "type": "string", - "description": "Operator notes." + "description": "The trust state: `unconfirmed` until the fingerprint is pinned." }, - "id": { + "host": { "type": "string", - "description": "The project's identity." + "description": "The PVE host." }, - "name": { + "id": { "type": "string", - "description": "The display name." + "description": "The account's identity." }, - "remote": { + "name": { "type": "string", - "description": "The normalized remote." + "description": "The operator-facing name." }, - "updatedAt": { + "port": { "type": "integer", - "format": "int64", - "description": "Last mutation (epoch milliseconds)." + "format": "int32", + "description": "The API port.", + "minimum": 0 + }, + "tokenId": { + "type": "string", + "description": "The API token id (`user@realm!tokenname`), not secret on its own." } } }, @@ -4437,6 +4895,187 @@ } } }, + "ProxmoxAccountDto": { + "type": "object", + "description": "One configured Proxmox account. The token secret is never here.", + "required": [ + "id", + "name", + "host", + "port", + "tokenId", + "fingerprintState", + "createdAt" + ], + "properties": { + "createdAt": { + "type": "integer", + "format": "int64", + "description": "When the account was created." + }, + "fingerprint": { + "type": [ + "string", + "null" + ], + "description": "The pinned fingerprint, once confirmed." + }, + "fingerprintState": { + "type": "string", + "description": "The trust state: `unconfirmed` until the fingerprint is pinned." + }, + "host": { + "type": "string", + "description": "The PVE host." + }, + "id": { + "type": "string", + "description": "The account's identity." + }, + "name": { + "type": "string", + "description": "The operator-facing name." + }, + "port": { + "type": "integer", + "format": "int32", + "description": "The API port.", + "minimum": 0 + }, + "tokenId": { + "type": "string", + "description": "The API token id (`user@realm!tokenname`), not secret on its own." + } + } + }, + "ProxmoxDiscoveryDto": { + "type": "object", + "description": "The discovery snapshot.", + "required": [ + "accountId", + "pveVersion", + "resources", + "warnings", + "reportedCount", + "observedAt" + ], + "properties": { + "accountId": { + "type": "string", + "description": "The account that produced the snapshot." + }, + "observedAt": { + "type": "integer", + "format": "int64", + "description": "When the snapshot was taken." + }, + "pveVersion": { + "type": "string", + "description": "The PVE version seen." + }, + "reportedCount": { + "type": "integer", + "description": "The count the API reported.", + "minimum": 0 + }, + "resources": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ProxmoxResourceDto" + }, + "description": "The normalized resources." + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The per-resource normalization warnings." + } + } + }, + "ProxmoxFingerprintDto": { + "type": "object", + "description": "The observed fingerprint report.", + "required": [ + "accountId", + "fingerprint" + ], + "properties": { + "accountId": { + "type": "string", + "description": "The account the fingerprint was observed for." + }, + "fingerprint": { + "type": "string", + "description": "The host certificate's SHA-256 fingerprint." + } + } + }, + "ProxmoxResourceDto": { + "type": "object", + "description": "One normalized discovery observation.", + "required": [ + "kind", + "id", + "accountId", + "pveVersion", + "observedAt" + ], + "properties": { + "accountId": { + "type": "string", + "description": "The account that observed the resource." + }, + "id": { + "type": "string", + "description": "The cluster-visible id." + }, + "kind": { + "type": "string", + "description": "The normalized kind: `node`, `qemu`, `lxc`, `qemu-template`, or\n`storage`." + }, + "name": { + "type": [ + "string", + "null" + ], + "description": "The display name, when carried." + }, + "node": { + "type": [ + "string", + "null" + ], + "description": "The hosting node, when the resource has one." + }, + "observedAt": { + "type": "integer", + "format": "int64", + "description": "When the observation was taken." + }, + "pveVersion": { + "type": "string", + "description": "The PVE version the observation came from." + }, + "status": { + "type": [ + "string", + "null" + ], + "description": "The PVE status string, when carried." + }, + "vmid": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "The VMID, when the resource has one.", + "minimum": 0 + } + } + }, "ReadyAuthDto": { "oneOf": [ { @@ -5232,6 +5871,150 @@ } } }, + "Resource_ProxmoxAccountDto": { + "type": "object", + "description": "A single resource.\n\nThe payload is nested under `data` so that later top-level fields are an\nadditive change rather than a breaking one.", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "object", + "description": "One configured Proxmox account. The token secret is never here.", + "required": [ + "id", + "name", + "host", + "port", + "tokenId", + "fingerprintState", + "createdAt" + ], + "properties": { + "createdAt": { + "type": "integer", + "format": "int64", + "description": "When the account was created." + }, + "fingerprint": { + "type": [ + "string", + "null" + ], + "description": "The pinned fingerprint, once confirmed." + }, + "fingerprintState": { + "type": "string", + "description": "The trust state: `unconfirmed` until the fingerprint is pinned." + }, + "host": { + "type": "string", + "description": "The PVE host." + }, + "id": { + "type": "string", + "description": "The account's identity." + }, + "name": { + "type": "string", + "description": "The operator-facing name." + }, + "port": { + "type": "integer", + "format": "int32", + "description": "The API port.", + "minimum": 0 + }, + "tokenId": { + "type": "string", + "description": "The API token id (`user@realm!tokenname`), not secret on its own." + } + } + } + } + }, + "Resource_ProxmoxDiscoveryDto": { + "type": "object", + "description": "A single resource.\n\nThe payload is nested under `data` so that later top-level fields are an\nadditive change rather than a breaking one.", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "object", + "description": "The discovery snapshot.", + "required": [ + "accountId", + "pveVersion", + "resources", + "warnings", + "reportedCount", + "observedAt" + ], + "properties": { + "accountId": { + "type": "string", + "description": "The account that produced the snapshot." + }, + "observedAt": { + "type": "integer", + "format": "int64", + "description": "When the snapshot was taken." + }, + "pveVersion": { + "type": "string", + "description": "The PVE version seen." + }, + "reportedCount": { + "type": "integer", + "description": "The count the API reported.", + "minimum": 0 + }, + "resources": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ProxmoxResourceDto" + }, + "description": "The normalized resources." + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The per-resource normalization warnings." + } + } + } + } + }, + "Resource_ProxmoxFingerprintDto": { + "type": "object", + "description": "A single resource.\n\nThe payload is nested under `data` so that later top-level fields are an\nadditive change rather than a breaking one.", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "object", + "description": "The observed fingerprint report.", + "required": [ + "accountId", + "fingerprint" + ], + "properties": { + "accountId": { + "type": "string", + "description": "The account the fingerprint was observed for." + }, + "fingerprint": { + "type": "string", + "description": "The host certificate's SHA-256 fingerprint." + } + } + } + } + }, "Resource_ReadyPlanDto": { "type": "object", "description": "A single resource.\n\nThe payload is nested under `data` so that later top-level fields are an\nadditive change rather than a breaking one.", @@ -5849,6 +6632,10 @@ "name": "tailnet", "description": "Optional Tailscale discovery: correlated tailnet devices and the import handoff into the onboarding flow. Correlation is evidence only; Fleet identity never derives from Tailscale." }, + { + "name": "proxmox", + "description": "Proxmox accounts, TLS fingerprint trust, and cluster discovery. The token secret is write-only; discovery is locked until the host fingerprint is confirmed." + }, { "name": "nodes", "description": "Node enrollment and identity: enrollment tokens, node state, and revocation. The machine-facing enrollment endpoints under /api/node/v1 are versioned with the node protocol and documented in proto/README.md, not here." diff --git a/packages/api-client/src/generated/fleet.ts b/packages/api-client/src/generated/fleet.ts index 0d18a73..30841f0 100644 --- a/packages/api-client/src/generated/fleet.ts +++ b/packages/api-client/src/generated/fleet.ts @@ -294,6 +294,14 @@ export interface ConfirmHostKeyRequest { fingerprint: string; } +/** + * The confirm-trust request: the fingerprint the caller observed. + */ +export interface ConfirmProxmoxFingerprintRequest { + /** The SHA-256 fingerprint as observed (colons optional). */ + fingerprint: string; +} + /** * One Fleet machine a tailnet device may be. */ @@ -451,6 +459,26 @@ export interface CreateProjectRequest { remote: string; } +/** + * The create-account request. The token secret is write-only. + */ +export interface CreateProxmoxAccountRequest { + /** The PVE host (IP or DNS name). */ + host: string; + /** The operator-facing name. */ + name: string; + /** + * The API port; 8006 when omitted. + * @minimum 0 + * @nullable + */ + port?: number | null; + /** The API token id (`user@realm!tokenname`). */ + tokenId: string; + /** The API token secret (write-only). */ + tokenSecret: string; +} + /** * One discovered checkout, as the discovery operation reported it. */ @@ -1201,6 +1229,47 @@ export interface PageProjectDto { page: PageInfo; } +/** + * One configured Proxmox account. The token secret is never here. + */ +export type PageProxmoxAccountDtoItemsItem = { + /** When the account was created. */ + createdAt: number; + /** + * The pinned fingerprint, once confirmed. + * @nullable + */ + fingerprint?: string | null; + /** The trust state: `unconfirmed` until the fingerprint is pinned. */ + fingerprintState: string; + /** The PVE host. */ + host: string; + /** The account's identity. */ + id: string; + /** The operator-facing name. */ + name: string; + /** + * The API port. + * @minimum 0 + */ + port: number; + /** The API token id (`user@realm!tokenname`), not secret on its own. */ + tokenId: string; +}; + +/** + * A page of resources. + * + * The concrete schema for a list endpoint appears when that endpoint does; + * [`PageInfo`] is the part of the shape that is fixed for every one of them. + */ +export interface PageProxmoxAccountDto { + /** The items on this page, in the endpoint's documented order. */ + items: PageProxmoxAccountDtoItemsItem[]; + /** Where this page sits in the result set. */ + page: PageInfo; +} + /** * A project as the detail view displays it. */ @@ -1221,6 +1290,105 @@ export interface ProjectDto { updatedAt: number; } +/** + * One configured Proxmox account. The token secret is never here. + */ +export interface ProxmoxAccountDto { + /** When the account was created. */ + createdAt: number; + /** + * The pinned fingerprint, once confirmed. + * @nullable + */ + fingerprint?: string | null; + /** The trust state: `unconfirmed` until the fingerprint is pinned. */ + fingerprintState: string; + /** The PVE host. */ + host: string; + /** The account's identity. */ + id: string; + /** The operator-facing name. */ + name: string; + /** + * The API port. + * @minimum 0 + */ + port: number; + /** The API token id (`user@realm!tokenname`), not secret on its own. */ + tokenId: string; +} + +/** + * One normalized discovery observation. + */ +export interface ProxmoxResourceDto { + /** The account that observed the resource. */ + accountId: string; + /** The cluster-visible id. */ + id: string; + /** + * The normalized kind: `node`, `qemu`, `lxc`, `qemu-template`, or + * `storage`. + */ + kind: string; + /** + * The display name, when carried. + * @nullable + */ + name?: string | null; + /** + * The hosting node, when the resource has one. + * @nullable + */ + node?: string | null; + /** When the observation was taken. */ + observedAt: number; + /** The PVE version the observation came from. */ + pveVersion: string; + /** + * The PVE status string, when carried. + * @nullable + */ + status?: string | null; + /** + * The VMID, when the resource has one. + * @minimum 0 + * @nullable + */ + vmid?: number | null; +} + +/** + * The discovery snapshot. + */ +export interface ProxmoxDiscoveryDto { + /** The account that produced the snapshot. */ + accountId: string; + /** When the snapshot was taken. */ + observedAt: number; + /** The PVE version seen. */ + pveVersion: string; + /** + * The count the API reported. + * @minimum 0 + */ + reportedCount: number; + /** The normalized resources. */ + resources: ProxmoxResourceDto[]; + /** The per-resource normalization warnings. */ + warnings: string[]; +} + +/** + * The observed fingerprint report. + */ +export interface ProxmoxFingerprintDto { + /** The account the fingerprint was observed for. */ + accountId: string; + /** The host certificate's SHA-256 fingerprint. */ + fingerprint: string; +} + /** * How the workflow's endpoint authenticates. */ @@ -1675,6 +1843,98 @@ export interface ResourceProjectDto { data: ResourceProjectDtoData; } +/** + * One configured Proxmox account. The token secret is never here. + */ +export type ResourceProxmoxAccountDtoData = { + /** When the account was created. */ + createdAt: number; + /** + * The pinned fingerprint, once confirmed. + * @nullable + */ + fingerprint?: string | null; + /** The trust state: `unconfirmed` until the fingerprint is pinned. */ + fingerprintState: string; + /** The PVE host. */ + host: string; + /** The account's identity. */ + id: string; + /** The operator-facing name. */ + name: string; + /** + * The API port. + * @minimum 0 + */ + port: number; + /** The API token id (`user@realm!tokenname`), not secret on its own. */ + tokenId: string; +}; + +/** + * A single resource. + * + * The payload is nested under `data` so that later top-level fields are an + * additive change rather than a breaking one. + */ +export interface ResourceProxmoxAccountDto { + /** One configured Proxmox account. The token secret is never here. */ + data: ResourceProxmoxAccountDtoData; +} + +/** + * The discovery snapshot. + */ +export type ResourceProxmoxDiscoveryDtoData = { + /** The account that produced the snapshot. */ + accountId: string; + /** When the snapshot was taken. */ + observedAt: number; + /** The PVE version seen. */ + pveVersion: string; + /** + * The count the API reported. + * @minimum 0 + */ + reportedCount: number; + /** The normalized resources. */ + resources: ProxmoxResourceDto[]; + /** The per-resource normalization warnings. */ + warnings: string[]; +}; + +/** + * A single resource. + * + * The payload is nested under `data` so that later top-level fields are an + * additive change rather than a breaking one. + */ +export interface ResourceProxmoxDiscoveryDto { + /** The discovery snapshot. */ + data: ResourceProxmoxDiscoveryDtoData; +} + +/** + * The observed fingerprint report. + */ +export type ResourceProxmoxFingerprintDtoData = { + /** The account the fingerprint was observed for. */ + accountId: string; + /** The host certificate's SHA-256 fingerprint. */ + fingerprint: string; +}; + +/** + * A single resource. + * + * The payload is nested under `data` so that later top-level fields are an + * additive change rather than a breaking one. + */ +export interface ResourceProxmoxFingerprintDto { + /** The observed fingerprint report. */ + data: ResourceProxmoxFingerprintDtoData; +} + /** * The dry run's plan response: the step vocabulary and the conditions * under which each step runs. @@ -4089,6 +4349,389 @@ const res = await fetch(getStartReadyWorkflowUrl(projectId), +export type listProxmoxAccountsResponse200 = { + data: PageProxmoxAccountDto + status: 200 +} + +export type listProxmoxAccountsResponse403 = { + data: ApiError + status: 403 +} + +export type listProxmoxAccountsResponseSuccess = (listProxmoxAccountsResponse200) & { + headers: Headers; +}; +export type listProxmoxAccountsResponseError = (listProxmoxAccountsResponse403) & { + headers: Headers; +}; + +export type listProxmoxAccountsResponse = (listProxmoxAccountsResponseSuccess | listProxmoxAccountsResponseError) + +export const getListProxmoxAccountsUrl = () => { + + + + + return `/api/v1/proxmox/accounts` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal or backend failure. + * @summary Lists the configured accounts. + */ +export const listProxmoxAccounts = async ( options?: RequestInit): Promise => { + + const res = await fetch(getListProxmoxAccountsUrl(), + { + ...options, + method: 'GET' + + + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: listProxmoxAccountsResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as listProxmoxAccountsResponse +} + + + +export type createProxmoxAccountResponse201 = { + data: ResourceProxmoxAccountDto + status: 201 +} + +export type createProxmoxAccountResponse400 = { + data: ApiError + status: 400 +} + +export type createProxmoxAccountResponse403 = { + data: ApiError + status: 403 +} + +export type createProxmoxAccountResponse409 = { + data: ApiError + status: 409 +} + +export type createProxmoxAccountResponseSuccess = (createProxmoxAccountResponse201) & { + headers: Headers; +}; +export type createProxmoxAccountResponseError = (createProxmoxAccountResponse400 | createProxmoxAccountResponse403 | createProxmoxAccountResponse409) & { + headers: Headers; +}; + +export type createProxmoxAccountResponse = (createProxmoxAccountResponseSuccess | createProxmoxAccountResponseError) + +export const getCreateProxmoxAccountUrl = () => { + + + + + return `/api/v1/proxmox/accounts` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, conflict, or backend + * failure. + * @summary Registers an account and stores its token secret. The account starts +`unconfirmed`: discovery stays locked until the fingerprint is confirmed. + */ +export const createProxmoxAccount = async (createProxmoxAccountRequest: CreateProxmoxAccountRequest, options?: RequestInit): Promise => { + + const getHeaders = (h?: NonNullable): Record => { + if (!h) return {}; + if (h instanceof Headers) return Object.fromEntries(h.entries()); + if (Array.isArray(h)) return Object.fromEntries(h); + return h; + }; +const res = await fetch(getCreateProxmoxAccountUrl(), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getHeaders(options?.headers) }, + body: JSON.stringify(createProxmoxAccountRequest) + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: createProxmoxAccountResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as createProxmoxAccountResponse +} + + + +export type deleteProxmoxAccountResponse204 = { + data: void + status: 204 +} + +export type deleteProxmoxAccountResponse403 = { + data: ApiError + status: 403 +} + +export type deleteProxmoxAccountResponse404 = { + data: ApiError + status: 404 +} + +export type deleteProxmoxAccountResponseSuccess = (deleteProxmoxAccountResponse204) & { + headers: Headers; +}; +export type deleteProxmoxAccountResponseError = (deleteProxmoxAccountResponse403 | deleteProxmoxAccountResponse404) & { + headers: Headers; +}; + +export type deleteProxmoxAccountResponse = (deleteProxmoxAccountResponseSuccess | deleteProxmoxAccountResponseError) + +export const getDeleteProxmoxAccountUrl = (accountId: string,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unknown account, or a + * backend failure. + * @summary Removes an account and its secret. + */ +export const deleteProxmoxAccount = async (accountId: string, options?: RequestInit): Promise => { + + const res = await fetch(getDeleteProxmoxAccountUrl(accountId), + { + ...options, + method: 'DELETE' + + + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: deleteProxmoxAccountResponse['data'] = body ? JSON.parse(body) : undefined + return { data, status: res.status, headers: res.headers } as deleteProxmoxAccountResponse +} + + + +export type confirmProxmoxFingerprintResponse200 = { + data: ResourceProxmoxAccountDto + status: 200 +} + +export type confirmProxmoxFingerprintResponse400 = { + data: ApiError + status: 400 +} + +export type confirmProxmoxFingerprintResponse403 = { + data: ApiError + status: 403 +} + +export type confirmProxmoxFingerprintResponse404 = { + data: ApiError + status: 404 +} + +export type confirmProxmoxFingerprintResponseSuccess = (confirmProxmoxFingerprintResponse200) & { + headers: Headers; +}; +export type confirmProxmoxFingerprintResponseError = (confirmProxmoxFingerprintResponse400 | confirmProxmoxFingerprintResponse403 | confirmProxmoxFingerprintResponse404) & { + headers: Headers; +}; + +export type confirmProxmoxFingerprintResponse = (confirmProxmoxFingerprintResponseSuccess | confirmProxmoxFingerprintResponseError) + +export const getConfirmProxmoxFingerprintUrl = (accountId: string,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}/confirm` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unknown account, or a + * malformed fingerprint. + * @summary Pins the confirmed fingerprint as the account's trust anchor. + */ +export const confirmProxmoxFingerprint = async (accountId: string, + confirmProxmoxFingerprintRequest: ConfirmProxmoxFingerprintRequest, options?: RequestInit): Promise => { + + const getHeaders = (h?: NonNullable): Record => { + if (!h) return {}; + if (h instanceof Headers) return Object.fromEntries(h.entries()); + if (Array.isArray(h)) return Object.fromEntries(h); + return h; + }; +const res = await fetch(getConfirmProxmoxFingerprintUrl(accountId), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getHeaders(options?.headers) }, + body: JSON.stringify(confirmProxmoxFingerprintRequest) + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: confirmProxmoxFingerprintResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as confirmProxmoxFingerprintResponse +} + + + +export type discoverProxmoxClusterResponse200 = { + data: ResourceProxmoxDiscoveryDto + status: 200 +} + +export type discoverProxmoxClusterResponse403 = { + data: ApiError + status: 403 +} + +export type discoverProxmoxClusterResponse404 = { + data: ApiError + status: 404 +} + +export type discoverProxmoxClusterResponse409 = { + data: ApiError + status: 409 +} + +export type discoverProxmoxClusterResponseSuccess = (discoverProxmoxClusterResponse200) & { + headers: Headers; +}; +export type discoverProxmoxClusterResponseError = (discoverProxmoxClusterResponse403 | discoverProxmoxClusterResponse404 | discoverProxmoxClusterResponse409) & { + headers: Headers; +}; + +export type discoverProxmoxClusterResponse = (discoverProxmoxClusterResponseSuccess | discoverProxmoxClusterResponseError) + +export const getDiscoverProxmoxClusterUrl = (accountId: string,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}/discovery` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unconfirmed account, or + * a source failure. + * @summary Discovers the cluster through one trusted account. + */ +export const discoverProxmoxCluster = async (accountId: string, options?: RequestInit): Promise => { + + const res = await fetch(getDiscoverProxmoxClusterUrl(accountId), + { + ...options, + method: 'GET' + + + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: discoverProxmoxClusterResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as discoverProxmoxClusterResponse +} + + + +export type observeProxmoxFingerprintResponse200 = { + data: ResourceProxmoxFingerprintDto + status: 200 +} + +export type observeProxmoxFingerprintResponse403 = { + data: ApiError + status: 403 +} + +export type observeProxmoxFingerprintResponse404 = { + data: ApiError + status: 404 +} + +export type observeProxmoxFingerprintResponse502 = { + data: ApiError + status: 502 +} + +export type observeProxmoxFingerprintResponseSuccess = (observeProxmoxFingerprintResponse200) & { + headers: Headers; +}; +export type observeProxmoxFingerprintResponseError = (observeProxmoxFingerprintResponse403 | observeProxmoxFingerprintResponse404 | observeProxmoxFingerprintResponse502) & { + headers: Headers; +}; + +export type observeProxmoxFingerprintResponse = (observeProxmoxFingerprintResponseSuccess | observeProxmoxFingerprintResponseError) + +export const getObserveProxmoxFingerprintUrl = (accountId: string,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}/observe` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unknown account, or an + * unreachable host. + * @summary Captures the host's certificate fingerprint without sending any +credential. The report is the input to the confirm step. + */ +export const observeProxmoxFingerprint = async (accountId: string, options?: RequestInit): Promise => { + + const res = await fetch(getObserveProxmoxFingerprintUrl(accountId), + { + ...options, + method: 'POST' + + + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: observeProxmoxFingerprintResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as observeProxmoxFingerprintResponse +} + + + export type getSystemInfoResponse200 = { data: SystemInfo status: 200 From 980f2c24ee0d1267c01b71d9bfd067640afc1fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= Date: Sun, 20 Sep 2026 16:04:40 +0000 Subject: [PATCH 2/3] FM-600: address the cubic review findings --- Cargo.lock | 16 ++ crates/fleet-api/src/proxmox.rs | 51 ++++- crates/fleet-application/src/proxmox.rs | 177 ++++++++++++++---- crates/fleet-application/tests/proxmox.rs | 106 ++++++----- crates/fleet-controller/src/main.rs | 5 +- crates/fleet-controller/src/proxmox_store.rs | 30 +-- crates/fleet-controller/tests/proxmox.rs | 8 + .../migrations/0017_proxmox_accounts.sql | 3 + crates/fleet-storage-sqlite/src/proxmox.rs | 89 +++++++-- crates/fleetctl/src/lib.rs | 2 +- .../fleet-provider-proxmox/Cargo.toml | 4 +- .../fleet-provider-proxmox/src/lib.rs | 139 +++++++------- .../fleet-provider-proxmox/tests/pin_live.rs | 33 ++-- packages/api-client/openapi.json | 22 +++ packages/api-client/src/generated/fleet.ts | 27 ++- 15 files changed, 503 insertions(+), 209 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0382916..edcfdc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -727,6 +727,7 @@ dependencies = [ "async-trait", "fleet-application", "fleet-core", + "futures-util", "reqwest", "rustls", "serde", @@ -2224,12 +2225,14 @@ dependencies = [ "sync_wrapper", "tokio", "tokio-rustls", + "tokio-util", "tower", "tower-http 0.6.11", "tower-service", "url", "wasm-bindgen", "wasm-bindgen-futures", + "wasm-streams", "web-sys", "webpki-roots", ] @@ -3425,6 +3428,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "wasm-streams" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15053d8d85c7eccdbefef60f06769760a563c7f0a9d6902a13d35c7800b0ad65" +dependencies = [ + "futures-util", + "js-sys", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-sys", +] + [[package]] name = "web-sys" version = "0.3.104" diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs index 93fadbb..8d9e0a1 100644 --- a/crates/fleet-api/src/proxmox.rs +++ b/crates/fleet-api/src/proxmox.rs @@ -11,7 +11,7 @@ use std::sync::Arc; use axum::{ Extension, Json, - extract::{Path, State}, + extract::{Path, Query, State}, http::StatusCode, }; use fleet_application::proxmox::{NewProxmoxAccount, ProxmoxAccount, ProxmoxUseCaseError}; @@ -208,6 +208,20 @@ pub struct ConfirmProxmoxFingerprintRequest { pub fingerprint: String, } +/// The list-accounts query parameters. +#[derive(Debug, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ListProxmoxAccountsParams { + /// The maximum number of accounts to return. + pub limit: Option, + /// The opaque cursor: the last account id of the previous page. + pub cursor: Option, +} + +/// The default and maximum page bounds, matching the machine read model. +const DEFAULT_PAGE_LIMIT: u32 = 50; +const MAX_PAGE_LIMIT: u32 = 200; + /// Lists the configured accounts. /// /// # Errors @@ -229,25 +243,50 @@ pub struct ConfirmProxmoxFingerprintRequest { description = "The caller may not read the Proxmox surface.", body = crate::error::ApiError ), + ), + params( + ( + "limit" = Option, + Query, + description = "The maximum number of accounts to return." + ), + ( + "cursor" = Option, + Query, + description = "The opaque cursor: the last account id of the previous page." + ), ) )] pub async fn list_proxmox_accounts( State(state): State>, principal: Option>, Extension(correlation_id): Extension, + Query(params): Query, ) -> Result>, ApiErrorResponse> { let proxmox = proxmox_or_error(&state, correlation_id)?; let principal = crate::operations::principal_or_error(principal, correlation_id)?; + // A zero or absent limit means the default; the page never advertises + // more than it returns. + let limit = params + .limit + .filter(|limit| *limit > 0) + .unwrap_or(DEFAULT_PAGE_LIMIT) + .min(MAX_PAGE_LIMIT); let accounts = proxmox - .list(state.authorizer.as_ref(), &principal) + .list( + state.authorizer.as_ref(), + &principal, + limit, + params.cursor.as_deref(), + ) .await .map_err(|error| map_proxmox_error(&error, correlation_id))?; + let next_cursor = (accounts.len() == usize::try_from(limit).unwrap_or(0)) + .then(|| accounts.last().map(|account| account.id.clone())) + .flatten(); let items: Vec = accounts.into_iter().map(Into::into).collect(); Ok(Json(Page { - page: PageInfo { - next_cursor: None, - limit: items.len().try_into().unwrap_or(u32::MAX), - }, + page: PageInfo { next_cursor, limit }, items, })) } diff --git a/crates/fleet-application/src/proxmox.rs b/crates/fleet-application/src/proxmox.rs index 96a4e6b..9059556 100644 --- a/crates/fleet-application/src/proxmox.rs +++ b/crates/fleet-application/src/proxmox.rs @@ -69,6 +69,10 @@ pub struct ProxmoxAccount { pub token_id: String, /// The pinned host-certificate fingerprint, once confirmed. pub fingerprint: Option, + /// The fingerprint the trust probe last observed, not yet confirmed. + /// `confirm` must match this; a caller cannot pin a digest it never + /// observed through the probe. + pub observed_fingerprint: Option, /// When the account was created (epoch millis). pub created_at: i64, } @@ -268,6 +272,17 @@ pub trait ProxmoxAccountPort: fmt::Debug + Send + Sync { id: &str, fingerprint: Option, ) -> Result; + /// Records the fingerprint the trust probe observed, for the confirm + /// step to match against. `None` clears the observation. + /// + /// # Errors + /// + /// Fails when unknown or the backend errors. + async fn set_observed_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result; /// Removes the account. /// /// # Errors @@ -382,11 +397,14 @@ pub trait ProxmoxDiscoverPort: fmt::Debug + Send + Sync { } /// The provider's own discovery shape, before application-layer enrichment. +/// The resources' provenance fields (`account_id`, `pve_version`, +/// `observed_at`) are placeholders here; [`ProxmoxAccounts::discover`] +/// overwrites all three with authoritative values. #[derive(Clone, Debug, PartialEq, Eq)] pub struct RawDiscovery { /// The PVE version seen. pub version: String, - /// The normalized resources, without provenance. + /// The normalized resources, provenance pending. pub resources: Vec, /// The per-resource normalization warnings. pub warnings: Vec, @@ -437,7 +455,10 @@ impl ProxmoxAccounts { } } - /// Lists the configured accounts with their trust states. + /// Lists the configured accounts with their trust states, bounded by + /// the page limit. The cursor is the last account id of the previous + /// page; the list is ordered newest first, so a following page really + /// advances. /// /// # Errors /// @@ -446,6 +467,8 @@ impl ProxmoxAccounts { &self, authorizer: &dyn Authorizer, principal: &ActingPrincipal, + limit: u32, + after_id: Option<&str>, ) -> Result, ProxmoxUseCaseError> { authorize( authorizer, @@ -456,20 +479,33 @@ impl ProxmoxAccounts { }, ) .map_err(ProxmoxUseCaseError::Denied)?; - self.accounts - .list() - .await - .map_err(|detail| ProxmoxUseCaseError::Backend { - context: "accounts", - detail, - }) + let mut accounts = + self.accounts + .list() + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + })?; + if let Some(after_id) = after_id { + let Some(position) = accounts.iter().position(|account| account.id == after_id) else { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the cursor names no account in the list".to_owned(), + }); + }; + accounts.drain(..=position); + } + accounts.truncate(usize::try_from(limit).unwrap_or(accounts.len())); + Ok(accounts) } /// Registers an account and stores its token secret. The secret goes /// into the encrypted store and is never echoed, logged, or audited; - /// the audit event names the account and the token id only. The new - /// account starts `Unconfirmed`: discovery stays locked until the - /// fingerprint is confirmed. + /// the audit intent lands before any mutation, carrying the account + /// name as provenance (the token id contains "token", which the audit + /// guard structurally rejects — and should). The new account starts + /// `Unconfirmed`: discovery stays locked until the fingerprint is + /// confirmed. /// /// # Errors /// @@ -494,11 +530,29 @@ impl ProxmoxAccounts { validate_name(&new.name)?; validate_host(&new.host)?; validate_token_id(&new.token_id)?; + if let Some(port) = new.port + && port == 0 + { + return Err(ProxmoxUseCaseError::Invalid { + detail: "the port must be 1..=65535".to_owned(), + }); + } if token_secret.is_empty() || token_secret.len() > 256 { return Err(ProxmoxUseCaseError::Invalid { detail: "the token secret must be 1..=256 characters".to_owned(), }); } + // The audit intent lands BEFORE any mutation: a failure to audit + // prevents the mutation, so durable state can never exist without + // its intent. + self.audit_event( + principal, + Permission::ProxmoxConfig, + None, + "proxmox_account_creating", + Some(("name", new.name.as_str())), + ) + .await?; let account = self.accounts.create(&new).await.map_err(|detail| { if is_taken(&detail) { ProxmoxUseCaseError::Conflict { detail } @@ -510,10 +564,19 @@ impl ProxmoxAccounts { } })?; // A failed secret write must not leave an account that pretends to - // be usable: the account is removed again. A clear of a record that - // was never written succeeds. + // be usable: the account is removed again. If even the rollback + // fails, the orphan is named — the name stays unusable until an + // operator removes it, which is honest rather than silent. if let Err(error) = self.credentials.store(&account.id, token_secret).await { - let _ = self.accounts.delete(&account.id).await; + if let Err(rollback) = self.accounts.delete(&account.id).await { + return Err(ProxmoxUseCaseError::Backend { + context: "credentials", + detail: format!( + "the secret write failed ({error}) and the rollback failed too: account {} lingers ({rollback})", + account.id + ), + }); + } return Err(ProxmoxUseCaseError::Backend { context: "credentials", detail: error.to_string(), @@ -554,27 +617,29 @@ impl ProxmoxAccounts { ) .map_err(ProxmoxUseCaseError::Denied)?; let account = self.require_account(account_id).await?; - self.accounts - .delete(account_id) - .await - .map_err(|detail| ProxmoxUseCaseError::Backend { - context: "accounts", - detail, - })?; - // The secret's removal is best effort: the record is gone from the - // account surface either way, and a stuck store must not make the - // account undeletable. The detail is logged at the boundary. - if let Err(error) = self.credentials.clear(account_id).await { - let _ = error; - } self.audit_event( principal, Permission::ProxmoxConfig, Some(account_id), - "proxmox_account_deleted", + "proxmox_account_deleting", Some(("name", account.name.as_str())), ) .await?; + // The secret is removed first: a delete that leaves the credential + // behind is a failure, not a success with a footnote. The account + // row is only removed once the credential is gone, so a retry is + // always safe and a half-deleted state cannot hold a secret. + self.credentials + .clear(account_id) + .await + .map_err(ProxmoxUseCaseError::Credentials)?; + self.accounts + .delete(account_id) + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + })?; Ok(()) } @@ -602,10 +667,21 @@ impl ProxmoxAccounts { ) .map_err(ProxmoxUseCaseError::Denied)?; let account = self.require_account(account_id).await?; - self.trust + let observed = self + .trust .observe(&account.host, account.port) .await - .map_err(ProxmoxUseCaseError::Source) + .map_err(ProxmoxUseCaseError::Source)?; + // The observation is persisted before it is reported, so `confirm` + // can only pin what this probe actually saw. + self.accounts + .set_observed_fingerprint(account_id, Some(observed.clone())) + .await + .map_err(|detail| ProxmoxUseCaseError::Backend { + context: "accounts", + detail, + })?; + Ok(observed) } /// Pins the confirmed fingerprint. Only a fingerprint this principal @@ -631,13 +707,40 @@ impl ProxmoxAccounts { }, ) .map_err(ProxmoxUseCaseError::Denied)?; - self.require_account(account_id).await?; + let account = self.require_account(account_id).await?; let normalized = normalize_fingerprint(fingerprint); if normalized.len() != 64 || !normalized.chars().all(|c| c.is_ascii_hexdigit()) { return Err(ProxmoxUseCaseError::Invalid { detail: "the fingerprint must be a SHA-256 digest (colons optional)".to_owned(), }); } + // Only a fingerprint this probe observed may be pinned: a digest + // typed from memory or copied from elsewhere is refused, so trust + // always flows through the observe step. + let Some(observed) = account.observed_fingerprint.as_deref() else { + return Err(ProxmoxUseCaseError::Invalid { + detail: + "observe the host's fingerprint first; confirm pins only what the probe saw" + .to_owned(), + }); + }; + if normalize_fingerprint(observed) != normalized { + return Err(ProxmoxUseCaseError::Invalid { + detail: + "the supplied fingerprint does not match the observed one; observe again if the host changed" + .to_owned(), + }); + } + // The audit intent lands BEFORE the mutation, per the two-phase + // audit rule. + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(account_id), + "proxmox_fingerprint_confirming", + Some(("fingerprint", normalized.as_str())), + ) + .await?; let account = self .accounts .set_fingerprint(account_id, Some(normalized)) @@ -646,14 +749,6 @@ impl ProxmoxAccounts { context: "accounts", detail, })?; - self.audit_event( - principal, - Permission::ProxmoxConfig, - Some(account_id), - "proxmox_fingerprint_confirmed", - Some(("fingerprint", account.fingerprint.as_deref().unwrap_or(""))), - ) - .await?; Ok(account) } @@ -803,7 +898,7 @@ fn validate_host(host: &str) -> Result<(), ProxmoxUseCaseError> { detail: "the host must be 1..=253 characters".to_owned(), }); } - if host.starts_with("http") { + if host.contains("://") { return Err(ProxmoxUseCaseError::Invalid { detail: "the host is a bare host or IP, not a URL".to_owned(), }); diff --git a/crates/fleet-application/tests/proxmox.rs b/crates/fleet-application/tests/proxmox.rs index a7f6511..a073141 100644 --- a/crates/fleet-application/tests/proxmox.rs +++ b/crates/fleet-application/tests/proxmox.rs @@ -77,16 +77,18 @@ impl ProxmoxAccountPort for FakeAccounts { { return Err(format!("the account name {:?} is already taken", new.name)); } + let mut accounts = self.accounts.lock().unwrap(); let account = fleet_application::proxmox::ProxmoxAccount { - id: format!("acc-{}", self.accounts.lock().unwrap().len() + 1), + id: format!("acc-{}", accounts.len() + 1), name: new.name.clone(), host: new.host.clone(), port: new.port.unwrap_or(8006), token_id: new.token_id.clone(), fingerprint: None, + observed_fingerprint: None, created_at: NOW, }; - self.accounts.lock().unwrap().push(account.clone()); + accounts.push(account.clone()); Ok(account) } @@ -113,6 +115,20 @@ impl ProxmoxAccountPort for FakeAccounts { Ok(account.clone()) } + async fn set_observed_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result { + let mut accounts = self.accounts.lock().unwrap(); + let account = accounts + .iter_mut() + .find(|account| account.id == id) + .ok_or_else(|| format!("account {id} not found"))?; + account.observed_fingerprint = fingerprint; + Ok(account.clone()) + } + async fn delete(&self, id: &str) -> Result<(), String> { let mut accounts = self.accounts.lock().unwrap(); let before = accounts.len(); @@ -286,12 +302,21 @@ async fn create_account(proxmox: &ProxmoxAccounts) -> fleet_application::proxmox .expect("the account is well formed") } +/// Observes and confirms the canned fingerprint, the honest trust flow. +async fn observe_and_confirm(proxmox: &ProxmoxAccounts, account_id: &str) { + let observed = proxmox + .observe(&AllowAll, &principal(), account_id) + .await + .expect("the probe answers"); + proxmox + .confirm(&AllowAll, &principal(), account_id, &observed) + .await + .expect("the observed fingerprint confirms"); +} + #[tokio::test] async fn discovery_is_locked_until_the_fingerprint_is_confirmed() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); let account = create_account(&proxmox).await; let error = proxmox .discover(&AllowAll, &principal(), &account.id, NOW) @@ -346,7 +371,7 @@ async fn observe_confirm_then_discover_walks_the_trust_flow() { intent .metadata .entries() - .any(|(key, value)| key == "event" && value == "proxmox_fingerprint_confirmed") + .any(|(key, value)| key == "event" && value == "proxmox_fingerprint_confirming") }), "{intents:?}" ); @@ -362,10 +387,7 @@ async fn a_fingerprint_mismatch_is_reported_as_evidence_not_an_empty_list() { FakeProbe::with(FP), ); let account = create_account(&proxmox).await; - proxmox - .confirm(&AllowAll, &principal(), &account.id, FP) - .await - .unwrap(); + observe_and_confirm(&proxmox, &account.id).await; let error = proxmox .discover(&AllowAll, &principal(), &account.id, NOW) .await @@ -393,15 +415,10 @@ async fn auth_and_privilege_failures_are_honest_states() { detail: "timeout".to_owned(), }, ] { - let (proxmox, _audit) = service( - FakeDiscovery::with(Err(source_error)), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = + service(FakeDiscovery::with(Err(source_error)), FakeProbe::with(FP)); let account = create_account(&proxmox).await; - proxmox - .confirm(&AllowAll, &principal(), &account.id, FP) - .await - .unwrap(); + observe_and_confirm(&proxmox, &account.id).await; let error = proxmox .discover(&AllowAll, &principal(), &account.id, NOW) .await @@ -412,15 +429,9 @@ async fn auth_and_privilege_failures_are_honest_states() { #[tokio::test] async fn the_token_secret_never_surfaces_in_audit_or_errors() { - let (proxmox, audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); let account = create_account(&proxmox).await; - proxmox - .confirm(&AllowAll, &principal(), &account.id, FP) - .await - .unwrap(); + observe_and_confirm(&proxmox, &account.id).await; let _ = proxmox .discover(&AllowAll, &principal(), &account.id, NOW) .await @@ -462,7 +473,7 @@ async fn a_failed_secret_write_removes_the_account() { Arc::new(FakeAccounts::default()), Arc::new(RefusingStore), FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), + FakeProbe::with(FP), audit, ); let error = proxmox @@ -483,16 +494,16 @@ async fn a_failed_secret_write_removes_the_account() { matches!(error, ProxmoxUseCaseError::Backend { .. }), "{error}" ); - let accounts = proxmox.list(&AllowAll, &principal()).await.unwrap(); + let accounts = proxmox + .list(&AllowAll, &principal(), 50, None) + .await + .unwrap(); assert!(accounts.is_empty(), "the half-made account is gone"); } #[tokio::test] async fn deleting_an_account_clears_its_secret() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); let account = create_account(&proxmox).await; proxmox .delete(&AllowAll, &principal(), &account.id) @@ -500,7 +511,7 @@ async fn deleting_an_account_clears_its_secret() { .unwrap(); assert!( proxmox - .list(&AllowAll, &principal()) + .list(&AllowAll, &principal(), 50, None) .await .unwrap() .is_empty() @@ -509,12 +520,12 @@ async fn deleting_an_account_clears_its_secret() { #[tokio::test] async fn a_denied_caller_never_reaches_the_ports() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); let account = create_account(&proxmox).await; - let error = proxmox.list(&DenyAll, &principal()).await.unwrap_err(); + let error = proxmox + .list(&DenyAll, &principal(), 50, None) + .await + .unwrap_err(); assert!(matches!(error, ProxmoxUseCaseError::Denied(_))); let error = proxmox .discover(&DenyAll, &principal(), &account.id, NOW) @@ -525,10 +536,7 @@ async fn a_denied_caller_never_reaches_the_ports() { #[tokio::test] async fn malformed_accounts_are_refused_before_any_write() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); for new in [ NewProxmoxAccount { name: String::new(), @@ -560,7 +568,7 @@ async fn malformed_accounts_are_refused_before_any_write() { } assert!( proxmox - .list(&AllowAll, &principal()) + .list(&AllowAll, &principal(), 50, None) .await .unwrap() .is_empty() @@ -569,10 +577,7 @@ async fn malformed_accounts_are_refused_before_any_write() { #[tokio::test] async fn a_conflicting_account_name_is_a_conflict() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); create_account(&proxmox).await; let error = proxmox .create( @@ -596,10 +601,7 @@ async fn a_conflicting_account_name_is_a_conflict() { #[tokio::test] async fn confirm_refuses_a_malformed_fingerprint() { - let (proxmox, _audit) = service( - FakeDiscovery::with(Ok(discovery_ok())), - FakeProbe::with("AA:BB"), - ); + let (proxmox, _audit) = service(FakeDiscovery::with(Ok(discovery_ok())), FakeProbe::with(FP)); let account = create_account(&proxmox).await; for fingerprint in ["", "nothex", "A".repeat(63).as_str(), &"G".repeat(64)] { let error = proxmox diff --git a/crates/fleet-controller/src/main.rs b/crates/fleet-controller/src/main.rs index 3cb6f81..8e91fc2 100644 --- a/crates/fleet-controller/src/main.rs +++ b/crates/fleet-controller/src/main.rs @@ -354,10 +354,7 @@ fn run_serve(config: fleet_config::ControllerConfig) -> ExitCode { std::sync::Arc::new(fleet_controller::proxmox_store::compose_proxmox( store.pool().clone(), secrets.clone(), - std::sync::Arc::new( - fleet_provider_proxmox::ReqwestPveTransport::new() - .expect("the PVE transport must build"), - ), + std::sync::Arc::new(fleet_provider_proxmox::ReqwestPveTransport::new()), std::sync::Arc::new(fleet_storage_sqlite::AuditSink::new(store.pool().clone())), )) }); diff --git a/crates/fleet-controller/src/proxmox_store.rs b/crates/fleet-controller/src/proxmox_store.rs index 2b6425c..4395761 100644 --- a/crates/fleet-controller/src/proxmox_store.rs +++ b/crates/fleet-controller/src/proxmox_store.rs @@ -252,6 +252,9 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { match self.client.discover(request).await { Ok(discovery) => Ok(RawDiscovery { version: discovery.version.clone(), + // The provenance fields are placeholders the application + // layer overwrites with authoritative values; the provider + // shape is shared so the DTO travels one boundary. resources: discovery .resources .into_iter() @@ -262,8 +265,8 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { vmid: resource.vmid, name: resource.name, status: resource.status, - account_id: account.id.clone(), - pve_version: discovery.version.clone(), + account_id: String::new(), + pve_version: String::new(), observed_at: 0, }) .collect(), @@ -282,10 +285,19 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { } Err(fleet_provider_proxmox::PveApiError::Transport( fleet_provider_proxmox::PveTransportError::FingerprintMismatch { observed, pinned }, - )) => Err(ProxmoxSourceError::FingerprintMismatch { - observed, - pinned: pinned.unwrap_or(pinned_placeholder()), - }), + )) => { + // The discovery request always pins; a mismatch without a + // pin is a transport invariant violation, reported as such + // rather than papered over with a fabricated value. + let Some(pinned) = pinned else { + return Err(ProxmoxSourceError::Connect { + detail: format!( + "the transport reported a fingerprint mismatch without a pin (observed {observed})" + ), + }); + }; + Err(ProxmoxSourceError::FingerprintMismatch { observed, pinned }) + } Err(fleet_provider_proxmox::PveApiError::Transport(other)) => { Err(ProxmoxSourceError::Connect { detail: other.to_string(), @@ -295,12 +307,6 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { } } -fn pinned_placeholder() -> String { - // Unreachable in practice: the discovery request always pins. Kept for - // exhaustive matching without a panic path. - String::new() -} - /// Composes the Proxmox use cases over its ports. #[must_use] pub fn compose_proxmox( diff --git a/crates/fleet-controller/tests/proxmox.rs b/crates/fleet-controller/tests/proxmox.rs index ba838d9..4736e14 100644 --- a/crates/fleet-controller/tests/proxmox.rs +++ b/crates/fleet-controller/tests/proxmox.rs @@ -325,6 +325,14 @@ async fn a_mismatched_fingerprint_is_reported_with_both_values() { .await; assert_eq!(status, axum::http::StatusCode::CREATED, "{body}"); let account_id = body["data"]["id"].as_str().unwrap().to_owned(); + // Trust flows through observe: capture, then confirm what was seen. + let (status, body) = harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/observe"), + json!({}), + ) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); harness .post( &format!("/api/v1/proxmox/accounts/{account_id}/confirm"), diff --git a/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql index fef5ab0..5d89d00 100644 --- a/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql +++ b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql @@ -2,6 +2,8 @@ -- state. The token secret never lives here: accounts reference Fleet's -- encrypted secret store by account id. The fingerprint is the pinned -- SHA-256 of the host certificate, empty until the trust step confirms it. +-- observed_fingerprint is the probe's last capture; confirm must match it, +-- so trust always flows through the observe step. CREATE TABLE proxmox_accounts ( id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE, @@ -9,6 +11,7 @@ CREATE TABLE proxmox_accounts ( port INTEGER NOT NULL, token_id TEXT NOT NULL, fingerprint TEXT NOT NULL DEFAULT '', + observed_fingerprint TEXT NOT NULL DEFAULT '', created_at INTEGER NOT NULL ) STRICT; diff --git a/crates/fleet-storage-sqlite/src/proxmox.rs b/crates/fleet-storage-sqlite/src/proxmox.rs index 766180e..b46c20d 100644 --- a/crates/fleet-storage-sqlite/src/proxmox.rs +++ b/crates/fleet-storage-sqlite/src/proxmox.rs @@ -35,6 +35,10 @@ impl ProxmoxAccountRepository { port: u16::try_from(row.get::("port")).unwrap_or(8006), token_id: row.get("token_id"), fingerprint: (!fingerprint.is_empty()).then_some(fingerprint), + observed_fingerprint: { + let observed: String = row.get("observed_fingerprint"); + (!observed.is_empty()).then_some(observed) + }, created_at: row.get("created_at"), } } @@ -47,8 +51,8 @@ impl ProxmoxAccountPort for ProxmoxAccountRepository { let now = fleet_core::SystemClock::now_unix_millis(); let port = account.port.unwrap_or(8006); let result = sqlx::query( - "INSERT INTO proxmox_accounts (id, name, host, port, token_id, fingerprint, created_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, '', ?6)", + "INSERT INTO proxmox_accounts (id, name, host, port, token_id, fingerprint, observed_fingerprint, created_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, '', '', ?6)", ) .bind(&id) .bind(&account.name) @@ -69,7 +73,7 @@ impl ProxmoxAccountPort for ProxmoxAccountRepository { } async fn get(&self, id: &str) -> Result { - sqlx::query("SELECT id, name, host, port, token_id, fingerprint, created_at FROM proxmox_accounts WHERE id = ?1") + sqlx::query("SELECT id, name, host, port, token_id, fingerprint, observed_fingerprint, created_at FROM proxmox_accounts WHERE id = ?1") .bind(id) .fetch_optional(&self.pool) .await @@ -79,7 +83,7 @@ impl ProxmoxAccountPort for ProxmoxAccountRepository { } async fn list(&self) -> Result, String> { - let rows = sqlx::query("SELECT id, name, host, port, token_id, fingerprint, created_at FROM proxmox_accounts ORDER BY created_at DESC") + let rows = sqlx::query("SELECT id, name, host, port, token_id, fingerprint, observed_fingerprint, created_at FROM proxmox_accounts ORDER BY created_at DESC, id DESC") .fetch_all(&self.pool) .await .map_err(|error| format!("list failed: {error}"))?; @@ -91,14 +95,22 @@ impl ProxmoxAccountPort for ProxmoxAccountRepository { id: &str, fingerprint: Option, ) -> Result { - let value = fingerprint.unwrap_or_default(); - sqlx::query("UPDATE proxmox_accounts SET fingerprint = ?2 WHERE id = ?1") - .bind(id) - .bind(&value) - .execute(&self.pool) + self.update_fingerprint_column(id, "fingerprint", fingerprint, "set_fingerprint") .await - .map_err(|error| format!("set_fingerprint failed: {error}"))?; - self.get(id).await + } + + async fn set_observed_fingerprint( + &self, + id: &str, + fingerprint: Option, + ) -> Result { + self.update_fingerprint_column( + id, + "observed_fingerprint", + fingerprint, + "set_observed_fingerprint", + ) + .await } async fn delete(&self, id: &str) -> Result<(), String> { @@ -114,6 +126,61 @@ impl ProxmoxAccountPort for ProxmoxAccountRepository { } } +impl ProxmoxAccountRepository { + /// Updates one fingerprint column and reads the row back inside one + /// `BEGIN IMMEDIATE` transaction, so a racing confirmation cannot + /// return (and audit) another caller's fingerprint. + async fn update_fingerprint_column( + &self, + id: &str, + column: &str, + fingerprint: Option, + context: &str, + ) -> Result { + // Two fixed queries rather than dynamic SQL: the column comes from + // the call site, and fixed strings keep the audit surface obvious. + let value = fingerprint.unwrap_or_default(); + let mut transaction = self + .pool + .begin_with("BEGIN IMMEDIATE") + .await + .map_err(|error| format!("{context} failed: {error}"))?; + let updated = match column { + "fingerprint" => { + sqlx::query("UPDATE proxmox_accounts SET fingerprint = ?2 WHERE id = ?1") + .bind(id) + .bind(&value) + .execute(&mut *transaction) + .await + .map_err(|error| format!("{context} failed: {error}"))? + } + "observed_fingerprint" => { + sqlx::query("UPDATE proxmox_accounts SET observed_fingerprint = ?2 WHERE id = ?1") + .bind(id) + .bind(&value) + .execute(&mut *transaction) + .await + .map_err(|error| format!("{context} failed: {error}"))? + } + other => return Err(format!("{context} failed: unknown column {other:?}")), + }; + if updated.rows_affected() == 0 { + return Err(format!("account {id} not found")); + } + let row = sqlx::query("SELECT id, name, host, port, token_id, fingerprint, observed_fingerprint, created_at FROM proxmox_accounts WHERE id = ?1") + .bind(id) + .fetch_one(&mut *transaction) + .await + .map_err(|error| format!("{context} failed: {error}"))?; + let account = Self::row_to_account(&row); + transaction + .commit() + .await + .map_err(|error| format!("{context} failed: {error}"))?; + Ok(account) + } +} + fn is_unique_violation(error: &sqlx::Error) -> bool { matches!( error diff --git a/crates/fleetctl/src/lib.rs b/crates/fleetctl/src/lib.rs index 0d5fc16..b7e3f21 100644 --- a/crates/fleetctl/src/lib.rs +++ b/crates/fleetctl/src/lib.rs @@ -1180,7 +1180,7 @@ fn parse_proxmox_command(verb: &str, rest: &[&str]) -> Result fn usage() -> String { format!( - "Usage: fleetctl [--url ] [--socket ] [--output json|text] \n\nCommands:\n status\n system\n operations list [--limit ]\n operations get \n operations cancel \n machines list [--tag ] [--group ] [--capability ] [--status ] [--limit ]\n machines get \n machines onboard create --user --host [--port ] [--name ] [--description ] [--tag ]... [--group ]... --auth agent|identity-file [--identity ]\n machines onboard list [--limit ]\n machines onboard get \n machines onboard test [--wait] [--timeout ]\n machines onboard discover [--wait] [--timeout ]\n machines onboard confirm --fingerprint \n machines onboard add \n machines onboard cancel \n projects list [--remote-prefix

] [--name-substring ] [--limit ]\n projects get \n projects create --remote --name [--description ]\n projects update --name [--description ]\n projects delete \n projects discover --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects record (the discovery result is read from stdin)\n projects ready --root [--dry-run] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects clone --root [--branch ] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects pull --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects status --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects write-config --root --file --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (contents from stdin)\n skills probe --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--artifact-url --artifact-sha256 ] [--wait] [--timeout ]\n skills deploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n skills undeploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n frogenv status|setup|login|request|sync --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n frogenv run --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n mise inventory|status --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise install --tool --version --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise exec --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n apply --plan-id --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (the plan JSON is read from stdin)\n tailnet status\n tailnet configure --client-id (the client secret is read from stdin)\n tailnet clear\n tailnet devices [--limit ]\n tailnet import --user [--port ]\n machines install-node --endpoint --auth agent|identity-file [--identity ] [--artifact-url --artifact-sha256 ] [--controller-url ] [--install-timeout ] [--connect-timeout ] [--wait] [--timeout ]\n\n`status` prefers the node's local socket (default {DEFAULT_SOCKET}); `--url` is the explicit direct-controller override. Other commands talk to the controller, which defaults to {DEFAULT_URL}." + "Usage: fleetctl [--url ] [--socket ] [--output json|text] \n\nCommands:\n status\n system\n operations list [--limit ]\n operations get \n operations cancel \n machines list [--tag ] [--group ] [--capability ] [--status ] [--limit ]\n machines get \n machines onboard create --user --host [--port ] [--name ] [--description ] [--tag ]... [--group ]... --auth agent|identity-file [--identity ]\n machines onboard list [--limit ]\n machines onboard get \n machines onboard test [--wait] [--timeout ]\n machines onboard discover [--wait] [--timeout ]\n machines onboard confirm --fingerprint \n machines onboard add \n machines onboard cancel \n projects list [--remote-prefix

] [--name-substring ] [--limit ]\n projects get \n projects create --remote --name [--description ]\n projects update --name [--description ]\n projects delete \n projects discover --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects record (the discovery result is read from stdin)\n projects ready --root [--dry-run] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects clone --root [--branch ] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects pull --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects status --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects write-config --root --file --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (contents from stdin)\n skills probe --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--artifact-url --artifact-sha256 ] [--wait] [--timeout ]\n skills deploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n skills undeploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n frogenv status|setup|login|request|sync --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n frogenv run --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n mise inventory|status --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise install --tool --version --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise exec --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n apply --plan-id --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (the plan JSON is read from stdin)\n tailnet status\n tailnet configure --client-id (the client secret is read from stdin)\n tailnet clear\n tailnet devices [--limit ]\n tailnet import --user [--port ]\n proxmox accounts\n proxmox create --name --host [--port ] --token-id (the token secret is read from stdin)\n proxmox delete \n proxmox observe \n proxmox confirm --fingerprint \n proxmox discover \n machines install-node --endpoint --auth agent|identity-file [--identity ] [--artifact-url --artifact-sha256 ] [--controller-url ] [--install-timeout ] [--connect-timeout ] [--wait] [--timeout ]\n\n`status` prefers the node's local socket (default {DEFAULT_SOCKET}); `--url` is the explicit direct-controller override. Other commands talk to the controller, which defaults to {DEFAULT_URL}." ) } diff --git a/crates/providers/fleet-provider-proxmox/Cargo.toml b/crates/providers/fleet-provider-proxmox/Cargo.toml index 8b89fcf..3b5229f 100644 --- a/crates/providers/fleet-provider-proxmox/Cargo.toml +++ b/crates/providers/fleet-provider-proxmox/Cargo.toml @@ -10,12 +10,12 @@ publish.workspace = true async-trait = "0.1.92" fleet-application = { path = "../../fleet-application" } fleet-core = { path = "../../fleet-core" } -reqwest = { version = "0.12.24", default-features = false, features = ["rustls-tls", "json"] } +futures-util = "0.3.31" +reqwest = { version = "0.12.24", default-features = false, features = ["rustls-tls", "json", "stream"] } rustls = { version = "0.23", default-features = false, features = ["ring", "std", "tls12", "logging"] } serde = { version = "1", features = ["derive"] } serde_json = "1" sha2 = "0.10" -tokio = { version = "1", features = ["rt", "macros"] } [dev-dependencies] tokio = { version = "1", features = ["rt", "macros"] } diff --git a/crates/providers/fleet-provider-proxmox/src/lib.rs b/crates/providers/fleet-provider-proxmox/src/lib.rs index 8f7af66..031100a 100644 --- a/crates/providers/fleet-provider-proxmox/src/lib.rs +++ b/crates/providers/fleet-provider-proxmox/src/lib.rs @@ -27,6 +27,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use fleet_core::SensitiveString; +use futures_util::StreamExt as _; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; use rustls::crypto::CryptoProvider; use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; @@ -161,26 +162,17 @@ pub trait PveTransport: fmt::Debug + Send + Sync { pub struct ReqwestPveTransport; impl ReqwestPveTransport { - /// Builds the transport. - /// - /// # Errors - /// - /// Fails when the HTTP client cannot be built. - pub fn new() -> Result { - // A throwaway client proves the TLS feature set resolves in this - // exact dependency graph; each call builds its own client over a - // fresh pinned verifier (the policy is per-call, not per-transport). - reqwest::Client::builder() - .timeout(REQUEST_TIMEOUT) - .build() - .map_err(|error| format!("cannot build the HTTP client: {error}"))?; - Ok(Self) + /// Builds the transport. The pinned verifier is per-call (each call + /// carries its own policy), so there is no client state to keep. + #[must_use] + pub fn new() -> Self { + Self } } impl Default for ReqwestPveTransport { fn default() -> Self { - Self::new().expect("the PVE transport must build") + Self::new() } } @@ -314,7 +306,7 @@ impl PveTransport for ReqwestPveTransport { captured: Arc::clone(&captured), }); let mut config = rustls::ClientConfig::builder_with_provider(provider) - .with_protocol_versions(&[&rustls::version::TLS13]) + .with_protocol_versions(&[&rustls::version::TLS12, &rustls::version::TLS13]) .map_err(|error| PveTransportError::Connect { detail: error.to_string(), })? @@ -331,7 +323,11 @@ impl PveTransport for ReqwestPveTransport { .map_err(|error| PveTransportError::Connect { detail: format!("the TLS configuration was rejected: {error}"), })?; - let url = format!("https://{}:{}{}", request.host, request.port, request.path); + let authority = match request.host.parse::() { + Ok(_) => format!("[{}]", request.host), + Err(_) => request.host.clone(), + }; + let url = format!("https://{authority}:{}{}", request.port, request.path); let response = client .get(&url) .header( @@ -353,32 +349,40 @@ impl PveTransport for ReqwestPveTransport { .expect("the capture lock is not poisoned") .clone(); match (observed, request.pinned_fingerprint.as_deref()) { - (Some(observed), Some(_)) => PveTransportError::FingerprintMismatch { - observed, - pinned: request.pinned_fingerprint.clone(), - }, - (Some(observed), None) => PveTransportError::ObserveRefused { observed }, - (None, _) => PveTransportError::Connect { + // A mismatch is only a mismatch when the fingerprints + // differ: a later TLS failure with a matching pin is a + // connection failure, not an instruction to re-confirm. + (Some(observed), Some(pinned)) + if normalize_fingerprint(&observed) != normalize_fingerprint(pinned) => + { + PveTransportError::FingerprintMismatch { + observed, + pinned: Some(pinned.to_owned()), + } + } + (Some(_), Some(_)) | (None, _) => PveTransportError::Connect { detail: error.to_string(), }, + (Some(observed), None) => PveTransportError::ObserveRefused { observed }, } })?; let status = u16::from(response.status()); - let body = response - .bytes() - .await - .map_err(|error| PveTransportError::Connect { + // The body bound is enforced while streaming: a hostile or broken + // host cannot make Fleet materialize an unbounded response. + let mut stream = response.bytes_stream(); + let mut body: Vec = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|error| PveTransportError::Connect { detail: error.to_string(), })?; - if body.len() > MAX_BODY_BYTES { - return Err(PveTransportError::BodyTooLarge { - limit: MAX_BODY_BYTES, - }); + if body.len() + chunk.len() > MAX_BODY_BYTES { + return Err(PveTransportError::BodyTooLarge { + limit: MAX_BODY_BYTES, + }); + } + body.extend_from_slice(&chunk); } - Ok(PveHttpResponse { - status, - body: body.to_vec(), - }) + Ok(PveHttpResponse { status, body }) } } @@ -611,25 +615,45 @@ fn type_name_of(value: &serde_json::Value) -> &'static str { } } +/// The bound on a resource id. +const MAX_ID_CHARS: usize = 128; +/// The bound on a node or display name. +const MAX_NAME_CHARS: usize = 256; +/// The bound on a status string. +const MAX_STATUS_CHARS: usize = 64; + +/// Reads a bounded string field, refusing overlong values rather than +/// truncating them: a silently altered identity is worse than a warning. +fn bounded_str(entry: &serde_json::Value, key: &str, max: usize) -> Result, String> { + match entry.get(key).and_then(serde_json::Value::as_str) { + Some(value) if value.chars().count() > max => Err(format!( + "the {key} field is {} characters, over the {max}-character bound", + value.chars().count() + )), + Some(value) => Ok(Some(value.to_owned())), + None => Ok(None), + } +} + +/// Reads a number that PVE may deliver as a JSON number or a string. +fn loose_number(entry: &serde_json::Value, key: &str) -> Option { + match entry.get(key) { + Some(serde_json::Value::Number(number)) => number.as_u64(), + Some(serde_json::Value::String(text)) => text.trim().parse().ok(), + _ => None, + } +} + /// Normalizes one cluster-resources entry. `Ok(None)` skips a non-resource /// row without warning; `Err` warns. fn normalize_resource(entry: &serde_json::Value) -> Result, String> { - let id = entry - .get("id") - .and_then(serde_json::Value::as_str) - .ok_or_else(|| "the entry carries no id".to_owned())? - .chars() - .take(128) - .collect::(); + let id = bounded_str(entry, "id", MAX_ID_CHARS)? + .ok_or_else(|| "the entry carries no id".to_owned())?; let pve_type = entry .get("type") .and_then(serde_json::Value::as_str) .ok_or_else(|| format!("entry {id} carries no type"))?; - let is_template = entry - .get("template") - .and_then(serde_json::Value::as_i64) - .unwrap_or(0) - == 1; + let is_template = loose_number(entry, "template").unwrap_or(0) == 1; let kind = match (pve_type, is_template) { ("node", _) => "node", ("qemu", false) => "qemu", @@ -643,23 +667,10 @@ fn normalize_resource(entry: &serde_json::Value) -> Result, )); } }; - let node = entry - .get("node") - .and_then(serde_json::Value::as_str) - .map(|value| value.chars().take(128).collect()); - let vmid = entry.get("vmid").and_then(|value| { - value - .as_u64() - .map(|value| u32::try_from(value).unwrap_or(0)) - }); - let name = entry - .get("name") - .and_then(serde_json::Value::as_str) - .map(|value| value.chars().take(256).collect()); - let status = entry - .get("status") - .and_then(serde_json::Value::as_str) - .map(|value| value.chars().take(64).collect()); + let node = bounded_str(entry, "node", MAX_NAME_CHARS)?; + let vmid = loose_number(entry, "vmid").and_then(|value| u32::try_from(value).ok()); + let name = bounded_str(entry, "name", MAX_NAME_CHARS)?; + let status = bounded_str(entry, "status", MAX_STATUS_CHARS)?; Ok(Some(PveResource { kind: kind.to_owned(), id, diff --git a/crates/providers/fleet-provider-proxmox/tests/pin_live.rs b/crates/providers/fleet-provider-proxmox/tests/pin_live.rs index 4e398f7..3349623 100644 --- a/crates/providers/fleet-provider-proxmox/tests/pin_live.rs +++ b/crates/providers/fleet-provider-proxmox/tests/pin_live.rs @@ -9,17 +9,21 @@ use fleet_provider_proxmox::{ }; use std::sync::Arc; +/// The live configuration, or `None` when the live gate is off. A set gate +/// with missing variables is a misconfiguration: fail loudly rather than +/// silently passing. fn live() -> Option<(String, String, String, String, String)> { if std::env::var("FLEET_PVE_LIVE").ok()?.trim() != "1" { return None; } - Some(( - std::env::var("PROXMOX_HOST").ok()?, - std::env::var("PROXMOX_PORT").unwrap_or_else(|_| "8006".to_owned()), - std::env::var("PROXMOX_TOKEN_ID").ok()?, - std::env::var("PROXMOX_API_KEY").ok()?, - std::env::var("PROXMOX_FINGERPRINT").ok()?, - )) + let host = std::env::var("PROXMOX_HOST").expect("FLEET_PVE_LIVE=1 requires PROXMOX_HOST"); + let port = std::env::var("PROXMOX_PORT").unwrap_or_else(|_| "8006".to_owned()); + let token_id = + std::env::var("PROXMOX_TOKEN_ID").expect("FLEET_PVE_LIVE=1 requires PROXMOX_TOKEN_ID"); + let key = std::env::var("PROXMOX_API_KEY").expect("FLEET_PVE_LIVE=1 requires PROXMOX_API_KEY"); + let fingerprint = std::env::var("PROXMOX_FINGERPRINT") + .expect("FLEET_PVE_LIVE=1 requires PROXMOX_FINGERPRINT"); + Some((host, port, token_id, key, fingerprint)) } #[tokio::test] @@ -27,7 +31,7 @@ async fn pinned_transport_converses_with_the_live_host() { let Some((host, port, token_id, key, fingerprint)) = live() else { return; }; - let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let transport = Arc::new(ReqwestPveTransport::new()); let request = PveHttpRequest { host, port: port.parse().unwrap(), @@ -50,10 +54,10 @@ async fn pinned_transport_converses_with_the_live_host() { #[tokio::test] async fn a_wrong_fingerprint_is_refused_at_the_handshake() { - let Some((host, port, token_id, key, _)) = live() else { + let Some((host, port, token_id, key, fingerprint)) = live() else { return; }; - let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let transport = Arc::new(ReqwestPveTransport::new()); let request = PveHttpRequest { host, port: port.parse().unwrap(), @@ -69,7 +73,12 @@ async fn a_wrong_fingerprint_is_refused_at_the_handshake() { match error { PveTransportError::FingerprintMismatch { observed, pinned } => { assert_eq!(pinned, Some("00".repeat(32))); - assert_eq!(observed.len(), 32 * 2 + 31, "{observed}"); + // The reporter carries the host's actual fingerprint, not just + // a well-formed string. + assert_eq!( + fleet_provider_proxmox::normalize_fingerprint(&observed), + fleet_provider_proxmox::normalize_fingerprint(&fingerprint) + ); } other => panic!("expected a fingerprint mismatch, got {other:?}"), } @@ -80,7 +89,7 @@ async fn an_unpinned_host_is_observed_not_conversed_with() { let Some((host, port, token_id, key, fingerprint)) = live() else { return; }; - let transport = Arc::new(ReqwestPveTransport::new().unwrap()); + let transport = Arc::new(ReqwestPveTransport::new()); let request = PveHttpRequest { host, port: port.parse().unwrap(), diff --git a/packages/api-client/openapi.json b/packages/api-client/openapi.json index 2068ef5..34e2545 100644 --- a/packages/api-client/openapi.json +++ b/packages/api-client/openapi.json @@ -1919,6 +1919,28 @@ "summary": "Lists the configured accounts.", "description": "# Errors\n\nReturns the public error envelope on refusal or backend failure.", "operationId": "listProxmoxAccounts", + "parameters": [ + { + "name": "limit", + "in": "query", + "description": "The maximum number of accounts to return.", + "required": false, + "schema": { + "type": "integer", + "format": "int32", + "minimum": 0 + } + }, + { + "name": "cursor", + "in": "query", + "description": "The opaque cursor: the last account id of the previous page.", + "required": false, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "The configured accounts, newest first.", diff --git a/packages/api-client/src/generated/fleet.ts b/packages/api-client/src/generated/fleet.ts index 30841f0..1d5125d 100644 --- a/packages/api-client/src/generated/fleet.ts +++ b/packages/api-client/src/generated/fleet.ts @@ -2309,6 +2309,18 @@ cursor?: string; limit?: number; }; +export type ListProxmoxAccountsParams = { +/** + * The maximum number of accounts to return. + * @minimum 0 + */ +limit?: number; +/** + * The opaque cursor: the last account id of the previous page. + */ +cursor?: string; +}; + export type ListTailnetDevicesParams = { /** * The maximum number of devices to return. @@ -4368,12 +4380,19 @@ export type listProxmoxAccountsResponseError = (listProxmoxAccountsResponse403) export type listProxmoxAccountsResponse = (listProxmoxAccountsResponseSuccess | listProxmoxAccountsResponseError) -export const getListProxmoxAccountsUrl = () => { +export const getListProxmoxAccountsUrl = (params?: ListProxmoxAccountsParams,) => { + const normalizedParams = new URLSearchParams(); + Object.entries(params || {}).forEach(([key, value]) => { + if (value !== undefined) { + normalizedParams.append(key, value === null ? 'null' : String(value)) + } + }); + const stringifiedParams = normalizedParams.toString(); - return `/api/v1/proxmox/accounts` + return stringifiedParams.length > 0 ? `/api/v1/proxmox/accounts?${stringifiedParams}` : `/api/v1/proxmox/accounts` } /** @@ -4382,9 +4401,9 @@ export const getListProxmoxAccountsUrl = () => { * Returns the public error envelope on refusal or backend failure. * @summary Lists the configured accounts. */ -export const listProxmoxAccounts = async ( options?: RequestInit): Promise => { +export const listProxmoxAccounts = async (params?: ListProxmoxAccountsParams, options?: RequestInit): Promise => { - const res = await fetch(getListProxmoxAccountsUrl(), + const res = await fetch(getListProxmoxAccountsUrl(params), { ...options, method: 'GET' From 2575b872b99969292b3e5c375a333e95a6727477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= Date: Sun, 20 Sep 2026 16:28:57 +0000 Subject: [PATCH 3/3] =?UTF-8?q?FM-600:=20second=20review=20round=20?= =?UTF-8?q?=E2=80=94=20completion=20audit=20events,=20shared=20page=20boun?= =?UTF-8?q?ds,=20migration=20split,=20regression=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/fleet-api/src/proxmox.rs | 10 +- crates/fleet-application/src/proxmox.rs | 19 +++- .../migrations/0017_proxmox_accounts.sql | 3 - .../0018_proxmox_observed_fingerprint.sql | 3 + .../fleet-provider-proxmox/src/lib.rs | 107 ++++++++++++++++-- 5 files changed, 120 insertions(+), 22 deletions(-) create mode 100644 crates/fleet-storage-sqlite/migrations/0018_proxmox_observed_fingerprint.sql diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs index 8d9e0a1..666e4a0 100644 --- a/crates/fleet-api/src/proxmox.rs +++ b/crates/fleet-api/src/proxmox.rs @@ -20,7 +20,7 @@ use serde::{Deserialize, Serialize}; use std::str::FromStr as _; use utoipa::ToSchema; -use crate::envelope::{Page, PageInfo, Resource}; +use crate::envelope::{DEFAULT_PAGE_LIMIT, MAX_PAGE_LIMIT, Page, PageInfo, Resource}; use crate::error::{ApiError, ApiErrorResponse}; /// Extracts the Proxmox use cases from the API state, or answers with the @@ -218,10 +218,6 @@ pub struct ListProxmoxAccountsParams { pub cursor: Option, } -/// The default and maximum page bounds, matching the machine read model. -const DEFAULT_PAGE_LIMIT: u32 = 50; -const MAX_PAGE_LIMIT: u32 = 200; - /// Lists the configured accounts. /// /// # Errors @@ -265,8 +261,8 @@ pub async fn list_proxmox_accounts( ) -> Result>, ApiErrorResponse> { let proxmox = proxmox_or_error(&state, correlation_id)?; let principal = crate::operations::principal_or_error(principal, correlation_id)?; - // A zero or absent limit means the default; the page never advertises - // more than it returns. + // A zero or absent limit means the default; the reported limit is the + // clamp applied to the page, matching the sibling list endpoints. let limit = params .limit .filter(|limit| *limit > 0) diff --git a/crates/fleet-application/src/proxmox.rs b/crates/fleet-application/src/proxmox.rs index 9059556..bc006ac 100644 --- a/crates/fleet-application/src/proxmox.rs +++ b/crates/fleet-application/src/proxmox.rs @@ -640,6 +640,14 @@ impl ProxmoxAccounts { context: "accounts", detail, })?; + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(account_id), + "proxmox_account_deleted", + Some(("name", account.name.as_str())), + ) + .await?; Ok(()) } @@ -732,7 +740,8 @@ impl ProxmoxAccounts { }); } // The audit intent lands BEFORE the mutation, per the two-phase - // audit rule. + // audit rule; the completion event follows success, so an intent + // without its completion is itself evidence of an aborted flow. self.audit_event( principal, Permission::ProxmoxConfig, @@ -749,6 +758,14 @@ impl ProxmoxAccounts { context: "accounts", detail, })?; + self.audit_event( + principal, + Permission::ProxmoxConfig, + Some(account_id), + "proxmox_fingerprint_confirmed", + Some(("fingerprint", account.fingerprint.as_deref().unwrap_or(""))), + ) + .await?; Ok(account) } diff --git a/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql index 5d89d00..fef5ab0 100644 --- a/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql +++ b/crates/fleet-storage-sqlite/migrations/0017_proxmox_accounts.sql @@ -2,8 +2,6 @@ -- state. The token secret never lives here: accounts reference Fleet's -- encrypted secret store by account id. The fingerprint is the pinned -- SHA-256 of the host certificate, empty until the trust step confirms it. --- observed_fingerprint is the probe's last capture; confirm must match it, --- so trust always flows through the observe step. CREATE TABLE proxmox_accounts ( id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE, @@ -11,7 +9,6 @@ CREATE TABLE proxmox_accounts ( port INTEGER NOT NULL, token_id TEXT NOT NULL, fingerprint TEXT NOT NULL DEFAULT '', - observed_fingerprint TEXT NOT NULL DEFAULT '', created_at INTEGER NOT NULL ) STRICT; diff --git a/crates/fleet-storage-sqlite/migrations/0018_proxmox_observed_fingerprint.sql b/crates/fleet-storage-sqlite/migrations/0018_proxmox_observed_fingerprint.sql new file mode 100644 index 0000000..79c28b2 --- /dev/null +++ b/crates/fleet-storage-sqlite/migrations/0018_proxmox_observed_fingerprint.sql @@ -0,0 +1,3 @@ +-- FM-600 review round: the trust probe's last capture. confirm must match +-- this, so trust always flows through the observe step. +ALTER TABLE proxmox_accounts ADD COLUMN observed_fingerprint TEXT NOT NULL DEFAULT ''; diff --git a/crates/providers/fleet-provider-proxmox/src/lib.rs b/crates/providers/fleet-provider-proxmox/src/lib.rs index 031100a..d730528 100644 --- a/crates/providers/fleet-provider-proxmox/src/lib.rs +++ b/crates/providers/fleet-provider-proxmox/src/lib.rs @@ -60,6 +60,7 @@ pub struct PveCredentials { pub struct PveHttpRequest { /// The host (IP or DNS name) without scheme or port. pub host: String, + /// The port; [`DEFAULT_PORT`] in the common case. pub port: u16, /// The URL path under `/api2/json`, starting with `/`. @@ -71,6 +72,17 @@ pub struct PveHttpRequest { pub credentials: Arc, } +impl PveHttpRequest { + /// The URL authority: IPv6 literals bracketed, everything else bare. + #[must_use] + pub fn authority(&self) -> String { + match self.host.parse::() { + Ok(_) => format!("[{}]", self.host), + Err(_) => self.host.clone(), + } + } +} + /// A transport response: status and bounded body. #[derive(Clone, Debug)] pub struct PveHttpResponse { @@ -323,11 +335,12 @@ impl PveTransport for ReqwestPveTransport { .map_err(|error| PveTransportError::Connect { detail: format!("the TLS configuration was rejected: {error}"), })?; - let authority = match request.host.parse::() { - Ok(_) => format!("[{}]", request.host), - Err(_) => request.host.clone(), - }; - let url = format!("https://{authority}:{}{}", request.port, request.path); + let url = format!( + "https://{}:{}{}", + request.authority(), + request.port, + request.path + ); let response = client .get(&url) .header( @@ -375,12 +388,7 @@ impl PveTransport for ReqwestPveTransport { let chunk = chunk.map_err(|error| PveTransportError::Connect { detail: error.to_string(), })?; - if body.len() + chunk.len() > MAX_BODY_BYTES { - return Err(PveTransportError::BodyTooLarge { - limit: MAX_BODY_BYTES, - }); - } - body.extend_from_slice(&chunk); + push_bounded(&mut body, &chunk)?; } Ok(PveHttpResponse { status, body }) } @@ -536,6 +544,18 @@ impl ProxmoxClient { } } +/// Appends one streamed chunk under the body bound, refusing the response +/// the moment it would exceed it. +fn push_bounded(body: &mut Vec, chunk: &[u8]) -> Result<(), PveTransportError> { + if body.len() + chunk.len() > MAX_BODY_BYTES { + return Err(PveTransportError::BodyTooLarge { + limit: MAX_BODY_BYTES, + }); + } + body.extend_from_slice(chunk); + Ok(()) +} + /// A bounded, credential-free body excerpt for error details. fn bounded_body(body: &[u8]) -> String { let text = String::from_utf8_lossy(body); @@ -727,6 +747,71 @@ mod tests { assert!(error.contains("unrecognized type"), "{error}"); } + #[test] + fn authorities_bracket_ipv6_literals() { + let request = |host: &str| PveHttpRequest { + host: host.to_owned(), + port: 8006, + path: "/".to_owned(), + pinned_fingerprint: None, + credentials: Arc::new(PveCredentials { + token_id: "t".to_owned(), + token: SensitiveString::new("s"), + }), + }; + assert_eq!(request("2001:db8::1").authority(), "[2001:db8::1]"); + assert_eq!(request("192.168.68.223").authority(), "192.168.68.223"); + assert_eq!(request("pve.localdomain").authority(), "pve.localdomain"); + } + + #[test] + fn the_body_bound_refuses_mid_stream() { + let mut body = Vec::new(); + let chunk = vec![0u8; MAX_BODY_BYTES]; + push_bounded(&mut body, &chunk).unwrap(); + let error = push_bounded(&mut body, &[0u8; 1]).unwrap_err(); + assert!(matches!(error, PveTransportError::BodyTooLarge { .. })); + } + + #[test] + fn overlong_fields_are_rejected_not_truncated() { + let long_id = "x".repeat(MAX_ID_CHARS + 1); + let entry = serde_json::json!({"id": long_id, "type": "node"}); + let error = normalize_resource(&entry).unwrap_err(); + assert!(error.contains("over the"), "{error}"); + + let long_name = "y".repeat(MAX_NAME_CHARS + 1); + let entry = serde_json::json!({ + "id": "qemu/1", "type": "qemu", "vmid": 1, "name": long_name + }); + let error = normalize_resource(&entry).unwrap_err(); + assert!(error.contains("over the"), "{error}"); + } + + #[test] + fn stringly_numbers_are_tolerated() { + let entry = serde_json::json!({ + "id": "qemu/7", "type": "qemu", "vmid": "7", "template": "1" + }); + let resource = normalize_resource(&entry).unwrap().unwrap(); + assert_eq!(resource.kind, "qemu-template"); + assert_eq!(resource.vmid, Some(7)); + + let entry = serde_json::json!({ + "id": "qemu/8", "type": "qemu", "vmid": 8, "template": 0 + }); + let resource = normalize_resource(&entry).unwrap().unwrap(); + assert_eq!(resource.kind, "qemu"); + assert_eq!(resource.vmid, Some(8)); + + // An out-of-range vmid is absent, never coerced to zero. + let entry = serde_json::json!({ + "id": "qemu/9", "type": "qemu", "vmid": 4_294_967_296_i64 + }); + let resource = normalize_resource(&entry).unwrap().unwrap(); + assert_eq!(resource.vmid, None); + } + #[test] fn null_and_missing_envelopes_mean_empty() { let body = serde_json::json!({"data": null});