From 72850424c11d8061d072edc260d23c53274393da Mon Sep 17 00:00:00 2001 From: echobt <154886644+echobt@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:23:03 +0000 Subject: [PATCH] refactor(server): remove legacy administration surfaces Remove unused server administration routes, role scaffolding, and proxy admin settings while preserving authentication and coding CLI commands. Cover removed routes, legacy claims, and retained network policy. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- Cargo.lock | 1 + docs/reference/app-server.md | 19 +- scripts/readiness/qa.py | 12 +- src/cortex-app-server/src/admin.rs | 697 ------------------------- src/cortex-app-server/src/auth.rs | 149 ------ src/cortex-app-server/src/lib.rs | 4 +- src/cortex-app-server/tests/router.rs | 116 +++- src/cortex-cli/tests/cli_schema.rs | 57 ++ src/cortex-network-proxy/Cargo.toml | 5 +- src/cortex-network-proxy/src/config.rs | 44 +- 10 files changed, 217 insertions(+), 887 deletions(-) delete mode 100644 src/cortex-app-server/src/admin.rs diff --git a/Cargo.lock b/Cargo.lock index 281fe929..163bc44d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1428,6 +1428,7 @@ name = "cortex-network-proxy" version = "0.1.7" dependencies = [ "serde", + "serde_json", "thiserror 2.0.20", "tokio", ] diff --git a/docs/reference/app-server.md b/docs/reference/app-server.md index f6ecea24..0e1a2543 100644 --- a/docs/reference/app-server.md +++ b/docs/reference/app-server.md @@ -41,12 +41,23 @@ The supported contract covers: | GET, DELETE | `/sessions/{id}` | Read/delete an in-memory session | | POST, GET | `/sessions/{id}/messages` | Store/list messages, no model inference | -Other development endpoints, including files, terminals, admin, SSE, and +Other development endpoints, including files, terminals, SSE, and WebSockets, are not yet part of this stable schema. Authentication applies to them too. A configured server API key is an operator credential, not a -multi-tenant sandbox. JWT admin routes require the `admin` role; ordinary -authenticated routes operate on the server's workspace. Do not host mutually -untrusted tenants in one process. +multi-tenant sandbox. Authenticated routes operate on the server's workspace. +Do not host mutually untrusted tenants in one process. + +The legacy `/api/v1/admin/*` API has been removed, including global statistics, +bulk session operations, CSV exports, and share administration. These paths +return 404 after authentication; unauthenticated requests still fail authentication. +Legacy JWT role/profile claims are ignored and grant no additional capabilities. +Use the existing CLI session, export, and stats commands for your own local data. +Session sharing and automatic expired-share cleanup remain available. + +The unused network-proxy `admin_url` and +`dangerously_allow_non_loopback_admin` settings have also been removed. Older +configuration files may still contain these unknown fields, but they have no +effect. Proxy domain/IP filtering, network modes, and sandbox protections remain. Send `Authorization: ApiKey ` or `Authorization: Bearer `. JWTs require issuer `Cortex` and audience `cortex-api`. diff --git a/scripts/readiness/qa.py b/scripts/readiness/qa.py index 0b67023f..d98d6ea6 100644 --- a/scripts/readiness/qa.py +++ b/scripts/readiness/qa.py @@ -84,8 +84,16 @@ def call(method, path, body=None, authenticated=True, headers=None): pass check(time.monotonic() < deadline, "Local server readiness timed out") time.sleep(.1) - for path in ["/sessions", "/metrics", "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/admin/stats", "/ws", "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/health/sessions"]: + for path in ["/sessions", "/metrics", "/ws", "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/health/sessions"]: check(call("GET", path, authenticated=False)[0] == 401, "Authentication boundary failed") + for method, path in [ + ("GET", "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/admin/stats"), ("GET", "/admin/stats/sessions"), + ("GET", "/admin/stats/usage"), ("GET", "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/admin/sessions"), + ("POST", "/admin/sessions/bulk"), ("GET", "/admin/sessions/export"), + ("GET", "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/admin/shares"), ("POST", "/admin/shares/cleanup"), + ]: + body = {"session_ids": [], "action": "delete"} if method == "POST" else None + check(call(method, path, body)[0] == 404, "Removed administration endpoint remains available") check(call("GET", "/sessions", headers={"Authorization": "ApiKey invalid-fixture"})[0] == 401, "Invalid key was accepted") status, headers, session = call("POST", "/sessions", {"model": "local-qa"}) check(status == 200, "Session creation failed") @@ -132,7 +140,7 @@ def call(method, path, body=None, authenticated=True, headers=None): "server.local_readiness", "server.authentication", "server.session_crud", "server.message_storage", "server.correlation", "server.metrics", "dast.body_limit", "dast.cors", "dast.workspace_traversal", "dast.symlink_escape", - "server.file_crud", "dast.file_mutations", + "server.file_crud", "dast.file_mutations", "dast.removed_admin_routes", ] def run(bin_dir): diff --git a/src/cortex-app-server/src/admin.rs b/src/cortex-app-server/src/admin.rs deleted file mode 100644 index 76593777..00000000 --- a/src/cortex-app-server/src/admin.rs +++ /dev/null @@ -1,697 +0,0 @@ -//! Admin API for cortex-app-server. -//! -//! Provides administrative endpoints for managing sessions, viewing statistics, -//! and performing bulk operations. - -use std::sync::Arc; - -use axum::{ - Json, Router, - extract::{Query, State}, - routing::{get, post}, -}; -use chrono::Timelike; -use serde::{Deserialize, Serialize}; - -use crate::error::{AppError, AppResult}; -use crate::state::AppState; - -/// Create admin routes. -pub fn routes() -> Router> { - Router::new() - // Statistics - .route("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/admin/stats", get(get_stats)) - .route("/admin/stats/sessions", get(get_session_stats)) - .route("/admin/stats/usage", get(get_usage_stats)) - // Sessions management - .route("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/admin/sessions", get(list_all_sessions)) - .route("/admin/sessions/bulk", post(bulk_action)) - .route("/admin/sessions/export", get(export_sessions_csv)) - // Shares management - .route("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/admin/shares", get(list_all_shares)) - .route("/admin/shares/cleanup", post(cleanup_expired_shares)) -} - -// ============================================================================ -// Statistics Types -// ============================================================================ - -/// Overall server statistics. -#[derive(Debug, Serialize)] -pub struct ServerStats { - /// Server uptime in seconds. - pub uptime_seconds: u64, - /// Total number of stored sessions. - pub total_sessions: usize, - /// Number of active WebSocket connections. - pub active_connections: usize, - /// Number of active shares. - pub active_shares: usize, - /// Total number of messages across all sessions. - pub total_messages: usize, - /// Sessions created today. - pub sessions_today: usize, - /// Average messages per session. - pub avg_messages_per_session: f64, -} - -/// Session statistics. -#[derive(Debug, Serialize)] -pub struct SessionStats { - /// Total sessions. - pub total: usize, - /// Sessions by status (if applicable). - pub by_status: serde_json::Value, - /// Sessions created in the last 24 hours. - pub last_24h: usize, - /// Sessions created in the last 7 days. - pub last_7d: usize, - /// Sessions created in the last 30 days. - pub last_30d: usize, - /// Top models used. - pub top_models: Vec, -} - -/// Model usage statistics. -#[derive(Debug, Serialize)] -pub struct ModelUsage { - /// Model name. - pub model: String, - /// Number of sessions using this model. - pub session_count: usize, - /// Percentage of total sessions. - pub percentage: f64, -} - -/// Usage statistics over time. -#[derive(Debug, Serialize)] -pub struct UsageStats { - /// Daily session counts for the last 30 days. - pub daily_sessions: Vec, - /// Daily message counts for the last 30 days. - pub daily_messages: Vec, - /// Peak usage hour (0-23). - pub peak_hour: u8, - /// Average sessions per day. - pub avg_sessions_per_day: f64, -} - -/// Daily count entry. -#[derive(Debug, Serialize)] -pub struct DailyCount { - /// Date (YYYY-MM-DD). - pub date: String, - /// Count for the day. - pub count: usize, -} - -// ============================================================================ -// Session Management Types -// ============================================================================ - -/// Query parameters for listing sessions. -#[derive(Debug, Deserialize)] -pub struct ListSessionsQuery { - /// Search term for filtering. - #[serde(default)] - pub search: Option, - /// Filter by model. - #[serde(default)] - pub model: Option, - /// Filter by date range start (ISO 8601). - #[serde(default)] - pub from: Option, - /// Filter by date range end (ISO 8601). - #[serde(default)] - pub to: Option, - /// Page number (1-indexed). - #[serde(default = "default_page")] - pub page: usize, - /// Items per page. - #[serde(default = "default_limit")] - pub limit: usize, - /// Sort field. - #[serde(default = "default_sort")] - pub sort: String, - /// Sort order (asc/desc). - #[serde(default = "default_order")] - pub order: String, -} - -fn default_page() -> usize { - 1 -} -fn default_limit() -> usize { - 50 -} -fn default_sort() -> String { - "updated_at".to_string() -} -fn default_order() -> String { - "desc".to_string() -} - -/// Paginated session list response. -#[derive(Debug, Serialize)] -pub struct SessionListResponse { - /// Sessions in the current page. - pub sessions: Vec, - /// Total number of sessions matching the query. - pub total: usize, - /// Current page. - pub page: usize, - /// Total number of pages. - pub total_pages: usize, -} - -/// Session information for admin view. -#[derive(Debug, Serialize)] -pub struct AdminSessionInfo { - /// Session ID. - pub id: String, - /// Session title. - pub title: Option, - /// Model used. - pub model: String, - /// Working directory. - pub cwd: String, - /// Number of messages. - pub message_count: usize, - /// Creation timestamp (ISO 8601). - pub created_at: String, - /// Last update timestamp (ISO 8601). - pub updated_at: String, -} - -/// Bulk action request. -#[derive(Debug, Deserialize)] -pub struct BulkActionRequest { - /// Session IDs to operate on. - pub session_ids: Vec, - /// Action to perform. - pub action: BulkAction, -} - -/// Bulk action types. -#[derive(Debug, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum BulkAction { - /// Delete the sessions. - Delete, - /// Export the sessions. - Export, -} - -/// Bulk action response. -#[derive(Debug, Serialize)] -pub struct BulkActionResponse { - /// Whether the action succeeded. - pub success: bool, - /// Number of sessions affected. - pub affected: usize, - /// Errors encountered (if any). - #[serde(skip_serializing_if = "Vec::is_empty")] - pub errors: Vec, -} - -// ============================================================================ -// Share Management Types -// ============================================================================ - -/// Query parameters for listing shares. -#[derive(Debug, Deserialize)] -pub struct ListSharesQuery { - /// Page number. - #[serde(default = "default_page")] - pub page: usize, - /// Items per page. - #[serde(default = "default_limit")] - pub limit: usize, - /// Include expired shares. - #[serde(default)] - pub include_expired: bool, -} - -/// Share list response. -#[derive(Debug, Serialize)] -pub struct ShareListResponse { - /// Shares in the current page. - pub shares: Vec, - /// Total number of shares. - pub total: usize, - /// Current page. - pub page: usize, - /// Total pages. - pub total_pages: usize, -} - -/// Share information for admin view. -#[derive(Debug, Serialize)] -pub struct AdminShareInfo { - /// Share token. - pub token: String, - /// Session ID. - pub session_id: String, - /// Session title. - pub title: Option, - /// View count. - pub view_count: u32, - /// Max views (if set). - pub max_views: Option, - /// Whether the share is expired. - pub expired: bool, - /// Creation timestamp. - pub created_at: String, - /// Expiration timestamp. - pub expires_at: String, -} - -/// Cleanup response. -#[derive(Debug, Serialize)] -pub struct CleanupResponse { - /// Number of items cleaned up. - pub cleaned: usize, -} - -// ============================================================================ -// Handlers -// ============================================================================ - -/// Get overall server statistics. -async fn get_stats(State(state): State>) -> AppResult> { - let sessions = state - .cli_sessions - .storage() - .list_sessions() - .unwrap_or_default(); - - let total_sessions = sessions.len(); - let active_shares = state.share_manager.count().await; - - // Count messages across all sessions - let mut total_messages = 0; - for session in &sessions { - if let Ok(history) = state.cli_sessions.storage().read_history(&session.id) { - total_messages += history.len(); - } - } - - // Count sessions created today - let today_start = chrono::Utc::now() - .date_naive() - .and_hms_opt(0, 0, 0) - .unwrap() - .and_utc() - .timestamp(); - let sessions_today = sessions - .iter() - .filter(|s| s.created_at >= today_start) - .count(); - - let avg_messages = if total_sessions > 0 { - total_messages as f64 / total_sessions as f64 - } else { - 0.0 - }; - - Ok(Json(ServerStats { - uptime_seconds: state.uptime().as_secs(), - total_sessions, - active_connections: state.cli_sessions.count().await, - active_shares, - total_messages, - sessions_today, - avg_messages_per_session: (avg_messages * 100.0).round() / 100.0, - })) -} - -/// Get session statistics. -async fn get_session_stats(State(state): State>) -> AppResult> { - let sessions = state - .cli_sessions - .storage() - .list_sessions() - .unwrap_or_default(); - - let total = sessions.len(); - let now = chrono::Utc::now().timestamp(); - let day_secs = 24 * 60 * 60; - - let last_24h = sessions - .iter() - .filter(|s| now - s.created_at < day_secs) - .count(); - let last_7d = sessions - .iter() - .filter(|s| now - s.created_at < 7 * day_secs) - .count(); - let last_30d = sessions - .iter() - .filter(|s| now - s.created_at < 30 * day_secs) - .count(); - - // Count model usage - let mut model_counts: std::collections::HashMap = - std::collections::HashMap::new(); - for session in &sessions { - *model_counts.entry(session.model.clone()).or_insert(0) += 1; - } - - let mut top_models: Vec = model_counts - .into_iter() - .map(|(model, count)| ModelUsage { - model, - session_count: count, - percentage: if total > 0 { - (count as f64 / total as f64 * 100.0 * 10.0).round() / 10.0 - } else { - 0.0 - }, - }) - .collect(); - top_models.sort_by(|a, b| b.session_count.cmp(&a.session_count)); - top_models.truncate(10); - - Ok(Json(SessionStats { - total, - by_status: serde_json::json!({ - "active": total, // All stored sessions are considered "active" - }), - last_24h, - last_7d, - last_30d, - top_models, - })) -} - -/// Get usage statistics over time. -async fn get_usage_stats(State(state): State>) -> AppResult> { - let sessions = state - .cli_sessions - .storage() - .list_sessions() - .unwrap_or_default(); - - // Build daily counts for the last 30 days - let mut daily_sessions: std::collections::HashMap = - std::collections::HashMap::new(); - let mut daily_messages: std::collections::HashMap = - std::collections::HashMap::new(); - let mut hour_counts: [usize; 24] = [0; 24]; - - let now = chrono::Utc::now(); - let thirty_days_ago = (now - chrono::Duration::days(30)).timestamp(); - - for session in &sessions { - if session.created_at >= thirty_days_ago { - // Format date - if let Some(dt) = chrono::DateTime::from_timestamp(session.created_at, 0) { - let date = dt.format("%Y-%m-%d").to_string(); - *daily_sessions.entry(date.clone()).or_insert(0) += 1; - - // Count hour for peak detection - let hour = dt.hour() as usize; - hour_counts[hour] += 1; - - // Count messages - if let Ok(history) = state.cli_sessions.storage().read_history(&session.id) { - *daily_messages.entry(date).or_insert(0) += history.len(); - } - } - } - } - - // Convert to sorted vectors - let mut daily_sessions_vec: Vec = daily_sessions - .into_iter() - .map(|(date, count)| DailyCount { date, count }) - .collect(); - daily_sessions_vec.sort_by(|a, b| a.date.cmp(&b.date)); - - let mut daily_messages_vec: Vec = daily_messages - .into_iter() - .map(|(date, count)| DailyCount { date, count }) - .collect(); - daily_messages_vec.sort_by(|a, b| a.date.cmp(&b.date)); - - // Find peak hour - let peak_hour = hour_counts - .iter() - .enumerate() - .max_by_key(|(_, count)| *count) - .map(|(hour, _)| hour as u8) - .unwrap_or(0); - - // Average sessions per day - let total_days = daily_sessions_vec.len().max(1); - let total_session_count: usize = daily_sessions_vec.iter().map(|d| d.count).sum(); - let avg_sessions_per_day = - (total_session_count as f64 / total_days as f64 * 10.0).round() / 10.0; - - Ok(Json(UsageStats { - daily_sessions: daily_sessions_vec, - daily_messages: daily_messages_vec, - peak_hour, - avg_sessions_per_day, - })) -} - -/// List all sessions with filtering and pagination. -async fn list_all_sessions( - State(state): State>, - Query(query): Query, -) -> AppResult> { - let mut sessions = state - .cli_sessions - .storage() - .list_sessions() - .unwrap_or_default(); - - // Apply filters - if let Some(search) = &query.search { - let search_lower = search.to_lowercase(); - sessions.retain(|s| { - s.id.to_lowercase().contains(&search_lower) - || s.title - .as_ref() - .map(|t| t.to_lowercase().contains(&search_lower)) - .unwrap_or(false) - }); - } - - if let Some(model) = &query.model { - sessions.retain(|s| &s.model == model); - } - - if let Some(from) = &query.from - && let Ok(from_dt) = chrono::DateTime::parse_from_rfc3339(from) - { - let from_ts = from_dt.timestamp(); - sessions.retain(|s| s.created_at >= from_ts); - } - - if let Some(to) = &query.to - && let Ok(to_dt) = chrono::DateTime::parse_from_rfc3339(to) - { - let to_ts = to_dt.timestamp(); - sessions.retain(|s| s.created_at <= to_ts); - } - - // Sort - match (query.sort.as_str(), query.order.as_str()) { - ("created_at", "asc") => sessions.sort_by_key(|s| s.created_at), - ("created_at", "desc") => sessions.sort_by_key(|s| std::cmp::Reverse(s.created_at)), - ("updated_at", "asc") => sessions.sort_by_key(|s| s.updated_at), - (_, _) => sessions.sort_by_key(|s| std::cmp::Reverse(s.updated_at)), - } - - let total = sessions.len(); - let total_pages = total.div_ceil(query.limit); - - // Paginate - let start = (query.page - 1) * query.limit; - let sessions: Vec = sessions - .into_iter() - .skip(start) - .take(query.limit) - .map(|s| { - let message_count = state - .cli_sessions - .storage() - .read_history(&s.id) - .map(|h| h.len()) - .unwrap_or(0); - - AdminSessionInfo { - id: s.id, - title: s.title, - model: s.model, - cwd: s.cwd, - message_count, - created_at: chrono::DateTime::from_timestamp(s.created_at, 0) - .map(|dt| dt.to_rfc3339()) - .unwrap_or_default(), - updated_at: chrono::DateTime::from_timestamp(s.updated_at, 0) - .map(|dt| dt.to_rfc3339()) - .unwrap_or_default(), - } - }) - .collect(); - - Ok(Json(SessionListResponse { - sessions, - total, - page: query.page, - total_pages, - })) -} - -/// Perform bulk actions on sessions. -async fn bulk_action( - State(state): State>, - Json(req): Json, -) -> AppResult> { - let mut affected = 0; - let mut errors = Vec::new(); - - match req.action { - BulkAction::Delete => { - for session_id in &req.session_ids { - match state.cli_sessions.storage().delete_session(session_id) { - Ok(_) => affected += 1, - Err(e) => errors.push(format!("{}: {}", session_id, e)), - } - } - } - BulkAction::Export => { - // Export is handled by the export endpoint - return Err(AppError::BadRequest( - "Use /admin/sessions/export endpoint for exporting".to_string(), - )); - } - } - - Ok(Json(BulkActionResponse { - success: errors.is_empty(), - affected, - errors, - })) -} - -/// Export sessions as CSV. -async fn export_sessions_csv( - State(state): State>, -) -> AppResult { - use axum::http::header; - use axum::response::IntoResponse; - - let sessions = state - .cli_sessions - .storage() - .list_sessions() - .unwrap_or_default(); - - let mut csv = String::new(); - csv.push_str("id,title,model,cwd,message_count,created_at,updated_at\n"); - - for session in sessions { - let message_count = state - .cli_sessions - .storage() - .read_history(&session.id) - .map(|h| h.len()) - .unwrap_or(0); - - let title = session - .title - .as_ref() - .map(|t| format!("\"{}\"", t.replace('"', "\"\""))) - .unwrap_or_default(); - - let created_at = chrono::DateTime::from_timestamp(session.created_at, 0) - .map(|dt| dt.to_rfc3339()) - .unwrap_or_default(); - let updated_at = chrono::DateTime::from_timestamp(session.updated_at, 0) - .map(|dt| dt.to_rfc3339()) - .unwrap_or_default(); - - csv.push_str(&format!( - "{},{},{},\"{}\",{},{},{}\n", - session.id, - title, - session.model, - session.cwd.replace('"', "\"\""), - message_count, - created_at, - updated_at, - )); - } - - Ok(( - [ - (header::CONTENT_TYPE, "text/csv"), - ( - header::CONTENT_DISPOSITION, - "attachment; filename=sessions.csv", - ), - ], - csv, - ) - .into_response()) -} - -/// List all shares. -async fn list_all_shares( - State(_state): State>, - Query(query): Query, -) -> AppResult> { - // Get all shares from the manager (assuming it's a user, we get their shares) - // For admin, we'd need a method to get all shares regardless of user - // For now, return what's available - - let _now = chrono::Utc::now().timestamp(); - - // Note: In a real implementation, ShareManager would have a list_all method - // For now, we use what's available - let shares: Vec = Vec::new(); // Placeholder - - let total = shares.len(); - let total_pages = (total + query.limit - 1).max(1) / query.limit.max(1); - - Ok(Json(ShareListResponse { - shares, - total, - page: query.page, - total_pages, - })) -} - -/// Cleanup expired shares. -async fn cleanup_expired_shares( - State(state): State>, -) -> AppResult> { - let cleaned = state.share_manager.cleanup_expired().await; - Ok(Json(CleanupResponse { cleaned })) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_bulk_action_deserialization() { - let json = r#"{"session_ids": ["a", "b"], "action": "delete"}"#; - let req: BulkActionRequest = serde_json::from_str(json).unwrap(); - assert_eq!(req.session_ids.len(), 2); - assert!(matches!(req.action, BulkAction::Delete)); - } - - #[test] - fn test_list_sessions_query_defaults() { - let query: ListSessionsQuery = serde_json::from_str("{}").unwrap(); - assert_eq!(query.page, 1); - assert_eq!(query.limit, 50); - assert_eq!(query.sort, "updated_at"); - assert_eq!(query.order, "desc"); - } -} diff --git a/src/cortex-app-server/src/auth.rs b/src/cortex-app-server/src/auth.rs index 8dc1ad30..f591b579 100644 --- a/src/cortex-app-server/src/auth.rs +++ b/src/cortex-app-server/src/auth.rs @@ -32,12 +32,6 @@ pub struct Claims { /// Audience. #[serde(default)] pub aud: Vec, - /// User roles. - #[serde(default)] - pub roles: Vec, - /// Additional metadata. - #[serde(default)] - pub metadata: HashMap, } impl Claims { @@ -54,23 +48,9 @@ impl Claims { iat: now, iss: "Cortex".to_string(), aud: vec!["cortex-api".to_string()], - roles: vec![], - metadata: HashMap::new(), } } - /// Add a role to the claims. - pub fn with_role(mut self, role: impl Into) -> Self { - self.roles.push(role.into()); - self - } - - /// Add metadata to the claims. - pub fn with_metadata(mut self, key: impl Into, value: serde_json::Value) -> Self { - self.metadata.insert(key.into(), value); - self - } - /// Check if the token is expired. pub fn is_expired(&self) -> bool { let now = SystemTime::now() @@ -79,16 +59,6 @@ impl Claims { .as_secs(); self.exp < now } - - /// Check if the user has a specific role. - pub fn has_role(&self, role: &str) -> bool { - self.roles.iter().any(|r| r == role) - } - - /// Check if the user has any of the specified roles. - pub fn has_any_role(&self, roles: &[&str]) -> bool { - roles.iter().any(|r| self.has_role(r)) - } } /// Authentication service. @@ -350,15 +320,6 @@ pub async fn auth_middleware( } }; - let is_admin = match &auth_result { - AuthResult::ApiKey(_) => true, - AuthResult::Jwt(claims) => claims.has_role("admin"), - AuthResult::Anonymous => false, - }; - if path.starts_with("/api/v1/admin/") && !is_admin { - return Err(StatusCode::FORBIDDEN); - } - // Add auth result to request extensions request.extensions_mut().insert(auth_result); @@ -378,89 +339,6 @@ fn constant_time_compare(a: &[u8], b: &[u8]) -> bool { result == 0 } -/// Role-based access control. -pub struct RoleGuard { - required_roles: Vec, - require_all: bool, -} - -impl RoleGuard { - /// Create a new role guard requiring any of the specified roles. - pub fn any_of(roles: &[&str]) -> Self { - Self { - required_roles: roles.iter().map(std::string::ToString::to_string).collect(), - require_all: false, - } - } - - /// Create a new role guard requiring all of the specified roles. - pub fn all_of(roles: &[&str]) -> Self { - Self { - required_roles: roles.iter().map(std::string::ToString::to_string).collect(), - require_all: true, - } - } - - /// Check if the claims satisfy the role requirements. - pub fn check(&self, claims: &Claims) -> bool { - if self.require_all { - self.required_roles.iter().all(|r| claims.has_role(r)) - } else { - self.required_roles.iter().any(|r| claims.has_role(r)) - } - } -} - -/// User information extracted from authentication. -#[derive(Debug, Clone, Serialize)] -pub struct User { - /// User ID. - pub id: String, - /// User email (if available). - pub email: Option, - /// User name (if available). - pub name: Option, - /// User roles. - pub roles: Vec, - /// Authentication method. - pub auth_method: String, -} - -impl From for User { - fn from(claims: Claims) -> Self { - Self { - id: claims.sub, - email: claims - .metadata - .get("email") - .and_then(|v| v.as_str().map(String::from)), - name: claims - .metadata - .get("name") - .and_then(|v| v.as_str().map(String::from)), - roles: claims.roles, - auth_method: "jwt".to_string(), - } - } -} - -/// API key information. -#[derive(Debug, Clone, Serialize)] -pub struct ApiKeyInfo { - /// Key ID (hash of the key). - pub id: String, - /// Key name/label. - pub name: Option, - /// Creation time. - pub created_at: u64, - /// Last used time. - pub last_used: Option, - /// Scopes/permissions. - pub scopes: Vec, - /// Rate limit tier. - pub rate_limit_tier: Option, -} - #[cfg(test)] mod tests { use super::*; @@ -472,17 +350,6 @@ mod tests { assert!(!claims.is_expired()); } - #[test] - fn test_claims_roles() { - let claims = Claims::new("user123", 3600) - .with_role("admin") - .with_role("user"); - - assert!(claims.has_role("admin")); - assert!(claims.has_role("user")); - assert!(!claims.has_role("superadmin")); - } - #[test] fn test_parse_bearer_token() { assert_eq!(parse_bearer_token("Bearer abc123"), Some("abc123")); @@ -496,20 +363,4 @@ mod tests { assert_eq!(parse_api_key("apikey abc123"), Some("abc123")); assert_eq!(parse_api_key("Bearer abc123"), None); } - - #[test] - fn test_role_guard() { - let claims = Claims::new("user123", 3600) - .with_role("admin") - .with_role("user"); - - let any_guard = RoleGuard::any_of(&["admin", "superadmin"]); - assert!(any_guard.check(&claims)); - - let all_guard = RoleGuard::all_of(&["admin", "user"]); - assert!(all_guard.check(&claims)); - - let missing_guard = RoleGuard::all_of(&["admin", "superadmin"]); - assert!(!missing_guard.check(&claims)); - } } diff --git a/src/cortex-app-server/src/lib.rs b/src/cortex-app-server/src/lib.rs index ef42c2cd..ff22c097 100644 --- a/src/cortex-app-server/src/lib.rs +++ b/src/cortex-app-server/src/lib.rs @@ -14,7 +14,6 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] -pub mod admin; pub mod api; pub mod auth; pub mod config; @@ -151,8 +150,7 @@ pub fn create_router_with_state(state: Arc) -> Router { let api_routes = api::routes() .merge(websocket::routes()) .merge(streaming::routes()) - .merge(share::routes()) - .merge(admin::routes()); + .merge(share::routes()); Router::new() .nest("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/api/v1", api_routes) diff --git a/src/cortex-app-server/tests/router.rs b/src/cortex-app-server/tests/router.rs index 2e0dfddd..62118087 100644 --- a/src/cortex-app-server/tests/router.rs +++ b/src/cortex-app-server/tests/router.rs @@ -110,7 +110,6 @@ async fn test_auth_is_enforced_for_rest_websocket_and_health_prefix() { "/api/v1/metrics", "/api/v1/ws", "/api/v1/health/sessions", - "/api/v1/admin/stats", ] { let response = router .clone() @@ -257,7 +256,7 @@ async fn test_cors_denies_unknown_origins_and_allows_configured_origin() { } #[tokio::test] -async fn test_jwt_validation_and_admin_role_are_enforced() { +async fn test_jwt_authentication_still_validates_identity_issuer_audience_and_expiry() { let mut config = ServerConfig::default(); config.auth.jwt_secret = Some(uuid::Uuid::new_v4().to_string()); let service = AuthService::new(config.auth.clone()); @@ -265,29 +264,112 @@ async fn test_jwt_validation_and_admin_role_are_enforced() { assert_eq!(service.validate_token(&token).unwrap().sub, "fixture-user"); let (router, _, _) = configured(config.clone()).await; let req = Request::builder() - .uri("/api/v1/admin/stats") + .uri("/api/v1/sessions") .header("authorization", format!("Bearer {token}")) .body(Body::empty()) .unwrap(); assert_eq!( router.clone().oneshot(req).await.unwrap().status(), - StatusCode::FORBIDDEN + StatusCode::OK ); - let mut claims = Claims::new("fixture-user", 3600).with_role("admin"); - claims.iss = "wrong-issuer".into(); let key = jsonwebtoken::EncodingKey::from_secret(config.auth.jwt_secret.as_ref().unwrap().as_bytes()); - let wrong = jsonwebtoken::encode(&jsonwebtoken::Header::default(), &claims, &key).unwrap(); - assert!(service.validate_token(&wrong).is_err()); - let req = Request::builder() - .uri("/api/v1/sessions") - .header("authorization", format!("Bearer {wrong}")) - .body(Body::empty()) - .unwrap(); - assert_eq!( - router.oneshot(req).await.unwrap().status(), - StatusCode::UNAUTHORIZED - ); + for invalid in ["issuer", "audience", "expiry", "signature"] { + let mut claims = Claims::new("fixture-user", 3600); + match invalid { + "issuer" => claims.iss = "wrong-issuer".into(), + "audience" => claims.aud = vec!["wrong-audience".into()], + "expiry" => claims.exp = 1, + _ => {} + } + let wrong_key = jsonwebtoken::EncodingKey::from_secret(uuid::Uuid::new_v4().as_bytes()); + let signing_key = if invalid == "signature" { + &wrong_key + } else { + &key + }; + let token = + jsonwebtoken::encode(&jsonwebtoken::Header::default(), &claims, signing_key).unwrap(); + assert!(service.validate_token(&token).is_err(), "{invalid}"); + let req = Request::builder() + .uri("/api/v1/sessions") + .header("authorization", format!("Bearer {token}")) + .body(Body::empty()) + .unwrap(); + assert_eq!( + router.clone().oneshot(req).await.unwrap().status(), + StatusCode::UNAUTHORIZED, + "{invalid}" + ); + } +} + +#[tokio::test] +async fn test_removed_admin_routes_are_unavailable_even_with_legacy_privileges() { + let secret = uuid::Uuid::new_v4().to_string(); + let api_key = uuid::Uuid::new_v4().to_string(); + let mut claims = serde_json::to_value(Claims::new("fixture-user", 3600)).unwrap(); + claims["roles"] = json!(["admin"]); + claims["metadata"] = json!({"name": "legacy profile"}); + let token = jsonwebtoken::encode( + &jsonwebtoken::Header::default(), + &claims, + &jsonwebtoken::EncodingKey::from_secret(secret.as_bytes()), + ) + .unwrap(); + let headers = [ + None, + Some(format!("ApiKey {api_key}")), + Some(format!("Bearer {token}")), + ]; + for authenticated in [false, true] { + let mut config = ServerConfig::default(); + config.auth.enabled = authenticated; + config.auth.api_keys = vec![api_key.clone()]; + config.auth.jwt_secret = Some(secret.clone()); + config.rate_limit.burst_size = 100; + let service = AuthService::new(config.auth.clone()); + let decoded = service.validate_token(&token).unwrap(); + assert_eq!(decoded.sub, "fixture-user"); + let decoded = serde_json::to_value(decoded).unwrap(); + assert!(decoded.get("roles").is_none()); + assert!(decoded.get("metadata").is_none()); + let state = Arc::new(AppState::new(config).await.unwrap()); + let router = create_router_with_state(state); + for (method, path) in [ + ("GET", "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/admin/stats"), + ("GET", "/admin/stats/sessions"), + ("GET", "/admin/stats/usage"), + ("GET", "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/admin/sessions"), + ("POST", "/admin/sessions/bulk"), + ("GET", "/admin/sessions/export"), + ("GET", "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/admin/shares"), + ("POST", "/admin/shares/cleanup"), + ] { + for header in &headers { + let mut req = request( + method, + &format!("/api/v1{path}"), + None, + Some(json!({"session_ids": [], "action": "delete"})), + ); + if let Some(header) = header { + req.headers_mut() + .insert("authorization", header.parse().unwrap()); + } + let expected = if authenticated && header.is_none() { + StatusCode::UNAUTHORIZED + } else { + StatusCode::NOT_FOUND + }; + assert_eq!( + router.clone().oneshot(req).await.unwrap().status(), + expected, + "{method} {path}" + ); + } + } + } } #[tokio::test] diff --git a/src/cortex-cli/tests/cli_schema.rs b/src/cortex-cli/tests/cli_schema.rs index f45ff6f2..6be9b1bd 100644 --- a/src/cortex-cli/tests/cli_schema.rs +++ b/src/cortex-cli/tests/cli_schema.rs @@ -6,6 +6,63 @@ fn test_entire_command_tree_is_valid() { Cli::command().debug_assert(); } +#[test] +fn test_development_commands_remain_available_without_admin_commands() { + let command = Cli::command(); + for name in [ + "run", + "exec", + "resume", + "sessions", + "export", + "import", + "delete", + "login", + "logout", + "whoami", + "agent", + "mcp", + "mcp-server", + "acp", + "config", + "models", + "features", + "init", + "github", + "pr", + "scrape", + "stats", + "completion", + "upgrade", + "uninstall", + "compact", + "cache", + "logs", + "feedback", + "lock", + "alias", + "plugin", + "debug", + "shell", + "dag", + "servers", + "history", + "workspace", + "sandbox", + "serve", + ] { + assert!(command.find_subcommand(name).is_some(), "missing {name}"); + } + fn check_no_admin(command: &clap::Command) { + assert_ne!(command.get_name(), "admin"); + assert!(!command.get_all_aliases().any(|alias| alias == "admin")); + for child in command.get_subcommands() { + check_no_admin(child); + } + } + check_no_admin(&command); +} + #[test] fn test_plugin_version_and_global_verbosity_are_distinct() { let matches = Cli::command() diff --git a/src/cortex-network-proxy/Cargo.toml b/src/cortex-network-proxy/Cargo.toml index 05316579..5118f08c 100644 --- a/src/cortex-network-proxy/Cargo.toml +++ b/src/cortex-network-proxy/Cargo.toml @@ -21,4 +21,7 @@ tokio = { workspace = true } serde = { workspace = true } # Error handling -thiserror = { workspace = true } \ No newline at end of file +thiserror = { workspace = true } + +[dev-dependencies] +serde_json = { workspace = true } \ No newline at end of file diff --git a/src/cortex-network-proxy/src/config.rs b/src/cortex-network-proxy/src/config.rs index fb4c1ea1..43b22fb3 100644 --- a/src/cortex-network-proxy/src/config.rs +++ b/src/cortex-network-proxy/src/config.rs @@ -93,17 +93,9 @@ pub struct NetworkProxyConfig { #[serde(default)] pub proxy_url: Option, - /// Admin interface URL. - #[serde(default)] - pub admin_url: Option, - /// Allow non-loopback proxy address. #[serde(default)] pub dangerously_allow_non_loopback_proxy: bool, - - /// Allow non-loopback admin address. - #[serde(default)] - pub dangerously_allow_non_loopback_admin: bool, } fn default_enabled() -> bool { @@ -214,12 +206,6 @@ impl NetworkProxyConfigBuilder { self } - /// Set admin URL. - pub fn admin_url(mut self, url: impl Into) -> Self { - self.config.admin_url = Some(url.into()); - self - } - /// Build the config. pub fn build(self) -> NetworkProxyConfig { self.config @@ -255,4 +241,34 @@ mod tests { assert_eq!(config.allowed_domains.len(), 2); assert_eq!(config.denied_domains.len(), 1); } + + #[test] + fn test_legacy_admin_settings_are_ignored_without_weakening_network_policy() { + let config: NetworkProxyConfig = serde_json::from_value(serde_json::json!({ + "enabled": true, + "mode": "limited", + "allowed_domains": ["api.cortex.foundation"], + "denied_domains": ["blocked.example"], + "proxy_url": "http://127.0.0.1:8080", + "admin_url": "http://0.0.0.0:9090", + "dangerously_allow_non_loopback_admin": true + })) + .unwrap(); + assert!(config.enabled); + assert_eq!(config.mode, NetworkMode::Limited); + assert!(config.mode.allows_method("GET")); + assert!(!config.mode.allows_method("POST")); + assert!(!config.allow_local_binding); + assert!(!config.dangerously_allow_non_loopback_proxy); + assert_eq!(config.allowed_domains, ["api.cortex.foundation"]); + assert_eq!(config.denied_domains, ["blocked.example"]); + assert_eq!(config.proxy_url.as_deref(), Some("http://127.0.0.1:8080")); + let serialized = serde_json::to_value(config).unwrap(); + assert!(serialized.get("admin_url").is_none()); + assert!( + serialized + .get("dangerously_allow_non_loopback_admin") + .is_none() + ); + } }