signal-bot/TODO-add-photo.md

TODO: !add_photo — add an attachment to the pack, tagged

REMOVE ME AFTER FINISH

Planned 2026-08-30, not implemented. Every decision below was settled with kuhy during planning; they are answers, not suggestions. Do not re-litigate them.

Context

signal-bot has a private photo album (the "pack") outside the repo at ~/.local/share/signal-bot/pack/. Today the only way to put a photo into it is tool/tagger.py, a browser tool run by hand on the PC — so every new photo means leaving Signal, moving the file to the machine, and running a tagger session.

This change lets a member of the pack's group add a photo from the chat: send a photo captioned !add_photo alfa beta and it lands in the album tagged with both, immediately retrievable through each of their existing commands. A label matching nobody becomes a custom tag, reachable through a new !random <label>.

The constraint is unchanged and absolute: no name and no photo may reach the repository, the journal, the SQLite database, a reply body, or any group outside the pack's own. DOCS-photo-pack.md and the personal-data pre-commit gate exist because that invariant was broken once already.

Decisions

New people	!add_photo never writes a person into people.json. An unmatched label becomes a custom tag.
Photo source	Attachment on the same message. Never a quote/reply — quote carries its own attachments array.
Who may run it	Anyone in the pack's groups. No operator concept.
Scrubbing	Labels resolve to keys at the dispatcher; the raw remainder never travels further.
Availability	Immediate: atomic write + validated hot-swap. No restart.
Formats	Explicit allowlist, magic-byte sniffed: jpg, jpeg, png, gif, mp4 + heic, heif, webp, mov, webm, m4v, avif, svg.
Stripping	exiftool, with a positive post-check. See below.
Filename	Content hash. The original filename is discarded.
Reply	Counts and pseudonyms only: dodano · p1: 4 · t3: 1. On a first custom-tag mint the reply also explains, generically, that the new tag is reached via !random — without echoing the label.
Limits	None on !add_photo — no size cap, no daily cap, no cooldown.
Undo	Out of scope. See "Deferred" below.
Unit	ReadWritePaths=%h/.local/share/signal-bot added. The pack does not move.

Why exiftool and not ffmpeg

The first choice was ffmpeg -map_metadata -1 -c copy. It was tested and fails silently on photos: given a JPEG carrying a real APP1 Exif segment it produced a byte-identical file (292 → 292 bytes, GPS marker intact) and exited 0. -c copy copies the compressed frame as an opaque packet and JPEG's Exif lives inside it. That ffmpeg also has no HEIF/HEIC demuxer (ffmpeg -demuxers | grep -ci heif → 0). Do not reintroduce ffmpeg for this.

Use perl-image-exiftool (Arch extra, 13.55-1), which covers the whole allowlist losslessly. The gate is a positive check on the output, not an exit code — an exit code is exactly what lied:

exiftool -all= -o <dst> <src>          # never --overwrite_original on the temp
exiftool -G0 -s -all <dst>
reject unless the surviving tag set is empty apart from the computed/filesystem
pseudo-groups File, Composite, ExifTool

Allowlist, not blocklist. The obvious gate — grepping -Exif:all -GPS:all -XMP:all — is blind to exactly the case that matters most: an MP4/MOV geotag lives in the QuickTime udta/©xyz atom and is reported under group QuickTime, not GPS. Enumerating groups to look for is how the next one gets missed; requiring the set to be empty cannot be. Verify at implementation time that exiftool's MP4 writer honours -all= on this version — if it does not, video is rejected rather than stored.

SVG has no magic bytes, so sniffing special-cases a leading <?xml / <svg prefix. It is not exempt from the check: SVG can carry <metadata> RDF and can embed a whole raster (<image xlink:href="data:image/jpeg;base64,…">) with its EXIF intact. exiftool has an SVG reader, so SVG runs the same strip-and-verify as everything else.

Temp files go in /tmp, which the unit already makes per-service via PrivateTmp=true. Staging next to 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 second copy

signal-cli keeps its own copy of every inbound attachment in the container's attachment store, outside the album's 0700 permissions. That is pre-existing for every photo the group sends, but this feature is the first path that ingests one deliberately, so it is in scope for "inaccessible from outside": Transport.delete_attachment already exists and is called nowhere in production — !add_photo calls it once the file is safely stored, whether the upload was accepted or rejected.

Implementation

New modules

File	Contents
signal_bot/pack_custom.py	custom.json load/mint/write. Labels folded like person triggers; keys t1, t2, …; atomic temp-then-Path.replace as tool/tagger.py::Album.write already does.
signal_bot/pack_add.py	The !add_photo handler: fetch → sniff → strip → verify → hash-name → write media/ → merge tags.json → hot-swap → reply.
signal_bot/pack_random.py	The !random handler and its non-minting resolver.
signal_bot/media_sniff.py	Magic-byte detection over the allowlist + the SVG prefix case; maps to suffix and MIME.
signal_bot/media_strip.py	Stripper protocol and the real exiftool implementation.
signal_bot/pack_holder.py	The mutable holder (below).

pack_load.py is at 243 of the 250-line cap, so its custom-tag validation moves into pack_custom.py rather than growing it. pack.py is at 155.

