From 96b9c1474413bec4b0ae0eca21a5e685d9b0e54c Mon Sep 17 00:00:00 2001 From: Gauri Yadav Date: Fri, 31 Jul 2026 14:08:48 +0530 Subject: [PATCH] XRAY-156190 - Implement all packages blocked in packument fallback logic for npm --- commands/curation/curationaudit.go | 243 +++++++++++++- commands/curation/curationaudit_test.go | 142 +++++++++ commands/curation/npmlogparser.go | 301 ++++++++++++++++++ commands/curation/npmlogparser_test.go | 182 +++++++++++ sca/bom/buildinfo/technologies/npm/npm.go | 20 ++ .../2026-01-01T00_00_00_000Z-debug-0.log | 31 ++ 6 files changed, 918 insertions(+), 1 deletion(-) create mode 100644 commands/curation/npmlogparser.go create mode 100644 commands/curation/npmlogparser_test.go create mode 100644 tests/testdata/curation/npmlogs/mixed-crash/_logs/2026-01-01T00_00_00_000Z-debug-0.log diff --git a/commands/curation/curationaudit.go b/commands/curation/curationaudit.go index 571c6c688..d81e7295f 100644 --- a/commands/curation/curationaudit.go +++ b/commands/curation/curationaudit.go @@ -105,6 +105,14 @@ const ( // hfUnresolvedReportKey is used when an HF scan found only dynamic references — no table, just warnings. hfUnresolvedReportKey = "huggingface (unresolved references)" + + // npmLogPartialReportWarning is shown when npm install crashed mid-resolution (e.g. a + // fully-blocked package's packument returned 403) and runNpmLogFallback recovered a + // partial report from npm's own debug log instead of aborting with no output. + npmLogPartialReportWarning = "npm install could not fully resolve the dependency tree because one or more packages " + + "are blocked by the curation policy. This report was reconstructed from npm's debug log after the installation " + + "failed, so it may be incomplete. Dependencies of blocked packages could not be analyzed because their metadata " + + "was unavailable." ) var CurationOutputFormats = []string{string(outFormat.Table), string(outFormat.Json)} @@ -289,6 +297,10 @@ type CurationReport struct { // Kept separate from isPartial, which also triggers the CVS-specific warning // and skips the waiver flow. hfPartial bool + // npmLogPartial marks a report produced by runNpmLogFallback. Set alongside isPartial + // (to reuse its waiver-skip gate), but kept distinct for its own warning text and + // partialReason. + npmLogPartial bool // warnings holds user-facing messages from tree-build (e.g. unresolved HF references). warnings []string // huggingFaceReport marks reports produced by the Hugging Face audit path (including @@ -426,7 +438,11 @@ func (ca *CurationAuditCommand) Run() (err error) { } for projectPath, report := range results { if report.isPartial { - log.Warn(fmt.Sprintf("[%s] %s", projectPath, cvsPartialReportWarning)) + warningText := cvsPartialReportWarning + if report.npmLogPartial { + warningText = npmLogPartialReportWarning + } + log.Warn(fmt.Sprintf("[%s] %s", projectPath, warningText)) } } @@ -481,6 +497,11 @@ func convertResultsToSummary(results map[string]*CurationReport) formats.Results } var partialReason string switch { + // npmLogPartial is checked before isPartial: an npm-log-fallback report sets both + // (isPartial to reuse the existing waiver-skip gate below), so npmLogPartial must + // win here or it would incorrectly report "cvs_fallback". + case packagesStatus.npmLogPartial: + partialReason = "npm_log_fallback" case packagesStatus.isPartial: partialReason = "cvs_fallback" case packagesStatus.hfPartial: @@ -949,6 +970,26 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if err != nil { return err } + // For npm, discover the user's own cache dir and logs-max setting before installing — + // --cache is never overridden, only used to locate the debug log runNpmLogFallback reads. + // If logs-max was already 0, our forced --logs-max=10 is the only reason a log exists, + // so we delete just that file afterward. + var npmCacheDir string + var npmLogsMaxWasZero bool + if tech == techutils.Npm { + workDir, wdErr := osGetwd() + if wdErr != nil { + return errorutils.CheckErrorf("failed to determine working directory for npm config lookup: %v", wdErr) + } + if npmCacheDir, err = npmtech.GetNpmConfigValue(workDir, "cache"); err != nil { + return errorutils.CheckErrorf("failed to determine npm cache directory: %v", err) + } + logsMaxValue, logsMaxErr := npmtech.GetNpmConfigValue(workDir, "logs-max") + if logsMaxErr != nil { + return errorutils.CheckErrorf("failed to determine npm logs-max config: %v", logsMaxErr) + } + npmLogsMaxWasZero = logsMaxValue == "0" + } depTreeResult, err := buildinfo.GetTechDependencyTree(params, serverDetails, tech) if err != nil { // When CVS strips a pinned version from the simple index, pip can't @@ -959,6 +1000,11 @@ func (ca *CurationAuditCommand) auditTree(tech techutils.Technology, results map if (tech == techutils.Pip || tech == techutils.Poetry) && errors.As(err, &cvsErr) { return ca.runCvsFallback(cvsErr, tech, results) } + // npm's install error is a plain opaque error, nothing to type-match on — trigger the + // debug-log fallback unconditionally on any npm tree-build failure. + if tech == techutils.Npm { + return ca.runNpmLogFallback(npmCacheDir, tech, results, err, npmLogsMaxWasZero) + } return err } // Validate the graph isn't empty. @@ -1724,6 +1770,201 @@ func (ca *CurationAuditCommand) runCvsFallback(cvsErr *python.CvsBlockedError, t return nil } +// runNpmLogFallback reconstructs a partial curation report from npm's debug log +// (cacheDir/_logs/) when the npm tree-build install crashes, since buildDeps completes +// before reify can fail. Falls back to originalErr unchanged if nothing is recoverable. +// See NPM_CURATION_DEBUGLOG_FALLBACK_DESIGN for the full design. +// +// logsMaxWasZero means our forced --logs-max=10 is the only reason a debug log exists for +// this run, so the log file read here is removed afterward; otherwise npm's own rotation +// manages it like any other run. +func (ca *CurationAuditCommand) runNpmLogFallback(cacheDir string, tech techutils.Technology, results map[string]*CurationReport, originalErr error, logsMaxWasZero bool) error { + entries, blockedPackages, logFilePath, parseErr := parseNpmDebugLog(cacheDir) + if logsMaxWasZero && logFilePath != "" { + defer func() { + _ = os.Remove(logFilePath) + }() + } + if parseErr != nil { + log.Debug(fmt.Sprintf("npm curation debug-log fallback: failed to parse debug log (%v); original error: %s", parseErr, originalErr.Error())) + return originalErr + } + if len(entries) == 0 { + // Nothing recoverable — install likely failed before buildDeps ever started. + return originalErr + } + + rtManager, serverDetails, err := ca.getRtManagerAndAuth(tech) + if err != nil { + return fmt.Errorf("npm curation debug-log fallback: failed to get Artifactory manager (%w); %s error: %w", err, tech, originalErr) + } + rtAuth, err := serverDetails.CreateArtAuthConfig() + if err != nil { + return fmt.Errorf("npm curation debug-log fallback: failed to create auth config (%w); %s error: %w", err, tech, originalErr) + } + + workPath, wdErr := osGetwd() + if wdErr != nil { + log.Warn(fmt.Sprintf("npm curation debug-log fallback: could not determine working directory (%v) — reporting under fallback key", wdErr)) + workPath = "unknown-project" + } + rootName, rootVersion, pkgErr := readNpmProjectNameVersion(workPath) + if pkgErr != nil { + // Same convention auditTree itself uses when a project doesn't declare a name. + rootName = filepath.Base(workPath) + } + + analyzer := treeAnalyzer{ + rtManager: rtManager, + extractPoliciesRegex: ca.extractPoliciesRegex, + rtAuth: rtAuth, + httpClientDetails: rtAuth.CreateHttpClientDetails(), + url: rtAuth.GetUrl(), + repo: ca.PackageManagerConfig.TargetRepo(), + tech: tech, + } + + // Used later to decide whether a whole-package-blocked row's recommendation should be + // "remove and replace" (direct) or name the real direct dependency (transitive). + directDepNames := map[string]bool{} + for _, entry := range entries { + if entry.ParentName == rootName && entry.ParentVersion == rootVersion { + directDepNames[entry.Name] = true + } + } + + preProcessMap := &sync.Map{} + var advisoryWarnings []string + seenKeys := map[string]bool{} + for _, entry := range sortedNpmLogEntries(entries) { + category := classifyBlankVersionEntry(entry, blockedPackages) + key, keyErr := npmPackageKey(tech, analyzer.url, analyzer.repo, entry.Name, entry.Version) + if keyErr != nil || seenKeys[key] { + continue + } + seenKeys[key] = true + + switch category { + case npmEntryResolved: + pkgStatus, probeErr := analyzer.getBlockedPackageDetails(key, entry.Name, entry.Version) + if probeErr != nil || pkgStatus == nil { + continue // network/auth error, or genuinely clean — no row either way + } + preProcessMap.Store(key, pkgStatus) + case npmEntryWholePackageBlocked: + preProcessMap.Store(key, wholePackageBlockedStatus(entry, blockedPackages[entry.Name], tech)) + case npmEntryNonRegistrySpecifier: + preProcessMap.Store(key, nonRegistrySpecifierStatus(entry, tech)) + case npmEntryUnresolvableRange: + advisoryWarnings = append(advisoryWarnings, fmt.Sprintf( + "Package '%s' (requested as '%s') could not be resolved to a specific version, so its curation status could not be checked. This is not necessarily a curation block — verify the dependency resolves correctly.", + entry.Name, entry.Specifier)) + case npmEntryETARGET: + etargetKey, etargetKeyErr := npmPackageKey(tech, analyzer.url, analyzer.repo, entry.Name, entry.Specifier) + if etargetKeyErr != nil { + continue + } + pkgStatus, probeErr := analyzer.getBlockedPackageDetails(etargetKey, entry.Name, entry.Specifier) + if probeErr != nil { + continue + } + if pkgStatus == nil { + // Plain 404, no curation signal — not curation-relevant, advisory only. + advisoryWarnings = append(advisoryWarnings, fmt.Sprintf( + "Package '%s@%s' could not be resolved (no matching version found in the registry) — this is not a curation policy block. Verify the version in package.json.", + entry.Name, entry.Specifier)) + continue + } + preProcessMap.Store(etargetKey, pkgStatus) + } + } + + graph := buildGraphFromLogEntries(entries, rootName, rootVersion) + var packagesStatus []*PackageStatus + analyzer.GraphsRelations([]*xrayUtils.GraphNode{graph}, preProcessMap, &packagesStatus) + + // Recommendation depends on ParentName, only known after GraphsRelations runs. + for _, ps := range packagesStatus { + if ps.PackageVersion == allVersionsBlockedText { + applyTransitiveAwareRecommendation(ps, directDepNames[ps.PackageName]) + } + } + + if len(packagesStatus) == 0 && len(advisoryWarnings) == 0 { + return originalErr + } + + results[uniqueReportKey(results, filepath.Base(workPath), tech)] = &CurationReport{ + packagesStatus: packagesStatus, + totalNumberOfPackages: len(packagesStatus), + isPartial: true, + npmLogPartial: true, + warnings: advisoryWarnings, + } + return nil +} + +// allVersionsBlockedText also doubles as the marker applyTransitiveAwareRecommendation uses +// to identify whole-package-blocked rows after the graph walk. +const allVersionsBlockedText = "All versions blocked" + +// wholePackageBlockedStatus synthesizes a *PackageStatus straight from the log's notice+403 +// evidence — no HEAD-check needed. Recommendation is filled in later by +// applyTransitiveAwareRecommendation, once the graph walk knows the parent. +func wholePackageBlockedStatus(entry npmLogEntry, info npmBlockedInfo, tech techutils.Technology) *PackageStatus { + return &PackageStatus{ + PackageName: entry.Name, + PackageVersion: allVersionsBlockedText, + Action: blocked, + BlockingReason: BlockingReasonPolicy, + PkgType: string(tech), + Policy: []Policy{{ + Policy: info.Policy, + Condition: info.Condition, + Explanation: allVersionsBlockedText, + // Recommendation is filled in by applyTransitiveAwareRecommendation. + }}, + } +} + +// applyTransitiveAwareRecommendation: "remove and replace" is only actionable when the +// blocked package is itself a direct dependency; otherwise name the real direct dependency +// so the user has an actual lever (waiver, or replace that dependency). +func applyTransitiveAwareRecommendation(ps *PackageStatus, isDirectDependency bool) { + for i := range ps.Policy { + if isDirectDependency { + ps.Policy[i].Recommendation = "Remove this package from your project and replace with an alternate package" + } else { + ps.Policy[i].Recommendation = fmt.Sprintf( + "%s is a transitive dependency of %s and cannot be removed directly. Apply a waiver if acceptable, or replace %s with an alternative that doesn't depend on %s.", + ps.PackageName, ps.ParentName, ps.ParentName, ps.PackageName) + } + } +} + +// nonRegistrySpecifierStatus reports a git URL/local-reference/npm-alias dependency as its +// own row with "—" Policy/Condition — a third state distinct from clean/blocked, since npm +// never contacts the registry for these at all and curation's enforcement never applies. +// Never makes a network call on this package's behalf. +func nonRegistrySpecifierStatus(entry npmLogEntry, tech techutils.Technology) *PackageStatus { + return &PackageStatus{ + PackageName: entry.Name, + PackageVersion: fmt.Sprintf("%s (not evaluated)", entry.Specifier), + Action: blocked, + BlockingReason: BlockingReasonUnknown, + PkgType: string(tech), + Policy: []Policy{{ + Policy: "—", + Condition: "—", + Explanation: "Not evaluated — this dependency's specifier is not a registry version (e.g. a git URL, local " + + "reference, or npm alias). npm never contacts Artifactory for it at all (no packument fetch, no tarball " + + "download), so curation's enforcement never applies.", + Recommendation: "If policy compliance is required for this dependency, use the equivalent published npm " + + "package instead of this reference, if one exists.", + }}, + } +} + // lookupPypiAllVersions calls the Artifactory PyPI metadata API for a package // name (no version — returns all releases) and returns all available version // strings. This endpoint is NOT filtered by CVS, so it includes versions that diff --git a/commands/curation/curationaudit_test.go b/commands/curation/curationaudit_test.go index 3eb746549..5b288b933 100644 --- a/commands/curation/curationaudit_test.go +++ b/commands/curation/curationaudit_test.go @@ -2823,6 +2823,148 @@ func TestRunCvsFallback_KeepsSeparateTableFromExistingReport(t *testing.T) { assert.False(t, pipReport.huggingFaceReport, "pip's own report must not carry the HF marker") } +// Feeds the mixed-crash.log fixture through the full runNpmLogFallback end to end. +func TestRunNpmLogFallback(t *testing.T) { + const repo = "my-pnpm-remote" + acceptsBlockMsg := "Package accepts:2.0.0 download was blocked by jfrog packages curation service due to the following policies violated " + + "{cve-high, CVE with CVSS score of 9 or above, Package version contains a vulnerability, Upgrade to a fixed version}." + acceptsBlockJSON := fmt.Sprintf(`{"errors":[{"status":403,"message":%q}]}`, acceptsBlockMsg) + + serverMock, serverDetails, _ := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/accepts-2.0.0.tgz"): + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(acceptsBlockJSON)) + case strings.Contains(r.URL.Path, "/lodash-99.99.99.tgz"): + // Genuine ETARGET: plain 404, no curation signal. + w.WriteHeader(http.StatusNotFound) + case strings.Contains(r.URL.Path, "/express-5.2.1.tgz"), strings.Contains(r.URL.Path, "/typescript-5.3.3.tgz"): + w.WriteHeader(http.StatusOK) + case strings.Contains(r.URL.Path, "/depd-"), strings.Contains(r.URL.Path, "/is-thirteen-"), strings.Contains(r.URL.Path, "/some-range-pkg-"): + t.Errorf("unexpected HEAD-check for a whole-package-blocked/non-registry/unresolvable-range entry: %s", r.URL.Path) + w.WriteHeader(http.StatusInternalServerError) + default: + w.WriteHeader(http.StatusNotFound) + } + }) + defer serverMock.Close() + + repoConfig := (&project.RepositoryConfig{}). + SetTargetRepo(repo). + SetServerDetails(serverDetails) + ca := &CurationAuditCommand{ + PackageManagerConfig: repoConfig, + extractPoliciesRegex: regexp.MustCompile(extractPoliciesRegexTemplate), + } + + projectDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(projectDir, "package.json"), + []byte(`{"name":"mypnpmproject-v11","version":"1.0.0"}`), 0644)) + orig := osGetwd + osGetwd = func() (string, error) { return projectDir, nil } + t.Cleanup(func() { osGetwd = orig }) + + results := map[string]*CurationReport{} + originalErr := errors.New("error while running 'npm install': exit status 1") + logDir := filepath.Join(TestDataDir, "curation", "npmlogs", "mixed-crash") + + err := ca.runNpmLogFallback(logDir, techutils.Npm, results, originalErr, false) + require.NoError(t, err) + require.Len(t, results, 1) + + var report *CurationReport + for _, r := range results { + report = r + } + require.NotNil(t, report) + assert.True(t, report.isPartial) + assert.True(t, report.npmLogPartial) + + byName := map[string]*PackageStatus{} + for _, ps := range report.packagesStatus { + byName[ps.PackageName] = ps + } + + // Whole-package-blocked, transitive via express: "All versions blocked" and the + // transitive-aware recommendation naming both packages. + depd := byName["depd"] + require.NotNil(t, depd, "depd should be reported as a whole-package block") + assert.Equal(t, allVersionsBlockedText, depd.PackageVersion) + assert.Equal(t, "express", depd.ParentName, "depd's direct-dependency attribution must walk to express, not stop at an intermediate parent") + require.Len(t, depd.Policy, 1) + assert.Contains(t, depd.Policy[0].Recommendation, "transitive dependency of express") + + // Resolved and blocked via the normal HEAD-check path (getBlockedPackageDetails, + // unmodified) — full policy detail recovered from the 403 body. + accepts := byName["accepts"] + require.NotNil(t, accepts, "accepts should be reported as blocked via the standard HEAD-check") + assert.Equal(t, "2.0.0", accepts.PackageVersion) + require.Len(t, accepts.Policy, 1) + assert.Equal(t, "cve-high", accepts.Policy[0].Policy) + + // Git-URL: reported as its own row, never probed. + gitDep := byName["is-thirteen"] + require.NotNil(t, gitDep, "git-URL dependency should be reported as its own row") + assert.Contains(t, gitDep.PackageVersion, "github:jonschlinkert/is-thirteen") + assert.Equal(t, "—", gitDep.Policy[0].Policy) + + // Clean packages never appear as rows. + assert.NotContains(t, byName, "express") + assert.NotContains(t, byName, "typescript") + + // Genuine ETARGET and unresolvable range: excluded from the table, advisory warnings only. + assert.NotContains(t, byName, "lodash") + assert.NotContains(t, byName, "some-range-pkg") + require.Len(t, report.warnings, 2) + warningsText := strings.Join(report.warnings, "\n") + assert.Contains(t, warningsText, "lodash@99.99.99") + assert.Contains(t, warningsText, "not a curation policy block") + assert.Contains(t, warningsText, "some-range-pkg") + assert.Contains(t, warningsText, "^3.0.0") +} + +// If the log has no placeDep lines at all, the original error is returned unchanged. +func TestRunNpmLogFallback_NoRecoverableEntries(t *testing.T) { + ca := &CurationAuditCommand{ + PackageManagerConfig: (&project.RepositoryConfig{}).SetTargetRepo("repo"), + extractPoliciesRegex: regexp.MustCompile(extractPoliciesRegexTemplate), + } + results := map[string]*CurationReport{} + originalErr := errors.New("npm install failed before any resolution") + + err := ca.runNpmLogFallback(t.TempDir(), techutils.Npm, results, originalErr, false) + assert.Equal(t, originalErr, err) + assert.Empty(t, results) +} + +// When logs-max was 0, only the specific log file read is removed — not the whole directory. +func TestRunNpmLogFallback_CleansUpLogWhenLogsMaxWasZero(t *testing.T) { + cacheDir := t.TempDir() + logsDir := filepath.Join(cacheDir, "_logs") + require.NoError(t, os.MkdirAll(logsDir, 0755)) + logPath := filepath.Join(logsDir, "2026-01-01T00_00_00_000Z-debug-0.log") + require.NoError(t, os.WriteFile(logPath, []byte("0 verbose cli npm\n"), 0644)) + otherPath := filepath.Join(logsDir, "unrelated-file.txt") + require.NoError(t, os.WriteFile(otherPath, []byte("leave me alone\n"), 0644)) + + ca := &CurationAuditCommand{ + PackageManagerConfig: (&project.RepositoryConfig{}).SetTargetRepo("repo"), + extractPoliciesRegex: regexp.MustCompile(extractPoliciesRegexTemplate), + } + results := map[string]*CurationReport{} + originalErr := errors.New("npm install failed before any resolution") + + // No placeDep lines in this fixture, so this hits the "nothing recoverable" early + // return — cleanup must still fire on that path, not only on a fully successful one. + err := ca.runNpmLogFallback(cacheDir, techutils.Npm, results, originalErr, true) + assert.Equal(t, originalErr, err) + + _, statErr := os.Stat(logPath) + assert.True(t, os.IsNotExist(statErr), "the log file itself should have been removed") + _, otherStatErr := os.Stat(otherPath) + assert.NoError(t, otherStatErr, "an unrelated file in the same directory must not be touched") +} + // TestUniqueReportKey covers the disambiguation loop, including a 3-way collision where both // the plain key and its first tech-suffixed candidate are already taken. func TestUniqueReportKey(t *testing.T) { diff --git a/commands/curation/npmlogparser.go b/commands/curation/npmlogparser.go new file mode 100644 index 000000000..a5153eb8b --- /dev/null +++ b/commands/curation/npmlogparser.go @@ -0,0 +1,301 @@ +package curation + +import ( + "bufio" + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/jfrog/jfrog-cli-security/utils/xray" + "github.com/jfrog/jfrog-client-go/utils/errorutils" + xrayUtils "github.com/jfrog/jfrog-client-go/xray/services/utils" +) + +// One reconstructed `silly placeDep` line: +// +// silly placeDep @ OK for: @ want: +// +// Version is blank when Arborist never resolved it — see classifyBlankVersionEntry. +type npmLogEntry struct { + Location string + Name string + Version string + ParentName string + ParentVersion string + Specifier string +} + +// npmEntryCategory classifies a blank-version npmLogEntry — see classifyBlankVersionEntry. +type npmEntryCategory int + +const ( + // Concrete version in the log — standard per-package HEAD-check, same as the normal flow. + npmEntryResolved npmEntryCategory = iota + // Packument fetch itself 403'd (all versions blocked) — no HEAD-check needed or possible. + npmEntryWholePackageBlocked + // Non-registry specifier (git URL/shorthand, local file/link/workspace/patch, npm alias) — never probed. + npmEntryNonRegistrySpecifier + // Range/wildcard/dist-tag with no concrete version — never probed, since guessing one would be + // misleading rather than just incomplete. Advisory only. + npmEntryUnresolvableRange + // Genuine "no matching version" for an already-bare, exact version — still probed to rule out a block. + npmEntryETARGET +) + +// npmBlockedInfo carries the policy/condition parsed from a whole-package-block notice line. +type npmBlockedInfo struct { + Policy string + Condition string +} + +var ( + // Matches: 42 notice All versions blocked - {policy:X,condition:Y} + npmNoticeBlockedRegex = regexp.MustCompile(`^\d+\s+notice\s+All versions blocked - \{policy:([^,]+),condition:([^}]+)\}`) + // Matches: 43 http fetch GET 403 https://.../api/npm// ... + npmFetch403Regex = regexp.MustCompile(`^\d+\s+http fetch GET 403\s+\S*/api/npm/[^/\s]+/(\S+)\s`) + // Matches: 110 silly placeDep ROOT accepts@2.0.0 OK for: express@5.2.1 want: ^2.0.0 + npmPlaceDepRegex = regexp.MustCompile( + `^\d+\s+silly\s+placeDep\s+(\S+)\s+((?:@[^@\s]+/)?[^@\s]+)@(\S*)\s+OK for:\s+((?:@[^@\s]+/)?[^@\s]+)@(\S+)\s+want:\s+(\S+)\s*$`) +) + +// parseNpmDebugLog reads the newest debug log under /_logs/ and returns every +// placeDep entry plus the set of packages whose packument fetch was itself blocked. Also +// returns the log file path read, so the caller can decide whether to remove it. +func parseNpmDebugLog(logDir string) (entries []npmLogEntry, blockedPackages map[string]npmBlockedInfo, logFilePath string, err error) { + logFile, err := findNewestNpmDebugLog(logDir) + if err != nil { + return nil, nil, "", err + } + if logFile == "" { + return nil, nil, "", nil + } + + f, err := os.Open(logFile) + if err != nil { + return nil, nil, "", errorutils.CheckErrorf("failed to open npm debug log %q: %v", logFile, err) + } + defer func() { + _ = f.Close() + }() + + blockedPackages = map[string]npmBlockedInfo{} + var pendingNotice *npmBlockedInfo + scanner := bufio.NewScanner(f) + // Some policy-violation lines are long; grow past bufio's 64KB default to avoid truncation. + buf := make([]byte, 0, 64*1024) + scanner.Buffer(buf, 8*1024*1024) + for scanner.Scan() { + line := scanner.Text() + + if m := npmNoticeBlockedRegex.FindStringSubmatch(line); m != nil { + pendingNotice = &npmBlockedInfo{Policy: strings.TrimSpace(m[1]), Condition: strings.TrimSpace(m[2])} + continue + } + if pendingNotice != nil { + notice := pendingNotice + pendingNotice = nil + if m := npmFetch403Regex.FindStringSubmatch(line); m != nil { + blockedPackages[m[1]] = *notice + } + // Fall through either way — this line may also be a placeDep line. + } + + if m := npmPlaceDepRegex.FindStringSubmatch(line); m != nil { + entries = append(entries, npmLogEntry{ + Location: m[1], + Name: m[2], + Version: m[3], + ParentName: m[4], + ParentVersion: m[5], + Specifier: m[6], + }) + } + } + if scanErr := scanner.Err(); scanErr != nil { + return nil, nil, "", errorutils.CheckErrorf("failed to scan npm debug log %q: %v", logFile, scanErr) + } + return entries, blockedPackages, logFile, nil +} + +// findNewestNpmDebugLog returns the most recently modified *-debug-0.log under /_logs/, +// or "" if none exist. +func findNewestNpmDebugLog(logDir string) (string, error) { + logsDir := filepath.Join(logDir, "_logs") + dirEntries, err := os.ReadDir(logsDir) + if err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", errorutils.CheckErrorf("failed to read npm debug log directory %q: %v", logsDir, err) + } + var newestPath string + var newestModTime int64 + for _, de := range dirEntries { + if de.IsDir() || !strings.HasSuffix(de.Name(), ".log") { + continue + } + info, infoErr := de.Info() + if infoErr != nil { + continue + } + if modTime := info.ModTime().UnixNano(); newestPath == "" || modTime > newestModTime { + newestPath = filepath.Join(logsDir, de.Name()) + newestModTime = modTime + } + } + return newestPath, nil +} + +// classifyBlankVersionEntry categorizes an npmLogEntry whose Version is blank (non-blank is +// always npmEntryResolved). A range/wildcard/dist-tag is never treated as probeable even after +// stripping its operator (e.g. "^2.0.0" -> "2.0.0") — that stripped value is a guess at what npm +// might have installed, not the real answer, so it'd be misleading rather than just incomplete. +func classifyBlankVersionEntry(entry npmLogEntry, blockedPackages map[string]npmBlockedInfo) npmEntryCategory { + if entry.Version != "" { + return npmEntryResolved + } + if _, blocked := blockedPackages[entry.Name]; blocked { + return npmEntryWholePackageBlocked + } + resolvedVer, probeable, rangeOrTag := classifyNpmSpecifier(entry.Specifier) + switch { + case !probeable && !rangeOrTag: + return npmEntryNonRegistrySpecifier + case !probeable && rangeOrTag: + return npmEntryUnresolvableRange + case entry.Specifier != resolvedVer: + return npmEntryUnresolvableRange + default: + return npmEntryETARGET + } +} + +// npmConcreteVersionRegex matches a single concrete semver, optional pre-release/build metadata. +var npmConcreteVersionRegex = regexp.MustCompile(`^\d+\.\d+\.\d+([-+][0-9A-Za-z.\-]+)*$`) + +// classifyNpmSpecifier inspects a package.json version specifier: probeable=true means it +// resolves to a single concrete semver after stripping range operators; rangeOrTag=true means +// a range/wildcard/dist-tag; both false means a non-registry protocol (file:, link:, workspace:, +// patch:, portal:, git+, git:, http(s):, npm:) or git host shorthand (github:, gitlab:, +// bitbucket:, gist:). +// +// Deliberate self-contained copy of yarn.go's classifyNpmVersionSpec (PR #759/XRAY-138688) — +// kept scoped to npm only so this doesn't touch yarn's code; see the note filed for a future +// yarn PR about unifying them. +func classifyNpmSpecifier(spec string) (resolvedVer string, probeable, rangeOrTag bool) { + s := strings.TrimSpace(spec) + if s == "" { + return "", false, false + } + lc := strings.ToLower(s) + for _, p := range []string{ + "file:", "link:", "workspace:", "patch:", "portal:", "git+", "git:", "http://", "https://", "npm:", + "github:", "gitlab:", "bitbucket:", "gist:", + } { + if strings.HasPrefix(lc, p) { + return "", false, false + } + } + for len(s) > 0 { + switch s[0] { + case '^', '~', '=': + s = s[1:] + continue + case '>', '<': + s = s[1:] + if len(s) > 0 && s[0] == '=' { + s = s[1:] + } + continue + } + break + } + s = strings.TrimSpace(s) + if npmConcreteVersionRegex.MatchString(s) { + return s, true, false + } + return "", false, true +} + +type npmProjectPackageJSON struct { + Name string `json:"name"` + Version string `json:"version"` +} + +func readNpmProjectNameVersion(projectDir string) (name, version string, err error) { + data, err := os.ReadFile(filepath.Join(projectDir, "package.json")) + if err != nil { + return "", "", errorutils.CheckErrorf("failed to read package.json in %q: %v", projectDir, err) + } + var pkg npmProjectPackageJSON + if err = json.Unmarshal(data, &pkg); err != nil { + return "", "", errorutils.CheckErrorf("failed to parse package.json in %q: %v", projectDir, err) + } + return pkg.Name, pkg.Version, nil +} + +// npmNodeId matches the "npm://name:version" format parseNpmDependenciesList uses in the +// normal flow, so the reconstructed tree is indistinguishable from a real one downstream. +func npmNodeId(name, version string) string { + return techutils.Npm.GetXrayPackageTypeId() + name + ":" + version +} + +// buildGraphFromLogEntries reconstructs a *xrayUtils.GraphNode tree from placeDep entries. +// Only immediate parent->child edges are recorded; resolving a deep node to its root-level +// direct-dependency ancestor is fillGraphRelations's job, reused unmodified. +func buildGraphFromLogEntries(entries []npmLogEntry, rootName, rootVersion string) *xrayUtils.GraphNode { + treeMap := make(map[string]xray.DepTreeNode) + for _, entry := range entries { + childId := npmNodeId(entry.Name, entry.Version) + parentId := npmNodeId(entry.ParentName, entry.ParentVersion) + depTreeNode := treeMap[parentId] + depTreeNode.Children = appendUniqueChildId(depTreeNode.Children, childId) + treeMap[parentId] = depTreeNode + } + rootId := npmNodeId(rootName, rootVersion) + graph, _ := xray.BuildXrayDependencyTree(treeMap, rootId) + return graph +} + +// npmSyntheticNodeForKey wraps just the id needed to derive a preProcessMap key, so +// whole-package-blocked/non-registry entries (synthesized directly) land under the same key +// fillGraphRelations will later compute for the same package. +func npmSyntheticNodeForKey(name, version string) *xrayUtils.GraphNode { + return &xrayUtils.GraphNode{Id: npmNodeId(name, version)} +} + +func npmPackageKey(tech techutils.Technology, artiUrl, repo, name, version string) (string, error) { + urls, _, _, _ := getUrlNameAndVersionByTech(tech, npmSyntheticNodeForKey(name, version), nil, artiUrl, repo) + if len(urls) == 0 { + return "", fmt.Errorf("could not derive a preProcessMap key for %s:%s", name, version) + } + return urls[0], nil +} + +// appendUniqueChildId is a local copy of the npm tech package's own unexported appendUniqueChild. +func appendUniqueChildId(children []string, childId string) []string { + for _, existing := range children { + if existing == childId { + return children + } + } + return append(children, childId) +} + +// sortedNpmLogEntries sorts by Name then Version for deterministic, reproducible output. +func sortedNpmLogEntries(entries []npmLogEntry) []npmLogEntry { + sorted := make([]npmLogEntry, len(entries)) + copy(sorted, entries) + sort.Slice(sorted, func(i, j int) bool { + if sorted[i].Name != sorted[j].Name { + return sorted[i].Name < sorted[j].Name + } + return sorted[i].Version < sorted[j].Version + }) + return sorted +} diff --git a/commands/curation/npmlogparser_test.go b/commands/curation/npmlogparser_test.go new file mode 100644 index 000000000..ae59f99bf --- /dev/null +++ b/commands/curation/npmlogparser_test.go @@ -0,0 +1,182 @@ +package curation + +import ( + "os" + "path/filepath" + "testing" + // TestDataDir is defined in curationaudit_test.go (same package). + + "github.com/jfrog/jfrog-cli-security/utils/techutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseNpmDebugLog(t *testing.T) { + entries, blockedPackages, logFilePath, err := parseNpmDebugLog(filepath.Join(TestDataDir, "curation", "npmlogs", "mixed-crash")) + require.NoError(t, err) + require.NotEmpty(t, entries) + assert.Equal(t, "2026-01-01T00_00_00_000Z-debug-0.log", filepath.Base(logFilePath)) + + // Whole-package block: notice immediately followed by the matching 403 fetch line. + require.Contains(t, blockedPackages, "depd") + assert.Equal(t, "blocks open ssf", blockedPackages["depd"].Policy) + assert.Equal(t, "open ssf", blockedPackages["depd"].Condition) + + byName := map[string]npmLogEntry{} + for _, e := range entries { + byName[e.Name] = e + } + + // Resolved. + require.Contains(t, byName, "express") + assert.Equal(t, "5.2.1", byName["express"].Version) + assert.Equal(t, "mypnpmproject-v11", byName["express"].ParentName) + + // Whole-package-blocked placeDep line: blank version, real parent. + require.Contains(t, byName, "depd") + assert.Empty(t, byName["depd"].Version) + assert.Equal(t, "express", byName["depd"].ParentName) + + // Git-URL: blank version, specifier contains "/". + require.Contains(t, byName, "is-thirteen") + assert.Empty(t, byName["is-thirteen"].Version) + assert.Equal(t, "github:jonschlinkert/is-thirteen", byName["is-thirteen"].Specifier) + + // Genuine ETARGET: blank version, no notice, specifier has no "/". + require.Contains(t, byName, "lodash") + assert.Empty(t, byName["lodash"].Version) + assert.Equal(t, "99.99.99", byName["lodash"].Specifier) + assert.NotContains(t, blockedPackages, "lodash") + + // Unresolvable range: blank version, not blocked, not a bare version. + require.Contains(t, byName, "some-range-pkg") + assert.Empty(t, byName["some-range-pkg"].Version) + assert.Equal(t, "^3.0.0", byName["some-range-pkg"].Specifier) + assert.NotContains(t, blockedPackages, "some-range-pkg") +} + +func TestParseNpmDebugLog_NoLogsDir(t *testing.T) { + entries, blockedPackages, logFilePath, err := parseNpmDebugLog(t.TempDir()) + require.NoError(t, err) + assert.Nil(t, entries) + assert.Nil(t, blockedPackages) + assert.Empty(t, logFilePath) +} + +// Git host shorthand (github:/gitlab:/bitbucket:/gist:) must not regress — real evidence +// (github:jonschlinkert/is-thirteen) depends on it. +func TestClassifyNpmSpecifier(t *testing.T) { + cases := []struct { + spec string + wantVer string + wantProbeable bool + wantRangeOrTag bool + }{ + {"3.0.1", "3.0.1", true, false}, + {"^3.0.1", "3.0.1", true, false}, + {"~1.2.3", "1.2.3", true, false}, + {"1.2.3-beta.1", "1.2.3-beta.1", true, false}, + {"1.x", "", false, true}, + {"*", "", false, true}, + {"latest", "", false, true}, + {"1.0.0 || 2.0.0", "", false, true}, + {"file:./local-pkg", "", false, false}, + {"link:../sibling", "", false, false}, + {"workspace:*", "", false, false}, + {"patch:react@npm%3A18.0.0", "", false, false}, + {"git+https://github.com/foo/bar.git", "", false, false}, + {"https://example.com/pkg.tgz", "", false, false}, + {"npm:other-name@1.0.0", "", false, false}, + // The exact gap found in yarn's own classifyNpmVersionSpec via real testing — + // git host shorthand, distinct from the explicit git:/git+ prefixes above. + {"github:jonschlinkert/is-thirteen", "", false, false}, + {"gitlab:owner/repo", "", false, false}, + {"bitbucket:owner/repo", "", false, false}, + {"gist:1234567890abcdef", "", false, false}, + {"", "", false, false}, + {" ", "", false, false}, + } + for _, tc := range cases { + t.Run(tc.spec, func(t *testing.T) { + ver, probeable, rangeOrTag := classifyNpmSpecifier(tc.spec) + assert.Equal(t, tc.wantVer, ver) + assert.Equal(t, tc.wantProbeable, probeable) + assert.Equal(t, tc.wantRangeOrTag, rangeOrTag) + }) + } +} + +func TestClassifyBlankVersionEntry(t *testing.T) { + blockedPackages := map[string]npmBlockedInfo{"depd": {Policy: "blocks open ssf", Condition: "open ssf"}} + + assert.Equal(t, npmEntryResolved, classifyBlankVersionEntry( + npmLogEntry{Name: "accepts", Version: "2.0.0"}, blockedPackages)) + + assert.Equal(t, npmEntryWholePackageBlocked, classifyBlankVersionEntry( + npmLogEntry{Name: "depd", Version: ""}, blockedPackages)) + + // Genuine git URL — non-registry, no probe. + assert.Equal(t, npmEntryNonRegistrySpecifier, classifyBlankVersionEntry( + npmLogEntry{Name: "is-thirteen", Version: "", Specifier: "github:jonschlinkert/is-thirteen"}, blockedPackages)) + + // Local workspace/file/link/patch reference and npm alias — also non-registry, no probe. + for _, spec := range []string{"workspace:*", "file:../local-pkg", "link:../sibling", "npm:other-name@^1.0.0", "patch:react@npm%3A18.0.0"} { + assert.Equal(t, npmEntryNonRegistrySpecifier, classifyBlankVersionEntry( + npmLogEntry{Name: "some-pkg", Version: "", Specifier: spec}, blockedPackages), "specifier %q", spec) + } + + // Genuinely bare, exact version — the only case that's safe to probe. + assert.Equal(t, npmEntryETARGET, classifyBlankVersionEntry( + npmLogEntry{Name: "lodash", Version: "", Specifier: "99.99.99"}, blockedPackages)) + + // Never probed, even reduced to a bare number after stripping its operator — a guess, not what was requested. + for _, spec := range []string{"^2.0.0", "~1.2.0", ">=1.0.0", "1.x", "*", "latest", "1.0.0 || 2.0.0"} { + assert.Equal(t, npmEntryUnresolvableRange, classifyBlankVersionEntry( + npmLogEntry{Name: "some-pkg", Version: "", Specifier: spec}, blockedPackages), "specifier %q", spec) + } +} + +func TestBuildGraphFromLogEntries(t *testing.T) { + entries := []npmLogEntry{ + {Name: "express", Version: "5.2.1", ParentName: "myproj", ParentVersion: "1.0.0"}, + {Name: "accepts", Version: "2.0.0", ParentName: "express", ParentVersion: "5.2.1"}, + {Name: "depd", Version: "", ParentName: "express", ParentVersion: "5.2.1"}, + } + graph := buildGraphFromLogEntries(entries, "myproj", "1.0.0") + require.NotNil(t, graph) + assert.Equal(t, npmNodeId("myproj", "1.0.0"), graph.Id) + require.Len(t, graph.Nodes, 1) + assert.Equal(t, npmNodeId("express", "5.2.1"), graph.Nodes[0].Id) + + childIds := map[string]bool{} + for _, child := range graph.Nodes[0].Nodes { + childIds[child.Id] = true + } + assert.True(t, childIds[npmNodeId("accepts", "2.0.0")]) + assert.True(t, childIds[npmNodeId("depd", "")]) +} + +func TestNpmPackageKey_ConsistentWithGetUrlNameAndVersionByTech(t *testing.T) { + // The key runNpmLogFallback stores under must be exactly what fillGraphRelations + // derives when it later walks the reconstructed tree and looks the node up — + // both must go through the same getUrlNameAndVersionByTech call. + key, err := npmPackageKey(techutils.Npm, "https://example.jfrog.io/artifactory", "npm-remote", "depd", "") + require.NoError(t, err) + + node := npmSyntheticNodeForKey("depd", "") + urls, name, _, version := getUrlNameAndVersionByTech(techutils.Npm, node, nil, "https://example.jfrog.io/artifactory", "npm-remote") + require.Len(t, urls, 1) + assert.Equal(t, urls[0], key) + assert.Equal(t, "depd", name) + assert.Empty(t, version) +} + +func TestReadNpmProjectNameVersion(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"myproj","version":"2.0.0"}`), 0644)) + + name, version, err := readNpmProjectNameVersion(dir) + require.NoError(t, err) + assert.Equal(t, "myproj", name) + assert.Equal(t, "2.0.0", version) +} diff --git a/sca/bom/buildinfo/technologies/npm/npm.go b/sca/bom/buildinfo/technologies/npm/npm.go index f2a51e064..76f018610 100644 --- a/sca/bom/buildinfo/technologies/npm/npm.go +++ b/sca/bom/buildinfo/technologies/npm/npm.go @@ -126,6 +126,21 @@ func GetNativeNpmRegistryConfig() (*NpmrcRegistryConfig, error) { }, nil } +// GetNpmConfigValue runs 'npm config get ' from workingDir, respecting the same +// .npmrc/env resolution a real npm command from that directory would see. +func GetNpmConfigValue(workingDir, key string) (string, error) { + npmVersion, npmExecPath, err := biutils.GetNpmVersionAndExecPath(log.Logger) + if err != nil { + return "", fmt.Errorf("failed to locate npm executable: %w", err) + } + disableWorkspaces := npmVersion.AtLeast("7.0.0") + data, _, err := biutils.RunNpmCmd(npmExecPath, workingDir, npmConfigGetArgs(key, disableWorkspaces), log.Logger) + if err != nil { + return "", fmt.Errorf("failed to run 'npm config get %s': %w", key, err) + } + return strings.TrimSpace(string(data)), nil +} + func npmConfigGetArgs(key string, disableWorkspaces bool) []string { args := []string{"config", "get", key} if disableWorkspaces { @@ -189,6 +204,11 @@ func createTreeDepsParam(params *technologies.BuildInfoBomGeneratorParams) biuti if params.NpmLegacyPeerDeps { installCommandArgs = appendUniqueFlag(installCommandArgs, LegacyPeerDepsFlag) } + if params.IsCurationCmd { + // Force a debug log for the crash-recovery fallback to read, without touching --cache. + // Appended last so this flag can't be shadowed by an earlier one. + installCommandArgs = append(installCommandArgs, "--logs-max", "10") + } npmTreeDepParam := biutils.NpmTreeDepListParam{ Args: addIgnoreScriptsFlag(params.Args), InstallCommandArgs: installCommandArgs, diff --git a/tests/testdata/curation/npmlogs/mixed-crash/_logs/2026-01-01T00_00_00_000Z-debug-0.log b/tests/testdata/curation/npmlogs/mixed-crash/_logs/2026-01-01T00_00_00_000Z-debug-0.log new file mode 100644 index 000000000..9b8772eb0 --- /dev/null +++ b/tests/testdata/curation/npmlogs/mixed-crash/_logs/2026-01-01T00_00_00_000Z-debug-0.log @@ -0,0 +1,31 @@ +0 verbose cli node npm +1 info using npm@10.5.0 +2 info using node@v20.12.2 +37 timing idealTree:init Completed in 5ms +38 silly idealTree buildDeps +39 silly fetch manifest express@^5.2.1 +40 http fetch GET 200 https://z0test.jfrogdev.org/artifactory/api/npm/my-pnpm-remote/express 700ms (cache revalidated) +41 silly fetch manifest lodash@99.99.99 +42 http fetch GET 200 https://z0test.jfrogdev.org/artifactory/api/npm/my-pnpm-remote/lodash 201ms (cache revalidated) +43 silly fetch manifest typescript@5.3.3 +44 http fetch GET 200 https://z0test.jfrogdev.org/artifactory/api/npm/my-pnpm-remote/typescript 216ms (cache revalidated) +45 silly fetch manifest is-thirteen@github:jonschlinkert/is-thirteen +46 silly placeDep ROOT express@5.2.1 OK for: mypnpmproject-v11@1.0.0 want: ^5.2.1 +47 silly placeDep ROOT lodash@ OK for: mypnpmproject-v11@1.0.0 want: 99.99.99 +48 silly placeDep ROOT typescript@5.3.3 OK for: mypnpmproject-v11@1.0.0 want: 5.3.3 +49 silly placeDep ROOT is-thirteen@ OK for: mypnpmproject-v11@1.0.0 want: github:jonschlinkert/is-thirteen +50 silly fetch manifest accepts@^2.0.0 +51 http fetch GET 200 https://z0test.jfrogdev.org/artifactory/api/npm/my-pnpm-remote/accepts 190ms (cache revalidated) +52 silly fetch manifest depd@^2.0.0 +53 notice All versions blocked - {policy:blocks open ssf,condition:open ssf} +54 http fetch GET 403 https://z0test.jfrogdev.org/artifactory/api/npm/my-pnpm-remote/depd 196ms (cache skip) +55 silly placeDep ROOT accepts@2.0.0 OK for: express@5.2.1 want: ^2.0.0 +56 silly placeDep node_modules/express depd@ OK for: express@5.2.1 want: ^2.0.0 +59 silly placeDep node_modules/express some-range-pkg@ OK for: express@5.2.1 want: ^3.0.0 +57 error command git --no-replace-objects ls-remote ssh://git@github.com/jonschlinkert/is-thirteen.git +58 error git@github.com: Permission denied (publickey). +293 timing idealTree:buildDeps Completed in 2959ms +296 timing command:install Completed in 2966ms +297 verbose stack lodash: No matching version found for lodash@99.99.99. +303 error code ETARGET +304 error notarget No matching version found for lodash@99.99.99.