From 1282ce47ecbb12767a66abbe7a8bb17fd1a84ed6 Mon Sep 17 00:00:00 2001 From: Gauri Yadav Date: Thu, 9 Jul 2026 09:47:37 +0530 Subject: [PATCH 1/3] XRAY-146488 - Onboarding pip env --- cli/docs/scan/curation/help.go | 4 +- commands/curation/curationaudit.go | 73 ++- commands/curation/curationaudit_test.go | 21 + .../buildinfo/technologies/python/python.go | 451 +++++++++++++- .../technologies/python/python_test.go | 557 ++++++++++++++++++ 5 files changed, 1088 insertions(+), 18 deletions(-) diff --git a/cli/docs/scan/curation/help.go b/cli/docs/scan/curation/help.go index 182736e7e..b2b7d2c28 100644 --- a/cli/docs/scan/curation/help.go +++ b/cli/docs/scan/curation/help.go @@ -8,13 +8,13 @@ func GetAIDescription() string { return `Inspect the project's package-manager dependencies against the JFrog Curation service and report which packages were blocked by curation policy along with the matching policies. Use when an agent suspects a package install failure was due to curation, or wants a preemptive curation report. When to use: -- Diagnose 403/forbidden errors during npm/pip/maven/gradle/nuget/go install steps in a curation-enabled remote. +- Diagnose 403/forbidden errors during npm/pip/pipenv/poetry/maven/gradle/nuget/go install steps in a curation-enabled remote. - Produce a curation-status report (blocked packages and policies) for the current project. - Run automatically after a failed install via the JFROG_CLI_SKIP_CURATION_AFTER_FAILURE workflow. Prerequisites: - A configured JFrog Platform server (jf c add) with JFrog Curation entitlement. -- Project must use a supported package manager (npm, yarn, pip, maven, gradle, nuget, go) resolved through a curation-configured remote. +- Project must use a supported package manager (npm, yarn, pnpm, pip, pipenv, poetry, maven, gradle, nuget, go) resolved through a curation-configured remote. - The package manager and its lockfile must be present in the working directory. Common patterns: diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 0d9bb4101..f8f2ae76f 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -128,6 +128,9 @@ var supportedTech = map[techutils.Technology]func(ca *CurationAuditCommand) (boo techutils.Poetry: func(ca *CurationAuditCommand) (bool, error) { return ca.checkSupportByVersionOrEnv(techutils.Poetry, MinArtiPassThroughSupport) }, + techutils.Pipenv: func(ca *CurationAuditCommand) (bool, error) { + return ca.checkSupportByVersionOrEnv(techutils.Pipenv, MinArtiPassThroughSupport) + }, } func (ca *CurationAuditCommand) checkSupportByVersionOrEnv(tech techutils.Technology, minArtiVersion string) (bool, error) { @@ -718,6 +721,14 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if err := validateRunNativeForTech(tech, ca.RunNative()); err != nil { return err } + // When JFROG_CLI_CURATION_SUPPORT=true, checkSupportByVersionOrEnv short-circuits + // before calling getRtVersion→GetAuth→SetRepo, so ca.DepsRepo() is never populated + // for Pipenv. Call SetRepo explicitly so getBuildInfoParamsByTech sees the right repo. + if tech == techutils.Pipenv && ca.PackageManagerConfig == nil { + if err := ca.SetRepo(tech); err != nil { + return err + } + } params, err := ca.getBuildInfoParamsByTech() if err != nil { return errorutils.CheckErrorf("failed to get build info params for %s: %v", tech.String(), err) @@ -751,7 +762,7 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map // Instead of aborting with no output, run the metadata-API fallback to // recover the curation policy and render a partial table. var cvsErr *python.CvsBlockedError - if (tech == techutils.Pip || tech == techutils.Poetry) && errors.As(err, &cvsErr) { + if (tech == techutils.Pip || tech == techutils.Poetry || tech == techutils.Pipenv) && errors.As(err, &cvsErr) { return ca.runCvsFallback(cvsErr, tech, results) } return err @@ -1056,6 +1067,10 @@ func (ca *CurationAuditCommand) SetRepo(tech techutils.Technology) error { return ca.setRepoFromNpmrcForPnpm() } + if tech == techutils.Pipenv { + return ca.setRepoFromPipfile() + } + // Yarn V4 uses native mode: no jf yarn-config / yarn.yaml required. // Detect the running yarn version and route to the appropriate path. // Version detection failures are fatal — silently falling through to the @@ -1106,6 +1121,60 @@ func (ca *CurationAuditCommand) SetRepo(tech techutils.Technology) error { return nil } +// setRepoFromPipfile detects the Artifactory PyPI source for pipenv curation. +// +// Detection priority: +// 1. pipenv.yaml — explicit 'jf pipenv-config' (highest priority, JFrog-specific). +// 2. ~/.pip/pip.conf index-url — customer followed the Artifactory "Set me up" +// guide for pip (most common real-world setup; no Pipfile changes needed). +// 3. Pipfile [[source]] — customer explicitly added an Artifactory source URL +// directly in the Pipfile. +func (ca *CurationAuditCommand) setRepoFromPipfile() error { + // 1. pipenv.yaml — explicit 'jf pipenv-config'. + resolverParams, yamlErr := ca.getRepoParams(techutils.Pipenv.GetProjectType()) + if yamlErr == nil { + ca.setPackageManagerConfig(resolverParams) + ca.SetDepsRepo(resolverParams.TargetRepo()) + return nil + } + + // 2. ~/.pip/pip.conf index-url (Artifactory "Set me up" path). + pipConfPath := python.DefaultPipConfPath() + sd, repo, err := python.ParsePipConfigIndexUrl(pipConfPath) + if err != nil { + log.Debug(fmt.Sprintf("pipenv native: failed to read pip.conf: %v", err)) + } + if repo != "" && sd != nil { + repoConfig := (&project.RepositoryConfig{}). + SetTargetRepo(repo). + SetServerDetails(sd) + ca.setPackageManagerConfig(repoConfig) + ca.SetDepsRepo(repo) + log.Info(fmt.Sprintf("pipenv: using Artifactory repository %q from %s", repo, pipConfPath)) + return nil + } + + // 3. Pipfile [[source]] — Artifactory URL declared directly in the Pipfile. + sd, repo, err = python.ParsePipfileArtifactorySource("Pipfile") + if err != nil { + log.Debug(fmt.Sprintf("pipenv native: failed to read Pipfile: %v", err)) + } + if repo != "" && sd != nil { + repoConfig := (&project.RepositoryConfig{}). + SetTargetRepo(repo). + SetServerDetails(sd) + ca.setPackageManagerConfig(repoConfig) + ca.SetDepsRepo(repo) + log.Info(fmt.Sprintf("pipenv: using Artifactory repository %q from Pipfile [[source]]", repo)) + return nil + } + + return errorutils.CheckErrorf( + "curation-audit for pipenv requires an Artifactory PyPI resolver. " + + "Either run 'jf pipenv-config', configure index-url in ~/.pip/pip.conf via " + + "Artifactory 'Set me up', or add an Artifactory [[source]] entry to your Pipfile.") +} + // validateRunNativeForTech rejects --run-native for techs that don't implement // native-config semantics. npm uses it to read Artifactory details from .npmrc; // pnpm accepts it as a no-op (it always resolves from .npmrc). Extend the @@ -1801,7 +1870,7 @@ func getUrlNameAndVersionByTech(tech techutils.Technology, node *xrayUtils.Graph return getGradleNameScopeAndVersion(node.Id, artiUrl, repo, node) case techutils.Gem: return getGemNameScopeAndVersion(node.Id, artiUrl, repo) - case techutils.Pip, techutils.Poetry: + case techutils.Pip, techutils.Poetry, techutils.Pipenv: downloadUrls, name, version = getPythonNameVersion(node.Id, downloadUrlsMap) return case techutils.Go: diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 96297df2c..e07c0767f 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -2875,3 +2875,24 @@ func TestPromoteYarnWorkspaceMember(t *testing.T) { assert.NotContains(t, result, yarn, "npm must not be promoted to yarn from a $HOME-level indicator") }) } + +// ============================================================================= +// Tests for Pipenv support added to curationaudit.go. +// ============================================================================= + +func TestSupportedTechContainsPipenv(t *testing.T) { + _, ok := supportedTech[techutils.Pipenv] + assert.True(t, ok, "techutils.Pipenv must be registered in supportedTech so that 'jf curation-audit' processes pipenv projects") +} + +func TestGetUrlNameAndVersionByTechPipenv(t *testing.T) { + const whlUrl = "https://test.jfrog.io/artifactory/api/pypi/pypi-remote/packages/aa/bb/requests-2.31.0-py3-none-any.whl" + downloadUrlsMap := map[string]string{"pypi://requests:2.31.0": whlUrl} + + downloadUrls, name, scope, ver := getUrlNameAndVersionByTech(techutils.Pipenv, &xrayUtils.GraphNode{Id: "pypi://requests:2.31.0"}, downloadUrlsMap, "https://test.jfrog.io", "pypi-remote") + + assert.Equal(t, []string{whlUrl}, downloadUrls) + assert.Equal(t, "requests", name) + assert.Equal(t, "", scope, "python packages have no scope") + assert.Equal(t, "2.31.0", ver) +} diff --git a/sca/bom/buildinfo/technologies/python/python.go b/sca/bom/buildinfo/technologies/python/python.go index 75e09317b..50ae25dd0 100644 --- a/sca/bom/buildinfo/technologies/python/python.go +++ b/sca/bom/buildinfo/technologies/python/python.go @@ -50,6 +50,8 @@ const ( PoetryNoInteractionFlag = "--no-interaction" pyprojectToml = "pyproject.toml" CurationPoetryMinimumVersion = "1.2.0" + CurationPipenvMinimumVersion = "2023.7.4" + pipfileFile = "Pipfile" poetryDownloadUrlWorkers = 8 ) @@ -63,6 +65,10 @@ var ( // deprecation notices on stdout before this line, so we scan the full // output rather than assuming a single-line response. poetryVersionRegex = regexp.MustCompile(`Poetry \(?version\s+([^)\s]+)\)?`) + + // pipenvVersionRegex matches the date-based version emitted by `pipenv --version`, + // e.g. "pipenv, version 2023.7.4" or "pipenv, version 2026.6.1". + pipenvVersionRegex = regexp.MustCompile(`pipenv,\s+version\s+(\S+)`) ) // parsePoetryVersion extracts the semantic version (e.g. "1.2.2") from the @@ -204,6 +210,9 @@ func getDependencies(params technologies.BuildInfoBomGeneratorParams, technology case techutils.Poetry: downloadUrls, err = buildPoetryDownloadUrlsMap(params.ServerDetails, params.DependenciesRepository) log.Debug(fmt.Sprintf("Poetry: curation download-URL map built — %d packages resolved", len(downloadUrls))) + case techutils.Pipenv: + downloadUrls, err = buildPipenvDownloadUrlsMap(params.ServerDetails, params.DependenciesRepository) + log.Debug(fmt.Sprintf("Pipenv: curation download-URL map built — %d packages resolved", len(downloadUrls))) } return } @@ -333,10 +342,8 @@ func resolvePoetryPackageURL(rtManager artifactory.ArtifactoryServicesManager, h return nil } -// buildPoetryDownloadUrl is the Poetry equivalent of npm's buildNpmDownloadUrl: given a -// package, it returns the absolute Artifactory download URL that curation will HEAD against. -// It does so by fetching the package's simple-index HTML and matching one of the filenames -// recorded in poetry.lock against the listed s. +// buildPoetryDownloadUrl fetches the simple-index HTML for a package and returns +// the Artifactory download URL matching a filename in poetry.lock. func buildPoetryDownloadUrl(rtManager artifactory.ArtifactoryServicesManager, clientDetails *httputils.HttpClientDetails, artiUrl, repository string, pkg poetryLockPackage) (string, error) { normalized := NormalizePypiName(pkg.Name) simpleIndexUrl := fmt.Sprintf("%s/api/pypi/%s/simple/%s/", artiUrl, repository, normalized) @@ -370,9 +377,8 @@ func buildPoetryDownloadUrl(rtManager artifactory.ArtifactoryServicesManager, cl return absolute, nil } -// pickPoetryHrefByFilename scans the simple-index body for an whose filename -// (after stripping the optional "#sha256=..." fragment) matches one of the wanted filenames. -// Returns "" when no href matches. Mirrors the focused-helper style of npm's appendUniqueChild. +// pickPoetryHrefByFilename returns the first href whose filename (sans "#sha256=…" fragment) +// matches one of wantedFiles, or "" if none match. func pickPoetryHrefByFilename(body []byte, wantedFiles []string) string { wanted := make(map[string]struct{}, len(wantedFiles)) for _, f := range wantedFiles { @@ -406,6 +412,204 @@ func NormalizePypiName(name string) string { return b.String() } +// pipfileLockEntry is one package entry in Pipfile.lock's "default" or "develop" section. +type pipfileLockEntry struct { + Version string `json:"version"` // e.g. "==2.0.7" + Hashes []string `json:"hashes"` // e.g. ["sha256:abc123..."] +} + +// pipfileLockContent is the top-level structure of Pipfile.lock. +type pipfileLockContent struct { + Default map[string]pipfileLockEntry `json:"default"` + Develop map[string]pipfileLockEntry `json:"develop"` +} + +// pipfileLockPackage holds the name, resolved version, and file hashes for one +// locked package, extracted from Pipfile.lock for use in the curation URL map. +type pipfileLockPackage struct { + Name string + Version string // bare version, e.g. "2.0.7" (== prefix stripped) + Hashes []string // e.g. ["sha256:abc123..."] +} + +// buildPipenvDownloadUrlsMap reads Pipfile.lock, then resolves each package's +// download URL from the simple index so fetchNodeStatus can HEAD-probe for blocks. +func buildPipenvDownloadUrlsMap(serverDetails *config.ServerDetails, repository string) (map[string]string, error) { + if serverDetails == nil || serverDetails.GetArtifactoryUrl() == "" { + return nil, errorutils.CheckErrorf("server details with Artifactory URL are required for pipenv curation") + } + if repository == "" { + return nil, errorutils.CheckErrorf("a pipenv Artifactory repository must be configured for curation") + } + packages, err := readPipfileLockPackages() + if err != nil { + return nil, err + } + log.Debug(fmt.Sprintf("Pipenv: parsed %d package entries from Pipfile.lock", len(packages))) + rtAuth, err := serverDetails.CreateArtAuthConfig() + if err != nil { + return nil, err + } + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 2, 0, false) + if err != nil { + return nil, err + } + httpClientDetails := rtAuth.CreateHttpClientDetails() + artiUrl := strings.TrimSuffix(serverDetails.GetArtifactoryUrl(), "/") + urls := map[string]string{} + skipped := 0 + var mu sync.Mutex + + g := new(errgroup.Group) + g.SetLimit(poetryDownloadUrlWorkers) + for _, pkg := range packages { + if pkg.Name == "" || pkg.Version == "" || len(pkg.Hashes) == 0 { + skipped++ + continue + } + g.Go(func() error { + return resolvePipenvPackageURL(rtManager, httpClientDetails, artiUrl, repository, pkg, urls, &mu) + }) + } + _ = g.Wait() + + expected := len(packages) - skipped + resolved := len(urls) + if resolved < expected { + log.Warn(fmt.Sprintf( + "Pipenv: resolved download URLs for %d/%d packages — %d package(s) will not be HEAD-checked by curation. "+ + "Re-run with JFROG_CLI_LOG_LEVEL=DEBUG to see per-package resolution errors.", + resolved, expected, expected-resolved)) + } + return urls, nil +} + +func resolvePipenvPackageURL(rtManager artifactory.ArtifactoryServicesManager, httpClientDetails httputils.HttpClientDetails, artiUrl, repository string, pkg pipfileLockPackage, urls map[string]string, mu *sync.Mutex) error { + localDetails := httpClientDetails.Clone() + downloadUrl, lookupErr := buildPipenvDownloadUrl(rtManager, localDetails, artiUrl, repository, pkg) + if lookupErr != nil { + log.Debug(fmt.Sprintf("Pipenv: could not resolve download URL for %s:%s: %v", pkg.Name, pkg.Version, lookupErr)) + return nil + } + normalizedName := strings.ReplaceAll(strings.ToLower(strings.TrimSpace(pkg.Name)), "-", "_") + compId := PythonPackageTypeIdentifier + normalizedName + ":" + pkg.Version + mu.Lock() + urls[compId] = downloadUrl + mu.Unlock() + return nil +} + +// buildPipenvDownloadUrl fetches the regular (non-curation) simple-index page for +// a package and returns the absolute download URL whose sha256 fragment matches +// one of the hashes recorded in Pipfile.lock. +func buildPipenvDownloadUrl(rtManager artifactory.ArtifactoryServicesManager, clientDetails *httputils.HttpClientDetails, artiUrl, repository string, pkg pipfileLockPackage) (string, error) { + normalized := NormalizePypiName(pkg.Name) + simpleIndexUrl := fmt.Sprintf("%s/api/pypi/%s/simple/%s/", artiUrl, repository, normalized) + log.Debug(fmt.Sprintf("Pipenv: GET simple-index %s (matching against %d hashes)", simpleIndexUrl, len(pkg.Hashes))) + resp, body, _, err := rtManager.Client().SendGet(simpleIndexUrl, true, clientDetails) + if err != nil { + return "", err + } + if resp == nil || resp.StatusCode != http.StatusOK { + status := 0 + if resp != nil { + status = resp.StatusCode + } + return "", fmt.Errorf("simple-index GET returned status %d for %s", status, simpleIndexUrl) + } + + // Pipfile.lock stores hashes as "sha256:abc123"; the simple-index URL fragment + // uses "#sha256=abc123". Convert and build a lookup set. + wantedFragments := make(map[string]struct{}, len(pkg.Hashes)) + for _, h := range pkg.Hashes { + // "sha256:abc123" → "#sha256=abc123" + wantedFragments["#"+strings.Replace(h, ":", "=", 1)] = struct{}{} + } + + href := pickHrefByHashFragment(body, wantedFragments) + if href == "" { + return "", fmt.Errorf("no matching href found in simple index for %s (checked %d hashes)", pkg.Name, len(pkg.Hashes)) + } + base, err := url.Parse(simpleIndexUrl) + if err != nil { + return "", err + } + ref, err := url.Parse(href) + if err != nil { + return "", err + } + absolute := base.ResolveReference(ref) + // Strip the fragment — fetchNodeStatus only needs the bare download URL. + absolute.Fragment = "" + log.Debug(fmt.Sprintf("Pipenv: resolved %s:%s -> %s", pkg.Name, pkg.Version, absolute)) + return absolute.String(), nil +} + +// pickHrefByHashFragment scans the simple-index body for an whose URL +// fragment (the "#sha256=..." part) matches one of the wanted fragments. +func pickHrefByHashFragment(body []byte, wantedFragments map[string]struct{}) string { + hrefMatches := simpleIndexHrefEntry.FindAllStringSubmatch(string(body), -1) + for _, m := range hrefMatches { + href := m[1] + if idx := strings.Index(href, "#"); idx >= 0 { + if _, ok := wantedFragments[href[idx:]]; ok { + return href + } + } + } + return "" +} + +func readPipfileLockPackages() ([]pipfileLockPackage, error) { + exists, err := fileutils.IsFileExists("Pipfile.lock", false) + if err != nil { + return nil, errorutils.CheckError(err) + } + if !exists { + return nil, errorutils.CheckErrorf("Pipfile.lock not found — run 'pipenv lock' to generate it before running 'jf ca'") + } + content, err := os.ReadFile("Pipfile.lock") + if err != nil { + return nil, errorutils.CheckError(err) + } + log.Debug(fmt.Sprintf("Pipenv: reading Pipfile.lock (%d bytes)", len(content))) + return parsePipfileLockPackages(content) +} + +func parsePipfileLockPackages(content []byte) ([]pipfileLockPackage, error) { + var lock pipfileLockContent + if err := json.Unmarshal(content, &lock); err != nil { + return nil, fmt.Errorf("failed to parse Pipfile.lock: %w", err) + } + seen := map[string]struct{}{} + var packages []pipfileLockPackage + + addEntry := func(name string, entry pipfileLockEntry) { + // Version field is like "==2.0.7" — strip the == operator. + version := strings.TrimPrefix(entry.Version, "==") + if name == "" || version == "" { + return + } + key := strings.ToLower(name) + ":" + version + if _, dup := seen[key]; dup { + return + } + seen[key] = struct{}{} + packages = append(packages, pipfileLockPackage{ + Name: name, + Version: version, + Hashes: entry.Hashes, + }) + } + for name, entry := range lock.Default { + addEntry(name, entry) + } + for name, entry := range lock.Develop { + addEntry(name, entry) + } + return packages, nil +} + func readPoetryLockIfExists() ([]poetryLockPackage, error) { exists, err := fileutils.IsFileExists(poetryLockFile, false) if err != nil { @@ -774,6 +978,160 @@ func validateMinimumPoetryVersion(minVersion string) (int, error) { return major, nil } +// parsePipenvVersion extracts the version string from `pipenv --version` output. +// Pipenv emits "pipenv, version 2023.7.4" (date-based versioning). +// Returns "" if the version line is not found. +func parsePipenvVersion(out string) string { + m := pipenvVersionRegex.FindStringSubmatch(out) + if len(m) < 2 { + return "" + } + return m[1] +} + +// validateMinimumPipenvVersion checks that the pipenv CLI on PATH meets minVersion. +func validateMinimumPipenvVersion(minVersion string) error { + out, err := executeCommand("pipenv", "--version") + if err != nil { + log.Debug(fmt.Sprintf("pipenv is not installed or not on PATH: %v", err)) + return errorutils.CheckErrorf("JFrog CLI pipenv curation requires pipenv %s or higher to be installed.", minVersion) + } + v := parsePipenvVersion(out) + if v == "" { + log.Debug(fmt.Sprintf("Could not parse pipenv version from output: %q", out)) + return errorutils.CheckErrorf("Could not parse pipenv version from output %q — ensure pipenv %s or higher is installed correctly", out, minVersion) + } + log.Debug(fmt.Sprintf("pipenv version: %s", v)) + if !version.NewVersion(v).AtLeast(minVersion) { + return errorutils.CheckErrorf("JFrog CLI pipenv curation requires pipenv %s or higher. The current version is: %s", minVersion, v) + } + return nil +} + +// pipfileSourceURLRegex extracts `url = "..."` values from Pipfile [[source]] blocks. +var pipfileSourceURLRegex = regexp.MustCompile(`(?m)^\s*url\s*=\s*"([^"]+)"`) + +// pipConfigIndexURLRegex matches the `index-url = ...` key in pip.conf / pip.ini. +// The value may or may not be quoted; leading/trailing whitespace is trimmed. +var pipConfigIndexURLRegex = regexp.MustCompile(`(?im)^\s*index-url\s*=\s*(.+)`) + +// parseArtifactoryPypiURL parses a raw PyPI index URL and, if it matches the +// Artifactory pattern (.../artifactory/api/pypi//simple[/]), returns the +// extracted ServerDetails (with credentials) and the repo name. +// Returns (nil, "", nil) when the URL is not an Artifactory PyPI URL. +func parseArtifactoryPypiURL(rawURL string) (*config.ServerDetails, string, error) { + u, err := url.Parse(strings.TrimSpace(rawURL)) + if err != nil || u.Host == "" { + return nil, "", nil + } + const pypiSegment = "/api/pypi/" + pypiIdx := strings.Index(u.Path, pypiSegment) + if pypiIdx < 0 { + return nil, "", nil + } + rest := strings.TrimPrefix(u.Path[pypiIdx:], pypiSegment) + repoSlashIdx := strings.Index(rest, "/") + if repoSlashIdx <= 0 { + return nil, "", nil + } + repoName := rest[:repoSlashIdx] + artURL := fmt.Sprintf("%s://%s%s", u.Scheme, u.Host, u.Path[:pypiIdx+1]) + sd := &config.ServerDetails{ArtifactoryUrl: artURL} + if u.User != nil { + sd.User = u.User.Username() + if p, ok := u.User.Password(); ok { + sd.Password = p + } + } + return sd, repoName, nil +} + +// ParsePipfileArtifactorySource reads the Pipfile at the given path and returns +// the first [[source]] entry whose URL matches the Artifactory PyPI pattern +// (.../artifactory/api/pypi//simple). Credentials embedded in the URL are +// extracted into the returned ServerDetails. +// Returns (nil, "", nil) when no matching source is found (not an error). +func ParsePipfileArtifactorySource(pipfilePath string) (serverDetails *config.ServerDetails, repoName string, err error) { + data, err := os.ReadFile(pipfilePath) + if err != nil { + return nil, "", err + } + for _, m := range pipfileSourceURLRegex.FindAllStringSubmatch(string(data), -1) { + sd, repo, parseErr := parseArtifactoryPypiURL(m[1]) + if parseErr != nil || repo == "" { + continue + } + return sd, repo, nil + } + return nil, "", nil +} + +// ParsePipConfigIndexUrl extracts Artifactory server details and repo name +// from the [global] index-url in the pip config file. Returns (nil, "", nil) +// when the file is missing or index-url is not an Artifactory URL. +func ParsePipConfigIndexUrl(pipConfPath string) (serverDetails *config.ServerDetails, repoName string, err error) { + data, readErr := os.ReadFile(pipConfPath) + if readErr != nil { + // Missing or unreadable — not an error, just no config available. + return nil, "", nil + } + m := pipConfigIndexURLRegex.FindSubmatch(data) + if m == nil { + return nil, "", nil + } + rawURL := strings.Trim(strings.TrimSpace(string(m[1])), `"'`) + sd, repo, parseErr := parseArtifactoryPypiURL(rawURL) + if parseErr != nil { + return nil, "", nil + } + return sd, repo, nil +} + +// DefaultPipConfPath returns the path for pip's per-user configuration file. +// It respects the PIP_CONFIG_FILE environment variable (which pip itself +// honours), making it overridable in tests and CI environments. +// Falls back to ~/.pip/pip.conf on Unix/macOS and %APPDATA%\pip\pip.ini on Windows. +func DefaultPipConfPath() string { + if configFile := os.Getenv("PIP_CONFIG_FILE"); configFile != "" { + return configFile + } + if runtime.GOOS == "windows" { + appData := os.Getenv("APPDATA") + if appData == "" { + appData = filepath.Join(os.Getenv("USERPROFILE"), "AppData", "Roaming") + } + return filepath.Join(appData, "pip", "pip.ini") + } + home, _ := os.UserHomeDir() + return filepath.Join(home, ".pip", "pip.conf") +} + +// rewriteCurationSourceInPipfile replaces the url of the [[source]] entry for +// repoName with curationUrl (the audit pass-through URL). Mirrors +// setCurationSourceInPyproject for Poetry. Returns found=false when no matching +// [[source]] exists (e.g. repo came from pipenv.yaml / pip.conf). +func rewriteCurationSourceInPipfile(pipfilePath, repoName, curationUrl string) (found bool, err error) { + data, err := os.ReadFile(pipfilePath) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, errorutils.CheckError(err) + } + // Match the [[source]] url whether or not it ends with /simple or /simple/. + pattern := regexp.MustCompile(`(?m)^(\s*url\s*=\s*")[^"]*/api/pypi/` + regexp.QuoteMeta(repoName) + `[^"]*(")`) + if !pattern.MatchString(string(data)) { + return false, nil + } + escapedUrl := strings.ReplaceAll(curationUrl, "$", "$$") + updated := pattern.ReplaceAllString(string(data), "${1}"+escapedUrl+"${2}") + if err = os.WriteFile(pipfilePath, []byte(updated), 0600); err != nil { + return false, errorutils.CheckError(err) + } + log.Info(fmt.Sprintf("Configured Pipfile [[source]] url for repository %q to use the curation pass-through endpoint", repoName)) + return true, nil +} + func installPipenvDeps(params technologies.BuildInfoBomGeneratorParams) (rootDetected bool, restoreEnv func() error, err error) { // Set virtualenv path to venv dir err = os.Setenv("WORKON_HOME", ".jfrog") @@ -783,10 +1141,42 @@ func installPipenvDeps(params technologies.BuildInfoBomGeneratorParams) (rootDet restoreEnv = func() error { return os.Unsetenv("WORKON_HOME") } - if params.DependenciesRepository != "" { - return false, restoreEnv, runPipenvInstallFromRemoteRegistry(params.ServerDetails, params.DependenciesRepository) + server := params.ServerDetails + repo := params.DependenciesRepository + if repo == "" && params.IsCurationCmd { + // 2. ~/.pip/pip.conf + pipConfPath := DefaultPipConfPath() + var pipConfServer *config.ServerDetails + pipConfServer, repo, err = ParsePipConfigIndexUrl(pipConfPath) + if err != nil { + log.Debug(fmt.Sprintf("pipenv native: failed to read pip.conf: %v", err)) + err = nil + } + if pipConfServer != nil { + server = pipConfServer + } + // 3. Pipfile [[source]] + if repo == "" { + var nativeServer *config.ServerDetails + nativeServer, repo, err = ParsePipfileArtifactorySource(pipfileFile) + if err != nil { + log.Debug(fmt.Sprintf("pipenv native: failed to read Pipfile: %v", err)) + err = nil + } + if nativeServer != nil { + server = nativeServer + } + } + } + if repo != "" { + return false, restoreEnv, runPipenvInstallFromRemoteRegistry(server, repo, params.IsCurationCmd) + } + if params.IsCurationCmd { + return false, restoreEnv, errorutils.CheckErrorf( + "curation-audit for pipenv requires an Artifactory PyPI resolver. " + + "Either run 'jf pipenv-config', configure index-url in ~/.pip/pip.conf via " + + "Artifactory 'Set me up', or add an Artifactory [[source]] entry to your Pipfile.") } - // Run 'pipenv install -d' _, err = executeCommand("pipenv", "install", "-d") return false, restoreEnv, err } @@ -940,14 +1330,47 @@ func parseCustomArgs(remoteUrl, cacheFolder, reportFileName string, customArgs . return } -func runPipenvInstallFromRemoteRegistry(server *config.ServerDetails, depsRepoName string) (err error) { - rtUrl, err := artifactoryutils.GetPypiRepoUrl(server, depsRepoName, false) +func runPipenvInstallFromRemoteRegistry(server *config.ServerDetails, depsRepoName string, isCurationCmd bool) (err error) { + if isCurationCmd { + if err = validateMinimumPipenvVersion(CurationPipenvMinimumVersion); err != nil { + return err + } + } + + rtUrl, err := artifactoryutils.GetPypiRepoUrl(server, depsRepoName, isCurationCmd) if err != nil { return err } + // Rewrite [[source]] url so pipenv's resolver hits the curation pass-through + // endpoint. --pypi-mirror is a no-op for explicit Artifactory sources + // (pipenv only applies it to the default pypi.org index). + if isCurationCmd { + found, rewriteErr := rewriteCurationSourceInPipfile(pipfileFile, depsRepoName, rtUrl) + if rewriteErr != nil { + return rewriteErr + } + if !found { + if exists, _ := fileutils.IsFileExists(pipfileFile, false); exists { + log.Warn(fmt.Sprintf( + "Pipfile exists but no [[source]] url matching repository %q was found — "+ + "pipenv will resolve against the original source URL", depsRepoName)) + } + } + } args := []string{"install", "-d", artifactoryutils.GetPypiRemoteRegistryFlag(pythonutils.Pipenv), rtUrl} - _, err = executeCommand("pipenv", args...) - return err + output, installErr := executeCommand("pipenv", args...) + if installErr != nil && isCurationCmd { + combined := output + " " + installErr.Error() + if isCvsVersionFilteredOutput(combined) { + return &CvsBlockedError{Packages: parseCvsFailedPackages(combined), Cause: installErr} + } + // build-info-go's IsForbiddenOutput has no pipenv case yet; check directly. + if strings.Contains(strings.ToLower(combined), "http error 403") { + msgToUser := fmt.Sprintf(technologies.CurationErrorMsgToUserTemplate, techutils.Pipenv) + return errors.Join(installErr, errors.New(msgToUser)) + } + } + return installErr } // Execute virtualenv command: "virtualenv venvdir" / "python3 -m venv venvdir" and set path diff --git a/sca/bom/buildinfo/technologies/python/python_test.go b/sca/bom/buildinfo/technologies/python/python_test.go index d720d2d05..a52aa4f84 100644 --- a/sca/bom/buildinfo/technologies/python/python_test.go +++ b/sca/bom/buildinfo/technologies/python/python_test.go @@ -1001,3 +1001,560 @@ func TestWrapPoetryCurationErrMsgToUserPath(t *testing.T) { assert.ErrorIs(t, wrapped, lockErr, "original lockErr must be in the error chain") assert.Contains(t, wrapped.Error(), "poetry", "user-facing message must mention the package manager") } + +func TestParsePipenvVersion(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"pipenv, version 2023.7.4", "2023.7.4"}, + {"pipenv, version 2026.6.1", "2026.6.1"}, + {"pipenv, version 2022.1.8", "2022.1.8"}, + // Extra whitespace / prefix lines must still match + {"some preamble\npipenv, version 2024.11.26\n", "2024.11.26"}, + // Non-matching strings + {"", ""}, + {"some unrelated output", ""}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + assert.Equal(t, tt.want, parsePipenvVersion(tt.in)) + }) + } +} + +func TestValidateMinimumPipenvVersionOk(t *testing.T) { + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\necho 'pipenv, version 2025.0.0'\n", + "@echo off\necho pipenv, version 2025.0.0\n", + ) + require.NoError(t, validateMinimumPipenvVersion(CurationPipenvMinimumVersion)) +} + +func TestValidateMinimumPipenvVersionTooOld(t *testing.T) { + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\necho 'pipenv, version 2022.1.1'\n", + "@echo off\necho pipenv, version 2022.1.1\n", + ) + err := validateMinimumPipenvVersion(CurationPipenvMinimumVersion) + require.Error(t, err) + assert.Contains(t, err.Error(), "2023.7.4", "error must mention the minimum required version") + assert.Contains(t, err.Error(), "2022.1.1", "error must mention the installed version") +} + +func TestValidateMinimumPipenvVersionUnparsable(t *testing.T) { + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\necho 'unrecognized output'\n", + "@echo off\necho unrecognized output\n", + ) + err := validateMinimumPipenvVersion(CurationPipenvMinimumVersion) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse") +} + +func TestInstallPipenvDepsCurationNoRepoNoNativeErrors(t *testing.T) { + // When IsCurationCmd=true, no DependenciesRepository, and no Pipfile with an + // Artifactory [[source]], installPipenvDeps must return an actionable error. + dir := t.TempDir() + t.Chdir(dir) + // Prevent real ~/.pip/pip.conf from being picked up. + t.Setenv("PIP_CONFIG_FILE", filepath.Join(dir, "no-such-pip.conf")) + params := technologies.BuildInfoBomGeneratorParams{ + IsCurationCmd: true, + DependenciesRepository: "", + } + _, _, err := installPipenvDeps(params) + require.Error(t, err) + assert.Contains(t, err.Error(), "pipenv-config") + assert.Contains(t, err.Error(), "Pipfile") +} + +func TestInstallPipenvDepsCurationNativeFromPipfile(t *testing.T) { + // When IsCurationCmd=true and no DependenciesRepository but Pipfile has an + // Artifactory [[source]], installPipenvDeps must detect the repo natively + // and call runPipenvInstallFromRemoteRegistry (which will fail here because + // pipenv isn't installed in the test env — but it must NOT error with the + // "pipenv-config" message; it must get past native detection). + fakeDir := t.TempDir() + pipfileContent := `[[source]] +url = "https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple" +verify_ssl = true +name = "artifactory" + +[packages] +requests = "==2.31.0" +` + require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "Pipfile"), []byte(pipfileContent), 0644)) + t.Chdir(fakeDir) + + params := technologies.BuildInfoBomGeneratorParams{ + IsCurationCmd: true, + DependenciesRepository: "", + } + _, _, err := installPipenvDeps(params) + // The call will fail because pipenv/virtualenv is not available in the test + // environment, but it must NOT fail with a "pipenv-config/Pipfile" config error. + if err != nil { + assert.NotContains(t, err.Error(), "pipenv-config", "native detection must succeed; error must come from pipenv CLI, not config") + assert.NotContains(t, err.Error(), "Artifactory [[source]]", "native detection must succeed; error must come from pipenv CLI, not config") + } +} + +func TestParsePipfileArtifactorySource(t *testing.T) { + tests := []struct { + name string + content string + wantRepo string + wantArtURL string + wantUser string + wantPass string + }{ + { + name: "standard Artifactory pypi source with credentials", + content: `[[source]] +url = "https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple" +verify_ssl = true +name = "artifactory"`, + wantRepo: "my-pip-repo", + wantArtURL: "https://myartifactory.jfrog.io/artifactory/", + wantUser: "myuser", + wantPass: "mytoken", + }, + { + name: "multiple sources — Artifactory one is picked", + content: `[[source]] +url = "https://pypi.org/simple" +name = "pypi" + +[[source]] +url = "https://admin:s3cret@acme.jfrog.io/artifactory/api/pypi/curation-repo/simple" +name = "jfrog"`, + wantRepo: "curation-repo", + wantArtURL: "https://acme.jfrog.io/artifactory/", + wantUser: "admin", + wantPass: "s3cret", + }, + { + name: "no Artifactory source — returns empty", + content: `[[source]] +url = "https://pypi.org/simple" +name = "pypi"`, + wantRepo: "", + }, + { + name: "empty Pipfile", + content: "", + wantRepo: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + pipfilePath := filepath.Join(dir, "Pipfile") + require.NoError(t, os.WriteFile(pipfilePath, []byte(tt.content), 0644)) + + sd, repo, err := ParsePipfileArtifactorySource(pipfilePath) + require.NoError(t, err) + assert.Equal(t, tt.wantRepo, repo) + if tt.wantRepo == "" { + assert.Nil(t, sd) + return + } + require.NotNil(t, sd) + assert.Equal(t, tt.wantArtURL, sd.ArtifactoryUrl) + assert.Equal(t, tt.wantUser, sd.User) + assert.Equal(t, tt.wantPass, sd.Password) + }) + } +} + +func TestParsePipfileArtifactorySourceNotFound(t *testing.T) { + _, _, err := ParsePipfileArtifactorySource("/nonexistent/path/Pipfile") + require.Error(t, err) +} + +func TestParsePipConfigIndexUrl(t *testing.T) { + cases := []struct { + name string + content string + wantRepo string + wantArtURL string + wantUser string + wantNoMatch bool + }{ + { + name: "artifactory index-url with credentials", + content: "[global]\n" + + "index-url = https://admin:mytoken@myrt.jfrogdev.org/artifactory/api/pypi/my-pip-repo/simple\n", + wantRepo: "my-pip-repo", + wantArtURL: "https://myrt.jfrogdev.org/artifactory/", + wantUser: "admin", + }, + { + name: "plain pypi.org — not an artifactory URL", + content: "[global]\nindex-url = https://pypi.org/simple\n", + wantNoMatch: true, + }, + { + name: "no index-url key", + content: "[global]\ntimeout = 60\n", + wantNoMatch: true, + }, + { + name: "empty file", + content: "", + wantNoMatch: true, + }, + { + name: "index-url with no credentials", + content: "[global]\n" + + "index-url = https://myrt.example.com/artifactory/api/pypi/libs-pypi/simple\n", + wantRepo: "libs-pypi", + wantArtURL: "https://myrt.example.com/artifactory/", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "pip.conf") + require.NoError(t, err) + _, err = f.WriteString(tc.content) + require.NoError(t, err) + require.NoError(t, f.Close()) + + sd, repo, err := ParsePipConfigIndexUrl(f.Name()) + require.NoError(t, err) + if tc.wantNoMatch { + assert.Empty(t, repo) + assert.Nil(t, sd) + return + } + assert.Equal(t, tc.wantRepo, repo) + require.NotNil(t, sd) + assert.Equal(t, tc.wantArtURL, sd.ArtifactoryUrl) + if tc.wantUser != "" { + assert.Equal(t, tc.wantUser, sd.User) + } + }) + } +} + +func TestParsePipConfigIndexUrlMissingFile(t *testing.T) { + // A missing pip.conf is not an error — just returns nil/empty (graceful no-op). + sd, repo, err := ParsePipConfigIndexUrl("/nonexistent/path/pip.conf") + require.NoError(t, err) + assert.Empty(t, repo) + assert.Nil(t, sd) +} + +func TestRunPipenvInstallFromRemoteRegistryCurationVersionGate(t *testing.T) { + // A pipenv below the minimum version must be rejected before any install attempt. + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\necho 'pipenv, version 2021.5.29'\n", + "@echo off\necho pipenv, version 2021.5.29\n", + ) + err := runPipenvInstallFromRemoteRegistry(nil, "my-repo", true) + require.Error(t, err) + assert.Contains(t, err.Error(), CurationPipenvMinimumVersion) +} + +func TestRunPipenvInstallFromRemoteRegistryCuration403Detection(t *testing.T) { + // Pipenv emits "HTTP error 403" during install of a blocked package. + // build-info-go's IsForbiddenOutput has no "pipenv" case, so we check the + // pattern directly. + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\n"+ + "if [ \"$1\" = \"--version\" ]; then echo 'pipenv, version 2025.0.0'; exit 0; fi\n"+ + "echo 'CRITICAL:pipenv.patched.pip._internal.network.download:HTTP error 403 while getting https://rt.example.com/api/pypi/repo/simple/urllib3/'\n"+ + "echo '[ResolutionFailure]: ...'\n"+ + "exit 1\n", + "@echo off\n"+ + "if \"%1\"==\"--version\" (echo pipenv, version 2025.0.0 & exit /b 0)\n"+ + "echo HTTP error 403 while getting https://rt.example.com/api/pypi/repo/simple/urllib3/\n"+ + "exit /b 1\n", + ) + origPath := os.Getenv("PATH") + t.Setenv("PATH", fakeDir+string(os.PathListSeparator)+origPath) + + wd := t.TempDir() + origWd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(wd)) + defer func() { _ = os.Chdir(origWd) }() + + sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} + err = runPipenvInstallFromRemoteRegistry(sd, "repo", true) + require.Error(t, err) + // Must contain the user-facing curation message, not just a raw install error. + assert.Contains(t, err.Error(), "Failed to retrieve the dependencies tree") +} + +func TestRunPipenvInstallFromRemoteRegistryNoLockFileSucceeds(t *testing.T) { + // Unlike the earlier (incorrect) assumption, 'pipenv install' does NOT need a + // pre-existing Pipfile.lock for curation. --pypi-mirror points at the audit + // pass-through endpoint, which — like pip's and poetry's pass-through routes — + // always returns the artifact (200) regardless of policy. So pipenv's internal + // auto-lock-on-install (triggered when Pipfile.lock is missing) succeeds even + // though it downloads wheels to compute hashes. + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then echo 'pipenv, version 2025.0.0'; exit 0; fi\nexit 0\n", + "@echo off\nif \"%1\"==\"--version\" (echo pipenv, version 2025.0.0 & exit /b 0)\nexit /b 0\n", + ) + origPath := os.Getenv("PATH") + t.Setenv("PATH", fakeDir+string(os.PathListSeparator)+origPath) + + // Run in a directory with NO Pipfile.lock — must not error just because of that. + wd := t.TempDir() + origWd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(wd)) + defer func() { _ = os.Chdir(origWd) }() + + sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} + require.NoError(t, runPipenvInstallFromRemoteRegistry(sd, "repo", true)) +} + +func TestRunPipenvInstallFromRemoteRegistryReturnsCvsBlockedError(t *testing.T) { + // When pipenv's internal pip emits a CVS-filtered error ("No matching distribution + // found" / "Could not find a version that satisfies the requirement"), + // runPipenvInstallFromRemoteRegistry must wrap it as *CvsBlockedError so that + // the curation-audit command can run the metadata-API fallback instead of + // aborting with no report. + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\n"+ + "if [ \"$1\" = \"--version\" ]; then echo 'pipenv, version 2025.0.0'; exit 0; fi\n"+ + "echo 'CRITICAL:pipenv.patched.pip._internal.resolution.resolvelib.factory:Could not find a version that satisfies the requirement urllib3==2.0.7 (from versions: none)'\n"+ + "echo 'ERROR: No matching distribution found for urllib3==2.0.7'\n"+ + "exit 1\n", + "@echo off\n"+ + "if \"%1\"==\"--version\" (echo pipenv, version 2025.0.0 & exit /b 0)\n"+ + "echo Could not find a version that satisfies the requirement urllib3==2.0.7\n"+ + "echo ERROR: No matching distribution found for urllib3==2.0.7\n"+ + "exit /b 1\n", + ) + origPath := os.Getenv("PATH") + t.Setenv("PATH", fakeDir+string(os.PathListSeparator)+origPath) + + wd := t.TempDir() + origWd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(wd)) + defer func() { _ = os.Chdir(origWd) }() + + sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} + err = runPipenvInstallFromRemoteRegistry(sd, "repo", true) + require.Error(t, err) + + var cvsErr *CvsBlockedError + require.ErrorAs(t, err, &cvsErr, "CVS-filtered pipenv error must be wrapped as *CvsBlockedError") + require.Len(t, cvsErr.Packages, 1) + assert.Equal(t, "urllib3", cvsErr.Packages[0].Name) + assert.Equal(t, "2.0.7", cvsErr.Packages[0].Version) +} + +func TestParsePipfileLockPackages(t *testing.T) { + t.Run("default and develop sections both parsed", func(t *testing.T) { + fixture := []byte(`{ + "_meta": {"hash": {"sha256": "abc"}}, + "default": { + "urllib3": { + "version": "==2.0.7", + "hashes": ["sha256:aaa", "sha256:bbb"] + }, + "certifi": { + "version": "==2023.7.22", + "hashes": ["sha256:ccc"] + } + }, + "develop": { + "pytest": { + "version": "==7.4.0", + "hashes": ["sha256:ddd"] + } + } + }`) + got, err := parsePipfileLockPackages(fixture) + require.NoError(t, err) + require.Len(t, got, 3) + + byName := map[string]pipfileLockPackage{} + for _, p := range got { + byName[p.Name] = p + } + require.Contains(t, byName, "urllib3") + assert.Equal(t, "2.0.7", byName["urllib3"].Version) + assert.ElementsMatch(t, []string{"sha256:aaa", "sha256:bbb"}, byName["urllib3"].Hashes) + + require.Contains(t, byName, "certifi") + assert.Equal(t, "2023.7.22", byName["certifi"].Version) + + require.Contains(t, byName, "pytest") + assert.Equal(t, "7.4.0", byName["pytest"].Version) + }) + + t.Run("entry with no version is skipped", func(t *testing.T) { + fixture := []byte(`{ + "default": { + "somepkg": {"hashes": ["sha256:aaa"]} + } + }`) + got, err := parsePipfileLockPackages(fixture) + require.NoError(t, err) + assert.Empty(t, got) + }) + + t.Run("duplicate name+version across sections is deduplicated", func(t *testing.T) { + fixture := []byte(`{ + "default": { + "urllib3": {"version": "==2.0.7", "hashes": ["sha256:aaa"]} + }, + "develop": { + "urllib3": {"version": "==2.0.7", "hashes": ["sha256:aaa"]} + } + }`) + got, err := parsePipfileLockPackages(fixture) + require.NoError(t, err) + assert.Len(t, got, 1) + }) + + t.Run("invalid json returns error", func(t *testing.T) { + _, err := parsePipfileLockPackages([]byte("not json")) + require.Error(t, err) + }) + + t.Run("empty content returns empty slice", func(t *testing.T) { + got, err := parsePipfileLockPackages([]byte(`{}`)) + require.NoError(t, err) + assert.Empty(t, got) + }) +} + +func TestPickHrefByHashFragment(t *testing.T) { + body := []byte(` +urllib3-2.0.6-py3-none-any.whl +urllib3-2.0.7-py3-none-any.whl +`) + + t.Run("returns href whose hash fragment matches", func(t *testing.T) { + wanted := map[string]struct{}{"#sha256=aaa111": {}} + got := pickHrefByHashFragment(body, wanted) + assert.Equal(t, "../../packages/cc/dd/urllib3-2.0.7-py3-none-any.whl#sha256=aaa111", got) + }) + + t.Run("returns empty when no hash matches", func(t *testing.T) { + wanted := map[string]struct{}{"#sha256=nonexistent": {}} + got := pickHrefByHashFragment(body, wanted) + assert.Equal(t, "", got) + }) + + t.Run("returns empty for empty body", func(t *testing.T) { + got := pickHrefByHashFragment(nil, map[string]struct{}{"#sha256=aaa111": {}}) + assert.Equal(t, "", got) + }) +} + +// TestBuildPipenvDownloadUrl_HTTP mirrors TestBuildPoetryDownloadUrl_HTTP: it spins +// up a mock Artifactory simple-index endpoint and verifies that +// buildPipenvDownloadUrl resolves the correct absolute URL by matching the +// sha256 hash fragment (Pipfile.lock hashes, not filenames like Poetry). +func TestBuildPipenvDownloadUrl_HTTP(t *testing.T) { + const repo = "my-pip-repo" + pkg := pipfileLockPackage{ + Name: "urllib3", + Version: "2.0.7", + Hashes: []string{"sha256:aaa111"}, + } + + t.Run("200 with matching hash returns absolute URL without fragment", func(t *testing.T) { + server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/simple/urllib3/") { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(` +urllib3-2.0.7-py3-none-any.whl +`)) + return + } + t.Fatalf("unexpected request to %s", r.URL.Path) + }) + defer server.Close() + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + got, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) + require.NoError(t, err) + assert.Contains(t, got, "/packages/cc/dd/urllib3-2.0.7-py3-none-any.whl") + assert.True(t, strings.HasPrefix(got, server.URL), "resolved URL must be absolute against the simple-index base, got %q", got) + assert.NotContains(t, got, "#", "fragment must be stripped from the returned URL") + }) + + t.Run("non-200 from simple-index surfaces status code", func(t *testing.T) { + server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + defer server.Close() + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) + require.Error(t, err) + assert.Contains(t, err.Error(), "404") + assert.Contains(t, err.Error(), "simple-index") + }) + + t.Run("200 with no matching hash returns error", func(t *testing.T) { + server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(` +urllib3-1.0.0-py3-none-any.whl +`)) + }) + defer server.Close() + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) + require.Error(t, err) + assert.Contains(t, err.Error(), "no matching href") + }) + + t.Run("uses normalized name in simple-index URL", func(t *testing.T) { + var seenPath string + server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + seenPath = r.URL.Path + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`Flask_Babel-1.0.tar.gz`)) + }) + defer server.Close() + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + quirky := pipfileLockPackage{Name: "Flask_Babel", Version: "1.0", Hashes: []string{"sha256:xyz"}} + _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, quirky) + require.NoError(t, err) + assert.Contains(t, seenPath, "/simple/flask-babel/", "must use PEP 503 normalized name in the simple-index URL, got %q", seenPath) + }) +} + +func TestBuildPipenvDownloadUrlsMapInputValidation(t *testing.T) { + t.Run("nil server details returns error", func(t *testing.T) { + _, err := buildPipenvDownloadUrlsMap(nil, "my-pip-repo") + require.Error(t, err) + assert.Contains(t, err.Error(), "server details") + }) + + t.Run("empty artifactory URL returns error", func(t *testing.T) { + _, err := buildPipenvDownloadUrlsMap(&config.ServerDetails{}, "my-pip-repo") + require.Error(t, err) + assert.Contains(t, err.Error(), "server details") + }) + + t.Run("empty repository returns error", func(t *testing.T) { + sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} + _, err := buildPipenvDownloadUrlsMap(sd, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "repository") + }) +} From 8f9299fa94945d5256e1898fdcf26f286e2705b9 Mon Sep 17 00:00:00 2001 From: Gauri Yadav Date: Tue, 14 Jul 2026 10:48:18 +0530 Subject: [PATCH 2/3] XRAY-146488 - Suppress gosec G304/G703 on local-file reads/writes in Pipenv curation helpers Co-authored-by: Cursor --- _typos.toml | 1 + cli/docs/scan/curation/help.go | 2 +- commands/curation/curationaudit.go | 157 ++- commands/curation/curationaudit_test.go | 105 ++ curation_test.go | 120 ++ go.mod | 6 +- .../buildinfo/technologies/python/python.go | 1168 ++++++++++++++--- .../technologies/python/python_test.go | 1001 +++++++++++++- .../pipenv/pipenv-curation-project/Pipfile | 10 + utils/http_redirect.go | 124 ++ utils/http_redirect_test.go | 160 +++ 11 files changed, 2624 insertions(+), 230 deletions(-) create mode 100644 tests/testdata/projects/package-managers/python/pipenv/pipenv-curation-project/Pipfile create mode 100644 utils/http_redirect.go create mode 100644 utils/http_redirect_test.go diff --git a/_typos.toml b/_typos.toml index 1ca105b31..7f485ae9a 100644 --- a/_typos.toml +++ b/_typos.toml @@ -2,6 +2,7 @@ testng = "testng" afe = "afe" Bouned = "Bouned" +certifi = "certifi" [files] extend-exclude = ["go.mod", "go.sum", "tests/testdata"] \ No newline at end of file diff --git a/cli/docs/scan/curation/help.go b/cli/docs/scan/curation/help.go index b2b7d2c28..d98529457 100644 --- a/cli/docs/scan/curation/help.go +++ b/cli/docs/scan/curation/help.go @@ -15,7 +15,7 @@ When to use: Prerequisites: - A configured JFrog Platform server (jf c add) with JFrog Curation entitlement. - Project must use a supported package manager (npm, yarn, pnpm, pip, pipenv, poetry, maven, gradle, nuget, go) resolved through a curation-configured remote. -- The package manager and its lockfile must be present in the working directory. +- The package manager must be present in the working directory. Lockfiles are required where the package manager's flow depends on one (e.g. npm, yarn, pnpm, poetry); pipenv generates/refreshes Pipfile.lock as part of the audit, so a pre-existing lockfile is not required. Common patterns: $ jf curation-audit diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index f8f2ae76f..173ed9392 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -662,6 +662,12 @@ func (ca *CurationAuditCommand) getRtManagerAndAuth(tech techutils.Technology) ( return } +// pipenvBoundedRedirectManager must use zero retries: HttpClient.Send retries +// on CheckRedirect errors, desyncing SendWithBoundedRedirects's hop counter. +func pipenvBoundedRedirectManager(serverDetails *config.ServerDetails) (artifactory.ArtifactoryServicesManager, error) { + return rtUtils.CreateServiceManager(serverDetails, 0, 0, false) +} + func (ca *CurationAuditCommand) GetAuth(tech techutils.Technology) (serverDetails *config.ServerDetails, err error) { if ca.PackageManagerConfig == nil { if err = ca.SetRepo(tech); err != nil { @@ -675,8 +681,18 @@ func (ca *CurationAuditCommand) GetAuth(tech techutils.Technology) (serverDetail return } +// getBuildInfoParamsByTech resolves install-time server details, preferring an already-set +// ca.PackageManagerConfig (native detection, e.g. Pipenv) over the generic server so install +// and the later GetAuth-based probes hit the same endpoint. Does not trigger SetRepo eagerly, +// so techs resolving their repo later via SetResolutionRepoInParamsIfExists are unaffected. func (ca *CurationAuditCommand) getBuildInfoParamsByTech() (technologies.BuildInfoBomGeneratorParams, error) { - serverDetails, err := ca.ServerDetails() + var serverDetails *config.ServerDetails + var err error + if ca.PackageManagerConfig != nil { + serverDetails, err = ca.PackageManagerConfig.ServerDetails() + } else { + serverDetails, err = ca.ServerDetails() + } return technologies.BuildInfoBomGeneratorParams{ XrayVersion: ca.GetXrayVersion(), ExclusionPattern: technologies.GetScaExcludePattern(ca.GetConfigProfile(), ca.IsRecursiveScan(), ca.Exclusions()...), @@ -721,9 +737,9 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if err := validateRunNativeForTech(tech, ca.RunNative()); err != nil { return err } - // When JFROG_CLI_CURATION_SUPPORT=true, checkSupportByVersionOrEnv short-circuits - // before calling getRtVersion→GetAuth→SetRepo, so ca.DepsRepo() is never populated - // for Pipenv. Call SetRepo explicitly so getBuildInfoParamsByTech sees the right repo. + // Resolve Pipenv's native repo/server before getBuildInfoParamsByTech so install and the + // later probes share an endpoint. Other techs resolve later via SetResolutionRepoInParamsIfExists + // and must not be forced through SetRepo this early (they tolerate having no config file yet). if tech == techutils.Pipenv && ca.PackageManagerConfig == nil { if err := ca.SetRepo(tech); err != nil { return err @@ -775,6 +791,12 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if err != nil { return err } + if tech == techutils.Pipenv { + rtManager, err = pipenvBoundedRedirectManager(serverDetails) + if err != nil { + return err + } + } rtAuth, err := serverDetails.CreateArtAuthConfig() if err != nil { return err @@ -1124,45 +1146,63 @@ func (ca *CurationAuditCommand) SetRepo(tech techutils.Technology) error { // setRepoFromPipfile detects the Artifactory PyPI source for pipenv curation. // // Detection priority: -// 1. pipenv.yaml — explicit 'jf pipenv-config' (highest priority, JFrog-specific). -// 2. ~/.pip/pip.conf index-url — customer followed the Artifactory "Set me up" -// guide for pip (most common real-world setup; no Pipfile changes needed). -// 3. Pipfile [[source]] — customer explicitly added an Artifactory source URL -// directly in the Pipfile. +// 1. pipenv.yaml — explicit 'jf pipenv-config'. +// 2. User pip.conf index-url — Artifactory "Set me up" for pip. +// 3. Pipfile [[source]] — Artifactory URL declared directly in the Pipfile. func (ca *CurationAuditCommand) setRepoFromPipfile() error { - // 1. pipenv.yaml — explicit 'jf pipenv-config'. - resolverParams, yamlErr := ca.getRepoParams(techutils.Pipenv.GetProjectType()) - if yamlErr == nil { - ca.setPackageManagerConfig(resolverParams) - ca.SetDepsRepo(resolverParams.TargetRepo()) + projectType := techutils.Pipenv.GetProjectType() + _, configExists, err := project.GetProjectConfFilePath(projectType) + if err != nil { + return err + } + + if configExists { + resolverParams, err := ca.getRepoParams(projectType) + if err != nil { + return err + } + configuredServer, err := resolverParams.ServerDetails() + if err != nil { + return err + } + fallbackServer, _ := ca.ServerDetails() + server, repo, err := python.ResolvePipfileArtifactorySource("Pipfile", configuredServer, resolverParams.TargetRepo(), fallbackServer) + if err != nil { + return err + } + repoConfig := (&project.RepositoryConfig{}).SetTargetRepo(repo).SetServerDetails(server) + ca.setPackageManagerConfig(repoConfig) + ca.SetDepsRepo(repo) return nil } - // 2. ~/.pip/pip.conf index-url (Artifactory "Set me up" path). - pipConfPath := python.DefaultPipConfPath() - sd, repo, err := python.ParsePipConfigIndexUrl(pipConfPath) + configuredServer, configuredRepo, pipConfPath, err := python.ParsePipConfigIndexUrl(python.DefaultPipConfPaths()...) if err != nil { - log.Debug(fmt.Sprintf("pipenv native: failed to read pip.conf: %v", err)) + return err } - if repo != "" && sd != nil { + fallbackServer, _ := ca.ServerDetails() + if configuredRepo != "" && configuredServer != nil { + server, repo, err := python.ResolvePipfileArtifactorySource("Pipfile", configuredServer, configuredRepo, fallbackServer) + if err != nil { + return err + } repoConfig := (&project.RepositoryConfig{}). SetTargetRepo(repo). - SetServerDetails(sd) + SetServerDetails(server) ca.setPackageManagerConfig(repoConfig) ca.SetDepsRepo(repo) log.Info(fmt.Sprintf("pipenv: using Artifactory repository %q from %s", repo, pipConfPath)) return nil } - // 3. Pipfile [[source]] — Artifactory URL declared directly in the Pipfile. - sd, repo, err = python.ParsePipfileArtifactorySource("Pipfile") + server, repo, err := python.ResolvePipfileArtifactorySource("Pipfile", nil, "", fallbackServer) if err != nil { - log.Debug(fmt.Sprintf("pipenv native: failed to read Pipfile: %v", err)) + return err } - if repo != "" && sd != nil { + if repo != "" && server != nil { repoConfig := (&project.RepositoryConfig{}). SetTargetRepo(repo). - SetServerDetails(sd) + SetServerDetails(server) ca.setPackageManagerConfig(repoConfig) ca.SetDepsRepo(repo) log.Info(fmt.Sprintf("pipenv: using Artifactory repository %q from Pipfile [[source]]", repo)) @@ -1171,7 +1211,7 @@ func (ca *CurationAuditCommand) setRepoFromPipfile() error { return errorutils.CheckErrorf( "curation-audit for pipenv requires an Artifactory PyPI resolver. " + - "Either run 'jf pipenv-config', configure index-url in ~/.pip/pip.conf via " + + "Either run 'jf pipenv-config', configure index-url in your user pip.conf via " + "Artifactory 'Set me up', or add an Artifactory [[source]] entry to your Pipfile.") } @@ -1196,6 +1236,12 @@ func validateRunNativeForTech(tech techutils.Technology, runNative bool) error { if _, ok := supported[tech]; ok { return nil } + if tech == techutils.Pipenv { + return errorutils.CheckErrorf( + "--run-native is not supported for 'pipenv' projects. " + + "Run 'jf ca' without --run-native; the repository is resolved automatically from " + + "'jf pipenv-config', ~/.pip/pip.conf, or the Artifactory [[source]] entry in your Pipfile.") + } return errorutils.CheckErrorf( "--run-native is not supported for '%s' projects. "+ "Run 'jf ca' without --run-native; configure the resolution repository using 'jf %s-config'.", @@ -1435,7 +1481,13 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e return nil } requestDetails := nc.httpClientDetails.Clone() - resp, _, err := nc.rtManager.Client().SendHead(packageUrl, requestDetails) + var resp *http.Response + var err error + if nc.tech == techutils.Pipenv { + resp, _, err = nc.sendPipenvRequest(http.MethodHead, packageUrl, requestDetails) + } else { + resp, _, err = nc.rtManager.Client().SendHead(packageUrl, requestDetails) + } if resp != nil && resp.StatusCode == http.StatusUnauthorized { // Store the error silently (not returned) so errorsQueue.AddError is never // called and the message is not logged here. fetchNodesStatus picks it up @@ -1481,6 +1533,16 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e return nil } +func (nc *treeAnalyzer) sendPipenvRequest(method, requestURL string, details *httputils.HttpClientDetails) (*http.Response, []byte, error) { + repositoryURL := fmt.Sprintf("%s/api/pypi/%s/", strings.TrimSuffix(nc.url, "/"), nc.repo) + boundary, err := utils.NewEndpointBoundary(repositoryURL) + if err != nil { + return nil, nil, err + } + return utils.SendWithBoundedRedirects(nc.rtManager.Client(), method, requestURL, details, + boundary, utils.MaxAuthenticatedRedirects) +} + // runCvsFallback is called when pip or poetry resolution failed because CVS // stripped a pinned version from the simple index (CvsBlockedError). It uses // the PyPI metadata API to recover each blocker's real download URL, probes @@ -1491,6 +1553,12 @@ func (ca *CurationAuditCommand) runCvsFallback(cvsErr *python.CvsBlockedError, t if err != nil { return fmt.Errorf("curation-blocked resolution fallback: failed to get Artifactory manager (%w); %s error: %w", err, tech, cvsErr) } + if tech == techutils.Pipenv { + rtManager, err = pipenvBoundedRedirectManager(serverDetails) + if err != nil { + return fmt.Errorf("curation-blocked resolution fallback: failed to create bounded HTTP manager: %w", err) + } + } rtAuth, err := serverDetails.CreateArtAuthConfig() if err != nil { return fmt.Errorf("curation-blocked resolution fallback: failed to create auth config (%w); %s error: %w", err, tech, cvsErr) @@ -1531,7 +1599,14 @@ func (nc *treeAnalyzer) lookupPypiAllVersions(name string) ([]string, error) { strings.TrimSuffix(nc.url, "/"), nc.repo, name) requestDetails := nc.httpClientDetails.Clone() - resp, body, _, err := nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) + var resp *http.Response + var body []byte + var err error + if nc.tech == techutils.Pipenv { + resp, body, err = nc.sendPipenvRequest(http.MethodGet, metadataURL, requestDetails) + } else { + resp, body, _, err = nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) + } if err != nil { return nil, fmt.Errorf("all-versions metadata API request failed for %s: %w", name, err) } @@ -1564,7 +1639,14 @@ func (nc *treeAnalyzer) lookupPypiNormalDownloadURL(name, ver string) (string, e strings.TrimSuffix(nc.url, "/"), nc.repo, name, ver) requestDetails := nc.httpClientDetails.Clone() - resp, body, _, err := nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) + var resp *http.Response + var body []byte + var err error + if nc.tech == techutils.Pipenv { + resp, body, err = nc.sendPipenvRequest(http.MethodGet, metadataURL, requestDetails) + } else { + resp, body, _, err = nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) + } if err != nil { return "", fmt.Errorf("metadata API request failed for %s==%s: %w", name, ver, err) } @@ -1659,7 +1741,13 @@ func (nc *treeAnalyzer) fetchCvsBlockedStatus(pins []python.PinnedRequirement) [ // ── Step 3a: HEAD probe — detect whether the version is download-blocked headDetails := nc.httpClientDetails.Clone() - headResp, _, headErr := nc.rtManager.Client().SendHead(dlURL, headDetails) + var headResp *http.Response + var headErr error + if nc.tech == techutils.Pipenv { + headResp, _, headErr = nc.sendPipenvRequest(http.MethodHead, dlURL, headDetails) + } else { + headResp, _, headErr = nc.rtManager.Client().SendHead(dlURL, headDetails) + } if headErr != nil && (headResp == nil || headResp.StatusCode != http.StatusForbidden) { log.Debug(fmt.Sprintf("curation-blocked resolution fallback: HEAD probe failed for %s==%s: %v", pin.Name, resolvedVersion, headErr)) @@ -1752,7 +1840,14 @@ func effectiveParentVersion(pin python.PinnedRequirement) string { func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, version string) (*PackageStatus, error) { requestDetails := nc.httpClientDetails.Clone() requestDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn" - getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, requestDetails) + var getResp *http.Response + var respBody []byte + var err error + if nc.tech == techutils.Pipenv { + getResp, respBody, err = nc.sendPipenvRequest(http.MethodGet, packageUrl, requestDetails) + } else { + getResp, respBody, _, err = nc.rtManager.Client().SendGet(packageUrl, true, requestDetails) + } if err != nil { if getResp == nil { return nil, err diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index e07c0767f..89ed7bede 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "testing" "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies/java" @@ -22,6 +23,7 @@ import ( biutils "github.com/jfrog/build-info-go/utils" "github.com/jfrog/gofrog/datastructures" + rtUtils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/common/project" coreCommonTests "github.com/jfrog/jfrog-cli-core/v2/common/tests" "github.com/jfrog/jfrog-cli-core/v2/utils/config" @@ -2405,6 +2407,109 @@ func TestEffectiveParentVersion(t *testing.T) { } } +func TestSetRepoFromPipfileRejectsInvalidPresentConfig(t *testing.T) { + t.Chdir(t.TempDir()) + require.NoError(t, os.MkdirAll(filepath.Join(".jfrog", "projects"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(".jfrog", "projects", "pipenv.yaml"), []byte("resolver: [\n"), 0600)) + require.NoError(t, os.WriteFile("Pipfile", []byte(`[[source]] +name = "jfrog" +url = "https://user:token@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`), 0600)) + + ca := NewCurationAuditCommand() + err := ca.setRepoFromPipfile() + require.Error(t, err) + assert.Nil(t, ca.PackageManagerConfig) +} + +func TestSendPipenvRequestRejectsRedirectOutsideRepository(t *testing.T) { + var outsideRequested atomic.Bool + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.URL.Path == "/api/system/configuration" { + outsideRequested.Store(true) + w.WriteHeader(http.StatusOK) + return + } + http.Redirect(w, r, "/api/system/configuration", http.StatusFound) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + rtAuth := rtManager.GetConfig().GetServiceDetails() + analyzer := treeAnalyzer{ + rtManager: rtManager, + httpClientDetails: rtAuth.CreateHttpClientDetails(), + url: rtAuth.GetUrl(), + repo: "repo", + tech: techutils.Pipenv, + } + requestDetails := analyzer.httpClientDetails.Clone() + requestDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn" + + _, _, err = analyzer.sendPipenvRequest(http.MethodGet, + strings.TrimSuffix(analyzer.url, "/")+"/api/pypi/repo/packages/pkg.whl", requestDetails) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.False(t, outsideRequested.Load()) + assert.Equal(t, int32(1), requests.Load()) +} + +func TestPipenvCvsMetadataRejectsRedirectOutsideRepository(t *testing.T) { + tests := []struct { + name string + call func(*treeAnalyzer) error + }{ + { + name: "all versions metadata", + call: func(analyzer *treeAnalyzer) error { + _, err := analyzer.lookupPypiAllVersions("urllib3") + return err + }, + }, + { + name: "version download metadata", + call: func(analyzer *treeAnalyzer) error { + _, err := analyzer.lookupPypiNormalDownloadURL("urllib3", "2.0.7") + return err + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var outsideRequested atomic.Bool + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.URL.Path == "/api/system/configuration" { + outsideRequested.Store(true) + w.WriteHeader(http.StatusOK) + return + } + http.Redirect(w, r, "/api/system/configuration", http.StatusFound) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + rtAuth := rtManager.GetConfig().GetServiceDetails() + analyzer := &treeAnalyzer{ + rtManager: rtManager, + httpClientDetails: rtAuth.CreateHttpClientDetails(), + url: rtAuth.GetUrl(), + repo: "repo", + tech: techutils.Pipenv, + } + + err = test.call(analyzer) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.False(t, outsideRequested.Load()) + assert.Equal(t, int32(1), requests.Load()) + }) + } +} + // TestValidateRunNativeForTech checks that --run-native is accepted for the // allow-listed native-config techs (npm, pnpm, yarn) and rejected for all other // techs with an error that names the offending tech. diff --git a/curation_test.go b/curation_test.go index da796d5e2..49554dd93 100644 --- a/curation_test.go +++ b/curation_test.go @@ -404,6 +404,126 @@ func getPoetryCurationExpectedResponse(config *config.ServerDetails, repo string } } +// TestPipenvCurationAudit drives a real 'pipenv install -d'. The mock's empty simple +// index makes pip's resolver report the pin as not found, exercising the CVS-fallback +// path (metadata API + HEAD/GET probe) rather than a direct install-time 403. +func TestPipenvCurationAudit(t *testing.T) { + integration.InitCurationTest(t) + const repo = "pypi-curation" + tempDirPath, cleanUp := securityTestUtils.CreateTestProjectEnvAndChdir(t, filepath.Join(filepath.FromSlash(securityTests.GetTestResourcesPath()), "projects", "package-managers", "python", "pipenv", "pipenv-curation-project")) + defer cleanUp() + + // Isolate from the test machine's real pip.conf so resolution falls through to Pipfile. + t.Setenv("PIP_CONFIG_FILE", filepath.Join(t.TempDir(), "nonexistent-pip.conf")) + + blockedURL := "/api/pypi/" + repo + "/packages/aa/urllib3-1.26.20-py2.py3-none-any.whl" + expectedRequest := map[string]bool{blockedURL: false} + requestToFail := map[string]bool{blockedURL: false} + serverMock, config := pipenvCurationServer(t, repo, expectedRequest, requestToFail) + defer serverMock.Close() + + cleanUpHome := integration.UseTestHomeWithDefaultXrayConfig(t) + defer cleanUpHome() + + config.User = "admin" + config.Password = "password" + config.ServerId = "test" + configCmd := commonCommands.NewConfigCommand(commonCommands.AddOrEdit, config.ServerId).SetDetails(config).SetUseBasicAuthOnly(true).SetInteractive(false) + assert.NoError(t, configCmd.Run()) + + // Native detection: Pipenv reads the repo straight from Pipfile [[source]]. + rewritePipfileSourceURL(t, filepath.Join(tempDirPath, "Pipfile"), config.ArtifactoryUrl+"api/pypi/"+repo+"/simple") + + localXrayCli := securityTests.PlatformCli.WithoutCredentials() + workingDirsFlag := fmt.Sprintf("--working-dirs=%s", tempDirPath) + output := localXrayCli.RunCliCmdWithOutput(t, "curation-audit", "--format="+string(format.Json), workingDirsFlag) + + expectedResp := getPipenvCurationExpectedResponse(config, repo) + var got []curation.PackageStatus + bracketIndex := strings.Index(output, "[") + require.Less(t, 0, bracketIndex, "Unexpected Curation output with missing '['") + err := json.Unmarshal([]byte(output[bracketIndex:]), &got) + assert.NoError(t, err) + assert.Equal(t, expectedResp, got) + for k, v := range expectedRequest { + assert.Truef(t, v, "didn't receive expected HEAD request for package url %s", k) + } +} + +// rewritePipfileSourceURL points the fixture's placeholder [[source]] url at the mock. +func rewritePipfileSourceURL(t *testing.T, pipfilePath, newURL string) { + const placeholder = "http://replace-with-mock-server.invalid/api/pypi/pypi-curation/simple" + content, err := os.ReadFile(pipfilePath) // #nosec G304 -- test fixture path built from t.TempDir() + require.NoError(t, err) + updated := strings.Replace(string(content), placeholder, newURL, 1) + require.NotEqual(t, string(content), updated, "placeholder source URL not found in fixture Pipfile") + require.NoError(t, os.WriteFile(pipfilePath, []byte(updated), 0600)) // #nosec G703 -- test fixture path built from t.TempDir() +} + +func getPipenvCurationExpectedResponse(config *config.ServerDetails, repo string) []curation.PackageStatus { + return []curation.PackageStatus{ + { + Action: "blocked", + PackageName: "urllib3", + PackageVersion: "1.26.20", + BlockedPackageUrl: config.ArtifactoryUrl + "api/pypi/" + repo + "/packages/aa/urllib3-1.26.20-py2.py3-none-any.whl", + BlockingReason: curation.BlockingReasonPolicy, + ParentName: "urllib3", + ParentVersion: "1.26.20", + DepRelation: "direct", + PkgType: "pipenv", + Policy: []curation.Policy{ + {Policy: "pol1", Condition: "cond1", Explanation: "explanation", Recommendation: "recommendation"}, + {Policy: "pol2", Condition: "cond2", Explanation: "explanation2", Recommendation: "recommendation2"}, + }, + }, + } +} + +// pipenvCurationServer serves version checks, an empty simple index (forces the +// CVS-fallback path), the metadata API it uses, and HEAD/GET blocked-download probes. +func pipenvCurationServer(t *testing.T, repo string, expectedRequest, requestToFail map[string]bool) (*httptest.Server, *config.ServerDetails) { + mapLock := sync.Mutex{} + metadataPath := "/api/pypi/" + repo + "/pypi/urllib3/1.26.20/json" + serverMock, serverConfig, _ := commonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodHead: + mapLock.Lock() + if _, exist := expectedRequest[r.RequestURI]; exist { + expectedRequest[r.RequestURI] = true + } + mapLock.Unlock() + if _, exist := requestToFail[r.RequestURI]; exist { + w.WriteHeader(http.StatusForbidden) + } + case http.MethodGet: + switch { + case r.RequestURI == "/api/system/version": + _, err := w.Write([]byte(`{"version": "7.82.0"}`)) + require.NoError(t, err) + case r.RequestURI == "/api/v1/system/version": + _, err := w.Write([]byte(`{"xray_version": "3.92.0"}`)) + require.NoError(t, err) + case r.RequestURI == metadataPath: + _, err := w.Write([]byte(`{"urls": [{"packagetype": "bdist_wheel", ` + + `"url": "https://files.pythonhosted.org/packages/aa/urllib3-1.26.20-py2.py3-none-any.whl"}]}`)) + require.NoError(t, err) + case strings.Contains(r.URL.Path, "/simple/"): + // Empty index: pip's resolver reports urllib3==1.26.20 as not found. + _, err := w.Write([]byte(``)) + require.NoError(t, err) + default: + if _, exist := requestToFail[r.RequestURI]; exist { + w.WriteHeader(http.StatusForbidden) + _, err := w.Write([]byte(curationBlockedTarballResponse)) + require.NoError(t, err) + } + } + } + }) + return serverMock, serverConfig +} + func curationServer(t *testing.T, expectedRequest map[string]bool, requestToFail map[string]bool, simpleIndex ...map[string]string) (*httptest.Server, *config.ServerDetails) { mapLockReadWrite := sync.Mutex{} var index map[string]string diff --git a/go.mod b/go.mod index 729e44ccb..10c0b7d8f 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.26.3 replace github.com/CycloneDX/cyclonedx-go => github.com/CycloneDX/cyclonedx-go v0.10.0 require ( + github.com/BurntSushi/toml v1.6.0 github.com/CycloneDX/cyclonedx-go v0.11.0 github.com/beevik/etree v1.6.0 github.com/go-git/go-git/v5 v5.19.1 @@ -25,6 +26,7 @@ require ( github.com/package-url/packageurl-go v0.1.3 github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.11.1 + github.com/subosito/gotenv v1.6.0 github.com/tidwall/gjson v1.18.0 github.com/tidwall/sjson v1.2.5 github.com/urfave/cli v1.22.17 @@ -33,12 +35,12 @@ require ( golang.org/x/sync v0.22.0 golang.org/x/sys v0.45.0 golang.org/x/text v0.37.0 + gopkg.in/ini.v1 v1.67.1 gopkg.in/yaml.v3 v3.0.1 ) require ( dario.cat/mergo v1.0.2 // indirect - github.com/BurntSushi/toml v1.6.0 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/ProtonMail/go-crypto v1.4.1 // indirect github.com/VividCortex/ewma v1.2.0 // indirect @@ -125,7 +127,6 @@ require ( github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/spf13/pflag v1.0.10 // indirect - github.com/subosito/gotenv v1.6.0 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect github.com/ulikunitz/xz v0.5.15 // indirect @@ -153,7 +154,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20260319201613-d00831a3d3e7 // indirect google.golang.org/grpc v1.79.3 // indirect google.golang.org/protobuf v1.36.11 // indirect - gopkg.in/ini.v1 v1.67.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect ) diff --git a/sca/bom/buildinfo/technologies/python/python.go b/sca/bom/buildinfo/technologies/python/python.go index 50ae25dd0..477e5655f 100644 --- a/sca/bom/buildinfo/technologies/python/python.go +++ b/sca/bom/buildinfo/technologies/python/python.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" + "github.com/BurntSushi/toml" "github.com/jfrog/gofrog/version" biutils "github.com/jfrog/build-info-go/utils" @@ -38,7 +39,9 @@ import ( "sync" "github.com/spf13/viper" + "github.com/subosito/gotenv" "golang.org/x/sync/errgroup" + "gopkg.in/ini.v1" ) const ( @@ -52,6 +55,7 @@ const ( CurationPoetryMinimumVersion = "1.2.0" CurationPipenvMinimumVersion = "2023.7.4" pipfileFile = "Pipfile" + pipfileLockFile = "Pipfile.lock" poetryDownloadUrlWorkers = 8 ) @@ -164,7 +168,8 @@ func getDependencies(params technologies.BuildInfoBomGeneratorParams, technology } pythonTool := pythonutils.PythonTool(technology) - if technology == techutils.Pipenv || !params.SkipAutoInstall { + + if shouldRunPythonInstall(params, technology) { var restoreEnv func() error rootDetected, restoreEnv, err = runPythonInstall(params, pythonTool) defer func() { @@ -174,8 +179,8 @@ func getDependencies(params technologies.BuildInfoBomGeneratorParams, technology return } } else { - log.Debug(fmt.Sprintf("JF_SKIP_AUTO_INSTALL was set to 'true' with one of the following technologies: %s, %s. Skipping installation...\n"+ - "NOTE: in this case all dependencies must be manually pre-installed by the user", techutils.Pip, techutils.Poetry)) + log.Debug(fmt.Sprintf("JF_SKIP_AUTO_INSTALL was set to 'true' for %s. Skipping installation...\n"+ + "NOTE: in this case all dependencies must be manually pre-installed by the user", technology)) } localDependenciesPath, err := config.GetJfrogDependenciesPath() @@ -217,6 +222,10 @@ func getDependencies(params technologies.BuildInfoBomGeneratorParams, technology return } +func shouldRunPythonInstall(params technologies.BuildInfoBomGeneratorParams, technology techutils.Technology) bool { + return !params.SkipAutoInstall || (technology == techutils.Pipenv && params.IsCurationCmd) +} + func processPipDownloadsUrlsFromReportFile() (map[string]string, error) { pipReport, err := readPipReportIfExists() if err != nil { @@ -414,8 +423,42 @@ func NormalizePypiName(name string) string { // pipfileLockEntry is one package entry in Pipfile.lock's "default" or "develop" section. type pipfileLockEntry struct { - Version string `json:"version"` // e.g. "==2.0.7" - Hashes []string `json:"hashes"` // e.g. ["sha256:abc123..."] + Version string `json:"version"` + Hashes []string `json:"hashes"` + Index string `json:"index"` + File string `json:"file"` + URL string `json:"url"` + Path string `json:"path"` + Editable bool `json:"editable"` + Git string `json:"git"` + Hg string `json:"hg"` + Svn string `json:"svn"` + Bzr string `json:"bzr"` + Unknown []string `json:"-"` +} + +func (e *pipfileLockEntry) UnmarshalJSON(data []byte) error { + type lockEntryAlias pipfileLockEntry + var decoded lockEntryAlias + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return err + } + known := map[string]struct{}{ + "version": {}, "hashes": {}, "index": {}, "file": {}, "url": {}, "path": {}, + "editable": {}, "git": {}, "hg": {}, "svn": {}, "bzr": {}, "ref": {}, + "subdirectory": {}, "markers": {}, "extras": {}, + } + for field := range fields { + if _, ok := known[field]; !ok { + decoded.Unknown = append(decoded.Unknown, field) + } + } + *e = pipfileLockEntry(decoded) + return nil } // pipfileLockContent is the top-level structure of Pipfile.lock. @@ -450,48 +493,46 @@ func buildPipenvDownloadUrlsMap(serverDetails *config.ServerDetails, repository if err != nil { return nil, err } - rtManager, err := rtUtils.CreateServiceManager(serverDetails, 2, 0, false) + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) if err != nil { return nil, err } httpClientDetails := rtAuth.CreateHttpClientDetails() artiUrl := strings.TrimSuffix(serverDetails.GetArtifactoryUrl(), "/") urls := map[string]string{} - skipped := 0 var mu sync.Mutex + var lookupErrs []error g := new(errgroup.Group) g.SetLimit(poetryDownloadUrlWorkers) for _, pkg := range packages { - if pkg.Name == "" || pkg.Version == "" || len(pkg.Hashes) == 0 { - skipped++ - continue - } g.Go(func() error { - return resolvePipenvPackageURL(rtManager, httpClientDetails, artiUrl, repository, pkg, urls, &mu) + return resolvePipenvPackageURL(rtManager, httpClientDetails, artiUrl, repository, pkg, urls, &mu, &lookupErrs) }) } - _ = g.Wait() + _ = g.Wait() // failures are collected into lookupErrs, not returned here - expected := len(packages) - skipped - resolved := len(urls) - if resolved < expected { - log.Warn(fmt.Sprintf( - "Pipenv: resolved download URLs for %d/%d packages — %d package(s) will not be HEAD-checked by curation. "+ - "Re-run with JFROG_CLI_LOG_LEVEL=DEBUG to see per-package resolution errors.", - resolved, expected, expected-resolved)) + if len(lookupErrs) > 0 { + // Unresolved packages are never HEAD-checked; fail loudly instead of a false-clean partial audit. + return nil, errorutils.CheckErrorf( + "pipenv: failed to resolve download URLs for %d of %d package(s) — curation cannot verify these packages: %s", + len(lookupErrs), len(packages), errors.Join(lookupErrs...)) } + log.Debug(fmt.Sprintf("Pipenv: resolved %d download URLs", len(urls))) return urls, nil } -func resolvePipenvPackageURL(rtManager artifactory.ArtifactoryServicesManager, httpClientDetails httputils.HttpClientDetails, artiUrl, repository string, pkg pipfileLockPackage, urls map[string]string, mu *sync.Mutex) error { +func resolvePipenvPackageURL(rtManager artifactory.ArtifactoryServicesManager, httpClientDetails httputils.HttpClientDetails, artiUrl, repository string, pkg pipfileLockPackage, urls map[string]string, mu *sync.Mutex, lookupErrs *[]error) error { localDetails := httpClientDetails.Clone() downloadUrl, lookupErr := buildPipenvDownloadUrl(rtManager, localDetails, artiUrl, repository, pkg) if lookupErr != nil { - log.Debug(fmt.Sprintf("Pipenv: could not resolve download URL for %s:%s: %v", pkg.Name, pkg.Version, lookupErr)) + mu.Lock() + *lookupErrs = append(*lookupErrs, fmt.Errorf("%s:%s: %w", pkg.Name, pkg.Version, lookupErr)) + mu.Unlock() return nil } - normalizedName := strings.ReplaceAll(strings.ToLower(strings.TrimSpace(pkg.Name)), "-", "_") + // PEP 503 normalization keeps the map key aligned with the hyphenated pipenv graph node IDs. + normalizedName := NormalizePypiName(pkg.Name) compId := PythonPackageTypeIdentifier + normalizedName + ":" + pkg.Version mu.Lock() urls[compId] = downloadUrl @@ -504,9 +545,15 @@ func resolvePipenvPackageURL(rtManager artifactory.ArtifactoryServicesManager, h // one of the hashes recorded in Pipfile.lock. func buildPipenvDownloadUrl(rtManager artifactory.ArtifactoryServicesManager, clientDetails *httputils.HttpClientDetails, artiUrl, repository string, pkg pipfileLockPackage) (string, error) { normalized := NormalizePypiName(pkg.Name) - simpleIndexUrl := fmt.Sprintf("%s/api/pypi/%s/simple/%s/", artiUrl, repository, normalized) + repositoryURL := fmt.Sprintf("%s/api/pypi/%s/", artiUrl, repository) + boundary, err := utils.NewEndpointBoundary(repositoryURL) + if err != nil { + return "", err + } + simpleIndexUrl := repositoryURL + "simple/" + normalized + "/" log.Debug(fmt.Sprintf("Pipenv: GET simple-index %s (matching against %d hashes)", simpleIndexUrl, len(pkg.Hashes))) - resp, body, _, err := rtManager.Client().SendGet(simpleIndexUrl, true, clientDetails) + resp, body, err := utils.SendWithBoundedRedirects(rtManager.Client(), http.MethodGet, simpleIndexUrl, + clientDetails, boundary, utils.MaxAuthenticatedRedirects) if err != nil { return "", err } @@ -539,6 +586,9 @@ func buildPipenvDownloadUrl(rtManager artifactory.ArtifactoryServicesManager, cl return "", err } absolute := base.ResolveReference(ref) + if err := boundary.Validate(absolute.String()); err != nil { + return "", fmt.Errorf("resolved href %q for %s escapes the configured Artifactory endpoint %q", href, pkg.Name, artiUrl) + } // Strip the fragment — fetchNodeStatus only needs the bare download URL. absolute.Fragment = "" log.Debug(fmt.Sprintf("Pipenv: resolved %s:%s -> %s", pkg.Name, pkg.Version, absolute)) @@ -561,14 +611,17 @@ func pickHrefByHashFragment(body []byte, wantedFragments map[string]struct{}) st } func readPipfileLockPackages() ([]pipfileLockPackage, error) { - exists, err := fileutils.IsFileExists("Pipfile.lock", false) + exists, err := fileutils.IsFileExists(pipfileLockFile, false) if err != nil { return nil, errorutils.CheckError(err) } if !exists { - return nil, errorutils.CheckErrorf("Pipfile.lock not found — run 'pipenv lock' to generate it before running 'jf ca'") + // 'pipenv install' (run earlier in this flow) auto-generates Pipfile.lock when + // missing, so reaching this point means it unexpectedly failed to do so — + // this is not something the user needs to fix manually. + return nil, errorutils.CheckErrorf("pipenv: Pipfile.lock is unexpectedly missing after 'pipenv install' completed successfully") } - content, err := os.ReadFile("Pipfile.lock") + content, err := os.ReadFile(pipfileLockFile) // #nosec G304 -- temp-dir copy of the project's Pipfile.lock if err != nil { return nil, errorutils.CheckError(err) } @@ -581,35 +634,102 @@ func parsePipfileLockPackages(content []byte) ([]pipfileLockPackage, error) { if err := json.Unmarshal(content, &lock); err != nil { return nil, fmt.Errorf("failed to parse Pipfile.lock: %w", err) } - seen := map[string]struct{}{} + if lock.Default == nil && lock.Develop == nil { + return nil, errors.New("pipenv: Pipfile.lock has no default or develop dependency sections") + } + seen := map[string]pipfileLockPackage{} var packages []pipfileLockPackage - addEntry := func(name string, entry pipfileLockEntry) { - // Version field is like "==2.0.7" — strip the == operator. - version := strings.TrimPrefix(entry.Version, "==") - if name == "" || version == "" { - return + addEntry := func(name string, entry pipfileLockEntry) error { + if name == "" { + return errors.New("pipenv: Pipfile.lock contains an entry with no package name") } - key := strings.ToLower(name) + ":" + version - if _, dup := seen[key]; dup { - return + if len(entry.Unknown) > 0 { + return fmt.Errorf("pipenv: package %q has unknown lock fields %q", name, entry.Unknown) } - seen[key] = struct{}{} - packages = append(packages, pipfileLockPackage{ - Name: name, - Version: version, - Hashes: entry.Hashes, - }) + vcsCount := 0 + for _, value := range []string{entry.Git, entry.Hg, entry.Svn, entry.Bzr} { + if value != "" { + vcsCount++ + } + } + directKinds := 0 + if entry.Path != "" { + directKinds++ + } + if entry.File != "" { + directKinds++ + } + if entry.URL != "" { + directKinds++ + } + if vcsCount > 0 { + directKinds++ + } + if vcsCount > 1 || directKinds > 1 || (directKinds > 0 && (entry.Version != "" || entry.Index != "")) { + return fmt.Errorf("pipenv: package %q has conflicting provenance in Pipfile.lock", name) + } + if entry.File != "" || entry.URL != "" { + return fmt.Errorf("pipenv: package %q uses an unsupported direct-file dependency", name) + } + if entry.Path != "" || vcsCount == 1 { + return nil + } + if entry.Editable { + return fmt.Errorf("pipenv: editable package %q has no recognized local path or VCS provenance", name) + } + if entry.Version == "" { + return fmt.Errorf("pipenv: package %q has unknown provenance in Pipfile.lock", name) + } + if !strings.HasPrefix(entry.Version, "==") || len(entry.Version) == 2 { + return fmt.Errorf("pipenv: registry package %q has unsupported locked version %q", name, entry.Version) + } + version := strings.TrimPrefix(entry.Version, "==") + if len(entry.Hashes) == 0 { + return fmt.Errorf("pipenv: registry package %q at version %q has no hashes in Pipfile.lock", name, version) + } + pkg := pipfileLockPackage{Name: name, Version: version, Hashes: entry.Hashes} + key := NormalizePypiName(name) + if existing, dup := seen[key]; dup { + if existing.Version != pkg.Version || !sameStringSet(existing.Hashes, pkg.Hashes) { + return fmt.Errorf("pipenv: package %q has conflicting locked entries", name) + } + return nil + } + seen[key] = pkg + packages = append(packages, pkg) + return nil } for name, entry := range lock.Default { - addEntry(name, entry) + if err := addEntry(name, entry); err != nil { + return nil, err + } } for name, entry := range lock.Develop { - addEntry(name, entry) + if err := addEntry(name, entry); err != nil { + return nil, err + } } return packages, nil } +func sameStringSet(left, right []string) bool { + if len(left) != len(right) { + return false + } + values := make(map[string]int, len(left)) + for _, value := range left { + values[value]++ + } + for _, value := range right { + if values[value] == 0 { + return false + } + values[value]-- + } + return true +} + func readPoetryLockIfExists() ([]poetryLockPackage, error) { exists, err := fileutils.IsFileExists(poetryLockFile, false) if err != nil { @@ -989,8 +1109,9 @@ func parsePipenvVersion(out string) string { return m[1] } -// validateMinimumPipenvVersion checks that the pipenv CLI on PATH meets minVersion. -func validateMinimumPipenvVersion(minVersion string) error { +// validateMinimumPipenvVersion checks that the pipenv CLI on PATH meets CurationPipenvMinimumVersion. +func validateMinimumPipenvVersion() error { + minVersion := CurationPipenvMinimumVersion out, err := executeCommand("pipenv", "--version") if err != nil { log.Debug(fmt.Sprintf("pipenv is not installed or not on PATH: %v", err)) @@ -1008,33 +1129,156 @@ func validateMinimumPipenvVersion(minVersion string) error { return nil } -// pipfileSourceURLRegex extracts `url = "..."` values from Pipfile [[source]] blocks. -var pipfileSourceURLRegex = regexp.MustCompile(`(?m)^\s*url\s*=\s*"([^"]+)"`) +var ( + pipfileEnvVarRegex = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}|\$([A-Za-z0-9_]+)`) + pipfileWindowsExpandVars = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}|\$([A-Za-z0-9_]+)|%[0-9A-Fa-f]{2}|%([A-Za-z0-9_]+)%|%%`) + pipfilePercentEncodedByte = regexp.MustCompile(`^%[0-9A-Fa-f]{2}$`) +) + +type pipfileSource struct { + Name string `toml:"name"` + URL string `toml:"url"` + VerifySSL bool `toml:"verify_ssl"` +} + +type pipfileConfig struct { + Sources []pipfileSource `toml:"source"` + Packages map[string]any `toml:"packages"` + DevPackages map[string]any `toml:"dev-packages"` + Pipenv struct { + InstallSearchAllSources bool `toml:"install_search_all_sources"` + } `toml:"pipenv"` +} + +type pipfileSourceRecord struct { + index int + name string + rawURL string + parsedURL *url.URL + server *config.ServerDetails + repository string + isPyPI bool +} + +type pipenvEndpoint struct { + scheme string + host string + basePath string + repository string +} + +func (e pipenvEndpoint) String() string { + return fmt.Sprintf("%s://%s%s/api/pypi/%s", e.scheme, e.host, e.basePath, e.repository) +} + +type pipfileVariableError struct { + Source string + Variable string +} + +func (e *pipfileVariableError) Error() string { + return fmt.Sprintf("pipenv: source %q references unset environment variable %q", e.Source, e.Variable) +} + +func expandPipfileEnvVars(raw, sourceName string, environment map[string]string) (string, error) { + return expandPipfileEnvVarsForOS(raw, sourceName, environment, runtime.GOOS) +} + +func expandPipfileEnvVarsForOS(raw, sourceName string, environment map[string]string, goos string) (string, error) { + if strings.Contains(pipfileEnvVarRegex.ReplaceAllString(raw, ""), "$") { + return "", fmt.Errorf("pipenv: source %q contains a malformed environment variable reference", sourceName) + } + var expansionErr error + expand := func(pattern *regexp.Regexp, value string) string { + return pattern.ReplaceAllStringFunc(value, func(match string) string { + if match == "%%" { // cmd.exe/ntpath convention: an escaped literal '%' + return "%" + } + if pipfilePercentEncodedByte.MatchString(match) { + // A URL percent-encoded byte (e.g. %40, %2F), not a %VAR% reference. + // Leave it untouched here; url.Parse decodes it once the caller parses the URL. + return match + } + groups := pattern.FindStringSubmatch(match) + name := "" + for _, group := range groups[1:] { + if group != "" { + name = group + break + } + } + expanded, ok := environment[name] + if !ok && goos == "windows" { + for environmentName, value := range environment { + if strings.EqualFold(environmentName, name) { + expanded, ok = value, true + break + } + } + } + if !ok { + expansionErr = &pipfileVariableError{Source: sourceName, Variable: name} + return match + } + return expanded + }) + } + if goos == "windows" { + expanded := expand(pipfileWindowsExpandVars, raw) + if expansionErr != nil { + return "", expansionErr + } + return expanded, nil + } + expanded := expand(pipfileEnvVarRegex, raw) + if expansionErr != nil { + return "", expansionErr + } + return expanded, nil +} -// pipConfigIndexURLRegex matches the `index-url = ...` key in pip.conf / pip.ini. -// The value may or may not be quoted; leading/trailing whitespace is trimmed. -var pipConfigIndexURLRegex = regexp.MustCompile(`(?im)^\s*index-url\s*=\s*(.+)`) +func pipfileEnvironment(pipfilePath string) (map[string]string, error) { + environment := map[string]string{} + dontLoad := strings.ToLower(strings.TrimSpace(os.Getenv("PIPENV_DONT_LOAD_ENV"))) + skipDotenv := dontLoad == "1" || dontLoad == "true" || dontLoad == "yes" || dontLoad == "on" + if !skipDotenv { + envPath := os.Getenv("PIPENV_DOTENV_LOCATION") + if envPath == "" { + envPath = filepath.Join(filepath.Dir(pipfilePath), ".env") + } + envFile, err := gotenv.Read(envPath) // #nosec G304 -- Pipenv supports a user-selected dotenv path + if err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("pipenv: failed to parse environment file %q", filepath.Base(envPath)) + } + for name, value := range envFile { + environment[name] = value + } + } + for _, entry := range os.Environ() { + name, value, ok := strings.Cut(entry, "=") + if ok { + environment[name] = value + } + } + return environment, nil +} -// parseArtifactoryPypiURL parses a raw PyPI index URL and, if it matches the -// Artifactory pattern (.../artifactory/api/pypi//simple[/]), returns the -// extracted ServerDetails (with credentials) and the repo name. -// Returns (nil, "", nil) when the URL is not an Artifactory PyPI URL. -func parseArtifactoryPypiURL(rawURL string) (*config.ServerDetails, string, error) { +func parseArtifactoryPypiURL(rawURL string) (*config.ServerDetails, string) { u, err := url.Parse(strings.TrimSpace(rawURL)) - if err != nil || u.Host == "" { - return nil, "", nil + if err != nil || u.Host == "" || u.RawQuery != "" || u.Fragment != "" || + (!strings.EqualFold(u.Scheme, "http") && !strings.EqualFold(u.Scheme, "https")) { + return nil, "" } const pypiSegment = "/api/pypi/" pypiIdx := strings.Index(u.Path, pypiSegment) if pypiIdx < 0 { - return nil, "", nil + return nil, "" } - rest := strings.TrimPrefix(u.Path[pypiIdx:], pypiSegment) - repoSlashIdx := strings.Index(rest, "/") - if repoSlashIdx <= 0 { - return nil, "", nil + parts := strings.Split(strings.Trim(strings.TrimPrefix(u.Path[pypiIdx:], pypiSegment), "/"), "/") + if len(parts) != 2 || !validPipenvRepository(parts[0]) || parts[1] != "simple" { + return nil, "" } - repoName := rest[:repoSlashIdx] + repoName := parts[0] artURL := fmt.Sprintf("%s://%s%s", u.Scheme, u.Host, u.Path[:pypiIdx+1]) sd := &config.ServerDetails{ArtifactoryUrl: artURL} if u.User != nil { @@ -1043,129 +1287,634 @@ func parseArtifactoryPypiURL(rawURL string) (*config.ServerDetails, string, erro sd.Password = p } } - return sd, repoName, nil + return sd, repoName } -// ParsePipfileArtifactorySource reads the Pipfile at the given path and returns -// the first [[source]] entry whose URL matches the Artifactory PyPI pattern -// (.../artifactory/api/pypi//simple). Credentials embedded in the URL are -// extracted into the returned ServerDetails. -// Returns (nil, "", nil) when no matching source is found (not an error). -func ParsePipfileArtifactorySource(pipfilePath string) (serverDetails *config.ServerDetails, repoName string, err error) { - data, err := os.ReadFile(pipfilePath) +func parsePipfileSource(index int, source pipfileSource, environment map[string]string) (pipfileSourceRecord, error) { + if strings.TrimSpace(source.Name) == "" { + return pipfileSourceRecord{}, fmt.Errorf("pipenv: [[source]] entry %d has no name", index+1) + } + if strings.TrimSpace(source.URL) == "" { + return pipfileSourceRecord{}, fmt.Errorf("pipenv: source %q has no URL", source.Name) + } + expanded, err := expandPipfileEnvVars(source.URL, source.Name, environment) if err != nil { - return nil, "", err + return pipfileSourceRecord{}, err + } + u, err := url.Parse(expanded) + if err != nil || u.Host == "" || u.RawQuery != "" || u.Fragment != "" || + (!strings.EqualFold(u.Scheme, "http") && !strings.EqualFold(u.Scheme, "https")) { + return pipfileSourceRecord{}, fmt.Errorf("pipenv: source %q has an invalid absolute URL", source.Name) + } + if u.User != nil { + user := u.User.Username() + password, hasPassword := u.User.Password() + if user == "" || !hasPassword || password == "" { + return pipfileSourceRecord{}, fmt.Errorf("pipenv: source %q has incomplete URL credentials", source.Name) + } + } + sd, repo := parseArtifactoryPypiURL(expanded) + return pipfileSourceRecord{ + index: index, + name: source.Name, + rawURL: source.URL, + parsedURL: u, + server: sd, + repository: repo, + isPyPI: strings.EqualFold(u.Hostname(), "pypi.org") && path.Clean(u.Path) == "/simple", + }, nil +} + +func packageIndex(spec any) (string, bool, error) { + value, ok := spec.(map[string]any) + if !ok { + return "", false, nil + } + indexValue, exists := value["index"] + if !exists { + return "", false, nil + } + index, ok := indexValue.(string) + if !ok || strings.TrimSpace(index) == "" { + return "", false, errors.New("index must be a non-empty string") + } + return index, true, nil +} + +func effectivePipfileSourceNames(cfg pipfileConfig) ([]string, error) { + if len(cfg.Sources) == 0 { + for packageName, spec := range cfg.Packages { + if _, assigned, err := packageIndex(spec); err != nil || assigned { + return nil, fmt.Errorf("pipenv: package %q assigns an index but Pipfile has no [[source]] entries", packageName) + } + } + for packageName, spec := range cfg.DevPackages { + if _, assigned, err := packageIndex(spec); err != nil || assigned { + return nil, fmt.Errorf("pipenv: dev package %q assigns an index but Pipfile has no [[source]] entries", packageName) + } + } + return nil, nil + } + + // Sources[0] is effective only for index-less packages (or when there are none); + // an index-assigned package never falls back to it (pipenv dependency-confusion fix). + names := map[string]struct{}{} + addSpecs := func(section string, specs map[string]any) error { + for packageName, spec := range specs { + index, assigned, err := packageIndex(spec) + if err != nil { + return fmt.Errorf("pipenv: %s package %q has invalid index assignment: %w", section, packageName, err) + } + if assigned { + names[index] = struct{}{} + } else { + names[cfg.Sources[0].Name] = struct{}{} + } + } + return nil + } + if err := addSpecs("regular", cfg.Packages); err != nil { + return nil, err + } + if err := addSpecs("development", cfg.DevPackages); err != nil { + return nil, err + } + if len(names) == 0 { + names[cfg.Sources[0].Name] = struct{}{} + } + if cfg.Pipenv.InstallSearchAllSources { + for _, source := range cfg.Sources { + names[source.Name] = struct{}{} + } + } + effective := make([]string, 0, len(names)) + for _, source := range cfg.Sources { + if _, ok := names[source.Name]; ok { + effective = append(effective, source.Name) + delete(names, source.Name) + } + } + if len(names) > 0 { + for name := range names { + return nil, fmt.Errorf("pipenv: package index assignment references unknown source %q", name) + } + } + return effective, nil +} + +func readPipfileConfig(pipfilePath string) (pipfileConfig, []pipfileSourceRecord, []string, error) { + var cfg pipfileConfig + if _, err := toml.DecodeFile(pipfilePath, &cfg); err != nil { + return cfg, nil, nil, err + } + effective, err := effectivePipfileSourceNames(cfg) + if err != nil { + return cfg, nil, nil, err + } + effectiveSet := make(map[string]struct{}, len(effective)) + for _, name := range effective { + effectiveSet[name] = struct{}{} + } + environment, err := pipfileEnvironment(pipfilePath) + if err != nil { + return cfg, nil, nil, err + } + records := make([]pipfileSourceRecord, 0, len(cfg.Sources)) + nameCounts := make(map[string]int, len(cfg.Sources)) + for _, source := range cfg.Sources { + nameCounts[source.Name]++ } - for _, m := range pipfileSourceURLRegex.FindAllStringSubmatch(string(data), -1) { - sd, repo, parseErr := parseArtifactoryPypiURL(m[1]) - if parseErr != nil || repo == "" { + for i, source := range cfg.Sources { + if _, isEffective := effectiveSet[source.Name]; isEffective && nameCounts[source.Name] > 1 { + return cfg, nil, nil, fmt.Errorf("pipenv: Pipfile declares duplicate source name %q", source.Name) + } + if _, isEffective := effectiveSet[source.Name]; !isEffective { + records = append(records, pipfileSourceRecord{index: i, name: source.Name, rawURL: source.URL}) continue } - return sd, repo, nil + record, err := parsePipfileSource(i, source, environment) + if err != nil { + return cfg, nil, nil, err + } + records = append(records, record) } - return nil, "", nil + return cfg, records, effective, nil } -// ParsePipConfigIndexUrl extracts Artifactory server details and repo name -// from the [global] index-url in the pip config file. Returns (nil, "", nil) -// when the file is missing or index-url is not an Artifactory URL. -func ParsePipConfigIndexUrl(pipConfPath string) (serverDetails *config.ServerDetails, repoName string, err error) { - data, readErr := os.ReadFile(pipConfPath) +func endpointFromServer(server *config.ServerDetails, repository string) (pipenvEndpoint, error) { + if server == nil || server.GetArtifactoryUrl() == "" || !validPipenvRepository(repository) { + return pipenvEndpoint{}, errors.New("server URL and repository are required") + } + u, err := url.Parse(server.GetArtifactoryUrl()) + if err != nil || u.Host == "" || u.User != nil || u.RawQuery != "" || u.Fragment != "" || + (!strings.EqualFold(u.Scheme, "http") && !strings.EqualFold(u.Scheme, "https")) { + return pipenvEndpoint{}, errors.New("invalid Artifactory URL") + } + cleanBasePath := strings.TrimSuffix(path.Clean(u.Path), "/") + if cleanBasePath != strings.TrimSuffix(u.Path, "/") { + return pipenvEndpoint{}, errors.New("invalid Artifactory URL path") + } + return pipenvEndpoint{ + scheme: strings.ToLower(u.Scheme), + host: normalizedURLHost(u), + basePath: cleanBasePath, + repository: repository, + }, nil +} + +func validPipenvRepository(repository string) bool { + return repository != "" && repository != "." && repository != ".." && + !strings.ContainsAny(repository, `/\`) +} + +func endpointFromSource(record pipfileSourceRecord) (pipenvEndpoint, error) { + if record.server == nil || record.repository == "" { + return pipenvEndpoint{}, fmt.Errorf("source %q is not an Artifactory PyPI source", record.name) + } + return endpointFromServer(record.server, record.repository) +} + +func normalizedURLHost(u *url.URL) string { + host := strings.ToLower(u.Hostname()) + port := u.Port() + if port == "" { + switch strings.ToLower(u.Scheme) { + case "http": + port = "80" + case "https": + port = "443" + } + } + return host + ":" + port +} + +func samePipenvEndpoint(left, right pipenvEndpoint) bool { + return left.scheme == right.scheme && left.host == right.host && + left.basePath == right.basePath && left.repository == right.repository +} + +type credentialState int + +const ( + noCredentials credentialState = iota + completeCredentials + partialCredentials +) + +func serverCredentialState(server *config.ServerDetails) credentialState { + if server == nil { + return noCredentials + } + if server.GetAccessToken() != "" { + return completeCredentials + } + hasUser := server.GetUser() != "" + hasPassword := server.GetPassword() != "" + if hasUser != hasPassword { + return partialCredentials + } + if hasUser { + return completeCredentials + } + return noCredentials +} + +func sameCredentials(left, right *config.ServerDetails) bool { + return left.GetUser() == right.GetUser() && left.GetPassword() == right.GetPassword() && + left.GetAccessToken() == right.GetAccessToken() +} + +func mergePipenvCredentials(target *config.ServerDetails, targetEndpoint pipenvEndpoint, sourceCredentials []*config.ServerDetails, fallback *config.ServerDetails) (*config.ServerDetails, error) { + merged := *target + fallbackMatches := false + if fallback != nil { + fallbackEndpoint, err := endpointFromServer(fallback, targetEndpoint.repository) + fallbackMatches = err == nil && samePipenvEndpoint(fallbackEndpoint, targetEndpoint) + if fallbackMatches { + merged = *fallback + merged.ArtifactoryUrl = target.GetArtifactoryUrl() + } + } + merged.User, merged.Password, merged.AccessToken = "", "", "" + + var selected *config.ServerDetails + for _, source := range sourceCredentials { + switch serverCredentialState(source) { + case partialCredentials: + return nil, errors.New("pipenv: selected Pipfile source has incomplete credentials") + case completeCredentials: + if selected != nil && !sameCredentials(selected, source) { + return nil, errors.New("pipenv: effective Pipfile sources contain conflicting credentials") + } + selected = source + } + } + if selected == nil { + switch serverCredentialState(target) { + case partialCredentials: + return nil, errors.New("pipenv: configured resolver has incomplete credentials") + case completeCredentials: + selected = target + } + } + if selected == nil && fallbackMatches { + switch serverCredentialState(fallback) { + case partialCredentials: + return nil, errors.New("pipenv: matching JFrog server has incomplete credentials") + case completeCredentials: + selected = fallback + } + } + if selected != nil { + merged.User = selected.GetUser() + if selected.GetAccessToken() != "" { + merged.AccessToken = selected.GetAccessToken() + } else { + merged.Password = selected.GetPassword() + } + } + return &merged, nil +} + +// ResolvePipfileArtifactorySource derives and validates Pipenv's effective sources. +func ResolvePipfileArtifactorySource(pipfilePath string, configuredServer *config.ServerDetails, configuredRepo string, fallbackServer *config.ServerDetails) (*config.ServerDetails, string, error) { + _, records, effectiveNames, err := readPipfileConfig(pipfilePath) + if err != nil { + return nil, "", err + } + byName := make(map[string]pipfileSourceRecord, len(records)) + for _, record := range records { + byName[record.name] = record + } + + var target *config.ServerDetails + var targetEndpoint pipenvEndpoint + if configuredRepo != "" { + target = configuredServer + targetEndpoint, err = endpointFromServer(configuredServer, configuredRepo) + if err != nil { + return nil, "", fmt.Errorf("pipenv: invalid configured resolver: %w", err) + } + } else { + for _, name := range effectiveNames { + record := byName[name] + if record.repository == "" { + continue + } + target = record.server + targetEndpoint, err = endpointFromSource(record) + if err != nil { + return nil, "", err + } + configuredRepo = record.repository + break + } + if target == nil { + return nil, "", nil + } + } + + var matchingSourceCredentials []*config.ServerDetails + for _, name := range effectiveNames { + record := byName[name] + if record.repository == "" { + if target != nil && record.isPyPI { + continue + } + return nil, "", fmt.Errorf("pipenv: source %q does not use the configured Artifactory repository", record.name) + } + sourceEndpoint, endpointErr := endpointFromSource(record) + if endpointErr != nil { + return nil, "", endpointErr + } + if !samePipenvEndpoint(sourceEndpoint, targetEndpoint) { + return nil, "", fmt.Errorf("pipenv: source %q uses %s instead of %s", record.name, sourceEndpoint, targetEndpoint) + } + matchingSourceCredentials = append(matchingSourceCredentials, record.server) + } + if target.GetArtifactoryUrl() == "" { + target = &config.ServerDetails{ArtifactoryUrl: targetEndpoint.scheme + "://" + targetEndpoint.host + targetEndpoint.basePath + "/"} + } + merged, err := mergePipenvCredentials(target, targetEndpoint, matchingSourceCredentials, fallbackServer) + if err != nil { + return nil, "", err + } + return merged, configuredRepo, nil +} + +func ParsePipfileArtifactorySource(pipfilePath string) (*config.ServerDetails, string, error) { + return ResolvePipfileArtifactorySource(pipfilePath, nil, "", nil) +} + +// ParsePipConfigIndexUrl extracts Artifactory server details and repo name from the +// [global] index-url, merging paths in order (later overrides earlier), pip-style. +// sourcePath is the file the winning value came from, for logging/errors. +func ParsePipConfigIndexUrl(paths ...string) (serverDetails *config.ServerDetails, repoName string, sourcePath string, err error) { + var rawURL string + for _, path := range paths { + found, indexURL, fileErr := pipConfigGlobalIndexURL(path) + if fileErr != nil { + return nil, "", "", fileErr + } + if found { + rawURL = indexURL + sourcePath = path + } + } + if rawURL == "" { + return nil, "", "", nil + } + sd, repo := parseArtifactoryPypiURL(rawURL) + if sd == nil { + parsed, parseErr := url.Parse(rawURL) + if parseErr != nil || parsed.Scheme == "" || parsed.Host == "" || strings.Contains(rawURL, "/api/pypi/") { + return nil, "", "", fmt.Errorf("pip config \"%s\" [global] index-url is not a valid Artifactory PyPI URL", sourcePath) + } + } + return sd, repo, sourcePath, nil +} + +// pipConfigGlobalIndexURL reads a single pip config file's [global] index-url. +// found is false (with no error) when the file is missing or has no such key. +func pipConfigGlobalIndexURL(pipConfPath string) (found bool, rawURL string, err error) { + data, readErr := os.ReadFile(pipConfPath) // #nosec G304 -- path is DefaultPipConfPaths() or PIP_CONFIG_FILE env var, not attacker-controlled if readErr != nil { - // Missing or unreadable — not an error, just no config available. - return nil, "", nil + if os.IsNotExist(readErr) { + log.Debug(fmt.Sprintf("pip.conf not found at %s", pipConfPath)) + return false, "", nil + } + return false, "", fmt.Errorf("failed to read pip config %q: %w", pipConfPath, readErr) } - m := pipConfigIndexURLRegex.FindSubmatch(data) - if m == nil { - return nil, "", nil + // AllowShadows/AllowNonUniqueSections surface duplicates instead of silently picking one. + cfg, loadErr := ini.LoadSources(ini.LoadOptions{AllowNonUniqueSections: true, AllowShadows: true}, data) + if loadErr != nil { + return false, "", fmt.Errorf("failed to parse pip config %q: %w", pipConfPath, loadErr) } - rawURL := strings.Trim(strings.TrimSpace(string(m[1])), `"'`) - sd, repo, parseErr := parseArtifactoryPypiURL(rawURL) - if parseErr != nil { - return nil, "", nil + var globalSections []*ini.Section + for _, section := range cfg.Sections() { + if strings.EqualFold(section.Name(), "global") { + globalSections = append(globalSections, section) + } } - return sd, repo, nil + if len(globalSections) > 1 { + return false, "", fmt.Errorf("pip config %q defines [global] more than once", pipConfPath) + } + if len(globalSections) == 0 { + return false, "", nil + } + var indexURLKey *ini.Key + for _, key := range globalSections[0].Keys() { + if !strings.EqualFold(key.Name(), "index-url") { + continue + } + if indexURLKey != nil { + return false, "", fmt.Errorf("pip config %q defines index-url more than once in [global]", pipConfPath) + } + indexURLKey = key + } + if indexURLKey == nil { + return false, "", nil + } + if shadows := indexURLKey.ValueWithShadows(); len(shadows) > 1 { + return false, "", fmt.Errorf("pip config %q defines index-url more than once in [global]", pipConfPath) + } + rawURL = strings.Trim(strings.TrimSpace(indexURLKey.Value()), `"'`) + return rawURL != "", rawURL, nil } -// DefaultPipConfPath returns the path for pip's per-user configuration file. -// It respects the PIP_CONFIG_FILE environment variable (which pip itself -// honours), making it overridable in tests and CI environments. -// Falls back to ~/.pip/pip.conf on Unix/macOS and %APPDATA%\pip\pip.ini on Windows. -func DefaultPipConfPath() string { - if configFile := os.Getenv("PIP_CONFIG_FILE"); configFile != "" { - return configFile +// DefaultPipConfPaths returns pip's user-level config candidates: legacy, +// then modern (overrides legacy), then PIP_CONFIG_FILE if set (overrides +// both). An existing PIP_CONFIG_FILE skips legacy/modern, matching pip. +func DefaultPipConfPaths() []string { + envConfigFile := os.Getenv("PIP_CONFIG_FILE") + if envConfigFile != "" { + if _, err := os.Stat(envConfigFile); err == nil { // #nosec G703 -- env-provided path, mirrors pip's own PIP_CONFIG_FILE handling + return []string{envConfigFile} + } } + home, _ := os.UserHomeDir() + basename := "pip.conf" + legacyDir := filepath.Join(home, ".pip") + if runtime.GOOS == "windows" { + basename = "pip.ini" + legacyDir = filepath.Join(home, "pip") + } + paths := []string{filepath.Join(legacyDir, basename), filepath.Join(modernPipConfDir(home), basename)} + if envConfigFile != "" { + paths = append(paths, envConfigFile) + } + return paths +} + +// modernPipConfDir mirrors pip's platformdirs: on macOS, Application Support +// only if it already exists, else XDG/APPDATA config dir. +func modernPipConfDir(home string) string { if runtime.GOOS == "windows" { appData := os.Getenv("APPDATA") if appData == "" { appData = filepath.Join(os.Getenv("USERPROFILE"), "AppData", "Roaming") } - return filepath.Join(appData, "pip", "pip.ini") + return filepath.Join(appData, "pip") } - home, _ := os.UserHomeDir() - return filepath.Join(home, ".pip", "pip.conf") + if runtime.GOOS == "darwin" { + appSupportDir := filepath.Join(home, "Library", "Application Support", "pip") + if info, err := os.Stat(appSupportDir); err == nil && info.IsDir() { // #nosec G703 -- fixed home-relative path, not attacker-controlled + return appSupportDir + } + } + configHome := os.Getenv("XDG_CONFIG_HOME") + if configHome == "" { + configHome = filepath.Join(home, ".config") + } + return filepath.Join(configHome, "pip") +} + +// pipfileSourceHeaderRegex matches the start of a [[source]] array-of-tables entry. +var pipfileSourceHeaderRegex = regexp.MustCompile(`(?m)^\s*\[\[\s*source\s*\]\]\s*(?:#.*)?$`) + +// pipfileTableHeaderRegex matches the start of any TOML table / array-of-tables +// header, used to find where a [[source]] block ends. +var pipfileTableHeaderRegex = regexp.MustCompile(`(?m)^\[`) + +// pipfileUrlValueRegex locates a `url = ...` assignment and captures the quoted +// value in group 1 (double-quoted) or group 2 (single-quoted), so callers can +// replace only that value's exact byte span — leaving whitespace, other keys, +// and comments untouched. +var pipfileUrlValueRegex = regexp.MustCompile(`(?m)^\s*url\s*=\s*(?:"([^"]*)"|'([^']*)')`) + +type pipfileURLSpan struct { + start int + end int +} + +func findPipfileSourceURLSpans(content string) []pipfileURLSpan { + var spans []pipfileURLSpan + for _, h := range pipfileSourceHeaderRegex.FindAllStringIndex(content, -1) { + blockStart := h[1] + blockEnd := len(content) + if next := pipfileTableHeaderRegex.FindStringIndex(content[blockStart:]); next != nil { + blockEnd = blockStart + next[0] + } + block := content[blockStart:blockEnd] + m := pipfileUrlValueRegex.FindStringSubmatchIndex(block) + if m == nil { + spans = append(spans, pipfileURLSpan{start: -1, end: -1}) + continue + } + localStart, localEnd := m[2], m[3] + if localStart == -1 { + localStart, localEnd = m[4], m[5] + } + if localStart == -1 { + spans = append(spans, pipfileURLSpan{start: -1, end: -1}) + continue + } + spans = append(spans, pipfileURLSpan{start: blockStart + localStart, end: blockStart + localEnd}) + } + return spans } -// rewriteCurationSourceInPipfile replaces the url of the [[source]] entry for -// repoName with curationUrl (the audit pass-through URL). Mirrors -// setCurationSourceInPyproject for Poetry. Returns found=false when no matching -// [[source]] exists (e.g. repo came from pipenv.yaml / pip.conf). func rewriteCurationSourceInPipfile(pipfilePath, repoName, curationUrl string) (found bool, err error) { - data, err := os.ReadFile(pipfilePath) + data, err := os.ReadFile(pipfilePath) // #nosec G304 -- temp-dir copy of the project's Pipfile, not attacker-controlled if err != nil { if os.IsNotExist(err) { return false, nil } return false, errorutils.CheckError(err) } - // Match the [[source]] url whether or not it ends with /simple or /simple/. - pattern := regexp.MustCompile(`(?m)^(\s*url\s*=\s*")[^"]*/api/pypi/` + regexp.QuoteMeta(repoName) + `[^"]*(")`) - if !pattern.MatchString(string(data)) { + cfg, records, effectiveNames, err := readPipfileConfig(pipfilePath) + if err != nil { + return false, err + } + content := string(data) + if len(cfg.Sources) == 0 { + source := fmt.Sprintf("[[source]]\nname = \"jfrog-curation\"\nurl = %q\nverify_ssl = true\n\n", curationUrl) + updated := source + content + if err = writePrivatePipfile(pipfilePath, []byte(updated)); err != nil { + return false, errorutils.CheckError(err) + } + return true, nil + } + spans := findPipfileSourceURLSpans(content) + if len(spans) != len(records) { + return false, errors.New("pipenv: could not map parsed [[source]] entries back to Pipfile") + } + effective := make(map[string]struct{}, len(effectiveNames)) + for _, name := range effectiveNames { + effective[name] = struct{}{} + } + var selected []pipfileURLSpan + for _, record := range records { + if _, isEffective := effective[record.name]; !isEffective { + continue + } + if record.repository != "" && record.repository != repoName { + return false, fmt.Errorf("pipenv: source %q uses repository %q instead of %q", record.name, record.repository, repoName) + } + if record.repository == "" && !record.isPyPI { + return false, fmt.Errorf("pipenv: source %q cannot be redirected safely", record.name) + } + span := spans[record.index] + if span.start < 0 { + return false, fmt.Errorf("pipenv: effective source %q has no quoted URL", record.name) + } + selected = append(selected, span) + } + if len(selected) == 0 { return false, nil } - escapedUrl := strings.ReplaceAll(curationUrl, "$", "$$") - updated := pattern.ReplaceAllString(string(data), "${1}"+escapedUrl+"${2}") - if err = os.WriteFile(pipfilePath, []byte(updated), 0600); err != nil { + updated := content + for i := len(selected) - 1; i >= 0; i-- { + span := selected[i] + updated = updated[:span.start] + curationUrl + updated[span.end:] + } + if err = writePrivatePipfile(pipfilePath, []byte(updated)); err != nil { return false, errorutils.CheckError(err) } log.Info(fmt.Sprintf("Configured Pipfile [[source]] url for repository %q to use the curation pass-through endpoint", repoName)) return true, nil } +func writePrivatePipfile(pipfilePath string, content []byte) error { + if err := os.WriteFile(pipfilePath, content, 0600); err != nil { // #nosec G703 -- temp-dir Pipfile copy + return err + } + return os.Chmod(pipfilePath, 0600) +} + func installPipenvDeps(params technologies.BuildInfoBomGeneratorParams) (rootDetected bool, restoreEnv func() error, err error) { // Set virtualenv path to venv dir + previousWorkonHome, hadWorkonHome := os.LookupEnv("WORKON_HOME") err = os.Setenv("WORKON_HOME", ".jfrog") if err != nil { return } restoreEnv = func() error { + if hadWorkonHome { + return os.Setenv("WORKON_HOME", previousWorkonHome) + } return os.Unsetenv("WORKON_HOME") } + if params.IsCurationCmd { + restorePipenvEnv, protectErr := protectPipenvCurationEnvironment(pipfileFile) + if protectErr != nil { + return false, restoreEnv, protectErr + } + restoreWorkonHome := restoreEnv + restoreEnv = func() error { + return errors.Join(restorePipenvEnv(), restoreWorkonHome()) + } + } server := params.ServerDetails repo := params.DependenciesRepository if repo == "" && params.IsCurationCmd { - // 2. ~/.pip/pip.conf - pipConfPath := DefaultPipConfPath() - var pipConfServer *config.ServerDetails - pipConfServer, repo, err = ParsePipConfigIndexUrl(pipConfPath) - if err != nil { - log.Debug(fmt.Sprintf("pipenv native: failed to read pip.conf: %v", err)) - err = nil - } - if pipConfServer != nil { - server = pipConfServer + pipConfServer, pipConfRepo, _, pipConfErr := ParsePipConfigIndexUrl(DefaultPipConfPaths()...) + if pipConfErr != nil { + return false, restoreEnv, pipConfErr } - // 3. Pipfile [[source]] - if repo == "" { - var nativeServer *config.ServerDetails - nativeServer, repo, err = ParsePipfileArtifactorySource(pipfileFile) - if err != nil { - log.Debug(fmt.Sprintf("pipenv native: failed to read Pipfile: %v", err)) - err = nil - } - if nativeServer != nil { - server = nativeServer - } + server, repo, err = ResolvePipfileArtifactorySource(pipfileFile, pipConfServer, pipConfRepo, params.ServerDetails) + if err != nil { + return false, restoreEnv, err } } if repo != "" { @@ -1174,13 +1923,52 @@ func installPipenvDeps(params technologies.BuildInfoBomGeneratorParams) (rootDet if params.IsCurationCmd { return false, restoreEnv, errorutils.CheckErrorf( "curation-audit for pipenv requires an Artifactory PyPI resolver. " + - "Either run 'jf pipenv-config', configure index-url in ~/.pip/pip.conf via " + + "Either run 'jf pipenv-config', configure index-url in your user pip.conf via " + "Artifactory 'Set me up', or add an Artifactory [[source]] entry to your Pipfile.") } _, err = executeCommand("pipenv", "install", "-d") return false, restoreEnv, err } +type environmentValue struct { + name string + value string + present bool +} + +func protectPipenvCurationEnvironment(pipfilePath string) (restore func() error, err error) { + absolutePipfile, err := filepath.Abs(pipfilePath) + if err != nil { + return nil, fmt.Errorf("pipenv: failed to resolve protected Pipfile path: %w", err) + } + names := []string{"PIPENV_PIPFILE", "PIPENV_SKIP_LOCK", "PIPENV_IGNORE_PIPFILE"} + previous := make([]environmentValue, 0, len(names)) + for _, name := range names { + value, present := os.LookupEnv(name) + previous = append(previous, environmentValue{name: name, value: value, present: present}) + } + restore = func() error { + var restoreErr error + for _, item := range previous { + if item.present { + restoreErr = errors.Join(restoreErr, os.Setenv(item.name, item.value)) + } else { + restoreErr = errors.Join(restoreErr, os.Unsetenv(item.name)) + } + } + return restoreErr + } + if err = os.Setenv("PIPENV_PIPFILE", absolutePipfile); err != nil { + return nil, errors.Join(err, restore()) + } + for _, name := range names[1:] { + if err = os.Unsetenv(name); err != nil { + return nil, errors.Join(err, restore()) + } + } + return restore, nil +} + func installPipDeps(params technologies.BuildInfoBomGeneratorParams) (setupFileUsed bool, restoreEnv func() error, err error) { restoreEnv, err = SetPipVirtualEnvPath() if err != nil { @@ -1331,35 +2119,39 @@ func parseCustomArgs(remoteUrl, cacheFolder, reportFileName string, customArgs . } func runPipenvInstallFromRemoteRegistry(server *config.ServerDetails, depsRepoName string, isCurationCmd bool) (err error) { - if isCurationCmd { - if err = validateMinimumPipenvVersion(CurationPipenvMinimumVersion); err != nil { + if !isCurationCmd { + rtURL, err := artifactoryutils.GetPypiRepoUrl(server, depsRepoName, false) + if err != nil { return err } + _, err = executeCommand("pipenv", "install", "-d", + artifactoryutils.GetPypiRemoteRegistryFlag(pythonutils.Pipenv), rtURL) + return err + } + if err = validateMinimumPipenvVersion(); err != nil { + return err } - rtUrl, err := artifactoryutils.GetPypiRepoUrl(server, depsRepoName, isCurationCmd) + rtURL, username, password, err := artifactoryutils.GetPypiRepoUrlWithCredentials(server, depsRepoName, true) if err != nil { return err } - // Rewrite [[source]] url so pipenv's resolver hits the curation pass-through - // endpoint. --pypi-mirror is a no-op for explicit Artifactory sources - // (pipenv only applies it to the default pypi.org index). - if isCurationCmd { - found, rewriteErr := rewriteCurationSourceInPipfile(pipfileFile, depsRepoName, rtUrl) - if rewriteErr != nil { - return rewriteErr - } - if !found { - if exists, _ := fileutils.IsFileExists(pipfileFile, false); exists { - log.Warn(fmt.Sprintf( - "Pipfile exists but no [[source]] url matching repository %q was found — "+ - "pipenv will resolve against the original source URL", depsRepoName)) - } - } + safeURL := rtURL.String() + if password != "" { + rtURL.User = url.UserPassword(username, password) } - args := []string{"install", "-d", artifactoryutils.GetPypiRemoteRegistryFlag(pythonutils.Pipenv), rtUrl} - output, installErr := executeCommand("pipenv", args...) - if installErr != nil && isCurationCmd { + credentialURL := rtURL.String() + found, rewriteErr := rewriteCurationSourceInPipfile(pipfileFile, depsRepoName, credentialURL) + if rewriteErr != nil { + return rewriteErr + } + if !found { + return errorutils.CheckErrorf("pipenv: no effective Pipfile source could be configured for repository %q", depsRepoName) + } + + args := []string{"install", "-d"} + output, installErr := executeCommandSanitized("pipenv", args, safeURL, credentialURL, password, server.GetAccessToken()) + if installErr != nil { combined := output + " " + installErr.Error() if isCvsVersionFilteredOutput(combined) { return &CvsBlockedError{Packages: parseCvsFailedPackages(combined), Cause: installErr} @@ -1373,6 +2165,66 @@ func runPipenvInstallFromRemoteRegistry(server *config.ServerDetails, depsRepoNa return installErr } +func executeCommandSanitized(executable string, args []string, safeURL string, secrets ...string) (string, error) { + installCmd := exec.Command(executable, args...) // #nosec G204 -- internal executable and arguments + environment, err := pipenvCurationEnvironment(pipfileFile) + if err != nil { + return "", err + } + installCmd.Env = environment + maskedCmdString := coreutils.GetMaskedCommandString(installCmd) + log.Debug("Running", maskedCmdString) + output, err := installCmd.CombinedOutput() + sanitized := string(output) + if safeURL != "" { + for _, secretURL := range secrets { + if strings.Contains(secretURL, "://") { + sanitized = strings.ReplaceAll(sanitized, secretURL, safeURL) + } + } + } + sanitized = utils.MaskSensitiveData("url", sanitized) + for _, secret := range secrets { + if secret == "" || strings.Contains(secret, "://") { + continue + } + sanitized = strings.ReplaceAll(sanitized, secret, "****") + sanitized = strings.ReplaceAll(sanitized, url.PathEscape(secret), "****") + sanitized = strings.ReplaceAll(sanitized, url.QueryEscape(secret), "****") + } + if err != nil { + technologies.LogExecutableVersion(executable) + return sanitized, errorutils.CheckErrorf("%q command failed: %s - %s", maskedCmdString, err.Error(), sanitized) + } + return sanitized, nil +} + +func pipenvCurationEnvironment(pipfilePath string) ([]string, error) { + absolutePipfile, err := filepath.Abs(pipfilePath) + if err != nil { + return nil, fmt.Errorf("pipenv: failed to resolve protected Pipfile path: %w", err) + } + blocked := []string{"PIPENV_PIPFILE", "PIPENV_SKIP_LOCK", "PIPENV_IGNORE_PIPFILE"} + environment := make([]string, 0, len(os.Environ())+1) + for _, entry := range os.Environ() { + name, _, ok := strings.Cut(entry, "=") + if !ok { + continue + } + remove := false + for _, blockedName := range blocked { + if strings.EqualFold(name, blockedName) { + remove = true + break + } + } + if !remove { + environment = append(environment, entry) + } + } + return append(environment, "PIPENV_PIPFILE="+absolutePipfile), nil +} + // Execute virtualenv command: "virtualenv venvdir" / "python3 -m venv venvdir" and set path func SetPipVirtualEnvPath() (restoreEnv func() error, err error) { restoreEnv = func() error { diff --git a/sca/bom/buildinfo/technologies/python/python_test.go b/sca/bom/buildinfo/technologies/python/python_test.go index a52aa4f84..a02f0b357 100644 --- a/sca/bom/buildinfo/technologies/python/python_test.go +++ b/sca/bom/buildinfo/technologies/python/python_test.go @@ -2,14 +2,17 @@ package python import ( "errors" + "fmt" "net/http" "os" "path/filepath" "runtime" "strings" + "sync/atomic" "testing" "github.com/jfrog/build-info-go/utils/pythonutils" + rtUtils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" coreCommonTests "github.com/jfrog/jfrog-cli-core/v2/common/tests" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-security/sca/bom/buildinfo/technologies" @@ -162,6 +165,15 @@ func TestBuildPipenvDependencyList(t *testing.T) { } } +func TestShouldRunPythonInstallForPipenv(t *testing.T) { + assert.False(t, shouldRunPythonInstall(technologies.BuildInfoBomGeneratorParams{SkipAutoInstall: true}, techutils.Pipenv)) + assert.True(t, shouldRunPythonInstall(technologies.BuildInfoBomGeneratorParams{ + SkipAutoInstall: true, + IsCurationCmd: true, + }, techutils.Pipenv)) + assert.True(t, shouldRunPythonInstall(technologies.BuildInfoBomGeneratorParams{}, techutils.Pipenv)) +} + func TestBuildPoetryDependencyList(t *testing.T) { // Create and change directory to test workspace _, cleanUp := technologies.CreateTestWorkspace(t, filepath.Join("projects", "package-managers", "python", "poetry", "my-poetry-project")) @@ -1029,7 +1041,7 @@ func TestValidateMinimumPipenvVersionOk(t *testing.T) { "#!/bin/sh\necho 'pipenv, version 2025.0.0'\n", "@echo off\necho pipenv, version 2025.0.0\n", ) - require.NoError(t, validateMinimumPipenvVersion(CurationPipenvMinimumVersion)) + require.NoError(t, validateMinimumPipenvVersion()) } func TestValidateMinimumPipenvVersionTooOld(t *testing.T) { @@ -1038,7 +1050,7 @@ func TestValidateMinimumPipenvVersionTooOld(t *testing.T) { "#!/bin/sh\necho 'pipenv, version 2022.1.1'\n", "@echo off\necho pipenv, version 2022.1.1\n", ) - err := validateMinimumPipenvVersion(CurationPipenvMinimumVersion) + err := validateMinimumPipenvVersion() require.Error(t, err) assert.Contains(t, err.Error(), "2023.7.4", "error must mention the minimum required version") assert.Contains(t, err.Error(), "2022.1.1", "error must mention the installed version") @@ -1050,7 +1062,7 @@ func TestValidateMinimumPipenvVersionUnparsable(t *testing.T) { "#!/bin/sh\necho 'unrecognized output'\n", "@echo off\necho unrecognized output\n", ) - err := validateMinimumPipenvVersion(CurationPipenvMinimumVersion) + err := validateMinimumPipenvVersion() require.Error(t, err) assert.Contains(t, err.Error(), "parse") } @@ -1060,13 +1072,23 @@ func TestInstallPipenvDepsCurationNoRepoNoNativeErrors(t *testing.T) { // Artifactory [[source]], installPipenvDeps must return an actionable error. dir := t.TempDir() t.Chdir(dir) - // Prevent real ~/.pip/pip.conf from being picked up. - t.Setenv("PIP_CONFIG_FILE", filepath.Join(dir, "no-such-pip.conf")) + pipConfigPath := filepath.Join(dir, "pip.conf") + require.NoError(t, os.WriteFile(pipConfigPath, nil, 0600)) + t.Setenv("PIP_CONFIG_FILE", pipConfigPath) + require.NoError(t, os.WriteFile(pipfileFile, []byte(`[[source]] +name = "pypi" +url = "https://pypi.org/simple" + +[packages] +requests = "*" +`), 0600)) params := technologies.BuildInfoBomGeneratorParams{ IsCurationCmd: true, DependenciesRepository: "", } - _, _, err := installPipenvDeps(params) + _, restore, err := installPipenvDeps(params) + require.NotNil(t, restore) + t.Cleanup(func() { require.NoError(t, restore()) }) require.Error(t, err) assert.Contains(t, err.Error(), "pipenv-config") assert.Contains(t, err.Error(), "Pipfile") @@ -1079,14 +1101,15 @@ func TestInstallPipenvDepsCurationNativeFromPipfile(t *testing.T) { // pipenv isn't installed in the test env — but it must NOT error with the // "pipenv-config" message; it must get past native detection). fakeDir := t.TempDir() - pipfileContent := `[[source]] -url = "https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple" + testURL := "https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple" // #nosec G101 -- test fixture, not a real credential + pipfileContent := fmt.Sprintf(`[[source]] +url = "%s" verify_ssl = true name = "artifactory" [packages] requests = "==2.31.0" -` +`, testURL) require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "Pipfile"), []byte(pipfileContent), 0644)) t.Chdir(fakeDir) @@ -1094,7 +1117,9 @@ requests = "==2.31.0" IsCurationCmd: true, DependenciesRepository: "", } - _, _, err := installPipenvDeps(params) + _, restore, err := installPipenvDeps(params) + require.NotNil(t, restore) + t.Cleanup(func() { require.NoError(t, restore()) }) // The call will fail because pipenv/virtualenv is not available in the test // environment, but it must NOT fail with a "pipenv-config/Pipfile" config error. if err != nil { @@ -1113,25 +1138,16 @@ func TestParsePipfileArtifactorySource(t *testing.T) { wantPass string }{ { - name: "standard Artifactory pypi source with credentials", - content: `[[source]] -url = "https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple" -verify_ssl = true -name = "artifactory"`, + name: "standard Artifactory pypi source with credentials", + content: "[[source]]\nurl = \"https://myuser:mytoken@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple\"\nverify_ssl = true\nname = \"artifactory\"", // #nosec G101 -- test fixture, not a real credential wantRepo: "my-pip-repo", wantArtURL: "https://myartifactory.jfrog.io/artifactory/", wantUser: "myuser", wantPass: "mytoken", }, { - name: "multiple sources — Artifactory one is picked", - content: `[[source]] -url = "https://pypi.org/simple" -name = "pypi" - -[[source]] -url = "https://admin:s3cret@acme.jfrog.io/artifactory/api/pypi/curation-repo/simple" -name = "jfrog"`, + name: "package index selects Artifactory source", + content: "[[source]]\nurl = \"https://acme.jfrog.io/artifactory/api/pypi/curation-repo/simple\"\nname = \"primary\"\n\n[[source]]\nurl = \"https://admin:s3cret@acme.jfrog.io/artifactory/api/pypi/curation-repo/simple\"\nname = \"jfrog\"\n\n[packages]\nrequests = {version = \"*\", index = \"jfrog\"}", // #nosec G101 -- test fixture, not a real credential wantRepo: "curation-repo", wantArtURL: "https://acme.jfrog.io/artifactory/", wantUser: "admin", @@ -1172,11 +1188,428 @@ name = "pypi"`, } } +func TestEffectivePipfileSourceNames(t *testing.T) { + t.Run("Sources[0] is effective for an index-less package", func(t *testing.T) { + cfg := pipfileConfig{ + Sources: []pipfileSource{ + {Name: "jfrog", URL: "https://acme.jfrog.io/artifactory/api/pypi/repo/simple"}, + {Name: "other", URL: "https://packages.example.com/simple"}, + }, + Packages: map[string]any{"requests": "*"}, + } + names, err := effectivePipfileSourceNames(cfg) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"jfrog"}, names) + }) + + t.Run("Sources[0] is not effective when every package has an explicit index", func(t *testing.T) { + cfg := pipfileConfig{ + Sources: []pipfileSource{ + {Name: "other", URL: "https://packages.example.com/simple"}, + {Name: "jfrog", URL: "https://acme.jfrog.io/artifactory/api/pypi/repo/simple"}, + }, + Packages: map[string]any{ + "requests": map[string]any{"version": "*", "index": "jfrog"}, + }, + } + names, err := effectivePipfileSourceNames(cfg) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"jfrog"}, names) + }) + + t.Run("no sources and no index assignment is valid", func(t *testing.T) { + cfg := pipfileConfig{Packages: map[string]any{"requests": "*"}} + names, err := effectivePipfileSourceNames(cfg) + require.NoError(t, err) + assert.Empty(t, names) + }) + + t.Run("no sources but an index assignment errors", func(t *testing.T) { + cfg := pipfileConfig{ + Packages: map[string]any{"requests": map[string]any{"version": "*", "index": "jfrog"}}, + } + _, err := effectivePipfileSourceNames(cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "no [[source]] entries") + }) + + t.Run("no packages falls back to Sources[0]", func(t *testing.T) { + cfg := pipfileConfig{ + Sources: []pipfileSource{ + {Name: "jfrog", URL: "https://acme.jfrog.io/artifactory/api/pypi/repo/simple"}, + {Name: "other", URL: "https://packages.example.com/simple"}, + }, + } + names, err := effectivePipfileSourceNames(cfg) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"jfrog"}, names) + }) +} + +func TestMergePipenvCredentials(t *testing.T) { + target := &config.ServerDetails{ArtifactoryUrl: "https://acme.jfrog.io/artifactory/"} + endpoint, err := endpointFromServer(target, "repo") + require.NoError(t, err) + + t.Run("no credentials anywhere is not an error", func(t *testing.T) { + merged, err := mergePipenvCredentials(target, endpoint, nil, nil) + require.NoError(t, err) + assert.Empty(t, merged.User) + assert.Empty(t, merged.Password) + assert.Empty(t, merged.AccessToken) + }) + + t.Run("complete source credentials are selected", func(t *testing.T) { + source := &config.ServerDetails{User: "u", Password: "p"} + merged, err := mergePipenvCredentials(target, endpoint, []*config.ServerDetails{source}, nil) + require.NoError(t, err) + assert.Equal(t, "u", merged.User) + assert.Equal(t, "p", merged.Password) + }) + + t.Run("access token wins over password on the selected source", func(t *testing.T) { + source := &config.ServerDetails{User: "u", Password: "p", AccessToken: "tok"} + merged, err := mergePipenvCredentials(target, endpoint, []*config.ServerDetails{source}, nil) + require.NoError(t, err) + assert.Equal(t, "tok", merged.AccessToken) + assert.Empty(t, merged.Password) + }) + + t.Run("partial source credentials error", func(t *testing.T) { + source := &config.ServerDetails{User: "u"} + _, err := mergePipenvCredentials(target, endpoint, []*config.ServerDetails{source}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "selected Pipfile source has incomplete credentials") + }) + + t.Run("conflicting complete source credentials error", func(t *testing.T) { + sourceA := &config.ServerDetails{User: "a", Password: "p"} + sourceB := &config.ServerDetails{User: "b", Password: "p"} + _, err := mergePipenvCredentials(target, endpoint, []*config.ServerDetails{sourceA, sourceB}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "conflicting credentials") + }) + + t.Run("agreeing complete source credentials do not conflict", func(t *testing.T) { + sourceA := &config.ServerDetails{User: "u", Password: "p"} + sourceB := &config.ServerDetails{User: "u", Password: "p"} + merged, err := mergePipenvCredentials(target, endpoint, []*config.ServerDetails{sourceA, sourceB}, nil) + require.NoError(t, err) + assert.Equal(t, "u", merged.User) + }) + + t.Run("configured resolver credentials are used when no source has any", func(t *testing.T) { + withCreds := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl, User: "u", Password: "p"} + merged, err := mergePipenvCredentials(withCreds, endpoint, nil, nil) + require.NoError(t, err) + assert.Equal(t, "u", merged.User) + }) + + t.Run("configured resolver partial credentials error", func(t *testing.T) { + withCreds := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl, User: "u"} + _, err := mergePipenvCredentials(withCreds, endpoint, nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "configured resolver has incomplete credentials") + }) + + t.Run("matching fallback credentials are used as a last resort", func(t *testing.T) { + fallback := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl, User: "u", Password: "p"} + merged, err := mergePipenvCredentials(target, endpoint, nil, fallback) + require.NoError(t, err) + assert.Equal(t, "u", merged.User) + assert.Equal(t, target.ArtifactoryUrl, merged.ArtifactoryUrl) + }) + + t.Run("matching fallback partial credentials error", func(t *testing.T) { + fallback := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl, User: "u"} + _, err := mergePipenvCredentials(target, endpoint, nil, fallback) + require.Error(t, err) + assert.Contains(t, err.Error(), "matching JFrog server has incomplete credentials") + }) + + t.Run("non-matching fallback is ignored even with credentials", func(t *testing.T) { + fallback := &config.ServerDetails{ArtifactoryUrl: "https://other.jfrog.io/artifactory/", User: "u", Password: "p"} + merged, err := mergePipenvCredentials(target, endpoint, nil, fallback) + require.NoError(t, err) + assert.Empty(t, merged.User) + }) +} + func TestParsePipfileArtifactorySourceNotFound(t *testing.T) { _, _, err := ParsePipfileArtifactorySource("/nonexistent/path/Pipfile") require.Error(t, err) } +func TestResolvePipfileArtifactorySource(t *testing.T) { + writePipfile := func(t *testing.T, content string) string { + path := filepath.Join(t.TempDir(), "Pipfile") + require.NoError(t, os.WriteFile(path, []byte(content), 0600)) + return path + } + target := &config.ServerDetails{ + ArtifactoryUrl: "https://acme.jfrog.io/artifactory/", + XrayUrl: "https://acme.jfrog.io/xray/", + User: "configured-user", + Password: "configured-token", // #nosec G101 -- test fixture + } + + t.Run("source aliases for the same endpoint are allowed", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[[source]] +name = "alias" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[packages] +requests = "*" +`) + server, repo, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.NoError(t, err) + assert.Equal(t, "repo", repo) + assert.Equal(t, "configured-token", server.Password) + }) + + t.Run("unused secondary endpoint does not affect resolution", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[[source]] +name = "other" +url = "https://bad-user-only@packages.example.com/simple" + +[packages] +requests = "*" +`) + server, repo, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.NoError(t, err) + assert.Equal(t, "repo", repo) + assert.Equal(t, "configured-token", server.Password) + }) + + t.Run("unused non-Artifactory Sources[0] does not affect resolution", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "other" +url = "https://bad-user-only@packages.example.com/simple" + +[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[packages] +requests = {version = "*", index = "jfrog"} +`) + server, repo, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.NoError(t, err) + assert.Equal(t, "repo", repo) + assert.Equal(t, "configured-token", server.Password) + }) + + t.Run("unused Artifactory source is not selected natively", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "pypi" +url = "https://pypi.org/simple" + +[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[packages] +requests = "*" +`) + server, repo, err := ResolvePipfileArtifactorySource(path, nil, "", nil) + require.NoError(t, err) + assert.Nil(t, server) + assert.Empty(t, repo) + }) + + t.Run("assigned secondary endpoint is rejected", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[[source]] +name = "other" +url = "https://packages.example.com/simple" + +[packages] +requests = {version = "*", index = "other"} +`) + _, _, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), `source "other"`) + }) + + t.Run("search all sources rejects another endpoint", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[[source]] +name = "other" +url = "https://packages.example.com/simple" + +[packages] +requests = "*" + +[pipenv] +install_search_all_sources = true +`) + _, _, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), `source "other"`) + }) + + t.Run("duplicate source names are rejected", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" + +[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/other/simple" +`) + _, _, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate source name") + }) + + t.Run("complete source credentials win", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://source-user:source-token@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + server, _, err := ResolvePipfileArtifactorySource(path, target, "repo", nil) + require.NoError(t, err) + assert.Equal(t, "source-user", server.User) + assert.Equal(t, "source-token", server.Password) + }) + + t.Run("matching fallback supplies credentials", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + credentialless := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl} + server, _, err := ResolvePipfileArtifactorySource(path, credentialless, "repo", target) + require.NoError(t, err) + assert.Equal(t, "configured-user", server.User) + assert.Equal(t, "configured-token", server.Password) + assert.Equal(t, target.XrayUrl, server.XrayUrl) + }) + + t.Run("access token takes precedence over configured password", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + withToken := *target + withToken.AccessToken = "access-token" // #nosec G101 -- test fixture + server, _, err := ResolvePipfileArtifactorySource(path, &withToken, "repo", nil) + require.NoError(t, err) + assert.Equal(t, "access-token", server.AccessToken) + assert.Empty(t, server.Password) + }) + + t.Run("different endpoint fallback credentials are ignored", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + credentialless := &config.ServerDetails{ArtifactoryUrl: target.ArtifactoryUrl} + other := &config.ServerDetails{ + ArtifactoryUrl: "https://other.jfrog.io/artifactory/", + User: "wrong", + Password: "wrong-token", // #nosec G101 -- test fixture + } + server, _, err := ResolvePipfileArtifactorySource(path, credentialless, "repo", other) + require.NoError(t, err) + assert.Empty(t, server.User) + assert.Empty(t, server.Password) + }) + + t.Run("unsafe repository name is rejected", func(t *testing.T) { + path := writePipfile(t, "[packages]\nrequests = \"*\"\n") + _, _, err := ResolvePipfileArtifactorySource(path, target, "..", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid configured resolver") + }) + + t.Run("unset credential variable is an error", func(t *testing.T) { + t.Setenv("PIPENV_TEST_USER", "user") + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://$PIPENV_TEST_USER:$PIPENV_MISSING_TOKEN@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + _, _, err := ResolvePipfileArtifactorySource(path, nil, "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "PIPENV_MISSING_TOKEN") + assert.NotContains(t, err.Error(), "https://") + }) + + t.Run("encoded credentials remain decoded once", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://user:p%40ss%2Fword@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + server, _, err := ResolvePipfileArtifactorySource(path, nil, "", nil) + require.NoError(t, err) + assert.Equal(t, "p@ss/word", server.Password) + }) + + t.Run("expanded credential may contain dollar sign", func(t *testing.T) { + t.Setenv("PIPENV_DOLLAR_TOKEN", "tok$en") + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://user:$PIPENV_DOLLAR_TOKEN@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + server, _, err := ResolvePipfileArtifactorySource(path, nil, "", nil) + require.NoError(t, err) + assert.Equal(t, "tok$en", server.Password) + }) + + t.Run("dotenv credentials are expanded", func(t *testing.T) { + path := writePipfile(t, `[[source]] +name = "jfrog" +url = "https://$PIPENV_DOTENV_USER:${PIPENV_DOTENV_TOKEN}@acme.jfrog.io/artifactory/api/pypi/repo/simple" +`) + require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(path), ".env"), + []byte("PIPENV_DOTENV_USER=dotenv-user\nPIPENV_DOTENV_TOKEN=dotenv-token\n"), 0600)) // #nosec G101 -- test fixture + server, _, err := ResolvePipfileArtifactorySource(path, nil, "", nil) + require.NoError(t, err) + assert.Equal(t, "dotenv-user", server.User) + assert.Equal(t, "dotenv-token", server.Password) + }) + + t.Run("environment expansion follows platform syntax", func(t *testing.T) { + environment := map[string]string{"PIPENV_WINDOWS_USER": "windows-user"} + expanded, err := expandPipfileEnvVarsForOS( + "https://%pipenv_windows_user%@acme.example.com/$PIPENV_WINDOWS_USER", "jfrog", environment, "windows") + require.NoError(t, err) + assert.Equal(t, "https://windows-user@acme.example.com/windows-user", expanded) + + expanded, err = expandPipfileEnvVarsForOS( + "https://%PIPENV_WINDOWS_USER%@acme.example.com/$PIPENV_WINDOWS_USER", "jfrog", environment, "linux") + require.NoError(t, err) + assert.Equal(t, "https://%PIPENV_WINDOWS_USER%@acme.example.com/windows-user", expanded) + }) + + t.Run("Windows %% collapses to a literal percent, like ntpath.expandvars", func(t *testing.T) { + expanded, err := expandPipfileEnvVarsForOS("https://acme.example.com/repo%%20name", "jfrog", nil, "windows") + require.NoError(t, err) + assert.Equal(t, "https://acme.example.com/repo%20name", expanded) + }) + + t.Run("adjacent Windows vars are parsed as two separate references", func(t *testing.T) { + environment := map[string]string{"PIPENV_A": "aaa", "PIPENV_B": "bbb"} + expanded, err := expandPipfileEnvVarsForOS("%PIPENV_A%%PIPENV_B%", "jfrog", environment, "windows") + require.NoError(t, err) + assert.Equal(t, "aaabbb", expanded) + }) +} + func TestParsePipConfigIndexUrl(t *testing.T) { cases := []struct { name string @@ -1216,6 +1649,15 @@ func TestParsePipConfigIndexUrl(t *testing.T) { wantRepo: "libs-pypi", wantArtURL: "https://myrt.example.com/artifactory/", }, + { + name: "only global index-url is used regardless of section order", + content: "[install]\n" + + "index-url = https://wrong.example.com/artifactory/api/pypi/wrong/simple\n" + + "[global]\n" + + "index-url = https://right.example.com/artifactory/api/pypi/right/simple\n", + wantRepo: "right", + wantArtURL: "https://right.example.com/artifactory/", + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -1225,7 +1667,7 @@ func TestParsePipConfigIndexUrl(t *testing.T) { require.NoError(t, err) require.NoError(t, f.Close()) - sd, repo, err := ParsePipConfigIndexUrl(f.Name()) + sd, repo, _, err := ParsePipConfigIndexUrl(f.Name()) require.NoError(t, err) if tc.wantNoMatch { assert.Empty(t, repo) @@ -1244,12 +1686,170 @@ func TestParsePipConfigIndexUrl(t *testing.T) { func TestParsePipConfigIndexUrlMissingFile(t *testing.T) { // A missing pip.conf is not an error — just returns nil/empty (graceful no-op). - sd, repo, err := ParsePipConfigIndexUrl("/nonexistent/path/pip.conf") + t.Setenv("PIP_CONFIG_FILE", "") + sd, repo, _, err := ParsePipConfigIndexUrl("/nonexistent/path/pip.conf") require.NoError(t, err) assert.Empty(t, repo) assert.Nil(t, sd) } +func TestParsePipConfigIndexUrlExplicitFailures(t *testing.T) { + t.Run("missing explicit file is silently skipped, like pip's own PIP_CONFIG_FILE handling", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing-pip.conf") + t.Setenv("PIP_CONFIG_FILE", path) + sd, repo, _, err := ParsePipConfigIndexUrl(path) + require.NoError(t, err) + assert.Empty(t, repo) + assert.Nil(t, sd) + }) + + t.Run("malformed explicit Artifactory URL", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "pip.conf") + require.NoError(t, os.WriteFile(path, []byte("[global]\nindex-url = https://rt.example.com/artifactory/api/pypi/repo\n"), 0600)) + t.Setenv("PIP_CONFIG_FILE", path) + _, _, _, err := ParsePipConfigIndexUrl(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "valid Artifactory PyPI URL") + assert.Contains(t, err.Error(), path) + }) + + t.Run("malformed INI", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "pip.conf") + require.NoError(t, os.WriteFile(path, []byte("[global\nindex-url = value\n"), 0600)) + t.Setenv("PIP_CONFIG_FILE", path) + _, _, _, err := ParsePipConfigIndexUrl(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse") + }) + + t.Run("duplicate index-url within one [global] section is rejected, not silently collapsed", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "pip.conf") + require.NoError(t, os.WriteFile(path, []byte( + "[global]\nindex-url = https://a.example.com/artifactory/api/pypi/a/simple\nindex-url = https://b.example.com/artifactory/api/pypi/b/simple\n"), 0600)) + _, _, _, err := ParsePipConfigIndexUrl(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "index-url more than once") + }) + + t.Run("case-colliding [global] sections are rejected, not silently collapsed", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "pip.conf") + require.NoError(t, os.WriteFile(path, []byte( + "[GLOBAL]\nindex-url = https://a.example.com/artifactory/api/pypi/a/simple\n[global]\nindex-url = https://b.example.com/artifactory/api/pypi/b/simple\n"), 0600)) + _, _, _, err := ParsePipConfigIndexUrl(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "[global] more than once") + }) +} + +func TestDefaultPipConfPaths(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix pip config paths") + } + t.Run("legacy then modern, legacy first for lowest priority", func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("PIP_CONFIG_FILE", "") + legacyPath := filepath.Join(home, ".pip", "pip.conf") + modernPath := filepath.Join(home, ".config", "pip", "pip.conf") + assert.Equal(t, []string{legacyPath, modernPath}, DefaultPipConfPaths()) + }) + if runtime.GOOS == "darwin" { + t.Run("macOS prefers Application Support only if it already exists", func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PIP_CONFIG_FILE", "") + legacyPath := filepath.Join(home, ".pip", "pip.conf") + xdgModernPath := filepath.Join(home, ".config", "pip", "pip.conf") + assert.Equal(t, []string{legacyPath, xdgModernPath}, DefaultPipConfPaths(), + "falls back to ~/.config when Application Support/pip doesn't exist") + + appSupportDir := filepath.Join(home, "Library", "Application Support", "pip") + require.NoError(t, os.MkdirAll(appSupportDir, 0700)) + appSupportPath := filepath.Join(appSupportDir, "pip.conf") + assert.Equal(t, []string{legacyPath, appSupportPath}, DefaultPipConfPaths(), + "prefers Application Support once the directory exists") + }) + } + t.Run("PIP_CONFIG_FILE overrides both when it exists", func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + envPath := filepath.Join(t.TempDir(), "explicit-pip.conf") + require.NoError(t, os.WriteFile(envPath, nil, 0600)) + t.Setenv("PIP_CONFIG_FILE", envPath) + assert.Equal(t, []string{envPath}, DefaultPipConfPaths(), + "legacy/modern are skipped entirely once PIP_CONFIG_FILE resolves to an existing file") + }) + t.Run("PIP_CONFIG_FILE appended as override when file doesn't exist yet", func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", "") + envPath := filepath.Join(t.TempDir(), "not-yet-created.conf") + t.Setenv("PIP_CONFIG_FILE", envPath) + legacyPath := filepath.Join(home, ".pip", "pip.conf") + modernPath := filepath.Join(home, ".config", "pip", "pip.conf") + assert.Equal(t, []string{legacyPath, modernPath, envPath}, DefaultPipConfPaths()) + }) +} + +func TestParsePipConfigIndexUrlMergesAcrossPaths(t *testing.T) { + dir := t.TempDir() + legacy := filepath.Join(dir, "legacy.conf") + modern := filepath.Join(dir, "modern.conf") + require.NoError(t, os.WriteFile(legacy, []byte( + "[global]\nindex-url = https://legacy.example.com/artifactory/api/pypi/legacy-repo/simple\n"), 0600)) + + t.Run("modern overrides legacy when both define index-url", func(t *testing.T) { + require.NoError(t, os.WriteFile(modern, []byte( + "[global]\nindex-url = https://modern.example.com/artifactory/api/pypi/modern-repo/simple\n"), 0600)) + sd, repo, sourcePath, err := ParsePipConfigIndexUrl(legacy, modern) + require.NoError(t, err) + require.NotNil(t, sd) + assert.Equal(t, "modern-repo", repo) + assert.Equal(t, modern, sourcePath) + }) + + t.Run("legacy value survives when modern file has no index-url", func(t *testing.T) { + require.NoError(t, os.WriteFile(modern, []byte("[global]\ntimeout = 60\n"), 0600)) + sd, repo, sourcePath, err := ParsePipConfigIndexUrl(legacy, modern) + require.NoError(t, err) + require.NotNil(t, sd) + assert.Equal(t, "legacy-repo", repo) + assert.Equal(t, legacy, sourcePath) + }) + + t.Run("missing files are skipped without error", func(t *testing.T) { + sd, repo, sourcePath, err := ParsePipConfigIndexUrl(filepath.Join(dir, "missing.conf"), legacy) + require.NoError(t, err) + require.NotNil(t, sd) + assert.Equal(t, "legacy-repo", repo) + assert.Equal(t, legacy, sourcePath) + }) +} + +func TestRunPipenvInstallNonCurationKeepsExistingInvocation(t *testing.T) { + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\nprintf '%s\\n' \"$@\" > pipenv-args.txt\nprintf '%s' \"$PIPENV_SKIP_LOCK\" > pipenv-skip-lock.txt\n", + "@echo off\necho %* > pipenv-args.txt\necho %PIPENV_SKIP_LOCK% > pipenv-skip-lock.txt\n", + ) + t.Chdir(t.TempDir()) + t.Setenv("PIPENV_SKIP_LOCK", "1") + + server := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} + require.NoError(t, runPipenvInstallFromRemoteRegistry(server, "repo", false)) + + args, err := os.ReadFile("pipenv-args.txt") + require.NoError(t, err) + assert.Contains(t, string(args), "install") + assert.Contains(t, string(args), "-d") + assert.Contains(t, string(args), "--pypi-mirror") + assert.Contains(t, string(args), "/api/pypi/repo/simple") + skipLock, err := os.ReadFile("pipenv-skip-lock.txt") + require.NoError(t, err) + assert.Equal(t, "1", strings.TrimSpace(string(skipLock))) +} + func TestRunPipenvInstallFromRemoteRegistryCurationVersionGate(t *testing.T) { // A pipenv below the minimum version must be rejected before any install attempt. fakeDir := t.TempDir() @@ -1286,6 +1886,7 @@ func TestRunPipenvInstallFromRemoteRegistryCuration403Detection(t *testing.T) { require.NoError(t, err) require.NoError(t, os.Chdir(wd)) defer func() { _ = os.Chdir(origWd) }() + require.NoError(t, os.WriteFile(pipfileFile, []byte("[[source]]\nname = \"pypi\"\nurl = \"https://pypi.org/simple\"\n"), 0600)) sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} err = runPipenvInstallFromRemoteRegistry(sd, "repo", true) @@ -1315,6 +1916,7 @@ func TestRunPipenvInstallFromRemoteRegistryNoLockFileSucceeds(t *testing.T) { require.NoError(t, err) require.NoError(t, os.Chdir(wd)) defer func() { _ = os.Chdir(origWd) }() + require.NoError(t, os.WriteFile(pipfileFile, []byte("[[source]]\nname = \"pypi\"\nurl = \"https://pypi.org/simple\"\n"), 0600)) sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} require.NoError(t, runPipenvInstallFromRemoteRegistry(sd, "repo", true)) @@ -1347,6 +1949,7 @@ func TestRunPipenvInstallFromRemoteRegistryReturnsCvsBlockedError(t *testing.T) require.NoError(t, err) require.NoError(t, os.Chdir(wd)) defer func() { _ = os.Chdir(origWd) }() + require.NoError(t, os.WriteFile(pipfileFile, []byte("[[source]]\nname = \"pypi\"\nurl = \"https://pypi.org/simple\"\n"), 0600)) sd := &config.ServerDetails{ArtifactoryUrl: "https://rt.example.com/artifactory/"} err = runPipenvInstallFromRemoteRegistry(sd, "repo", true) @@ -1359,6 +1962,87 @@ func TestRunPipenvInstallFromRemoteRegistryReturnsCvsBlockedError(t *testing.T) assert.Equal(t, "2.0.7", cvsErr.Packages[0].Version) } +func TestRunPipenvInstallKeepsCredentialsOutOfArgvAndErrors(t *testing.T) { + fakeDir := t.TempDir() + writeFakeExecutable(t, fakeDir, "pipenv", + "#!/bin/sh\n"+ + "if [ \"$1\" = \"--version\" ]; then echo 'pipenv, version 2025.0.0'; exit 0; fi\n"+ + "printf '%s\\n' \"$@\" > pipenv-args.txt\n"+ + "printf 'PIPFILE=%s\\nSKIP=%s\\nIGNORE=%s\\n' \"$PIPENV_PIPFILE\" \"$PIPENV_SKIP_LOCK\" \"$PIPENV_IGNORE_PIPFILE\" > pipenv-env.txt\n"+ + "echo 'failed https://user:super-secret@rt.example.com/artifactory/api/curation/audit/api/pypi/repo/simple'\n"+ + "exit 1\n", + "@echo off\n"+ + "if \"%1\"==\"--version\" (echo pipenv, version 2025.0.0 & exit /b 0)\n"+ + "echo %* > pipenv-args.txt\n"+ + "(echo PIPFILE=%PIPENV_PIPFILE%& echo SKIP=%PIPENV_SKIP_LOCK%& echo IGNORE=%PIPENV_IGNORE_PIPFILE%) > pipenv-env.txt\n"+ + "echo failed https://user:super-secret@rt.example.com/artifactory/api/curation/audit/api/pypi/repo/simple\n"+ + "exit /b 1\n", + ) + t.Setenv("PATH", fakeDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Chdir(t.TempDir()) + require.NoError(t, os.WriteFile(pipfileFile, []byte("[[source]]\nname = \"pypi\"\nurl = \"https://pypi.org/simple\"\n"), 0644)) + t.Setenv("PIPENV_PIPFILE", filepath.Join(t.TempDir(), "untrusted-Pipfile")) + t.Setenv("PIPENV_SKIP_LOCK", "1") + t.Setenv("PIPENV_IGNORE_PIPFILE", "1") + + server := &config.ServerDetails{ + ArtifactoryUrl: "https://rt.example.com/artifactory/", + User: "user", + Password: "super-secret", // #nosec G101 -- test fixture + } + err := runPipenvInstallFromRemoteRegistry(server, "repo", true) + require.Error(t, err) + assert.NotContains(t, err.Error(), "super-secret") + + args, readErr := os.ReadFile("pipenv-args.txt") + require.NoError(t, readErr) + assert.NotContains(t, string(args), "super-secret") + assert.NotContains(t, string(args), "https://") + + environment, readErr := os.ReadFile("pipenv-env.txt") + require.NoError(t, readErr) + absolutePipfile, absErr := filepath.Abs(pipfileFile) + require.NoError(t, absErr) + assert.Contains(t, string(environment), "PIPFILE="+absolutePipfile) + assert.Contains(t, string(environment), "SKIP=") + assert.NotContains(t, string(environment), "SKIP=1") + assert.Contains(t, string(environment), "IGNORE=") + assert.NotContains(t, string(environment), "IGNORE=1") + + content, readErr := os.ReadFile(pipfileFile) + require.NoError(t, readErr) + assert.Contains(t, string(content), "super-secret") + info, statErr := os.Stat(pipfileFile) + require.NoError(t, statErr) + if runtime.GOOS != "windows" { + // Windows/NTFS has no owner-only permission bits to assert on; os.Chmod + // there only toggles the read-only attribute, so Perm() always reports 0666. + assert.Equal(t, os.FileMode(0600), info.Mode().Perm()) + } +} + +func TestProtectPipenvCurationEnvironment(t *testing.T) { + t.Chdir(t.TempDir()) + t.Setenv("PIPENV_PIPFILE", "outside/Pipfile") + t.Setenv("PIPENV_SKIP_LOCK", "1") + t.Setenv("PIPENV_IGNORE_PIPFILE", "1") + + restore, err := protectPipenvCurationEnvironment(pipfileFile) + require.NoError(t, err) + absolutePipfile, err := filepath.Abs(pipfileFile) + require.NoError(t, err) + assert.Equal(t, absolutePipfile, os.Getenv("PIPENV_PIPFILE")) + _, skipLockPresent := os.LookupEnv("PIPENV_SKIP_LOCK") + assert.False(t, skipLockPresent) + _, ignorePipfilePresent := os.LookupEnv("PIPENV_IGNORE_PIPFILE") + assert.False(t, ignorePipfilePresent) + + require.NoError(t, restore()) + assert.Equal(t, "outside/Pipfile", os.Getenv("PIPENV_PIPFILE")) + assert.Equal(t, "1", os.Getenv("PIPENV_SKIP_LOCK")) + assert.Equal(t, "1", os.Getenv("PIPENV_IGNORE_PIPFILE")) +} + func TestParsePipfileLockPackages(t *testing.T) { t.Run("default and develop sections both parsed", func(t *testing.T) { fixture := []byte(`{ @@ -1399,17 +2083,62 @@ func TestParsePipfileLockPackages(t *testing.T) { assert.Equal(t, "7.4.0", byName["pytest"].Version) }) - t.Run("entry with no version is skipped", func(t *testing.T) { + t.Run("hashless unknown entry fails closed", func(t *testing.T) { fixture := []byte(`{ "default": { "somepkg": {"hashes": ["sha256:aaa"]} } }`) + _, err := parsePipfileLockPackages(fixture) + require.Error(t, err) + assert.Contains(t, err.Error(), "somepkg") + assert.Contains(t, err.Error(), "unknown provenance") + }) + + t.Run("hashless registry entry fails closed", func(t *testing.T) { + fixture := []byte(`{"default":{"somepkg":{"version":"==1.2.3","index":"jfrog"}}}`) + _, err := parsePipfileLockPackages(fixture) + require.Error(t, err) + assert.Contains(t, err.Error(), "somepkg") + assert.Contains(t, err.Error(), "no hashes") + }) + + t.Run("local path and VCS entries are skipped", func(t *testing.T) { + fixture := []byte(`{ + "default": { + "localpkg": {"path": ".", "editable": true}, + "gitpkg": {"git": "https://example.com/repo.git", "ref": "abc"} + } + }`) got, err := parsePipfileLockPackages(fixture) require.NoError(t, err) assert.Empty(t, got) }) + t.Run("direct file entry is rejected", func(t *testing.T) { + fixture := []byte(`{"default":{"filepkg":{"file":"https://example.com/file.whl","hashes":["sha256:aaa"]}}}`) + _, err := parsePipfileLockPackages(fixture) + require.Error(t, err) + assert.Contains(t, err.Error(), "filepkg") + assert.Contains(t, err.Error(), "direct-file") + }) + + t.Run("conflicting provenance is rejected", func(t *testing.T) { + fixture := []byte(`{"default":{"badpkg":{"version":"==1.0","path":".","hashes":["sha256:aaa"]}}}`) + _, err := parsePipfileLockPackages(fixture) + require.Error(t, err) + assert.Contains(t, err.Error(), "badpkg") + assert.Contains(t, err.Error(), "conflicting provenance") + }) + + t.Run("unknown provenance field is rejected", func(t *testing.T) { + fixture := []byte(`{"default":{"mystery":{"artifact":"x","version":"==1.0","hashes":["sha256:aaa"]}}}`) + _, err := parsePipfileLockPackages(fixture) + require.Error(t, err) + assert.Contains(t, err.Error(), "mystery") + assert.Contains(t, err.Error(), "unknown lock fields") + }) + t.Run("duplicate name+version across sections is deduplicated", func(t *testing.T) { fixture := []byte(`{ "default": { @@ -1429,10 +2158,10 @@ func TestParsePipfileLockPackages(t *testing.T) { require.Error(t, err) }) - t.Run("empty content returns empty slice", func(t *testing.T) { - got, err := parsePipfileLockPackages([]byte(`{}`)) - require.NoError(t, err) - assert.Empty(t, got) + t.Run("missing dependency sections are rejected", func(t *testing.T) { + _, err := parsePipfileLockPackages([]byte(`{}`)) + require.Error(t, err) + assert.Contains(t, err.Error(), "no default or develop") }) } @@ -1473,7 +2202,7 @@ func TestBuildPipenvDownloadUrl_HTTP(t *testing.T) { } t.Run("200 with matching hash returns absolute URL without fragment", func(t *testing.T) { - server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/simple/urllib3/") { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(` @@ -1484,6 +2213,8 @@ func TestBuildPipenvDownloadUrl_HTTP(t *testing.T) { t.Fatalf("unexpected request to %s", r.URL.Path) }) defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() got, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) @@ -1494,45 +2225,51 @@ func TestBuildPipenvDownloadUrl_HTTP(t *testing.T) { }) t.Run("non-200 from simple-index surfaces status code", func(t *testing.T) { - server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) }) defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() - _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) require.Error(t, err) assert.Contains(t, err.Error(), "404") assert.Contains(t, err.Error(), "simple-index") }) t.Run("200 with no matching hash returns error", func(t *testing.T) { - server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(` urllib3-1.0.0-py3-none-any.whl `)) }) defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() - _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, pkg) require.Error(t, err) assert.Contains(t, err.Error(), "no matching href") }) t.Run("uses normalized name in simple-index URL", func(t *testing.T) { var seenPath string - server, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { seenPath = r.URL.Path w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`Flask_Babel-1.0.tar.gz`)) }) defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() quirky := pipfileLockPackage{Name: "Flask_Babel", Version: "1.0", Hashes: []string{"sha256:xyz"}} - _, err := buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, quirky) + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, repo, quirky) require.NoError(t, err) assert.Contains(t, seenPath, "/simple/flask-babel/", "must use PEP 503 normalized name in the simple-index URL, got %q", seenPath) }) @@ -1558,3 +2295,193 @@ func TestBuildPipenvDownloadUrlsMapInputValidation(t *testing.T) { assert.Contains(t, err.Error(), "repository") }) } + +// TestBuildPipenvDownloadUrlRejectsCrossHostHref verifies that an absolute href +// in the simple-index response pointing at a different host than the configured +// Artifactory endpoint is rejected, rather than being handed back for an +// authenticated HEAD request against an unintended host. +func TestBuildPipenvDownloadUrlRejectsCrossHostHref(t *testing.T) { + pkg := pipfileLockPackage{ + Name: "urllib3", + Version: "2.0.7", + Hashes: []string{"sha256:aaa111"}, + } + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`urllib3-2.0.7.whl`)) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, "my-pip-repo", pkg) + require.Error(t, err) + assert.Contains(t, err.Error(), "escapes the configured Artifactory endpoint") +} + +func TestBuildPipenvDownloadUrlRejectsOutsideRepository(t *testing.T) { + pkg := pipfileLockPackage{Name: "urllib3", Version: "2.0.7", Hashes: []string{"sha256:aaa111"}} + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`private.whl`)) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, "my-pip-repo", pkg) + require.Error(t, err) + assert.Contains(t, err.Error(), "escapes the configured Artifactory endpoint") +} + +func TestBuildPipenvDownloadUrlRejectsUnsafeRedirect(t *testing.T) { + pkg := pipfileLockPackage{Name: "urllib3", Version: "2.0.7", Hashes: []string{"sha256:aaa111"}} + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + http.Redirect(w, r, "/api/system/configuration", http.StatusFound) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + httpDetails := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, err = buildPipenvDownloadUrl(rtManager, &httpDetails, server.URL, "my-pip-repo", pkg) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.Equal(t, int32(1), requests.Load()) +} + +func TestRewriteCurationSourceInPipfile(t *testing.T) { + writeTemp := func(t *testing.T, content string) string { + dir := t.TempDir() + p := filepath.Join(dir, "Pipfile") + require.NoError(t, os.WriteFile(p, []byte(content), 0644)) + return p + } + readBack := func(t *testing.T, path string) string { + data, err := os.ReadFile(path) + require.NoError(t, err) + return string(data) + } + + t.Run("double-quoted source is rewritten and comments/formatting preserved", func(t *testing.T) { + content := "[[source]]\n" + + "# our internal mirror\n" + + "url = \"https://acme.jfrog.io/artifactory/api/pypi/curation-repo/simple\"\n" + + "verify_ssl = true\n" + + "name = \"jfrog\"\n" + p := writeTemp(t, content) + + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.NoError(t, err) + assert.True(t, found) + + updated := readBack(t, p) + assert.Contains(t, updated, `url = "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple"`) + assert.Contains(t, updated, "# our internal mirror", "comments must be preserved") + assert.Contains(t, updated, "verify_ssl = true", "unrelated keys must be preserved") + assert.Contains(t, updated, `name = "jfrog"`, "unrelated keys must be preserved") + }) + + t.Run("single-quoted source is rewritten", func(t *testing.T) { + content := "[[source]]\n" + + "url = 'https://acme.jfrog.io/artifactory/api/pypi/curation-repo/simple'\n" + + "name = 'jfrog'\n" + p := writeTemp(t, content) + + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.NoError(t, err) + assert.True(t, found) + assert.Contains(t, readBack(t, p), "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + }) + + t.Run("only effective source is rewritten", func(t *testing.T) { + content := "[[source]]\n" + + "url = \"https://pypi.org/simple\"\n" + + "name = \"pypi\"\n\n" + + "[[source]]\n" + + "url = \"https://bad-user-only@packages.example.com/simple\"\n" + + "name = \"unused\"\n\n" + + "[packages]\n" + + "requests = \"*\"\n" + p := writeTemp(t, content) + + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.NoError(t, err) + assert.True(t, found) + + updated := readBack(t, p) + assert.Equal(t, 1, strings.Count(updated, `url = "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple"`)) + assert.NotContains(t, updated, "https://pypi.org/simple") + assert.Contains(t, updated, "https://bad-user-only@packages.example.com/simple") + assert.Contains(t, updated, `requests = "*"`, "sections after the source blocks must be preserved") + }) + + t.Run("external effective source is rejected", func(t *testing.T) { + content := "[[source]]\nurl = \"https://packages.example.com/simple\"\nname = \"external\"\n" + p := writeTemp(t, content) + + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.Error(t, err) + assert.False(t, found) + assert.Equal(t, content, readBack(t, p)) + }) + + t.Run("missing source gets a private default", func(t *testing.T) { + p := writeTemp(t, "[packages]\nrequests = \"*\"\n") + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://user:token@acme.jfrog.io/artifactory/api/curation/audit/api/pypi/curation-repo/simple") + require.NoError(t, err) + assert.True(t, found) + assert.Contains(t, readBack(t, p), `name = "jfrog-curation"`) + info, err := os.Stat(p) + require.NoError(t, err) + if runtime.GOOS != "windows" { + // Windows/NTFS has no owner-only permission bits to assert on; os.Chmod + // there only toggles the read-only attribute, so Perm() always reports 0666. + assert.Equal(t, os.FileMode(0600), info.Mode().Perm()) + } + }) + + t.Run("inline source array fails closed", func(t *testing.T) { + content := "source = [{name = \"jfrog\", url = \"https://acme.jfrog.io/artifactory/api/pypi/curation-repo/simple\", verify_ssl = true}]\n\n" + + "[packages]\nrequests = \"*\"\n" + p := writeTemp(t, content) + + found, err := rewriteCurationSourceInPipfile(p, "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.Error(t, err) + assert.False(t, found) + assert.Contains(t, err.Error(), "could not map") + assert.Equal(t, content, readBack(t, p)) + }) + + t.Run("missing file returns found=false, no error", func(t *testing.T) { + found, err := rewriteCurationSourceInPipfile(filepath.Join(t.TempDir(), "Pipfile"), "curation-repo", "https://acme.jfrog.io/artifactory/api/curation/audit/curation-repo/simple") + require.NoError(t, err) + assert.False(t, found) + }) +} + +// TestParsePipfileArtifactorySourceEnvVarExpansion verifies that $VAR/${VAR} +// credentials embedded in a Pipfile [[source]] url — a pattern Pipenv itself +// supports and expands at runtime — are resolved from the environment. +func TestParsePipfileArtifactorySourceEnvVarExpansion(t *testing.T) { + t.Setenv("PIPFILE_TEST_USER", "myuser") + t.Setenv("PIPFILE_TEST_PASS", "mytoken") + + dir := t.TempDir() + pipfilePath := filepath.Join(dir, "Pipfile") + content := "[[source]]\n" + + "url = \"https://$PIPFILE_TEST_USER:${PIPFILE_TEST_PASS}@myartifactory.jfrog.io/artifactory/api/pypi/my-pip-repo/simple\"\n" + + "name = \"jfrog\"\n" + require.NoError(t, os.WriteFile(pipfilePath, []byte(content), 0644)) + + sd, repo, err := ParsePipfileArtifactorySource(pipfilePath) + require.NoError(t, err) + assert.Equal(t, "my-pip-repo", repo) + require.NotNil(t, sd) + assert.Equal(t, "myuser", sd.User) + assert.Equal(t, "mytoken", sd.Password) +} diff --git a/tests/testdata/projects/package-managers/python/pipenv/pipenv-curation-project/Pipfile b/tests/testdata/projects/package-managers/python/pipenv/pipenv-curation-project/Pipfile new file mode 100644 index 000000000..df354c63f --- /dev/null +++ b/tests/testdata/projects/package-managers/python/pipenv/pipenv-curation-project/Pipfile @@ -0,0 +1,10 @@ +[[source]] +name = "pypi" +url = "http://replace-with-mock-server.invalid/api/pypi/pypi-curation/simple" +verify_ssl = true + +[packages] +urllib3 = "==1.26.20" + +[requires] +python_version = "*" diff --git a/utils/http_redirect.go b/utils/http_redirect.go new file mode 100644 index 000000000..1fda71bfa --- /dev/null +++ b/utils/http_redirect.go @@ -0,0 +1,124 @@ +package utils + +import ( + "fmt" + "net/http" + "net/url" + "path" + "strings" + + "github.com/jfrog/jfrog-client-go/http/jfroghttpclient" + "github.com/jfrog/jfrog-client-go/utils/io/httputils" +) + +const MaxAuthenticatedRedirects = 3 + +type EndpointBoundary struct { + scheme string + host string + path string +} + +func NewEndpointBoundary(rawBaseURL string) (EndpointBoundary, error) { + u, err := url.Parse(rawBaseURL) + if err != nil || u.Host == "" || u.User != nil || u.RawPath != "" || u.RawQuery != "" || u.Fragment != "" || + (!strings.EqualFold(u.Scheme, "http") && !strings.EqualFold(u.Scheme, "https")) { + return EndpointBoundary{}, fmt.Errorf("invalid endpoint boundary") + } + cleanPath, valid := normalizedBoundaryPath(u.Path) + if !valid { + return EndpointBoundary{}, fmt.Errorf("invalid endpoint boundary") + } + return EndpointBoundary{ + scheme: strings.ToLower(u.Scheme), + host: normalizedHost(u), + path: cleanPath, + }, nil +} + +func (b EndpointBoundary) Validate(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil || u.Scheme == "" || u.Host == "" || u.User != nil { + return fmt.Errorf("invalid request URL") + } + if hasAmbiguousPathEscape(u.EscapedPath()) { + return fmt.Errorf("ambiguous escaped request path") + } + requestPath, valid := normalizedBoundaryPath(u.Path) + if !valid { + return fmt.Errorf("request URL escapes the configured endpoint") + } + if strings.ToLower(u.Scheme) != b.scheme || normalizedHost(u) != b.host || + !strings.HasPrefix(requestPath, b.path) { + return fmt.Errorf("request URL escapes the configured endpoint") + } + return nil +} + +func normalizedBoundaryPath(rawPath string) (string, bool) { + if rawPath == "" { + rawPath = "/" + } + cleanPath := strings.TrimSuffix(path.Clean(rawPath), "/") + "/" + return cleanPath, cleanPath == strings.TrimSuffix(rawPath, "/")+"/" +} + +func hasAmbiguousPathEscape(escapedPath string) bool { + for { + lowerPath := strings.ToLower(escapedPath) + if strings.Contains(lowerPath, "%2f") || strings.Contains(lowerPath, "%2e") || + strings.Contains(lowerPath, "%5c") { + return true + } + decoded, err := url.PathUnescape(escapedPath) + if err != nil { + return true + } + if decoded == escapedPath { + return false + } + escapedPath = decoded + } +} + +func normalizedHost(u *url.URL) string { + port := u.Port() + if port == "" { + if strings.EqualFold(u.Scheme, "http") { + port = "80" + } else if strings.EqualFold(u.Scheme, "https") { + port = "443" + } + } + return strings.ToLower(u.Hostname()) + ":" + port +} + +func SendWithBoundedRedirects(client *jfroghttpclient.JfrogHttpClient, method, requestURL string, + details *httputils.HttpClientDetails, boundary EndpointBoundary, maxRedirects int, +) (*http.Response, []byte, error) { + // JfrogHttpClient has no per-request redirect hook, so validate each hop before forwarding auth. + if maxRedirects < 0 { + return nil, nil, fmt.Errorf("redirect limit must be non-negative") + } + // HttpClient.Send retries on CheckRedirect errors, which would desync this hop counter. + if retries := client.GetHttpClient().GetRetries(); retries != 0 { + return nil, nil, fmt.Errorf("bounded redirects require a zero-retry client, got %d retries configured", retries) + } + currentURL := requestURL + for redirects := 0; ; redirects++ { + if err := boundary.Validate(currentURL); err != nil { + return nil, nil, err + } + resp, body, redirectURL, err := client.Send(method, currentURL, nil, false, true, details.Clone(), "") + if redirectURL == "" { + return resp, body, err + } + if redirects == maxRedirects { + return resp, body, fmt.Errorf("redirect limit of %d exceeded", maxRedirects) + } + if err := boundary.Validate(redirectURL); err != nil { + return resp, body, fmt.Errorf("unsafe redirect: %w", err) + } + currentURL = redirectURL + } +} diff --git a/utils/http_redirect_test.go b/utils/http_redirect_test.go new file mode 100644 index 000000000..1fabbf9e7 --- /dev/null +++ b/utils/http_redirect_test.go @@ -0,0 +1,160 @@ +package utils + +import ( + "fmt" + "net/http" + "sync/atomic" + "testing" + + rtUtils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" + coreCommonTests "github.com/jfrog/jfrog-cli-core/v2/common/tests" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewEndpointBoundary(t *testing.T) { + t.Run("bare host covers root", func(t *testing.T) { + boundary, err := NewEndpointBoundary("https://example.com") + require.NoError(t, err) + assert.Equal(t, "/", boundary.path) + assert.NoError(t, boundary.Validate("https://example.com")) + assert.NoError(t, boundary.Validate("https://example.com/api/pypi/repo/file.whl")) + }) + + t.Run("trailing slash is normalized", func(t *testing.T) { + withoutSlash, err := NewEndpointBoundary("https://example.com/api/pypi/repo") + require.NoError(t, err) + withSlash, err := NewEndpointBoundary("https://example.com/api/pypi/repo/") + require.NoError(t, err) + assert.Equal(t, withoutSlash, withSlash) + }) + + t.Run("IPv6 and default port are normalized", func(t *testing.T) { + boundary, err := NewEndpointBoundary("https://[2001:db8::1]/api/pypi/repo/") + require.NoError(t, err) + assert.NoError(t, boundary.Validate("https://[2001:db8::1]:443/api/pypi/repo/pkg.whl")) + assert.Error(t, boundary.Validate("https://[2001:db8::2]/api/pypi/repo/pkg.whl")) + }) + + for _, rawURL := range []string{ + "https://user@example.com/api/pypi/repo/", + "https://example.com/api/pypi/repo/?token=value", + "https://example.com/api/pypi/repo/#fragment", + "ftp://example.com/api/pypi/repo/", + } { + t.Run("rejects invalid boundary "+rawURL, func(t *testing.T) { + _, err := NewEndpointBoundary(rawURL) + require.Error(t, err) + }) + } +} + +func TestEndpointBoundaryValidateRejectsEscapes(t *testing.T) { + boundary, err := NewEndpointBoundary("https://example.com/api/pypi/repo/") + require.NoError(t, err) + + for _, rawURL := range []string{ + "https://example.com/api/pypi/repo/%2Fsecret", + "https://example.com/api/pypi/repo/%252Fsecret", + "https://example.com/api/pypi/repo/%25252e%25252e%25252fsecret", + "https://example.com/api/pypi/repo/%255csecret", + } { + t.Run(rawURL, func(t *testing.T) { + err := boundary.Validate(rawURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "ambiguous escaped request path") + }) + } +} + +func TestEndpointBoundaryValidateRejectsOutsideEndpoint(t *testing.T) { + boundary, err := NewEndpointBoundary("https://example.com/api/pypi/repo/") + require.NoError(t, err) + + for _, rawURL := range []string{ + "http://example.com/api/pypi/repo/pkg.whl", + "https://other.example.com/api/pypi/repo/pkg.whl", + "https://example.com/api/pypi/other/pkg.whl", + "https://user@example.com/api/pypi/repo/pkg.whl", + } { + t.Run(rawURL, func(t *testing.T) { + assert.Error(t, boundary.Validate(rawURL)) + }) + } +} + +func TestSendWithBoundedRedirectsLimit(t *testing.T) { + tests := []struct { + name string + redirects int32 + wantErr bool + wantRequests int32 + }{ + {name: "three redirects succeed", redirects: MaxAuthenticatedRedirects, wantRequests: 4}, + {name: "fourth redirect is rejected", redirects: MaxAuthenticatedRedirects + 1, wantErr: true, wantRequests: 4}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requestNumber := requests.Add(1) + if requestNumber <= test.redirects { + http.Redirect(w, r, fmt.Sprintf("/api/pypi/repo/%d", requestNumber), http.StatusFound) + return + } + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte("ok")); err != nil { + t.Errorf("failed writing response: %v", err) + } + }) + defer server.Close() + // Zero retries required; see SendWithBoundedRedirects. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + boundary, err := NewEndpointBoundary(server.URL + "/api/pypi/repo/") + require.NoError(t, err) + details := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + resp, body, err := SendWithBoundedRedirects(rtManager.Client(), http.MethodGet, + server.URL+"/api/pypi/repo/0", &details, boundary, MaxAuthenticatedRedirects) + if test.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "redirect limit") + } else { + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "ok", string(body)) + } + assert.Equal(t, test.wantRequests, requests.Load()) + }) + } +} + +func TestSendWithBoundedRedirectsRejectsLateHopEscape(t *testing.T) { + // The first redirect is legitimate; only the second hop escapes the boundary. + // Every hop must be validated, not just the initial request. + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + switch requests.Add(1) { + case 1: + http.Redirect(w, r, "/api/pypi/repo/inner", http.StatusFound) + case 2: + http.Redirect(w, r, "/api/pypi/other-repo/secret", http.StatusFound) + default: + w.WriteHeader(http.StatusOK) + } + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + boundary, err := NewEndpointBoundary(server.URL + "/api/pypi/repo/") + require.NoError(t, err) + details := rtManager.GetConfig().GetServiceDetails().CreateHttpClientDetails() + + _, _, err = SendWithBoundedRedirects(rtManager.Client(), http.MethodGet, + server.URL+"/api/pypi/repo/start", &details, boundary, MaxAuthenticatedRedirects) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.Equal(t, int32(2), requests.Load(), "must stop at the escaping hop, not follow it") +} From 004bc99978b346df9a4edcaa6076585bfb5bbf95 Mon Sep 17 00:00:00 2001 From: Phavya Jayakumar Date: Mon, 27 Jul 2026 13:50:36 +0530 Subject: [PATCH 3/3] XRAY-156301 - Implement http HEAD GET security for pip and poetry --- commands/curation/curationaudit.go | 34 ++--- commands/curation/curationaudit_test.go | 163 +++++++++++++++++------- 2 files changed, 136 insertions(+), 61 deletions(-) diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 173ed9392..5555b3a59 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -662,9 +662,9 @@ func (ca *CurationAuditCommand) getRtManagerAndAuth(tech techutils.Technology) ( return } -// pipenvBoundedRedirectManager must use zero retries: HttpClient.Send retries +// boundedRedirectManager must use zero retries: HttpClient.Send retries // on CheckRedirect errors, desyncing SendWithBoundedRedirects's hop counter. -func pipenvBoundedRedirectManager(serverDetails *config.ServerDetails) (artifactory.ArtifactoryServicesManager, error) { +func boundedRedirectManager(serverDetails *config.ServerDetails) (artifactory.ArtifactoryServicesManager, error) { return rtUtils.CreateServiceManager(serverDetails, 0, 0, false) } @@ -791,8 +791,8 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if err != nil { return err } - if tech == techutils.Pipenv { - rtManager, err = pipenvBoundedRedirectManager(serverDetails) + if tech == techutils.Pip || tech == techutils.Poetry || tech == techutils.Pipenv { + rtManager, err = boundedRedirectManager(serverDetails) if err != nil { return err } @@ -1483,8 +1483,8 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e requestDetails := nc.httpClientDetails.Clone() var resp *http.Response var err error - if nc.tech == techutils.Pipenv { - resp, _, err = nc.sendPipenvRequest(http.MethodHead, packageUrl, requestDetails) + if nc.tech == techutils.Pip || nc.tech == techutils.Poetry || nc.tech == techutils.Pipenv { + resp, _, err = nc.sendBoundedRequest(http.MethodHead, packageUrl, requestDetails) } else { resp, _, err = nc.rtManager.Client().SendHead(packageUrl, requestDetails) } @@ -1533,7 +1533,7 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e return nil } -func (nc *treeAnalyzer) sendPipenvRequest(method, requestURL string, details *httputils.HttpClientDetails) (*http.Response, []byte, error) { +func (nc *treeAnalyzer) sendBoundedRequest(method, requestURL string, details *httputils.HttpClientDetails) (*http.Response, []byte, error) { repositoryURL := fmt.Sprintf("%s/api/pypi/%s/", strings.TrimSuffix(nc.url, "/"), nc.repo) boundary, err := utils.NewEndpointBoundary(repositoryURL) if err != nil { @@ -1553,8 +1553,8 @@ func (ca *CurationAuditCommand) runCvsFallback(cvsErr *python.CvsBlockedError, t if err != nil { return fmt.Errorf("curation-blocked resolution fallback: failed to get Artifactory manager (%w); %s error: %w", err, tech, cvsErr) } - if tech == techutils.Pipenv { - rtManager, err = pipenvBoundedRedirectManager(serverDetails) + if tech == techutils.Pip || tech == techutils.Poetry || tech == techutils.Pipenv { + rtManager, err = boundedRedirectManager(serverDetails) if err != nil { return fmt.Errorf("curation-blocked resolution fallback: failed to create bounded HTTP manager: %w", err) } @@ -1602,8 +1602,8 @@ func (nc *treeAnalyzer) lookupPypiAllVersions(name string) ([]string, error) { var resp *http.Response var body []byte var err error - if nc.tech == techutils.Pipenv { - resp, body, err = nc.sendPipenvRequest(http.MethodGet, metadataURL, requestDetails) + if nc.tech == techutils.Pip || nc.tech == techutils.Poetry || nc.tech == techutils.Pipenv { + resp, body, err = nc.sendBoundedRequest(http.MethodGet, metadataURL, requestDetails) } else { resp, body, _, err = nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) } @@ -1642,8 +1642,8 @@ func (nc *treeAnalyzer) lookupPypiNormalDownloadURL(name, ver string) (string, e var resp *http.Response var body []byte var err error - if nc.tech == techutils.Pipenv { - resp, body, err = nc.sendPipenvRequest(http.MethodGet, metadataURL, requestDetails) + if nc.tech == techutils.Pip || nc.tech == techutils.Poetry || nc.tech == techutils.Pipenv { + resp, body, err = nc.sendBoundedRequest(http.MethodGet, metadataURL, requestDetails) } else { resp, body, _, err = nc.rtManager.Client().SendGet(metadataURL, true, requestDetails) } @@ -1743,8 +1743,8 @@ func (nc *treeAnalyzer) fetchCvsBlockedStatus(pins []python.PinnedRequirement) [ headDetails := nc.httpClientDetails.Clone() var headResp *http.Response var headErr error - if nc.tech == techutils.Pipenv { - headResp, _, headErr = nc.sendPipenvRequest(http.MethodHead, dlURL, headDetails) + if nc.tech == techutils.Pip || nc.tech == techutils.Poetry || nc.tech == techutils.Pipenv { + headResp, _, headErr = nc.sendBoundedRequest(http.MethodHead, dlURL, headDetails) } else { headResp, _, headErr = nc.rtManager.Client().SendHead(dlURL, headDetails) } @@ -1843,8 +1843,8 @@ func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, var getResp *http.Response var respBody []byte var err error - if nc.tech == techutils.Pipenv { - getResp, respBody, err = nc.sendPipenvRequest(http.MethodGet, packageUrl, requestDetails) + if nc.tech == techutils.Pip || nc.tech == techutils.Poetry || nc.tech == techutils.Pipenv { + getResp, respBody, err = nc.sendBoundedRequest(http.MethodGet, packageUrl, requestDetails) } else { getResp, respBody, _, err = nc.rtManager.Client().SendGet(packageUrl, true, requestDetails) } diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 89ed7bede..5de996306 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -1945,12 +1945,16 @@ func TestGetBlockedPackageDetails_403UnparsableBodyReturnsBlocked(t *testing.T) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusForbidden) _, _ = w.Write([]byte(tt.respBody)) }) defer serverMock.Close() + // Poetry (like Pip/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() httpClientDetails := rtAuth.CreateHttpClientDetails() analyzer := treeAnalyzer{ @@ -2006,7 +2010,7 @@ func TestFetchCvsBlockedStatusTransitive(t *testing.T) { // Version-specific metadata JSON (returns the whl download URL). versionMetaJSON := fmt.Sprintf(`{"urls":[{"packagetype":"bdist_wheel","url":"../../%s"}]}`, whlRelativePath) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { // All-versions metadata: /api/pypi//pypi//json case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/pypi/"+blockedPkg+"/json"): @@ -2033,6 +2037,10 @@ func TestFetchCvsBlockedStatusTransitive(t *testing.T) { }) defer serverMock.Close() + // Pip (like Poetry/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() httpClientDetails := rtAuth.CreateHttpClientDetails() @@ -2102,7 +2110,7 @@ func TestFetchCvsBlockedStatusPoetry(t *testing.T) { blockResponse := fmt.Sprintf(`{"errors":[{"status":403,"message":%q}]}`, blockMsg) versionMetaJSON := fmt.Sprintf(`{"urls":[{"packagetype":"bdist_wheel","url":"../../%s"}]}`, whlRelativePath) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/pypi/"+blockedPkg+"/"+blockedVer+"/json"): w.WriteHeader(http.StatusOK) @@ -2118,6 +2126,10 @@ func TestFetchCvsBlockedStatusPoetry(t *testing.T) { }) defer serverMock.Close() + // Poetry (like Pip/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() httpClientDetails := rtAuth.CreateHttpClientDetails() @@ -2160,7 +2172,7 @@ func TestFetchCvsBlockedStatusNotInMetadataNotRendered(t *testing.T) { ver = "4.87.1000" // not in the metadata API ) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/pypi/"+pkg+"/"+ver+"/json"): w.WriteHeader(http.StatusNotFound) @@ -2170,6 +2182,10 @@ func TestFetchCvsBlockedStatusNotInMetadataNotRendered(t *testing.T) { }) defer serverMock.Close() + // Pip (like Poetry/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() analyzer := treeAnalyzer{ rtManager: rtManager, @@ -2201,7 +2217,7 @@ func TestFetchCvsBlockedStatusSetsDepRelation(t *testing.T) { whlRelativePath = "packages/ab/cd/langchain_core-1.4.7-py3-none-any.whl" ) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/pypi/"+blockedPkg+"/json"): w.WriteHeader(http.StatusOK) @@ -2218,6 +2234,10 @@ func TestFetchCvsBlockedStatusSetsDepRelation(t *testing.T) { }) defer serverMock.Close() + // Pip (like Poetry/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() analyzer := treeAnalyzer{ rtManager: rtManager, @@ -2265,7 +2285,7 @@ func TestFetchCvsBlockedStatusHeadErrorNoFalsePositive(t *testing.T) { whlRelativePath = "packages/ab/cd/foo-1.0-py3-none-any.whl" ) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodHead: w.WriteHeader(http.StatusInternalServerError) @@ -2278,6 +2298,10 @@ func TestFetchCvsBlockedStatusHeadErrorNoFalsePositive(t *testing.T) { }) defer serverMock.Close() + // Pip (like Poetry/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() analyzer := treeAnalyzer{ rtManager: rtManager, @@ -2306,7 +2330,7 @@ func TestFetchCvsBlockedStatusHeadOKNoFalsePositive(t *testing.T) { whlRelativePath = "packages/ab/cd/foo-1.0-py3-none-any.whl" ) - serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodHead: // Stale CVS cache cleared; package is now accessible. @@ -2320,6 +2344,10 @@ func TestFetchCvsBlockedStatusHeadOKNoFalsePositive(t *testing.T) { }) defer serverMock.Close() + // Pip (like Poetry/Pipenv) routes through sendBoundedRequest, which requires a + // zero-retry client — mirrors the production boundedRedirectManager construction. + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() analyzer := treeAnalyzer{ rtManager: rtManager, @@ -2422,41 +2450,45 @@ url = "https://user:token@acme.jfrog.io/artifactory/api/pypi/repo/simple" assert.Nil(t, ca.PackageManagerConfig) } -func TestSendPipenvRequestRejectsRedirectOutsideRepository(t *testing.T) { - var outsideRequested atomic.Bool - var requests atomic.Int32 - server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { - requests.Add(1) - if r.URL.Path == "/api/system/configuration" { - outsideRequested.Store(true) - w.WriteHeader(http.StatusOK) - return - } - http.Redirect(w, r, "/api/system/configuration", http.StatusFound) - }) - defer server.Close() - rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) - require.NoError(t, err) - rtAuth := rtManager.GetConfig().GetServiceDetails() - analyzer := treeAnalyzer{ - rtManager: rtManager, - httpClientDetails: rtAuth.CreateHttpClientDetails(), - url: rtAuth.GetUrl(), - repo: "repo", - tech: techutils.Pipenv, - } - requestDetails := analyzer.httpClientDetails.Clone() - requestDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn" +func TestSendBoundedRequestRejectsRedirectOutsideRepository(t *testing.T) { + for _, tech := range []techutils.Technology{techutils.Pip, techutils.Poetry, techutils.Pipenv} { + t.Run(tech.String(), func(t *testing.T) { + var outsideRequested atomic.Bool + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.URL.Path == "/api/system/configuration" { + outsideRequested.Store(true) + w.WriteHeader(http.StatusOK) + return + } + http.Redirect(w, r, "/api/system/configuration", http.StatusFound) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + rtAuth := rtManager.GetConfig().GetServiceDetails() + analyzer := treeAnalyzer{ + rtManager: rtManager, + httpClientDetails: rtAuth.CreateHttpClientDetails(), + url: rtAuth.GetUrl(), + repo: "repo", + tech: tech, + } + requestDetails := analyzer.httpClientDetails.Clone() + requestDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn" - _, _, err = analyzer.sendPipenvRequest(http.MethodGet, - strings.TrimSuffix(analyzer.url, "/")+"/api/pypi/repo/packages/pkg.whl", requestDetails) - require.Error(t, err) - assert.Contains(t, err.Error(), "unsafe redirect") - assert.False(t, outsideRequested.Load()) - assert.Equal(t, int32(1), requests.Load()) + _, _, err = analyzer.sendBoundedRequest(http.MethodGet, + strings.TrimSuffix(analyzer.url, "/")+"/api/pypi/repo/packages/pkg.whl", requestDetails) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.False(t, outsideRequested.Load()) + assert.Equal(t, int32(1), requests.Load()) + }) + } } -func TestPipenvCvsMetadataRejectsRedirectOutsideRepository(t *testing.T) { +func TestCvsMetadataRejectsRedirectOutsideRepository(t *testing.T) { tests := []struct { name string call func(*treeAnalyzer) error @@ -2476,8 +2508,48 @@ func TestPipenvCvsMetadataRejectsRedirectOutsideRepository(t *testing.T) { }, }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + for _, tech := range []techutils.Technology{techutils.Pip, techutils.Poetry, techutils.Pipenv} { + for _, test := range tests { + t.Run(tech.String()+"/"+test.name, func(t *testing.T) { + var outsideRequested atomic.Bool + var requests atomic.Int32 + server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + if r.URL.Path == "/api/system/configuration" { + outsideRequested.Store(true) + w.WriteHeader(http.StatusOK) + return + } + http.Redirect(w, r, "/api/system/configuration", http.StatusFound) + }) + defer server.Close() + rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) + require.NoError(t, err) + rtAuth := rtManager.GetConfig().GetServiceDetails() + analyzer := &treeAnalyzer{ + rtManager: rtManager, + httpClientDetails: rtAuth.CreateHttpClientDetails(), + url: rtAuth.GetUrl(), + repo: "repo", + tech: tech, + } + + err = test.call(analyzer) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe redirect") + assert.False(t, outsideRequested.Load()) + assert.Equal(t, int32(1), requests.Load()) + }) + } + } +} + +// TestFetchNodeStatusRoutesPipAndPoetryThroughBoundedRedirects exercises the actual +// tech-branch in fetchNodeStatus (not sendBoundedRequest directly) to guard against a +// regression that silently narrows the bounded-redirect condition back to Pipenv only. +func TestFetchNodeStatusRoutesPipAndPoetryThroughBoundedRedirects(t *testing.T) { + for _, tech := range []techutils.Technology{techutils.Pip, techutils.Poetry, techutils.Pipenv} { + t.Run(tech.String(), func(t *testing.T) { var outsideRequested atomic.Bool var requests atomic.Int32 server, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { @@ -2493,15 +2565,18 @@ func TestPipenvCvsMetadataRejectsRedirectOutsideRepository(t *testing.T) { rtManager, err := rtUtils.CreateServiceManager(serverDetails, 0, 0, false) require.NoError(t, err) rtAuth := rtManager.GetConfig().GetServiceDetails() - analyzer := &treeAnalyzer{ + nodeId := python.PythonPackageTypeIdentifier + "pkg:1.0.0" + packageUrl := strings.TrimSuffix(rtAuth.GetUrl(), "/") + "/api/pypi/repo/packages/pkg.whl" + analyzer := treeAnalyzer{ rtManager: rtManager, httpClientDetails: rtAuth.CreateHttpClientDetails(), url: rtAuth.GetUrl(), repo: "repo", - tech: techutils.Pipenv, + tech: tech, + downloadUrls: map[string]string{nodeId: packageUrl}, } - err = test.call(analyzer) + err = analyzer.fetchNodeStatus(xrayUtils.GraphNode{Id: nodeId}, &sync.Map{}) require.Error(t, err) assert.Contains(t, err.Error(), "unsafe redirect") assert.False(t, outsideRequested.Load())