Merge pull request #409 from GaneshPatil7517/security/protect-openjupyter-endpoint

security: protect /openJupyter endpoint with API key and process control (fixes #361)

Author: pradeeban

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
@@ -84,6 +89,21 @@ 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") 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__)
 secret_key = os.environ.get("FLASK_SECRET_KEY")
@@ -543,15 +563,50 @@ 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()
+
+    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 (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'])
+def stopJupyter():
+    global jupyter_process
+
+    require_api_key()
+
+    with jupyter_lock:
+        if not jupyter_process or jupyter_process.poll() is not None:
+            return jsonify({"message": "No running Jupyter instance"}), 404
+
+        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__":

tests/test_openjupyter_security.py

@@ -0,0 +1,156 @@
+"""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__))))
+
+# 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"
+
+
+@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=OSError("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()
+        mock_proc.wait.assert_called()