tbh/breadcrumb-the-shire
1
0
Fork 0
forked from fa/breadcrumb-the-shire
Code Pull Requests Activity

feat(guards): add GR-SEC-010 — output escaping enforcement in views

Browse Source 
Add architecture test ViewLayerOutputEscapingContractTest that flags
raw <?php echo in .phtml files. Legitimate uses (pre-built ARIA attrs
from navActive(), hardcoded role values, pre-rendered HTML fragments)
must carry an inline // raw-html-ok: reason marker.

Annotated all 11 existing raw echo sites with justification markers.
Added guard to catalog, enforcement map (automated), CLAUDE.md security
section, and never-do-this list.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Felix Schneider
2026-04-06 12:08:25 +02:00
parent 9a4d59eb4c
commit 576a0c51cd
12 changed files with 150 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.agents/checks/guard-catalog.json
View File

@@ -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."
}
]
}

7
.agents/checks/guard-enforcement-map.json
View File

@@ -168,6 +168,13 @@
"tests/Architecture/ApiResourceContractTest.php",
".agents/runs/<TASK>/review-security.json"
]
},
{
"guard_id": "GR-SEC-010",
"enforcement_mode": "automated",
"evidence_source": [
"tests/Architecture/ViewLayerOutputEscapingContractTest.php"
]
}
]
}

2
CLAUDE.md
View File

@@ -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

4
modules/bookmarks/templates/aside-bookmarks-panel.phtml
View File

@@ -101,7 +101,7 @@ $currentPath = trim((string) ($bookmarkNav['current_path'] ?? ''));
data-bookmark-group-id="">
<a href="<?php e(lurl($bookmarkUrl)); ?>"
class="<?php e($bookmarkActive ? 'active' : ''); ?>"
<?php echo $bookmarkActive ? 'aria-current="page"' : ''; ?>>
<?php echo $bookmarkActive ? 'aria-current="page"' : ''; // raw-html-ok: hardcoded ARIA attribute ?>>
<?php e($bookmarkName); ?>
</a>
<details class="app-sidebar-bookmark-action-menu" data-bookmark-action-menu>
@@ -199,7 +199,7 @@ $currentPath = trim((string) ($bookmarkNav['current_path'] ?? ''));
data-bookmark-group-id="<?php e($groupId); ?>">
<a href="<?php e(lurl($bookmarkUrl)); ?>"
class="<?php e($bookmarkActive ? 'active' : ''); ?>"
<?php echo $bookmarkActive ? 'aria-current="page"' : ''; ?>>
<?php echo $bookmarkActive ? 'aria-current="page"' : ''; // raw-html-ok: hardcoded ARIA attribute ?>>
<?php e($bookmarkName); ?>
</a>
<details class="app-sidebar-bookmark-action-menu" data-bookmark-action-menu>

2
modules/help-center/templates/aside-help-panel.phtml
View File

