Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Console] [Completion] Make bash completion run in non interactive mode #47394

Merged
merged 1 commit into from Sep 2, 2022

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Aug 26, 2022

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

Refs composer/composer#11024

Composer does prompt in some contexts, and that blocks/freezes the completion as it runs in an interactive context but is not actually visible. Explicitly setting it to be non interactive seems to be the fix to me.

TODO:

  • A similar fix probably should be applied to other completion types supported in newer versions than 5.4

@Seldaek Seldaek requested a review from chalasr as a code owner August 26, 2022 13:53
@carsonbot carsonbot added this to the 5.4 milestone Aug 26, 2022
@carsonbot carsonbot changed the title [Console][Completion] Make bash completion run in non interactive mode [Console] [Completion] Make bash completion run in non interactive mode Aug 26, 2022
@stof
Copy link
Member

stof commented Aug 26, 2022

this should bump the completion version, to make users regenerate their completion script.

@Seldaek
Copy link
Member Author

Seldaek commented Aug 26, 2022

I'm not sure, it's not really an API change, and the last bugfixes did not bump the version either?

@PowerKiKi
Copy link

I might be wrong but if we run non-interactively would this not potentially run untrusted code from plugins as root during completion ? would this not open up a security hole ?

Or is there no plugin possible during autocompletion workflow, now and in the foreseeable future ?

@stof
Copy link
Member

stof commented Aug 27, 2022

Which plugins are you talking about ?

@PowerKiKi
Copy link

Sorry I was not very clear. I am coming from composer/composer#11024, so I was referring to composer plugins.

I am not aware of whether composer plugins run or not during bash completion. But I know that the fact that composer warns against running as root is, partly, because plugins are third party code that might/should be considered dangerous.

So I was wondering if this patch, which will allow running bash completion as root, might indirectly allow untrusted plugin code to also be run as root without warnings?

I realize this is specific to composer, and this might be up to composer to implement something to prevent that, if necessary. But I thought it might be interesting to consider before this merging third PR nonetheless.

@Seldaek
Copy link
Member Author

Seldaek commented Aug 28, 2022

Yes it is a valid point however indeed something to handle on the composer end. Right now plug-ins are loaded during completion. I thought of disabling them for perf reason initially but then figured this might also cause things to break depending on the plug-in. Disabling them for root probably makes sense tho, unless COMPOSER_ALLOW_ROOT (or whatever the env was called) is set.

@fabpot
Copy link
Member

fabpot commented Sep 2, 2022

Thank you @Seldaek.

@Seldaek
Copy link
Member Author

Seldaek commented Sep 2, 2022

Great - added #47463 and #47464 to do the same on the other relevant files/branches.

fabpot added a commit that referenced this pull request Sep 2, 2022
…ractive mode (Seldaek)

This PR was merged into the 6.2 branch.

Discussion
----------

[Console] [Completion] Make zsh completion run in non interactive mode

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Port #47394 to 6.2 for zsh

Commits
-------

4e5add4 [Console] [Completion] Make zsh completion run in non interactive mode
fabpot added a commit that referenced this pull request Sep 2, 2022
…eractive mode (Seldaek)

This PR was merged into the 6.1 branch.

Discussion
----------

[Console] [Completion] Make fish completion run in non interactive mode

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Port #47394 to 6.1 for fish

Commits
-------

25ab956 [Console] [Completion] Make fish completion run in non interactive mode
This was referenced Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants