Copilot commented on code in PR #142:
URL:
https://github.com/apache/skywalking-infra-e2e/pull/142#discussion_r3004898913
##########
internal/components/collector/kind.go:
##########
@@ -151,6 +159,88 @@ func collectPodDescribe(kubeConfigPath, outputDir,
namespace, podName string) er
return nil
}
+// containsGlob reports whether the path contains glob metacharacters.
+func containsGlob(path string) bool {
+ if strings.ContainsAny(path, "*?") {
+ return true
+ }
+ // Only treat '[' as a glob when followed by a matching ']' with at
least
+ // one character between them, so literal brackets (e.g. "app[1].log")
+ // that don't form a valid character class are not misidentified.
+ for i := 0; i < len(path); i++ {
+ if path[i] != '[' {
+ continue
+ }
+ for j := i + 1; j < len(path); j++ {
+ if path[j] != ']' {
+ continue
+ }
+ // Check there is at least one non-']' char between '['
and ']'.
+ for k := i + 1; k < j; k++ {
+ if path[k] != ']' {
+ return true
+ }
+ }
+ break
+ }
+ }
+ return false
+}
+
+// validPathPattern matches paths that contain only safe characters for shell
interpolation.
+// Allowed: alphanumeric, /, ., -, _, *, ?, [, ].
+var validPathPattern = regexp.MustCompile(`^[a-zA-Z0-9/_.*?\[\]\-]+$`)
+
+// validateGlobPattern checks that a glob pattern contains only safe characters
+// to prevent shell injection when interpolated into sh -c commands.
+func validateGlobPattern(pattern string) error {
+ if !validPathPattern.MatchString(pattern) {
+ return fmt.Errorf("glob pattern %q contains unsupported
characters", pattern)
+ }
+ return nil
+}
+
+// expandPodGlob expands a glob pattern inside a pod. If the path has no glob
+// characters it is returned as-is. Otherwise kubectl exec runs sh to expand
+// the pattern and returns the matched paths.
+func expandPodGlob(kubeConfigPath, namespace, podName, container, pattern
string) ([]string, error) {
+ if !containsGlob(pattern) {
+ return []string{pattern}, nil
+ }
+
+ if err := validateGlobPattern(pattern); err != nil {
+ return nil, err
+ }
+
+ cmd := fmt.Sprintf("kubectl --kubeconfig %s -n %s exec %s",
kubeConfigPath, namespace, podName)
+ if container != "" {
+ cmd += fmt.Sprintf(" -c %s", container)
+ }
+ cmd += fmt.Sprintf(" -- sh -c 'ls -d -- %s 2>/dev/null || true'",
pattern)
Review Comment:
This command is built via string interpolation and executed through
`util.ExecuteCommand` (which runs `bash -ec`). Only the glob `pattern` is
validated, but `kubeConfigPath` (and to a lesser extent `container`) are still
unquoted, which can break when paths contain spaces and leaves room for
shell-injection if config inputs are attacker-controlled. Also, `2>/dev/null ||
true` suppresses real errors (e.g., permission denied / missing `ls`) and can
incorrectly report “matched no files”; consider a safer expansion approach that
distinguishes “no matches” from other failures and avoids shell parsing issues
(e.g., robust quoting/escaping or passing args without an outer shell).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
