Skip to content

allowExceptIn and allowExceptInMethods silently ignore allowParamsInAllowed and allowExceptParamsInAllowed #406

@spaze

Description

@spaze

Description

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.

For example, this configuration:

parameters:
    disallowedFunctionCalls:
        -
            function: 'redirect()'
            allowExceptInMethods:
                - 'App\Controller::handleRequest()'
            allowParamsInAllowed:
                -
                    position: 1
                    name: 'url'
                    typeString: 'App\SafeUrl'

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:

  • docs/allow-in-methods.md needs a "Combining with parameter conditions" section analogous to what was added to docs/allow-in-instance-of.md in Add param conditions support to allowInInstanceOf and disallowInInstanceOf #401.
  • docs/allow-in-paths.md needs the same treatment.
  • 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.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions