diff --git a/changedetectionio/api/Import.py b/changedetectionio/api/Import.py index 81b3d29b1..3f64739cd 100644 --- a/changedetectionio/api/Import.py +++ b/changedetectionio/api/Import.py @@ -9,6 +9,10 @@ import json # Number of URLs above which import switches to background processing IMPORT_SWITCH_TO_BACKGROUND_THRESHOLD = 20 +# Query params whose accepted values are wider than the `enum:` in the spec and are checked +# separately further down (plugin processors aren't listed in the static spec enum). +ENUM_VALIDATED_ELSEWHERE = {'processor'} + def default_content_type(content_type='text/plain'): """Decorator to set a default Content-Type header if none is provided.""" @@ -143,10 +147,23 @@ class Import(Resource): # Convert to appropriate type based on schema try: converted_value = convert_query_param_to_type(param_value, schema_properties[param_name]) - extras[param_name] = converted_value except (ValueError, json.JSONDecodeError) as e: return f"Invalid value for parameter '{param_name}': {str(e)}", 400 + # Enforce any `enum:` declared in the OpenAPI spec (notification_format, method, + # conditions_match_logic ...). /api/v1/watch gets this for free because its JSON body is + # unmarshalled against the spec, but the import query params never were - so something + # like ?notification_format=Text used to be stored verbatim and then raise + # "Invalid notification format" at notification-send time, long after the import. + # `processor` is exempt: plugin processors are legal but aren't in the static spec enum, + # it has its own check against available_processors() below. + allowed_values = schema_properties[param_name].get('enum') + if allowed_values and param_name not in ENUM_VALIDATED_ELSEWHERE and converted_value not in allowed_values: + return (f"Invalid value for parameter '{param_name}': '{param_value}'. " + f"Must be one of: {', '.join(str(v) for v in allowed_values)}"), 400 + + extras[param_name] = converted_value + # Validate processor if provided if 'processor' in extras: from changedetectionio.processors import available_processors diff --git a/changedetectionio/tests/test_api_openapi.py b/changedetectionio/tests/test_api_openapi.py index 5628cc7f0..3c99c2c3d 100644 --- a/changedetectionio/tests/test_api_openapi.py +++ b/changedetectionio/tests/test_api_openapi.py @@ -56,6 +56,62 @@ def test_openapi_merged_spec_contains_restock_fields(): f"WatchBase.processor_config_restock_diff should $ref the schema, got: {ref}" +def test_openapi_notification_format_enum_matches_code(): + """ + Unit test: the notification_format enum in api-spec.yaml must stay in step with + valid_notification_formats, otherwise the API either rejects a format the UI offers or + accepts one that blows up when the notification object is built. + """ + from changedetectionio.api import build_merged_spec_dict + from changedetectionio.notification import valid_notification_formats + + spec = build_merged_spec_dict() + spec_enum = spec['components']['schemas']['WatchBase']['properties']['notification_format'].get('enum') + + assert spec_enum is not None, "notification_format must declare an enum in api-spec.yaml" + assert set(spec_enum) == set(valid_notification_formats.keys()), \ + (f"api-spec.yaml notification_format enum {spec_enum} does not match " + f"valid_notification_formats {list(valid_notification_formats.keys())}") + + +def test_openapi_import_rejects_invalid_enum_query_param(client, live_server, measure_memory_usage, datastore_path): + """ + /api/v1/import takes watch config as query params - those must honour the `enum:` in the spec. + 'Text' is the classic one: it was the display name in an old release, so people still pass it, + and a stored 'Text' makes every notification for that watch raise ValueError at send time. + """ + api_key = live_server.app.config['DATASTORE'].data['settings']['application'].get('api_access_token') + + res = client.post( + url_for("import") + "?notification_format=Text", + data='https://website1.com', + headers={'x-api-key': api_key, 'content-type': 'text/plain'}, + ) + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + assert b'notification_format' in res.data + assert not live_server.app.config['DATASTORE'].data['watching'], "Nothing should have been imported" + + # Another enum field on the same code path + res = client.post( + url_for("import") + "?method=FETCH", + data='https://website1.com', + headers={'x-api-key': api_key, 'content-type': 'text/plain'}, + ) + assert res.status_code == 400, f"Expected 400 but got {res.status_code}" + + # ...and a valid value still imports and is stored + res = client.post( + url_for("import") + "?notification_format=htmlcolor", + data='https://website1.com', + headers={'x-api-key': api_key, 'content-type': 'text/plain'}, + ) + assert res.status_code == 200, f"Expected 200 but got {res.status_code}" + watch = live_server.app.config['DATASTORE'].data['watching'][res.json[0]] + assert watch.get('notification_format') == 'htmlcolor' + + delete_all_watches(client) + + def test_openapi_validation_invalid_content_type_on_create_watch(client, live_server, measure_memory_usage, datastore_path): """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')