Fix R01-N21: pin Twig auto-escape with regression tests

Twig `autoescape: 'html'` is the only barrier between user-supplied
strings (sprint name, task title, audit JSON, …) and stored XSS in the
views that render them — a single careless `|raw` filter or
`{% autoescape false %}` block opens the door. The fix is two tests in
`tests/Http/TwigAutoescapeTest.php` that catch both directions.

*Behaviour pin:* renders a known XSS payload through a synthetic Twig
template using the same `View` env the controllers wire up, and
asserts the output is HTML-escaped (no `<script>` survives;
`&lt;script&gt;` is present). A second case pins attribute-context
double-quote escaping (`title="…"` → `&quot;`) so a future flip from
`'html'` to anything else fails loudly.

*Static guard:* walks every `.twig` file under `views/` and fails if
any line carries `|\s*(raw|safe)` or `{%\s*autoescape`. Verified
end-to-end by temporarily appending `{{ "x"|raw }}` to `home.twig` —
the test reports the exact `path:line` of the offence with a
remediation hint. Today's scan: 0 offences.

No production code changed; the guards live entirely in the test
suite. Tests: 305 / 814 (was 302 / 791).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

SPEC.md

@@ -1475,6 +1475,27 @@ is gone — see `src/Auth/BootstrapAdmin.php`.
       Tests: 227 / 590 (was 211 / 562). Eleventh fix from
       `doc/REVIEW_01.md`.
 
+- [x] **R01-N21 — Twig auto-escape pinned by tests**
+      Twig `autoescape: 'html'` is the only barrier between user-supplied
+      strings (sprint name, task title, audit JSON, …) and stored XSS in
+      the views that render them — a single careless `|raw` filter or
+      `{% autoescape false %}` block opens the door. The fix is two
+      tests in `tests/Http/TwigAutoescapeTest.php` that catch both
+      directions:
+      *Behaviour pin:* renders a known XSS payload through a synthetic
+      Twig template using the same `View` env the controllers wire up,
+      and asserts the output is HTML-escaped (no `<script>` survives;
+      `&lt;script&gt;` is present). A second case pins
+      attribute-context double-quote escaping so a future flip from
+      `'html'` to `false` fails loudly. *Static guard:* walks every
+      `.twig` file under `views/` and fails if any line carries
+      `|\s*(raw|safe)` or `{%\s*autoescape`. Verified end-to-end by
+      temporarily appending `{{ "x"|raw }}` to `home.twig` — the test
+      reports the exact `path:line` of the offence with a remediation
+      hint. No production code changed; the guards live entirely in
+      the test suite. Tests: 305 / 814 (was 302 / 791). Eighteenth
+      fix from `doc/REVIEW_01.md`.
+
 - [x] **R01-N20 — `Response::redirect` rejects non-path locations**
       All callers in the app pass a path-only string (`/foo?error=…`,
       `/sprints/{id}`, etc.) — but `Response::redirect` accepted any

tests/Http/TwigAutoescapeTest.php

