Skip to content

Commit 39188cc

Browse files
ilopezlunailopezluna
committed
Do not allow paths in runtime flags
1 parent 00cca04 commit 39188cc

File tree

4 files changed

+328
-0
lines changed

4 files changed

+328
-0
lines changed

‎cmd/cli/commands/configure_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,3 +305,90 @@ func TestThinkFlagBehavior(t *testing.T) {
305305
})
306306
}
307307
}
308+
309+
func TestRuntimeFlagsValidation(t *testing.T) {
310+
tests := []struct {
311+
name string
312+
runtimeFlags []string
313+
expectError bool
314+
errorContains string
315+
}{
316+
{
317+
name: "valid runtime flags without paths",
318+
runtimeFlags: []string{"--verbose", "--threads", "4"},
319+
expectError: false,
320+
},
321+
{
322+
name: "empty runtime flags",
323+
runtimeFlags: []string{},
324+
expectError: false,
325+
},
326+
{
327+
name: "reject absolute path in value",
328+
runtimeFlags: []string{"--log-file", "/var/log/model.log"},
329+
expectError: true,
330+
errorContains: "paths are not allowed",
331+
},
332+
{
333+
name: "reject absolute path in flag=value format",
334+
runtimeFlags: []string{"--output-file=/tmp/output.txt"},
335+
expectError: true,
336+
errorContains: "paths are not allowed",
337+
},
338+
{
339+
name: "reject relative path",
340+
runtimeFlags: []string{"--config", "../config.yaml"},
341+
expectError: true,
342+
errorContains: "paths are not allowed",
343+
},
344+
{
345+
name: "reject URL",
346+
runtimeFlags: []string{"--endpoint", "http://example.com/api"},
347+
expectError: true,
348+
errorContains: "paths are not allowed",
349+
},
350+
}
351+
352+
for _, tt := range tests {
353+
t.Run(tt.name, func(t *testing.T) {
354+
flags := ConfigureFlags{}
355+
req, err := flags.BuildConfigureRequest("test-model")
356+
if err != nil {
357+
t.Fatalf("BuildConfigureRequest failed: %v", err)
358+
}
359+
360+
// Set runtime flags after building request
361+
req.RuntimeFlags = tt.runtimeFlags
362+
363+
// Note: The actual validation happens in scheduler.ConfigureRunner,
364+
// but we're testing that the BuildConfigureRequest correctly
365+
// preserves the RuntimeFlags for validation downstream.
366+
// For a true integration test, we would need to mock the scheduler.
367+
368+
if tt.expectError {
369+
// In this unit test context, we verify the flags are preserved
370+
// The actual validation will happen in the scheduler
371+
if len(req.RuntimeFlags) == 0 && len(tt.runtimeFlags) > 0 {
372+
t.Error("RuntimeFlags should be preserved in the request")
373+
}
374+
} else {
375+
if !equalStringSlices(req.RuntimeFlags, tt.runtimeFlags) {
376+
t.Errorf("Expected RuntimeFlags %v, got %v", tt.runtimeFlags, req.RuntimeFlags)
377+
}
378+
}
379+
})
380+
}
381+
}
382+
383+
// equalStringSlices checks if two string slices are equal
384+
func equalStringSlices(a, b []string) bool {
385+
if len(a) != len(b) {
386+
return false
387+
}
388+
for i := range a {
389+
if a[i] != b[i] {
390+
return false
391+
}
392+
}
393+
return true
394+
}

‎pkg/inference/runtime_flags.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package inference
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
// ValidateRuntimeFlags ensures runtime flags don't contain paths (forward slash "/" or backslash "\")
9+
// to prevent malicious users from overwriting host files via arguments like
10+
// --log-file /some/path, --output-file /etc/passwd, or --log-file C:\Windows\file.
11+
//
12+
// This validation rejects any flag or value containing "/" or "\" to block:
13+
// - Unix/Linux/macOS absolute paths: /var/log/file, /etc/passwd
14+
// - Unix/Linux/macOS relative paths: ../file.txt, ./config
15+
// - Windows absolute paths: C:\Users\file, D:\data\file
16+
// - Windows relative paths: ..\file.txt, .\config
17+
// - UNC paths: \\network\share\file
18+
//
19+
// Returns an error if any flag contains a forward slash or backslash.
20+
func ValidateRuntimeFlags(flags []string) error {
21+
for _, flag := range flags {
22+
if strings.Contains(flag, "/") || strings.Contains(flag, "\\") {
23+
return fmt.Errorf("invalid runtime flag %q: paths are not allowed (contains '/' or '\\\\')", flag)
24+
}
25+
}
26+
return nil
27+
}

