Skip to content

composer: warn when running as root - #12090

Merged
felixfontein merged 6 commits into
ansible-collections:mainfrom
russoz-ansible:2388-composer-warn-root
Sep 13, 2026
Merged

composer: warn when running as root#12090
felixfontein merged 6 commits into
ansible-collections:mainfrom
russoz-ansible:2388-composer-warn-root

Conversation

@russoz

@russoz russoz commented May 23, 2026

Copy link
Copy Markdown
Collaborator
SUMMARY

When the community.general.composer module runs as root, composer itself emits a security warning that is currently silently swallowed. This PR adds a module.warn() call when the effective UID is 0, so operators are made aware of the issue. The COMPOSER_ALLOW_SUPERUSER environment variable is documented in the module notes as the upstream escape hatch.

Fixes #2388

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

composer

ADDITIONAL INFORMATION

N/A


@ansibullbot

Copy link
Copy Markdown
Collaborator

cc @dmtrs
click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module plugins plugin (any type) tests tests unit tests/unit labels May 23, 2026
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-13 Automatically create a backport for the stable-13 branch labels May 23, 2026

@felixfontein felixfontein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right approach. I think it would be better to surface warnings emitted by composer to the user, instead of ignoring them.

In particular, https://getcomposer.org/doc/faqs/how-to-install-untrusted-packages-safely.md#running-composer-inside-docker-podman-containers shows that emitting a warning when root is used is simply wrong in general.

Especially after reading that, I really don't think we should try to second-guess composer, but simply pass its warnings on.

Comment thread plugins/modules/composer.py Outdated
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 30, 2026
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jun 7, 2026
russoz and others added 3 commits September 13, 2026 16:39
Fixes ansible-collections#2388

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The composer_as_root test case still expected the pre-rebase command
format (working-dir as a separate trailing option) instead of the
current --working-dir=<path> token placed before the subcommand.
@russoz
russoz force-pushed the 2388-composer-warn-root branch from d18f41a to 38b8fe3 Compare September 13, 2026 05:04
@russoz

russoz commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased (fixing conflict)

@ansibullbot ansibullbot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci CI is older than 7 days, rerun before merging labels Sep 13, 2026
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Sep 13, 2026
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Sep 13, 2026
…f reimplementing it

Per review feedback, stop independently deciding when running as root
is unsafe (which is wrong in cases like official containers) and
instead surface composer's own warning as an Ansible warning.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@russoz

russoz commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

I'm not sure this is the right approach. I think it would be better to surface warnings emitted by composer to the user, instead of ignoring them.

In particular, https://getcomposer.org/doc/faqs/how-to-install-untrusted-packages-safely.md#running-composer-inside-docker-podman-containers shows that emitting a warning when root is used is simply wrong in general.

Especially after reading that, I really don't think we should try to second-guess composer, but simply pass its warnings on.

Agree. I have rewritten the changes.

@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 13, 2026
…ble-core versions

CI on ansible-core 2.18 failed: module.warn() serializes warnings as
plain strings there, but as structured WarningSummary/Event dicts on
2.19+. Assert via substring match instead of exact equality so the
test passes on both.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 13, 2026
@russoz
russoz requested a review from felixfontein September 13, 2026 06:50
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 13, 2026
@felixfontein
felixfontein merged commit 9bb7aee into ansible-collections:main Sep 13, 2026
126 checks passed
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 13, 2026
@patchback

patchback Bot commented Sep 13, 2026

Copy link
Copy Markdown

Backport to stable-13: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-13/9bb7aee82639bf5f569d8cde375f87223c52ac72/pr-12090

Backported as #12722

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein

Copy link
Copy Markdown
Collaborator

@russoz thanks for implementing this!

felixfontein pushed a commit that referenced this pull request Sep 13, 2026
…as root (#12722)

composer: warn when running as root (#12090)

* feat(composer): warn when running as root

Fixes #2388



* feat(changelog): add fragment for PR 12090



* fix(test_composer): update expected working-dir command format

The composer_as_root test case still expected the pre-rebase command
format (working-dir as a separate trailing option) instead of the
current --working-dir=<path> token placed before the subcommand.

* reformat

* fix(composer): relay composer's own root/super-user warning instead of reimplementing it

Per review feedback, stop independently deciding when running as root
is unsafe (which is wrong in cases like official containers) and
instead surface composer's own warning as an Ansible warning.



* fix(test_composer): make root-warning assertion resilient across ansible-core versions

CI on ansible-core 2.18 failed: module.warn() serializes warnings as
plain strings there, but as structured WarningSummary/Event dicts on
2.19+. Assert via substring match instead of exact equality so the
test passes on both.



---------


(cherry picked from commit 9bb7aee)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-13 Automatically create a backport for the stable-13 branch feature This issue/PR relates to a feature request module module plugins plugin (any type) tests tests unit tests/unit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

composer: Warn users about running composer as root, based on upstream warning

3 participants