revolution icon indicating copy to clipboard operation
revolution copied to clipboard

MODX3 Core Folder Name cannot be renamed

Open sonicpunk opened this issue 3 years ago • 34 comments

Bug report

Summary

When hardening MODX, we always rename our core folder. It seems that when you rename the core folder to something else, autoloading does not work because the 'core' folder name is placed as a static link in /vendor/composer/autoload_psr4.php and /vendor/composer/autoload_static.php

Step to reproduce

You can reproduce this when upgrading a hardened MODX instance that has a renamed core folder from 2.x to 3.x. The core folder in this case is also OUTSIDE of the webroot.

Observed behavior

It gets noticed when going through the MODX setup process. It cannot find the database. xPDO can't connect.

Expected behavior

The setup should be able to readout the name of the core folder from the MODX config and adjust accordingly.

sonicpunk avatar Mar 05 '21 13:03 sonicpunk

@JoshuaLuckers Hey check out this issue!

sonicpunk avatar Mar 05 '21 13:03 sonicpunk

This may also be related...

After changing my core folder back to core, I was able to go through the installation process, but now it seems that still something is not correct.:

(ERROR @ /var/www/html/core/src/Revolution/Sources/modFileMediaSource.php : 30) Konnte die Medienquelle "MODX Components" nicht initialisieren! Impossible to create the root directory "". mkdir(): Invalid path

Could it be that we no longer are able to set the core outside of webroot?

sonicpunk avatar Mar 05 '21 13:03 sonicpunk

It is currently not possible to move the core directory in 3.x. Composer has to have the path to it for proper autoloading and this cannot be a variable path.

opengeek avatar Mar 05 '21 13:03 opengeek

Are you using an absolute basePath in that media source @sonicpunk or something like ../foobar?

Mark-H avatar Mar 05 '21 13:03 Mark-H

Is there not a way to add or rewrite the paths to the core folder during the setup? @opengeek

sonicpunk avatar Mar 05 '21 14:03 sonicpunk

No, there is not. Composer has to have the path before setup is executed.

opengeek avatar Mar 05 '21 14:03 opengeek

Protecting the core using .htaccess or a simple nginx rule is just as secure as the security-by-obscurity of moving the core.

opengeek avatar Mar 05 '21 14:03 opengeek

@Mark-H Yes, Mark to answer your question.. we have media sources that use absolute basepaths.

sonicpunk avatar Mar 05 '21 14:03 sonicpunk

I am not satifsfied with the way this issue is handled here from a communication point of view. While "not satisfied" is more an euphemism. In fact it upsets me inexpressibly and strongly reminds me why people reduced their participation in the project and community a lot over the last years - because its always the same.

  • closing an issue without any further discussion is a no go. It would be fine to say something like "i disagree because of..., and therefore consider this issue as closable", and if others support that, it may be closed.

  • what exactly is the message here? Hey, fine people, we recommended over years to harden your installs (and still do, see the link!), esp. the core to gain more security, and now we simply say "ha, you are fucked, because you chose the secure way and cannot update to the new version now"???

  • is it really true that its not possible to have a renamed/core? Ben already depicted that simply changing those two lines may be enough (to be determined of course). So possible solutions imho may be

    • at least document exactly those changes and that this needs to be done manually when updating to 3.x (and possibly every time again you do a 3.x update) plus the hint that hardening like this is not considered to be needed any more (is that so??)
    • have the setup at the checks before exactly check that, and tell the user then directly "hey, your installation was hardened, and setup cannot continue. please do XY and then run setup again"
    • or/and implement a tiny change in the setup, which offers to automatically change core/src to the configured MODX_CORE_PATH/src wherever needed prior to the stages in the setup where the autoloading is used

wuuti avatar Mar 05 '21 14:03 wuuti

This was already discussed extensively in multiple places. If everyone would participate beyond this two-day rush to do everything all at once, there wouldn't be a problem here. It is not possible currently without creating larger problems. This is because there is no way in composer to provide a variable path for autoloading. Just because the issue is closed does not mean there cannot be more discussion about it. If I lock it, then you can get upset.

