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

Add warning to set SENDMAIL_ARGS to -- #19102

Merged
merged 5 commits into from
Mar 18, 2022

Conversation

zeripath
Copy link
Contributor

Even with #17688 email addresses that contain an initial - may still be present in the db and it may in future still be possible to imagine a situation whereby initial - are repermitted.

This PR simply updates the documentation to warn users to set their SENDMAIL_ARGS with a terminal -- to prevent this possibility email addresses being interpreted as options.

Signed-off-by: Andrew Thornton art27@cantab.net

… that interprets options

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the type/docs This PR mainly updates/creates documentation label Mar 15, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 15, 2022
@silverwind
Copy link
Member

Maybe we could just automatically add -- final argument if it's not there in the config?

Co-authored-by: silverwind <me@silverwind.io>
@zeripath
Copy link
Contributor Author

Maybe we could just automatically add -- final argument if it's not there in the config?

Are you able to go through and check EVERY "sendmail" that exists and assert that -- actually terminates options?

If not we would need another option to not append the -- or perhaps just not put -- if the SENDMAIL_ARGS is not empty.

We'd also likely need another option which is to refuse to send emails via sendmail if the email address starts with a -

@silverwind
Copy link
Member

silverwind commented Mar 16, 2022

Are you able to go through and check EVERY "sendmail" that exists and assert that -- actually terminates options?

Can't say that for sure but -- is a very common mechanism for unix programs to terminate options arguments. Not sure about stuff like Windows implementations thought. I guess it's fine to not try the route of forcing the parameter.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 16, 2022
@@ -666,7 +666,7 @@ Define allowed algorithms and their minimum key length (use -1 to disable a type
- Enabling dummy will ignore all settings except `ENABLED`, `SUBJECT_PREFIX` and `FROM`.
- `SENDMAIL_PATH`: **sendmail**: The location of sendmail on the operating system (can be
command or full path).
- `SENDMAIL_ARGS`: **_empty_**: Specify any extra sendmail arguments.
- `SENDMAIL_ARGS`: **_empty_**: Specify any extra sendmail arguments. (NOTE: you should be aware that email addresses can look like options - if your `sendmail` command takes options you must set the option terminator `--`)
Copy link
Member

@6543 6543 Mar 17, 2022

Choose a reason for hiding this comment

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

can we add -- by default? (go code)

@6543
Copy link
Member

6543 commented Mar 18, 2022

🚀 - merge this first ... we can talk about defaults later

@6543 6543 merged commit fda5b9f into go-gitea:main Mar 18, 2022
@zeripath zeripath deleted the improve-mailer-documentation branch March 18, 2022 18:50
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 19, 2022
* giteaoffical/main:
  remove not needed (go-gitea#19128)
  Add warning to set SENDMAIL_ARGS to --  (go-gitea#19102)
  Do not send activation email if manual confirm is set (go-gitea#19119)
  Update tool dependencies (go-gitea#19120)
  Delete related notifications on issue deletion too (go-gitea#18953)
  nit fix (go-gitea#19116)
  Store the foreign ID of issues during migration  (go-gitea#18446)
  Remove italics for `due_date_not_set` (go-gitea#19113)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
Even with go-gitea#17688 email addresses that contain an initial `-` may still be present in the db and it may in future still be possible to imagine a situation whereby initial `-` are repermitted.

This PR simply updates the documentation to warn users to set their SENDMAIL_ARGS with a terminal `--` to prevent this possibility email addresses being interpreted as options.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Mar 29, 2022
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants