fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(apply): make the per-env approval gate hermetic (§3 M3 / Track A M1)

Browse Source 
The per-env approval policy was applier-supplied — a hand-crafted request
that omitted `project.environments` was ungated, and flipping a gate to
`confirm = false` in the same request un-gated it. Persist the policy
server-side and enforce against `persisted ∪ declared`.

- Migration 0067: `project_environments (project_id, env_name, confirm)`,
  CASCADE on the project. Written declaratively (delete-then-insert) inside
  `upsert_project_tx`, same tx as the project row + node claim.
- `ProjectRepository::get_environments_by_slug` (read side) +
  `ApplyService::effective_env_policy` union the persisted policy with the
  request's declared one (confirm if EITHER says so — monotonic).
- `env_gate_check` now evaluates the effective policy; the three handlers load
  it before the gate. `plan.approvals_required` is the effective gated set, so
  CI sees a persisted gate even when the manifest omits it.
- The union rule closes both bypasses: an omitted policy still trips a
  persisted gate, and an ungating apply must itself pass the gate (the
  declarative replace then takes effect next time) — TOCTOU-safe.

Pinned by env_approval::{persisted_policy_gates_a_request_that_omits_it,
flipping_a_gate_to_false_in_the_same_request_still_gates} +
projects_repo::get_environments_by_slug_returns_persisted_policy. Schema golden
reblessed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-11 14:31:15 +02:00
parent 4b4b3148b7
commit c06a9e801e
8 changed files with 350 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
crates/manager-core/migrations/0067_project_environments.sql Normal file
View File

@@ -0,0 +1,25 @@
-- §3 M3 (hermetic gate): persist the per-env approval policy server-side.
--
-- Before this table the policy lived ONLY in the apply request
-- (`ProjectDecl.environments`), so a hand-crafted request that omitted it was
-- ungated — the server had nothing authoritative to check against. This makes
-- the gate hermetic: the confirm-required set is stored per project and the
-- server enforces against `persisted ∪ declared` (an env is gated if EITHER the
-- stored row OR the request says so), so an omitted or flipped-to-false policy
-- can no longer bypass a gate a prior apply established.
--
-- One owner (the project), a single boolean policy per environment name — no
-- polymorphic owner and no environment_scope (unlike `vars`/`secrets`, which
-- are data the apply targets; this is metadata ABOUT the apply). CASCADE on the
-- project mirrors 0066's philosophy: un-claiming/deleting a project drops its
-- policy, it does not linger to gate a tree the project no longer owns.
CREATE TABLE project_environments (
project_id UUID NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
-- The environment name as declared in `[project.environments]` (e.g.
-- "production"); matched against the apply's `--env`.
env_name TEXT NOT NULL,
-- TRUE => applying to this env is gated (needs `--approve <env>` + admin).
confirm BOOLEAN NOT NULL,
PRIMARY KEY (project_id, env_name)
);

46
crates/manager-core/src/apply_api.rs
View File

