tbh/breadcrumb-the-shire
1
0
Fork 0
forked from fa/breadcrumb-the-shire
Code Pull Requests Activity

Harden module runtime: audited listener failures and DI-first resolver

Browse Source
This commit is contained in:
Felix Schneider
2026-03-25 10:02:03 +01:00
parent a721ec0043
commit cb5f7037b2
12 changed files with 314 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lib/App/Container/Registrars/ServiceFactoryRegistrar.php
View File

@@ -4,6 +4,7 @@ namespace MintyPHP\App\Container\Registrars;
use MintyPHP\App\AppContainer;
use MintyPHP\App\Container\ContainerRegistrar;
use MintyPHP\App\Module\ModuleClassResolver;
use MintyPHP\App\Module\ModuleEventDispatcher;
use MintyPHP\App\Module\ModuleRegistry;
use MintyPHP\Http\CookieStoreInterface;
@@ -116,7 +117,8 @@ final class ServiceFactoryRegistrar implements ContainerRegistrar
$container->set(AccessPolicyFactory::class, static fn (AppContainer $c): AccessPolicyFactory => new AccessPolicyFactory(
$c->get(PermissionService::class),
$c->get(TenantScopeService::class),
$c->has(ModuleRegistry::class) ? $c->get(ModuleRegistry::class) : null
$c->has(ModuleRegistry::class) ? $c->get(ModuleRegistry::class) : null,
static fn (string $class): ?object => ModuleClassResolver::resolve($c, $class)
));
$container->set(UserGatewayFactory::class, static fn (AppContainer $c): UserGatewayFactory => new UserGatewayFactory(
$c->get(AccessServicesFactory::class),

50
lib/App/Module/ModuleClassResolver.php Normal file
View File

@@ -0,0 +1,50 @@
<?php
namespace MintyPHP\App\Module;
use MintyPHP\App\AppContainer;
/**
* Resolve module-declared classes from DI first and only fall back to direct
* instantiation when the class is not container-bound and has no required
* constructor arguments.
*/
final class ModuleClassResolver
{
public static function resolve(AppContainer $container, string $class): ?object
{
$class = trim($class);
if ($class === '') {
return null;
}
if ($container->has($class)) {
try {
$resolved = $container->get($class);
return is_object($resolved) ? $resolved : null;
} catch (\Throwable) {
return null;
}
}
if (!class_exists($class)) {
return null;
}
try {
$reflection = new \ReflectionClass($class);
if (!$reflection->isInstantiable()) {
return null;
}
$constructor = $reflection->getConstructor();
if ($constructor !== null && $constructor->getNumberOfRequiredParameters() > 0) {
return null;
}
return $reflection->newInstance();
} catch (\Throwable) {
return null;
}
}
}

36
lib/App/Module/ModuleEventDispatcher.php
View File

@@ -4,6 +4,8 @@ namespace MintyPHP\App\Module;
use MintyPHP\App\AppContainer;
use MintyPHP\App\Module\Contracts\EventListener;
use MintyPHP\Http\RequestContext;
use MintyPHP\Service\Audit\SystemAuditService;
/**
* Fire-and-forget event dispatcher for module lifecycle events.
@@ -19,7 +21,8 @@ final class ModuleEventDispatcher
*/
public function __construct(
private readonly array $listenerMap,
private readonly AppContainer $container
private readonly AppContainer $container,
private readonly ?SystemAuditService $systemAuditService = null
) {
}
@@ -31,19 +34,42 @@ final class ModuleEventDispatcher
$listeners = $this->listenerMap[$event] ?? [];
foreach ($listeners as $descriptor) {
$class = '';
$method = '';
try {
$class = $descriptor['class'];
$method = $descriptor['method'];
$listener = $this->container->get($class);
$listener = ModuleClassResolver::resolve($this->container, $class);
if (!$listener instanceof EventListener) {
$listener = new $class();
continue;
}
$listener->{$method}($event, $payload);
} catch (\Throwable) {
// Swallow — one failing listener must not break the chain.
} catch (\Throwable $exception) {
$this->recordListenerFailure($event, $class, $method, $exception);
}
}
}
private function recordListenerFailure(
string $event,
string $listenerClass,
string $listenerMethod,
\Throwable $exception
): void {
if ($this->systemAuditService === null) {
return;
}
$this->systemAuditService->record('module.listener.failed', 'failed', [
'metadata' => [
'module_event' => $event,
'listener_class' => $listenerClass,
'listener_method' => $listenerMethod,
'exception_class' => $exception::class,
'request_id' => RequestContext::id(),
],
]);
}
}