@@ -0,0 +1,144 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Http;
+
+use App\Http\View;
+use App\Tests\TestCase;
+use FilesystemIterator;
+use RecursiveDirectoryIterator;
+use RecursiveIteratorIterator;
+
+/**
+ * R01-N21: Twig auto-escape is the only barrier between user-supplied
+ * strings (sprint name, task title, audit JSON, …) and stored XSS in
+ * the views that render them. One careless `|raw` or `{% autoescape
+ * false %}` opens the door.
+ *
+ * Two complementary checks:
+ *
+ *   - **Behaviour pin**: render a known XSS payload through a synthetic
+ *     Twig template using the same `View` env the controllers use, and
+ *     assert the output is HTML-escaped. This catches a future change
+ *     to `View`'s `autoescape` config (e.g. someone flips it to `'name'`
+ *     so a template-name-derived strategy kicks in).
+ *
+ *   - **Static guard**: scan every `views` `.twig` template for the
+ *     escape-bypass forms — `|raw`, `|safe`, `{% autoescape false %}`,
+ *     and any `{% autoescape ... %}` override — and fail if any are
+ *     present. Tests like this beat code review when the codebase
+ *     grows.
+ */
+final class TwigAutoescapeTest extends TestCase
+{
+    public function testHtmlAutoescapeEscapesXssPayloadEndToEnd(): void
+    {
+        $view = new View(__DIR__ . '/../../views');
+        $twig = $view->twig();
+
+        $payload = '<script>alert(1)</script>';
+        $tpl     = $twig->createTemplate('{{ x }}');
+        $out     = $tpl->render(['x' => $payload]);
+
+        // The literal `<script>` opener must NOT survive — a real
+        // autoescape=html env will produce `&lt;script&gt;…`. We
+        // assert both the negative (no raw tag) and the positive
+        // (the entity-encoded form is present) so a flip to
+        // `autoescape: false` would fail loudly.
+        self::assertStringNotContainsString(
+            '<script>',
+            $out,
+            'autoescape=html must HTML-escape user values rendered with {{ }}',
+        );
+        self::assertStringContainsString('&lt;script&gt;', $out);
+        self::assertStringContainsString('&lt;/script&gt;', $out);
+    }
+
+    public function testAttributeContextEscapesQuoteCharacters(): void
+    {
+        // The `e('html_attr')` filter is the documented attribute-context
+        // recipe in this codebase. Auto-escape inside `"…"` attribute
+        // values defaults to HTML escape, which is enough for the
+        // double-quote → `&quot;` replacement that closes the attribute
+        // injection vector.
+        $view = new View(__DIR__ . '/../../views');
+        $twig = $view->twig();
+
+        $tpl = $twig->createTemplate('<div title="{{ x }}"></div>');
+        $out = $tpl->render(['x' => '" onmouseover="alert(1)']);
+
+        self::assertStringNotContainsString('onmouseover="alert(1)', $out);
+        self::assertStringContainsString('&quot;', $out);
+    }
+
+    /**
+     * Scan every `.twig` file under `views` for escape-bypass forms.
+     * A future template with `|raw` (or `{% autoescape false %}`)
+     * would make stored XSS one careless commit away — this test
+     * fails fast with the offending lines so the reviewer sees
+     * them in CI.
+     */
+    public function testNoRawSafeOrAutoescapeOverrideInViews(): void
+    {
+        $viewsDir = realpath(__DIR__ . '/../../views');
+        self::assertNotFalse($viewsDir, 'views/ must exist');
+
+        $iter = new RecursiveIteratorIterator(
+            new RecursiveDirectoryIterator($viewsDir, FilesystemIterator::SKIP_DOTS)
+        );
+
+        // Two independent regexes — running both on the same line keeps
+        // the failure message precise about which guard tripped.
+        $rawFilter   = '/\|\s*(raw|safe)\b/';
+        $autoescape  = '/\{\%\s*autoescape\b/';
+
+        $offences = [];
+        $scanned  = 0;
+        /** @var \SplFileInfo $file */
+        foreach ($iter as $file) {
+            if (!$file->isFile() || $file->getExtension() !== 'twig') {
+                continue;
+            }
+            $scanned++;
+            $contents = file_get_contents($file->getPathname());
+            self::assertNotFalse($contents, "could not read {$file->getPathname()}");
+
+            $lines = preg_split('/\R/', $contents) ?: [];
+            foreach ($lines as $i => $line) {
+                if (preg_match($rawFilter, $line, $m)) {
+                    $offences[] = sprintf(
+                        '%s:%d  |%s filter — bypasses HTML autoescape (R01-N21)',
+                        self::relativePath($file->getPathname(), $viewsDir),
+                        $i + 1,
+                        $m[1],
+                    );
+                }
+                if (preg_match($autoescape, $line)) {
+                    $offences[] = sprintf(
+                        '%s:%d  {%% autoescape … %%} override — bypasses HTML autoescape (R01-N21)',
+                        self::relativePath($file->getPathname(), $viewsDir),
+                        $i + 1,
+                    );
+                }
+            }
+        }
+
+        self::assertGreaterThan(0, $scanned, 'no .twig files were scanned — wrong views directory?');
+        self::assertSame(
+            [],
+            $offences,
+            "Forbidden escape-bypass usage in views:\n  - " . implode("\n  - ", $offences)
+                . "\n\nIf one of these is genuinely safe (e.g., an HTML constant the operator controls),"
+                . " add an allow-list comment alongside this test and document why the input is trusted.",
+        );
+    }
+
+    private static function relativePath(string $abs, string $base): string
+    {
+        if (str_starts_with($abs, $base . '/')) {
+            return substr($abs, strlen($base) + 1);
+        }
+        return $abs;
+    }
+}