Files
PiCloud/security_audit/09_external_integrations.md
MechaCat02 ec4a2aa24a style: cargo fmt across audit-2026-06-11 tier-3 changes
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-12 18:38:28 +02:00

12 KiB
Raw Permalink Blame History

PiCloud Security Audit — External Integrations

Scope: Outbound HTTP from scripts (http::*), inbound email webhooks, outbound SMTP, third-party calls (SSRF / replay / auth), Postgres connection, Caddy admin, cluster RPC, pic CLI ↔ Manager, auto-HTTPS. Tree state: main at v1.1.9, commit 05ed9b0. Method: End-to-end trace of http::get(url) from Rhai bridge → HttpServiceImpl::requestreqwest::Client (with dns_resolver(SsrfResolver)); plus targeted review of email-inbound HMAC, email-outbound SMTP, CLI TLS, and infra topology.

Counts

Severity Count
Critical 0
High 1
Medium 3
Low 3
Info 4

The SSRF defense is unusually thorough for an MVP — full deny-list, hostname-vs-literal-IP split, resolver runs at every connect (including redirect hops), DNS-rebinding tested, cross-origin Authorization scrub tested. The remaining material gap is operational: scripts can send unlimited outbound email through the operator's SMTP relay because the only token-bucket lives in users_service (account-flow emails), not in EmailServiceImpl.


High

F-EXT-H-001 — email::send from scripts is not rate-limited; operator's SMTP relay is a free amplifier

File: /home/fabi/PiCloud/crates/manager-core/src/email_service.rs

EmailServiceImpl::send (lines 257271) checks authz, size cap, and address validity — no rate limit, no per-app daily cap, no recipient count cap. A token-bucket does exist (/home/fabi/PiCloud/crates/manager-core/src/users_service.rs:139216, EmailRateLimiter) but it is wired only into the user-account flows (verification, password reset). Scripts calling email::send(...) through the SDK never traverse users_service — they hit EmailServiceImpl directly.

A public, anonymous HTTP-triggered script with the AppEmailSend capability granted to the script (the default for the Editor role on the app) can therefore:

  • Send any number of mails per second (up to the global script-concurrency cap of 32).
  • Include any number of to: / cc: / bcc: recipients per message — build_message (lines 276321) iterates the lists unbounded.
  • Reuse the operator's SMTP credentials (PICLOUD_SMTP_*) for spam, abuse, or relay quota exhaustion. The provider's reputation is the operator's, not the script's.

Combined with no per-message recipient cap, one request can fan out to thousands of bcc: addresses ("BCC bomb"). The SMTP relay will eventually rate-limit, but the abuse will burn through the operator's daily allowance first.

