jetpack icon indicating copy to clipboard operation
jetpack copied to clipboard
verified publisher icon Automattic

Protect: Integrate Scan

Open nateweller opened this issue 3 years ago • 1 comments

This is the primary pull request for integrating Scan into the Protect plugin (pbuNQi-38Y-p2).

Changes proposed in this Pull Request:

Task PR Status
Formalize status, extension, and threat data with model classes #25426 🟣 Merged
Update use of 'vulnerability' naming to more generic 'threat' convention #25445 🟣 Merged
Add the Scan API as a data source for the Protect UI https://github.com/Automattic/jetpack/pull/26119 🟡 Pending Crew Review
Update existing core, theme, and plugin threat modals to include new data from the Scan API #26533 🟡 In Progress
Add support for File and Database threats to the Protect UI #26418 🟡 In Progress

External Dependent Pull Requests :

Task PR Status
Move generation of threats to WPCOM D86732-code 🟣 Merged
Add initial version of "scanifest" N/A 🟡 In Progress

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?

Jetpack product discussion

pbuNQi-38Y-p2

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

No!

Testing instructions:

  • Validate that all above PRs are reviewed, approved, and merged.

nateweller avatar Sep 06 '22 17:09 nateweller

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.
  • :warning: All commits were linted before commit.
  • :white_check_mark: Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • :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:

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Then, add the "[Status] Needs Team review" label and ask someone from your team review the code. Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Debug Helper plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Backup plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Boost plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Search plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Social plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 29, 2022.

Starter Plugin plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Protect plugin:

  • Next scheduled release: December 6, 2022.
  • Scheduled code freeze: November 28, 2022.

Videopress plugin:

  • Next scheduled release: December 2, 2022.
  • Scheduled code freeze: November 25, 2022.

github-actions[bot] avatar Sep 06 '22 17:09 github-actions[bot]

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run bin/jetpack-downloader test jetpack add/scan-in-protect to get started. More details: p9dueE-5Nn-p2

github-actions[bot] avatar Oct 11 '22 01:10 github-actions[bot]

In preparation for merge/release, we opened up a call for testing (peb6dq-7C-p2) a few weeks ago. Some significant work came out of the testing process, primarily around ensuring user connection and plan data was addressed appropriately in all possible scenarios. I've summarized the results of the first call for testing here: peb6dq-au-p2

Related PRs merged into this branch as part of this process:

  • https://github.com/Automattic/jetpack/pull/26966
  • https://github.com/Automattic/jetpack/pull/27011
  • https://github.com/Automattic/jetpack/pull/27036
  • https://github.com/Automattic/jetpack/pull/27041
  • https://github.com/Automattic/jetpack/pull/27042
  • https://github.com/Automattic/jetpack/pull/27174
  • https://github.com/Automattic/jetpack/pull/27175
  • https://github.com/Automattic/jetpack/pull/27176
  • https://github.com/Automattic/jetpack/pull/27104
  • https://github.com/Automattic/jetpack/pull/27335
  • https://github.com/Automattic/jetpack/pull/27330

nateweller avatar Nov 08 '22 19:11 nateweller

I did an initial functional test and did not see anything blocking. The wp-admin nav menu will show both Scan (off-site link to Cloud) and Protect so will want to ideally ship Jetpack when we ship the first version of this (or at least the first really publicized version).

Ran out of time for my day right now, so can't make a pass at the code yet.

kraftbj avatar Nov 09 '22 23:11 kraftbj

Myself and @dkmyta identified a potential issue yesterday with inconsistent plan data availability, that we are currently looking into: 1201069996155224-as-1203341574761584

nateweller avatar Nov 10 '22 16:11 nateweller

Issues resolved, and we're ready for a Crew re-review and merge :+1:

Major changes since approval:

  • https://github.com/Automattic/jetpack/pull/27425
  • https://github.com/Automattic/jetpack/pull/27355
  • https://github.com/Automattic/jetpack/pull/27408

nateweller avatar Nov 16 '22 05:11 nateweller

Thank you for the thoughtful review @coder-karen! I will make a note to review the scan progress flow in Calypso, and add a task to our work board for improving the translation strings.

In terms of the "History" tab in Calypso, I can explain how it currently functions, but having a more descriptive history like you're describing would be a valuable improvement. Currently, it is only a history of threats that have been addressed. Any current threats are shown in the main screen, and the history tab shows any threats that have been fixed or ignored. I imagine we'll probably end up integrating the history tab into the Protect plugin at some point, which might present a good opportunity to consider any potential improvements at that point. I'll make sure to note that as well :+1:

Going to merge this! :ship: :ship: :ship:

nateweller avatar Nov 16 '22 17:11 nateweller