[openipc-ai]

Published report photos kept their EXIF, so phone GPS was public: 15 photos from r-yqmjaqfm and r-wegm9rhs served exact coordinates (e.g. /api/v1/reports/r-wegm9rhs/files/2). Those were mitigated by hand on 2026-11-01 with a one-pixel blur (its re-encode drops all metadata). This fixes the cause.

What changes

• reports.StripPhoto (new, strip.go): a lossless walk of JPEG segments / PNG chunks / WebP chunks. It drops EXIF, XMP, IPTC, MPF, COM, tEXt/zTXt/iTXt/tIME/eXIf, and anything after the end of image (phones append a second picture with its own EXIF). It keeps the scans, tables, JFIF, the ICC profile and Adobe APP14, and keeps a non-upright orientation as a one-tag EXIF. A photo with nothing to drop comes back unchanged, so public_sha256 == sha256 as before.
• Every public copy of a photo goes through it:
  • at upload (prepare)
  • a reviewer's as_sent (SetPhotoPublic)
  • openipc reports redact, which now also re-makes photos published as sent and lists any it cannot walk under unreadable.
• A photo whose structure can't be walked is refused at upload with a 415.
• The member page's as_sent/blurred label comes from the last photo review, because a stripped copy also differs from the original.
• Test fixtures: the fake JPEGs in reports and club tests are now walkable (JFIF, a scan, EOI).

Checked

• service/run.sh test: all packages pass. New tests cover a phone JPEG with GPS, XMP, MPF, a trailer and orientation 6; PNG; WebP's VP8X flags and RIFF size; restart markers inside a scan; unreadable input; and the whole path of upload → serve → legacy row → Redo → blur → as_sent.
• A throwaway binary on the host ran StripPhoto over all 156 stored photo originals. All of them walk, they decode with the same bounds, and 65 had metadata to drop.

After deploy: run openipc reports redact --dry-run, then openipc reports redact. Then run reports photo … as_sent on the 15 photos blurred on 2026-11-01, so their senders see "as sent" again.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Strip metadata from public report photos

🐞 Bug fix 🕐 40+ Minutes

AI Description

• Strip sensitive photo metadata while preserving image data, color profiles, and orientation.
• Protect uploads, reviewer republication, and historical photos; reject unreadable uploads.
• Derive member-facing review labels from decisions rather than byte hashes.

Diagram

graph TD
  Upload["Photo upload"] --> Original[("Original files")] --> Strip["StripPhoto"] --> Public[("Public copies")] --> Serve["Public API"]
  Upload --> Strip
  Review["As-sent review"] --> Strip
  Redo["Redact CLI"] --> Strip

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Re-encode every public photo

• ➕ Would discard most container metadata without maintaining three chunk walkers.
• ➖ Changes image bytes and potentially quality; requires encoding support and careful orientation and color-profile handling.

2. Allowlist publishable chunks

• ➕ Provides a stronger boundary against metadata in unknown or vendor-specific chunks.
• ➖ May remove ancillary data needed to display some photos correctly; requires broader compatibility testing.

Recommendation: The lossless structural approach fits this fix because it preserves image fidelity and unchanged hashes when nothing needs stripping. Consider stricter chunk allowlisting if the privacy guarantee must also cover metadata in unrecognized ancillary chunks.

Files changed (11) +716 / -18

Bug fix (5) +365 / -10

club.goBase member photo labels on review decisions +5/-2

Base member photo labels on review decisions

• Uses the latest photo review to distinguish blurred from as-sent public photos, since stripping metadata can make an as-sent copy differ from its original.

service/internal/reports/club.go

handler.goStrip uploads and reject unreadable photo structures +36/-0

Strip uploads and reject unreadable photo structures

• Validates that uploaded photos can be stripped, returning HTTP 415 when they cannot. Prepares a stripped public copy while retaining the original and reusing its hash when no bytes change.

service/internal/reports/handler.go

photoreview.goStrip photos republished as sent +19/-0

Strip photos republished as sent

