Page MenuHomePhabricator

Security Review of Popups extension library
Closed, ResolvedPublic



The Reading Web team took on refactoring a lot of the Page-Previews extension so that it was more maintainable moving forward. A broad plan for the refactor is captured in T149801. We decided to use the third-party Redux library for state management, rather than writing a system that would end up being awfully close to it functionally. Since this library is a new addition, it'll need to be reviewed by a member of the security team.

This review blocks merging the feature branch for the refactor.

What needs reviewing?

We've used Redux v3.6.0. The source for the library is available here. Note well that we're using the "development" (non-minified/mangled) UMD build, available here or from npm, and relying on Resource Loader for minification.

What uses it?

The Popups extension (AKA Hovercards, AKA Page Previews) will use it.

Event Timeline

phuedx created this task.Nov 29 2016, 5:41 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 29 2016, 5:41 PM

@dpatrick: @dr0ptp4kt said that you were the person to get in touch with for this kind of work. How should I go about getting this scheduled?

@dpatrick: @dr0ptp4kt said that you were the person to get in touch with for this kind of work. How should I go about getting this scheduled?

You should fill out the template on first.

To clarify, this ticket is requesting review of Redux not Hovercards, correct?

phuedx added a comment.EditedDec 21 2016, 9:17 AM

@dpatrick: To confirm, this task is requesting the security review of the UMD build of Redux v3.6.0.

@Legoktm: Thanks for the pointer.

The project is in the process of being promoted from a beta feature. It's slowly being rewritten, using the Redux library for state management/as a tool to help improve quality. Hence the request to review the library. Note well that the rewrite is taking place in a feature branch that we intend to merge to master once the library has been security reviewed/we're confident in the quality of the rewrite.

Prior to the rewrite, the project did go through a security review before being deployed as a beta feature. I guess it'd be prudent to schedule a security review of the whole codebase before we promote it from a beta feature – @ovasileva can speak to the timeline for those Wikipedias who have okayed the promotion.

@dpatrick - any updates on this?

The schedule for release can be found at the hovercards page. All stage 0 wikipedias are ready for the feature.

Bawolff closed this task as Resolved.Jan 21 2017, 9:04 AM
Bawolff claimed this task.
Bawolff moved this task from Scheduled to Waiting on the deprecated-security-team-reviews board.

I reviewed redux and redux-thunk. Everything looks good - its a tiny library that only manages state, so there's basically no attack surface.

@Bawolff: Awesome! Thanks for your review.