From 654e38752d15358c67fe59a716adf33d5e2596a9 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 5 Jul 2026 19:51:11 +0200 Subject: [PATCH] =?UTF-8?q?feat(project-tool):=20per-env=20approval-policy?= =?UTF-8?q?=20gating=20(=C2=A74.2/=C2=A76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salvaged from the superseded feat/hierarchies-extension-points branch (M5), re-ported onto main's evolved tree-apply engine. Fills the §7/§11.1 "per-env approval-policy gating" that main's design doc still lists as intended but unbuilt. A project's root manifest declares which environments require explicit approval; `pic apply --dir --env ` to a confirm-required env needs an explicit `--approve ` (a blanket `--yes` does NOT cover it). The approval is admin-gated (admin on every declared node) and audited, and the policy folds into the bound-plan token. - manifest: `[project]` block → ManifestProject { environments[{name,confirm}] }, root-manifest-only (rejected elsewhere by build_tree). - discover: build_tree emits `project` into the bundle from the root manifest. - apply_service: TreeBundle.project + ProjectPolicy; policy folds into the state_token; plan surfaces approvals_required; ApplyError::ApprovalRequired → 409. - apply_api: enforce_env_approval (after authz_tree) — refuses a gated env not in approved_envs, requires AppAdmin/GroupAdmin on every declared node on approval, audits. Server re-derives policy from the bundle (authoritative). - CLI: --approve (repeatable); resolve_approvals refuses a gated env non-interactively, prompts on a TTY; plan renders gated envs. Single-node apply --file refuses a confirm-required env and directs to --dir. - approval journey + manifest/ProjectPolicy unit tests. No migration (policy lives in the manifest, like takeover/blast-radius). Co-Authored-By: Claude Opus 4.8 --- crates/manager-core/src/apply_api.rs | 77 ++++++++++- crates/manager-core/src/apply_service.rs | 88 ++++++++++++ crates/picloud-cli/src/client.rs | 7 + crates/picloud-cli/src/cmds/apply.rs | 75 +++++++++- crates/picloud-cli/src/cmds/init.rs | 1 + crates/picloud-cli/src/cmds/plan.rs | 11 +- crates/picloud-cli/src/cmds/pull.rs | 1 + crates/picloud-cli/src/discover.rs | 44 +++++- crates/picloud-cli/src/main.rs | 6 + crates/picloud-cli/src/manifest.rs | 44 ++++++ crates/picloud-cli/tests/approval.rs | 167 +++++++++++++++++++++++ crates/picloud-cli/tests/cli.rs | 1 + docs/design/groups-and-project-tool.md | 14 +- 13 files changed, 523 insertions(+), 13 deletions(-) create mode 100644 crates/picloud-cli/tests/approval.rs diff --git a/crates/manager-core/src/apply_api.rs b/crates/manager-core/src/apply_api.rs index 8b98a22..1e46cce 100644 --- a/crates/manager-core/src/apply_api.rs +++ b/crates/manager-core/src/apply_api.rs @@ -287,6 +287,15 @@ struct TreeApplyRequest { prune: bool, #[serde(default)] expected_token: Option, + /// Selected environment (the overlay applied). Drives per-env approval + /// gating (§4.2, M5). + #[serde(default)] + env: Option, + /// Environments the actor explicitly approved this apply for (§4.2, M5). A + /// confirm-required env (per the bundle's `[project]` policy) is refused + /// unless it appears here; `--yes` alone does NOT add it. + #[serde(default)] + approved_envs: Vec, } async fn tree_plan_handler( @@ -304,6 +313,14 @@ async fn tree_apply_handler( Json(req): Json, ) -> Result, ApplyError> { authz_tree(&svc, &principal, &req.bundle, Some(req.prune)).await?; + enforce_env_approval( + &svc, + &principal, + &req.bundle, + req.env.as_deref(), + &req.approved_envs, + ) + .await?; let report = svc .apply_tree( &req.bundle, @@ -315,6 +332,62 @@ async fn tree_apply_handler( Ok(Json(report)) } +/// Per-env approval gate (§4.2, §6, M5). If the apply selects a confirm-required +/// environment (per the bundle's `[project]` policy, re-derived here — the CLI's +/// enforcement is convenience, this is authoritative), it must be explicitly +/// approved via `approved_envs` (`pic apply --approve `); a blanket `--yes` +/// does NOT satisfy it. An approved gated apply additionally requires ADMIN +/// authority on every declared node — the "override a gate" capability (§4.2), a +/// deliberate step up from the editor-level write caps an ordinary apply needs — +/// and is audited. +async fn enforce_env_approval( + svc: &ApplyService, + principal: &Principal, + bundle: &TreeBundle, + env: Option<&str>, + approved_envs: &[String], +) -> Result<(), ApplyError> { + let (Some(policy), Some(env)) = (&bundle.project, env) else { + return Ok(()); + }; + if !policy.confirm_required(env) { + return Ok(()); + } + if !approved_envs.iter().any(|e| e == env) { + return Err(ApplyError::ApprovalRequired(env.to_string())); + } + // Approved: require admin authority on every declared node. + for node in &bundle.nodes { + match node.kind { + NodeKind::App => { + let app_id = resolve_app_id(svc.apps.as_ref(), &node.slug).await?; + require(svc.authz.as_ref(), principal, Capability::AppAdmin(app_id)) + .await + .map_err(map_authz)?; + } + NodeKind::Group => { + if let Some(g) = svc + .groups + .get_by_slug(&node.slug) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + { + require(svc.authz.as_ref(), principal, Capability::GroupAdmin(g.id)) + .await + .map_err(map_authz)?; + } + } + } + } + tracing::info!( + user_id = ?principal.user_id, + env = %env, + nodes = bundle.nodes.len(), + "apply: approved gated-environment apply (audit)" + ); + Ok(()) +} + /// Per-node capability check for a tree plan/apply. `prune` widens an apply's /// write requirement to every kind. A plan (`prune` ignored, no writes) needs /// only the per-node read cap. @@ -503,7 +576,9 @@ impl IntoResponse for ApplyError { StatusCode::UNPROCESSABLE_ENTITY, json!({ "error": self.to_string() }), ), - Self::StateMoved => (StatusCode::CONFLICT, json!({ "error": self.to_string() })), + Self::StateMoved | Self::ApprovalRequired(_) => { + (StatusCode::CONFLICT, json!({ "error": self.to_string() })) + } Self::Forbidden => (StatusCode::FORBIDDEN, json!({ "error": self.to_string() })), Self::AuthzRepo(e) => { tracing::error!(error = %e, "apply authz repo error"); diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index bef098a..e678e18 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -505,6 +505,46 @@ pub struct TreeNode { pub struct TreeBundle { #[serde(default)] pub nodes: Vec, + /// Project-level policy from the repo's root manifest `[project]` block + /// (§4.2/§6, M5). The server re-derives env approval policy from here — the + /// CLI's enforcement is convenience, this is authoritative. + #[serde(default)] + pub project: Option, +} + +/// Project-level policy (the root manifest's `[project]` block, M5). +#[derive(Debug, Clone, Default, Deserialize)] +pub struct ProjectPolicy { + #[serde(default)] + pub environments: Vec, +} + +/// One environment's apply policy. `confirm = true` makes applying to this env a +/// gated, default-off step: a per-env `--approve ` is required (a blanket +/// `--yes` does NOT cover it) and the act is admin-gated + audited (§4.2). +#[derive(Debug, Clone, Deserialize)] +pub struct EnvPolicy { + pub name: String, + #[serde(default)] + pub confirm: bool, +} + +impl ProjectPolicy { + /// Does environment `env` require explicit approval to apply? + #[must_use] + pub fn confirm_required(&self, env: &str) -> bool { + self.environments.iter().any(|e| e.name == env && e.confirm) + } + + /// Names of every confirm-required environment (for `plan` to surface). + #[must_use] + pub fn gated_envs(&self) -> Vec { + self.environments + .iter() + .filter(|e| e.confirm) + .map(|e| e.name.clone()) + .collect() + } } /// One node's diff within a tree plan. @@ -523,6 +563,11 @@ pub struct NodePlan { pub struct TreePlanResult { pub nodes: Vec, pub state_token: String, + /// Environments the project marks confirm-required (§4.2, M5): applying to + /// one needs an explicit `pic apply --approve `. Surfaced so CI sees + /// the gate at `plan` time, before an `apply` refuses. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub approvals_required: Vec, } // ---------------------------------------------------------------------------- @@ -627,6 +672,11 @@ pub enum ApplyError { StateMoved, #[error("forbidden")] Forbidden, + #[error( + "environment `{0}` requires explicit approval to apply; \ + re-run with `--approve {0}` (a blanket `--yes` does not cover it)" + )] + ApprovalRequired(String), #[error("authorization repo error: {0}")] AuthzRepo(String), #[error("backend: {0}")] @@ -1463,9 +1513,15 @@ impl ApplyService { plan: p.plan, }) .collect(); + let approvals_required = bundle + .project + .as_ref() + .map(ProjectPolicy::gated_envs) + .unwrap_or_default(); Ok(TreePlanResult { nodes, state_token: token, + approvals_required, }) } @@ -1719,6 +1775,18 @@ impl ApplyService { token_parts.push(format!("gs|{gid}|{}", g.structure_version)); } } + // Fold the env approval policy (desired state) into the token, so a + // policy edit between plan and apply trips StateMoved (M5). Sorted for + // order-independence. + if let Some(policy) = &bundle.project { + let mut envs: Vec = policy + .environments + .iter() + .map(|e| format!("{}={}", e.name, e.confirm)) + .collect(); + envs.sort_unstable(); + token_parts.push(format!("proj|{}", envs.join(","))); + } token_parts.sort_unstable(); Ok((prepared, combine_tokens(&token_parts))) } @@ -4732,4 +4800,24 @@ mod tests { "queue identity is queue_name-only: {p:?}" ); } + + #[test] + fn project_policy_gates_only_confirm_required_envs() { + let policy = ProjectPolicy { + environments: vec![ + EnvPolicy { + name: "production".into(), + confirm: true, + }, + EnvPolicy { + name: "staging".into(), + confirm: false, + }, + ], + }; + assert!(policy.confirm_required("production")); + assert!(!policy.confirm_required("staging")); + assert!(!policy.confirm_required("unknown")); + assert_eq!(policy.gated_envs(), vec!["production".to_string()]); + } } diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 791ff47..21dd195 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -1278,11 +1278,15 @@ impl Client { bundle: &serde_json::Value, prune: bool, expected_token: Option<&str>, + env: Option<&str>, + approved_envs: &[String], ) -> Result { let body = serde_json::json!({ "bundle": bundle, "prune": prune, "expected_token": expected_token, + "env": env, + "approved_envs": approved_envs, }); let resp = self .request(Method::POST, "/api/v1/admin/tree/apply") @@ -1531,6 +1535,9 @@ pub struct TreePlanDto { pub nodes: Vec, #[serde(default)] pub state_token: String, + /// Environments the project marks confirm-required (§4.2, M5). + #[serde(default)] + pub approvals_required: Vec, } #[derive(Debug, Deserialize)] diff --git a/crates/picloud-cli/src/cmds/apply.rs b/crates/picloud-cli/src/cmds/apply.rs index aa27aa8..7355df3 100644 --- a/crates/picloud-cli/src/cmds/apply.rs +++ b/crates/picloud-cli/src/cmds/apply.rs @@ -26,6 +26,26 @@ pub async fn run( let client = Client::from_creds(&creds)?; let manifest = Manifest::load_with_env(manifest_path, env)?; + + // Env approval gating (§4.2, M5) is a project-tree (`--dir`) feature: the + // admin-gated, audited per-env approval is enforced server-side only on the + // tree apply path. Single-node `apply --file` cannot provide that guarantee, + // so rather than silently bypass a confirm-required env, refuse and direct + // the user to `--dir`. + if let (Some(env), Some(project)) = (env, &manifest.project) { + if project + .environments + .iter() + .any(|e| e.name == env && e.confirm) + { + anyhow::bail!( + "environment `{env}` is confirm-required by this project's [project] policy; \ + single-node `pic apply --file` cannot provide the admin-gated, audited approval. \ + Use `pic apply --dir --env {env} --approve {env}` instead." + ); + } + } + let base_dir = manifest_path.parent().unwrap_or_else(|| Path::new(".")); let bundle = build_bundle(&manifest, base_dir)?; let kind = if manifest.is_group() { @@ -115,22 +135,30 @@ pub async fn run( /// `pic apply --dir ` (Phase 5): reconcile a whole project tree — every /// `picloud.toml` under `root` — in ONE atomic server transaction. +#[allow(clippy::too_many_arguments)] pub async fn run_tree( dir: &Path, prune: bool, yes: bool, force: bool, env: Option<&str>, + approve: &[String], mode: OutputMode, ) -> Result<()> { let creds = config::resolve()?; let client = Client::from_creds(&creds)?; - let (bundle, node_count) = crate::discover::build_tree(dir, env)?; + let (bundle, node_count, envs) = crate::discover::build_tree(dir, env)?; if prune && !yes { confirm_prune(&format!("{node_count} node(s) under {}", dir.display()))?; } + // Per-env approval gate (§4.2, M5): if the selected env is confirm-required, + // it needs an explicit `--approve ` (a TTY may confirm interactively; + // `--yes` does NOT cover it). The server re-checks this authoritatively — the + // local check just fails fast with a clear message. + let approved = resolve_approvals(env, &envs, approve)?; + let token_key = crate::cmds::plan::TREE_TOKEN_KEY; let expected_token = if force { None @@ -141,7 +169,7 @@ pub async fn run_tree( }; let report = client - .apply_tree(&bundle, prune, expected_token.as_deref()) + .apply_tree(&bundle, prune, expected_token.as_deref(), env, &approved) .await?; crate::linkstate::clear_plan(dir, token_key); @@ -205,6 +233,49 @@ pub async fn run_tree( /// irreversible. Require an explicit go-ahead: an interactive `y`, or `--yes` /// for non-interactive/CI use. Refuse a non-interactive prune without `--yes` /// rather than silently deleting (review the deletions first with `pic plan`). +/// Resolve the set of environments the actor approves for this apply (§4.2, M5). +/// If the selected `env` is confirm-required (per the root manifest's +/// `[project]` policy) it must be approved — via `--approve `, or an +/// interactive TTY confirmation that requires re-typing the env name. A blanket +/// `--yes` does NOT satisfy it. Returns the full approved set to send on the +/// wire (the server re-checks authoritatively). A non-gated env passes through. +fn resolve_approvals( + env: Option<&str>, + policy: &[crate::manifest::ManifestEnvironment], + approve: &[String], +) -> Result> { + let mut approved: Vec = approve.to_vec(); + let Some(env) = env else { + return Ok(approved); // no env selected → nothing env-specific to gate + }; + let gated = policy.iter().any(|e| e.name == env && e.confirm); + if !gated || approved.iter().any(|e| e == env) { + return Ok(approved); + } + // Gated and not pre-approved: prompt on a TTY, else refuse (CI must pass + // --approve explicitly; --yes is deliberately insufficient). + if std::io::stdin().is_terminal() { + eprint!( + "environment `{env}` requires explicit approval to apply. \ + Type the environment name to approve, or anything else to abort: " + ); + std::io::stderr().flush().ok(); + let mut answer = String::new(); + std::io::stdin() + .read_line(&mut answer) + .context("read approval")?; + if answer.trim() == env { + approved.push(env.to_string()); + return Ok(approved); + } + anyhow::bail!("aborted: environment `{env}` not approved"); + } + anyhow::bail!( + "environment `{env}` requires explicit approval: re-run with `--approve {env}` \ + (a blanket `--yes` does not cover a confirm-required environment)" + ); +} + fn confirm_prune(slug: &str) -> Result<()> { if !std::io::stdin().is_terminal() { anyhow::bail!( diff --git a/crates/picloud-cli/src/cmds/init.rs b/crates/picloud-cli/src/cmds/init.rs index 06aef33..588afe5 100644 --- a/crates/picloud-cli/src/cmds/init.rs +++ b/crates/picloud-cli/src/cmds/init.rs @@ -116,6 +116,7 @@ fn scaffold_manifest(slug: &str, name: &str) -> Manifest { extension_points: Vec::new(), }), group: None, + project: None, scripts: vec![ManifestScript { name: "hello".into(), file: "scripts/hello.rhai".into(), diff --git a/crates/picloud-cli/src/cmds/plan.rs b/crates/picloud-cli/src/cmds/plan.rs index 55da10f..2360e91 100644 --- a/crates/picloud-cli/src/cmds/plan.rs +++ b/crates/picloud-cli/src/cmds/plan.rs @@ -49,7 +49,7 @@ pub async fn run(manifest_path: &Path, env: Option<&str>, mode: OutputMode) -> R pub async fn run_tree(dir: &Path, env: Option<&str>, mode: OutputMode) -> Result<()> { let creds = config::resolve()?; let client = Client::from_creds(&creds)?; - let (bundle, _count) = crate::discover::build_tree(dir, env)?; + let (bundle, _count, _envs) = crate::discover::build_tree(dir, env)?; let plan = client.plan_tree(&bundle).await?; if !plan.state_token.is_empty() { if let Err(e) = crate::linkstate::write_plan(dir, TREE_TOKEN_KEY, &plan.state_token) { @@ -87,6 +87,15 @@ fn render_tree(plan: &TreePlanDto, mode: OutputMode) { } } table.print(mode); + + // Per-env approval gating (§4.2, M5): surface the confirm-required envs so + // CI sees the gate at plan time, before an apply refuses. + if mode != OutputMode::Json && !plan.approvals_required.is_empty() { + eprintln!("\nenvironments requiring explicit approval (apply with --approve ):"); + for e in &plan.approvals_required { + eprintln!(" - {e}"); + } + } } /// Assemble the wire bundle: scripts carry inlined source (read from diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index 8bbd4ff..935d5d9 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -242,6 +242,7 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> extension_points, }), group: None, + project: None, scripts: manifest_scripts, routes, triggers: manifest_triggers, diff --git a/crates/picloud-cli/src/discover.rs b/crates/picloud-cli/src/discover.rs index 499fdbf..3bc2f24 100644 --- a/crates/picloud-cli/src/discover.rs +++ b/crates/picloud-cli/src/discover.rs @@ -45,10 +45,15 @@ fn walk(dir: &Path, out: &mut Vec) -> Result<()> { Ok(()) } -/// Build the wire tree bundle (`{ nodes: [{kind, slug, bundle}] }`) from every -/// manifest under `root`. Returns the JSON plus the node count. Rejects a tree -/// that names the same (kind, slug) twice. -pub fn build_tree(root: &Path, env: Option<&str>) -> Result<(Value, usize)> { +/// Build the wire tree bundle (`{ nodes: [{kind, slug, bundle}], project }`) +/// from every manifest under `root`. Returns the JSON, the node count, and the +/// root manifest's `[project]` environment policy (M5, empty if none). Rejects a +/// tree that names the same (kind, slug) twice, or declares `[project]` anywhere +/// but the root manifest. +pub fn build_tree( + root: &Path, + env: Option<&str>, +) -> Result<(Value, usize, Vec)> { let paths = find_manifests(root)?; if paths.is_empty() { bail!( @@ -56,11 +61,25 @@ pub fn build_tree(root: &Path, env: Option<&str>) -> Result<(Value, usize)> { root.display() ); } + let root_manifest = root.join(MANIFEST_FILE); let mut nodes = Vec::with_capacity(paths.len()); let mut seen: HashSet<(String, String)> = HashSet::new(); + let mut project: Option = None; for path in &paths { let manifest = Manifest::load_with_env(path, env) .with_context(|| format!("loading {}", path.display()))?; + // `[project]` is project-level — valid only on the tree's root manifest. + if let Some(p) = &manifest.project { + if path == &root_manifest { + project = Some(p.clone()); + } else { + bail!( + "{} declares [project], which is only valid on the root manifest ({})", + path.display(), + root_manifest.display() + ); + } + } let base_dir = path.parent().unwrap_or_else(|| Path::new(".")); let bundle = build_bundle(&manifest, base_dir)?; let kind = if manifest.is_group() { "group" } else { "app" }; @@ -71,5 +90,20 @@ pub fn build_tree(root: &Path, env: Option<&str>) -> Result<(Value, usize)> { nodes.push(json!({ "kind": kind, "slug": slug, "bundle": bundle })); } let count = nodes.len(); - Ok((json!({ "nodes": nodes }), count)) + let envs = project + .as_ref() + .map(|p| p.environments.clone()) + .unwrap_or_default(); + let mut bundle = json!({ "nodes": nodes }); + if let Some(p) = &project { + bundle.as_object_mut().expect("json object").insert( + "project".into(), + json!({ + "environments": p.environments.iter() + .map(|e| json!({ "name": e.name, "confirm": e.confirm })) + .collect::>(), + }), + ); + } + Ok((bundle, count, envs)) } diff --git a/crates/picloud-cli/src/main.rs b/crates/picloud-cli/src/main.rs index 52d1ab4..bb3849f 100644 --- a/crates/picloud-cli/src/main.rs +++ b/crates/picloud-cli/src/main.rs @@ -250,6 +250,11 @@ struct ApplyArgs { /// top of the base manifest before applying. #[arg(long)] env: Option, + /// Tree apply only (`--dir`): explicitly approve applying to a + /// confirm-required environment (per the root manifest's `[project]` + /// policy, §4.2). Repeatable. A blanket `--yes` does NOT cover it. + #[arg(long = "approve", requires = "dir")] + approve: Vec, } #[derive(Args)] @@ -1422,6 +1427,7 @@ async fn main() -> ExitCode { args.yes, args.force, args.env.as_deref(), + &args.approve, mode, ) .await diff --git a/crates/picloud-cli/src/manifest.rs b/crates/picloud-cli/src/manifest.rs index 1ccad2f..817b7d3 100644 --- a/crates/picloud-cli/src/manifest.rs +++ b/crates/picloud-cli/src/manifest.rs @@ -37,6 +37,11 @@ pub struct Manifest { pub app: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub group: Option, + /// `[project]` — project-level policy (per-env approval gating, §4.2/§6). + /// Consumed only by `apply --dir` and only on the tree's ROOT manifest + /// (enforced in `build_tree`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub project: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub scripts: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -263,6 +268,26 @@ pub struct ManifestGroup { pub collections: Vec, } +/// `[project]` — project-level policy (§4.2/§6). Valid ONLY on the root +/// manifest of a `pic apply --dir` tree; rejected elsewhere by [`build_tree`]. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ManifestProject { + /// `[[project.environments]]` — per-env apply policy. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub environments: Vec, +} + +/// One `[[project.environments]]` entry. `confirm = true` gates applying to this +/// env behind an explicit `pic apply --approve `. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ManifestEnvironment { + pub name: String, + #[serde(default)] + pub confirm: bool, +} + /// The store kind of a shared collection (§11.6). #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] @@ -596,6 +621,7 @@ mod tests { extension_points: vec!["theme".into()], }), group: None, + project: None, scripts: vec![ ManifestScript { name: "create-post".into(), @@ -759,6 +785,7 @@ mod tests { extension_points: vec![], }), group: None, + project: None, scripts: vec![], routes: vec![], triggers: ManifestTriggers::default(), @@ -875,6 +902,23 @@ mod tests { ); } + #[test] + fn project_block_parses_environment_policy() { + let m = Manifest::parse( + "[app]\nslug = \"a\"\nname = \"A\"\n\n\ + [[project.environments]]\nname = \"production\"\nconfirm = true\n\n\ + [[project.environments]]\nname = \"staging\"\n", + ) + .expect("manifest with [project] parses"); + let p = m.project.expect("project present"); + assert_eq!(p.environments.len(), 2); + assert_eq!(p.environments[0].name, "production"); + assert!(p.environments[0].confirm); + // `confirm` defaults to false when omitted. + assert_eq!(p.environments[1].name, "staging"); + assert!(!p.environments[1].confirm); + } + #[test] fn group_manifest_parses_and_rejects_app_only_blocks() { // A [group] node: scripts + vars, no [app]. diff --git a/crates/picloud-cli/tests/approval.rs b/crates/picloud-cli/tests/approval.rs new file mode 100644 index 0000000..6850bff --- /dev/null +++ b/crates/picloud-cli/tests/approval.rs @@ -0,0 +1,167 @@ +//! M5 per-env approval gating (§4.2, §6) via `pic apply --dir`: +//! * a root manifest `[project]` block marks an environment confirm-required, +//! * applying to that env WITHOUT `--approve` is refused (a blanket `--yes` +//! does not cover it) — refused non-interactively at the CLI, +//! * `--approve ` (as an admin) lets it through, +//! * a non-gated environment applies with plain `--yes`, +//! * single-node `apply --file` to a gated env is refused (no silent bypass), +//! * approving a gated apply needs ADMIN authority on the node — a non-admin +//! editor with `--approve` is refused server-side (403). + +use std::fs; +use std::path::Path; + +use tempfile::TempDir; + +use crate::common; +use crate::common::cleanup::{AppGuard, GroupGuard}; +use crate::common::member; + +/// A single-app project dir whose root manifest declares a `[project]` policy: +/// `production` is confirm-required, `staging` is not. A `[vars]` entry gives +/// the app write-requiring content so an `editor` member's AppVarsWrite is +/// exercised (proving the admin gate is ABOVE editor-write). Vars cascade with +/// the app; scripts are ON DELETE RESTRICT and would break AppGuard teardown. +fn project_dir(app: &str) -> TempDir { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("picloud.toml"), + format!( + "[app]\nslug = \"{app}\"\nname = \"Gated App\"\n\n\ + [[project.environments]]\nname = \"production\"\nconfirm = true\n\n\ + [[project.environments]]\nname = \"staging\"\nconfirm = false\n\n\ + [vars]\nregion = \"eu\"\n" + ), + ) + .unwrap(); + // `--env ` requires the overlay file to exist; empty overlays keep the + // base slug (same app across envs — we're testing the gate, not env routing). + fs::write(dir.path().join("picloud.production.toml"), "").unwrap(); + fs::write(dir.path().join("picloud.staging.toml"), "").unwrap(); + dir +} + +fn apply(env: &common::TestEnv, dir: &Path, extra: &[&str]) -> std::process::Output { + let mut cmd = common::pic_as(env); + cmd.args(["apply", "--dir"]).arg(dir).args(extra); + cmd.output().expect("apply --dir") +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn confirm_required_env_needs_explicit_approval() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let group = common::unique_slug("appr-g"); + let app = common::unique_slug("appr-a"); + let _g = GroupGuard::new(&env.url, &env.token, &group); + common::pic_as(&env) + .args(["groups", "create", &group]) + .assert() + .success(); + let _a = AppGuard::new(&env.url, &env.token, &app); + common::pic_as(&env) + .args(["apps", "create", &app, "--group", &group]) + .assert() + .success(); + + let dir = project_dir(&app); + + // --- production is confirm-required: --yes alone is refused. --- + let out = apply(&env, dir.path(), &["--env", "production", "--yes"]); + assert!( + !out.status.success(), + "production apply without --approve must be refused" + ); + let err = String::from_utf8_lossy(&out.stderr).to_lowercase(); + assert!( + err.contains("approve"), + "refusal should mention --approve:\n{err}" + ); + + // --- with --approve production, it applies. --- + let ok = apply( + &env, + dir.path(), + &["--env", "production", "--approve", "production"], + ); + assert!( + ok.status.success(), + "approved production apply should succeed: {}", + String::from_utf8_lossy(&ok.stderr) + ); + + // --- staging is NOT gated: plain --yes applies. --- + let ok2 = apply(&env, dir.path(), &["--env", "staging", "--yes"]); + assert!( + ok2.status.success(), + "non-gated staging apply should succeed: {}", + String::from_utf8_lossy(&ok2.stderr) + ); + + // --- single-node `apply --file` to a gated env is refused (no bypass). --- + let single = common::pic_as(&env) + .args(["apply", "--file"]) + .arg(dir.path().join("picloud.toml")) + .args(["--env", "production", "--yes"]) + .output() + .expect("apply --file"); + assert!( + !single.status.success(), + "single-node apply to a confirm-required env must be refused" + ); + let serr = String::from_utf8_lossy(&single.stderr).to_lowercase(); + assert!( + serr.contains("confirm-required") || serr.contains("--dir"), + "single-node refusal should point at --dir:\n{serr}" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn approving_a_gated_apply_requires_admin() { + // §4.2: approving a confirm-required env is admin-gated — a second gate on + // top of the editor-level write caps an ordinary apply needs. An editor who + // CAN write the app (its `[vars]`) still cannot approve a gated apply. + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let group = common::unique_slug("appr2-g"); + let app = common::unique_slug("appr2-a"); + let _g = GroupGuard::new(&env.url, &env.token, &group); + common::pic_as(&env) + .args(["groups", "create", &group]) + .assert() + .success(); + let _a = AppGuard::new(&env.url, &env.token, &app); + common::pic_as(&env) + .args(["apps", "create", &app, "--group", &group]) + .assert() + .success(); + + // A member with `editor` (write) on the app — enough for an ordinary apply, + // not enough to approve a gated environment. + let m = member::member_user(fx, &common::unique_username("appr")); + member::grant_membership(fx, &app, &m.id, "editor"); + let member_env = common::custom_env(&fx.url, &m.token); + common::seed_credentials(&member_env, &m.username); + + let dir = project_dir(&app); + let out = apply( + &member_env, + dir.path(), + &["--env", "production", "--approve", "production"], + ); + assert!( + !out.status.success(), + "a non-admin editor must not be able to approve a gated apply" + ); + let err = String::from_utf8_lossy(&out.stderr).to_lowercase(); + assert!( + err.contains("forbidden") || err.contains("403"), + "approval denial should be an authz error:\n{err}" + ); +} diff --git a/crates/picloud-cli/tests/cli.rs b/crates/picloud-cli/tests/cli.rs index a90498b..daec0fb 100644 --- a/crates/picloud-cli/tests/cli.rs +++ b/crates/picloud-cli/tests/cli.rs @@ -16,6 +16,7 @@ mod common; mod admins; mod api_keys; mod apply; +mod approval; mod apps; mod auth; mod collections; diff --git a/docs/design/groups-and-project-tool.md b/docs/design/groups-and-project-tool.md index 44c6878..0080bd4 100644 --- a/docs/design/groups-and-project-tool.md +++ b/docs/design/groups-and-project-tool.md @@ -1124,10 +1124,16 @@ Resolved items now live inline next to their topic. What genuinely remains: > `picloud.toml` manifests (each declaring an `[app]` or `[group]` node) applies as ONE > server-computed plan in ONE Postgres transaction. Per the §11.1 review, the **multi-repo > single-owner / attach-point / takeover** layer (§7) is **deferred** for the solo-dev / single-repo - > start, as is **per-env approval-policy gating** (`[project.environments]`); env overlays - > (`picloud..toml`) already exist. Groups must **pre-exist** (`pic groups create`) — a manifest - > owns each node's *content*, not the tree *shape* (declarative group create/reparent is a later - > add). + > start. **Per-env approval-policy gating shipped (`[project.environments]`, M5, 2026-07-05)** — the + > root manifest declares which environments are confirm-required; `pic apply --dir --env ` to such + > an env needs an explicit `--approve ` (a blanket `--yes` does NOT cover it), the approval is + > admin-gated (the actor needs admin on every declared node, not just write) + audited, and the policy + > folds into the bound-plan token. The server re-derives the policy from the bundle (authoritative); + > single-node `apply --file --env ` refuses a gated env (can't carry the admin-gated approval) and + > directs to `--dir`. Like `--takeover`/blast-radius the policy lives in the manifest (not persisted), + > an accepted trust boundary. Env overlays (`picloud..toml`) already exist. Groups must + > **pre-exist** (`pic groups create`) — a manifest owns each node's *content*, not the tree *shape* + > (declarative group create/reparent is a later add). > > Shipped surface: > - **Engine:** the reconcile engine generalized from app-only to an `ApplyOwner { App | Group }`