Skip to content

XRAY-147280 - Fixed workplace issue for jf audit #801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gauriy-tech wants to merge 1 commit into
base: main
Choose a base branch
from
Open

XRAY-147280 - Fixed workplace issue for jf audit #801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 30 additions & 19 deletions sca/bom/buildinfo/technologies/yarn/yarn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,15 @@ func BuildDependencyTree(params technologies.BuildInfoBomGeneratorParams) (depen
}
return
}
// Yarn V2+ always emits the project root as "<name>@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 {
root = resolveYarnRoot(dependenciesMap, root, packageInfo.Name)
}
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
Expand Down Expand Up @@ -212,7 +209,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
}
Expand Down Expand Up @@ -1405,10 +1402,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:") {
Expand All @@ -1417,17 +1412,17 @@ 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 {
xrayDepId, err := getXrayDependencyId(dependency)
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
Expand All @@ -1444,9 +1439,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] {
Expand Down Expand Up @@ -1478,6 +1475,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
// "<name>@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=<X>' so the audit runs from the
Expand Down
142 changes: 110 additions & 32 deletions sca/bom/buildinfo/technologies/yarn/yarn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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")
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
Loading