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.

Sign up for GitHub

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

Jump to bottom

Add warning to set SENDMAIL_ARGS to -- #19102

Merged
merged 5 commits into from
Mar 18, 2022
Merged

Add warning to set SENDMAIL_ARGS to -- #19102

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

@zeripath
Add warning to set SENDMAIL_ARGS to -- if using a version of sendmail…
5d409ca 
… 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?

@zeripath @silverwind
Update docs/content/doc/usage/email-setup.en-us.md
1b4974e 
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 •
edited
Loading

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
6543
6543 reviewed Mar 17, 2022
View reviewed changes
docs/content/doc/advanced/config-cheat-sheet.en-us.md
@@ -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 •
edited
Loading

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
@zjjhot
Merge remote-tracking branch 'giteaoffical/main'
9d7ab3c 
* 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
@zeripath
Add warning to set SENDMAIL_ARGS to -- (go-gitea#19102)
d28bd38 
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.
Reviewers

@6543 6543 6543 left review comments

@techknowlogick techknowlogick techknowlogick approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
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
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
6 participants
@zeripath @silverwind @6543 @techknowlogick @wxiaoguang @GiteaBot