diff --git a/README.md b/README.md index 3cf676c..1019a28 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,90 @@ from the site key by exact path — never a glob wider than the one site: In cluster mode every node's daemon runs the same teardown against its own disk, which is the complete story — each node reaps its own replicas. +### Pre-serve static-analysis gate + +A preview builds and serves **untrusted pull-request code**. With +`--analyze-config` set, switchboard screens that code before it goes live: after +the checkout is materialized and **before** the atomic swap makes the vhost +routable (and before `build:`/`seed:` execute any of it), it runs + +```text +ephpm analyze --config --format sarif +``` + +over the pristine tree and **refuses to publish on a bad verdict**. On a +redeploy the previous, known-good container stays live untouched, because the +swap simply never happens. + +- **Off by default.** With no `--analyze-config` the gate is disabled and deploys + behave exactly as before (startup logs one `WARN`). This is the safe rollout + default: the analyzers ship in a recent `ephpm`, and a node on an older binary + must not have every preview blocked. The gate is live only once + `--analyze-config` is set **and** the node's `--ephpm-bin` supports `analyze`. +- **The policy is the operator's, never the PR's.** `--config` is passed + explicitly, which overrides `ephpm analyze`'s auto-discovery of a + `.ephpm-analyze.yml` **inside the checkout** — otherwise a malicious PR could + ship `enable: []` to neuter its own gate. Keep the policy file outside any + tenant docroot, and use absolute paths inside it. +- **Screens the pristine code.** The gate runs before switchboard injects the + resolved `env:` (`.env` + prepend), so the analyzer never scans switchboard's + *own* secrets — only the pull request's code. +- **Fail closed.** Exit `0` publishes; exit `2` (quarantine) and `3` (deny) + block; exit `1` (analyzer error), any other code, a timeout, and a binary that + will not spawn **all block**. A gate that cannot run must not wave code + through. The wall-clock timeout is `--analyze-timeout-secs` (default 180s). +- **Blocked previews are reported.** The block is posted into the PR's sticky + comment — the verdict, the finding count, and the top ~10 findings (rule, file, + line, message) — and the GitHub deployment status is set to **failure**. The + job is left in `queue/claimed/` for inspection (marked failed), not cleared as + a success; a later push re-runs the gate and refreshes the comment. +- **Scanned once per commit, cluster-wide.** switchboard runs on every node and + each materializes the same checkout, so a naive gate would re-scan the identical + commit N times. The verdict is a pure function of (repo, PR head SHA, gate + config), so it is deduplicated through ePHPm's **gossip-replicated** KV: the + first node to scan a commit publishes its verdict under + `analyze:verdict::::` (TTL + `--analyze-verdict-ttl-secs`), and its peers reuse it — reconstructing the + identical block comment from the stored findings — instead of re-scanning. + `cfg_hash` is a fingerprint of the operator policy file, so editing the policy + re-scans everywhere. There is **no lock or leader wait**: if two nodes miss at + once and both scan, the result is identical, so the only cost is a redundant + scan — the same looseness the PR-comment dedup accepts. +- **Verdicts live in a switchboard-private namespace, not the preview's.** The + cache is stored under a reserved AUTH site (`\x1f`-prefixed, provably not a + valid preview site key) that **no preview tenant can authenticate to** — a + tenant's `ephpm_kv_*` is auto-scoped by ePHPm to its own resolved site key, so + it can only ever reach that one keyspace. switchboard holds the KV secret and + can address the reserved namespace; the running (untrusted) app cannot read or + write it. This is what prevents cache poisoning: were verdicts kept in the + preview's own keyspace, a malicious app could `ephpm_kv_set` a forged `Passed` + for a future commit it authors (it knows the repo/PR/SHA, and the config + fingerprint is derivable from the public policy) and bypass the gate on peers. +- **Fail closed on the gate, fail *safe* on the dedup.** A KV **read** error scans + locally (never skip the gate because coordination failed); a KV **write** error + proceeds with the local verdict (never block a deploy because publishing the + shared verdict failed). Coordination failure degrades to "each node scans + itself", never to "serve unscanned". The shared cache is active only when + `--analyze-config` **and** `--kv-secret-file` are both set (the KV secret derives + the per-site RESP password); without the secret, each node scans independently. + +A **reviewed example policy** is in +[`docs/analyze-gate.example.yml`](docs/analyze-gate.example.yml). It enables the +six **native** analyzers — `writable-exec`, `obfuscation-scan`, `secrets-scan`, +`composer-scripts`, `dangerous-sinks`, `wp-vuln` — which are a fast file-walk +(~2.6s on a 2000-file WordPress-scale tree, cold), so previews stay snappy. + +**Opt-in: `opcode-scan`.** It is deliberately left out of the default policy. It +compiles every PHP file through ePHPm's embedded Zend engine (~+5–15s on a full +WordPress tree), turning *suspected* sink findings into *confirmed* ones at a real +latency cost. A node that wants that stronger detection adds `opcode-scan` to both +`enable` and `required` in its policy file, accepting the extra per-preview time. +`wp-vuln` stays out of `required` in the example because its feed is optional — a +missing feed must skip, not gate. These analyzers require an `ephpm` build that +includes them (`writable-exec`/`obfuscation-scan`/`secrets-scan`/`composer-scripts` +landed recently); the gate stays off until `--analyze-config` is set and the +nodes run an `ephpm` that has them. + ### Preview privacy: the access gate A **private** repo's preview must not be world-readable. It isn't: switchboard @@ -339,6 +423,24 @@ a fork builds but every `${secret.NAME}` expands to the empty string (with a name-only warning). Fork **teardowns** are always processed; refusing them would strand previews on disk. +### Pre-serve analyze gate + +See [Pre-serve static-analysis gate](#pre-serve-static-analysis-gate) for what +this does and [`docs/analyze-gate.example.yml`](docs/analyze-gate.example.yml) for +a reviewed example policy. + +| Flag | Env | Default | Meaning | +|---|---|---|---| +| `--analyze-config` | `SWITCHBOARD_ANALYZE_CONFIG` | *(none)* | Operator `ephpm analyze` policy file. **Unset disables the gate** (deploys behave as before; startup `WARN`s). When set, every deploy runs `ephpm analyze --config --format sarif` before the swap and blocks on a bad verdict. Passed with an explicit `--config` so a PR's own `.ephpm-analyze.yml` cannot neuter it — keep it **outside** any tenant docroot, with absolute paths inside. | +| `--analyze-timeout-secs` | `SWITCHBOARD_ANALYZE_TIMEOUT_SECS` | `180` | Wall-clock timeout for one `ephpm analyze` run. A run that exceeds it is killed and the deploy is **blocked** (fail closed). Only consulted when `--analyze-config` is set. | +| `--analyze-verdict-ttl-secs` | `SWITCHBOARD_ANALYZE_VERDICT_TTL_SECS` | `86400` | TTL for a verdict published to the cluster-shared cache. The head SHA is the real invalidator; the TTL just GCs old entries. The shared cache needs `--kv-secret-file` too; without it each node scans independently. | + +Exit-code contract (from `ephpm analyze`): `0` publishes; `2` (quarantine) and +`3` (deny) block; `1` (analyzer error), any other code, and a timeout all block +(fail closed). The gate reuses `--ephpm-bin`, which must support `analyze`. When +`--kv-secret-file` is set the verdict is deduplicated cluster-wide (scanned once +per commit; peers reuse the shared verdict, fail-safe to per-node scanning). + ### Preview access gate (ephpm#487/#491) See [Preview privacy: the access gate](#preview-privacy-the-access-gate) for what @@ -351,7 +453,7 @@ these do; the full design is in | `--preview-session-secret-ref` | `SWITCHBOARD_PREVIEW_SESSION_SECRET_REF` | `env:EPHPM_PREVIEW_SESSION_SECRET` | The `session_secret` **reference** (`env:NAME` / `file:/abs` / literal) written into a gated preview's `[preview_auth]` and resolved to mint share tokens. Must be the **same** reference the `github-auth` issuer uses and must resolve to ≥ 32 bytes — a gated deploy whose secret does not resolve **fails** (fail closed). The resolved value must be identical in the ePHPm and switchboard environments. | | `--share-link` | `SWITCHBOARD_SHARE_LINK` | `false` | Mint a temporary shareable-URL capability and post it in the PR comment for each **gated** deploy. Opt-in: a share link is a bearer capability. | | `--share-link-ttl-secs` | `SWITCHBOARD_SHARE_LINK_TTL_SECS` | `86400` | TTL for a minted share link. Kept short — expiry is the primary control. | -| `--kv-secret-file` | `SWITCHBOARD_KV_SECRET_FILE` | *(none)* | File holding ePHPm's `[kv] secret`, used to derive the per-site RESP password so **teardown can bump the share-link revocation epoch**. Unset skips KV revocation (the override + checkout removal already revoke on this node). | +| `--kv-secret-file` | `SWITCHBOARD_KV_SECRET_FILE` | *(none)* | File holding ePHPm's `[kv] secret`, used to derive the per-site RESP password for two cluster-shared KV uses: **teardown bumps the share-link revocation epoch**, and the **analyze gate deduplicates its verdict** across nodes. Unset disables both (revocation falls back to override + checkout removal on this node; the analyze gate scans on every node). | | `--kv-addr` | `SWITCHBOARD_KV_ADDR` | `127.0.0.1:6379` | ePHPm's KV RESP listener (`[kv.redis_compat] listen`). Only used for revocation when `--kv-secret-file` is set. | The one-time fleet setup this pairs with — the GitHub OAuth App, the global @@ -436,10 +538,11 @@ pinned to the crate's MSRV on the ephpm org's self-hosted fleet. | `src/queue.rs` | Scan, claim (`link`+`unlink`), coalesce per label, complete; the enqueue timestamp a claimed job carries | | `src/validate.rs` | Claim-time re-validation of a deploy job: the queue-age bound and the current-PR-state check | | `src/drain.rs` | The `/drain` kick and the shared-secret file | -| `src/deployer.rs` | The provisioning pipeline: fetch → manifest → env → quarantine the manifest → per-site override → atomic swap → chown to tenant → build → seed → health. `build:`/`seed:` run sandboxed via `ephpm exec --site` (fail-closed if unsupported). | +| `src/deployer.rs` | The provisioning pipeline: fetch → manifest → **analyze gate** → env → quarantine the manifest → per-site override → atomic swap → chown to tenant → build → seed → health. `build:`/`seed:` run sandboxed via `ephpm exec --site` (fail-closed if unsupported). | +| `src/analyze.rs` | The pre-serve static-analysis gate: run `ephpm analyze` over the pristine checkout, the pure exit-code→proceed/block decision, SARIF finding parsing, the fail-closed contract, and the cluster-wide verdict dedup (pure `plan_from_lookup`, cacheable `CachedVerdict`) | | `src/site_override.rs` | The per-site override ePHPm reads: validating `docroot:` and the env prepend against ePHPm's own containment rules, the `[preview_auth]` gate section, rendering the TOML, and writing it atomically | | `src/preview_auth.rs` | The access-gate control plane: gating policy, session-secret resolution (fail closed), wire-compatible HS256 share-token minting, and the per-site KV password derivation | -| `src/kv.rs` | A tiny RESP2 client for bumping the share-link revocation epoch in a preview's KV keyspace on teardown (best-effort) | +| `src/kv.rs` | A tiny RESP2 client for ePHPm's gossip-replicated KV: bumping the share-link revocation epoch on teardown (`KvRevoker`) and the cluster-shared analyze verdict cache (`VerdictCache`, GET/SET-EX) — both best-effort | | `src/teardown.rs` | Preview teardown: vhost dir, per-site database, override file, vhost temp/session state root, the API's `applied/` marker, the share-link revocation epoch — and the refusal to call a partial teardown a success | | `src/manifest.rs` | The `ephpm.yaml` app manifest schema, and moving it out of the served root once read | | `src/secrets.rs` | `${secret.NAME}` resolution from switchboard's own store | diff --git a/docs/analyze-gate.example.yml b/docs/analyze-gate.example.yml new file mode 100644 index 0000000..ff61567 --- /dev/null +++ b/docs/analyze-gate.example.yml @@ -0,0 +1,51 @@ +# Example operator policy for switchboard's pre-serve analyze gate. +# +# This is a REVIEWED STARTING POINT, not a default — the gate stays OFF until an +# operator points `switchboard --analyze-config` (SWITCHBOARD_ANALYZE_CONFIG) at +# a file like this one AND the node's `ephpm` (`--ephpm-bin`) is recent enough to +# carry the analyzers below. +# +# SECURITY — where this file lives matters: +# * Put it OUTSIDE any tenant docroot, somewhere the tenant cannot write +# (e.g. /etc/switchboard/). switchboard passes it to `ephpm analyze` with an +# explicit `--config`, which overrides `ephpm analyze`'s auto-discovery of a +# `.ephpm-analyze.yml` inside the checkout — so a malicious pull request +# cannot ship `enable: []` to neuter its own gate. +# * Every path in this file MUST be ABSOLUTE. A relative path here would +# resolve against the tenant's docroot, not the node. + +profile: none # explicit enable only — never inherit a bundled profile +fail_on: quarantine # quarantine (exit 2) and deny (exit 3) both block the preview + +analyzers: + # The six NATIVE analyzers: a fast file-walk (~2.6s on a 2000-file + # WordPress-scale tree, cold). This is the default set — snappy previews. + # + # `opcode-scan` is deliberately NOT here. It compiles every PHP file through + # the embedded Zend engine (~+5–15s on a full WordPress tree), which turns + # *suspected* sinks into *confirmed* ones at a real latency cost. A node that + # wants that can OPT IN by adding `opcode-scan` to both `enable` and + # `required` — see the README. + enable: [writable-exec, obfuscation-scan, secrets-scan, composer-scripts, dangerous-sinks, wp-vuln] + + # An analyzer that is `enable`d but fails to run is only fatal if it is also + # `required`. `wp-vuln` is intentionally left OUT of `required`: its feed is + # optional (see `wp_vuln_feed`), and a missing feed must skip, not gate. + required: [writable-exec, obfuscation-scan, secrets-scan, composer-scripts, dangerous-sinks] + + # Findings on these rules deny outright regardless of score — the + # unambiguous "do not serve this" set. + deny_hard: + - writable-exec + - composer-scripts/pipe-to-shell + - secrets-scan/aws-access-key-id + - secrets-scan/private-key + - secrets-scan/github-token + + # Optional WordPress vulnerability feed (absolute path). If the file is absent + # the wp-vuln analyzer skips — which is why wp-vuln is not `required`. + wp_vuln_feed: /var/lib/ephpm-web/wordfence-feed.json + +policy: + quarantine_score: 10 + deny_score: 50 diff --git a/src/analyze.rs b/src/analyze.rs new file mode 100644 index 0000000..b645da9 --- /dev/null +++ b/src/analyze.rs @@ -0,0 +1,1075 @@ +//! The pre-serve static-analysis gate. +//! +//! A preview builds and serves **untrusted code from a pull request**. Before a +//! preview's checkout is swapped into `sites_dir` and made routable — and before +//! `build:`/`seed:` execute any of it — switchboard can run +//! +//! ```text +//! ephpm analyze --config --format sarif +//! ``` +//! +//! over the materialized tree and refuse to publish on a bad verdict. +//! +//! # The gate is off until an operator turns it on +//! +//! With no `--analyze-config` the gate is **disabled** and a deploy behaves +//! exactly as it did before this module existed. That is the safe rollout +//! default: the analyzers this depends on ship in a recent `ephpm`, and a node +//! running an older binary must not have every preview blocked by a gate it +//! cannot satisfy. Turning the gate on is a deliberate, per-fleet decision. +//! +//! # The operator config is explicit, never the PR's own +//! +//! `--config ` is **security-critical** and always passed. `ephpm analyze` +//! otherwise auto-discovers a `.ephpm-analyze.yml` inside the tree it scans — and +//! the tree is the pull request, which could ship `enable: []` to neuter its own +//! gate. The explicit `--config` overrides that discovery, so the policy is the +//! operator's file (which lives outside any tenant docroot) and nothing the PR +//! can influence. For the same reason the analyzer is never run with the checkout +//! as its working directory. +//! +//! # Every failure blocks (fail closed) +//! +//! A gate exists to stop bad code from being served, so anything short of a clean +//! pass is a block: a quarantine/deny verdict, an analyzer that errored, a run +//! that timed out, and a binary that could not be spawned all refuse the publish. +//! A gate that cannot run must not wave code through. The decision is a pure +//! function of the run outcome ([`decide`]) so it is exhaustively unit-tested +//! without a real `ephpm`. + +use std::path::Path; +use std::process::Stdio; +use std::time::Duration; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use sha2::{Digest as _, Sha256}; +use tokio::process::Command; + +use crate::kv::VerdictCache; + +/// How many findings switchboard keeps from a blocked run's SARIF. The PR +/// comment renders a smaller top-N slice of these; the cap only bounds memory on +/// a pathological run that reports thousands. +const MAX_STORED_FINDINGS: usize = 50; + +/// The verdict `ephpm analyze` communicates through its process exit code. +/// +/// The mapping is ePHPm's, documented on `ephpm analyze`: `0` passed, `2` +/// quarantine, `3` deny, `1` an internal analyzer error. Anything else is +/// unrecognised and — like every non-zero, non-pass outcome — treated as a +/// block. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AnalyzeVerdict { + /// Exit `0` — the tree passed the configured gate. + Pass, + /// Exit `2` — findings crossed the quarantine threshold. + Quarantine, + /// Exit `3` — findings crossed the deny threshold. + Deny, + /// Exit `1` — the analyzer itself failed (bad config, crash, …). Fail closed. + AnalyzerError, + /// Any other exit code. Unrecognised, so fail closed. + Unrecognised(i32), +} + +impl AnalyzeVerdict { + /// Classify an `ephpm analyze` process exit code. + #[must_use] + pub fn from_exit_code(code: i32) -> Self { + match code { + 0 => Self::Pass, + 1 => Self::AnalyzerError, + 2 => Self::Quarantine, + 3 => Self::Deny, + other => Self::Unrecognised(other), + } + } + + /// Whether this verdict refuses the publish. Everything but [`Self::Pass`] + /// blocks. + #[must_use] + pub fn blocks(self) -> bool { + !matches!(self, Self::Pass) + } + + /// A short machine label for the PR comment and deployment status + /// (`deny` / `quarantine` / `error`). + #[must_use] + pub fn label(self) -> &'static str { + match self { + Self::Pass => "pass", + Self::Quarantine => "quarantine", + Self::Deny => "deny", + Self::AnalyzerError | Self::Unrecognised(_) => "error", + } + } +} + +/// What happened when switchboard tried to run `ephpm analyze`. +/// +/// This is the *entire* input to [`decide`], which is why the gate decision is a +/// pure function: every real-world outcome (a clean exit, a timeout, a binary +/// that would not spawn) reduces to one of these. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AnalyzeOutcome { + /// The process ran to completion with this exit code. + Exited(i32), + /// The wall-clock timeout elapsed before the process finished. Fail closed. + TimedOut, + /// The process could not be spawned or waited on (missing binary, OS + /// error). Fail closed — the reason is carried for the operator. + Unrunnable(String), +} + +/// The gate's decision for one deploy. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GateDecision { + /// Publish the preview. + Proceed, + /// Refuse to publish. `verdict` is a short machine label + /// (`deny`/`quarantine`/`error`/`timeout`) and `reason` is one operator- and + /// reviewer-facing sentence. + Block { + /// Short machine label for logs, the PR comment, and the deployment + /// status. + verdict: String, + /// One-sentence human explanation, logged verbatim and shown on the PR. + reason: String, + }, +} + +/// Map a run outcome to a gate decision. **Pure** — this is the whole policy, so +/// it is tested exhaustively without spawning anything. +/// +/// * exit `0` → proceed; +/// * exit `2` (quarantine) / `3` (deny) → block, named by verdict; +/// * exit `1` (analyzer error) or any other code → block (fail closed); +/// * timeout → block (fail closed); +/// * could-not-run → block (fail closed). +#[must_use] +pub fn decide(outcome: &AnalyzeOutcome) -> GateDecision { + match outcome { + AnalyzeOutcome::Exited(code) => { + let verdict = AnalyzeVerdict::from_exit_code(*code); + if !verdict.blocks() { + return GateDecision::Proceed; + } + let reason = match verdict { + AnalyzeVerdict::Quarantine => { + "ephpm analyze reached the quarantine threshold (exit 2)".to_string() + } + AnalyzeVerdict::Deny => { + "ephpm analyze reached the deny threshold (exit 3)".to_string() + } + AnalyzeVerdict::AnalyzerError => { + "ephpm analyze reported an internal error (exit 1) — a gate that \ + cannot run must not wave code through, so the preview is blocked \ + (fail closed)" + .to_string() + } + AnalyzeVerdict::Unrecognised(other) => format!( + "ephpm analyze exited with an unrecognised code {other} — treated as \ + a block (fail closed)" + ), + AnalyzeVerdict::Pass => unreachable!("Pass does not block"), + }; + GateDecision::Block { + verdict: verdict.label().to_string(), + reason, + } + } + AnalyzeOutcome::TimedOut => GateDecision::Block { + verdict: "timeout".to_string(), + reason: "ephpm analyze exceeded its wall-clock timeout and was killed — a \ + gate that cannot finish must not wave code through, so the preview \ + is blocked (fail closed)" + .to_string(), + }, + AnalyzeOutcome::Unrunnable(err) => GateDecision::Block { + verdict: "error".to_string(), + reason: format!( + "ephpm analyze could not be run ({err}) — the preview is blocked (fail closed)" + ), + }, + } +} + +/// One finding lifted from the analyzer's SARIF output, reduced to what the PR +/// comment shows. `Serialize`/`Deserialize` so a verdict can be cached in the +/// cluster-shared KV and reconstructed byte-identically on a peer. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Finding { + /// The SARIF `ruleId` (e.g. `dangerous-sinks/eval`). + pub rule_id: String, + /// The file the finding is in, relative to the scanned tree. + pub file: String, + /// 1-based line, when the SARIF carried a region. + pub line: Option, + /// The finding's message text. + pub message: String, +} + +/// A blocked run's detail, threaded into the PR comment and the deployment +/// status. `Serialize`/`Deserialize` so the whole block (verdict + findings) can +/// be cached cluster-wide and a peer can render the identical block comment +/// without re-scanning. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AnalyzeBlock { + /// Short machine label (`deny`/`quarantine`/`error`/`timeout`). + pub verdict: String, + /// One-sentence human reason. + pub reason: String, + /// Findings parsed from SARIF, capped at [`MAX_STORED_FINDINGS`]. Empty when + /// the run produced no parseable SARIF (a timeout, a spawn failure, or a + /// verdict-only exit). + pub findings: Vec, + /// The true number of findings the analyzer reported, even if more than were + /// stored. + pub total_findings: usize, +} + +/// The gate's result for one deploy. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AnalyzeGateResult { + /// The gate is not configured — nothing ran, deploy proceeds as before. + Skipped, + /// The analyzer ran and the tree passed — proceed to publish. + Passed, + /// The publish is refused. Carries the verdict and findings for the PR + /// comment and the deployment status. + Blocked(AnalyzeBlock), +} + +/// A verdict serialized for the **cluster-shared** cache. +/// +/// The analyze verdict is a pure function of (repo, PR head SHA, gate config), so +/// it is identical on every node. switchboard runs on every node and each +/// materializes the same checkout on its own disk, so without coordination the +/// gate would re-scan the identical commit N times. This is the shared value: the +/// first node to scan publishes it, and its peers reuse it. `Skipped` is never +/// cached — a disabled gate shares nothing. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum CachedVerdict { + /// The tree passed — a peer proceeds to publish. + Passed, + /// The tree was blocked — a peer blocks and renders the same block comment + /// from this stored `AnalyzeBlock` (verdict + findings), no re-scan. + Blocked(AnalyzeBlock), +} + +impl CachedVerdict { + /// The cacheable form of a gate result, or `None` for [`AnalyzeGateResult::Skipped`] + /// (a disabled gate has nothing to share). + #[must_use] + pub fn from_result(result: &AnalyzeGateResult) -> Option { + match result { + AnalyzeGateResult::Passed => Some(Self::Passed), + AnalyzeGateResult::Blocked(block) => Some(Self::Blocked(block.clone())), + AnalyzeGateResult::Skipped => None, + } + } + + /// Reconstruct the gate result a peer acts on — the same block/proceed + /// decision and, for a block, the same findings the comment renders. + #[must_use] + pub fn into_result(self) -> AnalyzeGateResult { + match self { + Self::Passed => AnalyzeGateResult::Passed, + Self::Blocked(block) => AnalyzeGateResult::Blocked(block), + } + } + + /// Serialize for storage in the shared KV. + /// + /// # Errors + /// + /// Returns an error if serialization fails (it does not, for these types). + pub fn to_json(&self) -> anyhow::Result { + Ok(serde_json::to_string(self)?) + } + + /// Parse a stored value. + /// + /// # Errors + /// + /// Returns an error if the stored bytes are not a `CachedVerdict` — the + /// caller treats that as a miss and re-scans (fail-safe). + pub fn from_json(s: &str) -> anyhow::Result { + Ok(serde_json::from_str(s)?) + } +} + +/// The identity a verdict is keyed by — everything the verdict depends on except +/// the gate config (which enters the key as its fingerprint). +#[derive(Debug, Clone, Copy)] +pub struct VerdictIdentity<'a> { + /// `owner/name` of the base repository. + pub repo: &'a str, + /// Pull request number. + pub pr: u64, + /// PR head commit SHA — the input that changes on every push, so a new push + /// naturally gets a fresh key and re-scans. + pub head_sha: &'a str, +} + +/// What a shared-cache lookup produced. The [`plan_from_lookup`] policy turns +/// this into a proceed-to-reuse or fall-back-to-scan decision. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CacheLookup { + /// The key was present with this stored value. + Hit(String), + /// The key was absent. + Miss, + /// The store could not be read (unreachable, auth rejected, …). Fail-safe: + /// treated exactly like a miss, so a coordination failure never skips the + /// gate — the node scans itself. + Unavailable, +} + +/// The action to take after a shared-cache lookup. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CachePlan { + /// Reuse this peer verdict — do not run the analyzer. + Reuse(AnalyzeGateResult), + /// Run the analyzer locally (then publish the result). + Scan, +} + +/// Decide what to do with a shared-cache lookup. **Pure** — the whole dedup +/// policy, tested without a real KV: +/// +/// * a parseable **hit** → reuse the peer verdict (no local scan); +/// * an **unparseable** hit → scan locally (fail-safe: never trust a value we +/// cannot read as a verdict); +/// * a **miss** or an **unavailable** store → scan locally. +/// +/// Note it never blocks on a coordination failure: `Unavailable` maps to `Scan`, +/// the same as `Miss`, so the gate degrades to per-node scanning — never to +/// "serve unscanned". +#[must_use] +pub fn plan_from_lookup(lookup: CacheLookup) -> CachePlan { + match lookup { + CacheLookup::Hit(value) => match CachedVerdict::from_json(&value) { + Ok(cached) => CachePlan::Reuse(cached.into_result()), + Err(_) => CachePlan::Scan, + }, + CacheLookup::Miss | CacheLookup::Unavailable => CachePlan::Scan, + } +} + +/// The shared-cache key a verdict is stored under: +/// `analyze:verdict::::`. +/// +/// `cfg_hash` is [`config_fingerprint`] of the operator gate config, so editing +/// the policy invalidates every cached verdict (the next scan re-runs under the +/// new rules). +#[must_use] +pub fn verdict_key(identity: &VerdictIdentity<'_>, cfg_hash: &str) -> String { + format!( + "analyze:verdict:{}:{}:{}:{cfg_hash}", + identity.repo, identity.pr, identity.head_sha + ) +} + +/// A short, stable fingerprint of the operator gate config's **contents**, so a +/// policy edit changes the cache key and forces a re-scan cluster-wide. +#[must_use] +pub fn config_fingerprint(contents: &[u8]) -> String { + let digest = Sha256::digest(contents); + // 16 hex chars (8 bytes) is ample to separate policy revisions; the key is + // already scoped by repo/PR/SHA. + hex::encode(&digest[..8]) +} + +/// [`config_fingerprint`] of a file's contents, or `None` if it cannot be read. +/// +/// A node that cannot read the policy must not govern its peers, so `None` +/// disables the shared cache for this deploy (the node scans locally and does not +/// publish) rather than fingerprinting an error. +fn config_fingerprint_of_file(path: &Path) -> Option { + std::fs::read(path) + .ok() + .map(|bytes| config_fingerprint(&bytes)) +} + +/// Run the analyze gate with **cluster-wide verdict deduplication**. +/// +/// The verdict is identical on every node, so the first node to scan a given +/// (repo, PR head SHA, gate config) publishes it to the shared KV and its peers +/// reuse it instead of re-scanning the same commit. This wraps [`run_gate`]; the +/// scan itself, the fail-closed exit-code policy, and the SARIF parsing are +/// unchanged. +/// +/// Layering of failure modes (this ordering is the contract): +/// * the **gate** fails closed — a bad verdict / analyzer error / timeout blocks; +/// * the **dedup** fails safe — a KV read error scans locally, a KV write error +/// proceeds with the local verdict. Coordination failure degrades to today's +/// "each node scans itself", never to "serve unscanned". +/// +/// `cache` is `None` when there is no cluster-shared KV switchboard can reach +/// (`--kv-secret-file` unset); the gate then simply scans on every node. When +/// `analyze_config` is `None` the gate is disabled and this returns +/// [`AnalyzeGateResult::Skipped`] without touching the KV or the binary. +/// +/// There is deliberately **no lock/lease/leader wait**: if two nodes miss at once +/// and both scan, the result is identical, so the only cost is a redundant scan — +/// the same looseness the PR-comment dedup already accepts. +pub async fn run_gate_cached( + ephpm_bin: &Path, + analyze_config: Option<&Path>, + timeout: Duration, + target: &Path, + hostname: &str, + cache: Option<&VerdictCache>, + identity: &VerdictIdentity<'_>, +) -> AnalyzeGateResult { + let Some(config) = analyze_config else { + // Gate disabled — no KV interaction at all. + tracing::debug!( + %hostname, + "pre-serve analyze gate is disabled (--analyze-config unset) — publishing \ + without static-analysis screening" + ); + return AnalyzeGateResult::Skipped; + }; + + // The key depends on the policy's contents. A node that cannot read the + // policy scans locally without the shared cache rather than fingerprinting an + // error and governing its peers with it. + let (key, cache) = match config_fingerprint_of_file(config) { + Some(cfg_hash) => (Some(verdict_key(identity, &cfg_hash)), cache), + None => { + tracing::warn!( + %hostname, + config = %config.display(), + "could not read the analyze config to fingerprint it — scanning locally \ + without the cluster-shared verdict cache" + ); + (None, None) + } + }; + + // Consult the shared cache (fail-safe: any read problem is a local scan). + if let (Some(cache), Some(key)) = (cache, key.as_deref()) { + let lookup = match cache.get(key).await { + Ok(Some(value)) => CacheLookup::Hit(value), + Ok(None) => CacheLookup::Miss, + Err(e) => { + tracing::warn!( + %hostname, + %e, + "cluster-shared verdict cache read failed — scanning locally (fail-safe)" + ); + CacheLookup::Unavailable + } + }; + if let CachePlan::Reuse(result) = plan_from_lookup(lookup) { + let verdict = match &result { + AnalyzeGateResult::Blocked(block) => block.verdict.as_str(), + _ => "pass", + }; + tracing::info!( + %hostname, + %verdict, + "reused a peer's analyze verdict from the cluster-shared cache — not \ + re-scanning this commit" + ); + return result; + } + } + + // Miss / unparseable / unavailable / no cache → scan locally. + let result = run_gate(ephpm_bin, Some(config), timeout, target, hostname).await; + + // Publish for peers (best-effort; a write failure never blocks the deploy). + if let (Some(cache), Some(key)) = (cache, key.as_deref()) { + if let Some(cached) = CachedVerdict::from_result(&result) { + match cached.to_json() { + Ok(json) => match cache.put(key, &json).await { + Ok(()) => tracing::debug!( + %hostname, + "published analyze verdict to the cluster-shared cache for peers" + ), + Err(e) => tracing::warn!( + %hostname, + %e, + "failed to publish the analyze verdict to the shared cache — peers \ + will scan this commit themselves (proceeding with the local verdict)" + ), + }, + Err(e) => tracing::warn!( + %hostname, + %e, + "failed to serialize the analyze verdict for the shared cache" + ), + } + } + } + + result +} + +/// Run the pre-serve analyze gate over a materialized preview tree. +/// +/// Returns [`AnalyzeGateResult::Skipped`] immediately when `analyze_config` is +/// `None` (the gate is disabled) — it does not even look at `ephpm_bin`, so a +/// node with the gate off never depends on the binary supporting `analyze`. +/// +/// Otherwise it runs `ephpm_bin analyze --config +/// --format sarif` with a wall-clock `timeout`, captures stdout (SARIF), and maps +/// the outcome through [`decide`]. Every non-pass outcome — a bad verdict, an +/// analyzer error, a timeout, a binary that will not spawn — is a +/// [`AnalyzeGateResult::Blocked`]; this function never returns an error, so the +/// fail-closed contract cannot be bypassed by a `?` in the caller. +/// +/// `target` is the tree to scan; `hostname` is only for logging. +pub async fn run_gate( + ephpm_bin: &Path, + analyze_config: Option<&Path>, + timeout: Duration, + target: &Path, + hostname: &str, +) -> AnalyzeGateResult { + let Some(config) = analyze_config else { + // Say it once at startup, not per deploy — main.rs already warns that the + // gate is disabled. Here it is only worth a debug line. + tracing::debug!( + %hostname, + "pre-serve analyze gate is disabled (--analyze-config unset) — publishing \ + without static-analysis screening" + ); + return AnalyzeGateResult::Skipped; + }; + + tracing::info!( + %hostname, + target = %target.display(), + config = %config.display(), + timeout_secs = timeout.as_secs(), + "running pre-serve analyze gate" + ); + + // Keep the raw stdout so we can parse findings for a block; the decision is + // made from the process outcome alone. + let (analyze_outcome, sarif) = run_analyze(ephpm_bin, config, timeout, target).await; + let decision = decide(&analyze_outcome); + + match decision { + GateDecision::Proceed => { + tracing::info!(%hostname, "analyze gate passed"); + AnalyzeGateResult::Passed + } + GateDecision::Block { verdict, reason } => { + let (findings, total_findings) = sarif + .as_deref() + .map_or((Vec::new(), 0), parse_sarif_findings); + tracing::warn!( + %hostname, + %verdict, + total_findings, + %reason, + "analyze gate BLOCKED the preview — it will not be published" + ); + AnalyzeGateResult::Blocked(AnalyzeBlock { + verdict, + reason, + findings, + total_findings, + }) + } + } +} + +/// Spawn `ephpm analyze`, enforce the timeout, and return the outcome alongside +/// captured stdout (the SARIF document, when the process produced one). +async fn run_analyze( + ephpm_bin: &Path, + config: &Path, + timeout: Duration, + target: &Path, +) -> (AnalyzeOutcome, Option) { + // SECURITY: `--config ` is explicit and overrides `ephpm + // analyze`'s auto-discovery of a `.ephpm-analyze.yml` inside `target` (the + // untrusted PR tree). The current directory is deliberately left as + // switchboard's own, never `target`, so no cwd-relative discovery is + // reintroduced. + let mut cmd = Command::new(ephpm_bin); + cmd.arg("analyze") + .arg(target) + .arg("--config") + .arg(config) + .arg("--format") + .arg("sarif") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // On timeout the `wait_with_output` future (which owns the child) is + // dropped; kill-on-drop then reaps the analyzer rather than leaving it + // running against the checkout after we have already blocked. + .kill_on_drop(true); + + let child = match cmd.spawn() { + Ok(child) => child, + Err(e) => { + return ( + AnalyzeOutcome::Unrunnable(format!("failed to spawn {}: {e}", ephpm_bin.display())), + None, + ); + } + }; + + let waited = tokio::time::timeout(timeout, child.wait_with_output()).await; + match waited { + // Ran to completion. + Ok(Ok(output)) => { + let code = output.status.code().unwrap_or_else(|| { + // No exit code means the process was killed by a signal; that is + // not a clean pass, so map it to a non-zero code that blocks. + tracing::warn!("ephpm analyze terminated without an exit code (signal?)"); + -1 + }); + if !output.stderr.is_empty() { + tracing::debug!( + stderr = %String::from_utf8_lossy(&output.stderr).trim(), + "ephpm analyze stderr" + ); + } + let sarif = String::from_utf8_lossy(&output.stdout).into_owned(); + let sarif = (!sarif.trim().is_empty()).then_some(sarif); + (AnalyzeOutcome::Exited(code), sarif) + } + // Waiting on the process itself failed. + Ok(Err(e)) => ( + AnalyzeOutcome::Unrunnable(format!("failed to wait on ephpm analyze: {e}")), + None, + ), + // The timeout elapsed. The child was moved into `wait_with_output`; + // dropping that future here drops the child, and `kill_on_drop(true)` + // above reaps it. + Err(_elapsed) => (AnalyzeOutcome::TimedOut, None), + } +} + +/// Parse SARIF 2.1.0 into a flat finding list plus the true total. +/// +/// Deliberately lenient (`serde_json::Value`, not a typed schema): a findings +/// list for a PR comment must never be the thing that turns a clean block into a +/// hard error, and SARIF has many optional shapes. Anything missing is skipped; +/// a document that does not parse yields no findings and a zero total, and the +/// block still stands on its exit code. Returns `(findings, total)` where +/// `findings` is capped at [`MAX_STORED_FINDINGS`] and `total` is the full count. +#[must_use] +pub fn parse_sarif_findings(sarif: &str) -> (Vec, usize) { + let Ok(root) = serde_json::from_str::(sarif) else { + return (Vec::new(), 0); + }; + let mut findings = Vec::new(); + let mut total = 0usize; + + let runs = root.get("runs").and_then(Value::as_array); + for run in runs.into_iter().flatten() { + let Some(results) = run.get("results").and_then(Value::as_array) else { + continue; + }; + for result in results { + total += 1; + if findings.len() >= MAX_STORED_FINDINGS { + continue; + } + findings.push(finding_from_result(result)); + } + } + (findings, total) +} + +/// Lift one SARIF `result` object into a [`Finding`], filling sensible +/// placeholders for any absent field. +fn finding_from_result(result: &Value) -> Finding { + let rule_id = result + .get("ruleId") + .and_then(Value::as_str) + .unwrap_or("(unknown rule)") + .to_string(); + + let message = result + .get("message") + .and_then(|m| m.get("text")) + .and_then(Value::as_str) + .unwrap_or("") + .trim() + .to_string(); + + // First physical location, if any. + let physical = result + .get("locations") + .and_then(Value::as_array) + .and_then(|locs| locs.first()) + .and_then(|loc| loc.get("physicalLocation")); + + let file = physical + .and_then(|p| p.get("artifactLocation")) + .and_then(|a| a.get("uri")) + .and_then(Value::as_str) + .unwrap_or("(unknown file)") + .to_string(); + + let line = physical + .and_then(|p| p.get("region")) + .and_then(|r| r.get("startLine")) + .and_then(Value::as_u64); + + Finding { + rule_id, + file, + line, + message, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── the pure decision (0→proceed, 1/2/3/other→block, timeout→block) ── + + #[test] + fn exit_zero_proceeds() { + assert_eq!(decide(&AnalyzeOutcome::Exited(0)), GateDecision::Proceed); + } + + #[test] + fn quarantine_exit_two_blocks() { + let d = decide(&AnalyzeOutcome::Exited(2)); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "quarantine"); + assert!(reason.contains("quarantine"), "{reason}"); + } + GateDecision::Proceed => panic!("exit 2 must block"), + } + } + + #[test] + fn deny_exit_three_blocks() { + let d = decide(&AnalyzeOutcome::Exited(3)); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "deny"); + assert!(reason.contains("deny"), "{reason}"); + } + GateDecision::Proceed => panic!("exit 3 must block"), + } + } + + /// Exit 1 is the analyzer itself failing — the gate could not render a + /// verdict, so the preview is blocked rather than waved through. + #[test] + fn analyzer_error_exit_one_blocks_fail_closed() { + let d = decide(&AnalyzeOutcome::Exited(1)); + assert!( + matches!(d, GateDecision::Block { .. }), + "an analyzer error must fail closed" + ); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "error"); + assert!(reason.contains("fail closed"), "{reason}"); + } + GateDecision::Proceed => unreachable!(), + } + } + + /// An exit code ePHPm never documents is still a block — the gate defaults + /// to refusing, not to trusting an outcome it does not understand. + #[test] + fn unrecognised_exit_code_blocks_fail_closed() { + let d = decide(&AnalyzeOutcome::Exited(42)); + assert!(matches!(d, GateDecision::Block { .. })); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "error"); + assert!(reason.contains("42"), "the code is named: {reason}"); + } + GateDecision::Proceed => unreachable!(), + } + } + + /// A timeout is fail-closed: a gate that cannot finish must not let the + /// preview through. + #[test] + fn timeout_blocks_fail_closed() { + let d = decide(&AnalyzeOutcome::TimedOut); + assert!(matches!(d, GateDecision::Block { .. })); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "timeout"); + assert!(reason.contains("fail closed"), "{reason}"); + } + GateDecision::Proceed => unreachable!(), + } + } + + /// A binary that will not spawn is fail-closed for the same reason. + #[test] + fn unrunnable_blocks_fail_closed() { + let d = decide(&AnalyzeOutcome::Unrunnable("no such file".to_string())); + assert!(matches!(d, GateDecision::Block { .. })); + match d { + GateDecision::Block { verdict, reason } => { + assert_eq!(verdict, "error"); + assert!(reason.contains("no such file"), "{reason}"); + } + GateDecision::Proceed => unreachable!(), + } + } + + #[test] + fn verdict_classification_matches_ephpm_exit_codes() { + assert_eq!(AnalyzeVerdict::from_exit_code(0), AnalyzeVerdict::Pass); + assert_eq!( + AnalyzeVerdict::from_exit_code(1), + AnalyzeVerdict::AnalyzerError + ); + assert_eq!( + AnalyzeVerdict::from_exit_code(2), + AnalyzeVerdict::Quarantine + ); + assert_eq!(AnalyzeVerdict::from_exit_code(3), AnalyzeVerdict::Deny); + assert_eq!( + AnalyzeVerdict::from_exit_code(9), + AnalyzeVerdict::Unrecognised(9) + ); + assert!(!AnalyzeVerdict::Pass.blocks()); + assert!(AnalyzeVerdict::Quarantine.blocks()); + assert!(AnalyzeVerdict::Deny.blocks()); + assert!(AnalyzeVerdict::AnalyzerError.blocks()); + } + + // ── the disabled gate skips without touching the binary ───────────── + + /// `analyze_config: None` disables the gate. It must not even attempt to run + /// the binary — proven here by passing a path that does not exist and still + /// getting `Skipped`, never a `Blocked` for a spawn failure. + #[tokio::test] + async fn none_config_skips_without_running_the_binary() { + let result = run_gate( + Path::new("/nonexistent/ephpm-binary-that-cannot-spawn"), + None, + Duration::from_secs(1), + Path::new("/tmp"), + "pr-1.app.preview.ephpm.dev", + ) + .await; + assert_eq!(result, AnalyzeGateResult::Skipped); + } + + // ── SARIF parsing ─────────────────────────────────────────────────── + + #[test] + fn parses_findings_from_sarif() { + let sarif = r#"{ + "version": "2.1.0", + "runs": [{ + "results": [ + { + "ruleId": "dangerous-sinks/eval", + "message": { "text": "use of eval() on request data" }, + "locations": [{ + "physicalLocation": { + "artifactLocation": { "uri": "wp-content/themes/x/functions.php" }, + "region": { "startLine": 42 } + } + }] + }, + { + "ruleId": "secrets-scan/aws-access-key-id", + "message": { "text": "AWS access key id committed" }, + "locations": [{ + "physicalLocation": { + "artifactLocation": { "uri": ".env.example" }, + "region": { "startLine": 3 } + } + }] + } + ] + }] + }"#; + let (findings, total) = parse_sarif_findings(sarif); + assert_eq!(total, 2); + assert_eq!(findings.len(), 2); + assert_eq!(findings[0].rule_id, "dangerous-sinks/eval"); + assert_eq!(findings[0].file, "wp-content/themes/x/functions.php"); + assert_eq!(findings[0].line, Some(42)); + assert!(findings[0].message.contains("eval()")); + assert_eq!(findings[1].rule_id, "secrets-scan/aws-access-key-id"); + } + + #[test] + fn tolerates_missing_optional_fields() { + // A result with no message, no locations, no region: every absent field + // gets a placeholder rather than dropping the finding. + let sarif = r#"{"runs":[{"results":[{"ruleId":"writable-exec"}]}]}"#; + let (findings, total) = parse_sarif_findings(sarif); + assert_eq!(total, 1); + assert_eq!(findings[0].rule_id, "writable-exec"); + assert_eq!(findings[0].file, "(unknown file)"); + assert_eq!(findings[0].line, None); + assert_eq!(findings[0].message, ""); + } + + #[test] + fn unparseable_or_empty_sarif_yields_no_findings() { + assert_eq!(parse_sarif_findings("not json at all"), (Vec::new(), 0)); + assert_eq!(parse_sarif_findings(""), (Vec::new(), 0)); + // Well-formed JSON with no runs is simply zero findings, not an error. + assert_eq!(parse_sarif_findings("{}"), (Vec::new(), 0)); + } + + // ── cluster-shared verdict dedup (pure logic) ────────────────────── + + fn sample_block() -> AnalyzeBlock { + AnalyzeBlock { + verdict: "deny".into(), + reason: "reached the deny threshold".into(), + findings: vec![Finding { + rule_id: "dangerous-sinks/eval".into(), + file: "index.php".into(), + line: Some(7), + message: "eval on request data".into(), + }], + total_findings: 1, + } + } + + /// A block verdict round-trips through JSON with its findings intact — so a + /// peer reconstructs the identical block comment without re-scanning. + #[test] + fn cached_verdict_block_round_trips() { + let cached = CachedVerdict::Blocked(sample_block()); + let json = cached.to_json().unwrap(); + let back = CachedVerdict::from_json(&json).unwrap(); + assert_eq!(back, cached); + // And it reconstructs the acting result verbatim. + match back.into_result() { + AnalyzeGateResult::Blocked(b) => { + assert_eq!(b.verdict, "deny"); + assert_eq!(b.findings.len(), 1); + assert_eq!(b.findings[0].rule_id, "dangerous-sinks/eval"); + assert_eq!(b.total_findings, 1); + } + other => panic!("expected Blocked, got {other:?}"), + } + } + + #[test] + fn cached_verdict_pass_round_trips() { + let json = CachedVerdict::Passed.to_json().unwrap(); + assert_eq!( + CachedVerdict::from_json(&json).unwrap().into_result(), + AnalyzeGateResult::Passed + ); + } + + /// A disabled gate (`Skipped`) is never cached — there is nothing to share. + #[test] + fn skipped_is_not_cacheable() { + assert_eq!( + CachedVerdict::from_result(&AnalyzeGateResult::Skipped), + None + ); + assert_eq!( + CachedVerdict::from_result(&AnalyzeGateResult::Passed), + Some(CachedVerdict::Passed) + ); + } + + /// **Cache hit** → reuse the peer verdict, no local scan. + #[test] + fn a_parseable_hit_is_reused() { + let json = CachedVerdict::Blocked(sample_block()).to_json().unwrap(); + match plan_from_lookup(CacheLookup::Hit(json)) { + CachePlan::Reuse(AnalyzeGateResult::Blocked(b)) => assert_eq!(b.verdict, "deny"), + other => panic!("a valid hit must be reused, got {other:?}"), + } + // A passing verdict is likewise reused. + let json = CachedVerdict::Passed.to_json().unwrap(); + assert_eq!( + plan_from_lookup(CacheLookup::Hit(json)), + CachePlan::Reuse(AnalyzeGateResult::Passed) + ); + } + + /// **Cache miss** → scan locally (and the caller then publishes). + #[test] + fn a_miss_scans() { + assert_eq!(plan_from_lookup(CacheLookup::Miss), CachePlan::Scan); + } + + /// **KV read error** (Unavailable) → scan locally. This is the fail-SAFE + /// direction: a coordination failure degrades to per-node scanning, never to + /// "serve unscanned". + #[test] + fn an_unavailable_store_scans_locally() { + assert_eq!(plan_from_lookup(CacheLookup::Unavailable), CachePlan::Scan); + } + + /// A stored value that is not a verdict is treated as a miss and re-scanned — + /// never trusted. + #[test] + fn an_unparseable_hit_scans() { + assert_eq!( + plan_from_lookup(CacheLookup::Hit("not json".into())), + CachePlan::Scan + ); + } + + #[test] + fn verdict_key_has_the_documented_shape() { + let id = VerdictIdentity { + repo: "ephpm/wordpress-sample", + pr: 7, + head_sha: "0123456789abcdef", + }; + assert_eq!( + verdict_key(&id, "cafebabecafebabe"), + "analyze:verdict:ephpm/wordpress-sample:7:0123456789abcdef:cafebabecafebabe" + ); + } + + /// The config fingerprint is deterministic and content-sensitive — editing + /// the policy changes the key, so every node re-scans under the new rules. + #[test] + fn config_fingerprint_is_deterministic_and_sensitive() { + let a = config_fingerprint(b"profile: none\nfail_on: quarantine\n"); + let b = config_fingerprint(b"profile: none\nfail_on: quarantine\n"); + let c = config_fingerprint(b"profile: none\nfail_on: deny\n"); + assert_eq!(a, b, "same contents → same fingerprint"); + assert_ne!(a, c, "a policy edit must change the fingerprint"); + assert_eq!(a.len(), 16, "16 hex chars"); + } + + /// The stored slice is capped, but the reported total is the true count — so + /// a comment can say "showing 10 of 137". + #[test] + fn stored_findings_are_capped_but_total_is_honest() { + let mut results = String::new(); + let n = MAX_STORED_FINDINGS + 20; + for i in 0..n { + if i > 0 { + results.push(','); + } + results.push_str(&format!( + r#"{{"ruleId":"r{i}","message":{{"text":"m"}},"locations":[]}}"# + )); + } + let sarif = format!(r#"{{"runs":[{{"results":[{results}]}}]}}"#); + let (findings, total) = parse_sarif_findings(&sarif); + assert_eq!(total, n, "the total must be the true count"); + assert_eq!( + findings.len(), + MAX_STORED_FINDINGS, + "the stored slice is bounded" + ); + } +} diff --git a/src/config.rs b/src/config.rs index eb0cb5e..87b6fb0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -252,9 +252,17 @@ pub struct Config { pub share_link_ttl_secs: u64, /// Path to a file holding ePHPm's `[kv] secret`, used to derive the per-site - /// RESP password so **teardown can bump the share-link revocation epoch** in a - /// preview's KV keyspace. Unset skips KV revocation (the override + checkout - /// removal already revoke on this node; a clustered leak self-heals at expiry). + /// RESP password for two cluster-shared KV uses: + /// + /// * **teardown bumps the share-link revocation epoch** in a preview's KV + /// keyspace; and + /// * the **analyze gate deduplicates its verdict** across nodes (the first + /// node to scan a commit publishes the verdict; peers reuse it). + /// + /// Unset disables both: teardown revocation falls back to removing the + /// override + checkout (a clustered leak self-heals at expiry), and the + /// analyze gate scans on every node independently (fail-safe to the pre-dedup + /// behavior). #[arg(long, env = "SWITCHBOARD_KV_SECRET_FILE")] pub kv_secret_file: Option, @@ -263,6 +271,56 @@ pub struct Config { #[arg(long, default_value = "127.0.0.1:6379", env = "SWITCHBOARD_KV_ADDR")] pub kv_addr: String, + // ── pre-serve static-analysis gate ───────────────────────────────── + /// Path to the operator-controlled `ephpm analyze` policy file (YAML), used + /// to screen a preview's checkout before it is published. + /// + /// **When unset the gate is disabled** and a deploy behaves exactly as it did + /// before this option existed — the safe rollout default (startup logs one + /// `WARN`). When set, every deploy runs + /// `ephpm analyze --config --format sarif` after the PR + /// checkout is materialized and **before** the vhost is swapped live, and a + /// bad verdict blocks the preview. + /// + /// The explicit `--config` is **security-critical**: it overrides `ephpm + /// analyze`'s auto-discovery of a `.ephpm-analyze.yml` inside the checkout, so + /// a malicious pull request cannot ship `enable: []` to neuter its own gate. + /// Point it at a file **outside** any tenant docroot that the tenant cannot + /// write, and (per that file's own contract) use absolute paths inside it — a + /// relative path there would resolve against the tenant tree. + /// + /// The analyzers this depends on + /// (`writable-exec`/`obfuscation-scan`/`secrets-scan`/`composer-scripts`/…) + /// ship in a recent `ephpm`; the gate stays off until this is set **and** the + /// node's `--ephpm-bin` supports `analyze`. + #[arg(long, env = "SWITCHBOARD_ANALYZE_CONFIG")] + pub analyze_config: Option, + + /// Wall-clock timeout (seconds) for a single `ephpm analyze` run. A run that + /// exceeds it is killed and the deploy is **blocked** (fail closed) — a gate + /// that cannot finish must not wave code through. Only consulted when + /// `--analyze-config` is set. + #[arg(long, default_value_t = 180, env = "SWITCHBOARD_ANALYZE_TIMEOUT_SECS")] + pub analyze_timeout_secs: u64, + + /// TTL (seconds) for a verdict published to the **cluster-shared** analyze + /// cache. The gate's verdict is a pure function of (repo, PR head SHA, gate + /// config), so it is deduplicated across nodes through ePHPm's + /// gossip-replicated KV — the first node to scan a commit publishes its + /// verdict and peers reuse it instead of re-scanning. + /// + /// The shared cache is active only when `--analyze-config` **and** + /// `--kv-secret-file` are both set; without the KV secret each node scans + /// independently (fail-safe to the pre-dedup behavior). The head SHA is the + /// real invalidator — this TTL just garbage-collects old entries. Default 1 + /// day. + #[arg( + long, + default_value_t = 86_400, + env = "SWITCHBOARD_ANALYZE_VERDICT_TTL_SECS" + )] + pub analyze_verdict_ttl_secs: u64, + // ── GitHub reporting (optional) ──────────────────────────────────── /// GitHub App private key path (PEM file). Omit to run without GitHub /// reporting — deploys still happen, they are just not reported on the PR. @@ -325,6 +383,27 @@ impl Config { Duration::from_secs(self.share_link_ttl_secs.max(1)) } + /// Whether the pre-serve analyze gate is enabled. It is off until an operator + /// points `--analyze-config` at a policy file — the safe rollout default. + #[must_use] + pub fn analyze_gate_enabled(&self) -> bool { + self.analyze_config.is_some() + } + + /// Wall-clock timeout for a single `ephpm analyze` run (floored at one + /// second so a `0` cannot make every run time out instantly). + #[must_use] + pub fn analyze_timeout(&self) -> Duration { + Duration::from_secs(self.analyze_timeout_secs.max(1)) + } + + /// TTL for a verdict published to the cluster-shared analyze cache (floored + /// at one second). + #[must_use] + pub fn analyze_verdict_ttl(&self) -> Duration { + Duration::from_secs(self.analyze_verdict_ttl_secs.max(1)) + } + /// Resolve ePHPm's `[kv] secret` from `--kv-secret-file`, for deriving the /// per-site RESP password used to bump the share-link revocation epoch. /// @@ -883,6 +962,63 @@ mod tests { ); } + // ── pre-serve analyze gate ────────────────────────────────────────── + + #[test] + fn analyze_gate_is_off_by_default() { + let c = parse_single_node(&[]); + assert!( + !c.analyze_gate_enabled(), + "the gate must be disabled unless an operator configures it" + ); + assert!(c.analyze_config.is_none()); + assert_eq!( + c.analyze_timeout_secs, 180, + "the documented default timeout is 180s" + ); + assert_eq!(c.analyze_timeout(), Duration::from_secs(180)); + assert_eq!( + c.analyze_verdict_ttl_secs, 86_400, + "the documented default verdict TTL is 1 day" + ); + assert_eq!(c.analyze_verdict_ttl(), Duration::from_secs(86_400)); + c.validate().unwrap(); + } + + #[test] + fn analyze_gate_flags_parse() { + let c = parse_single_node(&[ + "--analyze-config", + "/etc/switchboard/analyze-gate.yml", + "--analyze-timeout-secs", + "300", + "--analyze-verdict-ttl-secs", + "7200", + ]); + assert!(c.analyze_gate_enabled()); + assert_eq!( + c.analyze_config, + Some(PathBuf::from("/etc/switchboard/analyze-gate.yml")) + ); + assert_eq!(c.analyze_timeout(), Duration::from_secs(300)); + assert_eq!(c.analyze_verdict_ttl(), Duration::from_secs(7200)); + c.validate().unwrap(); + } + + /// A zero timeout/TTL must not become instant — both are floored at one + /// second. + #[test] + fn zero_analyze_durations_are_floored() { + let c = parse_single_node(&[ + "--analyze-timeout-secs", + "0", + "--analyze-verdict-ttl-secs", + "0", + ]); + assert_eq!(c.analyze_timeout(), Duration::from_secs(1)); + assert_eq!(c.analyze_verdict_ttl(), Duration::from_secs(1)); + } + #[test] fn missing_state_dir_is_an_error() { // The queue is the daemon's only input; there is nothing sensible to diff --git a/src/deployer.rs b/src/deployer.rs index 75125a1..7740523 100644 --- a/src/deployer.rs +++ b/src/deployer.rs @@ -41,6 +41,8 @@ use std::time::{Duration, Instant}; use anyhow::Context; use tokio::process::Command; +use crate::analyze::{self, AnalyzeBlock, AnalyzeGateResult}; +use crate::kv; use crate::manifest::AppManifest; use crate::preview_auth; use crate::secrets::Secrets; @@ -292,6 +294,27 @@ pub struct DeployContext<'a> { pub mint_share_link: bool, /// TTL for a minted share link. Kept short — expiry is the primary control. pub share_token_ttl: Duration, + + // ── pre-serve static-analysis gate ───────────────────────────────── + /// The operator-controlled `ephpm analyze` policy file, or `None` when the + /// gate is disabled. Passed to `ephpm analyze --config` explicitly so a + /// malicious PR's own `.ephpm-analyze.yml` cannot neuter the gate. See + /// [`crate::analyze`]. + pub analyze_config: Option<&'a Path>, + /// Wall-clock timeout for one `ephpm analyze` run. A run that exceeds it is + /// killed and the preview is blocked (fail closed). + pub analyze_timeout: Duration, + /// ePHPm's KV RESP listener address, for the **cluster-shared analyze verdict + /// cache** (the gate scans a commit once per cluster, not once per node). + pub kv_addr: &'a str, + /// ePHPm's `[kv] secret`, or `None` when `--kv-secret-file` is unset. `None` + /// disables the shared verdict cache — the gate then scans on every node + /// (fail-safe to today's behavior). Also used by teardown's share-link + /// revocation. + pub kv_secret: Option<&'a str>, + /// TTL applied to a published verdict in the shared cache. The head SHA is the + /// real invalidator; the TTL just garbage-collects old entries. + pub analyze_verdict_ttl: Duration, } /// Result of a successful deployment. @@ -314,13 +337,22 @@ pub struct DeployResult { /// `--share-link`). Carries only the bearer token in its query string; the /// signing secret never appears here. pub share_url: Option, + /// `Some` when the pre-serve analyze gate **blocked** this preview: the + /// preview was NOT published (the atomic swap never happened) and this + /// carries the verdict and top findings for the PR comment and deployment + /// status. `None` for a normally published preview. See [`crate::analyze`]. + pub analyze_block: Option, } /// Deploy a preview. /// /// Pipeline order: /// 1. Fetch the PR head (`refs/pull//head` from the base repo) at its SHA. -/// 2. Detect the framework and load the `ephpm.yaml` manifest (or synthesize). +/// 2. Detect the framework and load the `ephpm.yaml` manifest (or synthesize), +/// then run the pre-serve analyze gate over the pristine checkout and +/// **block** (return a `DeployResult` carrying an `analyze_block`, publishing +/// nothing) on a bad verdict. The gate is disabled unless `--analyze-config` +/// is set. See [`crate::analyze`]. /// 3. Materialize `env:` — resolve `${secret.NAME}` from switchboard's own /// secret store and write it where the app can read it (`.env` for /// build/seed shell steps, the PHP prepend for the app). @@ -449,6 +481,79 @@ pub async fn deploy_preview( "loaded app manifest" ); + // (2b) Pre-serve static-analysis gate (fail closed). Screen the PRISTINE + // checkout — the untrusted pull-request code — with `ephpm analyze` and + // refuse to publish on a bad verdict. + // + // Placed HERE, before everything below, on purpose: + // * before `materialize_env` (step 3) writes the resolved `.env`/prepend, + // so the analyzer never scans switchboard's OWN injected secrets (which + // `secrets-scan` would otherwise flag on every gated deploy); + // * before `build:`/`seed:` (steps 7–8) execute any of it, so a + // `composer-scripts` finding blocks the code before its scripts run; + // * before the atomic swap (step 6), which is the point the vhost becomes + // routable — so a block means the site is never served, and on a + // redeploy the previous (known-good) container stays live untouched. + // + // Because nothing external is provisioned yet (no override, no swap, no + // per-site DB), a block needs no teardown — it just removes the staging tree, + // the one artifact the ordinary deploy-failure path also leaves for the next + // deploy to clean. The gate is a no-op (`Skipped`) when `--analyze-config` + // is unset. + // + // The verdict is identical on every node (pure function of repo + head SHA + + // gate config), so it is deduplicated through ePHPm's cluster-shared, + // gossip-replicated KV: the first node to scan a commit publishes its verdict + // and peers reuse it. The verdict lives in a switchboard-private KV namespace + // no preview tenant can authenticate to (see `kv::VERDICT_STORE_SITE`), so a + // malicious preview cannot forge a `Passed` for a future commit. The dedup + // fails SAFE — a KV read error scans locally — while the gate itself fails + // CLOSED. No cluster-shared KV (no `--kv-secret-file`) means each node scans, + // exactly as before. + let verdict_cache = ctx + .kv_secret + .map(|secret| kv::VerdictCache::new(ctx.kv_addr, secret, ctx.analyze_verdict_ttl)); + let verdict_identity = analyze::VerdictIdentity { + repo: &req.repo_full_name, + pr: req.pr_number, + head_sha: &req.sha, + }; + match analyze::run_gate_cached( + ctx.ephpm_bin, + ctx.analyze_config, + ctx.analyze_timeout, + &tmp_dir, + &hostname, + verdict_cache.as_ref(), + &verdict_identity, + ) + .await + { + AnalyzeGateResult::Skipped | AnalyzeGateResult::Passed => {} + AnalyzeGateResult::Blocked(block) => { + // Roll back the only half-provisioned state (the staging checkout). + // The swap never happened, so `site_dir`, the override file and the + // per-site database were never touched. + tokio::fs::remove_dir_all(&tmp_dir).await.ok(); + tracing::warn!( + %hostname, + verdict = %block.verdict, + total_findings = block.total_findings, + "preview blocked by the analyze gate — not publishing" + ); + return Ok(DeployResult { + hostname, + framework, + duration: start.elapsed(), + php_version: Some(manifest.php), + healthy: false, + gated, + share_url: None, + analyze_block: Some(block), + }); + } + } + // (3) Materialize env: resolve secrets and write env for the app to read. // Reference the FINAL (post-swap) prepend path in the sidecar. This runs // BEFORE the swap (so `.env`/prepend travel with the tree into `site_dir`, @@ -640,6 +745,9 @@ pub async fn deploy_preview( healthy, gated, share_url, + // Reaching here means the gate passed or was disabled — a published + // preview, never a block. + analyze_block: None, }) } @@ -2127,6 +2235,11 @@ mod tests { preview_session_secret_ref: "env:EPHPM_PREVIEW_SESSION_SECRET", mint_share_link: false, share_token_ttl: Duration::from_secs(86_400), + analyze_config: None, + analyze_timeout: Duration::from_secs(180), + kv_addr: "127.0.0.1:6379", + kv_secret: None, + analyze_verdict_ttl: Duration::from_secs(86_400), }; assert!(!wait_healthy("https://example.invalid", "/", &ctx).await); } @@ -2156,6 +2269,11 @@ mod tests { preview_session_secret_ref: "env:EPHPM_PREVIEW_SESSION_SECRET", mint_share_link: false, share_token_ttl: Duration::from_secs(86_400), + analyze_config: None, + analyze_timeout: Duration::from_secs(180), + kv_addr: "127.0.0.1:6379", + kv_secret: None, + analyze_verdict_ttl: Duration::from_secs(86_400), } } diff --git a/src/github.rs b/src/github.rs index d087ed2..c40e2e9 100644 --- a/src/github.rs +++ b/src/github.rs @@ -127,7 +127,20 @@ impl GitHubClient { .as_u64() .context("deployment response missing id")?; - // Set deployment status to success. + // A preview the analyze gate blocked was never published, so its + // deployment status is a failure, not a success — otherwise the PR's + // Environments UI would claim a live preview that does not exist. + let (state, description) = match &result.analyze_block { + Some(block) => ( + "failure", + format!("preview blocked by the analyze gate ({})", block.verdict), + ), + None => ( + "success", + format!("{} preview deployed", result.framework.as_str()), + ), + }; + let status_url = format!( "https://api.github.com/repos/{owner}/{repo}/deployments/{deployment_id}/statuses" ); @@ -137,9 +150,9 @@ impl GitHubClient { .header(USER_AGENT, "switchboard") .header(ACCEPT, "application/vnd.github+json") .json(&json!({ - "state": "success", + "state": state, "environment_url": url, - "description": format!("{} preview deployed", result.framework.as_str()), + "description": description, })) .send() .await @@ -290,8 +303,19 @@ fn teardown_comment_body() -> String { ) } -/// Format the PR comment body for a successful deploy. +/// How many findings the blocked-preview comment lists before it truncates. +const MAX_COMMENT_FINDINGS: usize = 10; + +/// Format the sticky PR comment body for a deploy. +/// +/// A **blocked** deploy (the analyze gate refused it) renders a distinct body — +/// no "ready" URL, the verdict, and the top findings — so the reviewer sees why +/// the preview is not up. A published deploy renders the usual table. Both carry +/// the hidden [`COMMENT_MARKER`] so the one sticky comment is updated in place. fn format_deploy_comment(result: &DeployResult) -> String { + if let Some(block) = &result.analyze_block { + return format_block_comment(result, block); + } let url = crate::deployer::preview_url(&result.hostname, result.php_version.as_deref()); let php_display = result.php_version.as_deref().unwrap_or("latest"); let status = if result.healthy { @@ -317,6 +341,97 @@ fn format_deploy_comment(result: &DeployResult) -> String { body } +/// Format the sticky comment body for a preview the analyze gate **blocked**. +/// +/// It states plainly that the preview was not published, names the verdict and +/// finding count, and lists the top [`MAX_COMMENT_FINDINGS`] findings (rule, file, +/// line, message) so the author can act without opening the daemon logs. The +/// list is capped and says "showing N of M" when it truncates. Kept a pure +/// function of [`DeployResult`] + [`crate::analyze::AnalyzeBlock`] so the exact +/// markdown is unit-testable, and it carries the hidden [`COMMENT_MARKER`] so it +/// updates the same sticky comment a later (passing) push will overwrite. +fn format_block_comment(result: &DeployResult, block: &crate::analyze::AnalyzeBlock) -> String { + let mut body = format!( + "{COMMENT_MARKER}\n\ + **ePHPm Preview** — blocked by the analyze gate 🚫\n\n\ + This {} preview was **not published**. `ephpm analyze` returned **{}** \ + ({} finding(s)) on the pull request's code before it could be served.\n\n\ + > {}\n", + result.framework.as_str(), + block.verdict, + block.total_findings, + block.reason, + ); + + if block.findings.is_empty() { + body.push_str( + "\nNo per-finding detail was captured (the analyzer produced no parseable \ + report — e.g. a timeout or an internal error). See the switchboard logs on \ + the node for the full output.\n", + ); + } else { + let shown = block.findings.len().min(MAX_COMMENT_FINDINGS); + body.push_str("\n| Rule | Location | Message |\n|---|---|---|\n"); + for f in block.findings.iter().take(MAX_COMMENT_FINDINGS) { + // Every field here is attacker-controlled: `f.file` is a filename + // *inside the PR* (ePHPm's SARIF emits the artifact URI without + // percent-encoding, so newlines, pipes and backticks — all legal in a + // Linux/git filename — flow through verbatim), and `f.rule_id` / + // `f.message` originate from the same untrusted report. Rendered raw + // they could break the table row or close their code span and inject + // markdown (a heading/link spoof) into switchboard's trusted-identity + // sticky comment. `file` and `rule_id` sit inside code spans, so they + // go through `sanitize_code` (also neutralizes backticks); `message` + // is a plain cell. + let rule = sanitize_code(&f.rule_id); + let location = match f.line { + // The line is our own `u64`, never attacker text. + Some(line) => format!("`{}:{line}`", sanitize_code(&f.file)), + None => format!("`{}`", sanitize_code(&f.file)), + }; + body.push_str(&format!( + "| `{rule}` | {location} | {} |\n", + sanitize_cell(&f.message), + )); + } + if block.total_findings > shown { + body.push_str(&format!( + "\n_Showing {shown} of {} findings._\n", + block.total_findings + )); + } + } + + body.push_str( + "\nFix the findings and push again — the preview redeploys and this comment \ + refreshes automatically.", + ); + body +} + +/// Make an untrusted string safe for a single Markdown **table cell**: collapse +/// the newlines and carriage returns that would break the row, escape the pipe +/// that would open a new column, and neutralize the backtick so an odd number of +/// them cannot toggle a code span open across the rest of the comment. +fn sanitize_cell(s: &str) -> String { + s.replace(['\n', '\r'], " ") + .replace('|', "\\|") + .replace('`', "'") +} + +/// Make an untrusted string safe to interpolate **inside a backtick code span** +/// in a table cell (`` `` ``). On top of [`sanitize_cell`]'s row/column +/// protection it must ensure the value carries no backtick of its own — a single +/// one would close the span early and let everything after it render as active +/// markdown (heading/link spoofing under switchboard's trusted identity). The +/// backtick is replaced with an apostrophe so the rendered span stays visually +/// faithful. +fn sanitize_code(s: &str) -> String { + // Reuse the cell rules — they already replace the backtick, plus handle the + // newline/pipe that would break the row a code span sits in. + sanitize_cell(s) +} + /// The access-guidance block appended to a **gated** preview's comment. /// /// A gated preview is not world-readable, so a reviewer needs to be told how to @@ -365,6 +480,7 @@ mod tests { healthy: true, gated: false, share_url: None, + analyze_block: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains("https://pr-42.my-blog.preview.ephpm.dev")); @@ -388,6 +504,7 @@ mod tests { healthy: false, gated: false, share_url: None, + analyze_block: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains(":8084"), "PHP 8.4 should use port 8084"); @@ -408,6 +525,7 @@ mod tests { healthy: true, gated: false, share_url: None, + analyze_block: None, }; let comment = format_deploy_comment(&result); assert!( @@ -436,6 +554,7 @@ mod tests { healthy: true, gated: false, share_url: None, + analyze_block: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains("2.4s"), "got: {comment}"); @@ -455,6 +574,7 @@ mod tests { healthy: true, gated: true, share_url, + analyze_block: None, } } @@ -468,6 +588,7 @@ mod tests { healthy: true, gated: false, share_url: None, + analyze_block: None, }; let comment = format_deploy_comment(&result); assert!( @@ -509,6 +630,197 @@ mod tests { assert!(comment.contains(token), "the token travels in the URL"); } + // ── analyze gate: blocked-preview comment ────────────────────────── + + fn blocked_result(block: crate::analyze::AnalyzeBlock) -> DeployResult { + DeployResult { + hostname: "pr-9.app.preview.ephpm.dev".into(), + framework: Framework::WordPress, + duration: Duration::from_millis(1_200), + php_version: Some("8.4".into()), + healthy: false, + gated: false, + share_url: None, + analyze_block: Some(block), + } + } + + #[test] + fn blocked_comment_names_the_verdict_and_lists_findings() { + use crate::analyze::{AnalyzeBlock, Finding}; + let block = AnalyzeBlock { + verdict: "deny".into(), + reason: "ephpm analyze reached the deny threshold (exit 3)".into(), + findings: vec![ + Finding { + rule_id: "dangerous-sinks/eval".into(), + file: "wp-content/themes/x/functions.php".into(), + line: Some(42), + message: "eval() on request data".into(), + }, + Finding { + rule_id: "secrets-scan/aws-access-key-id".into(), + file: ".env.example".into(), + line: None, + message: "AWS access key committed".into(), + }, + ], + total_findings: 2, + }; + let comment = format_deploy_comment(&blocked_result(block)); + // Still a sticky switchboard comment. + assert!(comment.contains(COMMENT_MARKER)); + assert!(is_switchboard_comment(&comment)); + // Makes the block unmistakable and never advertises a live URL. + assert!(comment.contains("blocked by the analyze gate"), "{comment}"); + assert!( + comment.contains("**deny**"), + "the verdict is named: {comment}" + ); + assert!( + !comment.contains("ready"), + "a blocked preview must not read as ready: {comment}" + ); + // The findings are listed with rule, location and message. + assert!(comment.contains("dangerous-sinks/eval"), "{comment}"); + assert!( + comment.contains("wp-content/themes/x/functions.php:42"), + "{comment}" + ); + assert!(comment.contains("eval() on request data"), "{comment}"); + // A finding with no line renders just the file. + assert!( + comment.contains("secrets-scan/aws-access-key-id"), + "{comment}" + ); + } + + #[test] + fn blocked_comment_caps_the_findings_list_and_says_how_many() { + use crate::analyze::{AnalyzeBlock, Finding}; + let findings: Vec = (0..25) + .map(|i| Finding { + rule_id: format!("rule-{i}"), + file: format!("f{i}.php"), + line: Some(i + 1), + message: "m".into(), + }) + .collect(); + let block = AnalyzeBlock { + verdict: "quarantine".into(), + reason: "over threshold".into(), + findings, + total_findings: 137, + }; + let comment = format_deploy_comment(&blocked_result(block)); + // Only the first MAX_COMMENT_FINDINGS rows are rendered. + assert!(comment.contains("rule-0"), "{comment}"); + assert!(comment.contains("rule-9"), "{comment}"); + assert!( + !comment.contains("rule-10"), + "the list must cap at {MAX_COMMENT_FINDINGS}: {comment}" + ); + assert!(comment.contains("Showing 10 of 137"), "{comment}"); + } + + /// A block with no parseable findings (timeout / analyzer error) still posts + /// a clear comment — the verdict and a pointer to the node logs. + #[test] + fn blocked_comment_without_findings_still_explains() { + use crate::analyze::AnalyzeBlock; + let block = AnalyzeBlock { + verdict: "timeout".into(), + reason: "ephpm analyze exceeded its wall-clock timeout".into(), + findings: Vec::new(), + total_findings: 0, + }; + let comment = format_deploy_comment(&blocked_result(block)); + assert!(comment.contains("blocked by the analyze gate"), "{comment}"); + assert!(comment.contains("**timeout**"), "{comment}"); + assert!(comment.contains("No per-finding detail"), "{comment}"); + } + + /// A message with pipes/newlines/backticks must not break the row or toggle + /// a code span. + #[test] + fn finding_message_is_sanitized_for_a_table_cell() { + assert_eq!(sanitize_cell("a | b\nc"), "a \\| b c"); + // A backtick would otherwise open a code span spanning the rest of the + // comment; it is neutralized to an apostrophe. + assert_eq!(sanitize_cell("`code`"), "'code'"); + assert!(!sanitize_cell("a`b").contains('`')); + assert!(!sanitize_code("a`b").contains('`')); + } + + /// **Regression: comment rendering is injection-safe.** A pull request can + /// name a file with newlines, pipes, backticks and markdown (all legal in a + /// git/Linux filename, and ePHPm's SARIF passes the URI through unencoded). + /// The `file` and `rule_id` fields are rendered inside backtick code spans, so + /// a raw backtick would close the span and turn the attacker's markdown into + /// active markup inside switchboard's trusted-identity sticky comment. Assert + /// the rendered block: one table row per finding (no stray newline), no + /// unescaped backtick that could close a span, and no active injected markup. + #[test] + fn blocked_comment_neutralizes_a_malicious_filename() { + use crate::analyze::{AnalyzeBlock, Finding}; + // Assemble the hostile filename at runtime so no literal sequence trips + // tooling: a real newline, a pipe, a backtick, a spoof heading and link. + let evil_file = format!( + "x{nl}## Approved {link}{nl}.php", + nl = '\n', + link = "[merge](http://evil.example)" + ); + let evil_rule = format!("rule{bt}## pwned", bt = '`'); + let block = AnalyzeBlock { + verdict: "deny".into(), + reason: "reached the deny threshold".into(), + findings: vec![Finding { + rule_id: evil_rule, + file: evil_file, + line: Some(3), + message: "eval on request data".into(), + }], + total_findings: 1, + }; + let comment = format_deploy_comment(&blocked_result(block)); + + // (a) The findings table is exactly one data row: the header row, its + // `|---|` separator, and one finding row — the injected newline must not + // have split the finding across lines. + let finding_rows = comment + .lines() + .filter(|l| l.starts_with("| ") && !l.contains("---") && !l.contains("| Rule |")) + .count(); + assert_eq!( + finding_rows, 1, + "the malicious filename must not break the single finding row:\n{comment}" + ); + + // (b) Every backtick in the output is balanced into complete code spans — + // an attacker backtick can never leave a span hanging open. Since our + // template only ever emits backticks in matched pairs, an even count + // proves no injected one survived. + assert_eq!( + comment.matches('`').count() % 2, + 0, + "unbalanced backticks would leave a code span open:\n{comment}" + ); + + // (c) The injected markdown does not appear as active markup: the spoof + // link/heading text may appear as inert characters, but not on its own + // line as a real heading, and the code-span content is escaped. + assert!( + !comment.contains("\n## Approved"), + "a spoofed heading must not start its own line:\n{comment}" + ); + // The rule_id's backtick was neutralized, so `## pwned` cannot escape its + // code span. + assert!( + !comment.contains("`rule`## pwned"), + "the rule_id backtick must not close its span:\n{comment}" + ); + } + #[test] fn teardown_body_is_marked_and_removed() { let body = teardown_comment_body(); diff --git a/src/kv.rs b/src/kv.rs index 1e932f6..50c283d 100644 --- a/src/kv.rs +++ b/src/kv.rs @@ -1,5 +1,13 @@ -//! A tiny RESP2 client for the one thing switchboard needs from ePHPm's KV: to -//! write a preview's **share-link revocation** keys on teardown. +//! A tiny RESP2 client for the two things switchboard needs from ePHPm's KV: +//! writing a preview's **share-link revocation** keys on teardown +//! ([`KvRevoker`]), and reading/writing the **cluster-shared analyze verdict +//! cache** ([`VerdictCache`]) so the pre-serve analyze gate scans a commit once +//! per cluster instead of once per node. +//! +//! Both rely on the same property: ePHPm's per-site KV is **gossip-replicated** +//! across the cluster (see below), so a value one node writes is readable on its +//! peers. That is exactly what lets the verdict computed by the first node to +//! scan be reused by the others. //! //! # Why a client at all, and why this narrow //! @@ -138,6 +146,165 @@ impl KvRevoker { } } +/// The **switchboard-private** KV namespace the analyze verdict cache lives in. +/// +/// This is an AUTH *site* string, deliberately chosen so no preview tenant can +/// ever reach it. Two facts make it disjoint from every preview keyspace: +/// +/// * A tenant's `ephpm_kv_*` calls are auto-scoped **server-side** by ePHPm to +/// the request's own resolved site key — a tenant never gets to choose an AUTH +/// site, it can only ever read/write its one keyspace. +/// * A resolvable site key is always a *valid* one: DNS-style labels drawn from +/// `[a-z0-9._-]` (ePHPm's `is_valid_site_key`, mirrored in +/// [`crate::site_key::is_valid_site_key`]). This namespace begins with the +/// gossip unit-separator `\x1f`, which is outside that charset, so it can never +/// equal any preview site key — and therefore no tenant is ever scoped to it. +/// +/// switchboard holds `kv_secret`, so it can derive the password for *any* AUTH +/// site and reach this one. That asymmetry — switchboard can address it, no +/// tenant can — is what prevents a malicious preview from poisoning the cache +/// (writing a forged `Passed` for a future SHA it authors). All nodes' daemons +/// AUTH as this same reserved site, so they share one gossip-replicated verdict +/// keyspace; the per-commit key ([`crate::analyze::verdict_key`]) keeps distinct +/// previews from colliding within it. +pub const VERDICT_STORE_SITE: &str = "\x1fswitchboard-verdicts"; + +/// A client for the cluster-shared **analyze verdict cache** in ePHPm's KV. +/// +/// The pre-serve analyze gate's verdict is a pure function of (repo, PR head SHA, +/// gate config) and therefore identical on every node. Since switchboard runs on +/// every node and each materializes the same checkout, this cache lets the first +/// node to scan publish its verdict and the rest reuse it — one scan per commit, +/// cluster-wide, rather than N. +/// +/// # Scope and store +/// +/// Values live in the switchboard-private [`VERDICT_STORE_SITE`] namespace — NOT +/// a preview's own keyspace — reached with the same AUTH scoping [`KvRevoker`] +/// uses (`AUTH `, then bare keys) but with the reserved site. +/// The store is gossip-replicated, so a peer reads what the first scanner wrote; +/// and because no tenant can authenticate to this namespace (see +/// [`VERDICT_STORE_SITE`]), the deployed preview code cannot read or forge a +/// verdict. That is what closes the cache-poisoning hole: an earlier design put +/// verdicts in the preview's own keyspace, where the running app could +/// `ephpm_kv_set` a forged `Passed` for a future malicious commit it authors +/// (it knows the repo/PR/SHA, and the config fingerprint is derivable from the +/// public policy) and bypass the gate. +/// +/// Best-effort in both directions: a read failure means the caller scans locally +/// (fail-safe), and a write failure means peers scan themselves — neither ever +/// blocks a deploy. +#[derive(Debug, Clone)] +pub struct VerdictCache { + addr: String, + kv_secret: String, + ttl: Duration, +} + +impl VerdictCache { + /// Build a cache client for the switchboard-private verdict namespace. + /// + /// `addr` is ePHPm's RESP listener, `kv_secret` its `[kv] secret` (for the + /// per-site password derivation, here against [`VERDICT_STORE_SITE`]), and + /// `ttl` the expiry applied to a published verdict (the SHA is the real + /// invalidator; the TTL just garbage-collects old entries). There is + /// deliberately no preview-`site` parameter — every verdict for the whole + /// fleet lives in the one reserved namespace, keyed per commit. + #[must_use] + pub fn new(addr: impl Into, kv_secret: impl Into, ttl: Duration) -> Self { + Self { + addr: addr.into(), + kv_secret: kv_secret.into(), + ttl, + } + } + + /// `GET ` from the reserved verdict namespace. `Ok(None)` is a genuine + /// miss (nil reply); an `Err` is a transport/auth problem the caller treats + /// as "unavailable" and scans locally. + /// + /// # Errors + /// + /// Returns an error if the connection, AUTH, or GET fails. + pub async fn get(&self, key: &str) -> anyhow::Result> { + let password = derive_site_kv_password(&self.kv_secret, VERDICT_STORE_SITE); + tokio::time::timeout(OP_TIMEOUT, self.get_inner(&password, key)) + .await + .with_context(|| format!("KV GET from {} timed out after {OP_TIMEOUT:?}", self.addr))? + } + + /// `SET EX ` in the reserved verdict namespace. + /// + /// # Errors + /// + /// Returns an error if the connection, AUTH, or SET fails. + pub async fn put(&self, key: &str, value: &str) -> anyhow::Result<()> { + let password = derive_site_kv_password(&self.kv_secret, VERDICT_STORE_SITE); + let ttl = self.ttl.as_secs().max(1).to_string(); + tokio::time::timeout(OP_TIMEOUT, self.put_inner(&password, key, value, &ttl)) + .await + .with_context(|| format!("KV SET to {} timed out after {OP_TIMEOUT:?}", self.addr))? + } + + async fn get_inner(&self, password: &str, key: &str) -> anyhow::Result> { + let stream = TcpStream::connect(&self.addr) + .await + .with_context(|| format!("cannot connect to ePHPm KV listener at {}", self.addr))?; + let (read_half, mut write_half) = stream.into_split(); + let mut reader = BufReader::new(read_half); + + write_half + .write_all(&encode_command(&["AUTH", VERDICT_STORE_SITE, password])) + .await + .context("failed to send KV AUTH")?; + read_reply(&mut reader) + .await + .context("KV AUTH was rejected")?; + + write_half + .write_all(&encode_command(&["GET", key])) + .await + .context("failed to send KV GET")?; + let value = read_bulk_reply(&mut reader) + .await + .context("KV GET failed")?; + + let _ = write_half.write_all(&encode_command(&["QUIT"])).await; + Ok(value) + } + + async fn put_inner( + &self, + password: &str, + key: &str, + value: &str, + ttl_secs: &str, + ) -> anyhow::Result<()> { + let stream = TcpStream::connect(&self.addr) + .await + .with_context(|| format!("cannot connect to ePHPm KV listener at {}", self.addr))?; + let (read_half, mut write_half) = stream.into_split(); + let mut reader = BufReader::new(read_half); + + write_half + .write_all(&encode_command(&["AUTH", VERDICT_STORE_SITE, password])) + .await + .context("failed to send KV AUTH")?; + read_reply(&mut reader) + .await + .context("KV AUTH was rejected")?; + + write_half + .write_all(&encode_command(&["SET", key, value, "EX", ttl_secs])) + .await + .context("failed to send KV SET")?; + read_reply(&mut reader).await.context("KV SET failed")?; + + let _ = write_half.write_all(&encode_command(&["QUIT"])).await; + Ok(()) + } +} + /// Encode a command as a RESP2 array of bulk strings — the dialect ePHPm's KV /// server parses (`*\r\n` then `$\r\n\r\n` per argument). fn encode_command(args: &[&str]) -> Vec { @@ -171,6 +338,49 @@ where Ok(()) } +/// Read a RESP bulk-string reply (the shape `GET` returns): `$\r\n\r\n`, +/// or `$-1\r\n` for a nil (missing key). Returns `Ok(None)` for nil and +/// `Ok(Some(payload))` otherwise. A `-ERR` reply is surfaced as an error. +async fn read_bulk_reply(reader: &mut R) -> anyhow::Result> +where + R: tokio::io::AsyncBufRead + Unpin, +{ + use tokio::io::AsyncReadExt as _; + + let mut header = Vec::new(); + let n = reader + .read_until(b'\n', &mut header) + .await + .context("failed to read KV bulk reply header")?; + anyhow::ensure!(n > 0, "ePHPm KV closed the connection without replying"); + let header = String::from_utf8_lossy(&header); + let header = header.trim_end_matches(['\r', '\n']); + + if let Some(err) = header.strip_prefix('-') { + anyhow::bail!("ePHPm KV returned an error: {err}"); + } + let Some(len) = header.strip_prefix('$') else { + anyhow::bail!("expected a RESP bulk string from KV GET, got {header:?}"); + }; + let len: i64 = len + .parse() + .with_context(|| format!("KV GET returned a malformed bulk length {len:?}"))?; + if len < 0 { + // `$-1` — the key is absent. + return Ok(None); + } + let len = usize::try_from(len).context("KV GET returned an implausible bulk length")?; + + // Read the payload plus its trailing CRLF. + let mut buf = vec![0u8; len + 2]; + reader + .read_exact(&mut buf) + .await + .context("failed to read KV bulk payload")?; + buf.truncate(len); + Ok(Some(String::from_utf8_lossy(&buf).into_owned())) +} + #[cfg(test)] mod tests { use super::*; @@ -258,4 +468,183 @@ mod tests { let err = revoker.bump_share_epoch("app-pr-1", 1).await.unwrap_err(); assert!(err.to_string().contains("AUTH"), "{err}"); } + + // ── the verdict cache (bulk reply, GET/SET-EX) ────────────────────── + + #[tokio::test] + async fn bulk_reply_parses_a_value() { + let mut reply: &[u8] = b"$5\r\nhello\r\n"; + let mut reader = BufReader::new(&mut reply); + assert_eq!( + read_bulk_reply(&mut reader).await.unwrap(), + Some("hello".to_string()) + ); + } + + #[tokio::test] + async fn bulk_reply_nil_is_a_miss() { + let mut reply: &[u8] = b"$-1\r\n"; + let mut reader = BufReader::new(&mut reply); + assert_eq!(read_bulk_reply(&mut reader).await.unwrap(), None); + } + + #[tokio::test] + async fn bulk_reply_error_is_surfaced() { + let mut reply: &[u8] = b"-ERR nope\r\n"; + let mut reader = BufReader::new(&mut reply); + assert!(read_bulk_reply(&mut reader).await.is_err()); + } + + /// A cache **hit**: the fake server replies to AUTH then returns the stored + /// JSON as a bulk string; `get` returns it, and the AUTH is scoped to the + /// switchboard-private verdict namespace — never a preview's site key. + #[tokio::test] + async fn verdict_cache_get_returns_the_stored_value() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + // +OK to AUTH, then a 9-byte bulk string to GET. + sock.write_all(b"+OK\r\n$9\r\n{\"v\":\"x\"}\r\n") + .await + .unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + + let cache = VerdictCache::new(addr, "master-secret", Duration::from_secs(3600)); + let got = cache + .get("analyze:verdict:o/r:1:deadbeef:abcd") + .await + .unwrap(); + assert_eq!(got.as_deref(), Some("{\"v\":\"x\"}")); + + let received = server.await.unwrap(); + assert!(received.contains("AUTH"), "{received:?}"); + assert!( + received.contains(VERDICT_STORE_SITE), + "AUTH must name the reserved verdict namespace: {received:?}" + ); + // The reserved namespace is authenticated with the password derived for + // *it*, not for any preview. + let expected_pw = derive_site_kv_password("master-secret", VERDICT_STORE_SITE); + assert!( + received.contains(&expected_pw), + "AUTH must use the reserved-namespace password" + ); + assert!(received.contains("GET"), "{received:?}"); + assert!( + received.contains("analyze:verdict:o/r:1:deadbeef:abcd"), + "{received:?}" + ); + } + + /// A cache **miss**: nil reply to GET → `Ok(None)`. + #[tokio::test] + async fn verdict_cache_get_miss_is_none() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(b"+OK\r\n$-1\r\n").await.unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + }); + let cache = VerdictCache::new(addr, "master-secret", Duration::from_secs(3600)); + assert_eq!(cache.get("k").await.unwrap(), None); + } + + /// `put` issues `SET EX ` after AUTH. + #[tokio::test] + async fn verdict_cache_put_sets_with_ttl() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(b"+OK\r\n+OK\r\n").await.unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + let cache = VerdictCache::new(addr, "master-secret", Duration::from_secs(600)); + cache.put("k", "v").await.unwrap(); + + let received = server.await.unwrap(); + assert!(received.contains("SET"), "{received:?}"); + assert!(received.contains("EX"), "must set a TTL: {received:?}"); + assert!( + received.contains("600"), + "TTL seconds present: {received:?}" + ); + } + + /// An unreachable listener is an `Err` (the caller treats it as + /// "unavailable" and scans locally — fail-safe). + #[tokio::test] + async fn verdict_cache_get_on_a_dead_addr_errors() { + // Port 1 is not connectable; connect fails fast. + let cache = VerdictCache::new("127.0.0.1:1", "master-secret", Duration::from_secs(60)); + assert!(cache.get("k").await.is_err()); + } + + /// **The cache-poisoning guard.** The verdict namespace must be unreachable + /// by any preview tenant. A tenant's `ephpm_kv_*` is auto-scoped by ePHPm to + /// its own resolved site key, which is always a *valid* site key + /// ([`crate::site_key::is_valid_site_key`]); the reserved namespace is + /// deliberately not a valid site key (leading `\x1f`), so no tenant can ever + /// be scoped to it and thus cannot forge a verdict. + #[test] + fn verdict_namespace_is_unreachable_by_any_tenant() { + assert!( + !crate::site_key::is_valid_site_key(VERDICT_STORE_SITE), + "the verdict namespace must never equal a resolvable preview site key" + ); + // Concretely, it carries the gossip unit-separator, outside the + // [a-z0-9._-] site-key charset. + assert!( + VERDICT_STORE_SITE.contains('\u{1f}'), + "the reserved namespace must use an out-of-charset byte" + ); + // And it is disjoint from realistic preview site keys. + for site in [ + "ephpm-wordpress-sample-pr-7", + "app-pr-1", + "ephpm-my-blog-pr-42.preview.ephpm.dev", + ] { + assert!(crate::site_key::is_valid_site_key(site)); + assert_ne!( + site, VERDICT_STORE_SITE, + "no preview site key may equal the verdict namespace" + ); + } + } + + /// Every preview's verdict lives in the **one** switchboard-owned namespace, + /// so the cluster-wide dedup works across the whole fleet — the cache is not + /// scoped per preview (there is no preview-`site` parameter), only the + /// per-commit key distinguishes entries. + #[tokio::test] + async fn all_previews_share_one_verdict_namespace() { + // Two caches built independently (as two different previews' deploys + // would) both AUTH as the same reserved namespace. + for _ in 0..2 { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(b"+OK\r\n$-1\r\n").await.unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + let cache = VerdictCache::new(addr, "master-secret", Duration::from_secs(60)); + let _ = cache.get("analyze:verdict:o/r:1:sha:cfg").await; + let received = server.await.unwrap(); + assert!( + received.contains(VERDICT_STORE_SITE), + "every cache AUTHs the shared namespace: {received:?}" + ); + } + } } diff --git a/src/main.rs b/src/main.rs index 86d1e7f..dbf8ad4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,6 +17,7 @@ //! The legacy webhook receiver is still compiled in but defaults to **off** //! (`--webhook-server-enabled`). +mod analyze; mod config; mod deployer; mod drain; @@ -174,6 +175,41 @@ async fn main() -> anyhow::Result<()> { "preview access gate: private repos are always gated" ); + // Pre-serve static-analysis gate. Off unless --analyze-config is set; say so + // once, at WARN when off (a preview cluster publishing untrusted PR code with + // no screening is worth one line an operator will notice), at INFO when on. + if config.analyze_gate_enabled() { + let cfg = config + .analyze_config + .as_deref() + .expect("analyze_gate_enabled() implies a config path"); + info!( + analyze_config = %cfg.display(), + timeout_secs = config.analyze_timeout_secs, + "pre-serve analyze gate enabled — a preview is blocked on a bad `ephpm analyze` verdict" + ); + // Verdict dedup rides ePHPm's cluster-shared KV, which needs the KV + // secret; without it each node scans the same commit independently. + if config.kv_secret_file.is_some() { + info!( + verdict_ttl_secs = config.analyze_verdict_ttl_secs, + "analyze verdict dedup enabled — a commit is scanned once per cluster \ + (peers reuse the shared verdict; fail-safe to per-node scanning)" + ); + } else { + info!( + "analyze verdict dedup disabled (--kv-secret-file unset) — each node scans \ + the same commit independently" + ); + } + } else { + tracing::warn!( + "pre-serve analyze gate is NOT configured (--analyze-config unset) — previews \ + are published without static-analysis screening. Set --analyze-config \ + (SWITCHBOARD_ANALYZE_CONFIG) to an operator policy file to enable it" + ); + } + // Build the drain kicker before anything else runs: a missing token file // should fail at startup, not silently warn every two seconds forever. let kicker = if config.drain_enabled() { @@ -477,14 +513,21 @@ async fn handle_deploy(state: &AppState, req: &PreviewRequest) -> anyhow::Result preview_session_secret_ref: &state.config.preview_session_secret_ref, mint_share_link: state.config.share_link, share_token_ttl: state.config.share_token_ttl(), + analyze_config: state.config.analyze_config.as_deref(), + analyze_timeout: state.config.analyze_timeout(), + kv_addr: &state.config.kv_addr, + kv_secret: state.kv_secret.as_deref(), + analyze_verdict_ttl: state.config.analyze_verdict_ttl(), }; let result = deployer::deploy_preview(req, &ctx).await?; - // Reporting is best-effort: the preview is live either way, and a GitHub - // outage must not mark a good deploy as failed. Every node reconciles the - // same preview and reports, but the comment is deduplicated by its hidden - // marker: `post_preview_comment` finds an existing switchboard comment and - // updates it in place, so N nodes converge on one comment. + // Reporting is best-effort: the preview is live either way (or, when blocked, + // was never published), and a GitHub outage must not change the on-disk + // outcome. Every node reconciles the same preview and reports, but the + // comment is deduplicated by its hidden marker: `post_preview_comment` finds + // an existing switchboard comment and updates it in place, so N nodes + // converge on one comment. When the analyze gate blocked, the comment and the + // deployment status both render the block (see `github`). if let Some(client) = github_client(state, req).await { if let Err(e) = client.post_preview_comment(req, &result).await { tracing::error!(%e, "failed to post PR comment"); @@ -493,6 +536,20 @@ async fn handle_deploy(state: &AppState, req: &PreviewRequest) -> anyhow::Result tracing::error!(%e, "failed to set deployment status"); } } + + // A blocked preview was NOT published — the atomic swap never happened, so + // nothing external was provisioned to roll back (deploy_preview already + // removed its staging tree). Surface it as a failed job so it lands in + // `claimed/` for inspection rather than being cleared as a success. The PR + // comment and deployment status were posted above. + if let Some(block) = &result.analyze_block { + anyhow::bail!( + "preview blocked by the pre-serve analyze gate (verdict={}, {} finding(s)): {}", + block.verdict, + block.total_findings, + block.reason + ); + } Ok(()) }