@sbassett can I just upload the fix publicly and then backport it? I think this is okay because:

1. This does not affect production (because temporary accounts are not enabled), and as such does not need to be deployed there
2. CheckUser is not a bundled extension, and as such my understanding is that security patches can be backported at any time after production is fixed (which doesn't apply because of point 1)

I ask this because I want to run the patch I have through CI, which I can't do unless it is on gerrit.

I have the following which I think is ready for review. The tests pass on my local environment, but I do not know whether they will pass in CI.

In T355434#9486169, @Dreamy_Jazz wrote:

@sbassett can I just upload the fix publicly and then backport it? I think this is okay because:

1. This does not affect production (because temporary accounts are not enabled), and as such does not need to be deployed there
2. CheckUser is not a bundled extension, and as such my understanding is that security patches can be backported at any time after production is fixed (which doesn't apply because of point 1)

I'd agree that having this go through gerrit is low-risk given the above justifications. We can open up this task as well, if you'd like, though I did add @gerritbot as a subscriber. I'd also like to track this at T353904.

In T355434#9486206, @sbassett wrote:

In T355434#9486169, @Dreamy_Jazz wrote:

@sbassett can I just upload the fix publicly and then backport it? I think this is okay because:

1. This does not affect production (because temporary accounts are not enabled), and as such does not need to be deployed there
2. CheckUser is not a bundled extension, and as such my understanding is that security patches can be backported at any time after production is fixed (which doesn't apply because of point 1)

I'd agree that having this go through gerrit is low-risk given the above justifications. We can open up this task as well, if you'd like, though I did add @gerritbot as a subscriber. I'd also like to track this at T353904.

Thanks. I will upload to this gerrit now.

We can probably leave this protected until the backports are done, but happy either way.

Change 992795 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@master] [SECURITY] Check for deleted status in temp account REST APIs

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

Change 992769 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_41] [SECURITY] Check for deleted status in temp account REST APIs

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

Change 992770 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_40] [SECURITY] Check for deleted status in temp account REST APIs

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

Change 992770 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_40] [SECURITY] Check for deleted status in temp account REST APIs

Reason:

Discussed in TSP meeting and decided that we shouldn't need backports.

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

Change 992769 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_41] [SECURITY] Check for deleted status in temp account REST APIs

Reason:

Discussed in a TSP meeting and decided this shouldn't need backports.

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

Change 992795 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] [SECURITY] Check for deleted status in temp account REST APIs

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

Suggested QA steps:

1. Enable temporary account autocreation
2. Make at least two edits and two log actions on one temporary account username. Then make one or more edits on another temporary account username.
3. Log into an account with the checkuser and suppress groups
4. Block the second temporary account from step 2 with hideuser
5. Suppress the performer of one edit and one log action from the temporary user created first in step 2. Note down the revision ID and log ID you suppressed for future steps.
6. Make a request to /checkuser/v0/temporaryaccount/{name} where {name} is replaced with the temporary account you suppressed in step 4
7. Verify that you see IP addresses in the result
8. Log out and then log into an account with just the checkuser group
9. Repeat step 6 and verify that you instead receive a 404 error
10. Get the revision ID of the suppressed edit from step 5and one other revision ID for an edit performed by that temporary account. Use these to make a request to /checkuser/v0/temporaryaccount/{name}/revisions/{ids} where {ids} is generated by combining the revision IDs using the | character and {name} is the temporary account username. For example, /checkuser/v0/temporaryaccount/*Unregistered 10/revisions/101|102
11. Verify that the response to the request in step 10 has one entry in the ips array for the revision ID that did not have the performer suppressed, but the revision ID that was suppressed is not present.
12. Get the log ID of the suppressed log from step 5 and one other log ID performed by that temporary account. Use these to make a request to /checkuser/v0/temporaryaccount/{name}/logs/{ids} where {ids} is generated by combining the log IDs using the | character and {name} is the temporary account username. For example, /checkuser/v0/temporaryaccount/*Unregistered 10/logs/101|102
13. Verify that the response to the request in step 12 has one entry in the ips array for the log ID that did not have the performer suppressed, but the log ID that was suppressed is not present.

@Dreamy_Jazz Everything was good until the last couple of steps for me with the logs. Can you please review the .webm I have below to see if I'm missing something from steps 12-13?

Status: ❌ FAIL (Steps 12-13)
Environment: Local: 1.42.0-alpha (289a900)17:59, 13 February 2024; Checkuser: 2.5 (4642899) 11:11, 13 February 2024
OS: macOS Sonoma 14.2.1
Browser: Chrome 121
Skins. Vector 2022
Device: MBA M2
Emulated Device:: n/a
Test Links:
http://localhost:8080/w/index.php?title=Lion&action=history
http://localhost:8080/w/index.php?title=Samurai&action=history

Steps 1-3 ✅

Temp Acct (2 edits/2log)	Temp Acct just edits	User w/checkuser &suppress

Steps 4-7 ✅

Steps 8-9 ✅

Steps 10-11 ✅

Steps 12-13 Log ID 17 is suppressed Special:Log ❌
http://localhost:8080/w/rest.php/checkuser/v0/temporaryaccount/~2024-13/logs/16|17

@Dreamy_Jazz

I tried another log type with "move" and I'm seeing both IP addresses even though the other one is suppressed as seen in the .webm below with a user with only checkuser only rights.

@Dreamy_Jazz Previous steps have been passed as seen in https://phabricator.wikimedia.org/T355434#9539736. After suppressing in Special:Log for step 12, I was able to get the result as expected as seen in the screenshots below. The original issue that was noticed of that the CheckUser extension does not store the IP address associated with the log event for page creation, T357523: Temporary account IP reveal fails for log entries that are for the creation of a page has been created. This task will be moved to Done. Thanks for all your work and steps!

Status: ✅PASS (Steps 12-13)
Environment: Local: 1.42.0-alpha (289a900)17:59, 13 February 2024; Checkuser: 2.5 (4642899) 11:11, 13 February 2024
OS: macOS Sonoma 14.2.1
Browser: Chrome 121
Skins. Vector 2022
Device: MBA M2
Emulated Device:: n/a
Test Links:
http://localhost:8080/w/index.php?title=King_of_Beasts&action=history
Special:Log

Log Surpressed	Result