Skip to content

fix(security): remove is_safe html from Twig serial helpers #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
HecFranco merged 2 commits into from
Aug 5, 2026
Merged

fix(security): remove is_safe html from Twig serial helpers #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5db858a
fix(security): stop marking Twig serial helpers as HTML-safe
HecFranco Aug 5, 2026
1f346c8
test(security): assert Twig getSafe returns empty array without is_safe
HecFranco Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions demo/symfony8/config/reference.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

// This file is auto-generated and is for apps only. Bundles SHOULD NOT rely on its content.

namespace Symfony\Component\DependencyInjection\Loader\Configurator;
Expand Down
11 changes: 4 additions & 7 deletions docs/SECURITY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,11 @@ Mitigations address **resource exhaustion (DoS)** and document **XSS** considera

## 2. XSS and user-controlled data

Twig functions/filters use **`is_safe => ['html']`**, so Twig does not escape the output. That is appropriate when values are **application-controlled** (e.g. system-generated invoice numbers).
Twig function `serial_number` and filter `serial_number_mask` do **not** mark output as HTML-safe (`is_safe` is not set). Twig **auto-escapes** their results in HTML templates by default.

- **Recommendation:** Do not pass unsanitized user input (forms, query strings, etc.) directly into `serial_number()` or `serial_number_mask()` without validating/escaping the result for HTML.
- If serials or context values may contain user content:
- Escape in the template (e.g. `{{ serial|serial_number_mask(4)|e }}` when you need escaping for that value), or
- Ensure values are sanitized before they reach the bundle.

The bundle does not HTML-escape; the application must use serials safely in HTML context.
- Prefer application-controlled values (e.g. system-generated invoice numbers).
- If serials or context values may contain user content, keep auto-escaping enabled; do not pipe through `|raw` unless the value is already sanitized for HTML.
- Outside HTML contexts (e.g. attributes, JS), escape appropriately for that context.
Comment on lines +50 to +54

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

test -f docs/CHANGELOG.md
test -f docs/UPGRADING.md

rg -n -i 'unreleased|serial_number|serial_number_mask|html-safe|auto.?escap' \
  docs/CHANGELOG.md docs/UPGRADING.md

Repository: nowo-tech/SerialNumberBundle

Length of output: 1067

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== CHANGELOG around Unreleased/security-related entries =="
sed -n '1,120p' docs/CHANGELOG.md

echo
echo "== CHANGELOG serial_number mentions around end =="
sed -n '180,215p' docs/CHANGELOG.md

echo
echo "== UPGRADING Twig section =="
sed -n '80,115p' docs/UPGRADING.md

echo
echo "== SECURITY section context =="
sed -n '35,65p' docs/SECURITY.md

Repository: nowo-tech/SerialNumberBundle

Length of output: 11206

Add the Unreleased changelog entry for Twig XSS guidance.

docs/SECURITY.md documents visible XSS guidance for serial_number / serial_number_mask, but docs/CHANGELOG.md has no Unreleased entry covering this security behavior. Add it under docs/CHANGELOG.md section 2.6 (or the appropriate Unreleased Security entry) so this user-visible mitigation is recorded. No UPGRADING.md change is needed unless integrators must change code/config.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/SECURITY.md` around lines 50 - 54, Add an Unreleased security changelog
note in docs/CHANGELOG.md for the Twig serial_number and serial_number_mask XSS
guidance, using the existing 2.6/Unreleased Security section or the nearest
equivalent entry. Keep the note focused on the visible behavior that these Twig
outputs are not HTML-safe and remain auto-escaped by default, and do not add an
UPGRADING.md change unless the changelog entry implies an integration action.

Source: Path instructions


---

Expand Down
2 changes: 0 additions & 2 deletions src/Twig/SerialNumberTwigExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public function getFunctions(): array
new TwigFunction(
'serial_number',
$this->generateSerialNumber(...),
['is_safe' => ['html']],
),
];
}
Expand All @@ -62,7 +61,6 @@ public function getFilters(): array
new TwigFilter(
'serial_number_mask',
$this->maskSerialNumber(...),
['is_safe' => ['html']],
),
];
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Unit/Twig/SerialNumberTwigExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Nowo\SerialNumberBundle\Service\SerialNumberGenerator;
use Nowo\SerialNumberBundle\Twig\SerialNumberTwigExtension;
use PHPUnit\Framework\TestCase;
use Twig\Node\Node;

use function strlen;

Expand Down Expand Up @@ -72,13 +73,15 @@ public function testGetFunctionsReturnsSerialNumber(): void
$functions = $this->extension->getFunctions();
self::assertCount(1, $functions);
self::assertSame('serial_number', $functions[0]->getName());
self::assertSame([], $functions[0]->getSafe(new Node()));
}

public function testGetFiltersReturnsSerialNumberMask(): void
{
$filters = $this->extension->getFilters();
self::assertCount(1, $filters);
self::assertSame('serial_number_mask', $filters[0]->getName());
self::assertSame([], $filters[0]->getSafe(new Node()));
}

public function testMaskSerialNumberNegativeVisibleLastTreatedAsZero(): void
Expand Down
Loading