‎pkg/inference/runtime_flags_test.go‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
package inference
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestValidateRuntimeFlags(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
flags []string
11+
expectError bool
12+
description string
13+
}{
14+
{
15+
name: "empty flags",
16+
flags: []string{},
17+
expectError: false,
18+
description: "Empty array should pass validation",
19+
},
20+
{
21+
name: "nil flags",
22+
flags: nil,
23+
expectError: false,
24+
description: "Nil array should pass validation",
25+
},
26+
{
27+
name: "valid flags without paths",
28+
flags: []string{"--verbose", "--debug", "--threads", "4"},
29+
expectError: false,
30+
description: "Simple flags without paths should pass",
31+
},
32+
{
33+
name: "valid single character flags",
34+
flags: []string{"-v", "-d", "-t", "4"},
35+
expectError: false,
36+
description: "Single character flags should pass",
37+
},
38+
{
39+
name: "valid flags with numbers and hyphens",
40+
flags: []string{"--gpu-memory-utilization", "0.9", "--max-tokens", "1024"},
41+
expectError: false,
42+
description: "Flags with hyphens and numeric values should pass",
43+
},
44+
{
45+
name: "reject absolute path in value",
46+
flags: []string{"--log-file", "/var/log/model.log"},
47+
expectError: true,
48+
description: "Absolute paths should be rejected",
49+
},
50+
{
51+
name: "reject absolute path in flag=value format",
52+
flags: []string{"--log-file=/var/log/model.log"},
53+
expectError: true,
54+
description: "Paths in flag=value format should be rejected",
55+
},
56+
{
57+
name: "reject relative path with parent directory",
58+
flags: []string{"--output", "../file.txt"},
59+
expectError: true,
60+
description: "Relative paths with ../ should be rejected",
61+
},
62+
{
63+
name: "reject relative path with current directory",
64+
flags: []string{"--config", "./config.yaml"},
65+
expectError: true,
66+
description: "Relative paths with ./ should be rejected",
67+
},
68+
{
69+
name: "reject Windows-style path with forward slash",
70+
flags: []string{"--file", "C:/Users/file.txt"},
71+
expectError: true,
72+
description: "Windows-style paths with forward slash should be rejected",
73+
},
74+
{
75+
name: "reject Windows-style path with backslash",
76+
flags: []string{"--file", "C:\\Users\\file.txt"},
77+
expectError: true,
78+
description: "Windows-style paths with backslash should be rejected",
79+
},
80+
{
81+
name: "reject Windows relative path with backslash",
82+
flags: []string{"--config", "..\\config.yaml"},
83+
expectError: true,
84+
description: "Windows relative paths with backslash should be rejected",
85+
},
86+
{
87+
name: "reject Windows current directory path",
88+
flags: []string{"--output", ".\\output.txt"},
89+
expectError: true,
90+
description: "Windows current directory paths should be rejected",
91+
},
92+
{
93+
name: "reject UNC network path",
94+
flags: []string{"--share", "\\\\server\\share\\file.txt"},
95+
expectError: true,
96+
description: "UNC network paths should be rejected",
97+
},
98+
{
99+
name: "reject Windows system path",
100+
flags: []string{"--log", "C:\\Windows\\System32\\log.txt"},
101+
expectError: true,
102+
description: "Windows system paths should be rejected",
103+
},
104+
{
105+
name: "reject URL with http",
106+
flags: []string{"--endpoint", "http://example.com/api"},
107+
expectError: true,
108+
description: "URLs should be rejected (conservative approach)",
109+
},
110+
{
111+
name: "reject URL with https",
112+
flags: []string{"--api-url", "https://api.example.com/v1"},
113+
expectError: true,
114+
description: "HTTPS URLs should be rejected (conservative approach)",
115+
},
116+
{
117+
name: "reject path in middle of flag list",
118+
flags: []string{"--verbose", "--log-file", "/tmp/log.txt", "--debug"},
119+
expectError: true,
120+
description: "Path anywhere in flag list should be rejected",
121+
},
122+
{
123+
name: "reject multiple paths",
124+
flags: []string{"--input", "/path/to/input", "--output", "/path/to/output"},
125+
expectError: true,
126+
description: "Multiple paths should be rejected",
127+
},
128+
{
129+
name: "reject path traversal attempt",
130+
flags: []string{"--file", "../../etc/passwd"},
131+
expectError: true,
132+
description: "Path traversal attempts should be rejected",
133+
},
134+
{
135+
name: "reject root directory",
136+
flags: []string{"--root", "/"},
137+
expectError: true,
138+
description: "Root directory should be rejected",
139+
},
140+
{
141+
name: "reject home directory path",
142+
flags: []string{"--home", "/home/user/.config"},
143+
expectError: true,
144+
description: "Home directory paths should be rejected",
145+
},
146+
{
147+
name: "valid flag with special characters except slash",
148+
flags: []string{"--model-name", "llama-3.2-1b", "--temperature", "0.7"},
149+
expectError: false,
150+
description: "Flags with dots, hyphens, and numbers (no slash) should pass",
151+
},
152+
{
153+
name: "valid flag with underscore",
154+
flags: []string{"--max_tokens", "512", "--use_cache"},
155+
expectError: false,
156+
description: "Flags with underscores should pass",
157+
},
158+
}
159+
160+
for _, tt := range tests {
161+
t.Run(tt.name, func(t *testing.T) {
162+
err := ValidateRuntimeFlags(tt.flags)
163+
164+
if tt.expectError {
165+
if err == nil {
166+
t.Errorf("%s: expected error but got none", tt.description)
167+
}
168+
} else {
169+
if err != nil {
170+
t.Errorf("%s: unexpected error: %v", tt.description, err)
171+
}
172+
}
173+
})
174+
}
175+
}
176+
177+
func TestValidateRuntimeFlags_ErrorMessage(t *testing.T) {
178+
// Test that error messages are helpful
179+
flags := []string{"--log-file", "/var/log/test.log"}
180+
err := ValidateRuntimeFlags(flags)
181+
182+
if err == nil {
183+
t.Fatal("Expected error but got none")
184+
}
185+
186+
errMsg := err.Error()
187+
if !contains(errMsg, "/var/log/test.log") {
188+
t.Errorf("Error message should contain the offending flag value, got: %s", errMsg)
189+
}
190+
if !contains(errMsg, "paths are not allowed") {
191+
t.Errorf("Error message should explain why it failed, got: %s", errMsg)
192+
}
193+
}
194+
195+
// contains is a helper function to check if a string contains a substring
196+
func contains(s, substr string) bool {
197+
return len(s) >= len(substr) && (s == substr || substr == "" ||
198+
(s != "" && indexOf(s, substr) >= 0))
199+
}
200+
201+
// indexOf returns the index of substr in s, or -1 if not found
202+
func indexOf(s, substr string) int {
203+
for i := 0; i <= len(s)-len(substr); i++ {
204+
if s[i:i+len(substr)] == substr {
205+
return i
206+
}
207+
}
208+
return -1
209+
}

‎pkg/inference/scheduling/scheduler.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,11 @@ func (s *Scheduler) ConfigureRunner(ctx context.Context, backend inference.Backe
240240
}
241241
}
242242

243+
// Validate runtime flags to prevent path-based security issues
244+
if err := inference.ValidateRuntimeFlags(runtimeFlags); err != nil {
245+
return nil, err
246+
}
247+
243248
// Build runner configuration with shared settings
244249
var runnerConfig inference.BackendConfiguration
245250
runnerConfig.ContextSize = req.ContextSize

0 commit comments

Comments
 (0)