From 9fe3608a34259bc95b99f18d1bcdfebccdfd1410 Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Wed, 18 Feb 2026 17:27:22 +0530 Subject: [PATCH 1/3] security: protect /openJupyter endpoint with API key and process control (fixes #361) --- fri/server/main.py | 56 +++++++++-- tests/test_openjupyter_security.py | 152 +++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 9 deletions(-) create mode 100644 tests/test_openjupyter_security.py diff --git a/fri/server/main.py b/fri/server/main.py index c2e1e659..3f15a3fd 100644 --- a/fri/server/main.py +++ b/fri/server/main.py @@ -84,6 +84,20 @@ def get_error_output(e): cur_path = os.path.dirname(os.path.abspath(__file__)) concore_path = os.path.abspath(os.path.join(cur_path, '../../')) +# API key authentication for sensitive endpoints +API_KEY = os.environ.get("CONCORE_API_KEY") + +def require_api_key(): + """Require a valid API key via X-API-KEY header.""" + if not API_KEY: + abort(500, description="Server not configured with API key") + provided = request.headers.get("X-API-KEY") + if provided != API_KEY: + abort(403, description="Unauthorized") + +# Track single Jupyter process to prevent multiple concurrent launches +jupyter_process = None + app = Flask(__name__) secret_key = os.environ.get("FLASK_SECRET_KEY") @@ -536,15 +550,39 @@ def getFilesList(dir): @app.route('/openJupyter/', methods=['POST']) def openJupyter(): - proc = subprocess.Popen(['jupyter', 'lab'], shell=False, stdout=subprocess.PIPE, cwd=concore_path) - if proc.poll() is None: - resp = jsonify({'message': 'Successfuly opened Jupyter'}) - resp.status_code = 308 - return resp - else: - resp = jsonify({'message': 'There is an Error'}) - resp.status_code = 500 - return resp + global jupyter_process + + require_api_key() + + if jupyter_process and jupyter_process.poll() is None: + return jsonify({"message": "Jupyter already running"}), 409 + + try: + jupyter_process = subprocess.Popen( + ['jupyter', 'lab', '--no-browser'], + shell=False, + cwd=concore_path, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL + ) + return jsonify({"message": "Jupyter Lab started"}), 200 + except Exception: + return jsonify({"error": "Failed to start Jupyter"}), 500 + + +@app.route('/stopJupyter/', methods=['POST']) +def stopJupyter(): + global jupyter_process + + require_api_key() + + if not jupyter_process or jupyter_process.poll() is not None: + return jsonify({"message": "No running Jupyter instance"}), 404 + + jupyter_process.terminate() + jupyter_process = None + + return jsonify({"message": "Jupyter stopped"}), 200 if __name__ == "__main__": diff --git a/tests/test_openjupyter_security.py b/tests/test_openjupyter_security.py new file mode 100644 index 00000000..204f5b98 --- /dev/null +++ b/tests/test_openjupyter_security.py @@ -0,0 +1,152 @@ +"""Tests for the secured /openJupyter/ and /stopJupyter/ endpoints.""" +import os +import sys +import pytest +from unittest.mock import patch, MagicMock + +# Ensure the project root is on the path +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +# Set a test API key before importing the app module +TEST_API_KEY = "test-secret-key-12345" + + +@pytest.fixture(autouse=True) +def reset_jupyter_process(): + """Reset the module-level jupyter_process before each test.""" + import fri.server.main as mod + mod.jupyter_process = None + yield + mod.jupyter_process = None + + +@pytest.fixture +def client(): + """Create a Flask test client with the API key configured.""" + with patch.dict(os.environ, {"CONCORE_API_KEY": TEST_API_KEY}): + # Re-read env var after patching + import fri.server.main as mod + mod.API_KEY = TEST_API_KEY + mod.app.config["TESTING"] = True + with mod.app.test_client() as c: + yield c + + +@pytest.fixture +def client_no_key(): + """Create a Flask test client without API key configured.""" + import fri.server.main as mod + mod.API_KEY = None + mod.app.config["TESTING"] = True + with mod.app.test_client() as c: + yield c + + +class TestOpenJupyterAuth: + """Test authentication on /openJupyter/ endpoint.""" + + def test_missing_api_key_header_returns_403(self, client): + """Request without X-API-KEY header should be rejected.""" + resp = client.post("/openJupyter/") + assert resp.status_code == 403 + + def test_wrong_api_key_returns_403(self, client): + """Request with wrong key should be rejected.""" + resp = client.post("/openJupyter/", headers={"X-API-KEY": "wrong-key"}) + assert resp.status_code == 403 + + def test_server_without_api_key_configured_returns_500(self, client_no_key): + """If CONCORE_API_KEY is not set on server, return 500.""" + resp = client_no_key.post( + "/openJupyter/", headers={"X-API-KEY": "anything"} + ) + assert resp.status_code == 500 + + +class TestOpenJupyterProcess: + """Test process control on /openJupyter/ endpoint.""" + + @patch("fri.server.main.subprocess.Popen") + def test_authorized_request_starts_jupyter(self, mock_popen, client): + """Valid API key should start Jupyter Lab.""" + mock_proc = MagicMock() + mock_proc.poll.return_value = None # process running + mock_popen.return_value = mock_proc + + resp = client.post( + "/openJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp.status_code == 200 + data = resp.get_json() + assert data["message"] == "Jupyter Lab started" + + # Verify Popen was called with --no-browser and DEVNULL + call_args = mock_popen.call_args + assert "--no-browser" in call_args[0][0] + assert call_args[1].get("shell") is False + + @patch("fri.server.main.subprocess.Popen") + def test_duplicate_launch_returns_409(self, mock_popen, client): + """Second launch while first is still running should return 409.""" + mock_proc = MagicMock() + mock_proc.poll.return_value = None # still running + mock_popen.return_value = mock_proc + + # First launch + resp1 = client.post( + "/openJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp1.status_code == 200 + + # Second launch should be rejected + resp2 = client.post( + "/openJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp2.status_code == 409 + data = resp2.get_json() + assert data["message"] == "Jupyter already running" + + @patch("fri.server.main.subprocess.Popen", side_effect=Exception("fail")) + def test_popen_failure_returns_500(self, mock_popen, client): + """If Popen raises, return 500.""" + resp = client.post( + "/openJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp.status_code == 500 + data = resp.get_json() + assert "error" in data + + +class TestStopJupyter: + """Test /stopJupyter/ endpoint.""" + + def test_stop_without_auth_returns_403(self, client): + """Request without API key should be rejected.""" + resp = client.post("/stopJupyter/") + assert resp.status_code == 403 + + def test_stop_when_no_process_returns_404(self, client): + """Stop with no running process returns 404.""" + resp = client.post( + "/stopJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp.status_code == 404 + + @patch("fri.server.main.subprocess.Popen") + def test_stop_running_process_returns_200(self, mock_popen, client): + """Stop a running Jupyter instance returns 200.""" + mock_proc = MagicMock() + mock_proc.poll.return_value = None # running + mock_popen.return_value = mock_proc + + # Start first + client.post("/openJupyter/", headers={"X-API-KEY": TEST_API_KEY}) + + # Stop + resp = client.post( + "/stopJupyter/", headers={"X-API-KEY": TEST_API_KEY} + ) + assert resp.status_code == 200 + data = resp.get_json() + assert data["message"] == "Jupyter stopped" + mock_proc.terminate.assert_called_once() From 5bf69a253ded4de80949e8bcca880f72539a66fc Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Wed, 18 Feb 2026 17:40:23 +0530 Subject: [PATCH 2/3] test: skip openJupyter security tests when flask is not installed in CI --- tests/test_openjupyter_security.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_openjupyter_security.py b/tests/test_openjupyter_security.py index 204f5b98..d3a1023c 100644 --- a/tests/test_openjupyter_security.py +++ b/tests/test_openjupyter_security.py @@ -7,6 +7,9 @@ # Ensure the project root is on the path sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +# Skip entire module if flask is not installed (e.g. in CI with minimal deps) +pytest.importorskip("flask", reason="flask not installed — skipping server endpoint tests") + # Set a test API key before importing the app module TEST_API_KEY = "test-secret-key-12345" From 51e70762212d1a2a7109fc525880b0d3886033eb Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Wed, 18 Feb 2026 17:48:47 +0530 Subject: [PATCH 3/3] security: address PR review - timing-safe comparison, thread lock, specific exceptions, graceful termination --- fri/server/main.py | 59 +++++++++++++++++++----------- tests/test_openjupyter_security.py | 3 +- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/fri/server/main.py b/fri/server/main.py index 3f15a3fd..2f0fdb2b 100644 --- a/fri/server/main.py +++ b/fri/server/main.py @@ -6,10 +6,15 @@ from subprocess import call,check_output from pathlib import Path import json +import logging import platform import re +import secrets +import threading from flask_cors import CORS, cross_origin +logger = logging.getLogger(__name__) + # Input validation pattern for safe names (alphanumeric, dash, underscore, slash, dot, space) SAFE_INPUT_PATTERN = re.compile(r'^[a-zA-Z0-9_\-/. ]+$') # Pattern for filenames - no path separators or .. allowed @@ -91,12 +96,13 @@ def require_api_key(): """Require a valid API key via X-API-KEY header.""" if not API_KEY: abort(500, description="Server not configured with API key") - provided = request.headers.get("X-API-KEY") - if provided != API_KEY: + provided = request.headers.get("X-API-KEY") or "" + if not secrets.compare_digest(provided, API_KEY): abort(403, description="Unauthorized") # Track single Jupyter process to prevent multiple concurrent launches jupyter_process = None +jupyter_lock = threading.Lock() app = Flask(__name__) @@ -554,20 +560,22 @@ def openJupyter(): require_api_key() - if jupyter_process and jupyter_process.poll() is None: - return jsonify({"message": "Jupyter already running"}), 409 + with jupyter_lock: + if jupyter_process and jupyter_process.poll() is None: + return jsonify({"message": "Jupyter already running"}), 409 - try: - jupyter_process = subprocess.Popen( - ['jupyter', 'lab', '--no-browser'], - shell=False, - cwd=concore_path, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL - ) - return jsonify({"message": "Jupyter Lab started"}), 200 - except Exception: - return jsonify({"error": "Failed to start Jupyter"}), 500 + try: + jupyter_process = subprocess.Popen( + ['jupyter', 'lab', '--no-browser'], + shell=False, + cwd=concore_path, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL + ) + return jsonify({"message": "Jupyter Lab started"}), 200 + except (FileNotFoundError, PermissionError, OSError) as e: + logger.error("Failed to start Jupyter: %s", e) + return jsonify({"error": "Failed to start Jupyter"}), 500 @app.route('/stopJupyter/', methods=['POST']) @@ -576,13 +584,22 @@ def stopJupyter(): require_api_key() - if not jupyter_process or jupyter_process.poll() is not None: - return jsonify({"message": "No running Jupyter instance"}), 404 - - jupyter_process.terminate() - jupyter_process = None + with jupyter_lock: + if not jupyter_process or jupyter_process.poll() is not None: + return jsonify({"message": "No running Jupyter instance"}), 404 - return jsonify({"message": "Jupyter stopped"}), 200 + try: + jupyter_process.terminate() + # Wait for Jupyter to terminate gracefully + jupyter_process.wait(timeout=5) + except subprocess.TimeoutExpired: + # Force kill if it did not exit in time + jupyter_process.kill() + jupyter_process.wait(timeout=5) + finally: + jupyter_process = None + + return jsonify({"message": "Jupyter stopped"}), 200 if __name__ == "__main__": diff --git a/tests/test_openjupyter_security.py b/tests/test_openjupyter_security.py index d3a1023c..230a4819 100644 --- a/tests/test_openjupyter_security.py +++ b/tests/test_openjupyter_security.py @@ -109,7 +109,7 @@ def test_duplicate_launch_returns_409(self, mock_popen, client): data = resp2.get_json() assert data["message"] == "Jupyter already running" - @patch("fri.server.main.subprocess.Popen", side_effect=Exception("fail")) + @patch("fri.server.main.subprocess.Popen", side_effect=OSError("fail")) def test_popen_failure_returns_500(self, mock_popen, client): """If Popen raises, return 500.""" resp = client.post( @@ -153,3 +153,4 @@ def test_stop_running_process_returns_200(self, mock_popen, client): data = resp.get_json() assert data["message"] == "Jupyter stopped" mock_proc.terminate.assert_called_once() + mock_proc.wait.assert_called()