monitoring: validate report contents before the reader uses them

The body of an unsigned report is whatever the untrusted server chooses to
serve, but the reader took its shape on faith. A body that is not a JSON object
crashed borg monitor with an AttributeError traceback, a list where a string
belongs (e.g. "archive") crashed it with an unhashable-type TypeError when
grouping, and a report without a status was neither an error nor a warning -
so it silently counted as a success.

deserialize() now shape-checks every report: the fields the reader relies on
(command, time, status, rc) must be present and of the right type, the optional
ones (hostname, username, archive, archive_id, stats) must match their type if
present, and status must be one of success/warning/error. Anything else is
refused with a clean error. Reports from a real client always pass, so this only
affects bodies nobody signed.

An unparsable *value* in the time field is still tolerated (the report is
skipped with a warning, see the previous commit); this is about the shape.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: ThomasWaldmann

src/borg/monitoring.py

@@ -26,7 +26,9 @@
 A plaintext body is only legitimate for an unencrypted repository, which has no key to
 sign with. Once a monitoring key is configured, plaintext bodies are refused rather than
 reported as untrusted - otherwise the server could downgrade a failure it wants hidden
-into a mere warning by replacing the sealed report with an unsigned "success".
+into a mere warning by replacing the sealed report with an unsigned "success". What an
+unsigned body does contain is not trusted either: every report is shape-checked before
+the reader touches it, see validate_report().
 """
 
 import json
@@ -109,6 +111,35 @@ def build_report(
     return report
 
 
+# Fields the reader relies on: required ones must be present and of the given type,
+# optional ones only have to match their type if they are present at all. Anything else a
+# report carries is passed through untouched.
+REQUIRED_FIELDS = {"command": str, "time": str, "status": str, "rc": int}
+OPTIONAL_FIELDS = {"hostname": str, "username": str, "archive": str, "archive_id": str, "stats": dict}
+STATUS_VALUES = {"success", "warning", "error"}
+
+
+def validate_report(report):
+    """Return *report* if the reader can safely work with it, else raise ValueError.
+
+    The body of an unsigned report is whatever the untrusted server chooses to serve, so
+    its shape must not be taken on faith: a non-object body or a list where a string
+    belongs crashes borg monitor with a traceback, and a missing status would pass as a
+    success. A sealed report from a real client always satisfies this.
+    """
+    if not isinstance(report, dict):
+        raise ValueError("monitoring report: not a JSON object")
+    for field, field_type in REQUIRED_FIELDS.items():
+        if not isinstance(report.get(field), field_type):
+            raise ValueError(f"monitoring report: missing or invalid {field!r}")
+    for field, field_type in OPTIONAL_FIELDS.items():
+        if field in report and not isinstance(report[field], field_type):
+            raise ValueError(f"monitoring report: invalid {field!r}")
+    if report["status"] not in STATUS_VALUES:
+        raise ValueError(f"monitoring report: unknown status {report['status']!r}")
+    return report
+
+
 def report_aad(repo_id_bin, name):
     """Additional authenticated data a sealed report is bound to.
 
@@ -165,7 +196,7 @@ def deserialize(monitor_key, repo_id_bin, name, data):
         trusted = False
     else:
         raise ValueError("monitoring report: unknown body type")
-    return json.loads(payload.decode("utf-8")), trusted
+    return validate_report(json.loads(payload.decode("utf-8"))), trusted
 
 
 def publish(repository, key, report):

src/borg/testsuite/archiver/monitor_cmd_test.py

@@ -282,6 +282,20 @@ def test_injected_unsigned_report_is_refused(archivers, request, monkeypatch):
     assert "unsigned" in output
 
 
+def test_malformed_unsigned_report_gives_a_clean_error(archivers, request):
+    archiver = request.getfixturevalue(archivers)
+    create_regular_file(archiver.input_path, "file1", contents=b"some data")
+    cmd(archiver, "repo-create", "--encryption=none")
+    cmd(archiver, "create", "series", "input")
+    # an unencrypted repo's reports are unsigned, so their contents are whatever the
+    # server serves: borg monitor must refuse them, not crash on them
+    name = sorted(os.listdir(_monitoring_dir(archiver)))[0]
+    _write_plain_report(archiver, name, ["not", "a", "dict"])
+    output = cmd(archiver, "monitor", fork=True, exit_code=EXIT_ERROR)
+    assert "monitoring report" in output
+    assert "Traceback" not in output
+
+
 def test_monitor_key_export_is_deterministic(archivers, request):
     archiver = request.getfixturevalue(archivers)
     cmd(archiver, "repo-create", RK_ENCRYPTION)

src/borg/testsuite/monitoring_test.py

@@ -238,3 +238,50 @@ def test_plaintext_report_refused_when_monitor_key_is_configured():
     monitor_key = mc.parse_monitor_key(mc.export_monitor_key(make_key()))
     with pytest.raises(low_level.IntegrityError):
         m.deserialize(monitor_key, repo_id, NAME, data)
+
+
+# --- validation of untrusted report contents -------------------------------------------
+
+
+def test_validate_report_accepts_a_real_report():
+    report = sample_report("cc" * 32)
+    assert m.validate_report(report) is report
+    # the optional fields borg actually publishes are accepted, too
+    report["hostname"], report["username"] = "somehost", "someuser"
+    assert m.validate_report(report) is report
+
+
+@pytest.mark.parametrize(
+    "body",
+    [
+        ["not", "a", "dict"],  # a non-object body would crash report.get()
+        "a string",
+        None,
+        {},  # no fields at all
+        {"command": "create", "status": "success", "rc": 0},  # no time
+        {"command": "create", "time": "2026-06-17T11:59:58+00:00", "status": "success"},  # no rc
+        {"command": "create", "time": 12345, "status": "success", "rc": 0},  # time not a string
+        {"command": ["create"], "time": "2026-06-17T11:59:58+00:00", "status": "success", "rc": 0},
+        # an unhashable value in a grouping field would crash the reader
+        {
+            "command": "create",
+            "time": "2026-06-17T11:59:58+00:00",
+            "status": "success",
+            "rc": 0,
+            "archive": ["unhashable"],
+        },
+        # an unknown status would count as "not an error", i.e. as a success
+        {"command": "create", "time": "2026-06-17T11:59:58+00:00", "status": "all good", "rc": 0},
+    ],
+)
+def test_validate_report_rejects_unusable_bodies(body):
+    with pytest.raises(ValueError):
+        m.validate_report(body)
+
+
+def test_deserialize_rejects_unusable_plaintext_body():
+    # what an unencrypted repo (or an injecting server) can serve must not reach the reader
+    repo_id = os.urandom(32)
+    data = bytes([m.FORMAT_VERSION, m.BODY_PLAIN]) + b'["not", "a", "dict"]'
+    with pytest.raises(ValueError):
+        m.deserialize(None, repo_id, NAME, data)