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>
The architecture spec for the full vision — a declarative project tool
(picloud.toml + pull/plan/apply/prune) and a server-side nested-groups
hierarchy with live-resolve inheritance. Reviewed across several passes
for consistency, gaps, and contradictions; resolutions are folded in
beside the topics they settle.
This commit lands the spec only. The first slice of implementation (the
single-app reconcile foundation) follows; groups, env-scoping, and the
`enabled` toggle are later milestones called out in the doc.
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>
Rebuilt the rich To-Do app end-to-end through `pic` alone (no raw admin
curl) after the gap fixes landed, and appended a per-finding verification
table + transcript to the report. Confirms B1/B2/F1–F4/S1–S3/O1 closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- S2: document that the response envelope is statusCode-gated — a returned
map is only unwrapped into {statusCode, headers, body} when it contains
statusCode, else the whole map silently becomes the 200 body.
- S3: note that Rhai String.replace() mutates in place and returns (),
with the sub_string bearer-parsing idiom.
- Document ctx.request fields including the new `method`.
- O1: document PICLOUD_DEV_INSECURE_KEY next to PICLOUD_DEV_MODE in
CLAUDE.md (the acknowledgement var was previously only in the startup
error).
Co-Authored-By: Claude Opus 4.8 (1M context) <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>
A CLI-only walkthrough (E2E_TODO_REPORT.md) found two control-plane
operations with no `pic` command — every CLI-created app 404'd until it
claimed a domain, and external SSE feeds needed a raw topic-registration
call — plus several friction points. All admin APIs already existed; this
adds thin wrappers mirroring `routes`/`triggers`:
- B1: `pic apps domains {ls,add,rm}` over apps/{id}/domains.
- B2: `pic topics {ls,create,update,rm}` over apps/{id}/topics, and a
triggers help note distinguishing a pubsub trigger from topic
registration.
- F2: `pic users {ls,show,reset-password,revoke-sessions}` over the app
end-user admin surface (read + the two admin actions; create/invitations
deferred).
- F1: `apps create` and `deploy` now honor `--output json`, emitting the
created object so scripts can capture the id.
- F3: non-interactive login via `--username` + `--password-stdin` (inline
passwords still rejected, mirroring the `--token` rule).
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>
pic admins create/set --password and pic login --token accepted secrets
on argv, where they leak into shell history, ps aux, and
/proc/<pid>/cmdline. Now only --password -/--token - (stdin) is allowed;
an inline value is rejected with guidance. PICLOUD_TOKEN env still covers
CI for login.
Also (M2): the interactive admins password prompt used read_line, which
echoes despite a 'no echo' claim — switched to rpassword::prompt_password
(already a dep via login). Removed the now-dead ReadLine trait + Write
import.
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>
ExecError::Runtime strings (filesystem paths, "blocked by SSRF policy:
link-local" cloud-metadata reconnaissance, pool-exhaustion timing,
panic fragments, and attacker-thrown strings) were returned verbatim in
the public /api/v1/execute + user-route 502 bodies. This handler only
serves anonymous data-plane callers (the authenticated script-test path
is in manager-core and keeps verbose errors), so log the full text
server-side under a correlation id and return a stable generic message
plus the id for operator grep.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
C-2 follow-up: sanitize_stored_content_type previously let a CRLF pass
through an allowlisted prefix (e.g. "image/png\r\nX-Injected: 1" matched
the image/ branch and returned the original), which would inject a
response header / panic HeaderValue on download. Now any control byte
(<0x20 or 0x7f) coerces to application/octet-stream up front.
F-SE-H-03: disable_symbol("debug") to match "print" (the comment
already claimed both were disabled) so scripts can't write
attacker-controlled bytes to the operator's stderr.
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>
Postgres-backed integration tests surfaced two assertion fixes while
verifying the audit branch:
* email_inbound::create_without_inbound_secret_is_rejected (added in the
H-B2 commit) asserted 400; TriggersApiError::Invalid maps to 422
(UNPROCESSABLE_ENTITY). Corrected.
* api::version_includes_public_base_url had two rotted assertions —
`schema` pinned at 6 (it's migrations::latest_version(), which had
climbed to 41 and is now 42 after 0042_secrets_envelope_version.sql)
and `sdk` pinned at "1.1" (the crate is at "1.10"). The test is
#[ignore]-gated so the rot went unnoticed in normal runs. Both pinned
to current reality. These were pre-existing failures, not caused by
the security changes.
Verified against a throwaway Postgres: schema_snapshot, api (53),
authz (28 of 29 — see below), dispatcher_e2e, email_inbound, invoke_e2e,
queue_e2e all green.
Known pre-existing failure (NOT fixed here, out of Tier 1+2 scope):
authz::deactivating_user_revokes_their_api_keys fails identically on the
pre-branch merge-base. It's the PrincipalCache revocation-lag Medium the
audit flagged as unfixed (resolve_principal caches by token-hash with no
eviction on deactivation; lag bounded by the 60s TTL). Deferred to a
Tier 3/4 pass.
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>
The PATCH /api/v1/admin/admins/{id} handler's password branch updated
the password hash but explicitly preserved every live session and API
key for the target user. That made password rotation a non-event for
credential compromise: a hijacked session that triggered a password
change kept its grip after the rotation, and a leaked API key survived
its owner's "I'm rotating because I think I'm compromised" reaction.
Now the password branch mirrors the deactivation branch:
1. update the password hash (unchanged),
2. `state.sessions.delete_for_user(id)` — wipes every live session
including the caller's, which logs them out and forces re-login
under the new password,
3. `state.keys.expire_all_for_user(id)` — revokes every API key.
This matches the `cmd_reset_password` CLI flow already in the codebase.
The dashboard's 401 handler redirects to the login screen, so the UX
on self-change is "you're logged out; sign back in".
Audit ref: security_audit/01_authn_session.md (H-E1).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tokio::time::timeout over JoinHandle only drops the future — the
spawn_blocking OS thread keeps running until the Rhai script self-
completes or hits the per-op budget. A `loop {}` body with a generous
max_operations could pin a blocking worker for tens of seconds; on Pi-
class hardware one anonymous-callable script call could permanently
subtract a worker from the pool.
Installs an `engine.on_progress` hook that consults a thread-local
deadline; when `Instant::now() >= deadline` the hook returns `Some(_)`,
triggering `ErrorTerminated` inside the Rhai loop. The deadline is set
by `Engine::execute_with_deadline` / `Engine::execute_ast_with_deadline`
via an RAII `DeadlineGuard`, called from the orchestrator client; the
old `execute` / `execute_ast` paths are unchanged so tests, validation,
and the parse-only path keep working.
Invoke re-entry (`sdk/invoke.rs`) inherits the parent's deadline
transparently — the thread-local is set for the entire spawn_blocking
lifetime, and Rhai is single-threaded so the same thread services the
parent + every sub-script.
New tests:
* `deadline_terminates_a_runaway_loop` — `loop {}` body with
`max_operations = u64::MAX` and a 100 ms deadline aborts within 2 s
(typically <200 ms). Maps to `ExecError::Runtime`.
* `no_deadline_set_does_not_abort` — `None` deadline keeps the pre-
audit behavior; the op-budget still bites.
Audit ref: security_audit/05_sandbox_exec.md#f-se-h-01.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds defense-in-depth HTTP headers to the dev and prod Caddyfiles. CSP,
X-Frame-Options, Permissions-Policy, and Cache-Control: no-store apply
to the dashboard SPA (/admin/*) and admin API (/api/v1/admin/*). nosniff
and Referrer-Policy apply to every response (a sane default). HSTS is
unconditional in prod.
User-route responses (the catch-all `handle`) deliberately get NO CSP —
user scripts own their own response headers by design.
The CSP / X-Frame-Options / Cache-Control / Permissions-Policy headers
use the `?` operator ("set only if not already present") so application
responses with their own restrictive policy — notably the audit C-2
file-download handler, which sends a sandboxed CSP — win over Caddy's
defaults.
This downgrades H-A4 (dashboard token in localStorage) from acute to
defense-in-depth: with this CSP and no inline JS in the dashboard,
XSS-based token exfiltration is no longer a one-step exploit.
Validated with `caddy validate` (docker caddy:2-alpine) for both files;
also `caddy fmt --overwrite`'d.
Audit ref: security_audit/07_http_cors_csrf_xss.md (H07-03/04/05 +
M07-06/07/08/09).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stored XSS: the previous get_file handler streamed user-supplied bytes
with Content-Disposition: inline, the user-supplied Content-Type, and
no X-Content-Type-Options / CSP. A Rhai script could store an SVG or
HTML payload whose download URL rendered same-origin under the admin
session cookie.
Closes the response side and the storage side:
* shared::sanitize_stored_content_type: allowlist
(octet-stream/pdf/json/text-plain/text-csv/image-non-svg/audio/video)
with anything else coerced to application/octet-stream. New unit tests
cover the safe/unsafe/case-insensitive/parameter-preserving paths.
* files_service::create/update: sanitize the stored content_type after
the shape checks pass (sanitize-after-validate keeps the existing
MissingField / TooLong errors intact). Two new tests confirm text/html
and image/svg+xml are coerced to application/octet-stream on
create/update respectively.
* files_api::get_file (admin download):
- Content-Disposition: attachment (was inline)
- Content-Type re-sanitized via the shared helper as belt-and-
suspenders for any pre-existing row that pre-dates this change.
- X-Content-Type-Options: nosniff
- Content-Security-Policy: default-src 'none'; sandbox;
frame-ancestors 'none'
- Referrer-Policy: no-referrer
Audit ref: security_audit/07_http_cors_csrf_xss.md#c07-02 (response side)
+ security_audit/06_files_pathtraversal.md#f-fs-001 (storage side).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same-origin CSRF: the platform co-hosts user-route HTML on the same
origin as /api/v1/admin/*; SameSite=Lax did not block a same-origin POST
from a malicious script at /<route>, riding the admin's picloud_session
cookie. The dashboard already uses Bearer (dashboard/src/lib/auth.ts +
api.ts:495), so cookie auth was dead weight on the admin side and
exploitable.
Cuts the cookie path entirely:
- auth_middleware::extract_token is Bearer-only; cookie branch removed.
- auth_api::login no longer sets Set-Cookie.
- auth_api::logout no longer clears the cookie (the bearer token is
still revoked by deleting the session row).
- extract_token_for_logout matches.
- SESSION_COOKIE const + PICLOUD_COOKIE_SECURE env var deleted.
Audit ref: security_audit/07_http_cors_csrf_xss.md#c07-01.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stage 6 review Obs 1b: the 300s default lived as a literal in two
places (DEFAULT_ASYNC_EXEC_TIMEOUT in dispatcher.rs and
DEFAULT_SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS in triggers_api.rs). A
future change to the dispatcher would silently leave the
visibility-timeout warn threshold out of sync.
Extract a single pub const DEFAULT_ASYNC_EXEC_TIMEOUT_SECS: u32 = 300
in dispatcher.rs, derive the existing Duration constant from it, and
re-export it under the triggers_api name so the validator's
self-contained semantics are preserved at the call site. Tests still
inject the value explicitly via TEST_SAFE_LIMIT — no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stage 6 follow-up Obs 1: the hard-coded
SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS = 300 didn't follow
PICLOUD_DISPATCHER_ASYNC_EXEC_TIMEOUT_SEC. A deploy that raised the
executor budget produced false-positive warns; a deploy that lowered
it missed real visibility races.
Refactor validate_queue_visibility_timeout to take safe_limit_secs as
an explicit parameter. The call site reads the live env-overridable
value via safe_visibility_vs_exec_budget_secs() each call; unit tests
inject the boundary directly without touching global env state. New
test safe_limit_threshold_tracks_caller_value asserts both
budget-raised and budget-lowered code paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- F-T-003 actually closed: clients/typescript/src/subscribe.ts now
bounds the 401-refresh loop at 3 consecutive refusals, surfaces an
onError describing the loop, and resets on any successful stream
open. New test covers the cap. The Stage 2 dashboard fix only
addressed adminRequest in dashboard/src/lib/api.ts; the originally-
cited TS client file was untouched.
- users/invitations subtab dropped the redundant api.apps.get fetch
the other 5 subtabs already shed in Stage 6.
- Queue visibility-timeout validator pulled out as
validate_queue_visibility_timeout, with two thresholds: hard-reject
below MIN_QUEUE_VISIBILITY_TIMEOUT_SECS (30s — catches typos), warn-
log between MIN and SAFE_VISIBILITY_VS_EXEC_BUDGET_SECS (300s) so
operators see when their visibility is below the dispatcher's
per-message executor budget. Stage 6 only had the hard floor; the
reviewer caught that a 60s handler still races a 30s visibility
even after the floor. Four new unit tests cover none/above-safe/
between/below-min.
- pic dead-letters count subcommand: cheap headless probe for
unresolved DL totals, parallels the dashboard's badge query.
- pic dead-letters replay now has a happy-path integration test
(replay_against_real_dl_row_succeeds): inserts a synthetic DL row
directly via the rust-postgres sync driver (added as dev-dep),
drives `pic dead-letters replay`, asserts the row is resolved with
reason=replayed and count drops back to 0. Plus a count smoke test.
All 75 CLI integration tests + 16 TS client tests + 4 new
visibility-timeout unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes 4 dashboard hardening findings and 5 of the Lows from the audit.
Dashboard hardening:
- Subtabs no longer re-fetch the app via api.apps.get on every page
load. users/files/dead-letters drop the fetch outright (the variable
was set but never read); queues + queues/[name] now consume the
layout's AppContext via getContext for the page title. The layout's
reloadApp() owns the historical-slug redirect — subtab-local redirect
blocks are removed so there's no race.
- The global :global(details > summary::before) chevron is now scoped
to details.chevron. The script editor's "Advanced sandbox" details
and the inbound-email-shape help-text both opt in; the script
exec-list logs no longer inherit a spurious chevron.
- deriveTab now matches the path segment by anchored ===, so a future
/apps/<slug>/queues-archived route wouldn't activate the queues tab.
Lows cherry-pick:
- ExecError gains Serialize/Deserialize derives + a snake_case tag so
RemoteExecutorClient (cluster mode v1.3+) can round-trip the variant.
- triggers_api rejects queue triggers whose visibility_timeout_secs is
below the dispatcher's per-message executor budget; with no minimum
the reclaim task races the handler and the queue silently
double-delivers. Existing test using 5s updated to 30s.
- New migration 0040: execution_logs.script_id cascade switched from
ON DELETE CASCADE to ON DELETE SET NULL so deleting a script no
longer wipes the forensic history that motivated the delete.
- New migration 0041: dead_letters composite index on
(app_id, created_at DESC) so the "list all" dashboard view stops
falling back to seqscan + sort when unresolved=false.
- Schema snapshot re-blessed.
Deferred to v1.2: the ExecRequest principal serde(skip) marker
(documented in-place; the cluster-mode PR will introduce the wire-safe
snapshot at that point) and the `pic --help` mention of
`picloud admin reset-password` (one-line follow-up).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the audit's High-severity CLI coverage gap, raising the count
from ~25 to ~40+ subcommands and bringing the integration test count
from 63 to 73.
- pic triggers {ls, rm, create-kv, create-cron, create-dead-letter,
create-from-json}: three per-kind wrappers cover the most common
trigger shapes; the generic create-from-json is the escape hatch
for docs/files/pubsub/email/queue and any future advanced retry
knobs — body JSON inline, via @<file>, or "-" for stdin.
- pic dead-letters {ls, show, replay, resolve}: full operator
workflow for the dispatcher's dead_letters rows, including the
--unresolved filter and the per-row replay + manual-resolve actions.
- pic secrets {ls, set, rm}: list names + updated_at, set values
via stdin (the only secure channel — inline values would leak
into shell history), and delete by name. --json on set treats
stdin as raw JSON for non-string values.
Ten new integration tests follow the established #[ignore] pattern,
gated on DATABASE_URL. All 73 ignored tests pass against the local
dev stack.
The `pic admin reset-password` server-binary subcommand exists on
the picloud binary side already; the audit's "surface it in pic
--help" note is a one-line addition deferred to Stage 6 with the
other small UX touches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the audit's Critical "operator can't deploy a serverless endpoint
end-to-end from the CLI" finding. Two new subcommand families bring the
CLI coverage from 14 commands to ~25:
- pic routes {ls, create, rm, check, match}: full route CRUD plus the
dry-run conflict checker and the URL matcher already exposed by the
dashboard. The create form accepts --path-kind, --host-kind, and
--dispatch (sync|async) so async routes can finally be created
headlessly. The ls output adds a dispatch column.
- pic admins {ls, create, show, set, rm}: per-instance admin user
management. Create reads passwords from stdin via --password - so
shell history never sees the cleartext. Set is a JSON-Merge-Patch
shape that lets operators deactivate accounts or rotate roles
without touching the dashboard.
Six new ignored integration tests follow the established #[ignore]
pattern (DATABASE_URL gates them). They cover the happy-path round
trip, the async-dispatch persistence path, the password-required
error path, and the capability gate (a Member sees HTTP 403). All
pass against the local dev stack with PICLOUD_DEV_MODE=true +
PICLOUD_DEV_INSECURE_KEY=i-understand-this-is-insecure.
The `pic members` and `pic domains` subcommands the audit mentioned
are deferred — the apps_api shape may shift in v1.2 with per-app
roles and rebuilding the CLI surface twice would be churn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the queue DL fan-out gap (audit Medium) and surfaces the
KV/docs/files non-transactional emit gap to operators.
- handle_queue_failure previously called queue.dead_letter to write the
DL row but never invoked fan_out_dead_letter. The comment claimed
"the outbox arm fires registered dead_letter handlers off the new row"
— but queue DL rows are written via a separate path that bypasses the
outbox entirely, so handlers filtered on source="queue" sat idle
forever. Refactor fan_out_dead_letter to take a DeadLetterFanOutCtx
struct so both the outbox arm and the queue arm can call it; the
queue arm constructs a TriggerEvent::Queue from the claimed message
and passes it through.
- New integration test queue_dead_letter_fans_out_to_dead_letter_handler
registers a dead_letter trigger filtered on "queue", forces queue
exhaustion, asserts the handler fires with the correctly-shaped event.
- KV/docs/files services committed the data write then ran events.emit
as a separate operation, logging-and-swallowing on failure. Bump the
six call sites from tracing::warn to tracing::error with an
event_emit_failure=true marker so operators can grep them. The full
single-tx repo refactor (extending ServiceEventEmitter with
emit_in_tx + tx-aware *_repo methods) is documented in kv_service.rs
as a v1.2 follow-up — it's a meaningful redesign that deserves its
own pass (pubsub_service::fan_out_publish is the reference shape).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes 4 audit findings:
- Route TS interface now carries dispatch_mode (sync|async). Backend's
shared::route::Route has had this since 0012_routes_dispatch_mode but
the TS client silently always created sync routes and the list
display dropped the field. Add a Dispatch select to the new-route
form and an "ASYNC" badge in the route list.
- api.files.downloadUrl pointed to a never-registered backend endpoint.
The dashboard's live Download button was hitting 405. Add the GET
handler (AppFilesRead + FilesRepo::head + FilesRepo::get, Content-
Disposition: inline) at the same path that delete already used.
- F-T-003: adminRequest's 401 handler called goto(login) without a
recursion cap. If the login endpoint itself returned 401, the wrapper
looped until browser nav limits. Track consecutive 401s within a 10s
window and hard-reload to /login?reason=auth-loop after the third,
showing the user an explanatory banner. Any 2xx resets the counter.
- Login flow now honors a ?returnTo= query parameter. adminRequest and
the root layout both append the current location when redirecting to
/login; the login page validates the value is a same-origin admin
path (no open-redirect) and goto's there after successful sign-in.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes 4 High-severity audit findings:
- describe_event broken for HTTP-async payloads: was reading source/op
JSON fields that HttpDispatchPayload doesn't carry, producing empty
source on DL rows. from_wire("") then fell back to Kv on replay, the
dispatcher tried to resolve a non-existent trigger, and replay no-op'd
silently. Now branches on OutboxSourceKind: HTTP rows emit
"<method> <path>" + source="http"; Invoke rows emit "invoke_async" +
source="invoke"; TriggerEvent payloads keep top-level field extraction.
- HTTP outbound Authorization leaked across cross-origin redirects.
Manual redirect loop (Policy::none) reused header_map on every hop
and only scrubbed Content-Type for POST->GET. Now compares
url::Url::origin() across hops and strips Authorization,
Proxy-Authorization, Cookie when origin changes — matching reqwest's
default policy.
- F-S-010 inbound email HMAC had no replay protection. Signature was
computed over body only with no timestamp or nonce, so captured POSTs
were replayable indefinitely. Adds X-Picloud-Timestamp header bound
into the HMAC input (signed string is ts || "." || body), 300s
tolerance window, and a process-local LRU keyed on
(ts, sha256(body)) with 600s TTL to catch within-window replays.
Breaking change: webhook senders must include the timestamp header.
- expected_schema.txt re-blessed for migrations 0036-0039 (the previous
snapshot stopped at 0035 so CI would have gone red on first run).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses two interrelated UX complaints:
1. Browser-default controls leaking through the dark theme
(native <select> chevrons, OS checkbox/radio look, date-picker
icons, <details> triangle marker, number input spinners).
2. Subroute pages dropping the main app's tab bar in favor of a back
link, breaking navigation continuity across users/files/queues/
dead-letters/queues-[name].
Design tokens (dashboard/src/routes/+layout.svelte):
- Token vocabulary expanded with 18 new variables covering
text-strong, accent + accent-fg, danger/success/warning bg/fg/border
triplets, bg-elevated-hover, radii (sm/md/lg/pill), shadow-elev-2,
and z scale (popover/modal/toast). 7 alias tokens (--muted,
--link, --text, --color-error, --color-border, --chip-bg,
--code-bg) absorb the orphan references the F-U-004 remediation
partially renamed.
- Global :global(...) resets for <select>, <input type='checkbox'>,
<input type='radio'>, <input type='number'>, <input type='date'>,
and <details>/<summary> ensure native controls track the dark
palette out of the box. No per-page edits needed.
Tab consistency:
- New dashboard/src/lib/AppTabBar.svelte renders all 11 per-app
tabs (Scripts, Domains, Members, Triggers, Topics, Secrets,
Settings, Users, Files, Queues, Dead letters) as <a> links with
an active highlight derived from the URL. Tabs that switch
in-page panels go to ?tab=<id>; tabs that switch routes go to
the subroute. Admin-only tabs are hidden when canAdmin is false.
- New dashboard/src/routes/apps/[slug]/+layout.svelte loads the
app once, handles the historical-slug redirect, exposes the
shared app + canAdmin + canWrite + dead-letter-count state via
Svelte context, and renders the breadcrumb + AppTabBar above
every per-app page. The 5 subroute pages drop their own "← back"
headers since the layout owns them now.
- apps/[slug]/+page.svelte's local-state activeTab becomes URL-
driven via $page.url.searchParams.get('tab'). Defense-in-depth
redirect for non-admin viewers landing on admin-only tabs uses
goto({replaceState:true}) instead of mutating state.
Light-theme leftovers swept on 5 subroute pages:
- dead-letters: error banner, badge, pre/code blocks all swap to
--color-danger-*, --bg-elevated, --text-primary
- files: button.danger, var(--muted,#666) → token-only
- queues + queues/[name]: bare hex fallbacks removed; .toolbar and
.auto-refresh styled with tokens; data-testid for the queues
empty state (already added by previous commit, reaffirmed here)
- users + users/invitations: badge-ok/badge-pending now use
--color-success-bg/fg and --color-warning-bg/fg; chips use
--bg-elevated + --text-strong; .create-form gets a token-styled
surface; row-action buttons gain explicit dark-theme styling
E2E selector updates:
- members.spec.ts, integration.spec.ts, apps.spec.ts — the tab bar
is now <a> elements (role=link) without a count suffix. Test
selectors swap from getByRole('button', name: /^Scripts \(\d+\)$/)
to getByRole('link', { name: 'Scripts' }), etc.
- New navigation/tabs.spec.ts still passes; existing 60+ tests
unchanged except for the selector swap. Two pre-existing failures
(routing.spec.ts:79, integration.spec.ts:89) untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses every finding from the four-agent review of commit aa493b9.
Dashboard — honor redirect_to on subtab loadApp() (closes the silent
historical-slug redirect UX gap):
- queues/+page.svelte
- queues/[name]/+page.svelte
- files/+page.svelte
- dead-letters/+page.svelte
After a rename, the URL bar now reflects the canonical slug instead
of silently rendering the renamed app's data under the stale URL.
Mirrors the established pattern at apps/[slug]/+page.svelte:619-623.
manager-core:
- queues_api.rs IntoResponse now uses the JSON envelope shape
`{"error": "..."}` consistent with every sibling admin api file.
- triggers_api::delete_trigger reordered: cap check fires BEFORE the
trigger load, closing the 404-vs-403 existence side channel an
unauthorized caller could otherwise probe.
- InMemoryAppRepo mocks in topics_api + triggers_api now implement
get_by_slug + get_by_slug_or_history (previously
`unimplemented!()`), unblocking handler-level slug-input tests.
- Added 4 slug-acceptance tests to topics_api and 2 to triggers_api
(slug-resolves, unknown-slug-404, historical-slug-resolves,
create-via-slug). Also added delete-without-cap-is-forbidden test
pinning the new cap-first order.
e2e:
- navigation/tabs.spec.ts split per-tab so a regression on one tab
no longer masks regressions on the others.
- Negative assertion widened: captures every /api/v1/admin/apps/*
response and fails on any 4xx/5xx — not just the literal "Cannot
parse" string. Catches a broader regression shape.
- networkidle replaced with `expect(<main>).toBeVisible()` —
networkidle is officially discouraged for SPAs and was at risk of
timing out behind the queues auto-refresh.
- Cleanup registration moved BEFORE the create-app API call so a
flaky create still gets swept up.
- Queue drilldown route /queues/[name] now covered.
- Stable `data-testid="queues-empty-state"` replaces fragile
UI-copy substring match for the positive assertion.
- Header comment now spells out what this spec does and doesn't
catch.
docs:
- serverless_cloud_blueprint.md: slug-history described as
"200 OK + redirect_to" JSON envelope rather than "301 redirect"
— matches what apps_api actually implements (SPA can't honor a
mid-tree HTTP redirect).
Unit-test gap (acknowledged): queues_api, files_api, secrets_api,
dead_letters_api have zero in-process tests. Adding them properly
needs a shared mock-repo helper crate — the standalone trait surface
(QueueRepo + TriggerRepo + ScriptRepository + AuthzRepo + repo-
specific) is ~30 methods per file. Documented inline in queues_api.rs
near the resolver. Integration coverage via crates/picloud/tests/ and
the new e2e spec cover the same paths end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Compose convention is that override.yml is a per-developer file for
local-dev overrides (e.g., forwarding PICLOUD_DEV_MODE +
PICLOUD_DEV_INSECURE_KEY to the picloud container without
modifying the tracked docker-compose.yml).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Queues tab at /admin/apps/default/queues was returning
"Cannot parse `app_id` with value `default`: UUID parsing failed".
27 admin endpoints across 6 files used strict `Path<AppId>` instead
of the canonical `Path<String>` + `resolve_app()` pattern from
app_repo.rs:34. Working endpoints (apps, app_members, users_admin)
all use the lenient pattern; this commit brings the remaining 6
files into line:
- queues_api.rs — 2 handlers
- files_api.rs — 2 handlers
- secrets_api.rs — 3 handlers
- topics_api.rs — 4 handlers
- dead_letters_api.rs — 5 handlers
- triggers_api.rs — 10 handlers
The handler bodies (authz, repo calls) are unchanged; only the path
extractor and the per-file `ensure_app_exists` helper (now renamed
`resolve_app`) move. Lib tests updated to pass `.to_string()` at
the call site (Path now takes String, not AppId).
email_inbound_api.rs deliberately stays strict-UUID — it's a public
webhook receiver consumed by external providers, not by the
slug-based dashboard.
Adds Playwright spec `dashboard/tests/e2e/navigation/tabs.spec.ts`
covering every per-app tab (queues, files, dead-letters, users,
invitations, plus the main page hosting triggers/secrets/topics)
with a negative assertion against the "Cannot parse" error text
plus a focused regression test for the original queues report.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Multi-agent audit of main at v1.1.9 covering Security, Performance,
Code Quality, UI/UX, Migration/Schema, and TypeScript client.
115 actionable findings; severity-ranked and dedup'd.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cargo fmt regroups; three clippy fixes:
- F-S-006: hoist MAX_API_KEY_CANDIDATES to module scope to satisfy
clippy::items_after_statements.
- F-S-009: use map_or instead of map().unwrap_or() per
clippy::map_unwrap_or.
- F-P-012: replace `|c| c.encode()` closure with the method itself
per clippy::redundant_closure_for_method_calls.
No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConfirmModal focused the first input on mount and listened for Escape,
but didn't trap Tab. Keyboard users could Tab out of the modal into
the underlying page — a strong accessibility violation and a confusing
keyboard-UX moment.
Add a handleTrapTab keydown handler on the dialog div that:
- Enumerates focusables (button:not([disabled]), input:not([disabled]),
select, textarea, a[href], [tabindex]:not(-1)).
- On Tab from the last focusable, wraps to the first.
- On Shift+Tab from the first, wraps to the last.
The dialog ref is bound via bind:this; the handler is no-op if the
dialog isn't mounted yet.
AUDIT.md anchor: F-U-016.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
invoke_service::resolve and enqueue_async performed no authz check —
no AppInvoke capability existed. Same-app isolation was preserved
(cross-app guards work), but within one app an anonymous public-HTTP
script could trigger any other script (e.g. an admin-only worker that
hits secrets/files/external HTTP). Worse: invoke_async runs the
callee with principal: None, so the callee could hold capabilities
the original public caller shouldn't.
- Add Capability::AppInvoke(AppId). app_id() / scope_for_capability
(script:write) / role_satisfies (editor+) are all updated.
- InvokeServiceImpl gains an optional `authz: Option<Arc<dyn AuthzRepo>>`
+ a `with_authz` builder. When set, resolve() runs script_gate on
AppInvoke before doing the cross-app id check.
- picloud/src/lib.rs wires it: `InvokeServiceImpl::new(...).with_authz(...)`.
- Anonymous callers (cx.principal == None) continue to skip the check
via script_gate, preserving the public-HTTP convention.
Existing 5 invoke_service unit tests still pass (the tests use the
authz-less constructor, so the gate is a no-op there).
AUDIT.md anchor: F-S-012.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A 70-line newtype lived in picloud/src/lib.rs wrapping
Arc<PostgresScriptRepository> and re-implementing every ScriptRepository
method as `self.0.method(...).await`. Hand-delegation invited silent
skew when the trait gained a method, and forced 8 call sites to know
about the wrapper.
Add a blanket
impl<T: ScriptRepository + ?Sized> ScriptRepository for Arc<T>
in manager-core::repo. The implementations forward via (**self).method(...)
so any owner of an Arc<dyn ScriptRepository> (or Arc<ConcreteImpl>) can
be passed where the trait is expected.
Migrate the 7 picloud-binary call sites:
Arc::new(PostgresScriptRepoHandle(script_repo.clone())) → script_repo.clone()
Delete the newtype + its hand-delegated impl. Leaves a 6-line comment
documenting why the boilerplate is gone.
AUDIT.md anchor: F-Q-011.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Queue depths change continuously; the page was a load-time snapshot
with no way to update without a hard refresh. Dead-letters has a
Refresh button; queues didn't.
Add:
- A Refresh button that re-fires loadQueues() (and shows "Refreshing…"
while in-flight).
- An "Auto-refresh every 5s" checkbox. setInterval lifecycle is
managed via onDestroy so navigating away cancels cleanly.
Queue-detail page is unchanged in this commit; same pattern can be
applied there in a follow-up.
AUDIT.md anchor: F-U-013 (overview list; detail page deferred).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Listing showed name/content-type/size/created/id; only mutation was
Delete. No download link, no copy-id button (the UUID was rendered
unclickable), no preview, no metadata refresh. Operators had to use
the admin API directly.
- Add api.files.downloadUrl(slug, collection, id) helper that builds
the admin GET URL. The anchor uses HTML `download` so the browser
saves with the original filename.
- Add a ⧉ copy-id button next to the UUID column using
navigator.clipboard.writeText. Silently no-ops where the browser
doesn't expose clipboard (plain-http LAN).
- Preview / metadata-refresh are deferred to a UI overhaul.
AUDIT.md anchor: F-U-009 (partial — download + copy; preview deferred).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
