You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Since #401, allowExceptInInstanceOf supports param sub-conditions via allowParamsInAllowed and allowExceptParamsInAllowed. The analogous directives allowExceptIn (paths) and allowExceptInMethods/allowExceptInFunctions do not — they silently ignore any configured param conditions.
silently ignores allowParamsInAllowed. The call is disallowed in handleRequest() regardless of the param value — the param condition has no effect at all. There is no error or warning that the combination is unsupported.
The same applies to allowExceptIn (paths) combined with allowParamsInAllowed or allowExceptParamsInAllowed.
Why this is worth addressing
The silent failure is the core problem. The config looks valid, produces no error, and quietly does nothing with the param condition. Documentation alone ("it's not supported") is a "we told you so" — it only helps someone who read the right section before configuring. It doesn't help someone who already has a broken config.
The combination is also not obviously unnatural. If allowExceptInInstanceOf supports param sub-conditions (which it does, since #401), a user can reasonably expect allowExceptInMethods to behave consistently. "I cannot construct a use case" is not a valid reason to dismiss it — that's a limitation of imagination, not a property of the feature space.
Expected behavior
By analogy with allowExceptInInstanceOf (see #401), param sub-conditions should apply to the disallowed scope — i.e. the method or path listed in allowExceptIn/allowExceptInMethods:
allowParamsInAllowed: when inside the excepted method/path, allow the call if the param matches — otherwise disallow. Outside the excepted scope, always allow.
allowExceptParamsInAllowed: when inside the excepted method/path, disallow the call if the param matches — otherwise allow. Outside the excepted scope, always allow.
So the example config above should mean: "disallow redirect() in handleRequest(), unless the first argument is of type App\SafeUrl."
Implementation notes
Depends on #405 being merged first — the implementation notes describe the code as it will be after that fix.
The fix follows the same pattern as allowExceptInInstanceOf in src/Allowed/Allowed.php. After #405, when a call matches an entry in getAllowExceptInCalls(), the code returns false unconditionally. It should instead return $hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false). The $hasParams && guard is load-bearing: if no params are configured, $hasParams is false and the expression short-circuits to false (disallowed), which is correct. The false passed as $defaultResult to hasAllowedParamsInAllowed means "disallowed by default in this scope, but params can override to allow." The same change applies to the getAllowExceptIn() (paths) block.
The return true path — when the current scope does NOT match any entry in getAllowExceptInCalls() or getAllowExceptIn() — stays unchanged. Outside the excepted scope the call is always allowed, no param check needed there.
Tests need to cover two combinations per directive — allowParamsInAllowed and allowExceptParamsInAllowed — for both methods and paths. The model to follow is the disallowInInstanceOf (i.e. allowExceptInInstanceOf) combinations in tests/src/BarInstanceOfWithParams.php and tests/Calls/FunctionCallsTest.php::testInstanceOfWithParams() (str_ends_with and str_contains cases). Note: allowInMethods + params and allowIn (paths) + params already work and don't need implementing — only the allowExcept* variants are missing.
docs/allow-with-parameters.md line 36 currently reads: "calls will be allowed only when they are in one of the allowIn paths (or in a class hierarchy matched by allowInInstanceOf)". After the fix, allowExceptIn and allowExceptInMethods also support allowParamsInAllowed — but with inverted semantics (param overrides the disallow, not restricts the allow). This line needs updating and the inverted semantics need careful wording.
docs/allow-with-parameters.md line 118 currently reads: "It's also possible to disallow functions and methods previously allowed by path (using allowIn), by function/method name (allowInMethods), or by class hierarchy (allowInInstanceOf) when they're called with specified parameters". After the fix, allowExceptParamsInAllowed also works within allowExceptIn and allowExceptInMethods scopes. This line needs updating too.
Resolution
Either:
Implement param sub-condition support for allowExceptIn and allowExceptInMethods, consistent with how allowExceptInInstanceOf works (implementation notes above), or
Actively reject the combination at config time with a clear error, rather than silently ignoring it — and add tests that lock in the rejection so it cannot silently start working or stop working without notice.
Simply documenting that the combination is unsupported is not sufficient.
Description
Since #401,
allowExceptInInstanceOfsupports param sub-conditions viaallowParamsInAllowedandallowExceptParamsInAllowed. The analogous directivesallowExceptIn(paths) andallowExceptInMethods/allowExceptInFunctionsdo not — they silently ignore any configured param conditions.For example, this configuration:
silently ignores
allowParamsInAllowed. The call is disallowed inhandleRequest()regardless of the param value — the param condition has no effect at all. There is no error or warning that the combination is unsupported.The same applies to
allowExceptIn(paths) combined withallowParamsInAllowedorallowExceptParamsInAllowed.Why this is worth addressing
The silent failure is the core problem. The config looks valid, produces no error, and quietly does nothing with the param condition. Documentation alone ("it's not supported") is a "we told you so" — it only helps someone who read the right section before configuring. It doesn't help someone who already has a broken config.
The combination is also not obviously unnatural. If
allowExceptInInstanceOfsupports param sub-conditions (which it does, since #401), a user can reasonably expectallowExceptInMethodsto behave consistently. "I cannot construct a use case" is not a valid reason to dismiss it — that's a limitation of imagination, not a property of the feature space.Expected behavior
By analogy with
allowExceptInInstanceOf(see #401), param sub-conditions should apply to the disallowed scope — i.e. the method or path listed inallowExceptIn/allowExceptInMethods:allowParamsInAllowed: when inside the excepted method/path, allow the call if the param matches — otherwise disallow. Outside the excepted scope, always allow.allowExceptParamsInAllowed: when inside the excepted method/path, disallow the call if the param matches — otherwise allow. Outside the excepted scope, always allow.So the example config above should mean: "disallow
redirect()inhandleRequest(), unless the first argument is of typeApp\SafeUrl."Implementation notes
Depends on #405 being merged first — the implementation notes describe the code as it will be after that fix.
The fix follows the same pattern as
allowExceptInInstanceOfinsrc/Allowed/Allowed.php. After #405, when a call matches an entry ingetAllowExceptInCalls(), the code returnsfalseunconditionally. It should instead return$hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false). The$hasParams &&guard is load-bearing: if no params are configured,$hasParamsisfalseand the expression short-circuits tofalse(disallowed), which is correct. Thefalsepassed as$defaultResulttohasAllowedParamsInAllowedmeans "disallowed by default in this scope, but params can override to allow." The same change applies to thegetAllowExceptIn()(paths) block.The
return truepath — when the current scope does NOT match any entry ingetAllowExceptInCalls()orgetAllowExceptIn()— stays unchanged. Outside the excepted scope the call is always allowed, no param check needed there.Tests need to cover two combinations per directive —
allowParamsInAllowedandallowExceptParamsInAllowed— for both methods and paths. The model to follow is thedisallowInInstanceOf(i.e.allowExceptInInstanceOf) combinations intests/src/BarInstanceOfWithParams.phpandtests/Calls/FunctionCallsTest.php::testInstanceOfWithParams()(str_ends_withandstr_containscases). Note:allowInMethods+ params andallowIn(paths) + params already work and don't need implementing — only theallowExcept*variants are missing.Docs:
docs/allow-in-methods.mdneeds a "Combining with parameter conditions" section analogous to what was added todocs/allow-in-instance-of.mdin Add param conditions support toallowInInstanceOfanddisallowInInstanceOf#401.docs/allow-in-paths.mdneeds the same treatment.docs/allow-with-parameters.mdline 36 currently reads: "calls will be allowed only when they are in one of theallowInpaths (or in a class hierarchy matched byallowInInstanceOf)". After the fix,allowExceptInandallowExceptInMethodsalso supportallowParamsInAllowed— but with inverted semantics (param overrides the disallow, not restricts the allow). This line needs updating and the inverted semantics need careful wording.docs/allow-with-parameters.mdline 118 currently reads: "It's also possible to disallow functions and methods previously allowed by path (usingallowIn), by function/method name (allowInMethods), or by class hierarchy (allowInInstanceOf) when they're called with specified parameters". After the fix,allowExceptParamsInAllowedalso works withinallowExceptInandallowExceptInMethodsscopes. This line needs updating too.Resolution
Either:
allowExceptInandallowExceptInMethods, consistent with howallowExceptInInstanceOfworks (implementation notes above), orSimply documenting that the combination is unsupported is not sufficient.