Skip to content

Commit

Permalink
Log stdlib error on failed exec.LookPath
Browse files Browse the repository at this point in the history
The error returned by exec.LookPath was never surfaced to the user.
Without that detail, the user can't tell the difference between a
non-existent path and a permissions issue.

Additionally, when ExecuteCommand is an absolute path, we were still
attempting to prepend the CommandWorkingDirectory if the ExecuteCommand
was not found, which made it difficult to know which path the user
intended to execute.

This commit simplifies the logic to avoid multiple attempts with
ExecuteCommand is an absolute path and changes the error message from:

  error locating command: '/path/to/file'

to:

  error in exec: "/path/to/file": stat /path/to/file: no such file or directory
  error in exec: "/path/to/file": permission denied

Fixes adnanh#457
  • Loading branch information
moorereason committed Sep 25, 2020
1 parent c7a8fbc commit dd5fa20
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
14 changes: 7 additions & 7 deletions webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,16 +581,16 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in
var errors []error

// check the command exists
cmdPath, err := exec.LookPath(h.ExecuteCommand)
if err != nil {
// give a last chance, maybe is a relative path
relativeToCwd := filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand)
// check the command exists
cmdPath, err = exec.LookPath(relativeToCwd)
var lookpath string
if filepath.IsAbs(h.ExecuteCommand) || h.CommandWorkingDirectory == "" {
lookpath = h.ExecuteCommand
} else {
lookpath = filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand)
}

cmdPath, err := exec.LookPath(lookpath)
if err != nil {
log.Printf("[%s] error locating command: '%s'", rid, h.ExecuteCommand)
log.Printf("[%s] error in %s", rid, err)

// check if parameters specified in execute-command by mistake
if strings.IndexByte(h.ExecuteCommand, ' ') != -1 {
Expand Down
2 changes: 1 addition & 1 deletion webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00

// Check logs
{"static params should pass", "static-params-ok", nil, "POST", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
{"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error locating command.*use 'pass[-]arguments[-]to[-]command' to specify args`},
{"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error in exec:.*use 'pass[-]arguments[-]to[-]command' to specify args`},
{"unsupported content type error", "github", nil, "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`},
}

Expand Down

0 comments on commit dd5fa20

Please sign in to comment.