diff --git a/pkg/workflow/argument_injection_test.go b/pkg/workflow/argument_injection_test.go index e77bb8bfac0..3dfec340e2a 100644 --- a/pkg/workflow/argument_injection_test.go +++ b/pkg/workflow/argument_injection_test.go @@ -395,3 +395,34 @@ func TestValidatePipPackageName(t *testing.T) { }) } } + +// TestValidateUvPackages_RejectsInvalidPackageName verifies that uv package names +// which do not conform to the PyPI naming rules are rejected before being passed +// as arguments to the uv CLI. This validation happens before uv/pip is resolved +// or invoked, so the test is deterministic regardless of whether uv is installed +// on the host running the test. +func TestValidateUvPackages_RejectsInvalidPackageName(t *testing.T) { + compiler := NewCompiler() + err := compiler.validateUvPackages(&WorkflowData{ + CustomSteps: "uvx pkg;whoami", + }) + if err == nil { + t.Fatal("expected error for invalid uv package name but got none") + } + if !strings.Contains(err.Error(), "invalid pip package name") { + t.Errorf("expected error to mention invalid package name, got: %v", err) + } +} + +// TestValidateUvPackages_AcceptsVersionedUvxSpec verifies that versioned uvx +// package specs (e.g. "ruff@0.1.0") are not rejected by the PEP 508 name +// validation, since extractUvFromCommands explicitly supports this syntax. +func TestValidateUvPackages_AcceptsVersionedUvxSpec(t *testing.T) { + compiler := NewCompiler() + err := compiler.validateUvPackages(&WorkflowData{ + CustomSteps: "uvx ruff@0.1.0", + }) + if err != nil && strings.Contains(err.Error(), "invalid pip package name") { + t.Errorf("expected versioned uvx spec to be accepted as a valid package name, got: %v", err) + } +} diff --git a/pkg/workflow/pip_validation.go b/pkg/workflow/pip_validation.go index 318f62cdd97..0ce72bd6c16 100644 --- a/pkg/workflow/pip_validation.go +++ b/pkg/workflow/pip_validation.go @@ -56,11 +56,9 @@ func (c *Compiler) validatePythonPackagesWithPip(packages []string, packageType pipValidationLog.Printf("Validating %d %s packages using %s", len(packages), packageType, pipPath) for _, pkg := range packages { - // Extract package name without version specifier - pkgName := pkg - if eqIndex := strings.Index(pkg, "=="); eqIndex > 0 { - pkgName = pkg[:eqIndex] - } + // Extract package name without version specifier (pip-style "==version" + // or uvx-style "@version", e.g. "ruff@0.1.0"). + pkgName := stripUvPackageVersion(pkg) // Reject names starting with '-' to prevent argument injection if strings.HasPrefix(pkgName, "-") { @@ -130,6 +128,19 @@ func (c *Compiler) validatePipPackages(workflowData *WorkflowData) error { return nil } +// stripUvPackageVersion extracts the bare package name from a uv package spec, +// stripping a trailing "==version" (pip-style) or "@version" (uvx-style, e.g. +// "ruff@0.1.0") specifier if present. +func stripUvPackageVersion(pkg string) string { + if eqIndex := strings.Index(pkg, "=="); eqIndex > 0 { + return pkg[:eqIndex] + } + if atIndex := strings.Index(pkg, "@"); atIndex > 0 { + return pkg[:atIndex] + } + return pkg +} + // validateUvPackages validates that uv packages are available func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error { packages := extractUvPackages(workflowData) @@ -147,6 +158,27 @@ func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error { return err } + // Validate package name syntax (PEP 508) upfront, before resolving or invoking + // uv/pip and independent of whether those tools are installed. This ensures + // argument-injection attempts (e.g. "pkg;whoami") are rejected even in + // environments without uv or pip available. + var invalidNameErrors []string + for _, pkg := range packages { + pkgName := stripUvPackageVersion(pkg) + if err := validatePipPackageName(pkgName); err != nil { + pipValidationLog.Printf("Invalid uv package name %s: %v", pkgName, err) + invalidNameErrors = append(invalidNameErrors, fmt.Sprintf("uv package '%s' is invalid: %v", pkg, err)) + } + } + if len(invalidNameErrors) > 0 { + return NewValidationError( + "uv.packages", + fmt.Sprintf("%d package name(s) invalid", len(invalidNameErrors)), + "uv package name(s) do not conform to PyPI naming rules (PEP 508)", + "Package names must start and end with a letter or digit, with hyphens, underscores, or dots allowed inside (e.g. \"requests\" or \"my-package\"). Optionally followed by a \"==version\" or \"@version\" specifier.\n\nValidation details:\n"+strings.Join(invalidNameErrors, "\n"), + ) + } + // Check if uv is available uvPath, err := fileutil.ResolveExecutablePath("uv") if err != nil { @@ -180,15 +212,16 @@ func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error { pipValidationLog.Print("Using uv command for validation") // Validate with uv + // Package names were already validated against PyPI naming rules (PEP 508) above, + // before this point, so pkgName below is safe to pass as a command argument. var errors []string for _, pkg := range packages { - // Extract package name without version specifier - pkgName := pkg - if eqIndex := strings.Index(pkg, "=="); eqIndex > 0 { - pkgName = pkg[:eqIndex] - } + pkgName := stripUvPackageVersion(pkg) // Use uv pip show to check if package exists on PyPI + // #nosec G204 -- uvPath is resolved from the hardcoded executable name "uv" via + // fileutil.ResolveExecutablePath; pkgName is validated above by validatePipPackageName + // against the strict PyPI PEP 508 allowlist. cmd := exec.Command(uvPath, "pip", "show", pkgName, "--no-cache") _, err := cmd.CombinedOutput()