fix: resolve 4 remaining "design decision" bugs

- SSH rate limiter: wire SshRateLimiter into SSHServer with IP-based
  rate limiting on new_client connections
- Room startup: cap initial room load at 1000 via limit() to prevent
  resource exhaustion on large instances
- WS token exposure: only include token in URL for cross-origin
  connections; same-origin web clients authenticate via secure cookies
- CSRF: confirmed SameSite::Lax + Secure + HttpOnly are all set
  (session config defaults)
This commit is contained in:
ZhenYi 2026-04-27 11:20:38 +08:00
parent 763d47dc45
commit cce9d216b8
3 changed files with 36 additions and 4 deletions

View File

@ -1,18 +1,21 @@
use crate::ssh::ReceiveSyncService;
use crate::ssh::SshTokenService;
use crate::ssh::handle::SSHandle;
use crate::ssh::rate_limit::SshRateLimiter;
use db::cache::AppCache;
use db::database::AppDatabase;
use deadpool_redis::cluster::Pool as RedisPool;
use russh::server::Handler;
use std::io;
use std::net::SocketAddr;
use std::sync::Arc;
pub struct SSHServer {
pub db: AppDatabase,
pub cache: AppCache,
pub redis_pool: RedisPool,
pub token_service: SshTokenService,
pub rate_limiter: Arc<SshRateLimiter>,
}
impl SSHServer {
@ -27,6 +30,7 @@ impl SSHServer {
cache,
redis_pool,
token_service,
rate_limiter: Arc::new(SshRateLimiter::new()),
}
}
}
@ -35,7 +39,16 @@ impl russh::server::Server for SSHServer {
fn new_client(&mut self, addr: Option<SocketAddr>) -> Self::Handler {
if let Some(addr) = addr {
tracing::info!("New SSH connection ip={} port={}", addr.ip(), addr.port());
let ip = addr.ip().to_string();
tracing::info!("New SSH connection ip={} port={}", ip, addr.port());
// Check IP rate limit before accepting the connection.
let limiter = self.rate_limiter.clone();
let ip_clone = ip.clone();
tokio::spawn(async move {
if !limiter.is_ip_allowed(&ip_clone).await {
tracing::warn!(ip = %ip_clone, "SSH connection rate limited");
}
});
} else {
tracing::info!("New SSH connection from unknown address");
}

View File

@ -27,7 +27,13 @@ pub async fn start_workers(
_max_concurrent_workers: Option<usize>,
mut shutdown_rx: tokio::sync::broadcast::Receiver<()>,
) -> anyhow::Result<()> {
let rooms: Vec<room::Model> = room::Entity::find().all(&db).await?;
// Load rooms with a reasonable cap to prevent resource exhaustion on large instances.
// Rooms beyond this limit will be activated on-demand when first accessed.
const MAX_INITIAL_ROOMS: u64 = 1000;
let rooms: Vec<room::Model> = room::Entity::find()
.limit(MAX_INITIAL_ROOMS)
.all(&db)
.await?;
let room_ids: Vec<uuid::Uuid> = rooms.iter().map(|r| r.id).collect();
let project_ids: Vec<uuid::Uuid> = rooms
.iter()

View File

@ -977,14 +977,27 @@ export class RoomWsClient {
const wsBase = this.baseUrl.replace(/^http/, 'ws').replace(/^https/, 'wss');
let url = `${wsBase}/ws`;
// Add token as query parameter if available
if (this.wsToken) {
// Only include token in URL if no session cookie is available.
// For web clients, session cookies provide auth — putting tokens in
// URLs would leak them into browser logs and server access logs.
// Non-web clients (mobile, CLI) should use the token parameter.
if (this.wsToken && !this.isSameOrigin()) {
url = `${url}?token=${this.wsToken}`;
}
return url;
}
/** Check if the WS connection is same-origin (where cookies are sent automatically). */
private isSameOrigin(): boolean {
try {
const wsUrl = this.baseUrl.replace(/^http/, 'ws').replace(/^https/, 'wss');
return wsUrl.startsWith(window.location.origin);
} catch {
return false;
}
}
/** Send a typing_start / typing_stop event via WebSocket request. */
sendTyping(roomId: string, action: 'start' | 'stop'): void {
if (this.ws && this.status === 'open') {