Remediate the HIGH and security-relevant findings from the 2026-07-11 audit.
H1 — the per-env approval gate is now server-authoritative. The governing
project is resolved from the target node's nearest-claimed ancestor
(`governing_env_policy`/`_tree` + `governing_project_id` +
`ProjectRepository::get_environments_by_id`), independent of the client-supplied
`[project]`. Omitting or spoofing the project block can no longer skip a gate the
owning project established; a to-create group resolves its declared parent's
chain so a fresh subtree node inherits the gate. Fails closed on any read error.
H2 — the API-key prefix slice (`&rest[..8]`) is now the boundary-safe
`rest.get(..8)`, so an attacker-supplied multibyte bearer can't panic the request
task (unauthenticated per-request DoS). Regression test added.
C1 — admin sessions gain an absolute lifetime cap (migration 0070,
`PICLOUD_SESSION_ABSOLUTE_TTL_HOURS`, default 30d): `lookup` filters it, `touch`
clamps the sliding bump to it, so a continuously-used or stolen-but-warm token
self-expires. Mirrors the data-plane app-user cap.
C2 — `Cache-Control: no-store` on the login and API-key-mint responses (the two
that return a raw credential), so a proxy/CDN/browser cache can't retain it.
B8 — file downloads are header-safe: `sanitize_stored_filename` guarantees a
valid `HeaderValue` (no panic on a control-char name) and BOTH the per-app and
group download paths now set attachment + `X-Content-Type-Options: nosniff` +
a restrictive CSP, closing a group-path stored-XSS gap.
Also folds in the server-side plan-warning plumbing (`plan_warnings`,
`PlanResult::warnings`) and the `app_only_reject` message helper that the CLI
plan-preview change builds on, plus operator security notes (reads-open shared-
topic SSE; the `--env` label is advisory, not a boundary).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-env approval policy was applier-supplied — a hand-crafted request
that omitted `project.environments` was ungated, and flipping a gate to
`confirm = false` in the same request un-gated it. Persist the policy
server-side and enforce against `persisted ∪ declared`.
- Migration 0067: `project_environments (project_id, env_name, confirm)`,
CASCADE on the project. Written declaratively (delete-then-insert) inside
`upsert_project_tx`, same tx as the project row + node claim.
- `ProjectRepository::get_environments_by_slug` (read side) +
`ApplyService::effective_env_policy` union the persisted policy with the
request's declared one (confirm if EITHER says so — monotonic).
- `env_gate_check` now evaluates the effective policy; the three handlers load
it before the gate. `plan.approvals_required` is the effective gated set, so
CI sees a persisted gate even when the manifest omits it.
- The union rule closes both bypasses: an omitted policy still trips a
persisted gate, and an ungating apply must itself pass the gate (the
declarative replace then takes effect next time) — TOCTOU-safe.
Pinned by env_approval::{persisted_policy_gates_a_request_that_omits_it,
flipping_a_gate_to_false_in_the_same_request_still_gates} +
projects_repo::get_environments_by_slug_returns_persisted_policy. Schema golden
reblessed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-env approval gate (`[project.environments]`, `confirm = true`) was
client-side only — the policy was `#[serde(skip_serializing)]`, so a patched or
older CLI applying to a confirm-required env bypassed it entirely. This makes
the gate server-enforced, admin-gated, and audited (the deferred §4.2 piece).
- Wire: `ProjectDecl` gains `environments` (the CLI stops skip_serializing it and
sends the selected `env` + the `--approve` set on the apply request). The
server RE-DERIVES the gate from the request (`env_gate_check`): a
confirm-required env not in `approved_envs` → 409 `ApprovalRequired`.
- Admin gate: an approved override additionally requires `AppAdmin`/`GroupAdmin`
on EVERY declared node — a step up from the editor write caps an ordinary
apply needs (reusing the admin caps per §4.2) — and emits a `tracing::info!`
audit line. Enforced on all three handlers (single app, single group, tree);
the check runs before any tx, so a denial (403) precedes any mutation.
- Token: the env policy folds into the TREE bound-plan token (a policy edit
between plan and apply trips StateMoved). The single-node token is left
unchanged (a bare live-state hash), so plan/apply still match without threading
a project into it.
- CLI: single-node `apply`/`plan` now resolve the GOVERNING project (own block
else nearest ancestor's) so a leaf apply carries the root's policy; `plan`
surfaces `approvals_required`. Client-side `require_env_approval` fast-fail
kept as UX.
Adapts the earlier feat/ownership-and-approval M5 (654e387) to main's DTOs; main
addresses tree group nodes by slug only, so the admin loop mirrors authz_tree
(app: resolve_app_id fail-closed; group: get_by_slug, skip to-create).
Threat-model note (honest scope, in the doc + code comments): this closes the
patched/older-CLI bypass and admin-gates + audits the override, but is NOT a
hermetic boundary against a hand-crafted request that OMITS the policy — the
policy is applier-supplied, not persisted server state. Full closure (persist a
`project_environments` source of truth + a server-determined env) is deferred.
Review (adversarial pass): fixed a MEDIUM (single-node `plan` didn't resolve the
governing project, so a leaf plan under-reported the gate `apply` enforces) and a
LOW (duplicate clippy attr); corrected overstated "can't bypass" wording.
Tests: ProjectDecl gate unit test; env_approval journeys
`server_enforces_the_gate_against_a_non_stock_client` (raw request → 409 without
approval, admin-approved → 200) and `approving_a_gated_apply_requires_admin`
(editor + --approve → 403). 417 lib tests + 33 CLI journeys + workspace clippy
-D warnings + fmt clean. No migration.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review #2 (MEDIUM). The M3 `[project.environments]` approval gate silently
failed open in three shapes:
- `EnvPolicy.confirm` was `#[serde(default)]`, so `production = {}` parsed as
un-gated. `confirm` is now a REQUIRED field — an env listed with an empty
policy is a load error (`missing field 'confirm'`), not a silent no-gate.
- `pic apply --file <leaf>` where the leaf carried no `[project]` skipped the
gate even when the repo root gated the env. `run` now discovers the governing
`[project]` by walking up from the manifest to the nearest ancestor
`picloud.toml` that declares one (`find_governing_project`, loaded without the
env overlay — only the block matters).
- A `[project].environments` in a non-root manifest under `--dir` was dropped
with a generic "ignored" note; `build_tree` now warns explicitly that its
approval gating is not enforced.
Pinned by `tests/env_approval.rs` (empty-policy load error; leaf `--file` apply
honoring the root gate).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