• Routes reviewers’ photo as-sent decisions through public-copy creation instead of pointing publication directly at the original. Retains file locking and copy placement around the decision.

service/internal/reports/photoreview.go

redact.goRemake historical as-sent photo copies +52/-8

Remake historical as-sent photo copies

• Adds reusable stripped-photo copy creation and includes published, non-blurred photos in 'Redo'. Reports photos it cannot walk as 'unreadable' without changing them, while preserving dry-run behavior.

service/internal/reports/redact.go

strip.goAdd lossless JPEG, PNG, and WebP metadata stripping +253/-0

Add lossless JPEG, PNG, and WebP metadata stripping

• Walks supported image containers to remove specified metadata and trailing data without re-encoding picture data. Preserves a non-upright orientation as minimal EXIF and returns an error for structures it cannot walk.

service/internal/reports/strip.go

Tests (3) +340 / -3

newcamera_test.goMake club photo fixtures structurally walkable +5/-2

Make club photo fixtures structurally walkable

• Adds a scan and end-of-image marker to the synthetic JPEG fixture. Keeps generated variants distinct without changing the end marker.

service/internal/club/newcamera_test.go

api_test.goAdapt report fixtures and test truncated-photo rejection +5/-1

Adapt report fixtures and test truncated-photo rejection

• Makes the synthetic JPEG walkable and adds a truncated photo to the upload rejection cases.

service/internal/reports/api_test.go

strip_test.goCover metadata stripping and the publication lifecycle +330/-0

Cover metadata stripping and the publication lifecycle

• Tests JPEG, PNG, and WebP stripping, orientation, unchanged pixels, scan markers, and unreadable input. Exercises upload and serving, historical 'Redo', blur, and subsequent as-sent republication.

service/internal/reports/strip_test.go

Documentation (3) +11 / -5

CLAUDE.mdDocument the public-photo metadata policy +4/-0

Document the public-photo metadata policy

• Explains that as-sent public photos omit metadata and JPEG trailers, preserve orientation, and are refused when their structure cannot be walked.

CLAUDE.md

reports.goClarify historical photo remediation in CLI help +3/-2

Clarify historical photo remediation in CLI help

• Expands the 'reports redact' usage text to include stripping metadata from photos published as sent.

service/cmd/openipc/reports.go

photo.goClarify what as-sent photo publication means +4/-3

Clarify what as-sent photo publication means

• Documents that a photo published as sent retains its picture but not its metadata; the original remains available to its sender and maintainers.

service/internal/reports/photo.go

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. JPEG thumbnails can still expose GPS ✓ Resolved

Description

stripJPEG copies every APP0 segment unchanged, including JFXX extensions that can contain an
embedded JPEG with its own EXIF. When a sender uploads a JPEG with such a thumbnail, upload
validation accepts the outer image and the embedded location data remains in the public copy.

Code

service/internal/reports/strip.go[R122-125]

+		if m == 0xE0 && len(out) == 2 {
+			at = 2 + len(seg)
+		}
+		out = append(out, seg...)

Evidence

The new JPEG walker removes top-level APP1 but appends APP0 segments verbatim. The upload check
calls that same walker, and the resulting bytes are selected for public storage and serving.

service/internal/reports/strip.go[111-125]
service/internal/reports/handler.go[613-618]
service/internal/reports/handler.go[671-684]
service/internal/reports/handler.go[459-476]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
APP0 JFXX extensions can hold a second JPEG, whose EXIF survives unchanged in a published photo.
## Fix Focus Areas
- service/internal/reports/strip.go[111-125]
- service/internal/reports/strip_test.go[43-57]
## Recommended Fix
Distinguish the JFIF header needed for display from APP0 extensions. Drop JFXX embedded thumbnails or sanitize their nested JPEG bytes, and test that nested EXIF cannot appear in the public output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Other image chunks can expose metadata ✓ Resolved

Description

stripPNG and stripWebP remove named metadata chunks but copy unrecognized chunks without
checking whether they contain image data or private application data. A sender's PNG or WebP
containing location details in another ancillary or RIFF chunk passes strippable, leaving those
details in its public copy.

