From 30e47213b9be0046677f5e2b67c7b53c76fcba0a Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Tue, 18 Aug 2026 05:33:17 +0800 Subject: [PATCH] Expose portable Python requirement validation Add canonical direct-root requirement validation and normalized distribution-name extraction to the Python provider. Direct URLs, environment markers, embedded whitespace, malformed extras, and non-canonical specifier sets are rejected so an immutable catalog root cannot depend on external location or runtime state. Add canonical interpreter-version validation for the major.minor and major.minor.patch forms used by portable binding compatibility lists, rejecting non-numeric, non-canonical, and out-of-range components. These helpers are exposed for the portable tool catalog slices that follow and change no existing provider behavior. Delivers PTD-01 of docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md. --- internal/providers/python/package_request.go | 52 +++++++++++++ .../providers/python/package_request_test.go | 75 +++++++++++++++++++ internal/providers/python/version.go | 23 ++++++ .../providers/python/version_override_test.go | 13 ++++ 4 files changed, 163 insertions(+) diff --git a/internal/providers/python/package_request.go b/internal/providers/python/package_request.go index 678b5731..1cf1e07c 100644 --- a/internal/providers/python/package_request.go +++ b/internal/providers/python/package_request.go @@ -8,6 +8,7 @@ import ( "unicode" "unicode/utf8" + pep440 "github.com/aquasecurity/go-pep440-version" "github.com/omry/reploy/internal/canonical" "github.com/omry/reploy/internal/providers" ) @@ -62,6 +63,57 @@ func ValidateCanonicalPackageRequestV1(request providers.CanonicalPackageRequest return nil } +// PackageRootDistributionNameV1 validates the complete, resolver-supported +// grammar for a direct distribution root and returns its normalized +// distribution name. Catalog records intentionally exclude direct URLs and +// environment markers because those would make an immutable root depend on +// external location or runtime state. Extras are rejected for the same reason: +// a root that selects optional dependency groups is not an exact, immutable +// coordinate. +func PackageRootDistributionNameV1(requirement string) (string, error) { + request, err := CanonicalPackageRequestV1(requirement) + if err != nil { + return "", err + } + value := request.Value["requirement"].(string) + if strings.IndexFunc(value, unicode.IsSpace) >= 0 { + return "", fmt.Errorf("Python package root requirement must not contain whitespace") + } + name := requirementNamePattern.FindString(value) + if !validPackageRequirementIdentifierV1(name) { + return "", fmt.Errorf("invalid Python package root requirement %q", requirement) + } + remainder := strings.TrimPrefix(value, name) + if strings.HasPrefix(remainder, "[") { + return "", fmt.Errorf("Python package root requirement %q must not request extras", requirement) + } + if remainder == "" { + return NormalizeDistributionName(name), nil + } + specifiers, err := pep440.NewSpecifiers(remainder) + if err != nil || specifiers.String() != remainder { + return "", fmt.Errorf("invalid Python package root requirement %q", requirement) + } + return NormalizeDistributionName(name), nil +} + +func validPackageRequirementIdentifierV1(value string) bool { + if value == "" || !asciiAlphaNumericV1(value[0]) || !asciiAlphaNumericV1(value[len(value)-1]) { + return false + } + for _, character := range value { + if character >= 'A' && character <= 'Z' || character >= 'a' && character <= 'z' || character >= '0' && character <= '9' || character == '.' || character == '_' || character == '-' { + continue + } + return false + } + return true +} + +func asciiAlphaNumericV1(value byte) bool { + return value >= 'A' && value <= 'Z' || value >= 'a' && value <= 'z' || value >= '0' && value <= '9' +} + // ProviderRequestDistributionsV1 returns the normalized direct distribution // roots in one canonical Python provider request. It does not evaluate or // resolve dependencies. diff --git a/internal/providers/python/package_request_test.go b/internal/providers/python/package_request_test.go index 6f1a6a97..cf96ddd3 100644 --- a/internal/providers/python/package_request_test.go +++ b/internal/providers/python/package_request_test.go @@ -1,6 +1,7 @@ package python import ( + "fmt" "reflect" "strings" "testing" @@ -55,6 +56,80 @@ func TestCanonicalPackageRequestV1RejectsPackageManagerOptions(t *testing.T) { } } +func TestPackageRootDistributionNameV1(t *testing.T) { + for _, accepted := range []struct { + requirement string + want string + }{ + {requirement: "demo", want: "demo"}, + {requirement: "demo>=1.2,<2", want: "demo"}, + {requirement: "demo==1.2.3", want: "demo"}, + {requirement: "d", want: "d"}, + } { + name, err := PackageRootDistributionNameV1(accepted.requirement) + if err != nil { + t.Errorf("PackageRootDistributionNameV1(%q): %v", accepted.requirement, err) + } + if name != accepted.want { + t.Errorf("PackageRootDistributionNameV1(%q) = %q, want %q", accepted.requirement, name, accepted.want) + } + } + for _, testCase := range []struct { + name string + requirement string + }{ + {name: "whitespace", requirement: "demo ???"}, + {name: "trailing dash", requirement: "demo-"}, + {name: "trailing dots", requirement: "demo.."}, + {name: "invalid extra", requirement: "demo[http-]"}, + {name: "unterminated extras", requirement: "demo["}, + {name: "empty extras", requirement: "demo[]"}, + {name: "extras", requirement: "demo[http]"}, + {name: "extras with specifier", requirement: "demo[http]>=1.2,<2"}, + {name: "multiple extras", requirement: "demo[a,b,c]"}, + {name: "unsorted extras", requirement: "demo[b,a]"}, + {name: "duplicate extras", requirement: "demo[a,a]"}, + {name: "direct URL", requirement: "demo @ https://example.invalid/demo.whl"}, + {name: "environment marker", requirement: "demo; python_version > '3'"}, + {name: "empty", requirement: ""}, + } { + if _, err := PackageRootDistributionNameV1(testCase.requirement); err == nil { + t.Errorf("%s: PackageRootDistributionNameV1(%q) succeeded", testCase.name, testCase.requirement) + } + } +} + +func TestPackageRootDistributionNameV1Limits(t *testing.T) { + longName := strings.Repeat("a", 4096) + if name, err := PackageRootDistributionNameV1(longName); err != nil || name != longName { + t.Errorf("long distribution name = %q, %v", name, err) + } + manyExtras := make([]string, 128) + for index := range manyExtras { + manyExtras[index] = fmt.Sprintf("e%04d", index) + } + requirement := "demo[" + strings.Join(manyExtras, ",") + "]" + if _, err := PackageRootDistributionNameV1(requirement); err == nil { + t.Error("many extras succeeded") + } + longSpecifier := "demo" + strings.Repeat(">=1,", 64) + ">=1" + if _, err := PackageRootDistributionNameV1(longSpecifier); err != nil { + t.Errorf("long specifier set = %v", err) + } +} + +func TestPackageRootDistributionNameV1NormalizesIdentically(t *testing.T) { + for _, requirement := range []string{"Demo", "DEMO", "demo", "De_mo", "De-mo", "de.mo"} { + name, err := PackageRootDistributionNameV1(requirement) + if err != nil { + t.Fatalf("PackageRootDistributionNameV1(%q): %v", requirement, err) + } + if name != NormalizeDistributionName(requirement) { + t.Errorf("PackageRootDistributionNameV1(%q) = %q, want %q", requirement, name, NormalizeDistributionName(requirement)) + } + } +} + func TestProviderRequestDistributionsV1ReturnsSortedDirectRoots(t *testing.T) { zeta, err := CanonicalPackageRequestV1("Zeta[extra]>=1") if err != nil { diff --git a/internal/providers/python/version.go b/internal/providers/python/version.go index 47844093..0fc04623 100644 --- a/internal/providers/python/version.go +++ b/internal/providers/python/version.go @@ -21,6 +21,29 @@ func ValidatePackageVersionV1(value string) error { return nil } +// ValidateInterpreterVersionV1 accepts the canonical major.minor or +// major.minor.patch release form used by portable binding compatibility lists. +func ValidateInterpreterVersionV1(value string) error { + parts := strings.Split(value, ".") + if len(parts) < 2 || len(parts) > 3 { + return fmt.Errorf("Python interpreter version %q must use major.minor or major.minor.patch", value) + } + for _, part := range parts { + if part == "" || len(part) > 1 && part[0] == '0' { + return fmt.Errorf("Python interpreter version %q is not canonical", value) + } + for _, character := range part { + if character < '0' || character > '9' { + return fmt.Errorf("Python interpreter version %q is not canonical", value) + } + } + if _, err := strconv.Atoi(part); err != nil { + return fmt.Errorf("Python interpreter version %q has an out-of-range component", value) + } + } + return nil +} + // ComparePackageVersionsV1 compares valid PEP 440 versions. func ComparePackageVersionsV1(left string, right string) (int, error) { leftVersion, err := pep440.Parse(left) diff --git a/internal/providers/python/version_override_test.go b/internal/providers/python/version_override_test.go index f1045f87..501e6d08 100644 --- a/internal/providers/python/version_override_test.go +++ b/internal/providers/python/version_override_test.go @@ -21,3 +21,16 @@ func TestPackageOverrideVersionUsesPEP440ValidationAndOrdering(t *testing.T) { t.Fatalf("final release comparison = %d, want newer than development release", compared) } } + +func TestValidateInterpreterVersionV1(t *testing.T) { + for _, value := range []string{"3.11", "3.13.2"} { + if err := ValidateInterpreterVersionV1(value); err != nil { + t.Errorf("ValidateInterpreterVersionV1(%q): %v", value, err) + } + } + for _, value := range []string{"banana", "3..11", "03.11", "3", "999999999999999999999.1"} { + if err := ValidateInterpreterVersionV1(value); err == nil { + t.Errorf("ValidateInterpreterVersionV1(%q) succeeded", value) + } + } +}