Apply import validation consistently across the CSS and SCSS filters

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);
             }

src/Assetic/Filter/Scssphp/ValidatingCompiler.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Assetic\Filter\Scssphp;
+
+use ScssPhp\ScssPhp\Compiler;
+
+/**
+ * A scssphp compiler that runs every resolved `@import` target past a validator
+ * before the file is read.
+ *
+ * scssphp resolves `@import` targets against the compiler's import paths and, for
+ * nested imports, against the importing file's own directory — both with `..`
+ * traversal allowed. Consumers that need resolution bounded to a particular tree
+ * therefore need a hook; this compiler provides it by filtering the result of
+ * {@see Compiler::findImport()}.
+ *
+ * A rejected import is reported as unresolved, which makes scssphp emit the
+ * original `@import` statement verbatim instead of inlining the file — the same
+ * outcome as {@see \Assetic\Filter\CssImportFilter}'s rejected imports.
+ *
+ * @internal
+ */
+class ValidatingCompiler extends Compiler
+{
+    /** @var callable */
+    private $importValidator;
+
+    public function __construct(callable $importValidator)
+    {
+        parent::__construct();
+
+        $this->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;
+    }
+}

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);
         }

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();

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(); }');

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; }