From 5c33b7490b8b9508b6fe5f37b789f274d7e40fb8 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 5 Jul 2026 20:29:12 +0200 Subject: [PATCH] =?UTF-8?q?feat(project-tool):=20multi-repo=20single-owner?= =?UTF-8?q?=20ownership=20+=20takeover=20(=C2=A77=20M3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports the M3 milestone from the superseded feat/hierarchies branch onto main's evolved apply engine. A group node is authoritatively managed by at most one project-root; `pic apply --dir` claims, conflicts, takes over, and (with --prune) structurally reaps owned nodes. - Migration 0066: `projects(id, key UNIQUE)` + promote the inert `groups.owner_project` (0047) to a real FK (ON DELETE SET NULL) + index. - CLI mints a stable, gitignored project key in `.picloud/project.json` (`pic init`, or lazily on first tree plan/apply) and presents it on every tree request; `pic apply --dir --takeover` flag; `pic plan --dir` surfaces ownership conflicts + structural-prune candidates read-only. - Server: prepare_tree resolves ownership read-only (token folds each group's owner key, so a claim/takeover by another repo between plan and apply trips StateMoved). apply_tree upserts the project in-tx, claims unclaimed declared groups, and takes over owned ones under `--takeover`. --prune reaps owned-but-undeclared groups leaf-first (delete_group_tx RESTRICT — never another repo's or a UI-owned node). - Authz (§7.4, ownership ⟂ RBAC): takeover requires GroupAdmin per contested node — enforced in authz_tree (pre-tx) AND re-verified in-tx at the ownership decision, so --force (which waives the staleness token) can't open a takeover-without-admin window. The attacker-supplied project key is length/charset-validated server-side. Adaptation to main (which lacks the superseded branch's M2 declarative group create/reparent): groups pre-exist, so ownership stamps existing declared nodes rather than claim-on-create; the declarative attach-point is deferred. Review fixes (adversarial pass, 3 findings, all closed): - HIGH: an empty `[group]` node was claimed capability-free — require a baseline GroupScriptsWrite (editor) on every group apply node, so claiming ownership needs write authority (Ownership ⟂ RBAC). - MEDIUM: structural prune would silently CASCADE a group's §11.6 shared collections + secrets (which postdate M3's original design). Guard: a prune candidate holding shared data/secrets is KEPT with a warning (plus its candidate ancestors, so a preserved child never aborts a parent delete). - LOW: a corrupt `.picloud/project.json` silently re-minted a new key (orphaning ownership) — now fails loudly if the file exists but won't parse. Tests: tests/ownership.rs (claim → conflict → takeover → flip; non-admin takeover → 403; prune-owned-only; prune-refuses-to-cascade-shared-data); format_conflicts + validate_project_key unit tests; schema golden reblessed. Co-Authored-By: Claude Opus 4.8 --- .../migrations/0066_owner_project.sql | 39 ++ crates/manager-core/src/apply_api.rs | 86 +++- crates/manager-core/src/apply_service.rs | 412 +++++++++++++++++- crates/manager-core/src/group_repo.rs | 178 ++++++++ crates/manager-core/tests/expected_schema.txt | 16 + crates/picloud-cli/Cargo.toml | 2 + crates/picloud-cli/src/client.rs | 50 ++- crates/picloud-cli/src/cmds/apply.rs | 23 +- crates/picloud-cli/src/cmds/init.rs | 3 + crates/picloud-cli/src/cmds/plan.rs | 23 +- crates/picloud-cli/src/linkstate.rs | 65 +++ crates/picloud-cli/src/main.rs | 6 + crates/picloud-cli/tests/cli.rs | 1 + crates/picloud-cli/tests/ownership.rs | 288 ++++++++++++ docs/design/groups-and-project-tool.md | 34 +- 15 files changed, 1183 insertions(+), 43 deletions(-) create mode 100644 crates/manager-core/migrations/0066_owner_project.sql create mode 100644 crates/picloud-cli/tests/ownership.rs diff --git a/crates/manager-core/migrations/0066_owner_project.sql b/crates/manager-core/migrations/0066_owner_project.sql new file mode 100644 index 0000000..6bec36d --- /dev/null +++ b/crates/manager-core/migrations/0066_owner_project.sql @@ -0,0 +1,39 @@ +-- Phase 5 / M3 (v1.2 Hierarchies): multi-repo single-owner ownership (§7). +-- +-- A group node is authoritatively MANAGED by at most one project-root (the +-- repo whose manifest declares it as something it applies, not merely +-- references). `groups.owner_project` has carried this seam since 0047 as an +-- inert, FK-less UUID. M3 makes it a real reference: a `projects` table keyed +-- by a stable, gitignored project key the CLI mints in `.picloud/`, and a +-- foreign key from `groups.owner_project` to it. +-- +-- Ownership is recorded at GROUP granularity; apps inherit their owning project +-- from their group (apps are never claimed directly). A NULL `owner_project` +-- means the node is UI/API-owned — no manifest fights it (§7.5). First apply +-- claims; a second project's apply to an owned node is refused unless +-- `--takeover` (group-admin gated). Ownership ⟂ RBAC: owning the manifest does +-- NOT grant write — the actor still needs the usual group capabilities (§7.4). +-- +-- ON DELETE SET NULL: deleting a project row (not something the platform does +-- today) reverts its nodes to UI-owned rather than cascading away real groups. + +CREATE TABLE projects ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + -- Stable, opaque key minted by `pic init` and persisted (gitignored) in the + -- repo's `.picloud/`. The CLI presents it on every tree apply; the server + -- maps it to this row (upsert-by-key), so the same repo keeps the same + -- project identity across clones/CI without committing any server id. + key TEXT NOT NULL UNIQUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- Promote the inert `groups.owner_project` (0047) to a real FK now that the +-- referent exists. Existing rows are all NULL (no projects yet), so this adds +-- no validation burden. +ALTER TABLE groups + ADD CONSTRAINT groups_owner_project_fkey + FOREIGN KEY (owner_project) REFERENCES projects(id) ON DELETE SET NULL; + +-- The ownership reconcile reads "which groups does project P own?" (to find +-- owned-but-undeclared nodes for structural prune) and "who owns group G?". +CREATE INDEX groups_owner_project_idx ON groups (owner_project); diff --git a/crates/manager-core/src/apply_api.rs b/crates/manager-core/src/apply_api.rs index 1e46cce..bd81c9c 100644 --- a/crates/manager-core/src/apply_api.rs +++ b/crates/manager-core/src/apply_api.rs @@ -280,6 +280,16 @@ async fn group_apply_handler( // must hold the relevant read/write caps for EVERY node touched. // ---------------------------------------------------------------------------- +#[derive(Deserialize)] +struct TreePlanRequest { + bundle: TreeBundle, + /// Stable project key from the repo's `.picloud/` link state (§7, M3). + /// Lets `plan` surface ownership conflicts and prune candidates. Optional + /// for legacy callers (then no ownership diagnosis is reported). + #[serde(default)] + project_key: Option, +} + #[derive(Deserialize)] struct TreeApplyRequest { bundle: TreeBundle, @@ -296,15 +306,25 @@ struct TreeApplyRequest { /// unless it appears here; `--yes` alone does NOT add it. #[serde(default)] approved_envs: Vec, + /// Stable project key (§7, M3): the apply claims unclaimed declared groups + /// for this project and refuses ones another project owns. + #[serde(default)] + project_key: Option, + /// Take over declared groups owned by another project (group-admin gated). + #[serde(default)] + allow_takeover: bool, } async fn tree_plan_handler( State(svc): State, Extension(principal): Extension, - Json(bundle): Json, + Json(req): Json, ) -> Result, ApplyError> { - authz_tree(&svc, &principal, &bundle, None).await?; - Ok(Json(svc.plan_tree(&bundle).await?)) + authz_tree(&svc, &principal, &req.bundle, None, false).await?; + Ok(Json( + svc.plan_tree(&req.bundle, req.project_key.as_deref()) + .await?, + )) } async fn tree_apply_handler( @@ -312,7 +332,14 @@ async fn tree_apply_handler( Extension(principal): Extension, Json(req): Json, ) -> Result, ApplyError> { - authz_tree(&svc, &principal, &req.bundle, Some(req.prune)).await?; + authz_tree( + &svc, + &principal, + &req.bundle, + Some(req.prune), + req.allow_takeover, + ) + .await?; enforce_env_approval( &svc, &principal, @@ -325,8 +352,10 @@ async fn tree_apply_handler( .apply_tree( &req.bundle, req.prune, - principal.user_id, + &principal, req.expected_token.as_deref(), + req.project_key.as_deref(), + req.allow_takeover, ) .await?; Ok(Json(report)) @@ -396,6 +425,7 @@ async fn authz_tree( principal: &Principal, bundle: &TreeBundle, apply_prune: Option, + allow_takeover: bool, ) -> Result<(), ApplyError> { for node in &bundle.nodes { match node.kind { @@ -419,6 +449,28 @@ async fn authz_tree( Some(prune) => { require_group_node_writes(svc, principal, group_id, &node.bundle, prune) .await?; + // Takeover gate (§7.4): seizing a group another project + // owns needs GroupAdmin specifically — a second, independent + // gate on top of the write caps. We gate it for any + // already-claimed node under `--takeover` (a superset of + // genuinely-contested, never laxer); claiming an unclaimed + // node, or applying without `--takeover`, does not require + // admin. The authoritative owner rewrite happens in-tx in + // `apply_tree` (which re-checks under the tx lock). + if allow_takeover + && crate::group_repo::get_group_owner(&svc.pool, group_id) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + .is_some() + { + require( + svc.authz.as_ref(), + principal, + Capability::GroupAdmin(group_id), + ) + .await + .map_err(map_authz)?; + } } None => { require( @@ -509,15 +561,19 @@ async fn require_group_node_writes( bundle: &Bundle, prune: bool, ) -> Result<(), ApplyError> { - if prune || !bundle.scripts.is_empty() { - require( - svc.authz.as_ref(), - principal, - Capability::GroupScriptsWrite(group_id), - ) - .await - .map_err(map_authz)?; - } + // Baseline (§7.4, M3): applying a group node CLAIMS its ownership + // (`owner_project`) in-tx — a management write — even when the bundle has no + // scripts/vars to reconcile. Require an editor-level group write cap floor so + // an EMPTY `[group]` node can't be claimed capability-free (Ownership ⟂ RBAC: + // owning the manifest never grants write, and claiming still needs write). + // Subsumes the per-resource GroupScriptsWrite below. + require( + svc.authz.as_ref(), + principal, + Capability::GroupScriptsWrite(group_id), + ) + .await + .map_err(map_authz)?; if prune || !bundle.vars.is_empty() { require( svc.authz.as_ref(), @@ -576,7 +632,7 @@ impl IntoResponse for ApplyError { StatusCode::UNPROCESSABLE_ENTITY, json!({ "error": self.to_string() }), ), - Self::StateMoved | Self::ApprovalRequired(_) => { + Self::StateMoved | Self::ApprovalRequired(_) | Self::OwnershipConflict(_) => { (StatusCode::CONFLICT, json!({ "error": self.to_string() })) } Self::Forbidden => (StatusCode::FORBIDDEN, json!({ "error": self.to_string() })), diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index e678e18..3c5af40 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -32,15 +32,16 @@ use std::sync::Arc; use picloud_orchestrator_core::routing::{pattern, RouteTable}; use picloud_shared::{ AdminUserId, AppId, DispatchMode, DocsEventOp, FilesEventOp, GroupId, HostKind, KvEventOp, - MasterKey, PathKind, Route, Script, ScriptId, ScriptKind, ScriptOwner, ScriptSandbox, - ScriptValidator, TriggerId, + MasterKey, PathKind, Principal, Route, Script, ScriptId, ScriptKind, ScriptOwner, + ScriptSandbox, ScriptValidator, TriggerId, }; use serde::{Deserialize, Serialize}; use sqlx::PgPool; +use uuid::Uuid; use crate::app_domain_repo::AppDomainRepository; use crate::app_repo::AppRepository; -use crate::authz::AuthzRepo; +use crate::authz::{require, AuthzDenied, AuthzRepo, Capability}; use crate::config_resolver::CHAIN_LEVELS_CTE; use crate::repo::{ delete_script_tx, insert_script_tx, update_script_tx, NewScript, ScriptPatch, ScriptRepository, @@ -568,6 +569,23 @@ pub struct TreePlanResult { /// the gate at `plan` time, before an `apply` refuses. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub approvals_required: Vec, + /// Group nodes this manifest declares that another project already owns + /// (§7, M3). A read-only `plan` surfaces them so CI sees the conflict before + /// an `apply` refuses; `apply` proceeds only with `--takeover`. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub conflicts: Vec, + /// Slugs of groups this project owns that are absent from the manifest — + /// what `apply --prune` would delete (leaf-first, RESTRICT-guarded). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub structural_prunes: Vec, +} + +/// One ownership conflict: a declared group already claimed by another project. +#[derive(Debug, Clone, Serialize)] +pub struct OwnershipConflict { + pub slug: String, + /// The owning project's key (so the operator knows whose node it is). + pub owner_key: String, } // ---------------------------------------------------------------------------- @@ -677,6 +695,11 @@ pub enum ApplyError { re-run with `--approve {0}` (a blanket `--yes` does not cover it)" )] ApprovalRequired(String), + #[error( + "one or more declared groups are owned by another project; \ + re-run with `--takeover` to claim them (group-admin required): {0}" + )] + OwnershipConflict(String), #[error("authorization repo error: {0}")] AuthzRepo(String), #[error("backend: {0}")] @@ -1503,14 +1526,19 @@ impl ApplyService { /// /// # Errors /// `Invalid` for a malformed/unknown bundle; `Backend` for repo failures. - pub async fn plan_tree(&self, bundle: &TreeBundle) -> Result { - let (prepared, token) = self.prepare_tree(bundle).await?; - let nodes = prepared - .into_iter() + pub async fn plan_tree( + &self, + bundle: &TreeBundle, + project_key: Option<&str>, + ) -> Result { + let pt = self.prepare_tree(bundle, project_key).await?; + let nodes = pt + .prepared + .iter() .map(|p| NodePlan { kind: p.node.kind, slug: p.node.slug.clone(), - plan: p.plan, + plan: p.plan.clone(), }) .collect(); let approvals_required = bundle @@ -1520,8 +1548,10 @@ impl ApplyService { .unwrap_or_default(); Ok(TreePlanResult { nodes, - state_token: token, + state_token: pt.token, approvals_required, + conflicts: pt.conflicts, + structural_prunes: pt.prune_candidates.into_iter().map(|(_, s)| s).collect(), }) } @@ -1534,19 +1564,37 @@ impl ApplyService { /// # Errors /// `Invalid` for a bad bundle; `StateMoved` if the bound token is stale; /// `Backend` for repo/transaction failures. + #[allow(clippy::too_many_arguments, clippy::too_many_lines)] pub async fn apply_tree( &self, bundle: &TreeBundle, prune: bool, - actor: AdminUserId, + principal: &Principal, expected_token: Option<&str>, + project_key: Option<&str>, + allow_takeover: bool, ) -> Result { - let (prepared, token) = self.prepare_tree(bundle).await?; + let actor = principal.user_id; + let pt = self.prepare_tree(bundle, project_key).await?; if let Some(expected) = expected_token { - if token != expected { + if pt.token != expected { return Err(ApplyError::StateMoved); } } + // Ownership pre-check (§7, M3): refuse, before opening the tx, if any + // declared group is owned by another project and `--takeover` was not + // given. The in-tx re-read below is the authoritative gate against a + // concurrent claim; this is the fast, clear path for the common case. + if !allow_takeover && !pt.conflicts.is_empty() { + return Err(ApplyError::OwnershipConflict(format_conflicts( + &pt.conflicts, + ))); + } + let PreparedTree { + prepared, + existing_group_nodes, + .. + } = pt; let mut tx = self .pool @@ -1569,6 +1617,59 @@ impl ApplyService { let mut report = ApplyReport::default(); let mut group_script_index: HashMap> = HashMap::new(); + // Resolve (upsert) this repo's project row inside the tx, so declared + // group nodes can be stamped with a stable owner id (§7, M3). A tree + // apply with no project key (legacy callers) skips all ownership. + let our_project_id = match project_key { + Some(k) => Some( + crate::group_repo::upsert_project_tx(&mut tx, k) + .await + .map_err(map_group_err)?, + ), + None => None, + }; + + // Phase 0.5 — ownership reconcile (§7, M3): for each declared group, + // re-read the owner under this tx's lock (authoritative) and claim it + // (NULL owner) or take it over (`--takeover`). A conflict that appeared + // after `plan` and wasn't authorized aborts the whole apply. + if let Some(pid) = our_project_id { + for (gid, slug) in &existing_group_nodes { + let owner = crate::group_repo::get_group_owner_tx(&mut tx, *gid) + .await + .map_err(map_group_err)?; + match owner { + Some((oid, _)) if oid == pid => {} // already ours — no write + Some((_, okey)) => { + if !allow_takeover { + return Err(ApplyError::OwnershipConflict(format!( + "{slug} (owned by project {okey})" + ))); + } + // Authoritative takeover gate (§7.4): GroupAdmin is + // re-verified HERE, at the in-tx ownership decision, not + // only in the pre-tx `authz_tree` pool read — otherwise + // `--force` (which waives the staleness token) would let + // a non-admin seize a group that another project claimed + // after the pre-tx check saw it unowned. Ownership ⟂ RBAC. + require(self.authz.as_ref(), principal, Capability::GroupAdmin(*gid)) + .await + .map_err(map_authz_denied)?; + crate::group_repo::set_group_owner_tx(&mut tx, *gid, pid) + .await + .map_err(map_group_err)?; + report.groups_taken_over += 1; + } + None => { + crate::group_repo::set_group_owner_tx(&mut tx, *gid, pid) + .await + .map_err(map_group_err)?; + report.groups_claimed += 1; + } + } + } + } + // Phase A — groups first: reconcile each and record its post-create // `name -> id` so descendant app nodes can resolve inherited bindings. for p in &prepared { @@ -1623,6 +1724,17 @@ impl ApplyService { } } + // Phase C — structural prune (§7, M3): with `--prune`, delete groups + // THIS project owns that the manifest no longer declares, leaf-first + // (delete_group_tx is RESTRICT, so a group still holding apps/child-groups + // aborts the apply with a clear error rather than orphaning data). + if prune { + if let Some(pid) = our_project_id { + self.structural_prune_tx(&mut tx, pid, &existing_group_nodes, &mut report) + .await?; + } + } + tx.commit() .await .map_err(|e| ApplyError::Backend(e.to_string()))?; @@ -1641,6 +1753,107 @@ impl ApplyService { Ok(report) } + /// Phase C of a tree apply (§7, M3): delete groups THIS project owns that the + /// manifest no longer declares, leaf-first. `delete_group_tx` is RESTRICT, so + /// a candidate still holding apps or non-pruned child groups aborts the whole + /// apply (no silent orphaning). Scoped to project-owned nodes: another repo's + /// groups and UI-owned (unclaimed) groups are never touched. + async fn structural_prune_tx( + &self, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + project_id: Uuid, + existing_group_nodes: &[(GroupId, String)], + report: &mut ApplyReport, + ) -> Result<(), ApplyError> { + // Declared groups are kept; every other group this project owns is a + // prune candidate. + let declared: HashSet = existing_group_nodes + .iter() + .map(|(_, s)| s.to_lowercase()) + .collect(); + + let owned = crate::group_repo::list_owned_groups_tx(tx, project_id) + .await + .map_err(map_group_err)?; + let mut candidates: Vec<(GroupId, String, Option)> = owned + .into_iter() + .filter(|(_, slug, _)| !declared.contains(&slug.to_lowercase())) + .collect(); + if candidates.is_empty() { + return Ok(()); + } + + // Data-loss guard (§7 M3): the apps/child-group FKs are RESTRICT (a + // non-empty group aborts loudly via delete_group_tx), but the §11.6 + // shared surfaces (group_collections + secrets) CASCADE. Refuse to + // silently vaporize a tenant's shared data/secrets just because a + // manifest node was dropped — mark such a group KEEP (with a warning), + // AND keep every candidate ancestor of it (RESTRICT would abort the + // apply if we tried to delete a parent whose protected child we're + // preserving). Also re-confirm ownership under the tx here — a + // concurrent `--takeover` could have flipped a candidate (undeclared, so + // the bound-plan token doesn't cover it) since the candidate SELECT. + let parent_of: HashMap> = + candidates.iter().map(|(g, _, p)| (*g, *p)).collect(); + let mut keep: HashSet = HashSet::new(); + for (gid, slug, _) in &candidates { + let still_owned = crate::group_repo::get_group_owner_tx(tx, *gid) + .await + .map_err(map_group_err)? + .map(|(oid, _)| oid) + == Some(project_id); + let protected = crate::group_repo::group_has_protected_data_tx(tx, *gid) + .await + .map_err(map_group_err)?; + if !still_owned || protected { + if protected { + report.warnings.push(format!( + "group `{slug}` owns shared collections or secrets; \ + not pruned (delete it explicitly with `pic groups rm`)" + )); + } + // Walk up the candidate chain, keeping this node + every + // candidate ancestor (a kept descendant RESTRICT-pins them). + let mut cur = Some(*gid); + while let Some(g) = cur { + if !keep.insert(g) { + break; // already walked from here + } + cur = parent_of.get(&g).copied().flatten(); + } + } + } + candidates.retain(|(g, _, _)| !keep.contains(g)); + if candidates.is_empty() { + return Ok(()); + } + + // Delete leaf-first: defer any candidate that is still the parent of + // another candidate until that child is gone. + while !candidates.is_empty() { + let parents: HashSet = candidates.iter().filter_map(|(_, _, p)| *p).collect(); + let before = candidates.len(); + let mut remaining: Vec<(GroupId, String, Option)> = Vec::new(); + for (gid, slug, parent) in std::mem::take(&mut candidates) { + if parents.contains(&gid) { + remaining.push((gid, slug, parent)); + continue; + } + crate::group_repo::delete_group_tx(tx, gid) + .await + .map_err(map_group_err)?; + report.groups_pruned += 1; + } + if remaining.len() == before { + return Err(ApplyError::Invalid( + "structural prune could not order owned groups for deletion (cycle?)".into(), + )); + } + candidates = remaining; + } + Ok(()) + } + /// Resolve nodes to owners, validate each (apps' route/trigger targets may /// bind to in-tree ancestor group scripts), compute each node's plan, and /// fold a combined bound-plan token. Shared by `plan_tree` / `apply_tree`. @@ -1648,10 +1861,14 @@ impl ApplyService { async fn prepare_tree<'a>( &self, bundle: &'a TreeBundle, - ) -> Result<(Vec>, String), ApplyError> { + project_key: Option<&str>, + ) -> Result, ApplyError> { if bundle.nodes.is_empty() { return Err(ApplyError::Invalid("project tree has no nodes".into())); } + if let Some(k) = project_key { + validate_project_key(k)?; + } // Register in-tree group nodes first: their (resolved) ids and endpoint // script names, which an app node's binding validation may reference. let mut group_id_by_slug: HashMap = HashMap::new(); @@ -1787,8 +2004,70 @@ impl ApplyService { envs.sort_unstable(); token_parts.push(format!("proj|{}", envs.join(","))); } + + // Ownership (§7, M3): resolve this repo's project, then classify each + // declared group node as ours / unclaimed / owned-by-another. Folding + // the owner key into the token means a claim or takeover by a different + // repo between plan and apply trips StateMoved. (Every group node on the + // tree-apply path pre-exists — `resolve_group` errors otherwise — so + // there is no to-create node to skip, unlike a create-capable engine.) + let our_project_id = match project_key { + Some(k) => crate::group_repo::get_project_id_by_key(&self.pool, k) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?, + None => None, + }; + let mut existing_group_nodes: Vec<(GroupId, String)> = Vec::new(); + let mut declared_group_slugs: HashSet = HashSet::new(); + let mut conflicts: Vec = Vec::new(); + for n in &bundle.nodes { + if n.kind != NodeKind::Group { + continue; + } + let lc = n.slug.to_lowercase(); + declared_group_slugs.insert(lc.clone()); + let gid = group_id_by_slug[&lc]; + existing_group_nodes.push((gid, n.slug.clone())); + let owner = crate::group_repo::get_group_owner(&self.pool, gid) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + match &owner { + Some((_, key)) => token_parts.push(format!("own|{lc}|{key}")), + None => token_parts.push(format!("own|{lc}|none")), + } + if let Some((oid, okey)) = owner { + if Some(oid) != our_project_id { + conflicts.push(OwnershipConflict { + slug: n.slug.clone(), + owner_key: okey, + }); + } + } + } + + // Structural-prune candidates: groups THIS project owns that the + // manifest omits. Computed read-only here (so `plan` can list them); + // `apply --prune` re-derives the set in-tx and deletes leaf-first. + let mut prune_candidates: Vec<(GroupId, String)> = Vec::new(); + if let Some(pid) = our_project_id { + for (gid, slug) in crate::group_repo::list_owned_groups(&self.pool, pid) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + { + if !declared_group_slugs.contains(&slug.to_lowercase()) { + prune_candidates.push((gid, slug)); + } + } + } + token_parts.sort_unstable(); - Ok((prepared, combine_tokens(&token_parts))) + Ok(PreparedTree { + prepared, + token: combine_tokens(&token_parts), + existing_group_nodes, + conflicts, + prune_candidates, + }) } /// Resolve an app node's inherited route/trigger targets to script ids, @@ -3723,6 +4002,15 @@ pub struct ApplyReport { pub suppressions_created: u32, #[serde(default)] pub suppressions_deleted: u32, + /// Group nodes this apply claimed (NULL owner → this project), §7 M3. + #[serde(default)] + pub groups_claimed: u32, + /// Group nodes this apply took over from another project (`--takeover`). + #[serde(default)] + pub groups_taken_over: u32, + /// Owned-but-undeclared groups deleted by structural prune (`--prune`). + #[serde(default)] + pub groups_pruned: u32, #[serde(skip_serializing_if = "Vec::is_empty")] pub warnings: Vec, } @@ -3745,6 +4033,21 @@ struct PreparedNode<'a> { app_chain: Vec, } +/// The read-only result of `prepare_tree`: per-node plans + one combined token, +/// plus the ownership classification (§7, M3) the plan surfaces and the apply +/// acts on. +struct PreparedTree<'a> { + prepared: Vec>, + token: String, + /// Declared group nodes in the bundle: `(id, slug)` — the apply path + /// re-reads each owner in-tx (authoritative) to claim / detect takeover. + existing_group_nodes: Vec<(GroupId, String)>, + /// Declared groups already owned by ANOTHER project (read-only diagnosis). + conflicts: Vec, + /// Owned-but-undeclared groups: `(id, slug)` — structural-prune candidates. + prune_candidates: Vec<(GroupId, String)>, +} + /// FNV-1a over a set of already-sorted per-resource token parts — the same /// hashing `state_token` uses, lifted to combine many nodes into one tree /// token. Order-independent because the caller sorts `parts`. @@ -3863,6 +4166,57 @@ fn map_trig(e: TriggerRepoError) -> ApplyError { } } +/// Map a group-repo error to the apply surface (§7, M3). A `Conflict` (e.g. a +/// structural-prune candidate still holding descendants) is caller-actionable → +/// `Invalid`; a `NotFound` mid-apply is a lost race; `Db` is backend. +fn map_group_err(e: crate::group_repo::GroupRepositoryError) -> ApplyError { + use crate::group_repo::GroupRepositoryError as E; + match e { + E::Conflict(m) => ApplyError::Invalid(m), + E::NotFound(_) => ApplyError::Invalid("a referenced group was not found".into()), + E::Db(e) => ApplyError::Backend(e.to_string()), + } +} + +/// Map an in-service authz denial to the apply error surface (403 vs 500). +fn map_authz_denied(e: AuthzDenied) -> ApplyError { + match e { + AuthzDenied::Denied => ApplyError::Forbidden, + AuthzDenied::Repo(r) => ApplyError::AuthzRepo(r.to_string()), + } +} + +/// Validate an attacker-supplied project key (§7, M3). The key is a bearer +/// handle for ownership, so it must be bounded and opaque before it's reflected +/// into error messages or upserted. The CLI mints a v4 UUID; we accept any +/// short, printable, delimiter-free token (ASCII alphanumeric plus `-`/`_`). +fn validate_project_key(key: &str) -> Result<(), ApplyError> { + if key.is_empty() || key.len() > 128 { + return Err(ApplyError::Invalid( + "project key must be 1–128 characters".into(), + )); + } + if !key + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_') + { + return Err(ApplyError::Invalid( + "project key may contain only ASCII letters, digits, '-' and '_'".into(), + )); + } + Ok(()) +} + +/// Render an ownership-conflict list as `slug (owned by project key), …` for +/// the `OwnershipConflict` error message. +fn format_conflicts(conflicts: &[OwnershipConflict]) -> String { + conflicts + .iter() + .map(|c| format!("{} (owned by project {})", c.slug, c.owner_key)) + .collect::>() + .join(", ") +} + /// A stable fingerprint of an app's live state, covering exactly what the /// reconcile diff keys on: script identity + write-counter (`version` bumps on /// any script edit), route identity + binding/attrs, trigger semantic identity @@ -4820,4 +5174,34 @@ mod tests { assert!(!policy.confirm_required("unknown")); assert_eq!(policy.gated_envs(), vec!["production".to_string()]); } + + #[test] + fn format_conflicts_lists_each_node_and_owner() { + // The conflict message drives the CLI error a developer sees; pin it. + let c = vec![ + OwnershipConflict { + slug: "team-a".into(), + owner_key: "key-x".into(), + }, + OwnershipConflict { + slug: "team-b".into(), + owner_key: "key-y".into(), + }, + ]; + assert_eq!( + format_conflicts(&c), + "team-a (owned by project key-x), team-b (owned by project key-y)" + ); + assert_eq!(format_conflicts(&[]), ""); + } + + #[test] + fn validate_project_key_bounds_and_charset() { + assert!(validate_project_key("a1b2-c3_d4").is_ok()); + assert!(validate_project_key(&uuid::Uuid::nil().to_string()).is_ok()); + assert!(validate_project_key("").is_err()); + assert!(validate_project_key(&"x".repeat(129)).is_err()); + assert!(validate_project_key("has space").is_err()); + assert!(validate_project_key("semi;colon").is_err()); + } } diff --git a/crates/manager-core/src/group_repo.rs b/crates/manager-core/src/group_repo.rs index aaa1b4f..f438991 100644 --- a/crates/manager-core/src/group_repo.rs +++ b/crates/manager-core/src/group_repo.rs @@ -339,6 +339,184 @@ impl GroupRepository for PostgresGroupRepository { } } +// ---------------------------------------------------------------------------- +// Project ownership (§7, M3). A `projects` row is keyed by the stable, opaque +// key the CLI mints in `.picloud/`; `groups.owner_project` references it. These +// helpers are free functions (pool + tx variants) so the apply path can claim +// ownership inside the single apply transaction (first-commit-wins), while the +// read-only plan path can surface conflicts without writing. +// ---------------------------------------------------------------------------- + +/// Resolve a project key to its id, if the project has ever been persisted. +/// `None` means no apply has claimed under this key yet (so `plan` treats every +/// node as claimable). Read-only — used by the plan path. +pub(crate) async fn get_project_id_by_key( + pool: &PgPool, + key: &str, +) -> Result, GroupRepositoryError> { + let row: Option<(Uuid,)> = sqlx::query_as("SELECT id FROM projects WHERE key = $1") + .bind(key) + .fetch_optional(pool) + .await?; + Ok(row.map(|r| r.0)) +} + +/// Upsert the project keyed by `key` inside the apply tx and return its id. +/// Idempotent: re-applying the same repo reuses the same project identity. +pub(crate) async fn upsert_project_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + key: &str, +) -> Result { + let row: (Uuid,) = sqlx::query_as( + "INSERT INTO projects (key) VALUES ($1) \ + ON CONFLICT (key) DO UPDATE SET key = EXCLUDED.key RETURNING id", + ) + .bind(key) + .fetch_one(&mut **tx) + .await?; + Ok(row.0) +} + +/// The project that owns `group_id`, as `(project_id, project_key)`. `None` when +/// the node is UI/API-owned (no claim). Read-only — used by plan + the takeover +/// pre-gate. Joins through `projects` so the caller gets the human-facing key +/// for a conflict message. +pub(crate) async fn get_group_owner( + pool: &PgPool, + group_id: GroupId, +) -> Result, GroupRepositoryError> { + let row: Option<(Uuid, String)> = sqlx::query_as( + "SELECT p.id, p.key FROM groups g \ + JOIN projects p ON p.id = g.owner_project WHERE g.id = $1", + ) + .bind(group_id.into_inner()) + .fetch_optional(pool) + .await?; + Ok(row) +} + +/// The same owner lookup, but inside the apply tx — the authoritative read that +/// the claim/conflict decision keys on (so a concurrent claim that committed +/// after `plan` is seen under this tx's per-node advisory lock). +pub(crate) async fn get_group_owner_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + group_id: GroupId, +) -> Result, GroupRepositoryError> { + let row: Option<(Uuid, String)> = sqlx::query_as( + "SELECT p.id, p.key FROM groups g \ + JOIN projects p ON p.id = g.owner_project WHERE g.id = $1", + ) + .bind(group_id.into_inner()) + .fetch_optional(&mut **tx) + .await?; + Ok(row) +} + +/// Stamp (claim or take over) `group_id`'s owning project inside the apply tx. +/// Bumps `structure_version` so an ownership flip trips a bound plan token. +pub(crate) async fn set_group_owner_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + group_id: GroupId, + project_id: Uuid, +) -> Result<(), GroupRepositoryError> { + let res = sqlx::query( + "UPDATE groups SET owner_project = $2, \ + structure_version = structure_version + 1, updated_at = NOW() WHERE id = $1", + ) + .bind(group_id.into_inner()) + .bind(project_id) + .execute(&mut **tx) + .await?; + if res.rows_affected() == 0 { + return Err(GroupRepositoryError::NotFound(group_id)); + } + Ok(()) +} + +/// Every group `(id, slug)` owned by `project_id` — the candidate set for +/// structural prune (those absent from the manifest are the ones to delete). +/// Read-only; the apply path filters and deletes leaf-first under `delete_group_tx`. +pub(crate) async fn list_owned_groups( + pool: &PgPool, + project_id: Uuid, +) -> Result, GroupRepositoryError> { + let rows: Vec<(Uuid, String)> = + sqlx::query_as("SELECT id, slug FROM groups WHERE owner_project = $1") + .bind(project_id) + .fetch_all(pool) + .await?; + Ok(rows + .into_iter() + .map(|(id, slug)| (id.into(), slug)) + .collect()) +} + +/// The same owned-group enumeration inside the apply tx, with each node's +/// `parent_id` so the prune can delete leaf-first (children before parents). +pub(crate) async fn list_owned_groups_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + project_id: Uuid, +) -> Result)>, GroupRepositoryError> { + let rows: Vec<(Uuid, String, Option)> = + sqlx::query_as("SELECT id, slug, parent_id FROM groups WHERE owner_project = $1") + .bind(project_id) + .fetch_all(&mut **tx) + .await?; + Ok(rows + .into_iter() + .map(|(id, slug, parent)| (id.into(), slug, parent.map(GroupId::from))) + .collect()) +} + +/// Does this group hold data a structural prune would SILENTLY cascade away +/// (§7 M3 safety guard)? The `apps`/child-group FKs are RESTRICT (delete aborts +/// loudly), but the §11.6 group-owned surfaces are `ON DELETE CASCADE`: shared +/// collections (`group_collections`, covering kv/docs/files/topic/queue) and +/// group secrets (owner-polymorphic `secrets.group_id`). Rather than let +/// `apply --prune` vaporize a tenant's shared data + secrets because a manifest +/// node was dropped, the prune skips such a group with a warning — the operator +/// must delete it explicitly. Returns true if the group has any shared-collection +/// marker or any secret. +pub(crate) async fn group_has_protected_data_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + group_id: GroupId, +) -> Result { + let row: (bool,) = sqlx::query_as( + "SELECT EXISTS (SELECT 1 FROM group_collections WHERE group_id = $1) \ + OR EXISTS (SELECT 1 FROM secrets WHERE group_id = $1)", + ) + .bind(group_id.into_inner()) + .fetch_one(&mut **tx) + .await?; + Ok(row.0) +} + +/// Delete a group inside the apply tx (structural prune, §7 M3). RESTRICT is the +/// real guard: the FK from apps/child-groups aborts the whole apply (mapped to a +/// clean `Conflict`) rather than orphaning data, so the caller must delete +/// leaf-first and never reaches a group that still holds live descendants. +pub(crate) async fn delete_group_tx( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + group_id: GroupId, +) -> Result<(), GroupRepositoryError> { + let res = sqlx::query("DELETE FROM groups WHERE id = $1") + .bind(group_id.into_inner()) + .execute(&mut **tx) + .await; + match res { + Ok(r) if r.rows_affected() == 0 => Err(GroupRepositoryError::NotFound(group_id)), + Ok(_) => Ok(()), + Err(sqlx::Error::Database(e)) if e.is_foreign_key_violation() => { + Err(GroupRepositoryError::Conflict( + "group still has descendants (apps or subgroups); \ + structural prune refuses to orphan them" + .into(), + )) + } + Err(e) => Err(e.into()), + } +} + #[derive(sqlx::FromRow)] struct GroupRow { id: Uuid, diff --git a/crates/manager-core/tests/expected_schema.txt b/crates/manager-core/tests/expected_schema.txt index 4d721a4..bd2a83a 100644 --- a/crates/manager-core/tests/expected_schema.txt +++ b/crates/manager-core/tests/expected_schema.txt @@ -318,6 +318,11 @@ table: outbox claimed_by: text NULL created_at: timestamp with time zone NOT NULL default=now() +table: projects + id: uuid NOT NULL default=gen_random_uuid() + key: text NOT NULL + created_at: timestamp with time zone NOT NULL default=now() + table: pubsub_trigger_details trigger_id: uuid NOT NULL topic_pattern: text NOT NULL @@ -579,6 +584,7 @@ indexes on group_queue_messages: idx_group_queue_messages_group_collection: public.group_queue_messages USING btree (group_id, collection) indexes on groups: + groups_owner_project_idx: public.groups USING btree (owner_project) groups_parent_id_idx: public.groups USING btree (parent_id) groups_pkey: public.groups USING btree (id) groups_slug_key: public.groups USING btree (slug) @@ -595,6 +601,10 @@ indexes on outbox: idx_outbox_due: public.outbox USING btree (next_attempt_at) WHERE (claimed_at IS NULL) outbox_pkey: public.outbox USING btree (id) +indexes on projects: + projects_key_key: public.projects USING btree (key) + projects_pkey: public.projects USING btree (id) + indexes on pubsub_trigger_details: pubsub_trigger_details_pkey: public.pubsub_trigger_details USING btree (trigger_id) @@ -817,6 +827,7 @@ constraints on group_queue_messages: [PRIMARY KEY] group_queue_messages_pkey: PRIMARY KEY (id) constraints on groups: + [FOREIGN KEY] groups_owner_project_fkey: FOREIGN KEY (owner_project) REFERENCES projects(id) ON DELETE SET NULL [FOREIGN KEY] groups_parent_id_fkey: FOREIGN KEY (parent_id) REFERENCES groups(id) ON DELETE RESTRICT [PRIMARY KEY] groups_pkey: PRIMARY KEY (id) [UNIQUE] groups_slug_key: UNIQUE (slug) @@ -834,6 +845,10 @@ constraints on outbox: [FOREIGN KEY] outbox_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE [PRIMARY KEY] outbox_pkey: PRIMARY KEY (id) +constraints on projects: + [PRIMARY KEY] projects_pkey: PRIMARY KEY (id) + [UNIQUE] projects_key_key: UNIQUE (key) + constraints on pubsub_trigger_details: [FOREIGN KEY] pubsub_trigger_details_trigger_id_fkey: FOREIGN KEY (trigger_id) REFERENCES triggers(id) ON DELETE CASCADE [PRIMARY KEY] pubsub_trigger_details_pkey: PRIMARY KEY (trigger_id) @@ -973,3 +988,4 @@ constraints on vars: 0063: materialized unique 0064: shared topic queue kinds 0065: group queues + 0066: owner project diff --git a/crates/picloud-cli/Cargo.toml b/crates/picloud-cli/Cargo.toml index caa5d71..609b286 100644 --- a/crates/picloud-cli/Cargo.toml +++ b/crates/picloud-cli/Cargo.toml @@ -34,6 +34,8 @@ toml = "0.8" directories = "5" rpassword = "7" anyhow = "1" +# §7 M3: minting the stable project key in `.picloud/project.json`. +uuid = { version = "1", features = ["v4"] } [dev-dependencies] assert_cmd = "2" diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 21dd195..d5b6eea 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -1262,17 +1262,29 @@ impl Client { } /// `POST /api/v1/admin/tree/plan` — diff a whole project tree (Phase 5). - pub async fn plan_tree(&self, bundle: &serde_json::Value) -> Result { + /// `project_key` (§7, M3) lets the plan surface ownership conflicts + prune + /// candidates for this repo. + pub async fn plan_tree( + &self, + bundle: &serde_json::Value, + project_key: Option<&str>, + ) -> Result { + let body = serde_json::json!({ + "bundle": bundle, + "project_key": project_key, + }); let resp = self .request(Method::POST, "/api/v1/admin/tree/plan") - .json(bundle) + .json(&body) .send() .await?; decode(resp).await } /// `POST /api/v1/admin/tree/apply` — reconcile a whole project tree in one - /// transaction (Phase 5). + /// transaction (Phase 5). `project_key` + `allow_takeover` drive the §7 M3 + /// ownership claim / takeover. + #[allow(clippy::too_many_arguments)] pub async fn apply_tree( &self, bundle: &serde_json::Value, @@ -1280,6 +1292,8 @@ impl Client { expected_token: Option<&str>, env: Option<&str>, approved_envs: &[String], + project_key: Option<&str>, + allow_takeover: bool, ) -> Result { let body = serde_json::json!({ "bundle": bundle, @@ -1287,6 +1301,8 @@ impl Client { "expected_token": expected_token, "env": env, "approved_envs": approved_envs, + "project_key": project_key, + "allow_takeover": allow_takeover, }); let resp = self .request(Method::POST, "/api/v1/admin/tree/apply") @@ -1296,6 +1312,12 @@ impl Client { if resp.status() == reqwest::StatusCode::CONFLICT { let body = resp.text().await.unwrap_or_default(); let msg = parse_error_body(&body).unwrap_or(body); + // An ownership conflict (§7) is self-explanatory ("re-run with + // --takeover"); only a staleness (token) conflict warrants the + // re-plan hint. Distinguish on the server's message. + if msg.contains("owned by another project") { + return Err(anyhow!("{msg}")); + } return Err(anyhow!( "{msg}\nThe project tree changed since your last `pic plan`. Re-run \ `pic plan --dir` to review, then `pic apply --dir` — or add `--force`." @@ -1538,6 +1560,19 @@ pub struct TreePlanDto { /// Environments the project marks confirm-required (§4.2, M5). #[serde(default)] pub approvals_required: Vec, + /// Declared groups another project already owns (§7, M3). + #[serde(default)] + pub conflicts: Vec, + /// Owned-but-undeclared group slugs `apply --prune` would delete (§7, M3). + #[serde(default)] + pub structural_prunes: Vec, +} + +/// A group declared by this manifest that another project owns (§7, M3). +#[derive(Debug, Deserialize)] +pub struct OwnershipConflictDto { + pub slug: String, + pub owner_key: String, } #[derive(Debug, Deserialize)] @@ -1599,6 +1634,15 @@ pub struct ApplyReportDto { pub suppressions_created: u32, #[serde(default)] pub suppressions_deleted: u32, + /// Group nodes claimed this apply (§7, M3). + #[serde(default)] + pub groups_claimed: u32, + /// Group nodes taken over from another project this apply (§7, M3). + #[serde(default)] + pub groups_taken_over: u32, + /// Owned-but-undeclared groups deleted by structural prune (§7, M3). + #[serde(default)] + pub groups_pruned: u32, #[serde(default)] pub warnings: Vec, } diff --git a/crates/picloud-cli/src/cmds/apply.rs b/crates/picloud-cli/src/cmds/apply.rs index 7355df3..3661ddb 100644 --- a/crates/picloud-cli/src/cmds/apply.rs +++ b/crates/picloud-cli/src/cmds/apply.rs @@ -143,6 +143,7 @@ pub async fn run_tree( force: bool, env: Option<&str>, approve: &[String], + takeover: bool, mode: OutputMode, ) -> Result<()> { let creds = config::resolve()?; @@ -159,6 +160,11 @@ pub async fn run_tree( // local check just fails fast with a clear message. let approved = resolve_approvals(env, &envs, approve)?; + // Mint/read this repo's stable project key (§7, M3): the apply claims the + // declared groups for this project and refuses ones another project owns + // (unless `--takeover`, group-admin gated). + let project_key = crate::linkstate::ensure_project_key(dir)?; + let token_key = crate::cmds::plan::TREE_TOKEN_KEY; let expected_token = if force { None @@ -169,13 +175,28 @@ pub async fn run_tree( }; let report = client - .apply_tree(&bundle, prune, expected_token.as_deref(), env, &approved) + .apply_tree( + &bundle, + prune, + expected_token.as_deref(), + env, + &approved, + Some(&project_key), + takeover, + ) .await?; crate::linkstate::clear_plan(dir, token_key); let mut block = KvBlock::new(); block .field("nodes", node_count.to_string()) + .field( + "groups", + format!( + "claimed {} taken-over {} pruned {}", + report.groups_claimed, report.groups_taken_over, report.groups_pruned + ), + ) .field( "scripts", format!( diff --git a/crates/picloud-cli/src/cmds/init.rs b/crates/picloud-cli/src/cmds/init.rs index 588afe5..0dc9dfd 100644 --- a/crates/picloud-cli/src/cmds/init.rs +++ b/crates/picloud-cli/src/cmds/init.rs @@ -87,6 +87,9 @@ pub fn run( } fs::write(&manifest_path, body).with_context(|| format!("writing {MANIFEST_FILE}"))?; ensure_gitignored(dir)?; + // Mint this repo's stable, gitignored project identity (§7, M3) up front so + // the first tree apply already has an ownership handle to claim under. + crate::linkstate::ensure_project_key(dir)?; let mut block = KvBlock::new(); block diff --git a/crates/picloud-cli/src/cmds/plan.rs b/crates/picloud-cli/src/cmds/plan.rs index 2360e91..131b9f1 100644 --- a/crates/picloud-cli/src/cmds/plan.rs +++ b/crates/picloud-cli/src/cmds/plan.rs @@ -50,7 +50,10 @@ 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, _envs) = crate::discover::build_tree(dir, env)?; - let plan = client.plan_tree(&bundle).await?; + // Mint/read this repo's stable project key (§7, M3) so the plan can surface + // ownership conflicts + structural-prune candidates for us. + let project_key = crate::linkstate::ensure_project_key(dir)?; + let plan = client.plan_tree(&bundle, Some(&project_key)).await?; if !plan.state_token.is_empty() { if let Err(e) = crate::linkstate::write_plan(dir, TREE_TOKEN_KEY, &plan.state_token) { eprintln!("warning: could not record plan state for `pic apply`: {e}"); @@ -96,6 +99,24 @@ fn render_tree(plan: &TreePlanDto, mode: OutputMode) { eprintln!(" - {e}"); } } + + // Ownership (§7, M3): surface groups another project owns (apply refuses + // without `--takeover`) and owned-but-undeclared groups `--prune` would + // delete, so both are visible before an apply acts on them. + if mode != OutputMode::Json { + if !plan.conflicts.is_empty() { + eprintln!("\ngroups owned by another project (apply with --takeover to claim):"); + for c in &plan.conflicts { + eprintln!(" - {} (owned by project {})", c.slug, c.owner_key); + } + } + if !plan.structural_prunes.is_empty() { + eprintln!("\ngroups this project owns but no longer declares (--prune deletes):"); + for s in &plan.structural_prunes { + eprintln!(" - {s}"); + } + } + } } /// Assemble the wire bundle: scripts carry inlined source (read from diff --git a/crates/picloud-cli/src/linkstate.rs b/crates/picloud-cli/src/linkstate.rs index e799407..a1e6ab5 100644 --- a/crates/picloud-cli/src/linkstate.rs +++ b/crates/picloud-cli/src/linkstate.rs @@ -13,6 +13,71 @@ use serde::{Deserialize, Serialize}; const DIR: &str = ".picloud"; const PLAN_FILE: &str = "plan.json"; +const PROJECT_FILE: &str = "project.json"; + +/// The repo's stable project identity (§7, M3): a random, opaque key minted on +/// first need and persisted (gitignored) in `.picloud/`. It is the server-side +/// ownership handle — the same repo keeps the same project across clones/CI +/// because the key travels in the working tree, never in a committed file. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ProjectLink { + pub key: String, +} + +fn project_path(base: &Path) -> PathBuf { + base.join(DIR).join(PROJECT_FILE) +} + +/// Read the project key, if one has been minted under `base/.picloud/`. +#[must_use] +pub fn read_project_key(base: &Path) -> Option { + let body = fs::read(project_path(base)).ok()?; + serde_json::from_slice::(&body) + .ok() + .map(|l| l.key) +} + +/// Read the project key, minting and persisting a fresh one if absent. Used by +/// the tree `plan`/`apply` paths so a repo that never ran `pic init` still gets +/// a stable ownership identity on first use. The new key is a random v4 UUID. +pub fn ensure_project_key(base: &Path) -> Result { + if let Some(k) = read_project_key(base) { + return Ok(k); + } + // A present-but-unreadable/corrupt `project.json` must NOT be silently + // re-minted: a fresh key would orphan every group this repo already claimed + // (the old key is lost, so the next apply hits an ownership conflict). Fail + // loudly and let the operator restore or intentionally re-key it. + let path = project_path(base); + if path.exists() { + anyhow::bail!( + "{} exists but could not be parsed as a project key; refusing to mint a new one \ + (a new key would orphan this repo's group ownership). Restore or delete it.", + path.display() + ); + } + let key = uuid::Uuid::new_v4().to_string(); + write_project_key(base, &key)?; + Ok(key) +} + +/// Persist `key` as the project identity, creating `.picloud/` (self-ignored) +/// if needed. Idempotent overwrite — callers usually go through +/// [`ensure_project_key`]. +pub fn write_project_key(base: &Path, key: &str) -> Result<()> { + let dir = base.join(DIR); + fs::create_dir_all(&dir).with_context(|| format!("creating {}", dir.display()))?; + let ignore = dir.join(".gitignore"); + if !ignore.exists() { + fs::write(&ignore, "*\n").context("writing .picloud/.gitignore")?; + } + let body = serde_json::to_vec_pretty(&ProjectLink { + key: key.to_string(), + }) + .context("encoding .picloud/project.json")?; + fs::write(project_path(base), body).context("writing .picloud/project.json")?; + Ok(()) +} /// The recorded result of the last `pic plan`, scoped to the app it was for. #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/picloud-cli/src/main.rs b/crates/picloud-cli/src/main.rs index bb3849f..ecf90ca 100644 --- a/crates/picloud-cli/src/main.rs +++ b/crates/picloud-cli/src/main.rs @@ -255,6 +255,11 @@ struct ApplyArgs { /// policy, §4.2). Repeatable. A blanket `--yes` does NOT cover it. #[arg(long = "approve", requires = "dir")] approve: Vec, + /// Tree apply only (`--dir`): claim declared groups another project owns + /// (§7, M3). Group-admin gated. Without it, an owned-elsewhere group is a + /// hard conflict. + #[arg(long, requires = "dir")] + takeover: bool, } #[derive(Args)] @@ -1428,6 +1433,7 @@ async fn main() -> ExitCode { args.force, args.env.as_deref(), &args.approve, + args.takeover, mode, ) .await diff --git a/crates/picloud-cli/tests/cli.rs b/crates/picloud-cli/tests/cli.rs index daec0fb..7f5ea76 100644 --- a/crates/picloud-cli/tests/cli.rs +++ b/crates/picloud-cli/tests/cli.rs @@ -36,6 +36,7 @@ mod init; mod invoke; mod logs; mod output; +mod ownership; mod plan; mod prune; mod pull; diff --git a/crates/picloud-cli/tests/ownership.rs b/crates/picloud-cli/tests/ownership.rs new file mode 100644 index 0000000..177220e --- /dev/null +++ b/crates/picloud-cli/tests/ownership.rs @@ -0,0 +1,288 @@ +//! M3 multi-repo single-owner ownership (§7) via `pic apply --dir`. +//! +//! On this build groups pre-exist (created via `pic groups create`; the tree +//! apply reconciles a node's CONTENT and CLAIMS its ownership, it does not +//! create the group). So each test first creates the group(s), then: +//! * the first repo to `apply` a group CLAIMS it (its `.picloud/` project key +//! becomes the group's `owner_project`), +//! * a SECOND repo (a distinct dir → a distinct project key) applying the +//! same group is REJECTED with an ownership conflict, +//! * `--takeover` lets the second repo seize it — admin-gated, so the fixture +//! instance-owner succeeds, flipping ownership (proven by the first repo now +//! being the one rejected), but a group `editor` is 403'd (ownership ⟂ RBAC, +//! §7.4), +//! * `apply --prune` deletes an owned, now-undeclared, empty group; a group +//! this project never claimed is never touched. +//! +//! Each project lives in its own `TempDir`, so `pic` mints a distinct project +//! key per repo (`.picloud/project.json`) — the two-repo topology §7 governs. + +use std::fs; +use std::path::Path; + +use tempfile::TempDir; + +use crate::common; +use crate::common::cleanup::GroupGuard; +use crate::common::member; + +/// A single-group project dir declaring the (pre-existing) group `slug`. No +/// scripts, so a structural prune can delete it (a group holding scripts/apps +/// is RESTRICT-pinned). `extra` is appended to the `[group]` table verbatim. +fn group_dir(slug: &str, extra: &str) -> TempDir { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("picloud.toml"), + format!("[group]\nslug = \"{slug}\"\nname = \"Owned Group\"\n{extra}"), + ) + .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") +} + +fn create_group(env: &common::TestEnv, slug: &str, parent: Option<&str>) { + let mut cmd = common::pic_as(env); + cmd.args(["groups", "create", slug]); + if let Some(p) = parent { + cmd.args(["--parent", p]); + } + let out = cmd.output().expect("groups create"); + assert!( + out.status.success(), + "groups create {slug} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +fn ls_groups(env: &common::TestEnv) -> String { + String::from_utf8( + common::pic_as(env) + .args(["groups", "ls"]) + .output() + .expect("groups ls") + .stdout, + ) + .unwrap() +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn first_repo_claims_second_is_rejected_then_takeover_flips_ownership() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let slug = common::unique_slug("own-g"); + let _g = GroupGuard::new(&env.url, &env.token, &slug); + create_group(&env, &slug, None); + + // Repo A claims the group on first apply (owner NULL → project A). + let repo_a = group_dir(&slug, ""); + let a1 = apply(&env, repo_a.path(), &[]); + assert!( + a1.status.success(), + "repo A claim apply failed: {}", + String::from_utf8_lossy(&a1.stderr) + ); + + // Repo B (a different dir → a different project key) is rejected: the group + // is owned by project A. + let repo_b = group_dir(&slug, ""); + let b1 = apply(&env, repo_b.path(), &[]); + assert!( + !b1.status.success(), + "repo B apply must be rejected (group owned by A)" + ); + let b1_err = String::from_utf8_lossy(&b1.stderr).to_lowercase(); + assert!( + b1_err.contains("owned by another project") || b1_err.contains("takeover"), + "conflict message should name the ownership clash:\n{b1_err}" + ); + + // Repo B with --takeover succeeds (the fixture token is instance owner, so + // it holds GroupAdmin on every node) and flips ownership to project B. + let b2 = apply(&env, repo_b.path(), &["--takeover"]); + assert!( + b2.status.success(), + "repo B --takeover should succeed: {}", + String::from_utf8_lossy(&b2.stderr) + ); + + // Proof the flip took: repo A — formerly the owner — is now the one + // rejected without --takeover. + let a2 = apply(&env, repo_a.path(), &[]); + assert!( + !a2.status.success(), + "after takeover, repo A must now be rejected (B owns the group)" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn takeover_without_group_admin_is_forbidden() { + // §7.4 — ownership ⟂ RBAC: `--takeover` needs GroupAdmin specifically, a + // second gate beyond the write caps. A group `editor` (who CAN write group + // vars) still cannot seize the group for their repo. + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let slug = common::unique_slug("own-ta"); + let _g = GroupGuard::new(&env.url, &env.token, &slug); + create_group(&env, &slug, None); + + // Admin (repo A) claims the group. + let repo_a = group_dir(&slug, ""); + assert!( + apply(&env, repo_a.path(), &[]).status.success(), + "admin claim apply should succeed" + ); + + // A member granted `editor` (not app_admin/group_admin) on the group. + let m = member::member_user(fx, &common::unique_username("ta")); + common::pic_as(&env) + .args(["groups", "members", "add", &slug, &m.id, "--role", "editor"]) + .output() + .expect("add group member"); + let member_env = common::custom_env(&fx.url, &m.token); + common::seed_credentials(&member_env, &m.username); + + // The member's repo B (a distinct project key) declares a group var (so its + // write cap is satisfied) and attempts a takeover → 403 at the GroupAdmin + // gate, before any write lands. + let repo_b = group_dir(&slug, "\n[vars]\nregion = \"eu\"\n"); + let out = apply(&member_env, repo_b.path(), &["--takeover"]); + assert!( + !out.status.success(), + "non-admin --takeover must be forbidden" + ); + let err = String::from_utf8_lossy(&out.stderr).to_lowercase(); + assert!( + err.contains("forbidden") || err.contains("403"), + "takeover denial should be an authz error:\n{err}" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn prune_deletes_owned_undeclared_group_only() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let parent = common::unique_slug("own-par"); + let child = common::unique_slug("own-child"); + // Drop order: child (registered last → dropped first), then parent. + let _gp = GroupGuard::new(&env.url, &env.token, &parent); + let _gc = GroupGuard::new(&env.url, &env.token, &child); + create_group(&env, &parent, None); + create_group(&env, &child, Some(&parent)); + + // Repo declares parent (root manifest) + child (a nested dir); apply claims + // both for this project. + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("picloud.toml"), + format!("[group]\nslug = \"{parent}\"\nname = \"Parent\"\n"), + ) + .unwrap(); + fs::create_dir_all(dir.path().join("sub")).unwrap(); + fs::write( + dir.path().join("sub/picloud.toml"), + format!("[group]\nslug = \"{child}\"\nname = \"Child\"\n"), + ) + .unwrap(); + let out = apply(&env, dir.path(), &[]); + assert!( + out.status.success(), + "initial claim apply failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + let groups = ls_groups(&env); + assert!( + groups.contains(&parent) && groups.contains(&child), + "both groups should exist after claim" + ); + + // Drop the child manifest from the SAME repo (keeping its project key) so + // the child becomes owned-but-undeclared, then apply --prune: the empty + // child is deleted; the parent (still declared) is kept. + fs::remove_dir_all(dir.path().join("sub")).unwrap(); + let out2 = apply(&env, dir.path(), &["--prune", "--yes"]); + assert!( + out2.status.success(), + "prune apply failed: {}", + String::from_utf8_lossy(&out2.stderr) + ); + let groups = ls_groups(&env); + assert!(groups.contains(&parent), "declared parent must be kept"); + assert!( + !groups.contains(&child), + "owned-but-undeclared child must be pruned" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn prune_refuses_to_cascade_a_groups_shared_data() { + // §7 M3 data-loss guard: an owned-but-undeclared group is NOT structurally + // pruned if it holds §11.6 shared collections/secrets (those CASCADE) — it + // is kept with a warning instead of being silently vaporized. + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let parent = common::unique_slug("own-dpar"); + let child = common::unique_slug("own-dchild"); + let _gp = GroupGuard::new(&env.url, &env.token, &parent); + let _gc = GroupGuard::new(&env.url, &env.token, &child); + create_group(&env, &parent, None); + create_group(&env, &child, Some(&parent)); + + // Declare parent + child; the child declares a shared KV collection, so the + // first apply claims both AND creates a group-owned collection marker. + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("picloud.toml"), + format!("[group]\nslug = \"{parent}\"\nname = \"Parent\"\n"), + ) + .unwrap(); + fs::create_dir_all(dir.path().join("sub")).unwrap(); + fs::write( + dir.path().join("sub/picloud.toml"), + format!("[group]\nslug = \"{child}\"\nname = \"Child\"\n\ncollections = [\"catalog\"]\n"), + ) + .unwrap(); + assert!( + apply(&env, dir.path(), &[]).status.success(), + "initial claim+collection apply should succeed" + ); + + // Drop the child node, then prune: the child owns a shared collection, so it + // must be KEPT (with a warning), not cascaded away. + fs::remove_dir_all(dir.path().join("sub")).unwrap(); + let out = apply(&env, dir.path(), &["--prune", "--yes"]); + assert!( + out.status.success(), + "prune apply should succeed (skipping the protected group): {}", + String::from_utf8_lossy(&out.stderr) + ); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!( + combined.contains("shared collections"), + "prune should warn it skipped a group owning shared data:\n{combined}" + ); + assert!( + ls_groups(&env).contains(&child), + "a group owning shared collections must NOT be pruned" + ); +} diff --git a/docs/design/groups-and-project-tool.md b/docs/design/groups-and-project-tool.md index 0080bd4..5d6f4f4 100644 --- a/docs/design/groups-and-project-tool.md +++ b/docs/design/groups-and-project-tool.md @@ -1120,11 +1120,25 @@ Resolved items now live inline next to their topic. What genuinely remains: 5. **Project tool maps onto groups.** Nested manifests, attach point, single-owner, server-computed tree plan, per-env approval gating. - > **Status (Phase 5): ✅ shipped — single-repo nested tree apply, atomic.** A directory tree of - > `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. **Per-env approval-policy gating shipped (`[project.environments]`, M5, 2026-07-05)** — the + > **Status (Phase 5): ✅ shipped — nested tree apply, atomic, with multi-repo single-owner + > ownership.** A directory tree of `picloud.toml` manifests (each declaring an `[app]` or `[group]` + > node) applies as ONE server-computed plan in ONE Postgres transaction. **Multi-repo single-owner + > ownership + takeover shipped (§7, M3, 2026-07-05)** — each repo mints a stable, gitignored project + > key in `.picloud/project.json`; the first `pic apply --dir` to touch a group **claims** it + > (`groups.owner_project` FK → the new `projects` table, migration 0066 promoting the inert 0047 + > column). A second repo (a distinct key) applying an owned group is **refused** (`owned by another + > project; use --takeover`) unless it passes `--takeover`, which is **GroupAdmin-gated per contested + > node** — re-verified **in-tx** at the ownership decision (not only in the pre-tx pool read), so + > `--force` (which waives the staleness token) can't open a takeover-without-admin window. Ownership ⟂ + > RBAC: owning the manifest never grants write — the actor still needs the usual group caps. The owner + > key folds into the bound-plan token (a claim/takeover by another repo between plan and apply trips + > `StateMoved`), and the attacker-supplied key is length/charset-validated server-side. `apply --prune` + > **structurally reaps** groups THIS project owns that the manifest no longer declares, leaf-first + > (`delete_group_tx` RESTRICT — a group still holding apps/subgroups aborts the apply, never orphans), + > and never touches another repo's or an unclaimed (UI-owned) group. `pic plan --dir` surfaces + > conflicts + prune candidates read-only. **Deferred:** the declarative **attach-point** / a single + > repo *creating* the group tree (groups still pre-exist — a manifest owns each node's *content*, not + > the tree *shape*). **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 @@ -1249,10 +1263,12 @@ to take, not yet taken: - *Trigger/route templates (§4.5):* bundled into Phase 6 as "much later," but the per-app binding tax bites in Phase 5 at high tenant cardinality (100 tenants × 5 triggers = 500 declarations). Decide against **actual** target tenant counts before defaulting it late. - - *Multi-repo ownership (§7):* the single-owner / attach-point / takeover machinery is substantial - and only earns its keep with a **second** managing repo. For a solo-dev / single-repo start, a - "one repo owns the whole subtree" model covers the near term; confirm the multi-repo need exists - before building the ownership layer. + - *Multi-repo ownership (§7):* the single-owner / takeover machinery is substantial and only earns + its keep with a **second** managing repo. For a solo-dev / single-repo start, a "one repo owns the + whole subtree" model covers the near term. **Resolved: shipped as M3 (2026-07-05)** — single-owner + claim / conflict / `--takeover` (GroupAdmin-gated) + structural prune, over a `projects` table and + the promoted `groups.owner_project` FK. The declarative **attach-point** (a repo *creating* the + group tree, not just claiming pre-existing groups) remains deferred. ---