When I was putting together the patch for T385792, I'm pretty sure I ended up cross-referencing between multiple different pages on how/what should be done (probably the ones on MW.org & Wikitech IIRC); especially to make sure I got it right -- submitting a security patch is not really something you want to do something wrong with, which IMO makes good/clear documentation on the subject all the more important. Any improvements & consolidation to these docs would be greatly welcomed (for one) by me <3

As an idea, maybe the canonical location for 'how to develop/create a security patch' could be its own specific page? Not everyone developing a patch will also be a person in charge of deploying to WMF production, and so that might help to separate it from extra docs that the person developing the patch doesn't necessarily need to know/worry about.

I ended up keeping both pages since https://www.mediawiki.org/wiki/Developing_security_patches talks about more than just Gerrit. I still left the technical patch documentation in https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Security_patches. Definitely open to feedback about further changes.

Thinking about it; IMO, the instructions on how to create/export the actual patch itself might be better located on the MW.org page than on the Wikitech page, given (for one thing) that not everyone developing a patch will be responsible for deploying it to WMF production; and also because the idea of reviewing patches in private tasks (prior to pushing them to Gerrit) is not limited to WMF-deployed extensions (i.e., where a security deploy to Wikimedia wikis would be needed) — IIRC from reading security tasks for non-WMF-deployed extensions, it seems common to have security patches proposed privately within the task at first.

I'd therefore personally probably have the Wikitech page point to the MW.org page, rather than the other way around (both because Wikitech is IIUC intended to be for technical documentation specific to Wikimedia infrastructure, and as the rest of the instructions in the 'How to deploy code' page won't be relevant to a number of people who need to create a security patch).

In T398648#10990117, @Mstyles wrote:

I ended up keeping both pages since https://www.mediawiki.org/wiki/Developing_security_patches talks about more than just Gerrit. I still left the technical patch documentation in https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Security_patches.

Ok, it looks like a lot of (perhaps duplicate) content was excised from the mediawiki.org page as part of this: https://www.mediawiki.org/w/index.php?title=Developing_security_patches&diff=7735118&oldid=7645605.

In T398648#10991391, @A_smart_kitten wrote:

I'd therefore personally probably have the Wikitech page point to the MW.org page, rather than the other way around

I'd probably agree with this approach. Security patches can be developed anywhere and for anything related to Wikimedia, and the mw.org page should (eventually) have different sections on how to do all of that. Under the Security patches section on the wikitech page, there could be a link to something like mediawiki.org/wiki/Developing_security_patches#Wikimedia_production, with specific instructions for MediaWiki, extensions, skins, vendor and private code. And then likely Gitlab and Github sections (for code that lives there) and then other random things (a production microservice managed by a specific engineering team, etc.)

I'm sorry to say this, but - at least, looking at the current state of https://www.mediawiki.org/wiki/Developing_security_patches - I would probably disagree that the steps for creating a security patch are immediately clear from that page.
IMO, it would still be better for the instructions for how to create the actual patch itself to be located on the MW.org page, rather than on Wikitech. In addition, the MW.org page currently feels a bit self-repetitive, and (especially without the actual patch-making instructions) in many ways feels like it says many things except how to develop a patch: for a couple of things, the text under "Initial steps" feels like it's duplicative of https://www.mediawiki.org/wiki/Reporting_security_bugs, and it also feels to me like the important bits of the rest of the page could be summed up as pointers to wikitech:How to deploy code#Security patches, mw:GitLab/Workflows/Security patches, & security-help@wikimedia.org.

…and so, while I appreciate that everyone involved is acting in the best faith, what I feel like we're effectively left with is a MW.org page titled "developing security patches" that tells you nothing in itself about how to develop a security patch, and (in my opinion) just feels a bit disjointed.

Then again, I realise that the title of this task is "Consolidate docs on contributing / writing / deploying security patches" (my emphasis). Maybe we need another task to clean up the MW.org documentation, now that the duplication between the MW.org & Wikitech pages has been reduced/removed? /genq

[seen; will reply]

@A_smart_kitten just following up

Apologies for the delay in getting back to you here, @Mstyles.

In T398648#11040367, @Mstyles wrote:

I would be okay with getting rid of https://www.mediawiki.org/wiki/Developing_security_patches and moving that information to https://www.mediawiki.org/wiki/Reporting_security_bugs.

