Page MenuHomePhabricator

Clean up GrowthExperiments UserImpact timezone handling
Closed, ResolvedPublic

Description

Initially we used the user's time zone to cluster edits into daily stats, which caused T328643: CVE-2023-29137: GrowthExperiments: UserImpactHandler returns timezone preference data for arbitrary users. User data was removed in a quick-and-dirty way, but we should probably clean up the code.

We should probably use the wiki's time zone ($wgLocalTZoffset).

Event Timeline

kostajh triaged this task as Medium priority.Feb 16 2023, 9:48 AM
kostajh added a project: Essential-Work.
DMburugu raised the priority of this task from Low to Medium.Apr 28 2023, 5:16 PM

Change 936750 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] [WIP] User impact: timezone cleanup

https://gerrit.wikimedia.org/r/936750

Sgs changed the task status from Open to In Progress.Jul 13 2023, 2:41 PM
Sgs moved this task from In Progress to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.
Urbanecm_WMF subscribed.

CI fails on the patch consistently, moving out of Code Review

Sgs lowered the priority of this task from Medium to Low.Nov 9 2023, 11:44 AM
Sgs moved this task from Doing to Code Review on the Growth-Team (Sprint 2 (Growth Team)) board.
Urbanecm_WMF changed the task status from In Progress to Open.Nov 14 2023, 1:46 PM

Change 936750 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] User impact: timezone cleanup

https://gerrit.wikimedia.org/r/936750

Actually... There's no behaviour to check. Closing.

Change 977645 had a related patch set uploaded (by Urbanecm; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.5] User impact: timezone cleanup

https://gerrit.wikimedia.org/r/977645

FTR: I ended up backporting this to make the T351898 fixes easier to backport.

Change 977645 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.5] User impact: timezone cleanup

https://gerrit.wikimedia.org/r/977645

Mentioned in SAL (#wikimedia-operations) [2023-11-27T12:56:41Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:977613|Compress geui_data json blobs (T351898)]], [[gerrit:977645|User impact: timezone cleanup (T329700)]], [[gerrit:977629|UserImpact: Make smaller SQL queries (T351898)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-27T13:04:18Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:977613|Compress geui_data json blobs (T351898)]], [[gerrit:977645|User impact: timezone cleanup (T329700)]], [[gerrit:977629|UserImpact: Make smaller SQL queries (T351898)]] (duration: 07m 37s)

Change 977671 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] UserImpact: Bump VERSION to 10

https://gerrit.wikimedia.org/r/977671

Change 977614 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.5] UserImpact: Bump VERSION to 10

https://gerrit.wikimedia.org/r/977614

Change 977614 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.5] UserImpact: Bump VERSION to 10

https://gerrit.wikimedia.org/r/977614

Mentioned in SAL (#wikimedia-operations) [2023-11-27T14:02:06Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:977614|UserImpact: Bump VERSION to 10 (T329700)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-27T14:03:23Z] <urbanecm@deploy2002> urbanecm: Backport for [[gerrit:977614|UserImpact: Bump VERSION to 10 (T329700)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-11-27T14:10:03Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:977614|UserImpact: Bump VERSION to 10 (T329700)]] (duration: 07m 56s)

Change 977671 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] UserImpact: Bump VERSION to 10

https://gerrit.wikimedia.org/r/977671

FTR: I ended up backporting this to make the T351898 fixes easier to backport.

...and i saved us from fixing a train blocker by backporting a faulty patch. Fixed by bumping the version. Filled T352031: Improve UserImpact coverage: Test UserImpact::VERSION is bumped when required to prevent similar issues from reoccuring.