Skip to content

Commit f8aa6ad

Browse files
authored
Only send the document security headers on documents (#1839)
content_types defaulted to an empty list for both the CSP and clickjacking listeners, which means "every content type", so every JSON API response carried the full ~600 byte Content-Security-Policy and an X-Frame-Options header. The security advisory and download tracking endpoints alone account for around 4.5M responses per APM period. Neither header does anything for a non-document response: CSP governs document loading and X-Frame-Options governs framing, and a JSON body is neither. Restricting both listeners to text/html is route-drift-proof, unlike enumerating API path prefixes.
1 parent 8c2dee7 commit f8aa6ad

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

config/packages/nelmio_security.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ nelmio_security:
22
clickjacking:
33
paths:
44
'^/.*': DENY
5+
# Only documents can be framed, and the JSON APIs serve millions of responses per day
6+
content_types: ['text/html']
57
forced_ssl:
68
enabled: '%force_ssl%'
79
hosts: '%forced_ssl_hosts%'
@@ -10,7 +12,8 @@ nelmio_security:
1012
enabled: true
1113
report_logger_service: logger
1214
hosts: []
13-
content_types: []
15+
# CSP only governs document loading, so sending it on API responses is pure overhead
16+
content_types: ['text/html']
1417
enforce:
1518
browser_adaptive:
1619
enabled: false
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php declare(strict_types=1);
2+
3+
/*
4+
* This file is part of Packagist.
5+
*
6+
* (c) Jordi Boggiano <j.boggiano@seld.be>
7+
* Nils Adermann <naderman@naderman.de>
8+
*
9+
* For the full copyright and license information, please view the LICENSE
10+
* file that was distributed with this source code.
11+
*/
12+
13+
namespace App\Tests\Controller;
14+
15+
use App\Tests\IntegrationTestCase;
16+
17+
class SecurityHeadersTest extends IntegrationTestCase
18+
{
19+
public function testHtmlPagesCarryTheDocumentSecurityHeaders(): void
20+
{
21+
$package = self::createPackage('test/pkg', 'https://example.com/test/pkg');
22+
$this->store($package);
23+
24+
$this->client->request('GET', '/packages/test/pkg');
25+
26+
self::assertResponseIsSuccessful();
27+
self::assertResponseHeaderSame('X-Frame-Options', 'DENY');
28+
self::assertResponseHasHeader('Content-Security-Policy');
29+
}
30+
31+
public function testJsonApiResponsesSkipTheDocumentSecurityHeaders(): void
32+
{
33+
// Neither header does anything for a non-document response, and these endpoints carry
34+
// millions of responses per day, so they must not pay for ~600 bytes of CSP each.
35+
$this->client->request('GET', '/packages/list.json');
36+
37+
self::assertResponseIsSuccessful();
38+
self::assertResponseNotHasHeader('X-Frame-Options');
39+
self::assertResponseNotHasHeader('Content-Security-Policy');
40+
}
41+
}

0 commit comments

Comments
 (0)