Skip to content

fix: drop display_name from logs to avoid logging PII#145

Merged
Prashant-Surya merged 1 commit into
mainfrom
fix/drop-display-name-from-logs
Jun 9, 2026
Merged

fix: drop display_name from logs to avoid logging PII#145
Prashant-Surya merged 1 commit into
mainfrom
fix/drop-display-name-from-logs

Conversation

@pratapalakshmi

@pratapalakshmi pratapalakshmi commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

PR #142 added user_id and display_name to every JSON log line. The display name is PII — logging it on every request risks violating privacy rules.

Change

In plane_mcp/__main__.py, stop collecting and emitting display_name:

  • UserContextFilter now records only user_id (claims["sub"], an opaque UUID).
  • JSONFormatter no longer emits a display_name key.
  • Docstring updated to note PII (display name / email) is intentionally never logged.

The user_id UUID is sufficient to correlate a request with its user without exposing personal data. The enrichment-error signal added previously is unchanged.

Result

{..., "logger": "fastmcp.middleware.structured_logging",
 "message": "{\"event\": \"request_success\", ...}",
 "user_id": "b3fea12b-4c99-4b8f-b726-6121c79f12aa"}

Testing

  • ruff check / format pass.
  • Verified the filter+formatter: with a token whose claims include display_name/email, the formatted line contains only user_id — no PII leaks. No-context lines stay clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • System logs now capture only opaque user identifiers instead of display names and email addresses, enhancing privacy protection and reducing exposure of sensitive user information in log files.

The display name is PII; logging it on every request line risks
violating privacy rules. Keep only the opaque user_id (UUID), which is
sufficient to correlate a request with its user without exposing
personal data.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Log enrichment in plane_mcp/__main__.py updated to emit only the authenticated user's opaque ID (sub claim from token) in structured logs. User display names and email addresses are no longer logged; instead, UserContextFilter populates record.user_id and JSONFormatter includes only that identifier in JSON output.

Changes

User Context Logging Privacy

Layer / File(s) Summary
User context filtering and JSON formatting
plane_mcp/__main__.py
UserContextFilter extracts the opaque sub claim from access tokens to set record.user_id without adding display_name. JSONFormatter emits user_id in structured JSON output and removes the previous display_name field, reducing PII in logs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • makeplane/plane-mcp-server#142: Both PRs modify plane_mcp/__main__.py's UserContextFilter and JSONFormatter to enrich JSON logs with authenticated user data, with this PR narrowing output to user_id only (dropping display_name).

Suggested reviewers

  • Prashant-Surya

Poem

🐰 Logs now whisper only secrets opaque,
No names or emails on the trail they make,
Just sub claims shining, a rabbit's delight—
Privacy protected with user_id bright!
Hop along, security review takes flight! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately and concisely describes the main change: removing display_name from logs to prevent PII logging, which directly matches the primary objective of the pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/drop-display-name-from-logs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pratapalakshmi pratapalakshmi reopened this Jun 8, 2026
@pratapalakshmi pratapalakshmi changed the title fix: drop display_name from logs to avoid logging PII [DONOT MERGE] fix: drop display_name from logs to avoid logging PII Jun 8, 2026
@Prashant-Surya Prashant-Surya changed the title [DONOT MERGE] fix: drop display_name from logs to avoid logging PII fix: drop display_name from logs to avoid logging PII Jun 9, 2026
@Prashant-Surya
Prashant-Surya merged commit 3d54d1a into main Jun 9, 2026
1 check passed
@akhil-vamshi-konam
akhil-vamshi-konam deleted the fix/drop-display-name-from-logs branch June 11, 2026 04:33
@coderabbitai coderabbitai Bot mentioned this pull request Jun 17, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants