Making WordPress.org

Opened 5 years ago

Closed 5 years ago

#4634 closed enhancement (fixed)

Add new status for updated themes and prevent them being set live automatically

Reported by: dingo_d's profile dingo_d Owned by:
Milestone: Priority: high
Component: Theme Directory Keywords: needs-patch 2nd-opinion
Cc:

Description

Currently, theme updates aren't checked by the review team. So, technically, a user could create an ok theme, pass the review and his theme would be set live. Then they could modify the theme to include some forbidden things (obtrusive upselling, demo xml in the theme or some tracking code even), and we would be none the wiser, since updates are closed and set live automatically.

This is a potential security risk.

In addition to that, we have a problem with themes that haven't been updated for over 2 years. Once you update them, they are set live, but don't show in any of the current trac queues (https://themes.trac.wordpress.org/report), and are not actually searchable (https://meta.trac.wordpress.org/ticket/2939), or set live (they need to be manually checked and approved probably from the admin area by reviewers with proper clearance - not 100% sure how this is done, TRT admins would know more).

A proposal is to add a new status for those themes. Maybe updated or something similar, so that the reviewers could pay more attention to these (seeing diffs).

More input from the TRT is welcomed, but we should implement this asap.

Change History (15)

This ticket was mentioned in Slack in #themereview by dingo_d. View the logs.


5 years ago

#2 @poena
5 years ago

This is how themes that have not been updated in two years are set live:

1) Author uploads a theme and receives the notification that their theme is set live automatically
2) Author realizes their update is not actually live
3) Author updates their theme, again, thinking that something went wrong.
4) Author waits 5 hours
5) Author ask in the theme review teams Slack channel why their theme is not live
6) Anyone who can set themes live in the theme directory admin area reviews the theme and sets it live.

This ticket was mentioned in Slack in #themereview by dingo_d. View the logs.


5 years ago

#4 @acosmin
5 years ago

Maybe something can also be done for themes that get suspended and can't be updated without being reinstated. The process is a real waste of time if you need to reinstate/suspend the theme multiple times if the update doesn't result in a fix.

A new status for tickets should be created, something like suspended.

#5 @kafleg
5 years ago

In addition to the poena comment, what we actually do is to check quick differences among the two version. When we have time, we give a try to check the updates, however, because of the busy schedule of all the reviewers, we generally set live from the backend.

Moreover, it is really hard to check all the updates even two years older ticket updates.

#6 in reply to: ↑ description @dd32
5 years ago

Replying to dingo_d:

Currently, theme updates aren't checked by the review team. So, technically, a user could create an ok theme, pass the review and his theme would be set live. Then they could modify the theme to include some forbidden things (obtrusive upselling, demo xml in the theme or some tracking code even), and we would be none the wiser, since updates are closed and set live automatically.

You can see the Theme updates for that lot here:
https://themes.trac.wordpress.org/query?priority=theme+update&status=closed&col=id&col=summary&col=changetime&desc=1&order=id

That process was requested by the TRT and implemented in [2686] & [2691]

In addition to that, we have a problem with themes that haven't been updated for over 2 years. Once you update them, they are set live, but don't show in any of the current trac queues (https://themes.trac.wordpress.org/report)

Can you post an example theme for that? There's no intentional "theme not updated in 2 years" checks that I'm aware of, I think it's more of a side effect of the above changes.
Kind of need to see the state of everything before the Trac ticket and/or theme are touched/fixed though, so please feel free to ping me in slack next time you find one.

This ticket was mentioned in Slack in #themereview by dd32. View the logs.


5 years ago

#8 @kevinhaig
5 years ago

TRT did decide a while back to auto live updates. So I really do not know why this needs to be changed.

For sure it is a security risk, and severe penalties such as banning/theme suspension should apply when an author circumvents requirements in this way. But we seldom hold authors accountable.

At issue is the queue would be huge if we stopped doing this.

Updates should be spot checked and if problems are found and authors are abusing the trust of the update system, then severe penalties should be administered.

#9 follow-up: @poena
5 years ago

Updates should absolutely go live automatically.

With the exception of themes that have not been updated in two years.
Those themes needs a new status so that:
1) The author knows their theme is not live yet
2) Reviewers knows they need to review them

#10 @poena
5 years ago

The trac report above shows all updates, it does not show us if the theme is really live or not.

Here is an example ticket for a theme that did not go live automatically:
https://themes.trac.wordpress.org/ticket/72182

#11 in reply to: ↑ 9 @kevinhaig
5 years ago

Yes updates over 2 years should have a separate status.

#12 @dd32
5 years ago

In 9086:

Theme Directory: When a theme update older than two years is uploaded, do not auto-approve the Trac ticket.

Previously the Theme Directory was incorrectly applying the 2 year cutoff to the query that the Trac Sync uses, seemingly by accident as as it was coded all updates should've been auto-approved.

Now theme updates older than two years will not have their Trac ticket auto-closed as live, and theme reviewers can review the open theme update tickets, mark as live on Trac, and have the Trac sync mark the version as live in WordPress.

See #4634.

This ticket was mentioned in Slack in #themereview by dingo_d. View the logs.


5 years ago

#14 @dingo_d
5 years ago

@dd32 this seems to be fixed? If so can you close this?

CC @poena @kafleg can you verify this?

#15 @dd32
5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.