ci(sec): add more security jobs, improve build, enable race detector (#1849)

Carlos Alexandro Becker and Copilot created

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Change summary

.github/workflows/build.yml            | 28 +++++++-
.github/workflows/security.yml         | 88 ++++++++++++++++++++++++++++
Taskfile.yaml                          | 10 +-
go.mod                                 |  2 
go.sum                                 |  4 
internal/csync/maps_test.go            |  7 +
internal/permission/permission.go      | 13 +++
internal/permission/permission_test.go |  4 
internal/shell/background.go           | 43 +++++++++++--
internal/shell/background_test.go      | 16 ++--
internal/tui/styles/theme.go           | 32 ++++++----
11 files changed, 199 insertions(+), 48 deletions(-)

Detailed changes

.github/workflows/build.yml 🔗

@@ -1,11 +1,27 @@
 name: build
 on: [push, pull_request]
 
+permissions:
+  contents: read
+
+concurrency:
+  group: build-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
-    uses: charmbracelet/meta/.github/workflows/build.yml@main
-    with:
-      go-version: ""
-      go-version-file: ./go.mod
-    secrets:
-      gh_pat: "${{ secrets.PERSONAL_ACCESS_TOKEN }}"
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0
+        with:
+          go-version-file: go.mod
+      - run: go mod tidy
+      - run: git diff --exit-code
+      - run: go build -race ./...
+      - run: go test -race -failfast ./...

.github/workflows/security.yml 🔗

@@ -0,0 +1,88 @@
+name: "security"
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+  schedule:
+    - cron: "0 2 * * *"
+
+permissions:
+  contents: read
+
+concurrency:
+  group: security-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  codeql:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ["go", "actions"]
+    permissions:
+      actions: read
+      contents: read
+      pull-requests: read
+      security-events: write
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: github/codeql-action/init@cf1bb45a277cb3c205638b2cd5c984db1c46a412 # v4.31.7
+        with:
+          languages: ${{ matrix.language }}
+      - uses: github/codeql-action/autobuild@cf1bb45a277cb3c205638b2cd5c984db1c46a412 # v4.31.7
+      - uses: github/codeql-action/analyze@cf1bb45a277cb3c205638b2cd5c984db1c46a412 # v4.31.7
+
+  grype:
+    runs-on: ubuntu-latest
+    permissions:
+      security-events: write
+      actions: read
+      contents: read
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: anchore/scan-action@40a61b52209e9d50e87917c5b901783d546b12d0 # v7.2.1
+        id: scan
+        with:
+          path: "."
+          fail-build: true
+          severity-cutoff: critical
+      - uses: github/codeql-action/upload-sarif@cf1bb45a277cb3c205638b2cd5c984db1c46a412 # v4.31.7
+        with:
+          sarif_file: ${{ steps.scan.outputs.sarif }}
+
+  govulncheck:
+    runs-on: ubuntu-latest
+    permissions:
+      security-events: write
+      contents: read
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: golang/govulncheck-action@b625fbe08f3bccbe446d94fbf87fcc875a4f50ee # v1.0.4
+        with:
+          output-format: sarif
+          output-file: results.sarif
+      - uses: github/codeql-action/upload-sarif@cf1bb45a277cb3c205638b2cd5c984db1c46a412 # v4.31.7
+        with:
+          sarif_file: results.sarif
+
+  dependency-review:
+    runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request'
+    permissions:
+      contents: read
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          persist-credentials: false
+      - uses: actions/dependency-review-action@3c4e3dcb1aa7874d2c16be7d79418e9b7efd6261 # v4.8.2
+        with:
+          fail-on-severity: critical
+          allow-licenses: BSD-2-Clause, BSD-3-Clause, MIT, Apache-2.0, MPL-2.0, ISC

Taskfile.yaml 🔗

@@ -5,6 +5,8 @@ version: "3"
 vars:
   VERSION:
     sh: git describe --long 2>/dev/null || echo ""
+  RACE:
+    sh: test -f race.log && echo "1" || echo ""
 
 env:
   CGO_ENABLED: 0
@@ -37,20 +39,20 @@ tasks:
     vars:
       LDFLAGS: '{{if .VERSION}}-ldflags="-X github.com/charmbracelet/crush/internal/version.Version={{.VERSION}}"{{end}}'
     cmds:
-      - go build {{.LDFLAGS}} .
+      - "go build {{if .RACE}}-race{{end}} {{.LDFLAGS}} ."
     generates:
       - crush
 
   run:
     desc: Run build
     cmds:
-      - go build -o crush .
-      - ./crush {{.CLI_ARGS}}
+      - task: build
+      - "./crush {{.CLI_ARGS}} {{if .RACE}}2>race.log{{end}}"
 
   test:
     desc: Run tests
     cmds:
-      - go test ./... {{.CLI_ARGS}}
+      - go test -race -failfast ./... {{.CLI_ARGS}}
 
   test:record:
     desc: Run tests and record all VCR cassettes again

go.mod 🔗

@@ -29,7 +29,7 @@ require (
 	github.com/charmbracelet/x/exp/golden v0.0.0-20250806222409-83e3a29d542f
 	github.com/charmbracelet/x/exp/ordered v0.1.0
 	github.com/charmbracelet/x/exp/slice v0.0.0-20251201173703-9f73bfd934ff
-	github.com/charmbracelet/x/powernap v0.0.0-20251015113943-25f979b54ad4
+	github.com/charmbracelet/x/powernap v0.0.0-20260113142046-c1fa3de7983b
 	github.com/charmbracelet/x/term v0.2.2
 	github.com/denisbrodbeck/machineid v1.0.1
 	github.com/disintegration/imageorient v0.0.0-20180920195336-8147d86e83ec

go.sum 🔗

@@ -118,8 +118,8 @@ github.com/charmbracelet/x/exp/slice v0.0.0-20251201173703-9f73bfd934ff h1:Uwr+/
 github.com/charmbracelet/x/exp/slice v0.0.0-20251201173703-9f73bfd934ff/go.mod h1:vqEfX6xzqW1pKKZUUiFOKg0OQ7bCh54Q2vR/tserrRA=
 github.com/charmbracelet/x/json v0.2.0 h1:DqB+ZGx2h+Z+1s98HOuOyli+i97wsFQIxP2ZQANTPrQ=
 github.com/charmbracelet/x/json v0.2.0/go.mod h1:opFIflx2YgXgi49xVUu8gEQ21teFAxyMwvOiZhIvWNM=
-github.com/charmbracelet/x/powernap v0.0.0-20251015113943-25f979b54ad4 h1:i/XilBPYK4L1Yo/mc9FPx0SyJzIsN0y4sj1MWq9Sscc=
-github.com/charmbracelet/x/powernap v0.0.0-20251015113943-25f979b54ad4/go.mod h1:cmdl5zlP5mR8TF2Y68UKc7hdGUDiSJ2+4hk0h04Hsx4=
+github.com/charmbracelet/x/powernap v0.0.0-20260113142046-c1fa3de7983b h1:5ye9hzBKH623bMVz5auIuY6K21loCdxpRmFle2O9R/8=
+github.com/charmbracelet/x/powernap v0.0.0-20260113142046-c1fa3de7983b/go.mod h1:cmdl5zlP5mR8TF2Y68UKc7hdGUDiSJ2+4hk0h04Hsx4=
 github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk=
 github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI=
 github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY=

internal/csync/maps_test.go 🔗

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"maps"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"testing/synctest"
 	"time"
@@ -46,12 +47,12 @@ func TestNewLazyMap(t *testing.T) {
 
 		waiter := sync.Mutex{}
 		waiter.Lock()
-		loadCalled := false
+		var loadCalled atomic.Bool
 
 		loadFunc := func() map[string]int {
 			waiter.Lock()
 			defer waiter.Unlock()
-			loadCalled = true
+			loadCalled.Store(true)
 			return map[string]int{
 				"key1": 1,
 				"key2": 2,
@@ -63,7 +64,7 @@ func TestNewLazyMap(t *testing.T) {
 
 		waiter.Unlock() // Allow the load function to proceed
 		time.Sleep(100 * time.Millisecond)
-		require.True(t, loadCalled)
+		require.True(t, loadCalled.Load())
 		require.Equal(t, 2, m.Len())
 
 		value, ok := m.Get("key1")

internal/permission/permission.go 🔗

@@ -68,8 +68,9 @@ type permissionService struct {
 	allowedTools          []string
 
 	// used to make sure we only process one request at a time
-	requestMu     sync.Mutex
-	activeRequest *PermissionRequest
+	requestMu       sync.Mutex
+	activeRequest   *PermissionRequest
+	activeRequestMu sync.Mutex
 }
 
 func (s *permissionService) GrantPersistent(permission PermissionRequest) {
@@ -86,9 +87,11 @@ func (s *permissionService) GrantPersistent(permission PermissionRequest) {
 	s.sessionPermissions = append(s.sessionPermissions, permission)
 	s.sessionPermissionsMu.Unlock()
 
+	s.activeRequestMu.Lock()
 	if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
 		s.activeRequest = nil
 	}
+	s.activeRequestMu.Unlock()
 }
 
 func (s *permissionService) Grant(permission PermissionRequest) {
@@ -101,9 +104,11 @@ func (s *permissionService) Grant(permission PermissionRequest) {
 		respCh <- true
 	}
 
+	s.activeRequestMu.Lock()
 	if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
 		s.activeRequest = nil
 	}
+	s.activeRequestMu.Unlock()
 }
 
 func (s *permissionService) Deny(permission PermissionRequest) {
@@ -117,9 +122,11 @@ func (s *permissionService) Deny(permission PermissionRequest) {
 		respCh <- false
 	}
 
+	s.activeRequestMu.Lock()
 	if s.activeRequest != nil && s.activeRequest.ID == permission.ID {
 		s.activeRequest = nil
 	}
+	s.activeRequestMu.Unlock()
 }
 
 func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) {
@@ -190,7 +197,9 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe
 	}
 	s.sessionPermissionsMu.RUnlock()
 
+	s.activeRequestMu.Lock()
 	s.activeRequest = &permission
+	s.activeRequestMu.Unlock()
 
 	respCh := make(chan bool, 1)
 	s.pendingRequests.Set(permission.ID, respCh)

internal/permission/permission_test.go 🔗

@@ -189,7 +189,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
 		events := service.Subscribe(t.Context())
 
 		var wg sync.WaitGroup
-		results := make([]bool, 0)
+		results := make([]bool, 3)
 
 		requests := []CreatePermissionRequest{
 			{
@@ -220,7 +220,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) {
 			go func(index int, request CreatePermissionRequest) {
 				defer wg.Done()
 				result, _ := service.Request(t.Context(), request)
-				results = append(results, result)
+				results[index] = result
 			}(i, req)
 		}
 

internal/shell/background.go 🔗

@@ -19,6 +19,30 @@ const (
 	CompletedJobRetentionMinutes = 8 * 60
 )
 
+// syncBuffer is a thread-safe wrapper around bytes.Buffer.
+type syncBuffer struct {
+	buf bytes.Buffer
+	mu  sync.RWMutex
+}
+
+func (sb *syncBuffer) Write(p []byte) (n int, err error) {
+	sb.mu.Lock()
+	defer sb.mu.Unlock()
+	return sb.buf.Write(p)
+}
+
+func (sb *syncBuffer) WriteString(s string) (n int, err error) {
+	sb.mu.Lock()
+	defer sb.mu.Unlock()
+	return sb.buf.WriteString(s)
+}
+
+func (sb *syncBuffer) String() string {
+	sb.mu.RLock()
+	defer sb.mu.RUnlock()
+	return sb.buf.String()
+}
+
 // BackgroundShell represents a shell running in the background.
 type BackgroundShell struct {
 	ID          string
@@ -28,8 +52,8 @@ type BackgroundShell struct {
 	WorkingDir  string
 	ctx         context.Context
 	cancel      context.CancelFunc
-	stdout      *bytes.Buffer
-	stderr      *bytes.Buffer
+	stdout      *syncBuffer
+	stderr      *syncBuffer
 	done        chan struct{}
 	exitErr     error
 	completedAt int64 // Unix timestamp when job completed (0 if still running)
@@ -46,12 +70,17 @@ var (
 	idCounter             atomic.Uint64
 )
 
+// newBackgroundShellManager creates a new BackgroundShellManager instance.
+func newBackgroundShellManager() *BackgroundShellManager {
+	return &BackgroundShellManager{
+		shells: csync.NewMap[string, *BackgroundShell](),
+	}
+}
+
 // GetBackgroundShellManager returns the singleton background shell manager.
 func GetBackgroundShellManager() *BackgroundShellManager {
 	backgroundManagerOnce.Do(func() {
-		backgroundManager = &BackgroundShellManager{
-			shells: csync.NewMap[string, *BackgroundShell](),
-		}
+		backgroundManager = newBackgroundShellManager()
 	})
 	return backgroundManager
 }
@@ -80,8 +109,8 @@ func (m *BackgroundShellManager) Start(ctx context.Context, workingDir string, b
 		Shell:       shell,
 		ctx:         shellCtx,
 		cancel:      cancel,
-		stdout:      &bytes.Buffer{},
-		stderr:      &bytes.Buffer{},
+		stdout:      &syncBuffer{},
+		stderr:      &syncBuffer{},
 		done:        make(chan struct{}),
 	}
 

internal/shell/background_test.go 🔗

@@ -14,7 +14,7 @@ func TestBackgroundShellManager_Start(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	bgShell, err := manager.Start(ctx, workingDir, nil, "echo 'hello world'", "")
 	if err != nil {
@@ -51,7 +51,7 @@ func TestBackgroundShellManager_Get(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	bgShell, err := manager.Start(ctx, workingDir, nil, "echo 'test'", "")
 	if err != nil {
@@ -77,7 +77,7 @@ func TestBackgroundShellManager_Kill(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	// Start a long-running command
 	bgShell, err := manager.Start(ctx, workingDir, nil, "sleep 10", "")
@@ -106,7 +106,7 @@ func TestBackgroundShellManager_Kill(t *testing.T) {
 func TestBackgroundShellManager_KillNonExistent(t *testing.T) {
 	t.Parallel()
 
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	err := manager.Kill("non-existent-id")
 	if err == nil {
@@ -119,7 +119,7 @@ func TestBackgroundShell_IsDone(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	bgShell, err := manager.Start(ctx, workingDir, nil, "echo 'quick'", "")
 	if err != nil {
@@ -142,7 +142,7 @@ func TestBackgroundShell_WithBlockFuncs(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	blockFuncs := []BlockFunc{
 		CommandsBlocker([]string{"curl", "wget"}),
@@ -180,7 +180,7 @@ func TestBackgroundShellManager_List(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	// Start two shells
 	bgShell1, err := manager.Start(ctx, workingDir, nil, "sleep 1", "")
@@ -224,7 +224,7 @@ func TestBackgroundShellManager_KillAll(t *testing.T) {
 
 	ctx := context.Background()
 	workingDir := t.TempDir()
-	manager := GetBackgroundShellManager()
+	manager := newBackgroundShellManager()
 
 	// Start multiple long-running shells
 	shell1, err := manager.Start(ctx, workingDir, nil, "sleep 10", "")

internal/tui/styles/theme.go 🔗

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"image/color"
 	"strings"
+	"sync"
 
 	"charm.land/bubbles/v2/filepicker"
 	"charm.land/bubbles/v2/help"
@@ -97,7 +98,8 @@ type Theme struct {
 	AuthBorderUnselected lipgloss.Style
 	AuthTextUnselected   lipgloss.Style
 
-	styles *Styles
+	styles     *Styles
+	stylesOnce sync.Once
 }
 
 type Styles struct {
@@ -134,9 +136,9 @@ type Styles struct {
 }
 
 func (t *Theme) S() *Styles {
-	if t.styles == nil {
+	t.stylesOnce.Do(func() {
 		t.styles = t.buildStyles()
-	}
+	})
 	return t.styles
 }
 
@@ -500,27 +502,31 @@ type Manager struct {
 	current *Theme
 }
 
-var defaultManager *Manager
+var (
+	defaultManager     *Manager
+	defaultManagerOnce sync.Once
+)
+
+func initDefaultManager() *Manager {
+	defaultManagerOnce.Do(func() {
+		defaultManager = newManager()
+	})
+	return defaultManager
+}
 
 func SetDefaultManager(m *Manager) {
 	defaultManager = m
 }
 
 func DefaultManager() *Manager {
-	if defaultManager == nil {
-		defaultManager = NewManager()
-	}
-	return defaultManager
+	return initDefaultManager()
 }
 
 func CurrentTheme() *Theme {
-	if defaultManager == nil {
-		defaultManager = NewManager()
-	}
-	return defaultManager.Current()
+	return initDefaultManager().Current()
 }
 
-func NewManager() *Manager {
+func newManager() *Manager {
 	m := &Manager{
 		themes: make(map[string]*Theme),
 	}