chore: quality, performance, and CMS GA readiness#387
Closed
LennyObez wants to merge 1401 commits into
Closed
Conversation
LennyObez
changed the title
LennyObez
added
enhancement
LennyObez
added
testing
Qodana for PHP27791 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at qodana-support@jetbrains.com
|
LennyObez
force-pushed
the
chore/quality-and-performance
branch
7 times, most recently
from
21f947c to
6b124d7
Compare
Owner
Author
|
This PR should fix #389 (ExtensionBootstrap.loadFromPaths aborts all extension loading on single class-not-found). |
Owner
Author
|
Related issues from Studio Pair website project:
|
Owner
Author
|
Also fixes #396 -- Container::singleton() not defined, breaks Messaging and Tickets registration. |
LennyObez
force-pushed
the
chore/quality-and-performance
branch
from
6b124d7 to
69106d9
Compare
LennyObez
added a commit
that referenced
this pull request
May 12, 2026
* cms MediaBundleImporter: reject unsafe storage_path/filename from imported ZIP manifests via isSafeRelativePath() guard. Closes the path traversal exposure where an attacker-crafted bundle could write outside the media root through MediaDiskInterface::write(). Failed paths are recorded as media errors, not silently skipped. * cms AccountController + profile.pulse.php: pass `saved` flag through the view context instead of reading $_GET in the template. Removes a superglobal access from the rendering layer. * forum ForumDashboardWidget + ForumCmsDashboardWidget: inline the two table names in distinct queries instead of interpolating $table into a shared SQL string. Removes the brittle pattern that invited future callers to pass user input into a query template. Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
May 12, 2026
…d roots * CmsPluginManager::removeDirectory: early-return unless realpath(path) resolves inside the configured plugin storage root or sys_get_temp_dir(). Guards the recursive unlink/rmdir loop against any future regression in manifest slug validation or DB tampering that could place an arbitrary path in $plugin->storagePath. * StaticSiteChannel::removeStaticFile: skip the unlink unless the resolved file path lives inside the configured outputPath. Prevents an admin who controls $translation->path from escaping the static export directory through symlinks or relative segments. Defence-in-depth on top of existing upstream validation. Semgrep mcp will keep flagging the literal unlink($var) pattern (rule is purely syntactic), but the runtime exposure is now closed. Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
May 12, 2026
Same defence-in-depth pattern as CmsPluginManager: early-return unless
realpath(path) resolves inside the configured theme storage path, the public
asset directory ({publicPath}/cms-assets), or sys_get_temp_dir(). Guards the
recursive unlink/rmdir loop against any future regression in manifest slug
validation or DB tampering that could place an arbitrary path in
$theme->storagePath.
The Semgrep MCP rule on unlink($var) is purely syntactic and will keep firing,
but the runtime exposure is now closed.
SafeArchiveExtractor unlinks (lines 164/171) are already correct: they are
the explicit ZipSlip mitigation step that removes the offending file after
the realpath check fails - false positive on Semgrep's part.
Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
May 12, 2026
…on (10 files) Plus a security hardening of CmsServeCommand: replaced passthru() with shell command construction by proc_open() with an argv array (PHP 7.4+ shell-bypass form). Added strict whitelist validation of the --host option (regex match on host/IP characters only) and range validation of --port. Removed escapeshellarg dependency since the shell is no longer involved. @psalm-api markers on the Collaboration DTOs/repo interface, the two CMS console commands, the Docs DocFeedback DTO + two interfaces, the CmsException base class, and the CmsDdl migration helper utility. Refs Sprint 0.8 burndown + PR #387 follow-up security review.
New src/Security/AntiSpam/Risk: AdaptiveRiskEngine composes pluggable RiskSignalProviderInterface signals (probabilistic-OR: 1 - product(1 - s_i)) with RiskBypassProviderInterface short-circuit, and thresholds the score into a RiskDecision (Allow / Challenge / Block). Built-in BotScoreSignalProvider adapts the transparent BotDetector (0-100 -> 0-1) so it works out of the box. AdaptiveChallengeMiddleware rejects high-risk requests (403) and attaches the RiskAssessment as a request attribute so forms and the managed-challenge renderer can present a challenge only when the decision is Challenge — progressive friction instead of an always-on challenge. The score is never leaked in a response header. Wired globally by AntiSpamWiring when adaptive_risk.enabled (config/anti-spam.php). The architecture is the orchestrator for the remaining anti-bot work: JA4 fingerprinting plugs in as a signal provider, Private Access Tokens as a bypass provider, with no engine change. 17 tests; snapshot +7 #[Api].
Add an edge-supplied JA4/JA4+ TLS-fingerprint signal that feeds the adaptive risk engine. The application cannot compute a JA4 fingerprint (the TLS handshake is over by the time a request reaches PHP), so the fingerprint is computed at the TLS-terminating edge and forwarded in a header (default X-JA4). Because a direct client could forge that header, it is honoured only for requests arriving through a trusted proxy (trusted_proxies_only, on by default, gated via TrustedProxy::isTrustedSource). It is a denylist signal: an absent or unknown fingerprint adds no risk; an operator- supplied known-bad fingerprint scores match_score and escalates the engine decision. - Ja4Config (#[Api]): opt-in, header name, trusted-proxy gate, operator denylist, match score; fromArray filters non-string fingerprints. - Ja4SignalProvider (#[Internal]): RiskSignalProviderInterface; trust gate then denylist match. - AntiSpamWiring: load Ja4Config, add the provider to the engine's signal set when both adaptive_risk and ja4 are enabled. - config/anti-spam.php + docs/anti-spam.md: document the AI-scraper defense, adaptive risk engine, and JA4 signal (backfilling the config sample and docs for all three layers). - Tests: config parsing, trust gate, spoofing defense, header name, and behavioural wiring proof (denylisted JA4 escalates to block; inert when disabled).
…ation Add origin-side Private Access Token / Privacy Pass support (RFC 9577 / RFC 9578, token type 0x0002 — Blind RSA, publicly verifiable). A valid token proves the client passed an attester's checks without revealing its identity, so it bypasses the adaptive challenge: real users sail through while bots, which cannot obtain a token, still face the engine. Crypto is verified against the official RFC 9578 known-answer vector: RSASSA-PSS-VERIFY (SHA-384, MGF1-SHA-384, 48-byte salt) over the raw RSA primitive (m = s^e mod n via gmp) plus EMSA-PSS-VERIFY (RFC 8017), with token_key_id = SHA-256(issuer SPKI) and challenge_digest = SHA-256 of the TokenChallenge wire form. - TokenChallenge (#[Api]): RFC 9577 wire encoding + digest. - PrivateAccessTokenVerifier (#[Api]): stateless, fail-closed verify of token type, key id, challenge digest, and the PSS signature. - PrivacyPassConfig (#[Api]) + PrivacyPassChallengeIssuer (#[Api]). - PrivateTokenBypassProvider: a risk-engine bypass (valid token => allow). - PrivateTokenChallengeMiddleware: advertises WWW-Authenticate: PrivateToken on 401/403 so capable clients can redeem and retry. - Internal: RsaSsaPssVerifier, IssuerPublicKey (SPKI DER walk), PrivateToken (parse), PrivateTokenHeaderParser. - AntiSpamWiring: load PrivacyPassConfig, add the bypass to the engine and pipe the challenge advertiser (outermost) when configured and ext-gmp is present; degrade with a logged warning otherwise. - config/anti-spam.php + docs/anti-spam.md document the feature; the verifier requires the gmp extension. Verification is stateless (empty redemption_context); per-request single-use binding, issuer-directory discovery, and key rotation sets are documented follow-ups. Tests cover the RFC KAT, a self-issued round trip, tamper/wrong-key/wrong-challenge rejection, the header parser, the bypass provider, the challenge middleware, and the wiring (a valid token bypasses the engine).
…Spam config Add the advisory robots.txt layer for AI crawlers, driven by the same AiCrawlerConfig as request-time enforcement. robots.txt is the only control point for opt-out tokens that are never sent as a request User-Agent (Google-Extended, Applebot-Extended), and a polite first signal for the rest. - AiCrawlerRobotsPolicy (#[Api]): renders a 'User-agent: <token>' / 'Disallow: /' block for every crawler whose resolved action is Block (built-in and custom), including the opt-out-only tokens; allow and rate_limit crawlers are left unrestricted (rate limits are not expressible in robots.txt and are enforced at the middleware). Returns '' when the feature is disabled or nothing is blocked. - CMS RobotsTxtGenerator: optionally consumes the policy and inserts the directives before the Sitemap line; wired in CmsCoreServiceProvider when AiCrawlerConfig is bound. Backward compatible (no policy => unchanged). - docs/anti-spam.md documents the advisory layer. Driving both the advisory and enforced policies from one config keeps them from drifting apart. Tests cover the policy (block/allow/rate_limit, overrides, custom crawlers, opt-out-only tokens, disabled) and the generator integration (directives placed before the Sitemap).
…y rotation Close the two highest-severity gaps from the anti-bot self-audit on the PAT verifier, and tidy its public surface. - Single-use: after the RSASSA-PSS signature is proven valid, the token's nonce is recorded in the cache; a replay of the same token is rejected. Without a cache it degrades to reuse-within-lifetime (logged), mirroring the managed-challenge replay pattern. The nonce is consumed only after a valid signature, so forged tokens cannot burn nonces or poison the cache. - Key rotation: the verifier trusts a set of issuer keys at once, selecting by the token's token_key_id (token_key + token_keys), so a key can be rolled in and the old one retired with no downtime. The primary key is advertised in the challenge. - API cleanup: the constructor is now private; build via fromBase64UrlKey / fromBase64UrlKeys so the #[Internal] IssuerPublicKey/RsaSsaPssVerifier types no longer appear on the #[Api] surface (refines the unreleased RC API). - AntiSpamWiring resolves the TaggedCache for replay protection and builds the verifier from all configured keys; config/anti-spam.php + docs updated (token_keys, single_use, single_use_ttl_seconds). Tests: rotation (token under any configured key), single-use accept-then- reject with a cache, reuse-without-cache, empty key set rejected, plus the RFC 9578 KAT and round-trip still green.
Let operators point Privacy Pass at an issuer directory instead of pasting keys, completing the key-management story from the previous commit. - PrivacyPassDirectory: total parser for the RFC 9576 directory document; extracts token type 0x0002 keys, de-duplicated, ignoring other types and malformed input. - PrivacyPassDirectoryClient: fetches the directory over HTTP and caches the keys; cachedKeys() reads them. Resilient — a failed or non-200 refresh, or a directory with no usable keys, keeps the previously cached keys rather than dropping trust on a transient outage. - PrivacyPassRefreshKeysCommand (privacy-pass:keys:refresh): runs the refresh from CLI/scheduler so the request path never makes a network call. - AntiSpamWiring discovers directory keys from cache at boot and unions them with the static token_key/token_keys; registers the client + command when directory_url is set and HTTP + cache are bound. - config/anti-spam.php + docs document directory_url and the refresh step. Tests: parser (types, dedup, malformed), client (fetch/cache/read, stale-on-failure, stale-on-throw, empty-directory), and a wiring proof that a token signed under a directory-discovered key bypasses the engine.
Close the second high-severity gap from the anti-bot self-audit: a scraper could forge an allowed crawler's User-Agent to slip past. When enabled, a detected crawler's real client IP (resolved through trusted proxies) is checked against the crawler's operator-published identity. - CidrMatcher: IPv4/IPv6 CIDR membership (fail closed on malformed input). - CrawlerDnsResolver + SystemCrawlerDnsResolver: DNS abstraction for forward-confirmed reverse DNS (FCrDNS), so the verifier is testable and the blocking lookups are swappable. - CrawlerIdentityVerifier: Verified (IP in published ranges, or FCrDNS passes), Impersonator (verifiable but matches nothing), or Unverifiable (no data configured). FCrDNS results are cached off the hot path. - AiCrawlerVerificationConfig (#[Api]) + CrawlerIdentity (#[Internal]). - AiCrawlerMiddleware blocks impersonators with 403 regardless of the crawler's configured action; AntiSpamWiring builds the verifier (system DNS + cache) and passes it plus TrustedProxy when verification is enabled. - config/anti-spam.php + docs document ai_crawler_verification. Tests: CIDR matrix, verifier (range hit/miss, FCrDNS confirm/spoof/suffix- mismatch, disabled), config parsing, middleware impersonator-blocking, and a wiring proof that a forged GPTBot from a non-published IP is blocked.
Strengthen the JA4 TLS-fingerprint signal beyond exact-match denylisting, closing a medium gap from the anti-bot self-audit. - known_bad_prefixes: a family/partial match — when the fingerprint starts with a configured prefix (typically the JA4_a component, which summarises the TLS version and cipher list), it scores the lower-confidence partial_match_score. This catches a fingerprint family even as the later components vary. - known_good_fingerprints: an exact allowlist that is always safe (score 0) and overrides the prefix denylist, so a flagged family can still permit known-good members. - Precedence: allowlist > exact denylist > prefix denylist > no signal. - config/anti-spam.php + docs document the new keys. The constructor gains three optional parameters (named-arg compatible) and fromArray funnels all list parsing through a single string-filtering helper. Tests: prefix match, non-match, allowlist-overrides-prefix, exact-takes- precedence, allowlist-zero, plus config parsing and non-string filtering.
Broaden the adaptive engine's signal set with two self-hosted, per-origin signals — the local reputation it otherwise lacks — closing a medium gap from the anti-bot self-audit. - VelocitySignalProvider: counts a client's requests (keyed on the trusted-proxy-resolved IP) in a fixed window; below the threshold it adds no risk, above it the score scales with the overage up to max_score. The clock is injectable for deterministic tests. - DatacenterIpSignalProvider: scores a client whose IP falls in operator-supplied datacenter CIDR ranges (no bundled ASN database — a deliberate no-external-data choice). - CIDR matching is consolidated into a shared Pulsar\Support\Net\CidrMatcher (#[Api]); the AiCrawler-internal copy is removed and CrawlerIdentityVerifier now uses the shared one (whose FCrDNS cache key also drops the reserved ':'). - AntiSpamWiring resolves the trusted proxy once and feeds both signals into the engine when enabled; velocity also requires the cache. - config/anti-spam.php + docs document velocity and datacenter. Tests: velocity (below/above threshold, capping, window reset, disabled, no-IP), datacenter (in/out of range, disabled, trusted-proxy resolution), the shared CidrMatcher matrix, and wiring proofs that each signal escalates the engine to a block.
…ed cache keys Add an integration test that boots the real wiring (cache then anti-spam, as the Kernel does) and dispatches HTTP requests through the global middleware pipeline, proving the wired stack behaves end-to-end: - a declared AI crawler is blocked (403) with the TDM reservation header; - an ordinary request passes through; - the adaptive engine blocks a high-risk request; and - a valid Private Access Token bypasses that block, while the same request without a token is blocked. That last case surfaced a latent bug the unit tests' permissive fake caches hid: the per-token nonce, FCrDNS, and directory cache keys used ':', which CacheKeyValidator rejects (PSR-6 reserved), so against the real cache the PAT single-use check and directory read threw and failed closed. Switch those keys to '.' separators (matching the managed-challenge convention).
…plaints)
The mail webhook handler stack (signature verifiers, replay window,
deduplication, bounce/complaint handlers) existed but nothing wired it, so
provider bounce/complaint notifications were never received. Make it a
turnkey, opt-in endpoint.
- MailWebhookConfig (#[Api]): provider, secret, path, replay window, IP
allowlist. Disabled by default — an always-on webhook route is attack
surface — and usable only with a provider + secret (SES is cert-based, so
it needs no secret). Nested under MailConfig.webhooks.
- MailWebhookController: builds the WebhookRequest, enforces the optional
source-IP allowlist, runs the secure handler, and dispatches bounce /
complaint events to their handlers.
- CacheBackedDeduplicationStore: cross-process event dedup (the only existing
store was per-process in-memory); keys are hashed so arbitrary tenant/event
ids can never introduce PSR-6 reserved characters.
- MailWiring builds the verifier for the configured provider + dedup store
(cache-backed when available) + handlers + controller and registers
POST {path}; config/mail.php documents the block.
Tests: config (usability matrix incl. SES), dedup store (cross-tenant,
cache-safe keys via the real validator, no-op cleanup), controller
(bounce/complaint routing, 401 on rejection, 403 on disallowed IP), and
wiring (endpoint + route registered only when configured).
…systems Register three built-but-unwired subsystems in the default wiring list. All three default to off, so production behaviour is unchanged. Edge: EdgeMiddleware adapts the incoming HTTP request to an EdgeRequest and runs the configured edge-function pipeline (A/B tests, geo redirects), short-circuiting with the redirect/block response a function returns. Configured via config/edge.php; wired only when functions are present. Documentation: opt-in versioned documentation serving. DocumentationWiring registers the version registry and resolver middleware only when versions are configured (config/documentation.php). Profiler: an opt-in per-request profiler for dev/staging. ProfilerMiddleware emits a Server-Timing response header (total, DB, cache). Query timings flow through the monitored connection, including a profiler-only path so enabling the profiler alone is sufficient to capture them; slow-query and audit logging stay silent when SQL monitoring is off, so no audit lines leak. Cache hits/misses are recorded through a new CacheManager event listener. Register the three wirings as composition roots (ModuleMap) and reuse Coerce::mapOfString in the edge config/adapter in place of bespoke filtering.
…misleading httpsEnforced flag A literal `Strict-Transport-Security` or `Permissions-Policy` set in the `headers` array of config/security.php was silently dropped. The structured HstsConfig overwrote HSTS in the middleware, and PermissionsPolicyConfig (whose defaults are always non-empty) overwrote any literal Permissions-Policy in effectiveHeaders(). A literal HSTS with `preload` therefore never reached the wire. Literals are now authoritative — "what you write is what's emitted": - SecurityHeadersConfig::effectiveHstsHeader() resolves the value to emit, preferring a literal `Strict-Transport-Security` over the structured `hsts` block. effectiveHeaders() drops any literal HSTS so it is never sent unconditionally, and the middleware emits it only on secure requests (RFC 6797 §7.2 — also fixing a latent leak of a literal HSTS over plain HTTP). - A literal `Permissions-Policy` now wins over the structured config. - literalHeader() matches case-insensitively (RFC 9110 §5.1). - SecurityWiring logs a one-time boot warning (shadowedStructuredHeaders()) when a literal shadows an active structured block with a different value, so the override is never silent in either direction. Separately, SecurityAssertionRunner's `httpsEnforced` constructor parameter is renamed to `hstsEnabled`: it mirrors `headers.hsts.enabled` and only gates the posture self-check — it emits no redirect, cookie, or header. The docblock now states this explicitly. Docs (security-baseline.md) and the config scaffold document literal precedence, the secure-only HSTS rule, and the `include_sub_domains` (snake_case) key spelling.
…ry, not getenv() A master key provided only in `.env` (the common production layout) was reported missing by the security posture self-check, because SecurityAssertionRunner read it through getenv() — which never sees `.env`-loaded values (they are parsed into the Environment repository, not exported to the OS process env). Under PHP-FPM this logged a critical `master_key_present` warning on nearly every worker boot. The runtime crypto path (SecurityWiring -> MasterKey -> session/CSRF) already resolved the key through `$configManager->environment()->get()`, so the key WAS loaded for encryption; the assertion runner simply disagreed with it. This change makes the posture check and the cache-invalidation fingerprint consistent with the crypto path: - SecurityAssertionRunner now receives the framework-resolved master key (`?string $masterKeyHex`) and checks that, instead of calling getenv() itself. SecurityWiring passes the same Environment-resolved value it already uses to build the MasterKey. - FrameworkCache resolves its structural env-invalidation keys through the injected Environment (OS env + .env) when available, falling back to getenv() only when no Environment is wired (the dev bootstrap). A key present only in `.env` now participates in cache invalidation. The security path intentionally does NOT add a raw getenv() fallback: Environment::get() already merges OS env at load, and a bare getenv() would bypass the F4.9 allowlist hardening. Tests: a .env-only key (absent from the OS process env) now wires the MasterKey and produces no `master_key_present` warning (SecurityWiringTest, end-to-end); the runner honors a resolved key without getenv; the cache fingerprint tracks the Environment-resolved value.
…ut app wiring
API-based mail transports (mailgun/ses/postmark/sendgrid) require a
MailHttpClientInterface, but the framework shipped no implementation and
MailWiring only injected one if the application had already bound it. So every
API driver threw "MailHttpClientInterface is required for API-based transports"
on first send; only smtp/log/array worked out of the box.
Ship two implementations and auto-bind a default:
- CurlMailHttpClient: the zero-config default (ext-curl is already required).
Enforces TLS peer + host verification and bounded connect/total timeouts;
returns 4xx/5xx as responses and throws only on transport-level failure
(which the transport wraps in a driver-scoped MailException).
- Psr18MailHttpClient: adapts a container PSR-18 client (via the framework's
PSR-17 request/stream factories) so mail reuses the application's HTTP stack
— pooling, retries, proxy, observability, test doubles — when one is provided.
MailWiring now registers a default MailHttpClientInterface whenever mail is
enabled and the application has not bound one: it prefers a container PSR-18
client (Psr\Http\Client\ClientInterface), otherwise the cURL client. An
application-provided client is preserved.
Adds psr/http-client (^1.0) — a PHP-FIG interface-only package with no
transitive dependencies, the sibling of the already-present psr/http-factory.
Acceptance: with MAIL_DRIVER=mailgun and valid mailgun driver_options,
MailManager::driver('mailgun') resolves the transport instead of throwing, and
MailManager::raw() sends through the default client with no app-side binding.
…lues apply The global env() helper read getenv() only, so a value provided solely in .env (parsed into the Environment repository but never exported to the OS process env) was invisible to env() — including the env() calls inside the framework's own config/*.php files. .env effectively did not work for env(). Bind the loaded Environment as a process-global "active" instance: - Environment::activate()/active()/resetActive(), set by ConfigManager::load() and loadFromCache() before any config file is required. - env() now resolves: active environment (OS env + .env, merged, OS-wins) -> getenv() fallback -> default, keeping the existing true/false/null/empty coercion. Strictly additive: only .env-only keys that previously returned the default now resolve. - A PHPUnit extension resets the active environment before every test, so the process-global cannot leak across the suite. - ADR-0033 documents why this bootstrap global is acceptable under the project's "avoid global state" rule. Also adds Environment::read() — an un-coerced, .env-aware accessor for opaque secrets and paths — and routes the AWS / Azure / GCP credential getenv() reads through it, so cloud credentials provided only in .env resolve too. env() remains a bootstrap-time helper; runtime code should prefer typed config (ConfigManager / config DTOs) or Environment::get().
…hardcoded page CsrfMiddleware emitted a hardcoded <!DOCTYPE html> 403 (and a bespoke JSON body) that bypassed the application's configured error renderer, so the CSRF rejection could not be themed or localized like every other 4xx. CsrfMiddleware now content-negotiates its 403: - JSON for API clients (Accept: application/json, or X-Requested-With: XMLHttpRequest), preserving the prior XHR-as-JSON behaviour. - HTML rendered via the application's configured ExceptionRendererInterface — themed and localized like every other error page — when one is wired, falling back to a minimal inline document otherwise. The response is returned, not thrown. Throwing would let the Kernel's outer catch build the error response after the middleware stack has unwound, stripping the SecurityHeadersMiddleware headers off the 403; returning keeps the response flowing back out through the stack so security headers still apply on rejection (covered by SecurityPipelineTest::securityHeadersAreAppliedEvenOnCsrfRejection). The renderer is wired by ExceptionHandlerWiring, which runs after SecurityWiring, so SecurityWiring injects a lazy resolver closure (mirroring its own templateEngineResolver pattern) that the middleware invokes at request time. The optional constructor parameter is additive: callers that omit it get the minimal fallback page, exactly as before.
LennyObez
force-pushed
the
chore/quality-and-performance
branch
from
67941aa to
e4b9478
Compare
github-project-automation
Bot
moved this from Review
to Done
in Pulsar - Release & Delivery
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
* cms MediaBundleImporter: reject unsafe storage_path/filename from imported ZIP manifests via isSafeRelativePath() guard. Closes the path traversal exposure where an attacker-crafted bundle could write outside the media root through MediaDiskInterface::write(). Failed paths are recorded as media errors, not silently skipped. * cms AccountController + profile.pulse.php: pass `saved` flag through the view context instead of reading $_GET in the template. Removes a superglobal access from the rendering layer. * forum ForumDashboardWidget + ForumCmsDashboardWidget: inline the two table names in distinct queries instead of interpolating $table into a shared SQL string. Removes the brittle pattern that invited future callers to pass user input into a query template. Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
…d roots * CmsPluginManager::removeDirectory: early-return unless realpath(path) resolves inside the configured plugin storage root or sys_get_temp_dir(). Guards the recursive unlink/rmdir loop against any future regression in manifest slug validation or DB tampering that could place an arbitrary path in $plugin->storagePath. * StaticSiteChannel::removeStaticFile: skip the unlink unless the resolved file path lives inside the configured outputPath. Prevents an admin who controls $translation->path from escaping the static export directory through symlinks or relative segments. Defence-in-depth on top of existing upstream validation. Semgrep mcp will keep flagging the literal unlink($var) pattern (rule is purely syntactic), but the runtime exposure is now closed. Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
Same defence-in-depth pattern as CmsPluginManager: early-return unless
realpath(path) resolves inside the configured theme storage path, the public
asset directory ({publicPath}/cms-assets), or sys_get_temp_dir(). Guards the
recursive unlink/rmdir loop against any future regression in manifest slug
validation or DB tampering that could place an arbitrary path in
$theme->storagePath.
The Semgrep MCP rule on unlink($var) is purely syntactic and will keep firing,
but the runtime exposure is now closed.
SafeArchiveExtractor unlinks (lines 164/171) are already correct: they are
the explicit ZipSlip mitigation step that removes the offending file after
the realpath check fails - false positive on Semgrep's part.
Refs PR #387 security review.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
…on (10 files) Plus a security hardening of CmsServeCommand: replaced passthru() with shell command construction by proc_open() with an argv array (PHP 7.4+ shell-bypass form). Added strict whitelist validation of the --host option (regex match on host/IP characters only) and range validation of --port. Removed escapeshellarg dependency since the shell is no longer involved. @psalm-api markers on the Collaboration DTOs/repo interface, the two CMS console commands, the Docs DocFeedback DTO + two interfaces, the CmsException base class, and the CmsDdl migration helper utility. Refs Sprint 0.8 burndown + PR #387 follow-up security review.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
* AudioProcessor::cleanupTempFile / PdfThumbnailGenerator::cleanupTempFile appended DIRECTORY_SEPARATOR to the resolved sys_get_temp_dir() before the str_starts_with check. Without the trailing separator a directory whose name shared a prefix with the temp dir (e.g. "/tmp_evil/..." vs "/tmp") could pass the boundary check and have files unlinked from outside the temp area. Same hardening as CmsPluginManager / ThemeManager / StaticSiteChannel. * PdfThumbnailGenerator::generateWithGhostscript: nosemgrep on the proc_open() call. Argv-array form bypasses the shell entirely, and both $this->ghostscriptPath (server config) and $pdfPath (validated upstream by FileValidator/PdfValidator) are non-shell inputs. Same pattern as CmsServeCommand. Plus @psalm-api on cms public Audio + Document DTOs and processors: AudioConfig, AudioMetadata, AudioTranscodePreset, DocumentMetadata. Refs PR #387 security review + Sprint 0.8 burndown.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
…0031, F33.M2/F385.M1/F387.M1) The project's de-facto release process bundles whole release cycles into single mega-PRs: - PR #33: 51 K LOC, 1 reviewer (F33.M2 critique) - PR #380: 290 K LOC, closed without merge (F385.M3) - PR #385: 408 K LOC, 50 commits squash-merged into rc.11 (F385.M1) - PR #387 (open, GA target): 243 K LOC, 2 296 files (F387.M1) A 400 K-line diff cannot be reviewed in any meaningful sense. For a framework targeting banking / healthcare / legal compliance, the "PR was peer-reviewed" claim becomes self-documenting fiction -- exactly the kind of audit-trail gap regulators pull on. ADR-0031 establishes: 1. Hard cap: feature PRs MUST NOT exceed 1 500 LOC of substantive diff (excluding generated code, fixtures, lockfiles, renames). CI enforces via tools/ci/check-pr-size.sh against the diff vs main. 2. Soft target: ≤ 500 LOC where feasible. 3. Release PRs (the PR that bumps version + changelog) MUST be ≤ 100 LOC and may only touch composer.json/composer.lock, src/Core/Version.php, and CHANGELOG.md. Bundling features into a release PR is rejected. 4. Conventional Commits prefix MUST reflect actual scope: a PR that introduces a new src/<Module>/ MUST use feat(...), not chore(...) (closes F387.M2). 5. Closed-without-merge release PRs MUST carry a maintainer comment explaining why (closes the audit-trail gap from F385.2 + F385.4). 6. The 1.0.0 GA tag is gated on this policy being satisfied; PR #387 in its current form is not eligible for merge. Implementation phases: CI enforcement now, retrospective audit of PR #385's 50 commits during rc.12, GA gate confirmation in the release-PR description. Closes audit findings F33.M2, F385.M1, F385.M3, F387.M1, F387.M2.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
…, F387.M3, F385.1) The Version::PRERELEASE_SUFFIX constant has drifted from composer.json on every release cycle since rc.5: identified at PR #23, ignored on PRs #24-#32, fixed at PR #33, re-opened at rc.11 (PR #387 GA target). Without an automated check, this pattern is guaranteed to recur at rc.12, rc.13, and the GA tag. Add tools/ci/check-version-consistency.sh which composes the runtime version string from the parsed MAJOR/MINOR/PATCH ints + PRERELEASE_SUFFIX constant in src/Core/Version.php and compares it against the version field in composer.json. Mismatch exits non-zero with an explanation that points to the recurring audit finding. Wired into ci.yml's php-quality job before composer cs:check, so any release PR that bumps composer.json::version without flipping the Version constant (or vice-versa) is rejected before merge. Bonus: harden two pre-existing GitHub Actions injection sinks flagged by static analysis (CWE-78). git fetch origin ${{github.base_ref}} and its sibling now route through env: variables and quoted "$BASE_REF" substitution, eliminating the path that lets a maliciously named base ref inject shell commands into the runner. Closes audit findings F23.1 (recurring), F385.1, F385.M2, F387.1, F387.M3.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
…0 carry-over) RegistrationCeremonyTest::verifyLogsFailureOnException expected the audit log to be invoked with `actor: null`, but the production code in extensions/auth/src/WebAuthn/Ceremony/RegistrationCeremony.php:170 calls `actor: AuditActor::anonymous()` (the explicit-actor pattern mandated since F25.10 / Session 1 commit 358f149a forbade the silent 'system' fallback for null actors). The test was created together with the production code in rc.11 (PR #387 release bundle, commit ca0edbda) but its actor expectation predated the F25.10 fix. The earlier filtered test runs never exercised this file, so the failure surfaced only when the WebAuthn suite was widened in this Session 9 batch. Replace the `null` matcher with a callback that asserts an `AuditActor` of kind Anonymous (id 'anonymous'). 406 / 406 tests now pass on the WebAuthn + OAuth2Extension subset.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
F387.4 / F387.M2 / F380.M1: PRs #380, #385, and #387 all used `chore:` or `docs:` prefixes while adding new core modules (`src/Workflow/`, `src/Saga/`, `src/Codegen/`, `src/ServiceDiscovery/`) or new extensions (`extensions/accessibility/`). Reviewers triage by title — a `chore:` invites less scrutiny than a `feat(core):`, and that scrutiny gap is a compliance gap for a banking framework. Adds `tools/ci/check-pr-title-scope.sh`: reads $PR_TITLE and the base-ref diff, fails the build when title prefix is `chore:` or `docs:` but the diff adds a new top-level dir under `src/` or `extensions/`. Wired into ci.yml on pull_request events only. Allowed prefixes (feat, fix, security, perf, refactor, test, ci, build) bypass the rule — only the two deception-prone prefixes trigger it. Smoke-tested locally: `feat(core):` + module add → exit 0, `chore:` + diff containing src/X/foo.php → exit 1 with human-readable error.
LennyObez
added a commit
that referenced
this pull request
Jun 30, 2026
F387.M1 / ADR-0031: the ADR was accepted but its CI
enforcement was unwired — the policy existed on paper, the
mega-PR pattern continued unchecked. This commit ships the
enforcement script.
`tools/ci/check-pr-size.sh`:
- counts substantive diff (additions + deletions) between
`origin/<base>` and HEAD, excluding paths in
`.size-limit-ignore`;
- exits 0 under the 1500-line cap, exits 1 over;
- exemption requires BOTH the `oversize-pr-acknowledged`
label AND a `/oversize-pr-approved` comment by a
CODEOWNER who is NOT the PR author (mirrors the F28.3
adr-exempt two-party pattern).
`.size-limit-ignore` ships sane defaults: lockfiles
(composer/pnpm/npm/yarn), the regenerated public-API snapshot,
and pack-scaffold lockfiles. Operators can extend the list
without touching the script.
`.github/workflows/ci.yml` invokes the check on every
`pull_request` event, with `PR_NUMBER` / `PR_AUTHOR` /
`BASE_REF` / `REPO_FULL` / `GH_TOKEN` wired through.
Smoke-tested locally:
- missing BASE_REF (push event simulation) → skip + exit 0;
- syntax check (`bash -n`) clean.
The ADR-0031 policy is now load-bearing: PR #387 (or any
successor that retains the mega-PR shape) cannot merge
without the explicit two-party exemption flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive quality, performance, and feature additions across the entire Pulsar framework — preparing the CMS extension and core framework for GA readiness.
Core Framework Enhancements
GET /health) with cache, queue, disk checksSecurity & Compliance
CMS Extension
<picture>with WebP/AVIF srcset, lazy loading, blur-up placeholdersForum Extension
Commerce Extensions
Database Layer Audit
Developer Experience
Testing & Quality
Stats
Closes #357
Test plan
composer qapasses with 0 errorspnpm format:check && pnpm lint && pnpm typecheck && pnpm testall green