diff --git a/Cargo.lock b/Cargo.lock index 6734c52..ce51ffb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1962,6 +1962,7 @@ dependencies = [ "clap", "directories", "libc", + "percent-encoding", "picloud-shared", "postgres", "predicates", diff --git a/crates/executor-core/src/sdk/users.rs b/crates/executor-core/src/sdk/users.rs index 5f59d39..a0bdccc 100644 --- a/crates/executor-core/src/sdk/users.rs +++ b/crates/executor-core/src/sdk/users.rs @@ -10,6 +10,9 @@ //! // for anonymous public scripts. //! let free = users::email_available("a@b"); // bool — anonymous-safe pre-check //! // for self-serve registration. +//! // Leaks only "exists/doesn't" but +//! // is cheap + unthrottled: throttle +//! // the route if enumeration matters. //! users::update(id, #{ display_name: "Alicia" }); //! let removed = users::delete(id); // bool //! let page = users::list(#{ "$limit": 50, cursor: () }); diff --git a/crates/manager-core/src/users_service.rs b/crates/manager-core/src/users_service.rs index 06b21a9..6916f25 100644 --- a/crates/manager-core/src/users_service.rs +++ b/crates/manager-core/src/users_service.rs @@ -520,9 +520,17 @@ impl UsersService for UsersServiceImpl { // Gated like `create` (the registration write path) so the probe // is available exactly where self-serve registration is — and, // crucially, WITHOUT find_by_email's anonymous-principal rejection. - // It returns only a boolean, so it leaks no more than `create`'s - // own uniqueness error already does (F-S-003: enumeration risk is - // bounded to "exists / doesn't"). + // + // SECURITY (F-S-003, enumeration): a single probe reveals only + // "exists / doesn't" — the same bit a register form leaks via its + // "email already taken" path. But unlike `create`, this probe has + // no Argon2 cost and no side effect (a `create` miss writes a junk + // row), so it is a *cheaper, quieter* enumeration oracle when a + // script exposes it on an anonymous public route. It is NOT rate + // limited here, and there is no built-in per-route throttle/CAPTCHA + // primitive. Scripts that put this behind a public endpoint and + // care about enumeration must add their own `kv`-based counter. + // See docs/stdlib-reference.md. self.require(cx.principal.as_ref(), Capability::AppUsersWrite(cx.app_id)) .await?; let normalized = validate_email(email)?; diff --git a/crates/orchestrator-core/src/api.rs b/crates/orchestrator-core/src/api.rs index 852affe..8abd84a 100644 --- a/crates/orchestrator-core/src/api.rs +++ b/crates/orchestrator-core/src/api.rs @@ -170,7 +170,12 @@ where E: ExecutorClient + 'static, R: ScriptResolver + 'static, { - let method = request.method().as_str().to_string(); + // Uppercased so `ctx.request.method` honors its documented contract + // (extension methods arrive in their original case). Route matching is + // case-insensitive (see `routing::matcher::method_matches`), so this + // only normalizes the value surfaced to scripts / persisted in the + // async-dispatch payload. + let method = request.method().as_str().to_ascii_uppercase(); let uri = request.uri().clone(); let path = uri.path().to_string(); let query_str = uri.query().unwrap_or("").to_string(); diff --git a/crates/picloud-cli/Cargo.toml b/crates/picloud-cli/Cargo.toml index 57dcfec..caa5d71 100644 --- a/crates/picloud-cli/Cargo.toml +++ b/crates/picloud-cli/Cargo.toml @@ -24,6 +24,7 @@ path = "tests/cli.rs" [dependencies] picloud-shared.workspace = true reqwest = { workspace = true, features = ["json"] } +percent-encoding.workspace = true serde.workspace = true serde_json.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 9343c75..c95da4b 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -9,6 +9,7 @@ use std::collections::BTreeMap; use anyhow::{anyhow, Context, Result}; use chrono::{DateTime, Utc}; +use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; use picloud_shared::{ AdminUserId, ApiKeyId, App, AppDomain, AppId, AppRole, AppUser, DispatchMode, ExecutionLog, HostKind, InstanceRole, PathKind, Route, Scope, Script, ScriptId, @@ -19,6 +20,24 @@ use serde_json::Value; use crate::config::Credentials; +/// Characters that must be escaped inside a single URL **path segment** so +/// a free-form value (an app slug, a topic name) can't break out of its +/// segment — `/` would start a new segment, `?`/`#` would start the query +/// or fragment. Encodes controls, space, and the structural delimiters. +const PATH_SEGMENT: &AsciiSet = &CONTROLS + .add(b' ') + .add(b'/') + .add(b'?') + .add(b'#') + .add(b'%') + .add(b'&') + .add(b'+'); + +/// Percent-encode a value for safe interpolation into one path segment. +fn seg(s: &str) -> String { + utf8_percent_encode(s, PATH_SEGMENT).to_string() +} + pub struct Client { http: reqwest::Client, url: String, @@ -73,6 +92,7 @@ impl Client { /// `GET /api/v1/admin/apps/{id_or_slug}` — slug or UUID accepted. pub async fn apps_get(&self, ident: &str) -> Result { + let ident = seg(ident); let resp = self .request(Method::GET, &format!("/api/v1/admin/apps/{ident}")) .send() @@ -119,6 +139,7 @@ impl Client { /// Server requires `AppAdmin` capability; without `force`, returns /// 409 if the app still has scripts. pub async fn apps_delete(&self, ident: &str, force: bool) -> Result<()> { + let ident = seg(ident); let path = if force { format!("/api/v1/admin/apps/{ident}?force=true") } else { @@ -131,6 +152,7 @@ impl Client { /// `DELETE /api/v1/admin/scripts/{id}` — requires `AppAdmin` on the /// owning app (stricter than the edit endpoints, by design). pub async fn scripts_delete(&self, id: &str) -> Result<()> { + let id = seg(id); let resp = self .request(Method::DELETE, &format!("/api/v1/admin/scripts/{id}")) .send() @@ -151,6 +173,7 @@ impl Client { /// `PUT /api/v1/admin/scripts/{id}` — matches the dashboard, which /// uses PUT despite the field-level update semantics. pub async fn scripts_update_source(&self, id: &str, source: &str) -> Result