From 0704e5df447d0848929eb4702daaaa5d63c0e893 Mon Sep 17 00:00:00 2001 From: Chadin Anuwattanaporn Date: Wed, 23 Oct 2024 21:07:50 +0800 Subject: [PATCH 1/5] chore: add logs from permissions external command execution Signed-off-by: Chadin Anuwattanaporn --- server/events/external_team_allowlist_checker.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/events/external_team_allowlist_checker.go b/server/events/external_team_allowlist_checker.go index 9f3fe419ef..8cbdcca70d 100644 --- a/server/events/external_team_allowlist_checker.go +++ b/server/events/external_team_allowlist_checker.go @@ -26,7 +26,12 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForTeam(ctx models. return false } - return checker.checkOutputResults(out) + outputResults := checker.checkOutputResults(out) + if !outputResults { + ctx.Log.Info("command %q returns %q", cmd, out) + } + + return outputResults } func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx models.TeamAllowlistCheckerContext, teams []string, command string) bool { @@ -36,7 +41,12 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode return false } - return checker.checkOutputResults(out) + outputResults := checker.checkOutputResults(out) + if !outputResults { + ctx.Log.Info("command %q returns %q", cmd, out) + } + + return outputResults } func (checker *ExternalTeamAllowlistChecker) buildCommandString(ctx models.TeamAllowlistCheckerContext, teams []string, command string) string { From 45d47e0c05a620ab72cdc78203751b32f04ea828 Mon Sep 17 00:00:00 2001 From: Chadin Anuwattanaporn Date: Wed, 23 Oct 2024 21:08:35 +0800 Subject: [PATCH 2/5] chore: add logs when user is not allowed to execute the command Signed-off-by: Chadin Anuwattanaporn --- server/events/command_runner.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 0b3c112d3d..46a5e75e13 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -263,6 +263,7 @@ func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user model } ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(ctx, user.Teams, cmdName) if !ok { + ctx.Log.Info("user %q teams %q does not have permissions to execute %q command", user.Username, user.Teams, cmdName) return false, nil } return true, nil From 5d1f93fdacbcefe99b4ff0dd49e62f2db5e40b65 Mon Sep 17 00:00:00 2001 From: Chadin Anuwattanaporn Date: Thu, 24 Oct 2024 11:01:31 +0800 Subject: [PATCH 3/5] chore: update logs to use '%s' Signed-off-by: Chadin Anuwattanaporn --- server/events/command_runner.go | 2 +- server/events/external_team_allowlist_checker.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 46a5e75e13..4001a4cb0c 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -263,7 +263,7 @@ func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user model } ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(ctx, user.Teams, cmdName) if !ok { - ctx.Log.Info("user %q teams %q does not have permissions to execute %q command", user.Username, user.Teams, cmdName) + ctx.Log.Info("user '%s' teams '%s' does not have permissions to execute '%s' command", user.Username, user.Teams, cmdName) return false, nil } return true, nil diff --git a/server/events/external_team_allowlist_checker.go b/server/events/external_team_allowlist_checker.go index 8cbdcca70d..9b81c237ac 100644 --- a/server/events/external_team_allowlist_checker.go +++ b/server/events/external_team_allowlist_checker.go @@ -28,7 +28,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForTeam(ctx models. outputResults := checker.checkOutputResults(out) if !outputResults { - ctx.Log.Info("command %q returns %q", cmd, out) + ctx.Log.Info("command '%s' returns '%s'", cmd, out) } return outputResults @@ -43,7 +43,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode outputResults := checker.checkOutputResults(out) if !outputResults { - ctx.Log.Info("command %q returns %q", cmd, out) + ctx.Log.Info("command '%s' returns '%s'", cmd, out) } return outputResults From c959b14b71862c7a95ad70f98348557f2e5e2b28 Mon Sep 17 00:00:00 2001 From: Chadin Anuwattanaporn Date: Thu, 24 Oct 2024 11:15:15 +0800 Subject: [PATCH 4/5] chore: add logs when permissions external command execution errors out Signed-off-by: Chadin Anuwattanaporn --- server/events/external_team_allowlist_checker.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/events/external_team_allowlist_checker.go b/server/events/external_team_allowlist_checker.go index 9b81c237ac..441e371d8d 100644 --- a/server/events/external_team_allowlist_checker.go +++ b/server/events/external_team_allowlist_checker.go @@ -23,6 +23,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForTeam(ctx models. cmd := checker.buildCommandString(ctx, []string{team}, command) out, err := checker.ExternalTeamAllowlistRunner.Run(ctx, "sh", "-c", cmd) if err != nil { + ctx.Log.Info("command '%s' error '%s'", cmd, err) return false } @@ -38,6 +39,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode cmd := checker.buildCommandString(ctx, teams, command) out, err := checker.ExternalTeamAllowlistRunner.Run(ctx, "sh", "-c", cmd) if err != nil { + ctx.Log.Info("command '%s' error '%s'", cmd, err) return false } From c201f6f2d334b88a6246378389ce5401167f4e38 Mon Sep 17 00:00:00 2001 From: C <92499464+chadinwork@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:35:09 +0800 Subject: [PATCH 5/5] chore: apply suggestions from code review Co-authored-by: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Signed-off-by: C <92499464+chadinwork@users.noreply.github.com> --- server/events/command_runner.go | 2 +- server/events/external_team_allowlist_checker.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 5122020526..59ec3d5099 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -263,7 +263,7 @@ func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user model } ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(ctx, user.Teams, cmdName) if !ok { - ctx.Log.Info("user '%s' teams '%s' does not have permissions to execute '%s' command", user.Username, user.Teams, cmdName) + ctx.Log.Info("User '%s' in team '%s' does not have permissions to execute the '%s' command", user.Username, user.Teams, cmdName) return false, nil } return true, nil diff --git a/server/events/external_team_allowlist_checker.go b/server/events/external_team_allowlist_checker.go index 441e371d8d..491d3a6bc3 100644 --- a/server/events/external_team_allowlist_checker.go +++ b/server/events/external_team_allowlist_checker.go @@ -23,7 +23,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForTeam(ctx models. cmd := checker.buildCommandString(ctx, []string{team}, command) out, err := checker.ExternalTeamAllowlistRunner.Run(ctx, "sh", "-c", cmd) if err != nil { - ctx.Log.Info("command '%s' error '%s'", cmd, err) + ctx.Log.Err("Command '%s' error '%s'", cmd, err) return false } @@ -39,7 +39,7 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode cmd := checker.buildCommandString(ctx, teams, command) out, err := checker.ExternalTeamAllowlistRunner.Run(ctx, "sh", "-c", cmd) if err != nil { - ctx.Log.Info("command '%s' error '%s'", cmd, err) + ctx.Log.Err("Command '%s' error '%s'", cmd, err) return false }