Commit Graph

10 Commits

Author SHA1 Message Date
MechaCat02
bdcd3dc7f4 fix(executor): bound Rhai→JSON materialization to prevent anonymous OOM
The per-element Rhai sandbox caps (max_string_size 64 KiB, max_array_size /
max_map_size 10 000) do NOT bound the *materialized* size: Rhai shares strings
and arrays by Rc, so a 10 000-element array of one aliased 64 KiB string is
cheap to build (~10 000 ops, far under the 1 M op budget) yet dealiases to
~640 MiB of distinct JSON. Every Dynamic→JSON conversion deep-copies the
aliases; the HTTP response body path had NO size cap at all, and the KV/docs
value caps run only AFTER full materialization — so a handful of anonymous
requests could OOM the node (32 concurrent × ~640 MiB ≈ 20 GiB) on the stated
consumer-hardware target.

Add a byte-budgeted materializer that bails the moment the budget is exceeded,
so the transient allocation is bounded regardless of aliasing:
- bridge.rs: `dynamic_to_json_capped(value, max) -> Result<Json, JsonSizeError>`
  (charges a running budget) + `MAX_JSON_MATERIALIZE_BYTES` (16 MiB hard rail,
  far above the 256 KiB business caps). `dynamic_to_json` stays infallible for
  best-effort paths (logging) but is now internally bounded — it collapses an
  over-limit value to a marker string instead of OOMing.
- Route every user-value boundary through the fail-closed capped form: KV
  set/set_if (+ the CAS `expected`), docs create/update/find, secrets set,
  http request body, workflow input, `json::stringify`, and the HTTP RESPONSE
  body (previously the one fully-uncapped exit → now a 500). `invoke` args and
  the pubsub/queue message materializers get the same byte budget in place.
- `impl From<JsonSizeError> for Box<EvalAltResult>` so SDK sites protect
  themselves with a bare `?`.

Pinned by bridge unit tests: an aliased 10 000×64 KiB array errors on the
budget rather than materializing, and the infallible wrapper yields a marker
instead of OOMing. Workspace 914 + journeys 157/157 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 21:50:16 +02:00
MechaCat02
d08df88df5 fix(interceptors): harden the §9.4 slice — seal, fail-closed, re-entrancy, shared coverage
An end-to-end audit of the (unmerged) interceptor slice surfaced seven ways a
KV allow/deny guard could be silently defeated or misbehave. Close all of them:

1. Shared-collection bypass — `kv::shared_collection(...).set/delete` skipped
   the hook entirely, so any `(kv, set/delete)` guard was circumvented by
   choosing the shared handle. `GroupKvHandle` now runs the same before-op hook.

2. Seal to the declaring owner — the marker resolved nearest-owner-wins but its
   script name was then re-resolved on the CALLER's chain, so a descendant app
   could shadow a group's mandatory guard with a same-named local script.
   `resolve_before` now returns the script sealed to the owner that declared the
   marker (one LEFT JOIN in manager-core), so the executor never re-resolves by
   name. A descendant can only override with its OWN explicit marker.

3. Re-entrancy — an "allow + audit-log" guard that itself wrote KV re-triggered
   itself to the depth cap and then DENIED the original write (plus ~8x
   resolve/compile/execute amplification). A thread-local guard makes a write
   performed by an interceptor bypass interception.

4. Fail-closed verdict — only `#{ allowed: false }` denied; a bare bool, a
   typo'd key, a non-bool, or a bare unit all ALLOWED. Now allow ONLY on an
   explicit `#{ allowed: true }`; every other shape denies.

5/6. Fail-closed edges — a dangling (missing/disabled) interceptor script and a
   missing engine back-reference now DENY instead of allowing.

7. AppInvoke coupling — resolution no longer routes through `invoke.resolve`, so
   installing a guard no longer denies writes to principals who hold KV-write
   but not AppInvoke.

