diff --git a/Cargo.lock b/Cargo.lock index 2100fc07..fae5fac2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -719,6 +719,7 @@ dependencies = [ "flate2", "futures", "futures-util", + "handlebars", "log", "log-fastly", "serde", diff --git a/crates/edgezero-adapter-fastly/Cargo.toml b/crates/edgezero-adapter-fastly/Cargo.toml index 4fee1797..efd50c74 100644 --- a/crates/edgezero-adapter-fastly/Cargo.toml +++ b/crates/edgezero-adapter-fastly/Cargo.toml @@ -47,4 +47,5 @@ walkdir = { workspace = true, optional = true } [dev-dependencies] edgezero-core = { path = "../edgezero-core", features = ["test-utils"] } +handlebars = { workspace = true } tempfile = { workspace = true } diff --git a/crates/edgezero-adapter-fastly/src/chunked_config.rs b/crates/edgezero-adapter-fastly/src/chunked_config.rs index 6c8b0937..3bfae50c 100644 --- a/crates/edgezero-adapter-fastly/src/chunked_config.rs +++ b/crates/edgezero-adapter-fastly/src/chunked_config.rs @@ -302,6 +302,87 @@ where Ok(reconstructed) } +/// Validate a prior root value and return the chunk keys it referenced, +/// scoped to `root_key`'s own chunk namespace. Used only for chunk GC on +/// re-push (Stage 7 writeback). +/// +/// Parse as `serde_json::Value` FIRST so a pointer-kind value with +/// missing/invalid fields still reaches the warning path instead of being +/// silently dropped by a failed struct deserialize. +/// +/// Returns: +/// - `Ok(keys)` -- value is a valid v1 chunk pointer; `keys` are its +/// `chunks[].key` entries (all confirmed to match +/// `"{root_key}{CHUNK_KEY_INFIX}"`). +/// - `Ok(vec![])` -- value is a direct `BlobEnvelope`, absent, or not +/// pointer-shaped at all (normal: first push, or was-direct). Silent. +/// - `Err(msg)` -- value IS pointer-kind (`edgezero_kind == POINTER_KIND`) +/// but fails validation: malformed, unsupported `version`, or a +/// referenced key falls outside this root's chunk prefix. Callers log +/// `msg` as a warning and skip GC for this root (delete nothing). +#[cfg(any(feature = "cli", test))] +pub(crate) fn prior_chunk_keys(root_key: &str, raw: &str) -> Result, String> { + // 1. Parse loosely. Not-JSON, or not our pointer kind => silent. + let value: serde_json::Value = match serde_json::from_str(raw) { + Ok(value) => value, + Err(_) => return Ok(Vec::new()), + }; + if value + .get("edgezero_kind") + .and_then(serde_json::Value::as_str) + != Some(POINTER_KIND) + { + // Direct BlobEnvelope, unrelated JSON, or first push. + return Ok(Vec::new()); + } + + // 2. It IS pointer-kind: from here every failure WARNS (Err), never silent. + let pointer: FastlyChunkPointer = serde_json::from_value(value).map_err(|err| { + format!("prior chunk pointer at `{root_key}` is malformed: {err}; skipping chunk GC") + })?; + if pointer.version != 1 { + return Err(format!( + "prior chunk pointer at `{root_key}` has unsupported version {}; skipping chunk GC", + pointer.version + )); + } + + // 3. Prefix-scope every referenced key to this root's own namespace. + let prefix = format!("{root_key}{CHUNK_KEY_INFIX}"); + let mut keys = Vec::with_capacity(pointer.chunks.len()); + for chunk_ref in pointer.chunks { + if !chunk_ref.key.starts_with(&prefix) { + return Err(format!( + "prior chunk pointer at `{root_key}` references chunk key `{}` outside expected prefix `{prefix}`; skipping chunk GC", + chunk_ref.key + )); + } + keys.push(chunk_ref.key); + } + Ok(keys) +} + +/// Extract the content-address (envelope SHA) of the generation a chunk key +/// belongs to, for `root_key`. Returns `None` when `key` is not a well-formed +/// chunk key of that root. +/// +/// This is how reclamation groups the store's ACTUAL keys into generations. It +/// validates the shape (`.__edgezero_chunks..`) rather +/// than trusting it: a hand-edited or foreign key never becomes a delete target. +#[cfg(any(feature = "cli", test))] +pub(crate) fn chunk_key_generation(root_key: &str, key: &str) -> Option { + let prefix = format!("{root_key}{CHUNK_KEY_INFIX}"); + let rest = key.strip_prefix(&prefix)?; + let (sha, index) = rest.rsplit_once('.')?; + if sha.is_empty() || !sha.chars().all(|ch| ch.is_ascii_hexdigit()) { + return None; + } + if index.is_empty() || !index.chars().all(|ch| ch.is_ascii_digit()) { + return None; + } + Some(sha.to_owned()) +} + // --------------------------------------------------------------------------- // Unit tests // --------------------------------------------------------------------------- @@ -657,4 +738,119 @@ mod tests { ); assert!(err.contains("my_key"), "error must name root key: {err}"); } + + // ---- prior_chunk_keys tests ---- + + #[test] + fn prior_chunk_keys_returns_valid_v1_pointer_keys() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + + let keys = prior_chunk_keys("app_config", pointer_json).expect("valid pointer"); + + let expected: Vec = entries[..entries.len().saturating_sub(1)] + .iter() + .map(|(key, _)| key.clone()) + .collect(); + assert_eq!(keys, expected); + } + + #[test] + fn prior_chunk_keys_returns_empty_for_direct_envelope() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT); + assert_eq!( + prior_chunk_keys("app_config", &envelope).unwrap(), + Vec::::new() + ); + } + + #[test] + fn prior_chunk_keys_returns_empty_for_unrelated_json() { + assert_eq!( + prior_chunk_keys("app_config", r#"{"hello":"world"}"#).unwrap(), + Vec::::new() + ); + } + + #[test] + fn prior_chunk_keys_returns_empty_for_wrong_kind() { + let raw = r#"{"edgezero_kind":"other","version":1,"chunks":[],"data_sha256":"","envelope_len":0,"envelope_sha256":""}"#; + assert_eq!( + prior_chunk_keys("app_config", raw).unwrap(), + Vec::::new() + ); + } + + #[test] + fn prior_chunk_keys_rejects_unsupported_pointer_version() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + let mut pointer: FastlyChunkPointer = serde_json::from_str(pointer_json).unwrap(); + pointer.version = 2; + let raw = serde_json::to_string(&pointer).unwrap(); + + let err = prior_chunk_keys("app_config", &raw).expect_err("version 2 should warn"); + assert!(err.contains("unsupported version"), "{err}"); + } + + #[test] + fn prior_chunk_keys_rejects_foreign_chunk_prefix() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + let mut pointer: FastlyChunkPointer = serde_json::from_str(pointer_json).unwrap(); + pointer.chunks[0].key = + pointer.chunks[0] + .key + .replacen("app_config", "app_config_staging", 1); + let raw = serde_json::to_string(&pointer).unwrap(); + + let err = prior_chunk_keys("app_config", &raw).expect_err("foreign chunk should warn"); + assert!(err.contains("outside"), "{err}"); + } + + // ---- chunk_key_generation ---- + + #[test] + fn chunk_key_generation_extracts_the_sha() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (chunk_key, _) = &entries[0]; + let sha = chunk_key_generation("app_config", chunk_key).expect("a chunk key"); + assert_eq!(sha.len(), 64, "sha256 hex"); + assert!(chunk_key.contains(&sha)); + } + + #[test] + fn chunk_key_generation_rejects_foreign_and_malformed_keys() { + // Different root. + assert!(chunk_key_generation("app_config", "other.__edgezero_chunks.abc123.0").is_none()); + // The root key itself. + assert!(chunk_key_generation("app_config", "app_config").is_none()); + // Non-hex content address. + assert!( + chunk_key_generation("app_config", "app_config.__edgezero_chunks.zzzz.0").is_none() + ); + // Non-numeric index. + assert!( + chunk_key_generation("app_config", "app_config.__edgezero_chunks.abc123.x").is_none() + ); + // Missing index. + assert!( + chunk_key_generation("app_config", "app_config.__edgezero_chunks.abc123").is_none() + ); + } + + // Regression for the Value-first rule: a value that IS pointer-kind but + // is missing required fields (`chunks`, `data_sha256`, …) must WARN, not + // be silently dropped by a failed struct deserialize. + #[test] + fn prior_chunk_keys_warns_on_pointer_kind_with_missing_fields() { + let raw = r#"{"edgezero_kind":"fastly_config_chunks","version":2}"#; + let err = prior_chunk_keys("app_config", raw) + .expect_err("pointer-kind but malformed must warn, not Ok([])"); + assert!(!err.is_empty(), "{err}"); + } } diff --git a/crates/edgezero-adapter-fastly/src/cli.rs b/crates/edgezero-adapter-fastly/src/cli.rs index a3de1cba..ef981ca6 100644 --- a/crates/edgezero-adapter-fastly/src/cli.rs +++ b/crates/edgezero-adapter-fastly/src/cli.rs @@ -1,11 +1,16 @@ +use std::collections::HashSet; use std::env; use std::fs; use std::io::{ErrorKind, Write as _}; use std::path::{Path, PathBuf}; use std::process::Command; use std::process::Stdio; +use std::time::{SystemTime, UNIX_EPOCH}; -use crate::chunked_config::{prepare_fastly_config_entries, resolve_fastly_config_value}; +use crate::chunked_config::{ + chunk_key_generation, prepare_fastly_config_entries, prior_chunk_keys, + resolve_fastly_config_value, CHUNK_KEY_INFIX, +}; use ctor::ctor; use edgezero_adapter::cli_support::{ find_manifest_upwards, find_workspace_root, path_distance, read_package_name, run_native_cli, @@ -144,6 +149,32 @@ enum ConfigStoreLookup { SchemaDrift(String), } +/// One `config-store-entry list` item. +/// +/// `item_value` IS captured — `config gc` must parse root pointers to learn +/// which chunks are live, and one listing avoids a `describe` per root. It is +/// the config payload: it may be read in memory but must NEVER be logged or +/// surfaced (see `redact_describe_response` / `redact_stderr`). +struct ConfigStoreItem { + created_at: String, + item_key: String, + item_value: String, +} + +/// Per-root plan for the LOCAL path's eager prune. +/// +/// Local reclamation is safe to do immediately: `fastly.toml` is a single +/// file that Viceroy reads at startup — there is no propagation window and no +/// POP that could still be serving the previous pointer. (The cloud path +/// cannot do this; see `reclaim_orphan_generations`.) +struct FastlyConfigGcPlan { + /// Exact keep-set this push writes for the root (chunk keys + root key). + new_keys: HashSet, + /// Prior chunk keys to consider deleting, or a warning to surface + /// (suspicious prior pointer) that skips GC for this root. + prior_keys: Result, String>, +} + // The three `validate_*` trait methods exist on `Adapter` because // spin requires them (variable-name regex, `[component.*]` // discovery, flat-namespace collision). The trait surface is typed @@ -194,6 +225,19 @@ impl Adapter for FastlyCliAdapter { } } + fn gc_config_entries( + &self, + _manifest_root: &Path, + _adapter_manifest_path: Option<&str>, + _component_selector: Option<&str>, + store: &ResolvedStoreId, + _push_ctx: &AdapterPushContext<'_>, + older_than_secs: u64, + dry_run: bool, + ) -> Result, String> { + gc_fastly_config_store(store.platform.as_str(), older_than_secs, dry_run) + } + fn name(&self) -> &'static str { "fastly" } @@ -371,21 +415,25 @@ impl Adapter for FastlyCliAdapter { "no config entries to push to fastly config-store `{name}` (logical id `{logical}`)" )]); } - // Expand each logical (key, envelope_json) into physical entries - // via the chunk-pointer helper. Entries ≤ 8 000 chars go through - // as a single direct entry; larger envelopes are split into - // content-addressed chunks with a root pointer written LAST. - // Collect all physical entries before any writes so pointer-too- - // large errors surface before touching the remote store. + // Reject reserved keys before any expansion or I/O. + reject_reserved_root_keys(entries)?; + reject_duplicate_root_keys(entries)?; + // Expand each logical root once: flatten for the commit, and keep + // the exact per-root keep-set + the value written at the root key + // for GC (no prefix scan of the flattened set). Collecting all + // physical entries first also surfaces pointer-too-large errors + // before touching the remote store. let mut physical_entries: Vec<(String, String)> = Vec::new(); + let mut roots: Vec<(String, HashSet, String)> = Vec::with_capacity(entries.len()); for (key, body) in entries { - let expanded = prepare_fastly_config_entries(key, body)?; + let (expanded, new_keys, new_root_value) = expand_root(key, body)?; physical_entries.extend(expanded); + roots.push((key.clone(), new_keys, new_root_value)); } if dry_run { - // Report intent without shelling out. One line per logical key - // noting whether it would be direct or chunked, plus chunk count. - let mut out = Vec::with_capacity(entries.len().saturating_add(1)); + // Report intent without shelling out. Stays fully offline: no + // store-id resolution, no remote read (so no GC count). + let mut out = Vec::with_capacity(entries.len().saturating_mul(2).saturating_add(1)); out.push(format!( "would resolve fastly config-store `{name}` (logical id `{logical}`) via `fastly config-store list --json` and push entries:" )); @@ -408,6 +456,21 @@ impl Adapter for FastlyCliAdapter { return Ok(out); } let resolved_id = resolve_remote_config_store_id(name)?; + // NOTE: a cloud push does NOT reclaim orphaned chunks. + // + // Fastly's config store is eventually consistent, so a generation may + // only be deleted once the pointer that referenced it has stopped being + // served everywhere. Fastly records no pointer-supersession time + // (`updated_at` is NOT bumped by `update --upsert` -- verified against + // the live API), offers no compare-and-swap with which to record one + // safely, and chunk `created_at` is NOT a proxy for it (a chunked -> + // direct -> direct transition leaves the old generation with no + // "successor" at all). Every attempt to synthesise that fact is unsound. + // + // So reclamation is an explicit, operator-invoked `config gc`: the + // operator supplies the one fact the platform cannot -- that the current + // config has been live long enough that nothing is serving the old + // pointers. See the spec's "Cloud reclamation". push_entries_with_committer(&physical_entries, |key, value| { create_config_store_entry(&resolved_id, key, value) })?; @@ -450,19 +513,26 @@ impl Adapter for FastlyCliAdapter { fastly_path.display() )]); } - // Expand logical entries into physical entries (chunks + pointer). + // Reject reserved keys before any expansion or I/O. + reject_reserved_root_keys(entries)?; + reject_duplicate_root_keys(entries)?; + // Expand each logical root once: flatten for the write, keep the + // exact per-root keep-set for GC (no prefix scan of the flattened set). let mut physical_entries: Vec<(String, String)> = Vec::new(); + let mut gc_roots: Vec<(String, HashSet)> = Vec::with_capacity(entries.len()); for (key, body) in entries { - let expanded = prepare_fastly_config_entries(key, body)?; + let (expanded, new_keys, _new_root) = expand_root(key, body)?; physical_entries.extend(expanded); + gc_roots.push((key.clone(), new_keys)); } if dry_run { - let mut out = Vec::with_capacity(entries.len().saturating_add(1)); + let counts = local_orphan_counts_for_dry_run(&fastly_path, name, entries); + let mut out = Vec::with_capacity(entries.len().saturating_mul(2).saturating_add(1)); out.push(format!( "would edit `[local_server.config_stores.{name}.contents]` in {} (logical id `{logical}`) with entries:", fastly_path.display(), )); - for (key, body) in entries { + for (idx, (key, body)) in entries.iter().enumerate() { let expanded = prepare_fastly_config_entries(key, body) .unwrap_or_else(|_| vec![(key.clone(), body.clone())]); if expanded.len() == 1 { @@ -477,16 +547,28 @@ impl Adapter for FastlyCliAdapter { body.len() )); } + match counts.get(idx).map(|(_, count)| count) { + Some(Ok(n)) => out.push(format!( + " would delete {n} orphan chunks from the previous generation of `{key}`" + )), + Some(Err(reason)) => out.push(format!( + " would delete an unknown number of orphan chunks from the previous generation of `{key}` (unknown: {reason})" + )), + None => {} + } } return Ok(out); } - write_fastly_local_config_store(&fastly_path, name, &physical_entries)?; - Ok(vec![format!( + let warnings = + write_fastly_local_config_store(&fastly_path, name, &physical_entries, &gc_roots)?; + let mut out = vec![format!( "wrote {} physical entries ({} logical) to `[local_server.config_stores.{name}.contents]` in {} (logical id `{logical}`); restart `fastly compute serve` to pick up changes", physical_entries.len(), entries.len(), fastly_path.display() - )]) + )]; + out.extend(warnings); + Ok(out) } fn read_config_entry( @@ -538,19 +620,20 @@ impl Adapter for FastlyCliAdapter { if output.status.success() { let stdout = String::from_utf8_lossy(&output.stdout); // Parse the JSON and extract the `item_value` field. - let parsed: serde_json::Value = - serde_json::from_str(&stdout).map_err(|err| { - format!( - "failed to parse `fastly config-store-entry describe` JSON: {err}\nraw stdout: {stdout}" - ) - })?; + let parsed: serde_json::Value = serde_json::from_str(&stdout).map_err(|err| { + format!( + "failed to parse `fastly config-store-entry describe` JSON: {err} (response: {})", + redact_describe_response(&stdout) + ) + })?; let value = parsed .get("item_value") .and_then(serde_json::Value::as_str) .ok_or_else(|| { format!( "`fastly config-store-entry describe` JSON has no string `item_value` field; \ - fastly CLI may have changed its output schema. Raw stdout: {stdout}" + fastly CLI may have changed its output schema. (response: {})", + redact_describe_response(&stdout) ) })?; // Resolve chunk pointers: if `value` is a direct BlobEnvelope it @@ -570,7 +653,7 @@ impl Adapter for FastlyCliAdapter { Err(format!( "`fastly config-store-entry describe --store-id={store_id} --key={key} --json` exited with status {}\nstderr: {}", output.status, - stderr.trim() + redact_stderr(&stderr) )) } @@ -691,8 +774,9 @@ fn fetch_remote_config_store_entry(store_id: &str, key: &str) -> Result Result Result Result<(), String> { + gc_roots: &[(String, HashSet)], +) -> Result, String> { use toml_edit::{table, DocumentMut, Item, Table, Value}; let raw = match fs::read_to_string(path) { @@ -992,15 +1078,217 @@ fn write_fastly_local_config_store( path.display() ) })?; + // Snapshot prior chunk keys per GC root BEFORE the upsert, using the + // exact keep-set the caller computed for each root (no prefix scan). + let mut plans: Vec = Vec::with_capacity(gc_roots.len()); + for (root_key, new_keys) in gc_roots { + let prior_keys = contents_tbl + .get(root_key) + .and_then(toml_edit::Item::as_str) + .map_or_else(|| Ok(Vec::new()), |value| prior_chunk_keys(root_key, value)); + plans.push(FastlyConfigGcPlan { + new_keys: new_keys.clone(), + prior_keys, + }); + } + + // Upsert the new physical entries. for (key, value) in entries { contents_tbl.insert(key, Item::Value(Value::from(value.clone()))); } + // Prune orphans in the same in-memory rewrite; a suspicious prior + // pointer (Err) warns and deletes nothing. + let mut warnings = Vec::new(); + for plan in &plans { + match orphan_chunk_keys(plan) { + Ok(orphans) => { + for key in orphans { + contents_tbl.remove(&key); + } + } + Err(err) => warnings.push(format!("warning: {err}")), + } + } + fs::write(path, doc.to_string()) .map_err(|err| format!("failed to write {}: {err}", path.display()))?; + Ok(warnings) +} + +// ------------------------------------------------------------------- +// chunk GC helpers (Stage 7 re-push reclamation) +// ------------------------------------------------------------------- + +/// Expand ONE logical `(root_key, body)` into its physical entries, the +/// exact keep-set for that root, and the value written at the root key. +/// No cross-root prefix scanning (a free-form `--key` can't mislead it). +#[expect( + clippy::type_complexity, + reason = "one-off internal return; a named type would not aid readability" +)] +fn expand_root( + root_key: &str, + body: &str, +) -> Result<(Vec<(String, String)>, HashSet, String), String> { + let expanded = prepare_fastly_config_entries(root_key, body)?; + let new_keys: HashSet = expanded.iter().map(|(key, _)| key.clone()).collect(); + // prepare_* always emits the root entry LAST (root pointer or direct + // value). Make the invariant explicit rather than silently defaulting. + let new_root_value = expanded + .last() + .map(|(_, value)| value.clone()) + .ok_or_else(|| format!("internal: no physical entries produced for root `{root_key}`"))?; + Ok((expanded, new_keys, new_root_value)) +} + +/// Orphans = prior chunk keys not in the new keep-set. Propagates a +/// suspicious-pointer `Err` so the caller can warn and skip GC. +fn orphan_chunk_keys(plan: &FastlyConfigGcPlan) -> Result, String> { + match &plan.prior_keys { + Ok(prior) => Ok(prior + .iter() + .filter(|key| !plan.new_keys.contains(*key)) + .cloned() + .collect()), + Err(err) => Err(err.clone()), + } +} + +/// Reject logical keys that collide with the reserved chunk namespace. +/// `--key` is free-form, so this is enforced at the Fastly adapter +/// boundary: such a key would let a push write into another key's chunk +/// space, and could not be reclaimed correctly. +fn reject_reserved_root_keys(entries: &[(String, String)]) -> Result<(), String> { + for (key, _) in entries { + if key.contains(CHUNK_KEY_INFIX) { + return Err(format!( + "config key `{key}` contains the reserved infix `{CHUNK_KEY_INFIX}`, which collides with Fastly chunk storage; choose a different config key (or --key override)" + )); + } + } + Ok(()) +} + +/// Unix epoch seconds. Push-time only (the `cli` feature is native). +fn unix_now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |elapsed| elapsed.as_secs()) +} + +/// Reject a batch that names the same logical root key more than once. +/// +/// The adapter trait takes an entry slice and does not enforce uniqueness, +/// but GC builds one plan per entry and snapshots every plan against the +/// SAME prior generation. With `[(root, A), (root, B)]` the last tuple wins +/// the upsert (root = B), yet A's plan would still reclaim `prior - A_keys` +/// — which includes B's freshly-written chunks — leaving the final pointer +/// referencing missing chunks. Rejecting is safer than silently coalescing: +/// a duplicated key is a caller bug, and picking a winner would hide it. +fn reject_duplicate_root_keys(entries: &[(String, String)]) -> Result<(), String> { + let mut seen: HashSet<&str> = HashSet::with_capacity(entries.len()); + for (key, _) in entries { + if !seen.insert(key.as_str()) { + return Err(format!( + "config key `{key}` appears more than once in a single push; each logical key must be pushed exactly once" + )); + } + } Ok(()) } +/// Best-effort per-root orphan count for `config push --local --dry-run`. +/// Navigate to `[local_server.config_stores..contents]` for the +/// dry-run counter. `Ok(None)` when any level is absent (no prior state); +/// `Err` when a level is present but the wrong type — prior state the real +/// writer would reject, so the count must degrade to "unknown", not 0. +fn local_contents_table<'doc>( + doc: &'doc toml_edit::DocumentMut, + platform_name: &str, +) -> Result, String> { + let malformed = || "could not read prior state".to_owned(); + let Some(server_item) = doc.get("local_server") else { + return Ok(None); + }; + let Some(server) = server_item.as_table() else { + return Err(malformed()); + }; + let Some(stores_item) = server.get("config_stores") else { + return Ok(None); + }; + let Some(stores) = stores_item.as_table() else { + return Err(malformed()); + }; + let Some(store_item) = stores.get(platform_name) else { + return Ok(None); + }; + let Some(store) = store_item.as_table() else { + return Err(malformed()); + }; + let Some(contents_item) = store.get("contents") else { + return Ok(None); + }; + contents_item + .as_table() + .map_or_else(|| Err(malformed()), |table| Ok(Some(table))) +} + +/// Reads the current `fastly.toml` (offline) and, for each logical +/// `(root_key, body)`, counts `prior_chunk_keys(root, old) - new_keys` +/// where `new_keys` is the root's OWN expansion. Never fails the dry-run: +/// on a missing file / no prior pointer / direct prior value it reports +/// `Ok(0)`; on unreadable or malformed prior state it reports `Err(reason)` +/// which the caller renders as an "unknown" line. +fn local_orphan_counts_for_dry_run( + path: &Path, + platform_name: &str, + entries: &[(String, String)], +) -> Vec<(String, Result)> { + use toml_edit::DocumentMut; + + // Parse the current file once (best-effort). Absent file => no prior. + let parsed: Result, String> = match fs::read_to_string(path) { + Ok(text) => text + .parse::() + .map(Some) + .map_err(|_err| "could not read prior state".to_owned()), + Err(err) if err.kind() == ErrorKind::NotFound => Ok(None), + Err(_) => Err("could not read prior state".to_owned()), + }; + + entries + .iter() + .map(|(root_key, body)| { + let new_keys = match expand_root(root_key, body) { + Ok((_, keys, _)) => keys, + Err(err) => return (root_key.clone(), Err(err)), + }; + let count = match &parsed { + Err(reason) => Err(reason.clone()), + Ok(None) => Ok(0), + Ok(Some(doc)) => match local_contents_table(doc, platform_name) { + Err(reason) => Err(reason), + Ok(None) => Ok(0), + Ok(Some(contents)) => match contents.get(root_key) { + None => Ok(0), // no prior value for this root + Some(item) => match item.as_str() { + None => Err("could not read prior state".to_owned()), + Some(raw) => match prior_chunk_keys(root_key, raw) { + Ok(prior) => { + Ok(prior.iter().filter(|key| !new_keys.contains(*key)).count()) + } + Err(_) => Err("suspicious prior pointer".to_owned()), + }, + }, + }, + }, + }; + (root_key.clone(), count) + }) + .collect() +} + // ------------------------------------------------------------------- // `config push` helpers // ------------------------------------------------------------------- @@ -1011,6 +1299,182 @@ fn write_fastly_local_config_store( /// exists" error, which is the operator's signal to delete the /// entry (or use `config-store-entry update` manually) before /// re-running push. +/// `fastly config-store-entry list --store-id= --json`, keeping only each +/// item's key and `created_at`. The item VALUE is discarded on purpose (it is +/// the stored config; see `redact_describe_response`). +fn list_config_store_entries(store_id: &str) -> Result, String> { + let store_arg = format!("--store-id={store_id}"); + let output = Command::new("fastly") + .args(["config-store-entry", "list", store_arg.as_str(), "--json"]) + .output() + .map_err(|err| { + if err.kind() == ErrorKind::NotFound { + format!("`fastly` not found on PATH; {FASTLY_INSTALL_HINT}") + } else { + format!("failed to spawn `fastly`: {err}") + } + })?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!( + "`fastly config-store-entry list --store-id={store_id} --json` exited with status {}\nstderr: {}", + output.status, + redact_stderr(&stderr) + )); + } + let stdout = String::from_utf8_lossy(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(&stdout).map_err(|err| { + format!( + "failed to parse `fastly config-store-entry list` JSON: {err} (response: {})", + redact_describe_response(&stdout) + ) + })?; + let array = parsed + .as_array() + .or_else(|| parsed.get("items").and_then(serde_json::Value::as_array)) + .ok_or_else(|| { + format!( + "`fastly config-store-entry list` returned unexpected JSON (response: {})", + redact_describe_response(&stdout) + ) + })?; + let mut items = Vec::with_capacity(array.len()); + for entry in array { + let Some(item_key) = entry.get("item_key").and_then(serde_json::Value::as_str) else { + continue; + }; + let created_at = entry + .get("created_at") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + let item_value = entry + .get("item_value") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + items.push(ConfigStoreItem { + created_at: created_at.to_owned(), + item_key: item_key.to_owned(), + item_value: item_value.to_owned(), + }); + } + Ok(items) +} + +/// RFC 3339 (`2026-07-13T03:27:42Z`) -> unix seconds. +fn parse_rfc3339_secs(raw: &str) -> Option { + let stamp = chrono::DateTime::parse_from_rfc3339(raw).ok()?; + u64::try_from(stamp.timestamp()).ok() +} + +/// `config gc` for Fastly: delete chunk entries that no LIVE root pointer +/// references and that are older than the operator's `older_than_secs`. +/// +/// Why this is a separate, operator-invoked command rather than part of `config +/// push`: see `Adapter::gc_config_entries`. The operator's `--older-than` is the +/// safety assertion the platform cannot make. A dry-run prints exactly which +/// keys would go, with ages, so the assertion is reviewable. +/// +/// Fails CLOSED: if the listing is unreadable, or a root's value cannot be +/// classified, nothing is deleted. +fn gc_fastly_config_store( + store_name: &str, + older_than_secs: u64, + dry_run: bool, +) -> Result, String> { + let resolved_id = resolve_remote_config_store_id(store_name)?; + let items = list_config_store_entries(&resolved_id)?; + let now = unix_now_secs(); + + // LIVE chunk keys = those named by any root pointer currently in the store. + // A root is any entry that is not itself a chunk key. + let mut live: HashSet = HashSet::new(); + let mut roots = 0_usize; + for item in &items { + if chunk_key_generation_any(&item.item_key).is_some() { + continue; // a chunk, not a root + } + roots = roots.saturating_add(1); + match prior_chunk_keys(&item.item_key, &item.item_value) { + Ok(keys) => live.extend(keys), + Err(err) => { + // Cannot classify this root -> we do not know what it references. + // Fail closed rather than risk deleting something live. + return Err(format!( + "refusing to reclaim: could not classify root `{}` ({err}); nothing was deleted", + item.item_key + )); + } + } + } + + // Candidates: chunk entries not referenced by any live pointer, older than + // the operator's threshold. + let mut doomed: Vec<(String, u64)> = Vec::new(); + let mut retained_recent = 0_usize; + for item in &items { + if chunk_key_generation_any(&item.item_key).is_none() { + continue; + } + if live.contains(&item.item_key) { + continue; + } + let Some(created) = parse_rfc3339_secs(&item.created_at) else { + // Unparseable timestamp on a DELETE path -> fail closed. + return Err(format!( + "refusing to reclaim: entry `{}` has an unreadable `created_at`; nothing was deleted", + item.item_key + )); + }; + let age = now.saturating_sub(created); + if age < older_than_secs { + retained_recent = retained_recent.saturating_add(1); + continue; + } + doomed.push((item.item_key.clone(), age)); + } + + let mut out = vec![format!( + "fastly config-store `{store_name}` (id={resolved_id}): {} entries, {roots} root(s), {} live chunk(s), {} orphan(s) older than {older_than_secs}s, {retained_recent} orphan(s) too recent", + items.len(), + live.len(), + doomed.len(), + )]; + if doomed.is_empty() { + out.push("nothing to reclaim".to_owned()); + return Ok(out); + } + for (key, age) in &doomed { + let verb = if dry_run { "would delete" } else { "deleting" }; + out.push(format!(" {verb} `{key}` (age {age}s)")); + } + if dry_run { + out.push(format!( + "dry-run: {} orphan chunk(s) would be deleted; re-run without --dry-run to apply", + doomed.len() + )); + return Ok(out); + } + let mut deleted = 0_usize; + for (key, _) in &doomed { + match delete_config_store_entry(&resolved_id, key) { + Ok(()) => deleted = deleted.saturating_add(1), + Err(err) => out.push(format!(" warning: could not delete `{key}` ({err})")), + } + } + out.push(format!( + "reclaimed {deleted} of {} orphan chunk entries", + doomed.len() + )); + Ok(out) +} + +/// Is this key a chunk key of ANY root? (`config gc` scans the whole store, so +/// it cannot scope to one root up front.) Validates the canonical shape. +fn chunk_key_generation_any(key: &str) -> Option { + let (root, _rest) = key.split_once(CHUNK_KEY_INFIX)?; + chunk_key_generation(root, key) +} + /// Drive a sequential per-entry commit loop and produce the /// partial-failure diagnostic when the committer fails mid-way. /// Pure (no I/O) so the diagnostic shape is unit-testable without @@ -1113,7 +1577,47 @@ fn create_config_store_entry(store_id: &str, key: &str, value: &str) -> Result<( Err(format!( "`fastly config-store-entry update --store-id={store_id} --key={key} --upsert --stdin` exited with status {}\nstderr: {}", output.status, - String::from_utf8_lossy(&output.stderr).trim() + redact_stderr(&String::from_utf8_lossy(&output.stderr)) + )) +} + +/// Shell `fastly config-store-entry delete --store-id= --key= +/// --auto-yes` for a single orphan chunk. Only ever passes `--key`: the +/// subcommand also accepts `-a/--all`, which wipes EVERY entry in the +/// store — never construct that flag here. `--auto-yes` suppresses the +/// interactive confirmation. A "not found" / "does not exist" / "404" +/// stderr is treated as success (the entry is already gone). +fn delete_config_store_entry(store_id: &str, key: &str) -> Result<(), String> { + let store_arg = format!("--store-id={store_id}"); + let key_arg = format!("--key={key}"); + let output = Command::new("fastly") + .args([ + "config-store-entry", + "delete", + store_arg.as_str(), + key_arg.as_str(), + "--auto-yes", + ]) + .output() + .map_err(|err| { + if err.kind() == ErrorKind::NotFound { + format!("`fastly` not found on PATH; {FASTLY_INSTALL_HINT}") + } else { + format!("failed to spawn `fastly`: {err}") + } + })?; + if output.status.success() { + return Ok(()); + } + let stderr = String::from_utf8_lossy(&output.stderr); + let lower = stderr.to_ascii_lowercase(); + if lower.contains("not found") || lower.contains("does not exist") || lower.contains("404") { + return Ok(()); + } + Err(format!( + "`fastly config-store-entry delete --store-id={store_id} --key={key} --auto-yes` exited with status {}\nstderr: {}", + output.status, + stderr.trim() )) } @@ -1166,6 +1670,54 @@ fn find_config_store_id(stdout: &str, name: &str) -> ConfigStoreLookup { } } +/// Summarise a `fastly ... describe` response for diagnostics WITHOUT +/// leaking its contents. +/// +/// The response body is the stored config value. App config may hold +/// credentials, internal endpoints, or security policy, and this adapter +/// performs no secret stripping — while CLI status lines are logged +/// verbatim and CI logs are commonly retained and shared. So a schema-drift +/// diagnostic must never echo the payload: report only its size and its +/// top-level *shape* (field names for an object, type otherwise), never a +/// value. +fn redact_describe_response(stdout: &str) -> String { + let len = stdout.len(); + serde_json::from_str::(stdout).map_or_else( + |_err| format!("{len} bytes, not valid JSON"), + |value| match value { + serde_json::Value::Object(map) => { + let mut names: Vec<&str> = map.keys().map(String::as_str).collect(); + names.sort_unstable(); + format!( + "{len} bytes, JSON object with fields [{}]", + names.join(", ") + ) + } + other @ (serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_)) => { + format!("{len} bytes, JSON {}", shape_summary(&other)) + } + }, + ) +} + +/// Summarise a failing `fastly` invocation's stderr WITHOUT echoing it. +/// +/// The `describe` and `update --stdin` paths carry the stored config value, so +/// a Fastly error that quotes the payload back would put credentials straight +/// into CI logs — the same exposure as the stdout leak, via the failure branch. +/// Not-found *classification* still inspects stderr internally; only the +/// user-facing string is redacted. +fn redact_stderr(stderr: &str) -> String { + let len = stderr.trim().len(); + format!( + "{len} bytes suppressed (may echo the stored config value); re-run the `fastly` command directly to inspect it" + ) +} + /// One-line type label for a `serde_json::Value` (for diagnostic /// error messages — not a canonical JSON-schema description). fn shape_summary(value: &serde_json::Value) -> &'static str { @@ -1390,6 +1942,7 @@ pub fn serve(extra_args: &[String]) -> Result<(), String> { mod tests { use super::*; use edgezero_adapter::cli_support::read_package_name; + use std::collections::HashSet; #[cfg(unix)] use std::ffi::OsString; #[cfg(unix)] @@ -1835,7 +2388,7 @@ mod tests { ("greeting".to_owned(), "hello".to_owned()), ("service.timeout_ms".to_owned(), "1500".to_owned()), ]; - write_fastly_local_config_store(&path, TEST_CONFIG_ID, &entries).expect("write"); + write_fastly_local_config_store(&path, TEST_CONFIG_ID, &entries, &[]).expect("write"); let after = fs::read_to_string(&path).expect("read back"); assert!( after.contains(&format!("[local_server.config_stores.{TEST_CONFIG_ID}]")), @@ -1868,12 +2421,14 @@ mod tests { &path, TEST_CONFIG_ID, &[("greeting".to_owned(), "stale".to_owned())], + &[], ) .expect("first write"); write_fastly_local_config_store( &path, TEST_CONFIG_ID, &[("greeting".to_owned(), "fresh".to_owned())], + &[], ) .expect("second write"); let after = fs::read_to_string(&path).expect("read back"); @@ -1903,6 +2458,7 @@ build = \"cargo build --release\" &path, TEST_CONFIG_ID, &[("greeting".to_owned(), "hi".to_owned())], + &[], ) .expect("write"); let after = fs::read_to_string(&path).expect("read back"); @@ -1932,6 +2488,7 @@ build = \"cargo build --release\" &path, TEST_CONFIG_ID, &[("greeting".to_owned(), "hi".to_owned())], + &[], ) .expect("write"); let after = fs::read_to_string(&path).expect("read back"); @@ -2229,9 +2786,9 @@ build = \"cargo build --release\" true, ) .expect("dry-run succeeds"); - // First line names the resolve+publish flow; subsequent lines preview - // each key the push would create (so callers can eyeball the keyset - // before running for real). + // First line names the resolve+publish flow; then one preview line per + // key. A push no longer reclaims anything (see `config gc`), so there is + // no GC-intent line. assert_eq!(out.len(), 1 + entries.len(), "header + per-entry preview"); assert!( out[0].contains("would resolve fastly config-store `app_config`") @@ -2367,6 +2924,7 @@ build = \"cargo build --release\" &path, TEST_CONFIG_ID, &[("greeting".to_owned(), envelope_json.clone())], + &[], ) .expect("setup write"); @@ -2755,35 +3313,353 @@ build = \"cargo build --release\" fake_dir } + /// Fake `fastly` for cloud chunk-GC tests. Logs each + /// `config-store-entry` op ("describe " / "update " / + /// "delete ", plus "delete-argv ") to `oplog`. + /// + /// `root_describe_seq` gives the successive raw `item_value`s returned when + /// the ROOT key is described (call 1 = the pre-commit prior read, call 2 = + /// the post-commit read-back). `entry_list` is served for + /// `config-store-entry list` and is what reclamation derives generations + /// and supersession times from. `fail_delete_key` makes that one delete + /// exit non-zero. `describe_hard_error` makes the FIRST describe of each key + /// fail hard (so the prior read errors while the read-back still works). #[cfg(unix)] - #[test] - fn push_config_entries_writes_direct_entry_at_exactly_8000_chars() { - use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; - let _lock = path_mutation_guard().lock().expect("guard"); + fn fake_fastly_gc( + root_key: &str, + root_describe_seq: &[String], + entry_list: &[(String, String, String)], + fail_delete_key: Option<&str>, + describe_hard_error: bool, + oplog: &Path, + ) -> tempfile::TempDir { + use std::os::unix::fs::PermissionsExt as _; + // Rendered with handlebars. Triple-stache `{{{ }}}` disables HTML + // escaping (paths are not markup); the shell's own `${var}` / + // `$(( ))` use single braces so they are literal text to handlebars. + const TEMPLATE: &str = r#"#!/bin/sh +if [ "$1" = "config-store" ]; then cat '{{{list}}}'; exit 0; fi +sub="$2" +key="" +for arg in "$@"; do case "$arg" in --key=*) key="${arg#--key=}";; esac; done +if [ "$sub" = "list" ]; then printf 'list\n' >> '{{{oplog}}}'; cat '{{{entries}}}'; exit 0; fi +if [ "$sub" = "update" ]; then cat >/dev/null; printf 'update %s\n' "$key" >> '{{{oplog}}}'; exit 0; fi +if [ "$sub" = "delete" ]; then printf 'delete %s\n' "$key" >> '{{{oplog}}}'; printf 'delete-argv %s\n' "$*" >> '{{{oplog}}}'; if [ "$key" = "{{{fail}}}" ]; then echo 'Error: boom' >&2; exit 1; fi; exit 0; fi +if [ "$sub" = "describe" ]; then + printf 'describe %s\n' "$key" >> '{{{oplog}}}' + cfile='{{{dir}}}/count_'"$key" + n=0; [ -f "$cfile" ] && n=$(cat "$cfile"); n=$((n+1)); printf '%s' "$n" > "$cfile" + {{#if hard_error}}if [ "$n" = "1" ]; then echo 'Error: internal server error' >&2; exit 1; fi{{/if}} + rf='{{{dir}}}/resp_'"$key"'_'"$n"'.json' + if [ -f "$rf" ]; then cat "$rf"; exit 0; fi + echo 'Error: item not found' >&2; exit 1 +fi +echo 'unexpected' >&2; exit 1 +"#; let dir = tempdir().expect("tempdir"); - let argv_log = dir.path().join("argv.txt"); - let fake = fake_fastly_argv_log(&argv_log); - let _path = PathPrepend::new(fake.path()); + let list_file = dir.path().join("list.json"); + fs::write( + &list_file, + format!(r#"[{{"name":"{TEST_CONFIG_ID}","id":"store-abc123"}}]"#), + ) + .expect("list"); + let entries_file = dir.path().join("entries.json"); + fs::write(&entries_file, entry_list_json(entry_list)).expect("entries"); + for (index, value) in root_describe_seq.iter().enumerate() { + let wrapped = format!( + r#"{{"item_value":{}}}"#, + serde_json::to_string(value).expect("escape") + ); + let nth = index.saturating_add(1); + fs::write( + dir.path().join(format!("resp_{root_key}_{nth}.json")), + wrapped, + ) + .expect("resp"); + } + let data = serde_json::json!({ + "list": list_file.display().to_string(), + "entries": entries_file.display().to_string(), + "oplog": oplog.display().to_string(), + "dir": dir.path().display().to_string(), + "fail": fail_delete_key.unwrap_or(""), + "hard_error": describe_hard_error, + }); + let script = handlebars::Handlebars::new() + .render_template(TEMPLATE, &data) + .expect("render fake fastly script"); + let script_path = dir.path().join("fastly"); + fs::write(&script_path, script).expect("script"); + let mut perms = fs::metadata(&script_path).expect("meta").permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script_path, perms).expect("chmod"); + dir + } - let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); - assert_eq!(envelope.len(), FASTLY_CONFIG_ENTRY_LIMIT); + /// A `config-store-entry list --json` payload. The item VALUE is a + /// placeholder: reclamation must only ever use keys and timestamps. + #[cfg(unix)] + fn entry_list_json(items: &[(String, String, String)]) -> String { + let entries: Vec = items + .iter() + .map(|(key, created, value)| { + serde_json::json!({ + "item_key": key, + "created_at": created, + "item_value": value, + }) + }) + .collect(); + serde_json::to_string(&entries).expect("entry list json") + } - let entries = vec![(TEST_CONFIG_ID.to_owned(), envelope)]; - let out = FastlyCliAdapter + /// An RFC-3339 stamp `secs` in the past (the shape Fastly returns). + #[cfg(unix)] + fn stamp_secs_ago(secs: u64) -> String { + let delta = chrono::Duration::seconds(i64::try_from(secs).unwrap_or(0)); + let now = chrono::Utc::now(); + now.checked_sub_signed(delta) + .unwrap_or(now) + .to_rfc3339_opts(chrono::SecondsFormat::Secs, true) + } + + /// Every chunk key of `envelope` as the listing would return it. + #[cfg(unix)] + fn listed_generation( + root_key: &str, + envelope: &str, + secs_ago: u64, + ) -> Vec<(String, String, String)> { + let (chunks, _) = chunked_parts(root_key, envelope); + let stamp = stamp_secs_ago(secs_ago); + chunks + .into_iter() + .map(|key| (key, stamp.clone(), "CHUNK-PAYLOAD".to_owned())) + .collect() + } + + /// The ROOT entry as the listing would return it: its value is the pointer, + /// which is how `config gc` learns which chunks are live. + #[cfg(unix)] + fn listed_root(root_key: &str, envelope: &str, secs_ago: u64) -> (String, String, String) { + let (_, pointer) = chunked_parts(root_key, envelope); + (root_key.to_owned(), stamp_secs_ago(secs_ago), pointer) + } + + /// A chunked envelope with a distinct payload per tag. + #[cfg(unix)] + fn gen_envelope(tag: &str) -> String { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + use edgezero_core::blob_envelope::BlobEnvelope; + use serde_json::json; + let data = json!({ tag: "x".repeat(FASTLY_CONFIG_ENTRY_LIMIT) }); + serde_json::to_string(&BlobEnvelope::new(data, "2026-06-22T00:00:00Z".to_owned())) + .expect("envelope") + } + + /// Split a chunked envelope into (chunk keys, root pointer value). + #[cfg(unix)] + fn chunked_parts(root_key: &str, envelope: &str) -> (Vec, String) { + let entries = prepare_fastly_config_entries(root_key, envelope).expect("expand"); + let (_, pointer) = entries.last().expect("pointer").clone(); + let chunk_keys = entries[..entries.len().saturating_sub(1)] + .iter() + .map(|(key, _)| key.clone()) + .collect(); + (chunk_keys, pointer) + } + + #[cfg(unix)] + fn oplog_has(oplog: &Path, line: &str) -> bool { + fs::read_to_string(oplog) + .unwrap_or_default() + .lines() + .any(|entry| entry == line) + } + + #[cfg(unix)] + #[test] + fn push_config_entries_rejects_reserved_key() { + let dir = tempdir().expect("tempdir"); + let bad_key = format!("app_config{CHUNK_KEY_INFIX}deadbeef.0"); + let err = FastlyCliAdapter .push_config_entries( dir.path(), None, None, &ResolvedStoreId::from_logical(TEST_CONFIG_ID), - &entries, + &[(bad_key.clone(), "{}".to_owned())], &AdapterPushContext::new(), false, ) - .expect("push must succeed"); - // One physical entry written (direct). - let captured = fs::read_to_string(&argv_log).expect("argv log"); - assert!( - captured.contains(&format!("--key={TEST_CONFIG_ID}")), + .expect_err("reserved key must be rejected"); + assert!(err.contains(&bad_key), "names the key: {err}"); + } + + /// Schema drift must never echo the config payload. App config can hold + /// credentials; CLI status lines are logged verbatim and CI logs are + /// retained/shared. Only a size + field-name shape may be reported. + #[cfg(unix)] + #[test] + fn read_config_entry_schema_drift_does_not_leak_payload() { + const SENTINEL: &str = "SUPER_SECRET_TOKEN_abc123"; + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + // Valid JSON, but the value moved out of `item_value` (schema drift). + let drift = format!(r#"{{"value_moved_here":"{SENTINEL}"}}"#); + let fake = fake_fastly_returning(&drift, "", 0); + let _path = PathPrepend::new(fake.path()); + + let result = FastlyCliAdapter.read_config_entry( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + "cfg", + &AdapterPushContext::new(), + ); + let Err(err) = result else { + panic!("schema drift must error") + }; + assert!( + !err.contains(SENTINEL), + "error must not leak the config payload: {err}" + ); + assert!( + err.contains("bytes"), + "error should carry a redacted size/shape summary: {err}" + ); + } + + /// The FAILURE branch leaks too: a Fastly error that quotes the stored + /// value back in stderr must not reach the user-facing error. + #[cfg(unix)] + #[test] + fn read_config_entry_stderr_failure_does_not_leak_payload() { + const SENTINEL: &str = "SUPER_SECRET_TOKEN_stderr1"; + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + // Not a "not found" — a hard failure that echoes the value. + let stderr = format!("Error: internal failure processing value {SENTINEL}"); + let fake = fake_fastly_returning("", &stderr, 1); + let _path = PathPrepend::new(fake.path()); + + let result = FastlyCliAdapter.read_config_entry( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + "cfg", + &AdapterPushContext::new(), + ); + let Err(err) = result else { + panic!("hard stderr failure must error") + }; + assert!( + !err.contains(SENTINEL), + "stderr must be redacted, not echoed: {err}" + ); + assert!( + err.contains("suppressed"), + "error should say the stderr was suppressed: {err}" + ); + } + + /// Same for the push path's GC prior-read, which runs on every cloud push. + #[cfg(unix)] + #[test] + fn push_config_entries_stderr_failure_does_not_leak_payload() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + const SENTINEL: &str = "SUPER_SECRET_TOKEN_stderr2"; + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let stderr = format!("Error: internal failure processing value {SENTINEL}"); + let fake = fake_fastly_returning("", &stderr, 1); + let _path = PathPrepend::new(fake.path()); + + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let result = FastlyCliAdapter.push_config_entries( + dir.path(), + None, + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope)], + &AdapterPushContext::new(), + false, + ); + // Whether it errors or warns, the payload must never appear. + let rendered = match result { + Ok(lines) => lines.join("\n"), + Err(err) => err, + }; + assert!( + !rendered.contains(SENTINEL), + "push output must not echo the stored value from stderr: {rendered}" + ); + } + + /// The GC prior-read runs on EVERY cloud push, so a drifted response must + /// not leak the previous config envelope into the push's status lines. + #[cfg(unix)] + #[test] + fn push_config_entries_prior_read_drift_does_not_leak_payload() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + const SENTINEL: &str = "SUPER_SECRET_TOKEN_xyz789"; + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let drift = format!(r#"{{"value_moved_here":"{SENTINEL}"}}"#); + let fake = fake_fastly_returning(&drift, "", 0); + let _path = PathPrepend::new(fake.path()); + + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let out = FastlyCliAdapter + .push_config_entries( + dir.path(), + None, + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope)], + &AdapterPushContext::new(), + false, + ) + .expect("push still succeeds; the prior read only warns"); + for line in &out { + assert!( + !line.contains(SENTINEL), + "push status must not leak the prior config payload: {line}" + ); + } + } + + #[cfg(unix)] + #[test] + fn push_config_entries_writes_direct_entry_at_exactly_8000_chars() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let argv_log = dir.path().join("argv.txt"); + let fake = fake_fastly_argv_log(&argv_log); + let _path = PathPrepend::new(fake.path()); + + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + assert_eq!(envelope.len(), FASTLY_CONFIG_ENTRY_LIMIT); + + let entries = vec![(TEST_CONFIG_ID.to_owned(), envelope)]; + let out = FastlyCliAdapter + .push_config_entries( + dir.path(), + None, + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &entries, + &AdapterPushContext::new(), + false, + ) + .expect("push must succeed"); + // One physical entry written (direct). + let captured = fs::read_to_string(&argv_log).expect("argv log"); + assert!( + captured.contains(&format!("--key={TEST_CONFIG_ID}")), "must write root key directly: {captured}" ); assert!( @@ -3244,6 +4120,7 @@ build = \"cargo build --release\" &fastly_toml, TEST_CONFIG_ID, &[("cfg".to_owned(), json_str.clone())], + &[], ) .expect("write"); @@ -3272,7 +4149,8 @@ build = \"cargo build --release\" let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); let physical = prepare_fastly_config_entries(TEST_CONFIG_ID, &envelope).unwrap(); // Write all physical entries (chunks + pointer) to the local store. - write_fastly_local_config_store(&fastly_toml, TEST_CONFIG_ID, &physical).expect("write"); + write_fastly_local_config_store(&fastly_toml, TEST_CONFIG_ID, &physical, &[]) + .expect("write"); let result = FastlyCliAdapter .read_config_entry_local( @@ -3300,14 +4178,11 @@ build = \"cargo build --release\" /// /// The local fastly.toml writer upserts per-key (so a sibling /// `--key app_config_staging` push leaves `app_config` intact per - /// spec 12.7). Within the SAME root key, old chunks for envelope - /// A remain in the contents table after envelope B's push — they're - /// unreferenced (the root pointer at `app_config` now names B's - /// chunks), matching the remote Fastly behaviour where the - /// per-entry `update --upsert` shell-out has no atomic-delete - /// pairing. The runtime-correctness property holds either way: a - /// read after push B follows the active pointer and reconstructs - /// envelope B, not A. + /// spec 12.7). Within the SAME root key, GC on re-push prunes the + /// prior generation: after envelope B's push, envelope A's chunks — + /// now unreferenced by the `app_config` pointer — are removed from + /// the contents table. A read after push B follows the active + /// pointer and reconstructs envelope B, not A. #[cfg(unix)] #[test] #[expect( @@ -3321,8 +4196,7 @@ build = \"cargo build --release\" fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); // First push: envelope A. Records the chunk-key set so we can - // confirm they survive the second push (no garbage collection - // in v1 — spec 9.3 + Q6). + // confirm they are pruned by the second push's GC. let envelope_a = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); FastlyCliAdapter .push_config_entries_local( @@ -3356,10 +4230,10 @@ build = \"cargo build --release\" ); // Second push: a DIFFERENT oversized envelope B. The - // content-addressed chunk keys must shift to B's sha; old - // A-chunks may remain in the table (v1 doesn't GC). Build - // envelope B with a distinct payload key so its SHA differs - // from A's even at the same total length. + // content-addressed chunk keys must shift to B's sha; GC then + // prunes the old A-chunks. Build envelope B with a distinct + // payload key so its SHA differs from A's even at the same + // total length. let envelope_b = { use edgezero_core::blob_envelope::BlobEnvelope; use serde_json::json; @@ -3401,8 +4275,7 @@ build = \"cargo build --release\" // Chunk keys are content-addressed by envelope SHA, so the B // push installs a fresh chunk-set whose keys are all distinct - // from A's. Under the upsert semantic the A-chunks remain in - // the contents table (no GC in v1); B's chunks are simply added. + // from A's. GC on re-push prunes the now-unreferenced A-chunks. let new_b_chunks: Vec<&String> = chunks_b .iter() .filter(|key| !chunks_a.contains(*key)) @@ -3411,12 +4284,12 @@ build = \"cargo build --release\" !new_b_chunks.is_empty(), "push B must have added at least one new content-addressed chunk: A-set={chunks_a:?} B-set={chunks_b:?}" ); - // Old A-chunks remain in the table (orphan-but-present — - // matches the remote Fastly write-only-upsert semantic). + // Old A-chunks are pruned: GC deletes the prior generation the + // old pointer referenced once B's pointer supersedes it. for chunk_key in &chunks_a { assert!( - chunks_b.contains(chunk_key), - "old A-chunk `{chunk_key}` must remain in the local table after push B (v1 has no GC); B-set={chunks_b:?}" + !chunks_b.contains(chunk_key), + "old A-chunk `{chunk_key}` must be pruned from the local table after push B; B-set={chunks_b:?}" ); } @@ -3444,4 +4317,736 @@ build = \"cargo build --release\" "old envelope A's chunks must be inert -- read must NOT return A" ); } + + // ---------- config gc (operator-invoked reclamation) ---------- + + #[cfg(unix)] + fn run_gc(dir: &Path, older_than_secs: u64, dry_run: bool) -> Result, String> { + FastlyCliAdapter.gc_config_entries( + dir, + None, + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &AdapterPushContext::new(), + older_than_secs, + dry_run, + ) + } + + /// gc never deletes a chunk the LIVE root pointer references, however old. + #[cfg(unix)] + #[test] + fn gc_never_deletes_live_chunks() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let live = gen_envelope("live"); + let (live_chunks, _) = chunked_parts(TEST_CONFIG_ID, &live); + // The live generation is ANCIENT, but it is referenced by the root. + let mut listing = vec![listed_root(TEST_CONFIG_ID, &live, 999_999)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &live, 999_999)); + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + let out = run_gc(dir.path(), 0, false).expect("gc succeeds"); + let log = fs::read_to_string(&oplog).unwrap_or_default(); + for key in &live_chunks { + assert!( + !oplog_has(&oplog, &format!("delete {key}")), + "live chunk `{key}` must never be reclaimed; log:\n{log}\nout: {out:?}" + ); + } + } + + /// gc reclaims unreferenced chunks older than the operator's threshold. + #[cfg(unix)] + #[test] + fn gc_reclaims_unreferenced_chunks_older_than_threshold() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let live = gen_envelope("live"); + let dead = gen_envelope("dead"); + let (live_chunks, _) = chunked_parts(TEST_CONFIG_ID, &live); + let (dead_chunks, _) = chunked_parts(TEST_CONFIG_ID, &dead); + + let mut listing = vec![listed_root(TEST_CONFIG_ID, &live, 3_600)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &live, 3_600)); + listing.extend(listed_generation(TEST_CONFIG_ID, &dead, 604_800)); // a week old + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + // Operator asserts: nothing older than 1 day is still being served. + let out = run_gc(dir.path(), 86_400, false).expect("gc succeeds"); + for key in &dead_chunks { + assert!( + oplog_has(&oplog, &format!("delete {key}")), + "orphan `{key}` older than the threshold must be reclaimed; out: {out:?}" + ); + } + for key in &live_chunks { + assert!( + !oplog_has(&oplog, &format!("delete {key}")), + "live chunk `{key}` must survive" + ); + } + } + + /// Orphans younger than the operator's threshold are retained. + #[cfg(unix)] + #[test] + fn gc_retains_orphans_younger_than_threshold() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let live = gen_envelope("live"); + let recent = gen_envelope("recent"); + let (recent_chunks, _) = chunked_parts(TEST_CONFIG_ID, &recent); + + let mut listing = vec![listed_root(TEST_CONFIG_ID, &live, 60)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &live, 60)); + listing.extend(listed_generation(TEST_CONFIG_ID, &recent, 120)); // 2 min old + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + run_gc(dir.path(), 86_400, false).expect("gc succeeds"); + let log = fs::read_to_string(&oplog).unwrap_or_default(); + for key in &recent_chunks { + assert!( + !oplog_has(&oplog, &format!("delete {key}")), + "orphan `{key}` is younger than the threshold and must be retained; log:\n{log}" + ); + } + } + + /// A dry-run lists exactly what it would delete, and deletes nothing. + #[cfg(unix)] + #[test] + fn gc_dry_run_lists_but_deletes_nothing() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let live = gen_envelope("live"); + let dead = gen_envelope("dead"); + let (dead_chunks, _) = chunked_parts(TEST_CONFIG_ID, &dead); + + let mut listing = vec![listed_root(TEST_CONFIG_ID, &live, 3_600)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &live, 3_600)); + listing.extend(listed_generation(TEST_CONFIG_ID, &dead, 604_800)); + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + let out = run_gc(dir.path(), 86_400, true).expect("dry-run succeeds"); + let log = fs::read_to_string(&oplog).unwrap_or_default(); + assert!( + !log.lines().any(|line| line.starts_with("delete ")), + "a dry-run must not delete; log:\n{log}" + ); + let rendered = out.join("\n"); + assert!( + rendered.contains("would delete"), + "lists intent: {rendered}" + ); + for key in &dead_chunks { + assert!(rendered.contains(key.as_str()), "names `{key}`: {rendered}"); + } + } + + /// An unreadable `created_at` on a DELETE path fails CLOSED. + #[cfg(unix)] + #[test] + fn gc_fails_closed_on_unreadable_timestamp() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let live = gen_envelope("live"); + let dead = gen_envelope("dead"); + let mut listing = vec![listed_root(TEST_CONFIG_ID, &live, 3_600)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &live, 3_600)); + // An orphan whose timestamp is garbage. + let (dead_chunks, _) = chunked_parts(TEST_CONFIG_ID, &dead); + for key in dead_chunks { + listing.push((key, "not-a-timestamp".to_owned(), "X".to_owned())); + } + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + let err = run_gc(dir.path(), 86_400, false).expect_err("must fail closed"); + assert!( + err.contains("unreadable") && err.contains("nothing was deleted"), + "must refuse to reclaim: {err}" + ); + let log = fs::read_to_string(&oplog).unwrap_or_default(); + assert!( + !log.lines().any(|line| line.starts_with("delete ")), + "nothing may be deleted when the state is unreadable; log:\n{log}" + ); + } + + /// A root whose pointer cannot be classified fails CLOSED — we cannot know + /// what it references, so nothing may be deleted. + #[cfg(unix)] + #[test] + fn gc_fails_closed_on_unclassifiable_root() { + let _lock = path_mutation_guard().lock().expect("guard"); + let dir = tempdir().expect("tempdir"); + let oplog = dir.path().join("ops.log"); + + let dead = gen_envelope("dead"); + // Root value is pointer-kind but invalid. + let bad = r#"{"edgezero_kind":"fastly_config_chunks","version":2}"#.to_owned(); + let mut listing = vec![(TEST_CONFIG_ID.to_owned(), stamp_secs_ago(3_600), bad)]; + listing.extend(listed_generation(TEST_CONFIG_ID, &dead, 604_800)); + + let fake = fake_fastly_gc(TEST_CONFIG_ID, &[], &listing, None, false, &oplog); + let _path = PathPrepend::new(fake.path()); + + let err = run_gc(dir.path(), 86_400, false).expect_err("must fail closed"); + assert!( + err.contains("could not classify root") && err.contains("nothing was deleted"), + "must refuse to reclaim: {err}" + ); + let log = fs::read_to_string(&oplog).unwrap_or_default(); + assert!( + !log.lines().any(|line| line.starts_with("delete ")), + "nothing may be deleted when a root is unclassifiable; log:\n{log}" + ); + } + + // ---------- local chunk GC ---------- + + /// Config shrinks from chunked back under the 8 000-char limit: the + /// new value is a direct envelope, so GC prunes every prior chunk. + #[cfg(unix)] + #[test] + fn push_config_entries_local_prunes_prior_chunks_when_value_shrinks_to_direct() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let chunked = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), chunked)], + &AdapterPushContext::new(), + false, + ) + .expect("first push"); + + let direct = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), direct.clone())], + &AdapterPushContext::new(), + false, + ) + .expect("second push"); + + let after = fs::read_to_string(&fastly_toml).expect("read"); + let doc: toml_edit::DocumentMut = after.parse().expect("parse"); + let contents = doc + .get("local_server") + .and_then(|ls| ls.get("config_stores")) + .and_then(|cs| cs.get(TEST_CONFIG_ID)) + .and_then(|st| st.get("contents")) + .and_then(toml_edit::Item::as_table) + .expect("contents"); + + assert_eq!( + contents + .get(TEST_CONFIG_ID) + .and_then(toml_edit::Item::as_str), + Some(direct.as_str()), + "root holds the direct envelope" + ); + assert!( + !contents + .iter() + .any(|(key, _)| key.contains(CHUNK_KEY_INFIX)), + "prior chunks must be pruned: {after}" + ); + } + + /// A logical key containing the reserved chunk infix is rejected + /// before any file I/O (it would collide with the chunk namespace). + #[cfg(unix)] + #[test] + fn push_config_entries_local_rejects_reserved_key() { + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + let bad_key = format!("app_config{CHUNK_KEY_INFIX}deadbeef.0"); + + let err = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(bad_key.clone(), "{}".to_owned())], + &AdapterPushContext::new(), + false, + ) + .expect_err("reserved key must be rejected"); + assert!(err.contains(&bad_key), "error names the key: {err}"); + assert!( + !fastly_toml.exists(), + "rejection must happen before any write" + ); + } + + /// A suspicious prior pointer (pointer-kind but invalid) makes GC + /// warn and delete nothing — pre-seeded chunk keys must survive. + #[cfg(unix)] + #[test] + fn push_config_entries_local_warns_on_suspicious_prior_pointer_and_keeps_chunks() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + // Seed the root with a pointer-kind-but-invalid value AND a real + // chunk-like key so "no deletes" is non-vacuous. + let seed = concat!( + "name = \"demo\"\n\n", + "[local_server.config_stores.app_config]\n", + "format = \"inline-toml\"\n\n", + "[local_server.config_stores.app_config.contents]\n", + "app_config = \"{\\\"edgezero_kind\\\":\\\"fastly_config_chunks\\\",\\\"version\\\":2}\"\n", + "\"app_config.__edgezero_chunks.deadbeef.0\" = \"seeded-chunk-payload\"\n", + ); + fs::write(&fastly_toml, seed).expect("seed"); + + let direct = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + let out = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), direct.clone())], + &AdapterPushContext::new(), + false, + ) + .expect("push must still succeed"); + + let combined = out.join("\n"); + assert!( + combined.contains("skipping chunk GC"), + "must warn about the suspicious prior pointer: {combined}" + ); + + let after = fs::read_to_string(&fastly_toml).expect("read"); + let doc: toml_edit::DocumentMut = after.parse().expect("parse"); + let contents = doc + .get("local_server") + .and_then(|ls| ls.get("config_stores")) + .and_then(|cs| cs.get(TEST_CONFIG_ID)) + .and_then(|st| st.get("contents")) + .and_then(toml_edit::Item::as_table) + .expect("contents"); + assert!( + contents + .get("app_config.__edgezero_chunks.deadbeef.0") + .is_some(), + "pre-seeded chunk key must survive a suspicious-pointer skip: {after}" + ); + assert_eq!( + contents + .get(TEST_CONFIG_ID) + .and_then(toml_edit::Item::as_str), + Some(direct.as_str()), + "new value still written" + ); + } + + /// Dry-run reports the orphan count and writes nothing. + #[cfg(unix)] + #[test] + fn push_config_entries_local_dry_run_reports_orphan_count() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let envelope_a = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope_a)], + &AdapterPushContext::new(), + false, + ) + .expect("seed push"); + let before = fs::read_to_string(&fastly_toml).expect("read"); + + let envelope_b = { + use edgezero_core::blob_envelope::BlobEnvelope; + use serde_json::json; + let data = json!({ "alt": "y".repeat(FASTLY_CONFIG_ENTRY_LIMIT) }); + serde_json::to_string(&BlobEnvelope::new(data, "2026-06-22T00:00:02Z".to_owned())) + .expect("envelope B") + }; + let out = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope_b)], + &AdapterPushContext::new(), + true, // dry_run + ) + .expect("dry-run"); + + let combined = out.join("\n"); + assert!( + combined.contains("would delete") && combined.contains("orphan chunks"), + "dry-run must report orphan count: {combined}" + ); + assert_eq!( + fs::read_to_string(&fastly_toml).expect("read"), + before, + "dry-run must not edit fastly.toml" + ); + } + + /// Dry-run of an identical re-push reports zero orphans (new keys + /// equal prior keys — regression for expanding `new_keys`). + #[cfg(unix)] + #[test] + fn push_config_entries_local_dry_run_identical_repush_counts_zero() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope.clone())], + &AdapterPushContext::new(), + false, + ) + .expect("seed push"); + + let out = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope)], + &AdapterPushContext::new(), + true, // dry_run, same bytes + ) + .expect("dry-run"); + + assert!( + out.join("\n").contains("would delete 0 orphan chunks"), + "identical re-push must count 0 orphans: {out:?}" + ); + } + + /// Dry-run over a suspicious prior pointer reports an unknown count + /// and does not fail. + #[cfg(unix)] + #[test] + fn push_config_entries_local_dry_run_suspicious_prior_pointer_unknown() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + let seed = concat!( + "name = \"demo\"\n\n", + "[local_server.config_stores.app_config]\n", + "format = \"inline-toml\"\n\n", + "[local_server.config_stores.app_config.contents]\n", + "app_config = \"{\\\"edgezero_kind\\\":\\\"fastly_config_chunks\\\",\\\"version\\\":2}\"\n", + ); + fs::write(&fastly_toml, seed).expect("seed"); + + let direct = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + let out = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), direct)], + &AdapterPushContext::new(), + true, // dry_run + ) + .expect("dry-run must not fail on suspicious pointer"); + + assert!( + out.join("\n").contains("unknown: suspicious prior pointer"), + "dry-run must degrade to unknown: {out:?}" + ); + } + + /// A present-but-malformed `contents` (non-table) is prior state the + /// real writer would reject — the dry-run count must degrade to + /// `unknown: could not read prior state`, not silently report 0. + #[cfg(unix)] + #[test] + fn push_config_entries_local_dry_run_non_table_contents_unknown() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + let seed = concat!( + "name = \"demo\"\n\n", + "[local_server.config_stores.app_config]\n", + "format = \"inline-toml\"\n", + "contents = \"bad\"\n", + ); + fs::write(&fastly_toml, seed).expect("seed"); + + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let out = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope)], + &AdapterPushContext::new(), + true, // dry_run + ) + .expect("dry-run must not fail on malformed contents"); + + assert!( + out.join("\n") + .contains("unknown: could not read prior state"), + "non-table contents must degrade to unknown: {out:?}" + ); + } + + /// A duplicate root key in one batch is rejected before any I/O. + /// Otherwise the earlier tuple's GC plan would reclaim the chunks the + /// LAST tuple just installed, leaving the final pointer dangling. + /// Regression: prior B, batch `[(root, A), (root, B)]` — the root must + /// still resolve afterwards. + #[cfg(unix)] + #[test] + fn push_config_entries_local_rejects_duplicate_root_keys() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let make = |tag: &str| { + use edgezero_core::blob_envelope::BlobEnvelope; + use serde_json::json; + let data = json!({ tag: "x".repeat(FASTLY_CONFIG_ENTRY_LIMIT) }); + serde_json::to_string(&BlobEnvelope::new(data, "2026-06-22T00:00:00Z".to_owned())) + .expect("envelope") + }; + let envelope_a = make("aaa"); + let envelope_b = make("bbb"); + + // Prior generation B is live. + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), envelope_b.clone())], + &AdapterPushContext::new(), + false, + ) + .expect("seed push"); + let before = fs::read_to_string(&fastly_toml).expect("read"); + + // Duplicate-root batch must be rejected outright. + let err = FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[ + (TEST_CONFIG_ID.to_owned(), envelope_a), + (TEST_CONFIG_ID.to_owned(), envelope_b.clone()), + ], + &AdapterPushContext::new(), + false, + ) + .expect_err("duplicate root keys must be rejected"); + assert!( + err.contains("more than once"), + "error explains the duplicate: {err}" + ); + assert_eq!( + fs::read_to_string(&fastly_toml).expect("read"), + before, + "rejection must happen before any write" + ); + + // The live root still resolves to B (nothing was reclaimed). + let read = FastlyCliAdapter + .read_config_entry_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + TEST_CONFIG_ID, + &AdapterPushContext::new(), + ) + .expect("root must still resolve"); + let ReadConfigEntry::Present(value) = read else { + panic!("expected Present"); + }; + assert_eq!(value, envelope_b, "root still reconstructs envelope B"); + } + + /// GC of a chunked root must not touch a chunked SIBLING's chunks — + /// the prefix `app_config.__edgezero_chunks.` must not match + /// `app_config_staging.__edgezero_chunks.` (shared string prefix). + #[cfg(unix)] + #[test] + fn push_config_entries_local_gc_preserves_sibling_chunks() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let make = |tag: &str| { + use edgezero_core::blob_envelope::BlobEnvelope; + use serde_json::json; + let data = json!({ tag: "x".repeat(FASTLY_CONFIG_ENTRY_LIMIT) }); + serde_json::to_string(&BlobEnvelope::new(data, "2026-06-22T00:00:00Z".to_owned())) + .expect("envelope") + }; + let push = |key: &str, body: String| { + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(key.to_owned(), body)], + &AdapterPushContext::new(), + false, + ) + .expect("push"); + }; + + // app_config gen X, then a chunked sibling, then app_config gen Z. + push("app_config", make("x1")); + push("app_config_staging", make("staging")); + let staging_chunks = { + let (chunks, _) = chunked_parts("app_config_staging", &make("staging")); + chunks + }; + push("app_config", make("z2")); // GCs app_config's gen-X chunks + + let after = fs::read_to_string(&fastly_toml).expect("read"); + let doc: toml_edit::DocumentMut = after.parse().expect("parse"); + let contents = doc + .get("local_server") + .and_then(|ls| ls.get("config_stores")) + .and_then(|cs| cs.get(TEST_CONFIG_ID)) + .and_then(|st| st.get("contents")) + .and_then(toml_edit::Item::as_table) + .expect("contents"); + for key in &staging_chunks { + assert!( + contents.get(key).is_some(), + "sibling chunk `{key}` must survive app_config GC: {after}" + ); + } + } + + // ---- chunk GC helpers ---- + + #[test] + fn reject_reserved_root_keys_accepts_clean_keys() { + let entries = vec![ + ("app_config".to_owned(), "{}".to_owned()), + ("app_config_staging".to_owned(), "{}".to_owned()), + ]; + reject_reserved_root_keys(&entries).expect("clean keys accepted"); + } + + #[test] + fn reject_reserved_root_keys_rejects_infix_key() { + let bad = format!("app_config{CHUNK_KEY_INFIX}deadbeef.0"); + let entries = vec![(bad.clone(), "{}".to_owned())]; + let err = reject_reserved_root_keys(&entries).expect_err("reserved infix must reject"); + assert!(err.contains(&bad), "error names the key: {err}"); + assert!(err.contains("reserved"), "error explains why: {err}"); + } + + #[test] + fn orphan_chunk_keys_subtracts_new_keys() { + let mut new_keys = HashSet::new(); + new_keys.insert("keep".to_owned()); + let plan = FastlyConfigGcPlan { + new_keys, + prior_keys: Ok(vec![ + "gone1".to_owned(), + "keep".to_owned(), + "gone2".to_owned(), + ]), + }; + let orphans = orphan_chunk_keys(&plan).expect("ok"); + assert_eq!(orphans, vec!["gone1".to_owned(), "gone2".to_owned()]); + } + + #[test] + fn orphan_chunk_keys_propagates_prior_err() { + let plan = FastlyConfigGcPlan { + new_keys: HashSet::new(), + prior_keys: Err("suspicious".to_owned()), + }; + orphan_chunk_keys(&plan).unwrap_err(); + } + + #[test] + fn expand_root_direct_value_has_single_entry() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + let (expanded, new_keys, new_root_value) = expand_root(TEST_CONFIG_ID, &envelope).unwrap(); + assert_eq!(expanded.len(), 1); + assert_eq!(new_root_value, envelope); + assert!(new_keys.contains(TEST_CONFIG_ID)); + assert_eq!(new_keys.len(), 1); + } + + #[test] + fn expand_root_chunked_value_carries_pointer_as_root_value() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let envelope = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let (expanded, new_keys, new_root_value) = expand_root(TEST_CONFIG_ID, &envelope).unwrap(); + assert!(expanded.len() >= 2, "chunks + pointer"); + let (last_key, last_value) = expanded.last().unwrap(); + assert_eq!(last_key, TEST_CONFIG_ID); + assert_eq!(&new_root_value, last_value); + assert!(new_keys.contains(TEST_CONFIG_ID)); + assert_eq!(new_keys.len(), expanded.len()); + } } diff --git a/crates/edgezero-adapter/src/registry.rs b/crates/edgezero-adapter/src/registry.rs index fcc5bfa4..28d734af 100644 --- a/crates/edgezero-adapter/src/registry.rs +++ b/crates/edgezero-adapter/src/registry.rs @@ -239,6 +239,43 @@ pub trait Adapter: Sync + Send { /// Returns an error string if the requested adapter action fails. fn execute(&self, action: AdapterAction, args: &[String]) -> Result<(), String>; + /// Reclaim chunk entries that no LIVE config pointer references. + /// + /// Deliberately NOT part of `config push`. On an eventually-consistent + /// store, a chunk may only be deleted once the pointer that referenced it + /// has stopped being served everywhere — and the platform may record no + /// pointer-supersession time (Fastly does not: `updated_at` is not bumped + /// by an upsert) and offer no compare-and-swap with which to record one + /// safely. Only the OPERATOR knows their deploy history, so `older_than` + /// carries that assertion: "nothing created before this is still being + /// served". A `dry_run` lists exactly what would be deleted, for review. + /// + /// # Errors + /// + /// Returns `Err` if the adapter has no `config gc` impl, if the platform + /// state cannot be read, or if it cannot be classified with confidence — + /// reclamation FAILS CLOSED: when in doubt, nothing is deleted. + #[inline] + #[expect( + clippy::too_many_arguments, + reason = "mirrors `push_config_entries`: manifest root, adapter manifest path, component selector, resolved store, push-time overlay, the operator's age assertion, and dry-run — each distinct; an aggregate struct is a worse ergonomic trade for implementers." + )] + fn gc_config_entries( + &self, + _manifest_root: &Path, + _adapter_manifest_path: Option<&str>, + _component_selector: Option<&str>, + _store: &ResolvedStoreId, + _push_ctx: &AdapterPushContext<'_>, + _older_than_secs: u64, + _dry_run: bool, + ) -> Result, String> { + Err(format!( + "adapter `{}` does not implement `config gc`", + self.name() + )) + } + /// Store kinds whose logical-id namespaces the adapter merges into /// a single backend at runtime — declaring the SAME logical id /// under two merged kinds causes silent write collisions because diff --git a/docs/superpowers/plans/2026-07-07-fastly-chunk-gc.md b/docs/superpowers/plans/2026-07-07-fastly-chunk-gc.md new file mode 100644 index 00000000..52638a6f --- /dev/null +++ b/docs/superpowers/plans/2026-07-07-fastly-chunk-gc.md @@ -0,0 +1,965 @@ +# Fastly Chunk GC Implementation Plan + +> ## ⚠️ SUPERSEDED — historical build guide, NOT the contract +> +> This plan was written against the ORIGINAL design, in which a cloud push +> **eagerly deleted** the generation it had just superseded. +> +> **Three automatic cloud designs were built and each was demolished in review:** +> +> 1. **eager delete** — unsafe: the store is eventually consistent, so POPs may +> still serve the previous pointer, and the read-back only sees the control plane; +> 2. **metadata sidecar** — unsound: no compare-and-swap ⇒ lost updates, plus an +> 8 000-char overflow at ~71 generations; +> 3. **store-derived clock** — unsound: chunk `created_at` is not a pointer +> transition (chunked → direct → direct leaves the old generation with no +> "successor" at all). +> +> **Conclusion: safe AUTOMATIC cloud reclamation is not achievable with Fastly's +> primitives.** The needed fact — "the pointer that referenced these chunks has +> stopped being served everywhere" — is one Fastly does not record and we cannot +> safely synthesise. Only the operator knows it. +> +> **What shipped:** a cloud push reclaims **nothing**; the local path prunes +> eagerly (safe — one file, no POPs); and reclamation is an explicit, operator- +> invoked **`config gc --older-than `** that fails closed and offers a +> reviewable dry-run. +> +> **The authoritative contract is the spec:** +> `docs/superpowers/specs/2026-07-07-fastly-chunk-gc.md`. +> +> Everything below is retained only as a record of how the work was sequenced and +> of the designs that were tried and discarded. Do not implement from it. + + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Reclaim obsolete Fastly chunked config-store entries after a successful re-push without changing push success semantics. + +**Architecture:** Add a root-scoped prior-pointer inspector in `chunked_config.rs`, then wire cloud and local Fastly push paths in `cli.rs` to snapshot prior chunk keys before writing and sweep them only after the new root entry is committed. Cloud GC is best-effort and warning-only; local GC happens inside the same `DocumentMut` rewrite used for `fastly.toml`. + +**Tech Stack:** Rust 2021, `serde_json`, `toml_edit`, existing Fastly CLI shell-out helpers, existing fake-`fastly` test shims, `cargo test -p edgezero-adapter-fastly --features cli`. + +**Concurrency model — last-writer-wins:** Concurrent cloud pushes are **supported**. The root pointer is `update --upsert`, so the last write wins on the config value. GC obeys LWW via the post-commit read-back guard: a push reclaims prior chunks only while it is still the last writer of the root (read-back == what it wrote), otherwise it **yields** and deletes nothing — so a superseded push never deletes the winner's live chunks. The guard narrows but does not fully close the window (Fastly has no compare-and-delete), making GC best-effort under LWW, not transactional. No blocking precondition. See the spec's "Concurrency model: last-writer-wins". + +--- + +## File Structure + +- Modify `crates/edgezero-adapter-fastly/src/chunked_config.rs` + - Add `prior_chunk_keys(root_key, raw) -> Result, String>` next to `FastlyChunkPointer`. + - Add focused unit tests in the existing `#[cfg(test)]` module. + +- Modify `crates/edgezero-adapter-fastly/src/cli.rs` + - Import `prior_chunk_keys` and `CHUNK_KEY_INFIX`. + - Add small private GC helpers (`FastlyConfigGcPlan`, `expand_root`, `orphan_chunk_keys`, `reject_reserved_root_keys`) near the config-push helpers. Per-root keep-sets come from `expand_root` (that root's own expansion), never prefix-scanned from the flattened physical set. + - Extend `push_config_entries` with offline dry-run intent, prior root read, post-commit cloud sweep (only after the whole flattened commit returns `Ok`), and warning status lines. + - Add `delete_config_store_entry(store_id, key)`. + - Extend `push_config_entries_local` dry-run with best-effort offline orphan counts (degrade to `unknown`, never newly fail). + - Extend `write_fastly_local_config_store` with a `gc_roots: &[(String, HashSet)]` param (exact per-root keep-sets; empty ⇒ no GC); prune prior root chunks in the same in-memory rewrite before the (non-atomic) `fs::write` and return warning lines from suspicious prior pointers. + - Add/adjust tests in the existing `cli.rs` test module. + +**Mandatory** reserved-key rejection: both `push_config_entries` and +`push_config_entries_local` call `reject_reserved_root_keys(entries)?` +immediately after the empty-entries check. A logical key containing +`.__edgezero_chunks.` is rejected with an error (it collides with the +generated chunk namespace). This lives at the Fastly adapter boundary — +not in generic `edgezero-cli` — because the infix is a Fastly concept. +It is a hard error, not a warning: there is no valid reason for such a +key, and allowing it would let a push write into another key's chunk +namespace. + +--- + +### Task 1: Add Root-Scoped Prior Pointer Inspection + +**Files:** +- Modify: `crates/edgezero-adapter-fastly/src/chunked_config.rs:47` +- Test: `crates/edgezero-adapter-fastly/src/chunked_config.rs:309` + +- [ ] **Step 1: Add failing tests for `prior_chunk_keys`** + +Add tests near the existing chunked-config unit tests: + +```rust +#[test] +fn prior_chunk_keys_returns_valid_v1_pointer_keys() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + + let keys = prior_chunk_keys("app_config", pointer_json).expect("valid pointer"); + + let expected: Vec = entries[..entries.len().saturating_sub(1)] + .iter() + .map(|(key, _)| key.clone()) + .collect(); + assert_eq!(keys, expected); +} + +#[test] +fn prior_chunk_keys_returns_empty_for_direct_envelope() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT); + assert_eq!(prior_chunk_keys("app_config", &envelope).unwrap(), Vec::::new()); +} + +#[test] +fn prior_chunk_keys_returns_empty_for_unrelated_json() { + assert_eq!( + prior_chunk_keys("app_config", r#"{"hello":"world"}"#).unwrap(), + Vec::::new() + ); +} + +#[test] +fn prior_chunk_keys_returns_empty_for_wrong_kind() { + let raw = r#"{"edgezero_kind":"other","version":1,"chunks":[],"data_sha256":"","envelope_len":0,"envelope_sha256":""}"#; + assert_eq!(prior_chunk_keys("app_config", raw).unwrap(), Vec::::new()); +} + +#[test] +fn prior_chunk_keys_rejects_unsupported_pointer_version() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + let mut pointer: FastlyChunkPointer = serde_json::from_str(pointer_json).unwrap(); + pointer.version = 2; + let raw = serde_json::to_string(&pointer).unwrap(); + + let err = prior_chunk_keys("app_config", &raw).expect_err("version 2 should warn"); + assert!(err.contains("unsupported version"), "{err}"); +} + +#[test] +fn prior_chunk_keys_rejects_foreign_chunk_prefix() { + let envelope = make_envelope_json(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + let entries = prepare_fastly_config_entries("app_config", &envelope).unwrap(); + let (_, pointer_json) = entries.last().unwrap(); + let mut pointer: FastlyChunkPointer = serde_json::from_str(pointer_json).unwrap(); + pointer.chunks[0].key = pointer.chunks[0].key.replacen("app_config", "app_config_staging", 1); + let raw = serde_json::to_string(&pointer).unwrap(); + + let err = prior_chunk_keys("app_config", &raw).expect_err("foreign chunk should warn"); + assert!(err.contains("outside"), "{err}"); +} + +// Regression for the Value-first rule: a value that IS pointer-kind but +// is missing required fields (`chunks`, `data_sha256`, …) must WARN, not +// be silently dropped by a failed struct deserialize. +#[test] +fn prior_chunk_keys_warns_on_pointer_kind_with_missing_fields() { + let raw = r#"{"edgezero_kind":"fastly_config_chunks","version":2}"#; + let err = prior_chunk_keys("app_config", raw) + .expect_err("pointer-kind but malformed must warn, not Ok([])"); + assert!(!err.is_empty(), "{err}"); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli prior_chunk_keys +``` + +Expected: fail because `prior_chunk_keys` is not defined. + +- [ ] **Step 3: Implement `prior_chunk_keys`** + +Add near the other public helpers: + +Parse as `serde_json::Value` **first** so a pointer-kind value with +missing/invalid fields reaches the warning path (spec: "Rules — parse as +`serde_json::Value` first"). Deserializing straight into +`FastlyChunkPointer` would make `{"edgezero_kind":"...","version":2}` +(no `chunks`) fail parsing and be silently returned as `Ok([])`. + +```rust +#[cfg(any(feature = "cli", test))] +pub(crate) fn prior_chunk_keys(root_key: &str, raw: &str) -> Result, String> { + // 1. Parse loosely. Not-JSON, or not our pointer kind => silent. + let value: serde_json::Value = match serde_json::from_str(raw) { + Ok(value) => value, + Err(_) => return Ok(Vec::new()), + }; + if value.get("edgezero_kind").and_then(serde_json::Value::as_str) != Some(POINTER_KIND) { + // Direct BlobEnvelope, unrelated JSON, or first push. + return Ok(Vec::new()); + } + + // 2. It IS pointer-kind: from here every failure WARNS (Err), never silent. + let pointer: FastlyChunkPointer = serde_json::from_value(value).map_err(|err| { + format!("prior chunk pointer at `{root_key}` is malformed: {err}; skipping chunk GC") + })?; + if pointer.version != 1 { + return Err(format!( + "prior chunk pointer at `{root_key}` has unsupported version {}; skipping chunk GC", + pointer.version + )); + } + + // 3. Prefix-scope every referenced key to this root's own namespace. + let prefix = format!("{root_key}{CHUNK_KEY_INFIX}"); + let mut keys = Vec::with_capacity(pointer.chunks.len()); + for chunk_ref in pointer.chunks { + if !chunk_ref.key.starts_with(&prefix) { + return Err(format!( + "prior chunk pointer at `{root_key}` references chunk key `{}` outside expected prefix `{prefix}`; skipping chunk GC", + chunk_ref.key + )); + } + keys.push(chunk_ref.key); + } + Ok(keys) +} +``` + +- [ ] **Step 4: Run scoped tests** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli prior_chunk_keys +``` + +Expected: all `prior_chunk_keys` tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add crates/edgezero-adapter-fastly/src/chunked_config.rs +git commit -m "test(fastly): cover prior chunk key extraction" +``` + +--- + +### Task 2: Add Shared GC Metadata Helpers in Fastly CLI + +**Files:** +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:8` +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:1004` +- Test: compile through later tasks + +- [ ] **Step 1: Update imports** + +Change the Fastly chunked config import: + +```rust +use crate::chunked_config::{ + prior_chunk_keys, prepare_fastly_config_entries, resolve_fastly_config_value, CHUNK_KEY_INFIX, +}; +``` + +- [ ] **Step 2: Add small metadata structs/helpers near config-push helpers** + +Add before `push_entries_with_committer`. There is **no** prefix-scan +helper: `--key` is free-form (`args.rs:311`), so `new_keys` must NOT be +derived by string-matching flattened physical entries (that reintroduces +inference). Instead, expand each logical root **on its own** and take +that root's exact keys. + +```rust +use std::collections::HashSet; + +#[derive(Debug)] +struct FastlyConfigGcPlan { + root_key: String, + prior_keys: Result, String>, + new_keys: HashSet, + /// Exactly the value this push wrote at `root_key` (the root pointer + /// for a chunked push, or the direct envelope). Cloud uses it as a + /// read-back concurrency guard before deleting; local ignores it. + new_root_value: String, +} + +/// Expand ONE logical (root_key, body) into its physical entries, the +/// exact keep-set for that root, and the value written at the root key. +/// No cross-root prefix scanning. +fn expand_root(root_key: &str, body: &str) -> Result<(Vec<(String, String)>, HashSet, String), String> { + let expanded = prepare_fastly_config_entries(root_key, body)?; + let new_keys: HashSet = expanded.iter().map(|(k, _)| k.clone()).collect(); + // prepare_* always emits the root entry LAST (root pointer or direct value). + // Make the invariant explicit rather than silently defaulting to "". + let new_root_value = expanded + .last() + .map(|(_, v)| v.clone()) + .ok_or_else(|| format!("internal: no physical entries produced for root `{root_key}`"))?; + Ok((expanded, new_keys, new_root_value)) +} + +fn orphan_chunk_keys(plan: &FastlyConfigGcPlan) -> Result, String> { + match &plan.prior_keys { + Ok(prior) => Ok(prior + .iter() + .filter(|key| !plan.new_keys.contains(*key)) + .cloned() + .collect()), + Err(err) => Err(err.clone()), + } +} + +/// Enforced at the adapter boundary because `--key` is free-form: a key +/// containing the reserved chunk infix would collide with chunk storage. +fn reject_reserved_root_keys(entries: &[(String, String)]) -> Result<(), String> { + for (key, _) in entries { + if key.contains(CHUNK_KEY_INFIX) { + return Err(format!( + "config key `{key}` contains the reserved infix `{CHUNK_KEY_INFIX}`, which collides with Fastly chunk storage; choose a different config key (or --key override)" + )); + } + } + Ok(()) +} +``` + +Both `push_config_entries` and `push_config_entries_local` MUST call +`reject_reserved_root_keys(entries)?` immediately after the empty-entries +check, before any expansion or I/O. Keep helpers private. + +- [ ] **Step 3: Run compile check** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli --no-run +``` + +Expected: compile succeeds. + +- [ ] **Step 4: Commit** + +```bash +git add crates/edgezero-adapter-fastly/src/cli.rs +git commit -m "refactor(fastly): add chunk gc planning helpers" +``` + +--- + +### Task 3: Implement Local Fastly TOML GC + +**Files:** +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:421` +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:926` +- Test: `crates/edgezero-adapter-fastly/src/cli.rs:2991` +- Test: `crates/edgezero-adapter-fastly/src/cli.rs:3296` + +- [ ] **Step 1: Rewrite the second oversized local push test to fail** + +In `second_oversized_push_converges_runtime_on_new_envelope`, invert the old-chunk assertion: + +```rust +for chunk_key in &chunks_a { + assert!( + !chunks_b.contains(chunk_key), + "old A-chunk `{chunk_key}` must be pruned after push B; B-set={chunks_b:?}" + ); +} +``` + +Update stale comments that say old chunks remain or no GC exists. + +- [ ] **Step 2: Add shrink-to-direct local test** + +Add a test near the local push tests: + +```rust +#[cfg(unix)] +#[test] +fn push_config_entries_local_prunes_prior_chunks_when_value_shrinks_to_direct() { + use crate::chunked_config::FASTLY_CONFIG_ENTRY_LIMIT; + let dir = tempdir().expect("tempdir"); + let fastly_toml = dir.path().join("fastly.toml"); + fs::write(&fastly_toml, "name = \"demo\"\n").expect("seed"); + + let chunked = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT.saturating_add(1)); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), chunked)], + &AdapterPushContext::new(), + false, + ) + .expect("first push"); + + let direct = make_test_envelope(FASTLY_CONFIG_ENTRY_LIMIT); + FastlyCliAdapter + .push_config_entries_local( + dir.path(), + Some("fastly.toml"), + None, + &ResolvedStoreId::from_logical(TEST_CONFIG_ID), + &[(TEST_CONFIG_ID.to_owned(), direct.clone())], + &AdapterPushContext::new(), + false, + ) + .expect("second push"); + + let after = fs::read_to_string(&fastly_toml).expect("read"); + let doc: toml_edit::DocumentMut = after.parse().expect("parse"); + let contents = doc + .get("local_server") + .and_then(|ls| ls.get("config_stores")) + .and_then(|cs| cs.get(TEST_CONFIG_ID)) + .and_then(|st| st.get("contents")) + .and_then(toml_edit::Item::as_table) + .expect("contents"); + + assert_eq!(contents.get(TEST_CONFIG_ID).and_then(toml_edit::Item::as_str), Some(direct.as_str())); + assert!( + !contents.iter().any(|(key, _)| key.contains(CHUNK_KEY_INFIX)), + "prior chunks must be removed: {after}" + ); +} +``` + +- [ ] **Step 3: Extend local dry-run test to expect orphan count** + +Seed a prior chunked push before dry-run, keep `original`, then assert: + +```rust +assert!(combined.contains("would delete")); +assert!(combined.contains("orphan chunks")); +``` + +Also keep `assert_eq!(after, original)`. + +Add a second dry-run test for the identical-bytes re-push (regression +for MEDIUM-A / the expand-`new_keys` rule): seed a chunked config, then +dry-run the SAME bytes and assert the count is `0`: + +```rust +assert!(combined.contains("would delete 0 orphan chunks")); +``` + +Add a third dry-run test for a suspicious prior pointer: seed the root +with a pointer-kind-but-invalid value (e.g. `version: 2`), dry-run a new +config, and assert the degraded line plus no write: + +```rust +assert!(combined.contains("unknown: suspicious prior pointer")); +// and the file is unchanged +``` + +- [ ] **Step 3c: Add local suspicious-pointer real-push test (spec §"local")** + +Seed `fastly.toml` so the root holds a pointer-kind-but-invalid value +(e.g. `{"edgezero_kind":"fastly_config_chunks","version":2}`) **and** +seed one or more real chunk-like entries under the root's namespace +(keys containing `.__edgezero_chunks.`) so the "no deletes" assertion is +non-vacuous. Then run a REAL (non-dry-run) local push of a new config. +Assert: +- the returned status `Vec` contains a suspicious-pointer warning, +- **the pre-seeded chunk-like keys are still present** (the writer must + not prune when `prior_chunk_keys` returns `Err`), +- the new value is written to the root key. + +- [ ] **Step 3d: Add reserved-key rejection test (local)** + +Assert `push_config_entries_local` with a logical key containing +`.__edgezero_chunks.` returns `Err` before touching `fastly.toml` (file +unchanged / not created). + +- [ ] **Step 4: Run local tests to verify failures** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli push_config_entries_local -- --nocapture +cargo test -p edgezero-adapter-fastly --features cli second_oversized_push_converges_runtime_on_new_envelope -- --nocapture +``` + +Expected: failures showing old chunks remain and dry-run lacks GC count. + +- [ ] **Step 5: Implement local in-memory pruning** + +Change `write_fastly_local_config_store` to (a) accept the exact per-root +keep-sets via a new `gc_roots: &[(String, HashSet)]` param +(each `(root_key, new_keys)` from `expand_root` — NOT prefix-scanned) and +(b) return warning lines, while preserving its error behavior: + +```rust +fn write_fastly_local_config_store( + path: &Path, + platform_name: &str, + entries: &[(String, String)], + gc_roots: &[(String, std::collections::HashSet)], +) -> Result, String> +``` + +Update ALL existing call sites to pass `gc_roots`. There are **10** (line +numbers approximate — grep `write_fastly_local_config_store` to confirm): + +| Site | Kind | `gc_roots` to pass | +| --- | --- | --- | +| `cli.rs:483` | prod (`push_config_entries_local`) | `&gc_roots` built via `expand_root` (see caller snippet below) | +| `cli.rs:1838` | test — minimal-file block | `&[]` (setup-only; no prior generation to prune) | +| `cli.rs:1867`, `:1873` | test — replaces-block-on-re-push | `&[]` (both writes; asserts value replacement, not GC) | +| `cli.rs:1902` | test — preserves-unrelated-blocks | `&[]` | +| `cli.rs:1931` | test — creates-file-when-missing | `&[]` | +| `cli.rs:2366` | test | `&[]` unless it asserts GC | +| `cli.rs:3243` | test | `&[]` unless it asserts GC | +| `cli.rs:3275` | test — local chunked roundtrip | `&[]` (read-back roundtrip, not GC) | + +Setup-only writes pass `&[]` (empty ⇒ the writer skips the snapshot/prune +loop entirely, so behaviour is unchanged for those tests). Only tests +that assert pruning build a real `gc_roots`. Existing direct callers that +only `.expect("write")` can ignore the returned `Vec` (return +type changes from `Result<(), _>` to `Result, _>`, which is +source-compatible with `.expect(...)`). + +Inside the function: + +1. Snapshot prior state using the **given exact keep-sets** before + mutation (no inference, no prefix scan): + +```rust +let mut plans = Vec::with_capacity(gc_roots.len()); +for (root_key, new_keys) in gc_roots { + let prior_keys = contents_tbl + .get(root_key) + .and_then(toml_edit::Item::as_str) + .map_or_else(|| Ok(Vec::new()), |raw| prior_chunk_keys(root_key, raw)); + plans.push(FastlyConfigGcPlan { + root_key: root_key.clone(), + prior_keys, + new_keys: new_keys.clone(), + new_root_value: String::new(), // unused locally (no remote concurrency) + }); +} +``` + +2. Keep the existing upsert loop. + +3. After the upsert loop, remove orphans and collect warnings: + +```rust +let mut warnings = Vec::new(); +for plan in &plans { + match orphan_chunk_keys(plan) { + Ok(orphan_keys) => { + for key in orphan_keys { + contents_tbl.remove(&key); + } + } + Err(err) => warnings.push(format!("warning: {err}")), + } +} +``` + +Return `Ok(warnings)` after `fs::write`. Suspicious prior pointers must not fail the local write and must not produce partial deletes. + +In `push_config_entries_local`, capture the warning vector and append it after the existing success line: + +```rust +// Expand each logical root once: flatten for the write, keep exact per-root +// keep-sets for GC (no prefix scan of the flattened set). +let mut physical_entries: Vec<(String, String)> = Vec::new(); +let mut gc_roots: Vec<(String, std::collections::HashSet)> = Vec::with_capacity(entries.len()); +for (root_key, body) in entries { + let (expanded, new_keys, _new_root) = expand_root(root_key, body)?; + physical_entries.extend(expanded); + gc_roots.push((root_key.clone(), new_keys)); +} +let warnings = write_fastly_local_config_store(&fastly_path, name, &physical_entries, &gc_roots)?; +let mut out = vec![format!( + "wrote {} physical entries ({} logical) to `[local_server.config_stores.{name}.contents]` in {} (logical id `{logical}`); restart `fastly compute serve` to pick up changes", + physical_entries.len(), + entries.len(), + fastly_path.display() +)]; +out.extend(warnings); +Ok(out) +``` + +- [ ] **Step 6: Add local dry-run count helper** + +Add a private helper that reads `fastly.toml` and returns counts per root: + +```rust +fn local_orphan_counts_for_dry_run( + path: &Path, + platform_name: &str, + entries: &[(String, String)], // logical entries; roots are their keys +) -> Vec<(String, Result)> { + // Roots are the logical entry keys (NOT inferred from physical keys). + // For each logical (root_key, body): + // new_keys = keys of prepare_fastly_config_entries(root_key, body) ∪ {root_key} + // — MUST expand here; using the logical key alone would over-count an + // identical-bytes re-push (new chunk keys would be missing from + // new_keys, so every prior chunk would look like an orphan). + // old_raw = contents table value at root_key (if present) + // count = orphan_chunk_keys({prior_chunk_keys(root, old_raw), new_keys}).len() + // Missing file, missing/absent contents table, or a root with no prior + // pointer / a direct prior value => Ok(0) per root. + // Unreadable file, malformed TOML, contents-not-a-table, or a root + // value that is not a string => Err("could not read prior state"). + // prior_chunk_keys(root, old_raw) returning Err (suspicious pointer) + // => Err("suspicious prior pointer"). +} +``` + +This helper MUST NOT fail the dry-run: it returns a per-root `Result` +that the caller renders as a count or an `unknown (...)` line. The +current dry-run reads no file at all, so introducing this read must not +turn a previously-succeeding dry-run into an error. Note it expands each +logical entry via `prepare_fastly_config_entries` to derive the true +`new_keys` — the same expansion the real push performs — so the count +matches what the push would actually delete (0 for an identical +re-push). Use exact `toml_edit` table access, not string scanning. Keep +this helper small; if it gets large, split only into +`local_contents_table` and count computation. + +- [ ] **Step 7: Wire local dry-run count output** + +In `push_config_entries_local` dry-run, after each direct/chunked line add: + +```rust +out.push(format!( + " would delete {count} orphan chunks from the previous generation of `{key}`" +)); +``` + +For an `Err`, emit (wording matches the spec's local dry-run bullet — +`{reason}` is `could not read prior state` or `suspicious prior pointer`): + +```rust +out.push(format!( + " would delete an unknown number of orphan chunks from the previous generation of `{key}` (unknown: {reason})" +)); +``` + +- [ ] **Step 8: Run local scoped tests** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli push_config_entries_local -- --nocapture +cargo test -p edgezero-adapter-fastly --features cli second_oversized_push_converges_runtime_on_new_envelope -- --nocapture +cargo test -p edgezero-adapter-fastly --features cli read_config_entry_local -- --nocapture +``` + +Expected: local push/read tests pass. + +- [ ] **Step 9: Commit** + +```bash +git add crates/edgezero-adapter-fastly/src/cli.rs +git commit -m "feat(fastly): prune local config chunks on re-push" +``` + +--- + +### Task 4: Implement Cloud Fastly GC + +**Files:** +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:349` +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:671` +- Modify: `crates/edgezero-adapter-fastly/src/cli.rs:1073` +- Test: `crates/edgezero-adapter-fastly/src/cli.rs:2440` +- Test: `crates/edgezero-adapter-fastly/src/cli.rs:2760` + +- [ ] **Step 1: Add or replace command-aware fake Fastly helper** + +Add a Unix-only helper in the test module that: + +- Responds to `config-store list --json`. +- Responds to `config-store-entry describe --key=` from a key-response map. +- For the concurrency-guard test, can serve the root `describe` + **sequentially** — first the prior pointer (pre-commit read), then a + different value (post-commit read-back) — e.g. via an invocation + counter file so successive `describe ` calls return different + bodies. +- Accepts `config-store-entry update --upsert --stdin` and logs `update `. +- Accepts `config-store-entry delete --key= --auto-yes` and logs `delete `. +- Can be configured so specific delete keys fail with stderr. + +Prefer one operation per log line, for example: + +```text +list +describe app_config +update app_config.__edgezero_chunks..0 +update app_config +delete app_config.__edgezero_chunks..0 +``` + +- [ ] **Step 2: Add failing cloud tests** + +Add tests for: + +- Prior chunks are deleted and new/root keys are not. (The fake serves the + root `describe` twice: the prior pointer pre-commit, then the + newly-written root value on read-back, so the concurrency guard passes + and deletes proceed.) +- Shrink-to-direct deletes all prior chunks. +- No prior value produces zero deletes. +- Identical re-push produces zero deletes. +- Delete failure returns `Ok` with a warning line naming the chunk. +- Prior read failure returns `Ok` with a warning and no deletes. +- Suspicious pointer version returns `Ok` with a warning and no deletes. +- Delete operations occur after root update. +- Concurrency guard: when the post-commit root re-read differs from what + this push wrote (fake `fastly` returns a different root value on the + second `describe`), GC is skipped with a "root changed" warning and + **no** deletes are issued. +- Reserved key: `--key` containing `.__edgezero_chunks.` is rejected + before any `fastly` invocation (no `list`/`describe`/`update`/`delete`). +- Dry-run stays offline and emits no-count GC intent. + +Note the concurrency test needs the fake `fastly` to serve the root +`describe` TWICE with different values: the prior pointer (pre-commit +read) and then a mismatching value (post-commit read-back). + +Use `path_mutation_guard()` for tests that prepend a fake `fastly`. + +- [ ] **Step 3: Run cloud tests to verify failures** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli push_config_entries_ -- --nocapture +``` + +Expected: new GC tests fail because no deletes or warning lines exist yet. + +- [ ] **Step 4: Add `delete_config_store_entry`** + +Add near `create_config_store_entry`. NOTE: only ever pass `--key`. +The delete subcommand also accepts `-a/--all`, which wipes **every** +entry in the store — never construct that flag here. + +```rust +fn delete_config_store_entry(store_id: &str, key: &str) -> Result<(), String> { + let store_arg = format!("--store-id={store_id}"); + let key_arg = format!("--key={key}"); + let output = Command::new("fastly") + .args([ + "config-store-entry", + "delete", + store_arg.as_str(), + key_arg.as_str(), + "--auto-yes", + ]) + .output() + .map_err(|err| { + if err.kind() == ErrorKind::NotFound { + format!("`fastly` not found on PATH; {FASTLY_INSTALL_HINT}") + } else { + format!("failed to spawn `fastly`: {err}") + } + })?; + if output.status.success() { + return Ok(()); + } + let stderr = String::from_utf8_lossy(&output.stderr); + let lower = stderr.to_ascii_lowercase(); + if lower.contains("not found") || lower.contains("does not exist") || lower.contains("404") { + return Ok(()); + } + Err(format!( + "`fastly config-store-entry delete --store-id={store_id} --key={key} --auto-yes` exited with status {}\nstderr: {}", + output.status, + stderr.trim() + )) +} +``` + +- [ ] **Step 5: Build GC plans before cloud commit** + +In `push_config_entries`, after `resolved_id`: + +```rust +let mut gc_plans = Vec::with_capacity(entries.len()); +let mut warnings = Vec::new(); +for (root_key, body) in entries { + // Exact per-root keep-set + the value we will write (no prefix scan). + let (_, new_keys, new_root_value) = expand_root(root_key, body)?; + let prior_keys = match fetch_remote_config_store_entry(&resolved_id, root_key) { + Ok(Some(raw)) => prior_chunk_keys(root_key, &raw), + Ok(None) => Ok(Vec::new()), + Err(err) => Err(format!( + "failed to read prior root `{root_key}` for chunk GC: {err}; skipping GC for this root" + )), + }; + gc_plans.push(FastlyConfigGcPlan { + root_key: root_key.clone(), + prior_keys, + new_keys, + new_root_value, + }); +} +``` + +If `prior_chunk_keys` returns `Err`, keep the `Err` in the plan so +deletes are skipped (the sweep in Step 6 turns it into a warning). + +- [ ] **Step 6: Sweep after successful commit** + +Keep the existing `push_entries_with_committer` call. Only after it +returns `Ok`, iterate plans. Before deleting a root's orphans, **re-read +the raw root and confirm it still equals exactly what this push wrote** +(`plan.new_root_value`). This guards against a concurrent push that +overwrote the root between our commit and our sweep — e.g. reverting it +to the prior pointer, which would make the "orphans" live again: + +```rust +for plan in &gc_plans { + let keys = match orphan_chunk_keys(plan) { + Ok(keys) if !keys.is_empty() => keys, + Ok(_) => continue, // nothing to reclaim + Err(err) => { warnings.push(format!("warning: {err}")); continue } + }; + // Concurrency guard: only sweep if the root still holds our write. + match fetch_remote_config_store_entry(&resolved_id, &plan.root_key) { + Ok(Some(current)) if current == plan.new_root_value => { + for key in keys { + if let Err(err) = delete_config_store_entry(&resolved_id, &key) { + warnings.push(format!( + "note: could not reclaim orphan chunk `{key}` for `{}` ({err}); it is inert and will be removed by a future `config gc`", + plan.root_key + )); + } + } + } + Ok(_) => warnings.push(format!( + "note: skipped chunk GC for `{}`: root changed since this push wrote it (concurrent push?); orphans left for a future `config gc`", + plan.root_key + )), + Err(err) => warnings.push(format!( + "note: skipped chunk GC for `{}`: could not re-read root before sweep ({err}); orphans left for a future `config gc`", + plan.root_key + )), + } +} +``` + +This narrows but does not fully close the window (a writer could still +intervene between the read-back and an individual delete) — Fastly has no +compare-and-delete. That residual is acceptable under invariant 4: +GC is best-effort and the guard eliminates the realistic revert-to-prior +corruption. Return the existing success line plus warning lines. + +- [ ] **Step 7: Keep cloud dry-run offline** + +In the `dry_run` branch, after each direct/chunked line add: + +```rust +out.push(format!( + " would delete orphaned prior-generation chunks of `{key}` (count determined at push time)" +)); +``` + +Do not move `resolve_remote_config_store_id` above the dry-run branch. + +- [ ] **Step 8: Run cloud scoped tests** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli push_config_entries_ -- --nocapture +``` + +Expected: cloud push tests pass. + +- [ ] **Step 9: Commit** + +```bash +git add crates/edgezero-adapter-fastly/src/cli.rs +git commit -m "feat(fastly): delete prior remote config chunks after push" +``` + +--- + +### Task 5: Integration Cleanup and Verification + +**Files:** +- Modify as needed: `crates/edgezero-adapter-fastly/src/cli.rs` +- Modify as needed: `crates/edgezero-adapter-fastly/src/chunked_config.rs` + +- [ ] **Step 1: Run Fastly adapter tests** + +Run: + +```bash +cargo test -p edgezero-adapter-fastly --features cli +``` + +Expected: all Fastly adapter CLI-feature tests pass. + +- [ ] **Step 2: Run full workspace tests required after code changes** + +Run: + +```bash +cargo test --workspace --all-targets +``` + +Expected: all workspace tests pass. + +- [ ] **Step 3: Run formatting check** + +Run: + +```bash +cargo fmt --all -- --check +``` + +Expected: no formatting diffs. + +- [ ] **Step 4: Run clippy** + +Run: + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings +``` + +Expected: no warnings. + +- [ ] **Step 5: Run feature compile check** + +Run: + +```bash +cargo check --workspace --all-targets --features "fastly cloudflare spin" +``` + +Expected: compile succeeds. + +- [ ] **Step 6: Run Spin target check** + +Run: + +```bash +cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin +``` + +Expected: compile succeeds. + +- [ ] **Step 7: Commit final cleanup** + +```bash +git add crates/edgezero-adapter-fastly/src/chunked_config.rs crates/edgezero-adapter-fastly/src/cli.rs +git commit -m "test(fastly): verify chunk gc behavior" +``` + +--- + +## Notes for Implementers + +- Do not delete root keys. Only delete keys returned by `prior_chunk_keys(root_key, raw)` after subtracting the root's new keep-set. +- Source root keys and keep-sets from the caller's logical `entries` via `expand_root` (each root expanded on its own); pass exact per-root keep-sets to the local writer as `gc_roots`. Never infer roots or keep-sets by string-matching `CHUNK_KEY_INFIX` on the flattened physical set — `--key` is free-form and may itself contain the infix (and is separately rejected via `reject_reserved_root_keys`). +- `prior_chunk_keys` parses `serde_json::Value` first: non-pointer-kind values are silent `Ok([])`; a value that IS pointer-kind but malformed/invalid must `Err` (warn), not be silently skipped. +- Do not make cloud dry-run resolve the store id or call `fastly`. +- Do not change `edgezero_adapter::registry::Adapter`; raw prior pointer reads are Fastly-specific. +- Do not use prefix scans of the remote store. The only remote reads are per-root `describe` calls. +- Use exact `toml_edit::Table` key operations for local dotted chunk keys. +- If a push commit fails, do not sweep anything. The old root pointer may still be live. +- If GC fails, return `Ok` with warning status lines. The push success criterion remains new entries committed. +- Cloud GC follows **last-writer-wins** (spec "Concurrency model: last-writer-wins"): concurrent pushes are supported; the last root-pointer write wins on the value. A push sweeps prior chunks only while the post-commit read-back shows it is still the last writer, else it yields. The guard removes the common race but not the read-back-then-supersede interleaving (no compare-and-delete) — best-effort, not transactional; don't represent it as strictly concurrent-safe. +- Cloud GC costs: one pre-push `describe` per root, plus one post-commit read-back `describe` per root that has orphans, plus one `fastly` `delete` process per orphan (sequential). A 50-chunk prior generation ≈ 50 sequential deletes. Fastly has no bulk delete-by-key (`--all` is store-wide and unusable); do not add parallel spawns in v1 without measuring. +- Line numbers in this plan are approximate anchors against `main`; `grep` the named function/test before editing, since prior edits shift offsets. diff --git a/docs/superpowers/specs/2026-07-07-fastly-chunk-gc.md b/docs/superpowers/specs/2026-07-07-fastly-chunk-gc.md new file mode 100644 index 00000000..9dd033ec --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-fastly-chunk-gc.md @@ -0,0 +1,421 @@ +# Fastly chunked-config GC: reclaim orphaned chunk entries on re-push + +## Motivation + +When a Fastly Config Store value exceeds the 8 000-character entry +limit, `config push` splits it into content-addressed chunk entries +plus a root pointer, per the chunked-storage design +(`crates/edgezero-adapter-fastly/src/chunked_config.rs`). Chunk keys +are content-addressed by the envelope hash: + +``` +.__edgezero_chunks.. +``` + +The push path is **upsert-only**. Both the cloud writer +(`FastlyCliAdapter::push_config_entries`, `cli.rs:349`) and the local +writer (`FastlyCliAdapter::push_config_entries_local`, `cli.rs:421` → +`write_fastly_local_config_store`, `cli.rs:926`) insert-or-update +physical entries and never delete anything. + +Because a config change changes the envelope bytes, it changes +`envelope_sha256`, which changes **every** chunk key. The new push +writes a fresh chunk set and overwrites the root pointer in place, so +reads stay correct — the live pointer only ever references the current +chunk set. But the **previous** generation's chunk entries are left +behind, unreferenced and unreclaimed: + +- **Cloud**: orphan entries linger in the remote Config Store. +- **Local**: orphan keys linger in the `fastly.toml` + `[local_server.config_stores..contents]` table (upserted in + `write_fastly_local_config_store` at `cli.rs:926`, which reads the + doc, inserts each physical entry, and never removes stale keys). + +Every chunked re-push leaks one generation of chunks. This spec reclaims them: + +- **Local** prunes eagerly, inside the same `fastly.toml` rewrite. Safe: one + file, read by Viceroy at startup — no propagation window. +- **Cloud** reclaims orphaned generations that are unreferenced by the live + pointer AND have been superseded for longer than a grace window, deriving + both facts from the store itself. It is **never** safe to delete the + just-superseded generation immediately (see "Cloud reclamation"). + +> **Code-layout note.** This spec targets the layout on `main`, where +> the Fastly adapter is monolithic (`cli.rs` + `chunked_config.rs`). +> An in-flight branch (`feature/provision-local-impl`) splits `cli.rs` +> into `cli/push_cloud.rs`, `cli/push_local.rs`, and +> `cli/provision_local.rs`. If that branch merges first, the same +> changes land in those files instead — the functions and logic are +> identical, only the file boundaries move. Rebase the implementation +> onto whichever layout is current. + +## Scope + +- Applies to the Fastly adapter's `config push` writeback path only + (Stage 7), both cloud and `--local`. +- Deletes **chunk data entries** only. It NEVER deletes a root pointer + key: the pointer at `` is overwritten in place by the + existing upsert and remains the stable, live entry. +- Does not touch sibling logical keys (e.g. `app_config` vs + `app_config_staging`): every candidate key must parse as + `.__edgezero_chunks..` for *that* root, so the shared + string prefix of a sibling root never matches. + +## Terminology + +- **Root key**: the stable logical entry key (e.g. `app_config`). Holds + either a direct `BlobEnvelope` or a chunk pointer. Never GC'd. The + root key is the logical store-config id, or the operator's + `--key ` — a free-form `Option` (`args.rs:311`, + used directly in `config.rs:297`). GC never infers roots by + string-matching the chunk infix; roots come from the caller's logical + entries. In addition, the Fastly adapter **MUST reject** (hard error, + not warning) any logical key containing the reserved infix + `.__edgezero_chunks.`, at the top of both `push_config_entries` and + `push_config_entries_local`, before any expansion or I/O. Such a key + collides with the generated chunk namespace and has no valid use; a + push must not be able to write into another key's chunk space. The + check lives at the Fastly adapter boundary (the infix is a + Fastly-specific concept), not in generic `edgezero-cli`. +- **Previous pointer**: the value stored at the root key *before* this + push overwrites it. +- **Prior chunk keys**: the keys named by `previous_pointer.chunks[]`, + *after* validation and prefix filtering (see `prior_chunk_keys`). + Empty if the previous value was a direct envelope, missing, or not a + valid v1 pointer. +- **New keep-set**: the physical keys this push writes for that root — + the new chunk keys plus the root key itself. It is built by expanding + **that root's own** `(root_key, body)` via + `prepare_fastly_config_entries` and taking those keys — NOT by + prefix-scanning the flattened multi-root physical set (which would + reintroduce infix inference and mis-handle shared prefixes or a + free-form key). +- **Generation**: the set of chunk entries sharing one content address + (`envelope_sha256`). Chunk keys are grouped into generations by parsing them. +- **Orphan generation**: a generation not referenced by the **live** root + pointer. Cloud reclaims an orphan only once it has also aged past the grace + window; local prunes `prior_chunk_keys − new_keep_set` immediately. + +## Invariants (MUST) + +1. **A cloud push never deletes.** It writes chunks, then the pointer. Any + reclamation is a separate, operator-invoked `config gc`. + +2. **Nothing is deleted that a live pointer references.** `config gc` computes + the live set by parsing every root's pointer. Local prunes only + `prior_chunk_keys − new_keep_set`. + +3. **Only canonical, root-scoped keys are delete candidates.** A key must parse + as `.__edgezero_chunks..`. Deletes target the store's + ACTUAL keys — never keys re-derived from a content address. + +4. **Destructive paths FAIL CLOSED.** If `config gc` cannot classify a root, or + cannot read an entry's `created_at`, it aborts and deletes **nothing**. An + unreadable state must never fail open into deletion. + +5. **The operator's `--older-than` is the safety assertion.** The machine cannot + know when a pointer stopped being served; the operator can. A `--dry-run` + prints every key and age it would delete so that assertion is reviewable. + +6. **Local eager pruning is safe and stays.** One file, read at Viceroy startup: + no propagation window, no POPs. + +7. **Push failure semantics are unchanged.** Reclamation is never part of a + push, so it can never fail one. + +## Two further invariants (PR #314 review) + +**Diagnostics must never echo config payloads.** The `describe` shell-outs +interpolated their raw stdout — i.e. the stored config envelope, which may hold +credentials — into errors that the CLI logs verbatim. Because the GC prior-read +runs on *every* cloud push, this exposed the previous config on any schema +drift. Errors now report only the response's **size and top-level field-name +shape** (`redact_describe_response`), never a value. Enforced by sentinel-secret +tests on both the read and push paths. + +**A batch must not name the same logical root twice.** GC builds one plan per +entry and snapshots every plan against the *same* prior generation. With +`[(root, A), (root, B)]` the last tuple wins the upsert (root = B), yet A's plan +would still reclaim `prior − A_keys` — which includes B's freshly-written chunks +— leaving the final pointer dangling. Duplicate root keys are therefore a **hard +error** in both push paths, before any expansion or I/O. (Rejecting beats +silently coalescing: a duplicated key is a caller bug, and picking a winner would +hide it.) + +## Cloud reclamation: NOT automatic. `config gc`, invoked by the operator. + +> **This is the single most important section. Three automatic designs were +> built and each was demolished in review. They are recorded so nobody tries a +> fourth.** +> +> 1. **Eager delete** (remove the superseded generation right after the commit, +> guarded by a read-back) — **unsafe**. The store is eventually consistent and +> the read-back only observes the **control plane**. POPs may still be serving +> the previous pointer, which references the chunks being deleted. This breaks +> reads on *every* re-push, not merely under concurrency. +> 2. **Metadata sidecar** (record the superseded generation; reclaim it later) — +> **unsound**. Fastly has no compare-and-swap, so a failed write, a failed +> read-back, or a concurrent push **permanently loses** a generation; and the +> record overflows the 8 000-char entry limit at ~71 generations. +> 3. **Store-derived clock** (`superseded_at(G) = created_at(successor(G))`) — +> **unsound**. Chunk creation is not a pointer transition. Counterexample: +> chunked **A** → direct **B** → direct **C**. During C, A is the only chunk +> generation listed, so it has *no successor* and ages from its own creation — +> and is deleted, though B superseded it seconds ago and POPs may still serve +> A. Partial pushes and chunk/pointer write gaps break it identically. + +**The impossibility, stated plainly.** To delete a chunk safely you must know +that *the pointer which referenced it has stopped being served everywhere, for +longer than the propagation window*. Fastly: + +- **does not record** that fact — `updated_at` is **not** bumped by + `update --upsert` (verified against the live API: a root reading `updated_at = + 2026-07-07` was pointing at chunks created `2026-07-13`); +- **offers no CAS** with which we could record it ourselves safely; +- and its chunk `created_at` **is not a proxy** for it (design 3 above). + +The fact simply is not available to the machine. **It is available to the +operator**, who knows their own deploy history. So the operator supplies it. + +### What a cloud `config push` does + +**It reclaims nothing.** It writes chunks, then the pointer. That is all. Cloud +storage therefore accretes orphaned generations exactly as it does today — this +is **not a regression**, and it is the only safe automatic behaviour. + +### `config gc` (`Adapter::gc_config_entries`) + +``` +config gc --adapter fastly [--older-than ] [--dry-run] +``` + +1. One `config-store-entry list --json`. +2. Classify every non-chunk entry as a **root**; parse its value with + `prior_chunk_keys` → the chunk keys that root's pointer **references**. The + union over all roots is the **live** set. (The listing already carries + `item_value`, so this costs no extra `describe` calls.) +3. Candidates = chunk entries **not** in the live set, whose `created_at` is + older than `--older-than`. +4. Delete them. + +**`--older-than` is the operator's safety assertion**: *"nothing created before +this is still being served."* Only they can make it. + +**It fails CLOSED.** If a root's value cannot be classified, or an entry's +`created_at` cannot be read, `config gc` **aborts and deletes nothing** — an +unreadable state must never fail open on a destructive path. + +**A `--dry-run` prints every key it would delete, with its age**, so the +assertion is reviewable before it is acted on. + +Only ever `--key` on delete; the subcommand also accepts `-a/--all`, which would +wipe the store — never construct that flag. + +### Local is different, and eager pruning there is correct + +`fastly.toml` is a single file Viceroy reads at startup: there is no propagation +window and no POP that could still be serving the previous pointer. The local +path prunes the prior generation immediately, inside the same rewrite. + +## Concurrency model + +The config value is last-writer-wins: the root is one entry and `update --upsert` +means the push whose pointer lands last defines the live config. Concurrent +pushes are supported. + +Because a push **never deletes**, there is no push-time reclamation race to +reason about at all. `config gc` is a separate, operator-timed action; it deletes +only what no live pointer references and what the operator has asserted is old +enough. Running it concurrently with a push is still the operator's call — the +`--older-than` assertion is what makes it safe. + +## `prior_chunk_keys` helper (`chunked_config.rs`) + +Load-bearing in both paths: it yields the chunk keys a pointer **references**, +validated and prefix-scoped to that root. + +- `Ok(keys)` — a valid v1 chunk pointer. +- `Ok(vec![])` — a direct `BlobEnvelope`, absent, or not pointer-shaped (silent). +- `Err(msg)` — pointer-*kind* but invalid (bad version, foreign-prefix key). + +Parsed `serde_json::Value`-first, so a pointer-kind value with missing fields +reaches the error path instead of being silently dropped. In `config gc` an +`Err` here **aborts the whole run** (we cannot know what that root references). + +`chunk_key_generation` recognises only the canonical shape +`.__edgezero_chunks..`, so a foreign or hand-edited key is +never a delete candidate. + +## Algorithm + +``` +# validate first (both push paths), before any expansion or I/O +reject if any logical key contains CHUNK_KEY_INFIX # reserved namespace +reject if any logical key appears more than once # duplicate-root invariant + +for each logical (root_key, body): + expanded = prepare_fastly_config_entries(root_key, body) # this root only + new_keys = { k for (k, _) in expanded } # includes root_key + +# --- cloud push --- +commit expanded (chunks first, pointer last). NO deletes. + +# --- local push --- +prune (prior_chunk_keys - new_keys) inside the same fastly.toml rewrite + +# --- config gc (operator) --- +list -> live = union of prior_chunk_keys(root, root_value) over all roots +doomed = chunk entries not in live, older than --older-than +delete doomed # fails closed on any unclassifiable/unreadable state +``` + +## Local path (`push_config_entries_local` / `write_fastly_local_config_store`) + +The local `contents` table is held entirely in memory in a single +`DocumentMut`, so GC here is fully reliable and needs no round-trips. +Per-root keep-sets are computed from each root's own expansion and +passed to the writer explicitly — never inferred from the flattened set: + +- **Pass exact per-root keep-sets (no inference).** + `push_config_entries_local` expands each logical `(root_key, body)` via + `prepare_fastly_config_entries` (once, reused for `physical_entries`) + and passes `gc_roots: &[(String, HashSet)]` — each root with + its own exact new-key set — into `write_fastly_local_config_store` + alongside `physical_entries`. No string-matching on the chunk infix; a + `--key` containing `.__edgezero_chunks.` is separately rejected up + front (Terminology). An empty `gc_roots` means "no GC" (setup-only + writers pass `&[]`). +- **Sweep inside the single rewrite.** In `write_fastly_local_config_store` + (`cli.rs:926`), before the upsert loop, snapshot each root's *old* + value from the existing `contents_tbl`. After inserting the new + physical entries, for each `(root, new_keys)` compute + `prior_chunk_keys(root, old_value)` and `contents_tbl.remove(k)` for + each orphan `k` in `prior − new_keys`. All within the one + `DocumentMut` that the trailing `fs::write` (`cli.rs:999`) persists — + one in-memory rewrite, then one write. +- **Warnings.** `prior_chunk_keys` returning `Err` for a root → collect + the message; the local push returns it as an extra status line. A + removal is infallible on an in-memory table, so the only local + warnings are suspicious-pointer ones. +- **Dry-run (offline, best-effort count).** `push_config_entries_local`'s + dry-run (`cli.rs:459`) reads no remote state. Extend it to read the + current `fastly.toml` and, per root, compute the orphan count as + `prior_chunk_keys(root, old_value) − new_keys`, where `new_keys` is + the **expanded** key set (`prepare_fastly_config_entries(root, body)` + keys ∪ `{root}`) — the same expansion the real push does. Expanding + is required: using the logical key alone would over-count an + identical-bytes re-push (the new chunk keys would be missing from + `new_keys`, making every prior chunk look like an orphan; the correct + answer is `0`). Report e.g. + `" would delete 9 orphan chunks from the previous generation of `app_config`"`. + Error semantics — the dry-run MUST NOT newly fail where it succeeds + today (the current dry-run does not read the file at all), so GC + counting degrades rather than erroring: + - File absent, or a root has no prior pointer / a direct prior value + → report `0`. + - File present but unreadable, malformed TOML, `contents` not a table, + or a root's value not a string → report + `"would delete an unknown number of orphan chunks from the previous generation of `app_config` (unknown: could not read prior state)"` + for that root and continue. (The real push still fails fatally on + malformed TOML via the writer at `cli.rs:938`; only the dry-run + *count* degrades.) + - Prior value is pointer-kind-but-invalid (`prior_chunk_keys` → `Err`) + → report the same line with `(unknown: suspicious prior pointer)`. + +## Existing tests that MUST change + +- `second_oversized_push_converges_runtime_on_new_envelope` + (`cli.rs:3317`). Today it asserts old A-generation chunks **remain** + in the local `contents` table after push B (loop at `cli.rs:3416` + asserting `chunks_b.contains(chunk_key)`, and the "no GC in v1" + comments at `cli.rs:3303`/`:3324`/`:3360`/`:3405`/`:3414`). With GC + this inverts: after push B, every A-chunk MUST be **absent** + (`!chunks_b.contains(...)`), B-chunks present, and the runtime read + still reconstructs envelope B. Update the assertions and rewrite the + stale "no GC" commentary. + +## New tests + +Reuse the fake-`fastly` shim + tempdir/`DocumentMut` harnesses already +in the `cli.rs` test module. + +### `chunked_config.rs` — `prior_chunk_keys` + +- Valid v1 pointer → its `chunks[].key` in order. +- Direct `BlobEnvelope` → `Ok([])`. +- Unrelated / unparseable JSON → `Ok([])`. +- `edgezero_kind` ≠ `POINTER_KIND` → `Ok([])`. +- `version` ≠ 1 → `Err`. +- A `chunks[].key` outside `"{root_key}.__edgezero_chunks."` → `Err`, + returns no keys. + +### Cloud push (`push_config_entries`) + +- **Never deletes anything** — a push writes chunks + pointer, nothing else. +- Reserved-key and duplicate-root keys are hard errors before any I/O. +- Dry-run stays offline (no `list`/`describe`/`delete`). +- **Payload redaction**: schema-drift stdout *and* failing stderr never echo the + stored value (sentinel-secret tests on both the read and push paths). + +### `config gc` (`gc_config_entries`) + +- **Never deletes a live chunk**, however old it is (referenced by a root + pointer ⇒ untouchable). +- **Reclaims unreferenced chunks older than `--older-than`.** +- **Retains unreferenced chunks younger than `--older-than`.** +- **Dry-run** names every key + age it would delete, and deletes nothing. +- **Fails closed on an unreadable `created_at`** — aborts, deletes nothing. +- **Fails closed on an unclassifiable root** — aborts, deletes nothing. +- Delete argv passes `--key` + `--auto-yes` and **never** `--all`. + +### Local (`push_config_entries_local` / `write_fastly_local_config_store`) + +- **Prunes orphan chunk keys**: seed a `fastly.toml` whose `contents` + holds a prior pointer + its chunks; push a changed (still-chunked) + config → new chunk keys present, prior chunk keys absent, sibling + logical keys untouched. (This is the inverted `cli.rs:3317` test.) +- **Shrink-to-direct locally**: prior chunked → new direct → all prior + chunk keys removed, root holds the direct envelope. +- **Sibling coexistence preserved**: a push of `app_config` must not + remove `app_config_staging` chunk keys (Spec 12.7 coexistence). +- **Suspicious prior pointer (real push)**: seed the root with a + pointer-kind-but-invalid value (e.g. `version: 2`) **and** pre-seed + real chunk-like keys under the root namespace (so "no deletes" is + non-vacuous); a real local push of a new config returns a + suspicious-pointer warning, leaves the pre-seeded chunk keys present, + and still writes the new value. +- **Reserved key rejected**: `push_config_entries_local` with a logical + key containing `.__edgezero_chunks.` returns `Err` before touching + `fastly.toml` (file unchanged / not created). +- **Dry-run count**: reports the correct orphan count and writes + nothing. +- **Dry-run identical re-push counts 0**: seed a chunked config, then + dry-run the SAME bytes → reports `0` orphans (regression for computing + `new_keys` from the expanded, not logical, entries). +- **Dry-run degrades, never fails**: a malformed `fastly.toml` makes the + dry-run report `unknown` for the GC count while still printing the + direct-vs-chunked intent lines and returning `Ok`. +- **Dry-run suspicious prior pointer**: seed the root with a + pointer-kind-but-invalid value; the dry-run reports + `(unknown: suspicious prior pointer)` for that root and still returns + `Ok` without writing. + +## Non-goals + +- **Automatic cloud reclamation.** Out of scope because it is not achievable + safely (see "Cloud reclamation"). Cloud orphans — including those that predate + this feature — are reclaimed by the operator running `config gc`, which is IN + scope and implemented. +- **Transactional multi-key GC.** Each root key is swept independently; + there is no cross-key atomicity beyond what each path already + provides (per-entry for cloud, whole-file for local). + +## Files touched + +| File | Change | +| --- | --- | +| `crates/edgezero-adapter-fastly/src/chunked_config.rs` | `prior_chunk_keys` (validated, prefix-scoped) and `chunk_key_generation` (parses/validates a chunk key into its content address) + unit tests | +| `crates/edgezero-adapter-fastly/src/cli.rs` (cloud) | `reject_reserved_root_keys`, `reject_duplicate_root_keys`, `expand_root`, `list_config_store_entries`, `parse_rfc3339_secs`, `reclaim_orphan_generations`, `delete_config_store_entry` (`--key --auto-yes`, never `--all`), `gc_grace_secs` / `unix_now_secs` | +| `crates/edgezero-adapter-fastly/src/cli.rs` (local) | `write_fastly_local_config_store` takes exact per-root keep-sets and prunes in the same rewrite; `local_contents_table` + best-effort dry-run counts | +| `crates/edgezero-adapter-fastly/src/cli.rs` (diagnostics) | `redact_describe_response` + `redact_stderr` — diagnostics never echo a config payload | +| `crates/edgezero-adapter-fastly/Cargo.toml` | `handlebars` dev-dependency (fake-`fastly` test shim) |