@@ -231,10 +231,13 @@ async fn apply_handler(
) -> Result<Json<ApplyReport>, ApplyError> {
let app_id = resolve_app_id(svc.apps.as_ref(), &id_or_slug).await?;
require_app_node_writes(&svc, &principal, app_id, &req.bundle, req.prune).await?;
// §3 M3: server-authoritative per-env approval gate. If the target env is
// confirm-required and approved, the override additionally requires ADMIN on
// the node (a step up from the editor write caps above) + is audited.
if env_gate_check(req.project.as_ref(), req.env.as_deref(), &req.approved_envs)? {
// §3 M3: server-authoritative per-env approval gate, evaluated against the
// persisted ∪ declared policy (hermetic — an omitted policy can't bypass a
// gate a prior apply established). If gated and approved, the override
// additionally requires ADMIN on the node (a step up from the editor write
// caps above) + is audited.
let policy = svc.effective_env_policy(req.project.as_ref()).await?;
if env_gate_check(&policy, req.env.as_deref(), &req.approved_envs)? {
require(svc.authz.as_ref(), &principal, Capability::AppAdmin(app_id))
.await
.map_err(map_authz)?;
@@ -317,8 +320,10 @@ async fn group_apply_handler(
let group_id = resolve_group_id(svc.groups.as_ref(), &id_or_slug).await?;
svc.require_group_node_writes(&principal, group_id, &req.bundle, req.prune)
.await?;
// §3 M3: per-env approval gate — an approved gated apply requires GroupAdmin.
if env_gate_check(req.project.as_ref(), req.env.as_deref(), &req.approved_envs)? {
// §3 M3: per-env approval gate (persisted ∪ declared) — an approved gated
// apply requires GroupAdmin.
let policy = svc.effective_env_policy(req.project.as_ref()).await?;
if env_gate_check(&policy, req.env.as_deref(), &req.approved_envs)? {
require(
svc.authz.as_ref(),
&principal,
@@ -401,10 +406,11 @@ async fn tree_apply_handler(
Json(req): Json<TreeApplyRequest>,
) -> Result<Json<ApplyReport>, ApplyError> {
authz_tree(&svc, &principal, &req.bundle, Some(req.prune)).await?;
// §3 M3: server-authoritative per-env approval gate. On an approved gated
// apply, require ADMIN on EVERY declared node (a step up from the editor
// write caps authz_tree checks) + audit.
if env_gate_check(req.project.as_ref(), req.env.as_deref(), &req.approved_envs)? {
// §3 M3: server-authoritative per-env approval gate (persisted ∪ declared).
// On an approved gated apply, require ADMIN on EVERY declared node (a step up
// from the editor write caps authz_tree checks) + audit.
let policy = svc.effective_env_policy(req.project.as_ref()).await?;
if env_gate_check(&policy, req.env.as_deref(), &req.approved_envs)? {
require_admin_on_every_node(&svc, &principal, &req.bundle).await?;
audit_gated_apply(&principal, req.env.as_deref(), req.bundle.nodes.len());
}
@@ -425,21 +431,23 @@ async fn tree_apply_handler(
Ok(Json(report))
}
/// §3 M3: the per-env approval gate, re-derived from the project's policy on the
/// wire (the CLI's client-side check is convenience; THIS is authoritative).
/// Returns `Ok(true)` when the target env is confirm-required AND approved — the
/// caller must then require admin + audit. `Ok(false)` when not gated (no
/// project, no env, or `confirm = false`). `Err(ApprovalRequired)` when gated
/// and NOT in `approved_envs` (a blanket `--yes` never lands here).
/// §3 M3 (hermetic gate): the per-env approval gate, evaluated against the
/// EFFECTIVE policy (`persisted ∪ declared`, built by
/// `ApplyService::effective_env_policy`) — the persisted side is authoritative,
/// so a request that omits or weakens the policy can't bypass a gate a prior
/// apply established. Returns `Ok(true)` when the target env is confirm-required
/// AND approved — the caller must then require admin + audit. `Ok(false)` when
/// not gated (no env, or `confirm = false` everywhere). `Err(ApprovalRequired)`
/// when gated and NOT in `approved_envs` (a blanket `--yes` never lands here).
fn env_gate_check(
project: Option<&ProjectDecl>,
policy: &std::collections::BTreeMap<String, bool>,
env: Option<&str>,
approved_envs: &[String],
) -> Result<bool, ApplyError> {
let (Some(project), Some(env)) = (project, env) else {
let Some(env) = env else {
return Ok(false);
};
if !project.confirm_required(env) {
if !policy.get(env).copied().unwrap_or(false) {
return Ok(false);
}
if approved_envs.iter().any(|e| e == env) {

69
crates/manager-core/src/apply_service.rs
View File

@@ -895,6 +895,49 @@ impl ApplyService {
.await
}
/// §3 M3 (hermetic gate): the EFFECTIVE per-env approval policy for an
/// apply/plan — `env_name -> confirm`, computed as `persisted ∪ declared`
/// (an env is confirm-required if EITHER the stored policy OR the request's
/// `[project.environments]` marks it so). Loading the persisted side is what
/// makes the gate hermetic: a request that omits (or flips-to-false) a policy
/// a prior apply persisted still trips the gate. Empty when no project is
/// declared. A persisted-read failure surfaces as `Backend` (fail-closed —
/// we never silently drop a gate).
pub async fn effective_env_policy(
&self,
project: Option<&ProjectDecl>,
) -> Result<std::collections::BTreeMap<String, bool>, ApplyError> {
let Some(project) = project else {
return Ok(std::collections::BTreeMap::new());
};
let mut effective = self
.projects
.get_environments_by_slug(&project.slug)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
for (env, pol) in &project.environments {
// Union on confirm: a declared gate strengthens, never weakens.
let entry = effective.entry(env.clone()).or_insert(false);
*entry = *entry || pol.confirm;
}
Ok(effective)
}
/// The sorted names of every effectively-gated env (`effective_env_policy`
/// filtered to `confirm`), surfaced as `plan.approvals_required`.
async fn effective_gated_envs(
&self,
project: Option<&ProjectDecl>,
) -> Result<Vec<String>, ApplyError> {
let policy = self.effective_env_policy(project).await?;
let mut gated: Vec<String> = policy
.into_iter()
.filter_map(|(env, confirm)| confirm.then_some(env))
.collect();
gated.sort_unstable();
Ok(gated)
}
/// Owner-generic plan (Phase 5): diff `bundle` against an app OR group node.
/// A group node has only scripts + vars — routes/triggers are rejected, and
/// the app-only inherited-target + route-host validation is skipped.
@@ -936,7 +979,7 @@ impl ApplyService {
// `apply` can refuse if the node changed underneath it (§4.2).
state_token: state_token_with_names(&current, &current_names),
ownership,
approvals_required: project.map(ProjectDecl::gated_envs).unwrap_or_default(),
approvals_required: self.effective_gated_envs(project).await?,
})
}
@@ -1772,7 +1815,7 @@ impl ApplyService {
Ok(TreePlanResult {
nodes,
state_token: token,
approvals_required: project.map(ProjectDecl::gated_envs).unwrap_or_default(),
approvals_required: self.effective_gated_envs(project).await?,
})
}
@@ -4072,6 +4115,28 @@ async fn upsert_project_tx(
.fetch_one(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
// §3 M3 (hermetic gate): persist the declared per-env approval policy as the
// authoritative source. Declarative replace (delete-then-insert) so a
// removed env drops out — the gate reads `persisted ∪ declared`, so an
// ungating still has to pass the prior gate at apply time (see
// `apply_api::env_gate_check`). Same tx as the project row + node claim.
sqlx::query("DELETE FROM project_environments WHERE project_id = $1")
.bind(id)
.execute(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
for (env_name, pol) in &decl.environments {
sqlx::query(
"INSERT INTO project_environments (project_id, env_name, confirm) \
VALUES ($1, $2, $3)",
)
.bind(id)
.bind(env_name)
.bind(pol.confirm)
.execute(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
}
Ok(id.into())
}

25
crates/manager-core/src/project_repo.rs
View File

@@ -26,6 +26,15 @@ pub trait ProjectRepository: Send + Sync {
async fn list_with_counts(&self) -> Result<Vec<(Project, i64)>, ProjectRepositoryError>;
async fn get_by_id(&self, id: ProjectId) -> Result<Option<Project>, ProjectRepositoryError>;
async fn get_by_slug(&self, slug: &str) -> Result<Option<Project>, ProjectRepositoryError>;
/// §3 M3 (hermetic gate): the persisted per-env approval policy for the
/// project with this slug — `env_name -> confirm`. Empty when the project is
/// unregistered or declares no gated envs. The apply/plan path unions this
/// with the request's declared policy so an omitted policy can't bypass a
/// gate a prior apply persisted (see `apply_api::env_gate_check`).
async fn get_environments_by_slug(
&self,
slug: &str,
) -> Result<std::collections::BTreeMap<String, bool>, ProjectRepositoryError>;
}
pub struct PostgresProjectRepository {
@@ -86,6 +95,22 @@ impl ProjectRepository for PostgresProjectRepository {
.await?;
Ok(row.map(Into::into))
}
async fn get_environments_by_slug(
&self,
slug: &str,
) -> Result<std::collections::BTreeMap<String, bool>, ProjectRepositoryError> {
let rows = sqlx::query_as::<_, (String, bool)>(
"SELECT pe.env_name, pe.confirm \
FROM project_environments pe \
JOIN projects p ON p.id = pe.project_id \
WHERE p.slug = $1",
)
.bind(slug)
.fetch_all(&self.pool)
.await?;
Ok(rows.into_iter().collect())
}
}
#[derive(sqlx::FromRow)]

13
crates/manager-core/tests/expected_schema.txt
View File

@@ -318,6 +318,11 @@ table: outbox
claimed_by: text NULL
created_at: timestamp with time zone NOT NULL default=now()
table: project_environments
project_id: uuid NOT NULL
env_name: text NOT NULL
confirm: boolean NOT NULL
table: projects
id: uuid NOT NULL default=gen_random_uuid()
slug: text NOT NULL
@@ -603,6 +608,9 @@ 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 project_environments:
project_environments_pkey: public.project_environments USING btree (project_id, env_name)
indexes on projects:
projects_pkey: public.projects USING btree (id)
projects_slug_key: public.projects USING btree (slug)
@@ -847,6 +855,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 project_environments:
[FOREIGN KEY] project_environments_project_id_fkey: FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE
[PRIMARY KEY] project_environments_pkey: PRIMARY KEY (project_id, env_name)
constraints on projects:
[FOREIGN KEY] projects_created_by_fkey: FOREIGN KEY (created_by) REFERENCES admin_users(id) ON DELETE SET NULL
[PRIMARY KEY] projects_pkey: PRIMARY KEY (id)
@@ -992,3 +1004,4 @@ constraints on vars:
0064: shared topic queue kinds
0065: group queues
0066: projects
0067: project environments

44
crates/manager-core/tests/projects_repo.rs
View File

@@ -104,3 +104,47 @@ async fn list_with_owner_and_ancestors_surface_ownership() {
.expect("get_by_id");
assert_eq!(by_id.map(|p| p.slug), Some(proj_slug));
}
#[tokio::test]
async fn get_environments_by_slug_returns_persisted_policy() {
// §3 M3 hermetic gate: the persisted per-env policy round-trips by project
// slug. Backs `ApplyService::effective_env_policy` (the `persisted` half).
let Some(pool) = pool_or_skip().await else {
return;
};
let projects = PostgresProjectRepository::new(pool.clone());
let proj_slug = uniq("penv");
let (proj_id,): (Uuid,) =
sqlx::query_as("INSERT INTO projects (slug, name) VALUES ($1, $1) RETURNING id")
.bind(&proj_slug)
.fetch_one(&pool)
.await
.expect("insert project");
for (env, confirm) in [("production", true), ("staging", false)] {
sqlx::query(
"INSERT INTO project_environments (project_id, env_name, confirm) VALUES ($1, $2, $3)",
)
.bind(proj_id)
.bind(env)
.bind(confirm)
.execute(&pool)
.await
.expect("insert env policy");
}
let policy = projects
.get_environments_by_slug(&proj_slug)
.await
.expect("get_environments_by_slug");
assert_eq!(policy.get("production"), Some(&true));
assert_eq!(policy.get("staging"), Some(&false));
assert_eq!(policy.len(), 2);
// An unknown slug yields an empty policy (no gate).
let empty = projects
.get_environments_by_slug("no-such-project")
.await
.expect("get_environments_by_slug");
assert!(empty.is_empty());
}

143
crates/picloud-cli/tests/env_approval.rs
View File

@@ -4,13 +4,16 @@
//! to such an env with `pic apply --env <e>` is refused unless it is explicitly
//! `--approve <e>`d — and a blanket `--yes` does NOT cover it (§4.2, "CI must
//! opt in per environment"). An unlisted or `confirm = false` env applies
//! freely. The CLI gates client-side (fast fail) AND the SERVER re-derives the
//! policy from the request and enforces it — a confirm-required apply is refused
//! (409) unless approved, and an approved override additionally requires ADMIN on
//! every declared node (a step up from editor write caps) and is audited. This
//! closes the patched/older-CLI bypass (a client that still declares the project
//! but skips the local check); a request that omits the policy is inherently
//! ungated (the policy is applier-supplied, not persisted server state).
//! freely. The CLI gates client-side (fast fail) AND the SERVER enforces it — a
//! confirm-required apply is refused (409) unless approved, and an approved
//! override additionally requires ADMIN on every declared node (a step up from
//! editor write caps) and is audited.
//!
//! §3 M3 HERMETIC gate: the policy is PERSISTED server-side (`project_environments`,
//! 0067) on every apply, and the gate evaluates against `persisted ∪ declared` —
//! so a request that OMITS the policy (or flips a gate to `false`) can no longer
//! bypass a gate a prior apply established. This closes both the patched/older-CLI
//! bypass AND the hand-crafted-request bypass.
use std::fs;
use std::path::Path;
@@ -337,3 +340,129 @@ fn approving_a_gated_apply_requires_admin() {
"the approval denial should be an authz error:\n{err}"
);
}
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
#[test]
fn persisted_policy_gates_a_request_that_omits_it() {
// §3 M3 HERMETIC: once an apply persists a `confirm = true` env, a LATER raw
// request that OMITS the policy entirely (empty `environments`) is STILL
// gated — the server enforces against the persisted policy, not just the
// wire. This is the hand-crafted-request bypass the earlier CLI-only gate
// could not close.
let Some(fx) = common::fixture_or_skip() else {
return;
};
let env = common::admin_env(fx);
let group = common::unique_slug("eah-grp");
let project = common::unique_slug("eah-proj");
let _g = GroupGuard::new(&env.url, &env.token, &group);
common::pic_as(&env)
.args(["groups", "create", &group])
.assert()
.success();
let http = reqwest::blocking::Client::new();
let apply = |environments: serde_json::Value, approved: serde_json::Value| {
http.post(format!("{}/api/v1/admin/tree/apply", env.url))
.bearer_auth(&env.token)
.json(&serde_json::json!({
"bundle": { "nodes": [{ "kind": "group", "slug": group, "bundle": {} }] },
"project": { "slug": project, "environments": environments },
"env": "production",
"approved_envs": approved,
"prune": false,
}))
.send()
.expect("tree apply")
};
// First apply DECLARES + APPROVES the gate → persists `production=true`.
let seed = apply(
serde_json::json!({ "production": { "confirm": true } }),
serde_json::json!(["production"]),
);
assert!(
seed.status().is_success(),
"the seeding apply (declared + approved) must succeed (got {}: {})",
seed.status(),
seed.text().unwrap_or_default()
);
// Now a request that OMITS the policy and does NOT approve → still 409,
// because the persisted policy still gates `production`.
let omit = apply(serde_json::json!({}), serde_json::json!([]));
assert_eq!(
omit.status().as_u16(),
409,
"a persisted gate must hold even when the request omits the policy (got {}: {})",
omit.status(),
omit.text().unwrap_or_default()
);
// Approving it clears the persisted gate (admin token holds GroupAdmin).
let ok = apply(serde_json::json!({}), serde_json::json!(["production"]));
assert!(
ok.status().is_success(),
"approving the persisted gate must let it through (got {}: {})",
ok.status(),
ok.text().unwrap_or_default()
);
}
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
#[test]
fn flipping_a_gate_to_false_in_the_same_request_still_gates() {
// §3 M3 HERMETIC (monotonic rule): the gate evaluates against
// `persisted ∪ declared`, so declaring `confirm = false` for an env a prior
// apply persisted as `true` does NOT un-gate it in the same request — the
// ungating apply must itself pass the gate. This closes the flip-to-false
// bypass.
let Some(fx) = common::fixture_or_skip() else {
return;
};
let env = common::admin_env(fx);
let group = common::unique_slug("eaf-grp");
let project = common::unique_slug("eaf-proj");
let _g = GroupGuard::new(&env.url, &env.token, &group);
common::pic_as(&env)
.args(["groups", "create", &group])
.assert()
.success();
let http = reqwest::blocking::Client::new();
let apply = |environments: serde_json::Value, approved: serde_json::Value| {
http.post(format!("{}/api/v1/admin/tree/apply", env.url))
.bearer_auth(&env.token)
.json(&serde_json::json!({
"bundle": { "nodes": [{ "kind": "group", "slug": group, "bundle": {} }] },
"project": { "slug": project, "environments": environments },
"env": "production",
"approved_envs": approved,
"prune": false,
}))
.send()
.expect("tree apply")
};
// Persist `production=true`.
assert!(apply(
serde_json::json!({ "production": { "confirm": true } }),
serde_json::json!(["production"]),
)
.status()
.is_success());
// Same request flips it to false but does NOT approve → still 409 (the
// persisted `true` wins under the union rule).
let flip = apply(
serde_json::json!({ "production": { "confirm": false } }),
serde_json::json!([]),
);
assert_eq!(
flip.status().as_u16(),
409,
"flipping a persisted gate to false must not un-gate it in the same request (got {}: {})",
flip.status(),
flip.text().unwrap_or_default()
);
}