PiCloud/security_audit/10_info_disclosure_cli.md

10 — Information disclosure, logging, CLI/operator token handling

Scope: Error-response shaping, request/audit logging, CLI (pic) token handling, dashboard token storage, build/observability surface. Reviewed at: main (matches AUDIT.md head, v1.1.9).

Counts

Severity	Count
High	2
Medium	4
Low	4
Info	5

Trace samples actually observed

• Error path (unique-violation on admin create): manager-core/src/admin_users_api.rs:464-467 maps AdminUserRepositoryError::DuplicateUsername(_) | DuplicateEmail(_) to a 409 whose body is Json({"error": self.to_string()}). The thrown error string is the typed enum variant ("username already taken"), not the raw sqlx/Postgres message; the constraint name and column never reach the client. Generic Db(e) and AuthzRepo(e) paths sanitize to "internal error" while logging the underlying error server-side. Same shape repeats across every *_api.rs (api.rs, apps_api.rs, secrets_api.rs, dead_letters_api.rs, topics_api.rs, …). DB-error leakage is not present.

• Happy-path log emission (login): manager-core/src/auth_api.rs:75-110. The handler does the Argon2 verify, never logs input.username / input.password. Error paths use tracing::error!(?err, "admin_users credentials lookup failed") etc. — the field captured is the repo error, not the credentials. The cookie/Bearer token is hashed (hash_token) before any field appears in auth_middleware.rs:204-208; only token_hash ever lands in tracing::error!. tower_http::trace::TraceLayer::new_for_http() at crates/picloud/src/lib.rs:593 emits the URI but headers (including Authorization) are off by default for that layer.

• CLI token write (pic login): crates/picloud-cli/src/config.rs:92-109 write_private opens the credentials file with OpenOptions::new().mode(0o600) and then re-set_mode(0o600) as belt-and-suspenders. Unit test posix_mode_is_0600 (line 137) asserts the file mode at rest. The token is persisted as TOML (token = "pic_...") in plaintext — no symmetric encryption with a passphrase or OS keychain integration, so a stolen home dir reveals the token. Not surprising for a developer-targeted CLI, but worth flagging as Info.

---

High

H1 — Dashboard bearer token stored in localStorage (XSS-readable)

• Location: dashboard/src/lib/auth.ts:22-48 (TOKEN_KEY = 'picloud.admin.token', localStorage.getItem/setItem).
• Detail: The same string used as the Authorization: Bearer value AND as the session cookie value lives in localStorage. Any XSS anywhere in the SPA (CodeMirror script editor, an in-dashboard log viewer that renders user-controlled JSON, a third-party dep with a prototype-pollution sink, etc.) reads the token in one line of attacker JS and exfiltrates it — the token is bearer-only with no per-request signing, so theft = full account takeover for the saved TTL (default 24 h sliding). The cookie path is described in the file's comments as the "primary" channel ("the bearer token doubles as the cookie value on the server side") but no HttpOnly / Secure / SameSite cookie is actually set by the dashboard — setSession() only writes to the store, then to localStorage. The middleware accepts the cookie if Caddy/browser sets one, but the dashboard itself does not.
• Fix shape:
  • Move the token to memory only (Svelte store, no localStorage echo) + a sibling HttpOnly; Secure; SameSite=Strict cookie set by /auth/login server-side. auth_api.rs:125-145 already returns the token; have it also Set-Cookie and have auth_middleware.rs:373-383 already reads the cookie.
  • Or: keep localStorage but pin to a refresh token with a 60-300s sliding access token kept in memory.
  • Adding a CSP default-src 'self'; script-src 'self' for the dashboard ships materially mitigates the blast radius but does not eliminate it.

H2 — pic admins create/set --password VALUE accepts the password on argv

• Location: crates/picloud-cli/src/main.rs:558 (Create), :576 (Set); crates/picloud-cli/src/cmds/admins.rs:50-58.
• Detail: Both subcommands take --password <PASSWORD> as a plain Option<String>. The clap arg is not restricted to - (stdin). When operators do the natural thing (pic admins create alice --password hunter2), the plaintext password ends up in:
  • the shell history (~/.bash_history, ~/.zsh_history)
  • ps aux for the lifetime of the process
  • /proc/<pid>/cmdline for any other UID on the host while it runs
  • pic login --token <T> (main.rs:126) has the same flaw for an API key. The help text warns ("Passing the password inline puts it in shell history — pipe instead when scripting") but the warning is documentation-only.
• Fix shape:
  • Reject any --password value other than - outright (return an error with "use --password - to read from stdin"), or
  • Accept the inline form but actively zero out argv[N] after parsing (prctl(PR_SET_NAME) on Linux + memset of the underlying String, both of which require unsafe / nightly tricks for String), or
  • Drop the inline form entirely and require --password -.
  • Same fix for pic login --token VALUE — make it --token-stdin or just remove the flag (the env-var fallback already covers CI).

---

Medium

M1 — Distinguishable 404s for "unknown host" vs "no route on host" enable enumeration

• Location: crates/orchestrator-core/src/api.rs:188-208.
• Detail: Two distinct JSON bodies:
  • "no app claims host {host:?}" when the Host header doesn't match any app's domain claim.
  • "no route matches {method} {path}" when the host is claimed but no route fires. An unauthenticated attacker can enumerate the set of apps installed by walking common subdomains (api., app., admin., …) and watching which 404 they receive, even though all such hosts must resolve to the operator's IP. Combined with the /version public_base_url it also reveals the canonical host the operator picked.
