#7320 - Introduce helpcenter role

modules/adblockplus/templates/web/hooks_wrapper.sh.erb

Index: modules/adblockplus/templates/web/hooks_wrapper.sh.erb
===================================================================
new file mode 100644
--- /dev/null
+++ b/modules/adblockplus/templates/web/hooks_wrapper.sh.erb
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+# This script is a wrapper of accesible commands via ssh ForceCommand option

[mathias, 2018/04/05 01:12:31]

This should mention that it is to ensure only the defined hooks are actually
accessible. It should also mention where it comes from (`# ... generated in
Puppet: <%= @title %>`). And the description should be followed by an empty
line, not preceded.

[f.lopez, 2018/04/10 02:45:40]

Acknowledged.

+set -- $SSH_ORIGINAL_COMMAND

[mathias, 2018/04/05 01:12:30]

Instead of replacing "$@" here you should pass on $SSH_ORIGINAL_COMMAND to the
script when it is invoked by / configured with ForceCommand in static.pp -- or
instead of replacing  the scripts stack you should make sure it has the correct
one when it is invoked -- or
why the heck does the script need to know it is executed from SSH?

[f.lopez, 2018/04/10 02:45:40]

Acknowledged.

+command="$1"; shift
+case "$command" in
+    <%- @hooks.each do |command, values| -%>

[mathias, 2018/04/05 01:12:31]

I prefer when the template brackets (`<%`) are used in the beginning of the
line, not the whitespace getting deleted using the minus modifier. This allows
for indentation on the programmatic level, i.e.:

<% @a.each do |b| -%>
<%   b.each.do |fooobar| -%>
   ....
<%   end -%>
<% end -%>

It also makes identifying template logic lines more easy.

[f.lopez, 2018/04/10 02:45:40]

Acknowledged.

+    <%= command %>)
+        /home/<%= @deploy_user %>/bin/<%= command %> "$@"

[mathias, 2018/04/05 01:12:31]

This should not repeat the convention from hook.pp (creating a 2nd point of
failure). Either use a shared variable in both places or
`getparam(File["...#$command"], 'path')`. And quotes around the stuff are
probably useful as well, a path can contain white-space after all.

[f.lopez, 2018/04/10 02:45:39]

Acknowledged.

+        ;;
+    <%- end -%>
+    *)
+        echo "Sorry. Only these commands are available to you:"

[mathias, 2018/04/05 01:12:31]

Please don't use personal articulation, stuff like "sorry" and "you". Stick to
the facts, i.e. carp about an "unrecognized command" and offer to try a "help"
command. Speaking of which, that is also somehow required because otherwise you
force the user to trigger an error just to get an overview.

[f.lopez, 2018/04/10 02:45:40]

Acknowledged.

+        <%- @hooks.keys.select { |key| key.to_s}.each do |value| -%>

[mathias, 2018/04/05 01:12:30]

Aren't the key's strings anyway? And even if not, aren't they printable?

[f.lopez, 2018/04/10 02:45:40]

Acknowledged.

+        echo <%= value %>
+        <%- end -%>
+        exit 1
+        ;;
+esac