feat(csp): retirer unsafe-inline de script-src

SEC-WEB-001 / OPS-010, ferme. C'est cette directive qui laissait s'executer
le XSS stocke de SEC-XSS-001 au lieu de le bloquer.

Les cinq derniers gabarits sont migres : match_form 13, calendar 11,
teams 11, evaluate_player 9, view_tryout 8. Total sur le chantier : 82
gestionnaires en ligne retires dans 17 gabarits. Il n'en reste aucun.

Deux motifs generiques de plus dans main.js
  data-mirror             affichage direct de la valeur d'un curseur.
                          evaluate_player repetait le meme
                          oninput="this.nextElementSibling.textContent = ..."
                          sur ses neuf curseurs de note.
  data-submit-on-change   remplace onchange="this.form.submit()"

Markup genere dans des chaines JavaScript
  match_form construisait sept gestionnaires par concatenation, en y
  injectant l'identifiant du joueur. Le markup portait deja data-player-id :
  returnToPool et assignToTeam lisent desormais leurs arguments depuis
  l'element clique. Cela supprime a la fois l'attribut en ligne et la
  concatenation qui l'alimentait. Meme motif que dans coach_availability.

Bascule
  CSP_ALLOW_INLINE_SCRIPT passe a false. script-src vaut maintenant
  'self' 'nonce-<aleatoire par requete>' https://cdn.jsdelivr.net.
  La variable d'environnement reste, comme issue de secours si un
  deploiement rencontrait un gestionnaire oublie -- mais la laisser active
  revient a renoncer a la protection.

Le cliquet devient une garde
  Le budget par gabarit est vide et les tests deviennent absolus : aucun
  gestionnaire en ligne, et tout bloc <script> inline doit porter son
  nonce. Sans nonce, un bloc n'est simplement pas execute, et rien dans les
  journaux ne le signale -- d'ou le test.

Verifications
  22 pages parcourues avec les trois roles : toutes rendent en 200, aucune
  ne contient de gestionnaire en ligne, et chaque bloc inline porte bien le
  nonce de sa propre reponse. Syntaxe JavaScript de chaque gabarit verifiee
  par node --check.

193 tests. Le dernier xfail de SEC-WEB-001 reussissait, le marqueur est
retire. Il n'en reste qu'un : SEC-AUTH-006, enumeration de comptes.

