Skip to content

Commit 2812a14

Browse files
committed
fixup! fixup! fixup! fixup! fix
Signed-off-by: Oleksii Kurinnyi <[email protected]>
1 parent 4c076c6 commit 2812a14

File tree

2 files changed

+4
-567
lines changed

2 files changed

+4
-567
lines changed

controllers/workspace/devworkspace_controller.go

Lines changed: 4 additions & 209 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@ package controllers
1818
import (
1919
"context"
2020
"fmt"
21-
"regexp"
2221
"strconv"
2322
"strings"
2423
"time"
25-
"unicode/utf8"
2624

2725
"github.com/devfile/devworkspace-operator/pkg/library/initcontainers"
2826
"github.com/devfile/devworkspace-operator/pkg/library/ssh"
@@ -70,212 +68,13 @@ import (
7068
"sigs.k8s.io/controller-runtime/pkg/reconcile"
7169
)
7270

73-
const (
74-
// Validation limits for init-persistent-home container Command field
75-
maxCommandElements = 10
76-
maxCommandElementLength = 1024
77-
78-
// Validation limits for init-persistent-home container Args field
79-
maxArgsElements = 50
80-
maxArgsElementLength = 256 * 1024 // 256KB
81-
)
82-
83-
// validateNoAdvancedFields validates that the init-persistent-home container
84-
// does not use advanced Kubernetes container fields that could make behavior unpredictable.
85-
func validateNoAdvancedFields(c corev1.Container) error {
86-
if len(c.Ports) > 0 {
87-
return fmt.Errorf("ports are not allowed for %s", constants.HomeInitComponentName)
88-
}
89-
90-
if c.LivenessProbe != nil || c.ReadinessProbe != nil || c.StartupProbe != nil {
91-
return fmt.Errorf("probes are not allowed for %s", constants.HomeInitComponentName)
92-
}
93-
94-
if c.Lifecycle != nil {
95-
return fmt.Errorf("lifecycle hooks are not allowed for %s", constants.HomeInitComponentName)
96-
}
97-
98-
if c.Stdin || c.StdinOnce || c.TTY {
99-
return fmt.Errorf("stdin/tty fields are not allowed for %s", constants.HomeInitComponentName)
100-
}
101-
102-
if len(c.VolumeDevices) > 0 {
103-
return fmt.Errorf("volumeDevices are not allowed for %s", constants.HomeInitComponentName)
104-
}
105-
106-
if c.WorkingDir != "" {
107-
return fmt.Errorf("workingDir is not allowed for %s", constants.HomeInitComponentName)
108-
}
109-
110-
if c.TerminationMessagePath != "" || c.TerminationMessagePolicy != "" {
111-
return fmt.Errorf("termination message fields are not allowed for %s", constants.HomeInitComponentName)
112-
}
113-
114-
if c.SecurityContext != nil {
115-
return fmt.Errorf("securityContext is not allowed for %s", constants.HomeInitComponentName)
116-
}
117-
if c.Resources.Limits != nil || c.Resources.Requests != nil {
118-
return fmt.Errorf("resource limits/requests are not allowed for %s", constants.HomeInitComponentName)
119-
}
120-
121-
return nil
122-
}
123-
124-
// validateHomeInitContainer validates all aspects of the init-persistent-home container.
125-
// It only validates fields that are present, as missing fields will be filled by
126-
// the strategic merge with the default init container.
127-
func validateHomeInitContainer(c corev1.Container) error {
128-
// Only validate if present
129-
if c.Image != "" {
130-
if err := validateImageReference(c.Image); err != nil {
131-
return fmt.Errorf("invalid image reference for %s: %w", constants.HomeInitComponentName, err)
132-
}
133-
}
134-
135-
// Validate Command for security and resource limits (only if present)
136-
if len(c.Command) > 0 {
137-
if err := validateCommand(c.Command); err != nil {
138-
return fmt.Errorf("invalid command for %s: %w", constants.HomeInitComponentName, err)
139-
}
140-
}
141-
142-
// Validate Args for security and resource limits (only if present)
143-
if len(c.Args) > 0 {
144-
if err := validateArgs(c.Args); err != nil {
145-
return fmt.Errorf("invalid args for %s: %w", constants.HomeInitComponentName, err)
146-
}
147-
}
148-
149-
// Always validate - should not be provided
150-
if len(c.VolumeMounts) > 0 {
151-
return fmt.Errorf("volumeMounts are not allowed for %s; persistent-home is auto-mounted at /home/user/", constants.HomeInitComponentName)
152-
}
153-
154-
// Always validate - should not be provided
155-
if err := validateNoAdvancedFields(c); err != nil {
156-
return err
157-
}
158-
159-
return nil
160-
}
161-
162-
func validateImageReference(image string) error {
163-
if image == "" {
164-
return fmt.Errorf("image reference cannot be empty")
165-
}
166-
167-
// whitespace and control characters
168-
if strings.ContainsAny(image, "\n\r\t ") {
169-
return fmt.Errorf("contains invalid whitespace characters")
170-
}
171-
172-
// other control characters
173-
for _, r := range image {
174-
if r < 0x20 || r == 0x7F {
175-
return fmt.Errorf("contains invalid control characters")
176-
}
177-
}
178-
179-
// format: [registry[:port]/]repository[:tag][@digest]
180-
imagePattern := regexp.MustCompile(`^([a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])*|\[?[0-9a-fA-F:]+]?)(:\d{1,5})?(/[a-zA-Z0-9]([a-zA-Z0-9._/-]*[a-zA-Z0-9])*)*(:[a-zA-Z0-9_.-]+)?(@sha256:[a-f0-9]{64})?$`)
181-
if !imagePattern.MatchString(image) {
182-
return fmt.Errorf("invalid format: should match regex: %s", imagePattern.String())
183-
}
184-
185-
// port range
186-
portMatch := regexp.MustCompile(`:(\d{1,5})(/|:|@|$)`)
187-
matches := portMatch.FindStringSubmatch(image)
188-
if len(matches) > 1 {
189-
port, err := strconv.Atoi(matches[1])
190-
if err != nil {
191-
return fmt.Errorf("invalid port format: %w", err)
192-
}
193-
if port < 1 || port > 65535 {
194-
return fmt.Errorf("invalid port number: %d (must be 1-65535)", port)
195-
}
196-
}
197-
198-
// length check
199-
if len(image) > 4096 {
200-
return fmt.Errorf("length exceeds 4096 characters")
201-
}
202-
203-
return nil
204-
}
205-
206-
// validateCommand validates the Command field for the init-persistent-home container.
207-
// This validation prevents resource exhaustion and data corruption while allowing flexibility
208-
// for enterprise customization.
209-
// Generated by Claude Sonnet 4.5
210-
func validateCommand(command []string) error {
211-
if len(command) > maxCommandElements {
212-
return fmt.Errorf("command cannot exceed %d elements (got %d)", maxCommandElements, len(command))
213-
}
214-
for i, cmd := range command {
215-
if len(cmd) > maxCommandElementLength {
216-
return fmt.Errorf("command[%d] exceeds %d characters (got %d)", i, maxCommandElementLength, len(cmd))
217-
}
218-
if err := validateStringContent(cmd, fmt.Sprintf("command[%d]", i)); err != nil {
219-
return err
220-
}
221-
}
222-
return nil
223-
}
224-
225-
// validateArgs validates the Args field for the init-persistent-home container.
226-
// This validation prevents resource exhaustion and data corruption while allowing flexibility
227-
// for enterprise customization.
228-
// Generated by Claude Sonnet 4.5
229-
func validateArgs(args []string) error {
230-
if len(args) > maxArgsElements {
231-
return fmt.Errorf("args cannot exceed %d elements (got %d)", maxArgsElements, len(args))
232-
}
233-
for i, arg := range args {
234-
if len(arg) > maxArgsElementLength {
235-
return fmt.Errorf("args[%d] exceeds %d bytes (got %d bytes)", i, maxArgsElementLength, len(arg))
236-
}
237-
if err := validateStringContent(arg, fmt.Sprintf("args[%d]", i)); err != nil {
238-
return err
239-
}
240-
}
241-
return nil
242-
}
243-
244-
// validateStringContent validates the content of command/args strings to prevent
245-
// data corruption and ensure valid UTF-8 encoding.
246-
// Generated by Claude Sonnet 4.5
247-
func validateStringContent(s, fieldName string) error {
248-
// No null bytes (breaks JSON marshaling and container runtime)
249-
if strings.Contains(s, "\x00") {
250-
return fmt.Errorf("%s contains null bytes", fieldName)
251-
}
252-
253-
// Check for invalid control characters (allow \n, \t which are common in scripts)
254-
for _, r := range s {
255-
if r < 0x20 && r != '\n' && r != '\t' {
256-
return fmt.Errorf("%s contains invalid control character (U+%04X)", fieldName, r)
257-
}
258-
if r == 0x7F {
259-
return fmt.Errorf("%s contains DEL control character", fieldName)
260-
}
261-
}
262-
263-
// Must be valid UTF-8
264-
if !utf8.ValidString(s) {
265-
return fmt.Errorf("%s contains invalid UTF-8", fieldName)
266-
}
267-
268-
return nil
269-
}
270-
27171
// ensureHomeInitContainerFields ensures that an init-persistent-home container has
272-
// the correct Command, Args, and VolumeMounts.
72+
// the correct Command and VolumeMounts.
27373
func ensureHomeInitContainerFields(c *corev1.Container) error {
27474
// Set default command only if not provided
27575
if len(c.Command) == 0 {
27676
c.Command = []string{"/bin/sh", "-c"}
27777
}
278-
// Args are validated separately in validateCommandAndArgs
27978
c.VolumeMounts = []corev1.VolumeMount{{
28079
Name: constants.HomeVolumeName,
28180
MountPath: constants.HomeUserDirectory,
@@ -589,7 +388,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
589388
// Check if init-persistent-home should be disabled
590389
disableHomeInit := pointer.BoolDeref(workspace.Config.Workspace.PersistUserHome.DisableInitContainer, constants.DefaultDisableHomeInitContainer)
591390

592-
// Prepare patches: filter and preprocess init containers from config
391+
// Filter init containers from config based on workspace settings
593392
patches := []corev1.Container{}
594393
for _, container := range workspace.Config.Workspace.InitContainers {
595394
// Special handling for init-persistent-home
@@ -604,10 +403,6 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
604403
reqLogger.Info("Skipping init-persistent-home container: DisableInitContainer is true")
605404
continue
606405
}
607-
// Validate custom home init container
608-
if err := validateHomeInitContainer(container); err != nil {
609-
return r.failWorkspace(workspace, fmt.Sprintf("Invalid %s container: %s", constants.HomeInitComponentName, err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
610-
}
611406
}
612407
patches = append(patches, container)
613408
}
@@ -618,11 +413,11 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
618413
return r.failWorkspace(workspace, fmt.Sprintf("Failed to merge init containers: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
619414
}
620415

621-
// Ensure init-persistent-home container have correct fields after merge
416+
// Ensure init-persistent-home container has correct fields after merge
622417
for i := range merged {
623418
if merged[i].Name == constants.HomeInitComponentName {
624419
if err := ensureHomeInitContainerFields(&merged[i]); err != nil {
625-
return r.failWorkspace(workspace, fmt.Sprintf("Invalid %s container: %s", constants.HomeInitComponentName, err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
420+
return r.failWorkspace(workspace, fmt.Sprintf("Failed to configure %s container: %s", constants.HomeInitComponentName, err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
626421
}
627422
}
628423
}

0 commit comments

Comments
 (0)