4
lib/App/registerContainer.php
View File

@@ -12,6 +12,7 @@ use MintyPHP\App\Container\Registrars\SettingsRegistrar;
use MintyPHP\App\Container\Registrars\UserRegistrar;
use MintyPHP\App\Module\ModuleEventDispatcher;
use MintyPHP\App\Module\ModuleRegistry;
use MintyPHP\Service\Audit\SystemAuditService;
$container = new AppContainer();
@@ -46,7 +47,8 @@ $moduleRegistry = ModuleRegistry::boot($modulesDir, $enabledModules);
$container->set(ModuleRegistry::class, static fn (): ModuleRegistry => $moduleRegistry);
$container->set(ModuleEventDispatcher::class, static fn () => new ModuleEventDispatcher(
$moduleRegistry->getEventListeners(),
$container
$container,
$container->has(SystemAuditService::class) ? $container->get(SystemAuditService::class) : null
));
// Disallow overriding existing core bindings from module registrars.

26
lib/Service/Access/AccessPolicyFactory.php
View File

@@ -15,12 +15,17 @@ class AccessPolicyFactory
private ?TenantAuthorizationPolicy $tenantAuthorizationPolicy = null;
private ?DepartmentAuthorizationPolicy $departmentAuthorizationPolicy = null;
private ?AuthorizationService $authorizationService = null;
private \Closure $moduleClassResolver;
public function __construct(
private readonly PermissionService $permissionService,
private readonly TenantScopeService $tenantScopeService,
private readonly ?ModuleRegistry $moduleRegistry = null
private readonly ?ModuleRegistry $moduleRegistry = null,
?callable $moduleClassResolver = null
) {
$this->moduleClassResolver = $moduleClassResolver !== null
? \Closure::fromCallable($moduleClassResolver)
: static fn (string $class): ?object => null;
}
public function createUserAuthorizationPolicy(): UserAuthorizationPolicy
@@ -108,26 +113,11 @@ class AccessPolicyFactory
private function instantiateModulePolicy(string $policyClass): ?AuthorizationPolicyInterface
{
if (!class_exists($policyClass)) {
return null;
}
try {
$reflection = new \ReflectionClass($policyClass);
$constructor = $reflection->getConstructor();
if ($constructor === null || $constructor->getNumberOfRequiredParameters() === 0) {
$instance = $reflection->newInstance();
return $instance instanceof AuthorizationPolicyInterface ? $instance : null;
}
if ($constructor->getNumberOfRequiredParameters() === 1) {
$instance = $reflection->newInstance($this->permissionService);
return $instance instanceof AuthorizationPolicyInterface ? $instance : null;
}
$instance = ($this->moduleClassResolver)($policyClass);
return $instance instanceof AuthorizationPolicyInterface ? $instance : null;
} catch (\Throwable) {
return null;
}
return null;
}
}

6
lib/Service/Auth/AuthSessionTenantContextService.php
View File