Personally speaking, I'm not sure about this. My gut instinct would be that instructions on how to create a security patch might not be relevant to the general security bug reporter; and therefore, I'd worry that this would unintentionally clutter that (very important!) page.

I would prefer to leave the security patch workflow in Wikitech since it is related to scap and general deploys.

It definitely can be related to scap/WMF MediaWiki deployments, but I would argue that it isn't exclusively so. Not every security patch being developed will be for deployment via scap: to give a few examples, non-WMF-deployed extensions will (IIUC) have security patches uploaded directly to Gerrit/GitLab following their approval in private (as may some Toolforge/Cloud VPS projects); and - although I don't know the security-bug processes the mobile app teams follow - it occurs to me that patches for the Wikipedia mobile apps may also need to be reviewed in private prior to being submitted to the respective app stores.

Maybe this is worth an email to wikitech-l, in order to gauge the views of the wider developer community? (@SomeRandomDeveloper, I noticed you subscribed to this task — if you don't mind me putting you somewhat on the spot here(!), do you have any thoughts about any of this?)

In T398648#11040367, @Mstyles wrote:

I would prefer to leave the security patch workflow in Wikitech since it is related to scap and general deploys.

I feel like the current location of the workflow is suboptimal because it is located right in the middle of the guide on how to deploy security patches, which is not relevant at all to most vulnerability reporters. They are basically taken to a wall of text, with only one or two small paragraphs of useful information.
https://www.mediawiki.org/wiki/Reporting_security_bugs#Contributing_patches also links to deployment steps directly. I think there should at least be a disclaimer that makes it clear that this information is likely not relevant to the reader, unless they have shell access. Not everybody is aware what scap is and people might try to execute the deployment steps locally, given that they are barely separated from the steps explaining how to create security patches.
In my opinion, a better solution than a disclaimer like this is to clearly separate the steps for volunteer/non-WMF reporters from the rest, which could be done by keeping the security patch workflow steps on mediawiki.org.

@A_smart_kitten @SomeRandomDeveloper I personally like the documentation as is, but if you want to go ahead and make some changes, I am more than happy to review it.

In T398648#11094881, @Mstyles wrote:

@A_smart_kitten @SomeRandomDeveloper I personally like the documentation as is, but if you want to go ahead and make some changes, I am more than happy to review it.

To be honest, I don't know if I'd consider myself to be experienced enough with security patches to draft the docs myself (though I am also happy to provide review). Additionally, I likely wouldn't have the available time for it at the moment, unfortunately.

Genuine question: is there a reason why the Security-Team/Tech-Docs-Team can't do this? I only ask as (I assume) these will be the teams within the WMF with the greatest domain knowledge on security/documentation-writing matters. I appreciate that teams have limited amounts of time in which to do things, but this seems like it should hopefully be a relatively minor documentation refactor for those with the relevant knowledge.

@A_smart_kitten I've already made all of the documentation changes that I feel are necessary. If there's any other changes that you think should happen, those will need to come from you. If that won't work, I propose we close out this ticket as the documentation has already been updated.

Genuine question: is there a reason why the Security-Team/Tech-Docs-Team can't do this? I only ask as (I assume) these will be the teams within the WMF with the greatest domain knowledge on security/documentation-writing matters. I appreciate that teams have limited amounts of time in which to do things, but this seems like it should hopefully be a relatively minor documentation refactor for those with the relevant knowledge.

Hey folks!

I'm Sean Long, engineering manager for Quality Services and Technical Documentation teams. I'm new at the Foundation as of June but noticed the conversation here and wanted to offer a couple words.

There's room to clean up here but if we tackle this page it probably makes sense to tackle the ~70 other security related pages too, such as Application Security Reviews, Security for new services and the entries linked in the Mediawiki Security Guide.

We're pretty booked with work right now and I could see this project needing some careful scoping (do we want to review all security docs and consolidate them better? Should we rewrite everything that needs updating immediately? When and where do we stop?). Based on what I'm reading above, it's important to not just do a small iterative rework but plan for a more full review.

My thought is this:

This ticket might be completed based on @Mstyles' note above. Should we create a new task for the Tech Docs team to intake and plan, around reviewing/organizing the security documentation completely? Tech Docs could intake that and we could discuss it with other upcoming priorities we've got.

And until then, I wonder if it would make sense to make use of the Security_sidebar template.

Hey @SLong-WMF -

it probably makes sense to tackle the ~70 other security related pages too, such as Application Security Reviews, Security for new services and the entries linked in the Mediawiki Security Guide.

I'd agree and we've tried this before, a couple of times, e.g. T319318. But those efforts mostly failed, likely for not being pioritized or treated like an actual project.

This ticket might be completed based on @Mstyles' note above. Should we create a new task for the Tech Docs team to intake and plan, around reviewing/organizing the security documentation completely?

Sounds reasonable to me.

And until then, I wonder if it would make sense to make use of the Security_sidebar template.

That could be helpful, but would likely need to be cleaned up as several of those links reference dated or irrelevant topics and projects.

In T398648#11140417, @Mstyles wrote:

@A_smart_kitten I've already made all of the documentation changes that I feel are necessary. If there's any other changes that you think should happen, those will need to come from you. If that won't work, I propose we close out this ticket as the documentation has already been updated.

To be honest, I feel at least a little bit disappointed. From where I'm sitting, it feels a bit like the secteam has made some changes to these docs, received feedback that not all the changes made were necessarily positive/that some of the changes made to the docs could be improved; but is now declaring its own work to be complete (and saying that if the feedback-givers want any changes made to what the secteam's done with the docs, they need to make those changes themselves).

