diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3511f7fe0c4f487eb3fc9009795361ada8e2eff7..39b5923298e2f7fa8d5452327a6e8b2a08f0df97 100644 --- a/.github/workflows/build.yml +++ b/.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 ./... diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 0000000000000000000000000000000000000000..8fc56fa39e7b47d1fe5ba84c0f0e7cb65733a264 --- /dev/null +++ b/.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 diff --git a/Taskfile.yaml b/Taskfile.yaml index 68c805c599314cadde5c86fc37a0e3d1a6184f4e..0043f4f033e455a5800da2431848e620c37a0f5a 100644 --- a/Taskfile.yaml +++ b/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 diff --git a/go.mod b/go.mod index 8959596f7feca0d6df1ce50e88712e8cc1058fa9..fe3497de825754c0e60835a079f9d6014d9c603a 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 70582b7c92f86af89a03d9f9a43382e27235d2ca..d3d7696e9729d1a20dc45c7122a847e384bb72df 100644 --- a/go.sum +++ b/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= diff --git a/internal/csync/maps_test.go b/internal/csync/maps_test.go index 4c590f008dad91e8dcbc40d1b90d87ef1b3e5750..31e6fa0c3aef18a04c61ea3d4d36b5187228c3ff 100644 --- a/internal/csync/maps_test.go +++ b/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") diff --git a/internal/permission/permission.go b/internal/permission/permission.go index 9dc85e976238fdbe1ff2d3689b2a2c4160608760..e1bf1bae14b8473989b1c0890c58188591123d71 100644 --- a/internal/permission/permission.go +++ b/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) diff --git a/internal/permission/permission_test.go b/internal/permission/permission_test.go index 89e06916024cd1669f5e0d0a263d4a71548c8a97..79930f3ae1e2ef15257f09724fef64d3ea28dada 100644 --- a/internal/permission/permission_test.go +++ b/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) } diff --git a/internal/shell/background.go b/internal/shell/background.go index bc81369ec877586c92fa9bc701d8b78b669f23d5..cb1855836f64bdd56a90802c2bbb939a5a514100 100644 --- a/internal/shell/background.go +++ b/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{}), } diff --git a/internal/shell/background_test.go b/internal/shell/background_test.go index 5149861d94e457e8a78650c48d9c6765a57d369e..7c521bc1477b07775cffb69f310fa83d710d4634 100644 --- a/internal/shell/background_test.go +++ b/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", "") diff --git a/internal/tui/styles/theme.go b/internal/tui/styles/theme.go index f87ffd9de8b324cec4dcfd8b7cee61f71e0390eb..b03603c57439f5f950f9860d3287b0f9d13742e5 100644 --- a/internal/tui/styles/theme.go +++ b/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), }