Fix:

  1. Move EmailRateLimiter (or a sibling instance) into EmailServiceImpl. Apply it inside send before transport handoff. Per-app daily cap must remain per-app, not global (agent 8's note in the scope was correct).
  2. Add a per-message recipient cap: to.len() + cc.len() + bcc.len() <= MAX_RECIPIENTS_PER_MESSAGE (e.g. 50, env-overridable).
  3. Surface as EmailError::RateLimited / EmailError::TooManyRecipients so scripts can handle it.

Severity rationale: This is one capability away from "the platform sends spam in the operator's name." Reputation damage + cost amplification. Bumped to High (not Critical) because AppEmailSend is gated on app membership for authed principals — but the moment a script is anonymous-HTTP-exposed and grants itself email, the gate evaporates (the service skips authz when cx.principal == None).


Medium

F-EXT-M-001 — Email-inbound webhook accepts unsigned POSTs when inbound_secret is None

File: /home/fabi/PiCloud/crates/manager-core/src/email_inbound_api.rs:149156

if let (Some(ct), Some(nonce)) = (
    target.inbound_secret_encrypted.as_ref(),
    target.inbound_secret_nonce.as_ref(),
) {
    let secret = decrypt_secret(&s.master_key, ct, nonce)?;
    verify_signature(&headers, &body, secret.as_bytes(), &s.nonce_dedup)?;
}

If the trigger row has no inbound_secret, signature verification is silently skipped. Anyone who guesses or scrapes the trigger UUID can POST arbitrary email events into the outbox, which the dispatcher executes. The trigger UUID is a UUIDv7-ish secret but it leaks in admin API logs, dashboard URLs, and Mailgun/Postmark provider dashboards.

Agent 8 already flagged this as "the receiver accepts unsigned" — confirming here from the protocol shape. The remediation is to make inbound_secret mandatory at trigger creation (triggers_api.rs:589) or to refuse to mount the route unless a secret is present.

The replay protection (F-S-010) is correctly closed: (ts, sha256(body)) LRU with 600 s TTL, 300 s tolerance window, constant-time mac.verify_slice. Signature is HMAC-SHA256 over ts || "." || body. All good — but only when a secret exists.

F-EXT-M-002 — Inbound webhook nonce dedup is process-local; cluster mode (v1.3+) will silently degrade

File: /home/fabi/PiCloud/crates/manager-core/src/email_inbound_api.rs:6789

InboundNonceDedup is a Mutex<HashMap> in the process address space. In single-node MVP this is fine. In cluster mode with multiple manager instances behind Caddy, a replayed POST can land on a different node and the dedup misses. The replay window is short (~300 s) so impact is limited, but the comment at line 9799 ("Per-process is fine for single-node MVP and dev") acknowledges the gap. Flag now so it isn't forgotten when cluster mode lands.

Fix when cluster ships: push the dedup tuple into a Postgres table with a (trigger_id, ts, body_hash) PRIMARY KEY and a 10-minute partial cleanup job, or into Redis.

F-EXT-M-003 — PICLOUD_SMTP_PASSWORD lives in an env-derived String and stays in memory for process lifetime

File: /home/fabi/PiCloud/crates/manager-core/src/email_service.rs:104, 175

SmtpConfig::password: String is read from env and cloned into Credentials::new(user, password) inside LettreEmailTransport::build. Lettre's Credentials does not zero-on-drop. A heap dump (e.g. core dump shipped to a bug tracker) would expose the relay password.

This is acceptable for the operator-provided env-var model, but worth noting: the same secret-handling pattern as users.password_hash should eventually use secrecy::SecretString or equivalent to at least redact in Debug. SmtpConfig does derive Debug (line 87), which means tracing::debug! of the struct anywhere would print the password verbatim. Searched — no current call site prints it, but the foot-gun is loaded.

Fix: wrap password in secrecy::SecretString (already in tree for MasterKey), drop the Debug derive from SmtpConfig, or implement Debug manually with [REDACTED].


Low

F-EXT-L-001 — Bounded set of opt-in HTTP options is good; script_id flows to outbound User-Agent

File: /home/fabi/PiCloud/crates/manager-core/src/http_service.rs:371380

Default UA is picloud/<version> (script:<script_id>). Operator may not want to expose script IDs to third parties (e.g. for telemetry minimization). The script_id is a UUID — not directly sensitive, but it does fingerprint the platform. Consider making the UA opaque (picloud/<version>) in prod or stripping the (script:...) segment behind a config flag.

F-EXT-L-002 — validate_url blocks ports 22/25/465/587 but not 23/110/143/993/995/3306/6379

File: /home/fabi/PiCloud/crates/manager-core/src/http_service.rs:343345

if matches!(port, 22 | 25 | 465 | 587) {
    return Err(HttpError::BlockedPort(port));
}

The deny-list covers SSH + the canonical SMTP family. It misses telnet (23), POP3 (110/995), IMAP (143/993), MySQL (3306), Redis (6379), MongoDB (27017), memcached (11211). All of these are reachable from a script if the target's IP is public — typical for the cloud-VPC case where an internal service is on a public IP but firewalled.

The SSRF resolver handles the private-IP case, so the practical exploit is narrow: a public IP running e.g. an exposed Redis on 6379. Still, an allow-list (only 80/443/8080-8090/HTTPS-ALPN-ports) would be safer than the current deny-list. Mark as Low because the realistic exploit requires a misconfigured target.

F-EXT-L-003 — Postgres connection: DATABASE_URL is required but sslmode is unset by default

File: /home/fabi/PiCloud/crates/picloud/src/lib.rs:607618

PgPoolOptions::new().connect(url) honors whatever sslmode= is in DATABASE_URL. Default for sqlx/Postgres is prefer — it tries TLS, falls back to plaintext silently. In the dev docker-compose Postgres is bound to a private network, so plaintext is fine — but in a real prod deploy where Postgres lives on a managed service, an operator who forgets ?sslmode=require may transmit credentials cleartext on the LAN.

Fix (doc, not code): the production docker-compose / deploy guide should call this out. Optionally: parse DATABASE_URL at startup and warn if sslmode is missing or disable/prefer when host is not localhost/postgres/127.0.0.1.


Info

F-EXT-I-001 — SSRF defense is solid: confirmed end-to-end trace

Traced http::get("http://169.254.169.254/") from Rhai:

  1. sdk/http.rs:78invoke(...) builds HttpRequest { url: "http://169.254.169.254/", ... }.
  2. block_on (line 383) → HttpServiceImpl::request (http_service.rs:166).
  3. run (line 210) → validate_url (line 217) parses host as Host::Ipv4, calls policy.check(IpAddr::V4(169.254.169.254)).
  4. ssrf.rs:check_v4 line 77: [169, 254, ..] => Err("link-local").
  5. Returned to script as HttpError::Ssrf("link-local").

For hostnames (e.g. metadata.google.internal), validate_url passes through (it's a Host::Domain), the resolver in ssrf.rs:181-220 does the lookup, sees the resolved IP is 169.254.x.x, filters it out, and if all addresses are denied, returns the SSRF marker error which map_reqwest_err (line 401) → ssrf_reason (line 419) walks the chain for. Tested at http_service.rs:732, 750 with both literal-IP and hostname-via-localhost paths.

Redirect hops re-resolve through the same DNS resolver (because redirect(Policy::none()) + manual follow at line 225280 re-creates each request through the same client), so redirect-to-SSRF is blocked. Cross-origin Authorization scrub is implemented at lines 261265 and tested at lines 868892. DNS-rebinding is tested at ssrf.rs:411443.

F-EXT-I-002 — Cluster-mode RPC: no inter-service auth today

The three split binaries (picloud-manager, picloud-orchestrator, picloud-executor) are skeletons. The ExecutorClient trait abstraction is in place for the swap to remote HTTP, but cluster mode itself is unbuilt. Authentication between services is a v1.3+ design problem; nothing to flag at this stage beyond "don't ship cluster without it."

F-EXT-I-003 — Caddy admin: dev has admin off, prod uses Caddy's localhost-only default

Files: caddy/Caddyfile:20, caddy/Caddyfile.prod (no admin block).

Dev caddyfile sets admin off — closed. Prod inherits Caddy's default admin localhost:2019. As long as the container's port 2019 is not published in docker-compose.prod.yml (verified: only the front-facing 80/443 are exposed), the admin API is unreachable from the LAN. Good. Worth adding admin off to the prod caddyfile too, for defense in depth.

F-EXT-I-004 — pic CLI uses default rustls TLS; no --insecure flag exists

File: /home/fabi/PiCloud/crates/picloud-cli/src/client.rs:3343

reqwest::Client::builder() with no danger_accept_invalid_certs call. Bearer token transmitted in Authorization header (line 53), never URL. Audit-clean.


What was NOT found / out of scope

  • HTTP-async (http_async::request): the scope brief mentioned this surface, but a workspace-wide search for http_async / HttpAsync returned zero hits — the dispatcher (dispatcher.rs) does not invoke any HTTP client and there's no http_async SDK module. Either the feature is deferred (post-v1.1.9) or already collapsed into the same http_service path — both are fine.
  • Bounce handling: no bounce-processing code exists. Nothing to leak.
  • SMTP injection via CRLF: agent 4's scope. Not re-audited here.