I appreciate that the doc-updates felt necessary by the secteam have been made, but I would have hoped that the team would be [more] receptive to feedback from multiple people on where those updates could be improved, rather than passing the buck back to them. Unfortunately, in this task, I feel like my personal impression is that this hasn't really been the case.
[For what it's worth, if our docs went through a 'code-review'-type process (rather than being pages on a wiki), I feel like the onus would be more on the part of the person proposing the changes to adapt them in response to feedback, rather than being on the reviewers to propose further adaptations themselves post-merge.]

As I said, I don't think I'd consider myself experienced enough with security patches to draft the docs myself. However, if no further work is going to be done here, then I wonder if it might genuinely be worth reverting the MW.org page back to the status quo ante (ie., as it was before this task) or similar [maybe like https://www.mediawiki.org/w/index.php?oldid=7645605, but still removing the content from Deploying security patches to production onwards?]. The page will still feel a bit disjointed, and some of the same issues as identified before this task may still exist; but at least then the page will contain some information on the actions needed to create a security patch, and at least then the information will be available in a relatively clear place on MW.org (rather than in the middle of a page on how to deploy code to Wikimedia servers).

In T398648#11141698, @SLong-WMF wrote:

[...]
There's room to clean up here but if we tackle this page it probably makes sense to tackle the ~70 other security related pages too, such as Application Security Reviews, Security for new services and the entries linked in the Mediawiki Security Guide.

We're pretty booked with work right now and I could see this project needing some careful scoping (do we want to review all security docs and consolidate them better? Should we rewrite everything that needs updating immediately? When and where do we stop?). Based on what I'm reading above, it's important to not just do a small iterative rework but plan for a more full review.

My thought is this:

This ticket might be completed based on @Mstyles' note above. Should we create a new task for the Tech Docs team to intake and plan, around reviewing/organizing the security documentation completely? Tech Docs could intake that and we could discuss it with other upcoming priorities we've got.
[...]

Thanks for the response from the Tech-Docs-Team point-of-view! I've commented above with my thoughts about this specific ticket; but notwithstanding that, I think your proposal for the Tech-Docs-Team to intake/plan a project to do a full assessment/(re-)organization of the security docs is a good idea, and - speaking personally - I would be happy to see it go ahead :)

In T398648#11141750, @sbassett wrote:

I'd agree and we've tried this before, a couple of times, e.g. T319318. But those efforts mostly failed, likely for not being pioritized or treated like an actual project.

Interesting, thanks for sharing the link. As I'm fairly new to the org, I can't comment on what might have happened 3 years ago. Your summation certainly sounds likely though.

I'm tempted to simply re-open that and put it in the Tech Docs backlog for discussion though. Likely this would be something we would need to build capacity for in a longer-term plan, considering the potential scope, and also the priority is somewhat clear even now due to the focus I'm seeing on other security projects.