From d9b3e9973cda8f691222bbe1a1e95f82fbce45fe Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 22 Jun 2026 21:18:51 +0200 Subject: [PATCH] fix(project-tool): address review findings on the apply foundation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remediation of the single-app reconcile foundation after two independent review passes. No new feature surface — closes correctness, parity, and safety gaps in pull/plan/apply/prune. apply engine (manager-core): - validate_bundle reached parity with the interactive trigger API: reject an empty kv/docs/files collection_glob and a malformed pubsub topic_pattern (previously written and silently never matched), and lift the queue visibility floor to MIN_QUEUE_VISIBILITY_TIMEOUT_SECS (30) — apply had accepted a [5,29] value the dashboard refuses. Per-kind checks extracted into a pure, unit-tested validate_trigger_shape. - An omitted script `description` now means "leave as-is" (matching the other optional fields and the function's own documented contract) instead of clearing the stored value. - Doc fixes: drop stale "next milestone" notes; record the one deliberate plan/apply divergence (set-but-empty secret); document delete_route_tx as intentionally idempotent for reconcile. CLI: - Gate destructive `apply --prune` behind confirmation: interactive y/N, or `--yes` for CI; a non-interactive prune without `--yes` refuses rather than silently deleting. Journey tests pass `--yes`; a new test proves the gate refuses and deletes nothing. - Harden pull's filename safety: reject all control characters and the Unicode bidi-override / zero-width chars used for terminal/filename spoofing, and cap length at 200 bytes (NAME_MAX headroom). Tested: manager-core lib (incl. queue-floor + sparse-description parity) + CLI bins + 9 project-tool journeys green; clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/apply_service.rs | 246 ++++++++++++++++++----- crates/manager-core/src/route_repo.rs | 6 + crates/manager-core/src/triggers_api.rs | 2 +- crates/picloud-cli/src/cmds/apply.rs | 42 +++- crates/picloud-cli/src/cmds/pull.rs | 53 ++++- crates/picloud-cli/src/main.rs | 9 +- crates/picloud-cli/tests/email_queue.rs | 2 +- crates/picloud-cli/tests/prune.rs | 70 ++++++- 8 files changed, 364 insertions(+), 66 deletions(-) diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index 1d346f3..ed44d37 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -347,8 +347,8 @@ pub enum ApplyError { // Service // ---------------------------------------------------------------------------- -/// Reconcile engine. Holds trait-object repos for the read/diff path; -/// the transactional write path (next milestone) reuses the same handles. +/// Reconcile engine. Holds trait-object repos for the read/diff path; the +/// transactional write path (`apply`) reuses the same handles plus `pool`. #[derive(Clone)] pub struct ApplyService { /// Pool for the transactional write path (`apply`). @@ -385,9 +385,9 @@ impl ApplyService { Ok(compute_diff(¤t, bundle)) } - /// Reconcile app `app_id` to `bundle` in a single transaction. - /// Additive: creates and updates are applied; deletions are surfaced - /// in the diff but only executed when prune is set (next milestone). + /// Reconcile app `app_id` to `bundle` in a single transaction. Creates + /// and updates are always applied; deletions of resources absent from the + /// bundle are executed only when `prune` is set (secrets are never pruned). /// /// # Errors /// `Invalid` for a bad bundle; `Backend` for repo/transaction failures. @@ -471,7 +471,10 @@ impl ApplyService { let cur = current_scripts[&key]; let patch = ScriptPatch { name: None, - description: Some(bs.description.clone()), + // Sparse: `None` (omitted in the manifest) leaves the + // stored description untouched; `Some(text)` sets it. + // Mirrors `script_update_reason`'s leave-as-is rule. + description: bs.description.clone().map(Some), source: Some(bs.source.clone()), timeout_seconds: bs.timeout_seconds, memory_limit_mb: bs.memory_limit_mb, @@ -909,49 +912,7 @@ impl ApplyService { t.script() ))); } - match t { - BundleTrigger::Email { - inbound_secret_ref, .. - } if inbound_secret_ref.trim().is_empty() => { - return Err(ApplyError::Invalid(format!( - "email trigger on `{}` needs inbound_secret_ref (a secret name)", - t.script() - ))); - } - BundleTrigger::Queue { - queue_name, - visibility_timeout_secs, - .. - } => { - if queue_name.trim().is_empty() { - return Err(ApplyError::Invalid("queue trigger needs queue_name".into())); - } - if let Some(v) = visibility_timeout_secs { - if !(5..=3600).contains(v) { - return Err(ApplyError::Invalid(format!( - "queue `{queue_name}`: visibility_timeout_secs must be in [5, 3600]" - ))); - } - } - } - BundleTrigger::Cron { - schedule, timezone, .. - } => { - crate::cron_scheduler::validate_schedule(schedule).map_err(|e| { - ApplyError::Invalid(format!( - "cron trigger on `{}`: invalid schedule: {e}", - t.script() - )) - })?; - crate::cron_scheduler::validate_timezone(timezone).map_err(|e| { - ApplyError::Invalid(format!( - "cron trigger on `{}`: invalid timezone: {e}", - t.script() - )) - })?; - } - _ => {} - } + validate_trigger_shape(t)?; let id = t.identity(); if !trig_keys.insert(id.clone()) { return Err(ApplyError::Invalid(format!("duplicate trigger `{id}`"))); @@ -1084,8 +1045,13 @@ fn script_update_reason(cur: &Script, desired: &BundleScript) -> Option if cur.kind != desired.kind { return Some("kind changed".into()); } - if cur.description != desired.description { - return Some("description changed".into()); + // Sparse like the other optional fields: an omitted (`None`) description + // means "leave as-is", so only an explicitly-set value that differs + // forces an update. (Clearing a description is done in the dashboard.) + if let Some(d) = &desired.description { + if cur.description.as_ref() != Some(d) { + return Some("description changed".into()); + } } if let Some(t) = desired.timeout_seconds { if i64::from(cur.timeout_seconds) != i64::from(t) { @@ -1286,6 +1252,12 @@ fn diff_secrets(current: &CurrentState, bundle: &Bundle) -> Vec /// Reject any email trigger whose referenced secret isn't set, so `plan` and /// `apply` give the same answer (apply also fails in `resolve_and_seal`, but /// only after taking the lock — surfacing it here keeps plan honest). +/// +/// This checks the secret *reference exists*. `resolve_and_seal` additionally +/// requires the resolved value to be a non-empty string; `plan` deliberately +/// does not decrypt secrets, so a reference to a set-but-empty secret passes +/// `plan` and is caught at `apply` — the one residual plan/apply divergence, +/// and a deliberate one (plan stays read-only and crypto-free). fn validate_email_secrets_present( bundle: &Bundle, secret_names: &[String], @@ -1396,6 +1368,89 @@ fn reject_reserved_path(path: &str) -> Result<(), ApplyError> { Ok(()) } +/// Per-kind structural validation for one desired trigger — kept at parity +/// with the interactive trigger API so `apply` can't write a trigger the +/// dashboard would have rejected. Pure (no live state), so it's unit-tested +/// directly. The script-binding checks live in `validate_bundle`. +fn validate_trigger_shape(t: &BundleTrigger) -> Result<(), ApplyError> { + match t { + BundleTrigger::Email { + inbound_secret_ref, .. + } if inbound_secret_ref.trim().is_empty() => { + return Err(ApplyError::Invalid(format!( + "email trigger on `{}` needs inbound_secret_ref (a secret name)", + t.script() + ))); + } + BundleTrigger::Queue { + queue_name, + visibility_timeout_secs, + .. + } => { + if queue_name.trim().is_empty() { + return Err(ApplyError::Invalid("queue trigger needs queue_name".into())); + } + if let Some(v) = visibility_timeout_secs { + // Parity with the interactive API: the floor is the dispatcher + // tick/reclaim-cadence minimum (`triggers_api`), and the ceiling + // matches `trigger_repo`'s queue check. Apply previously allowed + // a [5, 29] floor the dashboard rejects. + let min = crate::triggers_api::MIN_QUEUE_VISIBILITY_TIMEOUT_SECS; + if !(min..=3600).contains(v) { + return Err(ApplyError::Invalid(format!( + "queue `{queue_name}`: visibility_timeout_secs must be in [{min}, 3600]" + ))); + } + } + } + BundleTrigger::Cron { + schedule, timezone, .. + } => { + crate::cron_scheduler::validate_schedule(schedule).map_err(|e| { + ApplyError::Invalid(format!( + "cron trigger on `{}`: invalid schedule: {e}", + t.script() + )) + })?; + crate::cron_scheduler::validate_timezone(timezone).map_err(|e| { + ApplyError::Invalid(format!( + "cron trigger on `{}`: invalid timezone: {e}", + t.script() + )) + })?; + } + // Parity with the interactive trigger API, which rejects an empty + // collection_glob (`create_{kv,docs,files}_trigger`). + BundleTrigger::Kv { + collection_glob, .. + } + | BundleTrigger::Docs { + collection_glob, .. + } + | BundleTrigger::Files { + collection_glob, .. + } if collection_glob.trim().is_empty() => { + return Err(ApplyError::Invalid(format!( + "{} trigger on `{}` needs a non-empty collection_glob", + t.kind_str(), + t.script() + ))); + } + // Parity with `create_pubsub_trigger`'s topic-pattern check — otherwise + // a malformed pattern is written and silently never matches at dispatch. + BundleTrigger::Pubsub { topic_pattern, .. } => { + picloud_shared::validate_topic_pattern(topic_pattern).map_err(|e| { + ApplyError::Invalid(format!( + "pubsub trigger on `{}`: invalid topic_pattern: {e}", + t.script() + )) + })?; + } + _ => {} + } + Ok(()) +} + /// Summary of what an `apply` changed. #[derive(Debug, Default, Serialize)] pub struct ApplyReport { @@ -1731,6 +1786,95 @@ mod tests { assert_eq!(compute_diff(¤t, &chg2).scripts[0].op, Op::Update); } + #[test] + fn description_is_sparse() { + // An omitted (`None`) description must mean "leave as-is", matching + // the other optional fields — not "clear it". + let mut cur = script("a", "let x = 1;"); + cur.description = Some("kept".into()); + let current = CurrentState { + scripts: vec![cur], + ..CurrentState::default() + }; + // Omitted → NoOp (leave the stored description alone). + let mut omit = empty_bundle(); + omit.scripts = vec![bundle_script("a", "let x = 1;")]; + assert_eq!(compute_diff(¤t, &omit).scripts[0].op, Op::NoOp); + // Same explicit value → NoOp. + let mut same = empty_bundle(); + let mut s = bundle_script("a", "let x = 1;"); + s.description = Some("kept".into()); + same.scripts = vec![s]; + assert_eq!(compute_diff(¤t, &same).scripts[0].op, Op::NoOp); + // Different explicit value → Update. + let mut chg = empty_bundle(); + let mut s2 = bundle_script("a", "let x = 1;"); + s2.description = Some("changed".into()); + chg.scripts = vec![s2]; + assert_eq!(compute_diff(¤t, &chg).scripts[0].op, Op::Update); + } + + #[test] + fn trigger_shape_validation_matches_interactive_api() { + // Empty collection_glob is rejected (kv/docs/files). + assert!(validate_trigger_shape(&BundleTrigger::Kv { + script: "h".into(), + collection_glob: " ".into(), + ops: vec![], + dispatch_mode: None, + retry_max_attempts: None, + }) + .is_err()); + // A non-empty glob passes. + assert!(validate_trigger_shape(&BundleTrigger::Docs { + script: "h".into(), + collection_glob: "users".into(), + ops: vec![], + dispatch_mode: None, + retry_max_attempts: None, + }) + .is_ok()); + // A malformed pubsub topic_pattern (mid-pattern wildcard) is rejected; + // a valid one passes. + assert!(validate_trigger_shape(&BundleTrigger::Pubsub { + script: "h".into(), + topic_pattern: "user.*.created".into(), + dispatch_mode: None, + retry_max_attempts: None, + }) + .is_err()); + assert!(validate_trigger_shape(&BundleTrigger::Pubsub { + script: "h".into(), + topic_pattern: "orders.created".into(), + dispatch_mode: None, + retry_max_attempts: None, + }) + .is_ok()); + // Queue visibility floor matches the interactive API (>= 30): a value + // below the floor is rejected; the floor itself and `None` are ok. + let queue = |vis| BundleTrigger::Queue { + script: "h".into(), + queue_name: "jobs".into(), + visibility_timeout_secs: vis, + dispatch_mode: None, + retry_max_attempts: None, + }; + assert!(validate_trigger_shape(&queue(Some(10))).is_err()); + assert!(validate_trigger_shape(&queue(Some( + crate::triggers_api::MIN_QUEUE_VISIBILITY_TIMEOUT_SECS + ))) + .is_ok()); + assert!(validate_trigger_shape(&queue(None)).is_ok()); + // Empty email inbound_secret_ref is rejected. + assert!(validate_trigger_shape(&BundleTrigger::Email { + script: "h".into(), + inbound_secret_ref: " ".into(), + dispatch_mode: None, + retry_max_attempts: None, + }) + .is_err()); + } + #[test] fn trigger_diff_create_noop_delete() { let s = script("h", "x"); diff --git a/crates/manager-core/src/route_repo.rs b/crates/manager-core/src/route_repo.rs index 17e2927..4a80f29 100644 --- a/crates/manager-core/src/route_repo.rs +++ b/crates/manager-core/src/route_repo.rs @@ -202,6 +202,12 @@ pub(crate) async fn insert_route_tx( } /// Delete a route by id within an existing transaction. +/// +/// Unlike the non-tx [`RouteRepository::delete`], this is intentionally +/// idempotent: a missing row is not an error. The only caller is the +/// reconcile engine (`ApplyService`), where "delete a route already gone" +/// (e.g. removed out-of-band between the diff read and the write) is a +/// no-op to converge on, not a failure to roll back the whole apply. pub(crate) async fn delete_route_tx( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, route_id: Uuid, diff --git a/crates/manager-core/src/triggers_api.rs b/crates/manager-core/src/triggers_api.rs index b16a367..ecb1803 100644 --- a/crates/manager-core/src/triggers_api.rs +++ b/crates/manager-core/src/triggers_api.rs @@ -38,7 +38,7 @@ use crate::trigger_repo::{ /// in practice (the dispatcher itself ticks every 100ms; reclaim ticks /// every 30s; an executor needs more wall-clock than this to do useful /// work). Reject below this with a 422 + actionable error. -const MIN_QUEUE_VISIBILITY_TIMEOUT_SECS: u32 = 30; +pub(crate) const MIN_QUEUE_VISIBILITY_TIMEOUT_SECS: u32 = 30; /// Default soft-warning ceiling when `PICLOUD_DISPATCHER_ASYNC_EXEC_TIMEOUT_SEC` /// is unset. Re-exported from the dispatcher so the single source of diff --git a/crates/picloud-cli/src/cmds/apply.rs b/crates/picloud-cli/src/cmds/apply.rs index 3241cf0..82c21c2 100644 --- a/crates/picloud-cli/src/cmds/apply.rs +++ b/crates/picloud-cli/src/cmds/apply.rs @@ -1,11 +1,12 @@ //! `pic apply [--file picloud.toml]` — reconcile the live app to the -//! manifest's desired state in one server-side transaction. Additive in -//! this milestone (creates + updates); pruning of stale resources lands -//! next. +//! manifest's desired state in one server-side transaction. Creates and +//! updates are applied; `--prune` additionally deletes live scripts/routes/ +//! triggers absent from the manifest (secrets are never pruned). +use std::io::{IsTerminal, Write}; use std::path::Path; -use anyhow::Result; +use anyhow::{Context, Result}; use crate::client::Client; use crate::cmds::plan::build_bundle; @@ -13,7 +14,7 @@ use crate::config; use crate::manifest::Manifest; use crate::output::{KvBlock, OutputMode}; -pub async fn run(manifest_path: &Path, prune: bool, mode: OutputMode) -> Result<()> { +pub async fn run(manifest_path: &Path, prune: bool, yes: bool, mode: OutputMode) -> Result<()> { let creds = config::resolve()?; let client = Client::from_creds(&creds)?; @@ -21,6 +22,10 @@ pub async fn run(manifest_path: &Path, prune: bool, mode: OutputMode) -> Result< let base_dir = manifest_path.parent().unwrap_or_else(|| Path::new(".")); let bundle = build_bundle(&manifest, base_dir)?; + if prune && !yes { + confirm_prune(&manifest.app.slug)?; + } + let report = client.apply(&manifest.app.slug, &bundle, prune).await?; let mut block = KvBlock::new(); @@ -50,3 +55,30 @@ pub async fn run(manifest_path: &Path, prune: bool, mode: OutputMode) -> Result< block.print(mode); Ok(()) } + +/// `--prune` deletes live scripts/routes/triggers absent from the manifest — +/// 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`). +fn confirm_prune(slug: &str) -> Result<()> { + if !std::io::stdin().is_terminal() { + anyhow::bail!( + "refusing to `apply --prune` non-interactively without `--yes`: prune \ + deletes resources absent from the manifest and cannot be undone. \ + Review with `pic plan`, then re-run with `--yes`." + ); + } + eprint!( + "apply --prune will DELETE live scripts/routes/triggers on `{slug}` that are \ + absent from the manifest. This cannot be undone. Continue? [y/N] " + ); + std::io::stderr().flush().ok(); + let mut answer = String::new(); + std::io::stdin() + .read_line(&mut answer) + .context("read confirmation")?; + if !matches!(answer.trim(), "y" | "Y" | "yes" | "Yes") { + anyhow::bail!("aborted"); + } + Ok(()) +} diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index 89320ac..7e7269e 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -62,8 +62,9 @@ pub async fn run(app_ident: &str, dir: &Path, mode: OutputMode) -> Result<()> { for s in &scripts { if !is_safe_filename(&s.name) { anyhow::bail!( - "script name {:?} is not filesystem-safe (contains a path \ - separator, `..`, or a leading dot); cannot pull", + "script name {:?} is not filesystem-safe (a path separator, \ + `..`, a leading dot, a control character, or longer than 200 \ + bytes); cannot pull", s.name ); } @@ -207,14 +208,35 @@ fn trigger_count(t: &ManifestTriggers) -> usize { } /// True if `name` is safe to use as a single path component in `scripts/`. -/// Rejects empty names, path separators, `.`/`..`, and leading dots. +/// Rejects empty/over-long names, path separators, `.`/`..`, leading dots, +/// and any deceptive display character — a server-returned name is otherwise +/// written verbatim as a filename and printed to the operator's terminal. fn is_safe_filename(name: &str) -> bool { + // Leave headroom under NAME_MAX (255 bytes on common filesystems) for the + // `.rhai` suffix. + const MAX_LEN: usize = 200; !name.is_empty() + && name.len() <= MAX_LEN && !name.starts_with('.') && !name.contains('/') && !name.contains('\\') && name != ".." - && !name.contains('\0') + && !name.chars().any(is_deceptive_char) +} + +/// Control characters (NUL, newlines, ANSI escapes) plus the Unicode +/// bidirectional-override and zero-width/format characters used to spoof how a +/// name renders in a terminal — both classes are unsafe to print verbatim. +fn is_deceptive_char(c: char) -> bool { + c.is_control() + || matches!(c, + '\u{200B}'..='\u{200F}' // zero-width space … LTR/RTL marks + | '\u{2028}'..='\u{2029}' // line / paragraph separators + | '\u{202A}'..='\u{202E}' // bidi embeddings / overrides + | '\u{2060}' // word joiner + | '\u{2066}'..='\u{2069}' // bidi isolates + | '\u{FEFF}' // BOM / zero-width no-break space + ) } /// Deserialize a trigger's `details` JSON, attributing failures to the kind. @@ -276,6 +298,29 @@ mod tests { } } + #[test] + fn rejects_control_chars_and_overlong() { + for bad in [ + "a\nb", + "a\tb", + "line\rdrop", + "esc\x1b[2Jseq", + "rtl\u{202E}gpj.exe", // bidi override (filename spoof) + "zero\u{200B}width", // zero-width space + "bom\u{FEFF}name", // BOM + ] { + assert!(!is_safe_filename(bad), "expected {bad:?} to be rejected"); + } + assert!( + !is_safe_filename(&"a".repeat(201)), + "expected an over-long name to be rejected" + ); + assert!( + is_safe_filename(&"a".repeat(200)), + "a 200-char name is at the limit and allowed" + ); + } + #[test] fn accepts_normal_names() { for ok in ["create-post", "nightly_digest", "Greet", "x", "a.b"] { diff --git a/crates/picloud-cli/src/main.rs b/crates/picloud-cli/src/main.rs index c5ff717..e740651 100644 --- a/crates/picloud-cli/src/main.rs +++ b/crates/picloud-cli/src/main.rs @@ -159,7 +159,8 @@ enum Cmd { }, /// Reconcile the live app to a `picloud.toml` manifest in one - /// transaction (additive: creates + updates). + /// transaction (creates + updates; `--prune` also deletes resources + /// absent from the manifest). Apply(ApplyArgs), /// Diff a `picloud.toml` manifest against the live app and print the @@ -181,6 +182,10 @@ struct ApplyArgs { /// Secrets are never pruned. #[arg(long)] prune: bool, + /// Skip the `--prune` confirmation prompt. Required to prune + /// non-interactively (CI). + #[arg(long)] + yes: bool, } #[derive(Args)] @@ -1086,7 +1091,7 @@ async fn main() -> ExitCode { } Cmd::Logout => cmds::logout::run().await, Cmd::Whoami => cmds::whoami::run(mode).await, - Cmd::Apply(args) => cmds::apply::run(&args.file, args.prune, mode).await, + Cmd::Apply(args) => cmds::apply::run(&args.file, args.prune, args.yes, mode).await, Cmd::Plan(args) => cmds::plan::run(&args.file, mode).await, Cmd::Pull(args) => cmds::pull::run(&args.app, &args.dir, mode).await, Cmd::Apps { cmd: AppsCmd::Ls } => cmds::apps::ls(mode).await, diff --git a/crates/picloud-cli/tests/email_queue.rs b/crates/picloud-cli/tests/email_queue.rs index cfb88fb..99b08bf 100644 --- a/crates/picloud-cli/tests/email_queue.rs +++ b/crates/picloud-cli/tests/email_queue.rs @@ -135,7 +135,7 @@ fn prune_refuses_to_orphan_email_trigger() { let out = common::pic_as(&env) .args(["apply", "--file"]) .arg(&manifest_path) - .arg("--prune") + .args(["--prune", "--yes"]) .output() .expect("apply --prune"); assert!( diff --git a/crates/picloud-cli/tests/prune.rs b/crates/picloud-cli/tests/prune.rs index e940674..4f3346b 100644 --- a/crates/picloud-cli/tests/prune.rs +++ b/crates/picloud-cli/tests/prune.rs @@ -72,11 +72,12 @@ fn prune_deletes_stale_resources() { "additive apply must not delete" ); - // Prune apply removes `drop` and its route. + // Prune apply removes `drop` and its route. `--yes` skips the + // confirmation prompt (this test runs non-interactively). let out = common::pic_as(&env) .args(["apply", "--file"]) .arg(&manifest_path) - .arg("--prune") + .args(["--prune", "--yes"]) .output() .expect("apply --prune"); assert!( @@ -109,3 +110,68 @@ fn prune_deletes_stale_resources() { "plan should be clean after prune:\n{p}" ); } + +/// The prune confirmation gate: `--prune` without `--yes`, run +/// non-interactively (no TTY, as in CI), must refuse and delete nothing. +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn prune_without_yes_refuses_noninteractively() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let slug = common::unique_slug("prune-gate"); + common::pic_as(&env) + .args(["apps", "create", &slug]) + .assert() + .success(); + let _guard = AppGuard::new(&env.url, &env.token, &slug); + + let dir = TempDir::new().unwrap(); + fs::create_dir_all(dir.path().join("scripts")).unwrap(); + fs::write(dir.path().join("scripts/keep.rhai"), "let x = 1; x").unwrap(); + fs::write(dir.path().join("scripts/drop.rhai"), "let y = 2; y").unwrap(); + let manifest_path = dir.path().join("picloud.toml"); + + // Deploy two scripts, then drop one from the manifest. + let v1 = format!( + "[app]\nslug = \"{slug}\"\nname = \"Gate Test\"\n\n\ + [[scripts]]\nname = \"keep\"\nfile = \"scripts/keep.rhai\"\n\n\ + [[scripts]]\nname = \"drop\"\nfile = \"scripts/drop.rhai\"\n" + ); + fs::write(&manifest_path, &v1).unwrap(); + common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&manifest_path) + .assert() + .success(); + let v2 = format!( + "[app]\nslug = \"{slug}\"\nname = \"Gate Test\"\n\n\ + [[scripts]]\nname = \"keep\"\nfile = \"scripts/keep.rhai\"\n" + ); + fs::write(&manifest_path, &v2).unwrap(); + + // `--prune` with no `--yes` and no TTY → refuse, non-zero exit. + let out = common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&manifest_path) + .arg("--prune") + .output() + .expect("apply --prune"); + assert!( + !out.status.success(), + "prune without --yes must refuse non-interactively" + ); + let err = String::from_utf8_lossy(&out.stderr); + assert!( + err.contains("--yes"), + "refusal should mention --yes:\n{err}" + ); + + // The dropped script must still be there — the gate blocked the delete. + let s = scripts_ls(&env, &slug); + assert!( + s.contains("drop"), + "refused prune must not delete anything:\n{s}" + ); +}