From ca7168aa9afe0abafc631d4920f2a101f30ec762 Mon Sep 17 00:00:00 2001 From: sheeki003 <36009418+sheeki03@users.noreply.github.com> Date: Fri, 15 May 2026 14:23:09 +0530 Subject: [PATCH] test(security): satisfy tirith CI gates --- pkg/config/config.go | 18 ++++++---- pkg/tools/tirith.go | 17 ++++++--- pkg/tools/tirith_test.go | 74 +++++++++++++++++++++++++++++++++------- 3 files changed, 84 insertions(+), 25 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 4edfd9fc8..3496a234b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -936,13 +936,17 @@ type TirithConfig struct { } type ExecConfig struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_EXEC_"` - EnableDenyPatterns bool ` json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"` - AllowRemote bool ` json:"allow_remote" env:"PICOCLAW_TOOLS_EXEC_ALLOW_REMOTE"` - CustomDenyPatterns []string ` json:"custom_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"` - CustomAllowPatterns []string ` json:"custom_allow_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"` - TimeoutSeconds int ` json:"timeout_seconds" env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS"` // 0 means use default (60s) - Tirith TirithConfig ` json:"tirith" envPrefix:"PICOCLAW_TOOLS_EXEC_TIRITH_"` + ToolConfig `envPrefix:"PICOCLAW_TOOLS_EXEC_"` + + EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"` + AllowRemote bool `json:"allow_remote" env:"PICOCLAW_TOOLS_EXEC_ALLOW_REMOTE"` + CustomDenyPatterns []string `json:"custom_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"` + CustomAllowPatterns []string `json:"custom_allow_patterns" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_ALLOW_PATTERNS"` + + // 0 means use default (60s) + TimeoutSeconds int `json:"timeout_seconds" env:"PICOCLAW_TOOLS_EXEC_TIMEOUT_SECONDS"` + + Tirith TirithConfig `json:"tirith" envPrefix:"PICOCLAW_TOOLS_EXEC_TIRITH_"` } type SkillsToolsConfig struct { diff --git a/pkg/tools/tirith.go b/pkg/tools/tirith.go index 96534584f..ff5d0980e 100644 --- a/pkg/tools/tirith.go +++ b/pkg/tools/tirith.go @@ -1,4 +1,4 @@ -// Tirith pre-exec security scanning. +// Package tools integrates Tirith pre-exec security scanning. // // Tirith (https://github.com/sheeki03/tirith) is a terminal security tool // that scans commands for content-level threats: homograph/punycode URLs, @@ -238,23 +238,30 @@ func tirithHandleExit(exitCode int, jsonOutput []byte, cfg TirithConfig) string log.Printf("[tirith] warning: %s", tirithSummarize(jsonOutput)) return "" case 3: - log.Printf("[tirith] warning acknowledgement required; allowing in non-interactive mode: %s", tirithSummarize(jsonOutput)) + log.Printf( + "[tirith] warning acknowledgement required; allowing in non-interactive mode: %s", + tirithSummarize(jsonOutput), + ) return "" default: - return tirithScannerFailure(normalizeTirithBin(cfg.BinPath), cfg, fmt.Errorf("unexpected exit code %d", exitCode)) + return tirithScannerFailure( + normalizeTirithBin(cfg.BinPath), + cfg, + fmt.Errorf("unexpected exit code %d", exitCode), + ) } } func tirithScannerFailure(cacheKey string, cfg TirithConfig, err error) string { message := tirithSanitizeText(err.Error()) if cfg.FailOpen { - tirithLogOnce(cacheKey, "scanner-failure", "[tirith] unavailable (fail-open): %s", message) + tirithLogOncef(cacheKey, "scanner-failure", "[tirith] unavailable (fail-open): %s", message) return "" } return fmt.Sprintf("Tirith security scan failed (fail-closed): %s", message) } -func tirithLogOnce(cacheKey, category, format string, args ...any) { +func tirithLogOncef(cacheKey, category, format string, args ...any) { cacheKey = normalizeTirithBin(cacheKey) key := cacheKey + "\x00" + category + "\x00" + fmt.Sprintf(format, args...) diff --git a/pkg/tools/tirith_test.go b/pkg/tools/tirith_test.go index 8524b54c4..5e4cfb1a8 100644 --- a/pkg/tools/tirith_test.go +++ b/pkg/tools/tirith_test.go @@ -133,7 +133,11 @@ func TestTirithGuard_DoesNotCacheFailedResolution(t *testing.T) { require.Empty(t, tirithGuard(context.Background(), "echo first", "", cfg)) - writeExecutableFile(t, bin, fakeTirithScript(`printf '{"findings":[{"severity":"HIGH","title":"later install"}]}'`+"\nexit 1\n")) + writeExecutableFile( + t, + bin, + fakeTirithScript(`printf '{"findings":[{"severity":"HIGH","title":"later install"}]}'`+"\nexit 1\n"), + ) got := tirithGuard(context.Background(), "echo second", "", cfg) require.Contains(t, got, "Command blocked by Tirith") require.Contains(t, got, "later install") @@ -151,7 +155,12 @@ exit 0 `)) t.Setenv("TIRITH_ARGS_FILE", argsFile) - got := tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}) + got := tirithGuard( + context.Background(), + "echo hello", + "", + TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}, + ) require.Empty(t, got) argsData, err := os.ReadFile(argsFile) @@ -201,7 +210,12 @@ exit 0 t.Setenv("GOOGLE_SAFE_BROWSING_API_KEY", "gsb-secret") t.Setenv("ABUSECH_AUTH_KEY", "abusech-secret") - got := tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}) + got := tirithGuard( + context.Background(), + "echo hello", + "", + TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}, + ) require.Empty(t, got) data, err := os.ReadFile(envFile) @@ -235,10 +249,11 @@ func TestTirithGuard_UsesCommandWorkingDirectory(t *testing.T) { pwd > "$TIRITH_PWD_FILE" printf '{"findings":[]}' exit 0 -`)) + `)) t.Setenv("TIRITH_PWD_FILE", pwdFile) - got := tirithGuard(context.Background(), "echo hello", cwd, TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}) + cfg := TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false} + got := tirithGuard(context.Background(), "echo hello", cwd, cfg) require.Empty(t, got) data, err := os.ReadFile(pwdFile) @@ -269,8 +284,12 @@ func TestTirithGuard_ExitCodes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { bin := filepath.Join(t.TempDir(), "tirith") - writeExecutableFile(t, bin, fakeTirithScript(`printf '{"findings":[{"severity":"HIGH","title":"Pipe to shell"}]}'`+"\nexit "+itoa(tt.exitCode)+"\n")) - got := tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: tt.failOpen}) + script := `printf '{"findings":[{"severity":"HIGH","title":"Pipe to shell"}]}'` + + "\nexit " + itoa(tt.exitCode) + "\n" + writeExecutableFile(t, bin, fakeTirithScript(script)) + + cfg := TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: tt.failOpen} + got := tirithGuard(context.Background(), "echo hello", "", cfg) if tt.wantBlock { require.NotEmpty(t, got) require.Contains(t, got, tt.wantText) @@ -287,7 +306,8 @@ func TestTirithGuard_InvalidJSONKeepsExitVerdict(t *testing.T) { bin := filepath.Join(t.TempDir(), "tirith") writeExecutableFile(t, bin, fakeTirithScript("printf 'not json'\nexit 1\n")) - got := tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false}) + cfg := TirithConfig{Enabled: true, BinPath: bin, Timeout: 5, FailOpen: false} + got := tirithGuard(context.Background(), "echo hello", "", cfg) require.Contains(t, got, "Command blocked by Tirith") require.Contains(t, got, "details unavailable") } @@ -298,14 +318,16 @@ func TestTirithGuard_TimeoutUsesFailOpen(t *testing.T) { bin := filepath.Join(t.TempDir(), "tirith") writeExecutableFile(t, bin, fakeTirithScript("sleep 2\nexit 0\n")) - require.Empty(t, tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 1, FailOpen: true})) + cfg := TirithConfig{Enabled: true, BinPath: bin, Timeout: 1, FailOpen: true} + require.Empty(t, tirithGuard(context.Background(), "echo hello", "", cfg)) - got := tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 1, FailOpen: false}) + cfg.FailOpen = false + got := tirithGuard(context.Background(), "echo hello", "", cfg) require.Contains(t, got, "fail-closed") require.Contains(t, got, "timed out") } -func TestTirithGuard_TimeoutKillsProcessTree(t *testing.T) { +func TestRunTirithCommand_CancelKillsProcessTree(t *testing.T) { skipWindowsProcessTest(t) childPIDFile := filepath.Join(t.TempDir(), "child.pid") @@ -318,8 +340,34 @@ wait t.Setenv("TIRITH_CHILD_PID_FILE", childPIDFile) start := time.Now() - require.Empty(t, tirithGuard(context.Background(), "echo hello", "", TirithConfig{Enabled: true, BinPath: bin, Timeout: 1, FailOpen: true})) - require.Less(t, time.Since(start), 5*time.Second) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cmd := exec.Command(bin) + cmd.Env = append(os.Environ(), "TIRITH_CHILD_PID_FILE="+childPIDFile) + + errCh := make(chan error, 1) + go func() { + errCh <- runTirithCommand(ctx, cmd) + }() + + require.Eventually(t, func() bool { + _, err := os.Stat(childPIDFile) + return err == nil + }, 5*time.Second, 100*time.Millisecond) + + cancel() + var runErr error + require.Eventually(t, func() bool { + select { + case runErr = <-errCh: + return true + default: + return false + } + }, 5*time.Second, 100*time.Millisecond) + require.Error(t, runErr) + require.Less(t, time.Since(start), 7*time.Second) data, err := os.ReadFile(childPIDFile) require.NoError(t, err)