Skip to content

Declare league/commonmark ^2.10.2 as a direct dependency of Storm - #253

Merged
LukeTowers merged 1 commit into
wintercms:wip/1.3from
zimudec:port-c415db82-wip-1-3
Oct 10, 2026
Merged

LukeTowers merged 1 commit into
wintercms:wip/1.3from
zimudec:port-c415db82-wip-1-3

Conversation

@zimudec

@zimudec zimudec commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Re-scoped PR: the two Markdown fixture hunks of the previous version arrived upstream via the develop → wip/1.3 sync of 2026-10-06 (compare merge_base = c415db82, the #247 merge commit on develop); the composer.json require was not — that sync resolved composer.json as a conflict. What remains is the packaging half: declare league/commonmark as a direct dependency of Storm.

Why

  • Storm imports League\CommonMark classes directly at runtime (~25 references across src/Parse: Markdown.php, the in-tree CommonMarkCoreExtension, StrikethroughExtension, the Markdown facade). Composer doctrine: what you import directly, you require directly.
  • Today the package only arrives transitively, via laravel/framework (composer why: laravel/framework v12.69.3 requires league/commonmark (^2.8.1)) — a floor below the latest security release, so nothing guarantees ≥ 2.10.2 for Storm consumers.
  • 2.10.2 is the security floor (two advisories published 2026-09-30, both first patched in 2.10.2):
    • GHSA-3q6v-r5mr-hxv8 (HIGH, CVSS 7.5): quadratic-time DoS in the GFM Table extension block-start scan. Storm registers TableExtension by default and this one is not mitigable by configuration.
    • GHSA-97jj-33gv-5xf9 (MEDIUM, CVSS 6.1): DisallowedRawHtml bypass → stored XSS. Storm loads the extension with commonmark's default html_input=allow, so it applies.
  • Parity with develop, where Fix the simple_table Markdown test for league/commonmark 2.10.2 #247 already left the direct require in place.

The caret keeps it below 3.0, where the in-tree Winter extensions (written against the 2.x API) would break. composer why-not league/commonmark 2.10.2 reports no conflicts with current requirements.

Verification

  • Fresh dependency resolution on this tip resolves league/commonmark 2.10.3 and the full suite is green: 866 tests, 3412 assertions, 8 skipped, 13 PHPUnit deprecations (PHP 8.5.9 / PHPUnit 11.5.57).
  • The upstream-corrected simple_table fixtures double as a floor test: forcing composer update --with league/commonmark:2.10.1 turns MarkdownTest::testParse data set Updated DotEnv call to pass repository #59 red (the table stops rendering); back on 2.10.3 everything is green.

Reference: https://github.com/thephpleague/commonmark/compare/2.10.1…2.10.2

CI note (the red ✗ jobs are not caused by this diff)

This PR's tree is identical to the wip/1.3 head except for the one composer.json line. Since 2026-10-08 19:41 UTC, three new advisories affect every < 1.0.0 version of enshrined/svg-sanitize (CVE-2026-107380/107381/107379; the patched release is 1.0.0, which the current ~0.16 constraint excludes), and Composer ≥ 2.9's dependency policy (config.policy.advisories.block, enabled by default) refuses to resolve any affected version — so a lock-less composer install (storm ships no lock) now fails on any storm commit, including the wip/1.3 head itself (its last green CI run predates those advisories). Reproduced locally with the same error. This PR stays single-concern; the svg-sanitize constraint is an upstream decision (^1.0 if API-compatible, or a policy.advisories.ignore stopgap).

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b3a36143-2154-4e31-beaa-01b080d22242

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zimudec
zimudec force-pushed the port-c415db82-wip-1-3 branch from f9bae2c to ab5855b Compare October 9, 2026 18:07
@zimudec zimudec changed the title Fix the simple_table Markdown test for league/commonmark 2.10.2 (#247) Declare league/commonmark ^2.10.2 as a direct dependency of Storm Oct 9, 2026
@LukeTowers
LukeTowers merged commit 10c8e6d into wintercms:wip/1.3 Oct 10, 2026
2 of 10 checks passed
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.

2 participants