Detail: _db_query builds docker compose exec -T --env MYSQL_PWD=<pw> db mysql .... The DB password ends up in the host's argv (visible to any co-tenant via /proc/<pid>/cmdline). Comment at api/utils/versions.py:233-269 documents the previous P0 fix that moved away from this exact pattern (stdin --defaults-file=/dev/stdin); db_tools.py did not get the same treatment and regressed the pattern. Every health detector, table sizes endpoint, cleanup orphan-postmeta, and every operator SELECT runs through this call site.
Recommendation: Replicate the stdin / --defaults-file=/dev/stdin pattern from api/utils/versions.py:258-280 here; never put MYSQL_PWD= on argv.
[HIGH] CSRF: three mutating /api/internal/migration/* proxy routes missing requireSameOrigin¶
Detail: These three are the only mutating internal proxy handlers in the diff that do not call requireSameOrigin from web/lib/csrf.ts. An attacker page can fire a same-site fetch(..., {credentials:"include"}) once an operator's OIDC cookie is live, causing the server to (a) call the Cloudflare API and create a real DNS record, (b) provision a new site row, or (c) upload a multipart ZIP. The dns-setup and fresh-install routes are JSON-bodied so adding requireSameOrigin is a one-line fix; import-zip is multipart so it needs the Origin-only branch of the helper (skip the JSON content-type check).
Recommendation: Add const denied = requireSameOrigin(req); if (denied) return denied; at the top of all three handlers. For import-zip, extend csrf.ts to allow an Origin-only path for multipart, or perform the origin check inline.
[HIGH] SSRF / path traversal via unvalidated slug reaching wp_org.fetch_zip(download_link)¶
File: /Users/ramonkamibayashicarrera/homelab-infra/services/wp-pulse-server/api/routers/themes.py:487-517 (install_theme_wp_org) and …/api/utils/wp_org.py:241,257,284
Class: SSRF / Path traversal
Detail: install_theme_wp_org accepts body.slug constrained only by min_length=1, max_length=200 (no character allowlist). It is interpolated directly into PLUGIN_INFO_URL.format(slug=slug) (which is https://api.wordpress.org/plugins/info/1.0/{slug}.json) — a slug like evil/../../../some/path reshapes the request URL. The plugin router does validate via _validate_plugin_slug (regex ^[A-Za-z0-9_\-]{1,80}$) but themes does not. Then fetch_zip(url) follows redirects and downloads from whatever URL appears in download_link — an attacker who can influence the upstream response (limited threat: WP.org compromise) or interact with the slug component (greater threat) gets server-side fetch of an arbitrary URL written into the worktree.
Recommendation: Add _validate_theme_slug mirroring _validate_plugin_slug and gate install_theme_wp_org (and any other WP.org caller) on it. Additionally validate that download_link in fetch_zip is a https URL whose host is downloads.wordpress.org (or a small allowlist) before issuing the GET.
[HIGH] Public approval decide endpoint disables the approval gate via in-place mutation¶
Detail: To prevent the apply-recursing-into-another-approval loop, decide_approval does site.require_approval = False BEFORE calling apply_all_to_prod, then restores it in finally. Because the SQLAlchemy session is shared and await db.commit() is called inside apply_all_to_prod (re-uses the same db session), the cleared require_approval will be flushed to the database mid-apply. A concurrent operator action (or HTTP request) read between the clear and the restore will see the gate as OFF and can push without approval. Worse, if the apply errors before finally runs, the gate is restored in-memory but the prior commit may have already persisted False.
Recommendation: Don't toggle require_approval on the live Site row. Plumb a skip_approval_check: bool kwarg through apply_all_to_prod and check that boolean instead of the column.
[MEDIUM] db_tools SQL allowlist accepts inline comments that can hide forbidden verbs (FP) and stacked-statement bypass via crafted whitespace¶
Detail: _FORBIDDEN_VERBS is a plain \b...\b regex that does not strip comments first, so SELECT /* harmless */ 1 works but SELECT 1 /* WHERE x = UPDATE */ is wrongly rejected (FP, not security). More importantly the _STATEMENT_TERMINATOR = r";\s*\S" only catches semicolons followed by non-whitespace — a payload ending in ; followed by trailing whitespace is permitted, and downstream the mysql client does read stdin as a script supporting multiple ;-separated statements. The verb allowlist still catches concrete writes; UNION-based data exfiltration to other tables remains in-bounds for a SELECT-only allowlist.
Recommendation: Strip C-style + line comments before matching forbidden verbs. Enforce "exactly one statement" by counting unescaped ; outside string literals rather than the regex hack. Document that the console is best-effort, not a sandbox.
[MEDIUM] Magic-link impersonation tokens live in-process only and survive process boundaries with no per-IP throttle¶
Detail: Tokens are 256-bit secrets.token_urlsafe(32) (good), single-use, TTL 5 min, capped at 256 live (good). The consume_impersonation_token endpoint is open by design but has no rate limiting and no per-IP/per-slug brute-force guard. With 256-bit entropy brute-force isn't feasible, but a stuck/slow attacker pummeling the endpoint could mask other operators' valid consume attempts (the LRU eviction _gc_locked will drop their token if 256 attacker tokens are minted faster than they consume — but _TOKENS only grows on mint_impersonation_token which is admin-gated, so this is bounded by admin actions). Acceptable risk; flagging because the comment block claims "single-use AND short-TTL" is the only defence.
Recommendation: Add a simple per-IP rate limit on /impersonate-consume (e.g. 10/min) so a broken mu-plugin or hostile WP shell can't generate spam.
[MEDIUM] Worktree containment relies on target.resolve().relative_to(staging_dir.resolve()) post-create¶
Detail: The path-traversal guard for the ZIP importer constructs target = staging_dir / info.filename then calls target.resolve() and compares to staging_dir.resolve(). pathlib.resolve() does NOT require the file to exist on Linux, but on a system where staging_dir is itself a symlink (e.g. operator pointed WPP_IMPORT_STAGING_ROOT at a symlinked directory or a previous import created one), resolve() will follow it and relative_to could falsely succeed. The check runs BEFORE zf.extractall(staging_dir) which does its own internal normalisation in Python 3.12+, but earlier versions do not normalise. The 5 GB uncompressed cap is good but absent a per-file uncompressed cap; a malicious archive with 5 GB / 100 = 50 MB per file × 100 files would still get extracted (acceptable, just noting).
Recommendation: After the target.resolve().relative_to(...) call, additionally reject any archive entry whose info.filename contains a literal .. segment or absolute path. Enforce both per-entry cap (e.g. 200 MB) and the existing total cap.
[MEDIUM] Theme + plugin slug not validated before being used in shell-out / on-disk path¶
File: /Users/ramonkamibayashicarrera/homelab-infra/services/wp-pulse-server/api/routers/themes.py:533-572 (delete_theme, preview_switch), …/themes.py:602-630 (preview_switch passes theme_slug to _run_wp(["theme","activate",theme_slug]))
Class: Argument injection / path traversal
Detail: theme_slug arrives in the path and is used as (a) a directory name under themes_dir / theme_slug (with a post-hoc target_resolved.parents check), and (b) a positional argument to wp theme activate. The post-hoc resolve check in delete_theme is correct, but in preview_switch the slug is passed straight to wp-cli without a regex check — a value like --path=... could be re-interpreted by wp-cli as a flag. Because argv is properly tokenised (no shell), classical command injection is impossible; argument injection to wp-cli is the residual risk.
Recommendation: Add _validate_theme_slug (regex ^[A-Za-z0-9_\-]{1,80}$) and call it at the top of every theme_slug route handler in themes.py. Same hardening that plugins.py:223,234 already does.
[MEDIUM] wp_org.fetch_zip follows redirects with no host pinning¶
Detail: httpx.AsyncClient(timeout=..., follow_redirects=True) follows redirects to arbitrary hosts. The 50 MB cap and the "starts with PK" check limit damage, but a redirect to an internal endpoint (e.g. http://192.168.0.196:8080/api/v1/sites) returning a small response would still be issued. The URL comes from WP.org's response (trusted) but is also passed-through by install_theme_wp_org callers.
Recommendation: Either disable redirects or restrict redirects to *.wordpress.org and downloads.wordpress.org hosts. Reject non-https schemes explicitly.
[MEDIUM] Customer-supplied feedback URL/UA/message rendered into operator inbox without explicit HTML escaping at storage¶
Detail: widget_submit stores url, user_agent, message, screenshot_b64 verbatim from the customer's browser via the widget. The operator-side inbox returns them via FeedbackListResponse and the React UI will (presumably) JSX-escape strings by default. But if any component renders these via dangerouslySetInnerHTML (e.g. a "rich" feedback formatter), stored XSS lands in the operator's authenticated session. The widget itself sets a maxlength=3500 and trims (good); the server caps message at 4000 chars but does NOT cap url or user_agent.
Recommendation: Add max_length constraints on WidgetSubmission.url and WidgetSubmission.user_agent (e.g. 2048 / 512). Audit the operator UI for any dangerouslySetInnerHTML on feedback content. Validate url is a http(s):// URL.
[MEDIUM] CF API token logged via response body on transport error¶
Detail: logger.warning("CF purge %s zone=%s rc=%s body=%s", host, zone_id, r.status_code, r.text[:200]) logs the raw response body. CF error responses do not echo the token, but rate-limit / 401 responses may include "X-Ratelimit" headers + the auth context. Risk is low (200-char cap, body not headers); flagging for hygiene.
Recommendation: Replace r.text[:200] with r.json().get("errors") or a redacted summary; never log raw upstream bodies for any endpoint that may echo auth state.
[MEDIUM] requireSameOrigin only enforces JSON content-type when a body is present¶
Detail: The helper deliberately allows body-less mutating requests to skip the content-type check (hasBody = contentLength > 0). An attacker page can fetch(url, {method:"POST", credentials:"include"}) with no body to trigger any "side-effect" POST that doesn't require a payload (e.g. bisect/cancel, bisect/finish, pushes/.../rollback). The Origin check still applies — but Origin is sent with simple cross-origin requests AND can match if the attacker hosts a page on an allowlisted origin (e.g. localhost:3000 during dev). For production where the only allowlisted origin is https://wp-pulse-app.monxas.casa, this is mitigated by the absence of attacker-controlled content there. Still: relaxing the content-type guard on body-less requests removes one defence layer for no real UX benefit.
Recommendation: Drop the hasBody exemption. If body-less mutating requests are needed, change them to JSON {} on the client and require the content-type unconditionally.
[LOW] Impersonation mu-plugin lands in worktree on every mint; promote-to-prod could ship it if operator clicks Apply All¶
Detail: _ensure_impersonate_mu_plugin_on_disk writes mu-plugins/wp-pulse-impersonate.php into the worktree on every mint. The header comments state the file short-circuits unless it sees WPP_PREVIEW, BUT the docstring acknowledges "if it accidentally lands on prod via a stale apply-all-to-prod". The "preview only" guard depends solely on a PHP constant the operator-side compose defines; a prod environment that happens to define WPP_PREVIEW (e.g. shared wp-config.php includes) would mint working impersonation links on prod.
Recommendation: Have the mu-plugin gate on hostname (containing -preview.) AND the WPP_PREVIEW constant; do both checks independently. Reject in _ensure_impersonate_mu_plugin_on_disk if the worktree's wp-config already defines WPP_PREVIEW in a way that wouldn't be true on prod.
[LOW] Site row pubkey for imported/fresh sites uses a non-secret prefix that's parseable¶
Detail: pubkey_ed25519 = f"wp-pulse-imported-{secrets.token_hex(16)}" and f"wp-pulse-fresh-{secrets.token_hex(16)}" are written into the pubkey_ed25519 NOT NULL UNIQUE column as placeholders. Any endpoint that exposes pubkeys (or operator-visible site listings) will leak the "this site is imported/fresh and has no real agent" state. Low impact — operator-only data.
Recommendation: Don't store provenance in the placeholder; use the existing import_status column the code already sets to "imported"/"fresh".
Detail: upsert_config accepts any hostname string, lowercases + strips, stores. The public lookup public_status then keys on this string. Two failure modes: (a) a malformed hostname could collide with another site's primary URL (unlikely but possible since _allowed_hosts_for_slug in feedback.py includes status-page hostname as an "allowed origin" → an attacker who can convince an operator to register attacker.com as a status hostname can then post widget feedback claiming any slug; (b) wildcards or empty strings could overflow the lookup.
Recommendation: Validate against the same _DOMAIN_RE used in dns_setup.py:32. Enforce uniqueness in the DB.
[LOW] Approval public endpoint reveals existence of expired/decided tokens¶
Detail: GET /api/v1/approvals/{token} returns 404 for "approval not found" and 200 with status="expired" / "rejected" / "approved" for known tokens. An attacker with a leaked or guessed (256-bit, unlikely) token can confirm prior decisions and timestamps. With 256-bit entropy this is theoretical, but the differential response is unnecessary.
Recommendation: Return 410 Gone for any decided/expired token without distinguishing — only "pending" returns 200 with details.
[LOW] Fleet search-replace builds raw SQL with operator-supplied prefix¶
Detail: The needle goes through _sql_quote (escapes \ and '). The prefix comes from _wpcli_table_prefix which runs wp config get table_prefix — the operator controls a site's wp-config, so a malicious prefix like wp_;DROP-- could land in the dynamically-built \`` identifier. Trust boundary: the prefix originates from a site we already control (we pulled the wp-config, the operator types it on our server). Low risk; flagging because the comment claims "table/column identifiers can't be parameterized" without then validating them.}{short_name
Recommendation: Pin prefix with a regex ^[A-Za-z0-9_]+$ before substituting into SQL. Same for short_name and column even though those come from a closed dict — defence in depth.
[LOW] Approval auto-expire on list is a TOCTOU window¶
Detail: list_site_approvals flips status from pending to expired lazily. Between read and commit, a concurrent decide_approval for the same token could land an approval against a row another request has just marked expired. SQLAlchemy's per-row UPDATE will resolve last-writer-wins but the apply may have already fired before the expire-commit lands.
Recommendation: Move the lazy expire to a SELECT ... FOR UPDATE or use a dedicated cron. For now document the race in _is_expired.
[INFO] Admin token sourced from environment only (good)¶
Detail: pw.chromium.launch(headless=True, args=["--no-sandbox"]) + page.set_content(html, ...). The HTML is server-rendered Jinja with autoescape on; no external URLs are fetched by Playwright (no page.goto). SSRF risk: nil. The --no-sandbox flag is only safe because the LXC itself is the sandbox. Acceptable; document the dependency.
[INFO] Widget cross-origin POST validates slug → origin host mapping; permissive preflight is fine¶
Detail: Preflight echoes any Origin (acceptable: preflight doesn't carry the slug). On the POST, the server resolves the slug → enrolled hostnames (sites.url + site_status_pages.hostname) and rejects with 403 if the request's Origin isn't in that set. This is the correct shape. Confirmed solid.