mirror of
https://github.com/guillaumemeyer/watermarks-remover.git
synced 2026-08-22 13:11:57 +02:00
fix: treat a failed c2patool run as inconclusive, not as "no C2PA" (#156)
c2patool exits non-zero both when an asset carries no manifest and when the binary itself fails, and run_optional_tools only ever substring-matched the output. A probe that died before main() produced has_manifest: False with no other signal, so inspect reported has_c2pa: false -- the same answer it gives for a genuinely clean asset. Reproduced on Apple Silicon: service/Dockerfile pins a multi-arch base digest, so an arm64 host builds an arm64 image while still installing the x86_64-unknown-linux-gnu c2patool release (upstream publishes no linux-aarch64 build). c2patool dies with "rosetta error: failed to open elf at /lib64/ld-linux-x86-64.so.2" and exit -5, and /capabilities kept reporting c2patool: true because which() only finds the file. Exposure is largest for PDF, where inspect_pdf documents exiftool and c2patool as the more reliable detectors. For JPEG/PNG the native APP11 and PNG-chunk scans still catch hard-bound C2PA, so there it costs corroboration rather than the only signal. - run_optional_tools marks a run conclusive only when it found a manifest or said in so many words that there is none; anything else sets ok: False - inspect_image and inspect_pdf surface that as an inconclusive note, worded to land in the informational confidence bucket - /capabilities probes each tool's version flag instead of trusting which() Issues #1 and #3 fixed the opposite direction of this same function ("No claim found" matching "claim"). This closes the false-negative side. Co-authored-by: Italo Rodrigues <italo@pvwi.com.br> Co-authored-by: Guillaume Meyer (The Opinionated Man) <1385518+guillaumemeyer@users.noreply.github.com>
This commit is contained in:
co-authored by
Italo Rodrigues
Guillaume Meyer
parent
47252419e4
commit
ce90a71c9b
@@ -165,7 +165,7 @@ The same machinery runs as a stdlib HTTP service (`service/scripts/server.py`)
|
||||
| Method | Path | Body | Returns |
|
||||
| --- | --- | --- | --- |
|
||||
| GET | `/health` | — | `{"ok": true, "version": ...}` |
|
||||
| GET | `/capabilities` | — | optional tools / backends present |
|
||||
| GET | `/capabilities` | — | optional tools / backends usable (each tool is version-probed, not just found on `PATH`) |
|
||||
| GET | `/openapi.json` | — | dynamically generated OpenAPI 3.0.3 spec |
|
||||
| POST | `/inspect` | `{"file": "<base64>", "name": "notes.md"}` | `{"ok", "kind", "suspicious", "report"}` |
|
||||
| POST | `/detect` | `{"file": "<base64>", "name": "notes.txt"}` | `{"ok", "kind", "detections": [...]}` |
|
||||
|
||||
@@ -408,6 +408,27 @@ def classify_finding_confidence(finding: str) -> str:
|
||||
return "informational"
|
||||
|
||||
|
||||
def c2patool_probe_note(tools: dict[str, Any]) -> str | None:
|
||||
"""Describe an inconclusive c2patool run, or None when it answered.
|
||||
|
||||
c2patool exits non-zero both for an asset with no manifest and for a
|
||||
binary that failed to run, so a caller that only reads `has_manifest`
|
||||
cannot tell "this asset is clean" from "the probe never ran". Reporting
|
||||
the second as the first is the dangerous direction: `has_c2pa: False`
|
||||
beside a dead probe reads as a clean bill of health on the one check a
|
||||
user would trust.
|
||||
|
||||
The wording deliberately avoids the substring "c2patool reports", which
|
||||
classify_finding_confidence() maps to `confirmed`; "not fully inspected"
|
||||
puts it in the `informational` bucket instead.
|
||||
"""
|
||||
ct = tools.get("c2patool") or {}
|
||||
if not ct.get("available") or ct.get("ok", True):
|
||||
return None
|
||||
detail = ct.get("error") or "no usable verdict"
|
||||
return f"c2patool probe inconclusive ({detail}); C2PA not fully inspected by this tool"
|
||||
|
||||
|
||||
def cleaned_path(src: Path, suffix: str = ".cleaned") -> Path:
|
||||
"""path/to/file.ext -> path/to/file.cleaned.ext"""
|
||||
return src.with_name(f"{src.stem}{suffix}{src.suffix}")
|
||||
|
||||
@@ -17,6 +17,7 @@ from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from common import (
|
||||
c2patool_probe_note,
|
||||
classify_finding_confidence,
|
||||
safe_arg,
|
||||
safe_write_bytes,
|
||||
@@ -1794,6 +1795,9 @@ def inspect_pdf(path: Path, data: bytes) -> tuple[bool, bool, list[str], dict]:
|
||||
if ct.get("has_manifest"):
|
||||
has_c2pa = True
|
||||
findings.append("c2patool reports C2PA-related manifest")
|
||||
probe_note = c2patool_probe_note(tools)
|
||||
if probe_note:
|
||||
findings.append(probe_note)
|
||||
return has_c2pa, has_ai or has_c2pa, findings, {"tools": tools}
|
||||
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ from typing import Any
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from common import (
|
||||
c2patool_probe_note,
|
||||
classify_finding_confidence,
|
||||
safe_arg,
|
||||
safe_write_bytes,
|
||||
@@ -1308,15 +1309,34 @@ def run_optional_tools(path: Path) -> dict[str, Any]:
|
||||
# missing manifest as "Error: No claim found", which contains
|
||||
# the substring "claim" and would otherwise read as a hit.
|
||||
no_manifest = "no claim" in low or "no jumbf" in low
|
||||
tools["c2patool"] = {
|
||||
has_manifest = (
|
||||
"claim" in low or "c2pa" in low or "manifest" in low
|
||||
) and not no_manifest
|
||||
# c2patool exits non-zero for a missing manifest too, so the exit
|
||||
# code alone cannot separate "asset is clean" from "the probe
|
||||
# never ran". Treat the run as conclusive only when it either
|
||||
# found a manifest or said in so many words that there is none.
|
||||
# Anything else -- a crash before main(), a kill, an unrecognized
|
||||
# error -- leaves the C2PA question unanswered, and callers must
|
||||
# not read that as a negative.
|
||||
conclusive = has_manifest or no_manifest
|
||||
entry: dict[str, Any] = {
|
||||
"available": True,
|
||||
"returncode": r.returncode,
|
||||
"snippet": out[:2000],
|
||||
"has_manifest": ("claim" in low or "c2pa" in low or "manifest" in low)
|
||||
and not no_manifest,
|
||||
"has_manifest": has_manifest,
|
||||
"ok": conclusive,
|
||||
}
|
||||
if not conclusive:
|
||||
entry["error"] = f"exit {r.returncode}, unrecognized output"
|
||||
tools["c2patool"] = entry
|
||||
except Exception as e:
|
||||
tools["c2patool"] = {"available": True, "error": str(e)}
|
||||
tools["c2patool"] = {
|
||||
"available": True,
|
||||
"ok": False,
|
||||
"has_manifest": False,
|
||||
"error": str(e),
|
||||
}
|
||||
else:
|
||||
tools["c2patool"] = {"available": False}
|
||||
|
||||
@@ -1675,6 +1695,9 @@ def inspect_image(
|
||||
if ct.get("has_manifest"):
|
||||
has_c2pa = True
|
||||
findings.append("c2patool reports a C2PA-related manifest")
|
||||
probe_note = c2patool_probe_note(tools)
|
||||
if probe_note:
|
||||
notes.append(probe_note)
|
||||
|
||||
return ImageInspectReport(
|
||||
path=str(path),
|
||||
|
||||
@@ -39,8 +39,10 @@ import base64
|
||||
import binascii
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from functools import cache
|
||||
from http import HTTPStatus
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
from pathlib import Path
|
||||
@@ -54,6 +56,7 @@ from common import (
|
||||
MAX_INPUT_BYTES,
|
||||
eprint,
|
||||
looks_binary,
|
||||
subprocess_preexec_fn,
|
||||
which,
|
||||
)
|
||||
from container_meta import clean_container, inspect_container
|
||||
@@ -94,13 +97,47 @@ def _json_ok(payload: dict[str, Any]) -> bytes:
|
||||
return json.dumps(payload, ensure_ascii=False, indent=2).encode("utf-8")
|
||||
|
||||
|
||||
# Flag that makes each tool print its version and exit 0. They disagree:
|
||||
# exiftool treats `--version` as an unknown option and prints usage instead.
|
||||
_VERSION_FLAG = {"c2patool": "--version", "exiftool": "-ver", "qpdf": "--version"}
|
||||
|
||||
|
||||
@cache
|
||||
def _tool_usable(cmd: str) -> bool:
|
||||
"""True only when the tool is on PATH *and* can actually execute.
|
||||
|
||||
`which` alone answers the wrong question. A binary built for another
|
||||
architecture sits on PATH and still dies before main() -- the published
|
||||
image pins a multi-arch base digest, so an arm64 host gets an arm64 image
|
||||
carrying the x86_64-only c2patool release. Advertising that as available
|
||||
is what lets a probe which never ran read as a clean verdict downstream.
|
||||
|
||||
Cached: a container's tool set cannot change while the process lives.
|
||||
"""
|
||||
path = which(cmd)
|
||||
if not path:
|
||||
return False
|
||||
try:
|
||||
r = subprocess.run(
|
||||
[path, _VERSION_FLAG.get(cmd, "--version")],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
preexec_fn=subprocess_preexec_fn,
|
||||
check=False,
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
return r.returncode == 0
|
||||
|
||||
|
||||
def capabilities() -> dict[str, Any]:
|
||||
return {
|
||||
"version": VERSION,
|
||||
"tools": {
|
||||
"c2patool": which("c2patool") is not None,
|
||||
"exiftool": which("exiftool") is not None,
|
||||
"qpdf": which("qpdf") is not None,
|
||||
"c2patool": _tool_usable("c2patool"),
|
||||
"exiftool": _tool_usable("exiftool"),
|
||||
"qpdf": _tool_usable("qpdf"),
|
||||
},
|
||||
"pixel_backends": {
|
||||
"ctrlregen": bool(os.environ.get("NOAI_WATERMARK_DIR")),
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
@@ -91,3 +92,125 @@ def test_missing_c2patool_is_reported_unavailable(monkeypatch, tmp_path):
|
||||
tools = image_meta.run_optional_tools(path)
|
||||
|
||||
assert tools["c2patool"] == {"available": False}
|
||||
|
||||
|
||||
# --- A probe that never ran must not read as "no C2PA" -----------------------
|
||||
#
|
||||
# c2patool exits non-zero both when an asset carries no manifest and when the
|
||||
# binary itself fails, so the exit code alone cannot separate the two. The
|
||||
# interpretation therefore has to key off whether c2patool actually answered
|
||||
# the question. Real-world trigger: the published Docker image pins a
|
||||
# multi-arch base digest, so on an arm64 host it resolves to linux/arm64 while
|
||||
# the Dockerfile still installs the x86_64-unknown-linux-gnu c2patool release
|
||||
# (upstream ships no linux-aarch64 build). The binary then dies before main().
|
||||
|
||||
ROSETTA_CRASH = "rosetta error: failed to open elf at /lib64/ld-linux-x86-64.so.2\n"
|
||||
|
||||
|
||||
def test_crashed_c2patool_is_not_a_clean_verdict(monkeypatch, tmp_path):
|
||||
"""A binary that dies before main() must be inconclusive, not negative."""
|
||||
_fake_c2patool(monkeypatch, ROSETTA_CRASH, -5, on_stderr=True)
|
||||
path = tmp_path / "maybe.jpg"
|
||||
path.write_bytes(b"\xff\xd8\xff")
|
||||
|
||||
entry = image_meta.run_optional_tools(path)["c2patool"]
|
||||
|
||||
assert entry["has_manifest"] is False, "a crash must never claim a manifest"
|
||||
assert entry["ok"] is False, "a crash must be flagged as an unusable verdict"
|
||||
|
||||
|
||||
def test_unrecognised_error_is_inconclusive(monkeypatch, tmp_path):
|
||||
"""Any error that is not a recognised 'no manifest' marker is inconclusive."""
|
||||
_fake_c2patool(monkeypatch, "Error: permission denied\n", 1, on_stderr=True)
|
||||
path = tmp_path / "maybe.png"
|
||||
path.write_bytes(b"\x89PNG\r\n\x1a\n")
|
||||
|
||||
entry = image_meta.run_optional_tools(path)["c2patool"]
|
||||
|
||||
assert entry["has_manifest"] is False
|
||||
assert entry["ok"] is False
|
||||
|
||||
|
||||
def test_conclusive_runs_are_marked_ok(monkeypatch, tmp_path):
|
||||
"""Both real verdicts -- manifest and no-manifest -- stay conclusive."""
|
||||
path = tmp_path / "a.png"
|
||||
path.write_bytes(b"\x89PNG\r\n\x1a\n")
|
||||
|
||||
_fake_c2patool(monkeypatch, "Error: No claim found\n", 1, on_stderr=True)
|
||||
absent = image_meta.run_optional_tools(path)["c2patool"]
|
||||
assert absent["has_manifest"] is False
|
||||
assert absent["ok"] is True
|
||||
|
||||
_fake_c2patool(monkeypatch, MANIFEST_OUTPUT, 0)
|
||||
present = image_meta.run_optional_tools(path)["c2patool"]
|
||||
assert present["has_manifest"] is True
|
||||
assert present["ok"] is True
|
||||
|
||||
|
||||
def test_timeout_is_inconclusive(monkeypatch, tmp_path):
|
||||
"""An exception path must still report the probe as unusable."""
|
||||
|
||||
def fake_which(cmd: str):
|
||||
return "/fake/bin/c2patool" if cmd == "c2patool" else None
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
raise subprocess.TimeoutExpired(cmd, 30)
|
||||
|
||||
monkeypatch.setattr(image_meta, "which", fake_which)
|
||||
monkeypatch.setattr(image_meta.subprocess, "run", fake_run)
|
||||
path = tmp_path / "slow.png"
|
||||
path.write_bytes(b"\x89PNG\r\n\x1a\n")
|
||||
|
||||
entry = image_meta.run_optional_tools(path)["c2patool"]
|
||||
|
||||
assert entry["ok"] is False
|
||||
assert entry["has_manifest"] is False
|
||||
|
||||
|
||||
def test_inconclusive_probe_is_surfaced_in_the_report(monkeypatch, tmp_path):
|
||||
"""The failure has to reach the caller, not just the tools dict.
|
||||
|
||||
`has_c2pa: False` next to a silently dead probe is the actual hazard: it
|
||||
reads as "this asset carries no C2PA" on the one check a user would trust.
|
||||
"""
|
||||
_fake_c2patool(monkeypatch, ROSETTA_CRASH, -5, on_stderr=True)
|
||||
path = tmp_path / "shot.jpg"
|
||||
path.write_bytes(b"\xff\xd8\xff")
|
||||
|
||||
report = image_meta.inspect_image(path)
|
||||
|
||||
assert report.has_c2pa is False
|
||||
joined = " ".join(report.notes).lower()
|
||||
assert "c2patool" in joined and "inconclusive" in joined, (
|
||||
f"inconclusive C2PA probe left no trace in the report: {report.notes}"
|
||||
)
|
||||
|
||||
|
||||
# --- /capabilities must not advertise a binary that cannot run ---------------
|
||||
|
||||
|
||||
def test_capabilities_rejects_a_present_but_unrunnable_tool(monkeypatch):
|
||||
"""Presence on PATH is not the same as being usable.
|
||||
|
||||
An x86_64 c2patool inside an arm64 image satisfies `which` and still dies
|
||||
before main(); advertising it as available is what makes the downstream
|
||||
probe failure look like a considered "no C2PA" answer.
|
||||
"""
|
||||
import server
|
||||
|
||||
server._tool_usable.cache_clear()
|
||||
monkeypatch.setattr(server, "which", lambda cmd: f"/usr/local/bin/{cmd}")
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
if cmd[0].endswith("c2patool"):
|
||||
return _Completed(-5, stderr=ROSETTA_CRASH)
|
||||
return _Completed(0, stdout="1.0\n")
|
||||
|
||||
monkeypatch.setattr(server.subprocess, "run", fake_run)
|
||||
|
||||
tools = server.capabilities()["tools"]
|
||||
|
||||
assert tools["c2patool"] is False, "a binary that cannot execute is not available"
|
||||
assert tools["exiftool"] is True
|
||||
assert tools["qpdf"] is True
|
||||
server._tool_usable.cache_clear()
|
||||
|
||||
Reference in New Issue
Block a user