The scrubbing seam (load-bearing)

registry.resolve_with_args currently returns Resolved(head, tuple(words[1:])) — raw words straight into CommandContext.args. Add an optional per-command arg_resolver set at registration; resolve_with_args applies it and yields keys only, so no handler, log line or command_uses row ever sees a typed label.

Two resolvers, deliberately different:

• pack:add — minting. Known label → the person's key; unknown label → a pending t-key held in memory only. Nothing is written until the upload succeeds; a rejected upload or a missing attachment discards the pending mint and leaves custom.json untouched. !add_photo dog with no attachment must change nothing on disk.
• pack:random — non-minting. Unknown label → None → the existing NO_PHOTO line. If it shared the minting resolver every typo on !random would mint a key.

!random semantics

No arguments: a random photo from the whole album. Two or more labels: intersection (reuse Pack.matching, which is already a superset test), ceiling MAX_NAMES = 6. !random resolves custom tags only, never people — !alfa already exists for that, and a second entrance with an unscoped argument list is one too many.

Hot-swap

pack_command.pack_handler closes over the frozen Pack (pack_command.py:62-78), so a newly-tagged photo would stay invisible to the existing pack:p1 handler. Introduce a PackHolder with a mutable .pack that every pack handler reads at call time. bot.py::build_bot constructs the holder once and passes it where it passes pack today.

Swap order: write media atomically → write tags.json/custom.json atomically → re-run pack_load.load → on success assign holder.pack; on failure roll back the files and keep serving the old pack. Never raise: Restart=always would loop.

Custom tags never become commands, so a swap only ever replaces Pack data — it never re-registers commands or recomputes Command.groups. That is what makes immediate availability cheap, and it is the property to preserve.

Gaps that must be filled

• models.py::_attachments keeps only contentType and discards the attachment id. It must carry the id without breaking counting.media_kind / should_count.
• Transport (transport_base.py:58-62) has list_attachments / delete_attachment but no fetch. Add fetch_attachment(attachment_id) -> bytes to the Protocol, RestTransport (GET /v1/attachments/{attachment}, verified present on the live container's /swagger/doc.json) and FakeTransport. tests/test_transport_base.py asserts the Protocol's member names and must be updated.
• fake_signal cannot inject an inbound attachment at all: no GET-by-id route, state.py stores no bytes, and scenario.py::Injection has no attachment field (_build rejects unknown keys). All three need extending or scripts/e2e.sh cannot prove any of this offline.
• .svg → image/svg+xml must join pack.MEDIA_TYPES, because pack_load._media_file validates the suffix against it and a stored SVG would otherwise fail the next load and take startup down.
• scripts/personal_data_sources.py::_pack_names must read custom.json labels alongside people.json triggers — a label can be a name (!add_photo alfa_wedding).
• Any new SQLite table uses CREATE TABLE IF NOT EXISTS and must not bump SCHEMA_VERSION (a bump DROPs the ephemeral tables).
• scripts/install_service.sh gains the perl-image-exiftool dependency (there is no install.sh; that script is the installer).
• DOCS-photo-pack.md and DOCS-commands.md both need updating.

Assumption to confirm with kuhy before coding

!random is a new command and inherits the standard pack cooldown decorator; !add_photo gets no decorator at all, per "NO CAP, NO LIMITS".

Verification

1. Unit — pytest, which enforces 100% branch coverage with no omits and no suppressions. The stripper must be an injected Stripper seam so unit tests use a fake: tests/pack_fixture.py builds packs from a 1×1 PNG and a fake mp4 (CLIP) that real exiftool would reject. New tests assert, explicitly, that a typed label appears in no log record, no command_uses row and no reply body.
2. End-to-end — new scripts/e2e.sh scenarios against the extended fake_signal: add tags an existing person and !<name> returns it; add mints a custom tag and !random <label> returns it; two labels tag both; !add_photo with no attachment changes nothing on disk; a rejected file is not stored; the same commands in another group produce zero sends.
3. Real exiftool — exercised only in e2e.sh, against generated media, covering all three regressions this plan came from:
   • a JPEG with a real APP1 Exif segment comes out with the marker gone;
   • an MP4 written with exiftool -GPSCoordinates=… comes out with no surviving QuickTime group tag (the case a naive -GPS:all gate would pass);
   • an SVG with <metadata> and an embedded base64 JPEG is rejected or cleaned.
4. The second copy — after an add, assert the attachment is gone from the fake's store, so "inaccessible from outside" is tested and not just asserted.
5. Live — pre-commit run --all-files, restart the unit, then in the real group send one photo captioned !add_photo <existing person> and confirm the reply is counts-only and the person's own command returns the new photo without a restart. Confirm with journalctl --user -u signal-bot that no label appears in the log.

Deferred (kuhy's Q15: "no functionality like this please — store it in todo")

!undo_photo — removing the most recent photo added by the calling user, within a short window. Not being built. It matters because anyone in the group may add and there is no removal path, so a bad upload is permanent until tags.json is edited by hand; tool/tagger.py is the escape hatch meanwhile. Implementing it later needs the adder's id recorded at add time, so consider recording it now even though nothing reads it yet.