Skip to content

Commit 6e34f09

Browse files
peteski22peteski22
authored
Refactor runtime specs with security fixes and Python support (#104)
- Fix Python runtime to properly extract module names after -m flag (unsed at present as only npx and uvx are supported) - Add security validation to block remote git+ and HTTP sources in NPX - Refactor specs into focused extractor functions for better maintainability - Add comprehensive test coverage for all runtime package extractors - Improve error handling with specific messages for different failure scenarios [!] This prevents potential security issues where NPX could load arbitrary remote packages and fixes broken Python module extraction.
1 parent 4e9dc30 commit 6e34f09

File tree

2 files changed

+407
-95
lines changed

2 files changed

+407
-95
lines changed

‎internal/runtime/spec.go

Lines changed: 125 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -5,116 +5,148 @@ import (
55
"strings"
66
)
77

8+
type pkgExtractor func(args []string) (string, error)
9+
10+
type flagNameSet map[string]struct{}
11+
812
type Spec struct {
913
ShouldIgnoreFlag func(string) bool
1014
ExtractPackageName func([]string) (string, error)
1115
}
1216

17+
func NewSpec(ignoreFlags flagNameSet, extractor pkgExtractor) Spec {
18+
return Spec{
19+
ShouldIgnoreFlag: func(flagName string) bool {
20+
_, ok := ignoreFlags[flagName]
21+
return ok
22+
},
23+
ExtractPackageName: extractor,
24+
}
25+
}
26+
1327
func Specs() map[Runtime]Spec {
1428
return map[Runtime]Spec{
15-
Docker: {
16-
ShouldIgnoreFlag: func(flag string) bool {
17-
switch flag {
18-
case "--rm", "--name", "--volume", "-v", "--network", "--detach", "-d", "-i":
19-
return true
29+
Docker: NewSpec(dockerIgnoreFlags(), dockerPackageExtractor()),
30+
NPX: NewSpec(flagNameSet{"-y": {}}, npxPackageExtractor()),
31+
UVX: NewSpec(flagNameSet{"--from": {}}, uvxPackageExtractor()),
32+
Python: NewSpec(flagNameSet{"-m": {}}, pythonPackageExtractor()),
33+
}
34+
}
35+
36+
func dockerIgnoreFlags() flagNameSet {
37+
return flagNameSet{
38+
"-d": {},
39+
"--detach": {},
40+
"-i": {},
41+
"--name": {},
42+
"--network": {},
43+
"--rm": {},
44+
"-v": {},
45+
"--volume": {},
46+
}
47+
}
48+
49+
func dockerPackageExtractor() pkgExtractor {
50+
return func(args []string) (string, error) {
51+
skip := true
52+
skipNext := false
53+
54+
flagsWithValues := flagNameSet{
55+
"-e": {},
56+
"--env": {},
57+
"-p": {},
58+
"--name": {},
59+
"--network": {},
60+
"-v": {},
61+
"--volume": {},
62+
}
63+
64+
for i := 0; i < len(args); i++ {
65+
arg := args[i]
66+
67+
if skip {
68+
if arg == "run" {
69+
skip = false
2070
}
21-
return false
22-
},
23-
ExtractPackageName: func(args []string) (string, error) {
24-
skip := true
25-
skipNext := false
26-
27-
// Flags that take a value (e.g. --name greptime)
28-
flagsWithValues := map[string]struct{}{
29-
"-e": {},
30-
"--env": {},
31-
"-p": {},
32-
"-v": {},
33-
"--volume": {},
34-
"--name": {},
35-
"--network": {},
71+
continue
72+
}
73+
74+
if skipNext {
75+
skipNext = false
76+
continue
77+
}
78+
79+
if strings.HasPrefix(arg, "-") {
80+
if _, ok := flagsWithValues[arg]; ok {
81+
skipNext = true
3682
}
83+
continue
84+
}
3785

38-
for i := 0; i < len(args); i++ {
39-
arg := args[i]
86+
return arg, nil
87+
}
4088

41-
if skip {
42-
if arg == "run" {
43-
skip = false
44-
}
45-
continue
46-
}
89+
return "", fmt.Errorf("no %s image found", Docker)
90+
}
91+
}
4792

48-
if skipNext {
49-
skipNext = false
50-
continue
51-
}
93+
func npxPackageExtractor() pkgExtractor {
94+
return func(args []string) (string, error) {
95+
for _, arg := range args {
96+
if strings.HasPrefix(arg, "-") {
97+
continue
98+
}
99+
normalizedArg := strings.ToLower(arg)
100+
if strings.HasPrefix(normalizedArg, "git+") || strings.HasPrefix(normalizedArg, "https://") {
101+
return "", fmt.Errorf("remote sources are unsupported")
102+
}
103+
return arg, nil
104+
}
105+
return "", fmt.Errorf("no %s package found", NPX)
106+
}
107+
}
52108

53-
if strings.HasPrefix(arg, "-") {
54-
_, takesValue := flagsWithValues[arg]
55-
skipNext = takesValue
56-
continue
57-
}
109+
func uvxPackageExtractor() pkgExtractor {
110+
return func(args []string) (string, error) {
111+
for i := 0; i < len(args); i++ {
112+
arg := strings.TrimSpace(args[i])
58113

59-
return arg, nil
114+
if arg == "--from" && i+1 < len(args) {
115+
next := strings.ToLower(strings.TrimSpace(args[i+1]))
116+
if strings.HasPrefix(next, "git+") {
117+
return "", fmt.Errorf("remote git repositories are unsupported")
60118
}
61-
62-
return "", fmt.Errorf("no %s image found", Docker)
63-
},
64-
},
65-
NPX: {
66-
ShouldIgnoreFlag: func(flag string) bool {
67-
return flag == "-y"
68-
},
69-
ExtractPackageName: func(args []string) (string, error) {
70-
// First non-flag value
71-
for _, arg := range args {
72-
if !strings.HasPrefix(arg, "-") {
73-
return arg, nil
74-
}
119+
if strings.HasPrefix(next, "https://") {
120+
return "", fmt.Errorf("arbitrary HTTP repositories are unsupported")
75121
}
76-
return "", fmt.Errorf("no %s package found", NPX)
77-
},
78-
},
79-
UVX: {
80-
ShouldIgnoreFlag: func(flag string) bool {
81-
switch flag {
82-
case "--from":
83-
return true
122+
123+
i++ // Skip the next value
124+
continue
125+
}
126+
127+
if !strings.HasPrefix(arg, "-") {
128+
return arg, nil
129+
}
130+
}
131+
132+
return "", fmt.Errorf("no %s package found", UVX)
133+
}
134+
}
135+
136+
func pythonPackageExtractor() pkgExtractor {
137+
return func(args []string) (string, error) {
138+
for i, arg := range args {
139+
if arg == "-m" {
140+
if i+1 >= len(args) {
141+
return "", fmt.Errorf("missing module name after -m")
84142
}
85-
return false
86-
},
87-
ExtractPackageName: func(args []string) (string, error) {
88-
for i := 0; i < len(args); i++ {
89-
arg := strings.TrimSpace(args[i])
90-
nextIndex := i + 1
91-
if args[i] == "--from" && nextIndex < len(args) {
92-
nextArg := strings.TrimSpace(args[nextIndex])
93-
if strings.HasPrefix(nextArg, "git+") {
94-
return "", fmt.Errorf("remote git repositories are unsupported")
95-
}
96-
if strings.HasPrefix(nextArg, "https://") {
97-
return "", fmt.Errorf("arbitrary HTTP repositories are unsupported")
98-
}
99-
100-
i++ // Skip next value.
101-
continue
102-
}
103-
if !strings.HasPrefix(arg, "-") {
104-
return arg, nil
105-
}
143+
next := args[i+1]
144+
if strings.HasPrefix(next, "-") {
145+
return "", fmt.Errorf("invalid module name after -m: %s", next)
106146
}
107-
return "", fmt.Errorf("no %s package found", UVX)
108-
},
109-
},
110-
Python: {
111-
ShouldIgnoreFlag: func(flag string) bool {
112-
return flag == "-m"
113-
},
114-
ExtractPackageName: func(args []string) (string, error) {
115-
// No clear package name? Could return empty or static
116-
return "", nil
117-
},
118-
},
147+
return next, nil
148+
}
149+
}
150+
return "", fmt.Errorf("no %s module found", Python)
119151
}
120152
}

0 commit comments

Comments
 (0)