mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-11 14:17:31 +01:00
tool/tagger.py did not run at all: 48957a1 renamed pack.default_dir to
default_dirs and missed the caller, so the tagger raised ImportError at
module scope through two releases. Nothing could have caught it -- tool/ is
outside --cov and mypy ran with [signal_bot, fake_signal]. Both are widened
here, and tests/test_tool_imports.py imports every script under tool/.
--import was a filename check and shutil.copy2. Pointed at a phone export
that overlaps the album it re-imported the same photo under whatever name
the exporter used that time. It now identifies files by magic bytes, strips
them through the same media_strip path !add_photo uses, and dedups on exact
bytes twice: the source hash, then -- only for what survives -- the hash
after stripping. sources.json records both, so a second run on the same
folder is a no-op. What the album refuses is moved to _skipped/<reason>/
beside the source, never deleted.
Two things that made "strip on import" impossible until now:
media_tags claims -all= zeroes the QuickTime date tags and allows them by
value. It does not; measured, it leaves CreateDate at its real value, and
every mp4 was being refused. -time:all= is what zeroes them.
And a tag allowlist cannot see the worst class at all. IMG_6924.mp4 carries
mebx timed-metadata holding per-frame face bounding boxes and a face id,
and those survive a complete tag strip because they live in mdat as opaque
samples. media_tracks.py drops the track with ffmpeg and verifies with
ffprobe that only video and audio survived. This is not the ffmpeg mistake
media_strip warns about: that was ffmpeg replacing exiftool for tags, this
is track removal with exiftool still doing every tag, and the gate is a
positive read-back rather than an exit code.
The roster was write-once. The setup form only appeared on an empty album,
Album.setup overwrote the file whole, and keys were positional (p{len+1}),
so inserting somebody renumbered everyone after them and repointed every
tags.json entry at the wrong person. Keys now come from the highest number
in use, the form merges, and a person can be added mid-session.
--untagged filters the page and never the server's list. /media/{index} and
/api/tag both index the full album, so a shorter list server-side would
show item i of the album while tagging item i of the filtered set.
Measured on the live album: 174 source files -> 1 not media, 82 already in
pack, 24 duplicate in batch, 67 imported, and re-running imports nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBrE1b9HvFw3xvG9bJiDFt
186 lines
7.8 KiB
Python
186 lines
7.8 KiB
Python
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
|
"""Removing metadata from an upload, and *proving* it is gone.
|
|
|
|
The gate is a positive check on the output, never an exit code, because an
|
|
exit code is exactly what lied the first time. ``ffmpeg -map_metadata -1 -c
|
|
copy`` was tested against a JPEG carrying a real APP1 Exif segment and
|
|
produced a byte-identical file -- 292 bytes in, 292 bytes out, GPS marker
|
|
still there -- and exited 0. ``-c copy`` hands the compressed frame through
|
|
as an opaque packet, and a JPEG's Exif lives inside it. Do not reintroduce
|
|
ffmpeg here.
|
|
|
|
So: strip with exiftool, then read the result back and require every
|
|
surviving tag to be structural (media_tags). A metadata atom nobody
|
|
anticipated shows up as a tag that is not on the allowlist, and the upload is
|
|
refused rather than stored.
|
|
|
|
Temp files go in /tmp, which the unit makes per-service with PrivateTmp=true.
|
|
Staging beside ``media/`` would let a crash mid-strip leave an unstripped
|
|
original inside the album permanently, where nothing in pack_load would ever
|
|
flag it.
|
|
|
|
The ``Stripper`` protocol is the seam: unit tests inject a fake so they never
|
|
shell out, and ``scripts/e2e.sh`` runs the real one against generated media.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
import os
|
|
import re
|
|
import shutil
|
|
import tempfile
|
|
from pathlib import Path
|
|
from typing import TYPE_CHECKING, Final, Protocol
|
|
|
|
from signal_bot.external import run
|
|
from signal_bot.media_tags import is_allowed
|
|
from signal_bot.media_tracks import ISO_SUFFIXES, flatten
|
|
|
|
if TYPE_CHECKING:
|
|
from signal_bot.media_sniff import MediaKind
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
EXIFTOOL: Final = "exiftool"
|
|
|
|
# Where to look when PATH does not have it. Arch's perl-image-exiftool
|
|
# installs to /usr/bin/vendor_perl, which is on an interactive shell's PATH
|
|
# (perl's profile.d adds it) and *not* on a systemd unit's, whose default is
|
|
# "/usr/local/bin:/usr/bin". Found the hard way on 2026-08-31: the stripper
|
|
# worked in every test and in the shell, and the first live run under the
|
|
# unit refused every upload with "exiftool is not installed". Looking in the
|
|
# known locations is the fix; setting PATH in the unit would only be a note
|
|
# somebody has to remember on the next machine.
|
|
FALLBACK_PATHS: Final = (
|
|
"/usr/bin/vendor_perl/exiftool",
|
|
"/usr/local/bin/exiftool",
|
|
"/usr/bin/exiftool",
|
|
"/opt/homebrew/bin/exiftool",
|
|
)
|
|
|
|
# How long either exiftool call may take. A phone video is a few tens of
|
|
# megabytes and exiftool rewrites the container, not the frames.
|
|
TIMEOUT_SECONDS: Final = 60
|
|
|
|
# "[Group] TagName : value", which is what -G0 -s prints.
|
|
_TAG_LINE: Final = re.compile(r"^\[(\w+)\]\s+(\S+)\s*:\s?(.*)$")
|
|
|
|
|
|
def locate() -> str | None:
|
|
"""The exiftool binary's absolute path, or None if it is not installed.
|
|
|
|
PATH first, then the known install locations -- see FALLBACK_PATHS for
|
|
why the second half is not redundant.
|
|
"""
|
|
found = shutil.which(EXIFTOOL)
|
|
if found is not None:
|
|
return found
|
|
return next((path for path in FALLBACK_PATHS if Path(path).is_file()), None)
|
|
|
|
|
|
class Stripper(Protocol):
|
|
"""Removes metadata from one upload, or refuses it."""
|
|
|
|
def strip(self, payload: bytes, kind: MediaKind) -> bytes | None:
|
|
"""Return the cleaned file, or None when it cannot be proven clean."""
|
|
|
|
|
|
class ExiftoolStripper:
|
|
"""The real stripper: exiftool 13.55, plus a positive post-check."""
|
|
|
|
def __init__(self, executable: str | None = None) -> None:
|
|
"""Locate exiftool once, by absolute path.
|
|
|
|
Args:
|
|
executable: Override for the binary, used by the e2e script.
|
|
"""
|
|
self._exiftool = executable or locate()
|
|
|
|
def strip(self, payload: bytes, kind: MediaKind) -> bytes | None:
|
|
"""Strip one upload and verify the result, or refuse it.
|
|
|
|
Args:
|
|
payload: The file exactly as it arrived.
|
|
kind: What the sniffer decided it is.
|
|
|
|
Returns:
|
|
The cleaned bytes, or None when the strip failed or the output
|
|
still carries a tag that is not structural.
|
|
"""
|
|
if not kind.strippable:
|
|
# kuhy's decision of 2026-08-31: .webm and .svg stay on the
|
|
# allowlist even though exiftool 13.55 can write neither, so they
|
|
# are stored exactly as they arrived, metadata included. Flagged
|
|
# at the time and recorded in DOCS-add-photo.md.
|
|
logger.warning("storing %s unstripped: exiftool cannot write it", kind.suffix)
|
|
return payload
|
|
if self._exiftool is None:
|
|
logger.error(
|
|
"exiftool not found on PATH (%s) or in %s; refusing to store %s",
|
|
os.environ.get("PATH", ""),
|
|
FALLBACK_PATHS,
|
|
kind.suffix,
|
|
)
|
|
return None
|
|
with tempfile.TemporaryDirectory(prefix="signal-bot-strip-") as workspace:
|
|
return self._run(Path(workspace), payload, kind)
|
|
|
|
def _run(self, workspace: Path, payload: bytes, kind: MediaKind) -> bytes | None:
|
|
"""Do the strip inside a temp directory that is removed either way."""
|
|
source = workspace / f"in{kind.suffix}"
|
|
destination = workspace / f"out{kind.suffix}"
|
|
if kind.suffix in ISO_SUFFIXES:
|
|
# Tracks first, tags second. exiftool cannot see a mebx track at
|
|
# all -- see media_tracks for the face-detection payload that
|
|
# survives a complete tag strip -- so this has to happen here or
|
|
# not at all.
|
|
flattened = flatten(payload, kind.suffix)
|
|
if flattened is None:
|
|
return None
|
|
payload = flattened
|
|
source.write_bytes(payload)
|
|
# Never --overwrite_original: writing in place would leave a partial
|
|
# file where the original was if exiftool died halfway.
|
|
#
|
|
# "-time:all=" is not redundant with "-all=". media_tags says the date
|
|
# tags are "zeroed, not deleted" by -all= and allows them by value;
|
|
# measured on 2026-08-31, -all= alone leaves QuickTime:CreateDate at
|
|
# its real value and every mp4 in the album was refused because of it.
|
|
# -time:all= is what actually zeroes them.
|
|
if not self._exiftool_ok(["-all=", "-time:all=", "-o", str(destination), str(source)]):
|
|
return None
|
|
if not destination.is_file():
|
|
logger.warning("exiftool produced no output for %s", kind.suffix)
|
|
return None
|
|
report = self._exiftool_output(["-G0", "-s", "-all", str(destination)])
|
|
if report is None or not self._verify(report, kind):
|
|
return None
|
|
return destination.read_bytes()
|
|
|
|
def _verify(self, report: str, kind: MediaKind) -> bool:
|
|
"""True when every tag the stripped file still reports is structural."""
|
|
for line in report.splitlines():
|
|
match = _TAG_LINE.match(line)
|
|
if match is None:
|
|
continue
|
|
group, tag, value = match.group(1), match.group(2), match.group(3)
|
|
if not is_allowed(group, tag, value):
|
|
# The tag name, never its value: the value is the metadata.
|
|
logger.warning("refusing %s: %s:%s survived the strip", kind.suffix, group, tag)
|
|
return False
|
|
return True
|
|
|
|
def _exiftool_ok(self, arguments: list[str]) -> bool:
|
|
"""Run exiftool for its effect, reporting whether it succeeded."""
|
|
return self._exiftool_output(arguments) is not None
|
|
|
|
def _exiftool_output(self, arguments: list[str]) -> str | None:
|
|
"""Run exiftool and return its stdout, or None if it failed at all.
|
|
|
|
Every subprocess in this repository goes through signal_bot.external;
|
|
see that module for why there is exactly one.
|
|
"""
|
|
result = run([str(self._exiftool), *arguments], timeout=TIMEOUT_SECONDS)
|
|
return result.stdout if result.ok else None
|