Skip to content

Stop setup:config:set from requiring writable pub/media (#38435) - #41154

Open
lbajsarowicz wants to merge 3 commits into
magento:2.4-developfrom
lbajsarowicz:fix/38435-config-set-permissions-check
Open

Stop setup:config:set from requiring writable pub/media (#38435)#41154
lbajsarowicz wants to merge 3 commits into
magento:2.4-developfrom
lbajsarowicz:fix/38435-config-set-permissions-check

Conversation

@lbajsarowicz

Copy link
Copy Markdown
Contributor

Description (*)

bin/magento setup:config:set fails when any file directly inside pub/media is not writable by the CLI user (read-only NFS/EFS mount, media owned by the web server user, a stray chmod 444):

Missing write permissions to the following paths:
/var/www/html/pub/media/logo.png

Root cause: Magento\Setup\Model\ConfigModel::process() runs FilePermissions::getMissingWritablePathsForInstallation() — the full setup:install precondition, which walks app/etc, var, pub/media and (outside production mode) generated and pub/static, one level deep. setup:config:set only ever writes app/etc/env.php / app/etc/config.php, so the state of pub/media is irrelevant to it.

Fix:

  • New FilePermissions::getMissingWritablePathsForDeploymentConfig() — checks app/etc only, with the same semantics as the existing getMissingWritableDirectoriesForDbUpgrade() (directory writable → OK; otherwise its direct entries must be writable).
  • ConfigModel uses it instead of the installation-wide check.
  • setup:install is unchanged: Installer::installDeploymentConfig() still runs checkInstallationFilePermissions() before delegating to ConfigModel.

No public signatures changed; one public method added.

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes Calling setup:config:set results in checking pub/media files permissions #38435

Manual testing scenarios (*)

  1. Install Magento 2.4-develop.
  2. Make one file in pub/media non-writable for the CLI user, e.g. touch pub/media/logo.png && chmod 444 pub/media/logo.png (must be a filesystem that honours permissions for that user).
  3. Run bin/magento setup:config:set --cache-id-prefix=test_ --no-interaction
    • before: exit code 1, Missing write permissions to the following paths: <root>/pub/media/logo.png
    • after: exit code 0, env.php updated.
  4. chmod 555 app/etc && chmod 444 app/etc/*, run the same command — still refused with Missing write permissions to the following paths: <root>/app/etc (the check that actually matters is kept).
  5. bin/magento setup:install ... with a non-writable pub/media still fails the file permissions check as before.

Questions or comments

Unit tests: FilePermissionsTest (writable app/etc → nothing reported and no other directory looked up; read-only app/etc with writable entries → nothing reported) and ConfigModelTest (installation-wide check no longer called). Both fail against 2.4-develop without the fix.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined templates (if applicable)
  • All automated tests passed successfully (all builds are green)

@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

@m2-assistant

m2-assistant Bot commented Aug 26, 2026

Copy link
Copy Markdown

Hi @lbajsarowicz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

setup:config:set ran the full installation permission check
(FilePermissions::getMissingWritablePathsForInstallation()), which walks
app/etc, var, pub/media and - outside production mode - generated and
pub/static. A single non-writable file directly inside pub/media (for
example a read-only NFS mount or media owned by the web server user) aborted
the command with:

    Missing write permissions to the following paths:
    <root>/pub/media/logo.png

setup:config:set writes to app/etc only, so that is the only location whose
permissions are relevant. Add
FilePermissions::getMissingWritablePathsForDeploymentConfig(), which checks
app/etc the same way the database upgrade check does, and use it from
ConfigModel. setup:install keeps the full installation check.
…allation

Static Tests flagged the pre-existing @deprecated tag in the touched file:
the Magento2 coding standard requires an explained motivation and an @see
reference to the replacement.
@lbajsarowicz
lbajsarowicz force-pushed the fix/38435-config-set-permissions-check branch from c0d8d95 to 26f58a3 Compare August 27, 2026 16:56
@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

1 similar comment
@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run WebAPI Tests

1 similar comment
@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run WebAPI Tests

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

Labels

Priority: P3 May be fixed according to the position in the backlog. Progress: pending review

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

Calling setup:config:set results in checking pub/media files permissions

2 participants