fix(conversations): stop the public shared endpoint leaking owner-internal fields#10273
Conversation
…ernal fields
GET /v1/conversations/{id}/shared is public and unauthenticated, but its response
returned the full Conversation model with only geolocation stripped — also exposing
the server-side encryption tier (data_protection_level) and external_data (merge
provenance: other conversation ids, plus integration metadata).
Strip data_protection_level and external_data alongside geolocation before serializing.
Scope is narrow: only owner-internal fields. The transcript, summary and audio the owner
chose to publish are unchanged, so the response schema is unchanged.
Verified: pytest test_shared_conversation_field_exposure.py -> 1 passed.
Failure-Class: none
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the focused privacy fix — this looks like the right direction to me.
I reviewed the diff and ran the new targeted backend unit test locally:
.venv/bin/python -m pytest tests/unit/test_shared_conversation_field_exposure.py -q
Result: 1 passed.
The change is narrow and correctly strips data_protection_level and external_data before conversation_to_dict() serializes the public unauthenticated shared-conversation response, alongside the already-stripped geolocation. The regression test covers that serialization boundary, which is the important part here.
I’m not formally approving because the current PR state is still blocked by the repo hygiene/formatting checks being cancelled/skipped in the reported status set, so this needs maintainer/check-state review before merge. From the code review side, I don’t see a blocking implementation issue.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
kodjima33
left a comment
There was a problem hiding this comment.
fix(conversations): stop public shared endpoint leaking owner-internal fields — security fix w/ test, 5/5
What
GET /v1/conversations/{id}/sharedis public and unauthenticated — anyone with the link gets the response. It is meant to publish the transcript/summary the owner shared, but it returned the fullConversationmodel with onlygeolocationstripped, also exposing:data_protection_level— the owner's server-side encryption tier (internal).external_data— merge provenance (other conversation ids the owner merged) and integration metadata.The endpoint already nulls
geolocation, which shows the intent was a curated public view — the other internal fields were just missed.Fix
Strip
data_protection_levelandexternal_dataalongsidegeolocationbefore serializing — same pattern, same spot. Scope is deliberately narrow: only the unambiguously-internal fields. The transcript, summary, structured data, people and audio the owner chose to publish are unchanged, so the response schema (and its generated clients) is unchanged.Tests (hermetic)
test_shared_conversation_field_exposure.py— the endpoint clearsgeolocation/data_protection_level/external_databefore serialization. 1 passed.Product invariants affected
none
Failure-Class: none