Don't export wp_navigation posts by non-current users

[dsas]

Fixes https://linear.app/a8c/issue/DOTCOM-13020

Proposed changes:

[!WARNING]
This code is overly broad and a hack. Maybe that's ok. 😞

In the past a fallback navigation was created when one didn't exist. This used to be created in render_block_core_navigation (see WordPress/gutenberg#47684) but no longer happens.

This has lead to a number of blogs having this fallback navigation created when an a12 visited their site. This means that the a12s details including - wpcom username, email address, etc are in the generated export file.

It feels unhelpful to have this in the export file, partly because it might need explaining if the legal team share an export as part of their operating procedures and fail to remove the non-material data, and partly because a12s data is shared to anyone exporting an affected blog.

This change tries to remove the information when generating the export file, but is somewhat hampered by a lack of relevant hooks and also by a way of identifying current and former a12s. Instead it chooses a relevant SQL generation, hackily identifies whether the query is the relevant part of the export, and then modifies it to add some SQL excluding wp_navigation post types that were created by users that are not currently part of the blog.

Other information:

• [ ] Have you written new tests for your changes, if applicable?
• [ ] Have you checked the E2E test CI results, and verified that your changes do not break them?
• [ ] Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

1. go to .wordpress.com/wp-admin/export.php for the second site mentioned in this P2 pcgXQF-2um-p2
2. Download the file
3. Apply this patch to your wpcom-sandbox, sandbox the site.
4. Download the file again

You should see that the wp_navigation CPT (and it's author) don't show up in the export, while the About page and it's author do.

[github-actions[bot]]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (WordPress.com Site Helper), and enable the dotcom-13020-a11n-data-appearing-in-docstorage-wxr-file branch.
• To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack-mu-wpcom-plugin dotcom-13020-a11n-data-appearing-in-docstorage-wxr-file

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions[bot]]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• :white_check_mark: Include a description of your PR changes.
  
• :white_check_mark: Add a "[Status]" label (In Progress, Needs Review, ...).
  
• :red_circle: Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  
• :white_check_mark: Add testing instructions.
  
• :white_check_mark: Specify whether this PR includes any changes to data or privacy.
  
• :white_check_mark: Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation :robot:

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

[jp-launch-control[bot]]

Code Coverage Summary

Coverage changed in 25 files. Only the first 5 are listed here.

File	Coverage	Δ%	Δ Uncovered
projects/packages/jetpack-mu-wpcom/src/features/admin-color-schemes/admin-color-schemes.php	1/2 (50.00%)	50.00%	-1 💚
projects/packages/jetpack-mu-wpcom/src/features/cloudflare-analytics/cloudflare-analytics.php	1/11 (9.09%)	9.09%	-1 💚
projects/packages/jetpack-mu-wpcom/src/features/css-monkey-patches/index.php	1/3 (33.33%)	33.33%	-1 💚
projects/packages/jetpack-mu-wpcom/src/features/first-posts-stream/first-posts-stream-helpers.php	1/11 (9.09%)	9.09%	-1 💚
projects/packages/jetpack-mu-wpcom/src/features/import-customizations/import-customizations.php	1/29 (3.45%)	3.45%	-1 💚

1 file is newly checked for coverage.

File	Coverage
projects/packages/jetpack-mu-wpcom/src/features/wpcom-navigation-export-filter/class-export-filter.php	16/23 (69.57%) 💚

Full summary · PHP report

[dsas]

The code "works" now. It means that wp_navigation posts will not be exported if they were authored by a user no longer on the site. I am somewhat concerned that this means people are going to export their site and then importing it isn't going to work.

I don't think this can be a problem on WoA sites. I'm going to port this change to wpcom so it limits the impact a tad.

[dsas]

I don't think this can be a problem on WoA sites. I'm going to port this change to wpcom so it limits the impact a tad.

Except WoA sites will not necessarily have been WoA sites during the period when these posts were being created. If we want to eliminate that problem then the solution needs to work on both WoA and Simple sites.

[mmtr]

It means that wp_navigation posts will not be exported if they were authored by a user no longer on the site. I am somewhat concerned that this means people are going to export their site and then importing it isn't going to work.

To alleviate that concern, would it be possible to change the author of the wp_navigation posts to the owner of the site? To have a more targeted approach, we can only change the wp_navigation posts that have been created by users with a @automattic.com or @a8c.com address who are not members of the site.

[dsas]

It means that wp_navigation posts will not be exported if they were authored by a user no longer on the site. I am somewhat concerned that this means people are going to export their site and then importing it isn't going to work.

To alleviate that concern, would it be possible to change the author of the wp_navigation posts to the owner of the site? To have a more targeted approach, we can only change the wp_navigation posts that have been created by users with a @automattic.com or @a8c.com address who are not members of the site.

That's a great suggestion, seems better than just omitting it.

We discussed the more focused targetting previously, the concern there is that we don't enforce that a12 wpcom accounts use an automattic email. Including the personal email is arguably less good than including an a8c email.

[rcrdortiz]

The code "works" now. It means that wp_navigation posts will not be exported if they were authored by a user no longer on the site. I am somewhat concerned that this means people are going to export their site and then importing it isn't going to work.

I don't think this can be a problem on WoA sites. I'm going to port this change to wpcom so it limits the impact a tad.

To mitigate your concern, could we continue to export the wp_navigation posts but changing the author to an existing user somehow?

Oh, sorry, I didn't continue reading the discussion haha. Miguel basically suggested the same thing.

[dsas]

To alleviate that concern, would it be possible to change the author of the wp_navigation posts to the owner of the site?

A constraint here is that there are no good places to hook, so the modifications have to be in SQL land. The SQL query to do that is more complicated than the current approach, so I'm more worried about its fragility. I'll finish it off and share it on this PR in a while.

[mmtr]

To alleviate that concern, would it be possible to change the author of the wp_navigation posts to the owner of the site?

A constraint here is that there are no good places to hook, so the modifications have to be in SQL land.

Can we do that outside of the export action?

It seems to me that the DB is polluted to some extent due to a bug, and some posts have a wrong author. So rather than waiting for the export to fix that, can we run a script to change the author or do it during a normal hook like admin_init?

Something like the logic that exempts some sites from paying for GS styles if they used it before becoming a paid feature:

• Normal hook like admin_init triggers
• If the site doesn't have a certain blog sticker, check if it has wp_navigation posts created by non-members
• If there are, update the post author and add the blog sticker

[dsas]

It seems to me that the DB is polluted to some extent due to a bug, and some posts have a wrong author. So rather than waiting for the export to fix that, can we run a script to change the author or do it during a normal hook like admin_init?

Something like the logic that exempts some sites from paying for GS styles if they used it before becoming a paid feature:

* Normal hook like `admin_init` triggers

* If the site doesn't have a certain blog sticker, check if it has wp_navigation posts created by non-members

* If there are, update the post author and add the blog sticker

I remember that originally we discussed and discarded the idea of running a script over "every" site to fix the db, though I can't remember specifically why.

Depolluting the DB seems like a fine idea though. We could do it on the export_wp hook if we want to be sure it's done whenever an export happens.

[dsas]

Marking as abandoned for now - I'll investigate an alternative solution.