• Fix shape: Collapse both to a single uniform "no route matches METHOD PATH" (the host case is the same kind of routing miss from the caller's perspective). Log the distinction server-side for debugging; don't expose it to the client.

M2 — pic admins "no echo when interactive" comment is misleading; stdin read does echo

• Location: crates/picloud-cli/src/cmds/admins.rs:60-83.
• Detail: read_password_from_stdin says

  "Prompt without echoing — rpassword would be cleaner, but the CLI already avoids extra crates" but the implementation calls stdin.lock().read_line(&mut buf) — which does echo characters to the terminal. crates/picloud-cli/src/cmds/login.rs:64 uses rpassword::prompt_password correctly. The mismatch means pic admins create alice --password - typed at a TTY echoes the password on screen. (rpassword is already a transitive dep via pic login, so the "avoid extra crates" rationale is moot.)

• Fix shape: Use rpassword::prompt_password in the same place. Same applies to crates/picloud/src/main.rs:203 (prompt_password_from_stdin) — also read_line-based with an "(will be read from stdin, no echo)" prompt that's a lie.

M3 — ExecError::Runtime / ExecError::Parse strings echoed verbatim into responses

• Location: crates/orchestrator-core/src/api.rs:745-753; executor-core/src/types.rs:138-160.
• Detail: Rhai parse and runtime error strings are passed straight into the HTTP error body (e.to_string() → JSON). Rhai's error messages include line/column positions and (for runtime errors) snippets of the script source. For a public unauthenticated route to a script the caller doesn't own, this reveals script internals to the world. The blast radius is limited (Rhai source is uploaded by the script owner so it's "their" data leaking back), but for multi-tenant setups where one app's script is invoked across apps via invoke() it's a cross-tenant leak.
• Fix shape: Two-tier — log full message server-side, return only a stable kind ("script error", "script parse error") plus a correlation id to the client. Keep the verbose form for /api/v1/admin/scripts/{id}/test (authenticated, intra-app).

M4 — Credentials file is plaintext TOML with no at-rest encryption

• Location: crates/picloud-cli/src/config.rs:81-89.
• Detail: Token sits as token = "pic_..." in $XDG_CONFIG_HOME/picloud/credentials. Mode 0600 protects against same-host other-UID reads, but does not protect against backup capture, full-disk forensics of a stolen laptop, or a cloud-sync daemon that ignores the mode and uploads it. Several comparable CLIs (gh, gcloud) ship the same plaintext shape, so this is best-practice in line with peers — but worth being explicit about.
• Fix shape: Optional: add pic login --keyring that stores via OS keychain (Secret Service / Keychain / Credential Manager via the keyring crate) and falls back to plaintext when no keychain is reachable. Not required for v1.x.

---

Low

L1 — prompt_password_from_stdin in picloud admin reset-password echoes the password

• Location: crates/picloud/src/main.rs:203-216. Prompt advertises "will be read from stdin, no echo" but uses read_line. Same fix as M2.

L2 — Authorization: Bearer pic_… accidentally placed in a URL query would land in tower-http TraceLayer logs

• Location: crates/picloud/src/lib.rs:593 .layer(TraceLayer::new_for_http()).
• Detail: TraceLayer logs full URI including query string at DEBUG/INFO. The platform doesn't accept tokens via query string today, but the trace layer would log any future ?token= or ?api_key= parameter. Belt-and-suspenders: add a MakeSpan that strips known sensitive query keys, OR explicitly document "tokens go in headers, not in queries" in the SDK.

L3 — eprintln! for env-var validation failure logs env-var value

• Location: crates/manager-core/src/secrets_service.rs:59-62.
• Detail: tracing::warn!(value = %v, "ignoring invalid PICLOUD_SECRET_MAX_VALUE_BYTES …") echoes the operator-supplied value. The env var holds an integer in normal operation, so this is harmless, but the same pattern is copy-pasted across the file caps. If an operator typos PICLOUD_FILES_ROOT as a path containing a secret (e.g. an embedded credential in a network path), it would land in logs. Tiny.

L4 — PICLOUD_HTTP_ALLOW_PRIVATE warning shouts loudly but does not name the listener (good)

• Location: crates/picloud/src/lib.rs:172-176. Info — included for completeness; the warning is the correct shape.

---

Info

• I1 — /healthz is literally "ok" (crates/picloud/src/lib.rs:621). No leak.
• I2 — /version returns only product / sdk / api / schema / wire / public_base_url (crates/picloud/src/lib.rs:632-642). No build SHA, no OS, no internal IP. Good.
• I3 — No custom Server header set anywhere. Hyper's default Server: hyper may leak via the framework, but no manual augmentation. Info.
• I4 — No dedicated admin-action audit log table. Grep for audit_log finds only execution_logs (script outcomes) and DL rows. Admin actions like "user X minted API key Y", "user X deleted app Z", "user X promoted user Y to owner" are not durably recorded. Out of scope for this audit (overlaps with agent 1/2) but flagging for the operator-trust story.
• I5 — Source maps not emitted in prod. dashboard/svelte.config.js uses adapter-static; Vite default build.sourcemap = false. No leak.

---

Most important finding (2-3 sentences)

H1 (dashboard token in localStorage) is the biggest single risk: a bearer token good for full admin takeover sits in DOM-readable storage with no XSS mitigation in front of it, and a CSP is not configured. Combined with the CodeMirror-based script editor (which evaluates user-typed Rhai) and a dashboard that renders trigger payloads + execution-log fields from JSONB columns, any one HTML-injection bug becomes a full account takeover. The fix is well-scoped — switch the server-side login response to also set an HttpOnly; Secure; SameSite=Strict cookie, keep an in-memory access token for SPA reloads, and drop the localStorage echo entirely.