@@ -4,6 +4,7 @@ namespace MintyPHP\Service\Auth;
use MintyPHP\App\AppContainer;
use MintyPHP\App\Module\Contracts\SessionProvider;
use MintyPHP\App\Module\ModuleClassResolver;
use MintyPHP\App\Module\ModuleRegistry;
use MintyPHP\Http\SessionStoreInterface;
use MintyPHP\Service\User\UserTenantContextService;
@@ -70,10 +71,7 @@ class AuthSessionTenantContextService
return [];
}
foreach ($registry->getSessionProviders() as $providerClass) {
if (!class_exists($providerClass)) {
continue;
}
$provider = new $providerClass();
$provider = ModuleClassResolver::resolve($this->container, $providerClass);
if ($provider instanceof SessionProvider) {
$providers[] = $provider;
}

12
lib/Service/Auth/LdapConnectionGateway.php
View File

@@ -31,8 +31,10 @@ class LdapConnectionGateway
return @ldap_bind($conn, $dn, $password);
}
/** @return \LDAP\Result|false */
public function search(\LDAP\Connection $conn, string $baseDn, string $filter, array $attributes = [], int $scope = 0): \LDAP\Result|false
/**
* @return mixed LDAP\Result on success, false on failure.
*/
public function search(\LDAP\Connection $conn, string $baseDn, string $filter, array $attributes = [], int $scope = 0): mixed
{
if ($scope === 1) {
return @ldap_list($conn, $baseDn, $filter, $attributes);
@@ -40,8 +42,12 @@ class LdapConnectionGateway
return @ldap_search($conn, $baseDn, $filter, $attributes);
}
public function getEntries(\LDAP\Connection $conn, \LDAP\Result $result): array|false
public function getEntries(\LDAP\Connection $conn, mixed $result): array|false
{
if (!$result instanceof \LDAP\Result) {
return false;
}
return ldap_get_entries($conn, $result);
}

37
lib/Support/SearchConfig.php
View File

@@ -20,10 +20,15 @@ class SearchConfig
/** @var (callable(): ModuleRegistry)|null */
private static $moduleRegistryResolver = null;
/** @var (callable(string): ?object)|null */
private static $moduleClassResolver = null;
public static function configure(callable $moduleRegistryResolver): void
public static function configure(callable $moduleRegistryResolver, ?callable $moduleClassResolver = null): void
{
self::$moduleRegistryResolver = $moduleRegistryResolver;
self::$moduleClassResolver = $moduleClassResolver !== null
? \Closure::fromCallable($moduleClassResolver)
: null;
self::resetModuleProviders();
}
@@ -155,10 +160,7 @@ class SearchConfig
return self::$moduleProviders;
}
foreach ($registry->getSearchResources() as $providerClass) {
if (!class_exists($providerClass)) {
continue;
}
$provider = new $providerClass();
$provider = self::resolveModuleProvider($providerClass);
if ($provider instanceof SearchResourceProvider) {
self::$moduleProviders[] = $provider;
foreach ($provider->resources() as $key => $resource) {
@@ -182,6 +184,31 @@ class SearchConfig
self::$moduleProviderByKey = [];
}
private static function resolveModuleProvider(string $providerClass): ?SearchResourceProvider
{
try {
if (is_callable(self::$moduleClassResolver)) {
$resolved = (self::$moduleClassResolver)($providerClass);
return $resolved instanceof SearchResourceProvider ? $resolved : null;
}
if (!class_exists($providerClass)) {
return null;
}
$reflection = new \ReflectionClass($providerClass);
$constructor = $reflection->getConstructor();
if ($constructor !== null && $constructor->getNumberOfRequiredParameters() > 0) {
return null;
}
$instance = $reflection->newInstance();
return $instance instanceof SearchResourceProvider ? $instance : null;
} catch (\Throwable) {
return null;
}
}
public static function hotkeyPreviewResource(string $query): ?array
{
return SearchSpecialResourceProvider::hotkeyPreviewResource($query);

8
lib/Support/helpers/app.php
View File

@@ -60,7 +60,8 @@ function setAppContainer(\MintyPHP\App\AppContainer $container): void
);
\MintyPHP\Support\SearchConfig::configure(
static fn (): \MintyPHP\App\Module\ModuleRegistry => $container->get(\MintyPHP\App\Module\ModuleRegistry::class)
static fn (): \MintyPHP\App\Module\ModuleRegistry => $container->get(\MintyPHP\App\Module\ModuleRegistry::class),
static fn (string $class): ?object => \MintyPHP\App\Module\ModuleClassResolver::resolve($container, $class)
);
}
@@ -691,10 +692,7 @@ function appBuildLayoutNavContext(array $layoutAuth, array $session, array $quer
}
foreach ($moduleRegistry->getLayoutContextProviders() as $providerClass) {
if (!class_exists($providerClass)) {
continue;
}
$provider = new $providerClass();
$provider = \MintyPHP\App\Module\ModuleClassResolver::resolve($container, $providerClass);
if ($provider instanceof \MintyPHP\App\Module\Contracts\LayoutContextProvider) {
$providerData = $provider->provide($session, $container);
if (!is_array($providerData)) {

104
tests/Service/Access/AccessPolicyFactoryModuleResolverTest.php Normal file
View File

@@ -0,0 +1,104 @@
<?php
namespace MintyPHP\Tests\Service\Access;
use MintyPHP\App\AppContainer;
use MintyPHP\App\Module\ModuleClassResolver;
use MintyPHP\App\Module\ModuleRegistry;
use MintyPHP\Service\Access\AccessPolicyFactory;
use MintyPHP\Service\Access\AuthorizationDecision;
use MintyPHP\Service\Access\AuthorizationPolicyInterface;
use MintyPHP\Service\Access\PermissionService;
use MintyPHP\Service\Tenant\TenantScopeService;
use PHPUnit\Framework\TestCase;
final class AccessPolicyFactoryModuleResolverTest extends TestCase
{
public function testModulePolicyWithMultipleDependenciesIsResolvedViaContainer(): void
{
$fixturesDir = sys_get_temp_dir() . '/access-policy-factory-' . uniqid('', true);
$moduleDir = $fixturesDir . '/mod-policy';
mkdir($moduleDir, 0777, true);
file_put_contents(
$moduleDir . '/module.php',
'<?php return ' . var_export([
'id' => 'mod-policy',
'authorization_policies' => [TestMultiDependencyPolicy::class],
], true) . ';'
);
try {
$registry = ModuleRegistry::boot($fixturesDir, ['mod-policy']);
$permissionService = $this->createMock(PermissionService::class);
$tenantScopeService = $this->createMock(TenantScopeService::class);
$dependency = new TestPolicyDependency();
$container = new AppContainer();
$container->set(
TestMultiDependencyPolicy::class,
static fn () => new TestMultiDependencyPolicy($permissionService, $dependency)
);
$factory = new AccessPolicyFactory(
$permissionService,
$tenantScopeService,
$registry,
static fn (string $class): ?object => ModuleClassResolver::resolve($container, $class)
);
$authorizationService = $factory->createAuthorizationService();
$decision = $authorizationService->authorize(TestMultiDependencyPolicy::ABILITY, [
'actor_user_id' => 7,
]);
self::assertTrue($decision->isAllowed());
self::assertSame('container', $decision->attribute('resolver'));
} finally {
@unlink($moduleDir . '/module.php');
@rmdir($moduleDir);
@rmdir($fixturesDir);
}
}
}
final class TestPolicyDependency
{
public function value(): string
{
return 'container';
}
}
final class TestMultiDependencyPolicy implements AuthorizationPolicyInterface
{
public const ABILITY = 'module.multi.dep';
public function __construct(
private readonly PermissionService $permissionService,
private readonly TestPolicyDependency $dependency
) {
}
public function supports(string $ability): bool
{
return $ability === self::ABILITY;
}
public function authorize(string $ability, array $context = []): AuthorizationDecision
{
if ($ability !== self::ABILITY) {
return AuthorizationDecision::deny(500, 'authorization_ability_not_supported');
}
if ((int) ($context['actor_user_id'] ?? 0) <= 0) {
return AuthorizationDecision::deny(403, 'forbidden');
}
return AuthorizationDecision::allow([
'resolver' => $this->dependency->value(),
'permission_service' => $this->permissionService::class,
]);
}
}

42
tests/Service/Auth/LdapAuthServiceTest.php
View File

@@ -299,43 +299,11 @@ class LdapAuthServiceTest extends TestCase
return $conn;
}
private function createMockSearchResult(): \LDAP\Result
private function createMockSearchResult(): object
{
// LDAP\Result can't be mocked directly, but since our gateway wraps
// all ldap_* calls, the actual result object is opaque to LdapAuthService.
// The gateway mock returns it, and we just need a type-compatible value.
// Use a real connection + search if ldap is available; otherwise skip.
if (!extension_loaded('ldap')) {
$this->markTestSkipped('LDAP extension not available');
}
// Create a real but unused Result object via reflection workaround.
// Since we mock getEntries(), the actual result content doesn't matter.
// But LDAP\Result has no public constructor, so we use a trick:
// We accept that the gateway returns LDAP\Result|false and our mocks
// already handle this. For this test helper, return a mock-compatible value.
// Since LdapConnectionGateway::search returns \LDAP\Result|false,
// and we mock the gateway, we can use createMock on a stdClass and
// rely on the mock returning it. But PHPUnit mock of LdapConnectionGateway
// allows any return value when configured via willReturn().
// So we just need any object - the gateway mock will return it to the service,
// and the service passes it back to gateway->getEntries().
$conn = ldap_connect('ldap://127.0.0.1:1');
if ($conn === false) {
$this->markTestSkipped('Could not create LDAP connection');
}
// We can't get a real LDAP\Result without a real server, but since
// the service only passes it to the mocked gateway, we can create
// a test double. PHPUnit allows returning any value from willReturn().
// Let's use a simple approach: the gateway mock already handles this.
// For the type system, we use an anonymous class that extends nothing
// but the mock system handles the actual passing.
// Actually, since we mock LdapConnectionGateway::search() to return
// this value, and LdapConnectionGateway::getEntries() is also mocked,
// the value just needs to be truthy (not false).
// We can use createStub to get a compatible value.
return $this->createStub(\LDAP\Result::class);
// The service treats the search handle as opaque and forwards it to
// gateway->getEntries(); in tests the gateway is mocked, so any object
// is sufficient and avoids doubling final internal LDAP classes.
return new \stdClass();
}
}

64
tests/Unit/Module/ModuleEventDispatcherTest.php
View File

@@ -5,6 +5,8 @@ namespace MintyPHP\Tests\Unit\Module;
use MintyPHP\App\AppContainer;
use MintyPHP\App\Module\Contracts\EventListener;
use MintyPHP\App\Module\ModuleEventDispatcher;
use MintyPHP\Http\RequestContext;
use MintyPHP\Service\Audit\SystemAuditService;
use PHPUnit\Framework\TestCase;
final class ModuleEventDispatcherTest extends TestCase
@@ -62,6 +64,8 @@ final class ModuleEventDispatcherTest extends TestCase
public function testDispatchContinuesAfterListenerException(): void
{
RequestContext::resetForTests();
$secondCalled = false;
$failingListener = new class () implements EventListener {
@@ -100,4 +104,64 @@ final class ModuleEventDispatcherTest extends TestCase
self::assertTrue($secondCalled, 'Second listener should be called even after the first one throws');
}
public function testDispatchAuditsListenerFailureWithoutPayloadLeakage(): void
{
RequestContext::resetForTests();
$failingListener = new class () implements EventListener {
public function handle(string $event, array $payload): void
{
throw new \RuntimeException('Do not leak this message');
}
};
$container = new AppContainer();
$container->set($failingListener::class, static fn () => $failingListener);
$systemAuditService = $this->createMock(SystemAuditService::class);
$systemAuditService->expects($this->once())
->method('record')
->with(
'module.listener.failed',
'failed',
$this->callback(static function (array $context): bool {
$metadata = is_array($context['metadata'] ?? null) ? $context['metadata'] : [];
$keys = array_keys($metadata);
sort($keys);
if ($keys !== ['exception_class', 'listener_class', 'listener_method', 'module_event', 'request_id']) {
return false;
}
if (($metadata['module_event'] ?? '') !== 'user.created') {
return false;
}
if (($metadata['exception_class'] ?? '') !== \RuntimeException::class) {
return false;
}
if (($metadata['listener_method'] ?? '') !== 'handle') {
return false;
}
if (!is_string($metadata['request_id'] ?? null) || trim((string) $metadata['request_id']) === '') {
return false;
}
return !array_key_exists('payload', $metadata)
&& !array_key_exists('exception_message', $metadata)
&& !array_key_exists('trace', $metadata);
})
);
$dispatcher = new ModuleEventDispatcher(
['user.created' => [['class' => $failingListener::class, 'method' => 'handle']]],
$container,
$systemAuditService
);
$dispatcher->dispatch('user.created', ['secret' => 'must-not-be-logged']);
}
}