Notify syscall 180709#457
Notify syscall 180709#457craigphicks wants to merge 7 commits intoisso-comments:masterfrom craigphicks:notify-syscall-180709
Conversation
| parser.read(user) | ||
|
|
||
| for item in setify(parser).difference(a): | ||
| if item[0]=="syscall": |
| @@ -0,0 +1,114 @@ | |||
|
|
|||
There was a problem hiding this comment.
Could you add this in the docs folder and link it so it appears on the website when @posativ will have time to update stuffs.
| if comment["email"]: | ||
| author += " <%s>" % comment["email"] | ||
|
|
||
| rv.write(author + " wrote:\n") |
There was a problem hiding this comment.
Use string formatting instead of concatenation. (it applies for all the function below :) )
Also calling only once rv.write for subsequent calls would be better.
|
|
||
| def _delete_comment(self, id): | ||
| logger.info('comment %i deleted', id) | ||
| logger.info("comments.delete %i ", id) |
There was a problem hiding this comment.
Remove the trailing space in the string
|
|
||
| def _activate_comment(self, id): | ||
| logger.info("comment %s activated" % id) | ||
| logger.info("comments.activate %s" % id) |
There was a problem hiding this comment.
Do not use string formatting here, let logging do it. It will compute strings only if needed according to the log level.
There was a problem hiding this comment.
I saw there's more above of this too to correct
| msgbody=_format(thread,comment,self.general_host,key) | ||
| subject = "..." + thread['uri'][-15:] + " :: " + comment['text'][:15] + "..." | ||
| cmdlist = [ a.replace('{{SUBJECT}}',subject,1) for a in self.conf.getiter('call') ] | ||
| logger.info(cmdlist); |
| key=self.isso.sign(comment["id"]) | ||
| msgbody=_format(thread,comment,self.general_host,key) | ||
| subject = "..." + thread['uri'][-15:] + " :: " + comment['text'][:15] + "..." | ||
| cmdlist = [ a.replace('{{SUBJECT}}',subject,1) for a in self.conf.getiter('call') ] |
There was a problem hiding this comment.
No space after opening bracket.
No space before opening bracket.
Spaces after comma.
| subject = "..." + thread['uri'][-15:] + " :: " + comment['text'][:15] + "..." | ||
| cmdlist = [ a.replace('{{SUBJECT}}',subject,1) for a in self.conf.getiter('call') ] | ||
| logger.info(cmdlist); | ||
| p=subprocess.run(cmdlist, |
There was a problem hiding this comment.
spaces around assignement operator
| + "stdout:\n" \ | ||
| + str(p.stdout) + "\n" \ | ||
| + "stderr:\n" \ | ||
| + str(p.stderr) + "\n" |
There was a problem hiding this comment.
Use string formatting. And maybe multiline string using triple quotes?
| input=str(msgbody).encode(), | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE) | ||
| s = "Syscall: return code " + str(p.returncode) + "\n" \ |
There was a problem hiding this comment.
Name it logged_string or something more significant.
|
Ok for now I checked the code and it seems ok. Some minor changes (even though there's a few). Important stuff to check:
I still need to test the PR too :) |
|
Stale and unlikely to ever be merged without a massive rebase. |
|
I should have closed this pull long ago - apologies. There really isn't any great need for the suggested feature - what exists is perfectly satisfactory. |
Send Notifications by arbitrary program via python systemcall
The [isso] repository was forked by github user craigphicks. A branch
notify-syscall-180709was created. The branch is described herein.The main uncertainly I have as author is the use of
subprocess.runwhich appeared in Python3.5. Which versions of Python must be supported? Must Python 2 be supported? (See Limitations at the end of this document).This comment is copied from the file ./README-notify-syscall-180709 in hte main directory.
Motivation
Security
When isso uses the SMTP notification backend, it also sends the username and password. The are various perspectives from which this could be problematic.
Password in clear: In the worst case TLS (Transport Layer Security) is not established before passing the password and so the password is transmitted in the clear.
Key better than password: Even with TLS, the password is still in the clear at either end. While this is usually OK, security can be improved by using a key which can be periodically refreshed. The point of the key is that is that it is only authorizes a narrow range of actions. Especially, it does not allow logging in as mail account owner, which could be very damaging.
Dependence on Python SMTP implementation
Isso is tied to the Python SMTP implementation. Even if SMTP is the desired protocol, the may be other software besides the Python SMTP implementation which gives a better result, e.g. in terms of enabling TLS or working with tokens.
Some examples of alternate software are the multitude of generic sendmail programs, e.g. sendmail, postfix, exim, nullmailer, all of which all share the linux sendmail application interface as a standard.
Also there utilities such as curl and swaks which are very flexible with many options. In case something is not working it is almost always possible to work through it with these programs (or change to another program). Although they are not designed for production use, they are sufficient on the usage scale expected by a typical isso installation.
Solution
Outline
The isso administrator specifies the program and it's arguments in the isso configuration file. When a comment notification event occurs the specified program and arguments are executed via a system call. The message is passed to the standard input of that program.
Administrator interface
Add
syscallto the list of notify backends:Create a section
[syscall]and therein specify the program and its arguments. The following example showscurlbeing used to transmit an email request through a (free) Mailgun account:The key
callappears on the first line followed by equal. The value is split between lines with exactly one argument per line. Each successive value line must have an empty space at the beginning.Often in a shell command line quotation mark must be used to prevent a parser from interpreting spaces as being argument delimiters. E.g., for the subject
a bash script would require quotes placed like
"subject=isso {{SUBJECT}}"orsubject="isso {{SUBJECT}}"to keep the argument together. The shell would interpret the quotes for their syntactic meaning, and then remove them before passing each argument phrase to the program as, e.g. argv[11] or equivalent.However, for the isso configuration
callvalue above, quotes are not required because the grouping is done by lines. In fact quotes for the purpose of syntactic grouping must NOT be used. The quotes will not be removed before passing to the program, and the input will likely be invalid. Quotes can still be used as part of the content of any argument, as required.Some (but not all) scenarios require the mail subject to be specified as part of the arguments, and not in the standard input data. The above
curlexample shows such a case. In order to enable variable subject line content a template parameter%SUBJECT%is provided. The isso software will render this template %SUBJECT% as the fixed formula:The last part of the last
curlargument: `<-' , is a curl specific instruction to accept data from standard input. Because the message body is variable it must be transmitted via standard input to the process. So any program used must be configured (or already be hard coded) to accept standard input.Be aware that protocol issues depend on the called program and the site receiving the data. It is not handled by the isso code. For example, although the above example appears to transact via HTTPS, there is some possibility it might downgrading to HTTP after authentication for the message transaction part. (The log file is still under interpretation.)
Read the current limitations section at the end of this document.
Implementation
The notify-syscall-180708 branch adds a new module
Syscallin theisso/ext/notifications.pyfile.It formats the mail message body in the same way as the existing
SMTPmodule, but without the SMTP header and quopri / base64 encoding.The command and arguments as specified in the configuration are supplied as a list which is the first parameter of
subprocess.run.The message body is encoded to binary and passed as standard input to the invoked process.
When complete,
subprocess.runreturns an object with membersreturncode,stdout, and 'stderr'. All are logged via the global logger (usually to/var/log/isso.log).Isso expects that success is indicated by a
returncodevalue of zero, and anything else is error. However, an error does not result in an isso program error. The only difference in isso program behavior is that for a non-zero result isso will log viaand for a zero result via
Other changes
The notification backend
Stdoutmodule was rewritten to fix a bug caused by trying to log a bloom filter as text. Also, the relation between function names, log content, and events was cleaned up.IMPORTANT: Current Limitations
Implementation
subprocess.runis not available before Python 3.5. Certainly not in Python2. In environments where the code will not run it should not be enabled in the configuration file, (if it even compiles).