Log MP4 metadata write map and add Producer write-key fallback#34
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds logging for the MP4 metadata write map (with lyrics filtered out) and introduces a QuickTime-compatible fallback for writing and verifying the Producer tag in MP4 files, all within server/processor.js. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
console.info('[process] metadata write map', metaToWriteWithoutLyrics)runs unconditionally and may be noisy or leak user metadata in production; consider gating it behind a debug flag or environment check. - The
metaToWriteWithoutLyricsfiltering logic is inline and regex-based; if similar filtered logging is needed elsewhere, consider extracting a small helper (e.g.filterLyricsKeys(meta)) for reuse and easier adjustment of the filtering rule.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `console.info('[process] metadata write map', metaToWriteWithoutLyrics)` runs unconditionally and may be noisy or leak user metadata in production; consider gating it behind a debug flag or environment check.
- The `metaToWriteWithoutLyrics` filtering logic is inline and regex-based; if similar filtered logging is needed elsewhere, consider extracting a small helper (e.g. `filterLyricsKeys(meta)`) for reuse and easier adjustment of the filtering rule.
## Individual Comments
### Comment 1
<location path="server/processor.js" line_range="164-165" />
<code_context>
const wipeMarkers = detectMarkers(wipeTags);
const wipeVerificationPassed = wipeMarkers.length === 0;
const metaToWrite = buildMetaToWrite(platform, metadata);
+ const metaToWriteWithoutLyrics = Object.fromEntries(Object.entries(metaToWrite).filter(([key]) => !/lyrics/i.test(key)));
+ console.info('[process] metadata write map', metaToWriteWithoutLyrics);
try {
await exiftool.write(outputPath, metaToWrite, ['-overwrite_original']);
</code_context>
<issue_to_address>
**🚨 issue (security):** Logging full metadata (minus lyrics) may expose sensitive user data and adds noise in production logs.
This logs nearly the entire metadata map for every file, which can include user-identifiable details and will be very noisy in production. Please either move this to a debug/trace logger with environment gating, or narrow it to only the minimal fields needed for diagnostics (e.g., a small subset of keys or summary counts).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const metaToWriteWithoutLyrics = Object.fromEntries(Object.entries(metaToWrite).filter(([key]) => !/lyrics/i.test(key))); | ||
| console.info('[process] metadata write map', metaToWriteWithoutLyrics); |
There was a problem hiding this comment.
🚨 issue (security): Logging full metadata (minus lyrics) may expose sensitive user data and adds noise in production logs.
This logs nearly the entire metadata map for every file, which can include user-identifiable details and will be very noisy in production. Please either move this to a debug/trace logger with environment gating, or narrow it to only the minimal fields needed for diagnostics (e.g., a small subset of keys or summary counts).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 630b6c70ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const wipeVerificationPassed = wipeMarkers.length === 0; | ||
| const metaToWrite = buildMetaToWrite(platform, metadata); | ||
| const metaToWriteWithoutLyrics = Object.fromEntries(Object.entries(metaToWrite).filter(([key]) => !/lyrics/i.test(key))); | ||
| console.info('[process] metadata write map', metaToWriteWithoutLyrics); |
There was a problem hiding this comment.
processMediaFile now unconditionally logs metaToWriteWithoutLyrics, which still contains user-supplied fields like title, artist, producer, description, comment, and keywords. In production this persists potentially sensitive user metadata in server logs for every job, expanding data exposure beyond what was previously logged (which only emitted booleans for description/tags/lyrics). This diagnostic should be opt-in (eg. debug-level/env-guarded) or redacted to avoid leaking user content.
Useful? React with 👍 / 👎.
1 participant
Motivation
ItemList:Title, so we need to determine whether missing Artist/Producer/Copyright is due to the frontend/backend not supplying values or due to ExifTool/QuickTime tag compatibility.Description
buildMetaToWritethat printsmetaToWritewith lyrics keys filtered out viametaToWriteWithoutLyricsusingconsole.info('[process] metadata write map', metaToWriteWithoutLyrics).ItemList:Producerand a QuickTime-compatible fallbackKeys:Producerwhen a producer is present.['Producer','ItemList:Producer','Keys:Producer']so the new fallback is recognized as a successful write.server/processor.jsonly.Testing
node --check server/processor.jswhich succeeded.server/processor.js(local diff inspection succeeded).Codex Task
Summary by Sourcery
Log MP4 metadata write map during processing and add a QuickTime-compatible Producer tag fallback for writing and verification.
New Features: