fix(gdpr): use connect.sid for Substack credentials - #5446
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryRe-reviewed the rewritten Files Reviewed (3 files)
Previous Review Summary (commit 200540c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 200540c)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the Substack Files Reviewed (6 files)
Reviewed by grok-4.6 · Input: 53K · Output: 6.1K · Cached: 339.5K Review guidance: REVIEW.md from base branch |
200540c to
f97de3f
Compare
pandemicsyn
left a comment
There was a problem hiding this comment.
Bot flagged one, but i think as long as theres no deletions in flight when this rolls out its kind of a non issue.
| return null; | ||
| } | ||
| return trimmed.includes('=') ? trimmed : `substack.sid=${trimmed}`; | ||
| return trimmed.includes('=') ? trimmed : `connect.sid=${trimmed}`; |
There was a problem hiding this comment.
bot: Legacy stored substack.sid cookies are not migrated to connect.sid.
Suggested fix: Recognize and rewrite the legacy cookie name in raw cookie material before returning it (including when it appears in a multi-cookie header), then add a regression test for substack.sid=.... Add or update a handler-level test that stores the legacy value and asserts the outbound deletion request sends connect.sid=.... This is needed because the deletion handler calls this normalizer for persisted credentials, while its existing test setup still stores substack.sid=test-cookie.
Summary
connect.sidcookie when normalizing stored credentialshttps://blog.kilo.aiand share that behavior with the deletion handlerVerification
oxfmt --list-differentpassedoxlintpassed with 0 warnings and 0 errorsgit diff --checkpassedts-node