From 576a0c51cdafba4dbd8f7d0b140c7a21437e7639 Mon Sep 17 00:00:00 2001 From: fs Date: Mon, 6 Apr 2026 12:08:25 +0200 Subject: [PATCH] =?UTF-8?q?feat(guards):=20add=20GR-SEC-010=20=E2=80=94=20?= =?UTF-8?q?output=20escaping=20enforcement=20in=20views?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add architecture test ViewLayerOutputEscapingContractTest that flags raw --- .agents/checks/guard-catalog.json | 7 + .agents/checks/guard-enforcement-map.json | 7 + CLAUDE.md | 2 + .../templates/aside-bookmarks-panel.phtml | 4 +- .../templates/aside-help-panel.phtml | 2 +- .../templates/aside-helpdesk-panel.phtml | 2 +- templates/partials/app-flash.phtml | 2 +- templates/partials/app-list-titlebar.phtml | 2 +- templates/partials/app-main-aside.phtml | 6 +- templates/partials/app-tile.phtml | 2 +- templates/partials/app-topbar.phtml | 2 +- .../ViewLayerOutputEscapingContractTest.php | 123 ++++++++++++++++++ 12 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 tests/Architecture/ViewLayerOutputEscapingContractTest.php diff --git a/.agents/checks/guard-catalog.json b/.agents/checks/guard-catalog.json index c6e19a0..0bf305e 100644 --- a/.agents/checks/guard-catalog.json +++ b/.agents/checks/guard-catalog.json @@ -154,6 +154,13 @@ "reviewer": "security", "title": "Server-side tenant scope", "requirement": "MUST enforce tenant scope server-side. Every query touching tenant-scoped data MUST filter by tenant_id. No cross-tenant data leakage." + }, + { + "id": "GR-SEC-010", + "area": "security", + "reviewer": "security", + "title": "Output escaping in views", + "requirement": "Views (.phtml) MUST use e() for all HTML output. Raw echo is only permitted for pre-built HTML fragments from framework helpers (e.g. ARIA attributes from navActive()). Each raw echo MUST carry an inline '// raw-html-ok: reason' comment." } ] } diff --git a/.agents/checks/guard-enforcement-map.json b/.agents/checks/guard-enforcement-map.json index ef079a9..d4353ba 100644 --- a/.agents/checks/guard-enforcement-map.json +++ b/.agents/checks/guard-enforcement-map.json @@ -168,6 +168,13 @@ "tests/Architecture/ApiResourceContractTest.php", ".agents/runs//review-security.json" ] + }, + { + "guard_id": "GR-SEC-010", + "enforcement_mode": "automated", + "evidence_source": [ + "tests/Architecture/ViewLayerOutputEscapingContractTest.php" + ] } ] } diff --git a/CLAUDE.md b/CLAUDE.md index 34fdf1c..6e014ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,6 +180,7 @@ docker/ # Dockerfiles, Nginx configs, PHP configs - Loading assets from external CDNs (GR-SEC-004) - Undeclared cross-module imports (must use `requires` in manifest) - Inline SQL or string-concatenated queries (GR-SEC-003) +- Raw `echo` in views without `// raw-html-ok` marker — use `e()` (GR-SEC-010) - OpenAPI changes without updating the spec (GR-CORE-007) ## Database @@ -202,6 +203,7 @@ These rules are enforced by guards GR-SEC-001 through GR-SEC-009 in `.agents/che - API requests must not start sessions (GR-SEC-007) - Authorization enforced server-side, not just in UI (GR-SEC-008) - Tenant scope (`tenant_id`) enforced on all queries — no unscoped data access (GR-SEC-009) +- Views use `e()` for all output — raw `echo` only with `// raw-html-ok: reason` marker (GR-SEC-010) ## Quality Gates diff --git a/modules/bookmarks/templates/aside-bookmarks-panel.phtml b/modules/bookmarks/templates/aside-bookmarks-panel.phtml index f1da638..221ce51 100644 --- a/modules/bookmarks/templates/aside-bookmarks-panel.phtml +++ b/modules/bookmarks/templates/aside-bookmarks-panel.phtml @@ -101,7 +101,7 @@ $currentPath = trim((string) ($bookmarkNav['current_path'] ?? '')); data-bookmark-group-id=""> > + >
@@ -199,7 +199,7 @@ $currentPath = trim((string) ($bookmarkNav['current_path'] ?? '')); data-bookmark-group-id=""> > + >
diff --git a/modules/help-center/templates/aside-help-panel.phtml b/modules/help-center/templates/aside-help-panel.phtml index 0de435f..7e82a12 100644 --- a/modules/help-center/templates/aside-help-panel.phtml +++ b/modules/help-center/templates/aside-help-panel.phtml @@ -67,7 +67,7 @@ $helpNavGroups = [ $active = $item['active'] ?? ['class' => '', 'aria' => '']; ?>
  • - > + >
    • diff --git a/modules/helpdesk/templates/aside-helpdesk-panel.phtml b/modules/helpdesk/templates/aside-helpdesk-panel.phtml index f5257dc..7dc78a2 100644 --- a/modules/helpdesk/templates/aside-helpdesk-panel.phtml +++ b/modules/helpdesk/templates/aside-helpdesk-panel.phtml @@ -64,7 +64,7 @@ foreach ($visibleItems as $item) { $active = $item['active'] ?? ['class' => '', 'aria' => '']; ?>
  • - > + >
    • diff --git a/templates/partials/app-flash.phtml b/templates/partials/app-flash.phtml index e14d042..16c1e68 100644 --- a/templates/partials/app-flash.phtml +++ b/templates/partials/app-flash.phtml @@ -23,7 +23,7 @@ $timeouts = [ -
    +
    diff --git a/templates/partials/app-list-titlebar.phtml b/templates/partials/app-list-titlebar.phtml index 0b72e75..1c3c690 100644 --- a/templates/partials/app-list-titlebar.phtml +++ b/templates/partials/app-list-titlebar.phtml @@ -12,7 +12,7 @@ if (!in_array($listTitleTag, $allowedTitleTags, true)) { <>>
    - +
    diff --git a/templates/partials/app-main-aside.phtml b/templates/partials/app-main-aside.phtml index 7ba69b7..5d7ceaa 100644 --- a/templates/partials/app-main-aside.phtml +++ b/templates/partials/app-main-aside.phtml @@ -255,7 +255,7 @@ $renderAdminNavGroup = static function (array $group, string $tenantQueryParam): $active = $item['active'] ?? ['class' => '', 'aria' => '']; ?>
  • - > + >
    • @@ -306,7 +306,7 @@ $modulePanelSlots = is_array($moduleSlots['aside.tab_panel'] ?? null) ? $moduleS
    • - > + >
      • @@ -325,7 +325,7 @@ $modulePanelSlots = is_array($moduleSlots['aside.tab_panel'] ?? null) ? $moduleS ?>
    • > + >
      • diff --git a/templates/partials/app-tile.phtml b/templates/partials/app-tile.phtml index 541353b..6978c31 100644 --- a/templates/partials/app-tile.phtml +++ b/templates/partials/app-tile.phtml @@ -33,7 +33,7 @@ $style = $styleParts ? implode(';', $styleParts) . ';' : ''; ?> style="" data-tooltip="" data-tooltip-pos="" diff --git a/templates/partials/app-topbar.phtml b/templates/partials/app-topbar.phtml index 141054a..b5b9e63 100644 --- a/templates/partials/app-topbar.phtml +++ b/templates/partials/app-topbar.phtml @@ -135,7 +135,7 @@ $moduleTopbarSlots = is_array($moduleSlots['topbar.right_item'] ?? null) ? $modu data-theme-option data-theme-value="" class="" - > + > diff --git a/tests/Architecture/ViewLayerOutputEscapingContractTest.php b/tests/Architecture/ViewLayerOutputEscapingContractTest.php new file mode 100644 index 0000000..d557341 --- /dev/null +++ b/tests/Architecture/ViewLayerOutputEscapingContractTest.php @@ -0,0 +1,123 @@ + // raw-html-ok: reason + * + * Typical justified uses: pre-escaped ARIA attributes from navActive(), hardcoded + * HTML-safe ternaries (e.g. 'aria-current="page"'), or pre-built HTML fragments + * from shared helpers. Each raw echo MUST carry the "raw-html-ok" marker. + */ +class ViewLayerOutputEscapingContractTest extends TestCase +{ + use ProjectFileAssertionSupport; + + /** + * Regex matching scanForRawEcho('pages'); + self::assertSame( + [], + $violations, + "Raw scanForRawEcho('templates'); + self::assertSame( + [], + $violations, + "Raw projectRootPath(); + $modulesDir = $root . '/modules'; + if (!is_dir($modulesDir)) { + self::markTestSkipped('modules/ directory missing.'); + } + + $allViolations = []; + foreach (scandir($modulesDir) ?: [] as $entry) { + if ($entry === '.' || $entry === '..' || !is_dir($modulesDir . '/' . $entry)) { + continue; + } + foreach (['pages', 'templates'] as $subDir) { + $rel = 'modules/' . $entry . '/' . $subDir; + if (is_dir($root . '/' . $rel)) { + $allViolations = array_merge($allViolations, $this->scanForRawEcho($rel)); + } + } + } + + sort($allViolations); + self::assertSame( + [], + $allViolations, + "Raw + */ + private function scanForRawEcho(string $relativeDirectory): array + { + $root = $this->projectRootPath(); + $basePath = $root . '/' . $relativeDirectory; + $this->assertDirectoryExists($basePath, 'Directory not found: ' . $relativeDirectory); + + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($basePath), + ); + + $violations = []; + + /** @var \SplFileInfo $file */ + foreach ($iterator as $file) { + if (!$file->isFile() || $file->getExtension() !== 'phtml') { + continue; + } + + $content = file_get_contents($file->getPathname()); + $this->assertNotFalse($content, 'Could not read file: ' . $file->getPathname()); + + $lines = explode("\n", $content); + foreach ($lines as $lineNumber => $line) { + if (!preg_match(self::ECHO_PATTERN, $line)) { + continue; + } + + if (str_contains($line, self::EXCEPTION_MARKER)) { + continue; + } + + $relativePath = str_replace($root . '/', '', $file->getPathname()); + $violations[] = sprintf('%s:%d: %s', $relativePath, $lineNumber + 1, trim($line)); + } + } + + sort($violations); + return $violations; + } +}