diff --git a/crates/ordo-platform/src/proxy.rs b/crates/ordo-platform/src/proxy.rs index 371af419..16cf2679 100644 --- a/crates/ordo-platform/src/proxy.rs +++ b/crates/ordo-platform/src/proxy.rs @@ -178,13 +178,9 @@ fn is_write_method(method: &Method) -> bool { /// 2. If the default env has a canary config and `rand < canary_percentage`, route to the canary env. /// 3. Otherwise route to the default env's server. /// 4. Fall back to the platform's configured `engine_url`. -pub(crate) async fn resolve_engine_url( - state: &AppState, - project_id: &str, - _org_id: &str, -) -> String { +pub(crate) async fn resolve_engine_url(state: &AppState, project_id: &str, org_id: &str) -> String { let Ok(Some(prod_env)) = state.store.get_default_environment(project_id).await else { - return resolve_fallback_engine_url(state).await; + return resolve_fallback_engine_url(state, org_id).await; }; // Canary check @@ -213,16 +209,26 @@ pub(crate) async fn resolve_engine_url( return url; } - resolve_fallback_engine_url(state).await + resolve_fallback_engine_url(state, org_id).await } -async fn resolve_fallback_engine_url(state: &AppState) -> String { +/// Pick a fallback engine when a project's environment has no usable bound server. +/// The candidate pool is restricted to this org's own servers plus deliberately +/// global (unowned) ones — never another org's engine. Otherwise, since the +/// auto-created `production` env has no server and thus commonly hits this path, +/// org A's eval traffic (its tenant id and, for ruleset writes, the full payload) +/// could be shipped to whichever engine registered most recently anywhere — e.g. +/// org B's. The platform's configured `engine_url` remains the final fallback. +async fn resolve_fallback_engine_url(state: &AppState, org_id: &str) -> String { if let Ok(servers) = state.store.list_servers(None).await { if let Some(server) = servers.into_iter().find(|server| { - matches!( - server.status, - crate::models::ServerStatus::Online | crate::models::ServerStatus::Degraded - ) + let same_org_or_global = + server.org_id.as_deref() == Some(org_id) || server.org_id.is_none(); + same_org_or_global + && matches!( + server.status, + crate::models::ServerStatus::Online | crate::models::ServerStatus::Degraded + ) }) { return server.url; } diff --git a/crates/ordo-platform/src/release/executions.rs b/crates/ordo-platform/src/release/executions.rs index cc47c471..91a43876 100644 --- a/crates/ordo-platform/src/release/executions.rs +++ b/crates/ordo-platform/src/release/executions.rs @@ -422,7 +422,7 @@ async fn update_batch_schedule_with_history( pub async fn list_release_execution_events( State(state): State, Extension(claims): Extension, - Path((org_id, project_id, _release_id, execution_id)): Path<(String, String, String, String)>, + Path((org_id, project_id, release_id, execution_id)): Path<(String, String, String, String)>, ) -> ApiResult>> { require_project_permission( &state, @@ -433,9 +433,26 @@ pub async fn list_release_execution_events( ) .await?; + // The permission check only proves the caller may view releases in this + // org/project; it does NOT prove `release_id`/`execution_id` belong here. + // Scope both to the path so a foreign (guessable) id can't be read. + let release = state + .store + .get_release_request(&org_id, &project_id, &release_id) + .await + .map_err(PlatformError::Internal)? + .ok_or_else(|| PlatformError::not_found("Release request not found"))?; + let execution = state + .store + .get_release_execution(&execution_id) + .await + .map_err(PlatformError::Internal)? + .filter(|execution| execution.request_id == release.id) + .ok_or_else(|| PlatformError::not_found("Release execution not found"))?; + let events = state .store - .list_release_execution_events(&execution_id) + .list_release_execution_events(&execution.id) .await .map_err(PlatformError::Internal)?; Ok(Json(events)) @@ -455,9 +472,19 @@ pub async fn get_release_execution_for_request( ) .await?; + // Scope release_id to this org/project — the permission check alone doesn't + // prove the (guessable) id belongs here, so resolve it through the org+project + // scoped getter, which returns None (→ 404) for a foreign release. + let release = state + .store + .get_release_request(&org_id, &project_id, &release_id) + .await + .map_err(PlatformError::Internal)? + .ok_or_else(|| PlatformError::not_found("Release request not found"))?; + let item = state .store - .find_release_execution_by_request_id(&release_id) + .find_release_execution_by_request_id(&release.id) .await .map_err(PlatformError::Internal)?; Ok(Json(item)) diff --git a/crates/ordo-platform/src/server_registry.rs b/crates/ordo-platform/src/server_registry.rs index 8288b4cd..0ead2532 100644 --- a/crates/ordo-platform/src/server_registry.rs +++ b/crates/ordo-platform/src/server_registry.rs @@ -277,10 +277,29 @@ pub async fn delete_connect_token( } } +/// Authorize a read of `server` by `user_id`. `server_id` is derived from the +/// engine URL (`derive_server_id` = sha256 of the normalized URL), so it is +/// guessable — a bare `get_server(id)` with no org check leaks another org's +/// engine (url, version, capabilities) and, for the metrics/health variants, +/// lets any authenticated user trigger a control-plane RPC to it. An org-owned +/// server therefore requires the caller to be a member of that org; a global +/// (unowned) server stays readable by any authenticated user, matching how +/// `delete_server` gates only when an org owns the server. +async fn authorize_server_read( + state: &AppState, + server: &ServerNode, + user_id: &str, +) -> ApiResult<()> { + if let Some(org_id) = &server.org_id { + load_org_and_check_role(state, org_id, user_id, Role::Viewer).await?; + } + Ok(()) +} + /// GET /api/v1/servers/:id pub async fn get_server( State(state): State, - Extension(_claims): Extension, + Extension(claims): Extension, Path(id): Path, ) -> ApiResult> { let server = state @@ -289,6 +308,7 @@ pub async fn get_server( .await .map_err(PlatformError::Internal)? .ok_or_else(|| PlatformError::not_found("Server not found"))?; + authorize_server_read(&state, &server, &claims.sub).await?; Ok(Json(server.into())) } @@ -358,7 +378,7 @@ fn rpc_body_as_text(body: &serde_json::Value) -> String { /// GET /api/v1/servers/:id/metrics — request server Prometheus metrics over NATS pub async fn get_server_metrics( State(state): State, - Extension(_claims): Extension, + Extension(claims): Extension, Path(id): Path, ) -> ApiResult { let server = state @@ -367,6 +387,7 @@ pub async fn get_server_metrics( .await .map_err(PlatformError::Internal)? .ok_or_else(|| PlatformError::not_found("Server not found"))?; + authorize_server_read(&state, &server, &claims.sub).await?; let rpc = request_server_rpc(&state, &server, "metrics").await?; let status = @@ -384,7 +405,7 @@ pub async fn get_server_metrics( /// GET /api/v1/servers/:id/health — request server health over NATS pub async fn get_server_health( State(state): State, - Extension(_claims): Extension, + Extension(claims): Extension, Path(id): Path, ) -> ApiResult> { let server = state @@ -393,6 +414,7 @@ pub async fn get_server_health( .await .map_err(PlatformError::Internal)? .ok_or_else(|| PlatformError::not_found("Server not found"))?; + authorize_server_read(&state, &server, &claims.sub).await?; match request_server_rpc(&state, &server, "health").await { Ok(rpc) => {