Lift the Phase-4-lite restrictions now that import resolution is lexical:
- `create_group_script` (group_scripts_api): allow `kind = module` and
group scripts with `import`s; record the import edges. Module-shape and
sandbox-ceiling validation stay.
- The declarative apply path already reconciles group modules generically
(`BundleScript.kind` + `owner.script_owner()` flow through
`reconcile_node_tx`); no change needed beyond stale-comment fixes.
Adds the §5.5 dangling-import plan check (single-node plan/apply):
`check_imports_resolve` rejects an `import "<m>"` that resolves to neither a
bundle-local module nor a module reachable lexically up the node's chain —
caught at plan time instead of as a runtime 404. Roots resolution at the
node's owner (an app node reaches ancestor group modules; a group node walks
its own ancestry). ApplyService gains an injected `ModuleSource`. The tree
path opts out (a node may import a module created by another node in the same
uncommitted tx); the runtime check is the backstop there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Carry the resolved script's owner (its defining node) from dispatch into
the executor so `import` resolution can be lexical (§5.5). The executing
app (`app_id`) stays the SDK isolation boundary; `script_owner` is a
separate axis — the lexical origin for imports.
- `ExecRequest.script_owner: Option<ScriptOwner>` (serde default; `None`
falls back to `App(app_id)` in the engine for old payloads / cluster wire).
- `ScriptOwner` gains `Serialize`/`Deserialize` for the wire.
- The engine computes `default_origin` and hands it to the resolver
(used fully in C3; the resolve() lookup already uses it).
- Every dispatch site sets it from the resolved `Script.owner()`:
the 4 dispatcher arms (queue/trigger/http/invoke_async, via a new
`ResolvedTrigger.script_owner`), the orchestrator id-bypass, and the
`invoke()` SDK path (new `ResolvedScript.owner`, so a group script
invoked by an app resolves imports from the group).
Behaviour-preserving: app scripts still resolve `App(app_id)`; group
scripts can't yet carry imports (lifted in C4).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the module-loading contract origin-aware so group-owned modules
become resolvable and imports can resolve lexically (§5.5).
- `ModuleScript` gains a polymorphic owner (`app_id`/`group_id` + `owner()`),
mirroring `Script`.
- `ModuleSource::lookup(&cx, name)` -> `resolve(origin: ScriptOwner, name)`:
resolution walks the group chain rooted at the importing script's
defining node (nearest-owner-wins), not the inheriting app's view.
- `PostgresModuleSource::resolve` branches on origin: app-rooted
(`CHAIN_LEVELS_CTE`) or a new group-rooted `GROUP_CHAIN_LEVELS_CTE`,
joining `kind='module'` rows.
The executor resolver passes a temporary `App(cx.app_id)` origin (behaviour-
preserving for app scripts; true lexical origin lands in C3). Group scripts
still can't carry imports until C4, so no trust-inversion window opens here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 5 C3 — the headline. A whole project subtree (group + app nodes) is
reconciled in ONE Postgres transaction, all-or-nothing.
`POST /api/v1/admin/tree/{plan,apply}` take a `TreeBundle { nodes: [{kind,
slug, bundle}] }`. The actor must hold the relevant read/write caps for EVERY
node (per-kind, widened on prune) — `authz_tree` + reusable
`require_{app,group}_node_writes` (now shared with the single-node handlers).
Engine:
* The in-tx reconcile body of `apply_owner` is extracted to `reconcile_node_tx`
(returns the node's post-create `name → script_id`), so single-node and
tree apply share one implementation — behaviour-preserving (391 unit tests
+ app journeys green).
* `prepare_tree` resolves each slug→owner, validates (an app's route/trigger
target may bind to an in-tree ancestor group's script), computes each
node's plan, and folds a combined bound-plan token = every node's
`state_token` + every in-scope group's `structure_version` (so a reparent
or content edit between plan and apply trips StateMoved).
* `apply_tree`: lock every node key (sorted, deadlock-free); reconcile GROUPS
first, recording each group's `name → id` in an in-memory index; then APPS,
resolving inherited route/trigger targets nearest-ancestor-wins across the
in-tree index (incl. scripts created earlier in THIS tx, invisible to the
pool resolver) and pre-existing out-of-tree ancestors. One commit, one
post-commit route refresh.
Live-validated against the dev DB:
* Headline: a group node creates `shared`; an app node in the SAME apply
binds `GET /greet` to it — the route ends up bound to the group-owned
script, and re-plan is all-noop (idempotent).
* Atomicity: one invalid node → 422 and ZERO rows written (the valid group
node's script is not created).
CLI tree discovery + `pic plan/apply` tree mode is C4; journeys + docs are C5.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 5 C1. Generalize the reconcile engine from app-only to an `ApplyOwner
{ App | Group }`, so a GROUP's own config + scripts can be declaratively
reconciled — the foundation for the tree-spanning project tool.
A group node is a subset of an app node (scripts + vars only; no routes /
triggers / secrets-values), so the diff, script/route/trigger/var CRUD, prune,
idempotency, and the Phase-4 inherited-target resolution are all reused
unchanged. Only the owner-varying bits switch on `ApplyOwner`:
* `load_current` — `list_for_group` + `VarOwner::Group` for a group (empty
routes/triggers/secrets); `list_for_app` for an app, exactly as before.
* script create owner (`NewScript{app_id|group_id}`), var writes (new
`set_group_var_tx`/`delete_group_var_tx` mirroring the app variants at
scope `*`), and the advisory lock (owner-tagged).
* `validate_bundle_for` rejects routes/triggers on a group bundle (422);
route-host validation, inherited-target resolution, the post-commit route
refresh, and the unreachable-endpoint warning are app-only.
`apply`/`plan` are unchanged thin wrappers over the new `apply_owner`/
`plan_owner`. New endpoints `POST /groups/{id}/{plan,apply}` gated by
`GroupScripts{Read,Write}` / `GroupVarsWrite`. `ApplyService` gains a `groups`
repo (the tree apply in C3 needs it too).
Live-validated: group plan → apply (group-owned script + var) → idempotent
re-plan noop → routes rejected 422 → DB confirms group ownership. App apply
unchanged: 391 manager-core unit tests + the apply/prune/staleness/overlay
journeys all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review of Phase 4-lite found two real gaps, both rooted in the
apply engine resolving script `id → name` from `current.scripts` (app-own)
only — so a route/trigger bound to an inherited GROUP script (whose id is not
app-own) couldn't be named:
* **Prune gap (reachable via pure declarative apply):** a trigger bound to an
inherited group script, once dropped from the manifest, never diffed to a
Delete (the diff couldn't resolve its identity once the bundle stopped
referencing the name), so `apply --prune` silently orphaned it.
* **Bound-plan false-negative:** `state_token` excluded such a trigger from
its fingerprint, so the StateMoved check missed a concurrent edit to that
binding class. (Routes were already safe — they hash the raw `script_id`.)
Fix: `resolve_current_target_names` resolves the names of group scripts the
app's CURRENT routes/triggers are bound to (by id, independent of the bundle),
and that map now feeds `compute_diff_with_names`, `state_token_with_names`, and
the prune trigger-identity map. App-owned bindings are unaffected (their names
are already in `current.scripts`). Also fixes a stale "in this app" conflict
message for group-owned scripts.
New journey `inherited_bound_trigger_is_pruned_when_dropped` proves prune now
removes an inherited-bound cron trigger. Full journey suite 105/105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 4-lite C4 — the headline declarative path. An app's manifest can now bind
a `[[routes]]` / `[[triggers]]` to a script it does NOT declare, resolving the
name to an inherited group-owned endpoint on the app's chain (nearest-owner
wins; an app's own script of the same name still shadows it — CoW).
* `resolve_inherited_targets(app_id, bundle)` resolves every route/trigger
target name that isn't an app-own manifest script to a group endpoint via
`get_by_name_inherited`, returning `name → script_id`. Run once before the
apply transaction (group scripts pre-exist, committed).
* `validate_bundle` takes the inherited endpoint names so "binds to unknown
script" / "is a module" checks pass for inherited targets.
* The apply transaction merges the inherited targets into `name_to_id`
(app-own keeps precedence), so route/trigger inserts bind to the group
script's id.
* `compute_diff_with_inherited` adds the inherited ids → names to the diff's
name map, so a route bound to a group script resolves its name and
re-apply is idempotent (without this it read as a perpetual rebind).
Live-validated: an app under group G with a manifest binding `GET /greet` to
the group's inherited `greet` plans as `route create → greet` (no script
create), applies (+1 route, +0 scripts), and re-plans as `noop` (idempotent).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 4-lite C3. A descendant app can now resolve and run a group-owned script
inherited down its ownership chain — the core of group-script inheritance.
Resolver (repo, reusing config_resolver's CHAIN_LEVELS_CTE):
* `get_by_name_inherited(app_id, name)` — walk app → ancestor groups
nearest-first, return the nearest script of that name (an app's own script
shadows an inherited group one: CoW).
* `is_invocable_by_app(script_id, app_id)` — EXISTS over the same chain; the
cross-app isolation predicate generalized to the hierarchy.
invoke(): `invoke("name", …)` resolves via `get_by_name_inherited`, so a script
can call a shared group endpoint by name. A group script always runs under the
*caller's* app context, never its owner.
Dispatch guards (the isolation backstops touched in C1) are now chain-aware via
a `Dispatcher::script_invocable` helper and the chain-aware
`validate_trigger_target`: app-owned scripts keep the in-memory fast path (no
query, behavior byte-identical to the pre-Phase-4 same-app guard); only a
group-owned candidate pays one chain query, and a non-member / query error
fails closed (dead-letter or reject — never run under another app's SdkCallCx).
The orchestrator HTTP route path needs no change: its per-app route trie
already scopes routes to one app, and it dispatches under the route's app
without reading script.app_id — a route bound to a group script just works.
Live-validated against the dev DB: an app under group G invoking the group's
`shared` returns its output; an app NOT under G cannot resolve it (isolation);
and after the app deploys its own `shared`, the same call returns the app's
version (CoW shadowing). Route/trigger binding of inherited scripts via the
declarative engine is C4.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 4-lite C2. Lets a group own scripts (templates inherited by descendant
apps), with the create/list/manage surface — but not yet the inherited
resolution (binding + runtime), which is C3.
Capabilities: add `GroupScriptsRead` (viewer+ on the group → script:read) and
`GroupScriptsWrite` (editor+ → script:write), mirroring the group-vars tier and
resolved through the same group-ancestor walk.
API:
* New `group_scripts_api`: `POST/GET /groups/{id}/scripts` — create a
group-owned endpoint script and list a group's own (non-inherited) rows.
Phase 4-lite is endpoint-only and self-contained: `kind=module` and any
`import` are rejected (group modules + the lexical resolver are Phase 4b).
Owner resolved first (slug-or-uuid); capability bound to the resolved id.
* The by-id `/scripts/{id}` get/update/delete/logs handlers are now
owner-polymorphic via a `script_cap` helper: app-owned scripts gate on
`App*` exactly as before; group-owned on `GroupScripts*`. This is what
makes `deploy --group` idempotent (update reuses the by-id PUT) and lets a
group script be deleted by id. (C1 had these fail closed for groups.)
Repo: `list_for_group(group_id)` (the group's own rows).
CLI: `pic scripts ls --group <g>`, `pic scripts deploy --group <g>` (create or
update by name; `--app`/`--group` are mutually exclusive, exactly one
required). `pic scripts delete <id>` already works for group scripts via the
owner-polymorphic by-id route.
Live-validated against the dev DB: create → update (v2 via the by-id PUT) →
list → delete, plus module rejection and the polymorphic row shape
(`app_id NULL`, `group_id` set). Group scripts still can't be routed/triggered
or invoked — that lands in C3.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 4-lite C1. Make script ownership polymorphic so a script can be owned
by a GROUP (a template inherited by descendant apps) instead of an app —
mirroring vars/secrets (0048/0049), but ON DELETE RESTRICT (code is not data).
Schema (0050): `scripts.group_id` (nullable FK→groups RESTRICT), `app_id` made
nullable, `scripts_owner_exactly_one` CHECK, and the per-app name index split
into two per-owner partial-unique indexes. Existing app-owned rows keep their
exact `(app_id, lower(name))` uniqueness.
Type: `Script.app_id` becomes `Option<AppId>`; add `Script.group_id` +
`ScriptOwner` / `is_owned_by_app()`. `NewScript` gains the same polymorphic
owner. The execution-context app (what a script runs *under*) is supplied by
the invoking route/trigger/caller, never read off the script — group scripts
have no single app.
Behavior is fully preserved for app-owned scripts (the only kind creatable
today): every isolation backstop and authz site now uses `is_owned_by_app`,
which is byte-identical for app owners and **fails closed** for group owners.
A group script therefore can't yet be run, route/trigger-bound, invoked, or
managed via the app-script API — those land in C2 (group-script creation) and
C3 (chain-membership resolution + binding). The `/execute/{id}` bypass 404s a
group script (no app context to run under).
Re-blesses expected_schema.txt; note the golden was last blessed at migration
0044, so this also captures the already-committed 0045–0049 schema.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends the Phase-1 apply engine to manage app `vars` declaratively,
alongside scripts/routes/triggers/secrets. The `Bundle` gains a `vars`
map (key → JSON value; values are non-secret, so they live inline, unlike
secret names). `diff_vars` is value-sensitive: a changed value is an
Update, a live var the manifest dropped is a Delete (applied only under
`--prune` — vars are prunable config, unlike never-pruned secrets).
Writes go through `set_app_var_tx`/`delete_app_var_tx` inside the apply
transaction (mirroring `PostgresVarsRepo::set` for `VarOwner::App`, scope
`*`), so they commit atomically with the rest of the apply and roll back
together on failure. `CurrentState` loads the app's OWN scope-`*`,
non-tombstone vars (inherited group vars and tombstones are out of scope
for the manifest); `state_token` folds them in so the bound-plan check
catches an out-of-band `pic vars set` between plan and apply. Keys are
validated against the same kebab rule as the vars admin API, and the
apply handler now requires `AppVarsWrite` when vars are touched or
`--prune` is set.
Scope: app-owned vars at scope `*` (an app *is* an environment). Group
vars via manifest and tombstone-via-manifest wait for the nested-manifest
model. Unit tests cover the create/update/noop/delete diff and the
state-token value sensitivity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`list_meta` orders by `(name, environment_scope)` — a group secret name
can carry several env-scoped rows — but the cursor encoded only `name`
and paginated with `name > $cursor`. When a name's scopes straddled a
page boundary, the remaining scope rows were silently skipped from the
admin listing. Switch to a composite `(name, environment_scope)` keyset
(base64url of `name \x1f scope`) and a Postgres row-comparison predicate.
App secrets (single `*` scope per name) were unaffected; group secrets
with multi-scope names now page completely. Found in final branch review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the Phase-3 admin surface on top of the group-secrets storage:
* `secrets_api` gains group routes under `/groups/{id}/secrets`
(set/list/delete, env-scoped) gated `GroupSecretsWrite` (editor+), plus
the ONE plaintext endpoint `GET /groups/{id}/secrets/{name}/value` gated
`GroupSecretsRead` (group_admin only). That is the masked-secret
boundary: a descendant app's dev sees a group secret EXISTS and consumes
it at runtime via `secrets::get`, but only a reader at the OWNING group
gets the value. App secrets stay env-agnostic (a stray `env` is rejected).
The owner is resolved first, then the capability binds to the resolved
id — never a path param.
* `config_api`: `GET /apps/{id}/config/effective` (gated `AppVarsRead`)
returns the resolved view a dev would get — every inherited var with its
value + provenance, and every inherited secret MASKED (name/owner/scope,
never the value). Backed by a new `fetch_effective_secret_meta`
(DISTINCT-ON nearest-wins, same ordering as the per-name resolver).
* authz: `GroupSecretsWrite` moves from `app:admin` to `script:write`
scope so its API-key scope matches its editor role tier (closing the
latent scope/role mismatch the checkpoint review flagged); the value
read `GroupSecretsRead` stays at `app:admin`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migration 0049 reshapes `secrets` to the same polymorphic-owner contract
as `vars` (0048): a secret is owned by an app XOR an ancestor group,
carries an `environment_scope`, and the old PK `(app_id, name)` becomes
two partial-unique indexes `(owner, environment_scope, name)`. Existing
rows backfill to `app_id` + scope `'*'`; the v1 AAD does NOT include the
scope, so every current ciphertext keeps decrypting byte-for-byte.
`SecretsRepo` is generalised to be owner+scope keyed (`SecretOwner` moves
down from `secrets_service` and is re-exported for path stability). The
SDK read path now goes through `SecretsRepo::resolve`, which reuses the
shared `CHAIN_LEVELS_CTE` to walk app→ancestor-group→root, env-filters,
and takes the nearest level (`@E` beating `*` within a level) — returning
the winning owner so the value is decrypted under the AAD it was sealed
with. Runtime injection stays anchored to `cx.app_id`: an app only ever
resolves its own and its ancestors' secrets.
All callers updated to owner+scope (`SecretOwner::App(_)`, scope `'*'`):
the app-secrets admin API, the SDK service, and the apply email-secret
path. The 21 secrets unit tests (incl. the app/group AAD-disjointness and
cross-row swap proofs) stay green; the chain-walk was live-verified
against Postgres (nearest-wins + env-filter). Admin API for group secrets
and the masked-read endpoint land next (Step E).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
§3 step 1 treats an environment scope as *eligibility*, not a merge tier:
within a single level a value scoped `@E` shadows the same level's `*`
fallback — it never layers on top of it. The resolver's map-run loop
collected every leading object row regardless of (depth, scope), so a
level holding both an `@E` map and a `*` map would deep-merge the two
(and report a bogus `*` layer in `merged_from`) rather than letting the
`@E` map win alone.
Dedup the sorted candidates by depth before the map run: each level is a
single owner (single-parent tree) with at most one `@E` and one `*` row
per key after env-filtering, and `@E` sorts first, so keeping the first
row per level yields the level's winner. Scalar/tombstone cases already
went through the boundary path and were unaffected; only same-level
map-vs-map mis-merged.
Adds two regression tests (same-level suppression; suppressed `*` stays
invisible to cross-level merge). Found in checkpoint review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The crypto foundation for group-owned secrets (the §5.3 'single hardest
correctness detail'), done first and proven before the storage/resolution
layer.
- SecretOwner{App(AppId)|Group(GroupId)}; secret_aad/seal/open take the
owner. The app AAD is byte-identical to the pre-Phase-3
'secret:{app_id}:{name}', so every existing v1 row decrypts unchanged;
group secrets use a disjoint 'secret:group:{group_id}:{name}' namespace
(the 'group:' infix separates app/group AAD even for equal UUIDs).
- All callers (SDK get/set, secrets_api, apply email-secret read) wrapped
in SecretOwner::App — behavior identical for app secrets.
- New audit test aad_distinguishes_app_and_group_owner proves the two
namespaces don't collide (both directions, even reusing the UUID); the
existing cross-app/cross-name swap audit tests stay green.
16 secrets_service tests pass; clippy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the vars half of Phase 3 end-to-end:
- vars_repo: VarOwner{Group|App} upsert/delete/list (owner-kind-specific
SQL; ON CONFLICT restates the partial-index predicate).
- vars_api: GET/PUT/DELETE under /apps/{id}/vars and /groups/{id}/vars,
resolve-then-require gated on App/GroupVars{Read,Write}, secrets-style
error mapping + key/env-scope validation.
- pic vars ls/set/rm (--group|--app, --env, --json, --tombstone).
- journey test: a group var is inherited by a descendant app's
vars::get(), and an app-level value overrides it (proximity) — green.
386 manager-core lib tests + the vars journey pass; clippy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scripts can now read group-inherited config via vars::get(key) / vars::all().
- shared: VarsService trait (read-only get/all) + NoopVarsService; added as
the 14th field on the Services bundle.
- manager-core: VarsServiceImpl resolves the calling app's config via
config_resolver (derives app_id from cx.app_id; AppVarsRead-gated for
authed principals, script-as-gate for anon).
- executor-core: vars:: Rhai bridge (get/all), registered in the SDK.
- authz: AppVarsRead/Write, GroupVarsRead/Write, GroupSecretsRead/Write
capabilities (group caps resolve via the Phase-2 ancestor walk; the
GroupSecretsRead human-read gate is group_admin-only, distinct from an
app's runtime injection).
- wired VarsServiceImpl into the picloud binary; updated the executor-core
test Services::new call sites.
386 manager-core lib tests green; clippy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 3 foundation (docs §3): env-filtered, proximity-first config
inheritance down the group tree.
- Migration 0048: `vars` (polymorphic group|app owner via two nullable FKs
+ CHECK exactly-one, env scope, JSONB value, explicit tombstone) +
`apps.environment` (the env marker the resolver filters on — 'an
environment is an app').
- config_resolver: a shared chain-walk CTE (mirrors effective_app_role) that
walks app → ancestor groups, env-filters in SQL, plus a pure Rust
resolution pass implementing the §3 semantics SQL can't — per-key
proximity-first, @E-over-* within a level, map deep-merge, tombstone
suppression, and provenance for --explain.
Verified: 8 unit tests (incl. the §3.2 proximity-beats-env-specificity call,
deep-merge, tombstone, boundary) + the candidate-fetch CTE against live
Postgres (env-filter drops a non-matching @production value; nearer level
shadows farther).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Server-side foundation for Phase-2 groups (no group-owned resources yet):
Shared types:
- GroupId, Group; App gains group_id; AppRole::{precedence,max} for
folding the highest effective role across the membership chain.
Repos:
- group_repo: tree CRUD with reparent (ancestor-walk cycle guard under a
coarse instance-wide structural advisory lock; slug frozen; bumps
structure_version) and delete=RESTRICT (refuses non-empty groups).
- group_members_repo: per-(user, group) role grants, mirroring app_members.
Hierarchy-aware authz (§5.3):
- AuthzRepo gains effective_app_role / effective_group_role (default to
direct membership / none, so the ~18 existing test stubs are untouched);
the Postgres impl resolves each via one depth-bounded recursive CTE that
MAXes the app's own row with every ancestor group_members row.
- can(): the Member path now folds inherited group roles, so a group_admin
on any ancestor is implicitly app_admin beneath it. New Capability
variants InstanceCreateGroup / Group{Read,Write,Admin}; group caps carry
no app_id (bound API keys can't manage groups). 8 new unit tests.
Admin API:
- groups_api: group CRUD + reparent (admin at both source and destination
parent, §5.6) + per-group members, all capability-gated.
- apps: POST /apps takes an optional parent group (default root); app
responses carry group_id; my_role now reflects the effective role.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2 (blueprint §5): groups form a single-parent org tree ABOVE apps.
This migration adds the tree + membership tables and gives every app a
parent (§9 adoption):
- groups: id, parent_id (self-FK ON DELETE RESTRICT), slug (instance-
global UNIQUE, frozen at creation), name, description, structure_version
(bumped on structural mutation), owner_project (inert §7 seam).
- group_members: (group_id, user_id, role) with the SAME three role
literals as app_members so AppRole round-trips and the authz rank table
covers both.
- apps.group_id: nullable add → backfill every app under a seeded 'root'
group → promote to NOT NULL + FK RESTRICT.
No group-owned resources yet (scripts/secrets stay app-owned — Phase 3).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Incidental `cargo fmt --all` normalization of two pre-existing over-width
`.lock()` chains in DevEmailSink; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The interactive create path rejects a module whose name shadows a built-in
SDK namespace (`kv`, `secrets`, …) so `import "kv"` can't resolve to a user
module, but `pic apply`'s `validate_bundle` skipped the check — a parity gap
that let a manifest create what the dashboard forbids. Gate it on
`ScriptKind::Module` and share the same `RESERVED_MODULE_NAMES` const (now
`pub(crate)`); endpoints remain unrestricted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `enabled` lifecycle's "a disabled script is not executable via ANY
path" guarantee leaked on the queue arm, and the outbox trigger arm lacked
the cross-app backstop its siblings carry.
- Queue arm: `dispatch_one_queue` bypasses the unified `dispatch_one`
fire-time gate, so its only `enabled` guard was the per-tick
`list_active_queue_consumers` snapshot — a TOCTOU window for a message
claimed in a tick where the script was still enabled. Re-check the
freshly-read `script.enabled` before executing and release the claim
(`nack`) when disabled; the message stays queued and resumes on
re-enable. The nack is load-bearing: `reclaim_visibility_timeouts` only
reclaims claims for enabled triggers, so without it the message would
sit claimed indefinitely.
- Trigger arm: `resolve_trigger` built the ExecRequest with
`app_id = row.app_id` while sourcing the body from `trigger.script_id`
with no same-app check — the guard the queue/http/invoke arms already
have. Add it so a hand-edited/restored row can't run one app's script
under another's SdkCallCx.
Both regression-locked by new unit tests (full mock harness, verified to
fail if the gate is removed):
- queue_enabled_gate::disabled_queue_consumer_releases_claim_without_executing
- outbox_enabled_gate::disabled_http_outbox_row_is_dropped_without_executing
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Holistic Phase-1 review found the "a disabled script can't execute via
any path" guarantee (§4.3) held only for the sync user-route, the
execute-by-id bypass, and the trigger outbox arm — three paths still ran
disabled scripts:
- Queue arm: `list_active_queue_consumers` filtered the trigger's
`enabled` but not the bound script's. Add `JOIN scripts … AND
s.enabled = TRUE` so a disabled script's queue trigger stops consuming.
- Async-HTTP (202) + queued invoke() arms: `build_http_request` /
`build_invoke_request` hardcoded `active: true`. Set it to
`script.enabled`, and MOVE the dispatcher's fire-time `active` drop to
after the source-kind match so it covers all three arms uniformly
(previously trigger-only).
- `invoke()` (script-to-script): `resolve_id`/`resolve_name` checked
cross-app isolation but not `enabled`. A disabled target now resolves
to NotFound (indistinguishable from absent), matching the data plane.
Also (review LOW/§4.7):
- The route-on/script-off 404 returned `NotFound(script_id)`, leaking the
internal id and distinguishable from absent. Return the same flat "no
route matches" 404 as the unmatched case.
- Add the §4.7 "enabled endpoint with no route and no trigger" reachability
warning (was unimplemented; only disabled-target shipped).
Tested: manager-core lib 368 + orchestrator 75 + 22 project-tool journeys
(incl. a new enabled-route→disabled-script flat-404 e2e) green; clippy
-D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read the new `name` column through the data model: `Trigger`/`TriggerRow`
carry it, every trigger SELECT/RETURNING includes it, and the row→Trigger
assembly + the interactive create paths populate it (the create INSERTs
still omit it, so they take the migration's default). Behavior unchanged
— the apply diff still keys on the semantic tuple; B-2 switches it to
name-keying (enabling Update) and wires the manifest.
Tested: manager-core lib 367 + trigger journeys green; clippy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Schema groundwork for the trigger `name` identity (the manifest merge key
that will let `apply` Update a trigger in place rather than only
Create/Delete). A `gen_random_uuid()` default keeps the existing write
paths valid and unique without code changes; existing rows are backfilled
to the readable `{kind}-{n}` form; `UNIQUE(app_id, name)` is enforced.
No behavior change yet — the manager-core write paths and the apply diff
start using the name in the follow-up. Verified the migration applies and
the trigger journeys (which create triggers via the unnamed path → default
name) stay green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- §4.7 warning: `apply` now reports when an enabled route or trigger is
bound to a disabled script — deployed but unreachable (route 404s,
trigger won't fire). A warning, not an error: it's valid desired state,
the operator just shouldn't be surprised by the silent 404.
- E2E journey (`tests/enabled.rs`): apply an active script and confirm
`/api/v1/execute/{id}` 200s; flip `enabled = false` in the manifest and
re-apply → 404; re-enable → 200. Exercises the whole path: manifest →
diff → apply → runtime honoring.
Tested: manager-core lib 367 + cli bins 31 + 15 project-tool journeys
(incl. the new enabled e2e) green; clippy -D warnings clean; versioning
gate passes (migration 0045).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the §4.3 outbox gap: the match-time `enabled` check can't see a
trigger (or its target script) disabled *after* an outbox row was
enqueued, so a pending row would still fire. `resolve_trigger` now folds
`trigger.enabled && script.enabled` into the resolved row, and
`dispatch_one` drops the row at fire time when inactive instead of firing
a stale event.
The queue arm needs no change here: `list_active_queue_consumers` already
re-lists only enabled queue triggers each tick, so a disable is honored
promptly without a stale-row window.
Tested: manager-core lib 366 green; clippy -D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the `enabled` flag bite at the data plane (§4.3).
- Routes: `compile_routes` drops disabled rows from the match table, so a
request to a disabled route 404s indistinguishably from an absent one
(no info leak). Rebuilt on every route write + at startup, as today.
- Scripts: the orchestrator's two execute paths — `/execute/{id}` bypass
and the user-route handler — 404 when the resolved script is disabled.
The route-handler check also covers an enabled-route-bound-to-disabled-
script (§4.7): the binding is unreachable rather than 500/executing.
Still pending: the dispatcher fire-time re-check for already-enqueued
outbox rows (next commit).
Tested: route_admin disabled-route-dropped unit test + orchestrator suite
(75) green; clippy -D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase-1 `enabled` three-state lifecycle (§4.3), data-model half. Triggers
already carried `enabled`; this adds it to scripts and routes and threads
it through the declarative project tool. Runtime honoring (disabled route
404 / script non-invocable / dispatcher fire-time re-check) is the next
commit — this change only stores and reconciles the flag.
- Migration 0045: `enabled BOOLEAN NOT NULL DEFAULT TRUE` on scripts +
routes. Default true ⇒ no behavior change on migrate.
- `Script`/`Route` (shared) gain `enabled` (serde default true via the new
`picloud_shared::default_true`); repos' SELECT/INSERT/UPDATE SQL, row
structs, `NewScript`/`NewRoute`/`ScriptPatch` all carry it.
- Apply diff treats `enabled` as a declarative field (omitted ⇒ active):
`script_update_reason` + `diff_routes` detect a toggle as an Update, and
the create/update/insert paths persist it. The bound-plan `state_token`
re-includes script/route `enabled` (removed in the earlier review fix
precisely because the diff didn't key on it yet — now it does).
- CLI manifest model + `build_bundle` + `pull` round-trip `enabled`
(serialized only when false; omitted ⇒ active). `pic init` scaffold and
the interactive script/route create paths default active.
Tested: manager-core lib 365 (incl. enabled diff + token sensitivity) +
cli bins 31 (incl. manifest skip-serialize) green; clippy -D warnings
clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remediation after an independent review of the two feature commits.
pic init:
- Fix the headline `pic init --dir <new-dir>` flow: slug derivation used
`canonicalize()`, which fails on a not-yet-created directory → empty
slug → bail. Derive from the path's own final component first, falling
back to canonicalize only for `.`. Add a test that exercised the gap
(the existing tests all passed an explicit slug).
- `ensure_gitignored` no longer overwrites an existing-but-unreadable
`.gitignore` (only a genuinely-absent file is treated as empty).
Bound-plan staleness:
- Token trigger coverage now mirrors the diff exactly via
`current_trigger_identity`: dead-letter triggers (which the diff
ignores and the manifest can't represent) and the currently-inert
`enabled` flag are no longer hashed, so an out-of-band dead-letter
change can't force a spurious `--force`. Add a test.
- Correct two overclaiming comments: the check shares the diff's
pool-read apply-vs-interactive-write window (it is not tx-scoped), and
the token deliberately mirrors the diff's inputs.
- `.picloud/` self-ignores via a `*` `.gitignore` written alongside the
plan token, so projects created with `pic pull`/`plan` (not just
`init`) keep it out of git.
- `clear_plan` is now app-scoped: it won't discard a token recorded for a
different app sharing the directory.
Tested: manager-core lib 364 + cli bins 31 + 14 project-tool journeys
green; clippy -D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`pic plan` now records a fingerprint of the live state it diffed against;
`pic apply` replays it and the server refuses (HTTP 409) if the app
changed underneath the reviewed plan — the §4.2 "apply exactly what you
reviewed" guarantee, in its content-addressed form (no migration, no
changes to interactive write paths).
Server (manager-core):
- `state_token(CurrentState)`: deterministic FNV-1a fingerprint over what
the diff keys on — script name+version (version bumps on any edit),
route identity+binding/attrs, trigger membership+enabled, secret names.
Order-independent; a collision can only yield a false "unchanged", never
a false refusal.
- `plan` returns it (flattened onto the plan JSON, so the wire stays
additive); `apply` takes an optional `expected_token` and, under the
apply lock before any write, returns `StateMoved` (409) on mismatch.
CLI:
- `.picloud/` link state (`linkstate`): `pic plan` writes the token scoped
to the app slug; `pic apply` replays it, then clears it on success (the
token is single-use — the next apply re-plans). `--force` skips the
check; apply with no recorded plan still works standalone (today's
behavior). `.picloud/` is already gitignored by `pic init`.
The tree-structure version (the other half of §4.2's counter) stays a
deliberate no-op until groups exist — it only guards reparent/structural
moves, which don't exist single-app.
Tested: state_token unit test (stable/order-independent/sensitive) + a
staleness journey (plan → out-of-band deploy → apply refuses → --force
applies); manager-core lib 363 + cli bins 31 + 13 journeys green; clippy
-D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remediation of the single-app reconcile foundation after two independent
review passes. No new feature surface — closes correctness, parity, and
safety gaps in pull/plan/apply/prune.
apply engine (manager-core):
- validate_bundle reached parity with the interactive trigger API: reject
an empty kv/docs/files collection_glob and a malformed pubsub
topic_pattern (previously written and silently never matched), and lift
the queue visibility floor to MIN_QUEUE_VISIBILITY_TIMEOUT_SECS (30) —
apply had accepted a [5,29] value the dashboard refuses. Per-kind checks
extracted into a pure, unit-tested validate_trigger_shape.
- An omitted script `description` now means "leave as-is" (matching the
other optional fields and the function's own documented contract)
instead of clearing the stored value.
- Doc fixes: drop stale "next milestone" notes; record the one deliberate
plan/apply divergence (set-but-empty secret); document delete_route_tx
as intentionally idempotent for reconcile.
CLI:
- Gate destructive `apply --prune` behind confirmation: interactive y/N,
or `--yes` for CI; a non-interactive prune without `--yes` refuses
rather than silently deleting. Journey tests pass `--yes`; a new test
proves the gate refuses and deletes nothing.
- Harden pull's filename safety: reject all control characters and the
Unicode bidi-override / zero-width chars used for terminal/filename
spoofing, and cap length at 200 bytes (NAME_MAX headroom).
Tested: manager-core lib (incl. queue-floor + sparse-description parity)
+ CLI bins + 9 project-tool journeys green; clippy -D warnings clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a server-side, atomic, declarative reconcile loop for a single app —
the foundation of the project-tool design. Developers describe an app's
scripts, routes, triggers, and secret-names in `picloud.toml`, then
`pic pull / plan / apply [--prune]` to converge live state to the manifest.
Server (manager-core):
- apply_service: a pure diff engine (compute_diff) shared by plan and
apply, plus an ApplyService that composes the existing per-repo writes
into ONE Postgres transaction. Identity keys mirror the DB UNIQUE
constraints (script=lower(name); route=(method,host_kind,host,
path_kind,path); trigger=per-kind semantic tuple; secret=name).
Apply takes a per-app advisory lock, recomputes the diff in-tx, applies
scripts -> routes -> triggers, prunes dependents-first, commits, then
refreshes the route table once post-commit.
- apply_api: POST /apps/{id}/plan (AppRead) and /apps/{id}/apply.
Apply requires the per-kind write caps the bundle exercises (all three
when --prune), plus AppSecretsRead when it binds an email trigger.
- tx-accepting repo siblings (insert/update/delete *_tx) so the existing
create/update/delete delegate to one SQL definition each.
- email triggers reference an inbound secret by NAME; the value is
resolved, decrypted (AAD-bound), and re-sealed server-side at apply —
it never travels in the manifest.
CLI (picloud-cli):
- manifest.rs (picloud.toml model), client plan/apply, and the pull/plan/
apply commands. pull rejects filesystem-unsafe script names up front.
Safety properties enforced and tested:
- idempotent: a freshly-pulled manifest re-applies as all-NoOp.
- atomic: a mid-bundle failure rolls back with nothing written.
- routes delete-before-insert so a freed binding is reusable in one apply.
- queue one-consumer invariant held inside the shared tx.
- email triggers are never pruned, and a script that still owns an
email/dead-letter trigger can't be pruned (the FK cascade would destroy
the sealed secret) — refused with a pointer to `pic triggers rm`.
- plan and apply agree on unset email-secret references.
No migration: the existing schema's UNIQUE constraints serve as identity
keys. Groups, env-scoping, and the `enabled` toggle are later milestones.
Tested: manager-core lib (360) + CLI bins (27) + 8 project-tool journeys
(pull/plan/apply/prune/email+queue), all green; clippy -D warnings clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the gaps and the one security finding from the second end-to-end
CLI test (E2E_STASH_REPORT.md), plus the H1 boot-regression found while
re-reviewing those fixes.
Security
- S6: reserved-path validation (`check_reserved`) now case-folds before
comparing, so `/API/v2/x`, `/HEALTHZ`, `/Admin/x` are rejected like
their lowercase forms. Request-time matching stays case-sensitive.
- S10: "public route != public data" callout in sdk-shape.md (script_gate
skips authz when the principal is anonymous).
Observability / features
- G1: trigger executions now write `execution_logs`. Migration 0043 adds
a `source` column (CHECK mirrors ExecutionSource/OutboxSourceKind,
DEFAULT 'http' backfills history); a shared `build_execution_log` helper
in executor-core; dispatcher logging for outbox triggers + queue
consumers (skips sync-HTTP rows the orchestrator already logs). `pic
logs` gains a source column + `--source` filter.
- G5: dev-only in-memory email capture under PICLOUD_DEV_MODE with no SMTP
(email::send succeeds locally), readable at GET /api/v1/admin/dev/emails
(Owner/Admin only; route mounted only in capture mode).
- G6: generalized the Rhai in-place-mutation footgun note (trim/replace/
make_upper/make_lower/crop/truncate/pad return ()).
- G2/G3/G4 (CLI): `pic members`, `pic files`, `pic queues`, read-only
`pic kv` (+ new kv_api.rs); `pic deploy --timeout/--memory/--kind/
--sandbox`; first-class `pic triggers create-{docs,files,pubsub,queue,
email}` wrappers. All new client path segments percent-encoded via seg().
H1 regression fix (found in re-review)
- The S6 change also runs in `compile_routes`, which compiles every stored
route at boot and on each route CRUD. A single stored route the new
validation rejects (creatable while the S6 gap existed) made the whole
compile Err and aborted startup. `compile_routes` is now lenient: it
skips an un-compilable row with a warning instead of bricking boot
(route creation still validates separately). Migration 0044 sweeps
pre-existing reserved-path routes on upgrade (WHERE mirrors
check_reserved exactly). Added regression tests for both.
Verified: cargo fmt, clippy --all-targets --all-features -D warnings, the
schema_snapshot test, and the new S6/lenient-compile unit tests all pass;
boot-resilience and G1/G5 confirmed live.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses findings from an independent review of the gap-closing commits:
- R1 (regression): add #[serde(default)] to HttpDispatchPayload.method so
async-HTTP outbox rows enqueued before the field existed still decode
after upgrade instead of dead-lettering on "missing field `method`".
Adds a regression test for the missing-key path.
- method case: uppercase ctx.request.method at the orchestrator boundary
so it honors its documented "uppercased" contract for extension verbs.
Route matching is already case-insensitive, so matching is unaffected.
- CLI hardening: percent-encode free-form path segments (app slug, topic
name, ids, secret name) in the reqwest client so a value containing
`/ ? #` can't break out of its URL segment. Adds a `seg()` helper +
unit test and applies it uniformly across all path-interpolating
methods (new and pre-existing).
- S1 (docs): correct the users::email_available enumeration framing — it
adds no new vector vs. create's uniqueness error, but is cheaper and
unthrottled; there is no built-in throttle/CAPTCHA primitive, so the
honest mitigation is a kv-based counter. Updated SDK doc-comments,
trait docs, and stdlib-reference.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`users::find_by_email` requires an authenticated principal (F-S-003,
anti-enumeration), so the natural check-then-create register pattern 502s
for anonymous public scripts. Add `users::email_available(email) -> bool`,
gated like `create` (the registration write path) but without the
anonymous rejection: it leaks only a single boolean — no more than
`create`'s own uniqueness error already does — so self-serve register
scripts can pre-check. Also document find_by_email's principal
requirement in the SDK doc-comment.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The request map exposed path/headers/body/params/query/rest but not the
HTTP method, forcing one script per verb (the E2E app needed 7 scripts
where ~3 would do). Add a `method` field to ExecRequest (populated from
the HTTP method on the sync-execute bypass and the HTTP outbox payload;
empty for non-HTTP triggers) and surface it as `ctx.request.method`,
uppercased. A single script can now branch GET/POST on one route.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review nit — the field is a non-optional Arc; the old '`None` for tests'
note was stale. Clarify that it's one shared Arc so revocation-side
evictions are visible to the middleware's resolve-side lookup.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extract_remote_ip took the FIRST X-Forwarded-For entry, but Caddy (the
single trusted proxy) appends the real peer as the LAST hop — earlier
entries are client-supplied. An attacker could prepend a rotating
X-Forwarded-For value to get a fresh per-IP bucket every request and
evade the per-IP login limiter (the per-username bucket + global Argon2
semaphore still bounded it, but the per-IP layer was defeated). Take the
last hop so the key reflects the real client. Added unit tests covering
the spoof, the single-hop case, and the missing-header fallback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Caddy request_body { max_size 12MB } in both Caddyfiles — a hard
ceiling replacing Caddy's multi-GB default; 12MB clears the
orchestrator's 10 MiB user-route read. Validated with caddy validate.
- email-inbound router gets an explicit DefaultBodyLimit::max(1MB):
the public unauthenticated webhook previously rode Axum's 2MB
extractor default; tightened so a flood can't force large
allocation + JSON parse per request.
Admin / execute handlers keep Axum's 2MB extractor default (already
bounded); the user-route path keeps its explicit 10 MiB manual read.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The admin files endpoints hit FsFilesRepo directly (not via
FilesServiceImpl, which validates), so only create/update guarded the
collection — head/get/list/delete built FS paths from an unvalidated
value. Today nothing can store a traversal-shaped collection, but one
future bad migration / restore tool inserting collection='../../etc'
would give the read/delete paths arbitrary host-file reach.
- FsFilesRepo::{head,get,list,delete} now call guard_collection (create
and update already did).
- The three admin endpoints (list/get/delete) call
validate_files_collection up front for a clean 422 instead of the
repo guard surfacing as an opaque 500.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PrincipalCache (token-hash -> resolved Principal, 60s TTL) had no
eviction hook, so deactivation / password change / API-key revocation
flipped the DB rows but the cache kept serving the stale principal for
up to the TTL window. Closes the audit's PrincipalCache revocation-lag
Medium and greens authz::deactivating_user_revokes_their_api_keys.
- PrincipalCache::evict_user(user_id) + evict_token(hash).
- One shared cache Arc threaded into AuthState / AdminsState /
ApiKeysState (a per-state cache would let the middleware's own copy
keep authenticating a revoked principal).
- Evict on: deactivation, password change (both evict_user), logout
(evict_token, precise), single API-key delete (evict_user).
- H-E1 note: DB-write invalidation stays best-effort by design (a blip
must not undo the rotation); the cache evict is what makes it take
effect on the next request.
- Renamed bearer_and_cookie_produce_same_principal ->
session_token_and_api_key_produce_same_principal (cookie auth was
removed in C-1; the test never tested cookies).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
At-rest secrets were AES-256-GCM sealed with no Associated
Authentication Data, so anyone with Postgres write access could
ciphertext-swap rows across apps (or rename via row edit) and the
decrypt would silently succeed under the wrong identity, returning
attacker-chosen plaintext. This breaks the cross-app isolation boundary
the moment DB write access is achieved.
Adds an AAD-bound envelope (v1) alongside the legacy no-AAD layout (v0),
discriminated by a per-row `version` column:
* shared::crypto — new encrypt_with_aad / decrypt_with_aad using
aes_gcm::aead::Payload { msg, aad }. Originals retained for v0 reads.
Tests: AAD round-trip, AAD-mismatch fails, empty-AAD round-trip.
* migration 0042 — adds `secrets.version SMALLINT NOT NULL DEFAULT 0`
and `app_secrets.realtime_signing_key_version SMALLINT NOT NULL
DEFAULT 0`. Existing rows stay v0; new writes are v1.
* secrets (SDK + admin API) — seal() now binds AAD =
"secret:{app_id}:{name}" and emits v1; open() dispatches on version.
StoredSecret gains a `version` field; SecretsRepo::set takes it.
Both secrets_service::set and secrets_api::set_secret go through the
v1 path. Tests prove a cross-app swap and a cross-name swap both
surface Corrupted, and that a hand-built v0 row still decrypts.
* app_secrets (realtime signing key) — get_or_create_signing_key writes
v1 with AAD = "app_secret:{app_id}:realtime_signing_key"; decode
dispatches on version. Tests cover v0 decode, v1 round-trip, and v1
decode-under-wrong-app failing.
* email-trigger inbound secret — kept on v0 (seal_legacy/open_legacy)
and explicitly deferred: email_trigger_details has no version column
and the trigger_id isn't known at seal time. The audit classes the
email-trigger AAD gap as Medium; folded into v1.2's key-versioning
pass per SECURITY_AUDIT.md.
* expected_schema.txt re-blessed by hand (no local Postgres) for the two
new columns + migration 0042.
Also folds in a let-else clippy fix in auth_api.rs (login Argon2
semaphore acquire, from the H-B1 commit) and two cargo-fmt reflows.
No re-encryption sweep — v0 rows decrypt as-is; the sweep is deferred to
v1.2's key-versioning pass (audit "Notes on remediation methodology").
Audit ref: security_audit/03_crypto_secrets.md (H-D1).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Rhai SDK email::send path went straight to the SMTP relay with no
admission control. EmailRateLimiter lived in users_service.rs (gating
verification + password-reset emails) but was unreachable from the SDK
surface. Any anonymous-callable HTTP-route script could loop email::send
to burn the operator's SMTP quota or BCC-bomb thousands of recipients
per request.
Two new caps on EmailServiceImpl, both fire BEFORE message assembly +
SMTP connect:
1. Per-message recipient cap (to+cc+bcc combined). Default 20, override
with PICLOUD_EMAIL_MAX_RECIPIENTS. New EmailError::TooManyRecipients
variant. Closes the BCC-bomb amplification path.
2. EmailRateLimiter inlined into EmailServiceImpl:
* Per-(app, recipient): RECIPIENT_BURST=5 / RECIPIENT_WINDOW=60min.
* Per-app daily: APP_DAILY_CAP=200 / 24h.
The structure mirrors users_service's private limiter; defense-in-
depth redundancy is fine since the gates are identical and never
conflict. New EmailError::RateLimited(&'static str) variant; the
string names which bucket tripped so the operator can act.
users_service::map_email_error grows two cases (mapped to
EmailTransport for the script-facing error shape).
Tests:
* too_many_recipients_rejected — 30 recipients, default cap 20, rejected
with TooManyRecipients.
* per_recipient_burst_caps_repeated_send_to_same_address — 6 sends to
the same address; 6th returns RateLimited("per-recipient burst").
Audit ref: security_audit/09_external_integrations.md#f-ext-h-001.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two gaps:
1. /api/v1/admin/apps/{id}/triggers/email accepted `inbound_secret: null`
and treated it as "this trigger is unsigned". The runtime then
skipped HMAC verification entirely. URL discovery (logs, ops
dashboards, bruteforce on the 122-bit TriggerId space) then fan-out-
amplified into the outbox for free — anonymous DoS via the
dispatcher.
2. Even signed triggers had no per-trigger rate limit on signature-
verify failures, so an attacker who knew the URL could pump unsigned
POSTs to force Argon2-equivalent work (HMAC verify + nonce-dedup +
stored-secret decrypt) at line rate.
Fix:
* triggers_api::create_email_trigger now requires a non-empty
inbound_secret. 400 on missing / empty / whitespace-only.
* email_inbound_api::receive_inbound_email returns 401 immediately when
the resolved target has no inbound_secret_encrypted column (pre-fix
rows). The previous `if let Some(ct), Some(nonce)` branch is gone.
* New `BadSignatureLimiter`: per-`(app_id, trigger_id)` sliding-window
bucket (10 fails / 60 s). On lockout the receiver returns 429 with
Retry-After instead of 401. record_failure runs on both the
"no-secret" 401 path and any verify_signature failure.
Test fallout:
* email_inbound integration tests that relied on None secret: removed
the now-impossible `unsigned_trigger_accepts_without_signature` test,
replaced with `create_without_inbound_secret_is_rejected` covering
null / "" / " "; updated `malformed_body_is_422` and
`cross_app_path_is_404` to pass + sign with a secret.
* Two new unit tests pin the limiter behavior (burst lockout + per-
trigger independence).
Audit refs: security_audit/08_dos_resource.md#h-3,
security_audit/09_external_integrations.md#f-ext-m-001.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The login handler had no admission control before
`spawn_blocking(verify_password)`. On Pi-class hardware Argon2id is ~50-
100 ms per attempt, so a few dozen concurrent anonymous POSTs to
/auth/login park every blocking worker, wedging the entire admin API
and any other path that uses `spawn_blocking`.
Adds two cheap, in-process guards modeled on EmailRateLimiter:
* `LoginRateLimiter` — two sliding-window token buckets, one per
`(remote_ip, username)` (burst 5/60s) and one per `username`
(burst 10/15min). Per-(ip, user) defeats a single attacker pounding
one account; per-user defeats credential-stuffing distributed across
many IPs. Username is case-folded so case variants share a bucket.
Maps GC lazily when they cross a soft size cap.
* Per-process `tokio::sync::Semaphore` — caps concurrent Argon2 verifies
during login. Default 2 permits (Pi-class), overridable via
`PICLOUD_LOGIN_ARGON2_PARALLELISM`. Acquired AFTER the bucket check so
attackers can't queue.
Limit check fires before the DB credentials lookup, so even an unknown
username doesn't cost a query. Denied attempts return 429 + Retry-After.
Real client IP comes from the first X-Forwarded-For entry (Caddy is the
trusted single hop); falls back to "unknown" so the per-user bucket
still gates.
4 unit tests cover bucket burst exhaustion, per-user crossing IPs, case
folding, and per-user independence.
Audit ref: security_audit/08_dos_resource.md (H-2 / H-B1).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
build_invoke_request already asserts `script.app_id == row.app_id`
(dispatcher.rs:752) before constructing an ExecRequest. The queue
dispatcher (`dispatch_one_queue`) and the HTTP outbox arm
(`build_http_request`) did not. validate_trigger_target blocks cross-
app registration at write time, so the gap was latent; but a hand-
edited row, a partial backup restore, or a script re-pointed across
apps post-hoc could otherwise execute one app's script under another
app's SdkCallCx, breaking the cross-app isolation boundary the SDK
relies on.
Adds the runtime guard at both sites:
* dispatch_one_queue — on mismatch, dead-letter the message so the
misfire is observable rather than silently dropped, and short-
circuit.
* build_http_request — on mismatch, return ResolveTrigger error; the
outer dispatch arm logs + drops the row (same path as decode
failures).
Audit ref: security_audit/02_authz_isolation.md (H-F1).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
