12 KiB
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::request → reqwest::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 257–271) 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:139–216, 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 276–321) 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:
- Move
EmailRateLimiter(or a sibling instance) intoEmailServiceImpl. Apply it insidesendbefore transport handoff. Per-app daily cap must remain per-app, not global (agent 8's note in the scope was correct). - Add a per-message recipient cap:
to.len() + cc.len() + bcc.len() <= MAX_RECIPIENTS_PER_MESSAGE(e.g. 50, env-overridable). - Surface as
EmailError::RateLimited/EmailError::TooManyRecipientsso 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:149–156
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:67–89
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 97–99 ("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:371–380
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:343–345
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:607–618
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:
sdk/http.rs:78→invoke(...)buildsHttpRequest { url: "http://169.254.169.254/", ... }.block_on(line 383) →HttpServiceImpl::request(http_service.rs:166).- →
run(line 210) →validate_url(line 217) parses host asHost::Ipv4, callspolicy.check(IpAddr::V4(169.254.169.254)). ssrf.rs:check_v4line 77:[169, 254, ..] => Err("link-local").- 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 225–280 re-creates each request through the same client), so redirect-to-SSRF is blocked. Cross-origin Authorization scrub is implemented at lines 261–265 and tested at lines 868–892. DNS-rebinding is tested at ssrf.rs:411–443.
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:33–43
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 forhttp_async/HttpAsyncreturned zero hits — the dispatcher (dispatcher.rs) does not invoke any HTTP client and there's nohttp_asyncSDK module. Either the feature is deferred (post-v1.1.9) or already collapsed into the samehttp_servicepath — 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.