feat: Implement OCR parameter validation and error handling

README.md

@@ -10,6 +10,8 @@ It's written in Python and provides a simple REST API for [ocrmypdf](https://ocr
   - [Installation](#installation)
   - [`docker-compose` Example](#docker-compose-example)
   - [HaRP Support (Nextcloud 32+)](#harp-support-nextcloud-32)
+  - [OCR API](#ocr-api)
+    - [Legacy `/process_ocr` (deprecated)](#legacy-process_ocr-deprecated)
 
 ## Prerequisites
 
@@ -175,3 +177,37 @@ Since Nextcloud 32, [HaRP (AppAPI HaProxy Reversed Proxy)](https://github.com/ne
 HaRP simplifies deployment and improves performance by enabling direct communication between clients and ExApps. The implementation is fully backward compatible with Docker Socket Proxy deployments.
 
 For installation and migration instructions, see the [HaRP documentation](https://github.com/nextcloud/HaRP#readme).
+
+## OCR API
+
+`POST /v1/ocr` is the current API: a multipart `file` plus an `options` part holding a JSON object
+validated against a typed, closed schema (`OcrOptions`, see
+[`workflow_ocr_backend/ocroptions.py`](workflow_ocr_backend/ocroptions.py)). Unknown fields are
+rejected with `422` rather than forwarded, every scalar is bounded or enumerated, and resource
+limits (`jobs`,
+`max_image_mpixels`, the `tesseract_timeout` ceiling) are operator policy set via environment
+variables (`OCR_JOBS`, `OCR_MAX_IMAGE_MPIXELS`, `OCR_MAX_TESSERACT_TIMEOUT_S`), never part of the
+request body. See `/docs` on a running instance for the generated OpenAPI schema.
+
+### Legacy `/process_ocr` (deprecated)
+
+`POST /process_ocr` accepts the older `ocrmypdf_parameters` flag string (e.g.
+`--skip-text --tesseract-pagesegmode 7 --language eng`) for backward compatibility. It is a thin
+shim: the string is parsed and translated onto the same `OcrOptions` schema `/v1/ocr` uses, so it
+inherits every validation rule from that schema rather than maintaining a separate allow/deny list.
+Responses carry `Deprecation`/`Sunset`/`Link` headers pointing at `/v1/ocr`. Concretely:
+
+- Only an explicit, hand-maintained table of legacy flag names is translated into schema fields.
+  A flag that isn't in that table - `plugins`, `plugin_manager`, `user_words`, `user_patterns`,
+  `keep_temporary_files`, `tesseract_config`, `unpaper_args`, the input/output/sidecar parameters,
+  and any operator-owned resource knob (`jobs`, `max_image_mpixels`) - can never reach OCRmyPDF,
+  because there is no path in the shim that puts it on the schema. `--plugins` in particular would
+  make OCRmyPDF load and execute arbitrary Python code; `--tesseract-config` and `--user-words`
+  would expose the backend's filesystem to the caller.
+- CLI-only flags with no API equivalent (`--quiet`, `--verbose`, `--no-progress-bar`) are accepted
+  and ignored rather than rejected.
+- Language codes must match `^[A-Za-z][A-Za-z0-9_]{0,31}$` (or `script/<Name>`, e.g. `chi_sim`,
+  `script/Latin`), the same allow-list the [workflow_ocr](https://github.com/R0Wi-DEV/workflow_ocr)
+  Nextcloud App uses.
+- Values are parsed with `shlex.split`, so a quoted value survives intact; duplicate flags and
+  unquoted multi-word values are now a `400` instead of being silently truncated or overwritten.

requirements.txt

@@ -1,3 +1,4 @@
 nc-py-api[app]==0.30.1
 ocrmypdf==17.4.2
-python-multipart==0.0.20
+python-multipart==0.0.20
+pydantic==2.13.4

test/ocrmypdf_signature.json

@@ -0,0 +1,21 @@
+{
+  "_comment": "Snapshot of ocrmypdf.ocr() keyword-only parameters. Regenerate DELIBERATELY after reviewing each added parameter for path/plugin/argv semantics.",
+  "ocrmypdf_version": "17.4.2",
+  "keyword_only_parameters": [
+    "author", "clean", "clean_final", "color_conversion_strategy",
+    "continue_on_soft_render_error", "deskew", "fast_web_view", "force_ocr",
+    "image_dpi", "invalidate_digital_signatures", "jbig2_lossy",
+    "jbig2_page_group_size", "jbig2_threshold", "jobs", "jpg_quality",
+    "keep_temporary_files", "keywords", "language",
+    "max_image_mpixels", "mode", "no_overwrite", "optimize", "output_type",
+    "oversample", "pages", "pdf_renderer", "pdfa_image_compression",
+    "plugin_manager", "plugins", "png_quality", "progress_bar", "rasterizer",
+    "redo_ocr", "remove_background", "remove_vectors", "rotate_pages",
+    "rotate_pages_threshold", "sidecar", "skip_big", "skip_text", "subject",
+    "tagged_pdf_mode", "tesseract_config", "tesseract_downsample_above",
+    "tesseract_downsample_large_images", "tesseract_non_ocr_timeout",
+    "tesseract_oem", "tesseract_pagesegmode", "tesseract_thresholding",
+    "tesseract_timeout", "title", "unpaper_args", "use_threads", "user_patterns",
+    "user_words"
+  ]
+}

test/test_app.py

@@ -77,6 +77,106 @@ def test_process_ocr_error_invalid_file():
     assert "ocrMyPdfExitCode" in response_json
     assert response_json["ocrMyPdfExitCode"] == 2
 
+def test_process_ocr_rejects_plugin_parameter(tmp_path):
+    # The "plugins" parameter would make OCRmyPDF load and execute an arbitrary
+    # Python file => must be rejected before ocrmypdf.ocr() is called.
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    marker = tmp_path / "pwned"
+    plugin = tmp_path / "evil_plugin.py"
+    plugin.write_text(f"open({str(marker)!r}, 'w').write('pwned')\n")
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers, raise_server_exceptions=False) as client:
+        response = client.post(
+            "/process_ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"ocrmypdf_parameters": f"--skip-text --plugins {plugin}"}
+        )
+    assert response.status_code == 400
+    assert response.json()["message"].startswith("Parameter 'plugins' is not allowed")
+    assert not marker.exists()
+
+def test_process_ocr_rejects_injected_language():
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers, raise_server_exceptions=False) as client:
+        response = client.post(
+            "/process_ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"ocrmypdf_parameters": "--skip-text --language eng+$(id)"}
+        )
+    assert response.status_code == 400
+    assert response.json()["message"].startswith("Invalid language value '$(id)'")
+
+def test_process_ocr_deprecation_headers():
+    # The legacy endpoint is kept as a shim, but must advertise that it is one.
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers) as client:
+        response = client.post(
+            "/process_ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"ocrmypdf_parameters": "--skip-text --language eng"}
+        )
+    assert response.status_code == 200
+    assert response.headers["Deprecation"] == "true"
+    assert "successor-version" in response.headers["Link"]
+
+def test_ocr_v1():
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    ocr_content = "This document is ready for OCR\n"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers) as client:
+        response = client.post(
+            "/v1/ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"options": '{"mode": "skip-text", "languages": ["eng"], "tesseract_pagesegmode": 7}'}
+        )
+    assert response.status_code == 200
+    response_json = response.json()
+    assert "recognizedText" in response_json
+    assert response_json["recognizedText"] == ocr_content
+
+def test_ocr_v1_defaults_to_english_when_no_options_given():
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    ocr_content = "This document is ready for OCR\n"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers) as client:
+        response = client.post(
+            "/v1/ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"options": '{"mode": "skip-text"}'}
+        )
+    assert response.status_code == 200
+    assert response.json()["recognizedText"] == ocr_content
+
+def test_ocr_v1_rejects_unknown_option_field():
+    # extra="forbid" - an unknown key is a 422, never silently forwarded.
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers, raise_server_exceptions=False) as client:
+        response = client.post(
+            "/v1/ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            data={"options": '{"plugins": "/tmp/evil.py"}'}
+        )
+    assert response.status_code == 422
+    assert response.json()["errors"][0]["loc"] == ["plugins"]
+
+def test_ocr_v1_rejects_uninstalled_language():
+    current_dir = os.path.dirname(__file__)
+    file_name = "document-ready-for-ocr.pdf"
+    with open(f"{current_dir}/testdata/{file_name}", "rb") as file, TestClient(APP, headers=headers, raise_server_exceptions=False) as client:
+        response = client.post(
+            "/v1/ocr",
+            files={"file": (file_name, file, "application/pdf")},
+            # Not a real tesseract language pack, and the CI/production image installs
+            # every language ocrmypdf-data ships, so a real-but-uninstalled code like
+            # "jpn" isn't a safe fixture here.
+            data={"options": '{"languages": ["zzzzzz"]}'}
+        )
+    assert response.status_code == 400
+    assert "not installed" in response.json()["message"]
+
 def test_installed_languages():
     with TestClient(APP, headers=headers) as client:
         response = client.get("/installed_languages")

test/test_legacy.py

@@ -0,0 +1,115 @@
+import pytest
+
+from workflow_ocr_backend.legacy import InvalidOcrParameterError, options_from_legacy_parameters
+from workflow_ocr_backend.ocroptions import TextMode
+
+
+def test_options_from_legacy_parameters_none():
+    options = options_from_legacy_parameters(None)
+    assert options.languages == ["eng"]
+    # No mode flag given -> no override, matching ocrmypdf's own conservative
+    # default of erroring on an already-processed document.
+    assert options.mode is None
+
+
+def test_options_from_legacy_parameters_valid():
+    options = options_from_legacy_parameters("--skip-text --tesseract-pagesegmode 7 --language eng+chi_sim")
+    assert options.mode == TextMode.SKIP
+    assert options.tesseract_pagesegmode == 7
+    assert options.languages == ["eng", "chi_sim"]
+
+
+def test_options_from_legacy_parameters_force_and_redo_ocr():
+    assert options_from_legacy_parameters("--force-ocr").mode == TextMode.FORCE
+    assert options_from_legacy_parameters("--redo-ocr").mode == TextMode.REDO
+
+
+@pytest.mark.parametrize("parameters", [
+    "--plugins /tmp/evil.py",
+    "--plugin-manager foo",
+    "--user-words /etc/passwd",
+    "--user-patterns /etc/passwd",
+    "--keep-temporary-files",
+    "--sidecar /tmp/out.txt",
+    "--output-file /tmp/out.pdf",
+    # tesseract_config is appended verbatim to the tesseract argv, so a caller-supplied
+    # value is an arbitrary config-file path in the same way user_words is.
+    "--tesseract-config /tmp/evil.conf",
+    "--unpaper-args --layout single",
+])
+def test_options_from_legacy_parameters_rejects_dangerous_parameters(parameters):
+    # These parameters aren't in the translation table at all, so they can never
+    # reach OcrOptions - the shim's allow-list is what it can express, not a
+    # denylist of what it blocks.
+    with pytest.raises(InvalidOcrParameterError):
+        options_from_legacy_parameters(parameters)
+
+
+@pytest.mark.parametrize("parameters", [
+    "--not-an-ocrmypdf-parameter",
+    "--some-unknown-option value",
+    # Operator-owned resource knobs are not caller options at all anymore.
+    "--jobs 10000",
+    "--max-image-mpixels 100000",
+])
+def test_options_from_legacy_parameters_rejects_unknown_parameters(parameters):
+    with pytest.raises(InvalidOcrParameterError):
+        options_from_legacy_parameters(parameters)
+
+
+@pytest.mark.parametrize("parameters", [
+    "--language eng;id",
+    "--language $(id)",
+    "--language `id`",
+    "--language |id",
+    "--language eng+;id",
+    "--language ../../etc/passwd",
+    "--language -eng",
+])
+def test_options_from_legacy_parameters_rejects_invalid_languages(parameters):
+    with pytest.raises(InvalidOcrParameterError, match="Invalid language value"):
+        options_from_legacy_parameters(parameters)
+
+
+@pytest.mark.parametrize("parameters,expected", [
+    ("--language eng", ["eng"]),
+    ("--language chi_sim", ["chi_sim"]),
+    ("--language eng+deu", ["eng", "deu"]),
+])
+def test_options_from_legacy_parameters_accepts_valid_languages(parameters, expected):
+    assert options_from_legacy_parameters(parameters).languages == expected
+
+
+@pytest.mark.parametrize("parameters,expected", [
+    ("--jpeg-quality 80", 80),
+    ("--jpg-quality 80", 80),
+])
+def test_options_from_legacy_parameters_accepts_both_jpeg_quality_spellings(parameters, expected):
+    # --jpeg-quality is the documented CLI flag; --jpg-quality is its hidden
+    # argparse.SUPPRESS-ed alias. Both must land on the same field.
+    assert options_from_legacy_parameters(parameters).jpeg_quality == expected
+
+
+@pytest.mark.parametrize("parameters", ["--quiet", "--verbose", "--no-progress-bar"])
+def test_options_from_legacy_parameters_drops_cli_only_flags(parameters):
+    # CLI-only logging flags have no OCRmyPDF API equivalent. They are dropped
+    # rather than rejected so existing workflow configurations keep working.
+    options = options_from_legacy_parameters(f"{parameters} --language eng")
+    assert options.languages == ["eng"]
+
+
+def test_options_from_legacy_parameters_rejects_duplicate_flags():
+    with pytest.raises(InvalidOcrParameterError, match="Duplicate"):
+        options_from_legacy_parameters("--language eng --language deu")
+
+
+def test_options_from_legacy_parameters_rejects_unquoted_multi_word_value():
+    # Unquoted multi-word values used to be silently truncated to their first
+    # token. They are now a 400 - the caller must quote the value.
+    with pytest.raises(InvalidOcrParameterError):
+        options_from_legacy_parameters("--title Hello World")
+
+
+def test_options_from_legacy_parameters_accepts_quoted_multi_word_value():
+    options = options_from_legacy_parameters('--title "Hello World"')
+    assert options.title == "Hello World"