From fb829760a21cf0b0253ed5d5514597f4067a01a4 Mon Sep 17 00:00:00 2001 From: bifrost0x Date: Fri, 4 Sep 2026 07:00:48 +0200 Subject: [PATCH 1/6] Fix SSH security boundaries --- .env.example | 17 + SECURITY.md | 3 + app/key_encryption.py | 67 +++- app/key_manager.py | 70 +++- app/socket_events.py | 88 +++-- app/ssh_manager.py | 7 +- app/ssh_output_flow.py | 324 ++++++++++++++++++ config.py | 44 +++ docs/wiki/Configuration-Reference.md | 22 ++ .../Terminal-and-Persistent-tmux-Sessions.md | 13 +- static/css/webssh-2.css | 6 + static/js/app.js | 48 ++- static/js/i18n.js | 24 ++ static/js/terminal-manager.js | 188 ++++++++-- tests/e2e/session-workspace.spec.js | 2 + tests/js/terminal-manager-layout.test.js | 104 +++++- tests/test_command_set_socket_events.py | 143 +++++++- tests/test_key_manager.py | 71 +++- tests/test_key_socket_events.py | 48 ++- tests/test_ssh_output_flow.py | 230 +++++++++++++ 20 files changed, 1405 insertions(+), 114 deletions(-) create mode 100644 app/ssh_output_flow.py create mode 100644 tests/test_ssh_output_flow.py diff --git a/.env.example b/.env.example index 623f73d1..f7a57919 100644 --- a/.env.example +++ b/.env.example @@ -210,6 +210,12 @@ RATELIMIT_DEFAULT=200 per hour RATELIMIT_REAUTH=5 per minute # Per-user SSH and quick-connect attempt rate. SSH_CONNECT_RATELIMIT=10 per minute +# Per-user upload/replacement rate for encrypted SSH keys. +SSH_KEY_WRITE_RATELIMIT=30 per minute +# Per-account encrypted SSH-key limits. Existing over-limit stores remain +# readable and can still be renamed, deleted, or replaced with smaller keys. +SSH_KEY_MAX_RECORDS=100 +SSH_KEY_STORE_MAX_BYTES=8388608 # Per-user command and command-set mutation rate. # COMMAND_MUTATION_RATELIMIT=60 per minute # Storage backend for rate-limit counters. @@ -221,6 +227,17 @@ SSH_CONNECT_RATELIMIT=10 per minute # periodically retries Redis without delaying every request. # RATELIMIT_STORAGE_URL=memory:// +# Bound live SSH output waiting for browser acknowledgements. A browser that +# keeps an acknowledgement budget blocked for the timeout is disconnected; +# its SSH/tmux session remains available for reconnect. +SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET=524288 +SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET=128 +SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER=1024 +SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL=8192 +SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER=4194304 +SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL=33554432 +SSH_OUTPUT_ACK_TIMEOUT_SECONDS=10 + # ─── File transfer limits (bytes) ──────────────────────────────────────────── # Max single-file download (default 100 MB). MAX_DOWNLOAD_SIZE=104857600 diff --git a/SECURITY.md b/SECURITY.md index 6a78d318..9c6f7fe4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -83,6 +83,9 @@ Instead, report vulnerabilities via: | Host Key Logging | New keys logged with fingerprint for audit | | Connection Isolation | Session ownership verified on every operation | | Credential Handling | Cleared from memory after use | +| Stored-key work | Connection attempt rate limiting precedes stored-key PBKDF2/decryption; encrypted key stores have atomic count and byte growth limits | +| Remote clipboard | Persistent-tmux OSC 52 requests require a fresh, visible browser action; replayed and plain-SSH output cannot write the clipboard | +| Live output | Per-browser ACK accounting with per-socket, per-user, and global bounds applies SSH backpressure; a stalled browser is disconnected without closing its SSH/tmux session | ## Security Best Practices for Deployment diff --git a/app/key_encryption.py b/app/key_encryption.py index e378d079..dc9d05e1 100644 --- a/app/key_encryption.py +++ b/app/key_encryption.py @@ -354,24 +354,11 @@ def read_key_content( raise return plaintext -@_serialized_key_operation -def write_key_content( - user_id: str, key_path: str, key_content: str, *, +def _write_prepared_key_content( + user_id: str, key_path: str, encrypted: bytes, *, allowed_root: Path = None) -> bool: - """ - Encrypt and write SSH key content to file. - - Args: - user_id: User identifier - key_path: Path to write the key - key_content: SSH private key content (PEM format) - - Returns: - True if successful - """ + """Write ciphertext prepared by ``encrypt_key_content``.""" try: - encrypted = encrypt_key_content(user_id, key_content) - path = Path(key_path) path.parent.mkdir(parents=True, exist_ok=True) with _key_file_lock( @@ -391,13 +378,32 @@ def write_key_content( @_serialized_key_operation -def replace_key_content( +def write_prepared_key_content( + user_id: str, key_path: str, encrypted: bytes, *, + allowed_root: Path = None) -> bool: + """Write server-prepared ciphertext without deriving the key twice.""" + return _write_prepared_key_content( + user_id, key_path, encrypted, allowed_root=allowed_root + ) + + +@_serialized_key_operation +def write_key_content( user_id: str, key_path: str, key_content: str, *, allowed_root: Path = None) -> bool: - """Replace an existing encrypted key and restore its bytes on failure.""" + """Encrypt and write SSH key content to file.""" + encrypted = encrypt_key_content(user_id, key_content) + return _write_prepared_key_content( + user_id, key_path, encrypted, allowed_root=allowed_root + ) + + +def _replace_prepared_key_content( + user_id: str, key_path: str, key_content: str, encrypted: bytes, *, + allowed_root: Path = None) -> bool: + """Replace an encrypted key using already prepared ciphertext.""" path = Path(key_path) try: - encrypted = encrypt_key_content(str(user_id), key_content) with _key_file_lock( path, allowed_root=allowed_root) as operation_path: original = operation_path.read_bytes() @@ -435,3 +441,26 @@ def replace_key_content( error_type=type(exc).__name__, ) return False + + +@_serialized_key_operation +def replace_prepared_key_content( + user_id: str, key_path: str, key_content: str, encrypted: bytes, *, + allowed_root: Path = None) -> bool: + """Atomically replace a key with server-prepared ciphertext.""" + return _replace_prepared_key_content( + user_id, key_path, key_content, encrypted, + allowed_root=allowed_root, + ) + + +@_serialized_key_operation +def replace_key_content( + user_id: str, key_path: str, key_content: str, *, + allowed_root: Path = None) -> bool: + """Replace an existing encrypted key and restore its bytes on failure.""" + encrypted = encrypt_key_content(str(user_id), key_content) + return _replace_prepared_key_content( + user_id, key_path, key_content, encrypted, + allowed_root=allowed_root, + ) diff --git a/app/key_manager.py b/app/key_manager.py index 5d2c406a..c590c179 100644 --- a/app/key_manager.py +++ b/app/key_manager.py @@ -2,6 +2,7 @@ import os import paramiko import stat +import config from datetime import datetime, timezone from pathlib import Path from cryptography.fernet import InvalidToken @@ -106,6 +107,23 @@ def _valid_key_document(value): _DELETE_STAGING_PREFIX = '.delete-' _DELETE_TOKEN_LENGTH = 32 +SSH_KEY_CONTENT_MAX_BYTES = 64 * 1024 +SSH_KEY_STORAGE_LIMIT_ERROR = ( + "SSH key storage limit reached; delete a key or replace one with a " + "smaller key" +) + + +def _stored_key_bytes(keys_dir, keys): + """Return exact encrypted bytes referenced by one locked key document.""" + total = 0 + for key in keys: + key_path = _safe_key_path(keys_dir, key['filename']) + try: + total += key_path.stat().st_size + except FileNotFoundError: + continue + return total def _pending_delete_filename(path): @@ -322,23 +340,28 @@ def replace_key(user_id, key_id, key_content): return None, "Key not found" if not isinstance(key_content, str) or not key_content: return None, "Invalid key content" + if len(key_content.encode('utf-8')) > SSH_KEY_CONTENT_MAX_BYTES: + return None, "Key content too large (max 64KB)" try: + try: + replacement_type = identify_private_key(key_content) + except paramiko.PasswordRequiredException: + return None, "Passphrase-encrypted private keys are not supported" + except UnsupportedPrivateKeyError as exc: + return None, str(exc) + except paramiko.SSHException: + return None, "Invalid key format" + encrypted = key_encryption.encrypt_key_content( + str(user_id), key_content + ) + with storage_lock(f'keys:{user_id}'): keys = _load_keys_with_lock_held(user_id) key = next((item for item in keys if item['id'] == key_id), None) if key is None: return None, "Key not found" - try: - replacement_type = identify_private_key(key_content) - except paramiko.PasswordRequiredException: - return None, "Passphrase-encrypted private keys are not supported" - except UnsupportedPrivateKeyError as exc: - return None, str(exc) - except paramiko.SSHException: - return None, "Invalid key format" - keys_dir = get_user_keys_dir(user_id) if not keys_dir: return None, "Key not found" @@ -359,10 +382,19 @@ def replace_key(user_id, key_id, key_content): "Replacement key must use the same key type " f"({stored_type})" ) - if not key_encryption.replace_key_content( + current_bytes = _stored_key_bytes(keys_dir, keys) + stored_bytes = key_path.stat().st_size + prospective_bytes = current_bytes - stored_bytes + len(encrypted) + if ( + prospective_bytes > config.SSH_KEY_STORE_MAX_BYTES + and prospective_bytes > current_bytes + ): + return None, SSH_KEY_STORAGE_LIMIT_ERROR + if not key_encryption.replace_prepared_key_content( str(user_id), str(key_path), key_content, + encrypted, allowed_root=keys_dir, ): return None, "Failed to replace key" @@ -397,6 +429,12 @@ def save_key(user_id, name, key_content): try: if not isinstance(name, str) or not name: return None, "Invalid key name" + if len(name) > 128: + return None, "Key name too long (max 128 characters)" + if not isinstance(key_content, str) or not key_content: + return None, "Invalid key content" + if len(key_content.encode('utf-8')) > SSH_KEY_CONTENT_MAX_BYTES: + return None, "Key content too large (max 64KB)" keys_dir = get_user_keys_dir(user_id) if not keys_dir: return None, "User not found" @@ -411,6 +449,9 @@ def save_key(user_id, name, key_content): return None, str(exc) except paramiko.SSHException: return None, "Invalid key format" + encrypted = key_encryption.encrypt_key_content( + str(user_id), key_content + ) key_meta = { 'id': key_id, @@ -424,10 +465,15 @@ def save_key(user_id, name, key_content): } with storage_lock(f'keys:{user_id}'): keys = _load_keys_with_lock_held(user_id) - if not key_encryption.write_key_content( + if len(keys) >= config.SSH_KEY_MAX_RECORDS: + return None, SSH_KEY_STORAGE_LIMIT_ERROR + current_bytes = _stored_key_bytes(keys_dir, keys) + if current_bytes + len(encrypted) > config.SSH_KEY_STORE_MAX_BYTES: + return None, SSH_KEY_STORAGE_LIMIT_ERROR + if not key_encryption.write_prepared_key_content( str(user_id), str(key_path), - key_content, + encrypted, allowed_root=keys_dir, ): return None, "Failed to encrypt and save key" diff --git a/app/socket_events.py b/app/socket_events.py index b7a8dc88..646c65fa 100644 --- a/app/socket_events.py +++ b/app/socket_events.py @@ -33,6 +33,7 @@ from .quota_manager import QuotaKind, quota_manager from .socket_capacity import socket_capacity from .ssh_input_budget import budget_from_config +from .ssh_output_flow import ssh_output_flow from .remote_transfer import ( RemoteTransferCancelled, RemoteTransferError, @@ -373,10 +374,13 @@ def handle_connect(): disconnect() return False + ssh_output_flow.register_socket(socket_sid) + user_agent = request.headers.get('User-Agent', '') try: register_socket_session(user.id, socket_sid, user_agent) except Exception: + ssh_output_flow.release_socket(socket_sid) socket_capacity.release(socket_sid) raise @@ -396,6 +400,7 @@ def handle_connect(): def handle_disconnect(): """Handle client disconnection - cleanup socket session.""" socket_sid = request.sid + ssh_output_flow.release_socket(socket_sid) _cancel_ssh_banner_prompts_for_socket(socket_sid) owner_id = socket_capacity.release(socket_sid) try: @@ -673,36 +678,55 @@ def request_auth_banner_decision(banner, context): if live_jump_host.get('auth_type') == 'password': proxy_jump['password'] = runtime_password - if auth_type == 'key' and key_id: - key_content, key_error = key_manager.read_key_content( - current_user.id, key_id - ) - if key_error: - emit_error(f'SSH key error: {key_error}') - return + # Preserve precise missing-reference errors without running PBKDF2 or + # decrypting attacker-selected stored keys before the attempt budget. + if ( + auth_type == 'key' + and key_id + and key_manager.get_key(current_user.id, key_id) is None + ): + emit_error('SSH key error: Key not found') + return + bastion_key_id = None if proxy_jump: bastion_password = proxy_jump.get('password') bastion_key_id = proxy_jump.get('key_id') if not bastion_password and not bastion_key_id: emit_error('Jump host password or SSH key required') return - if bastion_key_id: - bastion_key_content, bastion_key_error = ( - key_manager.read_key_content( - current_user.id, bastion_key_id - ) - ) - if bastion_key_error: - emit_error( - f'Jump host SSH key error: {bastion_key_error}' - ) - return + if ( + bastion_key_id + and key_manager.get_key( + current_user.id, bastion_key_id + ) is None + ): + emit_error('Jump host SSH key error: Key not found') + return if check_socket_rate_limit(current_user.id, 'ssh_connect', config.RATELIMIT_SSH_CONNECT): log_warning("SSH connect rate limit hit", user=current_user.username) emit_error('Too many connection attempts. Please wait a moment.') return + if auth_type == 'key' and key_id: + key_content, key_error = key_manager.read_key_content( + current_user.id, key_id + ) + if key_error: + emit_error(f'SSH key error: {key_error}') + return + if bastion_key_id: + bastion_key_content, bastion_key_error = ( + key_manager.read_key_content( + current_user.id, bastion_key_id + ) + ) + if bastion_key_error: + emit_error( + f'Jump host SSH key error: {bastion_key_error}' + ) + return + # The target may be internal when reached via a bastion (legitimate). host, port, username, error = _validate_ssh_params( data.get('host'), data.get('port', 22), data.get('username'), @@ -1414,19 +1438,28 @@ def handle_upload_key(data, current_user=None): return _key_mutation_error( 'Key name too long (max 128 characters)' ) - if len(key_content) > 64 * 1024: + if len(key_content.encode('utf-8')) > 64 * 1024: return _key_mutation_error( 'Key content too large (max 64KB)' ) + if check_socket_rate_limit( + current_user.id, + 'ssh_key_write', + config.RATELIMIT_SSH_KEY_WRITE, + ): + return _key_mutation_error( + 'Too many SSH key changes. Please wait a moment.' + ) + key_meta, error = key_manager.save_key(current_user.id, name, key_content) if error: log_key_upload(current_user.username, name, False, request.remote_addr) return _key_mutation_error(error) log_key_upload(current_user.username, name, True, request.remote_addr) - emit('key_uploaded', {'key': key_meta}) - handle_list_keys(current_user=current_user) - return {'success': True, 'key': key_meta} + usable_key = {**key_meta, 'usable': True} + emit('key_uploaded', {'key': usable_key}) + return {'success': True, 'key': usable_key} except StorageCorruptionError as error: return _emit_storage_error(error, current_user) @@ -1479,11 +1512,20 @@ def handle_replace_key(data, current_user=None): or not key_content ): return _key_mutation_error('Key ID and key content required') - if len(key_content) > 64 * 1024: + if len(key_content.encode('utf-8')) > 64 * 1024: return _key_mutation_error( 'Key content too large (max 64KB)' ) + if check_socket_rate_limit( + current_user.id, + 'ssh_key_write', + config.RATELIMIT_SSH_KEY_WRITE, + ): + return _key_mutation_error( + 'Too many SSH key changes. Please wait a moment.' + ) + key, error = key_manager.replace_key( current_user.id, key_id, diff --git a/app/ssh_manager.py b/app/ssh_manager.py index 4f01b586..183cb20f 100644 --- a/app/ssh_manager.py +++ b/app/ssh_manager.py @@ -25,6 +25,7 @@ quota_manager, release_reservation, ) +from .ssh_output_flow import emit_ssh_output sessions = {} sessions_lock = Lock() @@ -599,6 +600,7 @@ def read_ssh_output(session_id, socketio_instance, app, cancel_event=None): from datetime import datetime, timezone cached_room = None + cached_user_id = None last_db_update = 0 persistent_tmux_available = None @@ -620,6 +622,7 @@ def read_ssh_output(session_id, socketio_instance, app, cancel_event=None): time.sleep(0.1) if db_session: + cached_user_id = db_session.user_id cached_room = f'user_{db_session.user_id}' if not cached_room: @@ -653,11 +656,11 @@ def read_ssh_output(session_id, socketio_instance, app, cancel_event=None): ) if sequence is None: break - socketio_instance.emit('ssh_output', { + emit_ssh_output(socketio_instance, cached_room, cached_user_id, session_id, { 'session_id': session_id, 'data': decoded_data, 'sequence': sequence, - }, room=cached_room) + }, cancel_event=cancel_event) if now - last_db_update >= 10.0: last_db_update = now diff --git a/app/ssh_output_flow.py b/app/ssh_output_flow.py new file mode 100644 index 00000000..422a92eb --- /dev/null +++ b/app/ssh_output_flow.py @@ -0,0 +1,324 @@ +"""Bound acknowledged Socket.IO delivery for live SSH output.""" + +from collections import defaultdict +import json +import threading +import time +import uuid + +import config + +from .audit_logger import log_warning + + +class SSHOutputFlowController: + """Track live output until each browser acknowledges accepting it.""" + + def __init__(self): + self._condition = threading.Condition(threading.RLock()) + self._reservations = {} + self._socket_bytes = defaultdict(int) + self._socket_events = defaultdict(int) + self._user_bytes = defaultdict(int) + self._user_events = defaultdict(int) + self._global_bytes = 0 + self._global_events = 0 + self._active_sockets = set() + + @staticmethod + def event_size(payload): + """Return the exact UTF-8 size of the Socket.IO event JSON payload.""" + serialized = json.dumps( + ['ssh_output', payload], + ensure_ascii=True, + separators=(',', ':'), + ) + return len(serialized.encode('utf-8')) + + def register_socket(self, socket_sid): + with self._condition: + self._active_sockets.add(socket_sid) + + def can_reserve(self, socket_sid, user_id, size): + with self._condition: + return self._fits(socket_sid, user_id, size) + + def has_pending(self, socket_sid): + with self._condition: + return self._socket_events.get(socket_sid, 0) > 0 + + def _fits(self, socket_sid, user_id, size): + if socket_sid not in self._active_sockets: + return False + return ( + self._socket_bytes.get(socket_sid, 0) + size + <= config.SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET + and self._socket_events.get(socket_sid, 0) + 1 + <= config.SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET + and self._user_bytes.get(user_id, 0) + size + <= config.SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER + and self._user_events.get(user_id, 0) + 1 + <= config.SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER + and self._global_bytes + size + <= config.SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL + and self._global_events + 1 + <= config.SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL + ) + + def reserve( + self, + socket_sid, + user_id, + session_id, + size, + *, + cancel_event=None, + timeout=None, + ): + """Wait for bounded capacity and return ``(token, reason)``.""" + timeout = ( + config.SSH_OUTPUT_ACK_TIMEOUT_SECONDS + if timeout is None else timeout + ) + deadline = time.monotonic() + timeout + with self._condition: + while not self._fits(socket_sid, user_id, size): + if socket_sid not in self._active_sockets: + return None, 'disconnected' + if cancel_event is not None and cancel_event.is_set(): + return None, 'cancelled' + remaining = deadline - time.monotonic() + if remaining <= 0: + return None, 'timeout' + self._condition.wait(min(0.1, remaining)) + + token = uuid.uuid4().hex + self._reservations[token] = ( + socket_sid, user_id, session_id, size, time.monotonic() + ) + self._socket_bytes[socket_sid] += size + self._socket_events[socket_sid] += 1 + self._user_bytes[user_id] += size + self._user_events[user_id] += 1 + self._global_bytes += size + self._global_events += 1 + return token, None + + def release(self, token): + with self._condition: + reservation = self._reservations.pop(token, None) + if reservation is None: + return False + socket_sid, user_id, _session_id, size, _created_at = reservation + self._socket_bytes[socket_sid] -= size + self._socket_events[socket_sid] -= 1 + self._user_bytes[user_id] -= size + self._user_events[user_id] -= 1 + self._global_bytes -= size + self._global_events -= 1 + self._prune(socket_sid, user_id) + self._condition.notify_all() + return True + + def _prune(self, socket_sid, user_id): + if self._socket_bytes[socket_sid] == 0: + self._socket_bytes.pop(socket_sid, None) + if self._socket_events[socket_sid] == 0: + self._socket_events.pop(socket_sid, None) + if self._user_bytes[user_id] == 0: + self._user_bytes.pop(user_id, None) + if self._user_events[user_id] == 0: + self._user_events.pop(user_id, None) + + def stale_sockets(self, *, exclude=None, now=None, timeout=None): + """Return sockets holding ACK reservations past the timeout.""" + now = time.monotonic() if now is None else now + timeout = ( + config.SSH_OUTPUT_ACK_TIMEOUT_SECONDS + if timeout is None else timeout + ) + with self._condition: + oldest = {} + for reservation in self._reservations.values(): + socket_sid, _user_id, _session_id, _size, created_at = ( + reservation + ) + if socket_sid == exclude: + continue + oldest[socket_sid] = min( + oldest.get(socket_sid, created_at), created_at + ) + return [ + socket_sid + for socket_sid, created_at in oldest.items() + if now - created_at >= timeout + ] + + def release_socket(self, socket_sid): + with self._condition: + self._active_sockets.discard(socket_sid) + tokens = [ + token + for token, reservation in self._reservations.items() + if reservation[0] == socket_sid + ] + for token in tokens: + ( + socket, + user_id, + _session_id, + size, + _created_at, + ) = self._reservations.pop(token) + self._socket_bytes[socket] -= size + self._socket_events[socket] -= 1 + self._user_bytes[user_id] -= size + self._user_events[user_id] -= 1 + self._global_bytes -= size + self._global_events -= 1 + self._prune(socket, user_id) + self._socket_bytes.pop(socket_sid, None) + self._socket_events.pop(socket_sid, None) + self._condition.notify_all() + + def usage(self): + """Return a test/diagnostic snapshot without exposing payloads.""" + with self._condition: + return { + 'global_bytes': self._global_bytes, + 'global_events': self._global_events, + 'reservations': len(self._reservations), + 'socket_bytes': dict(self._socket_bytes), + 'socket_events': dict(self._socket_events), + 'user_bytes': dict(self._user_bytes), + 'user_events': dict(self._user_events), + } + + +ssh_output_flow = SSHOutputFlowController() + + +def _room_participants(socketio_instance, room): + server = getattr(socketio_instance, 'server', None) + manager = getattr(server, 'manager', None) + if manager is None: + return [] + participants = manager.get_participants('/', room) + return list(dict.fromkeys( + participant[0] if isinstance(participant, tuple) else participant + for participant in participants + )) + + +def _disconnect_lagging_socket(socketio_instance, socket_sid): + server = getattr(socketio_instance, 'server', None) + if server is None: + return False + server.disconnect(socket_sid, namespace='/') + # Production disconnect handlers release first; keep this idempotent + # fallback for test servers and disconnects without an application event. + ssh_output_flow.release_socket(socket_sid) + return True + + +def emit_ssh_output( + socketio_instance, + room, + user_id, + session_id, + payload, + *, + cancel_event=None, +): + """Deliver one output event individually with bounded ACK reservations.""" + try: + participants = _room_participants(socketio_instance, room) + except Exception: + participants = [] + if not participants: + return + + size = ssh_output_flow.event_size(payload) + # Browsers with immediate capacity receive the current chunk before a + # lagging peer can make the Paramiko reader wait for its bounded timeout. + participants.sort(key=lambda sid: ( + 0 if ssh_output_flow.can_reserve(sid, user_id, size) + else 1 if ssh_output_flow.has_pending(sid) + else 2 + )) + for socket_sid in participants: + while True: + token, reason = ssh_output_flow.reserve( + socket_sid, + user_id, + session_id, + size, + cancel_event=cancel_event, + ) + if reason == 'cancelled': + return + if reason == 'disconnected': + break + if reason != 'timeout': + break + if not ssh_output_flow.has_pending(socket_sid): + # A user/global budget can be occupied by a different browser. + # Keep bounded SSH backpressure without blaming this healthy + # subscriber; disconnect/ping cleanup elsewhere will wake us. + stale_sockets = ssh_output_flow.stale_sockets( + exclude=socket_sid + ) + if not stale_sockets: + continue + stale_sid = stale_sockets[0] + log_warning( + 'Disconnecting browser holding expired SSH output budget', + user_id=user_id, + session_id=session_id, + sid=stale_sid, + ) + try: + _disconnect_lagging_socket( + socketio_instance, stale_sid + ) + except Exception: + pass + continue + log_warning( + 'Disconnecting browser that stopped acknowledging SSH output', + user_id=user_id, + session_id=session_id, + sid=socket_sid, + ) + try: + _disconnect_lagging_socket(socketio_instance, socket_sid) + except Exception: + pass + break + if token is None: + continue + + release_state = {'released': False} + + def acknowledge(*_args, _token=token, _state=release_state): + if _state['released']: + return + _state['released'] = True + ssh_output_flow.release(_token) + + try: + socketio_instance.emit( + 'ssh_output', payload, to=socket_sid, callback=acknowledge + ) + except Exception as error: + try: + _disconnect_lagging_socket(socketio_instance, socket_sid) + except Exception: + pass + log_warning( + 'Failed to deliver SSH output to browser', + user_id=user_id, + session_id=session_id, + sid=socket_sid, + error_type=type(error).__name__, + ) diff --git a/config.py b/config.py index b03b9659..4dc5752a 100644 --- a/config.py +++ b/config.py @@ -495,6 +495,36 @@ def _validate_quota_pair(kind, global_limit, per_user_limit, fair_slots): SOCKETIO_PING_TIMEOUT = 60 SOCKETIO_PING_INTERVAL = 25 +# Per-browser acknowledged SSH output budgets. Output readers stop consuming +# Paramiko channels while a browser is at capacity, so TCP/SSH backpressure +# applies instead of growing Socket.IO's in-process queues without bound. +SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET', 512 * 1024, 256 * 1024, + 16 * 1024 * 1024, +) +SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET', 128, 8, 4096, +) +SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER', 1024, + SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET, 32768, +) +SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL', 8192, + SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER, 131072, +) +SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER', 4 * 1024 * 1024, + SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET, 64 * 1024 * 1024, +) +SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL = _bounded_int_env( + 'SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL', 32 * 1024 * 1024, + SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER, 256 * 1024 * 1024, +) +SSH_OUTPUT_ACK_TIMEOUT_SECONDS = _bounded_int_env( + 'SSH_OUTPUT_ACK_TIMEOUT_SECONDS', 10, 1, 120, +) + ALLOW_CORS_WILDCARD = ( os.environ.get('ALLOW_CORS_WILDCARD', 'false').lower() == 'true' ) @@ -549,6 +579,9 @@ def _validate_quota_pair(kind, global_limit, per_user_limit, fair_slots): # unthrottled SSH brute-force / port-scan proxy against third-party hosts. # Generous default so normal use and reconnects never hit it. RATELIMIT_SSH_CONNECT = os.environ.get('SSH_CONNECT_RATELIMIT', '10 per minute') +RATELIMIT_SSH_KEY_WRITE = os.environ.get( + 'SSH_KEY_WRITE_RATELIMIT', '30 per minute' +) RATELIMIT_COMMAND_MUTATION = os.environ.get( 'COMMAND_MUTATION_RATELIMIT', '60 per minute', @@ -583,6 +616,17 @@ def _validate_quota_pair(kind, global_limit, per_user_limit, fair_slots): if entry.strip() ) +# Encrypted SSH-key storage is bounded per account. Existing stores above the +# byte limit remain readable and may be renamed, deleted, or replaced by +# smaller keys; only further growth is rejected. +SSH_KEY_MAX_RECORDS = _bounded_int_env( + 'SSH_KEY_MAX_RECORDS', 100, 1, 1000 +) +SSH_KEY_STORE_MAX_BYTES = _bounded_int_env( + 'SSH_KEY_STORE_MAX_BYTES', 8 * 1024 * 1024, 64 * 1024, + 64 * 1024 * 1024, +) + # Optional browser-to-SMB file sources. Enabling the feature always requires # an exact, comma-separated target allowlist; TCP port and SMB dialect are not # configurable so deployments cannot weaken the protocol contract. diff --git a/docs/wiki/Configuration-Reference.md b/docs/wiki/Configuration-Reference.md index 57014612..801e3140 100644 --- a/docs/wiki/Configuration-Reference.md +++ b/docs/wiki/Configuration-Reference.md @@ -97,6 +97,28 @@ Connection, transfer, background-work, and thread limits form one capacity model | `RATELIMIT_LOGIN_LIMIT` | `5 per minute` | | `RATELIMIT_REAUTH` | `5 per minute` | | `SSH_CONNECT_RATELIMIT` | `10 per minute` | +| `SSH_KEY_WRITE_RATELIMIT` | `30 per minute` | + +## SSH key and live-output limits + +| Variable | Default | +|---|---:| +| `SSH_KEY_MAX_RECORDS` | `100` | +| `SSH_KEY_STORE_MAX_BYTES` | `8388608` (8 MiB encrypted) | +| `SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET` | `524288` (512 KiB) | +| `SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET` | `128` | +| `SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER` | `1024` | +| `SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL` | `8192` | +| `SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER` | `4194304` (4 MiB) | +| `SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL` | `33554432` (32 MiB) | +| `SSH_OUTPUT_ACK_TIMEOUT_SECONDS` | `10` seconds | + +The key limits reject only storage growth. A pre-existing store above the byte +limit remains readable and can be renamed, deleted, or replaced with smaller +keys. Live terminal output is acknowledged by each browser. If one browser +keeps an acknowledgement budget blocked for the configured timeout, WebSSH +applies SSH backpressure and then disconnects only that browser; the underlying +SSH or persistent tmux session remains available for reconnect. `memory://` is process-local and counters reset when the process restarts. Use a `redis://` URL for durable, shared counters. Redis does not change the one-worker architecture. diff --git a/docs/wiki/Terminal-and-Persistent-tmux-Sessions.md b/docs/wiki/Terminal-and-Persistent-tmux-Sessions.md index 54c2b2c3..2b8e8100 100644 --- a/docs/wiki/Terminal-and-Persistent-tmux-Sessions.md +++ b/docs/wiki/Terminal-and-Persistent-tmux-Sessions.md @@ -26,8 +26,10 @@ On macOS, use the platform's Command equivalents for copy and paste. character to the remote process. When tmux mouse mode owns the selection, WebSSH accepts tmux's bounded OSC 52 -clipboard update for that persistent session. Browser clipboard permissions -still apply. +clipboard request for that persistent session. A visible WebSSH notification +requires a fresh **Copy** click before the remote text reaches the browser +clipboard. Replayed output and ordinary non-tmux SSH sessions cannot request a +clipboard write. Browser clipboard permissions still apply. ## Broadcast input @@ -88,6 +90,13 @@ Terminal control and output use authenticated Socket.IO events around an owned, process-local SSH session. Bulk transfer bodies take the separate bounded HTTP path shown below; they are not encoded into terminal events. +Each live output event is charged to per-browser, per-user, and process-wide +budgets until the browser acknowledges accepting it. At capacity, WebSSH stops +reading the Paramiko channel so SSH/TCP backpressure applies. A browser that +keeps an acknowledgement budget blocked for the configured timeout is +disconnected, while its underlying SSH or persistent tmux session stays +available for reconnect. + The browser can restore UI state for live sessions after refresh without injecting terminal input. The underlying SSH transport remains process-local; a WebSSH process restart closes a normal SSH session. diff --git a/static/css/webssh-2.css b/static/css/webssh-2.css index 5cabb2f0..d2caecfa 100644 --- a/static/css/webssh-2.css +++ b/static/css/webssh-2.css @@ -3057,10 +3057,16 @@ body[data-theme="paper"]:has(.auth-access-dock) { .notification-action { flex: 0 0 auto; + appearance: none; + padding: 0; + border: 0; + background: transparent; color: inherit; + font: inherit; font-weight: 700; text-decoration: underline; text-underline-offset: 3px; + cursor: pointer; } .notification-copy { diff --git a/static/js/app.js b/static/js/app.js index eff23735..5fb8a5c8 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -33,10 +33,41 @@ copy.className = 'notification-copy'; copy.textContent = String(presentation.message || ''); notification.appendChild(copy); - if (presentation.action?.label && presentation.action?.url) { - const action = document.createElement('a'); + let dismissed = false; + let fadeTimer = null; + let removeTimer = null; + const dismiss = () => { + if (dismissed) return; + dismissed = true; + clearTimeout(fadeTimer); + clearTimeout(removeTimer); + notification.classList.add('fade-out'); + try { + presentation.onDismiss?.(); + } finally { + removeTimer = setTimeout(() => notification.remove(), 300); + } + }; + if ( + presentation.action?.label + && (presentation.action?.url || presentation.action?.onClick) + ) { + const action = document.createElement( + presentation.action.onClick ? 'button' : 'a' + ); action.className = 'notification-action'; - action.href = presentation.action.url; + if (presentation.action.onClick) { + action.type = 'button'; + action.addEventListener('click', () => { + try { + presentation.action.onClick(); + } finally { + dismiss(); + } + }); + } else { + action.href = presentation.action.url; + } action.textContent = presentation.action.label; notification.appendChild(action); } @@ -44,10 +75,8 @@ const timeout = presentation.duration || (notificationType === 'success' || notificationType === 'info' ? 2000 : 3000); - setTimeout(() => { - notification.classList.add('fade-out'); - setTimeout(() => notification.remove(), 300); - }, timeout); + fadeTimer = setTimeout(dismiss, timeout); + return dismiss; }; window.ModalManager = { @@ -1150,9 +1179,8 @@ }); - socket.on('ssh_output', (data) => { - TerminalManager.writeOutput(data.session_id, data.data, data.sequence); - + socket.on('ssh_output', (data, acknowledge) => { + TerminalManager.handleSocketOutput(data, acknowledge); }); socket.on('ssh_error', (data) => { diff --git a/static/js/i18n.js b/static/js/i18n.js index 92da6e32..70cdb0c9 100644 --- a/static/js/i18n.js +++ b/static/js/i18n.js @@ -34,6 +34,10 @@ const translations = { 'auth.togglePasswordVisibility': 'Toggle password visibility', 'terminal.mobileInputPlaceholder': 'Type or paste here...', + 'terminal.remoteClipboardPrompt': 'The remote tmux session wants to copy text to your clipboard.', + 'terminal.remoteClipboardApprove': 'Copy', + 'terminal.remoteClipboardCopied': 'Remote text copied to clipboard', + 'terminal.remoteClipboardDenied': 'Clipboard access denied', 'connection.recentConnections': 'Recent Connections', 'connection.recentConnectionsHint': 'Stored only in this browser for your account.', @@ -1292,6 +1296,10 @@ const translations = { 'auth.togglePasswordVisibility': 'Ẩn hoặc hiện mật khẩu', 'terminal.mobileInputPlaceholder': 'Nhập hoặc dán vào đây...', + 'terminal.remoteClipboardPrompt': 'Phiên tmux từ xa muốn sao chép văn bản vào bảng nhớ tạm của bạn.', + 'terminal.remoteClipboardApprove': 'Sao chép', + 'terminal.remoteClipboardCopied': 'Đã sao chép văn bản từ xa vào bảng nhớ tạm', + 'terminal.remoteClipboardDenied': 'Quyền truy cập bảng nhớ tạm bị từ chối', 'connection.recentConnections': 'Kết nối gần đây', 'connection.recentConnectionsHint': 'Chỉ được lưu trong trình duyệt này cho tài khoản của bạn.', @@ -2549,6 +2557,10 @@ const translations = { 'auth.togglePasswordVisibility': 'Passwortsichtbarkeit umschalten', 'terminal.mobileInputPlaceholder': 'Hier tippen oder einfügen...', + 'terminal.remoteClipboardPrompt': 'Die entfernte tmux-Sitzung möchte Text in Ihre Zwischenablage kopieren.', + 'terminal.remoteClipboardApprove': 'Kopieren', + 'terminal.remoteClipboardCopied': 'Entfernter Text wurde in die Zwischenablage kopiert', + 'terminal.remoteClipboardDenied': 'Zugriff auf die Zwischenablage verweigert', 'connection.recentConnections': 'Letzte Verbindungen', 'connection.recentConnectionsHint': 'Wird nur in diesem Browser für deinen Account gespeichert.', @@ -3805,6 +3817,10 @@ const translations = { 'auth.togglePasswordVisibility': 'Afficher ou masquer le mot de passe', 'terminal.mobileInputPlaceholder': 'Tapez ou collez ici...', + 'terminal.remoteClipboardPrompt': 'La session tmux distante souhaite copier du texte dans votre presse-papiers.', + 'terminal.remoteClipboardApprove': 'Copier', + 'terminal.remoteClipboardCopied': 'Texte distant copié dans le presse-papiers', + 'terminal.remoteClipboardDenied': 'Accès au presse-papiers refusé', 'connection.recentConnections': 'Connexions récentes', 'connection.recentConnectionsHint': 'Stockées uniquement dans ce navigateur pour votre compte.', @@ -5061,6 +5077,10 @@ const translations = { 'auth.togglePasswordVisibility': 'Mostrar u ocultar la contraseña', 'terminal.mobileInputPlaceholder': 'Escribe o pega aquí...', + 'terminal.remoteClipboardPrompt': 'La sesión tmux remota quiere copiar texto al portapapeles.', + 'terminal.remoteClipboardApprove': 'Copiar', + 'terminal.remoteClipboardCopied': 'Texto remoto copiado al portapapeles', + 'terminal.remoteClipboardDenied': 'Acceso al portapapeles denegado', 'connection.recentConnections': 'Conexiones recientes', 'connection.recentConnectionsHint': 'Se guardan solo en este navegador para tu cuenta.', @@ -6317,6 +6337,10 @@ const translations = { 'auth.togglePasswordVisibility': '切换密码可见性', 'terminal.mobileInputPlaceholder': '在这里输入或粘贴...', + 'terminal.remoteClipboardPrompt': '远程 tmux 会话请求将文本复制到剪贴板。', + 'terminal.remoteClipboardApprove': '复制', + 'terminal.remoteClipboardCopied': '远程文本已复制到剪贴板', + 'terminal.remoteClipboardDenied': '剪贴板访问被拒绝', 'connection.recentConnections': '最近连接', 'connection.recentConnectionsHint': '仅在此浏览器中为你的账户保存。', diff --git a/static/js/terminal-manager.js b/static/js/terminal-manager.js index 454f0876..3a274a92 100644 --- a/static/js/terminal-manager.js +++ b/static/js/terminal-manager.js @@ -4,6 +4,7 @@ const TerminalManager = { searchAddons: {}, terminalReady: {}, pendingOutput: {}, + pendingOutputSizes: {}, sessionTerminals: {}, transcripts: {}, transcriptSizes: {}, @@ -14,6 +15,7 @@ const TerminalManager = { scrollbarDisposers: {}, compositionDisposers: {}, clipboardDisposers: {}, + terminalWriteCallbacks: {}, osc52ClipboardAllowed: {}, isVirtualKeyboardVisible(visualViewportHeight, layoutViewportHeight) { @@ -23,6 +25,7 @@ const TerminalManager = { return (visualViewportHeight / layoutViewportHeight) < 0.75; }, maxTranscriptSize: 200000, + maxPendingOutputSize: 200000, getCssVar(name, fallback = '') { return getComputedStyle(document.body).getPropertyValue(name).trim() || fallback; @@ -96,37 +99,89 @@ const TerminalManager = { registerOsc52ClipboardHandler(terminal) { if (!terminal?.parser?.registerOscHandler) return null; let failureReported = false; + let pendingRequest = null; - return terminal.parser.registerOscHandler(52, data => { + const translation = (key, fallback) => { + const value = window.i18n?.t?.(key); + return value && value !== key ? value : fallback; + }; + + const reportFailure = () => { + if (failureReported) return; + failureReported = true; + window.showNotification?.( + translation( + 'terminal.remoteClipboardDenied', + 'Clipboard access denied', + ), + 'error', + ); + }; + + const oscDisposable = terminal.parser.registerOscHandler(52, data => { const text = this.decodeOsc52Clipboard(data); if (text === null) return true; const clipboard = navigator.clipboard; if (!clipboard || typeof clipboard.writeText !== 'function') { - if (!failureReported) { - failureReported = true; - window.showNotification?.('Clipboard access denied', 'error'); - } + reportFailure(); return true; } - try { - Promise.resolve(clipboard.writeText(text)).then(() => { - failureReported = false; - }).catch(() => { - if (!failureReported) { - failureReported = true; - window.showNotification?.('Clipboard access denied', 'error'); - } - }); - } catch { - if (!failureReported) { - failureReported = true; - window.showNotification?.('Clipboard access denied', 'error'); - } + if (pendingRequest) { + pendingRequest.text = text; + return true; } + + const request = {text}; + pendingRequest = request; + window.showNotification?.({ + message: translation( + 'terminal.remoteClipboardPrompt', + 'The remote tmux session wants to copy text to your clipboard.', + ), + type: 'info', + duration: 15000, + action: { + label: translation( + 'terminal.remoteClipboardApprove', + 'Copy', + ), + onClick: () => { + if (pendingRequest !== request) return; + const requestedText = request.text; + pendingRequest = null; + try { + Promise.resolve( + clipboard.writeText(requestedText) + ).then(() => { + failureReported = false; + window.showNotification?.( + translation( + 'terminal.remoteClipboardCopied', + 'Remote text copied to clipboard', + ), + 'success', + ); + }).catch(reportFailure); + } catch { + reportFailure(); + } + }, + }, + onDismiss: () => { + if (pendingRequest === request) pendingRequest = null; + }, + }); return true; }); + + return { + dispose() { + pendingRequest = null; + oscDisposable?.dispose?.(); + }, + }; }, activateOsc52ClipboardHandler(terminalKey, expectedTerminal) { @@ -329,6 +384,7 @@ const TerminalManager = { const existingOutput = [...(this.transcripts[sessionId] || [])]; this.pendingOutput[key] = []; + this.pendingOutputSizes[key] = 0; this.terminalReady[key] = false; if (!this.transcripts[sessionId]) { this.transcripts[sessionId] = []; @@ -351,17 +407,22 @@ const TerminalManager = { terminal.clear(); const pendingOutput = this.pendingOutput[key] || []; this.pendingOutput[key] = []; + this.pendingOutputSizes[key] = 0; this.terminalReady[key] = true; - const replayOutput = existingOutput.concat(pendingOutput); + const replayOutput = existingOutput.map(data => ({ + data, + onWritten: null, + })).concat(pendingOutput); if (replayOutput.length === 0) { this.activateOsc52ClipboardHandler(key, terminal); return; } let remainingWrites = replayOutput.length; - replayOutput.forEach(data => { - this.writeOutputToTerminal(key, data, () => { + replayOutput.forEach(entry => { + this.writeOutputToTerminal(key, entry.data, () => { + entry.onWritten?.(); remainingWrites -= 1; if (remainingWrites === 0) { this.activateOsc52ClipboardHandler(key, terminal); @@ -375,13 +436,16 @@ const TerminalManager = { return true; }, - writeOutput(sessionId, data, sequence = null) { + writeOutput(sessionId, data, sequence = null, onAccepted = null) { const normalizedSequence = Number.isSafeInteger(sequence) && sequence > 0 ? sequence : null; if (normalizedSequence !== null) { const lastSequence = this.lastOutputSequences[sessionId] || 0; - if (normalizedSequence <= lastSequence) return; + if (normalizedSequence <= lastSequence) { + onAccepted?.(); + return; + } this.lastOutputSequences[sessionId] = normalizedSequence; if (!this.sequencedOutput[sessionId]) { this.sequencedOutput[sessionId] = []; @@ -404,17 +468,44 @@ const TerminalManager = { this.appendTranscript(sessionId, data); if (terminalKeys.length === 0) { console.error('Terminal not found for output'); + onAccepted?.(); return; } + let remainingTerminals = terminalKeys.length; + const terminalAccepted = () => { + remainingTerminals -= 1; + if (remainingTerminals === 0) onAccepted?.(); + }; terminalKeys.forEach(key => { - this.writeOutputToTerminal(key, data); + this.writeOutputToTerminal(key, data, terminalAccepted); }); }, + handleSocketOutput(data, acknowledge) { + let acknowledged = false; + const acknowledgeOnce = () => { + if (acknowledged) return; + acknowledged = true; + if (typeof acknowledge === 'function') acknowledge(); + }; + try { + this.writeOutput( + data.session_id, + data.data, + data.sequence, + acknowledgeOnce, + ); + } catch (error) { + acknowledgeOnce(); + throw error; + } + }, + writeOutputToTerminal(terminalKey, data, onWritten = null) { const terminal = this.terminals[terminalKey]; if (!terminal) { + onWritten?.(); return; } @@ -422,14 +513,49 @@ const TerminalManager = { // Bare-pattern regexes were removed because they corrupt legitimate // output like "padding:0;color:red" or "cat file". data = data.replace(/\x1b\[[?>]?[0-9;]*c/g, ''); + if (!data) { + onWritten?.(); + return; + } if (this.terminalReady[terminalKey]) { - this.writeToTerminalWithScroll(terminal, data, onWritten); + if (!this.terminalWriteCallbacks[terminalKey]) { + this.terminalWriteCallbacks[terminalKey] = new Set(); + } + let completed = false; + const complete = () => { + if (completed) return; + completed = true; + this.terminalWriteCallbacks[terminalKey]?.delete(complete); + onWritten?.(); + }; + this.terminalWriteCallbacks[terminalKey].add(complete); + try { + this.writeToTerminalWithScroll(terminal, data, complete); + } catch (error) { + complete(); + throw error; + } } else { if (!this.pendingOutput[terminalKey]) { this.pendingOutput[terminalKey] = []; + this.pendingOutputSizes[terminalKey] = 0; + } + this.pendingOutput[terminalKey].push({data, onWritten}); + this.pendingOutputSizes[terminalKey] = ( + this.pendingOutputSizes[terminalKey] || 0 + ) + data.length; + while ( + this.pendingOutputSizes[terminalKey] > this.maxPendingOutputSize + && this.pendingOutput[terminalKey].length > 1 + ) { + const removed = this.pendingOutput[terminalKey].shift(); + this.pendingOutputSizes[terminalKey] -= removed.data.length; + // The chunk remains in the separately bounded transcript; it + // is safe to release server capacity without feeding an + // unbounded pre-attach xterm queue. + removed.onWritten?.(); } - this.pendingOutput[terminalKey].push(data); } }, @@ -897,6 +1023,12 @@ const TerminalManager = { destroyTerminalKey(terminalKey, sessionId) { const terminal = this.terminals[terminalKey]; + (this.pendingOutput[terminalKey] || []).forEach(entry => { + entry.onWritten?.(); + }); + (this.terminalWriteCallbacks[terminalKey] || new Set()).forEach( + callback => callback() + ); this.scrollbarDisposers[terminalKey]?.(); this.compositionDisposers[terminalKey]?.(); this.clipboardDisposers[terminalKey]?.dispose?.(); @@ -908,8 +1040,10 @@ const TerminalManager = { delete this.searchAddons[terminalKey]; delete this.terminalReady[terminalKey]; delete this.pendingOutput[terminalKey]; + delete this.pendingOutputSizes[terminalKey]; delete this.compositionDisposers[terminalKey]; delete this.clipboardDisposers[terminalKey]; + delete this.terminalWriteCallbacks[terminalKey]; delete this.osc52ClipboardAllowed[terminalKey]; if (sessionId && this.sessionTerminals[sessionId]) { diff --git a/tests/e2e/session-workspace.spec.js b/tests/e2e/session-workspace.spec.js index 69fdc88d..c92250e7 100644 --- a/tests/e2e/session-workspace.spec.js +++ b/tests/e2e/session-workspace.spec.js @@ -416,6 +416,8 @@ test('tmux ignores replayed OSC 52 and accepts a live clipboard selection', asyn ); }); + expect(await page.evaluate(() => window.__workspaceClipboard)).toBeNull(); + await page.locator('#notificationContainer .notification-action').click(); await expect.poll(() => page.evaluate(() => window.__workspaceClipboard)) .toBe('tmux selection'); await assertNoExternalRequests(page); diff --git a/tests/js/terminal-manager-layout.test.js b/tests/js/terminal-manager-layout.test.js index ad1ee828..89c7c71f 100644 --- a/tests/js/terminal-manager-layout.test.js +++ b/tests/js/terminal-manager-layout.test.js @@ -98,16 +98,18 @@ test('OSC 52 clipboard payloads are bounded, targeted, and decoded as UTF-8', () assert.equal(TerminalManager.decodeOsc52Clipboard('c;dG9vIGxhcmdl', 4), null); }); -test('OSC 52 handler writes valid tmux selections to the browser clipboard', async () => { +test('OSC 52 handler requires a user action before writing the clipboard', async () => { const writes = []; let handler; + let presentation; global.navigator.clipboard = { writeText(text) { writes.push(text); return Promise.resolve(); }, }; - const disposable = {dispose() {}}; + let disposed = false; + const disposable = {dispose() { disposed = true; }}; const terminal = { parser: { registerOscHandler(identifier, callback) { @@ -118,13 +120,109 @@ test('OSC 52 handler writes valid tmux selections to the browser clipboard', asy }, }; - assert.equal(TerminalManager.registerOsc52ClipboardHandler(terminal), disposable); + global.window.showNotification = value => { + presentation = value; + }; + const registered = TerminalManager.registerOsc52ClipboardHandler(terminal); assert.equal(handler(';dG11eCBzZWxlY3Rpb24='), true); + assert.deepEqual(writes, []); + assert.equal(typeof presentation.action.onClick, 'function'); + presentation.action.onClick(); await new Promise(resolve => setImmediate(resolve)); assert.deepEqual(writes, ['tmux selection']); + registered.dispose(); + assert.equal(disposed, true); + delete global.window.showNotification; delete global.navigator.clipboard; }); +test('socket output is acknowledged after TerminalManager accepts it', () => { + const calls = []; + const originalWriteOutput = TerminalManager.writeOutput; + let acceptOutput; + TerminalManager.writeOutput = (...args) => { + calls.push(['write', ...args.slice(0, 3)]); + acceptOutput = args[3]; + }; + + TerminalManager.handleSocketOutput( + {session_id: 'session-1', data: 'hello', sequence: 3}, + () => calls.push(['ack']), + ); + + assert.deepEqual(calls, [ + ['write', 'session-1', 'hello', 3], + ]); + acceptOutput(); + assert.deepEqual(calls, [ + ['write', 'session-1', 'hello', 3], + ['ack'], + ]); + TerminalManager.writeOutput = originalWriteOutput; +}); + +test('socket output waits for every visible xterm pane before ACK', () => { + const originalWriteOutputToTerminal = TerminalManager.writeOutputToTerminal; + const callbacks = []; + let acknowledgements = 0; + TerminalManager.sessionTerminals = {multi: ['pane-1', 'pane-2']}; + TerminalManager.transcripts = {}; + TerminalManager.transcriptSizes = {}; + TerminalManager.writeOutputToTerminal = (_key, _data, callback) => { + callbacks.push(callback); + }; + + TerminalManager.writeOutput('multi', 'output', 1, () => { + acknowledgements += 1; + }); + + assert.equal(acknowledgements, 0); + callbacks[0](); + assert.equal(acknowledgements, 0); + callbacks[1](); + assert.equal(acknowledgements, 1); + TerminalManager.writeOutputToTerminal = originalWriteOutputToTerminal; +}); + +test('pre-attach output stays bounded and releases trimmed chunks', () => { + const originalLimit = TerminalManager.maxPendingOutputSize; + const accepted = []; + TerminalManager.maxPendingOutputSize = 5; + TerminalManager.terminals = {pending: {}}; + TerminalManager.terminalReady = {pending: false}; + TerminalManager.pendingOutput = {pending: []}; + TerminalManager.pendingOutputSizes = {pending: 0}; + + TerminalManager.writeOutputToTerminal( + 'pending', '1234', () => accepted.push('first') + ); + TerminalManager.writeOutputToTerminal( + 'pending', '5678', () => accepted.push('second') + ); + + assert.deepEqual(accepted, ['first']); + assert.equal(TerminalManager.pendingOutputSizes.pending, 4); + assert.deepEqual( + TerminalManager.pendingOutput.pending.map(entry => entry.data), + ['5678'], + ); + TerminalManager.maxPendingOutputSize = originalLimit; +}); + +test('destroying a terminal releases in-flight xterm acceptance callbacks', () => { + const accepted = []; + TerminalManager.terminals = {closing: {dispose() {}}}; + TerminalManager.pendingOutput = {closing: []}; + TerminalManager.terminalWriteCallbacks = { + closing: new Set([() => accepted.push('accepted')]), + }; + + TerminalManager.destroyTerminalKey('closing', 'session-closing'); + + assert.deepEqual(accepted, ['accepted']); + assert.equal(TerminalManager.terminalWriteCallbacks.closing, undefined); +}); + test('copy shortcuts write xterm selection directly to the clipboard', async () => { const writes = []; global.navigator.clipboard = { diff --git a/tests/test_command_set_socket_events.py b/tests/test_command_set_socket_events.py index 698a57de..36618ac3 100644 --- a/tests/test_command_set_socket_events.py +++ b/tests/test_command_set_socket_events.py @@ -769,9 +769,7 @@ def test_deleted_saved_jump_host_stops_before_rate_limit_dns_and_network( monkeypatch.setattr( socket_events, 'check_socket_rate_limit', - lambda *_args, **_kwargs: (_ for _ in ()).throw( - AssertionError('rate limit must not run') - ), + lambda *_args, **_kwargs: False, ) monkeypatch.setattr( socket_events, @@ -842,9 +840,7 @@ def test_revoked_target_key_stops_before_rate_limit_dns_and_network( monkeypatch.setattr( socket_events, 'check_socket_rate_limit', - lambda *_args, **_kwargs: (_ for _ in ()).throw( - AssertionError('rate limit must not run') - ), + lambda *_args, **_kwargs: False, ) monkeypatch.setattr( socket_events, @@ -995,9 +991,7 @@ def test_target_key_internal_error_never_reaches_socket_client( monkeypatch.setattr( socket_events, 'check_socket_rate_limit', - lambda *_args, **_kwargs: (_ for _ in ()).throw( - AssertionError('rate limit must not run') - ), + lambda *_args, **_kwargs: False, ) monkeypatch.setattr( socket_events, @@ -1034,6 +1028,64 @@ def test_target_key_internal_error_never_reaches_socket_client( assert secret_error not in repr(emitted) +def test_rate_limited_target_key_never_runs_decryption( + app, monkeypatch, rsa_private_key_pem): + from flask import request + from app import key_manager, ssh_manager + import app.socket_events as socket_events + + user_id, sid = create_socket_user(app, 'rate_limited_target_key') + with app.app_context(): + key, error = key_manager.save_key( + user_id, 'Target key', rsa_private_key_pem + ) + assert error is None + monkeypatch.setattr( + key_manager, + 'read_key_content', + lambda *_args, **_kwargs: (_ for _ in ()).throw( + AssertionError('stored key decryption must not run') + ), + ) + emitted = [] + monkeypatch.setattr( + socket_events, + 'emit', + lambda event, payload=None, **_kwargs: emitted.append( + (event, payload) + ), + ) + monkeypatch.setattr( + socket_events, 'check_socket_rate_limit', lambda *_args: True + ) + monkeypatch.setattr( + ssh_manager, + 'create_ssh_connection', + lambda **_kwargs: (_ for _ in ()).throw( + AssertionError('network must not run') + ), + ) + + with app.test_request_context('/socket.io'): + request.sid = sid + socket_events.handle_ssh_connect({ + 'host': 'target.example', + 'port': 22, + 'username': 'target-user', + 'auth_type': 'key', + 'key_id': key['id'], + 'client_request_id': 'limited-key-request', + }) + + assert emitted == [( + 'ssh_error', + { + 'error': 'Too many connection attempts. Please wait a moment.', + 'client_request_id': 'limited-key-request', + }, + )] + + def test_jump_key_internal_error_never_reaches_socket_client( app, monkeypatch, rsa_private_key_pem): from flask import request @@ -1080,9 +1132,7 @@ def test_jump_key_internal_error_never_reaches_socket_client( monkeypatch.setattr( socket_events, 'check_socket_rate_limit', - lambda *_args, **_kwargs: (_ for _ in ()).throw( - AssertionError('rate limit must not run') - ), + lambda *_args, **_kwargs: False, ) monkeypatch.setattr( socket_events, @@ -1125,6 +1175,75 @@ def test_jump_key_internal_error_never_reaches_socket_client( assert secret_error not in repr(emitted) +def test_rate_limited_jump_key_never_runs_decryption( + app, monkeypatch, rsa_private_key_pem): + from flask import request + from app import jump_host_manager, key_manager, ssh_manager + import app.socket_events as socket_events + + user_id, sid = create_socket_user(app, 'rate_limited_jump_key') + with app.app_context(): + key, error = key_manager.save_key( + user_id, 'Jump key', rsa_private_key_pem + ) + assert error is None + jump_host, error = jump_host_manager.add_jump_host( + user_id, + 'Key Bastion', + 'bastion.example', + 22, + 'jump-user', + 'key', + key_id=key['id'], + ) + assert error is None + monkeypatch.setattr( + key_manager, + 'read_key_content', + lambda *_args, **_kwargs: (_ for _ in ()).throw( + AssertionError('stored jump-key decryption must not run') + ), + ) + emitted = [] + monkeypatch.setattr( + socket_events, + 'emit', + lambda event, payload=None, **_kwargs: emitted.append( + (event, payload) + ), + ) + monkeypatch.setattr( + socket_events, 'check_socket_rate_limit', lambda *_args: True + ) + monkeypatch.setattr( + ssh_manager, + 'create_ssh_connection', + lambda **_kwargs: (_ for _ in ()).throw( + AssertionError('network must not run') + ), + ) + + with app.test_request_context('/socket.io'): + request.sid = sid + socket_events.handle_ssh_connect({ + 'host': 'target.example', + 'port': 22, + 'username': 'target-user', + 'auth_type': 'password', + 'password': 'runtime-password', + 'client_request_id': 'limited-jump-request', + 'proxy_jump': {'jump_host_id': jump_host['id']}, + }) + + assert emitted == [( + 'ssh_error', + { + 'error': 'Too many connection attempts. Please wait a moment.', + 'client_request_id': 'limited-jump-request', + }, + )] + + def test_quick_connect_key_internal_error_never_reaches_socket_client( app, monkeypatch, rsa_private_key_pem): from app import connection_pool, key_encryption, key_manager diff --git a/tests/test_key_manager.py b/tests/test_key_manager.py index fa482ca4..f30c8d16 100644 --- a/tests/test_key_manager.py +++ b/tests/test_key_manager.py @@ -18,6 +18,75 @@ def create_user(app, username='key-user'): return user.id +def test_key_store_count_limit_is_atomic_and_delete_remains_available( + app, monkeypatch, rsa_private_key_pem): + import config + from app import key_manager + + user_id = create_user(app, 'key-count-limit') + monkeypatch.setattr(config, 'SSH_KEY_MAX_RECORDS', 1) + with app.app_context(): + first, error = key_manager.save_key( + user_id, 'First', rsa_private_key_pem + ) + assert error is None + + second, error = key_manager.save_key( + user_id, 'Second', rsa_private_key_pem + ) + + assert second is None + assert error == key_manager.SSH_KEY_STORAGE_LIMIT_ERROR + assert len(key_manager.load_keys(user_id)) == 1 + assert key_manager.delete_key(user_id, first['id']) is True + assert key_manager.load_keys(user_id) == [] + + +def test_over_limit_key_store_allows_no_growth_but_rejects_growth( + app, monkeypatch, rsa_private_key_pem): + import config + from app import key_manager + + user_id = create_user(app, 'key-byte-limit') + with app.app_context(): + key, error = key_manager.save_key( + user_id, 'Existing', rsa_private_key_pem + ) + assert error is None + key_path = Path(key_manager.get_key_path(user_id, key['id'])) + existing_bytes = key_path.stat().st_size + monkeypatch.setattr( + config, 'SSH_KEY_STORE_MAX_BYTES', existing_bytes - 1 + ) + + replaced, error = key_manager.replace_key( + user_id, key['id'], rsa_private_key_pem + ) + assert error is None + assert replaced['id'] == key['id'] + + replaced, error = key_manager.replace_key( + user_id, key['id'], rsa_private_key_pem + ('\n' * 256) + ) + assert replaced is None + assert error == key_manager.SSH_KEY_STORAGE_LIMIT_ERROR + assert key_manager.read_key_content(user_id, key['id']) == ( + rsa_private_key_pem, + None, + ) + + +def test_key_size_limit_counts_utf8_bytes_before_key_parsing(app): + from app import key_manager + + user_id = create_user(app, 'key-utf8-limit') + with app.app_context(): + key, error = key_manager.save_key(user_id, 'Too large', 'é' * 40000) + + assert key is None + assert error == 'Key content too large (max 64KB)' + + def test_rename_key_changes_only_owned_metadata_name( app, rsa_private_key_pem): from app import key_manager @@ -338,7 +407,7 @@ def test_replace_key_write_failure_preserves_active_key( before = key_path.read_bytes() monkeypatch.setattr( key_manager.key_encryption, - 'replace_key_content', + 'replace_prepared_key_content', lambda *_args, **_kwargs: False, ) diff --git a/tests/test_key_socket_events.py b/tests/test_key_socket_events.py index b2c2233c..7ea63213 100644 --- a/tests/test_key_socket_events.py +++ b/tests/test_key_socket_events.py @@ -56,9 +56,11 @@ def test_key_upload_and_rename_return_safe_acknowledgements( assert rsa_private_key_pem not in repr(uploaded) assert rsa_private_key_pem not in repr(emitted) assert set(uploaded['key']) == { - 'id', 'name', 'filename', 'key_type', 'encrypted', 'uploaded_at' + 'id', 'name', 'filename', 'key_type', 'encrypted', 'uploaded_at', + 'usable', } - assert any(event == 'keys_list' for event, _payload in emitted) + assert uploaded['key']['usable'] is True + assert not any(event == 'keys_list' for event, _payload in emitted) renamed, emitted = call_socket_handler( app, @@ -70,7 +72,14 @@ def test_key_upload_and_rename_return_safe_acknowledgements( assert renamed == { 'success': True, - 'key': {**uploaded['key'], 'name': 'Renamed'}, + 'key': { + **{ + field: value + for field, value in uploaded['key'].items() + if field != 'usable' + }, + 'name': 'Renamed', + }, } assert any(event == 'key_renamed' for event, _payload in emitted) assert rsa_private_key_pem not in repr(renamed) @@ -79,6 +88,39 @@ def test_key_upload_and_rename_return_safe_acknowledgements( assert key_manager.load_keys(user_id)[0]['name'] == 'Renamed' +def test_key_upload_rate_limit_runs_before_parsing_or_encryption( + app, monkeypatch, rsa_private_key_pem): + import app.socket_events as socket_events + + _user_id, sid = create_socket_user(app, 'key_write_limited') + monkeypatch.setattr( + socket_events, + 'check_socket_rate_limit', + lambda *_args, **_kwargs: True, + ) + monkeypatch.setattr( + socket_events.key_manager, + 'save_key', + lambda *_args, **_kwargs: (_ for _ in ()).throw( + AssertionError('key parsing/encryption must not run') + ), + ) + + acknowledgement, emitted = call_socket_handler( + app, + monkeypatch, + socket_events.handle_upload_key, + sid, + {'name': 'Limited', 'key_content': rsa_private_key_pem}, + ) + + assert acknowledgement == { + 'success': False, + 'error': 'Too many SSH key changes. Please wait a moment.', + } + assert all(event != 'key_uploaded' for event, _payload in emitted) + + def test_key_upload_rejection_never_returns_private_input( app, monkeypatch): import app.socket_events as socket_events diff --git a/tests/test_ssh_output_flow.py b/tests/test_ssh_output_flow.py new file mode 100644 index 00000000..656928ca --- /dev/null +++ b/tests/test_ssh_output_flow.py @@ -0,0 +1,230 @@ +import threading + +import pytest + + +class FakeManager: + def __init__(self, participants): + self.participants = participants + + def get_participants(self, namespace, room): + assert namespace == '/' + assert room == 'user_7' + return iter((sid, f'eio-{sid}') for sid in self.participants) + + +class FakeServer: + def __init__(self, participants): + self.manager = FakeManager(participants) + self.disconnected = [] + + def disconnect(self, sid, namespace='/'): + self.disconnected.append((sid, namespace)) + + +class FakeSocketIO: + def __init__(self, participants): + self.server = FakeServer(participants) + self.emitted = [] + + def emit(self, event, payload, to=None, callback=None): + self.emitted.append((event, payload, to, callback)) + + +def _configure_limits(monkeypatch, size, *, events=8, timeout=1): + import config + + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_BYTES_PER_SOCKET', size + ) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_SOCKET', events + ) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER', events * 4 + ) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL', events * 8 + ) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_BYTES_PER_USER', size * 4 + ) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL', size * 8 + ) + monkeypatch.setattr(config, 'SSH_OUTPUT_ACK_TIMEOUT_SECONDS', timeout) + + +def test_ssh_output_reservation_is_released_only_by_browser_ack( + monkeypatch): + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + monkeypatch.setattr(output_flow, 'ssh_output_flow', controller) + payload = {'session_id': 's1', 'data': 'hello', 'sequence': 1} + size = controller.event_size(payload) + _configure_limits(monkeypatch, size * 4) + controller.register_socket('browser-1') + socketio = FakeSocketIO(['browser-1']) + + output_flow.emit_ssh_output( + socketio, 'user_7', 7, 's1', payload + ) + + assert controller.usage()['reservations'] == 1 + assert socketio.emitted[0][2] == 'browser-1' + socketio.emitted[0][3]() + socketio.emitted[0][3]() + assert controller.usage()['reservations'] == 0 + assert controller.usage()['global_bytes'] == 0 + + +def test_lagging_browser_is_bounded_without_closing_ssh_session( + monkeypatch): + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + monkeypatch.setattr(output_flow, 'ssh_output_flow', controller) + payload = {'session_id': 's1', 'data': 'next', 'sequence': 2} + size = controller.event_size(payload) + _configure_limits(monkeypatch, size, events=1, timeout=0.01) + controller.register_socket('slow') + controller.register_socket('healthy') + token, reason = controller.reserve('slow', 7, 's1', size) + assert reason is None + socketio = FakeSocketIO(['slow', 'healthy']) + + output_flow.emit_ssh_output( + socketio, 'user_7', 7, 's1', payload + ) + + assert [entry[2] for entry in socketio.emitted] == ['healthy'] + assert socketio.server.disconnected == [('slow', '/')] + assert controller.release(token) is False + # Browser eviction never receives or closes the underlying SSH session. + assert controller.usage()['reservations'] == 1 + socketio.emitted[0][3]() + assert controller.usage()['reservations'] == 0 + + +def test_cancelled_reader_does_not_wait_for_ack_timeout(monkeypatch): + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + monkeypatch.setattr(output_flow, 'ssh_output_flow', controller) + payload = {'session_id': 's1', 'data': 'next', 'sequence': 2} + size = controller.event_size(payload) + _configure_limits(monkeypatch, size, events=1, timeout=30) + controller.register_socket('slow') + controller.reserve('slow', 7, 's1', size) + cancel_event = threading.Event() + cancel_event.set() + socketio = FakeSocketIO(['slow']) + + output_flow.emit_ssh_output( + socketio, + 'user_7', + 7, + 's1', + payload, + cancel_event=cancel_event, + ) + + assert socketio.emitted == [] + assert socketio.server.disconnected == [] + controller.release_socket('slow') + assert controller.usage()['reservations'] == 0 + + +def test_new_session_cannot_reuse_unacknowledged_socket_budget(monkeypatch): + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + size = 100 + _configure_limits(monkeypatch, size, events=1, timeout=0.01) + controller.register_socket('browser') + first, reason = controller.reserve('browser', 7, 'closed-session', size) + assert reason is None + + second, reason = controller.reserve( + 'browser', 7, 'new-session', size + ) + + assert second is None + assert reason == 'timeout' + assert controller.usage()['reservations'] == 1 + controller.release_socket('browser') + assert controller.release(first) is False + + +def test_expired_global_budget_holder_is_evicted_for_healthy_user( + monkeypatch): + import config + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + monkeypatch.setattr(output_flow, 'ssh_output_flow', controller) + payload = {'session_id': 'healthy-session', 'data': 'next', 'sequence': 1} + size = controller.event_size(payload) + _configure_limits(monkeypatch, size, timeout=0.01) + monkeypatch.setattr( + config, 'SSH_OUTPUT_MAX_UNACKED_BYTES_GLOBAL', size + ) + controller.register_socket('expired-holder') + controller.register_socket('healthy') + controller.reserve('expired-holder', 8, 'old-session', size) + socketio = FakeSocketIO(['healthy']) + + output_flow.emit_ssh_output( + socketio, 'user_7', 7, 'healthy-session', payload + ) + + assert socketio.server.disconnected == [('expired-holder', '/')] + assert [entry[2] for entry in socketio.emitted] == ['healthy'] + socketio.emitted[0][3]() + assert controller.usage()['reservations'] == 0 + + +def test_user_and_global_event_budgets_bound_small_callbacks(monkeypatch): + import config + import app.ssh_output_flow as output_flow + + size = 1 + _configure_limits(monkeypatch, 1000, events=8, timeout=0.01) + monkeypatch.setattr(config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER', 1) + controller = output_flow.SSHOutputFlowController() + controller.register_socket('user-a') + controller.register_socket('user-b') + controller.reserve('user-a', 7, 's1', size) + token, reason = controller.reserve('user-b', 7, 's2', size) + assert token is None + assert reason == 'timeout' + + controller.release_socket('user-a') + controller.register_socket('user-a') + monkeypatch.setattr(config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_PER_USER', 8) + monkeypatch.setattr(config, 'SSH_OUTPUT_MAX_UNACKED_EVENTS_GLOBAL', 1) + controller.reserve('user-a', 7, 's1', size) + token, reason = controller.reserve('user-b', 8, 's2', size) + assert token is None + assert reason == 'timeout' + + +def test_failed_socket_disconnect_does_not_free_retained_callbacks( + monkeypatch): + import app.ssh_output_flow as output_flow + + controller = output_flow.SSHOutputFlowController() + monkeypatch.setattr(output_flow, 'ssh_output_flow', controller) + _configure_limits(monkeypatch, 1000) + controller.register_socket('browser') + controller.reserve('browser', 7, 's1', 100) + socketio = FakeSocketIO(['browser']) + socketio.server.disconnect = lambda *_args, **_kwargs: (_ for _ in ()).throw( + RuntimeError('disconnect failed') + ) + + with pytest.raises(RuntimeError, match='disconnect failed'): + output_flow._disconnect_lagging_socket(socketio, 'browser') + + assert controller.usage()['reservations'] == 1 From c6016c1d93543bd28d9ea372aea6ffea078432bd Mon Sep 17 00:00:00 2001 From: bifrost0x Date: Fri, 4 Sep 2026 07:20:57 +0200 Subject: [PATCH 2/6] Restore clients after SSH output eviction --- app/ssh_output_flow.py | 10 +++++ static/js/app.js | 11 ++++- static/js/socket-reconnect-policy.js | 47 +++++++++++++++++++ templates/index.html | 1 + tests/js/browser-error-reporting.test.js | 4 ++ tests/js/socket-reconnect-policy.test.js | 57 ++++++++++++++++++++++++ tests/test_ssh_output_flow.py | 19 ++++++-- 7 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 static/js/socket-reconnect-policy.js create mode 100644 tests/js/socket-reconnect-policy.test.js diff --git a/app/ssh_output_flow.py b/app/ssh_output_flow.py index 422a92eb..91fa6d10 100644 --- a/app/ssh_output_flow.py +++ b/app/ssh_output_flow.py @@ -214,6 +214,16 @@ def _disconnect_lagging_socket(socketio_instance, socket_sid): server = getattr(socketio_instance, 'server', None) if server is None: return False + try: + socketio_instance.emit( + 'ssh_output_resync_required', + {'reason': 'backpressure'}, + to=socket_sid, + ) + except Exception: + # The disconnect still releases server resources if the advisory + # marker cannot be delivered over an already-broken transport. + pass server.disconnect(socket_sid, namespace='/') # Production disconnect handlers release first; keep this idempotent # fallback for test servers and disconnects without an application event. diff --git a/static/js/app.js b/static/js/app.js index 5fb8a5c8..d7af18b2 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -12,6 +12,9 @@ const APP_ROOT = document.querySelector('meta[name="app-root"]')?.content || ''; window.APP_ROOT = APP_ROOT; window.socket = io({ path: APP_ROOT + '/socket.io' }); + const outputFlowReconnect = window.WebSSHSocketReconnect.create( + window.socket + ); window.escapeHtml = function(text) { if (!text) return ''; @@ -1032,6 +1035,7 @@ window.FilePreview = FilePreview; socket.on('connect', () => { + outputFlowReconnect.clear(); const reconnectBar = document.getElementById('reconnectBar'); if (reconnectBar && reconnectBar.style.display !== 'none') { reconnectBar.style.display = 'none'; @@ -1120,7 +1124,11 @@ } }); - socket.on('disconnect', () => { + socket.on('ssh_output_resync_required', () => { + outputFlowReconnect.expectOutputResync(); + }); + + socket.on('disconnect', (reason) => { closeAuthBannerPrompt(); showNotification( window.i18n @@ -1136,6 +1144,7 @@ clearInterval(keepAliveInterval); keepAliveInterval = null; } + outputFlowReconnect.handleDisconnect(reason); }); socket.on('ssh_connected', (data) => { diff --git a/static/js/socket-reconnect-policy.js b/static/js/socket-reconnect-policy.js new file mode 100644 index 00000000..a5aaaa6a --- /dev/null +++ b/static/js/socket-reconnect-policy.js @@ -0,0 +1,47 @@ +(function (root, factory) { + const api = factory(); + if (typeof module === 'object' && module.exports) { + module.exports = api; + } + if (root) { + root.WebSSHSocketReconnect = api; + } +})(typeof window !== 'undefined' ? window : globalThis, function () { + 'use strict'; + + function create(socket, options = {}) { + const schedule = options.schedule || setTimeout; + const cancel = options.cancel || clearTimeout; + const markerLifetime = options.markerLifetime || 5000; + let pendingOutputResync = false; + let markerTimer = null; + + function clear() { + pendingOutputResync = false; + if (markerTimer !== null) { + cancel(markerTimer); + markerTimer = null; + } + } + + function expectOutputResync() { + clear(); + pendingOutputResync = true; + markerTimer = schedule(clear, markerLifetime); + } + + function handleDisconnect(reason) { + const shouldReconnect = ( + pendingOutputResync && reason === 'io server disconnect' + ); + clear(); + if (!shouldReconnect) return false; + socket.connect(); + return true; + } + + return { clear, expectOutputResync, handleDisconnect }; + } + + return { create }; +}); diff --git a/templates/index.html b/templates/index.html index 7b702f57..12a8301b 100644 --- a/templates/index.html +++ b/templates/index.html @@ -1456,6 +1456,7 @@

File Preview

+