Skip to content

Commit 6ecbccd

Browse files
Merge #895
895: Enable HSTS headers on all responses r=carols10cents **Deployment Warning:** This pull request uses a configuration syntax added in nginx-1.7.5 and requires switching to a new buildpack. ~~If this pull request is merged the buildpack configuration on *staging* and *production* will need to be changed on Heroku.~~ (This is no longer a concern with the updated PR as this is specified in `.buildpacks`.) It is unclear if there are any existing mirror deployments on Heroku or otherwise depending on our `nginx.conf.erb` template. This pull request enables HSTS headers for all responses. My tests show an upgrade from an F to a D on the Mozilla Observatory. (Out of 100 points, this scores 35 instead of 15.) Please also see the discussion in #597 which is what prompted my review. This pull request also fixes the buildpack URLs defined in `app.json` so that the Heroku deploy button works. # Nginx buildpack upgrade The previous buildpack was based on nginx-1.5.7 which was released in November of 2013. Fortunately for us, it looks like travis-ci recently cloned this buildpack and upgraded it to the latest release of 1.13.3. Note that the odd minor version number represents that this (as well as our previous release) is from the development branch. The stable release of 1.12.1 may be preferable, but I expect that the travis-ci clone is our best bet for a responsive upstream at this time. # Nginx configuration change Two changes to the nginx configuration file where necessary. See http://nginx.org/en/docs/http/ngx_http_headers_module.html for more details on both of these. ## Use the always parameter The `always` parameter was introduced in nginx-1.7.5. The HSTS header is now included for all response codes. ## Duplicate header declaration in both blocks This surprised me, but the documentation states: > There could be several add_header directives. These directives are inherited from the previous level **if and only if** there are no add_header directives defined on the current level. I figured it was best to explicitly duplicate the declaration rather than rely on inheritance at all. # Fix the Deploy to Heroku button This pull request also fixes the Deploy to Heroku button in `docs/MIRROR.md` by updating the `app.json` file. ~~It appears that Heroku does not support URLs with the #SHA fragment. If we want to fix ourselves to specific commits for the buildpacks it may be possible to do so with `archive/*.tar.gz` links, but I suspect that we are currently targeting the master branches for these in production anyway.~~ The PR now sets up `app.json` to use heroku-buildpack-multi which will pull in the other buildpacks from `.buildpacks`. New mirror deployments should now match our staging and production environments.
2 parents eb40aab + c3ce2c2 commit 6ecbccd

File tree

3 files changed

+20
-29
lines changed

3 files changed

+20
-29
lines changed

Diff for: .buildpacks

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
https://github.com/rcaught/heroku-buildpack-cmake#e4e2c9e
22
https://github.com/emk/heroku-buildpack-rust#578d630
33
https://codon-buildpacks.s3.amazonaws.com/buildpacks/heroku/emberjs.tgz
4-
https://github.com/ryandotsmith/nginx-buildpack.git#af813ba
4+
https://github.com/travis-ci/nginx-buildpack.git#2fbde35
55
https://github.com/sgrif/heroku-buildpack-diesel#f605edd

Diff for: app.json

+1-13
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,7 @@
5656
],
5757
"buildpacks": [
5858
{
59-
"url": "https://github.com/rcaught/heroku-buildpack-cmake.git#e4e2c9e"
59+
"url": "https://github.com/heroku/heroku-buildpack-multi"
6060
},
61-
{
62-
"url": "https://github.com/emk/heroku-buildpack-rust.git#578d630"
63-
},
64-
{
65-
"url": "https://codon-buildpacks.s3.amazonaws.com/buildpacks/heroku/emberjs.tgz"
66-
},
67-
{
68-
"url": "https://github.com/ryandotsmith/nginx-buildpack.git#af813ba"
69-
},
70-
{
71-
"url": "https://github.com/sgrif/heroku-buildpack-diesel.git#f605edd"
72-
}
7361
]
7462
}

Diff for: config/nginx.conf.erb

+18-15
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ events {
99
}
1010

1111
http {
12-
gzip on;
13-
gzip_comp_level 2;
14-
gzip_proxied any;
15-
gzip_min_length 512;
16-
gzip_types text/plain text/css application/json application/javascript application/x-javascript text/javascript text/xml application/xml application/rss+xml application/atom+xml application/rdf+xml image/svg+xml;
12+
gzip on;
13+
gzip_comp_level 2;
14+
gzip_proxied any;
15+
gzip_min_length 512;
16+
gzip_types text/plain text/css application/json application/javascript application/x-javascript text/javascript text/xml application/xml application/rss+xml application/atom+xml application/rdf+xml image/svg+xml;
1717

1818
server_tokens off;
1919

@@ -26,7 +26,7 @@ http {
2626
sendfile on;
2727

2828
client_body_timeout 30;
29-
client_max_body_size 50m;
29+
client_max_body_size 50m;
3030

3131
upstream app_server {
3232
server localhost:8888 fail_timeout=0;
@@ -36,20 +36,23 @@ http {
3636
listen <%= ENV["PORT"] %>;
3737
server_name _;
3838
keepalive_timeout 5;
39-
add_header Strict-Transport-Security "max-age=31536000";
4039

41-
location ~ ^/assets/ {
42-
add_header Cache-Control public;
43-
root dist;
44-
expires max;
45-
}
40+
location ~ ^/assets/ {
41+
add_header Strict-Transport-Security "max-age=31536000" always;
42+
add_header X-Content-Type-Options nosniff;
43+
add_header Cache-Control public;
44+
root dist;
45+
expires max;
46+
}
47+
4648
location / {
49+
add_header Strict-Transport-Security "max-age=31536000" always;
4750
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
4851
proxy_set_header Host $http_host;
4952
proxy_redirect off;
50-
if ($http_x_forwarded_proto != 'https') {
51-
rewrite ^ https://$host$request_uri? permanent;
52-
}
53+
if ($http_x_forwarded_proto != 'https') {
54+
rewrite ^ https://$host$request_uri? permanent;
55+
}
5356
proxy_pass http://app_server;
5457
}
5558
}

0 commit comments

Comments
 (0)