This repository was archived by the owner on May 29, 2026. It is now read-only.
fix(security): M3 FilesWidget upload hardening + M4 ADR-023 comments/reactions wiring#351
Merged
Merged
Conversation
…r comments/reactions M3 (FilesWidgetService): reject non-PHP-uploaded tmp_names via is_uploaded_file, enforce 50 MiB hard cap, apply mimeTypeFilter on write path (previously read-only), stream-write to avoid full-file memory load. M4 (DashboardCommentsApiController, DashboardReactionApiController): wire all eight endpoints through ActionAuthService.requireAction via the ADR-023 action matrix; add corresponding seeds to actions.seed.json; inject IUserSession instead of relying solely on the userId string param. Also fixes pre-existing test breakages: DemoShowcasesServiceTest missing ILockingProvider, DashboardMetadataControllerTest using removed IGroupManager dependency (replaced with PermissionService), FeedRefreshServiceTest failures caused by DNS resolution of mock URLs against the SSRF guard (now delegates to injectable UrlSafetyValidator, consistent with CalendarWidgetService pattern).
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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
tmp_namepaths viais_uploaded_file(prevents local-file-inclusion via crafted requests), enforce 50 MiB hard size cap, enforcemimeTypeFilteron the write path (previously filter was read-only), stream-write viafopen/stream_copy_to_streamto avoid loading whole file into memory.ActionAuthService::requireActionper ADR-023; add action seeds toactions.seed.json; replace$userIdnull-check withIUserSession::getUser()for consistency.DemoShowcasesServiceTestmissingILockingProviderarg;DashboardMetadataControllerTestusing removedIGroupManagerdep (nowPermissionService);FeedRefreshServiceTestfailing against SSRF guard DNS resolution — refactoredFeedRefreshServiceto injectUrlSafetyValidator(consistent withCalendarWidgetService/NewsWidgetServicepattern).Test plan
php vendor/bin/phpunit --no-coverage— 1085/1085 pass, 0 failures, 0 errorsphp vendor/bin/phpstan analyse— no errorsphp vendor/bin/phpcs --standard=phpcs.xml— 0 errors on modified files (warnings are pre-existing @SPEC tags)