@@ -67,7 +67,7 @@ $helpNavGroups = [
$active = $item['active'] ?? ['class' => '', 'aria' => ''];
?>
<li>
<a href="<?php e(lurl($item['path'])); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; ?>>
<a href="<?php e(lurl($item['path'])); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; // raw-html-ok: pre-built ARIA attribute from navActive() ?>>
<?php e($item['label']); ?>
</a>
</li>

2
modules/helpdesk/templates/aside-helpdesk-panel.phtml
View File

@@ -64,7 +64,7 @@ foreach ($visibleItems as $item) {
$active = $item['active'] ?? ['class' => '', 'aria' => ''];
?>
<li>
<a href="<?php e(lurl($item['path'])); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; ?>>
<a href="<?php e(lurl($item['path'])); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; // raw-html-ok: pre-built ARIA attribute from navActive() ?>>
<?php e($item['label']); ?>
</a>
</li>

2
templates/partials/app-flash.phtml
View File

@@ -23,7 +23,7 @@ $timeouts = [
<?php $type = $message['type'] ?? 'info'; ?>
<?php $timeout = $timeouts[$type] ?? 5000; ?>
<?php $isAssertive = $type === 'error' || $type === 'warning'; ?>
<div class="notice" data-variant="<?php e($type); ?>" data-flash-timeout="<?php e($timeout); ?>" role="<?php echo $isAssertive ? 'alert' : 'status'; ?>">
<div class="notice" data-variant="<?php e($type); ?>" data-flash-timeout="<?php e($timeout); ?>" role="<?php echo $isAssertive ? 'alert' : 'status'; // raw-html-ok: hardcoded ARIA role values ?>">
<span><?php e(t($message['message'] ?? '')); ?></span>
<form method="post" action="flash/dismiss/<?php e($message['id'] ?? ''); ?>">
<input type="hidden" name="return" value="<?php e($returnTarget); ?>">

2
templates/partials/app-list-titlebar.phtml
View File

@@ -12,7 +12,7 @@ if (!in_array($listTitleTag, $allowedTitleTags, true)) {
<<?php e($listTitleTag); ?>><?php e($listTitle); ?></<?php e($listTitleTag); ?>>
<?php if (trim($listTitleActionsHtml) !== ''): ?>
<div class="app-list-titlebar-actions">
<?php echo $listTitleActionsHtml; ?>
<?php echo $listTitleActionsHtml; // raw-html-ok: pre-built HTML from titlebar action renderer ?>
</div>
<?php endif; ?>
</div>

6
templates/partials/app-main-aside.phtml
View File

@@ -255,7 +255,7 @@ $renderAdminNavGroup = static function (array $group, string $tenantQueryParam):
$active = $item['active'] ?? ['class' => '', 'aria' => ''];
?>
<li>
<a href="<?php e($href); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; ?>>
<a href="<?php e($href); ?>" class="<?php e($active['class'] ?? ''); ?>" <?php echo $active['aria'] ?? ''; // raw-html-ok: pre-built ARIA attribute from navActive() ?>>
<?php e($item['label'] ?? ''); ?>
</a>
</li>
@@ -306,7 +306,7 @@ $modulePanelSlots = is_array($moduleSlots['aside.tab_panel'] ?? null) ? $moduleS
<ul>
<li>
<?php $home = navActive(['', 'admin'], false); ?>
<a href="<?php e(lurl('')); ?>" class="<?php e($home['class']); ?>" <?php echo $home['aria']; ?>>
<a href="<?php e(lurl('')); ?>" class="<?php e($home['class']); ?>" <?php echo $home['aria']; // raw-html-ok: pre-built ARIA attribute from navActive() ?>>
<?php e(t('Home')); ?>
</a>
</li>
@@ -325,7 +325,7 @@ $modulePanelSlots = is_array($moduleSlots['aside.tab_panel'] ?? null) ? $moduleS
?>
<li>
<a href="<?php e($explorerHref); ?>" class="<?php e($explorerActive['class'] ?? ''); ?>"
<?php echo $explorerActive['aria'] ?? ''; ?>>
<?php echo $explorerActive['aria'] ?? ''; // raw-html-ok: pre-built ARIA attribute from navActive() ?>>
<?php e(t($explorerLabel)); ?>
</a>
</li>

2
templates/partials/app-tile.phtml
View File

@@ -33,7 +33,7 @@ $style = $styleParts ? implode(';', $styleParts) . ';' : '';
?>
<a
class="app-tile<?php echo $class ? ' ' . $class : ''; ?>"
class="app-tile<?php echo $class ? ' ' . $class : ''; // raw-html-ok: internal CSS class from tile config ?>"
href="<?php e($href); ?>"
<?php if ($style) : ?>style="<?php e($style); ?>"<?php endif; ?>
<?php if ($tooltip !== '') : ?>data-tooltip="<?php e($tooltip); ?>" data-tooltip-pos="<?php e($tooltipPos); ?>"<?php endif; ?>

2
templates/partials/app-topbar.phtml
View File

@@ -135,7 +135,7 @@ $moduleTopbarSlots = is_array($moduleSlots['topbar.right_item'] ?? null) ? $modu
data-theme-option
data-theme-value="<?php e($key); ?>"
class="<?php e($isActiveTheme ? 'active' : ''); ?>"
<?php echo $isActiveTheme ? 'aria-current="true"' : ''; ?>>
<?php echo $isActiveTheme ? 'aria-current="true"' : ''; // raw-html-ok: hardcoded ARIA attribute ?>>
<?php e(t($label)); ?>
</a>
</li>

123
tests/Architecture/ViewLayerOutputEscapingContractTest.php Normal file
View File

@@ -0,0 +1,123 @@
<?php
namespace MintyPHP\Tests\Architecture;
use PHPUnit\Framework\TestCase;
/**
* Views (.phtml) MUST use e() for HTML output — raw echo is forbidden (GR-SEC-010).
*
* Allowed exceptions (must be marked with an inline comment):
* <?php echo $var; ?> // 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 <?php echo (with optional whitespace variations).
* Does NOT match <?php e( which is the escaped helper.
*/
private const ECHO_PATTERN = '/<\?php\s+echo\b/';
/** Inline marker that excuses a raw echo on the same line. */
private const EXCEPTION_MARKER = 'raw-html-ok';
public function testCorePagesViewsUseEscapedOutput(): void
{
$violations = $this->scanForRawEcho('pages');
self::assertSame(
[],
$violations,
"Raw <?php echo found in core pages views without '" . self::EXCEPTION_MARKER . "' marker:\n" . implode("\n", $violations),
);
}
public function testCoreTemplatesUseEscapedOutput(): void
{
$violations = $this->scanForRawEcho('templates');
self::assertSame(
[],
$violations,
"Raw <?php echo found in core templates without '" . self::EXCEPTION_MARKER . "' marker:\n" . implode("\n", $violations),
);
}
public function testModuleViewsUseEscapedOutput(): void
{
$root = $this->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 <?php echo found in module views without '" . self::EXCEPTION_MARKER . "' marker:\n" . implode("\n", $allViolations),
);
}
/**
* Scan .phtml files for raw echo statements, returning violations as "file:line: content".
*
* @return list<string>
*/
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;
}
}