Skip to content

Apply import validation consistently across the CSS and SCSS filters - #53

Merged
LukeTowers merged 2 commits into
masterfrom
feature/import-validation
Sep 15, 2026
Merged

LukeTowers merged 2 commits into
masterfrom
feature/import-validation

Conversation

@LukeTowers

@LukeTowers LukeTowers commented Sep 14, 2026

Copy link
Copy Markdown
Member

Two related gaps in how @import resolution can be confined by a consumer.

CssImportFilter — the validator skipped two import forms

setImportValidator() was only consulted on the local-file branch. An @import whose target carries a scheme, or is protocol-relative, is dispatched to the remote loader earlier in the same if/elseif chain, so those forms never reached the validator — a caller that had confined imports to a set of roots still had them 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. The docblock is updated to say so.

ScssphpFilter — no equivalent hook existed

CssImportFilter lets a caller authorise each import; ScssphpFilter had nothing comparable, so a consumer compiling SCSS it does 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 setImportValidator(), implemented by filtering the result of Compiler::findImport() in a small ValidatingCompiler subclass. All of ScssphpFilter's configuration is private, so the behaviour can't be layered on from outside the class. A rejected import is reported as unresolved, so scssphp emits the original statement verbatim instead of inlining the file — matching how CssImportFilter treats a rejected import.

Compatibility

Both hooks are opt-in and default to null. Callers that set no validator get byte-identical behaviour; nothing in the existing suite changed.

Tests

5 added:

  • CssImportFilterTest::testImportValidatorAuthorisesSchemeBearingImports — fails on master, passes here. Uses the reject path, so nothing is fetched or read.
  • CssImportFilterTest::testImportsAreUnaffectedWhenNoValidatorIsSet
  • ScssphpFilterTest::{testImportValidatorCanRejectImports, testImportValidatorReceivesResolvedPathAndCanAllow, testImportsAreUnaffectedWhenNoValidatorIsSet}

Full suite: 422 tests, 586 assertions, 0 failures (59 pre-existing skips for absent external binaries). phpcs clean against the repo ruleset.

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.
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.
@LukeTowers
LukeTowers requested a review from a team as a code owner September 14, 2026 22:32
@LukeTowers
LukeTowers force-pushed the feature/import-validation branch from 03015ec to 915eb98 Compare September 15, 2026 06:08
@LukeTowers
LukeTowers merged commit e6bd7f2 into master Sep 15, 2026
9 checks passed
@LukeTowers
LukeTowers deleted the feature/import-validation branch September 15, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant