From 1ac7bec4e72958685f11656dde9e43ee160a35f2 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:26:00 +0200 Subject: [PATCH 01/10] Add a provisional portfolio module to the API client List the client tenants of a portfolio credential and read one of them on data calls. The selection travels as one request header and the client refuses any answer that does not confirm the same tenant. The module is hand-written against a draft contract and isolated in portfolio.rs; the generated client is untouched. Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- crates/hodeishield-api/src/client.rs | 48 ++++-- crates/hodeishield-api/src/error.rs | 15 ++ crates/hodeishield-api/src/lib.rs | 1 + crates/hodeishield-api/src/portfolio.rs | 213 ++++++++++++++++++++++++ crates/hodeishield-api/tests/api.rs | 118 +++++++++++++ 5 files changed, 385 insertions(+), 10 deletions(-) create mode 100644 crates/hodeishield-api/src/portfolio.rs diff --git a/crates/hodeishield-api/src/client.rs b/crates/hodeishield-api/src/client.rs index 7ea302a..c4565a6 100644 --- a/crates/hodeishield-api/src/client.rs +++ b/crates/hodeishield-api/src/client.rs @@ -134,12 +134,7 @@ fn tenant_header(headers: &HeaderMap) -> Option { if values.next().is_some() { return None; } - let canonical = value.len() == 36 - && value.bytes().enumerate().all(|(i, b)| match i { - 8 | 13 | 18 | 23 => b == b'-', - _ => b.is_ascii_hexdigit(), - }); - canonical.then(|| value.to_ascii_lowercase()) + crate::portfolio::is_uuid(value).then(|| value.to_ascii_lowercase()) } /// What the client reports about each request it sends, for `--verbose`. It carries no header and @@ -175,6 +170,15 @@ pub struct RetryEvent<'a> { type Observer = Arc) + Send + Sync>; type RetryObserver = Arc) + Send + Sync>; type Renew = Arc Option + Send + Sync>; +type AnswerCheck = Arc Result<(), Error> + Send + Sync>; + +/// Extra headers a client sends on data requests, and the check its successful answers must pass. +/// The portfolio module fills it in; this file knows nothing about what the headers mean. +#[derive(Clone, Default)] +pub(crate) struct Scope { + pub(crate) headers: HeaderMap, + pub(crate) check: Option, +} /// Builds a [`Client`]. pub struct ClientBuilder { @@ -189,6 +193,7 @@ pub struct ClientBuilder { observer: Option, retry_observer: Option, renew: Option, + pub(crate) scope: Scope, } impl fmt::Debug for ClientBuilder { @@ -308,6 +313,7 @@ impl ClientBuilder { observer: self.observer, retry_observer: self.retry_observer, renew: self.renew, + scope: self.scope, }) } } @@ -325,6 +331,7 @@ pub struct Client { observer: Option, retry_observer: Option, renew: Option, + scope: Scope, } impl fmt::Debug for Client { @@ -361,6 +368,7 @@ impl Client { observer: None, retry_observer: None, renew: None, + scope: Scope::default(), } } @@ -414,6 +422,18 @@ impl Client { operation: &'static Operation, path: String, query: Vec<(&'static str, String)>, + ) -> Result, Error> { + self.get_with(operation, path, query, true) + } + + /// Like [`Client::get`]; with `scoped` false the client's scope headers are neither sent nor + /// checked, for an operation that sits outside the scope. + pub(crate) fn get_with( + &self, + operation: &'static Operation, + path: String, + query: Vec<(&'static str, String)>, + scoped: bool, ) -> Result, Error> { let url = self.url(&path, &query); let mut rate_limit_retries = 0; @@ -421,12 +441,14 @@ impl Client { let mut renewed = false; loop { let started = Instant::now(); - let response = match self + let mut request = self .http .get(url.clone()) - .header(AUTHORIZATION, self.authorization()?) - .send() - { + .header(AUTHORIZATION, self.authorization()?); + if scoped { + request = request.headers(self.scope.headers.clone()); + } + let response = match request.send() { Ok(response) => response, Err(e) if (e.is_connect() || e.is_timeout()) @@ -455,8 +477,14 @@ impl Client { elapsed: started.elapsed(), }); } + // A scoped answer that does not carry the scope that was asked for is never decoded. + let verdict = match (&self.scope.check, scoped, status.is_success()) { + (Some(check), true, true) => check(response.headers()), + _ => Ok(()), + }; let body = read_body(response, MAX_RESPONSE_BYTES).map_err(Error::Body)?; if status.is_success() { + verdict?; let raw: serde_json::Value = serde_json::from_slice(&body).map_err(|source| Error::Decode { operation: operation.id, diff --git a/crates/hodeishield-api/src/error.rs b/crates/hodeishield-api/src/error.rs index 02523e6..6f3569f 100644 --- a/crates/hodeishield-api/src/error.rs +++ b/crates/hodeishield-api/src/error.rs @@ -86,6 +86,21 @@ pub enum Error { /// An id or slug that would change which resource the path addresses. #[error("{0:?} is not a valid identifier")] InvalidPathArgument(String), + /// A client tenant identifier that is not a UUID. + #[error("{0:?} is not a valid client tenant identifier")] + InvalidTenant(String), + /// The API answered for another client tenant than the one asked for, or did not say which. + #[error( + "the API did not confirm the client tenant that was asked for (asked for {requested}, \ + answer says {})", + answered.as_deref().unwrap_or("nothing") + )] + TenantMismatch { + /// The tenant the request named. + requested: String, + /// What the answer's header said, when it said anything. + answered: Option, + }, } impl Error { diff --git a/crates/hodeishield-api/src/lib.rs b/crates/hodeishield-api/src/lib.rs index b1b39b5..8c46e27 100644 --- a/crates/hodeishield-api/src/lib.rs +++ b/crates/hodeishield-api/src/lib.rs @@ -24,6 +24,7 @@ mod client; mod error; +pub mod portfolio; #[allow(missing_docs)] #[path = "generated.rs"] pub mod v1; diff --git a/crates/hodeishield-api/src/portfolio.rs b/crates/hodeishield-api/src/portfolio.rs new file mode 100644 index 0000000..690027b --- /dev/null +++ b/crates/hodeishield-api/src/portfolio.rs @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Hodeitek S.L. + +//! Client tenants of a portfolio credential: list them, and read one of them. +//! +//! # Provisional +//! +//! **This module is written by hand against a draft of the `/v1` contract and is not part of the +//! published API description yet.** Nothing in `openapi/v1.json` or the generated client describes +//! it. When the contract is published, this file is the only place that changes: the operation, the +//! shape of [`Tenant`] and the way the selected tenant travels on a request are all decided here, +//! and callers see only the interface below. +//! +//! The draft assumes: +//! +//! - `GET /v1/portfolio/tenants`, scope `portfolio:read`, paginated like the other lists +//! (`page`, `per_page`, answer `{ data, pagination }`); +//! - the client is selected on every data operation with one `X-HS-Tenant` request header holding +//! the tenant's UUID, and every answer to such a request repeats it in an `X-HS-Tenant` response +//! header; +//! - the listing does not take that header. +//! +//! # Interface +//! +//! - [`Client::list_tenants`] lists the client tenants the credential may read. +//! - [`ClientBuilder::tenant`] makes every data call of the built client read that tenant. The +//! client then refuses any successful answer that does not name the same tenant, so data of one +//! client is never returned as if it were another's. + +use crate::client::{Operation, Scope}; +use crate::error::Error; +use crate::v1::Pagination; +use crate::{ApiResponse, Client, ClientBuilder}; +use reqwest::header::{HeaderMap, HeaderName, HeaderValue}; +use serde::{Deserialize, Serialize}; +use std::sync::Arc; + +/// Request and response header that selects, and confirms, the client tenant. +const TENANT_HEADER: HeaderName = HeaderName::from_static("x-hs-tenant"); + +/// `GET /v1/portfolio/tenants`: List the client tenants of the credential's portfolio. +pub const LIST_TENANTS: Operation = Operation { + id: "listPortfolioTenants", + method: "GET", + path: "/v1/portfolio/tenants", + summary: "List the client tenants of the credential's portfolio", + scopes: &["portfolio:read"], + known_values: &[], +}; + +/// A client tenant of the portfolio. Text fields come from the server and are untrusted. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct Tenant { + /// Stable identifier (a UUID): what selects the tenant. + pub id: String, + /// Display name. + pub name: String, + /// Short unique name, for people and scripts. + pub slug: String, + /// The client organisation the tenant belongs to, when it has one. + #[serde(default)] + pub organization: Option, + /// Whether this is the MSSP's own tenant rather than a client's. + #[serde(default)] + pub internal: bool, + /// Whether the tenant is active. + #[serde(default = "yes")] + pub active: bool, + /// `in_force`, `grace` or `inactive`. The vocabulary may grow. + #[serde(default)] + pub licence_status: Option, +} + +fn yes() -> bool { + true +} + +/// The organisation a [`Tenant`] belongs to. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct TenantOrganization { + /// Identifier of the organisation. + pub id: String, + /// Display name. + pub name: String, +} + +/// The answer of [`Client::list_tenants`]. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ListTenantsResponse { + /// One page of tenants. + pub data: Vec, + /// Where this page sits in the whole list. + pub pagination: Pagination, +} + +/// Paging of [`Client::list_tenants`]. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ListTenantsParams { + /// 1-based page number. + pub page: Option, + /// Items per page, 1..200. + pub per_page: Option, +} + +impl ListTenantsParams { + fn query_pairs(&self) -> Vec<(&'static str, String)> { + let mut pairs = Vec::new(); + if let Some(page) = self.page { + pairs.push(("page", page.to_string())); + } + if let Some(per_page) = self.per_page { + pairs.push(("per_page", per_page.to_string())); + } + pairs + } +} + +impl Client { + /// The client tenants this credential may read. The listing is never scoped to a tenant, even + /// on a client built with [`ClientBuilder::tenant`]. + /// + /// Required scope: `portfolio:read`. + /// + /// # Errors + /// + /// [`Error`] when the request cannot be sent, the API answers with an error (a credential + /// without portfolio scope gets `403`), or the body does not match the draft. + pub fn list_tenants( + &self, + params: &ListTenantsParams, + ) -> Result, Error> { + self.get_with( + &LIST_TENANTS, + LIST_TENANTS.path.to_owned(), + params.query_pairs(), + false, + ) + } +} + +impl ClientBuilder { + /// Makes every data call of the built client read the client tenant `id`. + /// + /// The built client sends the selection on each request and returns an answer only when it + /// confirms the same tenant; otherwise the call fails with [`Error::TenantMismatch`]. + /// + /// # Errors + /// + /// [`Error::InvalidTenant`] when `id` is not a UUID. + pub fn tenant(mut self, id: &str) -> Result { + if !is_uuid(id) { + return Err(Error::InvalidTenant(id.to_owned())); + } + let wanted = id.to_ascii_lowercase(); + let mut headers = HeaderMap::new(); + headers.insert( + TENANT_HEADER, + HeaderValue::from_str(&wanted).map_err(|_| Error::InvalidTenant(id.to_owned()))?, + ); + let check = move |answer: &HeaderMap| { + let answered = answer + .get(TENANT_HEADER) + .and_then(|v| v.to_str().ok()) + .map(|v| v.trim().to_ascii_lowercase()); + if answered.as_deref() == Some(wanted.as_str()) { + Ok(()) + } else { + Err(Error::TenantMismatch { + requested: wanted.clone(), + answered, + }) + } + }; + self.scope = Scope { + headers, + check: Some(Arc::new(check)), + }; + Ok(self) + } +} + +/// Whether `text` is a UUID in the 8-4-4-4-12 form, in any case. The one check for tenant ids: the +/// tenant selection and the `X-HS-Tenant` metadata both use it. +#[must_use] +pub fn is_uuid(text: &str) -> bool { + let groups: Vec<&str> = text.split('-').collect(); + groups.len() == 5 + && groups + .iter() + .zip([8, 4, 4, 4, 12]) + .all(|(g, len)| g.len() == len && g.bytes().all(|b| b.is_ascii_hexdigit())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn recognises_uuids_only() { + assert!(is_uuid("0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6e")); + assert!(is_uuid("0B8F2A52-6A0E-4D5C-9D3E-1F2A3B4C5D6E")); + for bad in [ + "", + "acme", + "0b8f2a52-6a0e-4d5c-9d3e", + "0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6", + "0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6g", + "a, b", + ] { + assert!(!is_uuid(bad), "{bad:?}"); + } + } +} diff --git a/crates/hodeishield-api/tests/api.rs b/crates/hodeishield-api/tests/api.rs index 0bcd2ea..bb9bd8b 100644 --- a/crates/hodeishield-api/tests/api.rs +++ b/crates/hodeishield-api/tests/api.rs @@ -662,3 +662,121 @@ fn retry_observer_receives_expected_events() { unavailable.assert(); success.assert(); } + +// The portfolio module is provisional: these tests pin its interface, not the published contract. + +const TENANT_A: &str = "11111111-1111-4111-8111-111111111111"; +const TENANT_B: &str = "22222222-2222-4222-8222-222222222222"; +const EMPTY_PAGE: &str = + r#"{"data":[],"pagination":{"page":1,"per_page":50,"total":0,"total_pages":0}}"#; + +fn tenant_client(server: &mockito::Server, tenant: &str) -> Client { + Client::builder( + server.url().parse().expect("url"), + SecretString::from("hsp_example"), + ) + .tenant(tenant) + .expect("a UUID") + .build() + .expect("client") +} + +#[test] +fn list_tenants_decodes_the_listing_and_never_sends_a_tenant() { + use hodeishield_api::portfolio::ListTenantsParams; + let mut server = mockito::Server::new(); + let mock = server + .mock("GET", "/v1/portfolio/tenants") + .match_header("authorization", "Bearer hsp_example") + .match_header("x-hs-tenant", mockito::Matcher::Missing) + .with_body(format!( + r#"{{"data":[{{"id":"{TENANT_A}","name":"Acme","slug":"acme","organization":{{"id":"o1","name":"Acme Group"}},"internal":false,"active":true,"licence_status":"in_force","later":1}}],"pagination":{{"page":1,"per_page":50,"total":1,"total_pages":1}}}}"# + )) + .create(); + // Even a client built for one tenant lists the whole portfolio. + let response = tenant_client(&server, TENANT_B) + .list_tenants(&ListTenantsParams::default()) + .expect("listing"); + mock.assert(); + assert_eq!(response.data.data[0].slug, "acme"); + assert_eq!( + response.data.data[0].licence_status.as_deref(), + Some("in_force") + ); + assert_eq!(response.raw["data"][0]["later"], 1); +} + +#[test] +fn a_selected_tenant_travels_on_data_calls_and_must_be_confirmed() { + let mut server = mockito::Server::new(); + let ok = server + .mock("GET", "/v1/vendors") + .match_header("x-hs-tenant", TENANT_A) + .with_header("x-hs-tenant", &TENANT_A.to_ascii_uppercase()) + .with_body(EMPTY_PAGE) + .create(); + tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect("confirmed answer"); + ok.assert(); +} + +#[test] +fn an_answer_for_another_tenant_or_for_none_is_refused() { + let mut server = mockito::Server::new(); + let other = server + .mock("GET", "/v1/vendors") + .with_header("x-hs-tenant", TENANT_B) + .with_body(EMPTY_PAGE) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("another tenant"); + assert!( + matches!(&err, Error::TenantMismatch { answered: Some(a), .. } if a == TENANT_B), + "{err:?}" + ); + other.remove(); + server + .mock("GET", "/v1/vendors") + .with_body(EMPTY_PAGE) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("no confirmation"); + assert!( + matches!(err, Error::TenantMismatch { answered: None, .. }), + "{err:?}" + ); +} + +#[test] +fn a_client_without_a_tenant_sends_no_selection_and_checks_nothing() { + let mut server = mockito::Server::new(); + let mock = server + .mock("GET", "/v1/vendors") + .match_header("x-hs-tenant", mockito::Matcher::Missing) + .with_body(EMPTY_PAGE) + .create(); + client(&server) + .list_vendors(&ListVendorsParams::default()) + .expect("plain call"); + mock.assert(); +} + +#[test] +fn a_tenant_that_is_not_a_uuid_is_refused_before_any_request() { + for bad in [ + "acme", + "", + "11111111-1111-4111-8111-111111111111, 22222222-2222-4222-8222-222222222222", + ] { + let err = Client::builder( + "https://api.example.test".parse().expect("url"), + SecretString::from("k"), + ) + .tenant(bad) + .expect_err("not a UUID"); + assert!(matches!(err, Error::InvalidTenant(_)), "{bad:?}"); + } +} From 58686e3e3e57a13d699592ea568758fb82d7af20 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:30:54 +0200 Subject: [PATCH 02/10] Read a portfolio's client tenants from the CLI Add 'tenants list' and the global --tenant and --all-tenants selection on read commands. A list across tenants labels every row, in the table and in --json and --csv, with the tenant whose own request returned it. A client outside the portfolio is reported as not found before any data is read, and a credential without portfolio scope behaves as before. Closes #54 Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- crates/hodeishield-cli/src/cli.rs | 94 ++++ crates/hodeishield-cli/src/commands/mod.rs | 29 +- .../hodeishield-cli/src/commands/resources.rs | 235 +++++++-- .../hodeishield-cli/src/commands/tenants.rs | 217 +++++++++ crates/hodeishield-cli/src/failure.rs | 27 +- crates/hodeishield-cli/src/output.rs | 64 +++ crates/hodeishield-cli/tests/cli.rs | 451 ++++++++++++++++++ 7 files changed, 1079 insertions(+), 38 deletions(-) create mode 100644 crates/hodeishield-cli/src/commands/tenants.rs diff --git a/crates/hodeishield-cli/src/cli.rs b/crates/hodeishield-cli/src/cli.rs index 2f17fbb..6e2d66f 100644 --- a/crates/hodeishield-cli/src/cli.rs +++ b/crates/hodeishield-cli/src/cli.rs @@ -19,6 +19,11 @@ Credentials: HODEISHIELD_API_KEY A tenant API key. When set, it is used for every API call. hodeishield login Otherwise, sign in to the app; the token is kept in the system keychain. +Several client tenants (portfolio credential): + hodeishield tenants list The client tenants you may read. + hodeishield --tenant vendors list One client. + hodeishield --all-tenants vendors list Every client, with a tenant column. + Exit codes: 0 ok · 1 error · 2 usage · 3 not authenticated · 4 missing scope · 5 not found 6 rate limited · 7 API unreachable or failing · 8 tenant licence not in force"; @@ -58,6 +63,27 @@ pub struct Cli { #[arg(short, long, global = true, help_heading = "Global options")] pub verbose: bool, + /// Read this client tenant (its slug or id) instead of the credential's own. Needs a + /// portfolio credential; see `hodeishield tenants list`. + #[arg( + long, + global = true, + help_heading = "Client selection (MSSP)", + value_name = "SLUG|ID", + conflicts_with = "all_tenants" + )] + pub tenant: Option, + + /// On a list command, read every client tenant of the portfolio and add a tenant column (or + /// `tenant_*` fields) to each row. Fetches every page of each tenant. + #[arg( + long, + global = true, + help_heading = "Client selection (MSSP)", + conflicts_with = "tenant" + )] + pub all_tenants: bool, + #[command(subcommand)] pub command: Command, } @@ -82,6 +108,9 @@ pub enum Command { /// Enrolled endpoint agents. #[command(subcommand)] Endpoints(EndpointsCommand), + /// Client tenants a portfolio (MSSP) credential may read. + #[command(subcommand)] + Tenants(TenantsCommand), /// Sign in to the app (browser, or --device without one). #[command(long_about = LOGIN_ABOUT)] Login(LoginArgs), @@ -134,8 +163,47 @@ impl Cli { ) .exit(); } + if let Some(message) = cli.selection_error() { + Self::command() + .error(clap::error::ErrorKind::ArgumentConflict, message) + .exit(); + } cli } + + /// Why the client selection cannot go with this command, if it cannot. It is refused rather + /// than ignored: a command that does not read the selected client must not look as if it did. + fn selection_error(&self) -> Option<&'static str> { + if self.tenant.is_some() && self.all_tenants { + return Some("the argument '--tenant' cannot be used with '--all-tenants'"); + } + if self.tenant.is_none() && !self.all_tenants { + return None; + } + if !self.command.reads_tenant_data() { + return Some( + "'--tenant' and '--all-tenants' only apply to the commands that read tenant data", + ); + } + if self.all_tenants { + match self.command.paging() { + None => { + return Some( + "'--all-tenants' applies to list commands; use '--tenant' to read one \ + client", + ); + } + Some(paging) if paging.page.is_some() => { + return Some( + "the argument '--all-tenants' cannot be used with '--page': it fetches \ + every page of each tenant", + ); + } + Some(_) => {} + } + } + None + } } impl Cli { @@ -150,6 +218,19 @@ impl Cli { } impl Command { + /// The commands that read a tenant's data, and so can read a selected client's. + fn reads_tenant_data(&self) -> bool { + matches!( + self, + Self::Vendors(_) + | Self::Alerts(_) + | Self::Risks(_) + | Self::Compliance(_) + | Self::Evidence(_) + | Self::Endpoints(_) + ) + } + /// The paging options, for the commands that list. fn paging(&self) -> Option<&Paging> { match self { @@ -159,6 +240,7 @@ impl Command { Self::Compliance(ComplianceCommand::Controls(list)) => Some(&list.paging), Self::Evidence(EvidenceCommand::List(list)) => Some(&list.paging), Self::Endpoints(EndpointsCommand::List(list)) => Some(&list.paging), + Self::Tenants(TenantsCommand::List(list)) => Some(&list.paging), _ => None, } } @@ -192,6 +274,18 @@ where PossibleValuesParser::new(values).try_map(|value| T::from_str(&value)) } +#[derive(Debug, Subcommand)] +pub enum TenantsCommand { + /// List the client tenants this credential may read. + List(TenantsList), +} + +#[derive(Debug, Args)] +pub struct TenantsList { + #[command(flatten)] + pub paging: Paging, +} + #[derive(Debug, Subcommand)] pub enum VendorsCommand { /// List vendors. diff --git a/crates/hodeishield-cli/src/commands/mod.rs b/crates/hodeishield-cli/src/commands/mod.rs index ad117b9..4339ecd 100644 --- a/crates/hodeishield-cli/src/commands/mod.rs +++ b/crates/hodeishield-cli/src/commands/mod.rs @@ -6,6 +6,7 @@ mod config; mod resources; mod session; +mod tenants; use crate::auth::{self, TokenStore}; use crate::cli::{Cli, Command}; @@ -14,11 +15,13 @@ use crate::failure::{ApiContext, CredentialSource, Result, from_api}; use hodeishield_api::Client; use std::io::Write; use std::sync::Arc; +use tenants::Selection; pub struct Context { pub overrides: Overrides, pub json: bool, pub verbose: bool, + pub selection: Selection, pub store: Arc, } @@ -27,8 +30,13 @@ impl Context { crate::config::resolve(&self.overrides) } - /// A `/v1` client with the credential in effect. - pub fn api(&self, settings: &Settings) -> Result<(Client, CredentialSource)> { + /// A `/v1` client with the credential in effect, reading `tenant` (a client tenant's id) when + /// one is given and the credential's own tenant otherwise. + pub fn api( + &self, + settings: &Settings, + tenant: Option<&str>, + ) -> Result<(Client, CredentialSource)> { let (credential, source) = auth::credential_for_api(settings, self.store.as_ref())?; let mut builder = Client::builder(settings.api_url.clone(), credential).user_agent(crate::USER_AGENT); @@ -36,6 +44,17 @@ impl Context { let (settings, store) = (settings.clone(), Arc::clone(&self.store)); builder = builder.renew_credential(move || auth::renew(&settings, store.as_ref())); } + if let Some(tenant) = tenant { + builder = builder.tenant(tenant).map_err(|e| { + from_api( + e, + &ApiContext { + source, + framework: None, + }, + ) + })?; + } if self.verbose { builder = builder.observer(|event| { eprintln!( @@ -80,6 +99,11 @@ pub fn run(cli: Cli, out: &mut dyn Write) -> Result<()> { }, json: cli.json, verbose: cli.verbose, + selection: match (cli.tenant, cli.all_tenants) { + (Some(tenant), _) => Selection::Tenant(tenant), + (None, true) => Selection::All, + (None, false) => Selection::Own, + }, store: Arc::new(auth::store::Keychain), }; match cli.command { @@ -89,6 +113,7 @@ pub fn run(cli: Cli, out: &mut dyn Write) -> Result<()> { Command::Compliance(command) => resources::compliance(&ctx, command, out), Command::Evidence(command) => resources::evidence(&ctx, command, out), Command::Endpoints(command) => resources::endpoints(&ctx, command, out), + Command::Tenants(command) => tenants::run(&ctx, command, out), Command::Login(args) => session::login(&ctx, &args), Command::Logout => session::logout(&ctx), Command::Whoami => session::whoami(&ctx, out), diff --git a/crates/hodeishield-cli/src/commands/resources.rs b/crates/hodeishield-cli/src/commands/resources.rs index 0ecfe99..6170241 100644 --- a/crates/hodeishield-cli/src/commands/resources.rs +++ b/crates/hodeishield-cli/src/commands/resources.rs @@ -4,16 +4,19 @@ //! The read commands: one per `/v1` collection. use super::Context; +use super::tenants::{Selection, Target}; use crate::cli::{ AlertsCommand, ComplianceCommand, EndpointsCommand, EvidenceCommand, Paging, RisksCommand, VendorsCommand, }; -use crate::failure::{ApiContext, CredentialSource, Result, from_api}; +use crate::failure::{ApiContext, CredentialSource, Failure, Kind, Result, from_api}; use crate::filters::warn_unknown; use crate::output::{ - cell, clean, print_csv, print_detail, print_json, print_table, short_time, table, + Rows, cell, clean, print_csv, print_detail, print_json, print_table, short_time, table, + tenant_table, }; -use comfy_table::Table; +use hodeishield_api::Client; +use hodeishield_api::portfolio::Tenant; use hodeishield_api::v1::{ Alert, ComplianceControl, Endpoint, Evidence, ListAlertsParams, ListComplianceControlsParams, ListEndpointsParams, ListEvidenceParams, ListRisksParams, ListVendorsParams, Pagination, Risk, @@ -27,6 +30,10 @@ use std::io::Write; type Fetch<'a, R> = dyn FnMut(Option, Option) -> std::result::Result, Error> + 'a; +/// A fetch that still has to be told which client to ask. +type FetchFrom<'a, R> = + dyn FnMut(&Client, Option, Option) -> std::result::Result, Error> + 'a; + /// Most pages `--all` fetches: 200 000 items at the largest page size. const MAX_PAGES: i64 = 1_000; @@ -35,9 +42,38 @@ const MAX_PAGES: i64 = 1_000; /// hundred MiB at most. const MAX_ITEMS: usize = 100_000; -/// Prints one page, or every page with `--all`. +/// Prints a list for the selected client, or for every client with `--all-tenants`. #[allow(clippy::too_many_arguments)] fn list( + ctx: &Context, + out: &mut dyn Write, + paging: &Paging, + noun: &str, + api: ApiContext<'_>, + targets: &[Target], + fetch: &mut FetchFrom<'_, R>, + split: fn(R) -> (Vec, Pagination), + render: fn(&[T]) -> Rows, +) -> Result<()> { + if matches!(ctx.selection, Selection::All) { + return list_across(ctx, out, paging, noun, api, targets, fetch, split, render); + } + let client = &targets[0].client; + list_one( + ctx, + out, + paging, + noun, + api, + &mut |page, per_page| fetch(client, page, per_page), + split, + render, + ) +} + +/// Prints one page, or every page with `--all`, of one tenant's list. +#[allow(clippy::too_many_arguments)] +pub(super) fn list_one( ctx: &Context, out: &mut dyn Write, paging: &Paging, @@ -45,7 +81,7 @@ fn list( api: ApiContext<'_>, fetch: &mut Fetch<'_, R>, split: fn(R) -> (Vec, Pagination), - render: fn(&[T]) -> Table, + render: fn(&[T]) -> Rows, ) -> Result<()> { if !paging.all { let response = fetch(paging.page, paging.per_page).map_err(|e| from_api(e, &api))?; @@ -61,7 +97,7 @@ fn list( let raw_items = response.raw.get("data").and_then(Value::as_array); print_csv(out, raw_items.map_or(&[], Vec::as_slice))?; } else { - print_table(out, &render(&items))?; + print_table(out, &render(&items).into_table())?; } if pagination.total_pages > 1 { let next = if pagination.page < pagination.total_pages { @@ -89,14 +125,138 @@ fn list( if paging.csv { print_csv(out, &raw_items)?; } else { - print_table(out, &render(&items))?; + print_table(out, &render(&items).into_table())?; } eprintln!("{} {noun}.", items.len()); Ok(()) } +/// Every page of every client tenant, each row labelled with the tenant whose request produced it. +/// +/// A tenant that cannot be read for its own reasons (no licence, left the portfolio) is reported +/// and skipped, and the command still fails at the end, so a script never takes a partial result +/// for a complete one. Any other failure stops the walk. +#[allow(clippy::too_many_arguments)] +fn list_across( + ctx: &Context, + out: &mut dyn Write, + paging: &Paging, + noun: &str, + api: ApiContext<'_>, + targets: &[Target], + fetch: &mut FetchFrom<'_, R>, + split: fn(R) -> (Vec, Pagination), + render: fn(&[T]) -> Rows, +) -> Result<()> { + let per_page = paging.per_page.or(Some(200)); + let mut collected: Vec<(&Tenant, Vec, Vec)> = Vec::new(); + let mut failures: Vec = Vec::new(); + let mut total = 0_usize; + for target in targets { + let Some(tenant) = &target.tenant else { + continue; + }; + let walked = collect_pages( + noun, + &api, + per_page, + MAX_ITEMS.saturating_sub(total), + &mut |page, per_page| fetch(&target.client, page, per_page), + split, + ); + match walked { + Ok((raw_items, items)) => { + total += items.len(); + collected.push((tenant, raw_items, items)); + } + Err(failure) + if matches!( + failure.kind, + Kind::Forbidden | Kind::NotFound | Kind::LicenceInactive + ) => + { + eprintln!( + "warning: client tenant {}: {}", + clean(&tenant.slug), + clean(&failure.message) + ); + failures.push(failure); + } + Err(failure) => return Err(failure), + } + } + + if ctx.json || paging.csv { + let mut labelled = Vec::new(); + for (tenant, raw_items, _) in &collected { + for item in raw_items { + labelled.push(with_tenant(item, tenant)?); + } + } + if ctx.json { + print_json(out, &Value::Array(labelled))?; + } else { + print_csv(out, &labelled)?; + } + } else { + let mut headers = None; + let mut groups = Vec::new(); + for (tenant, _, items) in &collected { + if items.is_empty() { + continue; + } + let rows = render(items); + headers.get_or_insert(rows.headers); + groups.push((tenant.slug.clone(), rows.rows)); + } + if let Some(headers) = headers { + print_table(out, &tenant_table(headers, groups))?; + } + } + if total == 0 { + eprintln!("No {noun} found."); + } else { + eprintln!("{total} {noun} across {} client tenants.", collected.len()); + } + match failures.first() { + None => Ok(()), + Some(first) => Err(Failure::new( + first.kind, + format!( + "Could not read {} of {} client tenants (see the warnings above); the output covers the others.", + failures.len(), + targets.len() + ), + )), + } +} + +/// The item with the tenant whose request returned it in front: `tenant_id`, `tenant_slug` and +/// `tenant_name`. An item that already has such a field is refused rather than overwritten. +fn with_tenant(item: &Value, tenant: &Tenant) -> Result { + let Value::Object(fields) = item else { + return Err(Failure::general( + "An item of the answer is not an object, so it cannot be labelled with its tenant.", + )); + }; + let mut labelled = serde_json::Map::new(); + labelled.insert("tenant_id".to_owned(), Value::String(tenant.id.clone())); + labelled.insert("tenant_slug".to_owned(), Value::String(tenant.slug.clone())); + labelled.insert("tenant_name".to_owned(), Value::String(tenant.name.clone())); + for (key, value) in fields { + if labelled.contains_key(key) { + return Err(Failure::general(format!( + "An item already has a field named {}, which the tenant label would overwrite.", + clean(key) + ))); + } + labelled.insert(key.clone(), value.clone()); + } + Ok(Value::Object(labelled)) +} + /// Walks every page, as received and decoded, stopping with an error past `max_items` items. -fn collect_pages( +pub(super) fn collect_pages( noun: &str, api: &ApiContext<'_>, per_page: Option, @@ -167,9 +327,9 @@ fn plain(source: CredentialSource) -> ApiContext<'static> { pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { VendorsCommand::List(args) => { + let (targets, source) = ctx.targets(&settings)?; let base = ListVendorsParams { q: args.query, category: args.category, @@ -185,7 +345,8 @@ pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> R &args.paging, "vendors", plain(source), - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_vendors(&ListVendorsParams { page, per_page, @@ -197,13 +358,14 @@ pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> R ) } VendorsCommand::Get { id } => { + let (client, source) = ctx.single(&settings)?; show(ctx, out, plain(source), client.get_vendor(&id), |r| &r.data) } } } -fn vendor_table(items: &[Vendor]) -> Table { - table( +fn vendor_table(items: &[Vendor]) -> Rows { + Rows::new( &["ID", "NAME", "DOMAIN", "CRITICALITY", "ACTIVE", "LAST SCAN"], items .iter() @@ -223,13 +385,13 @@ fn vendor_table(items: &[Vendor]) -> Table { pub fn alerts(ctx: &Context, command: AlertsCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { AlertsCommand::List(args) => { warn_unknown( &operations::LIST_ALERTS, &[("severity", args.severity.as_deref())], ); + let (targets, source) = ctx.targets(&settings)?; let base = ListAlertsParams { status: args.status, severity: args.severity, @@ -245,7 +407,8 @@ pub fn alerts(ctx: &Context, command: AlertsCommand, out: &mut dyn Write) -> Res &args.paging, "alerts", plain(source), - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_alerts(&ListAlertsParams { page, per_page, @@ -257,13 +420,14 @@ pub fn alerts(ctx: &Context, command: AlertsCommand, out: &mut dyn Write) -> Res ) } AlertsCommand::Get { id } => { + let (client, source) = ctx.single(&settings)?; show(ctx, out, plain(source), client.get_alert(&id), |r| &r.data) } } } -fn alert_table(items: &[Alert]) -> Table { - table( +fn alert_table(items: &[Alert]) -> Rows { + Rows::new( &["ID", "SEVERITY", "STATUS", "TYPE", "TITLE", "CREATED"], items .iter() @@ -283,7 +447,6 @@ fn alert_table(items: &[Alert]) -> Table { pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { RisksCommand::List(args) => { warn_unknown( @@ -293,6 +456,7 @@ pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Resul ("domain", args.domain.as_deref()), ], ); + let (targets, source) = ctx.targets(&settings)?; let base = ListRisksParams { status: args.status, domain: args.domain, @@ -308,7 +472,8 @@ pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Resul &args.paging, "risks", plain(source), - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_risks(&ListRisksParams { page, per_page, @@ -320,13 +485,14 @@ pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Resul ) } RisksCommand::Get { id } => { + let (client, source) = ctx.single(&settings)?; show(ctx, out, plain(source), client.get_risk(&id), |r| &r.data) } } } -fn risk_table(items: &[Risk]) -> Table { - table( +fn risk_table(items: &[Risk]) -> Rows { + Rows::new( &["ID", "TITLE", "DOMAIN", "STATUS", "INHERENT", "RESIDUAL"], items .iter() @@ -347,13 +513,13 @@ fn risk_table(items: &[Risk]) -> Table { pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { ComplianceCommand::Controls(args) => { warn_unknown( &operations::LIST_COMPLIANCE_CONTROLS, &[("status", args.status.as_deref())], ); + let (targets, source) = ctx.targets(&settings)?; let api = ApiContext { source, framework: Some(&args.framework), @@ -368,7 +534,8 @@ pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write &args.paging, "controls", api, - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_compliance_controls(&ListComplianceControlsParams { page, per_page, @@ -380,6 +547,7 @@ pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write ) } ComplianceCommand::Posture { framework } => { + let (client, source) = ctx.single(&settings)?; let api = ApiContext { source, framework: Some(&framework), @@ -413,8 +581,8 @@ pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write } } -fn control_table(items: &[ComplianceControl]) -> Table { - table( +fn control_table(items: &[ComplianceControl]) -> Rows { + Rows::new( &["CONTROL", "STATUS", "EVIDENCE", "LAST EVALUATED"], items .iter() @@ -432,9 +600,9 @@ fn control_table(items: &[ComplianceControl]) -> Table { pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { EvidenceCommand::List(args) => { + let (targets, source) = ctx.targets(&settings)?; let base = ListEvidenceParams { evidence_type: args.evidence_type, source_type: args.source_type, @@ -450,7 +618,8 @@ pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> &args.paging, "evidence items", plain(source), - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_evidence(&ListEvidenceParams { page, per_page, @@ -462,6 +631,7 @@ pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> ) } EvidenceCommand::Get { id } => { + let (client, source) = ctx.single(&settings)?; show(ctx, out, plain(source), client.get_evidence(&id), |r| { &r.data }) @@ -469,8 +639,8 @@ pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> } } -fn evidence_table(items: &[Evidence]) -> Table { - table( +fn evidence_table(items: &[Evidence]) -> Rows { + Rows::new( &["ID", "TYPE", "SOURCE", "TITLE", "VALID UNTIL"], items .iter() @@ -489,9 +659,9 @@ fn evidence_table(items: &[Evidence]) -> Table { pub fn endpoints(ctx: &Context, command: EndpointsCommand, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; - let (client, source) = ctx.api(&settings)?; match command { EndpointsCommand::List(args) => { + let (targets, source) = ctx.targets(&settings)?; let base = ListEndpointsParams { status: args.status, silent_since: args.silent_since, @@ -505,7 +675,8 @@ pub fn endpoints(ctx: &Context, command: EndpointsCommand, out: &mut dyn Write) &args.paging, "endpoints", plain(source), - &mut |page, per_page| { + &targets, + &mut |client, page, per_page| { client.list_endpoints(&ListEndpointsParams { page, per_page, @@ -519,8 +690,8 @@ pub fn endpoints(ctx: &Context, command: EndpointsCommand, out: &mut dyn Write) } } -fn endpoint_table(items: &[Endpoint]) -> Table { - table( +fn endpoint_table(items: &[Endpoint]) -> Rows { + Rows::new( &[ "ID", "HOSTNAME", diff --git a/crates/hodeishield-cli/src/commands/tenants.rs b/crates/hodeishield-cli/src/commands/tenants.rs new file mode 100644 index 0000000..43e37b9 --- /dev/null +++ b/crates/hodeishield-cli/src/commands/tenants.rs @@ -0,0 +1,217 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Hodeitek S.L. + +//! Client tenants: the `tenants` command and the selection of which client a read command reads. +//! +//! The CLI depends on the interface of `hodeishield_api::portfolio` only: it lists tenants and builds +//! a client bound to one of them. How the selection travels on the wire is not known here. + +use super::Context; +use super::resources::{collect_pages, list_one}; +use crate::cli::TenantsCommand; +use crate::config::Settings; +use crate::failure::{ApiContext, CredentialSource, Failure, Kind, Result}; +use crate::output::{Rows, cell, clean}; +use hodeishield_api::Client; +use hodeishield_api::portfolio::{ListTenantsParams, ListTenantsResponse, Tenant, is_uuid}; +use std::io::Write; + +/// Which client tenant the read commands read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Selection { + /// The credential's own tenant: what the CLI did before portfolios, with no tenant label. + Own, + /// One client, by slug or id. + Tenant(String), + /// Every client of the portfolio. + All, +} + +/// A client to ask, and the tenant it is bound to. `tenant` is `None` only for [`Selection::Own`]. +/// Each target has its own client, so a request for one tenant can never carry another's selection. +#[derive(Debug)] +pub struct Target { + pub client: Client, + pub tenant: Option, +} + +impl Context { + /// The clients to read for the selection. For [`Selection::Own`] this is one client and no + /// request is made; otherwise the portfolio is listed first. + pub fn targets(&self, settings: &Settings) -> Result<(Vec, CredentialSource)> { + let selected = match &self.selection { + Selection::Own => { + let (client, source) = self.api(settings, None)?; + return Ok(( + vec![Target { + client, + tenant: None, + }], + source, + )); + } + selected => selected, + }; + let (lister, source) = self.api(settings, None)?; + let portfolio = list_portfolio(&lister, source)?; + let tenants = match selected { + Selection::Tenant(wanted) => { + let tenant = find(&portfolio, wanted)?; + eprintln!( + "Client tenant: {} ({})", + clean(&tenant.slug), + clean(&tenant.id) + ); + vec![tenant] + } + _ => { + let (usable, idle): (Vec<_>, Vec<_>) = portfolio.into_iter().partition(readable); + if !idle.is_empty() { + eprintln!( + "Skipping {} inactive client tenants (see `hodeishield tenants list`).", + idle.len() + ); + } + usable + } + }; + let targets = tenants + .into_iter() + .map(|tenant| { + let (client, _) = self.api(settings, Some(&tenant.id))?; + Ok(Target { + client, + tenant: Some(tenant), + }) + }) + .collect::>()?; + Ok((targets, source)) + } + + /// The one client a single-item command reads. + pub fn single(&self, settings: &Settings) -> Result<(Client, CredentialSource)> { + let (mut targets, source) = self.targets(settings)?; + match (targets.pop(), targets.is_empty()) { + (Some(target), true) => Ok((target.client, source)), + _ => Err( + Failure::general("This command reads one client tenant at a time.") + .hint("Use --tenant ; --all-tenants works on list commands."), + ), + } + } +} + +/// Whether a tenant is worth asking: not inactive, and not without a licence in force. +fn readable(tenant: &Tenant) -> bool { + tenant.active && tenant.licence_status.as_deref() != Some("inactive") +} + +/// Every client tenant of the portfolio. +fn list_portfolio(lister: &Client, source: CredentialSource) -> Result> { + let api = ApiContext { + source, + framework: None, + }; + let (_, tenants) = collect_pages( + "client tenants", + &api, + Some(200), + 100_000, + &mut |page, per_page| lister.list_tenants(&ListTenantsParams { page, per_page }), + |r: ListTenantsResponse| (r.data, r.pagination), + ) + .map_err(|failure| { + if failure.kind == Kind::Forbidden { + failure.hint( + "--tenant and --all-tenants need a portfolio credential. Without them the CLI \ + reads the credential's own tenant.", + ) + } else { + failure + } + })?; + Ok(tenants) +} + +/// The tenant the user named, by id or slug. One that is not in the portfolio is reported the way +/// the API reports it: not found, with no hint about whether it exists elsewhere. +fn find(portfolio: &[Tenant], wanted: &str) -> Result { + let by_id = is_uuid(wanted); + let mut matches = portfolio.iter().filter(|t| { + if by_id { + t.id.eq_ignore_ascii_case(wanted) + } else { + t.slug.eq_ignore_ascii_case(wanted) + } + }); + match (matches.next(), matches.next()) { + (Some(tenant), None) => Ok(tenant.clone()), + (Some(_), Some(_)) => Err(Failure::general(format!( + "More than one client tenant is called {}.", + clean(wanted) + )) + .hint("Use its id: see `hodeishield tenants list`.")), + _ => Err(Failure::new( + Kind::NotFound, + format!( + "No client tenant {} in this credential's portfolio.", + clean(wanted) + ), + ) + .hint("List the client tenants you may read with `hodeishield tenants list`.")), + } +} + +pub fn run(ctx: &Context, command: TenantsCommand, out: &mut dyn Write) -> Result<()> { + let TenantsCommand::List(args) = command; + let settings = ctx.settings()?; + let (client, source) = ctx.api(&settings, None)?; + list_one( + ctx, + out, + &args.paging, + "client tenants", + ApiContext { + source, + framework: None, + }, + &mut |page, per_page| client.list_tenants(&ListTenantsParams { page, per_page }), + |r| (r.data, r.pagination), + tenant_rows, + ) + .map_err(|failure| { + if failure.kind == Kind::Forbidden { + failure.hint("Listing client tenants needs a portfolio credential.") + } else { + failure + } + }) +} + +fn tenant_rows(items: &[Tenant]) -> Rows { + Rows::new( + &[ + "SLUG", + "NAME", + "ORGANIZATION", + "TYPE", + "STATUS", + "LICENCE", + "ID", + ], + items + .iter() + .map(|t| { + vec![ + clean(&t.slug), + clean(&t.name), + cell(t.organization.as_ref().map(|o| o.name.as_str())), + if t.internal { "own" } else { "client" }.to_owned(), + if t.active { "active" } else { "inactive" }.to_owned(), + cell(t.licence_status.as_deref()), + clean(&t.id), + ] + }) + .collect(), + ) +} diff --git a/crates/hodeishield-cli/src/failure.rs b/crates/hodeishield-cli/src/failure.rs index 8d7c6eb..6ffb4c5 100644 --- a/crates/hodeishield-cli/src/failure.rs +++ b/crates/hodeishield-cli/src/failure.rs @@ -178,10 +178,24 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail format!("Rate limit exhausted{scope}.{wait}"), ) } - 400 => Failure::general(format!( - "The API rejected the request: {}.", - server_message.unwrap_or_else(|| "invalid request".to_owned()) - )), + 400 => { + let failure = Failure::general(format!( + "The API rejected the request: {}.", + server_message.unwrap_or_else(|| "invalid request".to_owned()) + )); + match detail(&api, "code") { + Some("TENANT_REQUIRED") => failure.hint( + "This credential covers several client tenants: pick one with \ + --tenant , or every one with --all-tenants on a list. \ + `hodeishield tenants list` shows them.", + ), + Some("TENANT_SELECTOR_NOT_SUPPORTED") => failure.hint( + "This credential reads one tenant only; leave out --tenant and \ + --all-tenants.", + ), + _ => failure, + } + } status if status >= 500 => Failure::new( Kind::Unavailable, format!( @@ -202,6 +216,11 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail "The answer to {operation} does not match the API description this CLI was built from: {source}." )) .hint("Your CLI may be older than the API. Check for a newer release."), + other @ hodeishield_api::Error::TenantMismatch { .. } => Failure::general(other.to_string()) + .hint( + "Nothing was shown, so that no client's data appears under another's name. Check \ + the API URL and try again; if it persists, contact support.", + ), other => Failure::general(other.to_string()), } } diff --git a/crates/hodeishield-cli/src/output.rs b/crates/hodeishield-cli/src/output.rs index 4357c2f..a7149fd 100644 --- a/crates/hodeishield-cli/src/output.rs +++ b/crates/hodeishield-cli/src/output.rs @@ -122,6 +122,42 @@ pub fn short_time(value: Option<&str>) -> String { } } +/// A table's header and rows, before they are laid out: what a command knows how to say about its +/// items. [`tenant_table`] adds the client column when a list spans tenants. +#[derive(Debug)] +pub struct Rows { + pub headers: &'static [&'static str], + pub rows: Vec>, +} + +impl Rows { + pub fn new(headers: &'static [&'static str], rows: Vec>) -> Self { + Self { headers, rows } + } + + pub fn into_table(self) -> Table { + table(self.headers, self.rows) + } +} + +/// Rows of several client tenants in one table, each led by the tenant's slug. The slug is the +/// server's text, so it is sanitised like every other cell. +pub fn tenant_table(headers: &[&str], groups: Vec<(String, Vec>)>) -> Table { + let mut all = vec!["TENANT"]; + all.extend_from_slice(headers); + let rows = groups + .into_iter() + .flat_map(|(slug, rows)| { + let label = clean(&slug); + rows.into_iter().map(move |mut row| { + row.insert(0, label.clone()); + row + }) + }) + .collect(); + table(&all, rows) +} + pub fn table(headers: &[&str], rows: Vec>) -> Table { let mut table = Table::new(); table @@ -417,6 +453,34 @@ mod tests { ); } + #[test] + fn a_tenant_column_leads_each_row_and_is_sanitised() { + let t = tenant_table( + &["ID"], + vec![ + ("acme".to_owned(), vec![vec!["1".into()]]), + ( + "ev\u{1b}il".to_owned(), + vec![vec!["2".into()], vec!["3".into()]], + ), + ], + ); + let mut out = Vec::new(); + print_table(&mut out, &t).expect("print"); + let text = String::from_utf8(out).expect("utf8"); + let lines: Vec<&str> = text.lines().collect(); + assert!( + lines[0].starts_with("TENANT") && lines[0].ends_with("ID"), + "{text}" + ); + assert!( + lines[1].starts_with("acme") && lines[1].ends_with('1'), + "{text}" + ); + assert!(lines[2].starts_with("ev\u{fffd}il"), "{text:?}"); + assert!(!text.contains('\u{1b}')); + } + #[test] fn tables_have_no_trailing_spaces() { let t = table( diff --git a/crates/hodeishield-cli/tests/cli.rs b/crates/hodeishield-cli/tests/cli.rs index 18a8fe6..6dd1f08 100644 --- a/crates/hodeishield-cli/tests/cli.rs +++ b/crates/hodeishield-cli/tests/cli.rs @@ -771,3 +771,454 @@ fn free_form_filters_are_sent_as_typed_without_a_warning() { mock.assert(); } } + +// Client tenants. The portfolio contract is a draft: these tests pin the CLI's behaviour against the +// mock in `hodeishield_api::portfolio`, and change with it. + +const ACME: &str = "11111111-1111-4111-8111-111111111111"; +const BETA: &str = "22222222-2222-4222-8222-222222222222"; +const OLD: &str = "33333333-3333-4333-8333-333333333333"; + +fn tenant(id: &str, slug: &str, active: bool, licence: &str) -> String { + format!( + r#"{{"id":"{id}","name":"{slug} S.L.","slug":"{slug}","organization":null,"internal":false,"active":{active},"licence_status":"{licence}"}}"# + ) +} + +fn portfolio(tenants: &[String]) -> String { + page(tenants, 1, tenants.len() as i64, 1) +} + +fn mock_portfolio(server: &mut mockito::Server, tenants: &[String]) -> mockito::Mock { + server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", mockito::Matcher::Missing) + .with_body(portfolio(tenants)) + .create() +} + +/// A vendor list for one tenant, confirmed the way the API confirms a selection. +fn mock_vendors(server: &mut mockito::Server, tenant_id: &str, names: &[&str]) -> mockito::Mock { + let items: Vec = names.iter().map(|n| vendor(&format!("v-{n}"), n)).collect(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", tenant_id) + .with_header("x-hs-tenant", tenant_id) + .with_body(page(&items, 1, items.len() as i64, 1)) + .create() +} + +fn two_clients() -> [String; 2] { + [ + tenant(ACME, "acme", true, "in_force"), + tenant(BETA, "beta", true, "grace"), + ] +} + +#[test] +fn tenants_list_prints_the_portfolio_as_table_json_and_csv() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let listing = mock_portfolio(&mut server, &two_clients()); + let table = env + .api(&server) + .args(["tenants", "list"]) + .output() + .expect("run"); + assert!(table.status.success(), "{table:?}"); + let stdout = String::from_utf8_lossy(&table.stdout); + assert!(stdout.starts_with("SLUG"), "{stdout}"); + assert!( + stdout.contains("acme") && stdout.contains(BETA) && stdout.contains("grace"), + "{stdout}" + ); + + let json = env + .api(&server) + .args(["tenants", "list", "--json"]) + .output() + .expect("run"); + let body: serde_json::Value = serde_json::from_slice(&json.stdout).expect("json"); + assert_eq!(body["data"][1]["slug"], "beta"); + + let csv = env + .api(&server) + .args(["tenants", "list", "--csv"]) + .output() + .expect("run"); + assert!(String::from_utf8_lossy(&csv.stdout).starts_with("id,name,slug,")); + listing.expect_at_least(3).assert(); +} + +#[test] +fn a_tenant_is_selected_by_slug_or_id_and_the_table_stays_as_it_was() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let beta = mock_vendors(&mut server, BETA, &["Beta Hosting"]); + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + for selector in ["beta", "BETA", BETA] { + let output = env + .api(&server) + .args(["--tenant", selector, "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{selector}: {output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.starts_with("ID") && stdout.contains("Beta Hosting"), + "{stdout}" + ); + assert!( + !stdout.contains("TENANT") && !stdout.contains("Acme"), + "{stdout}" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains(&format!("Client tenant: beta ({BETA})")), + "{stderr}" + ); + assert_no_key(&output); + } + beta.expect(3).assert(); + acme.expect(0).assert(); +} + +#[test] +fn a_selected_tenant_with_json_is_the_api_body_unchanged() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let body = format!(r#"{{"data":{}}}"#, vendor("v-1", "Acme Hosting")); + server + .mock("GET", "/v1/vendors/v-1") + .match_header("x-hs-tenant", ACME) + .with_header("x-hs-tenant", ACME) + .with_body(&body) + .create(); + let output = env + .api(&server) + .args(["vendors", "get", "v-1", "--json", "--tenant", "acme"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let shown: serde_json::Value = serde_json::from_slice(&output.stdout).expect("json"); + assert_eq!( + shown, + serde_json::from_str::(&body).expect("json") + ); +} + +#[test] +fn all_tenants_labels_every_row_with_the_tenant_that_returned_it() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + mock_vendors(&mut server, ACME, &["Acme Hosting", "Shared Name"]); + mock_vendors(&mut server, BETA, &["Beta Hosting", "Shared Name"]); + + let table = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(table.status.success(), "{table:?}"); + let stdout = String::from_utf8_lossy(&table.stdout); + let lines: Vec<&str> = stdout.lines().collect(); + assert!( + lines[0].starts_with("TENANT") && lines[0].contains("ID"), + "{stdout}" + ); + assert_eq!(lines.len(), 5, "{stdout}"); + let row = |name: &str, slug: &str| { + lines + .iter() + .any(|l| l.starts_with(slug) && l.contains(name)) + }; + assert!( + row("Acme Hosting", "acme") && row("Beta Hosting", "beta"), + "{stdout}" + ); + assert!( + !row("Acme Hosting", "beta") && !row("Beta Hosting", "acme"), + "{stdout}" + ); + assert!(String::from_utf8_lossy(&table.stderr).contains("4 vendors across 2 client tenants.")); + + let json = env + .api(&server) + .args(["--json", "vendors", "list", "--all-tenants"]) + .output() + .expect("run"); + let items: serde_json::Value = serde_json::from_slice(&json.stdout).expect("json"); + let items = items.as_array().expect("array"); + assert_eq!(items.len(), 4); + assert_eq!(items[0]["tenant_id"], ACME); + assert_eq!(items[0]["tenant_slug"], "acme"); + assert_eq!(items[0]["name"], "Acme Hosting"); + assert_eq!(items[0]["added_later"], 1, "unknown fields are kept"); + assert_eq!(items[3]["tenant_slug"], "beta"); + + let csv = env + .api(&server) + .args(["vendors", "list", "--all-tenants", "--csv"]) + .output() + .expect("run"); + let text = String::from_utf8_lossy(&csv.stdout); + assert!( + text.starts_with("tenant_id,tenant_slug,tenant_name,id,"), + "{text}" + ); + assert!( + text.contains(&format!("{BETA},beta,beta S.L.,v-Beta Hosting,")), + "{text}" + ); +} + +#[test] +fn all_tenants_skips_inactive_clients_and_says_so() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant(ACME, "acme", true, "in_force"), + tenant(OLD, "old", false, "inactive"), + ], + ); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("Skipping 1 inactive client tenants")); +} + +#[test] +fn a_tenant_that_cannot_be_read_is_reported_and_the_command_still_fails() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", BETA) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r1","details":{"code":"LICENCE_INACTIVE","licence":"expired"}}}"#) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(8), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Acme Hosting"), "{stdout}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("warning: client tenant beta"), "{stderr}"); + assert!( + stderr.contains("Could not read 1 of 2 client tenants"), + "{stderr}" + ); +} + +#[test] +fn a_client_outside_the_portfolio_fails_clearly_without_reading_any_data() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let data = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .expect(0) + .create(); + for selector in ["stranger", "44444444-4444-4444-8444-444444444444"] { + let output = env + .api(&server) + .args(["vendors", "list", "--tenant", selector]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(5), "{selector}: {output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains(&format!( + "No client tenant {selector} in this credential's portfolio." + )), + "{stderr}" + ); + assert!(output.stdout.is_empty()); + } + data.assert(); +} + +#[test] +fn the_server_refusing_a_selected_client_is_not_found_too() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"No such tenant in this credential's portfolio.","request_id":"r2"}}"#) + .create(); + let output = env + .api(&server) + .args(["vendors", "list", "--tenant", "acme"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(5), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("No such tenant")); +} + +#[test] +fn an_answer_that_does_not_confirm_the_client_prints_nothing() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + // The server answers with the other client's confirmation. + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_header("x-hs-tenant", BETA) + .with_body(page(&[vendor("v1", "Beta Hosting")], 1, 1, 1)) + .create(); + for args in [&["--tenant", "acme"][..], &["--all-tenants"][..]] { + let output = env + .api(&server) + .args(args) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{args:?}: {output:?}"); + assert!(output.stdout.is_empty(), "{output:?}"); + assert!( + String::from_utf8_lossy(&output.stderr).contains("did not confirm the client tenant") + ); + } +} + +#[test] +fn without_a_selection_nothing_changes_for_a_single_tenant_credential() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let listing = server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .expect(0) + .create(); + let vendors = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", mockito::Matcher::Missing) + .with_body(page(&[vendor("v1", "Example Hosting")], 1, 1, 1)) + .create(); + let output = env + .api(&server) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.starts_with("ID") && !stdout.contains("TENANT"), + "{stdout}" + ); + assert!(!String::from_utf8_lossy(&output.stderr).contains("Client tenant")); + listing.assert(); + vendors.assert(); +} + +#[test] +fn selecting_a_client_with_a_single_tenant_credential_explains_what_is_missing() { + let env = Env::new(); + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r3"}}"#) + .create(); + for args in [&["--tenant", "acme"][..], &["--all-tenants"][..]] { + let output = env + .api(&server) + .args(args) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(4), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("portfolio:read"), "{stderr}"); + assert!(stderr.contains("need a portfolio credential"), "{stderr}"); + } + let output = env + .api(&server) + .args(["tenants", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(4), "{output:?}"); +} + +#[test] +fn a_portfolio_credential_without_a_selection_is_told_how_to_select() { + let env = Env::new(); + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(400) + .with_body(r#"{"error":{"code":"invalid_request","message":"X-HS-Tenant is required","request_id":"r4","details":{"code":"TENANT_REQUIRED"}}}"#) + .create(); + let output = env + .api(&server) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&output.stderr).contains("--all-tenants")); +} + +#[test] +fn tenant_names_from_the_server_cannot_drive_the_terminal() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[tenant(ACME, "ac\\u001b[2Jme", true, "in_force")], + ); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let all = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(!all.contains('\u{1b}'), "{all:?}"); + assert!(all.contains("ac\u{fffd}[2Jme"), "{all:?}"); +} + +#[test] +fn the_selection_is_refused_where_it_cannot_apply() { + let env = Env::new(); + for args in [ + &["--tenant", "acme", "--all-tenants", "vendors", "list"][..], + &["--tenant", "acme", "whoami"][..], + &["--all-tenants", "tenants", "list"][..], + &["--all-tenants", "vendors", "get", "v1"][..], + &["--all-tenants", "compliance", "posture", "nis2"][..], + &["--all-tenants", "vendors", "list", "--page", "2"][..], + ] { + let output = env.cmd().args(args).output().expect("run"); + assert_eq!(output.status.code(), Some(2), "{args:?}: {output:?}"); + } +} From c6c2abf7c2d949249c9b7c8d47294d51577ba2f8 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:31:25 +0200 Subject: [PATCH 03/10] Document reading several client tenants with one credential Add the 'Several tenants (MSSP)' section to the README and stop pointing MSSPs at one profile per client. The section is a draft until the portfolio part of /v1 is published. Closes #45 Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- README.md | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 828be3a..d8cc30e 100644 --- a/README.md +++ b/README.md @@ -292,7 +292,7 @@ hodeishield logout # revokes the token at the app and removes it from You sign in to [app.hodeishield.com](https://app.hodeishield.com) as usual (2FA, passkey or SSO), and on the consent screen you choose **the one tenant** the CLI may read. A sign-in token never covers -more than that tenant, and it only reads. To use another tenant, define another profile and sign in +more than that tenant, and it only reads. To use an unrelated tenant, define another profile and sign in to it: ```sh @@ -305,6 +305,9 @@ A profile that has not been defined is refused (`Profile ... is not defined`). `login` first checks that the system keychain is available and stops before opening the browser or showing a code if it is not; use a tenant API key on such a machine. +An MSSP reads its clients with one credential instead: see +[Several tenants (MSSP)](#several-tenants-mssp). + With `--device`, open the address the CLI prints on any device and **type the code by hand**: the app does not accept a link with the code filled in, on purpose. @@ -350,6 +353,92 @@ rest. `--all` stops with an error past 100,000 items (or 1,000 pages); narrow th or walk it with `--page`. Every list takes `--sort` and `--order`; `--help` on any command lists what it accepts. +### Several tenants (MSSP) + +An MSSP reads all of its client tenants with **one** credential: a portfolio key, created in the +app by an administrator of an active MSSP organisation, in the MSSP area (API keys). You do not need +a profile, a key or a sign-in per client, and you should not set them up that way. + +A portfolio key reads only, always expires (90 days by default, 365 at most), and works on the +HodeiShield API only. It can be limited when it is created to some of your clients and to source IP +ranges; a request from another address is refused like an unknown key (exit code 3). Signing in +with `hodeishield login` does not cover a whole portfolio: use the key. + +**1. Use the credential as any other key.** + +```sh +export HODEISHIELD_API_KEY=hsp_3f2a9c0d1b4e_... # the portfolio key; keep it in your CI's secret store +``` + +The key starts with `hsp_`, a 12-character identifier and `_`, then a long secret. The first 16 +characters identify the key in the app and in audit entries; the CLI never prints any part of the +key. The same variable takes a tenant API key as before. + +**2. List the client tenants it may read.** + +```console +$ hodeishield tenants list +SLUG NAME ORGANIZATION TYPE STATUS LICENCE ID +acme Acme S.L. - client active in_force 11111111-1111-4111-8111-111111111111 +beta Beta Corp Beta Group client active grace 22222222-2222-4222-8222-222222222222 +``` + +It takes `--json`, `--csv`, `--page` and `--all` like any list, sorted by name. It includes your +own organisation's tenants (`TYPE` `own` for the internal one) and clients that are inactive or have +no licence in force, which the `STATUS` and `LICENCE` columns tell apart. The list is read from the API +each time: a client added to or removed from your portfolio shows up on the next command. + +**3. Select the client a read command reads.** The selection goes with the command and is never +remembered, so a later command cannot read another client by surprise. + +```sh +hodeishield --tenant acme vendors list # one client, by slug or id +hodeishield --all-tenants alerts list --status open +``` + +- `--tenant ` works on every read command (`list`, `get`, `compliance posture`). The CLI + prints `Client tenant: acme ()` on standard error, so a log shows which client was read; + standard output is exactly what it is without a selection. +- `--all-tenants` works on `list` commands and `compliance controls`. It fetches every page of each + client (`--per-page` applies, `--page` does not) and adds the tenant to each row: a `TENANT` + column in the table, and `tenant_id`, `tenant_slug` and `tenant_name` fields in `--json` and + `--csv`. Clients that are not readable are skipped, with a note on standard error: those that + are inactive, whose licence status is `inactive`, or whose licence status this CLI does not + know. A client in its grace period is read. Clients are asked one after the other. +- The tenant on a row is the one whose own request returned it. The CLI asks the API about one + client at a time, and refuses an answer that does not confirm that client. + +```sh +hodeishield --all-tenants alerts list --status open --json \ + | jq -r '.[] | [.tenant_slug, .severity, .title] | @tsv' +hodeishield --all-tenants vendors list --criticality critical --csv > critical-vendors.csv +``` + +**4. When something is not readable.** + +- A client that is not in your portfolio (or that you mistyped) fails with `No client tenant … in + this credential's portfolio.` and exit code 5, before anything is read. +- With `--all-tenants`, a client that cannot be read on its own account (for example its licence is + not in force, exit code 8) is reported on standard error and skipped; the command prints the + others and still exits non-zero, so a script does not mistake a partial result for a complete + one. +- Without `--tenant` or `--all-tenants`, a portfolio key is refused by the API, and the CLI tells + you to choose. A single-tenant key or sign-in behaves exactly as before; asking it for a client + fails with exit code 4 (it lacks the portfolio scope). +- A client you named that is deactivated, or whose licence is not in force, fails with exit code 8. +- A portfolio key is limited to 100 requests per minute, and all the portfolio keys of your + organisation together to 600 (the API may change these numbers). The CLI waits out a short + `Retry-After` and retries; if the wait is longer, or the limit is still exhausted, the command + stops with exit code 6 and says which limit it hit (`portfolio_key`, `portfolio_org`, + `portfolio_tenant` or `tenant`), instead of asking the next client. Use `--per-page 200` and + narrow with filters to need fewer requests. +- Against an API that does not offer portfolio access yet, `tenants list`, `--tenant` and + `--all-tenants` fail with `This API does not offer portfolio access yet.` and exit code 7. Nothing + else changes. + +**5. Revoke access.** Revoke the portfolio key in the app, in the MSSP area; it stops working on its +next request. A client leaving your portfolio is dropped from the list on the next command. + ### JSON for scripts `--json` prints the body the API returned, unchanged, including fields newer than your CLI. With @@ -389,7 +478,7 @@ hodeishield vendors list --all --csv > vendors.csv | 4 | The credential lacks the scope the command needs | | 5 | Not found (or not in your organisation) | | 6 | Rate limit exhausted (the CLI already retried short waits) | -| 7 | The API could not be reached or failed | +| 7 | The API could not be reached or failed, or does not offer portfolio access yet | | 8 | The tenant's licence is not in force: a tenant administrator must renew or reactivate it | Errors go to stderr with a hint and, when the API gave one, a **request id**: quote it when you From a370dfc4986ab81cd6ae6be02deb6af16895d315 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:31:25 +0200 Subject: [PATCH 04/10] Note the client tenant commands in the changelog Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4265079..1c76197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,17 @@ commands, `--json` output and exit codes. Each release's notes start with its se for each stable release: the release's own binary on a distroless base, running as a non-root user, tagged with the exact version only. It is signed by digest with Sigstore and carries the release's SBOM as a signed attestation. ([#36](https://github.com/Hodeitek/hodeishield-cli/issues/36)) +- Several client tenants with one MSSP credential. `hodeishield tenants list` lists the client + tenants the credential may read. The global `--tenant ` selects one client for any read + command, and `--all-tenants` reads every client on a `list` command and adds `tenant_id`, + `tenant_slug` and `tenant_name` to each row in `--json` and `--csv` (a `TENANT` column in the + table). An unknown client fails with exit code 5, before any data is read. Without these options + nothing changes. `--all-tenants` skips clients that are inactive, whose licence is not in force, or + whose licence status the CLI does not know, asks one client at a time, and stops with exit code 6 + (naming the limit) when a rate limit is exhausted. Against an API without portfolio access the + commands fail with exit code 7. The README has a new "Several tenants (MSSP)" section. + ([#54](https://github.com/Hodeitek/hodeishield-cli/issues/54), + [#45](https://github.com/Hodeitek/hodeishield-cli/issues/45)) ### Changed (scripting surface) From 1ac9a49fc6dadd4de6f09ea056a7d8e236c6be9e Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Thu, 8 Oct 2026 10:39:41 +0200 Subject: [PATCH 05/10] Follow the published portfolio contract Read the client tenants as the API describes them: ask for the listing in ascending name order, require the organisation, and treat a licence status the CLI does not know as not readable. Accept a tenant confirmation only as the single lowercase id that was asked for. An API with no listing route is reported with exit code 7 instead of a generic failure. A rate limit or a missing data scope now ends an all-tenants walk instead of being repeated for every client, and the messages name the limit that was hit and the tenant refusals the API distinguishes. Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- crates/hodeishield-api/src/error.rs | 6 + crates/hodeishield-api/src/portfolio.rs | 171 +++++++-- crates/hodeishield-api/tests/api.rs | 269 +++++++++++++- crates/hodeishield-cli/src/cli.rs | 4 +- .../hodeishield-cli/src/commands/resources.rs | 11 +- .../hodeishield-cli/src/commands/tenants.rs | 18 +- crates/hodeishield-cli/src/failure.rs | 69 +++- crates/hodeishield-cli/tests/cli.rs | 343 +++++++++++++++++- 8 files changed, 813 insertions(+), 78 deletions(-) diff --git a/crates/hodeishield-api/src/error.rs b/crates/hodeishield-api/src/error.rs index 6f3569f..02937b1 100644 --- a/crates/hodeishield-api/src/error.rs +++ b/crates/hodeishield-api/src/error.rs @@ -86,6 +86,12 @@ pub enum Error { /// An id or slug that would change which resource the path addresses. #[error("{0:?} is not a valid identifier")] InvalidPathArgument(String), + /// The API has no portfolio listing (it answered `404` or `405` for the route). + #[error("this API does not offer portfolio access (HTTP {status} for the tenant listing)")] + PortfolioNotOffered { + /// The status the route answered with. + status: u16, + }, /// A client tenant identifier that is not a UUID. #[error("{0:?} is not a valid client tenant identifier")] InvalidTenant(String), diff --git a/crates/hodeishield-api/src/portfolio.rs b/crates/hodeishield-api/src/portfolio.rs index 690027b..8be29d7 100644 --- a/crates/hodeishield-api/src/portfolio.rs +++ b/crates/hodeishield-api/src/portfolio.rs @@ -3,22 +3,31 @@ //! Client tenants of a portfolio credential: list them, and read one of them. //! -//! # Provisional +//! This module follows the published `/v1` portfolio contract (contract version `2026-10-16`): //! -//! **This module is written by hand against a draft of the `/v1` contract and is not part of the -//! published API description yet.** Nothing in `openapi/v1.json` or the generated client describes -//! it. When the contract is published, this file is the only place that changes: the operation, the -//! shape of [`Tenant`] and the way the selected tenant travels on a request are all decided here, -//! and callers see only the interface below. +//! - `GET /v1/portfolio/tenants` (`listPortfolioTenants`), scope `portfolio:read`, paginated like +//! every `/v1` list (`page`, `per_page`, `sort`, `order`; the API's default order is descending, +//! so this client asks for ascending); +//! - the client tenant is selected on each of the data operations with one optional `X-HS-Tenant` +//! request header holding the tenant's UUID, and every successful answer to such a request +//! repeats the tenant it read in an `X-HS-Tenant` response header (lowercase UUID). Error +//! answers never carry it, and the listing neither takes nor returns it; +//! - a tenant is readable when it is active and its `licence_status` is `in_force` or `grace`. The +//! `licence_status` vocabulary is open: a value this client does not know counts as not +//! readable. //! -//! The draft assumes: +//! The version string of the API description is not compared with anything. Whether an API offers +//! portfolio access is found by asking for the listing: a `404` or `405` on that route becomes +//! [`Error::PortfolioNotOffered`]. //! -//! - `GET /v1/portfolio/tenants`, scope `portfolio:read`, paginated like the other lists -//! (`page`, `per_page`, answer `{ data, pagination }`); -//! - the client is selected on every data operation with one `X-HS-Tenant` request header holding -//! the tenant's UUID, and every answer to such a request repeats it in an `X-HS-Tenant` response -//! header; -//! - the listing does not take that header. +//! # Relation to the generated client +//! +//! The vendored `openapi/v1.json` does not describe these operations yet, so this file is written +//! by hand. When the vendored document is synced to the contract version above, the listing +//! operation, [`Tenant`] and the paging types come from `cargo xtask codegen` (which first has to +//! learn header parameters and the new security scheme), and this file shrinks to what is not an +//! operation: [`ClientBuilder::tenant`] and its confirmation check, which the generated code does +//! not provide. //! //! # Interface //! @@ -29,7 +38,7 @@ use crate::client::{Operation, Scope}; use crate::error::Error; -use crate::v1::Pagination; +use crate::v1::{Order, Pagination}; use crate::{ApiResponse, Client, ClientBuilder}; use reqwest::header::{HeaderMap, HeaderName, HeaderValue}; use serde::{Deserialize, Serialize}; @@ -55,24 +64,26 @@ pub struct Tenant { pub id: String, /// Display name. pub name: String, - /// Short unique name, for people and scripts. + /// Short name, unique across the platform, for people and scripts. pub slug: String, - /// The client organisation the tenant belongs to, when it has one. - #[serde(default)] - pub organization: Option, - /// Whether this is the MSSP's own tenant rather than a client's. - #[serde(default)] + /// The organisation the tenant belongs to: the MSSP's own for the tenants it holds directly. + pub organization: TenantOrganization, + /// Whether this is the MSSP's own internal tenant rather than a client's. pub internal: bool, - /// Whether the tenant is active. - #[serde(default = "yes")] + /// Whether the tenant is enabled. pub active: bool, - /// `in_force`, `grace` or `inactive`. The vocabulary may grow. - #[serde(default)] - pub licence_status: Option, + /// `in_force`, `grace` or `inactive`. The vocabulary is open: other values may appear. + pub licence_status: String, } -fn yes() -> bool { - true +impl Tenant { + /// Whether reading this tenant can be expected to work: it is active and its licence is in + /// force or in its grace period. A licence status this client does not know counts as not + /// readable, since the vocabulary is open. + #[must_use] + pub fn readable(&self) -> bool { + self.active && matches!(self.licence_status.as_str(), "in_force" | "grace") + } } /// The organisation a [`Tenant`] belongs to. @@ -93,16 +104,51 @@ pub struct ListTenantsResponse { pub pagination: Pagination, } -/// Paging of [`Client::list_tenants`]. +/// What [`Client::list_tenants`] sends. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ListTenantsParams { /// 1-based page number. pub page: Option, /// Items per page, 1..200. pub per_page: Option, + /// Sort by `name` or `slug`. + pub sort: Option, + /// Direction. The API's default is descending. + pub order: Option, +} + +/// The fields the tenant listing can be sorted by. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TenantSort { + /// `name` + Name, + /// `slug` + Slug, +} + +impl TenantSort { + /// The value as sent on the wire. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Name => "name", + Self::Slug => "slug", + } + } } impl ListTenantsParams { + /// Alphabetical by name, the order a person expects. + #[must_use] + pub fn alphabetical(page: Option, per_page: Option) -> Self { + Self { + page, + per_page, + sort: Some(TenantSort::Name), + order: Some(Order::Asc), + } + } + fn query_pairs(&self) -> Vec<(&'static str, String)> { let mut pairs = Vec::new(); if let Some(page) = self.page { @@ -111,6 +157,12 @@ impl ListTenantsParams { if let Some(per_page) = self.per_page { pairs.push(("per_page", per_page.to_string())); } + if let Some(sort) = self.sort { + pairs.push(("sort", sort.as_str().to_owned())); + } + if let Some(order) = self.order { + pairs.push(("order", order.as_str().to_owned())); + } pairs } } @@ -124,7 +176,8 @@ impl Client { /// # Errors /// /// [`Error`] when the request cannot be sent, the API answers with an error (a credential - /// without portfolio scope gets `403`), or the body does not match the draft. + /// without portfolio scope gets `403`), or the body does not match the contract. + /// [`Error::PortfolioNotOffered`] when the API has no such route. pub fn list_tenants( &self, params: &ListTenantsParams, @@ -135,6 +188,12 @@ impl Client { params.query_pairs(), false, ) + .map_err(|error| match error { + Error::Api(api) if matches!(api.status, 404 | 405) => { + Error::PortfolioNotOffered { status: api.status } + } + other => other, + }) } } @@ -158,10 +217,13 @@ impl ClientBuilder { HeaderValue::from_str(&wanted).map_err(|_| Error::InvalidTenant(id.to_owned()))?, ); let check = move |answer: &HeaderMap| { - let answered = answer - .get(TENANT_HEADER) - .and_then(|v| v.to_str().ok()) - .map(|v| v.trim().to_ascii_lowercase()); + // Header names compare case-insensitively (the map does it); the answer must carry + // exactly one value, the lowercase UUID of the tenant that was read. + let mut values = answer.get_all(TENANT_HEADER).iter(); + let answered = match (values.next(), values.next()) { + (Some(value), None) => value.to_str().ok().map(str::to_owned), + _ => None, + }; if answered.as_deref() == Some(wanted.as_str()) { Ok(()) } else { @@ -210,4 +272,47 @@ mod tests { assert!(!is_uuid(bad), "{bad:?}"); } } + + fn tenant(active: bool, licence: &str) -> Tenant { + Tenant { + id: "0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6e".to_owned(), + name: "Acme".to_owned(), + slug: "acme".to_owned(), + organization: TenantOrganization { + id: "o".to_owned(), + name: "Acme Group".to_owned(), + }, + internal: false, + active, + licence_status: licence.to_owned(), + } + } + + #[test] + fn a_tenant_is_readable_only_when_active_with_a_known_good_licence() { + assert!(tenant(true, "in_force").readable()); + assert!(tenant(true, "grace").readable()); + assert!(!tenant(true, "inactive").readable()); + assert!(!tenant(false, "in_force").readable()); + assert!(!tenant(false, "grace").readable()); + // The vocabulary is open: what is not known is not assumed readable. + assert!(!tenant(true, "suspended_soon").readable()); + assert!(!tenant(true, "").readable()); + assert!(!tenant(true, "IN_FORCE").readable()); + } + + #[test] + fn the_listing_asks_for_ascending_names_when_told_to() { + let pairs = ListTenantsParams::alphabetical(Some(2), Some(200)).query_pairs(); + assert_eq!( + pairs, + vec![ + ("page", "2".to_owned()), + ("per_page", "200".to_owned()), + ("sort", "name".to_owned()), + ("order", "asc".to_owned()), + ] + ); + assert!(ListTenantsParams::default().query_pairs().is_empty()); + } } diff --git a/crates/hodeishield-api/tests/api.rs b/crates/hodeishield-api/tests/api.rs index bb9bd8b..07d98ad 100644 --- a/crates/hodeishield-api/tests/api.rs +++ b/crates/hodeishield-api/tests/api.rs @@ -663,7 +663,11 @@ fn retry_observer_receives_expected_events() { success.assert(); } -// The portfolio module is provisional: these tests pin its interface, not the published contract. +// The portfolio contract (version 2026-10-16): the listing, the tenant selection and its echo. + +/// The shape of a portfolio key: `hsp_`, 12 hex, `_`, 64 hex. +const PORTFOLIO_KEY: &str = + "hsp_3f2a9c0d1b4e_0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; const TENANT_A: &str = "11111111-1111-4111-8111-111111111111"; const TENANT_B: &str = "22222222-2222-4222-8222-222222222222"; @@ -673,7 +677,7 @@ const EMPTY_PAGE: &str = fn tenant_client(server: &mockito::Server, tenant: &str) -> Client { Client::builder( server.url().parse().expect("url"), - SecretString::from("hsp_example"), + SecretString::from(PORTFOLIO_KEY), ) .tenant(tenant) .expect("a UUID") @@ -687,10 +691,10 @@ fn list_tenants_decodes_the_listing_and_never_sends_a_tenant() { let mut server = mockito::Server::new(); let mock = server .mock("GET", "/v1/portfolio/tenants") - .match_header("authorization", "Bearer hsp_example") + .match_header("authorization", format!("Bearer {PORTFOLIO_KEY}").as_str()) .match_header("x-hs-tenant", mockito::Matcher::Missing) .with_body(format!( - r#"{{"data":[{{"id":"{TENANT_A}","name":"Acme","slug":"acme","organization":{{"id":"o1","name":"Acme Group"}},"internal":false,"active":true,"licence_status":"in_force","later":1}}],"pagination":{{"page":1,"per_page":50,"total":1,"total_pages":1}}}}"# + r#"{{"data":[{{"id":"{TENANT_A}","name":"Acme","slug":"acme","organization":{{"id":"o1","name":"Acme Group"}},"internal":false,"active":true,"licence_status":"in_force","later":1}},{{"id":"{TENANT_B}","name":"Beta","slug":"beta","organization":{{"id":"o2","name":"Beta Group"}},"internal":true,"active":false,"licence_status":"something_new"}}],"pagination":{{"page":1,"per_page":50,"total":2,"total_pages":1}}}}"# )) .create(); // Even a client built for one tenant lists the whole portfolio. @@ -699,11 +703,13 @@ fn list_tenants_decodes_the_listing_and_never_sends_a_tenant() { .expect("listing"); mock.assert(); assert_eq!(response.data.data[0].slug, "acme"); - assert_eq!( - response.data.data[0].licence_status.as_deref(), - Some("in_force") - ); + assert_eq!(response.data.data[0].licence_status, "in_force"); assert_eq!(response.raw["data"][0]["later"], 1); + // An unknown licence status decodes, and is not readable. + let beta = &response.data.data[1]; + assert_eq!(beta.licence_status, "something_new"); + assert!(!beta.readable() && beta.internal); + assert!(response.data.data[0].readable()); } #[test] @@ -712,7 +718,9 @@ fn a_selected_tenant_travels_on_data_calls_and_must_be_confirmed() { let ok = server .mock("GET", "/v1/vendors") .match_header("x-hs-tenant", TENANT_A) - .with_header("x-hs-tenant", &TENANT_A.to_ascii_uppercase()) + // Header names are case-insensitive; the value is the lowercase UUID. + .with_header("X-HS-Tenant", TENANT_A) + .with_header("Vary", "X-HS-Tenant") .with_body(EMPTY_PAGE) .create(); tenant_client(&server, TENANT_A) @@ -780,3 +788,246 @@ fn a_tenant_that_is_not_a_uuid_is_refused_before_any_request() { assert!(matches!(err, Error::InvalidTenant(_)), "{bad:?}"); } } + +#[test] +fn the_tenant_is_sent_in_lowercase_and_the_echo_must_be_exactly_that() { + let lower = "0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6e"; + let upper = lower.to_ascii_uppercase(); + let mut server = mockito::Server::new(); + // The request carries the lowercase form even when the caller gave capitals. + let sent = server + .mock("GET", "/v1/vendors") + .match_header("x-hs-tenant", lower) + .with_header("x-hs-tenant", lower) + .with_body(EMPTY_PAGE) + .create(); + tenant_client(&server, &upper) + .list_vendors(&ListVendorsParams::default()) + .expect("lowercase echo"); + sent.assert(); + // An echo in capitals is not what the contract promises: refused. + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .with_header("x-hs-tenant", &upper) + .with_body(EMPTY_PAGE) + .create(); + let err = tenant_client(&server, lower) + .list_vendors(&ListVendorsParams::default()) + .expect_err("not the contract's form"); + assert!(matches!(err, Error::TenantMismatch { .. }), "{err:?}"); +} + +#[test] +fn an_echo_that_names_two_tenants_is_refused() { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .with_header("x-hs-tenant", TENANT_A) + .with_header("x-hs-tenant", TENANT_B) + .with_body(EMPTY_PAGE) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("two values"); + assert!(matches!(err, Error::TenantMismatch { .. }), "{err:?}"); +} + +#[test] +fn every_data_operation_sends_the_selection_and_checks_the_echo() { + use hodeishield_api::v1::{ + ListAlertsParams, ListComplianceControlsParams, ListEndpointsParams, ListEvidenceParams, + ListRisksParams, + }; + let mut server = mockito::Server::new(); + let any = server + .mock("GET", mockito::Matcher::Any) + .match_header("x-hs-tenant", TENANT_A) + .with_header("x-hs-tenant", TENANT_B) + .with_body(EMPTY_PAGE) + .expect(5) + .create(); + let c = tenant_client(&server, TENANT_A); + fn refused(r: Result) -> bool { + matches!(r, Err(Error::TenantMismatch { .. })) + } + assert!(refused(c.list_alerts(&ListAlertsParams::default()))); + assert!(refused(c.list_risks(&ListRisksParams::default()))); + assert!(refused(c.list_evidence(&ListEvidenceParams::default()))); + assert!(refused(c.list_endpoints(&ListEndpointsParams::default()))); + assert!(refused(c.list_compliance_controls( + &ListComplianceControlsParams { + page: None, + per_page: None, + sort: None, + order: None, + framework: "nis2".to_owned(), + status: None, + } + ))); + any.assert(); +} + +#[test] +fn an_error_answer_is_reported_as_the_api_error_whatever_the_header_says() { + // The contract has no echo on errors; a client must not mistake one for a mismatch, nor hide + // the real refusal behind it. + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"No such tenant in this credential's portfolio.","request_id":"r","details":{"code":"TENANT_NOT_FOUND"}}}"#) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("not in the portfolio"); + let api = err.api().expect("the API's own error"); + assert_eq!(api.status, 404); + let details = api + .body + .as_ref() + .and_then(|b| b.details.as_ref()) + .expect("details"); + assert_eq!(details["code"], "TENANT_NOT_FOUND"); +} + +#[test] +fn the_listing_asks_for_the_order_it_is_given() { + use hodeishield_api::portfolio::ListTenantsParams; + let mut server = mockito::Server::new(); + let mock = server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::AllOf(vec![ + mockito::Matcher::UrlEncoded("page".into(), "2".into()), + mockito::Matcher::UrlEncoded("per_page".into(), "200".into()), + mockito::Matcher::UrlEncoded("sort".into(), "name".into()), + mockito::Matcher::UrlEncoded("order".into(), "asc".into()), + ])) + .with_body(EMPTY_PAGE) + .create(); + client(&server) + .list_tenants(&ListTenantsParams::alphabetical(Some(2), Some(200))) + .expect("listing"); + mock.assert(); +} + +#[test] +fn a_listing_that_breaks_the_contract_is_a_decode_error() { + use hodeishield_api::portfolio::ListTenantsParams; + let mut server = mockito::Server::new(); + // `organization` is never null. + server + .mock("GET", "/v1/portfolio/tenants") + .with_body(format!( + r#"{{"data":[{{"id":"{TENANT_A}","name":"A","slug":"a","organization":null,"internal":false,"active":true,"licence_status":"in_force"}}],"pagination":{{"page":1,"per_page":50,"total":1,"total_pages":1}}}}"# + )) + .create(); + let err = client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect_err("null organization"); + assert!(matches!(err, Error::Decode { .. }), "{err:?}"); +} + +#[test] +fn an_api_without_the_listing_route_is_told_apart_from_other_errors() { + use hodeishield_api::portfolio::ListTenantsParams; + for status in [404_usize, 405] { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .with_status(status) + .with_body(r#"{"error":{"code":"not_found","message":"Not found","request_id":"r"}}"#) + .create(); + let err = client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect_err("no route"); + assert!( + matches!(&err, Error::PortfolioNotOffered { status: s } if usize::from(*s) == status), + "{status}: {err:?}" + ); + } + // Other statuses stay what the API said, and a 404 on a data operation is not this. + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"INSUFFICIENT_SCOPE","missing":["portfolio:read"]}}}"#) + .create(); + server + .mock("GET", "/v1/vendors/v1") + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"x","request_id":"r"}}"#) + .create(); + let c = client(&server); + let err = c + .list_tenants(&ListTenantsParams::default()) + .expect_err("403"); + assert_eq!(err.api().map(|a| a.status), Some(403)); + let err = c.get_vendor("v1").expect_err("404"); + assert_eq!(err.api().map(|a| a.status), Some(404)); +} + +#[test] +fn a_portfolio_429_is_retried_after_a_short_retry_after_and_reports_its_scope_otherwise() { + use hodeishield_api::portfolio::ListTenantsParams; + let body = r#"{"error":{"code":"rate_limited","message":"x","request_id":"r","details":{"scope":"portfolio_key","limit":100,"window_seconds":60,"retry_after_seconds":1}}}"#; + // Within the wait the client accepts: waited out, then served. + let mut server = mockito::Server::new(); + let limited = server + .mock("GET", "/v1/portfolio/tenants") + .with_status(429) + .with_header("retry-after", "1") + .with_body(body) + .expect(1) + .create(); + let served = server + .mock("GET", "/v1/portfolio/tenants") + .with_body(EMPTY_PAGE) + .expect(1) + .create(); + client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect("served after the wait"); + limited.assert(); + served.assert(); + // Longer than the client accepts: one request, and the scope is in the error. + let mut server = mockito::Server::new(); + let once = server + .mock("GET", "/v1/portfolio/tenants") + .with_status(429) + .with_header("retry-after", "600") + .with_body(body) + .expect(1) + .create(); + let err = client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect_err("too long to wait"); + once.assert(); + let api = err.api().expect("API error"); + assert_eq!(api.status, 429); + assert_eq!(api.meta.retry_after, Some(600)); + let details = api + .body + .as_ref() + .and_then(|b| b.details.as_ref()) + .expect("details"); + assert_eq!(details["scope"], "portfolio_key"); +} + +#[test] +fn a_portfolio_key_is_sent_whole_and_never_shown() { + let server = mockito::Server::new(); + let c = tenant_client(&server, TENANT_A); + let shown = format!("{c:?}"); + let secret = PORTFOLIO_KEY.rsplit('_').next().expect("secret part"); + assert!( + !shown.contains(secret) && !shown.contains(PORTFOLIO_KEY), + "{shown}" + ); + let builder = Client::builder( + server.url().parse().expect("url"), + SecretString::from(PORTFOLIO_KEY), + ); + let shown = format!("{builder:?}"); + assert!(!shown.contains(secret), "{shown}"); +} diff --git a/crates/hodeishield-cli/src/cli.rs b/crates/hodeishield-cli/src/cli.rs index 6e2d66f..9f52e3d 100644 --- a/crates/hodeishield-cli/src/cli.rs +++ b/crates/hodeishield-cli/src/cli.rs @@ -16,7 +16,7 @@ use std::str::FromStr; const AFTER_HELP: &str = "\ Credentials: - HODEISHIELD_API_KEY A tenant API key. When set, it is used for every API call. + HODEISHIELD_API_KEY A tenant API key, or an MSSP portfolio key (hsp_…). When set, it is used for every API call. hodeishield login Otherwise, sign in to the app; the token is kept in the system keychain. Several client tenants (portfolio credential): @@ -26,7 +26,7 @@ Several client tenants (portfolio credential): Exit codes: 0 ok · 1 error · 2 usage · 3 not authenticated · 4 missing scope · 5 not found - 6 rate limited · 7 API unreachable or failing · 8 tenant licence not in force"; + 6 rate limited · 7 API unreachable, failing or without portfolio access · 8 tenant licence not in force"; /// Read-only command-line client for HodeiShield. /// diff --git a/crates/hodeishield-cli/src/commands/resources.rs b/crates/hodeishield-cli/src/commands/resources.rs index 6170241..d17459e 100644 --- a/crates/hodeishield-cli/src/commands/resources.rs +++ b/crates/hodeishield-cli/src/commands/resources.rs @@ -135,7 +135,9 @@ pub(super) fn list_one( /// /// A tenant that cannot be read for its own reasons (no licence, left the portfolio) is reported /// and skipped, and the command still fails at the end, so a script never takes a partial result -/// for a complete one. Any other failure stops the walk. +/// for a complete one. Any other failure stops the walk: tenants are asked one after the other, and +/// a rate limit (after the client has waited out the short ones) or a missing scope is not retried +/// against the next tenant. #[allow(clippy::too_many_arguments)] fn list_across( ctx: &Context, @@ -169,12 +171,7 @@ fn list_across( total += items.len(); collected.push((tenant, raw_items, items)); } - Err(failure) - if matches!( - failure.kind, - Kind::Forbidden | Kind::NotFound | Kind::LicenceInactive - ) => - { + Err(failure) if matches!(failure.kind, Kind::NotFound | Kind::LicenceInactive) => { eprintln!( "warning: client tenant {}: {}", clean(&tenant.slug), diff --git a/crates/hodeishield-cli/src/commands/tenants.rs b/crates/hodeishield-cli/src/commands/tenants.rs index 43e37b9..43205b0 100644 --- a/crates/hodeishield-cli/src/commands/tenants.rs +++ b/crates/hodeishield-cli/src/commands/tenants.rs @@ -11,7 +11,7 @@ use super::resources::{collect_pages, list_one}; use crate::cli::TenantsCommand; use crate::config::Settings; use crate::failure::{ApiContext, CredentialSource, Failure, Kind, Result}; -use crate::output::{Rows, cell, clean}; +use crate::output::{Rows, clean}; use hodeishield_api::Client; use hodeishield_api::portfolio::{ListTenantsParams, ListTenantsResponse, Tenant, is_uuid}; use std::io::Write; @@ -65,7 +65,8 @@ impl Context { vec![tenant] } _ => { - let (usable, idle): (Vec<_>, Vec<_>) = portfolio.into_iter().partition(readable); + let (usable, idle): (Vec<_>, Vec<_>) = + portfolio.into_iter().partition(Tenant::readable); if !idle.is_empty() { eprintln!( "Skipping {} inactive client tenants (see `hodeishield tenants list`).", @@ -101,11 +102,6 @@ impl Context { } } -/// Whether a tenant is worth asking: not inactive, and not without a licence in force. -fn readable(tenant: &Tenant) -> bool { - tenant.active && tenant.licence_status.as_deref() != Some("inactive") -} - /// Every client tenant of the portfolio. fn list_portfolio(lister: &Client, source: CredentialSource) -> Result> { let api = ApiContext { @@ -117,7 +113,7 @@ fn list_portfolio(lister: &Client, source: CredentialSource) -> Result Resul source, framework: None, }, - &mut |page, per_page| client.list_tenants(&ListTenantsParams { page, per_page }), + &mut |page, per_page| client.list_tenants(&ListTenantsParams::alphabetical(page, per_page)), |r| (r.data, r.pagination), tenant_rows, ) @@ -205,10 +201,10 @@ fn tenant_rows(items: &[Tenant]) -> Rows { vec![ clean(&t.slug), clean(&t.name), - cell(t.organization.as_ref().map(|o| o.name.as_str())), + clean(&t.organization.name), if t.internal { "own" } else { "client" }.to_owned(), if t.active { "active" } else { "inactive" }.to_owned(), - cell(t.licence_status.as_deref()), + clean(&t.licence_status), clean(&t.id), ] }) diff --git a/crates/hodeishield-cli/src/failure.rs b/crates/hodeishield-cli/src/failure.rs index 6ffb4c5..1af55fa 100644 --- a/crates/hodeishield-cli/src/failure.rs +++ b/crates/hodeishield-cli/src/failure.rs @@ -20,9 +20,10 @@ pub enum Kind { NotFound, /// A rate limit is exhausted (429). Exit code 6. RateLimited, - /// The API could not be reached or failed (network, 5xx). Exit code 7. + /// The API could not be reached, failed (network, 5xx), or lacks a feature the command needs + /// (no portfolio listing). Exit code 7. Unavailable, - /// The tenant holds no licence in force (403 `LICENCE_INACTIVE`). Exit code 8. + /// The tenant holds no licence in force, or is deactivated (403 `LICENCE_INACTIVE`). Exit code 8. LicenceInactive, } @@ -139,10 +140,12 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail && s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') }) .map_or_else(String::new, |s| format!(" (licence: {s})")); - Failure::new( - Kind::LicenceInactive, - format!("The tenant's licence is not in force{state}."), - ) + let message = if detail(&api, "licence") == Some("tenant_inactive") { + "The tenant is deactivated.".to_owned() + } else { + format!("The tenant's licence is not in force{state}.") + }; + Failure::new(Kind::LicenceInactive, message) .hint( "Ask a tenant administrator to renew or reactivate it. Signing in again or \ using another key of this tenant will not help.", @@ -163,20 +166,49 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail ) .hint("Ask a tenant administrator for a key or access with that scope.") } - 404 => Failure::new( - Kind::NotFound, - server_message.unwrap_or_else(|| "Not found.".to_owned()), - ), + 404 => { + let failure = Failure::new( + Kind::NotFound, + server_message.unwrap_or_else(|| "Not found.".to_owned()), + ); + if detail(&api, "code") == Some("TENANT_NOT_FOUND") { + failure.hint( + "The client tenant is not in this credential's portfolio (or it left \ + it). `hodeishield tenants list` shows the ones you may read.", + ) + } else { + failure + } + } 429 => { let wait = api .meta .retry_after .map_or_else(String::new, |s| format!(" Try again in {s} s.")); let scope = detail(&api, "scope").map_or_else(String::new, |s| format!(" (limit: {s})")); - Failure::new( + let hint = match detail(&api, "scope") { + Some("portfolio_key") => Some( + "This portfolio key has used its requests for the minute. A script \ + that reads many client tenants should pause between runs.", + ), + Some("portfolio_org") => Some( + "All portfolio keys of your organisation share this limit; another key \ + or script may be using it.", + ), + Some("portfolio_tenant" | "tenant") => Some( + "This client tenant's own limit is used up; other client tenants are \ + not affected.", + ), + _ => None, + }; + let failure = Failure::new( Kind::RateLimited, format!("Rate limit exhausted{scope}.{wait}"), - ) + ); + match hint { + Some(hint) => failure.hint(hint), + None => failure, + } } 400 => { let failure = Failure::general(format!( @@ -193,6 +225,10 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail "This credential reads one tenant only; leave out --tenant and \ --all-tenants.", ), + Some("TENANT_HEADER_INVALID") => failure.hint( + "The CLI sent a tenant selection the API could not read. This is a \ + bug: please report it.", + ), _ => failure, } } @@ -216,6 +252,15 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail "The answer to {operation} does not match the API description this CLI was built from: {source}." )) .hint("Your CLI may be older than the API. Check for a newer release."), + hodeishield_api::Error::PortfolioNotOffered { .. } => Failure::new( + Kind::Unavailable, + "This API does not offer portfolio access yet.", + ) + .hint( + "Reading several client tenants with one credential needs a newer API, or the API URL \ + is not the HodeiShield /v1 API. Without --tenant and --all-tenants the CLI reads the \ + credential's own tenant as before.", + ), other @ hodeishield_api::Error::TenantMismatch { .. } => Failure::general(other.to_string()) .hint( "Nothing was shown, so that no client's data appears under another's name. Check \ diff --git a/crates/hodeishield-cli/tests/cli.rs b/crates/hodeishield-cli/tests/cli.rs index 6dd1f08..a06d2c6 100644 --- a/crates/hodeishield-cli/tests/cli.rs +++ b/crates/hodeishield-cli/tests/cli.rs @@ -772,8 +772,11 @@ fn free_form_filters_are_sent_as_typed_without_a_warning() { } } -// Client tenants. The portfolio contract is a draft: these tests pin the CLI's behaviour against the -// mock in `hodeishield_api::portfolio`, and change with it. +// Client tenants, against a mock of the published portfolio contract (version 2026-10-16). + +/// The shape of a portfolio key: `hsp_`, 12 hex, `_`, 64 hex. Its secret part must never be shown. +const PORTFOLIO_KEY: &str = + "hsp_3f2a9c0d1b4e_0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; const ACME: &str = "11111111-1111-4111-8111-111111111111"; const BETA: &str = "22222222-2222-4222-8222-222222222222"; @@ -781,7 +784,7 @@ const OLD: &str = "33333333-3333-4333-8333-333333333333"; fn tenant(id: &str, slug: &str, active: bool, licence: &str) -> String { format!( - r#"{{"id":"{id}","name":"{slug} S.L.","slug":"{slug}","organization":null,"internal":false,"active":{active},"licence_status":"{licence}"}}"# + r#"{{"id":"{id}","name":"{slug} S.L.","slug":"{slug}","organization":{{"id":"o-{slug}","name":"{slug} Group"}},"internal":false,"active":{active},"licence_status":"{licence}"}}"# ) } @@ -792,7 +795,11 @@ fn portfolio(tenants: &[String]) -> String { fn mock_portfolio(server: &mut mockito::Server, tenants: &[String]) -> mockito::Mock { server .mock("GET", "/v1/portfolio/tenants") - .match_query(mockito::Matcher::Any) + // The API's default order is descending; the CLI asks for alphabetical. + .match_query(mockito::Matcher::AllOf(vec![ + mockito::Matcher::UrlEncoded("sort".into(), "name".into()), + mockito::Matcher::UrlEncoded("order".into(), "asc".into()), + ])) .match_header("x-hs-tenant", mockito::Matcher::Missing) .with_body(portfolio(tenants)) .create() @@ -1222,3 +1229,331 @@ fn the_selection_is_refused_where_it_cannot_apply() { assert_eq!(output.status.code(), Some(2), "{args:?}: {output:?}"); } } + +/// Runs the CLI with a real-shaped portfolio key, so a leak of the secret part would show. +fn portfolio_cmd(env: &Env, server: &mockito::Server) -> Command { + let mut cmd = env.cmd(); + cmd.env("HODEISHIELD_API_KEY", PORTFOLIO_KEY) + .env("HODEISHIELD_API_URL", server.url()); + cmd +} + +fn assert_no_portfolio_secret(output: &std::process::Output) { + let all = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let secret = PORTFOLIO_KEY.rsplit('_').next().expect("secret part"); + assert!( + !all.contains(secret), + "the key's secret part leaked:\n{all}" + ); + assert!(!all.contains(PORTFOLIO_KEY), "the key leaked:\n{all}"); +} + +#[test] +fn a_portfolio_key_in_its_published_format_is_sent_as_a_bearer_and_never_shown() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let bearer = format!("Bearer {PORTFOLIO_KEY}"); + let listing = server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .match_header("authorization", bearer.as_str()) + .with_body(portfolio(&two_clients())) + .create(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("authorization", bearer.as_str()) + .match_header("x-hs-tenant", ACME) + .with_header("x-hs-tenant", ACME) + .with_body(page(&[vendor("v1", "Acme Hosting")], 1, 1, 1)) + .create(); + for args in [ + &["tenants", "list"][..], + &["--tenant", "acme", "vendors", "list"][..], + ] { + let output = portfolio_cmd(&env, &server) + .args(args) + .output() + .expect("run"); + assert!(output.status.success(), "{args:?}: {output:?}"); + assert_no_portfolio_secret(&output); + } + // A refused key is reported without echoing it either. + let mut rejected = mockito::Server::new(); + rejected + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .with_status(401) + .with_body( + r#"{"error":{"code":"unauthorized","message":"Invalid API key","request_id":"r"}}"#, + ) + .create(); + let output = portfolio_cmd(&env, &rejected) + .args(["tenants", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(3), "{output:?}"); + assert_no_portfolio_secret(&output); + listing.expect_at_least(2).assert(); +} + +#[test] +fn all_tenants_skips_what_is_not_readable_by_the_contract_rule() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let strange = "44444444-4444-4444-8444-444444444444"; + let off = "55555555-5555-4555-8555-555555555555"; + mock_portfolio( + &mut server, + &[ + tenant(ACME, "acme", true, "in_force"), + tenant(BETA, "beta", true, "grace"), + // Listed by the server, but a client reading them would get a 403. + tenant(OLD, "old", true, "inactive"), + tenant(off, "off", false, "in_force"), + tenant(strange, "strange", true, "a_status_from_the_future"), + ], + ); + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let beta = mock_vendors(&mut server, BETA, &["Beta Hosting"]); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Skipping 3 inactive client tenants"), + "{stderr}" + ); + assert!( + stderr.contains("2 vendors across 2 client tenants."), + "{stderr}" + ); + acme.assert(); + beta.assert(); +} + +#[test] +fn an_api_that_does_not_offer_the_portfolio_says_so_with_its_own_exit_code() { + let env = Env::new(); + for status in [404, 405] { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .with_status(status) + .with_body(r#"{"error":{"code":"not_found","message":"Not found","request_id":"r"}}"#) + .create(); + let data = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .expect(0) + .create(); + for args in [ + &["tenants", "list"][..], + &["--tenant", "acme", "vendors", "list"][..], + &["--all-tenants", "vendors", "list"][..], + ] { + let output = env.api(&server).args(args).output().expect("run"); + assert_eq!( + output.status.code(), + Some(7), + "{status} {args:?}: {output:?}" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("does not offer portfolio access yet"), + "{stderr}" + ); + assert!(output.stdout.is_empty()); + } + data.assert(); + } +} + +#[test] +fn a_rate_limit_stops_an_all_tenants_walk_and_names_its_scope() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + // A wait too long to sit out: the walk ends, and the second client is never asked. + let limited = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", ACME) + .with_status(429) + .with_header("retry-after", "600") + .with_body(r#"{"error":{"code":"rate_limited","message":"x","request_id":"r9","details":{"scope":"portfolio_key","limit":100,"window_seconds":60,"retry_after_seconds":600}}}"#) + .expect(1) + .create(); + let second = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", BETA) + .expect(0) + .create(); + let output = portfolio_cmd(&env, &server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(6), "{output:?}"); + assert!(output.stdout.is_empty()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("(limit: portfolio_key)"), "{stderr}"); + assert!(stderr.contains("Try again in 600 s."), "{stderr}"); + assert_no_portfolio_secret(&output); + limited.assert(); + second.assert(); +} + +#[test] +fn a_short_rate_limit_is_waited_out_and_the_walk_goes_on() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let limited = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", ACME) + .with_status(429) + .with_header("retry-after", "1") + .with_body(r#"{"error":{"code":"rate_limited","message":"x","request_id":"r9","details":{"scope":"portfolio_tenant","limit":60,"window_seconds":60,"retry_after_seconds":1}}}"#) + .expect(1) + .create(); + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let beta = mock_vendors(&mut server, BETA, &["Beta Hosting"]); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + limited.assert(); + acme.assert(); + beta.assert(); +} + +#[test] +fn a_missing_data_scope_stops_the_walk_instead_of_failing_once_per_tenant() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let refused = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"INSUFFICIENT_SCOPE","missing":["supply_risk:read"]}}}"#) + .expect(1) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(4), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("supply_risk:read")); + refused.assert(); +} + +#[test] +fn a_deactivated_client_is_a_licence_refusal_not_a_missing_resource() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"LICENCE_INACTIVE","licence":"tenant_inactive"}}}"#) + .create(); + let output = env + .api(&server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(8), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("The tenant is deactivated.")); +} + +#[test] +fn a_tenant_the_server_does_not_know_gets_the_same_answer_as_one_it_hides() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"No such tenant in this credential's portfolio.","request_id":"r2","details":{"code":"TENANT_NOT_FOUND"}}}"#) + .create(); + let output = env + .api(&server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(5), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("No such tenant in this credential's portfolio."), + "{stderr}" + ); + assert!(stderr.contains("tenants list"), "{stderr}"); +} + +#[test] +fn the_selector_errors_of_the_contract_are_explained() { + let env = Env::new(); + for (code, expected) in [ + ("TENANT_SELECTOR_NOT_SUPPORTED", "leave out --tenant"), + ("TENANT_HEADER_INVALID", "please report it"), + ] { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(400) + .with_body(format!( + r#"{{"error":{{"code":"invalid_request","message":"x","request_id":"r","details":{{"code":"{code}","parameter":"X-HS-Tenant"}}}}}}"# + )) + .create(); + let output = env + .api(&server) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{code}: {output:?}"); + assert!( + String::from_utf8_lossy(&output.stderr).contains(expected), + "{code}" + ); + } +} + +#[test] +fn an_echo_in_the_wrong_form_prints_nothing() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let mixed = "0B8F2A52-6A0E-4D5C-9D3E-1F2A3B4C5D6E"; + mock_portfolio(&mut server, &[tenant(mixed, "acme", true, "in_force")]); + // The contract's echo is lowercase; capitals are not what an honest server sends. + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", mixed.to_ascii_lowercase().as_str()) + .with_header("x-hs-tenant", mixed) + .with_body(page(&[vendor("v1", "Acme Hosting")], 1, 1, 1)) + .create(); + let output = env + .api(&server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(output.stdout.is_empty()); +} From a96bdb598083017f39a7981c0d8e5dd2b435313c Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Thu, 8 Oct 2026 10:53:44 +0200 Subject: [PATCH 06/10] Harden the portfolio commands after review - Tenants with a licence status this version does not know are not read, are named on stderr, and make an --all-tenants walk exit non-zero; "inactive" is said of inactive tenants only. - A UUID-shaped input that matches one tenant's id and another's slug is ambiguous. - Slugs with characters outside [a-z0-9-] are shown with the start of the tenant id in the table. - The tenant listing is de-duplicated by id, and a changing total is reported. - A 404/405 on the listing that carries a portfolio error code is the API's own error, not "portfolio not offered". - The tenant rate-limit hint applies only when a client tenant is selected. - Tenant scoping is a property of each operation (tenant_scoped), set by the code generator. - Add tests for the echo on every operation, retried answers, redirects, 204/304, partial walks, CSV formula guarding and tenant resolution. Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- CHANGELOG.md | 3 +- README.md | 14 +- crates/hodeishield-api/src/client.rs | 20 +- crates/hodeishield-api/src/generated.rs | 11 + crates/hodeishield-api/src/portfolio.rs | 60 ++- crates/hodeishield-api/tests/api.rs | 197 ++++++++ crates/hodeishield-cli/src/commands/mod.rs | 11 + .../hodeishield-cli/src/commands/resources.rs | 62 ++- .../hodeishield-cli/src/commands/tenants.rs | 65 ++- crates/hodeishield-cli/src/failure.rs | 10 +- crates/hodeishield-cli/src/filters.rs | 4 + crates/hodeishield-cli/src/output.rs | 47 +- crates/hodeishield-cli/tests/cli.rs | 467 +++++++++++++++++- xtask/src/main.rs | 7 +- 14 files changed, 905 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c76197..191101a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,8 @@ commands, `--json` output and exit codes. Each release's notes start with its se `tenant_slug` and `tenant_name` to each row in `--json` and `--csv` (a `TENANT` column in the table). An unknown client fails with exit code 5, before any data is read. Without these options nothing changes. `--all-tenants` skips clients that are inactive, whose licence is not in force, or - whose licence status the CLI does not know, asks one client at a time, and stops with exit code 6 + whose licence status the CLI does not know (the latter are named on standard error and make the + command exit non-zero), asks one client at a time, and stops with exit code 6 (naming the limit) when a rate limit is exhausted. Against an API without portfolio access the commands fail with exit code 7. The README has a new "Several tenants (MSSP)" section. ([#54](https://github.com/Hodeitek/hodeishield-cli/issues/54), diff --git a/README.md b/README.md index d8cc30e..c6ea267 100644 --- a/README.md +++ b/README.md @@ -402,9 +402,13 @@ hodeishield --all-tenants alerts list --status open - `--all-tenants` works on `list` commands and `compliance controls`. It fetches every page of each client (`--per-page` applies, `--page` does not) and adds the tenant to each row: a `TENANT` column in the table, and `tenant_id`, `tenant_slug` and `tenant_name` fields in `--json` and - `--csv`. Clients that are not readable are skipped, with a note on standard error: those that - are inactive, whose licence status is `inactive`, or whose licence status this CLI does not - know. A client in its grace period is read. Clients are asked one after the other. + `--csv`. Clients that are inactive, or whose licence status is `inactive`, are skipped with a + note on standard error. A client in its grace period is read. A client whose licence status this + CLI does not know is not read either; it is named on standard error (`licence status … is not + known to this version; not read`) and the command exits non-zero, so nothing goes missing + silently. Clients are asked one after the other. In the table, a slug with characters other + than `a-z`, `0-9` and `-` is followed by the start of the client's id, so look-alike names can be + told apart; `--json` and `--csv` carry the full values. - The tenant on a row is the one whose own request returned it. The CLI asks the API about one client at a time, and refuses an answer that does not confirm that client. @@ -421,7 +425,9 @@ hodeishield --all-tenants vendors list --criticality critical --csv > critical-v - With `--all-tenants`, a client that cannot be read on its own account (for example its licence is not in force, exit code 8) is reported on standard error and skipped; the command prints the others and still exits non-zero, so a script does not mistake a partial result for a complete - one. + one. This is the only signal of a partial result: check the exit code, and read standard error + for which clients were left out. A client named by both an id and a slug of another client + (a slug can look like an id) is refused as ambiguous; use the id shown by `tenants list`. - Without `--tenant` or `--all-tenants`, a portfolio key is refused by the API, and the CLI tells you to choose. A single-tenant key or sign-in behaves exactly as before; asking it for a client fails with exit code 4 (it lacks the portfolio scope). diff --git a/crates/hodeishield-api/src/client.rs b/crates/hodeishield-api/src/client.rs index c4565a6..2259fb3 100644 --- a/crates/hodeishield-api/src/client.rs +++ b/crates/hodeishield-api/src/client.rs @@ -34,6 +34,10 @@ pub struct Operation { /// open: the server may accept a value that is not listed, so use this to help and to warn, /// never to refuse. Read it through [`Operation::known_values`]. pub known_values: &'static [(&'static str, &'static [&'static str])], + /// Whether a client built for one client tenant selects that tenant on this operation and + /// checks the answer's confirmation. True for the data operations, false for the portfolio + /// listing, which is never scoped. + pub tenant_scoped: bool, } impl Operation { @@ -417,24 +421,15 @@ impl Client { std::thread::sleep(wait); } + /// A GET. The operation decides whether the client's tenant selection applies: scope headers + /// are sent and checked only when `operation.tenant_scoped`. pub(crate) fn get( &self, operation: &'static Operation, path: String, query: Vec<(&'static str, String)>, ) -> Result, Error> { - self.get_with(operation, path, query, true) - } - - /// Like [`Client::get`]; with `scoped` false the client's scope headers are neither sent nor - /// checked, for an operation that sits outside the scope. - pub(crate) fn get_with( - &self, - operation: &'static Operation, - path: String, - query: Vec<(&'static str, String)>, - scoped: bool, - ) -> Result, Error> { + let scoped = operation.tenant_scoped; let url = self.url(&path, &query); let mut rate_limit_retries = 0; let mut transient_retries = 0; @@ -665,6 +660,7 @@ mod tests { summary: "List things", scopes: &[], known_values: &[("severity", &["critical", "high"])], + tenant_scoped: true, }; assert_eq!( operation.known_values("severity"), diff --git a/crates/hodeishield-api/src/generated.rs b/crates/hodeishield-api/src/generated.rs index 9b7ccdd..85e8b98 100644 --- a/crates/hodeishield-api/src/generated.rs +++ b/crates/hodeishield-api/src/generated.rs @@ -1103,6 +1103,7 @@ pub mod operations { summary: "List third parties", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/vendors/{id}`: Fetch one third party. @@ -1113,6 +1114,7 @@ pub mod operations { summary: "Fetch one third party", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/alerts`: List supply-chain alerts. @@ -1123,6 +1125,7 @@ pub mod operations { summary: "List supply-chain alerts", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/alerts/{id}`: Fetch one alert. @@ -1133,6 +1136,7 @@ pub mod operations { summary: "Fetch one alert", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/risks`: List risk-register entries. @@ -1143,6 +1147,7 @@ pub mod operations { summary: "List risk-register entries", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/risks/{id}`: Fetch one risk-register entry. @@ -1153,6 +1158,7 @@ pub mod operations { summary: "Fetch one risk-register entry", scopes: &["supply_risk:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/compliance/controls`: List control status for a framework. @@ -1163,6 +1169,7 @@ pub mod operations { summary: "List control status for a framework", scopes: &["compliance.:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/compliance/frameworks/{framework}`: Framework posture summary. @@ -1173,6 +1180,7 @@ pub mod operations { summary: "Framework posture summary", scopes: &["compliance.:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/evidence`: List security evidence. @@ -1183,6 +1191,7 @@ pub mod operations { summary: "List security evidence", scopes: &["evidence:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/evidence/{id}`: Fetch one evidence item. @@ -1193,6 +1202,7 @@ pub mod operations { summary: "Fetch one evidence item", scopes: &["evidence:read"], known_values: &[], + tenant_scoped: true, }; /// `GET /v1/endpoints`: List enrolled endpoint agents. @@ -1203,6 +1213,7 @@ pub mod operations { summary: "List enrolled endpoint agents", scopes: &["endpoints:read"], known_values: &[], + tenant_scoped: true, }; /// All operations, in document order. diff --git a/crates/hodeishield-api/src/portfolio.rs b/crates/hodeishield-api/src/portfolio.rs index 8be29d7..5c2e800 100644 --- a/crates/hodeishield-api/src/portfolio.rs +++ b/crates/hodeishield-api/src/portfolio.rs @@ -55,6 +55,7 @@ pub const LIST_TENANTS: Operation = Operation { summary: "List the client tenants of the credential's portfolio", scopes: &["portfolio:read"], known_values: &[], + tenant_scoped: false, }; /// A client tenant of the portfolio. Text fields come from the server and are untrusted. @@ -76,13 +77,39 @@ pub struct Tenant { pub licence_status: String, } +/// Whether a [`Tenant`] can be expected to be readable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Readability { + /// Active, with a licence in force or in its grace period. + Readable, + /// Deactivated, or its licence status is `inactive`: the API would answer `403`. + Inactive, + /// Active, with a licence status this client does not know. The vocabulary is open, so such a + /// tenant is neither assumed readable nor assumed inactive. + Unknown, +} + impl Tenant { + /// Whether this tenant is readable, inactive, or has a licence status this client does not + /// know. + #[must_use] + pub fn readability(&self) -> Readability { + if !self.active { + return Readability::Inactive; + } + match self.licence_status.as_str() { + "in_force" | "grace" => Readability::Readable, + "inactive" => Readability::Inactive, + _ => Readability::Unknown, + } + } + /// Whether reading this tenant can be expected to work: it is active and its licence is in /// force or in its grace period. A licence status this client does not know counts as not /// readable, since the vocabulary is open. #[must_use] pub fn readable(&self) -> bool { - self.active && matches!(self.licence_status.as_str(), "in_force" | "grace") + self.readability() == Readability::Readable } } @@ -182,14 +209,15 @@ impl Client { &self, params: &ListTenantsParams, ) -> Result, Error> { - self.get_with( + self.get( &LIST_TENANTS, LIST_TENANTS.path.to_owned(), params.query_pairs(), - false, ) .map_err(|error| match error { - Error::Api(api) if matches!(api.status, 404 | 405) => { + // A 404/405 that carries one of the portfolio contract's own codes comes from an + // API that does offer the portfolio: it stays the API's error. + Error::Api(api) if matches!(api.status, 404 | 405) && !is_portfolio_code(&api) => { Error::PortfolioNotOffered { status: api.status } } other => other, @@ -197,6 +225,16 @@ impl Client { } } +/// Whether the error answer names one of the portfolio contract's `details.code` values. +fn is_portfolio_code(api: &crate::ApiError) -> bool { + api.body + .as_ref() + .and_then(|body| body.details.as_ref()) + .and_then(|details| details.get("code")) + .and_then(serde_json::Value::as_str) + .is_some_and(|code| code.starts_with("TENANT_") || code == "LICENCE_INACTIVE") +} + impl ClientBuilder { /// Makes every data call of the built client read the client tenant `id`. /// @@ -301,6 +339,20 @@ mod tests { assert!(!tenant(true, "IN_FORCE").readable()); } + #[test] + fn inactive_and_unknown_tenants_are_told_apart() { + use Readability::{Inactive, Readable, Unknown}; + assert_eq!(tenant(true, "in_force").readability(), Readable); + assert_eq!(tenant(true, "grace").readability(), Readable); + assert_eq!(tenant(true, "inactive").readability(), Inactive); + assert_eq!(tenant(false, "in_force").readability(), Inactive); + // Deactivated wins over a status nobody knows. + assert_eq!(tenant(false, "suspended_soon").readability(), Inactive); + assert_eq!(tenant(true, "suspended_soon").readability(), Unknown); + assert_eq!(tenant(true, "").readability(), Unknown); + assert_eq!(tenant(true, "IN_FORCE").readability(), Unknown); + } + #[test] fn the_listing_asks_for_ascending_names_when_told_to() { let pairs = ListTenantsParams::alphabetical(Some(2), Some(200)).query_pairs(); diff --git a/crates/hodeishield-api/tests/api.rs b/crates/hodeishield-api/tests/api.rs index 07d98ad..85141e3 100644 --- a/crates/hodeishield-api/tests/api.rs +++ b/crates/hodeishield-api/tests/api.rs @@ -1031,3 +1031,200 @@ fn a_portfolio_key_is_sent_whole_and_never_shown() { let shown = format!("{builder:?}"); assert!(!shown.contains(secret), "{shown}"); } + +// Review follow-ups: the echo is checked on every operation, on every attempt, and only on success. + +fn tenant_client_retrying(server: &mockito::Server, tenant: &str) -> Client { + Client::builder( + server.url().parse().expect("url"), + SecretString::from(PORTFOLIO_KEY), + ) + .tenant(tenant) + .expect("a UUID") + .transient_retries(2, Duration::from_millis(1)) + .rate_limit_retries(1, Duration::from_secs(5)) + .build() + .expect("client") +} + +#[test] +fn every_single_item_operation_checks_the_echo() { + let mut server = mockito::Server::new(); + let any = server + .mock("GET", mockito::Matcher::Any) + .match_header("x-hs-tenant", TENANT_A) + .with_header("x-hs-tenant", TENANT_B) + .with_body("{}") + .expect(5) + .create(); + let c = tenant_client(&server, TENANT_A); + fn refused(r: Result) -> bool { + matches!(r, Err(Error::TenantMismatch { .. })) + } + assert!(refused(c.get_vendor("v1"))); + assert!(refused(c.get_alert("a1"))); + assert!(refused(c.get_risk("r1"))); + assert!(refused(c.get_evidence("e1"))); + assert!(refused(c.get_framework_posture("nis2"))); + any.assert(); +} + +#[test] +fn an_answer_after_a_retry_is_checked_too() { + // A 503 and then a 200 that confirms nothing, or another tenant: refused in both cases. + for echo in [None, Some(TENANT_B)] { + let mut server = mockito::Server::new(); + let unavailable = server + .mock("GET", "/v1/vendors") + .with_status(503) + .expect(1) + .create(); + let mut served = server.mock("GET", "/v1/vendors").with_body(EMPTY_PAGE); + if let Some(echo) = echo { + served = served.with_header("x-hs-tenant", echo); + } + let served = served.expect(1).create(); + let err = tenant_client_retrying(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("unconfirmed answer after a retry"); + assert!( + matches!(err, Error::TenantMismatch { .. }), + "{echo:?}: {err:?}" + ); + unavailable.assert(); + served.assert(); + } + // The same after a 429 that is waited out. + for echo in [None, Some(TENANT_B)] { + let mut server = mockito::Server::new(); + let limited = server + .mock("GET", "/v1/vendors") + .with_status(429) + .with_header("retry-after", "1") + .expect(1) + .create(); + let mut served = server.mock("GET", "/v1/vendors").with_body(EMPTY_PAGE); + if let Some(echo) = echo { + served = served.with_header("x-hs-tenant", echo); + } + let served = served.expect(1).create(); + let err = tenant_client_retrying(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("unconfirmed answer after a wait"); + assert!( + matches!(err, Error::TenantMismatch { .. }), + "{echo:?}: {err:?}" + ); + limited.assert(); + served.assert(); + } + // A confirmed answer after a retry is served. + let mut server = mockito::Server::new(); + server.mock("GET", "/v1/vendors").with_status(503).create(); + server + .mock("GET", "/v1/vendors") + .with_header("x-hs-tenant", TENANT_A) + .with_body(EMPTY_PAGE) + .create(); + tenant_client_retrying(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect("confirmed after a retry"); +} + +#[test] +fn a_redirect_on_a_scoped_client_is_an_error_and_the_target_gets_nothing() { + let mut server = mockito::Server::new(); + let mut elsewhere = mockito::Server::new(); + let never = elsewhere + .mock("GET", mockito::Matcher::Any) + .expect(0) + .create(); + server + .mock("GET", "/v1/vendors") + .match_header("x-hs-tenant", TENANT_A) + .with_status(302) + .with_header("location", &format!("{}/steal", elsewhere.url())) + .with_header("x-hs-tenant", TENANT_A) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("redirect"); + assert_eq!(err.api().map(|a| a.status), Some(302), "{err:?}"); + never.assert(); +} + +#[test] +fn only_a_decodable_success_is_one_even_with_the_right_echo() { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .with_status(204) + .with_header("x-hs-tenant", TENANT_A) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("no body"); + assert!(matches!(err, Error::Decode { .. }), "{err:?}"); + + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .with_status(304) + .with_header("x-hs-tenant", TENANT_A) + .create(); + let err = tenant_client(&server, TENANT_A) + .list_vendors(&ListVendorsParams::default()) + .expect_err("not modified is not data"); + assert_eq!(err.api().map(|a| a.status), Some(304), "{err:?}"); +} + +#[test] +fn the_listing_is_unscoped_by_its_operation_and_the_data_operations_are_scoped() { + use hodeishield_api::portfolio::LIST_TENANTS; + use hodeishield_api::v1::operations; + let listing = LIST_TENANTS; + assert!(!listing.tenant_scoped); + assert!(operations::ALL.len() >= 11); + assert!(operations::ALL.iter().all(|op| op.tenant_scoped)); +} + +#[test] +fn a_404_or_405_with_a_portfolio_code_is_the_apis_own_error() { + use hodeishield_api::portfolio::ListTenantsParams; + for (status, code) in [ + (404_usize, "TENANT_NOT_FOUND"), + (405, "TENANT_SELECTOR_NOT_SUPPORTED"), + (404, "TENANT_SOMETHING_NEW"), + ] { + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .with_status(status) + .with_body(format!( + r#"{{"error":{{"code":"not_found","message":"x","request_id":"r","details":{{"code":"{code}"}}}}}}"# + )) + .create(); + let err = client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect_err("API error"); + assert_eq!( + err.api().map(|a| a.status), + Some(u16::try_from(status).expect("status")), + "{status} {code}: {err:?}" + ); + } + // An unrelated code, or none, still means the route is not there. + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"x","request_id":"r","details":{"code":"SOMETHING_ELSE"}}}"#) + .create(); + let err = client(&server) + .list_tenants(&ListTenantsParams::default()) + .expect_err("no route"); + assert!( + matches!(err, Error::PortfolioNotOffered { status: 404 }), + "{err:?}" + ); +} diff --git a/crates/hodeishield-cli/src/commands/mod.rs b/crates/hodeishield-cli/src/commands/mod.rs index 4339ecd..f10afc3 100644 --- a/crates/hodeishield-cli/src/commands/mod.rs +++ b/crates/hodeishield-cli/src/commands/mod.rs @@ -51,6 +51,7 @@ impl Context { &ApiContext { source, framework: None, + tenant_selected: true, }, ) })?; @@ -84,11 +85,21 @@ impl Context { &ApiContext { source, framework: None, + tenant_selected: tenant.is_some(), }, ) })?; Ok((client, source)) } + + /// What a failure message needs to know about a call made with this context's selection. + pub fn call<'a>(&self, source: CredentialSource, framework: Option<&'a str>) -> ApiContext<'a> { + ApiContext { + source, + framework, + tenant_selected: !matches!(self.selection, Selection::Own), + } + } } pub fn run(cli: Cli, out: &mut dyn Write) -> Result<()> { diff --git a/crates/hodeishield-cli/src/commands/resources.rs b/crates/hodeishield-cli/src/commands/resources.rs index d17459e..6dafccd 100644 --- a/crates/hodeishield-cli/src/commands/resources.rs +++ b/crates/hodeishield-cli/src/commands/resources.rs @@ -158,6 +158,16 @@ fn list_across( let Some(tenant) = &target.tenant else { continue; }; + if target.unknown_licence { + // The licence vocabulary is open. Not read, but not silently dropped either. + eprintln!( + "warning: client tenant {}: licence status `{}` is not known to this version; not read", + clean(&tenant.slug), + clean(&tenant.licence_status) + ); + failures.push(Failure::general("licence status not known")); + continue; + } let walked = collect_pages( noun, &api, @@ -204,7 +214,7 @@ fn list_across( } let rows = render(items); headers.get_or_insert(rows.headers); - groups.push((tenant.slug.clone(), rows.rows)); + groups.push((tenant.slug.clone(), tenant.id.clone(), rows.rows)); } if let Some(headers) = headers { print_table(out, &tenant_table(headers, groups))?; @@ -315,11 +325,8 @@ fn show( Ok(print_detail(out, &value)?) } -fn plain(source: CredentialSource) -> ApiContext<'static> { - ApiContext { - source, - framework: None, - } +fn plain(ctx: &Context, source: CredentialSource) -> ApiContext<'static> { + ctx.call(source, None) } pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> Result<()> { @@ -341,7 +348,7 @@ pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> R out, &args.paging, "vendors", - plain(source), + plain(ctx, source), &targets, &mut |client, page, per_page| { client.list_vendors(&ListVendorsParams { @@ -356,7 +363,9 @@ pub fn vendors(ctx: &Context, command: VendorsCommand, out: &mut dyn Write) -> R } VendorsCommand::Get { id } => { let (client, source) = ctx.single(&settings)?; - show(ctx, out, plain(source), client.get_vendor(&id), |r| &r.data) + show(ctx, out, plain(ctx, source), client.get_vendor(&id), |r| { + &r.data + }) } } } @@ -403,7 +412,7 @@ pub fn alerts(ctx: &Context, command: AlertsCommand, out: &mut dyn Write) -> Res out, &args.paging, "alerts", - plain(source), + plain(ctx, source), &targets, &mut |client, page, per_page| { client.list_alerts(&ListAlertsParams { @@ -418,7 +427,9 @@ pub fn alerts(ctx: &Context, command: AlertsCommand, out: &mut dyn Write) -> Res } AlertsCommand::Get { id } => { let (client, source) = ctx.single(&settings)?; - show(ctx, out, plain(source), client.get_alert(&id), |r| &r.data) + show(ctx, out, plain(ctx, source), client.get_alert(&id), |r| { + &r.data + }) } } } @@ -468,7 +479,7 @@ pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Resul out, &args.paging, "risks", - plain(source), + plain(ctx, source), &targets, &mut |client, page, per_page| { client.list_risks(&ListRisksParams { @@ -483,7 +494,9 @@ pub fn risks(ctx: &Context, command: RisksCommand, out: &mut dyn Write) -> Resul } RisksCommand::Get { id } => { let (client, source) = ctx.single(&settings)?; - show(ctx, out, plain(source), client.get_risk(&id), |r| &r.data) + show(ctx, out, plain(ctx, source), client.get_risk(&id), |r| { + &r.data + }) } } } @@ -517,10 +530,7 @@ pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write &[("status", args.status.as_deref())], ); let (targets, source) = ctx.targets(&settings)?; - let api = ApiContext { - source, - framework: Some(&args.framework), - }; + let api = ctx.call(source, Some(&args.framework)); let mut base = ListComplianceControlsParams::new(args.framework.clone()); base.status = args.status; base.sort = args.sort; @@ -545,10 +555,7 @@ pub fn compliance(ctx: &Context, command: ComplianceCommand, out: &mut dyn Write } ComplianceCommand::Posture { framework } => { let (client, source) = ctx.single(&settings)?; - let api = ApiContext { - source, - framework: Some(&framework), - }; + let api = ctx.call(source, Some(&framework)); let response = client .get_framework_posture(&framework) .map_err(|e| from_api(e, &api))?; @@ -614,7 +621,7 @@ pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> out, &args.paging, "evidence items", - plain(source), + plain(ctx, source), &targets, &mut |client, page, per_page| { client.list_evidence(&ListEvidenceParams { @@ -629,9 +636,13 @@ pub fn evidence(ctx: &Context, command: EvidenceCommand, out: &mut dyn Write) -> } EvidenceCommand::Get { id } => { let (client, source) = ctx.single(&settings)?; - show(ctx, out, plain(source), client.get_evidence(&id), |r| { - &r.data - }) + show( + ctx, + out, + plain(ctx, source), + client.get_evidence(&id), + |r| &r.data, + ) } } } @@ -671,7 +682,7 @@ pub fn endpoints(ctx: &Context, command: EndpointsCommand, out: &mut dyn Write) out, &args.paging, "endpoints", - plain(source), + plain(ctx, source), &targets, &mut |client, page, per_page| { client.list_endpoints(&ListEndpointsParams { @@ -723,6 +734,7 @@ mod tests { let api = ApiContext { source: CredentialSource::ApiKey, framework: None, + tenant_selected: false, }; let total_pages = (total + per_page - 1) / per_page; collect_pages( diff --git a/crates/hodeishield-cli/src/commands/tenants.rs b/crates/hodeishield-cli/src/commands/tenants.rs index 43205b0..2bd64ba 100644 --- a/crates/hodeishield-cli/src/commands/tenants.rs +++ b/crates/hodeishield-cli/src/commands/tenants.rs @@ -13,7 +13,10 @@ use crate::config::Settings; use crate::failure::{ApiContext, CredentialSource, Failure, Kind, Result}; use crate::output::{Rows, clean}; use hodeishield_api::Client; -use hodeishield_api::portfolio::{ListTenantsParams, ListTenantsResponse, Tenant, is_uuid}; +use hodeishield_api::portfolio::{ + ListTenantsParams, ListTenantsResponse, Readability, Tenant, is_uuid, +}; +use std::collections::HashSet; use std::io::Write; /// Which client tenant the read commands read. @@ -33,6 +36,9 @@ pub enum Selection { pub struct Target { pub client: Client, pub tenant: Option, + /// Set for a tenant of `--all-tenants` whose licence status this version does not know: it is + /// not read, and the walk reports it and fails at the end. + pub unknown_licence: bool, } impl Context { @@ -46,6 +52,7 @@ impl Context { vec![Target { client, tenant: None, + unknown_licence: false, }], source, )); @@ -65,24 +72,34 @@ impl Context { vec![tenant] } _ => { - let (usable, idle): (Vec<_>, Vec<_>) = - portfolio.into_iter().partition(Tenant::readable); - if !idle.is_empty() { + // Inactive tenants are skipped on purpose. A tenant whose licence status this + // version does not know is not read either, but it stays in the list so the walk + // can report it and fail: nothing is silently missing. + let idle = portfolio + .iter() + .filter(|t| t.readability() == Readability::Inactive) + .count(); + if idle > 0 { eprintln!( - "Skipping {} inactive client tenants (see `hodeishield tenants list`).", - idle.len() + "Skipping {idle} inactive client tenants (see `hodeishield tenants list`)." ); } - usable + portfolio + .into_iter() + .filter(|t| t.readability() != Readability::Inactive) + .collect() } }; let targets = tenants .into_iter() .map(|tenant| { let (client, _) = self.api(settings, Some(&tenant.id))?; + let unknown_licence = matches!(selected, Selection::All) + && tenant.readability() == Readability::Unknown; Ok(Target { client, tenant: Some(tenant), + unknown_licence, }) }) .collect::>()?; @@ -107,13 +124,19 @@ fn list_portfolio(lister: &Client, source: CredentialSource) -> Result = Vec::new(); + let (_, mut tenants) = collect_pages( "client tenants", &api, Some(200), 100_000, - &mut |page, per_page| lister.list_tenants(&ListTenantsParams::alphabetical(page, per_page)), + &mut |page, per_page| { + let response = lister.list_tenants(&ListTenantsParams::alphabetical(page, per_page))?; + totals.push(response.data.pagination.total); + Ok(response) + }, |r: ListTenantsResponse| (r.data, r.pagination), ) .map_err(|failure| { @@ -126,19 +149,32 @@ fn list_portfolio(lister: &Client, source: CredentialSource) -> Result>() + .join(", then ") + ); + } + let mut seen = HashSet::new(); + tenants.retain(|tenant| seen.insert(tenant.id.to_ascii_lowercase())); Ok(tenants) } /// The tenant the user named, by id or slug. One that is not in the portfolio is reported the way /// the API reports it: not found, with no hint about whether it exists elsewhere. fn find(portfolio: &[Tenant], wanted: &str) -> Result { + // An input shaped like a UUID may be an id, but a slug can have that shape too: if it matches + // an id of one tenant and the slug of another, it is ambiguous, never resolved to one of them. let by_id = is_uuid(wanted); let mut matches = portfolio.iter().filter(|t| { - if by_id { - t.id.eq_ignore_ascii_case(wanted) - } else { - t.slug.eq_ignore_ascii_case(wanted) - } + t.slug.eq_ignore_ascii_case(wanted) || (by_id && t.id.eq_ignore_ascii_case(wanted)) }); match (matches.next(), matches.next()) { (Some(tenant), None) => Ok(tenant.clone()), @@ -170,6 +206,7 @@ pub fn run(ctx: &Context, command: TenantsCommand, out: &mut dyn Write) -> Resul ApiContext { source, framework: None, + tenant_selected: false, }, &mut |page, per_page| client.list_tenants(&ListTenantsParams::alphabetical(page, per_page)), |r| (r.data, r.pagination), diff --git a/crates/hodeishield-cli/src/failure.rs b/crates/hodeishield-cli/src/failure.rs index 1af55fa..f074a04 100644 --- a/crates/hodeishield-cli/src/failure.rs +++ b/crates/hodeishield-cli/src/failure.rs @@ -195,7 +195,13 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail "All portfolio keys of your organisation share this limit; another key \ or script may be using it.", ), - Some("portfolio_tenant" | "tenant") => Some( + Some("portfolio_tenant") => Some( + "This client tenant's own limit is used up; other client tenants are \ + not affected.", + ), + // The plain `tenant` limit is also what a single-tenant key hits: the + // client wording fits only when a client tenant was selected. + Some("tenant") if context.tenant_selected => Some( "This client tenant's own limit is used up; other client tenants are \ not affected.", ), @@ -308,6 +314,8 @@ pub struct ApiContext<'a> { pub source: CredentialSource, /// The framework of a compliance call, to fill `compliance.:read`. pub framework: Option<&'a str>, + /// Whether the call reads a selected client tenant (`--tenant` or `--all-tenants`). + pub tenant_selected: bool, } impl ApiContext<'_> { diff --git a/crates/hodeishield-cli/src/filters.rs b/crates/hodeishield-cli/src/filters.rs index 0f7805f..ba891a2 100644 --- a/crates/hodeishield-cli/src/filters.rs +++ b/crates/hodeishield-cli/src/filters.rs @@ -111,6 +111,7 @@ mod tests { summary: "List alerts", scopes: &[], known_values: &[("severity", &["critical", "high", "medium", "low"])], + tenant_scoped: true, }; const WITHOUT_LISTS: Operation = Operation { @@ -154,6 +155,7 @@ mod tests { const ENGLISH: Operation = Operation { known_values: &[("severity", &["critical", "high", "medium", "low", "info"])], + tenant_scoped: true, ..WITH_LISTS }; @@ -207,6 +209,7 @@ mod tests { // A list without the new name: nothing to point to, so the generic warning stays. let other_list = Operation { known_values: &[("severity", &["urgent"])], + tenant_scoped: true, ..WITH_LISTS }; let out = warnings(&other_list, &[("severity", Some("bajo"))]); @@ -221,6 +224,7 @@ mod tests { let other = Operation { id: "listRisks", known_values: &[("severity", &["high"])], + tenant_scoped: true, ..WITH_LISTS }; let out = warnings(&other, &[("severity", Some("alto"))]); diff --git a/crates/hodeishield-cli/src/output.rs b/crates/hodeishield-cli/src/output.rs index a7149fd..b4e3ee2 100644 --- a/crates/hodeishield-cli/src/output.rs +++ b/crates/hodeishield-cli/src/output.rs @@ -140,15 +140,31 @@ impl Rows { } } -/// Rows of several client tenants in one table, each led by the tenant's slug. The slug is the -/// server's text, so it is sanitised like every other cell. -pub fn tenant_table(headers: &[&str], groups: Vec<(String, Vec>)>) -> Table { +/// What leads a row of a multi-tenant table. A slug made only of `[a-z0-9-]` is shown as it is. Any +/// other slug (it may hold look-alike letters, so that two tenants would read the same) is shown +/// with the start of the tenant's id, which differs. `--json` and `--csv` carry the full values. +pub fn tenant_label(slug: &str, id: &str) -> String { + let plain = !slug.is_empty() + && slug + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-'); + if plain { + slug.to_owned() + } else { + let short: String = id.chars().take(8).collect(); + format!("{} [{}]", clean(slug), clean(&short)) + } +} + +/// Rows of several client tenants in one table, each led by the tenant's slug (see +/// [`tenant_label`]). The text is the server's, so it is sanitised like every other cell. +pub fn tenant_table(headers: &[&str], groups: Vec<(String, String, Vec>)>) -> Table { let mut all = vec!["TENANT"]; all.extend_from_slice(headers); let rows = groups .into_iter() - .flat_map(|(slug, rows)| { - let label = clean(&slug); + .flat_map(|(slug, id, rows)| { + let label = tenant_label(&slug, &id); rows.into_iter().map(move |mut row| { row.insert(0, label.clone()); row @@ -458,9 +474,14 @@ mod tests { let t = tenant_table( &["ID"], vec![ - ("acme".to_owned(), vec![vec!["1".into()]]), + ( + "acme".to_owned(), + "11111111-0000".to_owned(), + vec![vec!["1".into()]], + ), ( "ev\u{1b}il".to_owned(), + "22222222-0000".to_owned(), vec![vec!["2".into()], vec!["3".into()]], ), ], @@ -477,10 +498,22 @@ mod tests { lines[1].starts_with("acme") && lines[1].ends_with('1'), "{text}" ); - assert!(lines[2].starts_with("ev\u{fffd}il"), "{text:?}"); + assert!(lines[2].starts_with("ev\u{fffd}il [22222222]"), "{text:?}"); assert!(!text.contains('\u{1b}')); } + #[test] + fn only_plain_slugs_stand_alone_in_the_tenant_column() { + let id = "0b8f2a52-6a0e-4d5c-9d3e-1f2a3b4c5d6e"; + assert_eq!(tenant_label("acme-2", id), "acme-2"); + // Cyrillic "а" (U+0430) looks like Latin "a"; the id tells the tenants apart. + assert_eq!(tenant_label("\u{430}cme", id), "\u{430}cme [0b8f2a52]"); + assert_eq!(tenant_label("Acme", id), "Acme [0b8f2a52]"); + assert_eq!(tenant_label("ac me", id), "ac me [0b8f2a52]"); + assert_eq!(tenant_label("", id), " [0b8f2a52]"); + assert_ne!(tenant_label("acme", id), tenant_label("\u{430}cme", id)); + } + #[test] fn tables_have_no_trailing_spaces() { let t = table( diff --git a/crates/hodeishield-cli/tests/cli.rs b/crates/hodeishield-cli/tests/cli.rs index a06d2c6..9c015cf 100644 --- a/crates/hodeishield-cli/tests/cli.rs +++ b/crates/hodeishield-cli/tests/cli.rs @@ -1305,7 +1305,6 @@ fn a_portfolio_key_in_its_published_format_is_sent_as_a_bearer_and_never_shown() fn all_tenants_skips_what_is_not_readable_by_the_contract_rule() { let env = Env::new(); let mut server = mockito::Server::new(); - let strange = "44444444-4444-4444-8444-444444444444"; let off = "55555555-5555-4555-8555-555555555555"; mock_portfolio( &mut server, @@ -1315,7 +1314,6 @@ fn all_tenants_skips_what_is_not_readable_by_the_contract_rule() { // Listed by the server, but a client reading them would get a 403. tenant(OLD, "old", true, "inactive"), tenant(off, "off", false, "in_force"), - tenant(strange, "strange", true, "a_status_from_the_future"), ], ); let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); @@ -1328,7 +1326,7 @@ fn all_tenants_skips_what_is_not_readable_by_the_contract_rule() { assert!(output.status.success(), "{output:?}"); let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("Skipping 3 inactive client tenants"), + stderr.contains("Skipping 2 inactive client tenants"), "{stderr}" ); assert!( @@ -1557,3 +1555,466 @@ fn an_echo_in_the_wrong_form_prints_nothing() { assert_eq!(output.status.code(), Some(1), "{output:?}"); assert!(output.stdout.is_empty()); } + +// Review follow-ups. + +const STRANGE: &str = "44444444-4444-4444-8444-444444444444"; + +/// A tenant with a free-form name and slug (quotes and non-ASCII letters included). +fn tenant_named(id: &str, slug: &str, name: &str) -> String { + serde_json::json!({ + "id": id, + "name": name, + "slug": slug, + "organization": {"id": "o", "name": "Group"}, + "internal": false, + "active": true, + "licence_status": "in_force", + }) + .to_string() +} + +fn never_read(server: &mut mockito::Server) -> mockito::Mock { + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .expect(0) + .create() +} + +#[test] +fn an_unknown_licence_status_is_not_read_is_named_and_fails_the_command() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant(ACME, "acme", true, "in_force"), + tenant(OLD, "old", true, "inactive"), + tenant(STRANGE, "strange", true, "a_status_from_the_future"), + ], + ); + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let strange = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", STRANGE) + .expect(0) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Acme Hosting"), "{stdout}"); + let stderr = String::from_utf8_lossy(&output.stderr); + // "inactive" is said of the inactive tenant only. + assert!( + stderr.contains("Skipping 1 inactive client tenants"), + "{stderr}" + ); + assert!( + stderr.contains( + "warning: client tenant strange: licence status `a_status_from_the_future` is not known to this version; not read" + ), + "{stderr}" + ); + assert!( + stderr.contains("Could not read 1 of 2 client tenants"), + "{stderr}" + ); + acme.assert(); + strange.assert(); +} + +#[test] +fn a_walk_where_one_tenant_is_mismatched_prints_nothing() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + // Beta is asked for and answers with Acme's confirmation. + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", BETA) + .with_header("x-hs-tenant", ACME) + .with_body(page(&[vendor("v9", "Acme Hosting")], 1, 1, 1)) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(output.stdout.is_empty(), "{output:?}"); +} + +#[test] +fn a_second_page_that_is_not_confirmed_prints_nothing() { + let env = Env::new(); + for args in [ + &["--tenant", "acme", "vendors", "list", "--all"][..], + &["--all-tenants", "vendors", "list", "--all"][..], + ] { + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &[tenant(ACME, "acme", true, "in_force")]); + let page_of = |n: i64, name: &str| page(&[vendor(&format!("v{n}"), name)], n, 120, 3); + for (n, echo) in [(1_i64, ACME), (2, BETA)] { + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::UrlEncoded("page".into(), n.to_string())) + .match_header("x-hs-tenant", ACME) + .with_header("x-hs-tenant", echo) + .with_body(page_of(n, &format!("Item {n}"))) + .create(); + } + let output = env.api(&server).args(args).output().expect("run"); + assert_eq!(output.status.code(), Some(1), "{args:?}: {output:?}"); + assert!(output.stdout.is_empty(), "{args:?}: {output:?}"); + } +} + +#[test] +fn a_walk_stops_with_nothing_on_stdout_when_a_later_tenant_fails() { + let env = Env::new(); + let scope = r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"INSUFFICIENT_SCOPE","missing":["supply_risk:read"]}}}"#; + let unauthorised = r#"{"error":{"code":"unauthorized","message":"x","request_id":"r"}}"#; + let down = r#"{"error":{"code":"unavailable","message":"x","request_id":"r"}}"#; + for (status, body, expected) in [(401, unauthorised, 3), (503, down, 7), (403, scope, 4)] { + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", BETA) + .with_status(status) + // A zero wait keeps the retries of the 503 quick. + .with_header("retry-after", "0") + .with_body(body) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(expected), "{status}: {output:?}"); + assert!(output.stdout.is_empty(), "{status}: {output:?}"); + } +} + +#[test] +fn tenant_labels_in_csv_cannot_run_as_formulas() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant_named(ACME, "+cmd", r#"=HYPERLINK("http://example.invalid","x")"#), + tenant_named(BETA, "beta", "-2+3"), + ], + ); + mock_vendors(&mut server, ACME, &["Acme Hosting"]); + mock_vendors(&mut server, BETA, &["Beta Hosting"]); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list", "--csv"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let text = String::from_utf8_lossy(&output.stdout); + assert!( + text.contains(&format!( + "{ACME},'+cmd,\"'=HYPERLINK(\"\"http://example.invalid\"\",\"\"x\"\")\"," + )), + "{text}" + ); + assert!(text.contains(&format!("{BETA},beta,'-2+3,")), "{text}"); + assert!( + !text.contains(",+cmd,") && !text.contains(",=HYPERLINK"), + "{text}" + ); +} + +#[test] +fn slug_resolution_is_case_insensitive_and_never_guesses() { + let env = Env::new(); + // Mixed-case input finds the slug. + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let output = env + .api(&server) + .args(["--tenant", "AcMe", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + acme.assert(); + + // Two slugs that differ only in case are ambiguous. + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant_named(ACME, "Acme", "One"), + tenant_named(BETA, "acme", "Two"), + ], + ); + let data = never_read(&mut server); + let output = env + .api(&server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("More than one client tenant")); + assert!(output.stdout.is_empty()); + data.assert(); + + // A slug with a Cyrillic "а" is not matched by the ASCII word. + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[tenant_named(ACME, "\u{430}cme", "Look-alike")], + ); + let data = never_read(&mut server); + let output = env + .api(&server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(5), "{output:?}"); + data.assert(); + + // A slug shaped like a UUID that is also another tenant's id is ambiguous. + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant_named(ACME, BETA, "Slug is an id"), + tenant_named(BETA, "beta", "Has that id"), + ], + ); + let data = never_read(&mut server); + let output = env + .api(&server) + .args(["--tenant", BETA, "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("More than one client tenant")); + assert!(output.stdout.is_empty()); + data.assert(); +} + +#[test] +fn an_id_that_is_not_a_uuid_fails_before_any_data_request() { + let env = Env::new(); + for args in [ + &["--all-tenants", "vendors", "list"][..], + &["--tenant", "weird", "vendors", "list"][..], + ] { + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant(ACME, "acme", true, "in_force"), + tenant("not-a-uuid", "weird", true, "in_force"), + ], + ); + let data = never_read(&mut server); + let output = env.api(&server).args(args).output().expect("run"); + assert_eq!(output.status.code(), Some(1), "{args:?}: {output:?}"); + assert!(output.stdout.is_empty()); + data.assert(); + } +} + +#[test] +fn a_portfolio_key_without_a_selection_sends_no_tenant_header() { + let env = Env::new(); + let mut server = mockito::Server::new(); + let required = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", mockito::Matcher::Missing) + .with_status(400) + .with_body(r#"{"error":{"code":"invalid_request","message":"X-HS-Tenant is required","request_id":"r4","details":{"code":"TENANT_REQUIRED"}}}"#) + .expect(1) + .create(); + let output = portfolio_cmd(&env, &server) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(1), "{output:?}"); + assert!(String::from_utf8_lossy(&output.stderr).contains("--all-tenants")); + required.assert(); +} + +#[test] +fn look_alike_slugs_are_told_apart_by_id_in_the_table_only() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio( + &mut server, + &[ + tenant_named(ACME, "acme", "Real"), + tenant_named(BETA, "\u{430}cme", "Look-alike"), + ], + ); + mock_vendors(&mut server, ACME, &["Real Hosting"]); + mock_vendors(&mut server, BETA, &["Fake Hosting"]); + let table = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + let stdout = String::from_utf8_lossy(&table.stdout); + assert!( + stdout.lines().any(|l| l.starts_with("acme ")) && stdout.contains("\u{430}cme [22222222]"), + "{stdout}" + ); + let json = env + .api(&server) + .args(["--json", "--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + let items: serde_json::Value = serde_json::from_slice(&json.stdout).expect("json"); + assert_eq!(items[1]["tenant_slug"], "\u{430}cme"); +} + +#[test] +fn a_tenant_listed_on_two_pages_is_read_once_and_a_changing_total_is_reported() { + let env = Env::new(); + let mut server = mockito::Server::new(); + for (n, items, total) in [ + ( + 1_i64, + vec![ + tenant(ACME, "acme", true, "in_force"), + tenant(BETA, "beta", true, "grace"), + ], + 120, + ), + (2, vec![tenant(BETA, "beta", true, "grace")], 121), + ] { + server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::UrlEncoded("page".into(), n.to_string())) + .with_body(page(&items, n, total, 2)) + .create(); + } + let acme = mock_vendors(&mut server, ACME, &["Acme Hosting"]); + let beta = server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .match_header("x-hs-tenant", BETA) + .with_header("x-hs-tenant", BETA) + .with_body(page(&[vendor("vb", "Beta Hosting")], 1, 1, 1)) + .expect(1) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert!(output.status.success(), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("changed while they were listed (120, then 121)"), + "{stderr}" + ); + assert!( + stderr.contains("2 vendors across 2 client tenants."), + "{stderr}" + ); + acme.assert(); + beta.assert(); +} + +#[test] +fn a_404_with_a_portfolio_code_on_the_listing_is_not_taken_for_a_missing_feature() { + let env = Env::new(); + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/portfolio/tenants") + .match_query(mockito::Matcher::Any) + .with_status(404) + .with_body(r#"{"error":{"code":"not_found","message":"No such tenant","request_id":"r","details":{"code":"TENANT_NOT_FOUND"}}}"#) + .create(); + let output = env + .api(&server) + .args(["--all-tenants", "vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(5), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("does not offer portfolio access"), + "{stderr}" + ); +} + +#[test] +fn the_client_wording_of_a_tenant_rate_limit_needs_a_selected_client() { + let env = Env::new(); + let limit = |scope: &str| { + format!( + r#"{{"error":{{"code":"rate_limited","message":"x","request_id":"r","details":{{"scope":"{scope}","retry_after_seconds":600}}}}}}"# + ) + }; + let wording = "own limit is used up"; + // A single-tenant key, no selection: the previous message, with no client advice. + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(429) + .with_header("retry-after", "600") + .with_body(limit("tenant")) + .create(); + let output = env + .api(&server) + .args(["vendors", "list"]) + .output() + .expect("run"); + assert_eq!(output.status.code(), Some(6), "{output:?}"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("(limit: tenant)") && !stderr.contains(wording), + "{stderr}" + ); + + // With a client selected, or for the portfolio's per-client limit, the advice is given. + for (scope, args) in [ + ("tenant", &["--tenant", "acme", "vendors", "list"][..]), + ( + "portfolio_tenant", + &["--tenant", "acme", "vendors", "list"][..], + ), + ("portfolio_tenant", &["vendors", "list"][..]), + ] { + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(429) + .with_header("retry-after", "600") + .with_body(limit(scope)) + .create(); + let output = env.api(&server).args(args).output().expect("run"); + assert_eq!( + output.status.code(), + Some(6), + "{scope} {args:?}: {output:?}" + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains(wording), + "{scope} {args:?}: {output:?}" + ); + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 33acc18..a6304ea 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -758,7 +758,7 @@ impl<'a> Generator<'a> { .collect(); let _ = writeln!( out, - " /// `GET {}`: {}.\n pub const {}: Operation = Operation {{ id: {:?}, method: \"GET\", path: {:?}, summary: {:?}, scopes: &[{}], known_values: &[{}] }};\n", + " /// `GET {}`: {}.\n pub const {}: Operation = Operation {{ id: {:?}, method: \"GET\", path: {:?}, summary: {:?}, scopes: &[{}], known_values: &[{}], tenant_scoped: {} }};\n", op.path, op.summary, screaming(&op.id), @@ -766,7 +766,10 @@ impl<'a> Generator<'a> { op.path, op.summary, scopes.join(", "), - known.join(", ") + known.join(", "), + // A client tenant is selected on the data operations only; anything under the + // portfolio path (the tenant listing) is outside the selection. + !op.path.starts_with("/v1/portfolio/") ); } let all: Vec = self.operations.iter().map(|op| screaming(&op.id)).collect(); From 6a1854fecc92d0495bb031d2f05f1cbf1f0def85 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Thu, 8 Oct 2026 11:17:29 +0200 Subject: [PATCH 07/10] Align the portfolio wording with the published contract Name the role that creates a portfolio key (an organisation administrator of an active MSSP organisation), use the portfolio key name in the changelog, spell the listing column with the same British spelling as the rest of the text, and show a client organisation in the example. Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- CHANGELOG.md | 5 +++-- README.md | 10 ++++++---- crates/hodeishield-cli/src/commands/tenants.rs | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 191101a..3cd6da8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,8 +27,9 @@ commands, `--json` output and exit codes. Each release's notes start with its se for each stable release: the release's own binary on a distroless base, running as a non-root user, tagged with the exact version only. It is signed by digest with Sigstore and carries the release's SBOM as a signed attestation. ([#36](https://github.com/Hodeitek/hodeishield-cli/issues/36)) -- Several client tenants with one MSSP credential. `hodeishield tenants list` lists the client - tenants the credential may read. The global `--tenant ` selects one client for any read +- Several client tenants with one portfolio key. `hodeishield tenants list` lists the client + tenants the key may read. The key is created by an organisation administrator of an active MSSP + organisation and goes in `HODEISHIELD_API_KEY`. The global `--tenant ` selects one client for any read command, and `--all-tenants` reads every client on a `list` command and adds `tenant_id`, `tenant_slug` and `tenant_name` to each row in `--json` and `--csv` (a `TENANT` column in the table). An unknown client fails with exit code 5, before any data is read. Without these options diff --git a/README.md b/README.md index c6ea267..ff4b18e 100644 --- a/README.md +++ b/README.md @@ -355,8 +355,10 @@ what it accepts. ### Several tenants (MSSP) -An MSSP reads all of its client tenants with **one** credential: a portfolio key, created in the -app by an administrator of an active MSSP organisation, in the MSSP area (API keys). You do not need +An MSSP reads all of its client tenants with **one** credential: a **portfolio key**, created in the +app by an organisation administrator of an active MSSP organisation, in the MSSP area (API keys). +Nobody else can create one: not a tenant administrator, and not a platform operator acting outside +an MSSP organisation. You do not need a profile, a key or a sign-in per client, and you should not set them up that way. A portfolio key reads only, always expires (90 days by default, 365 at most), and works on the @@ -378,8 +380,8 @@ key. The same variable takes a tenant API key as before. ```console $ hodeishield tenants list -SLUG NAME ORGANIZATION TYPE STATUS LICENCE ID -acme Acme S.L. - client active in_force 11111111-1111-4111-8111-111111111111 +SLUG NAME ORGANISATION TYPE STATUS LICENCE ID +acme Acme S.L. Acme Group client active in_force 11111111-1111-4111-8111-111111111111 beta Beta Corp Beta Group client active grace 22222222-2222-4222-8222-222222222222 ``` diff --git a/crates/hodeishield-cli/src/commands/tenants.rs b/crates/hodeishield-cli/src/commands/tenants.rs index 2bd64ba..81b6172 100644 --- a/crates/hodeishield-cli/src/commands/tenants.rs +++ b/crates/hodeishield-cli/src/commands/tenants.rs @@ -226,7 +226,7 @@ fn tenant_rows(items: &[Tenant]) -> Rows { &[ "SLUG", "NAME", - "ORGANIZATION", + "ORGANISATION", "TYPE", "STATUS", "LICENCE", From 6883074cb7566c9e40651aac3f15c452eb481613 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Thu, 8 Oct 2026 11:26:56 +0200 Subject: [PATCH 08/10] Document portfolio key rotation and point portfolio keys to an organisation administrator Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- README.md | 4 +- crates/hodeishield-cli/src/auth/mod.rs | 5 +++ crates/hodeishield-cli/src/commands/mod.rs | 10 +++++ .../hodeishield-cli/src/commands/resources.rs | 1 + .../hodeishield-cli/src/commands/tenants.rs | 12 +++++- crates/hodeishield-cli/src/failure.rs | 10 ++++- crates/hodeishield-cli/tests/cli.rs | 42 +++++++++++++++++++ 7 files changed, 80 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ff4b18e..426b659 100644 --- a/README.md +++ b/README.md @@ -364,7 +364,9 @@ a profile, a key or a sign-in per client, and you should not set them up that wa A portfolio key reads only, always expires (90 days by default, 365 at most), and works on the HodeiShield API only. It can be limited when it is created to some of your clients and to source IP ranges; a request from another address is refused like an unknown key (exit code 3). Signing in -with `hodeishield login` does not cover a whole portfolio: use the key. +with `hodeishield login` does not cover a whole portfolio: use the key. Rotate your portfolio keys +before they expire, so scripts do not stop working; a new key and its predecessor both work for up +to 7 days while the old one is in force. **1. Use the credential as any other key.** diff --git a/crates/hodeishield-cli/src/auth/mod.rs b/crates/hodeishield-cli/src/auth/mod.rs index b7a7acd..53df31b 100644 --- a/crates/hodeishield-cli/src/auth/mod.rs +++ b/crates/hodeishield-cli/src/auth/mod.rs @@ -33,6 +33,11 @@ pub fn api_key_from_env() -> Option { .map(SecretString::from) } +/// Whether a credential string is a portfolio key (starts with `hsp_`). +pub fn is_portfolio_key(credential: &SecretString) -> bool { + credential.expose_secret().starts_with("hsp_") +} + /// The credential for an API call: the environment's API key, else the profile's sign-in token, /// refreshed if it is about to expire. pub fn credential_for_api( diff --git a/crates/hodeishield-cli/src/commands/mod.rs b/crates/hodeishield-cli/src/commands/mod.rs index f10afc3..0174e5e 100644 --- a/crates/hodeishield-cli/src/commands/mod.rs +++ b/crates/hodeishield-cli/src/commands/mod.rs @@ -13,6 +13,7 @@ use crate::cli::{Cli, Command}; use crate::config::{Overrides, Settings}; use crate::failure::{ApiContext, CredentialSource, Result, from_api}; use hodeishield_api::Client; +use std::cell::Cell; use std::io::Write; use std::sync::Arc; use tenants::Selection; @@ -23,6 +24,8 @@ pub struct Context { pub verbose: bool, pub selection: Selection, pub store: Arc, + /// Cached information about whether the credential is a portfolio key, set on first API call. + is_portfolio_key: Cell>, } impl Context { @@ -38,6 +41,9 @@ impl Context { tenant: Option<&str>, ) -> Result<(Client, CredentialSource)> { let (credential, source) = auth::credential_for_api(settings, self.store.as_ref())?; + let is_portfolio = + source == CredentialSource::ApiKey && auth::is_portfolio_key(&credential); + self.is_portfolio_key.set(Some(is_portfolio)); let mut builder = Client::builder(settings.api_url.clone(), credential).user_agent(crate::USER_AGENT); if source == CredentialSource::OAuth { @@ -52,6 +58,7 @@ impl Context { source, framework: None, tenant_selected: true, + is_portfolio_key: is_portfolio, }, ) })?; @@ -86,6 +93,7 @@ impl Context { source, framework: None, tenant_selected: tenant.is_some(), + is_portfolio_key: is_portfolio, }, ) })?; @@ -98,6 +106,7 @@ impl Context { source, framework, tenant_selected: !matches!(self.selection, Selection::Own), + is_portfolio_key: self.is_portfolio_key.get().unwrap_or(false), } } } @@ -116,6 +125,7 @@ pub fn run(cli: Cli, out: &mut dyn Write) -> Result<()> { (None, false) => Selection::Own, }, store: Arc::new(auth::store::Keychain), + is_portfolio_key: Cell::new(None), }; match cli.command { Command::Vendors(command) => resources::vendors(&ctx, command, out), diff --git a/crates/hodeishield-cli/src/commands/resources.rs b/crates/hodeishield-cli/src/commands/resources.rs index 6dafccd..11343d8 100644 --- a/crates/hodeishield-cli/src/commands/resources.rs +++ b/crates/hodeishield-cli/src/commands/resources.rs @@ -735,6 +735,7 @@ mod tests { source: CredentialSource::ApiKey, framework: None, tenant_selected: false, + is_portfolio_key: false, }; let total_pages = (total + per_page - 1) / per_page; collect_pages( diff --git a/crates/hodeishield-cli/src/commands/tenants.rs b/crates/hodeishield-cli/src/commands/tenants.rs index 81b6172..4a25c08 100644 --- a/crates/hodeishield-cli/src/commands/tenants.rs +++ b/crates/hodeishield-cli/src/commands/tenants.rs @@ -60,7 +60,8 @@ impl Context { selected => selected, }; let (lister, source) = self.api(settings, None)?; - let portfolio = list_portfolio(&lister, source)?; + let is_portfolio = self.is_portfolio_key.get().unwrap_or(false); + let portfolio = list_portfolio(&lister, source, is_portfolio)?; let tenants = match selected { Selection::Tenant(wanted) => { let tenant = find(&portfolio, wanted)?; @@ -120,11 +121,16 @@ impl Context { } /// Every client tenant of the portfolio. -fn list_portfolio(lister: &Client, source: CredentialSource) -> Result> { +fn list_portfolio( + lister: &Client, + source: CredentialSource, + is_portfolio_key: bool, +) -> Result> { let api = ApiContext { source, framework: None, tenant_selected: false, + is_portfolio_key, }; let mut totals: Vec = Vec::new(); let (_, mut tenants) = collect_pages( @@ -198,6 +204,7 @@ pub fn run(ctx: &Context, command: TenantsCommand, out: &mut dyn Write) -> Resul let TenantsCommand::List(args) = command; let settings = ctx.settings()?; let (client, source) = ctx.api(&settings, None)?; + let is_portfolio = ctx.is_portfolio_key.get().unwrap_or(false); list_one( ctx, out, @@ -207,6 +214,7 @@ pub fn run(ctx: &Context, command: TenantsCommand, out: &mut dyn Write) -> Resul source, framework: None, tenant_selected: false, + is_portfolio_key: is_portfolio, }, &mut |page, per_page| client.list_tenants(&ListTenantsParams::alphabetical(page, per_page)), |r| (r.data, r.pagination), diff --git a/crates/hodeishield-cli/src/failure.rs b/crates/hodeishield-cli/src/failure.rs index f074a04..296c88d 100644 --- a/crates/hodeishield-cli/src/failure.rs +++ b/crates/hodeishield-cli/src/failure.rs @@ -157,6 +157,12 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail .iter() .map(|s| context.fill_scope(s)) .collect(); + let hint = if context.is_portfolio_key { + "Ask an organisation administrator of your MSSP organisation for a key or \ + access with that scope." + } else { + "Ask a tenant administrator for a key or access with that scope." + }; Failure::new( Kind::Forbidden, format!( @@ -164,7 +170,7 @@ pub fn from_api(error: hodeishield_api::Error, context: &ApiContext<'_>) -> Fail scopes.join(", ") ), ) - .hint("Ask a tenant administrator for a key or access with that scope.") + .hint(hint) } 404 => { let failure = Failure::new( @@ -316,6 +322,8 @@ pub struct ApiContext<'a> { pub framework: Option<&'a str>, /// Whether the call reads a selected client tenant (`--tenant` or `--all-tenants`). pub tenant_selected: bool, + /// Whether the credential is a portfolio key (starts with `hsp_`). + pub is_portfolio_key: bool, } impl ApiContext<'_> { diff --git a/crates/hodeishield-cli/tests/cli.rs b/crates/hodeishield-cli/tests/cli.rs index 9c015cf..d904ec9 100644 --- a/crates/hodeishield-cli/tests/cli.rs +++ b/crates/hodeishield-cli/tests/cli.rs @@ -1459,6 +1459,48 @@ fn a_missing_data_scope_stops_the_walk_instead_of_failing_once_per_tenant() { refused.assert(); } +#[test] +fn a_missing_scope_with_a_portfolio_key_hints_organisation_administrator() { + let env = Env::new(); + let mut server = mockito::Server::new(); + mock_portfolio(&mut server, &two_clients()); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"INSUFFICIENT_SCOPE","missing":["supply_risk:read"]}}}"#) + .create(); + let output = portfolio_cmd(&env, &server) + .args(["--tenant", "acme", "vendors", "list"]) + .output() + .expect("run"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(4), "{output:?}"); + assert!(stderr.contains("organisation administrator"), "{stderr}"); + assert!(!stderr.contains("tenant administrator"), "{stderr}"); +} + +#[test] +fn a_missing_scope_with_a_regular_key_hints_tenant_administrator() { + let env = Env::new(); + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/vendors") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":{"code":"forbidden","message":"x","request_id":"r","details":{"code":"INSUFFICIENT_SCOPE","missing":["supply_risk:read"]}}}"#) + .create(); + let output = env + .api(&server) + .args(["vendors", "list"]) + .output() + .expect("run"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(4), "{output:?}"); + assert!(stderr.contains("tenant administrator"), "{stderr}"); + assert!(!stderr.contains("organisation administrator"), "{stderr}"); +} + #[test] fn a_deactivated_client_is_a_licence_refusal_not_a_missing_resource() { let env = Env::new(); From 904810a8a7528842603771e3d874b6afaf7c66f0 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Sun, 11 Oct 2026 12:48:11 +0200 Subject: [PATCH 09/10] Describe partial results and tenant errors in the portfolio entry The portfolio entry did not say what --all-tenants does when a client fails while it is read. In the code, a client that fails with a licence not in force (exit 8) or not found (exit 5) is named on standard error and skipped, the other clients' data is printed, and the command exits non-zero with the code of the first failure; any other failure stops the run before anything is printed. Say so, mention that --tenant also works on get and compliance posture and prints the "Client tenant" line, and rewrap the long line. Add the Changed entry for the errors that involve a client tenant: the hints for an unknown client, a missing scope on a portfolio key, the portfolio rate limits and a missing or unsupported tenant selection. Refs #45, #54 Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- CHANGELOG.md | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd6da8..008c0ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,16 +29,22 @@ commands, `--json` output and exit codes. Each release's notes start with its se release's SBOM as a signed attestation. ([#36](https://github.com/Hodeitek/hodeishield-cli/issues/36)) - Several client tenants with one portfolio key. `hodeishield tenants list` lists the client tenants the key may read. The key is created by an organisation administrator of an active MSSP - organisation and goes in `HODEISHIELD_API_KEY`. The global `--tenant ` selects one client for any read - command, and `--all-tenants` reads every client on a `list` command and adds `tenant_id`, - `tenant_slug` and `tenant_name` to each row in `--json` and `--csv` (a `TENANT` column in the - table). An unknown client fails with exit code 5, before any data is read. Without these options - nothing changes. `--all-tenants` skips clients that are inactive, whose licence is not in force, or - whose licence status the CLI does not know (the latter are named on standard error and make the - command exit non-zero), asks one client at a time, and stops with exit code 6 - (naming the limit) when a rate limit is exhausted. Against an API without portfolio access the - commands fail with exit code 7. The README has a new "Several tenants (MSSP)" section. - ([#54](https://github.com/Hodeitek/hodeishield-cli/issues/54), + organisation and goes in `HODEISHIELD_API_KEY`. The global `--tenant ` selects one client + for any read command, including `get` and `compliance posture`, and prints the line + `Client tenant: ()` on standard error. `--all-tenants` reads every client on a `list` command and adds + `tenant_id`, `tenant_slug` and `tenant_name` to each row in `--json` and `--csv` (a `TENANT` + column in the table). An unknown client fails with exit code 5, before any data is read. Without + these options nothing changes. `--all-tenants` asks one client at a time and skips clients that + are inactive or whose licence is not in force, with a note on standard error; the command still + exits 0 when only inactive clients are skipped. A client whose licence status the CLI does not + know is not read and is named on standard error. A client that fails when it is read because its + licence is not in force (exit code 8) or it is not found (exit code 5) is also named and skipped. + In both cases the other clients' data is printed and the command then exits non-zero (with the + code of the first client that failed, or 1 for an unknown licence status), so a script never takes + a partial result for a complete one. Any other failure stops the run before anything is printed, + for example exit code 6 (naming the limit) when a rate limit is exhausted. Against an API without + portfolio access the commands fail with exit code 7. The README has a new "Several tenants (MSSP)" + section. ([#54](https://github.com/Hodeitek/hodeishield-cli/issues/54), [#45](https://github.com/Hodeitek/hodeishield-cli/issues/45)) ### Changed (scripting surface) @@ -77,6 +83,14 @@ commands, `--json` output and exit codes. Each release's notes start with its se details, secure use, updating, removal and where the SBOM is published ([#127](https://github.com/Hodeitek/hodeishield-cli/issues/127), [#128](https://github.com/Hodeitek/hodeishield-cli/issues/128)). +- Errors that involve a client tenant now say so and what to do. An unknown client tenant (404 + `TENANT_NOT_FOUND`) points to `hodeishield tenants list`; a portfolio key rejected for a missing + scope points to an organisation administrator; a rate limit names whether it is the key's, the + organisation's or the client tenant's own, and says whether other clients are affected; a + credential that covers several clients without `--tenant` or `--all-tenants` is told to pick one; + and `--tenant` with a credential for one tenant only is told to leave it out. Messages and + hints go to standard error; exit codes do not change. ([#54](https://github.com/Hodeitek/hodeishield-cli/issues/54), + [#45](https://github.com/Hodeitek/hodeishield-cli/issues/45)) ### Fixed From 7b8a577144f3473d67912ca80833f4d577fd89d6 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:04:33 +0200 Subject: [PATCH 10/10] Say that a portfolio key limited to some tenants lists only those Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 426b659..2ef0394 100644 --- a/README.md +++ b/README.md @@ -389,8 +389,9 @@ beta Beta Corp Beta Group client active grace 22222222-2222-4222-8 It takes `--json`, `--csv`, `--page` and `--all` like any list, sorted by name. It includes your own organisation's tenants (`TYPE` `own` for the internal one) and clients that are inactive or have -no licence in force, which the `STATUS` and `LICENCE` columns tell apart. The list is read from the API -each time: a client added to or removed from your portfolio shows up on the next command. +no licence in force, which the `STATUS` and `LICENCE` columns tell apart. A key that was limited to +some tenants when it was created lists and reads only those. The list is read from the API each +time: a client added to or removed from your portfolio shows up on the next command. **3. Select the client a read command reads.** The selection goes with the command and is never remembered, so a later command cannot read another client by surprise.