From 3df37b2b85d4ac70a9055a98443f155cd74ddebd Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 1 Jul 2026 18:59:20 +0200 Subject: [PATCH] =?UTF-8?q?docs(suppress):=20flag=20advisory-by-default=20?= =?UTF-8?q?trust=20model;=20test=20dangling-suppress=20warning=20(=C2=A711?= =?UTF-8?q?=20tail=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings. 1. Per-app opt-out changes the group-template guarantee from "runs on every descendant" to "runs unless the descendant declines" — a footgun for compliance hooks (an audit/security trigger a tenant can silently opt out of). There is no non-suppressible flag. Document this in design §4.5 + CLAUDE.md, and record the deferred mitigation: a `sealed`/`mandatory` group-template marker the trigger anti-join + route filter skip (cheap — both filters are centralized). 2. The dangling-suppress warning path had no coverage. The suppress journey now applies a `[suppress] triggers=["does-not-exist"]`, asserts the apply still succeeds and the report warns "no effect". Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- crates/picloud-cli/tests/suppress.rs | 26 ++++++++++++++++++++++++++ docs/design/groups-and-project-tool.md | 14 ++++++++++++-- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0cdd65f..d95bf4d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ Authoritative design: [serverless_cloud_blueprint.md](serverless_cloud_blueprint **v1.1.x — SDK foundation + services — is complete.** The SDK shape (handle pattern, `::` namespaces, `Services`/`SdkCallCx`; see [docs/sdk-shape.md](docs/sdk-shape.md), stdlib at [docs/stdlib-reference.md](docs/stdlib-reference.md)) fixed in v1.1.0, then KV, docs, modules, HTTP, cron, files, pub/sub, email, users, and durable queues + `invoke()` filled it in through **v1.1.9** — blueprint §12 has the table. Earlier groundwork: blueprint Phase 3 (admin auth, multi-app scoping, Phase 3.5 capability gating — `manager-core::authz::{can, require, Capability}`, migration `0006_users_authz.sql`). -**Current focus: v1.2 _Hierarchies_ — groups + the declarative project tool** ([docs/design/groups-and-project-tool.md](docs/design/groups-and-project-tool.md)). That doc's §11 uses its own **Phase 1–6 numbering, distinct from the blueprint product-phase numbering above — do not conflate them** (its "Phase 3" = group-inherited config, not admin auth). Implemented on `feat/groups-*` branches: §11 Phase 1 (declarative `pic plan`/`apply`/`prune` + env overlays), Phase 2 (single-parent groups tree + hierarchy-aware RBAC), Phase 3 (group-inherited, env-scoped `vars` + secrets resolved **live** via a recursive CTE — no materialized cache), Phase 4-lite (group-owned **endpoint** scripts: `scripts` polymorphic owner in `0050_group_scripts.sql`, `get_by_name_inherited`/`is_invocable_by_app` chain resolution, inherited `invoke()` + declarative route/trigger binding — all **live**, no body materialization), Phase 5 (the **declarative project tool maps onto the group tree**: the reconcile engine generalized to `ApplyOwner{App|Group}`, a `[group]` manifest kind, and a single atomic **tree apply** — `pic plan/apply --dir` reconciles a whole directory tree of `picloud.toml` nodes in one Postgres transaction, groups-before-apps so an app route can bind a group script created in the same tx; the bound token folds in each group's `structure_version`. Multi-repo single-owner/attach-point and per-env approval gating are deferred; groups pre-exist), Phase 4b (group **modules** + the **lexical (sealed-by-default) import resolver**, §5.5: owner-polymorphic `ModuleScript`, origin-rooted `ModuleSource::resolve` walking the importing node's chain, `ExecRequest.script_owner` threaded from every dispatch + `invoke()` site, `_source`-driven lexical chaining in `PicloudModuleResolver` with the compiled-module cache re-keyed by `ScriptId`, group modules/imports allowed, single-node dangling-import `plan` check — an inherited group script's imports **seal to the group**, a leaf can't shadow them), §5.5 **extension points** (opt-in polymorphism — **§5.5 now complete**: marker table `0051_extension_points.sql` (owner-polymorphic, CASCADE — structurally a `secrets` name; default body = a co-located `kind=module` script), `ModuleSource::resolve_policy` with **nearest-declaration-kind-wins** — a concrete module resolves lexically, an EP marker resolves **dynamically against the inheriting app** (its override else the default body up-chain), `NoProvider` is a hard error; declarative-only authoring via the `[app]`/`[group]` manifest key `extension_points = [...]`, reconcile mirrors `secrets`, single-node no-provider `plan` check, read-only `pic extension-points ls` + `pull` round-trip — the app can **override** a group default, the deliberate inverse of the Phase 4b sealed import), §11.6 **group-level collections — KV + DOCS + FILES slices** (full cross-app shared read/write: a group declares a collection shared via the `[group]` manifest `collections = [...]` → owner-polymorphic marker `0052_group_collections.sql` with a `kind` discriminator + a per-kind group-keyed store: `0053_group_kv_entries.sql` (`kind='kv'`), `0054_group_docs.sql` (`kind='docs'`, the queryable-JSON store), and `0055_group_files.sql` (`kind='files'`, blob metadata in Postgres + bytes on disk under `/files/groups//...`, a `groups/` infix disjoint from the per-app `files//` subtree so the existing recursive orphan sweeper covers both with zero change) — no `app_id`, a shared row belongs to the group; CASCADE on group delete, an app delete leaves the data. Scripts use the **explicit** `kv::shared_collection("name")` / `docs::shared_collection("name")` / `files::shared_collection("name")` handles (`shared` alone is a Rhai reserved word); `GroupKv`/`GroupDocs`/`GroupFilesServiceImpl` resolve the owning group from `cx.app_id`'s ancestor chain **filtered by kind** (nearest-wins) — **that walk is the isolation boundary**, a foreign app gets `CollectionNotShared`; a `kv`, a `docs`, and a `files` collection of the same name are distinct stores. The docs slice reuses the `docs_filter` DSL — `build_find_query` generalized on its owner column (`docs`/`app_id` vs `group_docs`/`group_id`, both literals); the files slice likewise generalized the atomic-write + checksum-on-read path helpers on an owner-relative dir (one source for the security-sensitive disk mechanics). **Reads open** to any subtree script (anonymous incl. — the declaration is the grant), **writes require an authenticated editor+** on the owning group (`GroupKvRead/Write`, `GroupDocsRead/Write`, `GroupFilesRead/Write`, `script_gate_require_principal` fails closed on anon). Declarative authoring is the **string-or-table** form `collections = ["catalog", { name = "articles", kind = "docs" }, { name = "assets", kind = "files" }]` (bare string = kv); reconcile keys markers by `(name, kind)`; read-only `pic collections ls --group` shows a kind column. Deferred: shared-collection triggers (the "group trigger has no app to watch" problem), topics/queue shared collections (same trigger-centric gap), per-group quotas, an operator admin API for shared blobs (scripts use the SDK; `pic collections ls` shows the marker)), and **§4.5 group TRIGGER templates** (live, event kinds — a `[group]` declares a `[[triggers.kv|docs|files|pubsub]]` template binding a group-owned handler; `triggers` gained a polymorphic owner `0056_group_triggers.sql` mirroring `0050`; the dispatcher's `list_matching_kv/docs/files` + the pubsub publish fan-out prepend `CHAIN_LEVELS_CTE` + `JOIN chain c ON (t.app_id = c.app_owner OR t.group_id = c.group_owner)` so a descendant app's event matches its own triggers **plus** ancestor-group templates in one query, the handler running under the firing `app_id` — **the chain walk is the isolation boundary**, a sibling-subtree app never matches; stateful kinds cron/queue/email rejected on a group (need materialization), per-app opt-out deferred; read-only `pic triggers ls --group`), and **§4.5 group ROUTE templates** (live, inherited — a `[group]` declares a `[[routes]]` template binding a group-owned endpoint; `routes` gained a polymorphic owner `0057_group_routes.sql` mirroring `0056`. Unlike triggers (per-event SQL), routes serve from the in-memory `RouteTable`, so the HTTP hot path can't resolve inheritance per request — instead the table **rebuild** expands templates into each descendant app's slice via `RouteRepository::list_effective` (all-apps generalization of `CHAIN_LEVELS_CTE`: every app × its ancestor chain ⋈ routes), and `compile_effective_routes` applies **nearest-owner-wins shadowing** (an app's own identical binding shadows the inherited template; non-identical bindings coexist under the matcher's existing precedence — a route picks one winner, unlike a fanning trigger). Because the table is a cache, inheritance is rebuilt **full-live** through the single `rebuild_route_table` chokepoint on every edge that changes it: route CRUD, apply, **and tree mutations** — app create/delete (`apps_api`) + group reparent (`groups_api`) — so a new app under a group serves its templates instantly. Host-claim validation is skipped for a group template (descendants serve it on their own host claim; templates use `host_kind = any`). **The chain expansion is the isolation boundary** — a sibling-subtree app never inherits (pinned by `tests/group_route_templates.rs` + the `group_routes` journey); read-only `pic routes ls --group`. Deferred: multi-node snapshot propagation), and **§11 tail per-app opt-out (template suppression)** (a descendant declines an inherited group template: an `[app]` declares `[suppress]` with `triggers = [...]` (handler script names) + `routes = [...]` (paths) — **coarse by reference**, not a full definition, since template row-ids churn on re-apply but a reference is stable (re-apply NoOp, may decline several templates bound to the same script/path). App-only marker `0058_template_suppressions.sql` (`app_id NOT NULL` CASCADE, a `target_kind` discriminator), reconciled with the extension-point marker pattern (prunable → re-inherits). Consumed at the two resolution points: the trigger dispatch queries gain a correlated `NOT EXISTS` anti-join (gated to `t.group_id IS NOT NULL`), and `compile_effective_routes` drops an inherited (`depth > 0`) route at a suppressed path (loaded via `RouteRepository::list_route_suppressions`). **Inheritance-only** — the `group_id IS NOT NULL` / `depth > 0` gates mean an app can only decline what it inherits, never its own or a sibling's (pinned by `tests/template_suppression.rs` + the `suppress` journey); a dangling suppress is an apply-time warning; read-only `pic suppress ls --app`. Deferred: group-level suppression). Next: multi-node cluster mode, or extend templates to the stateful kinds, or multi-repo ownership. +**Current focus: v1.2 _Hierarchies_ — groups + the declarative project tool** ([docs/design/groups-and-project-tool.md](docs/design/groups-and-project-tool.md)). That doc's §11 uses its own **Phase 1–6 numbering, distinct from the blueprint product-phase numbering above — do not conflate them** (its "Phase 3" = group-inherited config, not admin auth). Implemented on `feat/groups-*` branches: §11 Phase 1 (declarative `pic plan`/`apply`/`prune` + env overlays), Phase 2 (single-parent groups tree + hierarchy-aware RBAC), Phase 3 (group-inherited, env-scoped `vars` + secrets resolved **live** via a recursive CTE — no materialized cache), Phase 4-lite (group-owned **endpoint** scripts: `scripts` polymorphic owner in `0050_group_scripts.sql`, `get_by_name_inherited`/`is_invocable_by_app` chain resolution, inherited `invoke()` + declarative route/trigger binding — all **live**, no body materialization), Phase 5 (the **declarative project tool maps onto the group tree**: the reconcile engine generalized to `ApplyOwner{App|Group}`, a `[group]` manifest kind, and a single atomic **tree apply** — `pic plan/apply --dir` reconciles a whole directory tree of `picloud.toml` nodes in one Postgres transaction, groups-before-apps so an app route can bind a group script created in the same tx; the bound token folds in each group's `structure_version`. Multi-repo single-owner/attach-point and per-env approval gating are deferred; groups pre-exist), Phase 4b (group **modules** + the **lexical (sealed-by-default) import resolver**, §5.5: owner-polymorphic `ModuleScript`, origin-rooted `ModuleSource::resolve` walking the importing node's chain, `ExecRequest.script_owner` threaded from every dispatch + `invoke()` site, `_source`-driven lexical chaining in `PicloudModuleResolver` with the compiled-module cache re-keyed by `ScriptId`, group modules/imports allowed, single-node dangling-import `plan` check — an inherited group script's imports **seal to the group**, a leaf can't shadow them), §5.5 **extension points** (opt-in polymorphism — **§5.5 now complete**: marker table `0051_extension_points.sql` (owner-polymorphic, CASCADE — structurally a `secrets` name; default body = a co-located `kind=module` script), `ModuleSource::resolve_policy` with **nearest-declaration-kind-wins** — a concrete module resolves lexically, an EP marker resolves **dynamically against the inheriting app** (its override else the default body up-chain), `NoProvider` is a hard error; declarative-only authoring via the `[app]`/`[group]` manifest key `extension_points = [...]`, reconcile mirrors `secrets`, single-node no-provider `plan` check, read-only `pic extension-points ls` + `pull` round-trip — the app can **override** a group default, the deliberate inverse of the Phase 4b sealed import), §11.6 **group-level collections — KV + DOCS + FILES slices** (full cross-app shared read/write: a group declares a collection shared via the `[group]` manifest `collections = [...]` → owner-polymorphic marker `0052_group_collections.sql` with a `kind` discriminator + a per-kind group-keyed store: `0053_group_kv_entries.sql` (`kind='kv'`), `0054_group_docs.sql` (`kind='docs'`, the queryable-JSON store), and `0055_group_files.sql` (`kind='files'`, blob metadata in Postgres + bytes on disk under `/files/groups//...`, a `groups/` infix disjoint from the per-app `files//` subtree so the existing recursive orphan sweeper covers both with zero change) — no `app_id`, a shared row belongs to the group; CASCADE on group delete, an app delete leaves the data. Scripts use the **explicit** `kv::shared_collection("name")` / `docs::shared_collection("name")` / `files::shared_collection("name")` handles (`shared` alone is a Rhai reserved word); `GroupKv`/`GroupDocs`/`GroupFilesServiceImpl` resolve the owning group from `cx.app_id`'s ancestor chain **filtered by kind** (nearest-wins) — **that walk is the isolation boundary**, a foreign app gets `CollectionNotShared`; a `kv`, a `docs`, and a `files` collection of the same name are distinct stores. The docs slice reuses the `docs_filter` DSL — `build_find_query` generalized on its owner column (`docs`/`app_id` vs `group_docs`/`group_id`, both literals); the files slice likewise generalized the atomic-write + checksum-on-read path helpers on an owner-relative dir (one source for the security-sensitive disk mechanics). **Reads open** to any subtree script (anonymous incl. — the declaration is the grant), **writes require an authenticated editor+** on the owning group (`GroupKvRead/Write`, `GroupDocsRead/Write`, `GroupFilesRead/Write`, `script_gate_require_principal` fails closed on anon). Declarative authoring is the **string-or-table** form `collections = ["catalog", { name = "articles", kind = "docs" }, { name = "assets", kind = "files" }]` (bare string = kv); reconcile keys markers by `(name, kind)`; read-only `pic collections ls --group` shows a kind column. Deferred: shared-collection triggers (the "group trigger has no app to watch" problem), topics/queue shared collections (same trigger-centric gap), per-group quotas, an operator admin API for shared blobs (scripts use the SDK; `pic collections ls` shows the marker)), and **§4.5 group TRIGGER templates** (live, event kinds — a `[group]` declares a `[[triggers.kv|docs|files|pubsub]]` template binding a group-owned handler; `triggers` gained a polymorphic owner `0056_group_triggers.sql` mirroring `0050`; the dispatcher's `list_matching_kv/docs/files` + the pubsub publish fan-out prepend `CHAIN_LEVELS_CTE` + `JOIN chain c ON (t.app_id = c.app_owner OR t.group_id = c.group_owner)` so a descendant app's event matches its own triggers **plus** ancestor-group templates in one query, the handler running under the firing `app_id` — **the chain walk is the isolation boundary**, a sibling-subtree app never matches; stateful kinds cron/queue/email rejected on a group (need materialization), per-app opt-out deferred; read-only `pic triggers ls --group`), and **§4.5 group ROUTE templates** (live, inherited — a `[group]` declares a `[[routes]]` template binding a group-owned endpoint; `routes` gained a polymorphic owner `0057_group_routes.sql` mirroring `0056`. Unlike triggers (per-event SQL), routes serve from the in-memory `RouteTable`, so the HTTP hot path can't resolve inheritance per request — instead the table **rebuild** expands templates into each descendant app's slice via `RouteRepository::list_effective` (all-apps generalization of `CHAIN_LEVELS_CTE`: every app × its ancestor chain ⋈ routes), and `compile_effective_routes` applies **nearest-owner-wins shadowing** (an app's own identical binding shadows the inherited template; non-identical bindings coexist under the matcher's existing precedence — a route picks one winner, unlike a fanning trigger). Because the table is a cache, inheritance is rebuilt **full-live** through the single `rebuild_route_table` chokepoint on every edge that changes it: route CRUD, apply, **and tree mutations** — app create/delete (`apps_api`) + group reparent (`groups_api`) — so a new app under a group serves its templates instantly. Host-claim validation is skipped for a group template (descendants serve it on their own host claim; templates use `host_kind = any`). **The chain expansion is the isolation boundary** — a sibling-subtree app never inherits (pinned by `tests/group_route_templates.rs` + the `group_routes` journey); read-only `pic routes ls --group`. Deferred: multi-node snapshot propagation), and **§11 tail per-app opt-out (template suppression)** (a descendant declines an inherited group template: an `[app]` declares `[suppress]` with `triggers = [...]` (handler script names) + `routes = [...]` (paths) — **coarse by reference**, not a full definition, since template row-ids churn on re-apply but a reference is stable (re-apply NoOp, may decline several templates bound to the same script/path). App-only marker `0058_template_suppressions.sql` (`app_id NOT NULL` CASCADE, a `target_kind` discriminator), reconciled with the extension-point marker pattern (prunable → re-inherits). Consumed at the two resolution points: the trigger dispatch queries gain a correlated `NOT EXISTS` anti-join (gated to `t.group_id IS NOT NULL`), and `compile_effective_routes` drops an inherited (`depth > 0`) route at a suppressed path (loaded via `RouteRepository::list_route_suppressions`). **Inheritance-only** — the `group_id IS NOT NULL` / `depth > 0` gates mean an app can only decline what it inherits, never its own or a sibling's (pinned by `tests/template_suppression.rs` + the `suppress` journey); a dangling suppress is an apply-time warning; read-only `pic suppress ls --app`. **Trust-model consequence:** group templates are now advisory-by-default (run *unless* a descendant declines) — a footgun for compliance hooks (audit/security triggers a tenant can opt out of); there is no non-suppressible flag yet. Deferred: a `sealed`/`mandatory` group-template marker the filters skip, and group-level suppression). Next: multi-node cluster mode, or extend templates to the stateful kinds, or multi-repo ownership. **Data-model invariant:** app-owned data-plane tables (KV, docs, files, …) start with `app_id UUID NOT NULL REFERENCES apps(id) ON DELETE CASCADE`; the group-inheritable tables — _config_ (`vars`, `secrets`) and now group-owned _code_ (`scripts`, `0050`) — instead carry a **polymorphic owner**: nullable `group_id` and `app_id` with an exactly-one CHECK and per-owner partial-unique indexes (config is `ON DELETE CASCADE`, scripts `RESTRICT` — code is not data). Inheritance resolves **live** down `apps.group_id → groups.parent_id` via `CHAIN_LEVELS_CTE` (no materialized view); nearest-owner-wins with an app's own row shadowing the inherited one (CoW). Every Rhai SDK call resolves its app from `cx.app_id`, never a script-passed arg, and a group script always runs under the *inheriting* app's `cx.app_id` (the cross-app isolation boundary). diff --git a/crates/picloud-cli/tests/suppress.rs b/crates/picloud-cli/tests/suppress.rs index ddc6ed3..2bfc33f 100644 --- a/crates/picloud-cli/tests/suppress.rs +++ b/crates/picloud-cli/tests/suppress.rs @@ -161,6 +161,32 @@ fn app_suppresses_inherited_route_template() { "re-apply should not create a second suppression:\n{report}" ); + // A suppress reference matching NO inherited template is a soft warning + // (typo guard), not an error — the apply still succeeds. + let dangling = format!( + "[app]\nslug = \"{blog}\"\nname = \"Blog\"\n\n\ + [suppress]\nroutes = [\"/ghello\"]\ntriggers = [\"does-not-exist\"]\n" + ); + fs::write(&apath, &dangling).unwrap(); + let out = common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&apath) + .output() + .expect("apply dangling"); + assert!( + out.status.success(), + "a dangling suppress must not fail apply" + ); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert!( + combined.contains("does-not-exist") && combined.to_lowercase().contains("no effect"), + "a dangling suppress must warn that it matches no inherited template:\n{combined}" + ); + // Drop the [suppress] block + prune → blog re-inherits the route. let bare = format!("[app]\nslug = \"{blog}\"\nname = \"Blog\"\n"); fs::write(&apath, &bare).unwrap(); diff --git a/docs/design/groups-and-project-tool.md b/docs/design/groups-and-project-tool.md index b566127..d30b260 100644 --- a/docs/design/groups-and-project-tool.md +++ b/docs/design/groups-and-project-tool.md @@ -436,8 +436,18 @@ Two distinct constraints: > is inheritance-only** — the `group_id IS NOT NULL` / `depth > 0` gates mean an app can only decline > what it inherits, never its own resource nor a sibling's (pinned by `tests/template_suppression.rs`). > A dangling suppress (matching no inherited template) is an apply-time **warning**, not an error; -> read-only `pic suppress ls --app`. **Deferred:** group-level suppression (decline an ancestor's -> template for a whole subtree). +> read-only `pic suppress ls --app`. +> +> **Trust-model consequence — group templates are now advisory-by-default.** Before opt-out, a group +> template was *guaranteed* to run on every descendant; with suppression the guarantee weakens to "runs +> unless the descendant declines." This is fine for convenience templates (a shared webhook, a default +> route) but is a **footgun for compliance** templates — an audit-logging or security trigger a +> multi-tenant operator deploys at a group can be silently opted out of by a tenant app. There is +> currently **no non-suppressible flag**; an operator who needs an unconditional hook must not rely on a +> group template for it (deploy it per-app, or gate at the platform layer). **Deferred:** a `sealed` / +> `mandatory` marker on a group template that the trigger anti-join + route filter skip (making it +> non-declinable) — cheap to add since both filters are centralized; and group-level suppression +> (decline an ancestor's template for a whole subtree). ### 4.6 Secrets & `pull`