From cc5455c3dca817c5c717288441b41a29ce45b5c7 Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Mon, 15 Sep 2025 13:50:29 +0200 Subject: [PATCH] API - OpenAPI call validation was being skipped on docker based installs, misc API fixes (#3424) --- .dockerignore | 1 - Dockerfile | 5 + changedetectionio/api/__init__.py | 45 +++-- changedetectionio/tests/test_api.py | 2 +- changedetectionio/tests/test_api_openapi.py | 199 ++++++++++++++++++++ docs/api-spec.yaml | 29 ++- 6 files changed, 252 insertions(+), 29 deletions(-) create mode 100644 changedetectionio/tests/test_api_openapi.py diff --git a/.dockerignore b/.dockerignore index 14fba462..282023b2 100644 --- a/.dockerignore +++ b/.dockerignore @@ -33,7 +33,6 @@ venv/ # Test and development files test-datastore/ tests/ -docs/ *.md !README.md diff --git a/Dockerfile b/Dockerfile index 56f63aac..3abf0bbc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -84,6 +84,11 @@ EXPOSE 5000 # The actual flask app module COPY changedetectionio /app/changedetectionio + +# Also for OpenAPI validation wrapper - needs the YML +RUN [ ! -d "/app/docs" ] && mkdir /app/docs +COPY docs/api-spec.yaml /app/docs/api-spec.yaml + # Starting wrapper COPY changedetection.py /app/changedetection.py diff --git a/changedetectionio/api/__init__.py b/changedetectionio/api/__init__.py index 82f6e337..4004e019 100644 --- a/changedetectionio/api/__init__.py +++ b/changedetectionio/api/__init__.py @@ -2,6 +2,7 @@ import copy import yaml import functools from flask import request, abort +from loguru import logger from openapi_core import OpenAPI from openapi_core.contrib.flask import FlaskOpenAPIRequest from . import api_schema @@ -31,17 +32,13 @@ schema_create_notification_urls['required'] = ['notification_urls'] schema_delete_notification_urls = copy.deepcopy(schema_notification_urls) schema_delete_notification_urls['required'] = ['notification_urls'] -# Load OpenAPI spec for validation -_openapi_spec = None - +@functools.cache def get_openapi_spec(): - global _openapi_spec - if _openapi_spec is None: - import os - spec_path = os.path.join(os.path.dirname(__file__), '../../docs/api-spec.yaml') - with open(spec_path, 'r') as f: - spec_dict = yaml.safe_load(f) - _openapi_spec = OpenAPI.from_dict(spec_dict) + import os + spec_path = os.path.join(os.path.dirname(__file__), '../../docs/api-spec.yaml') + with open(spec_path, 'r') as f: + spec_dict = yaml.safe_load(f) + _openapi_spec = OpenAPI.from_dict(spec_dict) return _openapi_spec def validate_openapi_request(operation_id): @@ -50,16 +47,25 @@ def validate_openapi_request(operation_id): @functools.wraps(f) def wrapper(*args, **kwargs): try: - spec = get_openapi_spec() - openapi_request = FlaskOpenAPIRequest(request) - result = spec.unmarshal_request(openapi_request) - if result.errors: - abort(400, message=f"OpenAPI validation failed: {result.errors}") - return f(*args, **kwargs) + # Skip OpenAPI validation for GET requests since they don't have request bodies + if request.method.upper() != 'GET': + spec = get_openapi_spec() + openapi_request = FlaskOpenAPIRequest(request) + result = spec.unmarshal_request(openapi_request) + if result.errors: + from werkzeug.exceptions import BadRequest + error_details = [] + for error in result.errors: + error_details.append(str(error)) + raise BadRequest(f"OpenAPI validation failed: {error_details}") + except BadRequest: + # Re-raise BadRequest exceptions (validation failures) + raise except Exception as e: - # If OpenAPI validation fails, log but don't break existing functionality - print(f"OpenAPI validation warning for {operation_id}: {e}") - return f(*args, **kwargs) + # If OpenAPI spec loading fails, log but don't break existing functionality + logger.critical(f"OpenAPI validation warning for {operation_id}: {e}") + abort(500) + return f(*args, **kwargs) return wrapper return decorator @@ -69,3 +75,4 @@ from .Tags import Tags, Tag from .Import import Import from .SystemInfo import SystemInfo from .Notifications import Notifications + diff --git a/changedetectionio/tests/test_api.py b/changedetectionio/tests/test_api.py index c019b532..fa433980 100644 --- a/changedetectionio/tests/test_api.py +++ b/changedetectionio/tests/test_api.py @@ -396,7 +396,7 @@ def test_api_import(client, live_server, measure_memory_usage): res = client.post( url_for("import") + "?tag=import-test", data='https://website1.com\r\nhttps://website2.com', - headers={'x-api-key': api_key}, + headers={'x-api-key': api_key, 'content-type': 'text/plain'}, follow_redirects=True ) diff --git a/changedetectionio/tests/test_api_openapi.py b/changedetectionio/tests/test_api_openapi.py new file mode 100644 index 00000000..22f440dd --- /dev/null +++ b/changedetectionio/tests/test_api_openapi.py @@ -0,0 +1,199 @@ +#!/usr/bin/env python3 +""" +OpenAPI validation tests for ChangeDetection.io API + +This test file specifically verifies that OpenAPI validation is working correctly +by testing various scenarios that should trigger validation errors. +""" + +import time +import json +from flask import url_for +from .util import live_server_setup, wait_for_all_checks + + +def test_openapi_validation_invalid_content_type_on_create_watch(client, live_server, measure_memory_usage): + """Test that creating a watch with invalid content-type triggers OpenAPI validation error.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Try to create a watch with JSON data but without proper content-type header + res = client.post( + url_for("createwatch"), + data=json.dumps({"url": "https://example.com", "title": "Test Watch"}), + headers={'x-api-key': api_key}, # Missing 'content-type': 'application/json' + follow_redirects=True + ) + + # Should get 400 error due to OpenAPI validation failure + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b"OpenAPI validation failed" in res.data, "Should contain OpenAPI validation error message" + + +def test_openapi_validation_missing_required_field_create_watch(client, live_server, measure_memory_usage): + """Test that creating a watch without required URL field triggers OpenAPI validation error.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Try to create a watch without the required 'url' field + res = client.post( + url_for("createwatch"), + data=json.dumps({"title": "Test Watch Without URL"}), # Missing required 'url' field + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + follow_redirects=True + ) + + # Should get 400 error due to missing required field + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b"OpenAPI validation failed" in res.data, "Should contain OpenAPI validation error message" + + +def test_openapi_validation_invalid_field_in_request_body(client, live_server, measure_memory_usage): + """Test that including invalid fields triggers OpenAPI validation error.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # First create a valid watch + res = client.post( + url_for("createwatch"), + data=json.dumps({"url": "https://example.com", "title": "Test Watch"}), + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + follow_redirects=True + ) + assert res.status_code == 201, "Watch creation should succeed" + + # Get the watch list to find the UUID + res = client.get( + url_for("createwatch"), + headers={'x-api-key': api_key} + ) + assert res.status_code == 200 + watch_uuid = list(res.json.keys())[0] + + # Now try to update the watch with an invalid field + res = client.put( + url_for("watch", uuid=watch_uuid), + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + data=json.dumps({ + "title": "Updated title", + "invalid_field_that_doesnt_exist": "this should cause validation error" + }), + ) + + # Should get 400 error due to invalid field (this will be caught by internal validation) + # Note: This tests the flow where OpenAPI validation passes but internal validation catches it + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b"Additional properties are not allowed" in res.data, "Should contain validation error about additional properties" + + +def test_openapi_validation_import_wrong_content_type(client, live_server, measure_memory_usage): + """Test that import endpoint with wrong content-type triggers OpenAPI validation error.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Try to import URLs with JSON content-type instead of text/plain + res = client.post( + url_for("import") + "?tag=test-import", + data='https://website1.com\nhttps://website2.com', + headers={'x-api-key': api_key, 'content-type': 'application/json'}, # Wrong content-type + follow_redirects=True + ) + + # Should get 400 error due to content-type mismatch + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b"OpenAPI validation failed" in res.data, "Should contain OpenAPI validation error message" + + +def test_openapi_validation_import_correct_content_type_succeeds(client, live_server, measure_memory_usage): + """Test that import endpoint with correct content-type succeeds (positive test).""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Import URLs with correct text/plain content-type + res = client.post( + url_for("import") + "?tag=test-import", + data='https://website1.com\nhttps://website2.com', + headers={'x-api-key': api_key, 'content-type': 'text/plain'}, # Correct content-type + follow_redirects=True + ) + + # Should succeed + assert res.status_code == 200, f"Expected 200 but got {res.status_code}" + assert len(res.json) == 2, "Should import 2 URLs" + + +def test_openapi_validation_get_requests_bypass_validation(client, live_server, measure_memory_usage): + """Test that GET requests bypass OpenAPI validation entirely.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Disable API token requirement first + res = client.post( + url_for("settings.settings_page"), + data={ + "requests-time_between_check-minutes": 180, + "application-fetch_backend": "html_requests", + "application-api_access_token_enabled": "" + }, + follow_redirects=True + ) + assert b"Settings updated." in res.data + + # Make GET request to list watches - should succeed even without API key or content-type + res = client.get(url_for("createwatch")) # No headers needed for GET + assert res.status_code == 200, f"GET requests should succeed without OpenAPI validation, got {res.status_code}" + + # Should return JSON with watch list (empty in this case) + assert isinstance(res.json, dict), "Should return JSON dictionary for watch list" + + +def test_openapi_validation_create_tag_missing_required_title(client, live_server, measure_memory_usage): + """Test that creating a tag without required title triggers OpenAPI validation error.""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # Try to create a tag without the required 'title' field + res = client.post( + url_for("tag"), + data=json.dumps({"notification_urls": ["mailto:test@example.com"]}), # Missing required 'title' field + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + follow_redirects=True + ) + + # Should get 400 error due to missing required field + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b"OpenAPI validation failed" in res.data, "Should contain OpenAPI validation error message" + + +def test_openapi_validation_watch_update_allows_partial_updates(client, live_server, measure_memory_usage): + """Test that watch updates allow partial updates without requiring all fields (positive test).""" + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + # First create a valid watch + res = client.post( + url_for("createwatch"), + data=json.dumps({"url": "https://example.com", "title": "Test Watch"}), + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + follow_redirects=True + ) + assert res.status_code == 201, "Watch creation should succeed" + + # Get the watch list to find the UUID + res = client.get( + url_for("createwatch"), + headers={'x-api-key': api_key} + ) + assert res.status_code == 200 + watch_uuid = list(res.json.keys())[0] + + # Update only the title (partial update) - should succeed + res = client.put( + url_for("watch", uuid=watch_uuid), + headers={'x-api-key': api_key, 'content-type': 'application/json'}, + data=json.dumps({"title": "Updated Title Only"}), # Only updating title, not URL + ) + + # Should succeed because UpdateWatch schema allows partial updates + assert res.status_code == 200, f"Partial updates should succeed, got {res.status_code}" + + # Verify the update worked + res = client.get( + url_for("watch", uuid=watch_uuid), + headers={'x-api-key': api_key} + ) + assert res.status_code == 200 + assert res.json.get('title') == 'Updated Title Only', "Title should be updated" + assert res.json.get('url') == 'https://example.com', "URL should remain unchanged" \ No newline at end of file diff --git a/docs/api-spec.yaml b/docs/api-spec.yaml index 11584984..27bc6bcc 100644 --- a/docs/api-spec.yaml +++ b/docs/api-spec.yaml @@ -1,4 +1,4 @@ -openapi: 3.0.3 +openapi: 3.0.4 info: title: ChangeDetection.io API description: | @@ -224,8 +224,6 @@ components: maxLength: 5000 required: [operation, selector, optional_value] description: Browser automation steps - required: - - url Watch: allOf: @@ -261,6 +259,16 @@ components: required: - url + UpdateWatch: + allOf: + - $ref: '#/components/schemas/WatchBase' + - type: object + properties: + last_viewed: + type: integer + description: Unix timestamp in seconds of the last time the watch was viewed. Setting it to a value higher than `last_changed` in the "Update watch" endpoint marks the watch as viewed. + minimum: 0 + Tag: type: object properties: @@ -281,8 +289,13 @@ components: notification_muted: type: boolean description: Whether notifications are muted for this tag - required: - - title + + CreateTag: + allOf: + - $ref: '#/components/schemas/Tag' + - type: object + required: + - title NotificationUrls: type: object @@ -572,7 +585,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Watch' + $ref: '#/components/schemas/UpdateWatch' responses: '200': description: Web page change monitor (watch) updated successfully @@ -815,7 +828,7 @@ paths: 'Content-Type': 'application/json' } data = {'title': 'Important Sites'} - response = requests.post('http://localhost:5000/api/v1/tag', + response = requests.post('http://localhost:5000/api/v1/tag', headers=headers, json=data) print(response.json()) requestBody: @@ -823,7 +836,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Tag' + $ref: '#/components/schemas/CreateTag' example: title: "Important Sites" responses: