diff --git a/sca/bom/buildinfo/technologies/yarn/yarn.go b/sca/bom/buildinfo/technologies/yarn/yarn.go index b351f1ef3..1753fe4fb 100644 --- a/sca/bom/buildinfo/technologies/yarn/yarn.go +++ b/sca/bom/buildinfo/technologies/yarn/yarn.go @@ -165,18 +165,19 @@ func BuildDependencyTree(params technologies.BuildInfoBomGeneratorParams) (depen } return } - // Yarn V2+ always emits the project root as "@workspace:.". Prefer - // that over build-info-go's heuristic, which can misidentify the root - // when package.json has no name field. - if workspaceRoot := findYarnWorkspaceRoot(dependenciesMap); workspaceRoot != nil { - root = workspaceRoot + // Curation-only: 'jf audit'/'jf scan' keep the root GetYarnDependencies resolved. + if params.IsCurationCmd { + packageName := "" + if packageInfo != nil { + packageName = packageInfo.Name + } + root = resolveYarnRoot(dependenciesMap, root, packageName) } + stripWorkspaceUseLocalSuffix(dependenciesMap) if root == nil { err = errorutils.CheckErrorf("could not identify the root workspace from yarn dependency output") return } - // Normalize workspace versions for display (drop Yarn's "-use.local"). - stripWorkspaceUseLocalSuffix(dependenciesMap) // When --working-dirs targets a workspace member, prune dependenciesMap // to the subgraph reachable from that member and reset root accordingly. // This keeps the dependency tree and the uniqueDeps list @@ -212,7 +213,7 @@ func BuildDependencyTree(params technologies.BuildInfoBomGeneratorParams) (depen if err != nil { return } - dependencyTree, uniqueDeps, err := parseYarnDependenciesMap(dependenciesMap, rootXrayId) + dependencyTree, uniqueDeps, err := parseYarnDependenciesMap(dependenciesMap, rootXrayId, params.IsCurationCmd) if err != nil { return } @@ -1405,10 +1406,8 @@ func registerYarnPluginInYarnrc(curWd string) error { return os.WriteFile(yarnrcPath, updated, 0600) } -// Parse the dependencies into a Xray dependency tree format -// stripWorkspaceUseLocalSuffix drops Yarn's "-use.local" marker so workspace -// versions display as declared (e.g. 0.0.0), consistent across lockfile/plugin -// paths. Display-only; members are excluded from the HEAD-check regardless. +// stripWorkspaceUseLocalSuffix drops Yarn's "-use.local" version marker +// from workspace entries so they display as declared (e.g. 0.0.0). func stripWorkspaceUseLocalSuffix(dependencies map[string]*bibuildutils.YarnDependency) { for _, dep := range dependencies { if dep != nil && strings.Contains(dep.Value, "@workspace:") { @@ -1417,7 +1416,7 @@ func stripWorkspaceUseLocalSuffix(dependencies map[string]*bibuildutils.YarnDepe } } -func parseYarnDependenciesMap(dependencies map[string]*bibuildutils.YarnDependency, rootXrayId string) (*xrayUtils.GraphNode, []string, error) { +func parseYarnDependenciesMap(dependencies map[string]*bibuildutils.YarnDependency, rootXrayId string, isCurationCmd bool) (*xrayUtils.GraphNode, []string, error) { treeMap := make(map[string]xray.DepTreeNode) workspaceMemberIds := make(map[string]bool) for _, dependency := range dependencies { @@ -1425,9 +1424,9 @@ func parseYarnDependenciesMap(dependencies map[string]*bibuildutils.YarnDependen if err != nil { return nil, nil, err } - // Workspace members are local packages, not registry artifacts (the root - // is exempt). Track them so they're skipped by the curation HEAD-check. - if strings.Contains(dependency.Value, "@workspace:") && xrayDepId != rootXrayId { + // Workspace members are local, not registry artifacts; skip them in the + // curation HEAD-check flat list (root exempt). Curation-only. + if isCurationCmd && strings.Contains(dependency.Value, "@workspace:") && xrayDepId != rootXrayId { workspaceMemberIds[xrayDepId] = true } var subDeps []string @@ -1444,9 +1443,11 @@ func parseYarnDependenciesMap(dependencies map[string]*bibuildutils.YarnDependen } } graph, uniqDeps := xray.BuildXrayDependencyTree(treeMap, rootXrayId) - // Drop workspace members from the flat list curation HEAD-checks: a local - // package coincidentally matching a public one would be a false positive. - // They stay in the graph so their dependencies remain attributed to them. + if !isCurationCmd { + return graph, slices.Collect(maps.Keys(uniqDeps)), nil + } + // Workspace members stay in the graph (deps attribute to them) but are + // dropped from the flat list to avoid false positives on public packages. uniqueDepsList := make([]string, 0, len(uniqDeps)) for id := range uniqDeps { if workspaceMemberIds[id] { @@ -1478,6 +1479,20 @@ func findYarnWorkspaceRoot(dependenciesMap map[string]*bibuildutils.YarnDependen return nil } +// resolveYarnRoot picks the dependency-tree root for 'jf ca' (curation-only; +// see the IsCurationCmd guard in BuildDependencyTree). Trusts heuristicRoot +// unless it's nil or packageName is empty, then falls back to the +// "@workspace:." locator. +func resolveYarnRoot(dependenciesMap map[string]*bibuildutils.YarnDependency, heuristicRoot *bibuildutils.YarnDependency, packageName string) *bibuildutils.YarnDependency { + if heuristicRoot != nil && packageName != "" { + return heuristicRoot + } + if workspaceRoot := findYarnWorkspaceRoot(dependenciesMap); workspaceRoot != nil { + return workspaceRoot + } + return heuristicRoot +} + // findClaimingYarnWorkspaceRoot walks upward from targetDir to find the nearest // ancestor that is a Yarn workspace root whose "workspaces" field expands to // targetDir. Used by 'jf ca --working-dirs=' so the audit runs from the diff --git a/sca/bom/buildinfo/technologies/yarn/yarn_test.go b/sca/bom/buildinfo/technologies/yarn/yarn_test.go index 18d0cae03..71f409e0f 100644 --- a/sca/bom/buildinfo/technologies/yarn/yarn_test.go +++ b/sca/bom/buildinfo/technologies/yarn/yarn_test.go @@ -32,6 +32,7 @@ func TestParseYarnDependenciesMap(t *testing.T) { name string yarnDependencies map[string]*bibuildutils.YarnDependency rootXrayId string + isCurationCmd bool expectedTree *xrayUtils.GraphNode expectedUniqueDeps []string errorExpected bool @@ -71,13 +72,15 @@ func TestParseYarnDependenciesMap(t *testing.T) { // stay in the graph (so their deps attribute to them) but be dropped from // the flat uniqueDeps list curation HEAD-checks, otherwise a coincidental // public package of the same name/version is reported as a false positive. - name: "Workspace member excluded from uniqueDeps but kept in tree", + // Curation-only; see the next case for the non-curation behavior. + name: "Workspace member excluded from uniqueDeps but kept in tree (curation)", yarnDependencies: map[string]*bibuildutils.YarnDependency{ "root@workspace:.": {Value: "root@workspace:.", Details: bibuildutils.YarnDepDetails{Version: "1.0.0", Dependencies: []bibuildutils.YarnDependencyPointer{{Locator: "ui@workspace:packages/ui"}}}}, "ui@workspace:packages/ui": {Value: "ui@workspace:packages/ui", Details: bibuildutils.YarnDepDetails{Version: "0.0.0", Dependencies: []bibuildutils.YarnDependencyPointer{{Locator: "express@npm:3.0.1"}}}}, "express@npm:3.0.1": {Value: "express@npm:3.0.1", Details: bibuildutils.YarnDepDetails{Version: "3.0.1"}}, }, - rootXrayId: npmId + "root:1.0.0", + rootXrayId: npmId + "root:1.0.0", + isCurationCmd: true, expectedTree: &xrayUtils.GraphNode{ Id: npmId + "root:1.0.0", Nodes: []*xrayUtils.GraphNode{ @@ -92,6 +95,29 @@ func TestParseYarnDependenciesMap(t *testing.T) { expectedUniqueDeps: []string{npmId + "root:1.0.0", npmId + "express:3.0.1"}, errorExpected: false, }, + { + // Same shape, non-curation: workspace member must stay in uniqueDeps. + name: "Workspace member kept in uniqueDeps for jf audit/jf scan (non-curation)", + yarnDependencies: map[string]*bibuildutils.YarnDependency{ + "root@workspace:.": {Value: "root@workspace:.", Details: bibuildutils.YarnDepDetails{Version: "1.0.0", Dependencies: []bibuildutils.YarnDependencyPointer{{Locator: "ui@workspace:packages/ui"}}}}, + "ui@workspace:packages/ui": {Value: "ui@workspace:packages/ui", Details: bibuildutils.YarnDepDetails{Version: "0.0.0", Dependencies: []bibuildutils.YarnDependencyPointer{{Locator: "express@npm:3.0.1"}}}}, + "express@npm:3.0.1": {Value: "express@npm:3.0.1", Details: bibuildutils.YarnDepDetails{Version: "3.0.1"}}, + }, + rootXrayId: npmId + "root:1.0.0", + isCurationCmd: false, + expectedTree: &xrayUtils.GraphNode{ + Id: npmId + "root:1.0.0", + Nodes: []*xrayUtils.GraphNode{ + {Id: npmId + "ui:0.0.0", + Nodes: []*xrayUtils.GraphNode{ + {Id: npmId + "express:3.0.1", + Nodes: []*xrayUtils.GraphNode{}}, + }}, + }, + }, + expectedUniqueDeps: []string{npmId + "root:1.0.0", npmId + "ui:0.0.0", npmId + "express:3.0.1"}, + errorExpected: false, + }, { name: "Incorrect formatted dependency name - error expected", yarnDependencies: map[string]*bibuildutils.YarnDependency{ @@ -104,7 +130,7 @@ func TestParseYarnDependenciesMap(t *testing.T) { for _, testcase := range testCases { t.Run(testcase.name, func(t *testing.T) { - xrayDependenciesTree, uniqueDeps, err := parseYarnDependenciesMap(testcase.yarnDependencies, testcase.rootXrayId) + xrayDependenciesTree, uniqueDeps, err := parseYarnDependenciesMap(testcase.yarnDependencies, testcase.rootXrayId, testcase.isCurationCmd) if !testcase.errorExpected { assert.NoError(t, err) assert.ElementsMatch(t, uniqueDeps, testcase.expectedUniqueDeps, "First is actual, Second is Expected") @@ -988,44 +1014,47 @@ func TestEnumerateAfterCurationInstallErrorMessage(t *testing.T) { } } -// TestBuildDependencyTreeWorkspaceRerouteIsCurationOnly is the regression -// guard for the scope contract: the workspace-member re-routing in -// BuildDependencyTree must fire only when params.IsCurationCmd is true. -// Generic 'jf audit' / 'jf scan' invocations must keep operating on the -// original currentDir so this change cannot regress them. We can't drive -// BuildDependencyTree end-to-end here (it shells out to yarn), but we can -// pin the gate by directly reading the gated condition in the source: -// the test fails compile-time if the IsCurationCmd guard is removed, and -// fails at runtime if findClaimingYarnWorkspaceRoot accidentally side- -// effects the rest of the audit. The helper itself is tested below; this -// test asserts the call-site gate is in place. -// TestRootResolutionAppliesToAuditAndScan pins two contracts: +// TestRootResolutionAppliesToAuditAndScan pins three contracts: +// +// 1. resolveYarnRoot runs only when params.IsCurationCmd is true (regression +// guard for #759, which ran it unconditionally and broke 'jf audit'/ +// 'jf scan' on Yarn workspace members). // -// 1. findYarnWorkspaceRoot is NOT gated behind IsCurationCmd in BuildDependencyTree — -// it improves root identification for every caller (jf audit, jf scan, jf ca). +// 2. @workspace:. is authoritative over build-info-go's heuristic, for +// 'jf ca' only. // -// 2. For Yarn V2+ projects the @workspace:. label is authoritative and takes -// precedence over build-info-go's name-based heuristic. This matters for nameless -// projects (no "name" in package.json) where the heuristic can fail. +// 3. stripWorkspaceUseLocalSuffix, unlike resolveYarnRoot, is NOT gated on +// IsCurationCmd — applies to all callers. func TestRootResolutionAppliesToAuditAndScan(t *testing.T) { - t.Run("source contract: findYarnWorkspaceRoot is not gated on IsCurationCmd", func(t *testing.T) { + t.Run("source contract: resolveYarnRoot is called only when IsCurationCmd is true", func(t *testing.T) { src, err := os.ReadFile("yarn.go") require.NoError(t, err) txt := string(src) - overrideIdx := strings.Index(txt, "if workspaceRoot := findYarnWorkspaceRoot(") - require.NotEqual(t, -1, overrideIdx, - "findYarnWorkspaceRoot override must be present in BuildDependencyTree") - // Look at the 200 characters immediately before the call site. - // If IsCurationCmd appears there, the override is gated — which would - // mean it no longer applies to jf audit/scan and this test should fail. - windowStart := overrideIdx - 200 + callIdx := strings.Index(txt, "root = resolveYarnRoot(") + require.NotEqual(t, -1, callIdx, + "resolveYarnRoot call must be present in BuildDependencyTree") + windowStart := callIdx - 200 if windowStart < 0 { windowStart = 0 } - context := txt[windowStart:overrideIdx] - assert.NotContains(t, context, "IsCurationCmd", - "findYarnWorkspaceRoot must not be gated on IsCurationCmd — "+ - "root resolution applies to jf audit/scan as well as jf ca") + context := txt[windowStart:callIdx] + assert.Contains(t, context, "if params.IsCurationCmd {", + "resolveYarnRoot must only run for 'jf ca', not jf audit/jf scan") + }) + + t.Run("source contract: stripWorkspaceUseLocalSuffix is called unconditionally", func(t *testing.T) { + // Must sit at top level (one leading tab), not nested under IsCurationCmd. + src, err := os.ReadFile("yarn.go") + require.NoError(t, err) + txt := string(src) + callStr := "stripWorkspaceUseLocalSuffix(dependenciesMap)" + callIdx := strings.Index(txt, callStr) + require.NotEqual(t, -1, callIdx, + "stripWorkspaceUseLocalSuffix call must be present in BuildDependencyTree") + lineStart := strings.LastIndex(txt[:callIdx], "\n") + 1 + line := txt[lineStart : callIdx+len(callStr)] + assert.Equal(t, "\t"+callStr, line, + "stripWorkspaceUseLocalSuffix must run unconditionally, not nested inside an if block") }) t.Run("Yarn V2 named project: @workspace:. root is found", func(t *testing.T) { @@ -1070,6 +1099,55 @@ func TestRootResolutionAppliesToAuditAndScan(t *testing.T) { }) } +// TestResolveYarnRootDoesNotOverrideWorkspaceMemberRoot unit-tests the +// resolveYarnRoot helper (curation-only; see TestRootResolutionAppliesToAuditAndScan +// for the IsCurationCmd gate at its call site). +func TestResolveYarnRootDoesNotOverrideWorkspaceMemberRoot(t *testing.T) { + projectRoot := &bibuildutils.YarnDependency{Value: "jf-audit-yarn-ws-repro@workspace:.", Details: bibuildutils.YarnDepDetails{Version: "0.0.0"}} + memberRoot := &bibuildutils.YarnDependency{ + Value: "@repro/app@workspace:app", + Details: bibuildutils.YarnDepDetails{Version: "1.0.0", Dependencies: []bibuildutils.YarnDependencyPointer{{Locator: "lodash@npm:4.17.21"}}}, + } + lodash := &bibuildutils.YarnDependency{Value: "lodash@npm:4.17.21", Details: bibuildutils.YarnDepDetails{Version: "4.17.21"}} + deps := map[string]*bibuildutils.YarnDependency{ + "jf-audit-yarn-ws-repro@workspace:.": projectRoot, + "@repro/app@workspace:app": memberRoot, + "lodash@npm:4.17.21": lodash, + } + + t.Run("named workspace member: build-info-go's own root is kept, not clobbered by the outer project root", func(t *testing.T) { + got := resolveYarnRoot(deps, memberRoot, "@repro/app") + assert.Same(t, memberRoot, got) + }) + + t.Run("named project root: build-info-go's own root is kept", func(t *testing.T) { + got := resolveYarnRoot(deps, projectRoot, "jf-audit-yarn-ws-repro") + assert.Same(t, projectRoot, got) + }) + + t.Run("nameless package.json: falls back to the @workspace:. root", func(t *testing.T) { + got := resolveYarnRoot(deps, nil, "") + require.NotNil(t, got) + assert.Equal(t, "jf-audit-yarn-ws-repro@workspace:.", got.Value) + }) + + t.Run("nil heuristic root with a name: still falls back to the @workspace:. root", func(t *testing.T) { + got := resolveYarnRoot(deps, nil, "@repro/app") + require.NotNil(t, got) + assert.Equal(t, "jf-audit-yarn-ws-repro@workspace:.", got.Value) + }) + + t.Run("Yarn V1 (no @workspace:. entries): heuristic result passes through untouched, even nil", func(t *testing.T) { + v1Deps := map[string]*bibuildutils.YarnDependency{ + "lodash": {Value: "lodash", Details: bibuildutils.YarnDepDetails{Version: "4.17.23"}}, + } + assert.Nil(t, resolveYarnRoot(v1Deps, nil, "")) + }) +} + +// TestBuildDependencyTreeWorkspaceRerouteIsCurationOnly guards that the +// workspace-member walk-up in BuildDependencyTree fires only for 'jf ca'; +// 'jf audit'/'jf scan' must keep operating on the original currentDir. func TestBuildDependencyTreeWorkspaceRerouteIsCurationOnly(t *testing.T) { // Synthesise a workspace structure that *would* be claimed by the // walk-up helper, so any future caller that forgets to gate on