diff --git a/application/api/internal/routes.py b/application/api/internal/routes.py index 8a771585..e531113a 100755 --- a/application/api/internal/routes.py +++ b/application/api/internal/routes.py @@ -26,12 +26,20 @@ internal = Blueprint("internal", __name__) @internal.before_request def verify_internal_key(): - """Verify INTERNAL_KEY for all internal endpoint requests.""" - if settings.INTERNAL_KEY: - internal_key = request.headers.get("X-Internal-Key") - if not internal_key or internal_key != settings.INTERNAL_KEY: - logger.warning(f"Unauthorized internal API access attempt from {request.remote_addr}") - return jsonify({"error": "Unauthorized", "message": "Invalid or missing internal key"}), 401 + """Verify INTERNAL_KEY for all internal endpoint requests. + + Deny by default: if INTERNAL_KEY is not configured, reject all requests. + """ + if not settings.INTERNAL_KEY: + logger.warning( + f"Internal API request rejected from {request.remote_addr}: " + "INTERNAL_KEY is not configured" + ) + return jsonify({"error": "Unauthorized", "message": "Internal API is not configured"}), 401 + internal_key = request.headers.get("X-Internal-Key") + if not internal_key or internal_key != settings.INTERNAL_KEY: + logger.warning(f"Unauthorized internal API access attempt from {request.remote_addr}") + return jsonify({"error": "Unauthorized", "message": "Invalid or missing internal key"}), 401 @internal.route("/api/download", methods=["get"]) diff --git a/tests/api/test_internal_routes.py b/tests/api/test_internal_routes.py index efa5a934..ba2aaabf 100644 --- a/tests/api/test_internal_routes.py +++ b/tests/api/test_internal_routes.py @@ -48,7 +48,7 @@ def internal_app(monkeypatch, mock_mongo_db): @pytest.mark.unit class TestVerifyInternalKey: - def test_no_internal_key_configured_allows_access( + def test_no_internal_key_configured_rejects_access( self, internal_app, monkeypatch ): app, db = internal_app @@ -63,9 +63,8 @@ class TestVerifyInternalKey: ), ) with app.test_client() as client: - # download will fail for missing file but should not be 401 resp = client.get("/api/download?user=u&name=n&file=f") - assert resp.status_code != 401 + assert resp.status_code == 401 def test_missing_key_returns_401(self, internal_app, monkeypatch): app, db = internal_app @@ -131,9 +130,12 @@ class TestVerifyInternalKey: @pytest.mark.unit class TestUploadIndex: + _TEST_INTERNAL_KEY = "test-internal-key" + _AUTH_HEADERS = {"X-Internal-Key": "test-internal-key"} + def _make_settings(self, vector_store="faiss"): return MagicMock( - INTERNAL_KEY=None, + INTERNAL_KEY=self._TEST_INTERNAL_KEY, UPLOAD_FOLDER="uploads", VECTOR_STORE=vector_store, EMBEDDINGS_NAME="test_embeddings", @@ -146,7 +148,7 @@ class TestUploadIndex: "application.api.internal.routes.settings", self._make_settings() ) with app.test_client() as client: - resp = client.post("/api/upload_index", data={}) + resp = client.post("/api/upload_index", data={}, headers=self._AUTH_HEADERS) assert resp.json["status"] == "no user" def test_missing_name_returns_no_name(self, internal_app, monkeypatch): @@ -155,7 +157,7 @@ class TestUploadIndex: "application.api.internal.routes.settings", self._make_settings() ) with app.test_client() as client: - resp = client.post("/api/upload_index", data={"user": "testuser"}) + resp = client.post("/api/upload_index", data={"user": "testuser"}, headers=self._AUTH_HEADERS) assert resp.json["status"] == "no name" def test_creates_new_source_entry(self, internal_app, monkeypatch): @@ -182,6 +184,7 @@ class TestUploadIndex: "id": doc_id, "type": "local", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -219,6 +222,7 @@ class TestUploadIndex: "id": str(doc_id), "type": "remote", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -252,6 +256,7 @@ class TestUploadIndex: "type": "local", "directory_structure": json.dumps(dir_struct), }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -285,6 +290,7 @@ class TestUploadIndex: "type": "local", "directory_structure": "not valid json", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -317,6 +323,7 @@ class TestUploadIndex: "type": "local", "file_name_map": json.dumps(fmap), }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -349,6 +356,7 @@ class TestUploadIndex: "id": doc_id, "type": "local", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "no file" @@ -379,6 +387,7 @@ class TestUploadIndex: "type": "local", "file_faiss": (io.BytesIO(b""), ""), }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "no file name" @@ -408,6 +417,7 @@ class TestUploadIndex: "remote_data": '{"url":"http://example.com"}', "sync_frequency": "daily", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -443,6 +453,7 @@ class TestUploadIndex: "file_pkl": (io.BytesIO(b"pkl data"), "index.pkl"), }, content_type="multipart/form-data", + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -477,6 +488,7 @@ class TestUploadIndex: "file_faiss": (io.BytesIO(b"faiss data"), "index.faiss"), }, content_type="multipart/form-data", + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "no file" @@ -508,9 +520,27 @@ class TestUploadIndex: "file_pkl": (io.BytesIO(b""), ""), }, content_type="multipart/form-data", + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "no file name" + def test_no_internal_key_rejects_upload(self, internal_app, monkeypatch): + """Verify that upload_index is rejected when INTERNAL_KEY is not set.""" + app, db = internal_app + monkeypatch.setattr( + "application.api.internal.routes.settings", + MagicMock( + INTERNAL_KEY=None, + UPLOAD_FOLDER="uploads", + VECTOR_STORE="faiss", + EMBEDDINGS_NAME="test", + MONGO_DB_NAME="docsgpt", + ), + ) + with app.test_client() as client: + resp = client.post("/api/upload_index", data={"user": "attacker"}) + assert resp.status_code == 401 + def test_update_existing_with_file_name_map(self, internal_app, monkeypatch): """Cover line 124: update existing entry with file_name_map.""" app, db = internal_app @@ -540,6 +570,7 @@ class TestUploadIndex: "type": "local", "file_name_map": json.dumps(fmap), }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" @@ -572,6 +603,7 @@ class TestUploadIndex: "type": "local", "file_name_map": "not valid json{{{", }, + headers=self._AUTH_HEADERS, ) assert resp.json["status"] == "ok" diff --git a/tests/test_coverage_gaps.py b/tests/test_coverage_gaps.py index 3a0a3d2f..043e243b 100644 --- a/tests/test_coverage_gaps.py +++ b/tests/test_coverage_gaps.py @@ -2105,23 +2105,35 @@ class TestInternalRoutes: app.register_blueprint(internal) return app + _TEST_KEY = "test-key" + _AUTH_HEADERS = {"X-Internal-Key": "test-key"} + def test_upload_index_no_user(self, app): with app.test_client() as client: with patch( "application.api.internal.routes.settings" ) as ms: - ms.INTERNAL_KEY = None - resp = client.post("/api/upload_index") + ms.INTERNAL_KEY = self._TEST_KEY + resp = client.post("/api/upload_index", headers=self._AUTH_HEADERS) assert resp.get_json()["status"] == "no user" def test_upload_index_no_name(self, app): + with app.test_client() as client: + with patch( + "application.api.internal.routes.settings" + ) as ms: + ms.INTERNAL_KEY = self._TEST_KEY + resp = client.post("/api/upload_index", data={"user": "u1"}, headers=self._AUTH_HEADERS) + assert resp.get_json()["status"] == "no name" + + def test_upload_index_rejected_without_internal_key(self, app): with app.test_client() as client: with patch( "application.api.internal.routes.settings" ) as ms: ms.INTERNAL_KEY = None resp = client.post("/api/upload_index", data={"user": "u1"}) - assert resp.get_json()["status"] == "no name" + assert resp.status_code == 401 # ---------------------------------------------------------------------------