opengeek avatar Mar 05 '21 14:03 opengeek

If changing the references to core/src in the two files mentioned by @sonicpunk, wouldn't it be a good temporary solution to have this in the docs as workaround?

JoshuaLuckers avatar Mar 05 '21 15:03 JoshuaLuckers

From where I see it, it is more of a Composer issue than a MODX issue. We could find workarounds, but that would potentially break things elsewhere.

The recommendation to harden the MODX 2.x is from the past when Composer didn't even exist.

zaigham avatar Mar 05 '21 15:03 zaigham

It comes from the composer.json so that's not configurable per project. Editing generated autoload files is also not something to encourage.

I don't think anyone would object to figuring out how to bring back the ability to move the core. As this is also likely to come up more over the coming weeks and months, I'm going to reopen the issue (for now, at least) to make sure people have a place to discuss ideas on how this might be possible again in the future.

This is also definitely something that should be more clearly stated in the upgrade docs. I thought it was already there but don't see it right now, so have logged an issue to make sure that gets added in the right places. If workarounds become available, that would also be good place to put those.

Mark-H avatar Mar 05 '21 15:03 Mark-H

The recommendation to harden the MODX 2.x is from the past when Composer didn't even exist.

Then this part of hardening docs should also be changed for the 3.x version, probably totally removed, because its incompatible with the version then.

wuuti avatar Mar 05 '21 15:03 wuuti

I would really appreciate a solution which mitigates that issue when doing the jump from 2.x to 3.x, automatically would be the best way. I can think of a script, maybe run by the setup preparation, which automatically does "undo" the core hardening, or at least enlist the already known steps to make an installation compatible with 3.x. I don't think it is a final (just one part) solution to have the docs updated. As mark said, once 3 is released, we can expect maaany people having issues during the setup and reporting them here or in slack. If our magic setup wand can mitigate already known issues they might have, or at least reduce the number of people gaining support by the community, we should take the chance. There will be enough currently unknown issues for sure we then need to concentrate on.

wuuti avatar Mar 05 '21 16:03 wuuti

There could certainly be a check in the setup that looks for the core directory in its default location and warns about that. Definitely agree on the docs for 3.0, too @wuuti … I can take a stab at the docs including default NGINX web rules and htaccess suggestions to make those inaccessible from the web.

rthrash avatar Mar 05 '21 16:03 rthrash

There could certainly be a check in the setup that looks for the core directory in its default location and warns about that.

Unfortunately, I don't think that would work. If you are running setup, you have already extracted the new core in its default location.

opengeek avatar Mar 05 '21 17:03 opengeek

At least the core/packages and maybe some parts of the core/cache content could be moved to the new location during the setup. The other parts normally don't have to be touched.

Jako avatar Mar 05 '21 17:03 Jako

Got my PR in for the docs: https://github.com/modxorg/Docs/pull/280 … that took way longer than I thought. LOL

@wuuti would appreciate your thoughts on it. I completely re-organized the page to focus on MODX first since that's what most folks reading that type of page will really be looking for and addressed the 2 vs 3 issues we're currently facing.

I didn't add anything to address what @Jako pointed out but that probably should be added. A quick rsync recipe for core/packages would be useful.

rthrash avatar Mar 06 '21 00:03 rthrash

My thoughts so far:

  • we should create a config check for the 2.x branch, which checks if the installation is hardenend and warns that the upcoming 3.x version won't work with that, showing a link to a documentation how to mitigate that. Imho such config checks for upcoming versions should be available in the future (like the "upgrade advisor" used in elasticsearch). Such an upgrade advisor could also contain more information (incompatible packages, etc), which will surely be found in the near future.
  • we may rethink the whole setup concept imho. @opengeek is right, once the user sees the setup, a lot is already done (except db maybe) So if something goes wrong during the setup, or any of the already implemented checks there fail, the user is left with a "half-installed" and possibly broken installation, because the files have already been "merged", a step which cannot be undone easily without restoring a whole backup. From my thoughts an update setup should work like this:
    • the whole downloaded tarball is extracted to a separate folder (and not "merged" into an existing installation)
    • you start the setup from there
    • the setup can check everything in advance then, before actually something has been changed
    • the setup should create a rollback point (or at least offer it), which is dumping the db and creating a copy of the important files
    • if something goes wrong during the update, the setup should offer to rollback the stuff to the previous restore point (the rollback should also be possible later, e.g. if you see unsolvable problems with your updated installation later und go back to the locked setup then)
    • if something like a hardened installation is detected during setup, which is automatically fixable, the setup could directly offer to do that, or at least give a step by step documentation how to solve the problem
    • if everything is checked fine, the setup then itself copies/merges files to the final destination and starts the db migration

