From fd881ff296b1dc27cece6a37ba05899ac9f8a54a Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 14 Sep 2026 16:30:45 -0600 Subject: [PATCH 1/2] Apply the CssImportFilter import validator to every import form setImportValidator() was only consulted on the local-file branch. Targets carrying a scheme, and protocol-relative targets, were dispatched to the remote loader before the validator ran, so a caller that had confined imports to a set of roots still had those forms resolved unchecked. The validator now runs once, before the branch dispatch, and receives the import source in whichever form it takes: a filesystem path for local imports, or the URL for scheme-bearing and protocol-relative ones. Callers that set no validator are unaffected. --- src/Assetic/Filter/CssImportFilter.php | 32 ++++++++++------ .../Test/Filter/CssImportFilterTest.php | 37 +++++++++++++++++++ .../fixtures/cssimport/schemeimport.css | 4 ++ 3 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 tests/Assetic/Test/Filter/fixtures/cssimport/schemeimport.css diff --git a/src/Assetic/Filter/CssImportFilter.php b/src/Assetic/Filter/CssImportFilter.php index 333b68d..5e3ea3f 100644 --- a/src/Assetic/Filter/CssImportFilter.php +++ b/src/Assetic/Filter/CssImportFilter.php @@ -36,16 +36,19 @@ public function __construct(?FilterInterface $importFilter = null) } /** - * Set an optional validator that authorises each local file import before it is - * inlined. The validator receives the resolved import path (assembled from the - * asset's source root and the `@import` URL) and must return true to allow the - * import or false to skip it, leaving the raw `@import` statement untouched. + * Set an optional validator that authorises each import before it is inlined. + * The validator receives the import source — a filesystem path assembled from + * the asset's source root and the `@import` URL, or the URL itself when the + * `@import` carries a scheme or is protocol-relative — and must return true to + * allow the import or false to skip it, leaving the raw `@import` statement + * untouched. * * This is an opt-in confinement hook for consumers that inline imports from - * potentially untrusted stylesheets: without it, `@import` targets are resolved - * relative to the source with `..` traversal allowed, which can disclose any - * readable `.css` file on the server. Defaults to null (no restriction) so - * existing behaviour is unchanged for callers that do not set it. + * stylesheets they do not control. It applies to every import form the filter + * handles: local paths resolved relative to the source, and scheme-bearing or + * protocol-relative targets, which are loaded through the same `file_get_contents()` + * path. Defaults to null (no restriction) so existing behaviour is unchanged for + * callers that do not set it. */ public function setImportValidator(?callable $importValidator): self { @@ -172,15 +175,20 @@ public function filterLoad(AssetInterface $asset) } $importSource = $importRoot . '/' . $importPath; + + // Authorise every import form before dispatching. Scheme-bearing and + // protocol-relative targets are resolved through the same + // `file_get_contents()` path as local ones, so the validator is applied + // here rather than in the local-file branch alone. + if (null !== $importValidator && !$importValidator($importSource)) { + return $matches[0]; + } + if (false !== strpos($importSource ?: '', '://') || 0 === strpos($importSource ?: '', '//')) { $import = new HttpAsset($importSource, array($importFilter), true); } elseif ('css' != pathinfo($importPath ?: '', PATHINFO_EXTENSION) || !file_exists($importSource)) { // ignore non-css and non-existant imports return $matches[0]; - } elseif (null !== $importValidator && !$importValidator($importSource)) { - // ignore imports the caller-supplied validator rejects (e.g. a path - // that escapes the allowed roots via `..` traversal) - return $matches[0]; } else { $import = new FileAsset($importSource, array($importFilter), $importRoot, $importPath); } diff --git a/tests/Assetic/Test/Filter/CssImportFilterTest.php b/tests/Assetic/Test/Filter/CssImportFilterTest.php index c647f90..cad43ea 100644 --- a/tests/Assetic/Test/Filter/CssImportFilterTest.php +++ b/tests/Assetic/Test/Filter/CssImportFilterTest.php @@ -88,6 +88,43 @@ public function testImportValidatorReceivesResolvedPathAndCanAllow() $this->assertStringContainsString('import.css', implode('|', $seen)); } + public function testImportValidatorAuthorisesSchemeBearingImports() + { + $asset = new FileAsset(__DIR__ . '/fixtures/cssimport/schemeimport.css', [], __DIR__ . '/fixtures/cssimport', 'schemeimport.css'); + $asset->load(); + + $seen = []; + $filter = new CssImportFilter(); + $filter->setImportValidator(function ($path) use (&$seen) { + $seen[] = $path; + return false; + }); + $filter->filterLoad($asset); + + // A target carrying a scheme resolves to the URL itself rather than a path + // under the source root, and is dispatched to the remote loader. The + // validator is consulted for it like any other import form. + $this->assertContains('file:///etc/hostname', $seen); + $this->assertContains('//example.com/remote.css', $seen); + + // Rejected imports are left as raw @import statements and never loaded. + $this->assertStringContainsString('@import url("file:///etc/hostname");', $asset->getContent()); + $this->assertStringContainsString('@import url("//example.com/remote.css");', $asset->getContent()); + $this->assertStringContainsString('body { color: blue; }', $asset->getContent()); + } + + public function testImportsAreUnaffectedWhenNoValidatorIsSet() + { + $asset = new FileAsset(__DIR__ . '/fixtures/cssimport/main.css', [], __DIR__ . '/fixtures/cssimport', 'main.css'); + $asset->load(); + + $filter = new CssImportFilter(); + $filter->filterLoad($asset); + + // Default behaviour is unchanged for callers that set no validator. + $this->assertStringContainsString('body { color: red; }', $asset->getContent()); + } + public function testIsHashableSoTheAssetCacheNeverSerializesIt() { $filter = new CssImportFilter(); diff --git a/tests/Assetic/Test/Filter/fixtures/cssimport/schemeimport.css b/tests/Assetic/Test/Filter/fixtures/cssimport/schemeimport.css new file mode 100644 index 0000000..4b579bd --- /dev/null +++ b/tests/Assetic/Test/Filter/fixtures/cssimport/schemeimport.css @@ -0,0 +1,4 @@ +/* schemeimport.css */ +@import url("file:///etc/hostname"); +@import url("//example.com/remote.css"); +body { color: blue; } From 915eb9803fec8e7e9749ae528d49ff331a52ec7b Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 14 Sep 2026 16:30:45 -0600 Subject: [PATCH 2/2] Add setImportValidator() to ScssphpFilter CssImportFilter lets a caller authorise each @import before it is inlined; ScssphpFilter had no equivalent, so consumers compiling SCSS they do not control had no way to bound where @import resolves. scssphp resolves targets against the configured import paths and against the importing file's own directory, both with `..` traversal allowed. This adds the same opt-in hook, implemented by filtering the result of Compiler::findImport() in a small subclass. A rejected import is reported as unresolved, so scssphp emits the original statement verbatim rather than inlining the file -- matching how CssImportFilter treats a rejected import. Callers that set no validator are unaffected. --- .../Filter/Scssphp/ValidatingCompiler.php | 48 +++++++++++++++++++ src/Assetic/Filter/ScssphpFilter.php | 40 +++++++++++++++- .../Assetic/Test/Filter/ScssphpFilterTest.php | 48 +++++++++++++++++++ 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 src/Assetic/Filter/Scssphp/ValidatingCompiler.php diff --git a/src/Assetic/Filter/Scssphp/ValidatingCompiler.php b/src/Assetic/Filter/Scssphp/ValidatingCompiler.php new file mode 100644 index 0000000..e3b5b5c --- /dev/null +++ b/src/Assetic/Filter/Scssphp/ValidatingCompiler.php @@ -0,0 +1,48 @@ +importValidator = $importValidator; + } + + /** + * {@inheritdoc} + */ + public function findImport($url, $currentDir = null) + { + $path = parent::findImport($url, $currentDir); + + if (null === $path) { + return null; + } + + return call_user_func($this->importValidator, $path) ? $path : null; + } +} diff --git a/src/Assetic/Filter/ScssphpFilter.php b/src/Assetic/Filter/ScssphpFilter.php index 31acd1c..bd83889 100644 --- a/src/Assetic/Filter/ScssphpFilter.php +++ b/src/Assetic/Filter/ScssphpFilter.php @@ -5,6 +5,7 @@ use Assetic\Contracts\Asset\AssetInterface; use Assetic\Contracts\Filter\DependencyExtractorInterface; use Assetic\Factory\AssetFactory; +use Assetic\Filter\Scssphp\ValidatingCompiler; use Assetic\Util\CssUtils; use ScssPhp\ScssPhp\Compiler; use ScssPhp\ScssPhp\OutputStyle; @@ -27,6 +28,9 @@ class ScssphpFilter extends BaseFilter implements DependencyExtractorInterface private $outputStyle; private $variables = []; + /** @var callable|null */ + private $importValidator; + public function enableCompass($enable = true) { trigger_deprecation( @@ -106,6 +110,26 @@ public function addImportPath($path) $this->importPaths[] = $path; } + /** + * Set an optional validator that authorises each `@import` target before scssphp + * reads it. The validator receives the resolved filesystem path and must return + * true to allow the import or false to reject it, in which case the original + * `@import` statement is emitted verbatim instead of being inlined. + * + * This is an opt-in confinement hook for consumers that compile SCSS they do + * not control. Without it, `@import` targets resolve against the configured + * import paths and against the importing file's own directory, with `..` + * traversal allowed, so resolution is not bounded to any particular tree. + * Defaults to null (no restriction) so existing behaviour is unchanged for + * callers that do not set it. + */ + public function setImportValidator(?callable $importValidator): self + { + $this->importValidator = $importValidator; + + return $this; + } + public function registerFunction($name, $callable, ?array $argumentDeclaration = null) { $this->customFunctions[$name] = [ @@ -116,7 +140,7 @@ public function registerFunction($name, $callable, ?array $argumentDeclaration = public function filterLoad(AssetInterface $asset) { - $sc = new Compiler(); + $sc = $this->createCompiler(); if ($dir = $asset->getSourceDirectory()) { $sc->addImportPath($dir); @@ -145,9 +169,21 @@ public function filterLoad(AssetInterface $asset) $asset->setContent($sc->compileString($asset->getContent())->getCss()); } + /** + * Creates the scssphp compiler, honouring the import validator if one is set. + */ + private function createCompiler(): Compiler + { + if (null === $this->importValidator) { + return new Compiler(); + } + + return new ValidatingCompiler($this->importValidator); + } + public function getChildren(AssetFactory $factory, $content, $loadPath = null) { - $sc = new Compiler(); + $sc = $this->createCompiler(); if ($loadPath !== null) { $sc->addImportPath($loadPath); } diff --git a/tests/Assetic/Test/Filter/ScssphpFilterTest.php b/tests/Assetic/Test/Filter/ScssphpFilterTest.php index d53be7c..7e69050 100644 --- a/tests/Assetic/Test/Filter/ScssphpFilterTest.php +++ b/tests/Assetic/Test/Filter/ScssphpFilterTest.php @@ -71,6 +71,54 @@ public function testSetImportPath() $this->assertStringContainsString('color: red', $asset->getContent(), 'Import paths are correctly used'); } + public function testImportValidatorCanRejectImports() + { + $asset = new FileAsset(__DIR__ . '/fixtures/sass/main.scss'); + $asset->load(); + + $filter = $this->getFilter(); + $filter->setImportValidator(function ($path) { + return false; + }); + $filter->filterLoad($asset); + + // A rejected import is reported as unresolved, so scssphp emits the original + // statement verbatim instead of inlining the file. + $this->assertStringNotContainsString('color: blue', $asset->getContent()); + $this->assertStringContainsString('@import "include"', $asset->getContent()); + $this->assertStringContainsString('color: red', $asset->getContent()); + } + + public function testImportValidatorReceivesResolvedPathAndCanAllow() + { + $asset = new FileAsset(__DIR__ . '/fixtures/sass/main.scss'); + $asset->load(); + + $seen = []; + $filter = $this->getFilter(); + $filter->setImportValidator(function ($path) use (&$seen) { + $seen[] = $path; + return true; + }); + $filter->filterLoad($asset); + + // Allowed imports inline as normal, and the validator sees a resolved path. + $this->assertStringContainsString('color: blue', $asset->getContent()); + $this->assertNotEmpty($seen); + $this->assertStringContainsString('_include.scss', implode('|', $seen)); + } + + public function testImportsAreUnaffectedWhenNoValidatorIsSet() + { + $asset = new FileAsset(__DIR__ . '/fixtures/sass/main.scss'); + $asset->load(); + + $this->getFilter()->filterLoad($asset); + + // Default behaviour is unchanged for callers that set no validator. + $this->assertStringContainsString('color: blue', $asset->getContent()); + } + public function testRegisterFunction() { $asset = new StringAsset('.foo{ color: bar(); }');