Skip to content

Commit d0a583b

Browse files
blink-so[bot]f0ssel
andcommitted
Fix race condition and panic in tests
- Add mutex synchronization to proxy server Start/Stop methods to prevent data races - Add empty command array check in MacOSNetJail.Command to prevent slice bounds panic - Ensure thread-safe access to httpServer and httpsServer fields - Fix TestJail_Command empty command test case Co-authored-by: f0ssel <[email protected]>
1 parent d17bf88 commit d0a583b

File tree

2 files changed

+38
-10
lines changed

2 files changed

+38
-10
lines changed

namespace/macos.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ func (m *MacOSNetJail) Start() error {
112112
func (m *MacOSNetJail) Command(command []string) *exec.Cmd {
113113
m.logger.Debug("Command called", "command", command)
114114

115+
// Check for empty command
116+
if len(command) == 0 {
117+
m.logger.Error("Cannot create command: empty command array")
118+
// Return a dummy command that will fail gracefully
119+
cmd := exec.Command("false") // false command that always exits with status 1
120+
cmd.Env = os.Environ()
121+
return cmd
122+
}
123+
115124
// Create command directly (no sg wrapper needed)
116125
m.logger.Debug("Creating command with group membership", "groupID", m.restrictedGid)
117126
cmd := exec.Command(command[0], command[1:]...)

proxy/proxy.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"log/slog"
99
"net/http"
1010
"net/url"
11+
"sync"
1112
"time"
1213

1314
"github.com/coder/jail/audit"
@@ -16,6 +17,7 @@ import (
1617

1718
// Server handles HTTP and HTTPS requests with rule-based filtering
1819
type Server struct {
20+
mu sync.Mutex
1921
ruleEngine rules.Evaluator
2022
auditor audit.Auditor
2123
logger *slog.Logger
@@ -51,6 +53,7 @@ func NewProxyServer(config Config) *Server {
5153

5254
// Start starts both HTTP and HTTPS proxy servers
5355
func (p *Server) Start(ctx context.Context) error {
56+
p.mu.Lock()
5457
// Create HTTP server
5558
p.httpServer = &http.Server{
5659
Addr: fmt.Sprintf(":%d", p.httpPort),
@@ -63,22 +66,33 @@ func (p *Server) Start(ctx context.Context) error {
6366
Handler: http.HandlerFunc(p.handleHTTPS),
6467
TLSConfig: p.tlsConfig,
6568
}
69+
p.mu.Unlock()
6670

6771
// Start HTTP server
6872
go func() {
6973
p.logger.Info("Starting HTTP proxy", "port", p.httpPort)
70-
err := p.httpServer.ListenAndServe()
71-
if err != nil && err != http.ErrServerClosed {
72-
p.logger.Error("HTTP proxy server error", "error", err)
74+
p.mu.Lock()
75+
httpServer := p.httpServer
76+
p.mu.Unlock()
77+
if httpServer != nil {
78+
err := httpServer.ListenAndServe()
79+
if err != nil && err != http.ErrServerClosed {
80+
p.logger.Error("HTTP proxy server error", "error", err)
81+
}
7382
}
7483
}()
7584

7685
// Start HTTPS server
7786
go func() {
7887
p.logger.Info("Starting HTTPS proxy", "port", p.httpsPort)
79-
err := p.httpsServer.ListenAndServeTLS("", "")
80-
if err != nil && err != http.ErrServerClosed {
81-
p.logger.Error("HTTPS proxy server error", "error", err)
88+
p.mu.Lock()
89+
httpsServer := p.httpsServer
90+
p.mu.Unlock()
91+
if httpsServer != nil {
92+
err := httpsServer.ListenAndServeTLS("", "")
93+
if err != nil && err != http.ErrServerClosed {
94+
p.logger.Error("HTTPS proxy server error", "error", err)
95+
}
8296
}
8397
}()
8498

@@ -92,12 +106,17 @@ func (p *Server) Stop() error {
92106
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
93107
defer cancel()
94108

109+
p.mu.Lock()
110+
httpServer := p.httpServer
111+
httpsServer := p.httpsServer
112+
p.mu.Unlock()
113+
95114
var httpErr, httpsErr error
96-
if p.httpServer != nil {
97-
httpErr = p.httpServer.Shutdown(ctx)
115+
if httpServer != nil {
116+
httpErr = httpServer.Shutdown(ctx)
98117
}
99-
if p.httpsServer != nil {
100-
httpsErr = p.httpsServer.Shutdown(ctx)
118+
if httpsServer != nil {
119+
httpsErr = httpsServer.Shutdown(ctx)
101120
}
102121

103122
if httpErr != nil {

0 commit comments

Comments
 (0)