Page MenuHomePhabricator

Security review for WikimediaUI Style Guide
Closed, ResolvedPublic

Description

Project Information

Description of the tool/project

Static website that holds the Design team's styleguide. The style guide provides a general design direction for different kinds of interactive projects to serve our diverse communities. Designers and developers will find design recommendations to solve general problems, so that they can focus on the specific needs of their projects.

Dependencies

The project is a static website and uses npm to manage dependencies.

The devDependencies can be found in package.json.

Has this project been reviewed before?

No.

Working test environment

You'll need npm and grunt to build the website. Apart from that its a simple static site so any HTTP server would do.

Post-deployment

Design @Volker_E @Prtksxna

Event Timeline

Prtksxna updated the task description. (Show Details)

You'll need npm and grunt to build the website. Apart from that its a simple static site so any HTTP server would do.

What's the deployment plan here? Is it expected that grunt/npm will be run on the server? (If so, that may be an issue)

What's the deployment plan here? Is it expected that grunt/npm will be run on the server? (If so, that may be an issue)

We build before commiting and check-in the built CSS files. So grunt/npm are only required for development.

The repo has everything to be directly deployed. We need to run npm install to get normalize and wikimedia-ui-base.

After seeing our build script and this pull request I am a bit confused about how we do this. Will wait for @Volker_E's response.

After further conversation with @Bawolff on IRC, current setup with built CSS files committed seems to be sufficient. Citing his reply:

As long as the server doesn't have to run npm/grunt/etc when a new version is put on the server, then its fine

We can proceed without changes from here.

"High" from our side. Needed to be able to deploy on design.wikimedia.org.

  • Any links with target="_blank" should also specify rel="noopener"

Otherwise, this is approved.

As an aside, once we move it to wmf, it would be cool if apache is configured to send an appropriate CSP header. However this is not a requirement.

CSP doesn't impact the issue with target="blank" if I'm not mistaken, but there are possible negative performance issues apart from security concerns. Filed T189823 for further discussion.

I am told on IRC that the reviews are done, but this ticket is open. Please clarify if link targets have been updated and this is resolved.