I am aware of the fact this may start a lot of discussion, and the setup things should better continued in a separate issue then. But having a new major version imho is the best time to introduce also conceptual changes in this part of modx, maybe not all at once, but step by step....

wuuti avatar Mar 12 '21 09:03 wuuti

  • we should create a config check for the 2.x branch, which checks if the installation is hardenend and warns that the upcoming 3.x version won't work with that, showing a link to a documentation how to mitigate that. Imho such config checks for upcoming versions should be available in the future (like the "upgrade advisor" used in elasticsearch). Such an upgrade advisor could also contain more information (incompatible packages, etc), which will surely be found in the near future.

I like the idea but I don't think it's a good idea to have it part of the core:

  • If we add the config check in 2.x, users need to update to this particular 2.x release first.
  • Updating the config check with new checks also require a new MODX release.

An extra would be much better because it doesn't require users to update MODX first and we can update it independently from MODX.

JoshuaLuckers avatar Mar 12 '21 10:03 JoshuaLuckers

This issue has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/dont-understand-verbiage-on-3-0-0-beta1-rel-9-nov-2021-release-announcement/4628/7

rthrash avatar Nov 21 '21 07:11 rthrash

Protecting the core using .htaccess or a simple nginx rule is just as secure as the security-by-obscurity of moving the core.

IIRC, this solution breaks some extras. I think Articles is one of them. It's been a while since I've seen the issue, so I could be wrong.

BobRay avatar Nov 21 '21 16:11 BobRay

Then Articles is wrong and in need of a fix, as protecting the core this way has also been valid throughout 2.x.

Mark-H avatar Nov 21 '21 17:11 Mark-H

We used the advanced upgrade from 2.8.x to 3.0.1 from command line with a xml configuration file that moves the core directory and rename other directories without any trouble.

Are we just lucky or is tgere a path here to investigate ? The xml file we use looks like this (change made for security in the names....)

` <database_type>mysql</database_type> <database_server>localhost</database_server> my_database <database_user>my_db_user_name</database_user> <database_password>my_db_password</database_password> <database_connection_charset>utf8mb4</database_connection_charset> <database_charset>utf8mb4</database_charset> <database_collation>utf8mb4_general_ci</database_collation> <table_prefix>prefix_modx_</table_prefix> <https_port>443</https_port> <http_host>localhost</http_host> <cache_disabled>0</cache_disabled>

<!-- Set this to 1 if you are using MODX from Git or extracted it from the full MODX package to the server prior
     to installation. -->
<inplace>0</inplace>

<!-- Set this to 1 if you have manually extracted the core package from the file core/packages/core.transport.zip.
     This will reduce the time it takes for the installation process on systems that do not allow the PHP time_limit
     and Apache script execution time settings to be altered. -->
<unpacked>0</unpacked>

<!-- The language to install MODX for. This will set the default manager language to this. Use IANA codes. -->
<language>fr</language>

<!-- Information for your administrator account -->
<cmsadmin>modx_user</cmsadmin>
<cmspassword>modx_password</cmspassword>
<cmsadminemail>[email protected]</cmsadminemail>

<!-- Paths for your MODX core directory -->
<core_path>/path/to/core/</core_path>

<!-- Paths for the default contexts that are installed. -->
<context_mgr_path>/path/to/webroot/manager_modx/</context_mgr_path>
<context_mgr_url>/manager_modx/</context_mgr_url>
<context_connectors_path>/path/to/webroot/connectors_modx/</context_connectors_path>
<context_connectors_url>/connectors_modx/</context_connectors_url>
<assets_path>/path/to/webroot/assets_modx/</assets_path>
<assets_url>/assets_modx/</assets_url>
<context_web_path>/path/to/webroot/</context_web_path>
<context_web_url>/</context_web_url>

<!-- Whether or not to remove the setup/ directory after installation. -->
<remove_setup_directory>1</remove_setup_directory>
`