The seam (`InterceptorService::resolve_before`) now returns an
`InterceptorResolution { None | Dangling | Run(ResolvedScript) }`; the executor
runs the sealed script straight through the shared `run_resolved_blocking` core
and drops its `InvokeService` dependency. executor-core stays Postgres-free.
No migration change (0073 unchanged).

Pinned by three new journeys in tests/interceptors.rs: the seal (a same-named
app script does NOT shadow the group's guard), shared-collection coverage, and
the fail-closed verdict. Full journey suite 157/157.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 21:20:20 +02:00
MechaCat02
2fc9476f9e feat(interceptors): §9.4 service interceptors — thin KV allow/deny slice
Smallest honest vertical slice of §9.4: a `[[interceptors]]` block (app OR
group) binds a script to run BEFORE `kv::set`/`delete`; it reads the operation
context (`ctx.request.body`: service, action, collection, key, value, caller
ids) and returns `#{ allowed, reason }` — `allowed == false` denies the op (the
write never runs, the caller gets a runtime error).

Reuses two existing mechanisms rather than inventing new ones:
- Registration mirrors extension points (§5.5): a marker table
  `0073_interceptors.sql` (owner-polymorphic app_id/group_id XOR, keyed
  (service, op) → script), `interceptor_repo` (insert/delete/list + the
  nearest-owner-wins `resolve_before` chain walk), reconciled through the
  declarative apply exactly like `vars` (create/update/delete, prunable).
- Execution reuses the `invoke()` re-entry path: the new `InterceptorService`
  (shared trait + Postgres-backed impl) only RESOLVES the script name (keeping
  executor-core Postgres-free); the executor's `sdk::interceptor::run_before`
  resolves that name and runs it via `run_resolved_blocking` (extracted from
  `invoke_blocking` — shared depth bound + AST cache). An un-hooked write pays
  one indexed `Ok(None)` resolve; no interceptor ⇒ zero overhead.

Nearest-owner-wins so an app overrides a group's interceptor, and a group
interceptor is inherited by every descendant app — the chain walk is the
isolation boundary (a sibling subtree never matches). `validate_bundle_for`
restricts the MVP to `service = "kv"`, `op ∈ {set, delete}`, one marker per
(service, op).

Deferred (documented in §9.4): the `data` transform return, services other
than kv, `after_*` hooks, chaining + circular-dependency guard, the timeout
policy, and a `pic interceptors ls` read surface (needs a server route).

Pinned by `tests/interceptors.rs` (deny blocks the write; allow passes;
group→app inheritance), schema snapshot re-blessed. 154/154 journeys pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 20:44:50 +02:00
MechaCat02
c8c4f012ff docs: reconcile blueprint/design-doc/CLAUDE.md to shipped v1.2 state
A five-agent roadmap-verification pass found the v1.2 Hierarchies code fully and
correctly implemented, but the three source-of-truth docs carried stale
"deferred/remaining/v1.3" notes — and one self-contradiction — for features that
have since shipped (§3 M3 server-side approval gate, §6 group create/reparent, the
Track A closeout, and the 2026-07-11 audit remediation). Reconcile all three to
reflect the true code; no behavior change.

CLAUDE.md: retract the inline "groups pre-exist" / "per-env approval deferred" /
"shared-topic SSE deferred" / "group dead-letter store deferred" / "email v0/no-AAD"
notes → mark §6, §3 M3, and Track A M1–M6 shipped; add per-group KV/docs byte
quotas + set_if; note migration 0063; refresh the "Out of MVP" section.

design doc: fix the header ("Phases 4–6 remain" → all shipped); resolve the §11.6
self-contradiction (the "Deferred" list still named shared-topic SSE / byte quotas /
set_if / operator admin API, all shipped — line 1317 already said SSE shipped); mark
the D3 dead-letter store shipped; retract the stale "groups pre-exist / §6 deferred"
forward references.

blueprint (comprehensive sweep): dashboard Alpine.js → SvelteKit + CodeMirror
(diagram, §3.3, tech table); Docker-per-execution → embedded in-process Rhai
(diagram + data flow); §12 Phase 4 "current focus" → shipped through v1.1.9; Phase 5
"in active development" + "Remaining" block → Hierarchies complete; per-app RBAC
"v1.3+" → shipped Phase 3.5; SDK reference → handle-pattern notation note, S3 tag
v1.1 → v1.3+; MVP schema + docker-compose flagged non-authoritative; §9 header noted
Hierarchies-shipped / Workflows-future; top status line refreshed.

Also fix two behavior-neutral stale in-code comments (sdk/kv.rs `kv::shared` →
`kv::shared_collection`; dispatcher.rs `q_terminal` "no group dead-letter store yet"
→ dead-letters to group_dead_letters, Track A M2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 14:53:34 +02:00
MechaCat02
0f05c270d1 feat(kv): set_if compare-and-swap for KV (per-app + shared + SDK) (Track A M5)
No atomic CAS existed, so concurrent writers to the same KV key raced. Add
set_if(key, expected, new): writes only when the current value equals expected,
or (expected absent) only when the key is missing — the primitive for lock-free
counters / optimistic concurrency.

- KvService + GroupKvService traits gain set_if with a defaulted "unsupported"
  impl (Noop + other impls untouched); the real services override it.
- kv_repo/group_kv_repo: atomic set_if — a single UPDATE ... WHERE value =
  $expected (JSONB semantic equality) or INSERT ... ON CONFLICT DO NOTHING.
  Atomic without a transaction; returns whether the write happened.
- Services enforce the same value-size cap + (group) writes-authed + row/byte
  quotas as set; the event fires only on a successful swap.
- Executor Rhai SDK: kv::collection(c).set_if(key, expected, new) and the
  shared_collection variant; Rhai () for expected means "only if absent".

Pinned by kv_service/group_kv_service set_if unit tests (CAS + fail-closed) +
sdk_kv::kv_set_if_compare_and_swap (through a real script). No migration.
docs/sdk-shape.md documents the primitive.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 15:19:03 +02:00
MechaCat02
0973344515 feat(cli): collections manifest + ls + journeys; rename to kv::shared_collection (§11.6 C5)
Finish the §11.6 KV slice: declarative authoring, read-only inspection, the
runtime journey, and docs — plus a forced SDK-name fix.

- SDK name: `shared` is a Rhai RESERVED keyword, so `kv::shared(...)` is a parse
  error. Renamed the handle constructor to `kv::shared_collection(...)`
  (keeps the user's "shared" intent; unambiguous). Updated everywhere.
- CLI manifest: `collections = [...]` on `[group]` only (ManifestApp has no such
  field + deny_unknown_fields → an app manifest carrying it is a hard error);
  Manifest::collections() accessor; build_bundle emits it.
- plan/apply: a `collection` row group in `pic plan`; created/deleted counts in
  the apply summary; collections threaded through PlanDto/NodePlanDto/
  ApplyReportDto.
- read-only `pic collections ls --group` → GET /admin/groups/{id}/collections
  (viewer+), backed by ApplyService::collection_report + CollectionInfo.
- journey: a group declares `catalog`; app A writes, app B (same subtree) reads
  it back; a sibling-subtree app gets CollectionNotShared; `collections ls` +
  re-apply NoOp. Full suite 114/114.
- docs: groups-and-project-tool §11.6 (KV-only MVP shipped + deferrals),
  sdk-shape.md (the shared-collection handle + trust model), CLAUDE.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 22:26:43 +02:00
MechaCat02
d9766bcbc7 feat(modules): GroupKvService + kv::shared SDK handle (§11.6 C3)
The runtime for shared group collections. A script reaches a shared store via
the explicit kv::shared("name") handle (distinct GroupKvHandle Rhai type, so
private-vs-shared scope is a deliberate choice). The service resolves the
owning group from cx.app_id's ancestor chain — the script never names a group,
and that walk is the isolation boundary (a foreign app's chain never contains
the owning group → CollectionNotShared).

- shared: GroupKvService trait + GroupKvError + NoopGroupKvService; threaded
  into the Services bundle as a field defaulted to noop, opted into via a new
  with_group_kv() (keeps the ~14 Services::new call sites unchanged).
- manager-core: GroupKvServiceImpl with the reads-open/writes-authed split —
  reads use script_gate (anonymous public scripts skip), writes use the new
  script_gate_require_principal (anonymous fails closed). Owner resolution
  behind a GroupCollectionResolver trait (Postgres impl + injectable fake);
  unit tests cover off-chain CollectionNotShared, anonymous-read-OK/
  anonymous-write-Forbidden, authed-no-role-Forbidden.
- executor-core: GroupKvHandle + kv::shared constructor + get/set/has/delete/
  list (Rhai dispatches by receiver type, reusing the block_on bridge).
- picloud: wire PostgresGroupKvRepo + resolver + GroupKvServiceImpl.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 21:44:53 +02:00
MechaCat02
6f2bd3e949 style: cargo fmt after Phase A
Whitespace + import-grouping fixups in the 9 SDK modules touched by
F-Q-002. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-07 20:02:19 +02:00
MechaCat02
5c50ce2e11 fix(executor-core): F-Q-002 promote sdk::bridge::block_on, migrate 9 SDK modules
Replace ten near-identical `block_on` helpers (one per SDK module)
with a single shared `sdk::bridge::block_on(service: &str, fut)`.
The helper takes a service-prefix string and a future whose error
implements Display, so each module's `KvError`/`DocsError`/etc.
still self-formats.

Migrated:
- kv, docs, pubsub, users, dead_letters, secrets, files, email
- queue.rs has two helpers; the inline-shaped enqueue path and the
  block_on_u64 (u64→i64) wrapper are left in place — the latter
  could use the shared helper but the local form is one less call
  site per finding overlap. Will revisit on a later pass if needed.
- http.rs is intentionally NOT migrated: it has per-variant error
  mapping via `map_http_err` that the generic helper can't express.

Net: -218 / +97 lines across the 9 migrated files. Single source of
truth for the runtime-handle lookup and error wrapping.

AUDIT.md anchor: F-Q-002.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-07 19:54:37 +02:00
MechaCat02
6b99f74c48 feat(v1.1.1-kv): Rhai kv:: SDK module + ctx.event wiring
Wires the KV store into Rhai scripts via the handle pattern:

    let widgets = kv::collection("widgets");
    widgets.set("k", #{ n: 1 });
    let v = widgets.get("k");          // value or () if absent
    widgets.has("k") / widgets.delete("k")
    let page = widgets.list();          // cursor-style pagination

`KvHandle` is a custom Rhai type holding `Arc<dyn KvService>` + the
per-call `Arc<SdkCallCx>`. Methods route async service calls through
`tokio::Handle::current().block_on(...)` — works because
`LocalExecutorClient` runs the script under `spawn_blocking` so a
runtime is reachable. The bridge surfaces `app_id` exclusively
through `cx.app_id`; no public-facing argument can spoof an app.

`TriggerEvent` lands in `picloud-shared` as the wire shape the
dispatcher will emit (KV + DeadLetter variants — KV exercised now,
DL hooks up with the dispatcher in commit 5/8). `SdkCallCx` and
`ExecRequest` grow `is_dead_letter_handler: bool` and
`event: Option<TriggerEvent>`. `engine.rs::build_ctx_map` flattens
the event into `ctx.event` for triggered handlers; direct ingress
leaves the key absent so scripts can `if "event" in ctx`.

Tests:
- 7 `sdk_kv.rs` integration tests covering the full Rhai surface
  (round-trip, missing-key unit, has bool, delete was-present,
  empty-collection rejection, cursor pagination, cross-app
  isolation through the bridge).
- 3 new `engine.rs` tests pinning `ctx.event` shape per
  design notes §4 (KV insert with value, delete with unit value,
  direct invocations have no `event` key).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-01 21:38:41 +02:00