Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: non dsa add-ons forwarded to Legal Escalations in Cinder don't seem to work with policies that take content down #15278

Closed
1 task done
ioanarusiczki opened this issue Jan 10, 2025 · 10 comments · Fixed by mozilla/addons-server#23024
Assignees
Milestone

Comments

@ioanarusiczki
Copy link

ioanarusiczki commented Jan 10, 2025

What happened?

Followup for #15255 (comment)
And the note from #15259 (comment)

First scenario

  1. An approved listed version is forwarded to Legal from review page with Request Review from Mozilla Legal
  2. Cinder Legal Escalations queue creates a new job
  3. Decision is taken AMO Disable Addon https://stage.cinder.nonprod.webservices.mozgcp.net/job/2853a9f6-3793-4980-a9a2-0e98c9989c43

Result:
content is not disabled on AMO

Escalate to AMO policy for such an add-on -> won't send back or flag for review the version in rev tools

Second scenario

  1. An awaiting review version is forwarded from the Manual Review queue to Legal. This could be a promoted add-on with a new version, or the auto-approval is disabled (manually or automatically)
  2. Request review from Mozilla Legal
  3. From Cinder Legal Escalations try a policy which disables the add-on https://stage.cinder.nonprod.webservices.mozgcp.net/job/b2028e94-0c12-4792-9d47-6934eb27b5e4

Result:
add-on is not disabled, the awaiting review versions are still flagged for HR in rev tools.

What did you expect to happen?

Disable content.

Are these add-ons supposed to go in Cinder if there are no reports/appeals attached? Right now any add-on can be forwarded to Legal queue in Cinder.

Is there an existing issue for this?

  • I have searched the existing issues

┆Issue is synchronized with this Jira Task

@eviljeff
Copy link
Member

@wagnerand would you expect the (promoted|auto-approval-disabled|etc) NHR for the review to be cleared in the reviewer tools if the add-on is forwarded on to legal? (I'm assuming so, but double checking)

@abyrne-moz
Copy link

I spoke with Andreas and we decided:

  1. The content should be disabled if legal chooses that action.
  2. In the event that legal forward a job back to the AMO reviewers, we need to ensure it flags for human review so they are aware it needs to be checked.
  3. If an add-on is forwarded to legal, we should clear the human review flag. However, we should add this scenario to the existing redash alerts that tell us when something gets "stuck" without action. Apparently @diox has set this up for other scenarios.

@ioanarusiczki
Copy link
Author

Looking at first scenario and trying Ignore/Not enough information policy won't send any email either.
I tried with https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/635597
Cinder job would be https://stage.cinder.nonprod.webservices.mozgcp.net/job/c18e2f37-bc16-4962-a929-1c61c9632825

@eviljeff
Copy link
Member

Looking at first scenario and trying Ignore/Not enough information policy won't send any email either. I tried with https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/635597 Cinder job would be https://stage.cinder.nonprod.webservices.mozgcp.net/job/c18e2f37-bc16-4962-a929-1c61c9632825

yes, there isn't any need to try to any other types of decision from cinder - they will all fail.

@ioanarusiczki
Copy link
Author

I've tried on AMO stage:

  • what I've described at scenario 1 or 2 about disabling from Legal is now working. Further if the developer is sending the appeal I see it's landing in Cinder T&S Escalation queue. Don't know if it's correct ❓

  • when versions are escalated back to amo they're being flagged for HR but it says "Escalated for an abuse report, via cinder". ❌
    I'll give an example https://reviewers.addons.allizom.org/en-US/reviewers/review-listed/2245207

  • versions in awaiting review would have a problem if the appeal sent by the developer is resolved from T&S Escalation with Approve policy (which would reinstate content). ❓

email is sent while versions in rev tools would be waiting review
https://reviewers.addons.allizom.org/en-US/reviewers/review-unlisted/2245223
https://stage.cinder.nonprod.webservices.mozgcp.net/job/e38ad5f9-f834-4942-a7de-5ff7827f7d9f

Image

  • ignore policy doesn't work on add-ons sent to Legal (I think that's expected, there is no abuse report)

  • 2nd level approvals if forwarded to Legal and the attempt is to disable it -> they're going back to rev tools (did not finish around these cases)

@ioanarusiczki
Copy link
Author

Forgot: release testing around DSA feature was tested today on AMO stage ✅

@ioanarusiczki
Copy link
Author

In case I'm running into something new I'll just file it
#15337
#15338

@eviljeff
Copy link
Member

I wasn't clear if any/all of these points were logged in #15337 or #15338 already. If they're not can you please log follow-up issues.

* what I've described at scenario 1 or 2 about disabling from Legal is now working. Further if the developer is sending the appeal I see it's landing in Cinder T&S Escalation queue.  Don't know if it's correct ❓

yes, that's correct

* when versions are escalated back to amo they're being flagged for HR but it says "Escalated for an abuse report, via cinder". ❌
  I'll give an example https://reviewers.addons.allizom.org/en-US/reviewers/review-listed/2245207

Wording is a pretty minor problem, but can you file an issue?

* versions in awaiting review would have a problem if the appeal sent by the developer is resolved from T&S Escalation with Approve policy (which would reinstate content). ❓

Currently, there is no code path for any kind of Approve policy or AMO_APPROVE action to approve versions. This will change in the future though, so it's something to keep considering as a possibility (we'll call out if changes are being made around AMO_APPROVE or AMO_APPROVE_VERSION at the time).

email is sent while versions in rev tools would be waiting review https://reviewers.addons.allizom.org/en-US/reviewers/review-unlisted/2245223 https://stage.cinder.nonprod.webservices.mozgcp.net/job/e38ad5f9-f834-4942-a7de-5ff7827f7d9f

I think the email being wrong for add-ons without any approved versions has been reported as an issue already.

* ignore policy doesn't work on add-ons sent to Legal (I think that's expected, there is no abuse report)

Legal should never be using Ignore for add-ons forwarded from the reviewer tools, correct, as the reviewer is explicitly asking them to make a decision.... but I'm not sure what we'd expect the outcome here to be in any case?

* 2nd level approvals if forwarded to Legal and the attempt is to disable it -> they're going back to rev tools (did not finish around these cases)

Is there something to follow-up on with this point? I'm not clear on it.

@ioanarusiczki
Copy link
Author

The wording issue is #15337

#15338 is about approve , could be a dupe, reason why it sounded somehow familiar 🙃

2nd level approvals if forwarded to Legal and the attempt is to disable it -> they're going back to rev tools (did not finish around these cases)

I don't know yet where this would fit for DSA testing phases and If it would be necessary to be tested now or later because I'd leave it aside and continue with https://mozilla-hub.atlassian.net/browse/QA-3203. My goal is to finish testing around regular extensions this week and start testing 2nd level approvals the next one so I'll see then if this scenario must be covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants