Skip to content

Commit eae0ae5

Browse files
committed
fix(security): add path traversal prevention and safe integer conversion
Add validatePathComponent to file backend to prevent path traversal attacks via malicious agent names, session IDs, or checkpoint IDs. Add safeIntToInt32 helper to prevent integer overflow in VertexAI provider. Add ExternalTLS option for service mesh deployments.
1 parent 6cf3811 commit eae0ae5

6 files changed

Lines changed: 175 additions & 33 deletions

File tree

examples/session-basic/main.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ func main() {
123123
// Example 7: Resume session (simulating application restart)
124124
fmt.Println("7. Simulating session resume...")
125125
sessionID := sess.ID()
126-
sess.Close(ctx) // Close current session
126+
if err := sess.Close(ctx); err != nil {
127+
log.Printf("Warning: failed to close session: %v", err)
128+
}
127129

128130
// Get session by ID (as if resuming after restart)
129131
resumedSess, err := mgr.Get(ctx, sessionID)

examples/session-react/main.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ func main() {
153153
// Step 9: Resume session (simulate app restart)
154154
fmt.Println("\n9. Simulating session resume...")
155155
sessionID := sess.ID()
156-
sess.Close(ctx)
156+
if err := sess.Close(ctx); err != nil {
157+
log.Printf("Warning: failed to close session: %v", err)
158+
}
157159

158160
// Re-open the session
159161
resumedSess, err := sessionMgr.Get(ctx, sessionID)

internal/llm/provider/vertexai.go

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,22 @@ import (
2121
// math/rand would be sufficient, but crypto/rand is used for defense in depth.
2222

2323
const (
24-
vertexAIMaxRetries = 5
25-
vertexAIBaseDelay = 1 * time.Second
26-
vertexAIMaxDelay = 32 * time.Second
27-
vertexAIJitterFactor = 0.3
24+
vertexAIMaxRetries = 5
25+
vertexAIBaseDelay = 1 * time.Second
26+
vertexAIMaxDelay = 32 * time.Second
27+
vertexAIJitterFactor = 0.3
2828
vertexAIClientTimeout = 30 * time.Second
2929
)
3030

31+
// safeIntToInt32 safely converts an int to int32, returning 0 if out of range.
32+
// This avoids integer overflow issues flagged by security scanners (G115).
33+
func safeIntToInt32(n int) (int32, bool) {
34+
if n < 0 || n > math.MaxInt32 {
35+
return 0, false
36+
}
37+
return int32(n), true
38+
}
39+
3140
func init() {
3241
RegisterFactory("vertexai", func(config map[string]any) (Provider, error) {
3342
projectID := ""
@@ -112,8 +121,8 @@ func (p *VertexAIProvider) CreateCompletion(ctx context.Context, req CompletionR
112121
// Always set temperature - 0 is a valid value for deterministic output
113122
// Use -1 as sentinel for "not set" if needed, but typically callers set explicit values
114123
config.Temperature = genai.Ptr(float32(req.Temperature))
115-
if req.MaxTokens > 0 && req.MaxTokens <= math.MaxInt32 {
116-
config.MaxOutputTokens = int32(req.MaxTokens)
124+
if maxTokens, ok := safeIntToInt32(req.MaxTokens); ok && maxTokens > 0 {
125+
config.MaxOutputTokens = maxTokens
117126
}
118127

119128
// Build contents from messages
@@ -174,8 +183,8 @@ func (p *VertexAIProvider) CreateStructured(ctx context.Context, req StructuredR
174183
}
175184
// Always set temperature - 0 is a valid value for deterministic output
176185
config.Temperature = genai.Ptr(float32(req.Temperature))
177-
if req.MaxTokens > 0 && req.MaxTokens <= math.MaxInt32 {
178-
config.MaxOutputTokens = int32(req.MaxTokens)
186+
if maxTokens, ok := safeIntToInt32(req.MaxTokens); ok && maxTokens > 0 {
187+
config.MaxOutputTokens = maxTokens
179188
}
180189

181190
// Add response schema if provided
@@ -244,8 +253,8 @@ func (p *VertexAIProvider) CreateStreaming(ctx context.Context, req CompletionRe
244253
config := &genai.GenerateContentConfig{}
245254
// Always set temperature - 0 is a valid value for deterministic output
246255
config.Temperature = genai.Ptr(float32(req.Temperature))
247-
if req.MaxTokens > 0 && req.MaxTokens <= math.MaxInt32 {
248-
config.MaxOutputTokens = int32(req.MaxTokens)
256+
if maxTokens, ok := safeIntToInt32(req.MaxTokens); ok && maxTokens > 0 {
257+
config.MaxOutputTokens = maxTokens
249258
}
250259

251260
// Build contents from messages
@@ -469,17 +478,9 @@ func isRetryableGenAIError(err error) bool {
469478
func (p *VertexAIProvider) calculateBackoff(attempt int) time.Duration {
470479
// Exponential backoff: 1s, 2s, 4s, 8s, 16s (capped at maxDelay)
471480
// Guard against negative or zero attempt to prevent uint overflow
472-
shift := attempt - 1
473-
if shift < 0 {
474-
shift = 0
475-
}
476-
if shift > 31 { // Prevent overflow for large values
477-
shift = 31
478-
}
479-
delay := time.Duration(1<<uint(shift)) * vertexAIBaseDelay
480-
if delay > vertexAIMaxDelay {
481-
delay = vertexAIMaxDelay
482-
}
481+
// Clamp shift to [0, 31] using min/max built-ins for safe uint conversion
482+
shift := uint(max(0, min(attempt-1, 31)))
483+
delay := min(time.Duration(1<<shift)*vertexAIBaseDelay, vertexAIMaxDelay)
483484
// Add jitter: delay ± 30% using crypto/rand for security compliance
484485
jitter := time.Duration(float64(delay) * vertexAIJitterFactor * (cryptoRandFloat64()*2 - 1))
485486
return delay + jitter

internal/runtime/distributed.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ type TLSConfig struct {
6363
// ServerName is used for SNI verification.
6464
ServerName string
6565
// InsecureSkipVerify skips certificate verification (development only).
66+
// Warning: This logs a security warning. Do not use in production.
6667
InsecureSkipVerify bool
68+
// ExternalTLS indicates TLS is handled by a service mesh (Istio, Linkerd, etc.).
69+
// When true, app-level TLS is disabled entirely since the mesh sidecar handles
70+
// encryption. This takes precedence over other TLS settings.
71+
ExternalTLS bool
6772
}
6873

6974
// remoteAgentClient represents a connection to a remote agent
@@ -189,10 +194,21 @@ func (r *DistributedRuntime) Connect(name, addr string) error {
189194
func (r *DistributedRuntime) buildDialOptions() ([]grpc.DialOption, error) {
190195
var opts []grpc.DialOption
191196

197+
// ExternalTLS means TLS is handled by service mesh (Istio, Linkerd, etc.)
198+
// Use plaintext transport since the sidecar handles encryption
199+
if r.tlsConfig != nil && r.tlsConfig.ExternalTLS {
200+
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
201+
return opts, nil
202+
}
203+
192204
if r.tlsConfig != nil && r.tlsConfig.Enabled {
205+
// InsecureSkipVerify is for development/testing only
206+
if r.tlsConfig.InsecureSkipVerify {
207+
log.Println("[DistributedRuntime] WARNING: TLS certificate verification is disabled. This is insecure and should only be used for development.")
208+
}
193209
tlsCfg := &tls.Config{
194210
MinVersion: tls.VersionTLS12,
195-
InsecureSkipVerify: r.tlsConfig.InsecureSkipVerify,
211+
InsecureSkipVerify: r.tlsConfig.InsecureSkipVerify, //nolint:gosec // G402: intentionally configurable for dev/test
196212
}
197213

198214
// Set server name for SNI
@@ -603,6 +619,11 @@ func (r *DistributedRuntime) Start(ctx context.Context) error {
603619
func (r *DistributedRuntime) buildServerOptions() ([]grpc.ServerOption, error) {
604620
var opts []grpc.ServerOption
605621

622+
// ExternalTLS means TLS is handled by service mesh - no server-side TLS needed
623+
if r.tlsConfig != nil && r.tlsConfig.ExternalTLS {
624+
return opts, nil
625+
}
626+
606627
if r.tlsConfig != nil && r.tlsConfig.Enabled {
607628
// Load server certificate
608629
cert, err := tls.LoadX509KeyPair(r.tlsConfig.CertFile, r.tlsConfig.KeyFile)

pkg/session/file_backend.go

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,30 @@ import (
44
"bufio"
55
"context"
66
"encoding/json"
7+
"errors"
78
"fmt"
89
"os"
910
"path/filepath"
1011
"sort"
12+
"strings"
1113
"sync"
1214
)
1315

16+
// ErrInvalidPathComponent is returned when a path component contains unsafe characters.
17+
var ErrInvalidPathComponent = errors.New("invalid path component: contains path separator or traversal sequence")
18+
19+
// validatePathComponent checks that a string is safe to use as a path component.
20+
// It rejects empty strings, path separators, and traversal sequences.
21+
func validatePathComponent(s string) error {
22+
if s == "" {
23+
return errors.New("path component cannot be empty")
24+
}
25+
if strings.ContainsAny(s, `/\`) || strings.Contains(s, "..") {
26+
return ErrInvalidPathComponent
27+
}
28+
return nil
29+
}
30+
1431
// FileBackend implements StorageBackend using JSONL files.
1532
// Storage layout:
1633
//
@@ -56,6 +73,14 @@ func (f *FileBackend) SaveSession(ctx context.Context, meta *SessionMetadata) er
5673
return ErrStorageClosed
5774
}
5875

76+
// Validate path components to prevent path traversal
77+
if err := validatePathComponent(meta.AgentName); err != nil {
78+
return fmt.Errorf("invalid agent name: %w", err)
79+
}
80+
if err := validatePathComponent(meta.ID); err != nil {
81+
return fmt.Errorf("invalid session ID: %w", err)
82+
}
83+
5984
// Ensure agent directory exists
6085
agentDir := filepath.Join(f.baseDir, meta.AgentName)
6186
if err := os.MkdirAll(agentDir, 0700); err != nil {
@@ -66,7 +91,7 @@ func (f *FileBackend) SaveSession(ctx context.Context, meta *SessionMetadata) er
6691
indexPath := filepath.Join(agentDir, "sessions.json")
6792
index := make(map[string]*SessionMetadata)
6893

69-
data, err := os.ReadFile(indexPath) // #nosec G304 - path is constructed from trusted base
94+
data, err := os.ReadFile(indexPath) // #nosec G304 - path components validated to prevent traversal
7095
if err == nil {
7196
if err := json.Unmarshal(data, &index); err != nil {
7297
return fmt.Errorf("parse sessions index: %w", err)
@@ -100,6 +125,11 @@ func (f *FileBackend) LoadSession(ctx context.Context, sessionID string) (*Sessi
100125
return nil, ErrStorageClosed
101126
}
102127

128+
// Validate session ID to prevent path traversal
129+
if err := validatePathComponent(sessionID); err != nil {
130+
return nil, fmt.Errorf("invalid session ID: %w", err)
131+
}
132+
103133
// Search all agent directories for the session
104134
entries, err := os.ReadDir(f.baseDir)
105135
if err != nil {
@@ -115,7 +145,7 @@ func (f *FileBackend) LoadSession(ctx context.Context, sessionID string) (*Sessi
115145
}
116146

117147
indexPath := filepath.Join(f.baseDir, entry.Name(), "sessions.json")
118-
data, err := os.ReadFile(indexPath) // #nosec G304 - path is constructed from trusted base
148+
data, err := os.ReadFile(indexPath) // #nosec G304 - path components validated to prevent traversal
119149
if err != nil {
120150
continue
121151
}
@@ -156,7 +186,7 @@ func (f *FileBackend) DeleteSession(ctx context.Context, sessionID string) error
156186

157187
// Remove from index
158188
indexPath := filepath.Join(agentDir, "sessions.json")
159-
data, err := os.ReadFile(indexPath) // #nosec G304 - path is constructed from trusted base
189+
data, err := os.ReadFile(indexPath) // #nosec G304 - path components validated to prevent traversal
160190
if err != nil {
161191
return fmt.Errorf("read sessions index: %w", err)
162192
}
@@ -189,10 +219,15 @@ func (f *FileBackend) ListSessions(ctx context.Context, agentName string, opts L
189219
return nil, ErrStorageClosed
190220
}
191221

222+
// Validate agent name to prevent path traversal
223+
if err := validatePathComponent(agentName); err != nil {
224+
return nil, fmt.Errorf("invalid agent name: %w", err)
225+
}
226+
192227
agentDir := filepath.Join(f.baseDir, agentName)
193228
indexPath := filepath.Join(agentDir, "sessions.json")
194229

195-
data, err := os.ReadFile(indexPath) // #nosec G304 - path is constructed from trusted base
230+
data, err := os.ReadFile(indexPath) // #nosec G304 - path validated above
196231
if err != nil {
197232
if os.IsNotExist(err) {
198233
return []*SessionMetadata{}, nil
@@ -254,7 +289,7 @@ func (f *FileBackend) AppendEntry(ctx context.Context, sessionID string, entry *
254289
entriesPath := filepath.Join(agentDir, sessionID+".jsonl")
255290

256291
// Open file for append
257-
file, err := os.OpenFile(entriesPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) // #nosec G304 - path is constructed from trusted base
292+
file, err := os.OpenFile(entriesPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) // #nosec G304 - path components validated to prevent traversal
258293
if err != nil {
259294
return fmt.Errorf("open entries file: %w", err)
260295
}
@@ -291,7 +326,7 @@ func (f *FileBackend) LoadEntries(ctx context.Context, sessionID string) ([]*Ses
291326
agentDir := filepath.Join(f.baseDir, meta.AgentName)
292327
entriesPath := filepath.Join(agentDir, sessionID+".jsonl")
293328

294-
file, err := os.Open(entriesPath) // #nosec G304 - path is constructed from trusted base
329+
file, err := os.Open(entriesPath) // #nosec G304 - path components validated to prevent traversal
295330
if err != nil {
296331
if os.IsNotExist(err) {
297332
return []*SessionEntry{}, nil
@@ -326,7 +361,12 @@ func (f *FileBackend) SaveCheckpoint(ctx context.Context, checkpoint *Checkpoint
326361
return ErrStorageClosed
327362
}
328363

329-
// Find the session to get the agent name
364+
// Validate checkpoint ID to prevent path traversal
365+
if err := validatePathComponent(checkpoint.ID); err != nil {
366+
return fmt.Errorf("invalid checkpoint ID: %w", err)
367+
}
368+
369+
// Find the session to get the agent name (validates sessionID)
330370
meta, err := f.loadSessionUnlocked(checkpoint.SessionID)
331371
if err != nil {
332372
return err
@@ -360,6 +400,11 @@ func (f *FileBackend) LoadCheckpoint(ctx context.Context, checkpointID string) (
360400
return nil, ErrStorageClosed
361401
}
362402

403+
// Validate checkpoint ID to prevent path traversal
404+
if err := validatePathComponent(checkpointID); err != nil {
405+
return nil, fmt.Errorf("invalid checkpoint ID: %w", err)
406+
}
407+
363408
// Search all agent directories for the checkpoint
364409
entries, err := os.ReadDir(f.baseDir)
365410
if err != nil {
@@ -375,7 +420,7 @@ func (f *FileBackend) LoadCheckpoint(ctx context.Context, checkpointID string) (
375420
}
376421

377422
checkpointPath := filepath.Join(f.baseDir, entry.Name(), "checkpoints", checkpointID+".json")
378-
data, err := os.ReadFile(checkpointPath) // #nosec G304 - path is constructed from trusted base
423+
data, err := os.ReadFile(checkpointPath) // #nosec G304 - path components validated to prevent traversal
379424
if err != nil {
380425
continue
381426
}
@@ -403,6 +448,11 @@ func (f *FileBackend) Close() error {
403448
// loadSessionUnlocked is an internal helper that loads session without acquiring locks.
404449
// Caller must hold appropriate lock.
405450
func (f *FileBackend) loadSessionUnlocked(sessionID string) (*SessionMetadata, error) {
451+
// Validate session ID to prevent path traversal
452+
if err := validatePathComponent(sessionID); err != nil {
453+
return nil, fmt.Errorf("invalid session ID: %w", err)
454+
}
455+
406456
// Search all agent directories for the session
407457
entries, err := os.ReadDir(f.baseDir)
408458
if err != nil {
@@ -418,7 +468,7 @@ func (f *FileBackend) loadSessionUnlocked(sessionID string) (*SessionMetadata, e
418468
}
419469

420470
indexPath := filepath.Join(f.baseDir, entry.Name(), "sessions.json")
421-
data, err := os.ReadFile(indexPath) // #nosec G304 - path is constructed from trusted base
471+
data, err := os.ReadFile(indexPath) // #nosec G304 - path components validated to prevent traversal
422472
if err != nil {
423473
continue
424474
}

pkg/session/manager_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,3 +770,69 @@ func TestDataToMessageWithMetadata(t *testing.T) {
770770
t.Errorf("Metadata[key] = %v, want value", msg.Metadata["key"])
771771
}
772772
}
773+
774+
func TestPathTraversalPrevention(t *testing.T) {
775+
tmpDir := t.TempDir()
776+
backend, err := NewFileBackend(tmpDir)
777+
if err != nil {
778+
t.Fatalf("NewFileBackend() error = %v", err)
779+
}
780+
defer backend.Close()
781+
782+
ctx := context.Background()
783+
784+
// Test path traversal in agent name
785+
traversalCases := []struct {
786+
name string
787+
agentName string
788+
sessionID string
789+
}{
790+
{"slash in agent name", "../etc", "valid-session"},
791+
{"backslash in agent name", "..\\etc", "valid-session"},
792+
{"dotdot in agent name", "foo..bar", "valid-session"},
793+
{"slash in session ID", "valid-agent", "../../../etc/passwd"},
794+
{"empty agent name", "", "valid-session"},
795+
{"empty session ID", "valid-agent", ""},
796+
}
797+
798+
for _, tc := range traversalCases {
799+
t.Run(tc.name, func(t *testing.T) {
800+
meta := &SessionMetadata{
801+
ID: tc.sessionID,
802+
AgentName: tc.agentName,
803+
CreatedAt: time.Now().UTC(),
804+
UpdatedAt: time.Now().UTC(),
805+
}
806+
807+
err := backend.SaveSession(ctx, meta)
808+
if err == nil {
809+
t.Errorf("SaveSession() should reject path traversal attempt: agent=%q session=%q", tc.agentName, tc.sessionID)
810+
}
811+
})
812+
}
813+
814+
// Test path traversal in checkpoint ID
815+
t.Run("slash in checkpoint ID", func(t *testing.T) {
816+
// First create a valid session
817+
validMeta := &SessionMetadata{
818+
ID: "valid-session",
819+
AgentName: "valid-agent",
820+
CreatedAt: time.Now().UTC(),
821+
UpdatedAt: time.Now().UTC(),
822+
}
823+
if err := backend.SaveSession(ctx, validMeta); err != nil {
824+
t.Fatalf("SaveSession() error = %v", err)
825+
}
826+
827+
checkpoint := &Checkpoint{
828+
ID: "../../../etc/passwd",
829+
SessionID: "valid-session",
830+
Timestamp: time.Now().UTC(),
831+
}
832+
833+
err := backend.SaveCheckpoint(ctx, checkpoint)
834+
if err == nil {
835+
t.Error("SaveCheckpoint() should reject path traversal in checkpoint ID")
836+
}
837+
})
838+
}

0 commit comments

Comments
 (0)