Skip to content

Class finder loads files it should not load #241

Open

Description

@enricobono

We were in the process of upgrading our application from graphqlite v6 to v8, and we discovered an issue with v8.

This is a consequence of what was introduces here: thecodingmachine/graphqlite#657.
With this PR, graphqlite will now look for Types not only in the src/ space, but in the vendors as well. Which bwt is totally legit. To reach this goal, the class explorer package was replaced with kcs/class-finder.

Now, we notices that, in the dev environment, kcs/class-finder was looking for all the classes in the vendor/ directory and in the tests/ directory as well. It iterates over all the .php files, looking for classes:

//class-finder/lib/Iterator/Psr0Iterator.php::62

static function (string $path, string $class): void {
    class_exists($class, true);
}

The issue now is that class_exists will include the file, if not already loaded.

In our case, we have a tests/bootstrap.php file which contains plain code, no class declarations.

So kcs/class-finder will do class_exists('tests/bootstrap.php', true), the file will be included and its content executed. So we are basically executing every php file (which does not contain a class) in both vendor/ and tests/ . Which should not be the case. For example, in our case, a simple run of:

bin/console cache:clear --env dev

will execute the tests/bootstrap.php, which has implementation specific for the test env.

And, moreover, it may pose some security issue, given it will execute any code in any plain php file in any vendor/ subfolder.

Moreover, as per the current configuration, kcs/class-finder is called several times, so it requests the lists of files many times in each session, which means our tests/bootstrap.php file is included more than once, which causes other issues and makes the process slower.

Has anybody else experienced similar issues related to this?

Activity

  1. andrew-demb commented on Feb 10, 2025

    @andrew-demb
    Collaborator

    @enricobono see discussion in this issue alekitto/class-finder#24

  2. enricobono commented on Feb 11, 2025

    @enricobono
    CollaboratorAuthor

    Thanks Andrii.
    I would suggest to do the opposite of what AleKitto suggested. He's suggesting to explicitly exclude problematic files. But still, chances are graphqlite-bundle is loading files it should not load.

    I'd propose the opposite:

    • by default graphqlite-bundle searches for types on userland src/. If the developer wants to look for Types in some additional directory, the developer will need to explicitly add those directories. For instance, we can add this info in a config param.

    What do you think?

  3. andrew-demb commented on Feb 11, 2025

    @andrew-demb
    Collaborator

    @enricobono seems like a useful configuration. But I suggest preserving the current behavior by default and making it opt-out (configured to use a path like an src/)

  4. enricobono commented on Feb 11, 2025

    @enricobono
    CollaboratorAuthor

    Make sense, thanks.
    I guess the default behaviour, right know, is to search everywhere.

  5. andrew-demb commented on Feb 11, 2025

    @andrew-demb
    Collaborator

    @enricobono it's not searching everywhere, but in relevant namespaces according to the dependencies (and project itself) - autoload config.

    If package configured autoload like this [1], it leads to "full scan" - so this is architecture limitation of class-finder's method ComposerFinder::inNamespace

    [1]

    "autoload" : {
        "psr-4" : {
          "TheCodingMachine\\GraphQLite\\Bundle\\" : ""
        }
      },
    
  6. github-actions commented on Aug 11, 2025

    @github-actions
  7. github-actions commented on Aug 17, 2025

    @github-actions
  8. homersimpsons commented on Aug 17, 2025

    @homersimpsons
  9. github-actions commented on Feb 14, 2026

    @github-actions
  10. github-actions commented on Aug 14, 2026

    @github-actions

    This issue is stale because it has been open 6 months with no activity. Remove stale label or comment or this will be closed in 5 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions