%!(MISSING) with include-command-output-in-response (RAW output)

[Htbaa]

I had the idea of using webhook in front of a CLI tool I wrote (also in Golang). My tool can output in plain text, XML and JSON. So instead of adding a server to my tool I thought using webhook would be a neat way of exposing a set of its commands to a HTTP API.

If a command outputs a % character the output a HTTP client gets differs from the output that's logged when you run webhook with -verbose.

Tested with webhook version 2.6.9.

hooks.json

[{
    "id": "foobar",
    "execute-command": "echo",
    "pass-arguments-to-command": [
        {
            "source": "string",
            "name": "%"
        }
    ],
    "include-command-output-in-response": true
}]

The output of webhook -hooks hooks.json -verbose:

[webhook] 2019/04/18 13:18:32 200 | 24.022555ms | localhost:9000 | GET /hooks/foobar
[webhook] 2019/04/18 13:18:38 [a66935] incoming HTTP request from [::1]:46698
[webhook] 2019/04/18 13:18:38 [a66935] foobar got matched
[webhook] 2019/04/18 13:18:38 [a66935] foobar hook triggered successfully
[webhook] 2019/04/18 13:18:38 [a66935] executing echo (/bin/echo) with arguments ["echo" "%"] and environment [] using  as cwd
[webhook] 2019/04/18 13:18:38 [a66935] command output: %

[webhook] 2019/04/18 13:18:38 [a66935] finished handling foobar

And the result of a HTTP client:

$ curl http:/localhost:9000/hooks/foobar
%!
(MISSING)

I would expect the result of the HTTP client be the same as what's being logged, namely %.

[moorereason]

The %!(MISSING) string tells me that someone is using a fmt.Printf style command and sending an untrusted string (containing printf escape codes) as the first argument. I'd have to see your code to know where to start looking, but you should use this approach:

fmt.Sprintf("%s", untrusted)

instead of:

fmt.Sprintf(untrusted)

[Htbaa]

Hi,

I'm aware it's an issue with fmt.Sprintf. I've thoroughly tested my cli tool to see if that's what causes it. But it wasn't. As you can see in the provided hooks.json all I'm doing here is an echo %. webhook uses this output and probably uses fmt.Sprintf(untrusted) itself. There's no extra code.

This is just plain bash echo.

[moorereason]

Thanks for the report and the clarification. The problems are in these lines with fmt.Fprintf. If you're up for submitting a PR, you can do so against the development branch.

[adnanh]

Good catch!

[Htbaa]

Thanks for the report and the clarification. The problems are in these lines with fmt.Fprintf. If you're up for submitting a PR, you can do so against the development branch.

Looks like replacing those calls with fmt.Fprint should do the trick?

[adnanh]

Yep, and anywhere else where the formatting hasn't been used.

[Htbaa]

Will try to submit a PR later today.

[Htbaa]

OK, after having some issues running the code, which was caused by VS Code refactoring my imports based on my Windows Go installation (and I was running webhook in a Linux VM) and getting lots of errors I was able to successfully fix this issue.

christiaan@localtestserver:~$ curl -v http://localhost:9000/hooks/foobar
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 9000 (#0)
> GET /hooks/foobar HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/7.58.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Wed, 24 Apr 2019 13:31:52 GMT
< Content-Length: 2
< Content-Type: text/plain; charset=utf-8
<
%
* Connection #0 to host localhost left intact

I've looked into adding a test to webhook_test.go but not sure how to proceed.

PS: to make testing forks easier, wouldn't it be better to change the import of github.com/adnanh/webhook/hook to ./hook or something? Because currently I have to change it to the name of my fork (probably my lack of understanding Golang though).

[Htbaa]

Is there anything else you'd like me have to do on this issue or should I submit the PR?

[moorereason]

@Htbaa, see this section of the Hugo contributing guide for a quick walk-thru on using git branches and remotes for Go development. That will work around the import path issue.

[Htbaa]

@moorereason thanks, will look into that. I'll submit the PR as well.

If I figure out how to do it :-S.