Code

service/internal/reports/strip.go[R180-185]

+		case "IDAT":
+			if at < 0 {
+				at = len(out)
+			}
+		}
+		out = append(out, chunk...)

Evidence

Both new walkers have a default path that copies chunks not on their short removal lists. Photo
upload checks only that the walker succeeds, then records its output as the public copy.

service/internal/reports/strip.go[172-185]
service/internal/reports/strip.go[224-236]
service/internal/reports/handler.go[613-618]
service/internal/reports/handler.go[671-684]
service/internal/reports/handler.go[468-476]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Unknown PNG and WebP chunks are copied into public photos despite being able to carry application metadata.
## Fix Focus Areas
- service/internal/reports/strip.go[172-185]
- service/internal/reports/strip.go[224-236]
## Recommended Fix
Use format-specific allowlists for chunks required to render the image and permitted color information; drop ancillary or application chunks that are not known safe. Add tests placing sensitive text in otherwise walkable PNG and WebP chunks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Large photo uploads can exhaust memory ✓ Resolved

Description

prepare captures each stripped photo's complete byte slice in a placement closure instead of
releasing it after processing that file. An accepted upload can contain 24 photos of up to 20 MiB
each, so the closures retain roughly 480 MiB of public-copy data until all files have been prepared
and the transaction begins.

Code

service/internal/reports/handler.go[R473-475]

+		f.PublicSHA256, f.PublicBytes = hex.EncodeToString(ps[:]), int64(len(pub))
+		if f.PublicSHA256 != f.SHA256 {
+			*places = append(*places, func() error { _, err := a.Files.Put(pub); return err })

Evidence

The limits permit 24 files of 20 MiB each. The new closure captures pub, and placement closures
execute only after the loop preparing all files.

service/internal/reports/handler.go[23-27]
service/internal/reports/handler.go[459-476]
service/internal/reports/handler.go[295-344]
service/internal/reports/handler.go[581-598]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Placement closures retain every stripped photo in memory across preparation of the entire upload.
## Fix Focus Areas
- service/internal/reports/handler.go[459-476]
- service/internal/reports/handler.go[295-344]
## Recommended Fix
Stage stripped copies in bounded temporary storage and have placement closures reopen them, or otherwise bound aggregate retained photo bytes while preserving the existing transaction and file-lock ordering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Some stripped WebP photos get a corrupt size ✓ Resolved

Description

stripWebP drops EXIF and XMP  chunks in every WebP, but when the file has no VP8X chunk (`vp8x
< 0) it returns before binary.LittleEndian.PutUint32(out[4:], ...)`, so the RIFF size still counts
the dropped bytes. A simple VP8/VP8L WebP with a trailing EXIF or XMP chunk is accepted at upload
and published with a header that claims more data than the file holds; strict decoders treat it as
truncated, and any orientation in that EXIF is silently lost.

Code

service/internal/reports/strip.go[R239-241]

+	if vp8x < 0 {
+		return out, nil
+	}

Evidence

Lines 225-236 skip appending EXIF/XMP chunks no matter whether VP8X was seen. Line 239 returns out
early when no VP8X was found, so the size fix at line 251 runs only on the VP8X path. out[4:8] was
copied from the source header at line 210 and still holds the original size. The tests in
strip_test.go only build VP8X+VP8L WebPs, so this path is never exercised.

service/internal/reports/strip.go[204-253]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`stripWebP` drops EXIF/XMP chunks but, for a WebP without a VP8X chunk, returns early without updating the RIFF size in bytes 4..8. The output then declares a larger size than its real length.
## Fix Focus Areas
- service/internal/reports/strip.go[239-252]
## Recommended Fix
In the `vp8x < 0` branch, call `binary.LittleEndian.PutUint32(out[4:], uint32(len(out)-8))` before returning, or move the size write so it runs on both paths. Add a test with a plain `VP8L` chunk followed by an `EXIF` chunk, and check that the RIFF size equals `len(out)-8` and that `image.Decode` succeeds.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