AmaZili avatar Aug 25 '22 10:08 AmaZili

What's in your /vendor/composer/autoload_psr4.php and /vendor/composer/autoload_static.php after that install? Does your manager and extras even work?

Mark-H avatar Aug 25 '22 15:08 Mark-H

Hi Mark, Extras including our ste management extra are working just fine as well as the manager. the content of the autoload... file is herunder : `<?php

// autoload_psr4.php @generated by Composer

$vendorDir = dirname(DIR); $baseDir = dirname(dirname($vendorDir));

return array( 'Symfony\Polyfill\Php80\' => array($vendorDir . '/symfony/polyfill-php80'), 'Symfony\Polyfill\Php73\' => array($vendorDir . '/symfony/polyfill-php73'), 'Symfony\Polyfill\Mbstring\' => array($vendorDir . '/symfony/polyfill-mbstring'), 'Symfony\Polyfill\Intl\Normalizer\' => array($vendorDir . '/symfony/polyfill-intl-normalizer'), 'Symfony\Polyfill\Intl\Grapheme\' => array($vendorDir . '/symfony/polyfill-intl-grapheme'), 'Symfony\Polyfill\Ctype\' => array($vendorDir . '/symfony/polyfill-ctype'), 'Symfony\Contracts\Service\' => array($vendorDir . '/symfony/service-contracts'), 'Symfony\Component\String\' => array($vendorDir . '/symfony/string'), 'Symfony\Component\CssSelector\' => array($vendorDir . '/symfony/css-selector'), 'Symfony\Component\Console\' => array($vendorDir . '/symfony/console'), 'SimplePie\' => array($vendorDir . '/simplepie/simplepie/src'), 'Psr\Http\Message\' => array($vendorDir . '/psr/http-message/src', $vendorDir . '/psr/http-factory/src'), 'Psr\Http\Client\' => array($vendorDir . '/psr/http-client/src'), 'Psr\Container\' => array($vendorDir . '/psr/container/src'), 'PHPMailer\PHPMailer\' => array($vendorDir . '/phpmailer/phpmailer/src'), 'MODX\' => array($baseDir . '/core/src'), 'League\MimeTypeDetection\' => array($vendorDir . '/league/mime-type-detection/src'), 'League\Flysystem\AwsS3V3\' => array($vendorDir . '/league/flysystem-aws-s3-v3'), 'League\Flysystem\' => array($vendorDir . '/league/flysystem/src'), 'JmesPath\' => array($vendorDir . '/mtdowling/jmespath.php/src'), 'GuzzleHttp\Psr7\' => array($vendorDir . '/guzzlehttp/psr7/src'), 'GuzzleHttp\Promise\' => array($vendorDir . '/guzzlehttp/promises/src'), 'GuzzleHttp\' => array($vendorDir . '/guzzlehttp/guzzle/src'), 'Aws\' => array($vendorDir . '/aws/aws-sdk-php/src'), ); `

AmaZili avatar Aug 26 '22 12:08 AmaZili

Capture d’écran 2022-08-26 182116

AmaZili avatar Aug 26 '22 13:08 AmaZili

'MODX' => array($baseDir . '/core/src'),

It's still looking into the /core/src path for the MODX namespace.

JoshuaLuckers avatar Aug 28 '22 10:08 JoshuaLuckers

I see that. But my point is that Modx is still working properly anyway, it's composer that may not be working. I am not familiar with composer, but i guess there must be a way to configure this path to go to the right place.

AmaZili avatar Aug 31 '22 07:08 AmaZili