Skip to content

Commit 3aa9cc3

Browse files
DefilanDefilan
authored
fix: prevent command injection in init container shell commands (#172)
Signed-off-by: Christopher Maher <[email protected]>
1 parent e7689d8 commit 3aa9cc3

File tree

2 files changed

+374
-50
lines changed

2 files changed

+374
-50
lines changed

‎internal/controller/inferenceservice_controller.go‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -59,31 +59,26 @@ func isLocalModelSource(source string) bool {
5959
return strings.HasPrefix(source, "file://") || strings.HasPrefix(source, "/")
6060
}
6161

62-
func buildModelInitCommand(source, cacheDir, modelPath string, useCache bool) string {
62+
func buildModelInitCommand(isLocal, useCache bool) string {
6363
if useCache {
64-
if isLocalModelSource(source) {
65-
localPath := getLocalPath(source)
66-
return fmt.Sprintf(
67-
"mkdir -p %s && if [ ! -f %s ]; then echo 'Copying model from local source %s...'; cp /host-model/model.gguf %s && echo 'Model copied successfully'; else echo 'Model already cached, skipping copy'; fi",
68-
cacheDir, modelPath, localPath, modelPath,
69-
)
64+
if isLocal {
65+
return `mkdir -p "$CACHE_DIR" && if [ ! -f "$MODEL_PATH" ]; then echo 'Copying model from local source...'; cp /host-model/model.gguf "$MODEL_PATH" && echo 'Model copied successfully'; else echo 'Model already cached, skipping copy'; fi`
7066
}
71-
return fmt.Sprintf(
72-
"mkdir -p %s && if [ ! -f %s ]; then echo 'Downloading model from %s...'; curl -f -L -o %s '%s' && echo 'Model downloaded successfully'; else echo 'Model already cached, skipping download'; fi",
73-
cacheDir, modelPath, source, modelPath, source,
74-
)
75-
}
76-
77-
if isLocalModelSource(source) {
78-
return fmt.Sprintf(
79-
"echo 'ERROR: Local model source requires model cache to be configured. Source: %s'; exit 1",
80-
source,
81-
)
82-
}
83-
return fmt.Sprintf(
84-
"if [ ! -f %s ]; then echo 'Downloading model from %s...'; curl -f -L -o %s '%s' && echo 'Model downloaded successfully'; else echo 'Model already exists, skipping download'; fi",
85-
modelPath, source, modelPath, source,
86-
)
67+
return `mkdir -p "$CACHE_DIR" && if [ ! -f "$MODEL_PATH" ]; then echo 'Downloading model...'; curl -f -L -o "$MODEL_PATH" "$MODEL_SOURCE" && echo 'Model downloaded successfully'; else echo 'Model already cached, skipping download'; fi`
68+
}
69+
70+
if isLocal {
71+
return `echo 'ERROR: Local model source requires model cache to be configured.'; exit 1`
72+
}
73+
return `if [ ! -f "$MODEL_PATH" ]; then echo 'Downloading model...'; curl -f -L -o "$MODEL_PATH" "$MODEL_SOURCE" && echo 'Model downloaded successfully'; else echo 'Model already exists, skipping download'; fi`
74+
}
75+
76+
func modelInitEnvVars(source, cacheDir, modelPath string) []corev1.EnvVar {
77+
return []corev1.EnvVar{
78+
{Name: "MODEL_SOURCE", Value: source},
79+
{Name: "CACHE_DIR", Value: cacheDir},
80+
{Name: "MODEL_PATH", Value: modelPath},
81+
}
8782
}
8883

8984
type modelStorageConfig struct {
@@ -138,7 +133,8 @@ func buildCachedStorageConfig(model *inferencev1alpha1.Model, caCertConfigMap st
138133
})
139134
}
140135

141-
cmd := buildModelInitCommand(model.Spec.Source, cacheDir, modelPath, true)
136+
cmd := buildModelInitCommand(isLocalModelSource(model.Spec.Source), true)
137+
env := modelInitEnvVars(model.Spec.Source, cacheDir, modelPath)
142138
if caCertConfigMap != "" {
143139
volumes = append(volumes, corev1.Volume{
144140
Name: "custom-ca-cert",
@@ -163,6 +159,7 @@ func buildCachedStorageConfig(model *inferencev1alpha1.Model, caCertConfigMap st
163159
Name: "model-downloader",
164160
Image: initContainerImage,
165161
Command: []string{"sh", "-c", cmd},
162+
Env: env,
166163
VolumeMounts: initVolumeMounts,
167164
},
168165
},
@@ -183,7 +180,8 @@ func buildEmptyDirStorageConfig(model *inferencev1alpha1.Model, namespace string
183180
},
184181
}
185182

186-
cmd := buildModelInitCommand(model.Spec.Source, "", modelPath, false)
183+
cmd := buildModelInitCommand(isLocalModelSource(model.Spec.Source), false)
184+
env := modelInitEnvVars(model.Spec.Source, "", modelPath)
187185
if caCertConfigMap != "" {
188186
volumes = append(volumes, corev1.Volume{
189187
Name: "custom-ca-cert",
@@ -208,6 +206,7 @@ func buildEmptyDirStorageConfig(model *inferencev1alpha1.Model, namespace string
208206
Name: "model-downloader",
209207
Image: initContainerImage,
210208
Command: []string{"sh", "-c", cmd},
209+
Env: env,
211210
VolumeMounts: initVolumeMounts,
212211
},
213212
},
@@ -335,7 +334,7 @@ func (r *InferenceServiceReconciler) Reconcile(ctx context.Context, req ctrl.Req
335334
return ctrl.Result{}, err
336335
}
337336

338-
service, result, err := r.reconcileService(ctx, inferenceService, modelReady, desiredReplicas)
337+
service, result, err := r.reconcileService(ctx, inferenceService, modelReady, desiredReplicas, isMetal)
339338
if err != nil || result != nil {
340339
if result != nil {
341340
return *result, err
@@ -415,9 +414,21 @@ func (r *InferenceServiceReconciler) reconcileDeployment(ctx context.Context, is
415414
return deployment, existingDeployment.Status.ReadyReplicas, nil, nil
416415
}
417416

418-
func (r *InferenceServiceReconciler) reconcileService(ctx context.Context, isvc *inferencev1alpha1.InferenceService, modelReady bool, desiredReplicas int32) (*corev1.Service, *ctrl.Result, error) {
417+
func (r *InferenceServiceReconciler) reconcileService(ctx context.Context, isvc *inferencev1alpha1.InferenceService, modelReady bool, desiredReplicas int32, isMetal bool) (*corev1.Service, *ctrl.Result, error) {
419418
log := logf.FromContext(ctx)
420419

420+
if isMetal {
421+
log.Info("Metal accelerator detected, skipping Service creation (managed by Metal Agent)")
422+
// Return a minimal Service object so constructEndpoint can still build
423+
// the endpoint URL from the sanitized name.
424+
return &corev1.Service{
425+
ObjectMeta: metav1.ObjectMeta{
426+
Name: sanitizeDNSName(isvc.Name),
427+
Namespace: isvc.Namespace,
428+
},
429+
}, nil, nil
430+
}
431+
421432
service := r.constructService(isvc)
422433
if err := controllerutil.SetControllerReference(isvc, service, r.Scheme); err != nil {
423434
log.Error(err, "Failed to set controller reference for Service")

0 commit comments

Comments
 (0)