Cleanup: dead code and minor correctness issues found during security audit #17

New issue

Closed

opened 2026-10-15 22:38:58 -07:00 by eric · 0 comments

eric commented

2026-10-15 22:38:58 -07:00

Owner

Type: cleanup / minor correctness. Found by security audit run 3 (commit 01195c8). None of these cross a security boundary.

HostResolver::isPrimary() (src/Request/HostResolver.php:41-44) checks the isPrimary flag rather than identity with Settings::primary(). With a stored config that has no flagged primary (not reachable through the sanitizer), redirect_canonical is suppressed on every host. Compare against Settings::primary() instead.
UrlRewriter::urlFor() (src/Rewrite/UrlRewriter.php:70-77) is unused and shares the request-scheme behaviour from #9. Remove it or fix it together with rewrite().
Host::scheme() (src/Config/Host.php:27-29) returns https for clearnet hosts but is never called; either use it (see #9) or delete it so the intent is not implied.
get_avatar_url is hooked twice: once via URL_FILTERS (src/Rewrite/HookBindings.php:38) and again explicitly (src/Rewrite/HookBindings.php:61). Harmless (idempotent) but redundant.
uninstall.php:22 calls get_sites( array( 'fields' => 'ids' ) ) with the default limit of 100, so on larger networks multi_domain_redux_settings is left on blogs after the 100th. Pass 'number' => 0 or paginate.

**Type:** cleanup / minor correctness. Found by security audit run 3 (commit `01195c8`). None of these cross a security boundary. - `HostResolver::isPrimary()` (`src/Request/HostResolver.php:41-44`) checks the `isPrimary` flag rather than identity with `Settings::primary()`. With a stored config that has no flagged primary (not reachable through the sanitizer), `redirect_canonical` is suppressed on every host. Compare against `Settings::primary()` instead. - `UrlRewriter::urlFor()` (`src/Rewrite/UrlRewriter.php:70-77`) is unused and shares the request-scheme behaviour from #9. Remove it or fix it together with `rewrite()`. - `Host::scheme()` (`src/Config/Host.php:27-29`) returns https for clearnet hosts but is never called; either use it (see #9) or delete it so the intent is not implied. - `get_avatar_url` is hooked twice: once via `URL_FILTERS` (`src/Rewrite/HookBindings.php:38`) and again explicitly (`src/Rewrite/HookBindings.php:61`). Harmless (idempotent) but redundant. - `uninstall.php:22` calls `get_sites( array( 'fields' => 'ids' ) )` with the default limit of 100, so on larger networks `multi_domain_redux_settings` is left on blogs after the 100th. Pass `'number' => 0` or paginate.

eric added the

enhancement

security

labels

2026-10-15 22:38:58 -07:00

eric referenced this issue from a commit

2026-10-16 13:48:09 -07:00

chore: clean up dead code and minor correctness issues from audit

eric closed this issue

2026-10-16 13:48:09 -07:00