diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 2b5d819c..27f3bc8d 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -90,3 +90,8 @@ **Vulnerability:** The `_safe_upload_filename` function used `filename.replace`, `PurePosixPath`, and `re.sub` on unbounded client input, making it vulnerable to ReDoS or CPU/memory exhaustion (DoS) when fed extremely long strings. **Learning:** Even fast standard library functions like `PurePosixPath` and string replacements can cause significant lag when chained on strings in the megabytes. String processing operations should always bound their inputs first if the input is untrusted and can be arbitrarily large. **Prevention:** Cap the length of client-provided filename strings early by slicing them (e.g. `filename = filename[-512:]`) before doing more complex string parsing or regex replacements, especially when only the basename suffix is relevant. + +## 2024-11-20 - Add Permissions-Policy security header +**Vulnerability:** Missing `Permissions-Policy` header allows potential abuse of sensitive browser features (e.g. geolocation, camera, microphone) if the API response is rendered or framed in a browser context. +**Learning:** Adding a restrictive `Permissions-Policy` header provides defense in depth by explicitly disabling unnecessary sensitive browser features for the API response. +**Prevention:** Always include `Permissions-Policy: geolocation=(), camera=(), microphone=()` along with other standard security headers like `Content-Security-Policy` and `X-Frame-Options`. diff --git a/src/newsdom_api/main.py b/src/newsdom_api/main.py index f61aafc2..46df2fcf 100644 --- a/src/newsdom_api/main.py +++ b/src/newsdom_api/main.py @@ -70,6 +70,7 @@ def _apply_security_headers(response: Response, request: Request) -> Response: ) response.headers["Referrer-Policy"] = "no-referrer" response.headers["Cache-Control"] = "no-store, no-cache, max-age=0" + response.headers["Permissions-Policy"] = "geolocation=(), camera=(), microphone=()" forwarded_proto = request.headers.get("x-forwarded-proto", "") is_https = request.url.scheme == "https" or forwarded_proto.lower() == "https" if is_https: diff --git a/src/newsdom_api/schemas.py b/src/newsdom_api/schemas.py index d4295412..f6695a31 100644 --- a/src/newsdom_api/schemas.py +++ b/src/newsdom_api/schemas.py @@ -97,7 +97,9 @@ class ArticleNode(BaseModel): body_blocks: List[str] = Field( default_factory=list, description="Ordered text blocks that make up the article body.", - json_schema_extra={"example": ["First paragraph of the article.", "Second paragraph."]}, + json_schema_extra={ + "example": ["First paragraph of the article.", "Second paragraph."] + }, ) images: List[ImageNode] = Field( default_factory=list, diff --git a/tests/test_auth.py b/tests/test_auth.py index 3dc8311b..b9731c98 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -28,9 +28,7 @@ security_boundary_middleware, ) -_PDF_FILES = { - "file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf") -} +_PDF_FILES = {"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")} def _settings( @@ -147,9 +145,12 @@ def test_development_disabled_mode_allows_parse_and_warns_once( assert client.post("/parse", files=_PDF_FILES).status_code == 200 assert parser_spy["count"] == 2 messages = [record.getMessage() for record in caplog.records] - assert messages.count( - "Parser authentication is disabled for the explicit development profile" - ) == 1 + assert ( + messages.count( + "Parser authentication is disabled for the explicit development profile" + ) + == 1 + ) def test_direct_runtime_settings_reject_invalid_security_invariants() -> None: @@ -350,11 +351,15 @@ def test_concurrent_requests_cannot_switch_authentication_state( ) def request(token: str) -> int: - return TestClient(application).post( - "/parse", - files=_PDF_FILES, - headers={"Authorization": f"Bearer {token}"}, - ).status_code + return ( + TestClient(application) + .post( + "/parse", + files=_PDF_FILES, + headers={"Authorization": f"Bearer {token}"}, + ) + .status_code + ) tokens = ["fixed" if index % 2 == 0 else "changed" for index in range(20)] with ThreadPoolExecutor(max_workers=8) as executor: diff --git a/tests/test_auth_deployment_contract.py b/tests/test_auth_deployment_contract.py index 33d862b9..2053091d 100644 --- a/tests/test_auth_deployment_contract.py +++ b/tests/test_auth_deployment_contract.py @@ -27,7 +27,7 @@ def _project_version(pyproject_text: str) -> str: ) match = ( re.search( - r'''^version\s*=\s*(["'])([^"']+)\1\s*(?:#.*)?$''', + r"""^version\s*=\s*(["'])([^"']+)\1\s*(?:#.*)?$""", project_table.group("body"), re.MULTILINE, ) diff --git a/tests/test_auth_fail_closed_contract.py b/tests/test_auth_fail_closed_contract.py index 3c565cb7..6bafc4d2 100644 --- a/tests/test_auth_fail_closed_contract.py +++ b/tests/test_auth_fail_closed_contract.py @@ -11,9 +11,7 @@ ) from newsdom_api.main import create_app -_PDF_FILES = { - "file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf") -} +_PDF_FILES = {"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")} def test_default_configuration_without_token_blocks_parser_before_work( @@ -36,12 +34,10 @@ def fake_parse_pdf(*_args, **_kwargs): monkeypatch.setattr("newsdom_api.main._validate_pdf_structure", lambda _: None) monkeypatch.setattr("newsdom_api.main.parse_pdf", fake_parse_pdf) - application = create_app( - settings, runtime_readiness_probe=lambda: True + application = create_app(settings, runtime_readiness_probe=lambda: True) + response = TestClient(application, raise_server_exceptions=False).post( + "/parse", files=_PDF_FILES ) - response = TestClient( - application, raise_server_exceptions=False - ).post("/parse", files=_PDF_FILES) assert response.status_code == 503 assert response.json() == {"detail": "Service Unavailable"} @@ -58,13 +54,9 @@ def test_ready_fails_closed_when_required_authentication_is_unconfigured( runtime_profile=RuntimeProfile.PRODUCTION, api_token=None, ) - application = create_app( - settings, runtime_readiness_probe=lambda: True - ) + application = create_app(settings, runtime_readiness_probe=lambda: True) - response = TestClient( - application, raise_server_exceptions=False - ).get("/ready") + response = TestClient(application, raise_server_exceptions=False).get("/ready") assert response.status_code == 503 assert response.json() == {"detail": "Service Unavailable"} @@ -82,13 +74,9 @@ def test_health_remains_liveness_only_when_authentication_is_unconfigured( runtime_profile=RuntimeProfile.PRODUCTION, api_token=None, ) - application = create_app( - settings, runtime_readiness_probe=lambda: False - ) + application = create_app(settings, runtime_readiness_probe=lambda: False) - response = TestClient( - application, raise_server_exceptions=False - ).get("/health") + response = TestClient(application, raise_server_exceptions=False).get("/health") assert response.status_code == 200 assert response.json() == {"status": "ok"} diff --git a/tests/test_auth_protocol_edges.py b/tests/test_auth_protocol_edges.py index dbd733be..6021a9a8 100644 --- a/tests/test_auth_protocol_edges.py +++ b/tests/test_auth_protocol_edges.py @@ -12,9 +12,7 @@ ) from newsdom_api.main import create_app -_PDF_FILES = { - "file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf") -} +_PDF_FILES = {"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")} _BEARER_PREFIX = "Bearer " diff --git a/tests/test_health.py b/tests/test_health.py index 01a3b06f..16b7b336 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -22,6 +22,10 @@ def test_healthcheck(): response.headers.get("Strict-Transport-Security") == "max-age=31536000; includeSubDomains" ) + assert ( + response.headers.get("Permissions-Policy") + == "geolocation=(), camera=(), microphone=()" + ) def test_healthcheck_omits_hsts_for_plain_http(): diff --git a/tests/test_parse_endpoint.py b/tests/test_parse_endpoint.py index 1491ada0..7099d89c 100644 --- a/tests/test_parse_endpoint.py +++ b/tests/test_parse_endpoint.py @@ -418,6 +418,10 @@ def fake_parse_pdf_bytes(file_path, filename, **kwargs): ) assert response.headers.get("Referrer-Policy") == "no-referrer" assert response.headers.get("Cache-Control") == "no-store, no-cache, max-age=0" + assert ( + response.headers.get("Permissions-Policy") + == "geolocation=(), camera=(), microphone=()" + ) assert "Strict-Transport-Security" not in response.headers diff --git a/tests/test_project_metadata.py b/tests/test_project_metadata.py index 324cb086..114c1fb6 100644 --- a/tests/test_project_metadata.py +++ b/tests/test_project_metadata.py @@ -172,12 +172,10 @@ def test_project_declares_python_compatible_locked_fuzz_extra(): assert "fuzz = [" in text assert ( - '"atheris==3.0.0 ; platform_system == \'Linux\' and ' - 'python_version == \'3.11\'"' + "\"atheris==3.0.0 ; platform_system == 'Linux' and python_version == '3.11'\"" ) in text assert ( - '"atheris==3.1.0 ; platform_system == \'Linux\' and ' - 'python_version >= \'3.12\'"' + "\"atheris==3.1.0 ; platform_system == 'Linux' and python_version >= '3.12'\"" ) in text assert _locked_package_versions("atheris") == {(3, 0, 0), (3, 1, 0)} assert '"pyinstaller==6.21.0"' in text diff --git a/tests/test_pypdf_security_floor.py b/tests/test_pypdf_security_floor.py index 6a641e83..56b741c8 100644 --- a/tests/test_pypdf_security_floor.py +++ b/tests/test_pypdf_security_floor.py @@ -71,9 +71,7 @@ def test_trivy_registry_exception_is_scoped_to_the_example_manifest() -> None: ignore_document = yaml.safe_load( Path(".trivyignore.yaml").read_text(encoding="utf-8") ) - exceptions = { - entry["id"]: entry for entry in ignore_document["misconfigurations"] - } + exceptions = {entry["id"]: entry for entry in ignore_document["misconfigurations"]} assert trivy_config["ignorefile"] == ".trivyignore.yaml" assert exceptions["KSV-0125"]["paths"] == [