style-src conserve 'unsafe-inline' : les attributs style="" sont partout et
ne sont pas un vecteur XSS a eux seuls. Migration distincte, non prioritaire.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
GGThed
2026-08-07 21:03:31 -04:00
co-authored by Claude Opus 5
parent 09453199b8
commit fcb58e8a17
10 changed files with 209 additions and 109 deletions
+46 -46
View File
@@ -1,17 +1,15 @@
"""Content Security Policy, and the migration away from 'unsafe-inline'.
"""Content Security Policy.
SEC-WEB-001 / OPS-010. script-src still carries 'unsafe-inline', which is
why the stored XSS of SEC-XSS-001 executed instead of being blocked.
SEC-WEB-001 / OPS-010. script-src no longer carries 'unsafe-inline': the
directive that let the stored XSS of SEC-XSS-001 execute instead of being
blocked. Inline scripts are authorised by a per-request nonce, and every
inline event handler has been replaced by a data-action attribute
dispatched from main.js.
Removing it is not a one-line change. A nonce authorises `<script>`
elements; it can do nothing for `onclick="..."` attributes, and there are
dozens of those across the templates. Under CSP level 3 a browser also
ignores 'unsafe-inline' the moment a nonce appears, so the two cannot
coexist as a gradual transition — the switch is atomic.
The counts below are a ratchet: they may only go down. Migrating a
template and lowering the number is a deliberate act, recorded in the
diff. Adding a new inline handler turns the suite red.
Getting here took removing 82 handlers across 17 templates, because a nonce
authorises `<script>` elements and can do nothing for `onclick`. These
tests keep it that way: one new inline handler, and the policy silently
stops applying to that page.
"""
import os
@@ -34,13 +32,9 @@ INLINE_HANDLER = re.compile(
#: Remaining inline handlers, per template. Lower these as you migrate;
#: never raise one. Templates absent from this map must have none.
HANDLER_BUDGET = {
'pages/match_form.html': 13,
'pages/calendar.html': 11,
'pages/teams.html': 11,
'pages/evaluate_player.html': 9,
'pages/view_tryout.html': 8,
}
#: No template may carry an inline event handler. The migration is done;
#: this is now a hard rule, not a countdown.
HANDLER_BUDGET = {}
#: What the ratchet is counting down to.
TOTAL_BUDGET = sum(HANDLER_BUDGET.values())
@@ -61,10 +55,12 @@ def _count_handlers(path):
class TestPolicyHeader:
def test_the_current_policy_still_allows_inline_script(self, client):
"""Documents where we are, not where we want to be."""
def test_script_src_no_longer_allows_inline(self, client):
csp = client.get('/auth/login').headers['Content-Security-Policy']
assert "'unsafe-inline'" in csp
script_src = next(d for d in csp.split(';') if 'script-src' in d)
assert "'unsafe-inline'" not in script_src
assert "'nonce-" in script_src
def test_the_policy_pins_the_dangerous_directives(self, client):
csp = client.get('/auth/login').headers['Content-Security-Policy']
@@ -75,10 +71,13 @@ class TestPolicyHeader:
assert "form-action 'self'" in csp
assert "object-src" not in csp or "object-src 'none'" in csp
def test_no_nonce_is_emitted_while_inline_script_is_allowed(self, client):
"""Emitting both would silently drop every inline script in modern
browsers, since a nonce makes them ignore 'unsafe-inline'."""
csp = client.get('/auth/login').headers['Content-Security-Policy']
def test_the_legacy_mode_still_builds(self):
"""The escape hatch is kept for a deployment that hits an overlooked
handler. Emitting both would be pointless: a nonce makes browsers
ignore 'unsafe-inline' entirely."""
csp = build_csp(allow_inline_script=True)
assert "'unsafe-inline'" in csp
assert 'nonce-' not in csp
def test_the_hardened_policy_carries_a_nonce_and_no_unsafe_inline(self):
@@ -113,34 +112,35 @@ class TestInlineHandlerRatchet:
f'data-action="..." and the delegated listener in main.js.'
)
def test_the_budget_map_has_no_stale_entries(self):
"""Lower an entry to zero and it must be deleted, so the map keeps
reflecting the real remaining work."""
actual = {rel: _count_handlers(full) for rel, full in _templates()}
def test_every_inline_script_block_carries_a_nonce(self):
"""Without a nonce a block is simply not executed now, and nothing
in the server logs says so."""
import re as _re
stale = [rel for rel, allowed in HANDLER_BUDGET.items()
if actual.get(rel, 0) < allowed]
offenders = []
for relative, full in _templates():
with open(full, encoding='utf-8') as handle:
content = handle.read()
for tag in _re.findall(r'<script[^>]*>', content):
if 'src=' in tag or 'nonce=' in tag:
continue
offenders.append(f'{relative}: {tag}')
assert not stale, (
f'Budget is now higher than reality for {stale}. Lower or remove '
f'these entries so the count keeps meaning something.'
assert not offenders, (
'inline <script> without nonce="{{ csp_nonce }}": ' + str(offenders)
)
def test_the_shared_layout_is_already_free_of_them(self):
"""base.html and macros.html render on every single page, so they
were migrated first."""
def test_the_shared_layout_is_free_of_them(self):
"""base.html and macros.html render on every single page."""
for relative in ('layouts/base.html', 'layouts/macros.html'):
full = os.path.join(TEMPLATE_ROOT, *relative.split('/'))
assert _count_handlers(full) == 0, f'{relative} regressed'
def test_progress_is_recorded(self):
"""Fails when the total drops, as a reminder to update the budget
and, once it reaches zero, to flip CSP_ALLOW_INLINE_SCRIPT."""
def test_no_template_carries_an_inline_handler(self):
total = sum(_count_handlers(full) for _rel, full in _templates())
assert total <= TOTAL_BUDGET
assert total == TOTAL_BUDGET, (
f'{TOTAL_BUDGET - total} handler(s) removed since the budget was '
f'last updated — lower HANDLER_BUDGET to {total}. At zero, set '
f'CSP_ALLOW_INLINE_SCRIPT to false and delete this ratchet.'
assert total == 0, (
f'{total} inline event handler(s) reintroduced. They are not '
f'covered by the nonce, so they will not run — use '
f'data-action="..." and registerActions() instead.'
)